managed='no' on an <interface> allows an unprivileged libvirt to use a
pre-created tap/macvtap device that libvirt has permission to
open/read/write, but no permission to modify (i.e. set the MTU or MAC
address). But when the XML had an <mtu size='blah'/> setting (which
was put there in order to tell the *guest* OS what MTU to set for the
emulated device at the other end of the tap) We were attempting to set
the MTU of the tap device on the host, paying no attention to the
setting of 'managed'. That would of course end in failure.

This patch only sets the MTU if managed='no' is *not* set (so, if it
is 'yes', or just not set at all).

Note that MTU of the tap is also set when connecting the tap to a
bridge device, but managed='no' is only allowed for <interface
type='ethernet', which would never attach to a bridge anyway, so we
don't need the check there.

Resolves: https://bugzilla.redhat.com/1905929
Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/qemu/qemu_command.c | 2 +-
 src/qemu/qemu_hotplug.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f970a3128..3eff09a9d3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8149,7 +8149,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
         }
     }
 
-    if (net->mtu &&
+    if (net->mtu && net->managed_tap != VIR_TRISTATE_BOOL_NO &&
         virNetDevSetMTU(net->ifname, net->mtu) < 0)
         goto cleanup;
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f336a90c8e..95f2bd0aca 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1387,7 +1387,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
         }
     }
 
-    if (net->mtu &&
+    if (net->mtu && net->managed_tap != VIR_TRISTATE_BOOL_NO &&
         virNetDevSetMTU(net->ifname, net->mtu) < 0)
         goto cleanup;
 
-- 
2.29.2

On 1/12/21 7:30 PM, Laine Stump wrote:
> managed='no' on an <interface> allows an unprivileged libvirt to use a
> pre-created tap/macvtap device that libvirt has permission to
> open/read/write, but no permission to modify (i.e. set the MTU or MAC
> address). But when the XML had an <mtu size='blah'/> setting (which
> was put there in order to tell the *guest* OS what MTU to set for the
> emulated device at the other end of the tap) We were attempting to set

I guess you meant to use lowcase "We" ?

"(...) the other end of the tap) we were attempting (...)"



> the MTU of the tap device on the host, paying no attention to the
> setting of 'managed'. That would of course end in failure.
> 
> This patch only sets the MTU if managed='no' is *not* set (so, if it
> is 'yes', or just not set at all).
> 
> Note that MTU of the tap is also set when connecting the tap to a
> bridge device, but managed='no' is only allowed for <interface
> type='ethernet', which would never attach to a bridge anyway, so we

Missed an enclosing ">":

"type='ethernet'>, which would ....."



> don't need the check there.
> 
> Resolves: https://bugzilla.redhat.com/1905929
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   src/qemu/qemu_command.c | 2 +-
>   src/qemu/qemu_hotplug.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6f970a3128..3eff09a9d3 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8149,7 +8149,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>           }
>       }
>   
> -    if (net->mtu &&
> +    if (net->mtu && net->managed_tap != VIR_TRISTATE_BOOL_NO &&
>           virNetDevSetMTU(net->ifname, net->mtu) < 0)
>           goto cleanup;
>   
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index f336a90c8e..95f2bd0aca 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1387,7 +1387,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>           }
>       }
>   
> -    if (net->mtu &&
> +    if (net->mtu && net->managed_tap != VIR_TRISTATE_BOOL_NO &&
>           virNetDevSetMTU(net->ifname, net->mtu) < 0)
>           goto cleanup;
>   
>