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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list