Re: [libvirt] [PATCH v4 06/14] qemu_ns: Allow /dev/mapper/control for PR
On 04/10/2018 10:58 AM, Michal Privoznik wrote:
VIR_FREE(dmPath);
John
If qemu-pr-helper is compiled with multipath support the first
thing it does is opening /dev/mapper/control. Since we're going
s/opening/open
to be running it inside qemu namespace we need to create it
there. Unfortunately, we don't know if it was compiled with or
without multipath so we have to create it anyway.
Not sure this explains whether it's multipath or namespaces that's the
focus/cause of this patch. I thought multipath allows multiple ways to
address the same LUN and namespace provides a mechanism to "control"
device events and protections so that something like udev doesn't change
something behind our back. For PR it's a mechanism to allow certain
ioctl calls to be able to control ownership and/or write processing to
the LUN so that it's not possible to have two writers.
So does this patch add /dev/mapper/control to the namespace list because
that's what will "control" the ioctl's to the LUN used by PR ???
BTW: This might be the ugliest piece of code I've ever written
but @devMapperControl really needs to be type of char * otherwise
some crazy check in VIR_APPEND_ELEMENT fails.
This hunk probably doesn't belong in a commit message... and well
doesn't necessarily provide the confidence level for acceptance ;-)!
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0856f04406..6fe4eb57e1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -108,6 +108,7 @@ VIR_ENUM_IMPL(qemuDomainNamespace, QEMU_DOMAIN_NS_LAST,
#define PROC_MOUNTS "/proc/mounts"
#define DEVPREFIX "/dev/"
#define DEV_VFIO "/dev/vfio/vfio"
+#define DEVICE_MAPPER_CONTROL_PATH "/dev/mapper/control"
struct _qemuDomainLogContext {
@@ -10269,6 +10270,11 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg
ATTRIBUTE_UNUSED,
goto cleanup;
}
+ /* qemu-pr-helper might require access to /dev/mapper/control. */
+ if (virStoragePRDefIsEnabled(disk->src->pr) &&
+ qemuDomainCreateDevice(DEVICE_MAPPER_CONTROL_PATH, data, true) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
VIR_FREE(dst);
@@ -11281,6 +11287,9 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm,
const char **paths = NULL;
size_t npaths = 0;
int ret = -1;
+ /* This is very nasty but we need it to work around some
+ * stupid checks in VIR_APPEND_ELEMENT macro. */
+ char *devMapperControl = (char *) DEVICE_MAPPER_CONTROL_PATH;
alternatively
char *dmPath = NULL;
if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
return 0;
@@ -11296,6 +11305,11 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm,
goto cleanup;
}
+ /* qemu-pr-helper might require access to /dev/mapper/control. */
+ if (virStoragePRDefIsEnabled(src->pr) &&
+ VIR_APPEND_ELEMENT_COPY(paths, npaths, devMapperControl) < 0)
Alternatively:
VIR_STRDUP(dmPath, DEVICE_MAPPER_CONTROL_PATH) < 0) &&
VIR_APPEND_ELEMENT_COPY(paths, npaths, dmPath)
+ goto cleanup;
+
if (qemuDomainNamespaceMknodPaths(vm, paths, npaths) < 0)
return -1;
BTW: Existing, but this leaks @paths