[libvirt] [PATCH 07/10] libxl: prevent attaching multiple netdevs with the same MAC

[Jim Fehlig]

Marek Marczykowski-Górecki wrote:
> It will not be possible to detach such device later. Also improve
> logging in such cases.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek at invisiblethingslab.com>
> ---
>  src/libxl/libxl_driver.c | 41 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index ce3a99b..005cc96 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -2787,6 +2787,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
>      int actualType;
>      libxl_device_nic nic;
>      int ret = -1;
> +    char mac[VIR_MAC_STRING_BUFLEN];
>  
>      /* preallocate new slot for device */
>      if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0)
> @@ -2801,6 +2802,14 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
>  
>      actualType = virDomainNetGetActualType(net);
>  
> +    /* -2 means "multiple matches" so then fail also */
>   

No longer true after commit 2fbae1b2.  I think you just want to check if
virDomainNetFindIdx() >= 0, meaning the def already contains a net
device with the same mac address.

> +    if (virDomainNetFindIdx(vm->def, net) != -1) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                _("device matching mac address %s already exists"),
> +                virMacAddrFormat(&net->mac, mac));
> +        return -1;
> +    }
> +
>      if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>          /* This is really a "smart hostdev", so it should be attached
>           * as a hostdev (the hostdev code will reach over into the
> @@ -2879,6 +2888,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
>      virDomainHostdevDefPtr hostdev;
>      virDomainHostdevDefPtr found;
>      virDomainHostdevSubsysPCIPtr pcisrc;
> +    char mac[VIR_MAC_STRING_BUFLEN];
>  
>      switch (dev->type) {
>          case VIR_DOMAIN_DEVICE_DISK:
> @@ -2896,6 +2906,12 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
>  
>          case VIR_DOMAIN_DEVICE_NET:
>              net = dev->data.net;
> +            if (virDomainNetFindIdx(vmdef, net) >= 0) {
> +                virReportError(VIR_ERR_INVALID_ARG,
> +                               _("network device with mac %s already exists."),
> +                               virMacAddrFormat(&net->mac, mac));
> +                return -1;
> +            }
>   

Ah, like you've done here :-).

>              if (virDomainNetInsert(vmdef, net))
>                  return -1;
>              dev->data.net = NULL;
> @@ -3060,8 +3076,18 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
>      char mac[VIR_MAC_STRING_BUFLEN];
>      int ret = -1;
>  
> -    if ((detachidx = virDomainNetFindIdx(vm->def, net)) < 0)
> +    if ((detachidx = virDomainNetFindIdx(vm->def, net)) < 0) {
> +        if (detachidx == -2) {
> +            virReportError(VIR_ERR_OPERATION_FAILED,
> +                    _("multiple devices matching mac address %s found"),
> +                    virMacAddrFormat(&net->mac, mac));
> +        } else {
> +            virReportError(VIR_ERR_OPERATION_FAILED,
> +                    _("network device %s not found"),
> +                    virMacAddrFormat(&net->mac, mac));
> +        }
>   

virDomainNetFindIdx() handles the error reporting now.

>          return -1;
> +    }
>  
>      detach = vm->def->nets[detachidx];
>  
> @@ -3136,6 +3162,7 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
>      virDomainHostdevDefPtr hostdev, det_hostdev;
>      virDomainNetDefPtr net;
>      int idx;
> +    char mac[VIR_MAC_STRING_BUFLEN];
>  
>      switch (dev->type) {
>          case VIR_DOMAIN_DEVICE_DISK:
> @@ -3150,8 +3177,18 @@ 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)) < 0) {
> +                if (idx == -2) {
> +                    virReportError(VIR_ERR_OPERATION_FAILED,
> +                            _("multiple devices matching mac address %s found"),
> +                            virMacAddrFormat(&dev->data.net->mac, mac));
> +                } else {
> +                    virReportError(VIR_ERR_OPERATION_FAILED,
> +                            _("network device %s not found"),
> +                            virMacAddrFormat(&dev->data.net->mac, mac));
> +                }
>   

Same here.

Regards,
Jim