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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxx>
>> ---
>>  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
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]