diff mbox series

[54/67] ui/vc: console-vc requires PIXMAN

Message ID 20230830093843.3531473-55-marcandre.lureau@redhat.com
State New
Headers show
Series Make pixman an optional dependency | expand

Commit Message

Marc-André Lureau Aug. 30, 2023, 9:38 a.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Add stubs for the fallback paths.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console-vc-stubs.c | 59 +++++++++++++++++++++++++++++++++++++++++++
 ui/meson.build        |  2 +-
 2 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 ui/console-vc-stubs.c

Comments

Daniel P. Berrangé Sept. 1, 2023, 5:28 p.m. UTC | #1 
On Wed, Aug 30, 2023 at 01:38:28PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Add stubs for the fallback paths.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/console-vc-stubs.c | 59 +++++++++++++++++++++++++++++++++++++++++++
>  ui/meson.build        |  2 +-
>  2 files changed, 60 insertions(+), 1 deletion(-)
>  create mode 100644 ui/console-vc-stubs.c
> 
> diff --git a/ui/console-vc-stubs.c b/ui/console-vc-stubs.c
> new file mode 100644
> index 0000000000..76ea880d27
> --- /dev/null
> +++ b/ui/console-vc-stubs.c
> @@ -0,0 +1,59 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * QEMU VC stubs
> + */
> +#include "qemu/osdep.h"
> +
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> +#include "qemu/option.h"
> +#include "chardev/char.h"
> +#include "ui/console-priv.h"
> +
> +void qemu_text_console_select(QemuTextConsole *c)
> +{
> +}
> +
> +const char * qemu_text_console_get_label(QemuTextConsole *c)
> +{
> +    return NULL;
> +}
> +
> +void qemu_text_console_update_cursor(void)
> +{
> +}
> +
> +void qemu_text_console_handle_keysym(QemuTextConsole *s, int keysym)
> +{
> +}
> +
> +#define TYPE_CHARDEV_VC "chardev-vc"
> +
> +static void vc_chr_parse(QemuOpts *opts, ChardevBackend *backend, Error **errp)
> +{
> +    const char *id = qemu_opts_id(opts);
> +
> +    warn_report("%s: this is a dummy VC driver. "
> +                "Use '-nographic' or a different chardev.", id);
> +}

Why should this be an error_setg() call given we have a errp
parameter, so make this unsupportable config into an error ?
Ignoring invalid user configs is not desirable in general.

