[libvirt] [PATCH RFC 1/2] conf: add net device prefix to capabilities
Joao Martins
joao.m.martins at oracle.com
Fri Jan 22 15:46:35 UTC 2016
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 at oracle.com>
>> ---
>> 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
>
More information about the libvir-list
mailing list