Re: [Qemu-devel] [PATCH 1/2] vl: Table-based select_vgahw()

[Eric Blake <eblake@redhat.com>]

[-- 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 --]