Couple of reasons for that: a) there's no monitor command to change path where the pr-helper connects to, or b) there's no monitor command to introduce a new pr-helper for a disk that already exists. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 8 ++++++++ src/util/virstoragefile.c | 18 ++++++++++++++++++ src/util/virstoragefile.h | 2 ++ 4 files changed, 29 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b0dc76b91..424f2ef64 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2719,6 +2719,7 @@ virStorageNetHostTransportTypeToString; virStorageNetProtocolTypeToString; virStoragePRDefFormat; virStoragePRDefFree; +virStoragePRDefIsEqual; virStoragePRDefParseNode; virStorageSourceBackingStoreClear; virStorageSourceClear; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 441bf5935..e8539dcab 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6865,6 +6865,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, CHECK_EQ(src->readonly, "readonly", true); CHECK_EQ(src->shared, "shared", true); + if (!virStoragePRDefIsEqual(disk->src->pr, + orig_disk->src->pr)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("cannot modify field '%s' of the disk"), + "reservations"); + return false; + } + #undef CHECK_EQ return true; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index fda6617f7..469f7c97b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2060,6 +2060,24 @@ virStoragePRDefFormat(virBufferPtr buf, } +bool +virStoragePRDefIsEqual(virStoragePRDefPtr a, + virStoragePRDefPtr b) +{ + if (!a && !b) + return true; + + if (!a || !b) + return false; + + if (a->enabled != b->enabled || + a->managed != b->managed || + STRNEQ_NULLABLE(a->path, b->path)) + return false; + + return true; +} + virSecurityDeviceLabelDefPtr virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src, const char *model) diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index f82c20cf4..d07035d65 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -383,6 +383,8 @@ virStoragePRDefPtr virStoragePRDefParseNode(xmlDocPtr xml, xmlNodePtr root); void virStoragePRDefFormat(virBufferPtr buf, virStoragePRDefPtr prd); +bool virStoragePRDefIsEqual(virStoragePRDefPtr a, + virStoragePRDefPtr b); virSecurityDeviceLabelDefPtr virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src, -- 2.13.6

