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

[John Ferlan]

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 at redhat.com>
> ---
>  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 ?

> +            }
> +
>              return -1;
>          }
>          if (PCIAddrSpecified) {
> @@ -15679,8 +15693,9 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net)
>              matchidx = i;
>          }
>      }
> +
>      if (matchidx < 0) {
> -        if (PCIAddrSpecified) {
> +        if (MACAddrSpecified && PCIAddrSpecified) {
>              virReportError(VIR_ERR_OPERATION_FAILED,
>                             _("no device matching mac address %s found on "
>                               "%.4x:%.2x:%.2x.%.1x"),
> @@ -15689,10 +15704,20 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net)
>                             net->info.addr.pci.bus,
>                             net->info.addr.pci.slot,
>                             net->info.addr.pci.function);
> -        } else {
> +        } else if (PCIAddrSpecified) {
> +            virReportError(VIR_ERR_OPERATION_FAILED,
> +                           _("no device found on %.4x:%.2x:%.2x.%.1x"), 

no device using generated mac address found on ... ?

> +                           net->info.addr.pci.domain,
> +                           net->info.addr.pci.bus,
> +                           net->info.addr.pci.slot,
> +                           net->info.addr.pci.function);
> +        } else if (MACAddrSpecified) {
>              virReportError(VIR_ERR_OPERATION_FAILED,
>                             _("no device matching mac address %s found"),
>                             virMacAddrFormat(&net->mac, mac));
> +        } else {
> +            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                           _("no matching device found"));

no device using generated mac address found ?

>          }
>      }
>      return matchidx;
>