Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
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é
> 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
> ---
> 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
>
>
>
>
> 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
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é 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
>> ---
>> 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
>>
>>
>
> With regards,
> Daniel
> --
> |: https://berrange.com
Re: [PATCH] virtio-balloon: only create statsq when VIRTIO_BALLOON_F_STATS_VQ exists
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
> ---
> 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
>
>
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
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
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
