[PATCH 0/5] qemu: Refresh rx-filters more often
See the last patch for explanation. I haven't found a gitlab issue for this nor a bug open. But I remember somebody complaining about problems during restore from a save file, on IRC perhaps? Michal Prívozník (5): processNicRxFilterChangedEvent: Free @guestFilter and @hostFilter automatically qemu: Move parts of NIC_RX_FILTER_CHANGED even handling into a function qemu: Acquire QUERY job instead of MODIFY when handling NIC_RX_FILTER_CHANGED event qemu: Refresh state after restore from a save image qemu: Refresh rx-filters more often src/qemu/qemu_domain.c | 254 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 5 + src/qemu/qemu_driver.c | 250 +------------------------------------ src/qemu/qemu_process.c | 27 ++++ src/qemu/qemu_saveimage.c | 2 + 5 files changed, 291 insertions(+), 247 deletions(-) -- 2.37.3
There's no need to call virNetDevRxFilterFree() explicitly, when corresponding variables can be declared as g_autoptr(virNetDevRxFilter). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5c75000742..afebae3b93 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3817,8 +3817,8 @@ processNicRxFilterChangedEvent(virDomainObj *vm, qemuDomainObjPrivate *priv = vm->privateData; virDomainDeviceDef dev; virDomainNetDef *def; - virNetDevRxFilter *guestFilter = NULL; - virNetDevRxFilter *hostFilter = NULL; + g_autoptr(virNetDevRxFilter) guestFilter = NULL; + g_autoptr(virNetDevRxFilter) hostFilter = NULL; int ret; VIR_DEBUG("Received NIC_RX_FILTER_CHANGED event for device %s " @@ -3826,7 +3826,7 @@ processNicRxFilterChangedEvent(virDomainObj *vm, devAlias, vm, vm->def->name); if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) - goto cleanup; + return; if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Domain is not running"); @@ -3907,10 +3907,6 @@ processNicRxFilterChangedEvent(virDomainObj *vm, endjob: virDomainObjEndJob(vm); - - cleanup: - virNetDevRxFilterFree(hostFilter); - virNetDevRxFilterFree(guestFilter); } -- 2.37.3
Parts of the code that responds to the NIC_RX_FILTER_CHANGED event are going to be re-used. Separate them into a function (qemuDomainSyncRxFilter()) and move the code into qemu_domain.c so that it can be re-used from other places of the driver. There's one slight change though: instead of passing device alias from the just received event to qemuMonitorQueryRxFilter(), I've switched to using the alias stored in our domain definition. But these two are guaranteed to be equal. virDomainDefFindDevice() made sure about that, if nothing else. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 251 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 + src/qemu/qemu_driver.c | 242 +-------------------------------------- 3 files changed, 256 insertions(+), 241 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4c14fc2aef..1b93ebe579 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11774,3 +11774,254 @@ qemuDomainObjWait(virDomainObj *vm) return 0; } + + +static void +syncNicRxFilterMacAddr(char *ifname, virNetDevRxFilter *guestFilter, + virNetDevRxFilter *hostFilter) +{ + char newMacStr[VIR_MAC_STRING_BUFLEN]; + + if (virMacAddrCmp(&hostFilter->mac, &guestFilter->mac)) { + virMacAddrFormat(&guestFilter->mac, newMacStr); + + /* set new MAC address from guest to associated macvtap device */ + if (virNetDevSetMAC(ifname, &guestFilter->mac) < 0) { + VIR_WARN("Couldn't set new MAC address %s to device %s " + "while responding to NIC_RX_FILTER_CHANGED", + newMacStr, ifname); + } else { + VIR_DEBUG("device %s MAC address set to %s", ifname, newMacStr); + } + } +} + + +static void +syncNicRxFilterGuestMulticast(char *ifname, virNetDevRxFilter *guestFilter, + virNetDevRxFilter *hostFilter) +{ + size_t i, j; + bool found; + char macstr[VIR_MAC_STRING_BUFLEN]; + + for (i = 0; i < guestFilter->multicast.nTable; i++) { + found = false; + + for (j = 0; j < hostFilter->multicast.nTable; j++) { + if (virMacAddrCmp(&guestFilter->multicast.table[i], + &hostFilter->multicast.table[j]) == 0) { + found = true; + break; + } + } + + if (!found) { + virMacAddrFormat(&guestFilter->multicast.table[i], macstr); + + if (virNetDevAddMulti(ifname, &guestFilter->multicast.table[i]) < 0) { + VIR_WARN("Couldn't add new multicast MAC address %s to " + "device %s while responding to NIC_RX_FILTER_CHANGED", + macstr, ifname); + } else { + VIR_DEBUG("Added multicast MAC %s to %s interface", + macstr, ifname); + } + } + } +} + + +static void +syncNicRxFilterHostMulticast(char *ifname, virNetDevRxFilter *guestFilter, + virNetDevRxFilter *hostFilter) +{ + size_t i, j; + bool found; + char macstr[VIR_MAC_STRING_BUFLEN]; + + for (i = 0; i < hostFilter->multicast.nTable; i++) { + found = false; + + for (j = 0; j < guestFilter->multicast.nTable; j++) { + if (virMacAddrCmp(&hostFilter->multicast.table[i], + &guestFilter->multicast.table[j]) == 0) { + found = true; + break; + } + } + + if (!found) { + virMacAddrFormat(&hostFilter->multicast.table[i], macstr); + + if (virNetDevDelMulti(ifname, &hostFilter->multicast.table[i]) < 0) { + VIR_WARN("Couldn't delete multicast MAC address %s from " + "device %s while responding to NIC_RX_FILTER_CHANGED", + macstr, ifname); + } else { + VIR_DEBUG("Deleted multicast MAC %s from %s interface", + macstr, ifname); + } + } + } +} + + +static void +syncNicRxFilterPromiscMode(char *ifname, + virNetDevRxFilter *guestFilter, + virNetDevRxFilter *hostFilter) +{ + bool promisc; + bool setpromisc = false; + + /* Set macvtap promisc mode to true if the guest has vlans defined */ + /* or synchronize the macvtap promisc mode if different from guest */ + if (guestFilter->vlan.nTable > 0) { + if (!hostFilter->promiscuous) { + setpromisc = true; + promisc = true; + } + } else if (hostFilter->promiscuous != guestFilter->promiscuous) { + setpromisc = true; + promisc = guestFilter->promiscuous; + } + + if (setpromisc) { + if (virNetDevSetPromiscuous(ifname, promisc) < 0) { + VIR_WARN("Couldn't set PROMISC flag to %s for device %s " + "while responding to NIC_RX_FILTER_CHANGED", + promisc ? "true" : "false", ifname); + } + } +} + + +static void +syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilter *guestFilter, + virNetDevRxFilter *hostFilter) +{ + if (hostFilter->multicast.mode != guestFilter->multicast.mode || + (guestFilter->multicast.overflow && + guestFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_NORMAL)) { + switch (guestFilter->multicast.mode) { + case VIR_NETDEV_RX_FILTER_MODE_ALL: + if (virNetDevSetRcvAllMulti(ifname, true) < 0) { + VIR_WARN("Couldn't set allmulticast flag to 'on' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", ifname); + } + break; + + case VIR_NETDEV_RX_FILTER_MODE_NORMAL: + if (guestFilter->multicast.overflow && + (hostFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_ALL)) { + break; + } + + if (virNetDevSetRcvMulti(ifname, true) < 0) { + VIR_WARN("Couldn't set multicast flag to 'on' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", ifname); + } + + if (virNetDevSetRcvAllMulti(ifname, + guestFilter->multicast.overflow) < 0) { + VIR_WARN("Couldn't set allmulticast flag to '%s' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", + virTristateSwitchTypeToString(virTristateSwitchFromBool(guestFilter->multicast.overflow)), + ifname); + } + break; + + case VIR_NETDEV_RX_FILTER_MODE_NONE: + if (virNetDevSetRcvAllMulti(ifname, false) < 0) { + VIR_WARN("Couldn't set allmulticast flag to 'off' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", ifname); + } + + if (virNetDevSetRcvMulti(ifname, false) < 0) { + VIR_WARN("Couldn't set multicast flag to 'off' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", + ifname); + } + break; + } + } +} + + +static void +syncNicRxFilterDeviceOptions(char *ifname, virNetDevRxFilter *guestFilter, + virNetDevRxFilter *hostFilter) +{ + syncNicRxFilterPromiscMode(ifname, guestFilter, hostFilter); + syncNicRxFilterMultiMode(ifname, guestFilter, hostFilter); +} + + +static void +syncNicRxFilterMulticast(char *ifname, + virNetDevRxFilter *guestFilter, + virNetDevRxFilter *hostFilter) +{ + syncNicRxFilterGuestMulticast(ifname, guestFilter, hostFilter); + syncNicRxFilterHostMulticast(ifname, guestFilter, hostFilter); +} + + +int +qemuDomainSyncRxFilter(virDomainObj *vm, + virDomainNetDef *def) +{ + qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(virNetDevRxFilter) guestFilter = NULL; + g_autoptr(virNetDevRxFilter) hostFilter = NULL; + int rc; + + qemuDomainObjEnterMonitor(vm); + rc = qemuMonitorQueryRxFilter(priv->mon, def->info.alias, &guestFilter); + qemuDomainObjExitMonitor(vm); + if (rc < 0) + return -1; + + if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_DIRECT) { + if (virNetDevGetRxFilter(def->ifname, &hostFilter)) { + VIR_WARN("Couldn't get current RX filter for device %s while responding to NIC_RX_FILTER_CHANGED", + def->ifname); + return -1; + } + + /* For macvtap connections, set the following macvtap network device + * attributes to match those of the guest network device: + * - MAC address + * - Multicast MAC address table + * - Device options: + * - PROMISC + * - MULTICAST + * - ALLMULTI + */ + syncNicRxFilterMacAddr(def->ifname, guestFilter, hostFilter); + syncNicRxFilterMulticast(def->ifname, guestFilter, hostFilter); + syncNicRxFilterDeviceOptions(def->ifname, guestFilter, hostFilter); + } + + if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_NETWORK) { + const char *brname = virDomainNetGetActualBridgeName(def); + + /* For libivrt network connections, set the following TUN/TAP network + * device attributes to match those of the guest network device: + * - QoS filters (which are based on MAC address) + */ + if (virDomainNetGetActualBandwidth(def) && + def->data.network.actual && + virNetDevBandwidthUpdateFilter(brname, &guestFilter->mac, + def->data.network.actual->class_id) < 0) + return -1; + } + + return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a22deaf113..f436861efc 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1098,3 +1098,7 @@ qemuDomainRemoveLogs(virQEMUDriver *driver, int qemuDomainObjWait(virDomainObj *vm); + +int +qemuDomainSyncRxFilter(virDomainObj *vm, + virDomainNetDef *def); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index afebae3b93..86bc35ca92 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3614,212 +3614,12 @@ processDeviceDeletedEvent(virQEMUDriver *driver, } -static void -syncNicRxFilterMacAddr(char *ifname, virNetDevRxFilter *guestFilter, - virNetDevRxFilter *hostFilter) -{ - char newMacStr[VIR_MAC_STRING_BUFLEN]; - - if (virMacAddrCmp(&hostFilter->mac, &guestFilter->mac)) { - virMacAddrFormat(&guestFilter->mac, newMacStr); - - /* set new MAC address from guest to associated macvtap device */ - if (virNetDevSetMAC(ifname, &guestFilter->mac) < 0) { - VIR_WARN("Couldn't set new MAC address %s to device %s " - "while responding to NIC_RX_FILTER_CHANGED", - newMacStr, ifname); - } else { - VIR_DEBUG("device %s MAC address set to %s", ifname, newMacStr); - } - } -} - - -static void -syncNicRxFilterGuestMulticast(char *ifname, virNetDevRxFilter *guestFilter, - virNetDevRxFilter *hostFilter) -{ - size_t i, j; - bool found; - char macstr[VIR_MAC_STRING_BUFLEN]; - - for (i = 0; i < guestFilter->multicast.nTable; i++) { - found = false; - - for (j = 0; j < hostFilter->multicast.nTable; j++) { - if (virMacAddrCmp(&guestFilter->multicast.table[i], - &hostFilter->multicast.table[j]) == 0) { - found = true; - break; - } - } - - if (!found) { - virMacAddrFormat(&guestFilter->multicast.table[i], macstr); - - if (virNetDevAddMulti(ifname, &guestFilter->multicast.table[i]) < 0) { - VIR_WARN("Couldn't add new multicast MAC address %s to " - "device %s while responding to NIC_RX_FILTER_CHANGED", - macstr, ifname); - } else { - VIR_DEBUG("Added multicast MAC %s to %s interface", - macstr, ifname); - } - } - } -} - - -static void -syncNicRxFilterHostMulticast(char *ifname, virNetDevRxFilter *guestFilter, - virNetDevRxFilter *hostFilter) -{ - size_t i, j; - bool found; - char macstr[VIR_MAC_STRING_BUFLEN]; - - for (i = 0; i < hostFilter->multicast.nTable; i++) { - found = false; - - for (j = 0; j < guestFilter->multicast.nTable; j++) { - if (virMacAddrCmp(&hostFilter->multicast.table[i], - &guestFilter->multicast.table[j]) == 0) { - found = true; - break; - } - } - - if (!found) { - virMacAddrFormat(&hostFilter->multicast.table[i], macstr); - - if (virNetDevDelMulti(ifname, &hostFilter->multicast.table[i]) < 0) { - VIR_WARN("Couldn't delete multicast MAC address %s from " - "device %s while responding to NIC_RX_FILTER_CHANGED", - macstr, ifname); - } else { - VIR_DEBUG("Deleted multicast MAC %s from %s interface", - macstr, ifname); - } - } - } -} - - -static void -syncNicRxFilterPromiscMode(char *ifname, - virNetDevRxFilter *guestFilter, - virNetDevRxFilter *hostFilter) -{ - bool promisc; - bool setpromisc = false; - - /* Set macvtap promisc mode to true if the guest has vlans defined */ - /* or synchronize the macvtap promisc mode if different from guest */ - if (guestFilter->vlan.nTable > 0) { - if (!hostFilter->promiscuous) { - setpromisc = true; - promisc = true; - } - } else if (hostFilter->promiscuous != guestFilter->promiscuous) { - setpromisc = true; - promisc = guestFilter->promiscuous; - } - - if (setpromisc) { - if (virNetDevSetPromiscuous(ifname, promisc) < 0) { - VIR_WARN("Couldn't set PROMISC flag to %s for device %s " - "while responding to NIC_RX_FILTER_CHANGED", - promisc ? "true" : "false", ifname); - } - } -} - - -static void -syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilter *guestFilter, - virNetDevRxFilter *hostFilter) -{ - if (hostFilter->multicast.mode != guestFilter->multicast.mode || - (guestFilter->multicast.overflow && - guestFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_NORMAL)) { - switch (guestFilter->multicast.mode) { - case VIR_NETDEV_RX_FILTER_MODE_ALL: - if (virNetDevSetRcvAllMulti(ifname, true) < 0) { - VIR_WARN("Couldn't set allmulticast flag to 'on' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", ifname); - } - break; - - case VIR_NETDEV_RX_FILTER_MODE_NORMAL: - if (guestFilter->multicast.overflow && - (hostFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_ALL)) { - break; - } - - if (virNetDevSetRcvMulti(ifname, true) < 0) { - VIR_WARN("Couldn't set multicast flag to 'on' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", ifname); - } - - if (virNetDevSetRcvAllMulti(ifname, - guestFilter->multicast.overflow) < 0) { - VIR_WARN("Couldn't set allmulticast flag to '%s' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", - virTristateSwitchTypeToString(virTristateSwitchFromBool(guestFilter->multicast.overflow)), - ifname); - } - break; - - case VIR_NETDEV_RX_FILTER_MODE_NONE: - if (virNetDevSetRcvAllMulti(ifname, false) < 0) { - VIR_WARN("Couldn't set allmulticast flag to 'off' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", ifname); - } - - if (virNetDevSetRcvMulti(ifname, false) < 0) { - VIR_WARN("Couldn't set multicast flag to 'off' for " - "device %s while responding to " - "NIC_RX_FILTER_CHANGED", - ifname); - } - break; - } - } -} - - -static void -syncNicRxFilterDeviceOptions(char *ifname, virNetDevRxFilter *guestFilter, - virNetDevRxFilter *hostFilter) -{ - syncNicRxFilterPromiscMode(ifname, guestFilter, hostFilter); - syncNicRxFilterMultiMode(ifname, guestFilter, hostFilter); -} - - -static void -syncNicRxFilterMulticast(char *ifname, - virNetDevRxFilter *guestFilter, - virNetDevRxFilter *hostFilter) -{ - syncNicRxFilterGuestMulticast(ifname, guestFilter, hostFilter); - syncNicRxFilterHostMulticast(ifname, guestFilter, hostFilter); -} - static void processNicRxFilterChangedEvent(virDomainObj *vm, const char *devAlias) { - qemuDomainObjPrivate *priv = vm->privateData; virDomainDeviceDef dev; virDomainNetDef *def; - g_autoptr(virNetDevRxFilter) guestFilter = NULL; - g_autoptr(virNetDevRxFilter) hostFilter = NULL; - int ret; VIR_DEBUG("Received NIC_RX_FILTER_CHANGED event for device %s " "from domain %p %s", @@ -3862,49 +3662,9 @@ processNicRxFilterChangedEvent(virDomainObj *vm, VIR_DEBUG("process NIC_RX_FILTER_CHANGED event for network " "device %s in domain %s", def->info.alias, vm->def->name); - qemuDomainObjEnterMonitor(vm); - ret = qemuMonitorQueryRxFilter(priv->mon, devAlias, &guestFilter); - qemuDomainObjExitMonitor(vm); - if (ret < 0) + if (qemuDomainSyncRxFilter(vm, def) < 0) goto endjob; - if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_DIRECT) { - - if (virNetDevGetRxFilter(def->ifname, &hostFilter)) { - VIR_WARN("Couldn't get current RX filter for device %s " - "while responding to NIC_RX_FILTER_CHANGED", - def->ifname); - goto endjob; - } - - /* For macvtap connections, set the following macvtap network device - * attributes to match those of the guest network device: - * - MAC address - * - Multicast MAC address table - * - Device options: - * - PROMISC - * - MULTICAST - * - ALLMULTI - */ - syncNicRxFilterMacAddr(def->ifname, guestFilter, hostFilter); - syncNicRxFilterMulticast(def->ifname, guestFilter, hostFilter); - syncNicRxFilterDeviceOptions(def->ifname, guestFilter, hostFilter); - } - - if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_NETWORK) { - const char *brname = virDomainNetGetActualBridgeName(def); - - /* For libivrt network connections, set the following TUN/TAP network - * device attributes to match those of the guest network device: - * - QoS filters (which are based on MAC address) - */ - if (virDomainNetGetActualBandwidth(def) && - def->data.network.actual && - virNetDevBandwidthUpdateFilter(brname, &guestFilter->mac, - def->data.network.actual->class_id) < 0) - goto endjob; - } - endjob: virDomainObjEndJob(vm); } -- 2.37.3
s/even/event/ in the commit summary Jano On a Tuesday in 2022, Michal Privoznik wrote:
Parts of the code that responds to the NIC_RX_FILTER_CHANGED event are going to be re-used. Separate them into a function (qemuDomainSyncRxFilter()) and move the code into qemu_domain.c so that it can be re-used from other places of the driver.
There's one slight change though: instead of passing device alias from the just received event to qemuMonitorQueryRxFilter(), I've switched to using the alias stored in our domain definition. But these two are guaranteed to be equal. virDomainDefFindDevice() made sure about that, if nothing else.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 251 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 + src/qemu/qemu_driver.c | 242 +-------------------------------------- 3 files changed, 256 insertions(+), 241 deletions(-)
We are not updating domain XML to new MAC address, just merely setting host side of macvtap. But we don't need a MODIFY job for that, QUERY is just fine. This allows us to process the event should it occur during migration. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 86bc35ca92..d5fb2913be 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3625,7 +3625,7 @@ processNicRxFilterChangedEvent(virDomainObj *vm, "from domain %p %s", devAlias, vm, vm->def->name); - if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0) return; if (!virDomainObjIsActive(vm)) { -- 2.37.3
When restoring a domain from a save image, we need to query QEMU for some runtime information that is not stored in status XML, or even if it is, it's not parsed (e.g. virtio-mem actual size, or soon rx-filters for macvtaps). During migration, this is done in qemuMigrationDstFinishFresh(), or in case of newly started domain in qemuProcessStart(). Except, the way that the code is written, when restoring from a save image (which is effectively a migration), the state is never refreshed, because qemuProcessStart() sees incoming migration so it does not refresh the state thinking it'll be done in the finish phase. But restoring from a save image has no finish phase. Therefore, refresh the state explicitly after the domain was restored but before vCPUs are resumed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_saveimage.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 79567bf17d..ef62303728 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -672,6 +672,8 @@ qemuSaveImageStartVM(virConnectPtr conn, VIR_DOMAIN_EVENT_STARTED_RESTORED); virObjectEventStateQueue(driver->domainEventState, event); + if (qemuProcessRefreshState(driver, vm, asyncJob) < 0) + goto cleanup; /* If it was running before, resume it now unless caller requested pause. */ if (header->was_running && !start_paused) { -- 2.37.3
There are couple of scenarios where we need to reflect MAC change done in the guest: 1) domain restore from a file (here, we don't store updated MAC in the save file and thus on restore create the macvtap with the original MAC), 2) reconnecting to a running domain (here, the guest might have changed the MAC while we were not running), 3) migration (here, guest might change the MAC address but we fail to respond to it, Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 9 ++++++--- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1b93ebe579..b408ec0607 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11975,14 +11975,17 @@ syncNicRxFilterMulticast(char *ifname, int qemuDomainSyncRxFilter(virDomainObj *vm, - virDomainNetDef *def) + virDomainNetDef *def, + virDomainAsyncJob asyncJob) { qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virNetDevRxFilter) guestFilter = NULL; g_autoptr(virNetDevRxFilter) hostFilter = NULL; int rc; - qemuDomainObjEnterMonitor(vm); + if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0) + return -1; + rc = qemuMonitorQueryRxFilter(priv->mon, def->info.alias, &guestFilter); qemuDomainObjExitMonitor(vm); if (rc < 0) @@ -11990,7 +11993,7 @@ qemuDomainSyncRxFilter(virDomainObj *vm, if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_DIRECT) { if (virNetDevGetRxFilter(def->ifname, &hostFilter)) { - VIR_WARN("Couldn't get current RX filter for device %s while responding to NIC_RX_FILTER_CHANGED", + VIR_WARN("Couldn't get current RX filter for device %s", def->ifname); return -1; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f436861efc..37e0a90452 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1101,4 +1101,5 @@ qemuDomainObjWait(virDomainObj *vm); int qemuDomainSyncRxFilter(virDomainObj *vm, - virDomainNetDef *def); + virDomainNetDef *def, + virDomainAsyncJob asyncJob); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d5fb2913be..803d2c1771 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3662,7 +3662,7 @@ processNicRxFilterChangedEvent(virDomainObj *vm, VIR_DEBUG("process NIC_RX_FILTER_CHANGED event for network " "device %s in domain %s", def->info.alias, vm->def->name); - if (qemuDomainSyncRxFilter(vm, def) < 0) + if (qemuDomainSyncRxFilter(vm, def, VIR_ASYNC_JOB_NONE) < 0) goto endjob; endjob: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1a9175f40f..fe98601fce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7756,6 +7756,26 @@ qemuProcessLaunch(virConnectPtr conn, } +static int +qemuProcessRefreshRxFilters(virDomainObj *vm, + virDomainAsyncJob asyncJob) +{ + size_t i; + + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDef *def = vm->def->nets[i]; + + if (!virDomainNetGetActualTrustGuestRxFilters(def)) + continue; + + if (qemuDomainSyncRxFilter(vm, def, asyncJob) < 0) + return -1; + } + + return 0; +} + + /** * qemuProcessRefreshState: * @driver: qemu driver data @@ -7787,6 +7807,10 @@ qemuProcessRefreshState(virQEMUDriver *driver, if (qemuProcessRefreshDisks(vm, asyncJob) < 0) return -1; + VIR_DEBUG("Updating rx-filter data"); + if (qemuProcessRefreshRxFilters(vm, asyncJob) < 0) + return -1; + return 0; } @@ -8807,6 +8831,9 @@ qemuProcessReconnect(void *opaque) if (qemuSecurityReserveLabel(driver->securityManager, obj->def, obj->pid) < 0) goto error; + if (qemuProcessRefreshRxFilters(obj, VIR_ASYNC_JOB_NONE) < 0) + goto error; + qemuProcessNotifyNets(obj->def); qemuProcessFiltersInstantiate(obj->def); -- 2.37.3
On a Tuesday in 2022, Michal Privoznik wrote:
See the last patch for explanation. I haven't found a gitlab issue for this nor a bug open. But I remember somebody complaining about problems during restore from a save file, on IRC perhaps?
Michal Prívozník (5): processNicRxFilterChangedEvent: Free @guestFilter and @hostFilter automatically qemu: Move parts of NIC_RX_FILTER_CHANGED even handling into a function qemu: Acquire QUERY job instead of MODIFY when handling NIC_RX_FILTER_CHANGED event qemu: Refresh state after restore from a save image qemu: Refresh rx-filters more often
src/qemu/qemu_domain.c | 254 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 5 + src/qemu/qemu_driver.c | 250 +------------------------------------ src/qemu/qemu_process.c | 27 ++++ src/qemu/qemu_saveimage.c | 2 + 5 files changed, 291 insertions(+), 247 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik