On 15.07.26 17:51, Chaney, Ben wrote:
On 7/14/26, 11:44 AM, "Vladimir Sementsov-Ogievskiy" <[email protected]
<mailto:[email protected]>> wrote:
+static void tap_vm_state_change(void *opaque, bool running, RunState state)
+{
+ TAPState *s = opaque;
+
+ if (running) {
+ if (s->read_poll_detached) {
+ tap_read_poll(s, true);
+ s->read_poll_detached = false;
+ }
+ } else if (state == RUN_STATE_FINISH_MIGRATE) {
+ if (s->read_poll) {
+ s->read_poll_detached = true;
+ tap_read_poll(s, false);
+ }
+ }
+}
+
tap_read_poll is called in a few other places. Is it necessary to update
read_poll_detached there as well?
I'm not sure that it is, I'm trying to wrap my head around any possible
ordering issues and it isn't obvious to me one way or the other.
Good question!
Hm. The whole patch is about source QEMU process, to not poll, when we expect
that target is already handle packets in TAP.
Let's go through calls to tap_read_poll (except for tap_vm_state_change):
1. tap_send_completed() -> tap_read_poll(true). Called asynchronously from
packet sending (to guest) code. Actually, that may be a problem. If callback comes
when
we are alredy switched to FINISH_MIGRATE, it breaks the logic, need to fix.
2. tap_send() -> tap_read_poll(false). It's safe. But it may break "if
(s->read_poll)" of this patch, if somehow interleave with sate change (not sure it
possible, but better to fix)
3. tap_cleanup() -> tap_read_poll(false) - safe
4. tap_post_load() -> tap_read_poll(true) - safe, it's not about source QEMU
5. net_tap_fd_init() -> tap_read_poll(true) - safe, it's on tap creation
6. through tap_poll() from frontend code, when transfer control to vhost.
--
I'll remake this patch. Better idea: modify tap_read_poll() itself:
--- a/net/tap.c
+++ b/net/tap.c
@@ -100,7 +100,7 @@ struct TAPState {
Notifier exit;
int queue_index;
- bool read_poll_detached;
+ bool enable_poll_on_resume;
VMChangeStateEntry *vmstate;
bool local_migration_supported;
};
@@ -157,6 +157,10 @@ static void tap_update_fd_handler(TAPState *s)
static void tap_read_poll(TAPState *s, bool enable)
{
+ if (enable && runstate_check(RUN_STATE_FINISH_MIGRATE)) {
+ s->enable_poll_on_resume = true;
+ return;
+ }
s->read_poll = enable;
tap_update_fd_handler(s);
}
@@ -166,13 +170,13 @@ static void tap_vm_state_change(void *opaque, bool
running, RunState state)
TAPState *s = opaque;
if (running) {
- if (s->read_poll_detached) {
+ if (s->enable_poll_on_resume) {
tap_read_poll(s, true);
- s->read_poll_detached = false;
+ s->enable_poll_on_resume = false;
}
} else if (state == RUN_STATE_FINISH_MIGRATE) {
if (s->read_poll) {
- s->read_poll_detached = true;
+ s->enable_poll_on_resume = true;
tap_read_poll(s, false);
}
}
@@ -964,7 +968,7 @@ static bool net_init_tap_one(const NetdevTapOptions *tap,
NetClientState *peer,
int sndbuf =
(tap->has_sndbuf && tap->sndbuf) ? MIN(tap->sndbuf, INT_MAX) : INT_MAX;
- s->read_poll_detached = false;
+ s->enable_poll_on_resume = false;
s->vmstate = qemu_add_vm_change_state_handler(tap_vm_state_change, s);
if (!tap_set_sndbuf(fd, sndbuf, sndbuf_required ? errp : NULL) &&
--
Best regards,
Vladimir
