5:10 p.m.
On 09/06/2016 08:58 AM, 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(a)linux.vnet.ibm.com>
---
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);
This is where id add the "naa." prefix, e.g wwpn=naa.%s" - with a
somewhat healthy comment behind it as to why it's being used.
+
+ 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;
I think this is what I would think auditing (during hotplug of course)
would end up using as the address rather than what will happen in patch
1 resulting in "?" being displayed.
+
+ 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)) {
I'm conflicted if we should check QEMU_CAPS_DEVICE_VHOST_SCSI here as
well... I know it's checked later, but we open vhost-scsi which I
assume wouldn't exist if the cap wasn't there.
Of course I see in hotplug things have to be a little different in order
to add that fd and name via monitor rather than command.
+ if (hostdev->source.subsys.u.host.protocol ==
VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_VHOST) {
80 cols for the line
+ 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);
80 cols for the line.
+
+ virCommandAddArg(cmd, "-device");
+ if (!(devstr = qemuBuildHostHostdevDevStr(def, hostdev, qemuCaps,
vhostfdName, vhostfdSize)))
This is a really long line - try to keep at 80 cols.
+ return -1;
We'd leak vhostfdName on failure (I think the vhostfd will be reaped by
Command cleanup.
It might just be easier to extract the whole hunk into a function and do
all the error processing there.
Hey - BTW: I see this PCI configfd - I betcha a bit of tracking on that
will help give you examples for security_*.c changes and whether they're
necessary or not. I didn't go chase.
+ 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)
Well this is dangerous... I can pass "any" value value here and really
cause some damage. Why the need for a loop?
I think you just attempt to open the file (you could even to a
virFileExists() if you want to care about checking if the
/dev/vhost-scsi exists before opening ...
+{
+ 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"));
Simplify your error _("failed to open /dev/vhost-scsi")
John
+ 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,