[Libvirt-cim] [PATCH] Fix the problem that libvirt-cim can't find cdrom device that do not have disk

[Eduardo Lima (Etrunko)]

On 09/16/2011 03:58 AM, Wayne Xia wrote:
[snip]

> diff -r fb09136deb49 -r 04b73e1cdc30 libxkutil/xmlgen.c
> --- a/libxkutil/xmlgen.c	Tue Aug 30 08:48:36 2011 -0700
> +++ b/libxkutil/xmlgen.c	Fri Sep 16 14:24:35 2011 +0800
> @@ -110,10 +110,15 @@
>                          xmlNewProp(tmp, BAD_CAST "cache", BAD_CAST dev->cache);
>          }
> 
> -        tmp = xmlNewChild(disk, NULL, BAD_CAST "source", NULL);
> -        if (tmp == NULL)
> -                return XML_ERROR;
> -        xmlNewProp(tmp, BAD_CAST "file", BAD_CAST dev->source);
> +        if ((XSTREQ(dev->device, "cdrom")) &&
> +                        (XSTREQ(dev->source, ""))) {
> +                /* do nothing */
> +        } else {
> +                tmp = xmlNewChild(disk, NULL, BAD_CAST "source", NULL);
> +                if (tmp == NULL)
> +                        return XML_ERROR;
> +                xmlNewProp(tmp, BAD_CAST "file", BAD_CAST dev->source);
> +        }

Well, I don't really like these types of conditionals where you actually
only do something in the else block, especially because you can always
achieve the same results by denying things.

> 
>          tmp = xmlNewChild(disk, NULL, BAD_CAST "target", NULL);
>          if (tmp == NULL)
> diff -r fb09136deb49 -r 04b73e1cdc30 src/Virt_VirtualSystemManagementService.c
> --- a/src/Virt_VirtualSystemManagementService.c	Tue Aug 30 08:48:36 2011 -0700
> +++ b/src/Virt_VirtualSystemManagementService.c	Fri Sep 16 14:24:35 2011 +0800
> @@ -879,6 +879,12 @@
>                  dev->dev.disk.device = strdup("disk");
>          else if (type == VIRT_DISK_TYPE_CDROM) {
>                  dev->dev.disk.device = strdup("cdrom");
> +                if ((XSTREQ(dev->dev.disk.source, "/dev/null")) ||
> +                        (XSTREQ(dev->dev.disk.source, ""))) {
> +                        dev->dev.disk.disk_type = DISK_FILE;
> +                        free(dev->dev.disk.source);
> +                        dev->dev.disk.source = strdup("");
> +                }
>                  if (dev->dev.disk.disk_type == DISK_UNKNOWN)
>                          dev->dev.disk.disk_type = DISK_PHY;
>          }

All in all the patch delivers the funcionality, so if you fix that logic
I give you a +1.

Best regards, Etrunko

-- 
Eduardo de Barros Lima
Software Engineer, Open Virtualization
Linux Technology Center - IBM/Brazil
eblima at br.ibm.com