[libvirt] [PATCH v4] qemu: call drive_del in DetachPciDiskDevice

[Daniel P. Berrange]

On Mon, Nov 08, 2010 at 08:44:05PM -0600, Ryan Harper wrote:
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e7b37e1..d01bb2c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9037,6 +9037,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
>      virDomainDiskDefPtr detach = NULL;
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virCgroupPtr cgroup = NULL;
> +    char drivestr[PATH_MAX];

We've been trying to remove use of PATH_MAX allocations on the stack
recently. 

>  
>      i = qemudFindDisk(vm->def, dev->data.disk->dst);
>  
> @@ -9077,6 +9078,20 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
>              goto cleanup;
>          }
>      }
> +
> +    /* build the actual drive id string as the disk->info.alias doesn't
> +     * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */
> +    if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s",
> +                   QEMU_DRIVE_HOST_PREFIX,
> +                   detach->info.alias))
> +        < 0 || ret >= sizeof(drivestr)) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }

Just switch this to virAsprintf(), and then free the string after
the cleanup: label. Also need move this allocation, to before the
initial qemuDomainObjEnterMonitorWithDriver() call.

> +
> +    /* disconnect guest from host device */
> +    qemuMonitorDriveDel(priv->mon, drivestr);
> +
>      qemuDomainObjExitMonitorWithDriver(driver, vm);
>  
>      qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0);
> @@ -9116,6 +9131,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
>      virDomainDiskDefPtr detach = NULL;
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virCgroupPtr cgroup = NULL;
> +    char drivestr[PATH_MAX];
>  
>      i = qemudFindDisk(vm->def, dev->data.disk->dst);
>  
> @@ -9147,6 +9163,19 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
>          qemuDomainObjExitMonitor(vm);
>          goto cleanup;
>      }
> +
> +    /* build the actual drive id string as the disk->info.alias doesn't
> +     * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */
> +    if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s",
> +                   QEMU_DRIVE_HOST_PREFIX,
> +                   detach->info.alias))
> +        < 0 || ret >= sizeof(drivestr)) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +    /* disconnect guest from host device */
> +    qemuMonitorDriveDel(priv->mon, drivestr);
> +
>      qemuDomainObjExitMonitorWithDriver(driver, vm);

Same note about using virAsprintf() + moving the allocation t
before the qemuDomainObjEnterMonitorWithDriver() call.


The patch looks fine apart from these small changes

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|