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

Re: [libvirt] [PATCH 1/2] qemu: Set iface MTU on hotplug



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>


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