We already have a "scsi" hostdev type, which refers to a single LUN that is passed through to a guest. But what of things where multiple LUNs are passed through via a single SCSI HBA, such as with the vhost-scsi target? Create a new hostdev type that will carry this, and its associated documentation and XML schema information. Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com&gt; --- docs/formatdomain.html.in | 24 ++++++++++++++++ docs/schemas/domaincommon.rng | 23 +++++++++++++++ src/conf/domain_audit.c | 2 ++ src/conf/domain_conf.c | 56 +++++++++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 17 +++++++++++ src/qemu/qemu_domain_address.c | 10 +++++++ src/qemu/qemu_hotplug.c | 1 + src/security/security_dac.c | 2 ++ tests/domaincapsschemadata/full.xml | 1 + 9 files changed, 134 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index feaeaa2..b79da95 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3655,6 +3655,17 @@ &lt;/devices&gt; ...</pre> + <p>or:</p> + +<pre> + ... + &lt;devices&gt; + &lt;hostdev mode='subsystem' type='scsi_host'&gt; + &lt;source protocol='vhost' wwpn='naa.50014057667280d8'/&gt; + &lt;/hostdev&gt; + &lt;/devices&gt; + ...</pre> + <dl> <dt><code>hostdev</code></dt> <dd>The <code>hostdev</code> element is the main container for describing @@ -3693,6 +3704,12 @@ If a disk lun in the domain already has the rawio capability, then this setting not required. </dd> + <dt><code>scsi_host</code></dt> + <dd><span class="since">since 2.2.0</span>For SCSI devices, user + is responsible to make sure the device is not used by host. This + <code>type</code> passes all LUNs presented by a single HBA to + the guest. + </dd> </dl> <p> Note: The <code>managed</code> attribute is only used with PCI devices @@ -3756,6 +3773,13 @@ credentials to the iSCSI server. </p> </dd> + <dt><code>scsi_host</code></dt> + <dd><span class="since">Since 2.2.0</span>, multiple LUNs behind a + single SCSI HBI are described by a <code>protocol</code> + attribute set to "vhost" and a <code>wwpn</code> attribute that + is the vhost_scsi wwpn (16 hexadecimal digits with a prefix of + "naa.") established in the host configfs. + </dd> </dl> </dd> <dt><code>vendor</code>, <code>product</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af32df8..7fd6bd2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3938,6 +3938,7 @@ <ref name="hostdevsubsyspci"/> <ref name="hostdevsubsysusb"/> <ref name="hostdevsubsysscsi"/> + <ref name="hostdevsubsyshost"/> </choice> </define> @@ -4066,6 +4067,28 @@ </element> </define> + <define name="hostdevsubsyshost"> + <attribute name="type"> + <value>scsi_host</value> + </attribute> + <element name="source"> + <choice> + <group> + <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> + <define name="hostdevcapsstorage"> <attribute name="type"> <value>storage</value> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 53a58ac..406dd8f 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -443,6 +443,8 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, } break; } + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: + break; default: VIR_WARN("Unexpected hostdev type while encoding audit message: %d", hostdev->source.subsys.type); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c8c4f61..45b643b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -646,7 +646,8 @@ VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST, VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST, "usb", "pci", - "scsi") + "scsi", + "scsi_host") VIR_ENUM_IMPL(virDomainHostdevSubsysPCIBackend, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST, @@ -660,6 +661,10 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIProtocol, "adapter", "iscsi") +VIR_ENUM_IMPL(virDomainHostdevSubsysHostProtocol, + VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_LAST, + "vhost") + VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST, "storage", "misc", @@ -2270,6 +2275,9 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) } else { VIR_FREE(scsisrc->u.host.adapter); } + } else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST) { + virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host; + VIR_FREE(hostsrc->wwpn); } break; } @@ -5977,6 +5985,31 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode, return ret; } +static int +virDomainHostdevSubsysHostDefParseXML(xmlNodePtr sourcenode, + virDomainHostdevDefPtr def) +{ + virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host; + + if (!(hostsrc->wwpn = virXMLPropString(sourcenode, "wwpn"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing vhost-scsi hostdev source path name")); + goto cleanup; + } + + if (!STRPREFIX(hostsrc->wwpn, "naa.") || + strlen(hostsrc->wwpn) != strlen("naa.") + 16) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("malformed 'wwpn' value")); + goto cleanup; + } + + return 0; + + cleanup: + VIR_FREE(hostsrc->wwpn); + return -1; +} + static int virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, @@ -6101,6 +6134,11 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, goto error; break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: + if (virDomainHostdevSubsysHostDefParseXML(sourcenode, def) < 0) + goto error; + break; + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("address type='%s' not supported in hostdev interfaces"), @@ -20521,9 +20559,11 @@ 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; + virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host; virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; @@ -20564,6 +20604,15 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, protocol, iscsisrc->path); } + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST) { + const char *protocol = + virDomainHostdevSubsysHostProtocolTypeToString(hostsrc->protocol); + closedSource = true; + + virBufferAsprintf(buf, " protocol='%s' wwpn='%s'/", + protocol, hostsrc->wwpn); + } + virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); @@ -20617,6 +20666,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, scsihostsrc->unit); } break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: + break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected hostdev type %d"), @@ -20632,7 +20683,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, } virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</source>\n"); + if (!closedSource) + virBufferAddLit(buf, "</source>\n"); return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0fe4154..781ea7b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -294,6 +294,7 @@ typedef enum { VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI, + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST } virDomainHostdevSubsysType; @@ -368,6 +369,21 @@ struct _virDomainHostdevSubsysSCSI { } u; }; +typedef enum { + VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_VHOST, + + VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_LAST, +} virDomainHostdevHostProtocolType; + +VIR_ENUM_DECL(virDomainHostdevSubsysHostProtocol) + +typedef struct _virDomainHostdevSubsysHost virDomainHostdevSubsysHost; +typedef virDomainHostdevSubsysHost *virDomainHostdevSubsysHostPtr; +struct _virDomainHostdevSubsysHost { + int protocol; + char *wwpn; +}; + typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys; typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr; struct _virDomainHostdevSubsys { @@ -376,6 +392,7 @@ struct _virDomainHostdevSubsys { virDomainHostdevSubsysUSB usb; virDomainHostdevSubsysPCI pci; virDomainHostdevSubsysSCSI scsi; + virDomainHostdevSubsysHost host; } u; }; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index bb16738..8419e6a 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -301,6 +301,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_HOST && + def->hostdevs[i]->source.subsys.u.host.protocol == + VIR_DOMAIN_HOSTDEV_HOST_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) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d13474a..e9140a9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3238,6 +3238,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, qemuDomainRemoveUSBHostDevice(driver, vm, hostdev); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: qemuDomainRemoveSCSIHostDevice(driver, vm, hostdev); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 442ce70..de2b921 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -676,6 +676,7 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr, break; } + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: ret = 0; break; @@ -805,6 +806,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr, break; } + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: ret = 0; break; diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml index 2f529ff..41fb1fa 100644 --- a/tests/domaincapsschemadata/full.xml +++ b/tests/domaincapsschemadata/full.xml @@ -75,6 +75,7 @@ <value>usb</value> <value>pci</value> <value>scsi</value> + <value>scsi_host</value> </enum> <enum name='capsType'> <value>storage</value> -- 1.9.1

On 09/12/2016 05:34 PM, John Ferlan wrote: I wasn't at KVM Forum, so I'm in the dark here. :( My experience with NPIV has been on s390, so I'm gonna plead some ignorance on NPIV for other platforms. Considering the lack of controls on the qemu side, I would imagine that it should work. Hopefully a consensus before too long. :) Fairly certain that it's more a vhost thing of which the hypervisor utilizes. Per a recommendation made on this list years ago, I used targetcli to do the configuration rather than direct manipulation of sysfs, and according to its man page: All WWNs are prefaced by their type, such as "iqn", "naa", or "ib", and may be given with or without colons. As to whether other hypervisors follow the same behavior, I do not know. I was just trying to conform to what it would allow in the case of vhost-scsi. Now, I suppose I'm confusing matters by saying "wwpn" here since as you say that's 16 hex digits, and should probably say just "wwn" ? Ugh, yeah. When I rebased after 2.2's release I forgot about the doc hits. My apologies. Hrm... nodedev-dumpxml is new to me, neat! I'll go down that bunny trail, but it seems like it'd be a bit of work to add the smarts for this. As near as I can tell, it's not anything that can be autogenerated off the bat. Somebody somewhere needs to first establish the sysfs entries for the vhost target, which can then be stored and loaded on a next (host) boot. The end result looks something like this: [root@xxxxxxxx ~]# ls -l /sys/kernel/config/target/vhost/naa.50014056e4794195/tpgt_1/lun/lun_0/ total 0 lrwxrwxrwx 1 root root 0 Sep 13 15:44 752e150d11 -> ../../../../../../target/core/iblock_0/disk0 -rw-r--r-- 1 root root 4096 Sep 13 15:45 alua_tg_pt_gp -rw-r--r-- 1 root root 4096 Sep 13 15:45 alua_tg_pt_offline -rw-r--r-- 1 root root 4096 Sep 13 15:45 alua_tg_pt_status -rw-r--r-- 1 root root 4096 Sep 13 15:45 alua_tg_pt_write_md drwxr-xr-x 5 root root 0 Sep 13 15:44 statistics [root@xxxxxxxx ~]# cat /sys/kernel/config/target/core/iblock_0/disk0/info Status: ACTIVATED Max Queue Depth: 0 SectorSize: 512 HwMaxSectors: 4592 iBlock device: dm-0 UDEV PATH: /dev/mapper/mpathb readonly: 0 Major: 252 Minor: 0 CLAIMED: IBLOCK (Aside: note the folder name in /sys/kernel/config that contains the wwn/wwpn name with the "naa." prefix.) Anything that's put behind the target gets seen to the guest. So that can be one or more sgN LUNs and their sdX equivalents, and as near as I can tell a scsi_hostM as you describe. A quick test with two LUNs behind one vhost-scsi target gives me the following in the guest: # ls -l /sys/bus/scsi/devices total 0 lrwxrwxrwx 1 root root 0 Sep 13 16:10 0:0:1:0 -> ../../../devices/css0/0.0.0002/0.0.beef/virtio2/host0/target0:0:1/0:0:1:0 lrwxrwxrwx 1 root root 0 Sep 13 16:10 0:0:1:1 -> ../../../devices/css0/0.0.0002/0.0.beef/virtio2/host0/target0:0:1/0:0:1:1 lrwxrwxrwx 1 root root 0 Sep 13 16:10 host0 -> ../../../devices/css0/0.0.0002/0.0.beef/virtio2/host0 lrwxrwxrwx 1 root root 0 Sep 13 16:11 target0:0:1 -> ../../../devices/css0/0.0.0002/0.0.beef/virtio2/host0/target0:0:1 # lsscsi [0:0:1:0] disk LIO-ORG disk0 4.0 /dev/sda [0:0:1:1] disk LIO-ORG disk1 4.0 /dev/sdb FYI, hotplugging/unplugging devices to a target works just fine, except there's no notification that a guest can pick up on to know that a device had been added. Or worse, taken away. This rings a bell with me, as to why I had the label be "wwpn" instead of "wwn" above. My attempt at trying to distinguish libvirt internals with something user-facing, maybe? Fair enough. Will go down that trail. Because "none" == "adapter" in the case of SCSI? Ah, okay. virValidateWWN can be used regardless I suppose. It does, because of the switch statement not having a "default" case. Oh darn, what happened. :( I'll go revisit my merging/rebasing. Excellent question. I'll give it a try. Thanks for the review. (Silent ACK on a lot of the above comments.) I'll try to get these all in place for a v3 with a little runway before the next freeze. - Eric

On 09/13/2016 04:49 PM, John Ferlan wrote: Agreed. v1 was perhaps overkill in breaking things down so small, because I was picking up an idea with a bit of dust on it. I always thought WWPN and WWNN as types of WWN... ...but have seen it this way also. Either way makes sense to me. Me neither. Of course, I'm using an s390, so that's a little different too. :) /sys/class/fc_host is just showing the individual SCSI disks, but nothing that looks to point to the vhost-scsi target that I'm passing to QEMU. /sys/class/fc_vports is empty. (Oh wait, is NPIV turned on here? Ugh, pick this up later.) No, I can't see anybody linking it from anywhere except the couple cross links in /sys/kernel/config/ shown below. Erg, so then the nodedev stuff becomes bigger still. Correct, it's just not a "scsi_hostN" HBA that exists as part of the normal fibre attachment, but a vHBA that's defined to the vhost-scsi driver (via targetcli or some other manual handshaking). Nope... (s390 host) # ls -l /sys/bus/ total 0 drwxr-xr-x 4 root root 0 Sep 13 23:12 ccw drwxr-xr-x 4 root root 0 Sep 13 23:12 clockevents drwxr-xr-x 4 root root 0 Sep 13 23:12 clocksource drwxr-xr-x 4 root root 0 Sep 13 23:12 container drwxr-xr-x 4 root root 0 Sep 13 23:12 cpu drwxr-xr-x 4 root root 0 Sep 13 23:12 css drwxr-xr-x 4 root root 0 Sep 13 23:12 etr drwxr-xr-x 4 root root 0 Sep 13 23:12 event_source drwxr-xr-x 4 root root 0 Sep 13 23:12 memory drwxr-xr-x 5 root root 0 Sep 13 23:12 pci drwxr-xr-x 4 root root 0 Sep 13 23:12 platform drwxr-xr-x 4 root root 0 Sep 13 23:12 scm drwxr-xr-x 4 root root 0 Sep 13 23:12 scsi drwxr-xr-x 4 root root 0 Sep 13 23:12 stp drwxr-xr-x 4 root root 0 Sep 13 23:12 virtio drwxr-xr-x 4 root root 0 Sep 13 23:12 workqueue Fair enough. I have a patch started, but kept it separate from this series because I got distracted/hoped to get v2 out before the last freeze. I will dust that off. Thanks for taking a few minutes away from those to look over this little pile. I appreciate the fires that pop up and get in the way of the usual todo list. :) Apologies for that. I've already rebased/resolved the capability conflict that occurred between posting to the list and now, and moved that to the head of my series. Taking a day's holiday tomorrow, so will get back to the v3 series Thursday. - Eric

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(a)linux.vnet.ibm.com&gt; Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ 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 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + 13 files changed, 17 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2ca7803..cf1e468 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -342,6 +342,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "smm", "virtio-pci-disable-legacy", "query-hotpluggable-cpus", + + "vhost-scsi", /* 235 */ ); @@ -1566,6 +1568,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 a74d39f..d8ddc8f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -376,6 +376,9 @@ typedef enum { QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, /* virtio-*pci.disable-legacy */ QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS, /* qmp command query-hotpluggable-cpus */ + /* 235 */ + 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 db778ef..020ea44 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -185,6 +185,7 @@ <flag name='intel-iommu'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <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 fc915ad..730d755 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -190,6 +190,7 @@ <flag name='intel-iommu'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <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 fd14665..73eb588 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -159,6 +159,7 @@ <flag name='display'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <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 eb708f8..6143af3 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -159,6 +159,7 @@ <flag name='display'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <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 482b384..f8d9e70 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -153,6 +153,7 @@ <flag name='display'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <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 60f1fcf..7cbfc1a 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -196,6 +196,7 @@ <flag name='intel-iommu'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <flag name='vhost-scsi'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 7a54040..73a060a 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -194,6 +194,7 @@ <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> <flag name='query-hotpluggable-cpus'/> + <flag name='vhost-scsi'/> <version>2006091</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0-rc1-52-g42e0d60)</package> -- 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(a)linux.vnet.ibm.com&gt; --- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 6 ++++ src/util/virscsi.c | 26 ++++++++++++++++ src/util/virscsi.h | 1 + 5 files changed, 114 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d62c74c..22fe14d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2265,6 +2265,7 @@ virSCSIDeviceListNew; virSCSIDeviceListSteal; virSCSIDeviceNew; virSCSIDeviceSetUsedBy; +virSCSIOpenVhost; # util/virseclabel.h diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 982c33c..479dde2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4711,6 +4711,52 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) } char * +qemuBuildHostHostdevDevStr(const virDomainDef *def, + virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps, + char *vhostfdName, + size_t vhostfdSize) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainHostdevSubsysHostPtr hostsrc = &dev->source.subsys.u.host; + + 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", hostsrc->wwpn); + + if (vhostfdSize == 1) { + virBufferAsprintf(&buf, ",vhostfd=%s", vhostfdName); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("QEMU supports a single vhost-scsi file descriptor")); + goto cleanup; + } + + virBufferAsprintf(&buf, ",id=%s", dev->info->alias); + + if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0) + goto cleanup; + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + return virBufferContentAndReset(&buf); + + cleanup: + virBufferFreeAndReset(&buf); + return NULL; +} + +char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -5156,6 +5202,40 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, return -1; } } + + /* SCSI_host */ + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { + if (hostdev->source.subsys.u.host.protocol == VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_VHOST) { + char *vhostfdName = NULL; + int vhostfd = -1; + size_t vhostfdSize = 1; + + if (virSCSIOpenVhost(&vhostfd, &vhostfdSize) < 0) + return -1; + + if (virAsprintf(&vhostfdName, "%d", vhostfd) < 0) { + VIR_FORCE_CLOSE(vhostfd); + return -1; + } + + virCommandPassFD(cmd, vhostfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildHostHostdevDevStr(def, hostdev, qemuCaps, vhostfdName, vhostfdSize))) + return -1; + virCommandAddArg(cmd, devstr); + + VIR_FREE(vhostfdName); + VIR_FREE(devstr); + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SCSI passthrough is not supported by this version of qemu")); + return -1; + } + } } return 0; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 9b9ccb6..78179de 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -158,6 +158,12 @@ char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev); char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); +char * +qemuBuildHostHostdevDevStr(const virDomainDef *def, + virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps, + char *vhostfdName, + size_t vhostfdSize); 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 09/12/2016 06:10 PM, John Ferlan wrote: My bad. Ooh, good lead. I said I'll chase down again, that's where I'll start. Well, I guess I only half-cleaned it up. I'm not aware of any mechanism to pass multiple fd's for vhost-scsi-pci or vhost-scsi-ccw into QEMU (unlike the virtio-scsi-{ccw|pci} paths), so I started ripping the loops out. Looking at it now, I guess I didn't rip out nearly as much as I thought I did. So, more to remove I guess. Unless leaving it there for an "if the future ever arrives" case is a problem. I like this. Many thanks again for the reviews. - Eric

On 09/13/2016 04:55 PM, John Ferlan wrote: Yeah, that was the starting point. What I'd discovered, and why I started ripping this apart, is that QEMU itself doesn't have an option to pass multiple fd's for a vhost-scsi device. So while for vhost-net has fd= and (vhost)fds=, vhost-scsi only has vhostfd=. qemu-system-s390x: -device vhost-scsi-ccw,wwpn=naa.500140594524c195,vhostfds=5: Property '.vhostfds' not found The queues parameter operates in conjunction with the number of virtqueue rings that are instantiated for the guest process. - Eric

On 09/06/2016 08:58 AM, Eric Farman wrote: This hunk seems to be out of place in this patch... Although I assume this is because of the remove device searches. Here again the capabilities conundrum. Oh, um, didn't I comment on this in patch 1 - there's no VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST in qemuSetupHostdevCgroup Likewise there's really not much going on... 'info' is already a virDomainDeviceInfoPtr (unlike where you cut-n-paste'd from), so no need for the '&' here (besides it induces a build failure). Long line > 80 cols Ironically we do nothing if the protocol isn't VHOST... This can be done outside holding the monitor lock s/true/(ret == 0)/ Since we've set up the addresses above - the auditing code should print more than "?" Here should be a "if (ret < 0) goto cleanup;" (on two lines). Other code will increase the size of hostdevs before the monitor calls, then just insert. This I would think would only be done on *failure* to AddDeviceWithFd, but I see you're following qemuDomainAttachHostPCIDevice Ah - and this is where that pesky virDomainHostdevFind causes the need for the domain_conf.c code above... But we need an error here otherwise we get a generic failed with some error message. John

On 09/06/2016 08:58 AM, Eric Farman wrote: This hunk would be with the qemu command line patch This hunk would be with the domain xml patch (along with the dataout file) I think this hunk belongs with the code that added the function... Interesting and existing - QEMU_CAPS_VIRTIO_SCSI listed twice... That's a separate patch though - I'll send that soon. This hunk would go with the qemu command line patch... John