On 04/16/2018 04:56 PM, Michal Privoznik wrote:
> On 04/14/2018 03:04 PM, John Ferlan wrote:
>>
>>
>> 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
So just to give everybody an idea what is this patch going to look like,
I'm attaching it here. I'd like to ask if this is really way we want to
go. Frankly, I find the patch ugly, because in order to not strdup() a
static string, I have to differentiate every time I need an alias. Just
look at qemuBuildDriveSourcePR(). I hate it so much, that I made
qemuBuildPRManagerInfoProps() to stdup() the string. Just think how it
would look like if I did no such thing.
Michal
>From a1d69c21da85be8d9ad3c2efc12ca896e397477d Mon Sep 17 00:00:00 2001
Message-Id: <a1d69c21da85be8d9ad3c2efc12ca896e397477d.1524063643.git.mprivozn@xxxxxxxxxx>
From: Michal Privoznik <mprivozn@xxxxxxxxxx>
Date: Wed, 18 Apr 2018 16:55:14 +0200
Subject: [PATCH] generate cmd line
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/libvirt_private.syms | 2 +
src/qemu/qemu_alias.c | 18 +++
src/qemu/qemu_alias.h | 4 +
src/qemu/qemu_command.c | 128 +++++++++++++++++++++
src/qemu/qemu_command.h | 5 +
src/qemu/qemu_domain.c | 22 ++++
src/qemu/qemu_domain.h | 3 +
src/qemu/qemu_process.h | 1 +
src/util/virstoragefile.c | 14 +++
src/util/virstoragefile.h | 2 +
.../disk-virtio-scsi-reservations.args | 38 ++++++
tests/qemuxml2argvtest.c | 4 +
12 files changed, 241 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ee2be0255c..b22c54b030 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2801,7 +2801,9 @@ virStorageNetHostTransportTypeToString;
virStorageNetProtocolTypeToString;
virStoragePRDefFormat;
virStoragePRDefFree;
+virStoragePRDefIsEnabled;
virStoragePRDefIsEqual;
+virStoragePRDefIsManaged;
virStoragePRDefParseXML;
virStorageSourceBackingStoreClear;
virStorageSourceClear;
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 95d1e0370a..9a3a92c82d 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -773,3 +773,21 @@ qemuAliasChardevFromDevAlias(const char *devAlias)
return ret;
}
+
+
+const char *
+qemuDomainGetManagedPRAlias(void)
+{
+ return "pr-helper0";
+}
+
+
+char *
+qemuDomainGetUnmanagedPRAlias(const virDomainDiskDef *disk)
+{
+ char *ret;
+
+ ignore_value(virAsprintf(&ret, "pr-helper-%s", disk->info.alias));
+
+ return ret;
+}
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index 8c744138ce..8e391c3f0c 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -92,4 +92,8 @@ char *qemuAliasTLSObjFromSrcAlias(const char *srcAlias)
char *qemuAliasChardevFromDevAlias(const char *devAlias)
ATTRIBUTE_NONNULL(1);
+const char * qemuDomainGetManagedPRAlias(void);
+
+char *qemuDomainGetUnmanagedPRAlias(const virDomainDiskDef *disk);
+
#endif /* __QEMU_ALIAS_H__*/
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 66ed1383c2..2d8b5c0be8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1462,6 +1462,29 @@ qemuDiskSourceGetProps(virStorageSourcePtr src)
}
+static int
+qemuBuildDriveSourcePR(virBufferPtr buf,
+ virDomainDiskDefPtr disk)
+{
+ char *alias = NULL;
+ const char *defaultAlias = NULL;
+
+ if (!virStoragePRDefIsEnabled(disk->src->pr))
+ return 0;
+
+ if (virStoragePRDefIsManaged(disk->src->pr)) {
+ defaultAlias = qemuDomainGetManagedPRAlias();
+ } else {
+ if (!(alias = qemuDomainGetUnmanagedPRAlias(disk)))
+ return -1;
+ }
+
+ virBufferAsprintf(buf, ",file.pr-manager=%s", alias ? alias : defaultAlias);
+ VIR_FREE(alias);
+ return 0;
+}
+
+
static int
qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
virQEMUCapsPtr qemuCaps,
@@ -1539,6 +1562,9 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
if (disk->src->debug)
virBufferAsprintf(buf, ",file.debug=%d", disk->src->debugLevel);
+
+ if (qemuBuildDriveSourcePR(buf, disk) < 0)
+ goto cleanup;
} else {
if (!(source = virQEMUBuildDriveCommandlineFromJSON(srcprops)))
goto cleanup;
@@ -9638,6 +9664,105 @@ 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(virDomainObjPtr vm,
+ const virDomainDiskDef *disk,
+ virJSONValuePtr *propsret,
+ char **aliasret)
+{
+ char *socketPath = NULL;
+ char *alias = NULL;
+ int ret = -1;
+
+ *propsret = NULL;
+ *aliasret = NULL;
+
+ if (!virStoragePRDefIsEnabled(disk->src->pr))
+ return 0;
+
+ if (!(socketPath = qemuDomainGetPRSocketPath(vm, disk->src->pr)))
+ return ret;
+
+ if (virStoragePRDefIsManaged(disk->src->pr)) {
+ if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0)
+ goto cleanup;
+ } else {
+ if (!(alias = qemuDomainGetUnmanagedPRAlias(disk)))
+ goto cleanup;
+ }
+
+ if (virJSONValueObjectCreate(propsret,
+ "s:path", socketPath,
+ NULL) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(*aliasret, alias);
+ ret = 0;
+ cleanup:
+ VIR_FREE(alias);
+ VIR_FREE(socketPath);
+ return ret;
+}
+
+
+static int
+qemuBuildMasterPRCommandLine(virDomainObjPtr vm,
+ virCommandPtr cmd,
+ const virDomainDef *def)
+{
+ size_t i;
+ bool managedAdded = false;
+ virJSONValuePtr props = NULL;
+ char *alias = NULL;
+ char *tmp = NULL;
+ int ret = -1;
+
+ for (i = 0; i < def->ndisks; i++) {
+ const virDomainDiskDef *disk = def->disks[i];
+
+ if (virStoragePRDefIsManaged(disk->src->pr)) {
+ if (managedAdded)
+ continue;
+
+ managedAdded = true;
+ }
+
+ if (qemuBuildPRManagerInfoProps(vm, disk, &props, &alias) < 0)
+ goto cleanup;
+
+ if (!props)
+ continue;
+
+ if (!(tmp = virQEMUBuildObjectCommandlineFromJSON("pr-manager-helper",
+ alias,
+ props)))
+ goto cleanup;
+ VIR_FREE(alias);
+ virJSONValueFree(props);
+ props = NULL;
+
+ virCommandAddArgList(cmd, "-object", tmp, NULL);
+ VIR_FREE(tmp);
+ }
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(alias);
+ virJSONValueFree(props);
+ return ret;
+}
+
+
/**
* qemuBuildCommandLineValidate:
*
@@ -9818,6 +9943,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0)
goto error;
+ if (qemuBuildMasterPRCommandLine(vm, 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..da1fe679fe 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -54,6 +54,11 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver,
size_t *nnicindexes,
int **nicindexes);
+/* Generate the object properties for pr-manager */
+int qemuBuildPRManagerInfoProps(virDomainObjPtr vm,
+ const virDomainDiskDef *disk,
+ virJSONValuePtr *propsret,
+ char **alias);
/* Generate the object properties for a secret */
int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 58a7272666..218559e9ac 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11889,3 +11889,25 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
}
VIR_FREE(event);
}
+
+
+char *
+qemuDomainGetPRSocketPath(virDomainObjPtr vm,
+ virStoragePRDefPtr pr)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ const char *defaultAlias = NULL;
+ char *ret = NULL;
+
+ if (!virStoragePRDefIsEnabled(pr))
+ return NULL;
+
+ if (virStoragePRDefIsManaged(pr)) {
+ defaultAlias = qemuDomainGetManagedPRAlias();
+ ignore_value(virAsprintf(&ret, "%s/%s.sock", priv->libDir, defaultAlias));
+ } else {
+ ignore_value(VIR_STRDUP(ret, pr->path));
+ }
+
+ return ret;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 40d9a6f247..5fb264675c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -994,4 +994,7 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
qemuDomainObjPrivatePtr priv,
virQEMUDriverConfigPtr cfg);
+char * qemuDomainGetPRSocketPath(virDomainObjPtr vm,
+ virStoragePRDefPtr pr);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 9dd5c97642..3bf66f71b7 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -24,6 +24,7 @@
# include "qemu_conf.h"
# include "qemu_domain.h"
+# include "virstoragefile.h"
int qemuProcessPrepareMonitorChr(virDomainChrSourceDefPtr monConfig,
const char *domainDir);
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 4e3c3ca5fa..877d8ba8e4 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2041,6 +2041,20 @@ virStoragePRDefIsEqual(virStoragePRDefPtr a,
}
+bool
+virStoragePRDefIsEnabled(virStoragePRDefPtr prd)
+{
+ return prd && prd->enabled == VIR_TRISTATE_BOOL_YES;
+}
+
+
+bool
+virStoragePRDefIsManaged(virStoragePRDefPtr prd)
+{
+ return prd && prd->managed == VIR_TRISTATE_BOOL_YES;
+}
+
+
virSecurityDeviceLabelDefPtr
virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src,
const char *model)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 77853eefe8..2127509ea3 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -385,6 +385,8 @@ void virStoragePRDefFormat(virBufferPtr buf,
virStoragePRDefPtr prd);
bool virStoragePRDefIsEqual(virStoragePRDefPtr a,
virStoragePRDefPtr b);
+bool virStoragePRDefIsEnabled(virStoragePRDefPtr prd);
+bool virStoragePRDefIsManaged(virStoragePRDefPtr prd);
virSecurityDeviceLabelDefPtr
virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src,
diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
new file mode 100644
index 0000000000..dce3fc4105
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
@@ -0,0 +1,38 @@
+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 \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-smp 8,sockets=8,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-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 aba946612f..90519a6c1b 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2738,6 +2738,10 @@ mymain(void)
QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4,
QEMU_CAPS_ICH9_USB_EHCI1);
+ 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;
--
2.16.1
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
