3:57 a.m.
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
3:57 a.m.
New subject: [PATCH 1/5] processNicRxFilterChangedEvent: Free @guestFilter and @hostFilter automatically
There's no need to call virNetDevRxFilterFree() explicitly, when
corresponding variables can be declared as
g_autoptr(virNetDevRxFilter).
Signed-off-by: Michal Privoznik <mprivozn(a)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
3:57 a.m.
New subject: [PATCH 2/5] qemu: Move parts of NIC_RX_FILTER_CHANGED even handling into a function
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(a)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
5:32 a.m.
New subject: [PATCH 2/5] qemu: Move parts of NIC_RX_FILTER_CHANGED even handling into a function
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(a)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(-)
3:57 a.m.
New subject: [PATCH 3/5] qemu: Acquire QUERY job instead of MODIFY when handling NIC_RX_FILTER_CHANGED event
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(a)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
3:57 a.m.
New subject: [PATCH 4/5] qemu: Refresh state after restore from a save image
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(a)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
3:57 a.m.
New subject: [PATCH 5/5] qemu: Refresh rx-filters more often
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(a)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
5:32 a.m.
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(a)redhat.com>
Jano
757
days inactive
758
days old
7 comments
2 participants
participants (2)
-
Ján Tomko -
Michal Privoznik