[libvirt] [PATCH v1 00/19] Implementation of QEMU vhost-scsi
This patch series provides a libvirt implementation of the vhost-scsi interface in QEMU. As near as I can see, this was discussed upstream in July 2014[1], and ended in a desire to replace a vhost-scsi controller in favor of a hostdev element instead[2]. There is no capability check in this series for vhost-scsi in the underlying QEMU. Using a recent QEMU built with --disable-vhost-scsi fails with "not a valid device model name." Host Filesystem Example: # ls /sys/kernel/config/target/vhost/ discovery_auth naa.5001405df3e54061 version # ls /sys/kernel/config/target/vhost/naa.5001405df3e54061/tpgt_1/lun/ lun_0 QEMU Example (snippet): -device vhost-scsi-ccw,wwpn=naa.5001405df3e54061,devno=fe.0.1000 Libvirt Example (snippet): <hostdev mode='subsystem' type='scsi'> <source protocol='vhost' wwpn='naa.5001405df3e54061'/> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1000'/> </hostdev> Guest Viewpoint: # lsscsi [1:0:1:0] disk LIO-ORG disk0 4.0 /dev/sda # dmesg | grep 1: [ 6.065735] scsi host1: Virtio SCSI HBA [ 6.093892] scsi 1:0:1:0: Direct-Access LIO-ORG disk0 4.0 PQ: 0 ANSI: 5 [ 6.313615] sd 1:0:1:0: Attached scsi generic sg0 type 0 [ 6.314981] sd 1:0:1:0: [sda] 29360128 512-byte logical blocks: (15.0 GB/14.0 GiB) [ 6.317290] sd 1:0:1:0: [sda] Write Protect is off [ 6.317566] sd 1:0:1:0: [sda] Mode Sense: 43 00 10 08 [ 6.317853] sd 1:0:1:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA [ 6.352722] sd 1:0:1:0: [sda] Attached SCSI disk [1] http://www.redhat.com/archives/libvir-list/2014-July/msg01235.html [2] http://www.redhat.com/archives/libvir-list/2014-July/msg01390.html Eric Farman (19): conf: Add definitions for "vhost" protocol in hostdev tags util: Allow a vhost protocol for scsi hostdev security: Allow a vhost protocol for scsi hostdev qemu: Refactor qemuIsSharedHostdev for readability/extendability qemu: Allow a vhost protocol for scsi hostdev conf: Parse vhost-scsi XML tag conf: Prevent use of shareable on vhost-scsi devices qemu: Introduce vhost-scsi capability qemu: Refactor qemuBuildHostdevCommandLine for non-vhost devices qemu: Add vhost-scsi string for -device parameter qemu: Add vhost-scsi to hostdev schema docs: Add vhost-scsi to documentation qemu: Refactor hotplug in preparation for vhost-scsi qemu: Allow hotplug of vhost-scsi device conf: Create vhost-scsi hostdev protocol in dumpxml conf: Set up vhost-scsi hostdev to self-close source tag tests: Introduce basic vhost-scsi test conf: Do not create a virtio-scsi controller for vhost-scsi hostdev qemu: Allow the specification of a vhost-scsi devno docs/formatdomain.html.in | 16 +++ docs/schemas/domaincommon.rng | 12 +++ src/conf/domain_conf.c | 94 ++++++++++++++-- src/conf/domain_conf.h | 8 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_cgroup.c | 5 + src/qemu/qemu_command.c | 120 +++++++++++++++++++-- src/qemu/qemu_command.h | 6 ++ src/qemu/qemu_conf.c | 21 +++- src/qemu/qemu_domain_address.c | 10 ++ src/qemu/qemu_hotplug.c | 76 +++++++++---- src/qemu/qemu_monitor.c | 21 ++++ src/qemu/qemu_monitor.h | 4 + src/security/security_apparmor.c | 5 +- src/security/security_dac.c | 10 +- src/security/security_selinux.c | 10 +- src/util/virhostdev.c | 13 ++- src/util/virscsi.c | 26 +++++ src/util/virscsi.h | 1 + tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + .../qemuxml2argv-hostdev-scsi-vhost-scsi.args | 24 +++++ .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml | 33 ++++++ tests/qemuxml2argvmock.c | 12 +++ tests/qemuxml2argvtest.c | 3 + 35 files changed, 486 insertions(+), 58 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml -- 1.9.1
Add some vhost definitions, in preparation for a new protocol that will allow vhost-scsi devices to be specified on a hostdev tag. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6dfcf81..9681d6c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -655,7 +655,8 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysPCIBackend, VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIProtocol, VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST, "adapter", - "iscsi") + "iscsi", + "vhost") VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST, "storage", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 00041c9..1956bdc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -313,6 +313,7 @@ VIR_ENUM_DECL(virDomainHostdevSubsysPCIBackend) typedef enum { VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE, VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI, + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST, VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST, } virDomainHostdevSCSIProtocolType; @@ -356,6 +357,12 @@ struct _virDomainHostdevSubsysSCSIiSCSI { virStorageAuthDefPtr auth; }; +typedef struct _virDomainHostdevSubsysSCSIVhost virDomainHostdevSubsysSCSIVhost; +typedef virDomainHostdevSubsysSCSIVhost *virDomainHostdevSubsysSCSIVhostPtr; +struct _virDomainHostdevSubsysSCSIVhost { + char *wwpn; +}; + typedef struct _virDomainHostdevSubsysSCSI virDomainHostdevSubsysSCSI; typedef virDomainHostdevSubsysSCSI *virDomainHostdevSubsysSCSIPtr; struct _virDomainHostdevSubsysSCSI { @@ -365,6 +372,7 @@ struct _virDomainHostdevSubsysSCSI { union { virDomainHostdevSubsysSCSIHost host; virDomainHostdevSubsysSCSIiSCSI iscsi; + virDomainHostdevSubsysSCSIVhost vhost; } u; }; -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Add some vhost definitions, in preparation for a new protocol that will allow vhost-scsi devices to be specified on a hostdev tag.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6dfcf81..9681d6c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -655,7 +655,8 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysPCIBackend, VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIProtocol, VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST, "adapter", - "iscsi") + "iscsi", + "vhost")
VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST, "storage", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 00041c9..1956bdc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -313,6 +313,7 @@ VIR_ENUM_DECL(virDomainHostdevSubsysPCIBackend) typedef enum { VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE, VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI, + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST,
VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST, } virDomainHostdevSCSIProtocolType; @@ -356,6 +357,12 @@ struct _virDomainHostdevSubsysSCSIiSCSI { virStorageAuthDefPtr auth; };
+typedef struct _virDomainHostdevSubsysSCSIVhost virDomainHostdevSubsysSCSIVhost; +typedef virDomainHostdevSubsysSCSIVhost *virDomainHostdevSubsysSCSIVhostPtr; +struct _virDomainHostdevSubsysSCSIVhost { + char *wwpn; +}; + typedef struct _virDomainHostdevSubsysSCSI virDomainHostdevSubsysSCSI; typedef virDomainHostdevSubsysSCSI *virDomainHostdevSubsysSCSIPtr; struct _virDomainHostdevSubsysSCSI { @@ -365,6 +372,7 @@ struct _virDomainHostdevSubsysSCSI { union { virDomainHostdevSubsysSCSIHost host; virDomainHostdevSubsysSCSIiSCSI iscsi; + virDomainHostdevSubsysSCSIVhost vhost; } u; };
I think this change should be squashed into 06/19. I understand you need these declarations here, so you can do the opposite and squash 06/19 into this one. Michal
Make sure that the new vhost protocol does not drive the existing virtio SCSI code. Also, do a little minor formatting in virHostdevReAttachSCSIDevices, to match the similar functions elsewhere. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/util/virhostdev.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9b5ca6f..7e3bf34 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1106,6 +1106,8 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr, if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { continue; /* Not supported for iSCSI */ + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + continue; /* Not supported for vhost */ } else { if (virHostdevUpdateActiveSCSIHostDevices(mgr, hostdev, scsisrc, drv_name, dom_name) < 0) @@ -1405,6 +1407,8 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr mgr, if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { continue; /* Not supported for iSCSI */ + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + continue; /* Not supported for vhost */ } else { if (virHostdevPrepareSCSIHostDevices(hostdev, scsisrc, list) < 0) goto cleanup; @@ -1595,11 +1599,14 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr mgr, hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) continue; - if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) - continue; /* Not supported for iSCSI */ - else + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + continue; /* Not supported for iSCSI */ + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + continue; /* Not supported for vhost */ + } else { virHostdevReAttachSCSIHostDevices(mgr, hostdev, scsisrc, drv_name, dom_name); + } } virObjectUnlock(mgr->activeSCSIHostdevs); } -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Make sure that the new vhost protocol does not drive the existing virtio SCSI code.
Also, do a little minor formatting in virHostdevReAttachSCSIDevices, to match the similar functions elsewhere.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/util/virhostdev.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9b5ca6f..7e3bf34 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1106,6 +1106,8 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr,
if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { continue; /* Not supported for iSCSI */ + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + continue; /* Not supported for vhost */ } else { if (virHostdevUpdateActiveSCSIHostDevices(mgr, hostdev, scsisrc, drv_name, dom_name) < 0)
I wonder if we should turn those if - else if - else into: switch((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { ... } Not a show stopper, could be done in a follow up patch.
@@ -1405,6 +1407,8 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr mgr,
if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { continue; /* Not supported for iSCSI */ + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + continue; /* Not supported for vhost */ } else { if (virHostdevPrepareSCSIHostDevices(hostdev, scsisrc, list) < 0) goto cleanup; @@ -1595,11 +1599,14 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr mgr, hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) continue;
- if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) - continue; /* Not supported for iSCSI */ - else + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + continue; /* Not supported for iSCSI */ + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + continue; /* Not supported for vhost */ + } else { virHostdevReAttachSCSIHostDevices(mgr, hostdev, scsisrc, drv_name, dom_name); + } } virObjectUnlock(mgr->activeSCSIHostdevs); }
Michal
Make sure that the new vhost protocol does not drive the existing virtio SCSI code. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/security/security_apparmor.c | 5 +++-- src/security/security_dac.c | 10 ++++++---- src/security/security_selinux.c | 10 ++++++---- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index af2b639..e3fcc58 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -842,10 +842,11 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, return 0; /* Like AppArmorRestoreSecurityImageLabel() for a networked disk, - * do nothing for an iSCSI hostdev + * do nothing for an iSCSI or vhost-scsi hostdev */ if (dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI || + scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST)) return 0; if (profile_loaded(secdef->imagelabel) < 0) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 442ce70..75b5819 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -601,10 +601,11 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr, return 0; /* Like virSecurityDACSetImageLabel() for a networked disk, - * do nothing for an iSCSI hostdev + * do nothing for an iSCSI or vhost-scsi hostdev */ if (dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI || + scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST)) return 0; cbdata.manager = mgr; @@ -742,10 +743,11 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr, return 0; /* Like virSecurityDACRestoreImageLabelInt() for a networked disk, - * do nothing for an iSCSI hostdev + * do nothing for an iSCSI or vhost-scsi hostdev */ if (dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI || + scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST)) return 0; switch ((virDomainHostdevSubsysType) dev->source.subsys.type) { diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 4be946d..8632d0f 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1430,10 +1430,11 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr, int ret = -1; /* Like virSecuritySELinuxSetImageLabelInternal() for a networked - * disk, do nothing for an iSCSI hostdev + * disk, do nothing for an iSCSI or vhost-scsi hostdev */ if (dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI || + scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST)) return 0; switch (dev->source.subsys.type) { @@ -1634,10 +1635,11 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr, int ret = -1; /* Like virSecuritySELinuxRestoreImageLabelInt() for a networked - * disk, do nothing for an iSCSI hostdev + * disk, do nothing for an iSCSI or vhost-scsi hostdev */ if (dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI || + scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST)) return 0; switch (dev->source.subsys.type) { -- 1.9.1
We're going to make some changes here, and the boolean logic does not lend itself to be easily expanded. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_conf.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fa9d65e..64f85dd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1096,11 +1096,20 @@ qemuAddSharedDisk(virQEMUDriverPtr driver, static bool qemuIsSharedHostdev(virDomainHostdevDefPtr hostdev) { - return (hostdev->shareable && - (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - hostdev->source.subsys.u.scsi.protocol != - VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)); + if (!hostdev->shareable) + return false; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return false; + + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) + return false; + + if (hostdev->source.subsys.u.scsi.protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + return false; + + return true; } -- 1.9.1
Make sure that the new vhost protocol does not drive the existing virtio SCSI code. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_cgroup.c | 5 +++++ src/qemu/qemu_conf.c | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2dca874..f14cfe8 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -356,6 +356,11 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm, */ VIR_DEBUG("Not updating cgroups for hostdev iSCSI path '%s'", iscsisrc->path); + } else if (scsisrc->protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; + VIR_DEBUG("Not updating cgroups for hostdev vhost path '%s'", + vhostsrc->wwpn); } else { virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 64f85dd..1175d08 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1106,7 +1106,9 @@ qemuIsSharedHostdev(virDomainHostdevDefPtr hostdev) return false; if (hostdev->source.subsys.u.scsi.protocol == - VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI || + hostdev->source.subsys.u.scsi.protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) return false; return true; -- 1.9.1
Add code that will permit a hostdev tag to specify a vhost protocol, and parse the associated XML within it. But don't do anything with that information just yet. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9681d6c..d6b9c42 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2225,6 +2225,14 @@ virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc iscsisrc->auth = NULL; } +static void +virDomainHostdevSubsysSCSIVhostClear(virDomainHostdevSubsysSCSIVhostPtr vhostsrc) +{ + if (!vhostsrc) + return; + VIR_FREE(vhostsrc->wwpn); +} + void virDomainHostdevDefClear(virDomainHostdevDefPtr def) { if (!def) @@ -2261,6 +2269,9 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { virDomainHostdevSubsysSCSIiSCSIClear(&scsisrc->u.iscsi); + } else if (scsisrc->protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + virDomainHostdevSubsysSCSIVhostClear(&scsisrc->u.vhost); } else { VIR_FREE(scsisrc->u.host.adapter); } @@ -5943,6 +5954,31 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, } static int +virDomainHostdevSubsysSCSIVhostDefParseXML(xmlNodePtr sourcenode, + virDomainHostdevSubsysSCSIPtr def) +{ + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &def->u.vhost; + + if (!(vhostsrc->wwpn = virXMLPropString(sourcenode, "wwpn"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing vhost-scsi hostdev source path name")); + goto cleanup; + } + + if (!STRPREFIX(vhostsrc->wwpn, "naa.") || + strlen(vhostsrc->wwpn) != strlen("naa.") + 16) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("malformed 'wwpn' value")); + goto cleanup; + } + + return 0; + + cleanup: + VIR_FREE(vhostsrc->wwpn); + return -1; +} + +static int virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode, virDomainHostdevSubsysSCSIPtr scsisrc) { @@ -5962,6 +5998,8 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode, if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) ret = virDomainHostdevSubsysSCSIiSCSIDefParseXML(sourcenode, scsisrc); + else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) + ret = virDomainHostdevSubsysSCSIVhostDefParseXML(sourcenode, scsisrc); else ret = virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, scsisrc); -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Add code that will permit a hostdev tag to specify a vhost protocol, and parse the associated XML within it. But don't do anything with that information just yet.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
Unfortunately, even if this patch is good I cannot ack it because every parser/formatter extension must go hand in hand with RNG schema & documentation update. The code looks good though. Michal
Starting two guests using the same vhost-scsi target results in an error when opening /dev/vhost-scsi. Since "shareable" suggests there's a way around this, let's disallow this tag when using vhost-scsi protocol for our hostdev device. Suggested-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d6b9c42..b67d0ea 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12840,6 +12840,14 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, def->readonly = true; if (virXPathBoolean("boolean(./shareable)", ctxt)) def->shareable = true; + if (def->source.subsys.u.scsi.protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST && + def->shareable) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Devices connected with vhost-scsi protocol " + "cannot be marked shareable")); + goto error; + } break; } } -- 1.9.1
Do all the stuff for the vhost-scsi capability in QEMU, so it's in place for our checks later. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + 12 files changed, 13 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f600ce9..c39bc22 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -339,6 +339,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "tls-creds-x509", /* 230 */ "display", "intel-iommu", + "vhost-scsi", ); @@ -1566,6 +1567,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "pxb-pcie", QEMU_CAPS_DEVICE_PXB_PCIE }, { "tls-creds-x509", QEMU_CAPS_OBJECT_TLS_CREDS_X509 }, { "intel-iommu", QEMU_CAPS_DEVICE_INTEL_IOMMU }, + { "vhost-scsi", QEMU_CAPS_DEVICE_VHOST_SCSI }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index ca84f27..c16b7e3 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -372,6 +372,7 @@ typedef enum { QEMU_CAPS_OBJECT_TLS_CREDS_X509, /* -object tls-creds-x509 */ QEMU_CAPS_DISPLAY, /* -display */ QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */ + QEMU_CAPS_DEVICE_VHOST_SCSI, /* -device vhost-scsi-{ccw,pci} */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml index 0d048da..eeaf3d5 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml @@ -143,6 +143,7 @@ <flag name='device-tray-moved-event'/> <flag name='nec-usb-xhci-ports'/> <flag name='display'/> + <flag name='vhost-scsi'/> <version>1005003</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml index a6d4561..b72c4df 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml @@ -148,6 +148,7 @@ <flag name='device-tray-moved-event'/> <flag name='nec-usb-xhci-ports'/> <flag name='display'/> + <flag name='vhost-scsi'/> <version>1006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml index f756a41..74a4292 100644 --- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml @@ -150,6 +150,7 @@ <flag name='device-tray-moved-event'/> <flag name='nec-usb-xhci-ports'/> <flag name='display'/> + <flag name='vhost-scsi'/> <version>1007000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml index a77ad9e..cc829da 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml @@ -165,6 +165,7 @@ <flag name='name-guest'/> <flag name='drive-detect-zeroes'/> <flag name='display'/> + <flag name='vhost-scsi'/> <version>2001001</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index 80085d5..c16f4b8 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -183,6 +183,7 @@ <flag name='drive-detect-zeroes'/> <flag name='display'/> <flag name='intel-iommu'/> + <flag name='vhost-scsi'/> <version>2004000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index fad3291..b02613e 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -188,6 +188,7 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='intel-iommu'/> + <flag name='vhost-scsi'/> <version>2005000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index 4ed88bc..d7be3f0 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -157,6 +157,7 @@ <flag name='drive-detect-zeroes'/> <flag name='tls-creds-x509'/> <flag name='display'/> + <flag name='vhost-scsi'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index 024596d..8121dd3 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -157,6 +157,7 @@ <flag name='drive-detect-zeroes'/> <flag name='tls-creds-x509'/> <flag name='display'/> + <flag name='vhost-scsi'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index e66433c..ad99886 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -151,6 +151,7 @@ <flag name='drive-detect-zeroes'/> <flag name='tls-creds-x509'/> <flag name='display'/> + <flag name='vhost-scsi'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 653ec75..8e0a4bf 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -194,6 +194,7 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='intel-iommu'/> + <flag name='vhost-scsi'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Do all the stuff for the vhost-scsi capability in QEMU, so it's in place for our checks later.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + 12 files changed, 13 insertions(+)
Here the qemucapabilitiestest fails, but it is because yet another caps have been introduced meanwhile. I've fixed it. Michal
Let's adjust some of the indentation in the command line code for the upcoming vhost-scsi support, so we can keep existing code separate from the new code that we'll add. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4558b9f..dd15ff8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4954,18 +4954,20 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { - char *drvstr; + if (hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + char *drvstr; - virCommandAddArg(cmd, "-drive"); - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev))) - return -1; - virCommandAddArg(cmd, drvstr); - VIR_FREE(drvstr); + virCommandAddArg(cmd, "-drive"); + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev))) + return -1; + virCommandAddArg(cmd, drvstr); + VIR_FREE(drvstr); - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev, - qemuCaps))) - return -1; + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev, + qemuCaps))) + return -1; + } virCommandAddArg(cmd, devstr); VIR_FREE(devstr); } else { -- 1.9.1
Open /dev/vhost-scsi, and record the resulting file descriptor, so that the guest has access to the host device outside of the libvirt daemon. Pass this information, along with data parsed from the XML file, to build a device string for the qemu command line. That device string will be for either a vhost-scsi-ccw device in the case of an s390 machine, or vhost-scsi-pci for any others. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 5 ++++ src/util/virscsi.c | 26 +++++++++++++++++++ src/util/virscsi.h | 1 + 5 files changed, 99 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9396c4e..b91c7a8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2253,6 +2253,7 @@ virSCSIDeviceListNew; virSCSIDeviceListSteal; virSCSIDeviceNew; virSCSIDeviceSetUsedBy; +virSCSIOpenVhost; # util/virseclabel.h diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dd15ff8..31b30a4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4533,6 +4533,67 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) } char * +qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, + virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps, + virCommandPtr cmd) +{ + size_t i; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; + int *vhostfd = NULL; + size_t vhostfdSize = 1; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support vhost-scsi devices")); + goto cleanup; + } + + if (ARCH_IS_S390(def->os.arch)) + virBufferAddLit(&buf, "vhost-scsi-ccw"); + else + virBufferAddLit(&buf, "vhost-scsi-pci"); + + virBufferAsprintf(&buf, ",wwpn=%s", vhostsrc->wwpn); + + if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0) + goto cleanup; + + memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); + + if (virSCSIOpenVhost(vhostfd, &vhostfdSize) < 0) + goto cleanup; + + for (i = 0; i < vhostfdSize; i++) { + if (cmd) { + virCommandPassFD(cmd, vhostfd[i], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + } + } + + if (vhostfdSize == 1) { + virBufferAsprintf(&buf, ",vhostfd=%d", vhostfd[0]); + } else { + /* FIXME if 'vhostfds' became a valid vhost-scsi property in QEMU */ + goto cleanup; + } + + virBufferAsprintf(&buf, ",id=%s", dev->info->alias); + + VIR_FREE(vhostfd); + return virBufferContentAndReset(&buf); + + cleanup: + for (i = 0; vhostfd && i < vhostfdSize; i++) + VIR_FORCE_CLOSE(vhostfd[i]); + VIR_FREE(vhostfd); + virBufferFreeAndReset(&buf); + return NULL; +} + +char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4954,7 +5015,11 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { - if (hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + if (hostdev->source.subsys.u.scsi.protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildSCSIVhostHostdevDevStr(def, hostdev, qemuCaps, cmd))) + return -1; + } else { char *drvstr; virCommandAddArg(cmd, "-drive"); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index c4d0567..5c2dcb0 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -159,6 +159,11 @@ char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev); char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); +char * +qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, + virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps, + virCommandPtr cmd); char *qemuBuildRedirdevDevStr(const virDomainDef *def, virDomainRedirdevDefPtr dev, diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 4843367..290b692 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -105,6 +105,32 @@ virSCSIDeviceGetAdapterId(const char *adapter, return -1; } +int +virSCSIOpenVhost(int *vhostfd, + size_t *vhostfdSize) +{ + size_t i; + + for (i = 0; i < *vhostfdSize; i++) { + vhostfd[i] = open("/dev/vhost-scsi", O_RDWR); + + if (vhostfd[i] < 0) { + virReportSystemError(errno, "%s", + _("vhost-scsi was requested for an interface, " + "but is unavailable")); + goto error; + } + } + + return 0; + + error: + while (i--) + VIR_FORCE_CLOSE(vhostfd[i]); + + return -1; +} + char * virSCSIDeviceGetSgName(const char *sysfs_prefix, const char *adapter, diff --git a/src/util/virscsi.h b/src/util/virscsi.h index df40d7f..cb37da8 100644 --- a/src/util/virscsi.h +++ b/src/util/virscsi.h @@ -33,6 +33,7 @@ typedef virSCSIDevice *virSCSIDevicePtr; typedef struct _virSCSIDeviceList virSCSIDeviceList; typedef virSCSIDeviceList *virSCSIDeviceListPtr; +int virSCSIOpenVhost(int *vhostfd, size_t *vhostfdSize); char *virSCSIDeviceGetSgName(const char *sysfs_prefix, const char *adapter, unsigned int bus, -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Open /dev/vhost-scsi, and record the resulting file descriptor, so that the guest has access to the host device outside of the libvirt daemon. Pass this information, along with data parsed from the XML file, to build a device string for the qemu command line. That device string will be for either a vhost-scsi-ccw device in the case of an s390 machine, or vhost-scsi-pci for any others.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 5 ++++ src/util/virscsi.c | 26 +++++++++++++++++++ src/util/virscsi.h | 1 + 5 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9396c4e..b91c7a8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2253,6 +2253,7 @@ virSCSIDeviceListNew; virSCSIDeviceListSteal; virSCSIDeviceNew; virSCSIDeviceSetUsedBy; +virSCSIOpenVhost;
# util/virseclabel.h diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dd15ff8..31b30a4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4533,6 +4533,67 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) }
char * +qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, + virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps, + virCommandPtr cmd) +{ + size_t i; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; + int *vhostfd = NULL; + size_t vhostfdSize = 1; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support vhost-scsi devices")); + goto cleanup; + } + + if (ARCH_IS_S390(def->os.arch)) + virBufferAddLit(&buf, "vhost-scsi-ccw"); + else + virBufferAddLit(&buf, "vhost-scsi-pci"); + + virBufferAsprintf(&buf, ",wwpn=%s", vhostsrc->wwpn); + + if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0) + goto cleanup; + + memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); + + if (virSCSIOpenVhost(vhostfd, &vhostfdSize) < 0) + goto cleanup; + + for (i = 0; i < vhostfdSize; i++) { + if (cmd) {
If cmd == NULL here we are in way bigger trouble. Just drop the check.
+ virCommandPassFD(cmd, vhostfd[i], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + } + } + + if (vhostfdSize == 1) { + virBufferAsprintf(&buf, ",vhostfd=%d", vhostfd[0]); + } else { + /* FIXME if 'vhostfds' became a valid vhost-scsi property in QEMU */ + goto cleanup;
Instead of failing here silently, we should report an error. I'll leave the error message up to you ;-)
+ } + + virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
Here we should check if there has been an error while constructing @buf. For instance an OOM might have occurred. if (virBufferCheckError(&buf) < 0) goto cleanup;
+ + VIR_FREE(vhostfd); + return virBufferContentAndReset(&buf); + + cleanup: + for (i = 0; vhostfd && i < vhostfdSize; i++) + VIR_FORCE_CLOSE(vhostfd[i]); + VIR_FREE(vhostfd); + virBufferFreeAndReset(&buf); + return NULL; +}
The rest looks okay. Perhaps, if you want you can introduce virSCSIOpenVhost() in a separate patch. But only if you want. I don't care that much. Michal
On Mon, Jul 25, 2016 at 04:48:11PM -0400, Eric Farman wrote:
Open /dev/vhost-scsi, and record the resulting file descriptor, so that the guest has access to the host device outside of the libvirt daemon. Pass this information, along with data parsed from the XML file, to build a device string for the qemu command line. That device string will be for either a vhost-scsi-ccw device in the case of an s390 machine, or vhost-scsi-pci for any others.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 5 ++++ src/util/virscsi.c | 26 +++++++++++++++++++ src/util/virscsi.h | 1 + 5 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9396c4e..b91c7a8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2253,6 +2253,7 @@ virSCSIDeviceListNew; virSCSIDeviceListSteal; virSCSIDeviceNew; virSCSIDeviceSetUsedBy; +virSCSIOpenVhost;
# util/virseclabel.h diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dd15ff8..31b30a4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4533,6 +4533,67 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) }
char * +qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, + virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps, + virCommandPtr cmd) +{ + size_t i; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; + int *vhostfd = NULL; + size_t vhostfdSize = 1; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support vhost-scsi devices")); + goto cleanup; + } + + if (ARCH_IS_S390(def->os.arch)) + virBufferAddLit(&buf, "vhost-scsi-ccw"); + else + virBufferAddLit(&buf, "vhost-scsi-pci"); + + virBufferAsprintf(&buf, ",wwpn=%s", vhostsrc->wwpn); + + if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0) + goto cleanup; + + memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); + + if (virSCSIOpenVhost(vhostfd, &vhostfdSize) < 0) + goto cleanup; + + for (i = 0; i < vhostfdSize; i++) { + if (cmd) { + virCommandPassFD(cmd, vhostfd[i], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + } + } + + if (vhostfdSize == 1) { + virBufferAsprintf(&buf, ",vhostfd=%d", vhostfd[0]); + } else { + /* FIXME if 'vhostfds' became a valid vhost-scsi property in QEMU */ + goto cleanup; + } + + virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
This is missing setting device address info - ie a call to qemuBuildDeviceAddressStr() Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Validate an XML schema that contains the vhost protocol in its hostdev tag. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/schemas/domaincommon.rng | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 741f268..5c691dc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3991,6 +3991,18 @@ </optional> </interleave> </group> + <group> <!-- vhost_scsi adapter --> + <attribute name="protocol"> + <choice> + <value>vhost</value> <!-- vhost, required --> + </choice> + </attribute> + <attribute name="wwpn"> + <data type="string"> + <param name="pattern">(naa\.)[0-9a-fA-F]{16}</param> + </data> + </attribute> + </group> </choice> </element> </define> -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Validate an XML schema that contains the vhost protocol in its hostdev tag.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/schemas/domaincommon.rng | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 741f268..5c691dc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3991,6 +3991,18 @@ </optional> </interleave> </group> + <group> <!-- vhost_scsi adapter --> + <attribute name="protocol"> + <choice> + <value>vhost</value> <!-- vhost, required --> + </choice> + </attribute> + <attribute name="wwpn"> + <data type="string"> + <param name="pattern">(naa\.)[0-9a-fA-F]{16}</param> + </data> + </attribute> + </group> </choice> </element> </define>
AAh, this is the change I was missing in 06/19. Squash it there please. The reasoning behind is to make life easier for downstream maintainers It's very easy to forget to backport one patch - and now that we have schema validation enabled before actually parsing the XML downstream packages might be broken. Moreover, it is okay if the feature can be configured in domain XML but yet is declined on driver level. Michal
Add an example of the vhost-scsi hostdev tag, and its associated definitions, to the domain XML helpfile. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 59a8bb9..2f2bbd0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3547,6 +3547,17 @@ </devices> ...</pre> + <p>or:</p> + +<pre> + ... + <devices> + <hostdev mode='subsystem' type='scsi'> + <source protocol='vhost' wwpn='naa.50014057667280d8'/> + </hostdev> + </devices> + ...</pre> + <dl> <dt><code>hostdev</code></dt> <dd>The <code>hostdev</code> element is the main container for describing @@ -3646,6 +3657,11 @@ device using the same <code>name</code> attribute and optionally using the <code>auth</code> element to provide the authentication credentials to the iSCSI server. + <!-- FIXME as releases come and go --> + <span class="since">Since 2.1.0</span>, the <code>protocol</code> + attribute may be set to "vhost" and accept a <code>wwpn</code> + attribute that is the vhost_scsi wwpn (16 hexadecimal digits with + a prefix of "naa.") established in the host configfs. </p> </dd> </dl> -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Add an example of the vhost-scsi hostdev tag, and its associated definitions, to the domain XML helpfile.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 59a8bb9..2f2bbd0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3547,6 +3547,17 @@ </devices> ...</pre>
+ <p>or:</p> + +<pre> + ... + <devices> + <hostdev mode='subsystem' type='scsi'> + <source protocol='vhost' wwpn='naa.50014057667280d8'/> + </hostdev> + </devices> + ...</pre> + <dl> <dt><code>hostdev</code></dt> <dd>The <code>hostdev</code> element is the main container for describing @@ -3646,6 +3657,11 @@ device using the same <code>name</code> attribute and optionally using the <code>auth</code> element to provide the authentication credentials to the iSCSI server. + <!-- FIXME as releases come and go --> + <span class="since">Since 2.1.0</span>, the <code>protocol</code> + attribute may be set to "vhost" and accept a <code>wwpn</code> + attribute that is the vhost_scsi wwpn (16 hexadecimal digits with + a prefix of "naa.") established in the host configfs. </p> </dd> </dl>
Yet again, this should be squashed into 06/19. Michal
Let's adjust some of the indentation in the hotplug code for the upcoming vhost-scsi support, so we can keep existing code separate from the new code that we'll add. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4e4bf82..02f7017 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2048,23 +2048,25 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0) goto cleanup; - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev))) - goto cleanup; + if (hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev))) + goto cleanup; - if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, hostdev, priv->qemuCaps))) - goto cleanup; + if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, hostdev, priv->qemuCaps))) + goto cleanup; - if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) - goto cleanup; + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) + goto cleanup; - qemuDomainObjEnterMonitor(driver, vm); + qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorAddDrive(priv->mon, drvstr) < 0) - goto exit_monitor; - driveAdded = true; + if (qemuMonitorAddDrive(priv->mon, drvstr) < 0) + goto exit_monitor; + driveAdded = true; - if (qemuMonitorAddDevice(priv->mon, devstr) < 0) - goto exit_monitor; + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) + goto exit_monitor; + } if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; -- 1.9.1
Adjust the device string that is built for vhost-scsi devices so that it can be invoked from hotplug.
From the QEMU command line, the file descriptors are expect to be numeric only. However, for hotplug, the file descriptors are expected to begin with at least one alphabetic character else this error occurs:
# virsh attach-device guest_0001 ~/vhost.xml error: Failed to attach device from /root/vhost.xml error: internal error: unable to execute QEMU command 'getfd': Parameter 'fdname' expects a name not starting with a digit We also close the file descriptor in this case, so that shutting down the guest cleans up the host cgroup entries and allows future guests to use vhost-scsi devices. (Otherwise the guest will silently end.) Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 34 ++++++++++++++++++++++++++++---- src/qemu/qemu_command.h | 3 ++- src/qemu/qemu_hotplug.c | 52 ++++++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_monitor.c | 21 ++++++++++++++++++++ src/qemu/qemu_monitor.h | 4 ++++ 5 files changed, 98 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 31b30a4..6677c28 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4536,12 +4536,14 @@ char * qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps, - virCommandPtr cmd) + virCommandPtr cmd, + qemuMonitorPtr mon) { size_t i; virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; + char **vhostfdName = NULL; int *vhostfd = NULL; size_t vhostfdSize = 1; @@ -4551,6 +4553,9 @@ qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, goto cleanup; } + if ((cmd && mon) || (!cmd && !mon)) + goto cleanup; + if (ARCH_IS_S390(def->os.arch)) virBufferAddLit(&buf, "vhost-scsi-ccw"); else @@ -4563,18 +4568,28 @@ qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize); + if (VIR_ALLOC_N(vhostfdName, vhostfdSize) < 0) + goto cleanup; + if (virSCSIOpenVhost(vhostfd, &vhostfdSize) < 0) goto cleanup; + if (mon) { + if (qemuMonitorAttachVhostSCSI(mon, vhostfd, vhostfdName, vhostfdSize) < 0) + goto cleanup; + } + for (i = 0; i < vhostfdSize; i++) { if (cmd) { virCommandPassFD(cmd, vhostfd[i], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) + goto cleanup; } } if (vhostfdSize == 1) { - virBufferAsprintf(&buf, ",vhostfd=%d", vhostfd[0]); + virBufferAsprintf(&buf, ",vhostfd=%s", vhostfdName[0]); } else { /* FIXME if 'vhostfds' became a valid vhost-scsi property in QEMU */ goto cleanup; @@ -4582,13 +4597,24 @@ qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, virBufferAsprintf(&buf, ",id=%s", dev->info->alias); + for (i = 0; vhostfd && i < vhostfdSize; i++) { + if (mon) + VIR_FORCE_CLOSE(vhostfd[i]); + if (vhostfdName) + VIR_FREE(vhostfdName[i]); + } VIR_FREE(vhostfd); + VIR_FREE(vhostfdName); return virBufferContentAndReset(&buf); cleanup: - for (i = 0; vhostfd && i < vhostfdSize; i++) + for (i = 0; vhostfd && i < vhostfdSize; i++) { VIR_FORCE_CLOSE(vhostfd[i]); + if (vhostfdName) + VIR_FREE(vhostfdName[i]); + } VIR_FREE(vhostfd); + VIR_FREE(vhostfdName); virBufferFreeAndReset(&buf); return NULL; } @@ -5017,7 +5043,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { if (hostdev->source.subsys.u.scsi.protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildSCSIVhostHostdevDevStr(def, hostdev, qemuCaps, cmd))) + if (!(devstr = qemuBuildSCSIVhostHostdevDevStr(def, hostdev, qemuCaps, cmd, NULL))) return -1; } else { char *drvstr; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 5c2dcb0..d5a6256 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -163,7 +163,8 @@ char * qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps, - virCommandPtr cmd); + virCommandPtr cmd, + qemuMonitorPtr mon); char *qemuBuildRedirdevDevStr(const virDomainDef *def, virDomainRedirdevDefPtr dev, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 02f7017..77e436c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2003,16 +2003,29 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, return -1; } - /* Let's make sure the disk has a controller defined and loaded before - * trying to add it. The controller used by the disk must exist before a - * qemu command line string is generated. - * - * Ensure that the given controller and all controllers with a smaller index - * exist; there must not be any missing index in between. - */ - for (i = 0; i <= hostdev->info->addr.drive.controller; i++) { - if (!qemuDomainFindOrCreateSCSIDiskController(driver, vm, i)) - return -1; + if (hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + /* Let's make sure the disk has a controller defined and loaded before + * trying to add it. The controller used by the disk must exist before a + * qemu command line string is generated. + * + * Ensure that the given controller and all controllers with a smaller index + * exist; there must not be any missing index in between. + */ + for (i = 0; i <= hostdev->info->addr.drive.controller; i++) { + if (!qemuDomainFindOrCreateSCSIDiskController(driver, vm, i)) + return -1; + } + } else { + if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + if (qemuDomainMachineIsS390CCW(vm->def) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) + hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; + } + if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (virDomainCCWAddressAssign(hostdev->info, priv->ccwaddrs, + !hostdev->info->addr.ccw.assigned) < 0) + goto cleanup; + } } if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name, @@ -2023,6 +2036,11 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to prepare scsi hostdev for iSCSI: %s"), iscsisrc->path); + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to prepare scsi hostdev for vhost: %s"), + vhostsrc->wwpn); } else { virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2048,7 +2066,19 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0) goto cleanup; - if (hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + if (hostdev->source.subsys.u.scsi.protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + qemuDomainObjEnterMonitor(driver, vm); + if (!(devstr = qemuBuildSCSIVhostHostdevDevStr(vm->def, hostdev, priv->qemuCaps, NULL, priv->mon))) + goto cleanup; + + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) + goto cleanup; + + if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0) { + ignore_value(qemuDomainObjExitMonitor(driver, vm) < 0); + goto cleanup; + } + } else { if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev))) goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 098e654..51c3531 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2689,6 +2689,27 @@ qemuMonitorGetChardevInfo(qemuMonitorPtr mon, } +int +qemuMonitorAttachVhostSCSI(qemuMonitorPtr mon, + int *vhostfd, + char **vhostfdName, + int vhostfdSize) +{ + size_t i; + + QEMU_CHECK_MONITOR(mon); + + for (i = 0; i < vhostfdSize; i++) { + if (virAsprintf(&vhostfdName[i], "vhost-%d", vhostfd[i]) < 0) + return -1; + if (qemuMonitorSendFileHandle(mon, vhostfdName[i], vhostfd[i]) < 0) + return -1; + } + + return 0; +} + + /** * qemuMonitorDriveDel: * @mon: monitor object diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index cb4cca8..67050c5 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -679,6 +679,10 @@ void qemuMonitorChardevInfoFree(void *data, const void *name); int qemuMonitorGetChardevInfo(qemuMonitorPtr mon, virHashTablePtr *retinfo); +int qemuMonitorAttachVhostSCSI(qemuMonitorPtr mon, + int *vhostfd, + char **vhostfdName, + int vhostfdSize); int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, virPCIDeviceAddress *guestAddr); -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Adjust the device string that is built for vhost-scsi devices so that it can be invoked from hotplug.
From the QEMU command line, the file descriptors are expect to be numeric only. However, for hotplug, the file descriptors are expected to begin with at least one alphabetic character else this error occurs:
# virsh attach-device guest_0001 ~/vhost.xml error: Failed to attach device from /root/vhost.xml error: internal error: unable to execute QEMU command 'getfd': Parameter 'fdname' expects a name not starting with a digit
We also close the file descriptor in this case, so that shutting down the guest cleans up the host cgroup entries and allows future guests to use vhost-scsi devices. (Otherwise the guest will silently end.)
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 34 ++++++++++++++++++++++++++++---- src/qemu/qemu_command.h | 3 ++- src/qemu/qemu_hotplug.c | 52 ++++++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_monitor.c | 21 ++++++++++++++++++++ src/qemu/qemu_monitor.h | 4 ++++ 5 files changed, 98 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 31b30a4..6677c28 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4536,12 +4536,14 @@ char * qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps, - virCommandPtr cmd) + virCommandPtr cmd, + qemuMonitorPtr mon) { size_t i; virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; + char **vhostfdName = NULL; int *vhostfd = NULL; size_t vhostfdSize = 1;
@@ -4551,6 +4553,9 @@ qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, goto cleanup; }
+ if ((cmd && mon) || (!cmd && !mon)) + goto cleanup; + if (ARCH_IS_S390(def->os.arch)) virBufferAddLit(&buf, "vhost-scsi-ccw"); else @@ -4563,18 +4568,28 @@ qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def,
memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize);
+ if (VIR_ALLOC_N(vhostfdName, vhostfdSize) < 0) + goto cleanup; + if (virSCSIOpenVhost(vhostfd, &vhostfdSize) < 0) goto cleanup;
+ if (mon) { + if (qemuMonitorAttachVhostSCSI(mon, vhostfd, vhostfdName, vhostfdSize) < 0) + goto cleanup; + } + for (i = 0; i < vhostfdSize; i++) { if (cmd) { virCommandPassFD(cmd, vhostfd[i], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) + goto cleanup; } }
if (vhostfdSize == 1) { - virBufferAsprintf(&buf, ",vhostfd=%d", vhostfd[0]); + virBufferAsprintf(&buf, ",vhostfd=%s", vhostfdName[0]); } else { /* FIXME if 'vhostfds' became a valid vhost-scsi property in QEMU */ goto cleanup; @@ -4582,13 +4597,24 @@ qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def,
virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
+ for (i = 0; vhostfd && i < vhostfdSize; i++) { + if (mon) + VIR_FORCE_CLOSE(vhostfd[i]); + if (vhostfdName) + VIR_FREE(vhostfdName[i]); + } VIR_FREE(vhostfd); + VIR_FREE(vhostfdName); return virBufferContentAndReset(&buf);
cleanup: - for (i = 0; vhostfd && i < vhostfdSize; i++) + for (i = 0; vhostfd && i < vhostfdSize; i++) { VIR_FORCE_CLOSE(vhostfd[i]); + if (vhostfdName) + VIR_FREE(vhostfdName[i]); + } VIR_FREE(vhostfd); + VIR_FREE(vhostfdName); virBufferFreeAndReset(&buf); return NULL; }
While now I understand why you needed the check for cmd == NULL that I've pointed out in 10/19, this just doesn't feel right. You have to consider situation where this function succeeds (and sends some FDs to qemu prematurely, but then something fails later in the process leaving all the FDs in qemu which will never ever close them. I'd suggest writing a separate function just to handle the hotplug instead of bending this one like this. You can take a look at qemuDomainAttachNetDevice() if you want. The FDs are allocated but not passed to QEMU until the last moment.
@@ -5017,7 +5043,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { if (hostdev->source.subsys.u.scsi.protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildSCSIVhostHostdevDevStr(def, hostdev, qemuCaps, cmd))) + if (!(devstr = qemuBuildSCSIVhostHostdevDevStr(def, hostdev, qemuCaps, cmd, NULL))) return -1; } else { char *drvstr; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 5c2dcb0..d5a6256 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -163,7 +163,8 @@ char * qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps, - virCommandPtr cmd); + virCommandPtr cmd, + qemuMonitorPtr mon);
char *qemuBuildRedirdevDevStr(const virDomainDef *def, virDomainRedirdevDefPtr dev, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 02f7017..77e436c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2003,16 +2003,29 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, return -1; }
- /* Let's make sure the disk has a controller defined and loaded before - * trying to add it. The controller used by the disk must exist before a - * qemu command line string is generated. - * - * Ensure that the given controller and all controllers with a smaller index - * exist; there must not be any missing index in between. - */ - for (i = 0; i <= hostdev->info->addr.drive.controller; i++) { - if (!qemuDomainFindOrCreateSCSIDiskController(driver, vm, i)) - return -1; + if (hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + /* Let's make sure the disk has a controller defined and loaded before + * trying to add it. The controller used by the disk must exist before a + * qemu command line string is generated. + * + * Ensure that the given controller and all controllers with a smaller index + * exist; there must not be any missing index in between. + */ + for (i = 0; i <= hostdev->info->addr.drive.controller; i++) { + if (!qemuDomainFindOrCreateSCSIDiskController(driver, vm, i)) + return -1; + } + } else { + if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + if (qemuDomainMachineIsS390CCW(vm->def) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) + hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; + } + if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (virDomainCCWAddressAssign(hostdev->info, priv->ccwaddrs,
This won't fly. Use 'git grep -C10 virDomainCCWAddressAssign' to see how this function is used.
+ !hostdev->info->addr.ccw.assigned) < 0) + goto cleanup; + } }
if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name, @@ -2023,6 +2036,11 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to prepare scsi hostdev for iSCSI: %s"), iscsisrc->path); + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to prepare scsi hostdev for vhost: %s"), + vhostsrc->wwpn); } else { virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2048,7 +2066,19 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0) goto cleanup;
- if (hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + if (hostdev->source.subsys.u.scsi.protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + qemuDomainObjEnterMonitor(driver, vm); + if (!(devstr = qemuBuildSCSIVhostHostdevDevStr(vm->def, hostdev, priv->qemuCaps, NULL, priv->mon))) + goto cleanup; + + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) + goto cleanup; + + if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0) { + ignore_value(qemuDomainObjExitMonitor(driver, vm) < 0); + goto cleanup; + } + } else { if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev))) goto cleanup;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 098e654..51c3531 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2689,6 +2689,27 @@ qemuMonitorGetChardevInfo(qemuMonitorPtr mon, }
+int +qemuMonitorAttachVhostSCSI(qemuMonitorPtr mon, + int *vhostfd, + char **vhostfdName, + int vhostfdSize) +{ + size_t i; + + QEMU_CHECK_MONITOR(mon); + + for (i = 0; i < vhostfdSize; i++) { + if (virAsprintf(&vhostfdName[i], "vhost-%d", vhostfd[i]) < 0) + return -1; + if (qemuMonitorSendFileHandle(mon, vhostfdName[i], vhostfd[i]) < 0) + return -1; + } + + return 0; +}
This function will disappear once the code is reworked following my suggestions above.
+ + /** * qemuMonitorDriveDel: * @mon: monitor object diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index cb4cca8..67050c5 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -679,6 +679,10 @@ void qemuMonitorChardevInfoFree(void *data, const void *name); int qemuMonitorGetChardevInfo(qemuMonitorPtr mon, virHashTablePtr *retinfo);
+int qemuMonitorAttachVhostSCSI(qemuMonitorPtr mon, + int *vhostfd, + char **vhostfdName, + int vhostfdSize); int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, virPCIDeviceAddress *guestAddr);
Michal
When issuing virsh dumpxml and parsing a hostdev element, the vhost-scsi protocol needs to be handled separately from the existing options, to create valid XML. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b67d0ea..79087b2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20441,6 +20441,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost; if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && pcisrc->backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { @@ -20470,13 +20471,22 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virBufferAddLit(buf, " missing='yes'"); } - if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { const char *protocol = virDomainHostdevSubsysSCSIProtocolTypeToString(scsisrc->protocol); - virBufferAsprintf(buf, " protocol='%s' name='%s'", - protocol, iscsisrc->path); + switch (scsisrc->protocol) { + case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI: + virBufferAsprintf(buf, " protocol='%s' name='%s'", + protocol, iscsisrc->path); + break; + case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST: + virBufferAsprintf(buf, " protocol='%s' wwpn='%s'", + protocol, vhostsrc->wwpn); + break; + default: + break; + } } virBufferAddLit(buf, ">\n"); @@ -20522,6 +20532,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virBufferEscapeString(buf, " name='%s'", iscsisrc->hosts[0].name); virBufferEscapeString(buf, " port='%s'", iscsisrc->hosts[0].port); virBufferAddLit(buf, "/>\n"); + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + /* Nothing to do here */ } else { virBufferAsprintf(buf, "<adapter name='%s'/>\n", scsihostsrc->adapter); -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
When issuing virsh dumpxml and parsing a hostdev element, the vhost-scsi protocol needs to be handled separately from the existing options, to create valid XML.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
Yet again, this should go into very same patch that changes the parser.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b67d0ea..79087b2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20441,6 +20441,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; + virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost;
if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && pcisrc->backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { @@ -20470,13 +20471,22 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virBufferAddLit(buf, " missing='yes'"); }
- if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { const char *protocol = virDomainHostdevSubsysSCSIProtocolTypeToString(scsisrc->protocol);
- virBufferAsprintf(buf, " protocol='%s' name='%s'", - protocol, iscsisrc->path); + switch (scsisrc->protocol) {
The idea of switch() is that compiler will check for all the missing enum members. For some reasons, we store @protocol as int, therefore you should type cast this to virDomainHostdevSCSIProtocolType and drop the 'default' label. Next time somebody is introducing new SCSI protocol, they will thank you because compiler will identify majority of the places they need too look at.
+ case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI: + virBufferAsprintf(buf, " protocol='%s' name='%s'", + protocol, iscsisrc->path); + break; + case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST: + virBufferAsprintf(buf, " protocol='%s' wwpn='%s'", + protocol, vhostsrc->wwpn); + break; + default: + break; + } }
virBufferAddLit(buf, ">\n"); @@ -20522,6 +20532,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virBufferEscapeString(buf, " name='%s'", iscsisrc->hosts[0].name); virBufferEscapeString(buf, " port='%s'", iscsisrc->hosts[0].port); virBufferAddLit(buf, "/>\n"); + } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + /* Nothing to do here */ } else { virBufferAsprintf(buf, "<adapter name='%s'/>\n", scsihostsrc->adapter);
Michal
Since the vhost-scsi protocol for hostdev tags has no child elements within its source tag, we can use a self-closing source tag to make the resulting XML a little cleaner. Suggested-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 79087b2..8d6d208 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20436,6 +20436,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, unsigned int flags, bool includeTypeInAddr) { + bool closedSource = false; virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; @@ -20481,7 +20482,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, protocol, iscsisrc->path); break; case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST: - virBufferAsprintf(buf, " protocol='%s' wwpn='%s'", + closedSource = true; + virBufferAsprintf(buf, " protocol='%s' wwpn='%s'/", protocol, vhostsrc->wwpn); break; default: @@ -20533,7 +20535,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virBufferEscapeString(buf, " port='%s'", iscsisrc->hosts[0].port); virBufferAddLit(buf, "/>\n"); } else if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { - /* Nothing to do here */ + /* Nothing to do here; closedSource already set to true */ } else { virBufferAsprintf(buf, "<adapter name='%s'/>\n", scsihostsrc->adapter); @@ -20559,7 +20561,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, } virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</source>\n"); + if (!closedSource) + virBufferAddLit(buf, "</source>\n"); return 0; } -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
Since the vhost-scsi protocol for hostdev tags has no child elements within its source tag, we can use a self-closing source tag to make the resulting XML a little cleaner.
Suggested-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Squash this one into the previous one. Michal
The qemuxml2argv test was cloned from hostdev-scsi-virtio-scsi Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemuxml2argv-hostdev-scsi-vhost-scsi.args | 24 ++++++++++++++++ .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml | 33 ++++++++++++++++++++++ tests/qemuxml2argvmock.c | 12 ++++++++ tests/qemuxml2argvtest.c | 3 ++ 4 files changed, 72 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args new file mode 100644 index 0000000..a2c7c18 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest2 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml new file mode 100644 index 0000000..3fc4ef0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <hostdev mode='subsystem' type='scsi'> + <source protocol='vhost' wwpn='naa.5123456789abcde0'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 78a224b..568dcc0 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -107,6 +107,18 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix ATTRIBUTE_UNUSED, } int +virSCSIOpenVhost(int *vhostfd, + size_t *vhostfdSize) +{ + size_t i; + + for (i = 0; i < *vhostfdSize; i++) + vhostfd[i] = STDERR_FILENO + 1 + i; + + return 0; +} + +int virNetDevTapCreate(char **ifname, const char *tunpath ATTRIBUTE_UNUSED, int *tapfd, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 356f843..6ec4a78 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1749,6 +1749,9 @@ mymain(void) DO_TEST("hostdev-scsi-virtio-iscsi-auth", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_SCSI_GENERIC); + DO_TEST("hostdev-scsi-vhost-scsi", + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI, + QEMU_CAPS_DEVICE_SCSI_GENERIC); DO_TEST("mlock-on", QEMU_CAPS_MLOCK); DO_TEST_FAILURE("mlock-on", NONE); -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
The qemuxml2argv test was cloned from hostdev-scsi-virtio-scsi
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemuxml2argv-hostdev-scsi-vhost-scsi.args | 24 ++++++++++++++++ .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml | 33 ++++++++++++++++++++++ tests/qemuxml2argvmock.c | 12 ++++++++ tests/qemuxml2argvtest.c | 3 ++ 4 files changed, 72 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args new file mode 100644 index 0000000..a2c7c18 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest2 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \
Because this patch has been sitting on the list for too long, this part of command line is now being generated slightly differently. Otherwise looking good.
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml new file mode 100644 index 0000000..3fc4ef0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <hostdev mode='subsystem' type='scsi'> + <source protocol='vhost' wwpn='naa.5123456789abcde0'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 78a224b..568dcc0 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -107,6 +107,18 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix ATTRIBUTE_UNUSED, }
int +virSCSIOpenVhost(int *vhostfd, + size_t *vhostfdSize) +{ + size_t i; + + for (i = 0; i < *vhostfdSize; i++) + vhostfd[i] = STDERR_FILENO + 1 + i; + + return 0; +}
Good. Very good :) Michal
On Tue, Aug 09, 2016 at 02:13:29PM +0200, Michal Privoznik wrote:
On 25.07.2016 22:48, Eric Farman wrote:
The qemuxml2argv test was cloned from hostdev-scsi-virtio-scsi
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemuxml2argv-hostdev-scsi-vhost-scsi.args | 24 ++++++++++++++++ .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml | 33 ++++++++++++++++++++++ tests/qemuxml2argvmock.c | 12 ++++++++ tests/qemuxml2argvtest.c | 3 ++ 4 files changed, 72 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args new file mode 100644 index 0000000..a2c7c18 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest2 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \
Because this patch has been sitting on the list for too long, this part of command line is now being generated slightly differently. Otherwise looking good.
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml new file mode 100644 index 0000000..3fc4ef0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <hostdev mode='subsystem' type='scsi'> + <source protocol='vhost' wwpn='naa.5123456789abcde0'/> + </hostdev>
I'm not sure this syntax really makes sense. IIRC, currently <hostdev type="scsi"> is used to passthrough an individual SCSI LUNs from the host to the guest. With vhost-scsi, IIUC, you are passing through an entire (virtual) SCSI HBA to the guest. So I think it better for us to not reuse type="scsi" for that purpose. Instead it feels like we should be adding a type="scsi_host" for passthrough of entire HBAs Obviously this has major implications for almost all patches in this series Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/09/2016 08:22 AM, Daniel P. Berrange wrote:
On Tue, Aug 09, 2016 at 02:13:29PM +0200, Michal Privoznik wrote:
On 25.07.2016 22:48, Eric Farman wrote:
The qemuxml2argv test was cloned from hostdev-scsi-virtio-scsi
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemuxml2argv-hostdev-scsi-vhost-scsi.args | 24 ++++++++++++++++ .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml | 33 ++++++++++++++++++++++ tests/qemuxml2argvmock.c | 12 ++++++++ tests/qemuxml2argvtest.c | 3 ++ 4 files changed, 72 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args new file mode 100644 index 0000000..a2c7c18 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest2 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ Because this patch has been sitting on the list for too long, this part of command line is now being generated slightly differently. Otherwise looking good.
Fair enough, that's probably a common thread throughout your comments. Easy to fixup.
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml new file mode 100644 index 0000000..3fc4ef0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <hostdev mode='subsystem' type='scsi'> + <source protocol='vhost' wwpn='naa.5123456789abcde0'/> + </hostdev>
I'm not sure this syntax really makes sense.
IIRC, currently <hostdev type="scsi"> is used to passthrough an individual SCSI LUNs from the host to the guest.
With vhost-scsi, IIUC, you are passing through an entire (virtual) SCSI HBA to the guest. So I think it better for us to not reuse type="scsi" for that purpose. Instead it feels like we should be adding a type="scsi_host" for passthrough of entire HBAs
Would that cause confusion amongst an existing type='scsi' hostdev, since a "scsi_hostX" turns up in the source tag? Example from the libvirt doc: <devices> <hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'> <source> <adapter name='scsi_host0'/> <address bus='0' target='0' unit='0'/> </source> <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </hostdev> </devices> Could make it be "type='vhost_scsi'" or something else, if that clarifies things. (Question from my own ignorance; does an iSCSI target with more than one LUN behind it get rejected if specified here, instead of using a <pool> tag?)
Obviously this has major implications for almost all patches in this series
Splitting this out as a new hostdev type would mean it's not being shoehorned into the existing SCSI parts. That would probably make the entire series less unwieldy by dropping the separate protocol that exists herein. - Eric
Regards, Daniel
On Tue, Aug 09, 2016 at 10:44:44AM -0400, Eric Farman wrote:
On 08/09/2016 08:22 AM, Daniel P. Berrange wrote:
On Tue, Aug 09, 2016 at 02:13:29PM +0200, Michal Privoznik wrote:
On 25.07.2016 22:48, Eric Farman wrote:
The qemuxml2argv test was cloned from hostdev-scsi-virtio-scsi
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemuxml2argv-hostdev-scsi-vhost-scsi.args | 24 ++++++++++++++++ .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml | 33 ++++++++++++++++++++++ tests/qemuxml2argvmock.c | 12 ++++++++ tests/qemuxml2argvtest.c | 3 ++ 4 files changed, 72 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args new file mode 100644 index 0000000..a2c7c18 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest2 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ Because this patch has been sitting on the list for too long, this part of command line is now being generated slightly differently. Otherwise looking good.
Fair enough, that's probably a common thread throughout your comments. Easy to fixup.
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml new file mode 100644 index 0000000..3fc4ef0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <hostdev mode='subsystem' type='scsi'> + <source protocol='vhost' wwpn='naa.5123456789abcde0'/> + </hostdev>
I'm not sure this syntax really makes sense.
IIRC, currently <hostdev type="scsi"> is used to passthrough an individual SCSI LUNs from the host to the guest.
With vhost-scsi, IIUC, you are passing through an entire (virtual) SCSI HBA to the guest. So I think it better for us to not reuse type="scsi" for that purpose. Instead it feels like we should be adding a type="scsi_host" for passthrough of entire HBAs
Would that cause confusion amongst an existing type='scsi' hostdev, since a "scsi_hostX" turns up in the source tag? Example from the libvirt doc:
<devices> <hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'> <source> <adapter name='scsi_host0'/> <address bus='0' target='0' unit='0'/> </source> <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </hostdev> </devices>
Could make it be "type='vhost_scsi'" or something else, if that clarifies things.
No, it should be type=scsi_host, so that in the future if we want to pass through entire SCSI or iSCSI HBAs without vhost-scsi, then we can do it.
(Question from my own ignorance; does an iSCSI target with more than one LUN behind it get rejected if specified here, instead of using a <pool> tag?)
The IQN name has to include the LUN in it - if that's left out, QEMU would complain that no LUN was specified.
Obviously this has major implications for almost all patches in this series
Splitting this out as a new hostdev type would mean it's not being shoehorned into the existing SCSI parts. That would probably make the entire series less unwieldy by dropping the separate protocol that exists herein.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/09/2016 10:54 AM, Daniel P. Berrange wrote:
On Tue, Aug 09, 2016 at 10:44:44AM -0400, Eric Farman wrote:
On 08/09/2016 08:22 AM, Daniel P. Berrange wrote:
+ <hostdev mode='subsystem' type='scsi'> + <source protocol='vhost' wwpn='naa.5123456789abcde0'/> + </hostdev> I'm not sure this syntax really makes sense.
IIRC, currently <hostdev type="scsi"> is used to passthrough an individual SCSI LUNs from the host to the guest.
With vhost-scsi, IIUC, you are passing through an entire (virtual) SCSI HBA to the guest. So I think it better for us to not reuse type="scsi" for that purpose. Instead it feels like we should be adding a type="scsi_host" for passthrough of entire HBAs Would that cause confusion amongst an existing type='scsi' hostdev, since a "scsi_hostX" turns up in the source tag? Example from the libvirt doc:
<devices> <hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'> <source> <adapter name='scsi_host0'/> <address bus='0' target='0' unit='0'/> </source> <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </hostdev> </devices>
Could make it be "type='vhost_scsi'" or something else, if that clarifies things.
No, it should be type=scsi_host, so that in the future if we want to pass through entire SCSI or iSCSI HBAs without vhost-scsi, then we can do it.
Ok. I'll keep the "type='vhost'" in the protocol to distinguish that in the future, rather than dropping it as an implicit "if hostdev type='scsi_host' then it's a SCSI HBA with vhost-scsi"
(Question from my own ignorance; does an iSCSI target with more than one LUN behind it get rejected if specified here, instead of using a <pool> tag?) The IQN name has to include the LUN in it - if that's left out, QEMU would complain that no LUN was specified.
Thanks for the clarification. Haven't played with that configuration yet.
Obviously this has major implications for almost all patches in this series Splitting this out as a new hostdev type would mean it's not being shoehorned into the existing SCSI parts. That would probably make the entire series less unwieldy by dropping the separate protocol that exists herein. Regards, Daniel
The idea of vhost-scsi is to have a controller-less host device that carries all LUNs, so (silently) creating a virtio-scsi controller is not necessary here. Let's get adjust the comparison so we don't accidentally make one. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8d6d208..1768292 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15401,6 +15401,7 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) hostdev = def->hostdevs[i]; if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + hostdev->source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST && (int)hostdev->info->addr.drive.controller > maxController) { maxController = hostdev->info->addr.drive.controller; } -- 1.9.1
In the case of other SCSI host devices, the device that is created within the guest will be within the usual SCSI namespace (e.g., host:bus:target:lun). But for vhost-scsi, the actual naming structure will come from the host configuration, and will be invisible to both QEMU and Libvirt. So specifying one with an <address type='drive' ...> tag (as is often done for virtio-scsi) doesn't make sense in this case, as it will be ignored. Nevertheless, we need something to identify our vhost-scsi device, so for that we'll drop back to the devno mapping that we use for other disks. (This option exists only for vhost-scsi-ccw, but not vhost-scsi-pci. Hrm...) This becomes <address type='ccw' ...> in the case of s390 systems, and puts the device number on the resulting QEMU command line. Thus, existing device number conflict detection can be used across other devices that may or may not be specified in the guest XML. If one is not specified, we need to be sure to allocate one so that we can avoid polluting the device numbers with silently-created entries. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 19 +++++++++++++++++-- src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_domain_address.c | 10 ++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1768292..ae070ec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4067,6 +4067,12 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, size_t i; int ret; + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + hostdev->source.subsys.u.scsi.protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST) { + return 0; + } + if (xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_WIDE_SCSI) max_unit = SCSI_WIDE_BUS_MAX_CONT_UNIT; else @@ -12829,8 +12835,17 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - if (def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { + if (def->source.subsys.u.scsi.protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST && + def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("vhost-scsi device must use 'ccw' address type")); + goto error; + } else if (def->source.subsys.u.scsi.protocol != + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST && + def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { virReportError(VIR_ERR_XML_ERROR, "%s", _("SCSI host device must use 'drive' " "address type")); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6677c28..ca800c0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4597,6 +4597,13 @@ qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def, virBufferAsprintf(&buf, ",id=%s", dev->info->alias); + /* vhost-scsi-ccw has a devno parameter */ + if (dev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) + virBufferAsprintf(&buf, ",devno=%x.%x.%04x", + dev->info->addr.ccw.cssid, + dev->info->addr.ccw.ssid, + dev->info->addr.ccw.devno); + for (i = 0; vhostfd && i < vhostfdSize; i++) { if (mon) VIR_FORCE_CLOSE(vhostfd[i]); diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 787b357..238efd5 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -317,6 +317,16 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, def->controllers[i]->info.type = type; } + for (i = 0; i < def->nhostdevs; i++) { + if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + def->hostdevs[i]->source.subsys.type == + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + def->hostdevs[i]->source.subsys.u.scsi.protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_VHOST && + def->hostdevs[i]->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->hostdevs[i]->info->type = type; + } + if (def->memballoon && def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO && def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) -- 1.9.1
On 25.07.2016 22:48, Eric Farman wrote:
In the case of other SCSI host devices, the device that is created within the guest will be within the usual SCSI namespace (e.g., host:bus:target:lun). But for vhost-scsi, the actual naming structure will come from the host configuration, and will be invisible to both QEMU and Libvirt. So specifying one with an <address type='drive' ...> tag (as is often done for virtio-scsi) doesn't make sense in this case, as it will be ignored.
Nevertheless, we need something to identify our vhost-scsi device, so for that we'll drop back to the devno mapping that we use for other disks. (This option exists only for vhost-scsi-ccw, but not vhost-scsi-pci. Hrm...) This becomes <address type='ccw' ...> in the case of s390 systems, and puts the device number on the resulting QEMU command line. Thus, existing device number conflict detection can be used across other devices that may or may not be specified in the guest XML.
If one is not specified, we need to be sure to allocate one so that we can avoid polluting the device numbers with silently-created entries.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 19 +++++++++++++++++-- src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_domain_address.c | 10 ++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-)
I don't feel confident enough to review this one. The code looks okay, but from higher perspective I have no idea what it does. Sorry. Michal
On 08/09/2016 08:13 AM, Michal Privoznik wrote:
On 25.07.2016 22:48, Eric Farman wrote:
In the case of other SCSI host devices, the device that is created within the guest will be within the usual SCSI namespace (e.g., host:bus:target:lun). But for vhost-scsi, the actual naming structure will come from the host configuration, and will be invisible to both QEMU and Libvirt. So specifying one with an <address type='drive' ...> tag (as is often done for virtio-scsi) doesn't make sense in this case, as it will be ignored.
Nevertheless, we need something to identify our vhost-scsi device, so for that we'll drop back to the devno mapping that we use for other disks. (This option exists only for vhost-scsi-ccw, but not vhost-scsi-pci. Hrm...) This becomes <address type='ccw' ...> in the case of s390 systems, and puts the device number on the resulting QEMU command line. Thus, existing device number conflict detection can be used across other devices that may or may not be specified in the guest XML.
If one is not specified, we need to be sure to allocate one so that we can avoid polluting the device numbers with silently-created entries.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 19 +++++++++++++++++-- src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_domain_address.c | 10 ++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) I don't feel confident enough to review this one. The code looks okay, but from higher perspective I have no idea what it does. Sorry.
In testing my rework to Daniel Berrange's comment to patch 10 (https://www.redhat.com/archives/libvir-list/2016-August/msg00464.html) this patch ends up going away. So, no worries on the uncertainty here! - Eric
On 25.07.2016 22:48, Eric Farman wrote:
This patch series provides a libvirt implementation of the vhost-scsi interface in QEMU. As near as I can see, this was discussed upstream in July 2014[1], and ended in a desire to replace a vhost-scsi controller in favor of a hostdev element instead[2].
Hey, thank you for the patches. Unfortunately there wasn't enough review bandwidth available at the time you've sent them and now we are currently in the freeze preparing for the release. But I'll take a look at them after the release. Michal
On 08/01/2016 04:08 AM, Michal Privoznik wrote:
On 25.07.2016 22:48, Eric Farman wrote:
This patch series provides a libvirt implementation of the vhost-scsi interface in QEMU. As near as I can see, this was discussed upstream in July 2014[1], and ended in a desire to replace a vhost-scsi controller in favor of a hostdev element instead[2]. Hey, thank you for the patches. Unfortunately there wasn't enough review bandwidth available at the time you've sent them and now we are currently in the freeze preparing for the release. But I'll take a look at them after the release.
Sounds awesome, thanks. Looking forward to it! - Eric
Michal
On 08/01/2016 08:07 AM, Eric Farman wrote:
On 08/01/2016 04:08 AM, Michal Privoznik wrote:
On 25.07.2016 22:48, Eric Farman wrote:
This patch series provides a libvirt implementation of the vhost-scsi interface in QEMU. As near as I can see, this was discussed upstream in July 2014[1], and ended in a desire to replace a vhost-scsi controller in favor of a hostdev element instead[2]. Hey, thank you for the patches. Unfortunately there wasn't enough review bandwidth available at the time you've sent them and now we are currently in the freeze preparing for the release. But I'll take a look at them after the release.
Ping? I'm sure the start of the new release has gotten everyone buried again, but I'd appreciate any feedback (like if I've gone off the rails). Thanks. - Eric
On 08/09/2016 08:13 AM, Michal Privoznik wrote:
On 25.07.2016 22:48, Eric Farman wrote: So, I went through the patches. They look okay. But require some polishing before they can be merged.
Your review and feedback is much appreciated, thank you! I'll get a v2 up as soon as I can, thought it might take a moment with the refactoring per Daniel's comment to patch 17, and hoping I don't mess things up while squashing patches. :)
Also, I'd love to test this but I don't have HW to do so :(
Looking forward to v2.
Michal
participants (3)
-
Daniel P. Berrange -
Eric Farman -
Michal Privoznik