On 09/06/2016 08:58 AM, 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(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 6 ++
src/qemu/qemu_hotplug.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 154 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 45b643b..123bbcb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13720,6 +13720,12 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
return virDomainHostdevMatchSubsysSCSIiSCSI(a, b);
else
return virDomainHostdevMatchSubsysSCSIHost(a, b);
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+ if (a->source.subsys.u.host.protocol !=
+ b->source.subsys.u.host.protocol)
+ return 0;
+ if (STREQ(a->source.subsys.u.host.wwpn, b->source.subsys.u.host.wwpn))
+ return 1;
}
return 0;
}
This hunk seems to be out of place in this patch... Although I assume
this is because of the remove device searches.
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e9140a9..5f8d411 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2165,6 +2165,119 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
goto cleanup;
}
+static int
+qemuDomainAttachHostSCSIHostDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainHostdevDefPtr hostdev)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainCCWAddressSetPtr ccwaddrs = NULL;
+ char *vhostfdName = NULL;
+ int vhostfd = -1;
+ size_t vhostfdSize = 1;
+ char *devstr = NULL;
+ bool teardowncgroup = false;
+ bool teardownlabel = false;
+ bool releaseaddr = false;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
Here again the capabilities conundrum.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("SCSI passthrough is not supported by this version of
qemu"));
+ goto cleanup;
+ }
+
+ if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
+ goto cleanup;
Oh, um, didn't I comment on this in patch 1 - there's no
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST in qemuSetupHostdevCgroup
+ teardowncgroup = true;
+
+ if (virSecurityManagerSetHostdevLabel(driver->securityManager,
+ vm->def, hostdev, NULL) < 0)
+ goto cleanup;
+ teardownlabel = true;
Likewise there's really not much going on...
+
+ if (virSCSIOpenVhost(&vhostfd, &vhostfdSize) < 0)
+ goto cleanup;
+
+ if (virAsprintf(&vhostfdName, "vhostfd-%d", vhostfd) < 0)
+ goto cleanup;
+
+ 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_NONE ||
+ hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ) {
+ if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) <
0)
'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).
+ goto cleanup;
+ } else if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+ if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+ goto cleanup;
+ if (virDomainCCWAddressAssign(hostdev->info, ccwaddrs,
+ !hostdev->info->addr.ccw.assigned) < 0)
+ goto cleanup;
+ }
+ releaseaddr = true;
+
+ if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
+ goto cleanup;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ if (hostdev->source.subsys.u.host.protocol ==
VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_VHOST) {
Long line > 80 cols
Ironically we do nothing if the protocol isn't VHOST...
+ if (!(devstr = qemuBuildHostHostdevDevStr(vm->def,
+ hostdev,
+ priv->qemuCaps,
+ vhostfdName,
+ vhostfdSize)))
This can be done outside holding the monitor lock
+ goto exit_monitor;
+
+ if ((ret = qemuMonitorAddDeviceWithFd(priv->mon,
+ devstr,
+ vhostfd,
+ vhostfdName)) < 0) {
+ goto exit_monitor;
+ }
+ }
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ virDomainAuditHostdev(vm, hostdev, "attach", true);
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).
+
+ if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
+ goto cleanup;
+
+ vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
Other code will increase the size of hostdevs before the monitor calls,
then just insert.
+
+ virDomainCCWAddressSetFree(ccwaddrs);
+
+ VIR_FORCE_CLOSE(vhostfd);
This I would think would only be done on *failure* to AddDeviceWithFd,
but I see you're following qemuDomainAttachHostPCIDevice
+ VIR_FREE(vhostfdName);
+ VIR_FREE(devstr);
+ return 0;
+
+ exit_monitor:
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ cleanup:
+ if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
+ VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail");
+ if (teardownlabel &&
+ virSecurityManagerRestoreHostdevLabel(driver->securityManager,
+ vm->def, hostdev, NULL) < 0)
+ VIR_WARN("Unable to restore host device labelling on hotplug fail");
+ if (releaseaddr)
+ qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
+
+ VIR_FORCE_CLOSE(vhostfd);
+ VIR_FREE(vhostfdName);
+ VIR_FREE(devstr);
+ return ret;
+}
+
int
qemuDomainAttachHostDevice(virConnectPtr conn,
@@ -2198,6 +2311,11 @@ qemuDomainAttachHostDevice(virConnectPtr conn,
goto error;
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+ if (qemuDomainAttachHostSCSIHostDevice(driver, vm, hostdev) < 0)
+ goto error;
+ break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("hostdev subsys type '%s' not supported"),
@@ -3960,6 +4078,31 @@ qemuDomainDetachHostSCSIDevice(virQEMUDriverPtr driver,
}
static int
+qemuDomainDetachHostSCSIHostDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainHostdevDefPtr detach)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret = -1;
+
+ if (!detach->info->alias) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ "%s", _("device cannot be detached without a
device alias"));
+ return -1;
+ }
+
+ qemuDomainMarkDeviceForRemoval(vm, detach->info);
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ ret = qemuMonitorDelDevice(priv->mon, detach->info->alias);
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ return -1;
+
+ return ret;
+}
+
+static int
qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainHostdevDefPtr detach)
@@ -3981,6 +4124,9 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
ret = qemuDomainDetachHostSCSIDevice(driver, vm, detach);
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+ ret = qemuDomainDetachHostSCSIHostDevice(driver, vm, detach);
+ break;
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("hostdev subsys type '%s' not supported"),
@@ -4058,6 +4204,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
Ah - and this is where that pesky virDomainHostdevFind causes the need
for the domain_conf.c code above...
}
break;
}
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
But we need an error here otherwise we get a generic failed with some
error message.
John
+ break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected hostdev type %d"), subsys->type);