Re: [libvirt] [PATCH RFC 1/2] conf: add net device prefix to capabilities

On Wed, Jan 20, 2016 at 11:41:26PM +0000, Joao Martins wrote: > In the reverted 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. > > This patch defines a new capability for setting the network device > prefix that will be used in the driver. This prefix will be then copied > when domain is created as def->netprefix, which will be then used by > virDomainNetDefFormat(). Valid prefixes are VIR_NET_GENERATED_PREFIX or > the one announced by the driver. > > Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com&gt; > --- > src/conf/capabilities.c | 21 +++++++++++++++++++++ > src/conf/capabilities.h | 4 ++++ > src/conf/domain_conf.c | 20 +++++++++++++++----- > src/conf/domain_conf.h | 2 ++ > src/libvirt_private.syms | 1 + > src/network/bridge_driver.c | 2 +- > 6 files changed, 44 insertions(+), 6 deletions(-) > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c > index 86ea212..eafa7a9 100644 > --- a/src/conf/capabilities.c > +++ b/src/conf/capabilities.c > @@ -269,6 +269,23 @@ virCapabilitiesAddHostMigrateTransport(virCapsPtr caps, > return 0; > } > > +/** > + * virCapabilitiesSetNetPrefix: > + * @caps: capabilities to extend > + * @name: prefix for host generated network interfaces > + * > + * Registers the prefix that is used for generated network interfaces > + */ > +int > +virCapabilitiesSetNetPrefix(virCapsPtr caps, > + const char *prefix) > +{ > + if (VIR_STRDUP(caps->host.netprefix, prefix) < 0) > + return -1; > + > + return 0; > +} You'll need to update virCapabilitiesDispose to free this string too.

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index a9706b0..70d5556 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -8403,6 +8403,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, > xmlNodePtr node, > xmlXPathContextPtr ctxt, > virHashTablePtr bootHash, > + char *prefix, > unsigned int flags) > { > virDomainNetDefPtr def; > @@ -8569,7 +8570,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) || > + (prefix && STRPREFIX(ifname, prefix)))) { > /* An auto-generated target name, blank it out */ > VIR_FREE(ifname); > } > @@ -12525,6 +12527,7 @@ virDomainDeviceDefParse(const char *xmlStr, > xmlNodePtr node; > xmlXPathContextPtr ctxt = NULL; > virDomainDeviceDefPtr dev = NULL; > + char *prefix; > > if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) > goto error; > @@ -12567,8 +12570,9 @@ virDomainDeviceDefParse(const char *xmlStr, > goto error; > break; > case VIR_DOMAIN_DEVICE_NET: > + prefix = caps->host.netprefix; > if (!(dev->data.net = virDomainNetDefParseXML(xmlopt, node, ctxt, > - NULL, flags))) > + NULL, prefix, flags))) > goto error; > break; > case VIR_DOMAIN_DEVICE_INPUT: > @@ -15885,11 +15889,15 @@ virDomainDefParseXML(xmlDocPtr xml, > goto error; > if (n && VIR_ALLOC_N(def->nets, n) < 0) > goto error; > + if (caps->host.netprefix && > + VIR_STRDUP(def->netprefix, caps->host.netprefix) < 0) > + goto error; > for (i = 0; i < n; i++) { > virDomainNetDefPtr net = virDomainNetDefParseXML(xmlopt, > nodes[i], > ctxt, > bootHash, > + def->netprefix, > flags); > if (!net) > goto error; > @@ -19880,6 +19888,7 @@ virDomainVirtioNetDriverFormat(char **outstr, > int > virDomainNetDefFormat(virBufferPtr buf, > virDomainNetDefPtr def, > + char *prefix, > unsigned int flags) > { > unsigned int actualType = virDomainNetGetActualType(def); > @@ -20057,7 +20066,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) || > + (prefix && STRPREFIX(def->ifname, prefix))))) { > /* Skip auto-generated target names for inactive config. */ > virBufferEscapeString(buf, "<target dev='%s'/>\n", def->ifname); > } > @@ -22310,7 +22320,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, > goto error; > > for (n = 0; n < def->nnets; n++) > - if (virDomainNetDefFormat(buf, def->nets[n], flags) < 0) > + if (virDomainNetDefFormat(buf, def->nets[n], def->netprefix, flags) < 0) > goto error; > > for (n = 0; n < def->nsmartcards; n++) > @@ -23535,7 +23545,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, > rc = virDomainFSDefFormat(&buf, src->data.fs, flags); > break; > case VIR_DOMAIN_DEVICE_NET: > - rc = virDomainNetDefFormat(&buf, src->data.net, flags); > + rc = virDomainNetDefFormat(&buf, src->data.net, def->netprefix, flags); > break; > case VIR_DOMAIN_DEVICE_INPUT: > rc = virDomainInputDefFormat(&buf, src->data.input, flags); > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 0141009..9e085a7 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -2254,6 +2254,7 @@ struct _virDomainDef { > > virDomainOSDef os; > char *emulator; > + char *netprefix; > /* These three options are of type virTristateSwitch, > * except VIR_DOMAIN_FEATURE_CAPABILITIES that is of type > * virDomainCapabilitiesPolicy */ > @@ -2748,6 +2749,7 @@ int virDomainDiskSourceFormat(virBufferPtr buf, > > int virDomainNetDefFormat(virBufferPtr buf, > virDomainNetDefPtr def, > + char *prefix, > unsigned int flags); It will result in a ripple effect across the drivers, but rather than storing 'netprefix' in virDomainDef, I think we should probably change virDomainDefFormat to pass in the virCapsPtr object, so we can directly access the canonical source for this data. It is probably best to add virCapsPtr to virDomainDefFormat as a separate patch to this one to keep it clean

Regards, Daniel