Re: [PATCH 4/5] migration: Fix rare hang of migration_channel_read_peek()

On Tue, Jul 28, 2026 at 05:24:36PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 28, 2026 at 11:52:46AM -0400, Peter Xu wrote:
> > In an unlikely case, when a migration stream is attached to the destination
> > QEMU and only send <4 bytes to the channel as magic, it's possible that
> > migration_channel_read_peek() may spin forever without yielding in the main
> > thread causing two unwanted consequences:
> > 
> > - CPU will spin 100% waiting for the rest bytes until it reaches 4
> > - (more importantly..) Main thread is stuck during this process as the qio
> >   operation won't really yield the coroutine
> > 
> > Fix it by consuming the bytes that arrived.
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3889
> > Cc: Daniel P. Berrangé <[email protected]>
> > Reported-by: Feifan Qian <[email protected]>
> > Signed-off-by: Peter Xu <[email protected]>
> > ---
> >  migration/channel.c | 26 +++++++++++++++++++++++---
> >  1 file changed, 23 insertions(+), 3 deletions(-)
> > 
> > diff --git a/migration/channel.c b/migration/channel.c
> > index 1e2935f926..28fe1d2906 100644
> > --- a/migration/channel.c
> > +++ b/migration/channel.c
> > @@ -294,11 +294,31 @@ int migration_channel_read_peek(QIOChannel *ioc,
> >              return -1;
> >          }
> >  
> > -        if (len == buflen) {
> > +        if (len == iov.iov_len) {
> >              break;
> > -        }
> > +        } else if (len == 0) {
> > +            qio_channel_wait_cond(ioc, G_IO_IN);
> > +        } else {
> > +            ssize_t received = len;
> >  
> > -        qio_channel_wait_cond(ioc, G_IO_IN);
> > +            /*
> > +             * Partially arrived, read out to make qio_channel_wait_cond()
> > +             * won't return immediately, causing an unwanted spin on this
> > +             * CPU.
> > +             */
> > +            iov.iov_len = len;
> > +            len = qio_channel_readv_full(ioc, &iov, 1, NULL, NULL, 0, errp);
> 
> Sure this breaks the API behaviour that the caller is expecting to
> see.
> 
> migration_channel_identify will call migration_channel_read_peek
> to match the magic bytes.
> 
> But something later in the flow will actually try to read the magic
> bytes.  By consuming them in this migration_channel_read_peek
> method, surely we're breaking the code that wants to read the bytes
> later.

Ah right, stupid me.  The best then is for partial read we apply a manual
wait.

I can also revert 604bb1badc ("migration: Properly wait on G_IO_IN when
peeking messages"), looping with 1ms interval for data isn't too bad.  But
the best is we only do that for partial, so:

diff --git a/migration/channel.c b/migration/channel.c
index 1e2935f926..88f89521f8 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -296,9 +296,19 @@ int migration_channel_read_peek(QIOChannel *ioc,

         if (len == buflen) {
             break;
+        } else if (len == 0) {
+            qio_channel_wait_cond(ioc, G_IO_IN);
+        } else {
+            /*
+             * When partially ready, we can't use qio_channel_wait_cond()
+             * because it will return immediately.  Apply a manual wait.
+             */
+            if (qemu_in_coroutine()) {
+                qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000);
+            } else {
+                g_usleep(1000);
+            }
         }
-
-        qio_channel_wait_cond(ioc, G_IO_IN);
     }

     return 0;

Any preference?

Thanks,

> 
> > +            /*
> > +             * QIO_CHANNEL_ERR_BLOCK also shouldn't happen, due to the
> > +             * prior peek just happened.  We should be pretty sure we will
> > +             * read what we peeked, or the channel was broken.
> > +             */
> > +            if (len != received) {
> > +                return -1;
> > +            }
> > +            iov.iov_base += received;
> > +            iov.iov_len = buflen - received;
> > +        }
> >      }
> >  
> >      return 0;
> > -- 
> > 2.54.0
> > 
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
> |: https://libvirt.org          ~~          https://entangle-photo.org :|
> |: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
> 

-- 
Peter Xu