* [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it
@ 2024-11-29 13:55 Daniel P. Berrangé
2024-11-29 13:55 ` [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats Daniel P. Berrangé
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2024-11-29 13:55 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, David Hildenbrand, Martin Pitt,
Richard W . M . Jones, Fabiano Rosas, Michael S. Tsirkin,
Daniel P. Berrangé
See patch 1 for the background info on the problem
Changed in v2:
* Add qtest coverage for the crash scenario
Daniel P. Berrangé (3):
hw/virtio: fix crash in processing balloon stats
tests/qtest: drop 'fuzz-' prefix from virtio-balloon test
tests/qtest: add test for querying balloon guest stats
hw/virtio/virtio-balloon.c | 16 +++++++-
tests/qtest/fuzz-virtio-balloon-test.c | 37 -----------------
tests/qtest/meson.build | 2 +-
tests/qtest/virtio-balloon-test.c | 57 ++++++++++++++++++++++++++
4 files changed, 73 insertions(+), 39 deletions(-)
delete mode 100644 tests/qtest/fuzz-virtio-balloon-test.c
create mode 100644 tests/qtest/virtio-balloon-test.c
--
2.46.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats
2024-11-29 13:55 [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it Daniel P. Berrangé
@ 2024-11-29 13:55 ` Daniel P. Berrangé
2024-11-29 13:56 ` David Hildenbrand
2024-12-02 18:02 ` Michael Tokarev
2024-11-29 13:55 ` [PATCH v2 2/3 for 9.2] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test Daniel P. Berrangé
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2024-11-29 13:55 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, David Hildenbrand, Martin Pitt,
Richard W . M . Jones, Fabiano Rosas, Michael S. Tsirkin,
Daniel P. Berrangé
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/3 for 9.2] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test
2024-11-29 13:55 [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it Daniel P. Berrangé
2024-11-29 13:55 ` [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats Daniel P. Berrangé
@ 2024-11-29 13:55 ` Daniel P. Berrangé
2024-11-29 14:01 ` Fabiano Rosas
2024-11-29 15:44 ` Philippe Mathieu-Daudé
2024-11-29 13:55 ` [PATCH v2 3/3 for 9.2] tests/qtest: add test for querying balloon guest stats Daniel P. Berrangé
2024-12-02 17:05 ` [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it David Hildenbrand
3 siblings, 2 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2024-11-29 13:55 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, David Hildenbrand, Martin Pitt,
Richard W . M . Jones, Fabiano Rosas, Michael S. Tsirkin,
Daniel P. Berrangé
This test file is expected to be extended for arbitrary virtio-balloon
related tests, not merely those discovered by fuzzing.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/meson.build | 2 +-
.../{fuzz-virtio-balloon-test.c => virtio-balloon-test.c} | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
rename tests/qtest/{fuzz-virtio-balloon-test.c => virtio-balloon-test.c} (84%)
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index f2f35367ae..bd41c9da5f 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -88,7 +88,7 @@ qtests_i386 = \
(config_all_devices.has_key('CONFIG_MEGASAS_SCSI_PCI') ? ['fuzz-megasas-test'] : []) + \
(config_all_devices.has_key('CONFIG_LSI_SCSI_PCI') ? ['fuzz-lsi53c895a-test'] : []) + \
(config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ? ['fuzz-virtio-scsi-test'] : []) + \
- (config_all_devices.has_key('CONFIG_VIRTIO_BALLOON') ? ['fuzz-virtio-balloon-test'] : []) + \
+ (config_all_devices.has_key('CONFIG_VIRTIO_BALLOON') ? ['virtio-balloon-test'] : []) + \
(config_all_devices.has_key('CONFIG_Q35') ? ['q35-test'] : []) + \
(config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \
(config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) + \
diff --git a/tests/qtest/fuzz-virtio-balloon-test.c b/tests/qtest/virtio-balloon-test.c
similarity index 84%
rename from tests/qtest/fuzz-virtio-balloon-test.c
rename to tests/qtest/virtio-balloon-test.c
index ecb597fbee..6bea33b590 100644
--- a/tests/qtest/fuzz-virtio-balloon-test.c
+++ b/tests/qtest/virtio-balloon-test.c
@@ -1,5 +1,5 @@
/*
- * QTest fuzzer-generated testcase for virtio balloon device
+ * QTest test cases for virtio balloon device
*
* Copyright (c) 2024 Gao Shiyuan <gaoshiyuan@baidu.com>
*
@@ -30,7 +30,7 @@ int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
- qtest_add_func("fuzz/virtio/oss_fuzz_71649", oss_fuzz_71649);
+ qtest_add_func("virtio-balloon/oss_fuzz_71649", oss_fuzz_71649);
return g_test_run();
}
--
2.46.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/3 for 9.2] tests/qtest: add test for querying balloon guest stats
2024-11-29 13:55 [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it Daniel P. Berrangé
2024-11-29 13:55 ` [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats Daniel P. Berrangé
2024-11-29 13:55 ` [PATCH v2 2/3 for 9.2] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test Daniel P. Berrangé
@ 2024-11-29 13:55 ` Daniel P. Berrangé
2024-11-29 14:04 ` Fabiano Rosas
2024-12-02 17:05 ` [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it David Hildenbrand
3 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrangé @ 2024-11-29 13:55 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, David Hildenbrand, Martin Pitt,
Richard W . M . Jones, Fabiano Rosas, Michael S. Tsirkin,
Daniel P. Berrangé
This test would have identified the crash caused by the addition of new
balloon stats fields.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/virtio-balloon-test.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/tests/qtest/virtio-balloon-test.c b/tests/qtest/virtio-balloon-test.c
index 6bea33b590..ecdd363b06 100644
--- a/tests/qtest/virtio-balloon-test.c
+++ b/tests/qtest/virtio-balloon-test.c
@@ -8,6 +8,7 @@
#include "qemu/osdep.h"
#include "libqtest.h"
+#include "standard-headers/linux/virtio_balloon.h"
/*
* https://gitlab.com/qemu-project/qemu/-/issues/2576
@@ -26,11 +27,30 @@ static void oss_fuzz_71649(void)
qtest_quit(s);
}
+static void query_stats(void)
+{
+ QTestState *s = qtest_init("-device virtio-balloon,id=balloon"
+ " -nodefaults");
+ QDict *ret = qtest_qmp_assert_success_ref(
+ s,
+ "{ 'execute': 'qom-get', 'arguments': " \
+ "{ 'path': '/machine/peripheral/balloon', " \
+ " 'property': 'guest-stats' } }");
+ QDict *stats = qdict_get_qdict(ret, "stats");
+
+ /* We expect 1 entry in the dict for each known kernel stat */
+ assert(qdict_size(stats) == VIRTIO_BALLOON_S_NR);
+
+ qobject_unref(ret);
+ qtest_quit(s);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
qtest_add_func("virtio-balloon/oss_fuzz_71649", oss_fuzz_71649);
+ qtest_add_func("virtio-balloon/query-stats", query_stats);
return g_test_run();
}
--
2.46.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats
2024-11-29 13:55 ` [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats Daniel P. Berrangé
@ 2024-11-29 13:56 ` David Hildenbrand
2024-12-02 18:02 ` Michael Tokarev
1 sibling, 0 replies; 13+ messages in thread
From: David Hildenbrand @ 2024-11-29 13:56 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, Martin Pitt, Richard W . M . Jones,
Fabiano Rosas, Michael S. Tsirkin
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3 for 9.2] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test
2024-11-29 13:55 ` [PATCH v2 2/3 for 9.2] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test Daniel P. Berrangé
@ 2024-11-29 14:01 ` Fabiano Rosas
2024-11-29 15:44 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 13+ messages in thread
From: Fabiano Rosas @ 2024-11-29 14:01 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, David Hildenbrand, Martin Pitt,
Richard W . M . Jones, Michael S. Tsirkin,
Daniel P. Berrangé
Daniel P. Berrangé <berrange@redhat.com> writes:
> This test file is expected to be extended for arbitrary virtio-balloon
> related tests, not merely those discovered by fuzzing.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3 for 9.2] tests/qtest: add test for querying balloon guest stats
2024-11-29 13:55 ` [PATCH v2 3/3 for 9.2] tests/qtest: add test for querying balloon guest stats Daniel P. Berrangé
@ 2024-11-29 14:04 ` Fabiano Rosas
0 siblings, 0 replies; 13+ messages in thread
From: Fabiano Rosas @ 2024-11-29 14:04 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, David Hildenbrand, Martin Pitt,
Richard W . M . Jones, Michael S. Tsirkin,
Daniel P. Berrangé
Daniel P. Berrangé <berrange@redhat.com> writes:
> This test would have identified the crash caused by the addition of new
> balloon stats fields.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qtest/virtio-balloon-test.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/tests/qtest/virtio-balloon-test.c b/tests/qtest/virtio-balloon-test.c
> index 6bea33b590..ecdd363b06 100644
> --- a/tests/qtest/virtio-balloon-test.c
> +++ b/tests/qtest/virtio-balloon-test.c
> @@ -8,6 +8,7 @@
>
> #include "qemu/osdep.h"
> #include "libqtest.h"
> +#include "standard-headers/linux/virtio_balloon.h"
>
> /*
> * https://gitlab.com/qemu-project/qemu/-/issues/2576
> @@ -26,11 +27,30 @@ static void oss_fuzz_71649(void)
> qtest_quit(s);
> }
>
> +static void query_stats(void)
> +{
> + QTestState *s = qtest_init("-device virtio-balloon,id=balloon"
> + " -nodefaults");
> + QDict *ret = qtest_qmp_assert_success_ref(
> + s,
> + "{ 'execute': 'qom-get', 'arguments': " \
> + "{ 'path': '/machine/peripheral/balloon', " \
> + " 'property': 'guest-stats' } }");
> + QDict *stats = qdict_get_qdict(ret, "stats");
> +
> + /* We expect 1 entry in the dict for each known kernel stat */
> + assert(qdict_size(stats) == VIRTIO_BALLOON_S_NR);
> +
> + qobject_unref(ret);
> + qtest_quit(s);
> +}
> +
> int main(int argc, char **argv)
> {
> g_test_init(&argc, &argv, NULL);
>
> qtest_add_func("virtio-balloon/oss_fuzz_71649", oss_fuzz_71649);
> + qtest_add_func("virtio-balloon/query-stats", query_stats);
>
> return g_test_run();
> }
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3 for 9.2] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test
2024-11-29 13:55 ` [PATCH v2 2/3 for 9.2] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test Daniel P. Berrangé
2024-11-29 14:01 ` Fabiano Rosas
@ 2024-11-29 15:44 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-11-29 15:44 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, David Hildenbrand, Martin Pitt,
Richard W . M . Jones, Fabiano Rosas, Michael S. Tsirkin
On 29/11/24 14:55, Daniel P. Berrangé wrote:
> This test file is expected to be extended for arbitrary virtio-balloon
> related tests, not merely those discovered by fuzzing.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qtest/meson.build | 2 +-
> .../{fuzz-virtio-balloon-test.c => virtio-balloon-test.c} | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
> rename tests/qtest/{fuzz-virtio-balloon-test.c => virtio-balloon-test.c} (84%)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it
2024-11-29 13:55 [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it Daniel P. Berrangé
` (2 preceding siblings ...)
2024-11-29 13:55 ` [PATCH v2 3/3 for 9.2] tests/qtest: add test for querying balloon guest stats Daniel P. Berrangé
@ 2024-12-02 17:05 ` David Hildenbrand
2024-12-02 19:50 ` Philippe Mathieu-Daudé
3 siblings, 1 reply; 13+ messages in thread
From: David Hildenbrand @ 2024-12-02 17:05 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, Martin Pitt, Richard W . M . Jones,
Fabiano Rosas, Michael S. Tsirkin
On 29.11.24 14:55, Daniel P. Berrangé wrote:
> See patch 1 for the background info on the problem
>
> Changed in v2:
>
> * Add qtest coverage for the crash scenario
>
> Daniel P. Berrangé (3):
> hw/virtio: fix crash in processing balloon stats
> tests/qtest: drop 'fuzz-' prefix from virtio-balloon test
> tests/qtest: add test for querying balloon guest stats
>
> hw/virtio/virtio-balloon.c | 16 +++++++-
> tests/qtest/fuzz-virtio-balloon-test.c | 37 -----------------
> tests/qtest/meson.build | 2 +-
> tests/qtest/virtio-balloon-test.c | 57 ++++++++++++++++++++++++++
> 4 files changed, 73 insertions(+), 39 deletions(-)
> delete mode 100644 tests/qtest/fuzz-virtio-balloon-test.c
> create mode 100644 tests/qtest/virtio-balloon-test.c
>
@MST, do you want to queue this or should I do it? We should get this
into 9.2.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats
2024-11-29 13:55 ` [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats Daniel P. Berrangé
2024-11-29 13:56 ` David Hildenbrand
@ 2024-12-02 18:02 ` Michael Tokarev
1 sibling, 0 replies; 13+ messages in thread
From: Michael Tokarev @ 2024-12-02 18:02 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, David Hildenbrand, Martin Pitt,
Richard W . M . Jones, Fabiano Rosas, Michael S. Tsirkin
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it
2024-12-02 17:05 ` [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it David Hildenbrand
@ 2024-12-02 19:50 ` Philippe Mathieu-Daudé
2024-12-03 8:08 ` Michael S. Tsirkin
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-02 19:50 UTC (permalink / raw)
To: David Hildenbrand, Michael S. Tsirkin
Cc: Laurent Vivier, qemu-devel, Daniel P. Berrangé,
Paolo Bonzini, Martin Pitt, Richard W . M . Jones, Fabiano Rosas
Hi,
On 2/12/24 18:05, David Hildenbrand wrote:
> On 29.11.24 14:55, Daniel P. Berrangé wrote:
>> See patch 1 for the background info on the problem
>>
>> Changed in v2:
>>
>> * Add qtest coverage for the crash scenario
>>
>> Daniel P. Berrangé (3):
>> hw/virtio: fix crash in processing balloon stats
>> tests/qtest: drop 'fuzz-' prefix from virtio-balloon test
>> tests/qtest: add test for querying balloon guest stats
>>
>> hw/virtio/virtio-balloon.c | 16 +++++++-
>> tests/qtest/fuzz-virtio-balloon-test.c | 37 -----------------
>> tests/qtest/meson.build | 2 +-
>> tests/qtest/virtio-balloon-test.c | 57 ++++++++++++++++++++++++++
>> 4 files changed, 73 insertions(+), 39 deletions(-)
>> delete mode 100644 tests/qtest/fuzz-virtio-balloon-test.c
>> create mode 100644 tests/qtest/virtio-balloon-test.c
>>
>
> @MST, do you want to queue this or should I do it? We should get this
> into 9.2.
I'm preparing a PR for tomorrow; I included this series
amending:
-- >8 --
diff --git a/MAINTAINERS b/MAINTAINERS
index 2b1c4abed65..51e3a79b6bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2227,6 +2227,7 @@ F: hw/virtio/virtio-balloon*.c
F: include/hw/virtio/virtio-balloon.h
F: system/balloon.c
F: include/sysemu/balloon.h
+F: tests/qtest/virtio-balloon-test.c
virtio-9p
M: Greg Kurz <groug@kaod.org>
---
Tell me if it isn't appropriate and I'll drop it.
Regards,
Phil.
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it
2024-12-02 19:50 ` Philippe Mathieu-Daudé
@ 2024-12-03 8:08 ` Michael S. Tsirkin
2024-12-03 15:42 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2024-12-03 8:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: David Hildenbrand, Laurent Vivier, qemu-devel,
Daniel P. Berrangé, Paolo Bonzini, Martin Pitt,
Richard W . M . Jones, Fabiano Rosas
On Mon, Dec 02, 2024 at 08:50:55PM +0100, Philippe Mathieu-Daudé wrote:
> Hi,
>
> On 2/12/24 18:05, David Hildenbrand wrote:
> > On 29.11.24 14:55, Daniel P. Berrangé wrote:
> > > See patch 1 for the background info on the problem
> > >
> > > Changed in v2:
> > >
> > > * Add qtest coverage for the crash scenario
> > >
> > > Daniel P. Berrangé (3):
> > > hw/virtio: fix crash in processing balloon stats
> > > tests/qtest: drop 'fuzz-' prefix from virtio-balloon test
> > > tests/qtest: add test for querying balloon guest stats
> > >
> > > hw/virtio/virtio-balloon.c | 16 +++++++-
> > > tests/qtest/fuzz-virtio-balloon-test.c | 37 -----------------
> > > tests/qtest/meson.build | 2 +-
> > > tests/qtest/virtio-balloon-test.c | 57 ++++++++++++++++++++++++++
> > > 4 files changed, 73 insertions(+), 39 deletions(-)
> > > delete mode 100644 tests/qtest/fuzz-virtio-balloon-test.c
> > > create mode 100644 tests/qtest/virtio-balloon-test.c
> > >
> >
> > @MST, do you want to queue this or should I do it? We should get this
> > into 9.2.
>
> I'm preparing a PR for tomorrow; I included this series
> amending:
>
> -- >8 --
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2b1c4abed65..51e3a79b6bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2227,6 +2227,7 @@ F: hw/virtio/virtio-balloon*.c
> F: include/hw/virtio/virtio-balloon.h
> F: system/balloon.c
> F: include/sysemu/balloon.h
> +F: tests/qtest/virtio-balloon-test.c
>
> virtio-9p
> M: Greg Kurz <groug@kaod.org>
> ---
>
> Tell me if it isn't appropriate and I'll drop it.
>
> Regards,
>
> Phil.
Go ahead.
Series:
Acked-by: Michael S. Tsirkin <mst@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it
2024-12-03 8:08 ` Michael S. Tsirkin
@ 2024-12-03 15:42 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-03 15:42 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Hildenbrand, Laurent Vivier, qemu-devel,
Daniel P. Berrangé, Paolo Bonzini, Martin Pitt,
Richard W . M . Jones, Fabiano Rosas
On 3/12/24 09:08, Michael S. Tsirkin wrote:
> On Mon, Dec 02, 2024 at 08:50:55PM +0100, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> On 2/12/24 18:05, David Hildenbrand wrote:
>>> On 29.11.24 14:55, Daniel P. Berrangé wrote:
>>>> See patch 1 for the background info on the problem
>>>>
>>>> Changed in v2:
>>>>
>>>> * Add qtest coverage for the crash scenario
>>>>
>>>> Daniel P. Berrangé (3):
>>>> hw/virtio: fix crash in processing balloon stats
>>>> tests/qtest: drop 'fuzz-' prefix from virtio-balloon test
>>>> tests/qtest: add test for querying balloon guest stats
> Series:> Acked-by: Michael S. Tsirkin <mst@redhat.com>
Series queued, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-12-03 15:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-29 13:55 [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it Daniel P. Berrangé
2024-11-29 13:55 ` [PATCH v2 1/3 for 9.2] hw/virtio: fix crash in processing balloon stats Daniel P. Berrangé
2024-11-29 13:56 ` David Hildenbrand
2024-12-02 18:02 ` Michael Tokarev
2024-11-29 13:55 ` [PATCH v2 2/3 for 9.2] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test Daniel P. Berrangé
2024-11-29 14:01 ` Fabiano Rosas
2024-11-29 15:44 ` Philippe Mathieu-Daudé
2024-11-29 13:55 ` [PATCH v2 3/3 for 9.2] tests/qtest: add test for querying balloon guest stats Daniel P. Berrangé
2024-11-29 14:04 ` Fabiano Rosas
2024-12-02 17:05 ` [PATCH v2 0/3 for 9.2] hw/virtio: fix crash in virtio-balloon and test it David Hildenbrand
2024-12-02 19:50 ` Philippe Mathieu-Daudé
2024-12-03 8:08 ` Michael S. Tsirkin
2024-12-03 15:42 ` Philippe Mathieu-Daudé
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).