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

On 01/22/2016 02:50 PM, Daniel P. Berrange wrote: > 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. > OK. >> 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 >

Got it. Thanks for the review! > Regards, > Daniel >