On 04/10/2018 10:58 AM, Michal Privoznik wrote:
> This is the easier part. All we need to do here is put -object
> pr-manager-helper,id=$alias,path=$socketPath and then just
> reference the object in -drive file.pr-manager=$alias.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 94 ++++++++++++++++++++++
> src/qemu/qemu_command.h | 3 +
> .../disk-virtio-scsi-reservations.args | 35 ++++++++
> tests/qemuxml2argvtest.c | 4 +
> 4 files changed, 136 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
>
This patch fails qemuxml2argv for me because of commits 'cc32731a' and
'a32539de'...
1430) QEMU XML-2-ARGV disk-virtio-scsi-reservations ...
In
'/home/jferlan/git/libvirt.work/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args':
Offset 405
Expect [defaults -chardev
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait
-mon chardev=charmonitor,id=monitor,mode=readline]
Actual [-user-config -nodefaults -chardev
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait
-mon chardev=charmonitor,id=monitor,mode=control]
... FAILED
Simple fix... regenerating the output gets:
diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
index 195e83a048..0bdd35a18a 100644
--- a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
+++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
@@ -16,10 +16,11 @@ path=/path/to/qemu-pr-helper.sock \
-smp 8,sockets=8,cores=1,threads=1 \
-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
-nographic \
+-no-user-config \
-nodefaults \
-chardev
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
server,nowait \
--mon chardev=charmonitor,id=monitor,mode=readline \
+-mon chardev=charmonitor,id=monitor,mode=control \
-no-acpi \
-boot c \
-device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 514c3ab2ef..81f6025207 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1514,6 +1514,20 @@ qemuDiskSourceGetProps(virStorageSourcePtr src)
> }
>
>
> +static void
> +qemuBuildDriveSourcePR(virBufferPtr buf,
> + virStorageSourcePtr src)
> +{
> + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
> + qemuDomainDiskPRDPtr prd = srcPriv->prd;
> +
> + if (!prd || !prd->alias)
> + return;
> +
> + virBufferAsprintf(buf, ",file.pr-manager=%s", prd->alias);
> +}
> +
> +
> 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;
> @@ -9872,6 +9888,81 @@ qemuBuildPanicCommandLine(virCommandPtr cmd,
> }
>
>
> +/**
> + * qemuBuildPRManagerInfoProps:
> + * @prd: disk PR runtime info
> + * @propsret: JSON properties to return
> + *
> + * 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(qemuDomainDiskPRDPtr prd,
> + virJSONValuePtr *propsret)
> +{
> + *propsret = NULL;
> +
> + if (!prd || !prd->alias)
> + return 0;
> +
> + if (virJSONValueObjectCreate(propsret,
> + "s:path", prd->path,
> + NULL) < 0)
> + return -1;
> +
> + return 0;
> +}
> +
> +
> +static int
> +qemuBuildMasterPRCommandLine(virCommandPtr cmd,
> + const virDomainDef *def)
> +{
> + size_t i;
> + bool managedAdded = false;
> + virJSONValuePtr props = NULL;
> + char *tmp = NULL;
> + int ret = -1;
> +
> + for (i = 0; i < def->ndisks; i++) {
> + const virDomainDiskDef *disk = def->disks[i];
> + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
> + qemuDomainDiskPRDPtr prd = srcPriv->prd;
> +
> +
> + if (virStoragePRDefIsManaged(disk->src->pr)) {
> + if (managedAdded)
> + continue;
> +
> + managedAdded = true;
As soon as we find one that needs managing we should break from the loop
and then only add pr-manager-helper if we have one to manage. No sense
going through the rest of the list of disks. Move @disk into the top
level and then use it to decide whether or not to add the object. Then
in some way signify that the object was added so that future hotplugs
wouldn't need to somehow guess - a simple check that it was added
already would seem to suffice.
> + }
The next hunk would seem to be useful for the hotplug path, no?
Including perhaps setting some sort of qemu_domain level boolean that
the object was added already so that subsequent or consecutive hotplugs
wouldn't try to add it again.
John
> +
> + if (qemuBuildPRManagerInfoProps(prd, &props) < 0)
> + goto cleanup;
> +
> + if (!props)
> + continue;
> +
> + if (!(tmp = virQEMUBuildObjectCommandlineFromJSON("pr-manager-helper",
> + prd->alias,
> + props)))
> + goto cleanup;
> + virJSONValueFree(props);
> + props = NULL;
> +
> + virCommandAddArgList(cmd, "-object", tmp, NULL);
> + VIR_FREE(tmp);
> + }
> +
> + ret = 0;
> + cleanup:
> + virJSONValueFree(props);
> + return ret;
> +}
> +
> +
> /**
> * qemuBuildCommandLineValidate:
> *
> @@ -10024,6 +10115,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
> if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0)
> goto error;
>
> + if (qemuBuildMasterPRCommandLine(cmd, def) < 0)
> + goto error;
> +
> if (enableFips)
> virCommandAddArg(cmd, "-enable-fips");
>
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 31c9da673c..32f3697181 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -54,6 +54,9 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver,
> size_t *nnicindexes,
> int **nicindexes);
>
> +/* Generate the object properties for pr-manager */
> +int qemuBuildPRManagerInfoProps(qemuDomainDiskPRDPtr prd,
> + virJSONValuePtr *propsret);
>
> /* Generate the object properties for a secret */
> int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
> diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
> new file mode 100644
> index 0000000000..195e83a048
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
> @@ -0,0 +1,35 @@
> +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 \
> +-object pr-manager-helper,id=pr-helper-scsi0-0-0-1,\
> +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-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 \
> +-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,\
> +drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 165137e93c..fdaeb473d0 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -3055,6 +3055,10 @@ mymain(void)
> QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4,
> QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_PCI_MULTIFUNCTION);
>
> + DO_TEST("disk-virtio-scsi-reservations",
> + QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_SCSI,
> + QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_PR_MANAGER_HELPER);
> +
> /* Test disks with format probing enabled for legacy reasons.
> * New tests should not go in this section. */
> driver.config->allowDiskFormatProbing = true;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
