From aecc6bb85d40f19356720a846297858e91fa4565 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Fri, 8 Aug 2014 10:55:30 +0200 Subject: [PATCH] qemu: hotplug: Sanitize shared device removal on media change Instead of tediously copying of the disk source to remove it later ensure that the media change function removes the old device after it succeeds. --- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_conf.h | 5 ++++ src/qemu/qemu_driver.c | 38 ++++++------------------- src/qemu/qemu_hotplug.c | 62 +++++++++++++++++++---------------------- 4 files changed, 43 insertions(+), 64 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 920e9f117a..25e6d5e6ec 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1097,7 +1097,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, } -static int +int qemuRemoveSharedDisk(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, const char *name) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 3276412777..ae7ac56b3d 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -302,6 +302,11 @@ int qemuRemoveSharedDevice(virQEMUDriverPtr driver, const char *name) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int qemuRemoveSharedDisk(virQEMUDriverPtr driver, + virDomainDiskDefPtr disk, + const char *name) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev); int qemuDriverAllocateID(virQEMUDriverPtr driver); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 628109dcfe..0aa139393e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6637,9 +6637,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, { virDomainDiskDefPtr disk = dev->data.disk; virDomainDiskDefPtr orig_disk = NULL; - virDomainDiskDefPtr tmp = NULL; - virDomainDeviceDefPtr dev_copy = NULL; - virStorageSourcePtr newsrc; virCapsPtr caps = NULL; int ret = -1; @@ -6664,38 +6661,20 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto end; - tmp = dev->data.disk; - dev->data.disk = orig_disk; - - if (!(dev_copy = virDomainDeviceDefCopy(dev, vm->def, - caps, driver->xmlopt))) { - dev->data.disk = tmp; - goto end; - } - dev->data.disk = tmp; - /* Add the new disk src into shared disk hash table */ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) goto end; - newsrc = disk->src; - disk->src = NULL; - - ret = qemuDomainChangeEjectableMedia(driver, conn, vm, - orig_disk, newsrc, force); - /* 'disk' must not be accessed now - it has been freed. - * 'orig_disk' now points to the new disk, while 'dev_copy' - * now points to the old disk */ - - /* Need to remove the shared disk entry for the original - * disk src if the operation is either ejecting or updating. - */ - if (ret == 0) { - dev->data.disk = NULL; - ignore_value(qemuRemoveSharedDevice(driver, dev_copy, - vm->def->name)); + if (qemuDomainChangeEjectableMedia(driver, conn, vm, + orig_disk, dev->data.disk->src, force) < 0) { + ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, vm->def->name)); + goto end; } + + dev->data.disk->src = NULL; + ret = 0; break; + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk bus '%s' cannot be updated."), @@ -6705,7 +6684,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, end: virObjectUnref(caps); - virDomainDeviceDefFree(dev_copy); return ret; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1a2b4b27ff..bf43a41978 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -138,12 +138,29 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, } -int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, - virConnectPtr conn, - virDomainObjPtr vm, - virDomainDiskDefPtr disk, - virStorageSourcePtr newsrc, - bool force) +/** + * qemuDomainChangeEjectableMedia: + * @driver: qemu driver structure + * @conn: connection structure + * @vm: domain definition + * @disk: disk definition to change the source of + * @newsrc: new disk source to change to + * @force: force the change of media + * + * Change the media in an ejectable device to the one described by + * @newsrc. This function also removes the old source from the + * shared device table if appropriate. Note that newsrc is consumed + * on success and the old source is freed on success. + * + * Returns 0 on success, -1 on error and reports libvirt error + */ +int +qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr newsrc, + bool force) { int ret = -1; char *driveAlias = NULL; @@ -225,6 +242,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (ret < 0) goto error; + /* remove the old source from shared device list */ + ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); virStorageSourceFree(disk->src); @@ -232,7 +251,6 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, newsrc = NULL; cleanup: - virStorageSourceFree(newsrc); VIR_FREE(driveAlias); VIR_FREE(sourcestr); return ret; @@ -740,9 +758,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, { virDomainDiskDefPtr disk = dev->data.disk; virDomainDiskDefPtr orig_disk = NULL; - virDomainDeviceDefPtr dev_copy = NULL; - virStorageSourcePtr newsrc; - virDomainDiskDefPtr tmp = NULL; virCapsPtr caps = NULL; int ret = -1; const char *driverName = virDomainDiskGetDriver(disk); @@ -784,32 +799,14 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto end; - - tmp = dev->data.disk; - dev->data.disk = orig_disk; - - if (!(dev_copy = virDomainDeviceDefCopy(dev, vm->def, - caps, driver->xmlopt))) { - dev->data.disk = tmp; + if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, + disk->src, false) < 0) goto end; - } - dev->data.disk = tmp; - newsrc = disk->src; disk->src = NULL; - - ret = qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, newsrc, false); - /* 'newsrc' must not be accessed now - it has been free'd. - * 'orig_disk' now points to the new disk, while 'dev_copy' - * now points to the old disk */ - - /* Need to remove the shared disk entry for the original disk src - * if the operation is either ejecting or updating. - */ - if (ret == 0) - ignore_value(qemuRemoveSharedDevice(driver, dev_copy, - vm->def->name)); + ret = 0; break; + case VIR_DOMAIN_DISK_DEVICE_DISK: case VIR_DOMAIN_DISK_DEVICE_LUN: if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { @@ -841,7 +838,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (ret != 0) ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name)); virObjectUnref(caps); - virDomainDeviceDefFree(dev_copy); return ret; }