7:14 a.m.
A bunch of refactors which could be separated from my blockdev add
branch.
The patches modify the disk hotplug/unplug and media change code so that
we can easily add support for -blockdev later on.
Peter Krempa (10):
qemu: hotplug: Reuse qemuHotplugPrepareDiskAccess in
qemuDomainRemoveDiskDevice
qemu: hotplug: Remove pointless variable
qemu: hotplug: Don't format NULL in %s in qemuHotplugPrepareDiskAccess
qemu: hotplug: Prepare for multiple backing chain member hotplug
qemu: hotplug: Don't leak 'disk' if VM crashes during unplug finishing
qemu: hotplug: Reuse qemuHotplugDiskSourceRemove for disk backend
removal
qemu: hotplug: Simplify removal of managed PR infrastructure on unplug
qemu: hotplug: Refactor/simplify PR managed addition to VM
qemu: hotplug: Extract legacy disk media changing bits
qemu: hotplug: Make qemuHotplugWaitForTrayEject reusable
src/qemu/qemu_hotplug.c | 508 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 358 insertions(+), 150 deletions(-)
--
2.16.2
7:14 a.m.
New subject: [libvirt] [PATCH 01/10] qemu: hotplug: Reuse qemuHotplugPrepareDiskAccess in qemuDomainRemoveDiskDevice
qemuHotplugPrepareDiskAccess can be used to tear down disk access so we
can replace the open-coded version collecting the same function calls.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2b6633a998..ee26fb4f52 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3830,17 +3830,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
qemuDomainReleaseDeviceAddress(vm, &disk->info, src);
- if (qemuSecurityRestoreDiskLabel(driver, vm, disk) < 0)
- VIR_WARN("Unable to restore security label on %s", src);
-
- if (qemuTeardownDiskCgroup(vm, disk) < 0)
- VIR_WARN("Failed to tear down cgroup for disk path %s", src);
-
- if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
- VIR_WARN("Unable to release lock on %s", src);
-
- if (qemuDomainNamespaceTeardownDisk(vm, disk->src) < 0)
- VIR_WARN("Unable to remove /dev entry for %s", src);
+ /* tear down disk security access */
+ qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true);
dev.type = VIR_DOMAIN_DEVICE_DISK;
dev.data.disk = disk;
--
2.16.2
9:21 a.m.
New subject: [libvirt] [PATCH 01/10] qemu: hotplug: Reuse qemuHotplugPrepareDiskAccess in qemuDomainRemoveDiskDevice
On Tue, Jul 17, 2018 at 02:14:21PM +0200, Peter Krempa wrote:
qemuHotplugPrepareDiskAccess can be used to tear down disk access so
we
can replace the open-coded version collecting the same function calls.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:14 a.m.
New subject: [libvirt] [PATCH 02/10] qemu: hotplug: Remove pointless variable
Now that there's only one use of it, replace it directly by the code
filling it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ee26fb4f52..20efb01dbf 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3757,7 +3757,6 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
virDomainDeviceDef dev;
virObjectEventPtr event;
size_t i;
- const char *src = virDomainDiskGetSource(disk);
qemuDomainObjPrivatePtr priv = vm->privateData;
char *drivestr;
bool prManaged = priv->prDaemonRunning;
@@ -3828,7 +3827,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
if (prManaged && !prUsed)
qemuProcessKillManagedPRDaemon(vm);
- qemuDomainReleaseDeviceAddress(vm, &disk->info, src);
+ qemuDomainReleaseDeviceAddress(vm, &disk->info,
virDomainDiskGetSource(disk));
/* tear down disk security access */
qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true);
--
2.16.2
9:22 a.m.
New subject: [libvirt] [PATCH 02/10] qemu: hotplug: Remove pointless variable
On Tue, Jul 17, 2018 at 02:14:22PM +0200, Peter Krempa wrote:
Now that there's only one use of it, replace it directly by the
code
filling it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:14 a.m.
New subject: [libvirt] [PATCH 03/10] qemu: hotplug: Don't format NULL in %s in qemuHotplugPrepareDiskAccess
The warning messages which include the disk source could potentially
format NULL using %s as virDomainDiskGetSource may return NULL for e.g.
NBD disks. As most of the APIs are NOOP for remote disks the usage of
the source string only should be fine for now.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 20efb01dbf..624341baa1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -126,21 +126,21 @@ qemuHotplugPrepareDiskAccess(virQEMUDriverPtr driver,
rollback_cgroup:
if (qemuTeardownDiskCgroup(vm, disk) < 0)
VIR_WARN("Unable to tear down cgroup access on %s",
- virDomainDiskGetSource(disk));
+ NULLSTR(virDomainDiskGetSource(disk)));
rollback_label:
if (qemuSecurityRestoreDiskLabel(driver, vm, disk) < 0)
VIR_WARN("Unable to restore security label on %s",
- virDomainDiskGetSource(disk));
+ NULLSTR(virDomainDiskGetSource(disk)));
rollback_namespace:
if (qemuDomainNamespaceTeardownDisk(vm, disk->src) < 0)
VIR_WARN("Unable to remove /dev entry for %s",
- virDomainDiskGetSource(disk));
+ NULLSTR(virDomainDiskGetSource(disk)));
rollback_lock:
if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
VIR_WARN("Unable to release lock on %s",
- virDomainDiskGetSource(disk));
+ NULLSTR(virDomainDiskGetSource(disk)));
cleanup:
if (origsrc)
--
2.16.2
9:22 a.m.
New subject: [libvirt] [PATCH 03/10] qemu: hotplug: Don't format NULL in %s in qemuHotplugPrepareDiskAccess
On Tue, Jul 17, 2018 at 02:14:23PM +0200, Peter Krempa wrote:
The warning messages which include the disk source could potentially
format NULL using %s as virDomainDiskGetSource may return NULL for e.g.
NBD disks. As most of the APIs are NOOP for remote disks the usage of
the source string only should be fine for now.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:14 a.m.
New subject: [libvirt] [PATCH 04/10] qemu: hotplug: Prepare for multiple backing chain member hotplug
Similarly to how we've intergrated data belonging to a single
virStorageSource for purposes of attaching it to a qemu instance we will
need to agregate data relevant for the whole disk. With blockdev there
will be some disk-wide backing chain members such as the copy-on-read
handler.
Introduce qemuHotplugDiskSourceData which agregates the backing chain
and other data relevant for the disk and functions which generate it
and apply and rollback it.
In addition to disk hotplug this will also be reused for media changing
where we need to exchange the full disk backend.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 118 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 109 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 624341baa1..2ddcc19cc8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -350,6 +350,110 @@ qemuDomainDiskAttachManagedPR(virDomainObjPtr vm,
}
+struct _qemuHotplugDiskSourceData {
+ qemuBlockStorageSourceAttachDataPtr *backends;
+ size_t nbackends;
+};
+typedef struct _qemuHotplugDiskSourceData qemuHotplugDiskSourceData;
+typedef qemuHotplugDiskSourceData *qemuHotplugDiskSourceDataPtr;
+
+
+static void
+qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr data)
+{
+ size_t i;
+
+ if (!data)
+ return;
+
+ for (i = 0; i < data->nbackends; i++)
+ qemuBlockStorageSourceAttachDataFree(data->backends[i]);
+
+ VIR_FREE(data);
+}
+
+
+/**
+ * qemuHotplugDiskSourceAttachPrepare:
+ * @disk: disk to generate attachment data for
+ * @qemuCaps: capabilities of the qemu process
+ *
+ * Prepares and returns qemuHotplugDiskSourceData structure filled with all data
+ * which will fully attach the source backend of the disk to a given VM.
+ */
+static qemuHotplugDiskSourceDataPtr
+qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps)
+{
+ qemuBlockStorageSourceAttachDataPtr backend;
+ qemuHotplugDiskSourceDataPtr data;
+ qemuHotplugDiskSourceDataPtr ret = NULL;
+
+ if (VIR_ALLOC(data) < 0)
+ return NULL;
+
+ if (!(backend = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps, false)))
+ goto cleanup;
+
+ if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, backend, qemuCaps) <
0)
+ goto cleanup;
+
+ if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(ret, data);
+
+ cleanup:
+ qemuBlockStorageSourceAttachDataFree(backend);
+ qemuHotplugDiskSourceDataFree(data);
+ return ret;
+}
+
+
+/**
+ * qemuHotplugDiskSourceAttach:
+ * @mon: monitor object
+ * @data: disk backend data object describing what to remove
+ *
+ * Attach a disk source backend with all relevant pieces. Caller must enter the
+ * monitor context for @mon.
+ */
+static int
+qemuHotplugDiskSoureceAttach(qemuMonitorPtr mon,
+ qemuHotplugDiskSourceDataPtr data)
+{
+ size_t i;
+
+ for (i = data->nbackends; i > 0; i--) {
+ if (qemuBlockStorageSourceAttachApply(mon, data->backends[i - 1]) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/**
+ * qemuHotplugDiskSourceRemove:
+ * @mon: monitor object
+ * @data: disk backend data object describing what to remove
+ *
+ * Remove a disk source backend with all relevant pieces. This function
+ * preserves the error which was set prior to calling it. Caller must enter the
+ * monitor context for @mon.
+ */
+static void
+qemuHotplugDiskSourceRemove(qemuMonitorPtr mon,
+ qemuHotplugDiskSourceDataPtr data)
+
+{
+ size_t i;
+
+ for (i = 0; i < data->nbackends; i++)
+ qemuBlockStorageSourceAttachRollback(mon, data->backends[i]);
+}
+
+
/**
* qemuDomainAttachDiskGeneric:
*
@@ -362,7 +466,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
- qemuBlockStorageSourceAttachDataPtr data = NULL;
+ qemuHotplugDiskSourceDataPtr diskdata = NULL;
virErrorPtr orig_err;
char *devstr = NULL;
char *managedPrmgrAlias = NULL;
@@ -381,11 +485,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuDomainDiskAttachManagedPR(vm, disk, &managedPrmgrProps) < 0)
goto error;
- if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, priv->qemuCaps,
- false)))
- goto error;
-
- if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, data, priv->qemuCaps)
< 0)
+ if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, priv->qemuCaps)))
goto error;
if (!(devstr = qemuBuildDiskDeviceStr(vm->def, disk, 0, priv->qemuCaps)))
@@ -400,7 +500,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
qemuMonitorAddObject(priv->mon, &managedPrmgrProps,
&managedPrmgrAlias) < 0)
goto exit_monitor;
- if (qemuBlockStorageSourceAttachApply(priv->mon, data) < 0)
+ if (qemuHotplugDiskSoureceAttach(priv->mon, diskdata) < 0)
goto exit_monitor;
if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
@@ -417,7 +517,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
ret = 0;
cleanup:
- qemuBlockStorageSourceAttachDataFree(data);
+ qemuHotplugDiskSourceDataFree(diskdata);
virJSONValueFree(managedPrmgrProps);
qemuDomainSecretDiskDestroy(disk);
VIR_FREE(managedPrmgrAlias);
@@ -426,7 +526,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
return ret;
exit_monitor:
- qemuBlockStorageSourceAttachRollback(priv->mon, data);
+ qemuHotplugDiskSourceRemove(priv->mon, diskdata);
virErrorPreserveLast(&orig_err);
if (managedPrmgrAlias)
--
2.16.2
8:11 a.m.
New subject: [libvirt] [PATCH 04/10] qemu: hotplug: Prepare for multiple backing chain member hotplug
On Tue, Jul 17, 2018 at 14:14:24 +0200, Peter Krempa wrote:
Similarly to how we've intergrated data belonging to a single
virStorageSource for purposes of attaching it to a qemu instance we will
need to agregate data relevant for the whole disk. With blockdev there
will be some disk-wide backing chain members such as the copy-on-read
handler.
Introduce qemuHotplugDiskSourceData which agregates the backing chain
and other data relevant for the disk and functions which generate it
and apply and rollback it.
In addition to disk hotplug this will also be reused for media changing
where we need to exchange the full disk backend.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 118 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 109 insertions(+), 9 deletions(-)
[...]
+ * qemuHotplugDiskSourceAttach:
+ * @mon: monitor object
+ * @data: disk backend data object describing what to remove
+ *
+ * Attach a disk source backend with all relevant pieces. Caller must enter the
+ * monitor context for @mon.
+ */
+static int
+qemuHotplugDiskSoureceAttach(qemuMonitorPtr mon,
+ qemuHotplugDiskSourceDataPtr data)
+{
I forgot to squash in a patch that fixes the typo in the name. I've done
the following modification to my private branch:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5f3ff0f7ba..4251c79bf0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -532,8 +532,8 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk,
* monitor context for @mon.
*/
static int
-qemuHotplugDiskSoureceAttach(qemuMonitorPtr mon,
- qemuHotplugDiskSourceDataPtr data)
+qemuHotplugDiskSourceAttach(qemuMonitorPtr mon,
+ qemuHotplugDiskSourceDataPtr data)
{
size_t i;
@@ -658,7 +658,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuHotplugDiskSoureceAttach(priv->mon, diskdata) < 0)
+ if (qemuHotplugDiskSourceAttach(priv->mon, diskdata) < 0)
goto exit_monitor;
if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
10:28 a.m.
New subject: [libvirt] [PATCH 04/10] qemu: hotplug: Prepare for multiple backing chain member hotplug
On Tue, Jul 17, 2018 at 02:14:24PM +0200, Peter Krempa wrote:
Similarly to how we've intergrated data belonging to a single
virStorageSource for purposes of attaching it to a qemu instance we will
need to agregate data relevant for the whole disk. With blockdev there
will be some disk-wide backing chain members such as the copy-on-read
handler.
Introduce qemuHotplugDiskSourceData which agregates the backing chain
and other data relevant for the disk and functions which generate it
and apply and rollback it.
In addition to disk hotplug this will also be reused for media changing
where we need to exchange the full disk backend.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 118 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 109 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:14 a.m.
New subject: [libvirt] [PATCH 05/10] qemu: hotplug: Don't leak 'disk' if VM crashes during unplug finishing
qemuDomainRemoveDiskDevice would leak the disk to be removed if the VM
crashed since it was removed from the definition but not freed.
Broken in commit 105bcdde76b which moved the removal from the definition
earlier.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2ddcc19cc8..f1b18fcc7d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3863,6 +3863,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
bool prUsed = false;
const char *authAlias = NULL;
const char *encAlias = NULL;
+ int ret = -1;
VIR_DEBUG("Removing disk %s from domain %p %s",
disk->info.alias, vm, vm->def->name);
@@ -3917,7 +3918,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
ignore_value(qemuMonitorDelObject(priv->mon, qemuDomainGetManagedPRAlias()));
if (qemuDomainObjExitMonitor(driver, vm) < 0)
- return -1;
+ goto cleanup;
virDomainAuditDisk(vm, disk->src, NULL, "detach", true);
@@ -3937,8 +3938,11 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name));
virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
+ ret = 0;
+
+ cleanup:
virDomainDiskDefFree(disk);
- return 0;
+ return ret;
}
--
2.16.2
10:34 a.m.
New subject: [libvirt] [PATCH 05/10] qemu: hotplug: Don't leak 'disk' if VM crashes during unplug finishing
On Tue, Jul 17, 2018 at 02:14:25PM +0200, Peter Krempa wrote:
qemuDomainRemoveDiskDevice would leak the disk to be removed if the
VM
crashed since it was removed from the definition but not freed.
Broken in commit 105bcdde76b which moved the removal from the definition
earlier.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:14 a.m.
New subject: [libvirt] [PATCH 06/10] qemu: hotplug: Reuse qemuHotplugDiskSourceRemove for disk backend removal
Add code which will convert a disk definition into
qemuHotplugDiskSourceData and then reuse qemuHotplugDiskSourceRemove to
remove all the backend related objects.
This unifies the detach code as much as possible with the already
existing helpers and will allow reuse this infrastructure when changing
removable disk media.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 129 +++++++++++++++++++++++++++++++++++-------------
1 file changed, 94 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f1b18fcc7d..3ee74c8e40 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -373,6 +373,96 @@ qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr data)
}
+/**
+ * qemuDomainRemoveDiskStorageSourcePrepareData:
+ * @src: disk source structure
+ * @driveAlias: Alias of the -drive backend, the pointer is always consumed
+ *
+ * Prepare qemuBlockStorageSourceAttachDataPtr for detaching a single source
+ * from a VM. If @driveAlias is NULL -blockdev is assumed.
+ */
+static qemuBlockStorageSourceAttachDataPtr
+qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src,
+ char *driveAlias)
+
+{
+ qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+ qemuBlockStorageSourceAttachDataPtr data;
+ qemuBlockStorageSourceAttachDataPtr ret = NULL;
+
+ if (VIR_ALLOC(data) < 0)
+ goto cleanup;
+
+ if (driveAlias) {
+ VIR_STEAL_PTR(data->driveAlias, driveAlias);
+ data->driveAdded = true;
+ } else {
+ data->formatNodeName = src->nodeformat;
+ data->formatAttached = true;
+ data->storageNodeName = src->nodestorage;
+ data->storageAttached = true;
+ }
+
+ if (src->pr &&
+ !virStoragePRDefIsManaged(src->pr) &&
+ VIR_STRDUP(data->prmgrAlias, src->pr->mgralias) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(data->tlsAlias, src->tlsAlias) < 0)
+ goto cleanup;
+
+ if (srcpriv) {
+ if (srcpriv->secinfo &&
+ srcpriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES &&
+ VIR_STRDUP(data->authsecretAlias, srcpriv->secinfo->s.aes.alias)
< 0)
+ goto cleanup;
+
+ if (srcpriv->encinfo &&
+ srcpriv->encinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES &&
+ VIR_STRDUP(data->encryptsecretAlias, srcpriv->encinfo->s.aes.alias)
< 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(ret, data);
+
+ cleanup:
+ VIR_FREE(driveAlias);
+ qemuBlockStorageSourceAttachDataFree(data);
+ return ret;
+}
+
+
+static qemuHotplugDiskSourceDataPtr
+qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
+{
+ qemuBlockStorageSourceAttachDataPtr backend;
+ qemuHotplugDiskSourceDataPtr data;
+ qemuHotplugDiskSourceDataPtr ret = NULL;
+ char *drivealias = NULL;
+
+ if (VIR_ALLOC(data) < 0)
+ return NULL;
+
+ if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
+ goto cleanup;
+
+ if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(disk->src,
+ drivealias)))
+ goto cleanup;
+
+ if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(ret, data);
+
+ cleanup:
+ qemuBlockStorageSourceAttachDataFree(backend);
+ qemuHotplugDiskSourceDataFree(data);
+ return ret;
+}
+
+
/**
* qemuHotplugDiskSourceAttachPrepare:
* @disk: disk to generate attachment data for
@@ -3853,36 +3943,21 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
- qemuDomainStorageSourcePrivatePtr diskPriv =
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+ qemuHotplugDiskSourceDataPtr diskbackend = NULL;
virDomainDeviceDef dev;
virObjectEventPtr event;
size_t i;
qemuDomainObjPrivatePtr priv = vm->privateData;
- char *drivestr;
bool prManaged = priv->prDaemonRunning;
bool prUsed = false;
- const char *authAlias = NULL;
- const char *encAlias = NULL;
int ret = -1;
VIR_DEBUG("Removing disk %s from domain %p %s",
disk->info.alias, vm, vm->def->name);
- /* 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 (!(drivestr = qemuAliasDiskDriveFromDisk(disk)))
+ if (!(diskbackend = qemuHotplugDiskSourceRemovePrepare(disk, priv->qemuCaps)))
return -1;
- if (diskPriv) {
- if (diskPriv->secinfo &&
- diskPriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES)
- authAlias = diskPriv->secinfo->s.aes.alias;
-
- if (diskPriv->encinfo &&
- diskPriv->encinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES)
- encAlias = diskPriv->encinfo->s.aes.alias;
- }
-
for (i = 0; i < vm->def->ndisks; i++) {
if (vm->def->disks[i] == disk) {
virDomainDiskRemove(vm->def, i);
@@ -3895,24 +3970,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
- qemuMonitorDriveDel(priv->mon, drivestr);
- VIR_FREE(drivestr);
-
- /* If it fails, then so be it - it was a best shot */
- if (authAlias)
- ignore_value(qemuMonitorDelObject(priv->mon, authAlias));
-
- /* If it fails, then so be it - it was a best shot */
- if (encAlias)
- ignore_value(qemuMonitorDelObject(priv->mon, encAlias));
-
- /* If it fails, then so be it - it was a best shot */
- if (disk->src->pr &&
- !virStoragePRDefIsManaged(disk->src->pr))
- ignore_value(qemuMonitorDelObject(priv->mon,
disk->src->pr->mgralias));
-
- if (disk->src->tlsAlias)
- ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias));
+ qemuHotplugDiskSourceRemove(priv->mon, diskbackend);
if (prManaged && !prUsed)
ignore_value(qemuMonitorDelObject(priv->mon, qemuDomainGetManagedPRAlias()));
@@ -3941,6 +3999,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
ret = 0;
cleanup:
+ qemuHotplugDiskSourceDataFree(diskbackend);
virDomainDiskDefFree(disk);
return ret;
}
--
2.16.2
7:52 a.m.
New subject: [libvirt] [PATCH 06/10] qemu: hotplug: Reuse qemuHotplugDiskSourceRemove for disk backend removal
On Tue, Jul 17, 2018 at 02:14:26PM +0200, Peter Krempa wrote:
Add code which will convert a disk definition into
qemuHotplugDiskSourceData and then reuse qemuHotplugDiskSourceRemove to
remove all the backend related objects.
This unifies the detach code as much as possible with the already
existing helpers and will allow reuse this infrastructure when changing
removable disk media.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 129 +++++++++++++++++++++++++++++++++++-------------
1 file changed, 94 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f1b18fcc7d..3ee74c8e40 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -373,6 +373,96 @@ qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr data)
}
+/**
+ * qemuDomainRemoveDiskStorageSourcePrepareData:
+ * @src: disk source structure
+ * @driveAlias: Alias of the -drive backend, the pointer is always consumed
+ *
+ * Prepare qemuBlockStorageSourceAttachDataPtr for detaching a single source
+ * from a VM. If @driveAlias is NULL -blockdev is assumed.
+ */
+static qemuBlockStorageSourceAttachDataPtr
+qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src,
+ char *driveAlias)
+
+{
+ qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+ qemuBlockStorageSourceAttachDataPtr data;
+ qemuBlockStorageSourceAttachDataPtr ret = NULL;
+
+ if (VIR_ALLOC(data) < 0)
+ goto cleanup;
+
+ if (driveAlias) {
+ VIR_STEAL_PTR(data->driveAlias, driveAlias);
+ data->driveAdded = true;
+ } else {
+ data->formatNodeName = src->nodeformat;
+ data->formatAttached = true;
+ data->storageNodeName = src->nodestorage;
+ data->storageAttached = true;
+ }
+
+ if (src->pr &&
+ !virStoragePRDefIsManaged(src->pr) &&
+ VIR_STRDUP(data->prmgrAlias, src->pr->mgralias) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(data->tlsAlias, src->tlsAlias) < 0)
+ goto cleanup;
+
+ if (srcpriv) {
+ if (srcpriv->secinfo &&
+ srcpriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES &&
+ VIR_STRDUP(data->authsecretAlias, srcpriv->secinfo->s.aes.alias)
< 0)
+ goto cleanup;
+
+ if (srcpriv->encinfo &&
+ srcpriv->encinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES &&
+ VIR_STRDUP(data->encryptsecretAlias, srcpriv->encinfo->s.aes.alias)
< 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(ret, data);
+
+ cleanup:
+ VIR_FREE(driveAlias);
+ qemuBlockStorageSourceAttachDataFree(data);
+ return ret;
+}
+
+
+static qemuHotplugDiskSourceDataPtr
+qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
+{
+ qemuBlockStorageSourceAttachDataPtr backend;
+ qemuHotplugDiskSourceDataPtr data;
+ qemuHotplugDiskSourceDataPtr ret = NULL;
+ char *drivealias = NULL;
+
+ if (VIR_ALLOC(data) < 0)
+ return NULL;
+
+ if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
+ goto cleanup;
+
qemu/qemu_hotplug.c:447:9: error: variable 'backend' is used uninitialized
whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ if (!(backend =
qemuHotplugRemoveStorageSourcePrepareData(disk->src,
+ drivealias)))
+ goto cleanup;
+
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:14 a.m.
New subject: [libvirt] [PATCH 07/10] qemu: hotplug: Simplify removal of managed PR infrastructure on unplug
Extract the (possible) removal of the PR backend and daemon into a
separate helper which enters monitor on it's own. This simplifies the
code and allows reuse of this function in the future e.g. for blockjobs
where removing a image with PR may result into PR not being necessary.
Since the PR is not used often the overhead of entering monitor again
should be negligible.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 38 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3ee74c8e40..57ab753974 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -350,6 +350,41 @@ qemuDomainDiskAttachManagedPR(virDomainObjPtr vm,
}
+/**
+ * qemuHotplugRemoveManagedPR:
+ * @driver: QEMU driver object
+ * @vm: domain object
+ * @asyncJob: asynchronous job identifier
+ *
+ * Removes the managed PR object from @vm if the configuration does not require
+ * it any more.
+ */
+static int
+qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
+ virErrorPreserveLast(&orig_err);
+
+ if (!priv->prDaemonRunning ||
+ virDomainDefHasManagedPR(vm->def))
+ return 0;
+
+ if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+ return -1;
+ ignore_value(qemuMonitorDelObject(priv->mon, qemuDomainGetManagedPRAlias()));
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ return -1;
+
+ qemuProcessKillManagedPRDaemon(vm);
+ virErrorRestore(&orig_err);
+
+ return 0;
+}
+
+
struct _qemuHotplugDiskSourceData {
qemuBlockStorageSourceAttachDataPtr *backends;
size_t nbackends;
@@ -3948,8 +3983,6 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
virObjectEventPtr event;
size_t i;
qemuDomainObjPrivatePtr priv = vm->privateData;
- bool prManaged = priv->prDaemonRunning;
- bool prUsed = false;
int ret = -1;
VIR_DEBUG("Removing disk %s from domain %p %s",
@@ -3965,16 +3998,10 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
}
}
- /* check if the last disk with managed PR was just removed */
- prUsed = virDomainDefHasManagedPR(vm->def);
-
qemuDomainObjEnterMonitor(driver, vm);
qemuHotplugDiskSourceRemove(priv->mon, diskbackend);
- if (prManaged && !prUsed)
- ignore_value(qemuMonitorDelObject(priv->mon, qemuDomainGetManagedPRAlias()));
-
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
@@ -3983,9 +4010,6 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
event = virDomainEventDeviceRemovedNewFromObj(vm, disk->info.alias);
virObjectEventStateQueue(driver->domainEventState, event);
- if (prManaged && !prUsed)
- qemuProcessKillManagedPRDaemon(vm);
-
qemuDomainReleaseDeviceAddress(vm, &disk->info,
virDomainDiskGetSource(disk));
/* tear down disk security access */
@@ -3996,6 +4020,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name));
virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
+ if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
--
2.16.2
8 a.m.
New subject: [libvirt] [PATCH 07/10] qemu: hotplug: Simplify removal of managed PR infrastructure on unplug
On Tue, Jul 17, 2018 at 02:14:27PM +0200, Peter Krempa wrote:
Extract the (possible) removal of the PR backend and daemon into a
separate helper which enters monitor on it's own. This simplifies the
its
code and allows reuse of this function in the future e.g. for
blockjobs
where removing a image with PR may result into PR not being necessary.
Since the PR is not used often the overhead of entering monitor again
should be negligible.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 38 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
6:23 a.m.
New subject: [libvirt] [PATCH 07/10] qemu: hotplug: Simplify removal of managed PR infrastructure on unplug
On 07/17/2018 08:14 AM, Peter Krempa wrote:
Extract the (possible) removal of the PR backend and daemon into a
separate helper which enters monitor on it's own. This simplifies the
code and allows reuse of this function in the future e.g. for blockjobs
where removing a image with PR may result into PR not being necessary.
Since the PR is not used often the overhead of entering monitor again
should be negligible.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 38 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3ee74c8e40..57ab753974 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -350,6 +350,41 @@ qemuDomainDiskAttachManagedPR(virDomainObjPtr vm,
}
+/**
+ * qemuHotplugRemoveManagedPR:
+ * @driver: QEMU driver object
+ * @vm: domain object
+ * @asyncJob: asynchronous job identifier
+ *
+ * Removes the managed PR object from @vm if the configuration does not require
+ * it any more.
+ */
+static int
+qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
+ virErrorPreserveLast(&orig_err);
Coverity points out that orig_err is leaked at each subsequent return
before the virErrorRestore below
John
+
+ if (!priv->prDaemonRunning ||
+ virDomainDefHasManagedPR(vm->def))
+ return 0;
+
+ if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+ return -1;
+ ignore_value(qemuMonitorDelObject(priv->mon, qemuDomainGetManagedPRAlias()));
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ return -1;
+
+ qemuProcessKillManagedPRDaemon(vm);
+ virErrorRestore(&orig_err);
+
+ return 0;
+}
+
+
[...]
7:14 a.m.
New subject: [libvirt] [PATCH 08/10] qemu: hotplug: Refactor/simplify PR managed addition to VM
Similarly to qemuDomainDiskRemoveManagedPR make it enter monitor on
it's own so that it can be reused. Future users will be in the snapshot
code and in removable media change code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 55 ++++++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 57ab753974..c751660403 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -310,29 +310,31 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
/**
- * qemuDomainMaybeStartPRDaemon:
+ * qemuHotplugAttachManagedPR:
+ * @driver: QEMU driver object
* @vm: domain object
- * @disk: disk to hotplug
- * @retProps: properties of the managed pr-manager-helper object which needs
- * to be added to the running vm
+ * @src: new disk source to be attached to @vm
+ * @asyncJob: asynchronous job identifier
*
* Checks if it's needed to start qemu-pr-helper and add the corresponding
* pr-manager-helper object.
*
- * Returns: 0 on success, -1 on error. If @retProps is populated the
- * qemu-pr-helper daemon was started.
+ * Returns: 0 on success, -1 on error.
*/
static int
-qemuDomainDiskAttachManagedPR(virDomainObjPtr vm,
- virDomainDiskDefPtr disk,
- virJSONValuePtr *retProps)
+qemuHotplugAttachManagedPR(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ qemuDomainAsyncJob asyncJob)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
virJSONValuePtr props = NULL;
+ bool daemonStarted = false;
int ret = -1;
+ int rc;
if (priv->prDaemonRunning ||
- !virStorageSourceChainHasManagedPR(disk->src))
+ !virStorageSourceChainHasManagedPR(src))
return 0;
if (!(props = qemuBuildPRManagedManagerInfoProps(priv)))
@@ -341,10 +343,21 @@ qemuDomainDiskAttachManagedPR(virDomainObjPtr vm,
if (qemuProcessStartManagedPRDaemon(vm) < 0)
goto cleanup;
- VIR_STEAL_PTR(*retProps, props);
+ daemonStarted = true;
+
+ if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+ goto cleanup;
+
+ rc = qemuMonitorAddObject(priv->mon, &props, NULL);
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
+ if (ret < 0 && daemonStarted)
+ qemuProcessKillManagedPRDaemon(vm);
virJSONValueFree(props);
return ret;
}
@@ -592,7 +605,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuHotplugDiskSourceDataPtr diskdata = NULL;
- virErrorPtr orig_err;
char *devstr = NULL;
char *managedPrmgrAlias = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
@@ -607,9 +619,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
goto error;
- if (qemuDomainDiskAttachManagedPR(vm, disk, &managedPrmgrProps) < 0)
- goto error;
-
if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, priv->qemuCaps)))
goto error;
@@ -619,11 +628,10 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks + 1) < 0)
goto error;
- qemuDomainObjEnterMonitor(driver, vm);
+ if (qemuHotplugAttachManagedPR(driver, vm, disk->src, QEMU_ASYNC_JOB_NONE) <
0)
+ goto error;
- if (managedPrmgrProps &&
- qemuMonitorAddObject(priv->mon, &managedPrmgrProps,
&managedPrmgrAlias) < 0)
- goto exit_monitor;
+ qemuDomainObjEnterMonitor(driver, vm);
if (qemuHotplugDiskSoureceAttach(priv->mon, diskdata) < 0)
goto exit_monitor;
@@ -653,20 +661,15 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
exit_monitor:
qemuHotplugDiskSourceRemove(priv->mon, diskdata);
- virErrorPreserveLast(&orig_err);
- if (managedPrmgrAlias)
- ignore_value(qemuMonitorDelObject(priv->mon, managedPrmgrAlias));
if (qemuDomainObjExitMonitor(driver, vm) < 0)
ret = -2;
- virErrorRestore(&orig_err);
+ if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+ ret = -2;
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
error:
ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true));
- if (priv->prDaemonRunning &&
- !virDomainDefHasManagedPR(vm->def))
- qemuProcessKillManagedPRDaemon(vm);
goto cleanup;
}
--
2.16.2
8:10 a.m.
New subject: [libvirt] [PATCH 08/10] qemu: hotplug: Refactor/simplify PR managed addition to VM
On Tue, Jul 17, 2018 at 02:14:28PM +0200, Peter Krempa wrote:
Similarly to qemuDomainDiskRemoveManagedPR make it enter monitor on
it's own so that it can be reused. Future users will be in the snapshot
its
code and in removable media change code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 55 ++++++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 26 deletions(-)
@@ -619,11 +628,10 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr
driver,
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks + 1) < 0)
goto error;
- qemuDomainObjEnterMonitor(driver, vm);
+ if (qemuHotplugAttachManagedPR(driver, vm, disk->src, QEMU_ASYNC_JOB_NONE) <
0)
+ goto error;
- if (managedPrmgrProps &&
- qemuMonitorAddObject(priv->mon, &managedPrmgrProps,
&managedPrmgrAlias) < 0)
Both variables are unused after this change.
- goto exit_monitor;
+ qemuDomainObjEnterMonitor(driver, vm);
if (qemuHotplugDiskSoureceAttach(priv->mon, diskdata) < 0)
goto exit_monitor;
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:14 a.m.
New subject: [libvirt] [PATCH 09/10] qemu: hotplug: Extract legacy disk media changing bits
Prepare for the -blockdev implementation of ejectable media changing by
splitting up the old bits.
Additionally since both callers make sure that the device is a cdrom or
floppy the check is no longer necessary.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 99 +++++++++++++++++++++++++++++++------------------
1 file changed, 62 insertions(+), 37 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c751660403..1a7d8f0ca3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -192,7 +192,7 @@ qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
/**
- * qemuDomainChangeEjectableMedia:
+ * qemuDomainChangeMediaLegacy:
* @driver: qemu driver structure
* @vm: domain definition
* @disk: disk definition to change the source of
@@ -206,12 +206,12 @@ qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
*
* Returns 0 on success, -1 on error and reports libvirt error
*/
-int
-qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virDomainDiskDefPtr disk,
- virStorageSourcePtr newsrc,
- bool force)
+static int
+qemuDomainChangeMediaLegacy(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr newsrc,
+ bool force)
{
int ret = -1, rc;
char *driveAlias = NULL;
@@ -231,19 +231,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (srcPriv)
secinfo = srcPriv->secinfo;
- if (disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
- disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Removable media not supported for %s device"),
- virDomainDiskDeviceTypeToString(disk->device));
- goto cleanup;
- }
-
- if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0)
- goto cleanup;
-
if (!(driveAlias = qemuAliasDiskDriveFromDisk(disk)))
- goto error;
+ goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
rc = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
@@ -255,16 +244,16 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_TRAY_MOVED)) {
rc = qemuHotplugWaitForTrayEject(driver, vm, disk, driveAlias);
if (rc < 0)
- goto error;
+ goto cleanup;
} else {
/* otherwise report possible errors from the attempt to eject the media*/
if (rc < 0)
- goto error;
+ goto cleanup;
}
if (!virStorageSourceIsEmpty(newsrc)) {
if (qemuGetDriveSourceString(newsrc, secinfo, &sourcestr) < 0)
- goto error;
+ goto cleanup;
if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) {
if (newsrc->format > 0) {
@@ -283,29 +272,15 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
goto cleanup;
}
- virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0);
-
if (rc < 0)
- goto error;
-
- /* remove the old source from shared device list */
- ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
- ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true));
+ goto cleanup;
- virStorageSourceFree(disk->src);
- disk->src = newsrc;
- newsrc = NULL;
ret = 0;
cleanup:
VIR_FREE(driveAlias);
VIR_FREE(sourcestr);
return ret;
-
- error:
- virDomainAuditDisk(vm, disk->src, newsrc, "update", false);
- ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true));
- goto cleanup;
}
@@ -592,6 +567,56 @@ qemuHotplugDiskSourceRemove(qemuMonitorPtr mon,
}
+/**
+ * qemuDomainChangeEjectableMedia:
+ * @driver: qemu driver 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,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr newsrc,
+ bool force)
+{
+ int ret = -1;
+ int rc;
+
+ if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0)
+ goto cleanup;
+
+ rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
+
+ virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0);
+
+ if (rc < 0) {
+ ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true));
+ goto cleanup;
+ }
+
+ /* remove the old source from shared device list */
+ ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
+ ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true));
+
+ virStorageSourceFree(disk->src);
+ VIR_STEAL_PTR(disk->src, newsrc);
+ ret = 0;
+
+ cleanup:
+ return ret;
+}
+
+
/**
* qemuDomainAttachDiskGeneric:
*
--
2.16.2
8:14 a.m.
New subject: [libvirt] [PATCH 09/10] qemu: hotplug: Extract legacy disk media changing bits
On Tue, Jul 17, 2018 at 02:14:29PM +0200, Peter Krempa wrote:
Prepare for the -blockdev implementation of ejectable media changing
by
splitting up the old bits.
Additionally since both callers make sure that the device is a cdrom or
floppy the check is no longer necessary.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 99 +++++++++++++++++++++++++++++++------------------
1 file changed, 62 insertions(+), 37 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:14 a.m.
New subject: [libvirt] [PATCH 10/10] qemu: hotplug: Make qemuHotplugWaitForTrayEject reusable
Remove the issue of the monitor command to the caller so that the
function can be used with the modern approach.
Additionally improve the error message.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1a7d8f0ca3..774ee25286 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -155,10 +155,8 @@ qemuHotplugPrepareDiskAccess(virQEMUDriverPtr driver,
static int
-qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virDomainDiskDefPtr disk,
- const char *driveAlias)
+qemuHotplugWaitForTrayEject(virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
{
unsigned long long now;
int rc;
@@ -174,19 +172,14 @@ qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
/* the caller called qemuMonitorEjectMedia which usually reports an
* error. Report the failure in an off-chance that it didn't. */
if (virGetLastErrorCode() == VIR_ERR_OK) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("timed out waiting for disk tray status
update"));
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("timed out waiting to open tray of
'%s'"),
+ disk->dst);
}
return -1;
}
}
- /* re-issue ejection command to pop out the media */
- qemuDomainObjEnterMonitor(driver, vm);
- rc = qemuMonitorEjectMedia(qemuDomainGetMonitor(vm), driveAlias, false);
- if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
- return -1;
-
return 0;
}
@@ -242,9 +235,16 @@ qemuDomainChangeMediaLegacy(virQEMUDriverPtr driver,
/* If the tray is present and tray change event is supported wait for it to open. */
if (!force && diskPriv->tray &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_TRAY_MOVED)) {
- rc = qemuHotplugWaitForTrayEject(driver, vm, disk, driveAlias);
+ rc = qemuHotplugWaitForTrayEject(vm, disk);
if (rc < 0)
goto cleanup;
+
+ /* re-issue ejection command to pop out the media */
+ qemuDomainObjEnterMonitor(driver, vm);
+ rc = qemuMonitorEjectMedia(priv->mon, driveAlias, false);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+ goto cleanup;
+
} else {
/* otherwise report possible errors from the attempt to eject the media*/
if (rc < 0)
--
2.16.2
8:20 a.m.
New subject: [libvirt] [PATCH 10/10] qemu: hotplug: Make qemuHotplugWaitForTrayEject reusable
On Tue, Jul 17, 2018 at 02:14:30PM +0200, Peter Krempa wrote:
Remove the issue of the monitor command to the caller so that the
function can be used with the modern approach.
Additionally improve the error message.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
6:36 a.m.
New subject: [libvirt] [PATCH 11/10] qemu: hotplug: Add/remove managed PR objects on media change
When changing cdrom media we did not handle the managed PR objects thus
we'd either have a stale PR object left behind or the media change would
fail.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
Opps this one was also left behind in the blockdev-add series but
belongs to this posting.
src/qemu/qemu_hotplug.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f416db2c60..a1e523013d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -595,6 +595,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0)
goto cleanup;
+ if (qemuHotplugAttachManagedPR(driver, vm, newsrc, QEMU_ASYNC_JOB_NONE) < 0)
+ goto cleanup;
+
rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0);
@@ -610,6 +613,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virStorageSourceFree(disk->src);
VIR_STEAL_PTR(disk->src, newsrc);
+
+ ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE));
+
ret = 0;
cleanup:
--
2.16.2
8:21 a.m.
New subject: [libvirt] [PATCH 11/10] qemu: hotplug: Add/remove managed PR objects on media change
On Wed, Jul 18, 2018 at 01:36:54PM +0200, Peter Krempa wrote:
When changing cdrom media we did not handle the managed PR objects
thus
we'd either have a stale PR object left behind or the media change would
fail.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
Opps this one was also left behind in the blockdev-add series but
belongs to this posting.
src/qemu/qemu_hotplug.c | 6 ++++++
1 file changed, 6 insertions(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
2316
days inactive
2320
days old
24 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Peter Krempa