Polling when VM is stopped doesn't make real sense, as stopped VM can't
handle incoming traffic anyway.

And it's critical for introduction of local TAP migration
feature in the next commit: the TAP device will be transferred
to the target (open fd will be passed through migration channel),
and if we continue polling on source, we may get a package, which
we'll never handle on source (already stopped), it will be lost.
Better is save this package for target VM to handle.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/net/tap.c b/net/tap.c
index 95bcfaaf1ea..2bd4b089573 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -36,6 +36,7 @@
 #include "net/net.h"
 #include "clients.h"
 #include "monitor/monitor.h"
+#include "system/runstate.h"
 #include "system/system.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
@@ -92,6 +93,8 @@ struct TAPState {
     Notifier exit;
 
     int queue_index;
+    bool read_poll_detached;
+    VMChangeStateEntry *vmstate;
 };
 
 static void launch_script(const char *setup_script, const char *ifname,
@@ -150,6 +153,23 @@ static void tap_read_poll(TAPState *s, bool enable)
     tap_update_fd_handler(s);
 }
 
+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);
+        }
+    }
+}
+
 static void tap_write_poll(TAPState *s, bool enable)
 {
     s->write_poll = enable;
@@ -380,6 +400,11 @@ static void tap_cleanup(NetClientState *nc)
         s->exit.notify = NULL;
     }
 
+    if (s->vmstate) {
+        qemu_del_vm_change_state_handler(s->vmstate);
+        s->vmstate = NULL;
+    }
+
     tap_read_poll(s, false);
     tap_write_poll(s, false);
     close(s->fd);
@@ -819,6 +844,9 @@ 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->vmstate = qemu_add_vm_change_state_handler(tap_vm_state_change, s);
+
     if (!tap_set_sndbuf(fd, sndbuf, sndbuf_required ? errp : NULL) &&
         sndbuf_required) {
         goto failed;
-- 
2.43.0

On 7/14/26, 11:44 AM, "Vladimir Sementsov-Ogievskiy" <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> 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.

Thanks,
        Ben

On 15.07.26 17:51, Chaney, Ben wrote:
> 
> On 7/14/26, 11:44 AM, "Vladimir Sementsov-Ogievskiy" <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> 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