9:52 a.m.
When we are changing media (or doing other hotplug operations) we need
to setup cgroups, locking and seclabels on the new disk. This is a
multi-step process where every piece can fail. To simplify dealing with
this introduce qemuDomainPrepareDisk that similarly to
qemuDomainPrepareDiskChainElement initializes/tears down a whole new
disk to be used with the domain.
Additionally the function supports passing a different source struct for
media changes of cdroms that will be refactored later.
---
src/qemu/qemu_driver.c | 8 ---
src/qemu/qemu_hotplug.c | 144 ++++++++++++++++++++++++++++--------------------
2 files changed, 85 insertions(+), 67 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a82aa..03fe96f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6632,9 +6632,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
goto end;
- if (qemuSetupDiskCgroup(vm, disk) < 0)
- goto end;
-
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -6685,11 +6682,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
break;
}
- if (ret != 0 &&
- qemuTeardownDiskCgroup(vm, disk) < 0)
- VIR_WARN("Failed to teardown cgroup for disk path %s",
- NULLSTR(virDomainDiskGetSource(disk)));
-
end:
virObjectUnref(caps);
virDomainDeviceDefFree(dev_copy);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0ad467e..5894f43 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -60,6 +60,83 @@ VIR_LOG_INIT("qemu.qemu_hotplug");
unsigned long long qemuDomainRemoveDeviceWaitTime = 1000ull * 5;
+/**
+ * qemuDomainPrepareDisk:
+ * @driver: qemu driver struct
+ * @vm: domain object
+ * @disk: disk to prepare
+ * @overridesrc: Source different than @disk->src when necessary
+ * @teardown: Teardown the disk instead of adding it to a vm
+ *
+ * Setup the locks, cgroups and security permissions on a disk of a VM.
+ * If @overridesrc is specified the source struct is used instead of the
+ * one present in @disk. If @teardown is true, then the labels and cgroups
+ * are removed instead.
+ *
+ * Returns 0 on success and -1 on error. Reports libvirt error.
+ */
+static int
+qemuDomainPrepareDisk(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr overridesrc,
+ bool teardown)
+{
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ int ret = -1;
+ virStorageSourcePtr origsrc = NULL;
+
+ if (overridesrc) {
+ origsrc = disk->src;
+ disk->src = overridesrc;
+ }
+
+ /* just tear down the disk access */
+ if (teardown) {
+ ret = 0;
+ goto rollback_cgroup;
+ }
+
+ if (virDomainLockDiskAttach(driver->lockManager, cfg->uri,
+ vm, disk) < 0)
+ goto cleanup;
+
+ if (virSecurityManagerSetDiskLabel(driver->securityManager,
+ vm->def, disk) < 0)
+ goto rollback_lock;
+
+ if (qemuSetupDiskCgroup(vm, disk) < 0)
+ goto rollback_label;
+
+ ret = 0;
+ goto cleanup;
+
+ rollback_cgroup:
+ if (qemuTeardownDiskCgroup(vm, disk) < 0)
+ VIR_WARN("Unable to tear down cgroup access on %s",
+ virDomainDiskGetSource(disk));
+
+ rollback_label:
+ if (virSecurityManagerRestoreDiskLabel(driver->securityManager,
+ vm->def, disk) < 0)
+ VIR_WARN("Unable to restore security label on %s",
+ virDomainDiskGetSource(disk));
+
+ rollback_lock:
+ if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
+ VIR_WARN("Unable to release lock on %s",
+ virDomainDiskGetSource(disk));
+
+ cleanup:
+ if (origsrc)
+ disk->src = origsrc;
+
+ virObjectUnref(cfg);
+
+ return ret;
+}
+
+
int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
@@ -87,17 +164,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
goto cleanup;
}
- if (virDomainLockDiskAttach(driver->lockManager, cfg->uri,
- vm, disk) < 0)
- goto cleanup;
-
- if (virSecurityManagerSetDiskLabel(driver->securityManager,
- vm->def, disk) < 0) {
- if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
- VIR_WARN("Unable to release lock on %s",
- virDomainDiskGetSource(disk));
+ if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
- }
if (!(driveAlias = qemuDeviceDriveHostAlias(origdisk, priv->qemuCaps)))
goto error;
@@ -159,14 +227,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (ret < 0)
goto error;
- if (virSecurityManagerRestoreDiskLabel(driver->securityManager,
- vm->def, origdisk) < 0)
- VIR_WARN("Unable to restore security label on ejected image %s",
- virDomainDiskGetSource(origdisk));
-
- if (virDomainLockDiskDetach(driver->lockManager, vm, origdisk) < 0)
- VIR_WARN("Unable to release lock on disk %s",
- virDomainDiskGetSource(origdisk));
+ ignore_value(qemuDomainPrepareDisk(driver, vm, origdisk, NULL, true));
if (virDomainDiskSetSource(origdisk, src) < 0)
goto error;
@@ -182,12 +243,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
error:
virDomainAuditDisk(vm, origdisk->src, disk->src, "update", false);
- if (virSecurityManagerRestoreDiskLabel(driver->securityManager,
- vm->def, disk) < 0)
- VIR_WARN("Unable to restore security label on new media %s", src);
-
- if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
- VIR_WARN("Unable to release lock on %s", src);
+ ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
goto cleanup;
}
@@ -266,17 +322,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
}
}
- if (virDomainLockDiskAttach(driver->lockManager, cfg->uri,
- vm, disk) < 0)
+ if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
- if (virSecurityManagerSetDiskLabel(driver->securityManager,
- vm->def, disk) < 0) {
- if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
- VIR_WARN("Unable to release lock on %s", src);
- goto cleanup;
- }
-
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs,
@@ -347,6 +395,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (releaseaddr)
qemuDomainReleaseDeviceAddress(vm, &disk->info, src);
+ ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
+
if (virSecurityManagerRestoreDiskLabel(driver->securityManager,
vm->def, disk) < 0)
VIR_WARN("Unable to restore security label on %s", src);
@@ -495,7 +545,6 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
char *devstr = NULL;
int ret = -1;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- const char *src = virDomainDiskGetSource(disk);
for (i = 0; i < vm->def->ndisks; i++) {
if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -505,17 +554,9 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
}
}
- if (virDomainLockDiskAttach(driver->lockManager, cfg->uri,
- vm, disk) < 0)
+ if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
- if (virSecurityManagerSetDiskLabel(driver->securityManager,
- vm->def, disk) < 0) {
- if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
- VIR_WARN("Unable to release lock on %s", src);
- goto cleanup;
- }
-
/* We should have an address already, so make sure */
if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -597,13 +638,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
return ret;
error:
- if (virSecurityManagerRestoreDiskLabel(driver->securityManager,
- vm->def, disk) < 0)
- VIR_WARN("Unable to restore security label on %s", src);
-
- if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
- VIR_WARN("Unable to release lock on %s", src);
-
+ ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
goto cleanup;
}
@@ -736,9 +771,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
goto end;
- if (qemuSetupDiskCgroup(vm, disk) < 0)
- goto end;
-
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -805,12 +837,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
break;
}
- if (ret != 0 &&
- qemuTeardownDiskCgroup(vm, disk) < 0) {
- VIR_WARN("Failed to teardown cgroup for disk path %s",
- NULLSTR(src));
- }
-
end:
if (ret != 0)
ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
--
2.0.2