Re: [Xen-devel] [libvirt] [FOR 1.3.0 PATCH] conf: add net device prefix for Xen

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Mon, Dec 07, 2015 at 09:42:21AM -0700, Jim Fehlig wrote:
> In commit d2e5538b1, the libxl driver was changed to copy interface
> names autogenerated by libxl to the corresponding network def in the
> domain's virDomainDef object. The copied name is freed when the domain
> transitions to the shutoff state. But when migrating a domain, the
> autogenerated name is included in the XML sent to the destination host.
> It is possible an interface with the same name already exists on the
> destination host, causing migration to fail. Indeed the Xen project's
> OSSTEST CI already encountered such a failure.
> 
> This patch defines another VIR_NET_GENERATED_PREFIX for Xen, allowing
> the autogenerated names to be excluded when parsing and formatting
> inactive config.
> 
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
> 
> This is an alternative approach to Joao's fix for this regression
> 
> https://www.redhat.com/archives/libvir-list/2015-December/msg00197.html
> 
> I think it is the same approach used by the qemu driver. My only
> reservation is that it expands the potential for clashes with
> user-defined names. I.e. with this change both 'vnet' and 'vif' are
> reserved prefixes.

Hmm, yes, tricky one.

If we only care about XML parsing, then you could register a post
parse callback instead to do this.

I'm not clear why we also have it in the virDomainNetDefFormat
method - and we can't solve that with a post-parse callback.


The other option would be to make the reserved prefix be a
capability that the parser/formatter could read.

> 
>  src/conf/domain_conf.c   | 6 ++++--
>  src/conf/domain_conf.h   | 4 ++++
>  src/libxl/libxl_domain.c | 5 +++--
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2f5c0ed..debcf4e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8428,7 +8428,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                  ifname = virXMLPropString(cur, "dev");
>                  if (ifname &&
>                      (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
> -                    STRPREFIX(ifname, VIR_NET_GENERATED_PREFIX)) {
> +                    (STRPREFIX(ifname, VIR_NET_GENERATED_PREFIX) ||
> +                     STRPREFIX(ifname, VIR_NET_GENERATED_PREFIX_XEN))) {
>                      /* An auto-generated target name, blank it out */
>                      VIR_FREE(ifname);
>                  }
> @@ -19790,7 +19791,8 @@ virDomainNetDefFormat(virBufferPtr buf,
>      virBufferEscapeString(buf, "<backenddomain name='%s'/>\n", 
> def->domain_name);
>      if (def->ifname &&
>          !((flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
> -          (STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) {
> +          (STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX) ||
> +           STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX_XEN)))) {
>          /* Skip auto-generated target names for inactive config. */
>          virBufferEscapeString(buf, "<target dev='%s'/>\n", def->ifname);
>      }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 90d8e13..d2cc26f 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1085,6 +1085,10 @@ struct _virDomainNetDef {
>   * by libvirt, and cannot be used for a persistent network name.  */
>  # define VIR_NET_GENERATED_PREFIX "vnet"
>  
> +/* Used for prefix of ifname of any network name generated dynamically
> + * by libvirt for Xen, and cannot be used for a persistent network name.  */
> +# define VIR_NET_GENERATED_PREFIX_XEN "vif"
> +
>  typedef enum {
>      VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT = 0,
>      VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED,
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index ef92974..c5d84a4 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -734,7 +734,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
>          for (i = 0; i < vm->def->nnets; i++) {
>              virDomainNetDefPtr net = vm->def->nets[i];
>  
> -            if (STRPREFIX(net->ifname, "vif"))
> +            if (STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX_XEN))
>                  VIR_FREE(net->ifname);
>          }
>      }
> @@ -918,7 +918,8 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def, 
> libxl_domain_config *d_config)
>          if (net->ifname)
>              continue;
>  
> -        ignore_value(virAsprintf(&net->ifname, "vif%d.%d%s",
> +        ignore_value(virAsprintf(&net->ifname,
> +                                 VIR_NET_GENERATED_PREFIX_XEN "%d.%d%s",
>                                   def->id, x_nic->devid, suffix));
>      }
>  }
> -- 
> 2.6.1
> 
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel