For command line we need two things:
1) -object pr-manager-helper,id=$alias,path=$socketPath
2) -drive file.pr-manager=$alias
In -object pr-manager-helper we tell qemu which socket to connect
to, then in -drive file-pr-manager we just reference the object
the drive in question should use.
For managed PR helper the alias is always "pr-helper0" and socket
path "${vm->priv->libDir}/pr-helper0.sock".
It was decided in reviews to previous versions of this patch that
it should not allocate memory in order to simplify code. This
promise is not fulfilled though. For instance,
qemuBuildPRManagerInfoProps() is an offender.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/libvirt_private.syms | 2 +
src/qemu/qemu_alias.c | 18 +++
src/qemu/qemu_alias.h | 4 +
src/qemu/qemu_command.c | 134 +++++++++++++++++++++
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, 247 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 27c5bb5bce..108b44f6f4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2799,7 +2799,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 b666f3715f..8b1b5934b1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1462,6 +1462,28 @@ 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 +1561,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;
@@ -9665,6 +9690,112 @@ 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)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ char *socketPath = NULL;
+ char *alias = NULL;
+ int ret = -1;
+
+ *propsret = NULL;
+ *aliasret = NULL;
+
+ if (!virStoragePRDefIsEnabled(disk->src->pr))
+ return 0;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("reservations not supported with this QEMU binary"));
+ return ret;
+ }
+
+ 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:
*
@@ -9833,6 +9964,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 0dc5a88e79..53827d5396 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11983,3 +11983,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 2c27dfb9f3..612d234113 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1000,4 +1000,7 @@ qemuDomainDiskCachemodeFlags(int cachemode,
bool *direct,
bool *noflush);
+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 d0050eca1f..e22dd727fa 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 09e9ae73f1..059214a517 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -384,6 +384,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 74d930ebe2..ad17a5c333 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2814,6 +2814,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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/23/2018 09:14 AM, Michal Privoznik wrote:
> For command line we need two things:
>
> 1) -object pr-manager-helper,id=$alias,path=$socketPath
> 2) -drive file.pr-manager=$alias
>
> In -object pr-manager-helper we tell qemu which socket to connect
> to, then in -drive file-pr-manager we just reference the object
> the drive in question should use.
>
> For managed PR helper the alias is always "pr-helper0" and socket
> path "${vm->priv->libDir}/pr-helper0.sock".
>
> It was decided in reviews to previous versions of this patch that
> it should not allocate memory in order to simplify code. This
> promise is not fulfilled though. For instance,
> qemuBuildPRManagerInfoProps() is an offender.
Last paragraph is irrelevant and partially incorrect trivia for above
the ---. I would assume everyone has received review comments that they
may not agree with, but that's what they are review comments. Voicing
frustration in a commit message is probably not a good thing. Good thing
I work at home with usually no one listening 0-).
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_alias.c | 18 +++
> src/qemu/qemu_alias.h | 4 +
> src/qemu/qemu_command.c | 134 +++++++++++++++++++++
> 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, 247 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 27c5bb5bce..108b44f6f4 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2799,7 +2799,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";
This works, IDC but you could have gone with
# define QEMU_DOMAIN_MANAGED_PR_ALIAS "pr-helper0"
in src/qemu/qemu_alias.h too and used it that way.
Ironically later on, we have:
#define QEMU_PR_HELPER "/usr/bin/qemu-pr-helper"
and use
VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0)
So why not use the same construct for this alias?
> +}
> +
> +
> +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);
s/* q/*q/
or as I suggest:
# define QEMU_DOMAIN_MANAGED_PR_ALIAS "pr-helper0"
> +
> +char *qemuDomainGetUnmanagedPRAlias(const virDomainDiskDef *disk);
> +
> #endif /* __QEMU_ALIAS_H__*/
[...]
Since the decision was made in other reviews to wait until the last
patch to turn on the capability in the caps*.xml files, the current
.args usage and test config using the DO_TEST macro will have to
suffice; however, when we get to that last patch this will need to
change in order to follow what is being requested of other upstream
posts now too.
With no changes to alias, a reluctant:
Reviewed-by: John Ferlan <jferlan@redhat.com>
a less reluctant one mean change to use the # define construct - I've
even attached a diff that worked for me.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/29/2018 02:16 PM, John Ferlan wrote:
>
>
> On 04/23/2018 09:14 AM, Michal Privoznik wrote:
>> For command line we need two things:
>>
>> 1) -object pr-manager-helper,id=$alias,path=$socketPath
>> 2) -drive file.pr-manager=$alias
>>
>> In -object pr-manager-helper we tell qemu which socket to connect
>> to, then in -drive file-pr-manager we just reference the object
>> the drive in question should use.
>>
>> For managed PR helper the alias is always "pr-helper0" and socket
>> path "${vm->priv->libDir}/pr-helper0.sock".
>>
>> It was decided in reviews to previous versions of this patch that
>> it should not allocate memory in order to simplify code. This
>> promise is not fulfilled though. For instance,
>> qemuBuildPRManagerInfoProps() is an offender.
>
> Last paragraph is irrelevant and partially incorrect trivia for above
> the ---. I would assume everyone has received review comments that they
> may not agree with, but that's what they are review comments. Voicing
> frustration in a commit message is probably not a good thing. Good thing
> I work at home with usually no one listening 0-).
>
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>> src/libvirt_private.syms | 2 +
>> src/qemu/qemu_alias.c | 18 +++
>> src/qemu/qemu_alias.h | 4 +
>> src/qemu/qemu_command.c | 134 +++++++++++++++++++++
>> 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, 247 insertions(+)
>> create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.args
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 27c5bb5bce..108b44f6f4 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -2799,7 +2799,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";
>
> This works, IDC but you could have gone with
>
> # define QEMU_DOMAIN_MANAGED_PR_ALIAS "pr-helper0"
>
> in src/qemu/qemu_alias.h too and used it that way.
I rather not. Everything else uses a function to get/set aliases. In
order to stay consistent I'd rather have this as a function.
>
>
> Ironically later on, we have:
>
> #define QEMU_PR_HELPER "/usr/bin/qemu-pr-helper"
Do we? Not in the code, but in config.h produced by configure. Well,
that's standardized way of defining values in configure. I'm not sure
that logic applies to the rest of our code.
>
> and use
>
> VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0)
>
> So why not use the same construct for this alias?
Because QEMU_PR_HELPER is detected at compile time (for the default, can
be overridden in qemu.conf later). There is no other way to do it. We
can't have configure generating a function like this:
qemuGetPrHelperPath() {
return "/usr/bin/qemu-pr-helper";
}
But we can have our own code do that, esp. if other functions from the
family (qemu alias handling functions that is) are functions indeed
instead of some #define-s.
>
>> +}
>> +
>> +
>> +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);
>
> s/* q/*q/
>
Oh, right.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.