balloon_stats_get_all will iterate over guest stats upto the max
VIRTIO_BALLOON_S_NR value, calling visit_type_uint64 to populate
the QObject dict. The dict keys are obtained from the static
array balloon_stat_names which is VIRTIO_BALLOON_S_NR in size.
Unfortunately the way that array is declared results in any
unassigned stats getting a NULL name, which will then cause
visit_type_uint64 to trigger an assert in qobject_output_add_obj.
The balloon_stat_names array was fortunately fully populated with
names until recently:
commit 0d2eeef77a33315187df8519491a900bde4a3d83
Author: Bibo Mao <maobibo@loongson.cn>
Date: Mon Oct 28 10:38:09 2024 +0800
linux-headers: Update to Linux v6.12-rc5
pulled a change to include/standard-headers/linux/virtio_balloon.h
which increased VIRTIO_BALLOON_S_NR by 6, and failed to add the new
names to balloon_stat_names.
This commit fills in the missing names, and uses a static assert to
guarantee that any future changes to VIRTIO_BALLOON_S_NR will cause
a build failure until balloon_stat_names is updated.
This problem was detected by the Cockpit Project's automated
integration tests on QEMU 9.2.0-rc1.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2329448
Fixes: 0d2eeef77a33315187df8519491a900bde4a3d83
Reported-by: Martin Pitt <mpitt@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/virtio/virtio-balloon.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 609e39a821..afd2ad6dd6 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -167,19 +167,33 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
}
}
+/*
+ * All stats upto VIRTIO_BALLOON_S_NR /must/ have a
+ * non-NULL name declared here, since these are used
+ * as keys for populating the QDict with stats
+ */
static const char *balloon_stat_names[] = {
[VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
[VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
[VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults",
[VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
[VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
+
[VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
[VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
[VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
[VIRTIO_BALLOON_S_HTLB_PGALLOC] = "stat-htlb-pgalloc",
[VIRTIO_BALLOON_S_HTLB_PGFAIL] = "stat-htlb-pgfail",
- [VIRTIO_BALLOON_S_NR] = NULL
+
+ [VIRTIO_BALLOON_S_OOM_KILL] = "stat-oom-kills",
+ [VIRTIO_BALLOON_S_ALLOC_STALL] = "stat-alloc-stalls",
+ [VIRTIO_BALLOON_S_ASYNC_SCAN] = "stat-async-scans",
+ [VIRTIO_BALLOON_S_DIRECT_SCAN] = "stat-direct-scans",
+ [VIRTIO_BALLOON_S_ASYNC_RECLAIM] = "stat-async-reclaims",
+
+ [VIRTIO_BALLOON_S_DIRECT_RECLAIM] = "stat-direct-reclaims",
};
+G_STATIC_ASSERT(G_N_ELEMENTS(balloon_stat_names) == VIRTIO_BALLOON_S_NR);
/*
* reset_stats - Mark all items in the stats array as unset
--
2.46.0
29.11.2024 16:55, Daniel P. Berrangé wrote: > balloon_stats_get_all will iterate over guest stats upto the max > VIRTIO_BALLOON_S_NR value, calling visit_type_uint64 to populate > the QObject dict. The dict keys are obtained from the static > array balloon_stat_names which is VIRTIO_BALLOON_S_NR in size. > > Unfortunately the way that array is declared results in any > unassigned stats getting a NULL name, which will then cause > visit_type_uint64 to trigger an assert in qobject_output_add_obj. > > The balloon_stat_names array was fortunately fully populated with > names until recently: > > commit 0d2eeef77a33315187df8519491a900bde4a3d83 > Author: Bibo Mao <maobibo@loongson.cn> > Date: Mon Oct 28 10:38:09 2024 +0800 > > linux-headers: Update to Linux v6.12-rc5 > > pulled a change to include/standard-headers/linux/virtio_balloon.h > which increased VIRTIO_BALLOON_S_NR by 6, and failed to add the new > names to balloon_stat_names. > > This commit fills in the missing names, and uses a static assert to > guarantee that any future changes to VIRTIO_BALLOON_S_NR will cause > a build failure until balloon_stat_names is updated. > > This problem was detected by the Cockpit Project's automated > integration tests on QEMU 9.2.0-rc1. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2329448 > Fixes: 0d2eeef77a33315187df8519491a900bde4a3d83 > Reported-by: Martin Pitt <mpitt@redhat.com> > Reviewed-by: Richard W.M. Jones <rjones@redhat.com> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru> /mjt
On 29.11.24 14:55, Daniel P. Berrangé wrote:
> balloon_stats_get_all will iterate over guest stats upto the max
> VIRTIO_BALLOON_S_NR value, calling visit_type_uint64 to populate
> the QObject dict. The dict keys are obtained from the static
> array balloon_stat_names which is VIRTIO_BALLOON_S_NR in size.
>
> Unfortunately the way that array is declared results in any
> unassigned stats getting a NULL name, which will then cause
> visit_type_uint64 to trigger an assert in qobject_output_add_obj.
>
> The balloon_stat_names array was fortunately fully populated with
> names until recently:
>
> commit 0d2eeef77a33315187df8519491a900bde4a3d83
> Author: Bibo Mao <maobibo@loongson.cn>
> Date: Mon Oct 28 10:38:09 2024 +0800
>
> linux-headers: Update to Linux v6.12-rc5
>
> pulled a change to include/standard-headers/linux/virtio_balloon.h
> which increased VIRTIO_BALLOON_S_NR by 6, and failed to add the new
> names to balloon_stat_names.
>
> This commit fills in the missing names, and uses a static assert to
> guarantee that any future changes to VIRTIO_BALLOON_S_NR will cause
> a build failure until balloon_stat_names is updated.
>
> This problem was detected by the Cockpit Project's automated
> integration tests on QEMU 9.2.0-rc1.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2329448
> Fixes: 0d2eeef77a33315187df8519491a900bde4a3d83
> Reported-by: Martin Pitt <mpitt@redhat.com>
> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> hw/virtio/virtio-balloon.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 609e39a821..afd2ad6dd6 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -167,19 +167,33 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
> }
> }
>
> +/*
> + * All stats upto VIRTIO_BALLOON_S_NR /must/ have a
> + * non-NULL name declared here, since these are used
> + * as keys for populating the QDict with stats
> + */
> static const char *balloon_stat_names[] = {
> [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
> [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
> [VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults",
> [VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
> [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
> +
> [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
> [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
> [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
> [VIRTIO_BALLOON_S_HTLB_PGALLOC] = "stat-htlb-pgalloc",
> [VIRTIO_BALLOON_S_HTLB_PGFAIL] = "stat-htlb-pgfail",
> - [VIRTIO_BALLOON_S_NR] = NULL
> +
> + [VIRTIO_BALLOON_S_OOM_KILL] = "stat-oom-kills",
> + [VIRTIO_BALLOON_S_ALLOC_STALL] = "stat-alloc-stalls",
> + [VIRTIO_BALLOON_S_ASYNC_SCAN] = "stat-async-scans",
> + [VIRTIO_BALLOON_S_DIRECT_SCAN] = "stat-direct-scans",
> + [VIRTIO_BALLOON_S_ASYNC_RECLAIM] = "stat-async-reclaims",
> +
> + [VIRTIO_BALLOON_S_DIRECT_RECLAIM] = "stat-direct-reclaims",
> };
> +G_STATIC_ASSERT(G_N_ELEMENTS(balloon_stat_names) == VIRTIO_BALLOON_S_NR);
>
> /*
> * reset_stats - Mark all items in the stats array as unset
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
© 2016 - 2024 Red Hat, Inc.