From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43839) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqbhu-0005fD-8a for qemu-devel@nongnu.org; Thu, 14 Apr 2016 03:28:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aqbhp-0001er-5M for qemu-devel@nongnu.org; Thu, 14 Apr 2016 03:28:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40053) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqbho-0001ec-Tn for qemu-devel@nongnu.org; Thu, 14 Apr 2016 03:28:49 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CD6046406B for ; Thu, 14 Apr 2016 07:28:47 +0000 (UTC) References: <1460595118-23127-1-git-send-email-ehabkost@redhat.com> <1460595118-23127-3-git-send-email-ehabkost@redhat.com> From: Marcel Apfelbaum Message-ID: <570F46AD.5040501@redhat.com> Date: Thu, 14 Apr 2016 10:28:45 +0300 MIME-Version: 1.0 In-Reply-To: <1460595118-23127-3-git-send-email-ehabkost@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.7 v2 2/3] vl: Table-based select_vgahw() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , qemu-devel@nongnu.org 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 > Signed-off-by: Eduardo Habkost > --- > Changes v1 -> v2: > * Keep assert(vga_interface_type == VGA_NONE) line > Suggested-by: Eric Blake > * Fix spacing on select_vgahw() definition > Suggested-by: Eric Blake > --- > 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 Thanks, Marcel