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 <ehabk...@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 <ebl...@redhat.com>
Thanks!
--
Eduardo
