https://bugzilla.redhat.com/show_bug.cgi?id=1408701

While implementing MTU (572eda12ad and friends), I've forgotten
to actually set MTU on the host NIC in case of hotplug. We
correctly tell qemu on the monitor what the MTU should be, but we
are not actually setting it on the host NIC.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_hotplug.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6701bd9bc..8066acae3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1134,6 +1134,10 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
         }
     }
 
+    if (net->mtu &&
+        virNetDevSetMTU(net->ifname, net->mtu) < 0)
+        goto cleanup;
+
     for (i = 0; i < tapfdSize; i++) {
         if (qemuSecuritySetTapFDLabel(driver->securityManager,
                                       vm->def, tapfd[i]) < 0)
-- 
2.13.0

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

On 06/08/2017 07:50 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1408701
> 
> While implementing MTU (572eda12ad and friends), I've forgotten
> to actually set MTU on the host NIC in case of hotplug. We
> correctly tell qemu on the monitor what the MTU should be, but we
> are not actually setting it on the host NIC.

Well, it *was* being set by passing the configured MTU down into
virNetDevTapCreateInBridgePort(), until I had to revert patch 2841e675,
which automatically propagated the bridge's MTU back in the opposite
direction (it turns out the same patch made MTU propagate in both
directions).


> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 6701bd9bc..8066acae3 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1134,6 +1134,10 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>          }
>      }
>  
> +    if (net->mtu &&
> +        virNetDevSetMTU(net->ifname, net->mtu) < 0)
> +        goto cleanup;


On one hand, I dislike the idea that the MTU of the tap is now being set
twice (once when it's created, to match the MTU of the bridge, and again
here). On the other hand, by setting it here, you've taken care of
type='direct' (macvtap) and type='ethernet' (tap not connected to any
bridges) as well, which is a win, so ACK.


> +
>      for (i = 0; i < tapfdSize; i++) {
>          if (qemuSecuritySetTapFDLabel(driver->securityManager,
>                                        vm->def, tapfd[i]) < 0)
> 

Reviewed-by: Laine Stump <laine@laine.org>

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