[PATCH] qemu_hotplug: Deny changing @rss and @rss_hash_report attributes of virtio vNICs

We have virDomainUpdateDeviceFlags() API that allows changing of some attributes of a device whilst domain is still running (e.g. setting different QoS, link state change on vNICs). But only very limited set of attributes can be changed and we have to check whether user isn't trying to sneak in a change that's not allowed. Well, in case of a virtio vNIC we forgot to check for @rss and @rss_hash_report attributes of <driver/>. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2082540 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 24df66cc9f..f795568299 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3662,7 +3662,9 @@ qemuDomainChangeNet(virQEMUDriver *driver, olddev->driver.virtio.guest.tso4 != newdev->driver.virtio.guest.tso4 || olddev->driver.virtio.guest.tso6 != newdev->driver.virtio.guest.tso6 || olddev->driver.virtio.guest.ecn != newdev->driver.virtio.guest.ecn || - olddev->driver.virtio.guest.ufo != newdev->driver.virtio.guest.ufo)) { + olddev->driver.virtio.guest.ufo != newdev->driver.virtio.guest.ufo || + olddev->driver.virtio.rss != newdev->driver.virtio.rss || + olddev->driver.virtio.rss_hash_report != newdev->driver.virtio.rss_hash_report)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify virtio network device driver attributes")); goto cleanup; -- 2.35.1

On 5/23/22 11:22 AM, Michal Privoznik wrote:
We have virDomainUpdateDeviceFlags() API that allows changing of some attributes of a device whilst domain is still running (e.g. setting different QoS, link state change on vNICs). But only very limited set of attributes can be changed and we have to check whether user isn't trying to sneak in a change that's not allowed. Well, in case of a virtio vNIC we forgot to check for @rss and @rss_hash_report attributes of <driver/>.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2082540 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Laine Stump <laine@redhat.com> (too bad there's not an elegant automated way of pointing out when a new field is added that can't be updated at runtime)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 24df66cc9f..f795568299 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3662,7 +3662,9 @@ qemuDomainChangeNet(virQEMUDriver *driver, olddev->driver.virtio.guest.tso4 != newdev->driver.virtio.guest.tso4 || olddev->driver.virtio.guest.tso6 != newdev->driver.virtio.guest.tso6 || olddev->driver.virtio.guest.ecn != newdev->driver.virtio.guest.ecn || - olddev->driver.virtio.guest.ufo != newdev->driver.virtio.guest.ufo)) { + olddev->driver.virtio.guest.ufo != newdev->driver.virtio.guest.ufo || + olddev->driver.virtio.rss != newdev->driver.virtio.rss || + olddev->driver.virtio.rss_hash_report != newdev->driver.virtio.rss_hash_report)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify virtio network device driver attributes")); goto cleanup;

On 5/23/22 18:29, Laine Stump wrote:
On 5/23/22 11:22 AM, Michal Privoznik wrote:
We have virDomainUpdateDeviceFlags() API that allows changing of some attributes of a device whilst domain is still running (e.g. setting different QoS, link state change on vNICs). But only very limited set of attributes can be changed and we have to check whether user isn't trying to sneak in a change that's not allowed. Well, in case of a virtio vNIC we forgot to check for @rss and @rss_hash_report attributes of <driver/>.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2082540 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Laine Stump <laine@redhat.com>
Thanks pushed.
(too bad there's not an elegant automated way of pointing out when a new field is added that can't be updated at runtime)
Well, so far we have just ints to compare, so we could do memcmp() but that won't work when a pointer is introduced into the struct. I thought about another version of this patch that would switch to memcmp() instead of adding two more comparisons, but realized the problem with pointer. Michal
participants (3)
-
Laine Stump
-
Michal Privoznik
-
Michal Prívozník