[libvirt] [PATCH v3 4/4] domain_conf: autodetect vhostuser ifname

[Michal Privoznik]

On 18.11.2016 23:51, Mehdi Abaakouk wrote:
> From: Mehdi Abaakouk <sileht at redhat.com>
> 
> This change puts the socket filename as ifname for vhostuser netwok
> interface.
> 
> The filename is the name of the openvswitch interface, this allows the
> qemuDomainInterfaceStats to work out of the box without having to
> manually set the ifname.
> ---
>  src/conf/domain_conf.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6e008e2..0f91ab3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9098,6 +9098,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      virDomainActualNetDefPtr actual = NULL;
>      xmlNodePtr oldnode = ctxt->node;
>      int ret, val;
> +    char **tokens = NULL;
> +    size_t ntokens = 0;
>  
>      if (VIR_ALLOC(def) < 0)
>          return NULL;
> @@ -9394,6 +9396,13 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>  
>          def->data.vhostuser->type = VIR_DOMAIN_CHR_TYPE_UNIX;
>          def->data.vhostuser->data.nix.path = vhostuser_path;
> +
> +        if (!ifname && (tokens = virStringSplitCount(vhostuser_path, "/", 0,
> +                                                     &ntokens))) {
> +            if (VIR_STRDUP(ifname, tokens[ntokens-1]) < 0)
> +                goto error;
> +        }
> +

I don't think this is a good idea. For instance, for the following XML:

      <interface type='vhostuser'>
        <mac address='52:54:00:ee:96:6b'/>
        <source type='unix' path='/tmp/vhost0.sock' mode='server'/>
        <model type='virtio'/>
      </interface>

this code would produce ifname of "vhost0.sock", which is obviously
wrong. Moreover, tests are failing with this change. Not only that, for
auto-filling values in XML we have so called post parse callbacks.
Historically we put everything into XML parsing code and it ended up
being this one big mess. So we worked hard to split it and although we
are not there 100%, we are getting there slowly.

>          vhostuser_path = NULL;
>  
>          if (STREQ(vhostuser_mode, "server")) {
> @@ -9842,6 +9851,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      VIR_FREE(localaddr);
>      VIR_FREE(localport);
>      virNWFilterHashTableFree(filterparams);
> +    virStringFreeListCount(tokens, ntokens);


Ah, due to me not reviewing the patches early, this function was renamed
in c2a5a4e7ea930.

>  
>      return def;
>  
> 

Michal