qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-2.7 v2 0/3] vl: Simplify vga interface configuration code
@ 2016-04-14  0:51 Eduardo Habkost
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 1/3] vl: Use exit(1) when requested VGA interface is unavailable Eduardo Habkost
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Eduardo Habkost @ 2016-04-14  0:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Michael S. Tsirkin, Marcel Apfelbaum

Instead of duplicating class lookup and strstart() code for each
VGA interface type, use a table to implement select_vgahw().

Changes v1 -> v2:
* Keep assert(vga_interface_type == VGA_NONE) line
  Suggested-by: Eric Blake <eblake@redhat.com>
* Fix spacing on select_vgahw() definition
  Suggested-by: Eric Blake <eblake@redhat.com>
* Include "vl: Use exit(1) when requested VGA interface is
  unavailable" in the series, also

Eduardo Habkost (3):
  vl: Use exit(1) when requested VGA interface is unavailable
  vl: Table-based select_vgahw()
  vl: Replace *_vga_available() functions with class_names field

 include/sysemu/sysemu.h |   1 +
 vl.c                    | 157 ++++++++++++++++++++++--------------------------
 2 files changed, 73 insertions(+), 85 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH for-2.7 v2 1/3] vl: Use exit(1) when requested VGA interface is unavailable
  2016-04-14  0:51 [Qemu-devel] [PATCH for-2.7 v2 0/3] vl: Simplify vga interface configuration code Eduardo Habkost
@ 2016-04-14  0:51 ` Eduardo Habkost
  2016-04-14  7:23   ` Marcel Apfelbaum
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 2/3] vl: Table-based select_vgahw() Eduardo Habkost
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 3/3] vl: Replace *_vga_available() functions with class_names field Eduardo Habkost
  2 siblings, 1 reply; 7+ messages in thread
From: Eduardo Habkost @ 2016-04-14  0:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Michael S. Tsirkin, Marcel Apfelbaum

Instead of using exit(0), use exit(1) when an unavailable VGA
interface is used in the command-line to indicate it's an error.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/vl.c b/vl.c
index 9df534f..3c565bb 100644
--- a/vl.c
+++ b/vl.c
@@ -2025,28 +2025,28 @@ static void select_vgahw (const char *p)
             vga_interface_type = VGA_STD;
         } else {
             error_report("standard VGA not available");
-            exit(0);
+            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(0);
+            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(0);
+            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(0);
+            exit(1);
         }
     } else if (strstart(p, "xenfb", &opts)) {
         vga_interface_type = VGA_XENFB;
@@ -2055,21 +2055,21 @@ static void select_vgahw (const char *p)
             vga_interface_type = VGA_QXL;
         } else {
             error_report("QXL VGA not available");
-            exit(0);
+            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(0);
+            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(0);
+            exit(1);
         }
     } else if (!strstart(p, "none", &opts)) {
     invalid_vga:
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH for-2.7 v2 2/3] vl: Table-based select_vgahw()
  2016-04-14  0:51 [Qemu-devel] [PATCH for-2.7 v2 0/3] vl: Simplify vga interface configuration code Eduardo Habkost
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 1/3] vl: Use exit(1) when requested VGA interface is unavailable Eduardo Habkost
@ 2016-04-14  0:51 ` Eduardo Habkost
  2016-04-14  7:28   ` Marcel Apfelbaum
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 3/3] vl: Replace *_vga_available() functions with class_names field Eduardo Habkost
  2 siblings, 1 reply; 7+ messages in thread
From: Eduardo Habkost @ 2016-04-14  0:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Michael S. Tsirkin, Marcel Apfelbaum

Instead of implementing separate check functions for each vga
interface type, add a table enumerating the possible VGA
interfaces.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Keep assert(vga_interface_type == VGA_NONE) line
  Suggested-by: Eric Blake <eblake@redhat.com>
* Fix spacing on select_vgahw() definition
  Suggested-by: Eric Blake <eblake@redhat.com>
---
 include/sysemu/sysemu.h |   1 +
 vl.c                    | 115 ++++++++++++++++++++++++++----------------------
 2 files changed, 64 insertions(+), 52 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 38fb3ca..2033f8c 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -146,6 +146,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 3c565bb..b99f22f 100644
--- a/vl.c
+++ b/vl.c
@@ -2015,63 +2015,74 @@ static bool virtio_vga_available(void)
     return object_class_by_name("virtio-vga");
 }
 
-static void select_vgahw (const char *p)
+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] 7+ messages in thread

* [Qemu-devel] [PATCH for-2.7 v2 3/3] vl: Replace *_vga_available() functions with class_names field
  2016-04-14  0:51 [Qemu-devel] [PATCH for-2.7 v2 0/3] vl: Simplify vga interface configuration code Eduardo Habkost
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 1/3] vl: Use exit(1) when requested VGA interface is unavailable Eduardo Habkost
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 2/3] vl: Table-based select_vgahw() Eduardo Habkost
@ 2016-04-14  0:51 ` Eduardo Habkost
  2016-04-14  7:40   ` Marcel Apfelbaum
  2 siblings, 1 reply; 7+ messages in thread
