1. Patchew
  2. QEMU
  3. View series

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

Aaron Lo posted 1 patch 2 days, 6 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251211-balloon-check-stats-feature-v1-1-ae8951957b80@gmail.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, David Hildenbrand <david@kernel.org>
hw/virtio/virtio-balloon.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
Posted by Aaron Lo 2 days, 6 hours ago 
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);
+    }
 
     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>
Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
Posted by Daniel P. Berrangé 2 days, 1 hour ago 
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 :|
Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
Posted by Aaron Lo 1 day, 19 hours ago 
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

> 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 <https://berrange.com/>      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org <https://libvirt.org/>         -o-            https://fstop138.berrange.com <https://fstop138.berrange.com/> :|
> |: https://entangle-photo.org <https://entangle-photo.org/>    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
Posted by Michael S. Tsirkin 1 day, 17 hours ago 
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
>      :|
> 
>
Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
Posted by Michael S. Tsirkin 2 days, 2 hours ago 
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>


did you test this with linux guests and with
free page hinting?



The issue is that sadly everyone is out of spec here.


We will have to fix the spec.


> ---
>  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);
> +    }
>  
>      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>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.