6:50 a.m.
I'm inclined to pushed them as trivial, but since they have
a bugzilla assigned I'd rather have a proper review.
Michal Privoznik (2):
qemu: Set iface MTU on hotplug
qemuDomainChangeNet: Forbid changing MTU
src/qemu/qemu_hotplug.c | 10 ++++++++++
1 file changed, 10 insertions(+)
--
2.13.0
6:50 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Set iface MTU on hotplug
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(a)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
9:38 a.m.
New subject: [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(a)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(a)laine.org>
6:50 a.m.
New subject: [libvirt] [PATCH 2/2] qemuDomainChangeNet: Forbid changing MTU
https://bugzilla.redhat.com/show_bug.cgi?id=1447618
Currently, any attempt to change MTU on an interface that is
plugged to a running domain is silently ignored. We should either
do what's asked or error out. Well, we can update the host side
of the interface, but we cannot change 'host_mtu' attribute for
the virtio-net device. Therefore we have to error out.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8066acae3..d46956d98 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3138,6 +3138,12 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
/* vlan can be modified, and will be checked later */
/* linkstate can be modified */
+ if (olddev->mtu != newdev->mtu) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot modify MTU"));
+ goto cleanup;
+ }
+
/* allocate new actual device to compare to old - we will need to
* free it if we fail for any reason
*/
--
2.13.0
9:43 a.m.
New subject: [libvirt] [PATCH 2/2] qemuDomainChangeNet: Forbid changing MTU
On 06/08/2017 07:50 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1447618
Currently, any attempt to change MTU on an interface that is
plugged to a running domain is silently ignored. We should either
do what's asked or error out. Well, we can update the host side
of the interface, but we cannot change 'host_mtu' attribute for
the virtio-net device. Therefore we have to error out.
Interesting conundrum. There's nothing to stop a user from intervening
directly in the guest and changing the guest's MTU from there. So if we
allowed changing the mtu of the tap device this way, at least it would
be *possible* to modify the MTU of an active interface. But if we did
that, then behavior would be inconsistent between startup/hotplug time
and runtime.
So for now at least, ACK. Better to not allow something than to have it
behave inconsistently.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)laine.org>
---
src/qemu/qemu_hotplug.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8066acae3..d46956d98 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3138,6 +3138,12 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
/* vlan can be modified, and will be checked later */
/* linkstate can be modified */
+ if (olddev->mtu != newdev->mtu) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot modify MTU"));
+ goto cleanup;
+ }
+
/* allocate new actual device to compare to old - we will need to
* free it if we fail for any reason
*/
2724
days inactive
2724
days old
4 comments
2 participants
participants (2)
-
Laine Stump -
Michal Privoznik