[libvirt] [PATCH RFC] libxl: reverse defaults on HVM net device attach

[Joao Martins]

On 12/09/2016 11:35 AM, Joao Martins wrote:
> libvirt libxl picks its own default with respect to the default NIC
> to use. libxlMakeNic is the one responsible for this and on boot it
> picks LIBXL_NIC_TYPE_VIF_IOEMU such that it accomodates both PV and
> emulated one. The good behaving guest at boot will then select the pv
> and unplug the emulated device.
Short note, in this paragraph I meant for HVM guest. For PV this isn't
applicable. I can correct the commit message and resend after
comments (depending ofc if folks think this is right way to go).

> Now, on HVM when attaching an interface it will pick the same default
> that is LIBXL_NIC_TYPE_VIF_IOEMU which as a result will fail the attach
> (see xen commit 32e9d0f ("libxl: nic type defaults to vif in hotplug for
> hvm guest"). Xen doesn't yet support the hotplug of emulated devices,
> but we don't want to rule out that case either, which might get support
> in the future. Hence we simply reverse the defaults when we are
> attaching the interface which allows libvirt to prefer the PV nic first
> without adding "model='netfront'" following the same pattern as above
> commit. Also to avoid ruling out the emulated one we set to
> LIBXL_NIC_TYPE_IOEMU when setting a model type that is not 'netfront'.
> 
> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
> ---
> 
> This allows Openstack to attach network interfaces, which currently
> is broken on libxl if it doesn't set model 'netfront'. I am not sure
> whether this is the best way (hence RFC) or if users should really be
> setting the model='netfront' when attaching devices. But sounds to me
> that it would be better to have by default the supported, and if users
> want emulated attach to specify a nic model. Thoughts?
> ---
>  src/libxl/libxl_conf.c   | 9 ++++++---
>  src/libxl/libxl_conf.h   | 3 ++-
>  src/libxl/libxl_driver.c | 2 +-
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index dcf8e7e..50aa958 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -881,9 +881,10 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
>  int
>  libxlMakeNic(virDomainDefPtr def,
>               virDomainNetDefPtr l_nic,
> -             libxl_device_nic *x_nic)
> +             libxl_device_nic *x_nic,
> +             bool attach)
>  {
> -    bool ioemu_nic = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
> +    bool ioemu_nic = def->os.type == VIR_DOMAIN_OSTYPE_HVM && !attach;
>      virDomainNetType actual_type = virDomainNetGetActualType(l_nic);
>      virNetworkPtr network = NULL;
>      virConnectPtr conn = NULL;
> @@ -917,6 +918,8 @@ libxlMakeNic(virDomainDefPtr def,
>              goto cleanup;
>          if (STREQ(l_nic->model, "netfront"))
>              x_nic->nictype = LIBXL_NIC_TYPE_VIF;
> +        else
> +            x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
>      }
>  
>      if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
> @@ -1047,7 +1050,7 @@ libxlMakeNicList(virDomainDefPtr def,  libxl_domain_config *d_config)
>          if (virDomainNetGetActualType(l_nics[i]) == VIR_DOMAIN_NET_TYPE_HOSTDEV)
>              continue;
>  
> -        if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics]))
> +        if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics], false))
>              goto error;
>          /*
>           * The devid (at least right now) will not get initialized by
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 0ea76b4..851f3af 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -178,7 +178,8 @@ libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev);
>  int
>  libxlMakeNic(virDomainDefPtr def,
>               virDomainNetDefPtr l_nic,
> -             libxl_device_nic *x_nic);
> +             libxl_device_nic *x_nic,
> +             bool attach);
>  int
>  libxlMakeVfb(virPortAllocatorPtr graphicsports,
>               virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb);
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index b19b17e..7e5d9b6 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3379,7 +3379,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
>          goto cleanup;
>      }
>  
> -    if (libxlMakeNic(vm->def, net, &nic) < 0)
> +    if (libxlMakeNic(vm->def, net, &nic, true) < 0)
>          goto cleanup;
>  
>      if (libxl_device_nic_add(cfg->ctx, vm->def->id, &nic, 0)) {
>