When we hot-plug a disk device into a qemu guest, we need to retain its PCI address so that it can be removed again later. Currently, we do retain the slot number, but not across libvirtd restarts. Add <state devaddr="xxxx:xx:xx"/> to the disk device XML config when the VIR_DOMAIN_XML_INTERNAL_STATUS flag is used. We still don't parse the domain and bus number, but the format allows us to do that in future. * src/domain_conf.h: replace slotnum with pci_addr * src/domain_conf.c: handle formatting and parsing the address * src/qemu_driver.c: store the parsed slot number as a full PCI address, and use this address with the pci_del monitor command * src/vbox/vbox_tmpl.c: we're debug printing slotnum here even though it can never be set, just delete it --- src/domain_conf.c | 22 +++++++++++++++++++--- src/domain_conf.h | 2 +- src/qemu_driver.c | 42 +++++++++++++++++++++++++++++------------- src/vbox/vbox_tmpl.c | 1 - 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 10e6ac6..16f7d73 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -287,6 +287,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->dst); VIR_FREE(def->driverName); VIR_FREE(def->driverType); + VIR_FREE(def->pci_addr); VIR_FREE(def); } @@ -643,7 +644,7 @@ int virDomainDiskCompare(virDomainDiskDefPtr a, static virDomainDiskDefPtr virDomainDiskDefParseXML(virConnectPtr conn, xmlNodePtr node, - int flags ATTRIBUTE_UNUSED) { + int flags) { virDomainDiskDefPtr def; xmlNodePtr cur; char *type = NULL; @@ -654,6 +655,7 @@ virDomainDiskDefParseXML(virConnectPtr conn, char *target = NULL; char *bus = NULL; char *cachetag = NULL; + char *pci_addr = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(conn); @@ -708,6 +710,9 @@ virDomainDiskDefParseXML(virConnectPtr conn, def->readonly = 1; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { def->shared = 1; + } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && + xmlStrEqual(cur->name, BAD_CAST "state")) { + pci_addr = virXMLPropString(cur, "devaddr"); } } cur = cur->next; @@ -815,6 +820,8 @@ virDomainDiskDefParseXML(virConnectPtr conn, driverName = NULL; def->driverType = driverType; driverType = NULL; + def->pci_addr = pci_addr; + pci_addr = NULL; cleanup: VIR_FREE(bus); @@ -825,6 +832,7 @@ cleanup: VIR_FREE(driverType); VIR_FREE(driverName); VIR_FREE(cachetag); + VIR_FREE(pci_addr); return def; @@ -3388,7 +3396,8 @@ virDomainLifecycleDefFormat(virConnectPtr conn, static int virDomainDiskDefFormat(virConnectPtr conn, virBufferPtr buf, - virDomainDiskDefPtr def) + virDomainDiskDefPtr def, + int flags) { const char *type = virDomainDiskTypeToString(def->type); const char *device = virDomainDiskDeviceTypeToString(def->device); @@ -3446,6 +3455,13 @@ virDomainDiskDefFormat(virConnectPtr conn, if (def->shared) virBufferAddLit(buf, " <shareable/>\n"); + if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && def->pci_addr) { + virBufferAddLit(buf, " <state"); + if (def->pci_addr) + virBufferEscapeString(buf, " devaddr='%s'", def->pci_addr); + virBufferAddLit(buf, "/>\n"); + } + virBufferAddLit(buf, " </disk>\n"); return 0; @@ -4048,7 +4064,7 @@ char *virDomainDefFormat(virConnectPtr conn, def->emulator); for (n = 0 ; n < def->ndisks ; n++) - if (virDomainDiskDefFormat(conn, &buf, def->disks[n]) < 0) + if (virDomainDiskDefFormat(conn, &buf, def->disks[n], flags) < 0) goto cleanup; for (n = 0 ; n < def->nfss ; n++) diff --git a/src/domain_conf.h b/src/domain_conf.h index 6e111fa..1766b61 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -106,7 +106,7 @@ struct _virDomainDiskDef { int cachemode; unsigned int readonly : 1; unsigned int shared : 1; - int slotnum; /* pci slot number for unattach */ + char *pci_addr; /* for detach */ }; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 00dc6e5..3dec630 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -4390,6 +4390,8 @@ try_command: return -1; } + VIR_FREE(cmd); + DEBUG ("%s: pci_add reply: %s", vm->def->name, reply); /* If the command succeeds qemu prints: * OK bus 0, slot XXX... @@ -4399,22 +4401,28 @@ try_command: if ((s = strstr(reply, "OK ")) && (s = strstr(s, "slot "))) { char *dummy = s; + int domain, bus, slot; + s += strlen("slot "); - if (virStrToLong_i ((const char*)s, &dummy, 10, &dev->data.disk->slotnum) == -1) - VIR_WARN("%s", _("Unable to parse slot number\n")); /* XXX not neccessarily always going to end up in domain 0 / bus 0 :-( */ - /* XXX this slotnum is not persistant across restarts :-( */ + domain = bus = 0; + if (virStrToLong_i ((const char*)s, &dummy, 10, &slot) == -1) + VIR_WARN("%s", _("Unable to parse slot number\n")); + else if (virAsprintf(&dev->data.disk->pci_addr, "%.4x:%.2x:%.2x", + domain, bus, slot) < 0) { + virReportOOMError(conn); + VIR_FREE(reply); + return -1; + } } else if (!tryOldSyntax && strstr(reply, "invalid char in expression")) { VIR_FREE(reply); - VIR_FREE(cmd); tryOldSyntax = 1; goto try_command; } else { qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("adding %s disk failed: %s"), type, reply); VIR_FREE(reply); - VIR_FREE(cmd); return -1; } @@ -4423,7 +4431,7 @@ try_command: virDomainDiskQSort); VIR_FREE(reply); - VIR_FREE(cmd); + return 0; } @@ -4660,21 +4668,29 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, goto cleanup; } - if (detach->slotnum < 1) { + if (!detach->pci_addr) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("disk %s cannot be detached - invalid slot number %d"), - detach->dst, detach->slotnum); + _("disk %s cannot be detached - no PCI address for device"), + detach->dst); goto cleanup; } try_command: if (tryOldSyntax) { - if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) { + const char *slot = strrchr(detach->pci_addr, ':'); + if (!slot) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("Invalid PCI address for device '%s' : %s"), + detach->dst, detach->pci_addr); + goto cleanup; + } + ++slot; + if (virAsprintf(&cmd, "pci_del 0 %s", slot) < 0) { virReportOOMError(conn); goto cleanup; } } else { - if (virAsprintf(&cmd, "pci_del pci_addr=0:0:%d", detach->slotnum) < 0) { + if (virAsprintf(&cmd, "pci_del pci_addr=%s", detach->pci_addr) < 0) { virReportOOMError(conn); goto cleanup; } @@ -4698,8 +4714,8 @@ try_command: if (strstr(reply, "invalid slot") || strstr(reply, "Invalid pci address")) { qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("failed to detach disk %s: invalid slot %d: %s"), - detach->dst, detach->slotnum, reply); + _("failed to detach disk %s: invalid PCI address %s: %s"), + detach->dst, detach->pci_addr, reply); goto cleanup; } diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 3208f03..a2b958a 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2712,7 +2712,6 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) { DEBUG("disk(%d) cachemode: %d", i, def->disks[i]->cachemode); DEBUG("disk(%d) readonly: %s", i, def->disks[i]->readonly ? "True" : "False"); DEBUG("disk(%d) shared: %s", i, def->disks[i]->shared ? "True" : "False"); - DEBUG("disk(%d) slotnum: %d", i, def->disks[i]->slotnum); if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE) { -- 1.6.2.5 -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list