7:16 a.m.
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(a)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(a)redhat.com>
a less reluctant one mean change to use the # define construct - I've
even attached a diff that worked for me.
John