From: Eduardo Habkost @ 2016-04-14  0:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Michael S. Tsirkin, Marcel Apfelbaum

Instead of requiring a separate function for each VGA interface,
just enumerate the corresponding class names on struct
VGAInterfaceInfo.

Reviewed-by: Eric Blake <eblake@redhat.com>
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 b99f22f..36593c9 100644
--- a/vl.c
+++ b/vl.c
@@ -1979,46 +1979,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] = {
@@ -2028,43 +1994,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;
@@ -2074,7 +2050,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);
             }
@@ -4492,9 +4468,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] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v2 1/3] vl: Use exit(1) when requested VGA interface is unavailable
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 1/3] vl: Use exit(1) when requested VGA interface is unavailable Eduardo Habkost
@ 2016-04-14  7:23   ` Marcel Apfelbaum
  0 siblings, 0 replies; 7+ messages in thread
From: Marcel Apfelbaum @ 2016-04-14  7:23 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Michael S. Tsirkin

On 04/14/2016 03:51 AM, Eduardo Habkost wrote:
> Instead of using exit(0), use exit(1) when an unavailable VGA
> interface is used in the command-line to indicate it's an error.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   vl.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 9df534f..3c565bb 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2025,28 +2025,28 @@ static void select_vgahw (const char *p)
>               vga_interface_type = VGA_STD;
>           } else {
>               error_report("standard VGA not available");
> -            exit(0);
> +            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(0);
> +            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(0);
> +            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(0);
> +            exit(1);
>           }
>       } else if (strstart(p, "xenfb", &opts)) {
>           vga_interface_type = VGA_XENFB;
> @@ -2055,21 +2055,21 @@ static void select_vgahw (const char *p)
>               vga_interface_type = VGA_QXL;
>           } else {
>               error_report("QXL VGA not available");
> -            exit(0);
> +            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(0);
> +            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(0);
> +            exit(1);
>           }
>       } else if (!strstart(p, "none", &opts)) {
>       invalid_vga:
>

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v2 2/3] vl: Table-based select_vgahw()
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 2/3] vl: Table-based select_vgahw() Eduardo Habkost
@ 2016-04-14  7:28   ` Marcel Apfelbaum
  0 siblings, 0 replies; 7+ messages in thread
From: Marcel Apfelbaum @ 2016-04-14  7:28 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Michael S. Tsirkin

On 04/14/2016 03:51 AM, Eduardo Habkost wrote:
> Instead of implementing separate check functions for each vga
> interface type, add a table enumerating the possible VGA
> interfaces.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Keep assert(vga_interface_type == VGA_NONE) line
>    Suggested-by: Eric Blake <eblake@redhat.com>
> * Fix spacing on select_vgahw() definition
>    Suggested-by: Eric Blake <eblake@redhat.com>
> ---
>   include/sysemu/sysemu.h |   1 +
>   vl.c                    | 115 ++++++++++++++++++++++++++----------------------
>   2 files changed, 64 insertions(+), 52 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 38fb3ca..2033f8c 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -146,6 +146,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 3c565bb..b99f22f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2015,63 +2015,74 @@ static bool virtio_vga_available(void)
>       return object_class_by_name("virtio-vga");
>   }
>
> -static void select_vgahw (const char *p)
> +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);
>

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v2 3/3] vl: Replace *_vga_available() functions with class_names field
  2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 3/3] vl: Replace *_vga_available() functions with class_names field Eduardo Habkost
@ 2016-04-14  7:40   ` Marcel Apfelbaum
  0 siblings, 0 replies; 7+ messages in thread
From: Marcel Apfelbaum @ 2016-04-14  7:40 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Michael S. Tsirkin

On 04/14/2016 03:51 AM, Eduardo Habkost wrote:
> Instead of requiring a separate function for each VGA interface,
> just enumerate the corresponding class names on struct
> VGAInterfaceInfo.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 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 b99f22f..36593c9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1979,46 +1979,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] = {
> @@ -2028,43 +1994,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;
> @@ -2074,7 +2050,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);
>               }
> @@ -4492,9 +4468,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";
>           }
>       }
>

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-04-14  7:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-14  0:51 [Qemu-devel] [PATCH for-2.7 v2 0/3] vl: Simplify vga interface configuration code Eduardo Habkost
2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 1/3] vl: Use exit(1) when requested VGA interface is unavailable Eduardo Habkost
2016-04-14  7:23   ` Marcel Apfelbaum
2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 2/3] vl: Table-based select_vgahw() Eduardo Habkost
2016-04-14  7:28   ` Marcel Apfelbaum
2016-04-14  0:51 ` [Qemu-devel] [PATCH for-2.7 v2 3/3] vl: Replace *_vga_available() functions with class_names field Eduardo Habkost
2016-04-14  7:40   ` Marcel Apfelbaum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).