When we try to detach a network device without specifying the mac address, random mac address is generated. As the generated mac address will not be available in the running vm, detaching device will fail erroring out "error: operation failed: no device matching mac address xx:xx:xx:xx:xx:xx found". This patch allows to match DetachDeviec xml using PCI address when mac address is not specified. Signed-off-by: Kothapally Madhu Pavan <kmp(a)linux.vnet.ibm.com&gt; --- src/conf/domain_conf.c | 116 +++++++++++++++++++++++++++++----------------- src/conf/domain_conf.h | 6 ++ src/libxl/libxl_driver.c | 4 +- src/lxc/lxc_driver.c | 6 +- src/qemu/qemu_driver.c | 40 ++++++++++++++-- src/qemu/qemu_hotplug.c | 16 ++++-- src/qemu/qemu_hotplug.h | 3 + 7 files changed, 132 insertions(+), 59 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 568c699..f07f178 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13613,14 +13613,20 @@ int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net) return 0; } -/* virDomainNetFindIdx: search according to mac address and guest side - * PCI address (if specified) +/* virDomainNetFindIdx: + * search according to mac address and guest side PCI address (if specified). + * + * @def: pointer to domain definition + * @net: pointer to network definition that has to be looked up for + * @MACAddrNotSpecified: when detaching a device it takes "true" when mac address is + * not specified in device xml and a random mac is generated which + * cannot be found in the domain def. * * Return: index of match if unique match found * -1 otherwise and an error is logged */ int -virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net) +virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net, bool MACAddrNotSpecified) { size_t i; int matchidx = -1; @@ -13628,50 +13634,76 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net) bool PCIAddrSpecified = virDomainDeviceAddressIsValid(&net->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI); - for (i = 0; i < def->nnets; i++) { - if (virMacAddrCmp(&def->nets[i]->mac, &net->mac)) - continue; - - if ((matchidx >= 0) && !PCIAddrSpecified) { - /* there were multiple matches on mac address, and no - * qualifying guest-side PCI address was given, so we must - * fail (NB: a USB address isn't adequate, since it may - * specify only vendor and product ID, and there may be - * multiples of those. - */ - virReportError(VIR_ERR_OPERATION_FAILED, - _("multiple devices matching mac address %s found"), - virMacAddrFormat(&net->mac, mac)); - return -1; - } + /* MAC address is not specified in device xml. So, we need not check for it. + * Here we check for matching PCI address if specified. */ + if (MACAddrNotSpecified) { if (PCIAddrSpecified) { - if (virPCIDeviceAddressEqual(&def->nets[i]->info.addr.pci, - &net->info.addr.pci)) { - /* exit early if the pci address was specified and - * it matches, as this guarantees no duplicates. + for (i = 0; i < def->nnets; i++) { + if (virPCIDeviceAddressEqual(&def->nets[i]->info.addr.pci, + &net->info.addr.pci)) { + matchidx = i; + break; + } + } + if (matchidx < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("no device found on " + "%.4x:%.2x:%.2x.%.1x"), + net->info.addr.pci.domain, + net->info.addr.pci.bus, + net->info.addr.pci.slot, + net->info.addr.pci.function); + } + } else { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("mac address and pci address not specified in device xml")); + } + } else { + for (i = 0; i < def->nnets; i++) { + if (virMacAddrCmp(&def->nets[i]->mac, &net->mac)) + continue; + + if ((matchidx >= 0) && !PCIAddrSpecified) { + /* there were multiple matches on mac address, and no + * qualifying guest-side PCI address was given, so we must + * fail (NB: a USB address isn't adequate, since it may + * specify only vendor and product ID, and there may be + * multiples of those. */ + virReportError(VIR_ERR_OPERATION_FAILED, + _("multiple devices matching mac address %s found"), + virMacAddrFormat(&net->mac, mac)); + return -1; + } + if (PCIAddrSpecified) { + if (virPCIDeviceAddressEqual(&def->nets[i]->info.addr.pci, + &net->info.addr.pci)) { + /* exit early if the pci address was specified and + * it matches, as this guarantees no duplicates. + */ + matchidx = i; + break; + } + } else { + /* no PCI address given, so there may be multiple matches */ matchidx = i; - break; } - } else { - /* no PCI address given, so there may be multiple matches */ - matchidx = i; } - } - if (matchidx < 0) { - if (PCIAddrSpecified) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("no device matching mac address %s found on " - "%.4x:%.2x:%.2x.%.1x"), - virMacAddrFormat(&net->mac, mac), - net->info.addr.pci.domain, - net->info.addr.pci.bus, - net->info.addr.pci.slot, - net->info.addr.pci.function); - } else { - virReportError(VIR_ERR_OPERATION_FAILED, - _("no device matching mac address %s found"), - virMacAddrFormat(&net->mac, mac)); + if (matchidx < 0) { + if (PCIAddrSpecified) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("no device matching mac address %s found on " + "%.4x:%.2x:%.2x.%.1x"), + virMacAddrFormat(&net->mac, mac), + net->info.addr.pci.domain, + net->info.addr.pci.bus, + net->info.addr.pci.slot, + net->info.addr.pci.function); + } else { + virReportError(VIR_ERR_OPERATION_FAILED, + _("no device matching mac address %s found"), + virMacAddrFormat(&net->mac, mac)); + } } } return matchidx; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b1953b3..d3ebcdc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2581,6 +2581,8 @@ typedef enum { VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS = 1 << 8, /* allow updates in post parse callback that would break ABI otherwise */ VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 9, + /* indicates mac address is generated when not provided in device xml */ + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR = 1 << 10, } virDomainDefParseFlags; typedef enum { @@ -2710,7 +2712,9 @@ int virDomainDiskSourceParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virStorageSourcePtr src); -int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net); +int virDomainNetFindIdx(virDomainDefPtr def, + virDomainNetDefPtr net, + bool MACAddrNotSpecified); virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device); bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net); int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 95dfc01..1c53c5f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3422,7 +3422,7 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, libxl_device_nic_init(&nic); - if ((detachidx = virDomainNetFindIdx(vm->def, net)) < 0) + if ((detachidx = virDomainNetFindIdx(vm->def, net, false)) < 0) goto cleanup; detach = vm->def->nets[detachidx]; @@ -3521,7 +3521,7 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) case VIR_DOMAIN_DEVICE_NET: net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + if ((idx = virDomainNetFindIdx(vmdef, net, false)) < 0) return -1; /* this is guaranteed to succeed */ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 67f14fe..3d1396a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3929,7 +3929,7 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef, switch (dev->type) { case VIR_DOMAIN_DEVICE_NET: net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + if ((idx = virDomainNetFindIdx(vmdef, net, false)) < 0) goto cleanup; virDomainNetDefFree(vmdef->nets[idx]); @@ -3975,7 +3975,7 @@ lxcDomainDetachDeviceConfig(virDomainDefPtr vmdef, case VIR_DOMAIN_DEVICE_NET: net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + if ((idx = virDomainNetFindIdx(vmdef, net, false)) < 0) goto cleanup; /* this is guaranteed to succeed */ @@ -4759,7 +4759,7 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm, virDomainNetDefPtr detach = NULL; virNetDevVPortProfilePtr vport = NULL; - if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) + if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net, false)) < 0) goto cleanup; detach = vm->def->nets[detachidx]; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10d3e3d..e6c811c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4105,6 +4105,31 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, virObjectUnref(cfg); } +static bool qemuIsMacAvailable(const char *xmlStr) +{ + xmlNodePtr cur; + xmlXPathContextPtr ctxt = NULL; + + if (!virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt)) + goto endjob; + + cur = ctxt->node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "mac")) + goto cleanup; + } + cur = cur->next; + } + + endjob: + xmlXPathFreeContext(ctxt); + return false; + + cleanup: + xmlXPathFreeContext(ctxt); + return true; +} static void syncNicRxFilterMacAddr(char *ifname, virNetDevRxFilterPtr guestFilter, @@ -7587,7 +7612,8 @@ qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver, static int qemuDomainDetachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, - virDomainPtr dom) + virDomainPtr dom, + unsigned int parse_flags) { virQEMUDriverPtr driver = dom->conn->privateData; int ret = -1; @@ -7603,7 +7629,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, ret = qemuDomainDetachLease(driver, vm, dev->data.lease); break; case VIR_DOMAIN_DEVICE_NET: - ret = qemuDomainDetachNetDevice(driver, vm, dev); + ret = qemuDomainDetachNetDevice(driver, vm, dev, parse_flags); break; case VIR_DOMAIN_DEVICE_HOSTDEV: ret = qemuDomainDetachHostDevice(driver, vm, dev); @@ -7932,6 +7958,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainChrDefPtr chr; virDomainFSDefPtr fs; int idx; + bool skipMac = parse_flags & VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR ? true : false; switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -7946,7 +7973,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, case VIR_DOMAIN_DEVICE_NET: net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + if ((idx = virDomainNetFindIdx(vmdef, net, skipMac)) < 0) return -1; /* this is guaranteed to succeed */ @@ -8115,7 +8142,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef, case VIR_DOMAIN_DEVICE_NET: net = dev->data.net; - if ((pos = virDomainNetFindIdx(vmdef, net)) < 0) + if ((pos = virDomainNetFindIdx(vmdef, net, false)) < 0) return -1; virDomainNetDefFree(vmdef->nets[pos]); @@ -8452,6 +8479,9 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; + if (!qemuIsMacAvailable(xml)) + parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR; + dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); @@ -8495,7 +8525,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) goto endjob; - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0) + if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom, parse_flags)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6ce0a84..2bf82b4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2163,7 +2163,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, int ret = -1; int changeidx = -1; - if ((changeidx = virDomainNetFindIdx(vm->def, newdev)) < 0) + if ((changeidx = virDomainNetFindIdx(vm->def, newdev, false)) < 0) goto cleanup; devslot = &vm->def->nets[changeidx]; @@ -3825,7 +3825,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, * function so that mac address / virtualport are reset */ if (detach->parent.type == VIR_DOMAIN_DEVICE_NET) - return qemuDomainDetachNetDevice(driver, vm, &detach->parent); + return qemuDomainDetachNetDevice(driver, vm, &detach->parent, 0); else return qemuDomainDetachThisHostDevice(driver, vm, detach); } @@ -3833,14 +3833,20 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, int qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev) + virDomainDeviceDefPtr dev, + unsigned int parse_flags) { int detachidx, ret = -1; virDomainNetDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) - goto cleanup; + if (parse_flags & VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR) { + if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net, true)) < 0) + goto cleanup; + } else { + if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net, false)) < 0) + goto cleanup; + } detach = vm->def->nets[detachidx]; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 165d345..f144879 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -82,7 +82,8 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, virDomainDeviceDefPtr dev); int qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev); + virDomainDeviceDefPtr dev, + unsigned int parse_flags); int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev);