[libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi

[Cole Robinson]

Guido Günther wrote:
> Hi,
> use qemudDiskDeviceName to determine the block device name (as suggested
> by Cole).
>  -- Guido
> 

> From 6985fee585561b04942036d283632e7cb2bb708f Mon Sep 17 00:00:00 2001
> From: Guido Guenther <agx at sigxcpu.org>
> Date: Thu, 2 Oct 2008 21:12:20 +0200
> Subject: [PATCH] support virtio and scsi disks in qemudDomainBlockStats
> 
> ---
>  src/qemu_driver.c |   56 ++++++++++++++++++++++++----------------------------
>  1 files changed, 26 insertions(+), 30 deletions(-)
> 
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index b2ea0d1..5ffaf21 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -3397,11 +3397,13 @@ qemudDomainBlockStats (virDomainPtr dom,
>  {
>      const struct qemud_driver *driver =
>          (struct qemud_driver *)dom->conn->privateData;
> -    char *dummy, *info;
> +    char *dummy, *info = NULL;
>      const char *p, *eol;
> -    char qemu_dev_name[32];
> +    const char *qemu_dev_name = NULL;
>      size_t len;
> +    int ret = -1;
>      const virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id);
> +    virDomainDiskDefPtr disk;
>  
>      if (!vm) {
>          qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
> @@ -3414,34 +3416,31 @@ qemudDomainBlockStats (virDomainPtr dom,
>          return -1;
>      }
>  
> -    /*
> -     * QEMU internal block device names are different from the device
> -     * names we use in libvirt, so we need to map between them:
> -     *
> -     *   hd[a-]   to  ide0-hd[0-]
> -     *   cdrom    to  ide1-cd0
> -     *   fd[a-]   to  floppy[0-]
> -     */
> -    if (STRPREFIX (path, "hd") && c_islower(path[2]))
> -        snprintf (qemu_dev_name, sizeof (qemu_dev_name),
> -                  "ide0-hd%d", path[2] - 'a');
> -    else if (STREQ (path, "cdrom"))
> -        strcpy (qemu_dev_name, "ide1-cd0");
> -    else if (STRPREFIX (path, "fd") && c_islower(path[2]))
> -        snprintf (qemu_dev_name, sizeof (qemu_dev_name),
> -                  "floppy%d", path[2] - 'a');
> -    else {
> +    disk = vm->def->disks;
> +    while (disk) {
> +        if (STREQ(disk->dst, path))
> +            break;
> +        disk = disk->next;
> +    }
> +
> +    if (!disk) {
>          qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG,
>                            _("invalid path: %s"), path);
>          return -1;
>      }
>  
> +    qemu_dev_name = qemudDiskDeviceName(dom, disk);
> +    if (!qemu_dev_name) {
> +        qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG,
> +                          _("unknown path %s"), path);
> +        return -1;
> +    }

You should probably just return -1 here, since qemudDiskDeviceName
sets its own error message.

The rest looks fine to me.

Thanks,
Cole