[libvirt] [PATCH v2 04/11] conf: net: Add model enum, and netfront value

[Michal Privoznik]

On 3/13/19 4:51 PM, Cole Robinson wrote:
> This adds a network model enum. The virDomainNetDef property
> is named 'model' like most other devices.
> 
> When the XML parser or a driver calls NetSetModelString, if
> the passed string is in the enum, we will set net->model,
> otherwise we copy the string into net->modelstr
> 
> Add a single example for the 'netfront' xen model, and wire
> that up, just to verify it's all working
> 
> Signed-off-by: Cole Robinson <crobinso at redhat.com>
> ---
>   src/conf/domain_conf.c     | 55 +++++++++++++++++++++++++++-----------
>   src/conf/domain_conf.h     | 10 +++++++
>   src/libvirt_private.syms   |  2 ++
>   src/libxl/libxl_conf.c     |  4 +--
>   src/qemu/qemu_hotplug.c    |  8 ++++++
>   src/xenconfig/xen_common.c | 16 +++++------
>   src/xenconfig/xen_sxpr.c   | 15 ++++++-----
>   7 files changed, 78 insertions(+), 32 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index fe1945b644..571f2eea39 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -472,6 +472,11 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
>                 "udp",
>   );
>   
> +VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST,

This is supposed to be on a separate line now ;-)

> +              "unknown",
> +              "netfront",
> +);
> +
>   VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST,
>                 "default",
>                 "qemu",
> @@ -2228,6 +2233,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
>           return;
>   
>       VIR_FREE(def->modelstr);
> +    def->model = VIR_DOMAIN_NET_MODEL_UNKNOWN;
>   
>       switch (def->type) {
>       case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
> @@ -11483,20 +11489,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>               goto error;
>       }
>   
> -    /* NIC model (see -net nic,model=?).  We only check that it looks
> -     * reasonable, not that it is a supported NIC type.  FWIW kvm
> -     * supports these types as of April 2008:
> -     * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio
> -     * QEMU PPC64 supports spapr-vlan
> -     */
> -    if (model != NULL) {
> -        if (strspn(model, NET_MODEL_CHARS) < strlen(model)) {
> -            virReportError(VIR_ERR_INVALID_ARG, "%s",
> -                           _("Model name contains invalid characters"));
> -            goto error;
> -        }
> -        VIR_STEAL_PTR(def->modelstr, model);
> -    }
> +    if (model != NULL &&
> +        virDomainNetSetModelString(def, model) < 0)
> +        goto error;
>   
>       switch (def->type) {
>       case VIR_DOMAIN_NET_TYPE_NETWORK:
> @@ -21739,6 +21734,14 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src,
>           return false;
>       }
>   
> +    if (src->model != dst->model) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Target network card model %s does not match source %s"),
> +                       virDomainNetModelTypeToString(dst->model),
> +                       virDomainNetModelTypeToString(src->model));
> +        return false;
> +    }
> +
>       if (src->mtu != dst->mtu) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                          _("Target network card MTU %d does not match source %d"),
> @@ -29379,6 +29382,8 @@ virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface)
>   const char *
>   virDomainNetGetModelString(const virDomainNetDef *net)
>   {
> +    if (net->model)
> +        return virDomainNetModelTypeToString(net->model);
>       return net->modelstr;
>   }
>   
> @@ -29386,13 +29391,31 @@ int
>   virDomainNetSetModelString(virDomainNetDefPtr net,
>                              const char *model)
>   {
> -    return VIR_STRDUP(net->modelstr, model);
> +    VIR_FREE(net->modelstr);
> +    if ((net->model = virDomainNetModelTypeFromString(model)) >= 0)
> +        return 0;
> +
> +    net->model = VIR_DOMAIN_NET_MODEL_UNKNOWN;
> +    if (!model)
> +        return 0;

Is this a safe thing to do? I mean virEnumFromString() (which is called 
by any TypeFromString() in the end) doesn't handle NULL gracefully. 
You'll need to swap some lines and probably have a temp variable to 
store virDomainNetModelTypeFromString() retval, ...

> +
> +    if (strspn(model, NET_MODEL_CHARS) < strlen(model)) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("Model name contains invalid characters"));
> +        return -1;
> +    }
> +
> +    if (VIR_STRDUP(net->modelstr, model) < 0)
> +        return -1;
> +    return 0;

Or simply 'return VIR_STRDUP();'

>   }
>   

Michal