qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [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).