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. 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. 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.5/src/xen/xend_internal.c =================================================================== --- libvirt-1.0.5.orig/src/xen/xend_internal.c +++ libvirt-1.0.5/src/xen/xend_internal.c @@ -60,8 +60,8 @@ #define XEND_RCV_BUF_MAX_LEN (256 * 1024) static int -virDomainXMLDevID(virDomainPtr domain, virDomainDeviceDefPtr dev, char *class, - char *ref, int ref_len); +virDomainXMLDevID(virDomainPtr domain, virDomainDefPtr domDef, + virDomainDeviceDefPtr dev, char *class, char *ref, int ref_len); /** * do_connect: @@ -2586,7 +2586,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr sexpr = virBufferContentAndReset(&buf); - if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref))) { + if (virDomainXMLDevID(domain, def, dev, class, ref, sizeof(ref))) { /* device doesn't exist, define it */ ret = xend_op(domain->conn, domain->name, "op", "device_create", "config", sexpr, NULL); @@ -2701,7 +2701,7 @@ xenDaemonUpdateDeviceFlags(virDomainPtr sexpr = virBufferContentAndReset(&buf); - if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref))) { + if (virDomainXMLDevID(domain, def, dev, class, ref, sizeof(ref))) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("requested device does not exist")); goto cleanup; @@ -2790,7 +2790,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref))) + if (virDomainXMLDevID(domain, def, dev, class, ref, sizeof(ref))) goto cleanup; if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { @@ -3682,31 +3682,32 @@ struct xenUnifiedDriver xenDaemonDriver /** * virDomainXMLDevID: * @domain: pointer to domain object + * @domDef: pointer to domain definition object * @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, + virDomainDefPtr domDef, virDomainDeviceDefPtr dev, char *class, char *ref, int ref_len) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; - char *xref; - char *tmp; + unsigned int i; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->dst == NULL) + return -1; if (dev->data.disk->driverName && STREQ(dev->data.disk->driverName, "tap")) strcpy(class, "tap"); @@ -3716,19 +3717,17 @@ virDomainXMLDevID(virDomainPtr domain, else strcpy(class, "vbd"); - if (dev->data.disk->dst == NULL) - return -1; - xenUnifiedLock(priv); - xref = xenStoreDomainGetDiskID(domain->conn, domain->id, - dev->data.disk->dst); - xenUnifiedUnlock(priv); - if (xref == NULL) - return -1; - - tmp = virStrcpy(ref, xref, ref_len); - VIR_FREE(xref); - if (tmp == NULL) - return -1; + /* For disks, the device name can be used directly. */ + for (i = 0; i < domDef->ndisks; i++) { + virDomainDiskDefPtr disk = domDef->disks[i]; + if (STREQ(dev->data.disk->dst, disk->dst)) { + if (virStrcpy(ref, disk->dst, ref_len) == NULL) + return -1; + else + return 0; + } + } + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { char mac[VIR_MAC_STRING_BUFLEN]; virDomainNetDefPtr def = dev->data.net; @@ -3736,16 +3735,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) { @@ -3763,17 +3768,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.addr.domain, + hostdev->source.subsys.u.pci.addr.bus, + hostdev->source.subsys.u.pci.addr.slot, + hostdev->source.subsys.u.pci.addr.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"));