[libvirt] [PATCH 0/4] qemu: Handle managed persisten reservations separately
Keep the handling of the singleton managed pr-manager-helper object separate from the unmanaged ones which are instantiated one-per-disk-source. This applies on top of my branch collecting all ACKed postings of recent blockdev-related work. Current version can be fetched by: git fetch git://pipo.sk/pipo/libvirt.git blockdev-staging Peter Krempa (4): util: storage: Add helper for determining whether a backing chain requires PR qemu: command: Pass in 'src' rather than 'disk' to qemuBuildPRManagerInfoProps qemu: command: Return props as return value in qemuBuildPRManagerInfoProps qemu: Split handling of managed and unmanaged persistent reservations src/conf/domain_conf.c | 2 +- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 123 ++++++++++++++------- src/qemu/qemu_command.h | 4 +- src/qemu/qemu_hotplug.c | 101 ++++++++--------- src/util/virstoragefile.c | 14 +++ src/util/virstoragefile.h | 3 + ...isk-virtio-scsi-reservations.x86_64-latest.args | 4 +- 8 files changed, 154 insertions(+), 98 deletions(-) -- 2.16.2
With blockdev support we will need to introspect whether any of the backing chain members requires PR rather just one of them. Add a helper and reuse it in virDomainDefHasManagedPR. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 14 ++++++++++++++ src/util/virstoragefile.h | 3 +++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bfe863d76d..be78d388df 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29928,7 +29928,7 @@ virDomainDefHasManagedPR(const virDomainDef *def) size_t i; for (i = 0; i < def->ndisks; i++) { - if (virStoragePRDefIsManaged(def->disks[i]->src->pr)) + if (virStorageSourceChainHasManagedPR(def->disks[i]->src)) return true; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6001635916..68a1056f83 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2813,6 +2813,7 @@ virStoragePRDefIsEqual; virStoragePRDefIsManaged; virStoragePRDefParseXML; virStorageSourceBackingStoreClear; +virStorageSourceChainHasManagedPR; virStorageSourceClear; virStorageSourceCopy; virStorageSourceFindByNodeName; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 54de2c1c30..6c71d0cdf9 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2025,6 +2025,20 @@ virStoragePRDefIsManaged(virStoragePRDefPtr prd) } +bool +virStorageSourceChainHasManagedPR(virStorageSourcePtr src) +{ + virStorageSourcePtr n; + + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (virStoragePRDefIsManaged(src->pr)) + return true; + } + + return false; +} + + virSecurityDeviceLabelDefPtr virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src, const char *model) diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1631c4cf66..b1ff5142fb 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -401,6 +401,9 @@ bool virStoragePRDefIsEqual(virStoragePRDefPtr a, virStoragePRDefPtr b); bool virStoragePRDefIsManaged(virStoragePRDefPtr prd); +bool +virStorageSourceChainHasManagedPR(virStorageSourcePtr src); + virSecurityDeviceLabelDefPtr virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src, const char *model); -- 2.16.2
On Thu, May 31, 2018 at 07:30:22PM +0200, Peter Krempa wrote:
With blockdev support we will need to introspect whether any of the backing chain members requires PR rather just one of them. Add a helper and reuse it in virDomainDefHasManagedPR.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 14 ++++++++++++++ src/util/virstoragefile.h | 3 +++ 4 files changed, 19 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Everything is contained in the virStorageSourceStructure. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 12 +++++------- src/qemu/qemu_command.h | 2 +- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 26e61f26f4..9256104f27 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9691,7 +9691,7 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, /** * qemuBuildPRManagerInfoProps: - * @disk: disk definition + * @src: storage source * @propsret: Returns JSON object containing properties of the pr-manager-helper object * * Build the JSON properties for the pr-manager object. @@ -9700,14 +9700,12 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, * -1 on failure (with error message set). */ int -qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, +qemuBuildPRManagerInfoProps(virStorageSourcePtr src, virJSONValuePtr *propsret) { return qemuMonitorCreateObjectProps(propsret, - "pr-manager-helper", - disk->src->pr->mgralias, - "s:path", disk->src->pr->path, - NULL); + "pr-manager-helper", src->pr->mgralias, + "s:path", src->pr->path, NULL); } @@ -9734,7 +9732,7 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, managedAdded = true; } - if (qemuBuildPRManagerInfoProps(disk, &props) < 0) + if (qemuBuildPRManagerInfoProps(disk->src, &props) < 0) goto cleanup; if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index e85efcc980..60b4dcf054 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -55,7 +55,7 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, int **nicindexes); /* Generate the object properties for pr-manager */ -int qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, +int qemuBuildPRManagerInfoProps(virStorageSourcePtr src, virJSONValuePtr *propsret); /* Generate the object properties for a secret */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c656409eaa..44bd41ccb6 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -401,7 +401,7 @@ qemuMaybeBuildPRManagerInfoProps(virDomainObjPtr vm, return 0; } - return qemuBuildPRManagerInfoProps(disk, propsret); + return qemuBuildPRManagerInfoProps(disk->src, propsret); } -- 2.16.2
On Thu, May 31, 2018 at 07:30:23PM +0200, Peter Krempa wrote:
Everything is contained in the virStorageSourceStructure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 12 +++++------- src/qemu/qemu_command.h | 2 +- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Also since we don't do any conditional formatting, fix the comment for the function. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 22 +++++++++++----------- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_hotplug.c | 5 ++++- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9256104f27..2d559938ed 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9692,20 +9692,20 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, /** * qemuBuildPRManagerInfoProps: * @src: storage source - * @propsret: Returns JSON object containing properties of the pr-manager-helper object * * Build the JSON properties for the pr-manager object. - * - * Returns: 0 on success (@propsret is NULL if no properties are needed), - * -1 on failure (with error message set). */ -int -qemuBuildPRManagerInfoProps(virStorageSourcePtr src, - virJSONValuePtr *propsret) +virJSONValuePtr +qemuBuildPRManagerInfoProps(virStorageSourcePtr src) { - return qemuMonitorCreateObjectProps(propsret, - "pr-manager-helper", src->pr->mgralias, - "s:path", src->pr->path, NULL); + virJSONValuePtr ret = NULL; + + if (qemuMonitorCreateObjectProps(&ret, + "pr-manager-helper", src->pr->mgralias, + "s:path", src->pr->path, NULL) < 0) + return NULL; + + return ret; } @@ -9732,7 +9732,7 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, managedAdded = true; } - if (qemuBuildPRManagerInfoProps(disk->src, &props) < 0) + if (!(props = qemuBuildPRManagerInfoProps(disk->src))) goto cleanup; if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 60b4dcf054..10aa21bcdd 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -55,8 +55,7 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, int **nicindexes); /* Generate the object properties for pr-manager */ -int qemuBuildPRManagerInfoProps(virStorageSourcePtr src, - virJSONValuePtr *propsret); +virJSONValuePtr qemuBuildPRManagerInfoProps(virStorageSourcePtr src); /* Generate the object properties for a secret */ int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 44bd41ccb6..e78aff7adf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -401,7 +401,10 @@ qemuMaybeBuildPRManagerInfoProps(virDomainObjPtr vm, return 0; } - return qemuBuildPRManagerInfoProps(disk->src, propsret); + if (!(*propsret = qemuBuildPRManagerInfoProps(disk->src))) + return -1; + + return 0; } -- 2.16.2
On Thu, May 31, 2018 at 07:30:24PM +0200, Peter Krempa wrote:
Also since we don't do any conditional formatting, fix the comment for the function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 22 +++++++++++----------- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_hotplug.c | 5 ++++- 3 files changed, 16 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Add code that will handle the managed persistent reservations object separately from the unmanaged one. There is only one managed object so handling it with disks is awkward and does not scale well when backing chains come into view. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 113 +++++++++++++++------ src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 104 ++++++++----------- ...isk-virtio-scsi-reservations.x86_64-latest.args | 4 +- 4 files changed, 130 insertions(+), 92 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2d559938ed..a42acb3cfc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2201,6 +2201,35 @@ qemuBulildFloppyCommandLineOptions(virCommandPtr cmd, } +static int +qemuBuildDiskUnmanagedPRCommandLine(virCommandPtr cmd, + virStorageSourcePtr src) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + virJSONValuePtr props = NULL; + int ret = -1; + + if (!src->pr || + virStoragePRDefIsManaged(src->pr)) + return 0; + + if (!(props = qemuBuildPRManagerInfoProps(src))) + return -1; + + if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0) + goto cleanup; + + virCommandAddArg(cmd, "-object"); + virCommandAddArgBuffer(cmd, &buf); + + ret = 0; + cleanup: + virBufferFreeAndReset(&buf); + virJSONValueFree(props); + return ret; +} + + static int qemuBuildDiskDriveCommandLine(virCommandPtr cmd, const virDomainDef *def, @@ -2268,6 +2297,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, } } + if (qemuBuildDiskUnmanagedPRCommandLine(cmd, disk->src) < 0) + return -1; + if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0) return -1; @@ -9689,58 +9721,77 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, } +static virJSONValuePtr +qemuBuildPRManagerInfoPropsInternal(const char *alias, + const char *path) +{ + virJSONValuePtr ret = NULL; + + if (qemuMonitorCreateObjectProps(&ret, + "pr-manager-helper", alias, + "s:path", path, NULL) < 0) + return NULL; + + return ret; +} + + /** - * qemuBuildPRManagerInfoProps: - * @src: storage source + * qemuBuildPRManagedManagerInfoProps: * - * Build the JSON properties for the pr-manager object. + * Build the JSON properties for the pr-manager object corresponding to the PR + * daemon managed by libvirt. */ virJSONValuePtr -qemuBuildPRManagerInfoProps(virStorageSourcePtr src) +qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivatePtr priv) { + char *path = NULL; virJSONValuePtr ret = NULL; - if (qemuMonitorCreateObjectProps(&ret, - "pr-manager-helper", src->pr->mgralias, - "s:path", src->pr->path, NULL) < 0) + if (!(path = qemuDomainGetManagedPRSocketPath(priv))) return NULL; + ret = qemuBuildPRManagerInfoPropsInternal(qemuDomainGetManagedPRAlias(), + path); + + VIR_FREE(path); return ret; } +/** + * qemuBuildPRManagerInfoProps: + * @src: storage source + * + * Build the JSON properties for the pr-manager object. + */ +virJSONValuePtr +qemuBuildPRManagerInfoProps(virStorageSourcePtr src) +{ + return qemuBuildPRManagerInfoPropsInternal(src->pr->mgralias, src->pr->path); +} + + static int -qemuBuildMasterPRCommandLine(virCommandPtr cmd, - const virDomainDef *def) +qemuBuildManagedPRCommandLine(virCommandPtr cmd, + const virDomainDef *def, + qemuDomainObjPrivatePtr priv) { virBuffer buf = VIR_BUFFER_INITIALIZER; - size_t i; - bool managedAdded = false; virJSONValuePtr props = NULL; int ret = -1; - for (i = 0; i < def->ndisks; i++) { - const virDomainDiskDef *disk = def->disks[i]; - - if (!disk->src->pr) - continue; - - if (virStoragePRDefIsManaged(disk->src->pr)) { - if (managedAdded) - continue; - - managedAdded = true; - } + if (!virDomainDefHasManagedPR(def)) + return 0; - if (!(props = qemuBuildPRManagerInfoProps(disk->src))) - goto cleanup; + if (!(props = qemuBuildPRManagedManagerInfoProps(priv))) + return -1; - if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0) - goto cleanup; + if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0) + goto cleanup; - virCommandAddArg(cmd, "-object"); - virCommandAddArgBuffer(cmd, &buf); - } + virCommandAddArg(cmd, "-object"); + virCommandAddArgBuffer(cmd, &buf); ret = 0; cleanup: @@ -9955,7 +10006,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) goto error; - if (qemuBuildMasterPRCommandLine(cmd, def) < 0) + if (qemuBuildManagedPRCommandLine(cmd, def, priv) < 0) goto error; if (enableFips) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 10aa21bcdd..b7580b4796 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -56,6 +56,7 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, /* Generate the object properties for pr-manager */ virJSONValuePtr qemuBuildPRManagerInfoProps(virStorageSourcePtr src); +virJSONValuePtr qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivatePtr priv); /* Generate the object properties for a secret */ int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e78aff7adf..21503b3905 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -351,60 +351,40 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, * qemuDomainMaybeStartPRDaemon: * @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 * - * Checks if it's needed to start qemu-pr-helper and starts it. + * Checks if it's needed to start qemu-pr-helper and add the corresponding + * pr-manager-helper object. * - * Returns: 0 if qemu-pr-helper is not needed - * 1 if it is needed and was started - * -1 otherwise. + * Returns: 0 on success, -1 on error. If @retProps is populated the + * qemu-pr-helper daemon was started. */ static int -qemuDomainMaybeStartPRDaemon(virDomainObjPtr vm, - virDomainDiskDefPtr disk) +qemuDomainDiskAttachManagedPR(virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virJSONValuePtr *retProps) { qemuDomainObjPrivatePtr priv = vm->privateData; + virJSONValuePtr props = NULL; + int ret = -1; - if (!virStoragePRDefIsManaged(disk->src->pr)) { - /* @disk itself does not require qemu-pr-helper. */ - return 0; - } - - if (priv->prDaemonRunning) { - /* @disk requires qemu-pr-helper but there's already one running. */ + if (priv->prDaemonRunning || + !virStorageSourceChainHasManagedPR(disk->src)) return 0; - } - /* @disk requires qemu-pr-helper but none is running. - * Start it now. */ - if (qemuProcessStartManagedPRDaemon(vm) < 0) + if (!(props = qemuBuildPRManagedManagerInfoProps(priv))) return -1; - return 1; -} - - -static int -qemuMaybeBuildPRManagerInfoProps(virDomainObjPtr vm, - const virDomainDiskDef *disk, - virJSONValuePtr *propsret) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - - *propsret = NULL; - - if (!disk->src->pr) - return 0; - - if (virStoragePRDefIsManaged(disk->src->pr) && - priv->prDaemonRunning) { - /* @disk requires qemu-pr-helper but there's already one running. */ - return 0; - } + if (qemuProcessStartManagedPRDaemon(vm) < 0) + goto cleanup; - if (!(*propsret = qemuBuildPRManagerInfoProps(disk->src))) - return -1; + VIR_STEAL_PTR(*retProps, props); + ret = 0; - return 0; + cleanup: + virJSONValueFree(props); + return ret; } @@ -419,21 +399,21 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, virDomainDiskDefPtr disk) { int ret = -1; - int rv; qemuDomainObjPrivatePtr priv = vm->privateData; virErrorPtr orig_err; char *devstr = NULL; char *drivestr = NULL; char *drivealias = NULL; - char *prmgrAlias = NULL; + char *unmanagedPrmgrAlias = NULL; + char *managedPrmgrAlias = NULL; char *encobjAlias = NULL; char *secobjAlias = NULL; bool driveAdded = false; - bool prdStarted = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virJSONValuePtr secobjProps = NULL; virJSONValuePtr encobjProps = NULL; - virJSONValuePtr prmgrProps = NULL; + virJSONValuePtr unmanagedPrmgrProps = NULL; + virJSONValuePtr managedPrmgrProps = NULL; qemuDomainStorageSourcePrivatePtr srcPriv; qemuDomainSecretInfoPtr secinfo = NULL; qemuDomainSecretInfoPtr encinfo = NULL; @@ -461,16 +441,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) goto error; - if (qemuMaybeBuildPRManagerInfoProps(vm, disk, &prmgrProps) < 0) + if (qemuDomainDiskAttachManagedPR(vm, disk, &managedPrmgrProps) < 0) goto error; - /* Start daemon only after prmgrProps is built. Otherwise - * qemuDomainMaybeStartPRDaemon() might start daemon and set - * priv->prDaemonRunning which confuses props building code. */ - if ((rv = qemuDomainMaybeStartPRDaemon(vm, disk)) < 0) + if (disk->src->pr && + virStoragePRDefIsManaged(disk->src->pr) && + !(unmanagedPrmgrProps = qemuBuildPRManagerInfoProps(disk->src))) goto error; - else if (rv > 0) - prdStarted = true; if (disk->src->haveTLS && qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src) < 0) @@ -498,8 +475,12 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuMonitorAddObject(priv->mon, &encobjProps, &encobjAlias) < 0) goto exit_monitor; - if (prmgrProps && - qemuMonitorAddObject(priv->mon, &prmgrProps, &prmgrAlias) < 0) + if (managedPrmgrProps && + qemuMonitorAddObject(priv->mon, &managedPrmgrProps, &managedPrmgrAlias) < 0) + goto exit_monitor; + + if (unmanagedPrmgrProps && + qemuMonitorAddObject(priv->mon, &unmanagedPrmgrProps, &unmanagedPrmgrAlias) < 0) goto exit_monitor; if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) @@ -520,11 +501,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, ret = 0; cleanup: - virJSONValueFree(prmgrProps); + virJSONValueFree(managedPrmgrProps); + virJSONValueFree(unmanagedPrmgrProps); virJSONValueFree(encobjProps); virJSONValueFree(secobjProps); qemuDomainSecretDiskDestroy(disk); - VIR_FREE(prmgrAlias); + VIR_FREE(managedPrmgrAlias); + VIR_FREE(unmanagedPrmgrAlias); VIR_FREE(secobjAlias); VIR_FREE(encobjAlias); VIR_FREE(drivealias); @@ -543,8 +526,10 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, secobjAlias)); if (encobjAlias) ignore_value(qemuMonitorDelObject(priv->mon, encobjAlias)); - if (prmgrAlias) - ignore_value(qemuMonitorDelObject(priv->mon, prmgrAlias)); + if (unmanagedPrmgrAlias) + ignore_value(qemuMonitorDelObject(priv->mon, unmanagedPrmgrAlias)); + if (managedPrmgrAlias) + ignore_value(qemuMonitorDelObject(priv->mon, managedPrmgrAlias)); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; virErrorRestore(&orig_err); @@ -554,7 +539,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, error: qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src); ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true)); - if (prdStarted) + if (priv->prDaemonRunning && + !virDomainDefHasManagedPR(vm->def)) qemuProcessKillManagedPRDaemon(vm); goto cleanup; } diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.x86_64-latest.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.x86_64-latest.args index 768bc22f9f..90843a19f5 100644 --- a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.x86_64-latest.args @@ -11,8 +11,6 @@ QEMU_AUDIO_DRV=none \ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -object pr-manager-helper,id=pr-helper0,\ path=/tmp/lib/domain--1-QEMUGuest1/pr-helper0.sock \ --object pr-manager-helper,id=pr-helper-scsi0-0-0-1,\ -path=/path/to/qemu-pr-helper.sock \ -machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \ -m 214 \ -realtime mlock=off \ @@ -34,6 +32,8 @@ server,nowait \ if=none,id=drive-scsi0-0-0-0 \ -device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 \ +-object pr-manager-helper,id=pr-helper-scsi0-0-0-1,\ +path=/path/to/qemu-pr-helper.sock \ -drive file=/dev/HostVG/QEMUGuest2,file.pr-manager=pr-helper-scsi0-0-0-1,\ format=raw,if=none,id=drive-scsi0-0-0-1 \ -device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=1,\ -- 2.16.2
On Thu, May 31, 2018 at 19:30:25 +0200, Peter Krempa wrote:
Add code that will handle the managed persistent reservations object separately from the unmanaged one. There is only one managed object so handling it with disks is awkward and does not scale well when backing chains come into view.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 113 +++++++++++++++------ src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 104 ++++++++----------- ...isk-virtio-scsi-reservations.x86_64-latest.args | 4 +- 4 files changed, 130 insertions(+), 92 deletions(-)
[...]
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e78aff7adf..21503b3905 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c
[...]
@@ -461,16 +441,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) goto error;
- if (qemuMaybeBuildPRManagerInfoProps(vm, disk, &prmgrProps) < 0) + if (qemuDomainDiskAttachManagedPR(vm, disk, &managedPrmgrProps) < 0) goto error;
- /* Start daemon only after prmgrProps is built. Otherwise - * qemuDomainMaybeStartPRDaemon() might start daemon and set - * priv->prDaemonRunning which confuses props building code. */ - if ((rv = qemuDomainMaybeStartPRDaemon(vm, disk)) < 0) + if (disk->src->pr && + virStoragePRDefIsManaged(disk->src->pr) &&
This condition term is supposed to be inverted, since we are adding only non-managed objects here. I'll fix it in my branch.
+ !(unmanagedPrmgrProps = qemuBuildPRManagerInfoProps(disk->src))) goto error;
On Thu, May 31, 2018 at 07:59:35PM +0200, Peter Krempa wrote:
On Thu, May 31, 2018 at 19:30:25 +0200, Peter Krempa wrote:
Add code that will handle the managed persistent reservations object separately from the unmanaged one. There is only one managed object so handling it with disks is awkward and does not scale well when backing chains come into view.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 113 +++++++++++++++------ src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 104 ++++++++----------- ...isk-virtio-scsi-reservations.x86_64-latest.args | 4 +- 4 files changed, 130 insertions(+), 92 deletions(-)
[...]
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e78aff7adf..21503b3905 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c
[...]
@@ -461,16 +441,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) goto error;
- if (qemuMaybeBuildPRManagerInfoProps(vm, disk, &prmgrProps) < 0) + if (qemuDomainDiskAttachManagedPR(vm, disk, &managedPrmgrProps) < 0) goto error;
- /* Start daemon only after prmgrProps is built. Otherwise - * qemuDomainMaybeStartPRDaemon() might start daemon and set - * priv->prDaemonRunning which confuses props building code. */ - if ((rv = qemuDomainMaybeStartPRDaemon(vm, disk)) < 0) + if (disk->src->pr && + virStoragePRDefIsManaged(disk->src->pr) &&
This condition term is supposed to be inverted, since we are adding only non-managed objects here. I'll fix it in my branch.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa