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

Laine Stump posted 1 patch 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170327020518.13560-1-laine@laine.org
There is a newer version of this series
docs/formatdomain.html.in | 6 +++---
src/conf/domain_conf.c    | 6 +++++-
src/conf/domain_conf.h    | 4 ++++
3 files changed, 12 insertions(+), 4 deletions(-)
[libvirt] [PATCH] conf: don't ignore <target dev='blah'/> for macvtap interfaces
Posted by Laine Stump 7 years ago
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"
+
 typedef enum {
     VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT = 0,
     VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED,
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: don't ignore <target dev='blah'/> for macvtap interfaces
Posted by Martin Kletzander 7 years ago
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@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list