<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Verified, This patch fixes the namespace issue while PR in guest with lun passed-through multipath device node.</div>
<div>
<div id="appendonsend"></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
Lin</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> libvir-list-bounces@redhat.com <libvir-list-bounces@redhat.com> on behalf of Michal Privoznik <mprivozn@redhat.com><br>
<b>Sent:</b> Friday, March 6, 2020 1:11 AM<br>
<b>To:</b> libvir-list@redhat.com <libvir-list@redhat.com><br>
<b>Subject:</b> [PATCH] qemu: Create multipath targets for PRs</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">If a disk has persistent reservations enabled, qemu-pr-helper<br>
might open not only /dev/mapper/control but also individual<br>
targets of the multipath device. We are already querying for them<br>
in CGroups, but now we have to create them in the namespace too.<br>
This was brought up in [1].<br>
<br>
1: <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1711045#c61">https://bugzilla.redhat.com/show_bug.cgi?id=1711045#c61</a><br>
<br>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com><br>
---<br>
src/qemu/qemu_domain.c | 64 ++++++++++------<br>
src/util/virdevmapper.h | 4 +-<br>
src/util/virutil.h | 2 +-<br>
tests/qemuhotplugmock.c | 75 +++++++++++++++++++<br>
tests/qemuhotplugtest.c | 13 ++++<br>
.../qemuhotplug-disk-scsi-multipath.xml | 8 ++<br>
...uhotplug-base-live+disk-scsi-multipath.xml | 62 +++++++++++++++<br>
7 files changed, 204 insertions(+), 24 deletions(-)<br>
create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-disk-scsi-multipath.xml<br>
create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi-multipath.xml<br>
<br>
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c<br>
index 33c2158eb5..c8e6be7fae 100644<br>
--- a/src/qemu/qemu_domain.c<br>
+++ b/src/qemu/qemu_domain.c<br>
@@ -62,6 +62,7 @@<br>
#include "virdomaincheckpointobjlist.h"<br>
#include "backup_conf.h"<br>
#include "virutil.h"<br>
+#include "virdevmapper.h"<br>
<br>
#ifdef __linux__<br>
# include <sys/sysmacros.h><br>
@@ -14574,6 +14575,9 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED,<br>
bool hasNVMe = false;<br>
<br>
for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {<br>
+ VIR_AUTOSTRINGLIST targetPaths = NULL;<br>
+ size_t i;<br>
+<br>
if (next->type == VIR_STORAGE_TYPE_NVME) {<br>
g_autofree char *nvmePath = NULL;<br>
<br>
@@ -14592,6 +14596,19 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED,<br>
<br>
if (qemuDomainCreateDevice(next->path, data, false) < 0)<br>
return -1;<br>
+<br>
+ if (virDevMapperGetTargets(next->path, &targetPaths) < 0 &&<br>
+ errno != ENOSYS && errno != EBADF) {<br>
+ virReportSystemError(errno,<br>
+ _("Unable to get devmapper targets for %s"),<br>
+ next->path);<br>
+ return -1;<br>
+ }<br>
+<br>
+ for (i = 0; targetPaths && targetPaths[i]; i++) {<br>
+ if (qemuDomainCreateDevice(targetPaths[i], data, false) < 0)<br>
+ return -1;<br>
+ }<br>
}<br>
}<br>
<br>
@@ -15607,21 +15624,19 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm,<br>
virStorageSourcePtr src)<br>
{<br>
virStorageSourcePtr next;<br>
- char **paths = NULL;<br>
+ VIR_AUTOSTRINGLIST paths = NULL;<br>
size_t npaths = 0;<br>
bool hasNVMe = false;<br>
- g_autofree char *dmPath = NULL;<br>
- g_autofree char *vfioPath = NULL;<br>
- int ret = -1;<br>
<br>
for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) {<br>
+ VIR_AUTOSTRINGLIST targetPaths = NULL;<br>
g_autofree char *tmpPath = NULL;<br>
<br>
if (next->type == VIR_STORAGE_TYPE_NVME) {<br>
hasNVMe = true;<br>
<br>
if (!(tmpPath = virPCIDeviceAddressGetIOMMUGroupDev(&next->nvme->pciAddr)))<br>
- goto cleanup;<br>
+ return -1;<br>
} else {<br>
if (virStorageSourceIsEmpty(next) ||<br>
!virStorageSourceIsLocalStorage(next)) {<br>
@@ -15632,30 +15647,35 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm,<br>
tmpPath = g_strdup(next->path);<br>
}<br>
<br>
- if (VIR_APPEND_ELEMENT(paths, npaths, tmpPath) < 0)<br>
- goto cleanup;<br>
+ if (virStringListAdd(&paths, tmpPath) < 0)<br>
+ return -1;<br>
+<br>
+ if (virDevMapperGetTargets(next->path, &targetPaths) < 0 &&<br>
+ errno != ENOSYS && errno != EBADF) {<br>
+ virReportSystemError(errno,<br>
+ _("Unable to get devmapper targets for %s"),<br>
+ next->path);<br>
+ return -1;<br>
+ }<br>
+<br>
+ if (virStringListMerge(&paths, &targetPaths) < 0)<br>
+ return -1;<br>
}<br>
<br>
/* qemu-pr-helper might require access to /dev/mapper/control. */<br>
- if (src->pr) {<br>
- dmPath = g_strdup(QEMU_DEVICE_MAPPER_CONTROL_PATH);<br>
- if (VIR_APPEND_ELEMENT_COPY(paths, npaths, dmPath) < 0)<br>
- goto cleanup;<br>
- }<br>
+ if (src->pr &&<br>
+ virStringListAdd(&paths, QEMU_DEVICE_MAPPER_CONTROL_PATH) < 0)<br>
+ return -1;<br>
<br>
- if (hasNVMe) {<br>
- vfioPath = g_strdup(QEMU_DEV_VFIO);<br>
- if (VIR_APPEND_ELEMENT(paths, npaths, vfioPath) < 0)<br>
- goto cleanup;<br>
- }<br>
+ if (hasNVMe &&<br>
+ virStringListAdd(&paths, QEMU_DEV_VFIO) < 0)<br>
+ return -1;<br>
<br>
+ npaths = virStringListLength((const char **) paths);<br>
if (qemuDomainNamespaceMknodPaths(vm, (const char **) paths, npaths) < 0)<br>
- goto cleanup;<br>
+ return -1;<br>
<br>
- ret = 0;<br>
- cleanup:<br>
- virStringListFreeCount(paths, npaths);<br>
- return ret;<br>
+ return 0;<br>
}<br>
<br>
<br>
diff --git a/src/util/virdevmapper.h b/src/util/virdevmapper.h<br>
index e576d2bf7e..87bbc63cfd 100644<br>
--- a/src/util/virdevmapper.h<br>
+++ b/src/util/virdevmapper.h<br>
@@ -20,6 +20,8 @@<br>
<br>
#pragma once<br>
<br>
+#include "internal.h"<br>
+<br>
int<br>
virDevMapperGetTargets(const char *path,<br>
- char ***devPaths);<br>
+ char ***devPaths) G_GNUC_NO_INLINE;<br>
diff --git a/src/util/virutil.h b/src/util/virutil.h<br>
index cc6b82a177..ee23f0c1f4 100644<br>
--- a/src/util/virutil.h<br>
+++ b/src/util/virutil.h<br>
@@ -120,7 +120,7 @@ bool virValidateWWN(const char *wwn);<br>
<br>
int virGetDeviceID(const char *path,<br>
int *maj,<br>
- int *min);<br>
+ int *min) G_GNUC_NO_INLINE;<br>
int virSetDeviceUnprivSGIO(const char *path,<br>
const char *sysfs_dir,<br>
int unpriv_sgio);<br>
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c<br>
index 43a9d79051..8e5b07788d 100644<br>
--- a/tests/qemuhotplugmock.c<br>
+++ b/tests/qemuhotplugmock.c<br>
@@ -19,7 +19,24 @@<br>
#include <config.h><br>
<br>
#include "qemu/qemu_hotplug.h"<br>
+#include "qemu/qemu_process.h"<br>
#include "conf/domain_conf.h"<br>
+#include "virdevmapper.h"<br>
+#include "virutil.h"<br>
+#include "virmock.h"<br>
+<br>
+static int (*real_virGetDeviceID)(const char *path, int *maj, int *min);<br>
+static bool (*real_virFileExists)(const char *path);<br>
+<br>
+static void<br>
+init_syms(void)<br>
+{<br>
+ if (real_virFileExists)<br>
+ return;<br>
+<br>
+ VIR_MOCK_REAL_INIT(virGetDeviceID);<br>
+ VIR_MOCK_REAL_INIT(virFileExists);<br>
+}<br>
<br>
unsigned long long<br>
qemuDomainGetUnplugTimeout(virDomainObjPtr vm G_GNUC_UNUSED)<br>
@@ -31,3 +48,61 @@ qemuDomainGetUnplugTimeout(virDomainObjPtr vm G_GNUC_UNUSED)<br>
return 200;<br>
return 100;<br>
}<br>
+<br>
+<br>
+int<br>
+virDevMapperGetTargets(const char *path,<br>
+ char ***devPaths)<br>
+{<br>
+ *devPaths = NULL;<br>
+<br>
+ if (STREQ(path, "/dev/mapper/virt")) {<br>
+ *devPaths = g_new(char *, 4);<br>
+ (*devPaths)[0] = g_strdup("/dev/block/8:0"); /* /dev/sda */<br>
+ (*devPaths)[1] = g_strdup("/dev/block/8:16"); /* /dev/sdb */<br>
+ (*devPaths)[2] = g_strdup("/dev/block/8:32"); /* /dev/sdc */<br>
+ (*devPaths)[3] = NULL;<br>
+ }<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+<br>
+int<br>
+virGetDeviceID(const char *path, int *maj, int *min)<br>
+{<br>
+ init_syms();<br>
+<br>
+ if (STREQ(path, "/dev/mapper/virt")) {<br>
+ *maj = 254;<br>
+ *min = 0;<br>
+ return 0;<br>
+ }<br>
+<br>
+ return real_virGetDeviceID(path, maj, min);<br>
+}<br>
+<br>
+<br>
+bool<br>
+virFileExists(const char *path)<br>
+{<br>
+ init_syms();<br>
+<br>
+ if (STREQ(path, "/dev/mapper/virt"))<br>
+ return true;<br>
+<br>
+ return real_virFileExists(path);<br>
+}<br>
+<br>
+<br>
+int<br>
+qemuProcessStartManagedPRDaemon(virDomainObjPtr vm G_GNUC_UNUSED)<br>
+{<br>
+ return 0;<br>
+}<br>
+<br>
+<br>
+void<br>
+qemuProcessKillManagedPRDaemon(virDomainObjPtr vm G_GNUC_UNUSED)<br>
+{<br>
+}<br>
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c<br>
index e008c1bf0d..8b411d63f0 100644<br>
--- a/tests/qemuhotplugtest.c<br>
+++ b/tests/qemuhotplugtest.c<br>
@@ -87,6 +87,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,<br>
virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VNC);<br>
virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE);<br>
virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE);<br>
+ virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER);<br>
+ virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SCSI_BLOCK);<br>
<br>
if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0)<br>
return -1;<br>
@@ -750,6 +752,17 @@ mymain(void)<br>
"device_del", QMP_DEVICE_DELETED("scsi3-0-5-6") QMP_OK,<br>
"human-monitor-command", HMP(""));<br>
<br>
+ DO_TEST_ATTACH("base-live", "disk-scsi-multipath", false, true,<br>
+ "object-add", QMP_OK,<br>
+ "human-monitor-command", HMP("OK\\r\\n"),<br>
+ "device_add", QMP_OK);<br>
+ DO_TEST_DETACH("base-live", "disk-scsi-multipath", true, true,<br>
+ "device_del", QMP_OK,<br>
+ "human-monitor-command", HMP(""));<br>
+ DO_TEST_DETACH("base-live", "disk-scsi-multipath", false, false,<br>
+ "device_del", QMP_DEVICE_DELETED("scsi0-0-0-0") QMP_OK,<br>
+ "human-monitor-command", HMP(""));<br>
+<br>
DO_TEST_ATTACH("base-live", "qemu-agent", false, true,<br>
"chardev-add", QMP_OK,<br>
"device_add", QMP_OK);<br>
diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-disk-scsi-multipath.xml b/tests/qemuhotplugtestdevices/qemuhotplug-disk-scsi-multipath.xml<br>
new file mode 100644<br>
index 0000000000..5a6f955284<br>
--- /dev/null<br>
+++ b/tests/qemuhotplugtestdevices/qemuhotplug-disk-scsi-multipath.xml<br>
@@ -0,0 +1,8 @@<br>
+<disk type='block' device='lun'><br>
+ <driver name='qemu' type='raw'/><br>
+ <source dev='/dev/mapper/virt'><br>
+ <reservations managed='yes'/><br>
+ </source><br>
+ <target dev='sda' bus='scsi'/><br>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/><br>
+</disk><br>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi-multipath.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi-multipath.xml<br>
new file mode 100644<br>
index 0000000000..40af064d10<br>
--- /dev/null<br>
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi-multipath.xml<br>
@@ -0,0 +1,62 @@<br>
+<domain type='kvm' id='7'><br>
+ <name>hotplug</name><br>
+ <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid><br>
+ <memory unit='KiB'>4194304</memory><br>
+ <currentMemory unit='KiB'>4194304</currentMemory><br>
+ <vcpu placement='static'>4</vcpu><br>
+ <os><br>
+ <type arch='x86_64' machine='pc'>hvm</type><br>
+ <boot dev='hd'/><br>
+ </os><br>
+ <features><br>
+ <acpi/><br>
+ <apic/><br>
+ <pae/><br>
+ </features><br>
+ <clock offset='utc'/><br>
+ <on_poweroff>destroy</on_poweroff><br>
+ <on_reboot>restart</on_reboot><br>
+ <on_crash>restart</on_crash><br>
+ <devices><br>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator><br>
+ <disk type='block' device='lun'><br>
+ <driver name='qemu' type='raw'/><br>
+ <source dev='/dev/mapper/virt'><br>
+ <reservations managed='yes'><br>
+ <source type='unix' path='/tmp/lib/domain-7-hotplug/pr-helper0.sock' mode='client'/><br>
+ </reservations><br>
+ </source><br>
+ <backingStore/><br>
+ <target dev='sda' bus='scsi'/><br>
+ <alias name='scsi0-0-0-0'/><br>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/><br>
+ </disk><br>
+ <controller type='usb' index='0'><br>
+ <alias name='usb'/><br>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/><br>
+ </controller><br>
+ <controller type='ide' index='0'><br>
+ <alias name='ide'/><br>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/><br>
+ </controller><br>
+ <controller type='scsi' index='0' model='virtio-scsi'><br>
+ <alias name='scsi0'/><br>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/><br>
+ </controller><br>
+ <controller type='pci' index='0' model='pci-root'><br>
+ <alias name='pci'/><br>
+ </controller><br>
+ <controller type='virtio-serial' index='0'><br>
+ <alias name='virtio-serial0'/><br>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/><br>
+ </controller><br>
+ <input type='mouse' bus='ps2'><br>
+ <alias name='input0'/><br>
+ </input><br>
+ <input type='keyboard' bus='ps2'><br>
+ <alias name='input1'/><br>
+ </input><br>
+ <memballoon model='none'/><br>
+ </devices><br>
+ <seclabel type='none' model='none'/><br>
+</domain><br>
-- <br>
2.24.1<br>
<br>
</div>
</span></font></div>
</div>
</body>
</html>