Re: [PATCH v2] hw/virtio: reset virtio balloon stats on machine reset

2025-02-04 Thread David Hildenbrand

On 04.02.25 10:42, Daniel P. Berrangé wrote:

When a machine is first booted, all virtio balloon stats are initialized
to their default value -1 (18446744073709551615 when represented as
unsigned).

They remain that way while the firmware is loading, and early phase of
guest OS boot, until the virtio-balloon driver is activated. Thereafter
the reported stats reflect the guest OS activity.

When a machine reset is performed, however, the virtio-balloon stats are
left unchanged by QEMU, despite the guest OS no longer updating them,
nor indeed even still existing.

IOW, the mgmt app keeps getting stale stats until the guest OS starts
once more and loads the virtio-balloon driver (if ever). At that point
the app will see a discontinuity in the reported values as they sudden
jump from the stale value to the new value. This jump is indigituishable
from a valid data update.

While there is an "last-updated" field to report on the freshness of
the stats, that does not unambiguously tell the mgmt app whether the
stats are still conceptually relevant to the current running workload.

It is more conceptually useful to reset the stats to their default
values on machine reset, given that the previous guest workload the
stats reflect no longer exists. The mgmt app can now clearly identify
that there are is no stats information available from the current
executing workload.

The 'last-updated' time is also reset back to 0.

IOW, on every machine reset, the virtio stats are in the same clean
state they were when the macine first powered on.

A functional test is added to validate this behaviour with a real
world guest OS.

Signed-off-by: Daniel P. Berrangé 
---


Acked-by: David Hildenbrand 

--
Cheers,

David / dhildenb




[PATCH v2] hw/virtio: reset virtio balloon stats on machine reset

2025-02-04 Thread Daniel P . Berrangé
When a machine is first booted, all virtio balloon stats are initialized
to their default value -1 (18446744073709551615 when represented as
unsigned).

They remain that way while the firmware is loading, and early phase of
guest OS boot, until the virtio-balloon driver is activated. Thereafter
the reported stats reflect the guest OS activity.

When a machine reset is performed, however, the virtio-balloon stats are
left unchanged by QEMU, despite the guest OS no longer updating them,
nor indeed even still existing.

IOW, the mgmt app keeps getting stale stats until the guest OS starts
once more and loads the virtio-balloon driver (if ever). At that point
the app will see a discontinuity in the reported values as they sudden
jump from the stale value to the new value. This jump is indigituishable
from a valid data update.

While there is an "last-updated" field to report on the freshness of
the stats, that does not unambiguously tell the mgmt app whether the
stats are still conceptually relevant to the current running workload.

It is more conceptually useful to reset the stats to their default
values on machine reset, given that the previous guest workload the
stats reflect no longer exists. The mgmt app can now clearly identify
that there are is no stats information available from the current
executing workload.

The 'last-updated' time is also reset back to 0.

IOW, on every machine reset, the virtio stats are in the same clean
state they were when the macine first powered on.

A functional test is added to validate this behaviour with a real
world guest OS.

Signed-off-by: Daniel P. Berrangé 
---

In v2:

 - Rename virtio_balloon_system_reset_enter to
   virtio_balloon_reset_enter
 - Adapt for 'reset.h' header moving from 'sysemu' to 'system'
 - Add missing tests/functional/meson.build registration

 MAINTAINERS |   1 +
 hw/virtio/virtio-balloon.c  |  30 -
 include/hw/virtio/virtio-balloon.h  |   4 +
 tests/functional/meson.build|   2 +
 tests/functional/test_virtio_balloon.py | 161 
 5 files changed, 197 insertions(+), 1 deletion(-)
 create mode 100755 tests/functional/test_virtio_balloon.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 0cf37fce7b..160c5717e6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2252,6 +2252,7 @@ F: include/hw/virtio/virtio-balloon.h
 F: system/balloon.c
 F: include/system/balloon.h
 F: tests/qtest/virtio-balloon-test.c
+F: tests/functional/test_virtio_balloon.py
 
 virtio-9p
 M: Greg Kurz 
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index ad05768ded..2eb5a14fa2 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -31,7 +31,7 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 #include "migration/misc.h"
-
+#include "system/reset.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 
@@ -910,6 +910,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, 
Error **errp)
 }
 
 reset_stats(s);
+s->stats_last_update = 0;
+qemu_register_resettable(OBJECT(dev));
 }
 
 static void virtio_balloon_device_unrealize(DeviceState *dev)
@@ -917,6 +919,7 @@ static void virtio_balloon_device_unrealize(DeviceState 
*dev)
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VirtIOBalloon *s = VIRTIO_BALLOON(dev);
 
+qemu_unregister_resettable(OBJECT(dev));
 if (s->free_page_bh) {
 qemu_bh_delete(s->free_page_bh);
 object_unref(OBJECT(s->iothread));
@@ -987,6 +990,27 @@ static void virtio_balloon_set_status(VirtIODevice *vdev, 
uint8_t status)
 }
 }
 
+static ResettableState *virtio_balloon_get_reset_state(Object *obj)
+{
+VirtIOBalloon *s = VIRTIO_BALLOON(obj);
+return &s->reset_state;
+}
+
+static void virtio_balloon_reset_enter(Object *obj, ResetType type)
+{
+VirtIOBalloon *s = VIRTIO_BALLOON(obj);
+
+/*
+ * When waking up from standby/suspend-to-ram, do not reset stats.
+ */
+if (type == RESET_TYPE_WAKEUP) {
+return;
+}
+
+reset_stats(s);
+s->stats_last_update = 0;
+}
+
 static void virtio_balloon_instance_init(Object *obj)
 {
 VirtIOBalloon *s = VIRTIO_BALLOON(obj);
@@ -1038,6 +1062,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, 
void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+ResettableClass *rc = RESETTABLE_CLASS(klass);
 
 device_class_set_props(dc, virtio_balloon_properties);
 dc->vmsd = &vmstate_virtio_balloon;
@@ -1050,6 +1075,9 @@ static void virtio_balloon_class_init(ObjectClass *klass, 
void *data)
 vdc->get_features = virtio_balloon_get_features;
 vdc->set_status = virtio_balloon_set_status;
 vdc->vmsd = &vmstate_virtio_balloon_device;
+
+rc->get_state = virtio_balloon_get_reset_state;
+rc->phases.enter = virtio_balloon_reset_enter;
 }
 
 static const TypeInfo virtio_balloon_info = {
diff --git a/include/h