> +
> +static void char_vc_class_init(ObjectClass *oc, void *data)
> +{
> +    ChardevClass *cc = CHARDEV_CLASS(oc);
> +
> +    cc->parse = vc_chr_parse;
> +}
> +
> +static const TypeInfo char_vc_type_info = {
> +    .name = TYPE_CHARDEV_VC,
> +    .parent = TYPE_CHARDEV,
> +    .class_init = char_vc_class_init,
> +};
> +
> +void qemu_console_early_init(void)
> +{
> +    /* set the default vc driver */
> +    if (!object_class_by_name(TYPE_CHARDEV_VC)) {
> +        type_register(&char_vc_type_info);
> +    }
> +}
> diff --git a/ui/meson.build b/ui/meson.build
> index 0a1e8272a3..3085e10a72 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -6,7 +6,6 @@ system_ss.add(png)
>  system_ss.add(files(
>    'clipboard.c',
>    'console.c',
> -  'console-vc.c',
>    'cursor.c',
>    'input-keymap.c',
>    'input-legacy.c',
> @@ -19,6 +18,7 @@ system_ss.add(files(
>    'ui-qmp-cmds.c',
>    'util.c',
>  ))
> +system_ss.add(when: pixman, if_true: files('console-vc.c'), if_false: files('console-vc-stubs.c'))
>  if dbus_display
>    system_ss.add(files('dbus-module.c'))
>  endif
> -- 
> 2.41.0
> 
> 

With regards,
Daniel
Marc-André Lureau Sept. 4, 2023, 1:41 p.m. UTC | #2 
Hi

On Fri, Sep 1, 2023 at 9:28 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Aug 30, 2023 at 01:38:28PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Add stubs for the fallback paths.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  ui/console-vc-stubs.c | 59 +++++++++++++++++++++++++++++++++++++++++++
> >  ui/meson.build        |  2 +-
> >  2 files changed, 60 insertions(+), 1 deletion(-)
> >  create mode 100644 ui/console-vc-stubs.c
> >
> > diff --git a/ui/console-vc-stubs.c b/ui/console-vc-stubs.c
> > new file mode 100644
> > index 0000000000..76ea880d27
> > --- /dev/null
> > +++ b/ui/console-vc-stubs.c
> > @@ -0,0 +1,59 @@
> > +/*
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + * QEMU VC stubs
> > + */
> > +#include "qemu/osdep.h"
> > +
> > +#include "qapi/error.h"
> > +#include "qemu/error-report.h"
> > +#include "qemu/option.h"
> > +#include "chardev/char.h"
> > +#include "ui/console-priv.h"
> > +
> > +void qemu_text_console_select(QemuTextConsole *c)
> > +{
> > +}
> > +
> > +const char * qemu_text_console_get_label(QemuTextConsole *c)
> > +{
> > +    return NULL;
> > +}
> > +
> > +void qemu_text_console_update_cursor(void)
> > +{
> > +}
> > +
> > +void qemu_text_console_handle_keysym(QemuTextConsole *s, int keysym)
> > +{
> > +}
> > +
> > +#define TYPE_CHARDEV_VC "chardev-vc"
> > +
> > +static void vc_chr_parse(QemuOpts *opts, ChardevBackend *backend, Error **errp)
> > +{
> > +    const char *id = qemu_opts_id(opts);
> > +
> > +    warn_report("%s: this is a dummy VC driver. "
> > +                "Use '-nographic' or a different chardev.", id);
> > +}
>
> Why should this be an error_setg() call given we have a errp
> parameter, so make this unsupportable config into an error ?
> Ignoring invalid user configs is not desirable in general.

This was to keep starting as default, regardless of pixman support.

Since, by default, QEMU will create the following VCs (vl.c):

            add_device_config(DEV_SERIAL, "vc:80Cx24C");
            add_device_config(DEV_PARALLEL, "vc:80Cx24C");
            monitor_parse("vc:80Cx24C", "readline", false);

With SDL or VNC, this will associate the QemuTextConsole(s), which is
disabled without pixman. Maybe we shouldn't create those VC by default
when the backend doesn't support it (but to null instead).

Yes, if the user supplied -chardev vc manually, we should error out
instead. I'll try to update the patch.



(btw, I agree with you it would have been nice to make VC an abstract
base chardev, and only allow final types)

>
> > +
> > +static void char_vc_class_init(ObjectClass *oc, void *data)
> > +{
> > +    ChardevClass *cc = CHARDEV_CLASS(oc);
> > +
> > +    cc->parse = vc_chr_parse;
> > +}
> > +
> > +static const TypeInfo char_vc_type_info = {
> > +    .name = TYPE_CHARDEV_VC,
> > +    .parent = TYPE_CHARDEV,
> > +    .class_init = char_vc_class_init,
> > +};
> > +
> > +void qemu_console_early_init(void)
> > +{
> > +    /* set the default vc driver */
> > +    if (!object_class_by_name(TYPE_CHARDEV_VC)) {
> > +        type_register(&char_vc_type_info);
> > +    }
> > +}
> > diff --git a/ui/meson.build b/ui/meson.build
> > index 0a1e8272a3..3085e10a72 100644
> > --- a/ui/meson.build
> > +++ b/ui/meson.build
> > @@ -6,7 +6,6 @@ system_ss.add(png)
> >  system_ss.add(files(
> >    'clipboard.c',
> >    'console.c',
> > -  'console-vc.c',
> >    'cursor.c',
> >    'input-keymap.c',
> >    'input-legacy.c',
> > @@ -19,6 +18,7 @@ system_ss.add(files(
> >    'ui-qmp-cmds.c',
> >    'util.c',
> >  ))
> > +system_ss.add(when: pixman, if_true: files('console-vc.c'), if_false: files('console-vc-stubs.c'))
> >  if dbus_display
> >    system_ss.add(files('dbus-module.c'))
> >  endif
> > --
> > 2.41.0
> >
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
>
Daniel P. Berrangé Sept. 4, 2023, 1:44 p.m. UTC | #3 
On Mon, Sep 04, 2023 at 05:41:23PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Sep 1, 2023 at 9:28 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, Aug 30, 2023 at 01:38:28PM +0400, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Add stubs for the fallback paths.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  ui/console-vc-stubs.c | 59 +++++++++++++++++++++++++++++++++++++++++++
> > >  ui/meson.build        |  2 +-
> > >  2 files changed, 60 insertions(+), 1 deletion(-)
> > >  create mode 100644 ui/console-vc-stubs.c
> > >
> > > diff --git a/ui/console-vc-stubs.c b/ui/console-vc-stubs.c
> > > new file mode 100644
> > > index 0000000000..76ea880d27
> > > --- /dev/null
> > > +++ b/ui/console-vc-stubs.c
> > > @@ -0,0 +1,59 @@
> > > +/*
> > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > + * QEMU VC stubs
> > > + */
> > > +#include "qemu/osdep.h"
> > > +
> > > +#include "qapi/error.h"
> > > +#include "qemu/error-report.h"
> > > +#include "qemu/option.h"
> > > +#include "chardev/char.h"
> > > +#include "ui/console-priv.h"
> > > +
> > > +void qemu_text_console_select(QemuTextConsole *c)
> > > +{
> > > +}
> > > +
> > > +const char * qemu_text_console_get_label(QemuTextConsole *c)
> > > +{
> > > +    return NULL;
> > > +}
> > > +
> > > +void qemu_text_console_update_cursor(void)
> > > +{
> > > +}
> > > +
> > > +void qemu_text_console_handle_keysym(QemuTextConsole *s, int keysym)
> > > +{
> > > +}
> > > +
> > > +#define TYPE_CHARDEV_VC "chardev-vc"
> > > +
> > > +static void vc_chr_parse(QemuOpts *opts, ChardevBackend *backend, Error **errp)
> > > +{
> > > +    const char *id = qemu_opts_id(opts);
> > > +
> > > +    warn_report("%s: this is a dummy VC driver. "
> > > +                "Use '-nographic' or a different chardev.", id);
> > > +}
> >
> > Why should this be an error_setg() call given we have a errp
> > parameter, so make this unsupportable config into an error ?
> > Ignoring invalid user configs is not desirable in general.
> 
> This was to keep starting as default, regardless of pixman support.
> 
> Since, by default, QEMU will create the following VCs (vl.c):
> 
>             add_device_config(DEV_SERIAL, "vc:80Cx24C");
>             add_device_config(DEV_PARALLEL, "vc:80Cx24C");
>             monitor_parse("vc:80Cx24C", "readline", false);
> 
> With SDL or VNC, this will associate the QemuTextConsole(s), which is
> disabled without pixman. Maybe we shouldn't create those VC by default
> when the backend doesn't support it (but to null instead).

Yeah, probably best to just use 'null' of 'vc' for that scenario,
since the stub is effectively 'null' anyway.


With regards,
Daniel
diff mbox series

Patch

diff --git a/ui/console-vc-stubs.c b/ui/console-vc-stubs.c
new file mode 100644
index 0000000000..76ea880d27
--- /dev/null
+++ b/ui/console-vc-stubs.c
@@ -0,0 +1,59 @@ 
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * QEMU VC stubs
+ */
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/option.h"
+#include "chardev/char.h"
+#include "ui/console-priv.h"
+
+void qemu_text_console_select(QemuTextConsole *c)
+{
+}
+
+const char * qemu_text_console_get_label(QemuTextConsole *c)
+{
+    return NULL;
+}
+
+void qemu_text_console_update_cursor(void)
+{
+}
+
+void qemu_text_console_handle_keysym(QemuTextConsole *s, int keysym)
+{
+}
+
+#define TYPE_CHARDEV_VC "chardev-vc"
+
+static void vc_chr_parse(QemuOpts *opts, ChardevBackend *backend, Error **errp)
+{
+    const char *id = qemu_opts_id(opts);
+
+    warn_report("%s: this is a dummy VC driver. "
+                "Use '-nographic' or a different chardev.", id);
+}
+
+static void char_vc_class_init(ObjectClass *oc, void *data)
+{
+    ChardevClass *cc = CHARDEV_CLASS(oc);
+
+    cc->parse = vc_chr_parse;
+}
+
+static const TypeInfo char_vc_type_info = {
+    .name = TYPE_CHARDEV_VC,
+    .parent = TYPE_CHARDEV,
+    .class_init = char_vc_class_init,
+};
+
+void qemu_console_early_init(void)
+{
+    /* set the default vc driver */
+    if (!object_class_by_name(TYPE_CHARDEV_VC)) {
+        type_register(&char_vc_type_info);
+    }
+}
diff --git a/ui/meson.build b/ui/meson.build
index 0a1e8272a3..3085e10a72 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -6,7 +6,6 @@  system_ss.add(png)
 system_ss.add(files(
   'clipboard.c',
   'console.c',
-  'console-vc.c',
   'cursor.c',
   'input-keymap.c',
   'input-legacy.c',
@@ -19,6 +18,7 @@  system_ss.add(files(
   'ui-qmp-cmds.c',
   'util.c',
 ))
+system_ss.add(when: pixman, if_true: files('console-vc.c'), if_false: files('console-vc-stubs.c'))
 if dbus_display
   system_ss.add(files('dbus-module.c'))
 endif