[PATCH v2 0/2] qemu_process: Skip over non-virtio non-TAP NIC models when refreshing rx-filter

This was reported here: https://lists.libvirt.org/archives/list/users@lists.libvirt.org/thread/QZ43K... and v1 was posted here: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/AVRYA... diff to v1: - Expanded commit message in 1/2 - Added NEWS entry - Expanded list of vNIC types for which query-rx-filter is issued to basically every TUN/TAP based type. Might be an overkill, but should keep us covered for foreseeable future (e.g. when we decide to report changed MAC address in domain XML). Michal Prívozník (2): qemu_process: Skip over non-virtio non-TAP NIC models when refreshing rx-filter NEWS: Document recent rx-filter bugfix NEWS.rst | 9 +++++++++ src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) -- 2.43.0

After guest is started, or we are reconnecting to already running one (after daemon restart), qemuProcessRefreshRxFilters() is called to refresh rx-filters (basically MAC addresses of guest NICs) as they might have changed while we were not running (for the case when reconnecting to an already running guest), or we need to enable them by running a command (for freshly started guest - see processNicRxFilterChangedEvent()). Now, our XML parser allowed trustGuestRxFilters attribute for all types and models of <interface/> while in reality, only virtio model AND TUN/TAP based types can see MAC address changes. For other combinations, QEMU reports an error. This all means that when the daemon is restarted and it reconnects to a guest with, well invalid configuration, or when such guest is restored from a saved image, or migrated then we issue the monitor command, to which QEMU replies with an error which is then propagated to users. While on one hand users should fix their configuration (and after v10.0.0-rc1~123 they can do that even on live domains), libvirt can also has some logic built in that prevent issuing the command in the first place (for obviously wrong cases). Fixes: 060d4c83ef436cf56abfad51a4d64c39448e199d Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3563ad215c..c1115b440f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7958,6 +7958,33 @@ qemuProcessRefreshRxFilters(virDomainObj *vm, if (!virDomainNetGetActualTrustGuestRxFilters(def)) continue; + /* rx-filters are supported only for virtio model and TUN/TAP based + * types. */ + if (def->model != VIR_DOMAIN_NET_MODEL_VIRTIO) + continue; + + switch (virDomainNetGetActualType(def)) { + case VIR_DOMAIN_NET_TYPE_ETHERNET: + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_BRIDGE: + case VIR_DOMAIN_NET_TYPE_DIRECT: + break; + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_INTERNAL: + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + case VIR_DOMAIN_NET_TYPE_UDP: + case VIR_DOMAIN_NET_TYPE_VDPA: + case VIR_DOMAIN_NET_TYPE_NULL: + case VIR_DOMAIN_NET_TYPE_VDS: + case VIR_DOMAIN_NET_TYPE_LAST: + default: + continue; + } + if (qemuDomainSyncRxFilter(vm, def, asyncJob) < 0) return -1; } -- 2.43.0

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- NEWS.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 4c799999fe..e2796fd8b2 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -27,6 +27,15 @@ v10.1.0 (unreleased) * **Bug fixes** + * qemu_process: Skip over non-virtio non-TAP NIC models when refreshing rx-filter + + If ``trustGuestRxFilters`` is enabled for a vNIC that doesn't support it, + libvirt may throw an error when such domain is being started, loaded from a + saved state, migrated, etc. These errors are now silenced, but make sure to + fix such configurations (after previous release it is even possible to + change ``trustGuestRxFilters`` value on live domains via + ``virDomainUpdateDeviceFlags()`` or ``virsh device-update``). + v10.0.0 (2024-01-15) ==================== -- 2.43.0

On Thu, Jan 25, 2024 at 14:49:47 +0100, Michal Privoznik wrote:
This was reported here:
https://lists.libvirt.org/archives/list/users@lists.libvirt.org/thread/QZ43K...
and v1 was posted here:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/AVRYA...
diff to v1: - Expanded commit message in 1/2 - Added NEWS entry - Expanded list of vNIC types for which query-rx-filter is issued to basically every TUN/TAP based type. Might be an overkill, but should keep us covered for foreseeable future (e.g. when we decide to report changed MAC address in domain XML).
Please add the exact verbatim copy of the error message the user gets if they have the broken configuration both into the commit message and into the news entry for anyone searching for that problem. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Michal Privoznik
-
Peter Krempa