From c36a4fa1e056025e5c05b20d42bd7050a34aadc158f57188b61b37f6a20638ac Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Mon, 25 Feb 2013 21:55:36 +0000 Subject: [PATCH] - Fix detach of managed PCI devices from inactive domains. Detected while running test cases for FATE #313570. Modified xen-name-for-devid.patch OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=250 --- libvirt.changes | 7 ++ xen-name-for-devid.patch | 149 ++++++++++++++++++++++++++++++++------- 2 files changed, 132 insertions(+), 24 deletions(-) diff --git a/libvirt.changes b/libvirt.changes index 135318a..e5a21cb 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon Feb 25 12:30:24 MST 2013 - jfehlig@suse.com + +- Fix detach of managed PCI devices from inactive domains. + Detected while running test cases for FATE #313570. + Modified xen-name-for-devid.patch + ------------------------------------------------------------------- Thu Feb 21 15:48:39 MST 2013 - jfehlig@suse.com diff --git a/xen-name-for-devid.patch b/xen-name-for-devid.patch index d9a6c76..33d4646 100644 --- a/xen-name-for-devid.patch +++ b/xen-name-for-devid.patch @@ -1,17 +1,18 @@ -commit 7906a668fa8d5c21cc729db8a13b08e3dd1d241f -Author: Jim Fehlig -Date: Wed Jan 27 16:11:41 2010 -0700 + Do not search xenstore for disk/network/PCI device IDs + + Disk, network, and PCI devices can be referenced by name in Xen, + e.g. when modifying their configuration or remvoving them. As such, + don't search xenstore for a device ID corresponding to these devices. + Instead, search the devices contained in the domain definition and use + the devices's target name if found. - Do not search xenstore for disk device IDs + Note that for network devices, the mac address is used for the device + name. For PCI devices, the bdf (bus:dev:fun) specifier is used for + the device name. - Disk devices can be referenced by name in Xen, e.g. when modifying - their configuration or remvoving them. As such, don't search - xenstore for a device ID corresponding to the disk device. Instead, - search the disks contained in the domain definition and use the - disk's target name if found. - - This approach allows removing a disk when domain is inactive. We - obviously can't search xenstore when the domain is inactive. + This approach allows removing a disk/network/PCI device when domain + is inactive. We obviously can't search xenstore when the domain is + inactive. Index: libvirt-1.0.2/src/xen/xend_internal.c =================================================================== @@ -52,7 +53,22 @@ Index: libvirt-1.0.2/src/xen/xend_internal.c goto cleanup; if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { -@@ -3924,6 +3925,7 @@ struct xenUnifiedDriver xenDaemonDriver +@@ -3912,18 +3913,18 @@ struct xenUnifiedDriver xenDaemonDriver + * @dev: pointer to device config object + * @class: Xen device class "vbd" or "vif" (OUT) + * @ref: Xen device reference (OUT) ++ * @ref_len: Length of character buffer proviced by the ref parameter + * + * Set class according to XML root, and: + * - 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. ++ * - if network, copy in ref the target MAC address from description ++ * - if pci, copy in ref the target BDF from description + * + * Returns 0 in case of success, -1 in case of failure. */ static int virDomainXMLDevID(virDomainPtr domain, @@ -60,12 +76,12 @@ Index: libvirt-1.0.2/src/xen/xend_internal.c virDomainDeviceDefPtr dev, char *class, char *ref, -@@ -3932,8 +3934,12 @@ virDomainXMLDevID(virDomainPtr domain, +@@ -3931,9 +3932,11 @@ virDomainXMLDevID(virDomainPtr domain, + { xenUnifiedPrivatePtr priv = domain->conn->privateData; char *xref; - char *tmp; +- char *tmp; + unsigned int i; -+ virDomainDiskDefPtr disk; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->dst == NULL) @@ -73,7 +89,7 @@ Index: libvirt-1.0.2/src/xen/xend_internal.c if (dev->data.disk->driverName && STREQ(dev->data.disk->driverName, "tap")) strcpy(class, "tap"); -@@ -3943,19 +3949,21 @@ virDomainXMLDevID(virDomainPtr domain, +@@ -3943,19 +3946,17 @@ virDomainXMLDevID(virDomainPtr domain, else strcpy(class, "vbd"); @@ -90,15 +106,11 @@ Index: libvirt-1.0.2/src/xen/xend_internal.c - VIR_FREE(xref); - if (tmp == NULL) - return -1; -+ /* For disks, the device name can be used directly. -+ * If disk device exists in domain definintion, -+ * copy it to ref and return success. -+ */ ++ /* For disks, the device name can be used directly. */ + for (i = 0; i < domDef->ndisks; i++) { -+ disk = domDef->disks[i]; ++ virDomainDiskDefPtr disk = domDef->disks[i]; + if (STREQ(dev->data.disk->dst, disk->dst)) { -+ tmp = virStrcpy(ref, disk->dst, ref_len); -+ if (tmp == NULL) ++ if (virStrcpy(ref, disk->dst, ref_len) == NULL) + return -1; + else + return 0; @@ -108,3 +120,92 @@ Index: libvirt-1.0.2/src/xen/xend_internal.c } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { char mac[30]; virDomainNetDefPtr def = dev->data.net; +@@ -3965,17 +3966,22 @@ virDomainXMLDevID(virDomainPtr domain, + + strcpy(class, "vif"); + +- xenUnifiedLock(priv); +- xref = xenStoreDomainGetNetworkID(domain->conn, domain->id, +- mac); +- xenUnifiedUnlock(priv); +- if (xref == NULL) +- return -1; +- +- tmp = virStrcpy(ref, xref, ref_len); +- VIR_FREE(xref); +- if (tmp == NULL) +- return -1; ++ /* For nics, the mac address can be used directly. */ ++ for (i = 0; i < domDef->nnets; i++) { ++ char dst_mac[30]; ++ virDomainNetDefPtr dst_net = domDef->nets[i]; ++ snprintf(dst_mac, sizeof(dst_mac), "%02x:%02x:%02x:%02x:%02x:%02x", ++ dst_net->mac.addr[0], dst_net->mac.addr[1], ++ dst_net->mac.addr[2], dst_net->mac.addr[3], ++ dst_net->mac.addr[4], dst_net->mac.addr[5]); ++ if (STREQ(mac, dst_mac)) { ++ if (virStrcpy(ref, dst_mac, ref_len) == NULL) ++ return -1; ++ else ++ return 0; ++ } ++ } ++ return -1; + } 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) { +@@ -3993,17 +3999,44 @@ virDomainXMLDevID(virDomainPtr domain, + + strcpy(class, "pci"); + +- xenUnifiedLock(priv); +- xref = xenStoreDomainGetPCIID(domain->conn, domain->id, bdf); +- xenUnifiedUnlock(priv); +- VIR_FREE(bdf); +- if (xref == NULL) +- return -1; ++ /* For PCI devices, the device BFD can be used directly. */ ++ for (i = 0 ; i < domDef->nhostdevs ; i++) { ++ char *dst_bdf; ++ virDomainHostdevDefPtr hostdev = domDef->hostdevs[i]; ++ ++ if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) ++ continue; ++ if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) ++ continue; ++ ++ if (virAsprintf(&dst_bdf, "%04x:%02x:%02x.%0x", ++ hostdev->source.subsys.u.pci.domain, ++ hostdev->source.subsys.u.pci.bus, ++ hostdev->source.subsys.u.pci.slot, ++ hostdev->source.subsys.u.pci.function) < 0) { ++ virReportOOMError(); ++ VIR_FREE(bdf); ++ return -1; ++ } + +- tmp = virStrcpy(ref, xref, ref_len); +- VIR_FREE(xref); +- if (tmp == NULL) +- return -1; ++ if (STREQ(bdf, dst_bdf)) { ++ if (virStrcpy(ref, dst_bdf, ref_len) == NULL) { ++ virReportOOMError(); ++ VIR_FREE(dst_bdf); ++ VIR_FREE(bdf); ++ return -1; ++ } ++ else { ++ VIR_FREE(dst_bdf); ++ VIR_FREE(bdf); ++ return 0; ++ } ++ } ++ VIR_FREE(dst_bdf); ++ } ++ ++ VIR_FREE(bdf); ++ return -1; + } else { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("hotplug of device type not supported"));