Re: [libvirt] [PATCH 3/3] virDomainNetFindIdx: Ignore auto generated MAC addresses

On 10/02/2017 07:01 AM, Michal Privoznik wrote: > When detaching an <interface/> from domain, it's MAC address is s/from domain/from a domain/ s/it's/the > parsed and if not present one is generated. If, however, no s/, however,// > corresponding interface is found in the domain, the following > error is reported: > > error: operation failed: no device matching mac address 52:54:00:75:32:5b found > > where the MAC address is the auto generated one. This might be > very confusing. Solution to this is to ignore auto generated MAC > address when looking up the device. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/conf/domain_conf.c | 45 +++++++++++++++++++++++++++++++++++---------- > 1 file changed, 35 insertions(+), 10 deletions(-) > I had looked at this series too, but forgot to send this when something else caused me to reboot.... Patch 1 and 2 I had no comments... My primary comment here is should the new message generated in some way indicate that it's the generated mac... It's no big deal, but just a thought and may help compared to having to look for the generic message. > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 87192eb2d..aab43d307 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -15634,11 +15634,17 @@ int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net) > return 0; > } > > -/* virDomainNetFindIdx: search according to mac address and guest side > - * PCI address (if specified) > +/** > + * virDomainNetFindIdx: > + * @def: domain definition > + * @net: interface definition > * > - * Return: index of match if unique match found > - * -1 otherwise and an error is logged > + * Lookup domain's network interface based on passed @net > + * definition. If @net's MAC address was auto generated, > + * the MAC comparison is ignored. > + * > + * Return: index of match if unique match found, > + * -1 otherwise and an error is logged. > */ > int > virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net) > @@ -15646,11 +15652,13 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net) > size_t i; > int matchidx = -1; > char mac[VIR_MAC_STRING_BUFLEN]; > + bool MACAddrSpecified = !net->mac.generated; > 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)) > + if (MACAddrSpecified && > + virMacAddrCmp(&def->nets[i]->mac, &net->mac) != 0) > continue; > > if ((matchidx >= 0) && !PCIAddrSpecified) { > @@ -15660,9 +15668,15 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net) > * 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)); > + if (MACAddrSpecified) { > + virReportError(VIR_ERR_OPERATION_FAILED, > + _("multiple devices matching mac address %s found"), > + virMacAddrFormat(&net->mac, mac)); > + } else { > + virReportError(VIR_ERR_OPERATION_FAILED, "%s", > + _("multiple matching devices found")); multiple devices matching generated mac address found ?