The capability tracks if qemu has pr-manager-helper object. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ab0ea8ec0..f6abc33ce 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -457,6 +457,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 280 */ "pl011", "machine.pseries.max-cpu-compat", + "pr-manager-helper", ); @@ -1690,6 +1691,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE }, { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, + { "pr-manager-helper", QEMU_CAPS_PR_MANAGER_HELPER }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3dfc77f87..44eba98ec 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -443,6 +443,7 @@ typedef enum { /* 280 */ QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ + QEMU_CAPS_PR_MANAGER_HELPER, /* -object pr-manager-helper */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 2.13.6

This is an extended definition of virStoragePRDef because it contains runtime information (like path to pr helper socket, its pid and alias). Since these are driver dependant we should have a driver specific structure instead of putting all of that into driver agnostic structure. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 18 ++++++++ 2 files changed, 138 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e8539dcab..7fa8c93b7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -65,6 +65,7 @@ #endif #include <sys/time.h> #include <fcntl.h> +#include <signal.h> #if defined(HAVE_SYS_MOUNT_H) # include <sys/mount.h> #endif @@ -1829,6 +1830,9 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) virBitmapFree(priv->migrationCaps); priv->migrationCaps = NULL; + + virHashFree(priv->prHelpers); + priv->prHelpers = NULL; } @@ -10917,6 +10921,122 @@ qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver, } +static void +qemuDomainDiskPRObjectHashFree(void *payload, + const void *name) +{ + qemuDomainDiskPRObjectPtr tmp = payload; + + if (tmp->managed && + tmp->pid != (pid_t) -1) { + VIR_DEBUG("Forcibly killing pr-manager: %s", (const char *) name); + virProcessKillPainfully(tmp->pid, true); + } + VIR_FREE(tmp->path); + VIR_FREE(tmp); +} + + +/** + * qemuDomainDiskPRObjectRegister: + * @priv: Domain private data + * @alias: alias of the pr-manager object + * @managed: true if pr-managed object is manged by libvirt + * @path: socket path for the pr-manager object + * + * Records [alias, managed, path] tuple for pr-manager objects. + * On successful return @path is stolen and set to NULL. + * + * Returns 0 on success (with @path stolen), + * -1 otherwise (with error reported). + */ +int +qemuDomainDiskPRObjectRegister(qemuDomainObjPrivatePtr priv, + const char *alias, + bool managed, + char **path) +{ + qemuDomainDiskPRObjectPtr tmp; + int ret = -1; + + if (!priv->prHelpers && + !(priv->prHelpers = virHashCreate(10, qemuDomainDiskPRObjectHashFree))) + return -1; + + if ((tmp = virHashLookup(priv->prHelpers, alias))) { + /* Entry exists, check if it matches path. This shouldn't + * happen, but it's better to be safe than sorry. */ + if (STRNEQ(tmp->path, *path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("trying to change path for pr helper object")); + + return -1; + } + + /* Claim success */ + VIR_FREE(*path); + return 0; + } + + if (VIR_ALLOC(tmp) < 0) + goto cleanup; + + tmp->managed = managed, + tmp->path = *path; + tmp->pid = (pid_t) -1; + + if (virHashAddEntry(priv->prHelpers, alias, tmp) < 0) + goto cleanup; + + *path = NULL; + tmp = NULL; + ret = 0; + cleanup: + VIR_FREE(tmp); + return ret; +} + + +static int +qemuDomainDiskPRObjectKillOne(void *payload, + const void *name, + void *data ATTRIBUTE_UNUSED) +{ + qemuDomainDiskPRObjectPtr tmp = payload; + + if (!tmp->managed) + return 0; + + VIR_DEBUG("Killing pr-manager: %s", (const char *) name); + if (tmp->pid != (pid_t) -1 && + virProcessKill(tmp->pid, SIGTERM) < 0) { + virReportSystemError(errno, + _("Unable to kill pr-manager: %s"), + (const char *) name); + /* Don't return error; we want to kill as many as + * possible. */ + } else { + tmp->pid = (pid_t) -1; + } + + return 0; +} + + +void +qemuDomainDiskPRObjectKillAll(qemuDomainObjPrivatePtr priv) +{ + if (!priv->prHelpers) + return; + + virHashForEach(priv->prHelpers, + qemuDomainDiskPRObjectKillOne, NULL); + + virHashFree(priv->prHelpers); + priv->prHelpers = NULL; +} + + int qemuDomainPrepareDiskSource(virConnectPtr conn, virDomainDiskDefPtr disk, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ddfc46dcd..f741f3039 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -329,6 +329,8 @@ struct _qemuDomainObjPrivate { /* Migration capabilities. Rechecked on reconnect, not to be saved in * private XML. */ virBitmapPtr migrationCaps; + + virHashTablePtr prHelpers; }; # define QEMU_DOMAIN_PRIVATE(vm) \ @@ -990,4 +992,20 @@ qemuDomainPrepareDiskSource(virConnectPtr conn, qemuDomainObjPrivatePtr priv, virQEMUDriverConfigPtr cfg); +typedef struct _qemuDomainDiskPRObject qemuDomainDiskPRObject; +typedef qemuDomainDiskPRObject *qemuDomainDiskPRObjectPtr; +struct _qemuDomainDiskPRObject { + bool managed; + char *path; /* socket path */ + pid_t pid; /* daemon pid */ +}; + +int +qemuDomainDiskPRObjectRegister(qemuDomainObjPrivatePtr priv, + const char *alias, + bool managed, + char **path); +void +qemuDomainDiskPRObjectKillAll(qemuDomainObjPrivatePtr priv); + #endif /* __QEMU_DOMAIN_H__ */ -- 2.13.6

While we're not generating the command line just yet (look for the next commit), we can generate the alias for pr-manager. A domain can have up to one managed pr-manager (in which case socket path is decided by libvirt and pr-helper is spawned by libvirt too), but up to ndisks of unmanaged ones (one per disk basically). In case of the former we can generate a simple alias and be sure it'll not conflict. But in case of the latter to avoid any conflicts let's generate alias that's based on something unique - like disk target. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ 2 files changed, 64 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7fa8c93b7..e8d2adf56 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -972,6 +972,8 @@ qemuDomainStorageSourcePrivateDispose(void *obj) qemuDomainSecretInfoFree(&priv->secinfo); qemuDomainSecretInfoFree(&priv->encinfo); + + VIR_FREE(priv->prAlias); } @@ -11037,6 +11039,62 @@ qemuDomainDiskPRObjectKillAll(qemuDomainObjPrivatePtr priv) } +static int +qemuDomainPrepareDiskPR(qemuDomainObjPrivatePtr priv, + virDomainDiskDefPtr disk) +{ + qemuDomainStorageSourcePrivatePtr srcPriv; + virStoragePRDefPtr prd = disk->src->pr; + char *prPath = NULL; + bool managed; + int ret = -1; + + if (!prd || + prd->enabled != VIR_TRISTATE_BOOL_YES) + return 0; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("reservations not supported with this QEMU binary")); + goto cleanup; + } + + if (!disk->src->privateData && + !(disk->src->privateData = qemuDomainStorageSourcePrivateNew())) + return -1; + + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + managed = prd->managed == VIR_TRISTATE_BOOL_YES; + + if (managed) { + /* Generate PR helper socket path & alias that are same + * for each disk in the domain. */ + + if (VIR_STRDUP(srcPriv->prAlias, "pr-helper0") < 0) + return -1; + + if (virAsprintf(&prPath, "%s/pr-helper0.sock", priv->libDir) < 0) + return -1; + + } else { + if (virAsprintf(&srcPriv->prAlias, "pr-helper-%s", disk->dst) < 0) + return -1; + + if (VIR_STRDUP(prPath, prd->path) < 0) + return -1; + } + + if (qemuDomainDiskPRObjectRegister(priv, srcPriv->prAlias, + managed, &prPath) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(prPath); + return ret; +} + + int qemuDomainPrepareDiskSource(virConnectPtr conn, virDomainDiskDefPtr disk, @@ -11049,6 +11107,9 @@ qemuDomainPrepareDiskSource(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) return -1; + if (qemuDomainPrepareDiskPR(priv, disk) < 0) + return -1; + if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f741f3039..5fdb5b931 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -375,6 +375,9 @@ struct _qemuDomainStorageSourcePrivate { /* data required for decryption of encrypted storage source */ qemuDomainSecretInfoPtr encinfo; + + /* alias for pr-manager-helper */ + char *prAlias; }; virObjectPtr qemuDomainStorageSourcePrivateNew(void); -- 2.13.6

Now that we generate pr-manger alias and socket path store them in status XML so that they are preserved across daemon restarts. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e8d2adf56..97996b053 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2449,6 +2449,74 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, } +static int +qemuStorageSourcePrivateDataParsePR(xmlXPathContextPtr ctxt, + virStorageSourcePtr src) +{ + qemuDomainStorageSourcePrivatePtr srcPriv; + + if (!src->pr || + src->pr->enabled != VIR_TRISTATE_BOOL_YES) + return 0; + + if (!src->privateData && + !(src->privateData = qemuDomainStorageSourcePrivateNew())) + return -1; + + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + if (!(srcPriv->prAlias = virXPathString("string(./prAlias)", ctxt))) + return -1; + + return 0; +} + + +static int +qemuStorageSourcePrivateDataFormatPR(virStorageSourcePtr src, + virBufferPtr buf) +{ + qemuDomainStorageSourcePrivatePtr srcPriv; + + if (!src->pr || + src->pr->enabled != VIR_TRISTATE_BOOL_YES) + return 0; + + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + virBufferAsprintf(buf, "<prAlias>%s</prAlias>\n", srcPriv->prAlias); + return 0; +} + + +static int +qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, + virStorageSourcePtr src) +{ + if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0) + return -1; + + if (qemuStorageSourcePrivateDataParsePR(ctxt, src) < 0) + return -1; + + return 0; +} + + +static int +qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, + virBufferPtr buf) +{ + if (virStorageSourcePrivateDataFormatRelPath(src, buf) < 0) + return -1; + + if (qemuStorageSourcePrivateDataFormatPR(src, buf) < 0) + return -1; + + return 0; +} + + virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .alloc = qemuDomainObjPrivateAlloc, .free = qemuDomainObjPrivateFree, @@ -2457,8 +2525,8 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .chrSourceNew = qemuDomainChrSourcePrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, - .storageParse = virStorageSourcePrivateDataParseRelPath, - .storageFormat = virStorageSourcePrivateDataFormatRelPath, + .storageParse = qemuStorageSourcePrivateDataParse, + .storageFormat = qemuStorageSourcePrivateDataFormat, }; -- 2.13.6

This is the easier part. All we need to do here is put -object pr-manger-helper,id=$alias,path=$socketPath and then just reference the object in -drive file.pr-manger=$alias. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 59 ++++++++++++++++++++++ .../disk-virtio-scsi-reservations-not-managed.args | 28 ++++++++++ .../disk-virtio-scsi-reservations.args | 29 +++++++++++ tests/qemuxml2argvtest.c | 8 +++ 4 files changed, 124 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b8aede32d..13f2e4fd0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1515,6 +1515,20 @@ qemuDiskSourceGetProps(virStorageSourcePtr src) } +static void +qemuBuildDriveSourcePR(virBufferPtr buf, + virStorageSourcePtr src) +{ + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + if (!src->pr || + src->pr->enabled != VIR_TRISTATE_BOOL_YES) + return; + + virBufferAsprintf(buf, ",file.pr-manager=%s", srcPriv->prAlias); +} + + static int qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps, @@ -1591,6 +1605,8 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, if (disk->src->debug) virBufferAsprintf(buf, ",file.debug=%d", disk->src->debugLevel); + + qemuBuildDriveSourcePR(buf, disk->src); } else { if (!(source = virQEMUBuildDriveCommandlineFromJSON(srcprops))) goto cleanup; @@ -10033,6 +10049,46 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, } +struct qemuBuildMasterPRCommandLineData { + virCommandPtr cmd; +}; + + +static int +qemuBuildMasterPRCommandLineHelper(void *payload, + const void *name, + void *opaque) +{ + qemuDomainDiskPRObjectPtr obj = payload; + struct qemuBuildMasterPRCommandLineData *data = opaque; + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *alias = name; + + virBufferAsprintf(&buf, "pr-manager-helper,id=%s,path=%s", alias, obj->path); + virCommandAddArg(data->cmd, "-object"); + virCommandAddArgBuffer(data->cmd, &buf); + return 0; +} + + +static int +qemuBuildMasterPRCommandLine(virCommandPtr cmd, + qemuDomainObjPrivatePtr priv) +{ + struct qemuBuildMasterPRCommandLineData data = {.cmd = cmd }; + int ret = -1; + + if (priv->prHelpers && + virHashForEach(priv->prHelpers, + qemuBuildMasterPRCommandLineHelper, &data) < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} + + /** * qemuBuildCommandLineValidate: * @@ -10185,6 +10241,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) goto error; + if (qemuBuildMasterPRCommandLine(cmd, priv) < 0) + goto error; + if (enableFips) virCommandAddArg(cmd, "-enable-fips"); diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args new file mode 100644 index 000000000..387432c18 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-object pr-manager-helper,id=pr-helper-sdb,path=/path/to/qemu-pr-helper.sock \ +-M pc \ +-m 214 \ +-smp 8,sockets=8,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,file.pr-manager=pr-helper-sdb,format=raw,\ +if=none,id=drive-scsi0-0-0-0,boot=on \ +-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args new file mode 100644 index 000000000..2dc53cf05 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args @@ -0,0 +1,29 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-object pr-manager-helper,id=pr-helper0,\ +path=/tmp/lib/domain--1-QEMUGuest1/pr-helper0.sock \ +-M pc \ +-m 214 \ +-smp 8,sockets=8,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,file.pr-manager=pr-helper0,format=raw,\ +if=none,id=drive-scsi0-0-0-0,boot=on \ +-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index be32d891e..153098e8c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2959,6 +2959,14 @@ mymain(void) QEMU_CAPS_HDA_DUPLEX); DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI); + DO_TEST("disk-virtio-scsi-reservations", + QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_PR_MANAGER_HELPER); + + DO_TEST("disk-virtio-scsi-reservations-not-managed", + QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_PR_MANAGER_HELPER); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); -- 2.13.6

Just like we allow users overriding path to bridge-helper detected at compile time we can allow them to override path to qemu-pr-helper. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-driver-qemu.m4 | 5 +++++ src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 4 ++++ src/qemu/qemu_conf.c | 7 ++++++- src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 18 insertions(+), 1 deletion(-) diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4 index b9bafdab9..0e0f261d6 100644 --- a/m4/virt-driver-qemu.m4 +++ b/m4/virt-driver-qemu.m4 @@ -57,6 +57,11 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [ [/usr/libexec:/usr/lib/qemu:/usr/lib]) AC_DEFINE_UNQUOTED([QEMU_BRIDGE_HELPER], ["$QEMU_BRIDGE_HELPER"], [QEMU bridge helper]) + AC_PATH_PROG([QEMU_PR_HELPER], [qemu-pr-helper], + [/usr/libexec/qemu-pr-helper], + [/usr/libexec:/usr/lib/qemu:/usr/lib]) + AC_DEFINE_UNQUOTED([QEMU_PR_HELPER], ["$QEMU_PR_HELPER"], + [QEMU PR helper]) ]) AC_DEFUN([LIBVIRT_DRIVER_RESULT_QEMU], [ diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index c19bf3a43..2dc16e91f 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -86,6 +86,7 @@ module Libvirtd_qemu = let process_entry = str_entry "hugetlbfs_mount" | bool_entry "clear_emulator_capabilities" | str_entry "bridge_helper" + | str_entry "pr_helper" | bool_entry "set_process_name" | int_entry "max_processes" | int_entry "max_files" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 43dd561cc..4bc668406 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -775,3 +775,7 @@ # This directory is used for memoryBacking source if configured as file. # NOTE: big files will be stored here #memory_backing_dir = "/var/lib/libvirt/qemu/ram" + +# Path to the SCSI persistent reservations helper. This helper is +# used whenever <reservations/> are enabled for SCSI disks. +#pr_helper = "/usr/libexec/qemu-pr-helper" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index af503d31c..b842fb4ab 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -307,7 +307,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) goto error; } - if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0) + if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0 || + VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0) goto error; cfg->clearEmulatorCapabilities = true; @@ -392,6 +393,7 @@ static void virQEMUDriverConfigDispose(void *obj) } VIR_FREE(cfg->hugetlbfs); VIR_FREE(cfg->bridgeHelperName); + VIR_FREE(cfg->prHelperName); VIR_FREE(cfg->saveImageFormat); VIR_FREE(cfg->dumpImageFormat); @@ -759,6 +761,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (virConfGetValueString(conf, "bridge_helper", &cfg->bridgeHelperName) < 0) goto cleanup; + if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "mac_filter", &cfg->macFilter) < 0) goto cleanup; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a553e30e2..11d986a53 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -153,6 +153,7 @@ struct _virQEMUDriverConfig { size_t nhugetlbfs; char *bridgeHelperName; + char *prHelperName; bool macFilter; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 688e5b9fd..c0efae47b 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -100,3 +100,4 @@ module Test_libvirtd_qemu = { "1" = "mount" } } { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" } +{ "pr_helper" = "/usr/libexec/qemu-pr-helper" } -- 2.13.6

Again, for hotplug we need to be able to spawn just one process. Not all of them. Expose the static function we already have for that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 9 +++++++++ src/qemu/qemu_process.h | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 02608c1f3..b4c4c64fa 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2635,6 +2635,15 @@ qemuProcessSetupOnePRDaemon(void *payload, } +int +qemuProcessSetupPRDaemon(virDomainObjPtr vm, + qemuDomainDiskPRObjectPtr prObj, + const char *prAlias) +{ + return qemuProcessSetupOnePRDaemon(prObj, prAlias, vm); +} + + static int qemuProcessSetupPRDaemons(virDomainObjPtr vm) { diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index cd9a72031..50946e16d 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -199,4 +199,8 @@ int qemuProcessRefreshDisks(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob); +int qemuProcessSetupPRDaemon(virDomainObjPtr vm, + qemuDomainDiskPRObjectPtr prObj, + const char *prAlias); + #endif /* __QEMU_PROCESS_H__ */ -- 2.13.6

Surprisingly, nothing special is happening here. If we are the first to use the managed helper then spawn it. If not, we're almost done. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6b245bd6a..ded666633 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -350,6 +350,84 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } +static int +qemuBuildPRDefInfoProps(virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virJSONValuePtr *prmgrProps, + const char **prAlias) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainStorageSourcePrivatePtr srcPriv; + qemuDomainDiskPRObjectPtr tmp; + virJSONValuePtr props = NULL; + int ret = -1; + + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + + *prmgrProps = NULL; + + if (!priv->prHelpers || + !srcPriv->prAlias || + !(tmp = virHashLookup(priv->prHelpers, srcPriv->prAlias))) + return 0; + + if (qemuDomainGetPRUsageCount(vm->def, srcPriv->prAlias) != 0) { + /* We're not the first ones to use pr-manager with that + * alias. Return early and leave @prmgrProps NULL so + * that caller knows this. */ + return 0; + } + + if (virJSONValueObjectCreate(&props, + "s:path", tmp->path, + NULL) < 0) + goto cleanup; + + if (qemuProcessSetupPRDaemon(vm, tmp, srcPriv->prAlias) < 0) + goto cleanup; + + *prAlias = srcPriv->prAlias; + *prmgrProps = props; + props = NULL; + ret = 0; + cleanup: + virJSONValueFree(props); + return ret; +} + + +static void +qemuDestroyPRDefObject(virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainStorageSourcePrivatePtr srcPriv; + qemuDomainDiskPRObjectPtr tmp; + + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + + if (!priv->prHelpers || + !srcPriv->prAlias || + !(tmp = virHashLookup(priv->prHelpers, srcPriv->prAlias))) + return; + + if (qemuDomainGetPRUsageCount(vm->def, srcPriv->prAlias) > 1) { + /* The function might return one or even zero if the + * pr-manager is unused depending whether the disk was + * added to domain def already or not. Anyway, if the + * return value is greater than one we are certain that + * there's another disk using it so return early. */ + return; + } + + /* This also kills the pr-manager daemon. See + * qemuDomainDiskPRObjectHashFree. */ + virHashRemoveEntry(priv->prHelpers, srcPriv->prAlias); + + VIR_FREE(srcPriv->prAlias); +} + + /** * qemuDomainAttachDiskGeneric: * @@ -368,12 +446,15 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn, char *devstr = NULL; char *drivestr = NULL; char *drivealias = NULL; + const char *prAlias = NULL; bool driveAdded = false; bool secobjAdded = false; bool encobjAdded = false; + bool prmgrAdded = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virJSONValuePtr secobjProps = NULL; virJSONValuePtr encobjProps = NULL; + virJSONValuePtr prmgrProps = NULL; qemuDomainStorageSourcePrivatePtr srcPriv; qemuDomainSecretInfoPtr secinfo = NULL; qemuDomainSecretInfoPtr encinfo = NULL; @@ -406,6 +487,9 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn, disk->info.alias) < 0) goto error; + if (qemuBuildPRDefInfoProps(vm, disk, &prmgrProps, &prAlias) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; @@ -438,6 +522,15 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn, encobjAdded = true; } + if (prmgrProps) { + rv = qemuMonitorAddObject(priv->mon, "pr-manager-helper", prAlias, + prmgrProps); + prmgrProps = NULL; /* qemuMonitorAddObject consumes */ + if (rv < 0) + goto exit_monitor; + prmgrAdded = true; + } + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto exit_monitor; driveAdded = true; @@ -458,6 +551,7 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn, cleanup: virJSONValueFree(secobjProps); virJSONValueFree(encobjProps); + virJSONValueFree(prmgrProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -475,6 +569,8 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn, ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias)); if (encobjAdded) ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); + if (prmgrAdded) + ignore_value(qemuMonitorDelObject(priv->mon, prAlias)); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; virErrorRestore(&orig_err); @@ -484,6 +580,7 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn, error: qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src); ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true)); + qemuDestroyPRDefObject(vm, disk); goto cleanup; } -- 2.13.6

