All,
The Xen code for making HVM VT-d PCI passthrough attach and detach is
currently not properly working. There are 2 problems:
1) In xenDaemonAttachDevice(), we were always trying to reconfigure
a PCI passthrough device, even the first time we added it. This was
because the code in virDomainXMLDevID() was not checking xenstore for
the existence of the device, and always returning 0 (meaning that
the device already existed).
2) In xenDaemonDetachDevice(), we were trying to use "device_destroy"
to detach a PCI device. While you would think that is the right
method to call, it's actually wrong for PCI devices. In particular,
in upstream Xen (and soon in RHEL-5 Xen), device_configure is actually
used to destroy a PCI device.
The attached patch fixes both of these problems. To fix the attach
problem I add a lookup into xenstore to see if the device we are
trying to attach already exists. To fix the detach problem I change
it so that for PCI detach (only), we use device_configure with the
appropriate sxpr to do the detachment.
Tested by me on RHEL-5 on a VT-d capable machine, and, in combination
with the RHEL-5 xen patch, fixes the problem for me in testing.
(note: this solves https://bugzilla.redhat.com/show_bug.cgi?id=546671)
Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 241a102..cf7e926 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -88,7 +88,8 @@ xenDaemonFormatSxprNet(virConnectPtr conn ATTRIBUTE_UNUSED,
static int
xenDaemonFormatSxprOnePCI(virConnectPtr conn,
virDomainHostdevDefPtr def,
- virBufferPtr buf);
+ virBufferPtr buf,
+ int detach);
static int
virDomainXMLDevID(virDomainPtr domain,
@@ -4162,7 +4163,7 @@ xenDaemonAttachDevice(virDomainPtr domain, const char *xml)
dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
if (xenDaemonFormatSxprOnePCI(domain->conn,
dev->data.hostdev,
- &buf) < 0)
+ &buf, 0) < 0)
goto cleanup;
} else {
virXendError(domain->conn, VIR_ERR_NO_SUPPORT, "%s",
@@ -4214,6 +4215,8 @@ xenDaemonDetachDevice(virDomainPtr domain, const char *xml)
virDomainDeviceDefPtr dev = NULL;
virDomainDefPtr def = NULL;
int ret = -1;
+ char *xendev = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
@@ -4244,8 +4247,28 @@ xenDaemonDetachDevice(virDomainPtr domain, const char *xml)
if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref)))
goto cleanup;
- ret = xend_op(domain->conn, domain->name, "op", "device_destroy",
- "type", class, "dev", ref, "force", "0", "rm_cfg", "1", NULL);
+ if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
+ if (dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+ dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+ if (xenDaemonFormatSxprOnePCI(domain->conn,
+ dev->data.hostdev,
+ &buf, 1) < 0)
+ goto cleanup;
+ } else {
+ virXendError(domain->conn, VIR_ERR_NO_SUPPORT, "%s",
+ _("unsupported device type"));
+ goto cleanup;
+ }
+ xendev = virBufferContentAndReset(&buf);
+ ret = xend_op(domain->conn, domain->name, "op", "device_configure",
+ "config", xendev, "dev", ref, NULL);
+ VIR_FREE(xendev);
+ }
+ else {
+ ret = xend_op(domain->conn, domain->name, "op", "device_destroy",
+ "type", class, "dev", ref, "force", "0", "rm_cfg", "1",
+ NULL);
+ }
cleanup:
virDomainDefFree(def);
@@ -5555,7 +5578,8 @@ xenDaemonFormatSxprPCI(virDomainHostdevDefPtr def,
static int
xenDaemonFormatSxprOnePCI(virConnectPtr conn,
virDomainHostdevDefPtr def,
- virBufferPtr buf)
+ virBufferPtr buf,
+ int detach)
{
if (def->managed) {
virXendError(conn, VIR_ERR_NO_SUPPORT, "%s",
@@ -5565,6 +5589,10 @@ xenDaemonFormatSxprOnePCI(virConnectPtr conn,
virBufferAddLit(buf, "(pci ");
xenDaemonFormatSxprPCI(def, buf);
+ if (detach)
+ virBufferAddLit(buf, "(state 'Closing')");
+ else
+ virBufferAddLit(buf, "(state 'Initialising')");
virBufferAddLit(buf, ")");
return 0;
@@ -5596,9 +5624,8 @@ xenDaemonFormatSxprAllPCI(virConnectPtr conn,
* )
* )
*
- * Normally there is one (device ...) block per device, but in
- * wierd world of Xen PCI, once (device ...) covers multiple
- * devices.
+ * Normally there is one (device ...) block per device, but in the
+ * weird world of Xen PCI, one (device ...) covers multiple devices.
*/
virBufferAddLit(buf, "(device (pci ");
@@ -5937,6 +5964,8 @@ error:
* - if disk, copy in ref the target name from description
* - if network, get MAC address from description, scan XenStore and
* copy in ref the corresponding vif number.
+ * - if pci, get BDF from description, scan XenStore and
+ * copy in ref the corresponding dev number.
*
* Returns 0 in case of success, -1 in case of failure.
*/
@@ -5994,6 +6023,31 @@ virDomainXMLDevID(virDomainPtr domain,
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+ char *bdf;
+ virDomainHostdevDefPtr def = dev->data.hostdev;
+
+ if (virAsprintf(&bdf, "%04x:%02x:%02x.%0x",
+ def->source.subsys.u.pci.domain,
+ def->source.subsys.u.pci.bus,
+ def->source.subsys.u.pci.slot,
+ def->source.subsys.u.pci.function) < 0) {
+ virReportOOMError(NULL);
+ return -1;
+ }
+
+ strcpy(class, "pci");
+
+ xenUnifiedLock(priv);
+ xref = xenStoreDomainGetPCIID(domain->conn, domain->id, bdf);
+ xenUnifiedUnlock(priv);
+ VIR_FREE(bdf);
+ if (xref == NULL)
+ return -1;
+
+ tmp = virStrcpy(ref, xref, ref_len);
+ VIR_FREE(xref);
+ if (tmp == NULL)
+ return -1;
} else {
virXendError(NULL, VIR_ERR_NO_SUPPORT,
"%s", _("hotplug of device type not supported"));
diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
index 10b1f8e..8a64d4e 100644
--- a/src/xen/xs_internal.c
+++ b/src/xen/xs_internal.c
@@ -1056,6 +1056,61 @@ xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev) {
}
/*
+ * xenStoreDomainGetPCIID:
+ * @conn: pointer to the connection.
+ * @id: the domain id
+ * @bdf: the PCI BDF
+ *
+ * Get the reference (i.e. the string number) for the device on that domain
+ * which uses the given PCI address
+ *
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ *
+ * Returns the new string or NULL in case of error, the string must be
+ * freed by the caller.
+ */
+char *
+xenStoreDomainGetPCIID(virConnectPtr conn, int id, const char *bdf)
+{
+ char dir[80], path[128], **list = NULL, *val = NULL;
+ unsigned int len, i, num;
+ char *ret = NULL;
+ xenUnifiedPrivatePtr priv;
+
+ if (id < 0)
+ return(NULL);
+
+ priv = (xenUnifiedPrivatePtr) conn->privateData;
+ if (priv->xshandle == NULL)
+ return (NULL);
+ if (bdf == NULL)
+ return (NULL);
+
+ snprintf(dir, sizeof(dir), "/local/domain/0/backend/pci/%d", id);
+ list = xs_directory(priv->xshandle, 0, dir, &num);
+ if (list == NULL)
+ return(NULL);
+ for (i = 0; i < num; i++) {
+ snprintf(path, sizeof(path), "%s/%s/%s", dir, list[i], "dev-0");
+ if ((val = xs_read(priv->xshandle, 0, path, &len)) == NULL)
+ break;
+
+ bool match = STREQ(val, bdf);
+
+ VIR_FREE(val);
+
+ if (match) {
+ ret = strdup(list[i]);
+ break;
+ }
+ }
+
+ VIR_FREE(list);
+ return(ret);
+}
+
+/*
* The caller must hold the lock on the privateData
* associated with the 'conn' parameter.
*/
diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h
index 29e680f..6e0f40d 100644
--- a/src/xen/xs_internal.h
+++ b/src/xen/xs_internal.h
@@ -50,6 +50,9 @@ char * xenStoreDomainGetNetworkID(virConnectPtr conn,
char * xenStoreDomainGetDiskID(virConnectPtr conn,
int id,
const char *dev);
+char * xenStoreDomainGetPCIID(virConnectPtr conn,
+ int domid,
+ const char *bdf);
char * xenStoreDomainGetName(virConnectPtr conn,
int id);
int xenStoreDomainGetUUID(virConnectPtr conn,
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list