[libvirt] [PATCH] conf: don't ignore <target dev='blah'/> for macvtap interfaces

[Martin Kletzander]

On Sun, Mar 26, 2017 at 10:05:18PM -0400, Laine Stump wrote:
>The parser had been clearing out *all* suggested device names for
>type='direct' (aka macvtap) interfaces. All of the code implementing
>macvtap allows for a user-specified device name, so we should allow
>it. In the case that an interface name starts with "macvtap" or
>"macvlan" though, we do still clear it out, just as we do with "vnet"
>(which is the prefix used for automatically generated tap device
>names), since those are the prefixes for the names we autogenerate for
>macvtap and macvlan devices.
>
>Resolves: https://bugzilla.redhat.com/1335798
>---
> docs/formatdomain.html.in | 6 +++---
> src/conf/domain_conf.c    | 6 +++++-
> src/conf/domain_conf.h    | 4 ++++
> 3 files changed, 12 insertions(+), 4 deletions(-)
>
>diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>index 4a3123e..8a2294f 100644
>--- a/docs/formatdomain.html.in
>+++ b/docs/formatdomain.html.in
>@@ -5125,9 +5125,9 @@ qemu-kvm -net nic,model=? /dev/null
>       If no target is specified, certain hypervisors will
>       automatically generate a name for the created tun device. This
>       name can be manually specified, however the name <i>should not
>-      start with either 'vnet' or 'vif'</i>, which are prefixes
>-      reserved by libvirt and certain hypervisors. Manually specified
>-      targets using these prefixes may be ignored.
>+      start with either 'vnet', 'vif', 'macvtap', or 'macvlan'</i>,
>+      which are prefixes reserved by libvirt and certain hypervisors.
>+      Manually specified targets using these prefixes may be ignored.
>     </p>
>
>     <p>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 2139ab0..4b80018 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -9826,8 +9826,12 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>         def->data.direct.linkdev = dev;
>         dev = NULL;
>
>-        if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
>+        if (ifname &&
>+            flags & VIR_DOMAIN_DEF_PARSE_INACTIVE &&
>+            (STRPREFIX(ifname, VIR_NET_GENERATED_MACVTAP_PREFIX) ||
>+             STRPREFIX(ifname, VIR_NET_GENERATED_MACVTAP_PREFIX))) {
>             VIR_FREE(ifname);
>+        }
>
>         break;
>
>diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>index 8e6d874..38a15f1 100644
>--- a/src/conf/domain_conf.h
>+++ b/src/conf/domain_conf.h
>@@ -1033,6 +1033,10 @@ struct _virDomainNetDef {
>  * by libvirt, and cannot be used for a persistent network name.  */
> # define VIR_NET_GENERATED_PREFIX "vnet"
>
>+/* except to macvtap/macvlan interfaces, which start with "macv(tap|lan) */
>+# define VIR_NET_GENERATED_MACVTAP_PREFIX "macvtap"
>+# define VIR_NET_GENERATED_MACVLAN_PREFIX "macvlan"
>+

You're duplicating MACV(LAN|TAP)_NAME_PREFIX from
src/util/virnetdevmacvlan.c.  Even though these are pretty
un-changeable.  Could we use one macro for the same things?  I don't
feel like these defines are adding much more information =D

> typedef enum {
>     VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT = 0,
>     VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED,
>--
>2.9.3
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170327/e89fe277/attachment-0001.sig>