Just line in previous commit, if we are the last ones that are using the pr-manager delete it and also kill the pr-helper process. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 18 ++++++++++++++++++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_hotplug.c | 13 +++++++++++++ 3 files changed, 35 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c327dfc86..d15c29f4a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11263,6 +11263,24 @@ qemuDomainGetPRUsageCount(const virDomainDef *def, } +bool +qemuDomainDiskNeedRemovePR(virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + qemuDomainStorageSourcePrivatePtr srcPriv; + + if (!disk->src) + return false; + + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + if (!srcPriv || + !srcPriv->prAlias) + return false; + + return qemuDomainGetPRUsageCount(vm->def, srcPriv->prAlias) == 1; +} + + static int qemuDomainPrepareDiskPR(qemuDomainObjPrivatePtr priv, virDomainDiskDefPtr disk) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 231583256..6f33d3f36 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1015,4 +1015,8 @@ size_t qemuDomainGetPRUsageCount(const virDomainDef *def, const char *prAlias); +bool +qemuDomainDiskNeedRemovePR(virDomainObjPtr vm, + virDomainDiskDefPtr disk); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ded666633..e2064648b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3825,6 +3825,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, char *drivestr; char *objAlias = NULL; char *encAlias = NULL; + const char *prAlias = NULL; VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name); @@ -3862,6 +3863,13 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } } + if (qemuDomainDiskNeedRemovePR(vm, disk)) { + qemuDomainStorageSourcePrivatePtr srcPriv; + + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + prAlias = srcPriv->prAlias; + } + qemuDomainObjEnterMonitor(driver, vm); qemuMonitorDriveDel(priv->mon, drivestr); @@ -3877,6 +3885,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, encAlias)); VIR_FREE(encAlias); + if (prAlias) + ignore_value(qemuMonitorDelObject(priv->mon, prAlias)); + if (disk->src->haveTLS) ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias)); @@ -3895,6 +3906,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } } + qemuDestroyPRDefObject(vm, disk); + qemuDomainReleaseDeviceAddress(vm, &disk->info, src); if (qemuSecurityRestoreDiskLabel(driver, vm, disk) < 0) -- 2.13.6