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

["Daniel P. Berrangé" <berrange@redhat.com>]

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é <berrange@redhat.com>
> Reported-by: Feifan Qian <bea1e@proton.me>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  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.

> +            /*
> +             * 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 :|