* [Qemu-devel] [PATCH 0/2] vl: Simplify vga interface configuration code
@ 2015-11-11 21:27 Eduardo Habkost
2015-11-11 21:27 ` [Qemu-devel] [PATCH 1/2] vl: Table-based select_vgahw() Eduardo Habkost
2015-11-11 21:27 ` [Qemu-devel] [PATCH 2/2] vl: Replace *_vga_available() functions with class_names field Eduardo Habkost
0 siblings, 2 replies; 6+ messages in thread
From: Eduardo Habkost @ 2015-11-11 21:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
Instead of duplicating class lookup and strstart() code for each
VGA interface type, use a table to implement select_vgahw().
This series needs the following patch to be applied first:
From: Eduardo Habkost <ehabkost@redhat.com>
Date: Wed, 11 Nov 2015 18:39:18 -0200
Message-Id: <1447274358-2772-1-git-send-email-ehabkost@redhat.com>
Subject: [PATCH] vl: Use exit(1) when requested VGA interface is unavailable
Eduardo Habkost (2):
vl: Table-based select_vgahw()
vl: Replace *_vga_available() functions with class_names field
include/sysemu/sysemu.h | 1 +
vl.c | 156 ++++++++++++++++++++++--------------------------
2 files changed, 72 insertions(+), 85 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] vl: Table-based select_vgahw()
2015-11-11 21:27 [Qemu-devel] [PATCH 0/2] vl: Simplify vga interface configuration code Eduardo Habkost
@ 2015-11-11 21:27 ` Eduardo Habkost
2015-11-11 22:05 ` Eric Blake
2015-11-11 21:27 ` [Qemu-devel] [PATCH 2/2] vl: Replace *_vga_available() functions with class_names field Eduardo Habkost
1 sibling, 1 reply; 6+ messages in thread
From: Eduardo Habkost @ 2015-11-11 21:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
Instead of implementing separate check functions for each vga
interface type, add a table enumerating the possible VGA
interfaces.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/sysemu/sysemu.h | 1 +
vl.c | 114 ++++++++++++++++++++++++++----------------------
2 files changed, 63 insertions(+), 52 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index f992494..6406906 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -147,6 +147,7 @@ extern int autostart;
typedef enum {
VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL,
VGA_TCX, VGA_CG3, VGA_DEVICE, VGA_VIRTIO,
+ VGA_TYPE_MAX,
} VGAInterfaceType;
extern int vga_interface_type;
diff --git a/vl.c b/vl.c
index 7b81998..a5dd9a6 100644
--- a/vl.c
+++ b/vl.c
@@ -2001,63 +2001,73 @@ static bool virtio_vga_available(void)
return object_class_by_name("virtio-vga");
}
+typedef struct VGAInterfaceInfo {
+ const char *opt_name; /* option name */
+ const char *name; /* human-readable name */
+ bool (*available)(void);
+} VGAInterfaceInfo;
+
+static VGAInterfaceInfo vga_interfaces[VGA_TYPE_MAX] = {
+ [VGA_NONE] = {
+ .opt_name = "none",
+ },
+ [VGA_STD] = {
+ .opt_name = "std",
+ .name = "standard VGA",
+ .available = vga_available,
+ },
+ [VGA_CIRRUS] = {
+ .opt_name = "cirrus",
+ .name = "Cirrus VGA",
+ .available = cirrus_vga_available,
+ },
+ [VGA_VMWARE] = {
+ .opt_name = "vmware",
+ .name = "VMWare SVGA",
+ .available = vmware_vga_available,
+ },
+ [VGA_VIRTIO] = {
+ .opt_name = "virtio",
+ .name = "Virtio VGA",
+ .available = virtio_vga_available,
+ },
+ [VGA_QXL] = {
+ .opt_name = "qxl",
+ .name = "QXL VGA",
+ .available = qxl_vga_available,
+ },
+ [VGA_TCX] = {
+ .opt_name = "tcx",
+ .name = "TCX framebuffer",
+ .available = tcx_vga_available,
+ },
+ [VGA_CG3] = {
+ .opt_name = "cg3",
+ .name = "CG3 framebuffer",
+ .available = cg3_vga_available,
+ },
+ [VGA_XENFB] = {
+ .opt_name = "xenfb",
+ },
+};
+
static void select_vgahw (const char *p)
{
const char *opts;
+ int t;
- assert(vga_interface_type == VGA_NONE);
- if (strstart(p, "std", &opts)) {
- if (vga_available()) {
- vga_interface_type = VGA_STD;
- } else {
- error_report("standard VGA not available");
- exit(1);
- }
- } else if (strstart(p, "cirrus", &opts)) {
- if (cirrus_vga_available()) {
- vga_interface_type = VGA_CIRRUS;
- } else {
- error_report("Cirrus VGA not available");
- exit(1);
- }
- } else if (strstart(p, "vmware", &opts)) {
- if (vmware_vga_available()) {
- vga_interface_type = VGA_VMWARE;
- } else {
- error_report("VMWare SVGA not available");
- exit(1);
- }
- } else if (strstart(p, "virtio", &opts)) {
- if (virtio_vga_available()) {
- vga_interface_type = VGA_VIRTIO;
- } else {
- error_report("Virtio VGA not available");
- exit(1);
- }
- } else if (strstart(p, "xenfb", &opts)) {
- vga_interface_type = VGA_XENFB;
- } else if (strstart(p, "qxl", &opts)) {
- if (qxl_vga_available()) {
- vga_interface_type = VGA_QXL;
- } else {
- error_report("QXL VGA not available");
- exit(1);
- }
- } else if (strstart(p, "tcx", &opts)) {
- if (tcx_vga_available()) {
- vga_interface_type = VGA_TCX;
- } else {
- error_report("TCX framebuffer not available");
- exit(1);
- }
- } else if (strstart(p, "cg3", &opts)) {
- if (cg3_vga_available()) {
- vga_interface_type = VGA_CG3;
- } else {
- error_report("CG3 framebuffer not available");
- exit(1);
+ for (t = 0; t < VGA_TYPE_MAX; t++) {
+ VGAInterfaceInfo *ti = &vga_interfaces[t];
+ if (ti->opt_name && strstart(p, ti->opt_name, &opts)) {
+ if (ti->available && !ti->available()) {
+ error_report("%s not available", ti->name);
+ exit(1);
+ }
+ vga_interface_type = t;
+ break;
}
- } else if (!strstart(p, "none", &opts)) {
+ }
+ if (t == VGA_TYPE_MAX) {
invalid_vga:
error_report("unknown vga type: %s", p);
exit(1);
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] vl: Replace *_vga_available() functions with class_names field
2015-11-11 21:27 [Qemu-devel] [PATCH 0/2] vl: Simplify vga interface configuration code Eduardo Habkost
2015-11-11 21:27 ` [Qemu-devel] [PATCH 1/2] vl: Table-based select_vgahw() Eduardo Habkost
@ 2015-11-11 21:27 ` Eduardo Habkost
2015-11-11 22:09 ` Eric Blake
1 sibling, 1 reply; 6+ messages in thread
From: Eduardo Habkost @ 2015-11-11 21:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
Instead of requiring a separate function for each VGA interface,
just enumerate the corresponding class names on struct
VGAInterfaceInfo.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
vl.c | 70 +++++++++++++++++++++++---------------------------------------------
1 file changed, 23 insertions(+), 47 deletions(-)
diff --git a/vl.c b/vl.c
index a5dd9a6..c899adf 100644
--- a/vl.c
+++ b/vl.c
@@ -1965,46 +1965,12 @@ static const QEMUOption qemu_options[] = {
{ NULL },
};
-static bool vga_available(void)
-{
- return object_class_by_name("VGA") || object_class_by_name("isa-vga");
-}
-
-static bool cirrus_vga_available(void)
-{
- return object_class_by_name("cirrus-vga")
- || object_class_by_name("isa-cirrus-vga");
-}
-
-static bool vmware_vga_available(void)
-{
- return object_class_by_name("vmware-svga");
-}
-
-static bool qxl_vga_available(void)
-{
- return object_class_by_name("qxl-vga");
-}
-
-static bool tcx_vga_available(void)
-{
- return object_class_by_name("SUNW,tcx");
-}
-
-static bool cg3_vga_available(void)
-{
- return object_class_by_name("cgthree");
-}
-
-static bool virtio_vga_available(void)
-{
- return object_class_by_name("virtio-vga");
-}
-
typedef struct VGAInterfaceInfo {
const char *opt_name; /* option name */
const char *name; /* human-readable name */
- bool (*available)(void);
+ /* Class names indicating that support is available.
+ * If no class is specified, the interface is always available */
+ const char *class_names[2];
} VGAInterfaceInfo;
static VGAInterfaceInfo vga_interfaces[VGA_TYPE_MAX] = {
@@ -2014,43 +1980,53 @@ static VGAInterfaceInfo vga_interfaces[VGA_TYPE_MAX] = {
[VGA_STD] = {
.opt_name = "std",
.name = "standard VGA",
- .available = vga_available,
+ .class_names = { "VGA", "isa-vga" },
},
[VGA_CIRRUS] = {
.opt_name = "cirrus",
.name = "Cirrus VGA",
- .available = cirrus_vga_available,
+ .class_names = { "cirrus-vga", "isa-cirrus-vga" },
},
[VGA_VMWARE] = {
.opt_name = "vmware",
.name = "VMWare SVGA",
- .available = vmware_vga_available,
+ .class_names = { "vmware-svga" },
},
[VGA_VIRTIO] = {
.opt_name = "virtio",
.name = "Virtio VGA",
- .available = virtio_vga_available,
+ .class_names = { "virtio-vga" },
},
[VGA_QXL] = {
.opt_name = "qxl",
.name = "QXL VGA",
- .available = qxl_vga_available,
+ .class_names = { "qxl-vga" },
},
[VGA_TCX] = {
.opt_name = "tcx",
.name = "TCX framebuffer",
- .available = tcx_vga_available,
+ .class_names = { "SUNW,tcx" },
},
[VGA_CG3] = {
.opt_name = "cg3",
.name = "CG3 framebuffer",
- .available = cg3_vga_available,
+ .class_names = { "cgthree" },
},
[VGA_XENFB] = {
.opt_name = "xenfb",
},
};
+static bool vga_interface_available(VGAInterfaceType t)
+{
+ VGAInterfaceInfo *ti = &vga_interfaces[t];
+
+ assert(t < VGA_TYPE_MAX);
+ return !ti->class_names[0] ||
+ object_class_by_name(ti->class_names[0]) ||
+ object_class_by_name(ti->class_names[1]);
+}
+
static void select_vgahw (const char *p)
{
const char *opts;
@@ -2059,7 +2035,7 @@ static void select_vgahw (const char *p)
for (t = 0; t < VGA_TYPE_MAX; t++) {
VGAInterfaceInfo *ti = &vga_interfaces[t];
if (ti->opt_name && strstart(p, ti->opt_name, &opts)) {
- if (ti->available && !ti->available()) {
+ if (!vga_interface_available(t)) {
error_report("%s not available", ti->name);
exit(1);
}
@@ -4503,9 +4479,9 @@ int main(int argc, char **argv, char **envp)
if (default_vga) {
if (machine_class->default_display) {
vga_model = machine_class->default_display;
- } else if (cirrus_vga_available()) {
+ } else if (vga_interface_available(VGA_CIRRUS)) {
vga_model = "cirrus";
- } else if (vga_available()) {
+ } else if (vga_interface_available(VGA_STD)) {
vga_model = "std";
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vl: Table-based select_vgahw()
2015-11-11 21:27 ` [Qemu-devel] [PATCH 1/2] vl: Table-based select_vgahw() Eduardo Habkost
@ 2015-11-11 22:05 ` Eric Blake
2015-11-12 15:08 ` Eduardo Habkost
0 siblings, 1 reply; 6+ messages in thread
From: Eric Blake @ 2015-11-11 22:05 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 2099 bytes --]
On 11/11/2015 02:27 PM, Eduardo Habkost wrote:
> Instead of implementing separate check functions for each vga
> interface type, add a table enumerating the possible VGA
> interfaces.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/sysemu/sysemu.h | 1 +
> vl.c | 114 ++++++++++++++++++++++++++----------------------
> 2 files changed, 63 insertions(+), 52 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index f992494..6406906 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -147,6 +147,7 @@ extern int autostart;
> typedef enum {
> VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL,
> VGA_TCX, VGA_CG3, VGA_DEVICE, VGA_VIRTIO,
> + VGA_TYPE_MAX,
> } VGAInterfaceType;
Would it be worth exposing this enum in qapi someday? But doesn't affect
the correctness of this patch.
> static void select_vgahw (const char *p)
Worth dropping the space before '(' while in the neighborhood?
> {
> const char *opts;
> + int t;
>
> - assert(vga_interface_type == VGA_NONE);
Are you intentionally dropping the assert? It protects us from
select_vgahw() being called more than once.
> - } else if (strstart(p, "cg3", &opts)) {
> - if (cg3_vga_available()) {
> - vga_interface_type = VGA_CG3;
> - } else {
> - error_report("CG3 framebuffer not available");
> - exit(1);
> + for (t = 0; t < VGA_TYPE_MAX; t++) {
> + VGAInterfaceInfo *ti = &vga_interfaces[t];
> + if (ti->opt_name && strstart(p, ti->opt_name, &opts)) {
> + if (ti->available && !ti->available()) {
> + error_report("%s not available", ti->name);
> + exit(1);
> + }
> + vga_interface_type = t;
> + break;
Nice compression in code size.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] vl: Replace *_vga_available() functions with class_names field
2015-11-11 21:27 ` [Qemu-devel] [PATCH 2/2] vl: Replace *_vga_available() functions with class_names field Eduardo Habkost
@ 2015-11-11 22:09 ` Eric Blake
0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2015-11-11 22:09 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]
On 11/11/2015 02:27 PM, Eduardo Habkost wrote:
> Instead of requiring a separate function for each VGA interface,
> just enumerate the corresponding class names on struct
> VGAInterfaceInfo.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> vl.c | 70 +++++++++++++++++++++++---------------------------------------------
> 1 file changed, 23 insertions(+), 47 deletions(-)
>
> [VGA_CG3] = {
> .opt_name = "cg3",
> .name = "CG3 framebuffer",
> - .available = cg3_vga_available,
> + .class_names = { "cgthree" },
> },
> [VGA_XENFB] = {
> .opt_name = "xenfb",
> },
> };
>
> +static bool vga_interface_available(VGAInterfaceType t)
> +{
> + VGAInterfaceInfo *ti = &vga_interfaces[t];
> +
> + assert(t < VGA_TYPE_MAX);
> + return !ti->class_names[0] ||
> + object_class_by_name(ti->class_names[0]) ||
> + object_class_by_name(ti->class_names[1]);
Does the correct thing when ti->class_names[1] is NULL (but I had to look).
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vl: Table-based select_vgahw()
2015-11-11 22:05 ` Eric Blake
@ 2015-11-12 15:08 ` Eduardo Habkost
0 siblings, 0 replies; 6+ messages in thread
From: Eduardo Habkost @ 2015-11-12 15:08 UTC (permalink / raw)
To: Eric Blake; +Cc: Paolo Bonzini, qemu-devel
On Wed, Nov 11, 2015 at 03:05:07PM -0700, Eric Blake wrote:
> On 11/11/2015 02:27 PM, Eduardo Habkost wrote:
> > Instead of implementing separate check functions for each vga
> > interface type, add a table enumerating the possible VGA
> > interfaces.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > include/sysemu/sysemu.h | 1 +
> > vl.c | 114 ++++++++++++++++++++++++++----------------------
> > 2 files changed, 63 insertions(+), 52 deletions(-)
> >
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index f992494..6406906 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -147,6 +147,7 @@ extern int autostart;
> > typedef enum {
> > VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL,
> > VGA_TCX, VGA_CG3, VGA_DEVICE, VGA_VIRTIO,
> > + VGA_TYPE_MAX,
> > } VGAInterfaceType;
>
> Would it be worth exposing this enum in qapi someday? But doesn't affect
> the correctness of this patch.
I don't know. Maybe not, if we have any plans to configure this
using QOM classes somehow (like the CPU model stuff), not using
enums.
>
> > static void select_vgahw (const char *p)
>
> Worth dropping the space before '(' while in the neighborhood?
>
> > {
> > const char *opts;
> > + int t;
> >
> > - assert(vga_interface_type == VGA_NONE);
>
> Are you intentionally dropping the assert? It protects us from
> select_vgahw() being called more than once.
That was not intentional. I will fix it in a new version. Thanks!
>
> > - } else if (strstart(p, "cg3", &opts)) {
> > - if (cg3_vga_available()) {
> > - vga_interface_type = VGA_CG3;
> > - } else {
> > - error_report("CG3 framebuffer not available");
> > - exit(1);
> > + for (t = 0; t < VGA_TYPE_MAX; t++) {
> > + VGAInterfaceInfo *ti = &vga_interfaces[t];
> > + if (ti->opt_name && strstart(p, ti->opt_name, &opts)) {
> > + if (ti->available && !ti->available()) {
> > + error_report("%s not available", ti->name);
> > + exit(1);
> > + }
> > + vga_interface_type = t;
> > + break;
>
> Nice compression in code size.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
--
Eduardo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-12 15:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-11 21:27 [Qemu-devel] [PATCH 0/2] vl: Simplify vga interface configuration code Eduardo Habkost
2015-11-11 21:27 ` [Qemu-devel] [PATCH 1/2] vl: Table-based select_vgahw() Eduardo Habkost
2015-11-11 22:05 ` Eric Blake
2015-11-12 15:08 ` Eduardo Habkost
2015-11-11 21:27 ` [Qemu-devel] [PATCH 2/2] vl: Replace *_vga_available() functions with class_names field Eduardo Habkost
2015-11-11 22:09 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.