[libvirt] PATCH: Fix default bus type selection for disks

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> We recently added a new bus type attribute to disks to select between IDE,
> SCSI, etc. This attribute is optional and many tools won't set it. In such
> cases we default to IDE, which is clearly wrong. This patch updates it to
> pick the default bus based on the prefix of the disk target name, so that
> it DWIM, eg  hdXXX == IDE, sdXXX == SCSI, xvdXXX == Xen, etc.
>
>  qemu_conf.c |   16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> Dan
>
> diff -r 68901592c1ce src/qemu_conf.c
> --- a/src/qemu_conf.c	Sat May 10 13:18:35 2008 -0400
> +++ b/src/qemu_conf.c	Sat May 10 13:55:59 2008 -0400
> @@ -698,10 +698,20 @@
>      }
>
>      if (!bus) {
> -        if (disk->device == QEMUD_DISK_FLOPPY)
> +        if (disk->device == QEMUD_DISK_FLOPPY) {
>              disk->bus = QEMUD_DISK_BUS_FDC;
> -        else
> -            disk->bus = QEMUD_DISK_BUS_IDE;
> +        } else {
> +            if (STRPREFIX((const char *)target, "hd"))
> +                disk->bus = QEMUD_DISK_BUS_IDE;
> +            else if (STRPREFIX((const char *)target, "sd"))
> +                disk->bus = QEMUD_DISK_BUS_SCSI;
> +            else if (STRPREFIX((const char *)target, "vd"))
> +                disk->bus = QEMUD_DISK_BUS_VIRTIO;
> +            else if (STRPREFIX((const char *)target, "xvd"))
> +                disk->bus = QEMUD_DISK_BUS_XEN;
> +            else
> +                disk->bus = QEMUD_DISK_BUS_IDE;
> +        }
>      } else if (STREQ((const char *)bus, "ide"))
>          disk->bus = QEMUD_DISK_BUS_IDE;
>      else if (STREQ((const char *)bus, "fdc"))

ACK,

in spite of the proliferation of casts --
That's not good for readability/maintainability.

What do you think of this?

    static inline char *xml2char(xmlChar *x) { return (char *) x; }

The uses are still ugly, but at least they're safer:
(note that the parameter cannot be a "const" pointer because the
incoming xmlChar* is almost always non-const, as it must be, since
it's going to be freed).

>      if (!bus) {
> -        if (disk->device == QEMUD_DISK_FLOPPY)
> +        if (disk->device == QEMUD_DISK_FLOPPY) {
>              disk->bus = QEMUD_DISK_BUS_FDC;
> -        else
> -            disk->bus = QEMUD_DISK_BUS_IDE;
> +        } else {
> +            if (STRPREFIX(xml2char(target), "hd"))
> +                disk->bus = QEMUD_DISK_BUS_IDE;
> +            else if (STRPREFIX(xml2char(target), "sd"))
> +                disk->bus = QEMUD_DISK_BUS_SCSI;
> +            else if (STRPREFIX(xml2char(target), "vd"))
> +                disk->bus = QEMUD_DISK_BUS_VIRTIO;
> +            else if (STRPREFIX(xml2char(target), "xvd"))
> +                disk->bus = QEMUD_DISK_BUS_XEN;
> +            else
> +                disk->bus = QEMUD_DISK_BUS_IDE;
> +        }
>      } else if (STREQ(xml2char(bus), "ide"))
>          disk->bus = QEMUD_DISK_BUS_IDE;
>      else if (STREQ(xml2char(bus), "fdc"))