* [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
@ 2025-12-11 9:05 Aaron Lo
2025-12-11 13:14 ` Michael S. Tsirkin
2025-12-11 14:22 ` Daniel P. Berrangé
0 siblings, 2 replies; 5+ messages in thread
From: Aaron Lo @ 2025-12-11 9:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, David Hildenbrand, qemu-trivial, Aaron Lo
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>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
2025-12-11 9:05 [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists Aaron Lo
@ 2025-12-11 13:14 ` Michael S. Tsirkin
2025-12-11 14:22 ` Daniel P. Berrangé
1 sibling, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2025-12-11 13:14 UTC (permalink / raw)
To: Aaron Lo; +Cc: qemu-devel, David Hildenbrand, qemu-trivial
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
2025-12-11 9:05 [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists Aaron Lo
2025-12-11 13:14 ` Michael S. Tsirkin
@ 2025-12-11 14:22 ` Daniel P. Berrangé
2025-12-11 20:15 ` Aaron Lo
1 sibling, 1 reply; 5+ messages in thread
From: Daniel P. Berrangé @ 2025-12-11 14:22 UTC (permalink / raw)
To: Aaron Lo; +Cc: qemu-devel, Michael S. Tsirkin, David Hildenbrand, qemu-trivial
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 :|
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
2025-12-11 14:22 ` Daniel P. Berrangé
@ 2025-12-11 20:15 ` Aaron Lo
2025-12-11 22:18 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Aaron Lo @ 2025-12-11 20:15 UTC (permalink / raw)
To: "Daniel P. Berrangé"
Cc: qemu-devel, Michael S. Tsirkin, David Hildenbrand, qemu-trivial
[-- Attachment #1: Type: text/plain, Size: 2897 bytes --]
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 :|
[-- Attachment #2: Type: text/html, Size: 16197 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
2025-12-11 20:15 ` Aaron Lo
@ 2025-12-11 22:18 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2025-12-11 22:18 UTC (permalink / raw)
To: Aaron Lo
Cc: "Daniel P. Berrangé", qemu-devel, David Hildenbrand,
qemu-trivial
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
> :|
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-12-11 22:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-11 9:05 [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists Aaron Lo
2025-12-11 13:14 ` Michael S. Tsirkin
2025-12-11 14:22 ` Daniel P. Berrangé
2025-12-11 20:15 ` Aaron Lo
2025-12-11 22:18 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).