Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Dec 11, 2025 at 02:15:58PM -0600, Aaron Lo wrote:
> Hi Daniel and Michael,
> 
> Thanks for the quick feedback.
> 
> Given the migration concerns and issues with the spec itself, I think it’ll be
> best to drop the code changes for now.
> 
> Thanks again for the guidance,
> Aaron

You can do the spec changes though ;)

> 
>     On Dec 11, 2025, at 8:22 AM, Daniel P. Berrangé <berrange@redhat.com>
>     wrote:
> 
>     On Thu, Dec 11, 2025 at 03:05:49AM -0600, Aaron Lo wrote:
> 
>         The VirtIO specification (section 5.5.2) states that the stats queue
>         is only present if the VIRTIO_BALLOON_F_STATS_VQ feature is
>         negotiated. QEMU currently creates the statsq unconditionally.
> 
>         This patch guards statsq creation so it occurs only when the
>         feature bit is enabled.
> 
>         Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3188
> 
>         Signed-off-by: Aaron Lo <aaronlo0929@gmail.com>
>         ---
>         hw/virtio/virtio-balloon.c | 9 +++++++--
>         1 file changed, 7 insertions(+), 2 deletions(-)
> 
>         diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>         index 02cdd807d7..f5d4d5f60c 100644
>         --- a/hw/virtio/virtio-balloon.c
>         +++ b/hw/virtio/virtio-balloon.c
>         @@ -892,7 +892,10 @@ static void virtio_balloon_device_realize
>         (DeviceState *dev, Error **errp)
> 
>             s->ivq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
>             s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
>         -    s->svq = virtio_add_queue(vdev, 128,
>         virtio_balloon_receive_stats);
>         +
>         +    if (virtio_has_feature(s->host_features,
>         VIRTIO_BALLOON_F_STATS_VQ)) {
>         +        s->svq = virtio_add_queue(vdev, 128,
>         virtio_balloon_receive_stats);
>         +    }
> 
> 
>     This seems like a change that is liable to break live migration
>     state compatibility, as IIUC the queues are encoded in the state ?
> 
> 
> 
>             if (virtio_has_feature(s->host_features,
>         VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
>                 s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE,
>         @@ -932,7 +935,9 @@ static void virtio_balloon_device_unrealize
>         (DeviceState *dev)
> 
>             virtio_delete_queue(s->ivq);
>             virtio_delete_queue(s->dvq);
>         -    virtio_delete_queue(s->svq);
>         +    if (s->svq) {
>         +        virtio_delete_queue(s->svq);
>         +    }
>             if (s->free_page_vq) {
>                 virtio_delete_queue(s->free_page_vq);
>             }
> 
>         ---
>         base-commit: 9c23f2a7b0b45277693a14074b1aaa827eecdb92
>         change-id: 20251211-balloon-check-stats-feature-7ea658e038ce
> 
>         Best regards,
>         -- 
>         Aaron Lo <aaronlo0929@gmail.com>
> 
> 
> 
> 
>     With regards,
>     Daniel
>     -- 
>     |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange
>      :|
>     |: https://libvirt.org         -o-            https://fstop138.berrange.com
>      :|
>     |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange
>      :|
> 
>