From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1792501215; b=eZbGqeszitYhv06vSx+G+eM9Vx8DfKxoLYaXyHSvE2c6ofpvZYQYciHEdsp6xYq+Q97m HjACXuHlTC9E97aemYM3skoJ0NGdSaD896vtzFX3+dazxZW1XmdfS6V3O/wAGRr6wCOx/ vTyiZia4pZeVD+z1crvwWVWMeC5VyuduO8DroZ4DSGRwl6K+blNIb2djvDdU/niggk7Wv D8SbYfMWZSyKU7+qrlyNCR/gCRUVoO2MXZ9iFvok3g8GAwqzEMXfTBvE6kLIbx3JaluWg vsAu45DE8/LwOdtG0BNq/kMQf4dkWN3Z+HZvEnxfmFgXs8MtwBjINlPjTqXYHHS3LyQ== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1792501215; h=DMARC-Filter:DKIM-Signature:Received:X-MC-Unique: X-Mimecast-MFC-AGG-ID:Received:Received:Received:From:To:Cc:Subject: In-Reply-To:References:Date:Message-ID:User-Agent:MIME-Version: Content-Type:Content-Transfer-Encoding:X-Scanned-By; bh=BQVuh4DygXzKrPTBQ4jAJMJ/vAel5plTNThDMoKLGCs=; b=jcWCPKYrytAkVsNhhXPYtDF9d8K5cDXdB23tN0BmHrwSUGo1LKRyc5IzYcqmg44t+ZR8 mv5exX+GSuReg3ExcNkWS/s8p18/HogF5ASFoxqhbiUvnR8sYRAlzHaj4g8caWMV5xeCy PVOpnv4qq3EAREGN+RKdzsa6bTojHv7YoNyPGrWJDfozMopDIbdgj1dhqSmiw4VUjz6fX 7s93Pn18yDzAYxEZt562XZ50Ab2wHKJCdZco0phvI8jvQI90yAv7xyhZFOpKzkBvdTvev 0klJJTG9vm3euQ0/4/u8oy/X7pgAuiZ7WwtRh7Me3Nya9UHPe5cVkJxON6jicC3/jMA== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=hGnHWT8b; arc=none smtp.remote-ip=170.10.133.124 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 32F6960A2609 for ; Tue, 20 Oct 2026 13:00:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.2 smtp.subspace.kernel.org 32F6960A2609 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=hGnHWT8b DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1792501214; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BQVuh4DygXzKrPTBQ4jAJMJ/vAel5plTNThDMoKLGCs=; b=hGnHWT8bAI4sSJvWIHvapc88d5AhGuwKC4NYP6jd4dlTdRJwLeu0EoDua5n2KpA47lxaod Fbk85UOTPtz/KyDFy4+3jKn1AhQF47306qwSoP6cVVR4J9DRZriSvf3/bnVtHN/nKpXsr6 6TBWHl9mvhRblae0PPzDVVORLnOSYWA= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-394-SCYlsoeCN1mldLXfI2uYiQ-1; Tue, 20 Oct 2026 13:00:08 +0000 X-MC-Unique: SCYlsoeCN1mldLXfI2uYiQ-1 X-Mimecast-MFC-AGG-ID: SCYlsoeCN1mldLXfI2uYiQ_1792501202 Received: from mx-prod-int-06.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-06.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.93]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id ABD0219539BD; Tue, 20 Oct 2026 13:00:00 +0000 (UTC) Received: from blackfin.pond.sub.org (unknown [10.44.22.5]) by mx-prod-int-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id E085418002B1; Tue, 20 Oct 2026 12:59:56 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 65E3F21E6A04; Tue, 20 Oct 2026 14:59:54 +0200 (CEST) From: Markus Armbruster To: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Cc: qemu-devel@nongnu.org, Michael Roth , John Snow , Peter Maydell , Mauro Carvalho Chehab , Pierrick Bouvier , Paolo Bonzini , Daniel P. =?utf-8?Q?Berrang=C3=A9?= , Kostiantyn Kostiuk , Eric Blake , David Hildenbrand , Igor Mammedov , "Edgar E. Iglesias" , Jason Wang , Marcelo Tosatti , kvm@vger.kernel.org, Alberto Garcia , Kevin Wolf , Hanna Reitz , Philippe =?utf-8?Q?Mathieu-Daud=C3=A9?= , Zhao Liu , Jonathan Cameron , Junjie Cao , Anisa Su , Elena Ufimtseva , Jagannathan Raman , John Levon , Thanos Makatos , =?utf-8?Q?C=C3=A9dric?= Le Goater , Cornelia Huck , Eric Farman , Matthew Rosato , Richard Henderson , Ilya Leoshkevich , qemu-block@nongnu.org, linux-cxl@vger.kernel.org, qemu-s390x@nongnu.org, "Michael S. Tsirkin" , Sergio Lopez , qemu-arm@nongnu.org, Palmer Dabbelt , Alistair Francis , Weiwei Li , Daniel Henrique Barboza , Liu Zhiwei , Chao Liu , qemu-riscv@nongnu.org, Song Gao <17746591750@163.com>, Bibo Mao , Xianglai Li , Jiaxun Yang , Pedro Barbuda , Mohamed Mediouni , Nicholas Piggin , Harsh Prateek Bora , Amit Machhiwal , qemu-ppc@nongnu.org, Zhang Chen , Li Zhijian , Alistair Francis , Alexander Graf , "Gonglei (Arei)" , zhenwei pi , Ani Sinha , Luc Michel , =?utf-8?Q?C=C3=A9dric?= Le Goater , Jamin Lin , Kane Chen , Samuel Tardieu , "Denis V. Lunev" , FangSheng Huang , Tyrone Ting , Hao Wu , Keith Busch , Klaus Jensen , Jesper Devantier , Aditya Gupta , Glenn Miles , Conor Dooley , Sebastian Huber , Halil Pasic , Christian Borntraeger , Jason Herne , Titus Rwantare , Stefano Stabellini , Anthony PERARD , Peter Xu , Chinmay Rath , Hendrik Brueckner , xen-devel@lists.xenproject.org, Roman Bolshakov , Phil Dennis-Jordan , Fam Zheng , Farhan Ali , Alex Williamson , Tao Tang , Stefan Berger , Fabiano Rosas , Mark Cave-Ayland , Artyom Tarasenko , Yoshinori Sato , Jiri Pirko , Eric Auger , Laurent Vivier Subject: Re: [PATCH v7 42/65] virtio-balloon: convert guest-stats property to QAPI type In-Reply-To: <20261020-qom-qapi-v7-42-431a86b02b58@redhat.com> (=?utf-8?Q?=22Marc-Andr=C3=A9?= Lureau"'s message of "Tue, 20 Oct 2026 14:45:59 +0400") References: <20261020-qom-qapi-v7-0-431a86b02b58@redhat.com> <20261020-qom-qapi-v7-42-431a86b02b58@redhat.com> Date: Tue, 20 Oct 2026 14:59:54 +0200 Message-ID: <87h5igplx1.fsf@pond.sub.org> User-Agent: Gnus/5.13 (Gnus v5.13) X-Mailing-List: kvm@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.93 Marc-Andr=C3=A9 Lureau writes: > Define VirtioBalloonStats and VirtioBalloonGuestStats QAPI struct types > in the schema, replacing the hand-rolled visitor in balloon_stats_get_all= () > with a generated visit_type_VirtioBalloonGuestStats() call. > > The wire format is preserved: the JSON output from qom-get is identical > to the previous hand-rolled visitor output. > > Signed-off-by: Markus Armbruster > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > docs/interop/virtio-balloon-stats.rst | 22 +-------- > hw/virtio/virtio-balloon.c | 84 ++++++++++++-----------------= --- > qapi/machine.json | 92 +++++++++++++++++++++++++++++= ++++++ > 3 files changed, 123 insertions(+), 75 deletions(-) > > diff --git a/docs/interop/virtio-balloon-stats.rst b/docs/interop/virtio-= balloon-stats.rst > index b9a6a6edb229..88d3271bbb02 100644 > --- a/docs/interop/virtio-balloon-stats.rst > +++ b/docs/interop/virtio-balloon-stats.rst > @@ -22,27 +22,7 @@ polling the guest's balloon driver for new stats in th= e specified time > interval. >=20=20 > To retrieve those stats, clients have to query the guest-stats property, > -which will return a dictionary containing: > - > - * A key named 'stats', containing all available stats. If the guest > - doesn't support a particular stat, or if it couldn't be retrieved, > - its value will be -1. Currently, the following stats are supported: > - > - - stat-swap-in > - - stat-swap-out > - - stat-major-faults > - - stat-minor-faults > - - stat-free-memory > - - stat-total-memory > - - stat-available-memory > - - stat-disk-caches > - - stat-htlb-pgalloc > - - stat-htlb-pgfail Missing here: - stat-oom-kills - stat-alloc-stalls - stat-async-scans - stat-direct-scans - stat-async-reclaims - stat-direct-reclaims Review note, not a request for anything. > - > - * A key named last-update, which contains the last stats update > - timestamp in seconds. Since this timestamp is generated by the host, > - a buggy guest can't influence its value. The value is 0 if the guest > - has not updated the stats (yet). > +which will return a VirtioBalloonGuestStats (see QAPI documentation). >=20=20 > It's also important to note the following: >=20=20 > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index 7ffbe48e5134..bfc75f5dfc72 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -29,6 +29,8 @@ > #include "qapi/error.h" > #include "qapi/qapi-builtin-type-infos.h" > #include "qapi/qapi-events-machine.h" > +#include "qapi/qapi-type-infos-machine.h" > +#include "qapi/qapi-visit-machine.h" > #include "qapi/visitor.h" > #include "trace.h" > #include "qemu/error-report.h" > @@ -170,33 +172,8 @@ static void balloon_deflate_page(VirtIOBalloon *ball= oon, > } > } >=20=20 > -/* > - * 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[] =3D { > - [VIRTIO_BALLOON_S_SWAP_IN] =3D "stat-swap-in", > - [VIRTIO_BALLOON_S_SWAP_OUT] =3D "stat-swap-out", > - [VIRTIO_BALLOON_S_MAJFLT] =3D "stat-major-faults", > - [VIRTIO_BALLOON_S_MINFLT] =3D "stat-minor-faults", > - [VIRTIO_BALLOON_S_MEMFREE] =3D "stat-free-memory", > - > - [VIRTIO_BALLOON_S_MEMTOT] =3D "stat-total-memory", > - [VIRTIO_BALLOON_S_AVAIL] =3D "stat-available-memory", > - [VIRTIO_BALLOON_S_CACHES] =3D "stat-disk-caches", > - [VIRTIO_BALLOON_S_HTLB_PGALLOC] =3D "stat-htlb-pgalloc", > - [VIRTIO_BALLOON_S_HTLB_PGFAIL] =3D "stat-htlb-pgfail", > - > - [VIRTIO_BALLOON_S_OOM_KILL] =3D "stat-oom-kills", > - [VIRTIO_BALLOON_S_ALLOC_STALL] =3D "stat-alloc-stalls", > - [VIRTIO_BALLOON_S_ASYNC_SCAN] =3D "stat-async-scans", > - [VIRTIO_BALLOON_S_DIRECT_SCAN] =3D "stat-direct-scans", > - [VIRTIO_BALLOON_S_ASYNC_RECLAIM] =3D "stat-async-reclaims", > - > - [VIRTIO_BALLOON_S_DIRECT_RECLAIM] =3D "stat-direct-reclaims", Not missing here. > -}; > -G_STATIC_ASSERT(G_N_ELEMENTS(balloon_stat_names) =3D=3D VIRTIO_BALLOON_S= _NR); > +/* Update VirtioBalloonStats QAPI type when new stats are added */ > +G_STATIC_ASSERT(VIRTIO_BALLOON_S_NR =3D=3D 16); The assertion becomes awkward. What about ... >=20=20 > /* > * reset_stats - Mark all items in the stats array as unset > @@ -258,33 +235,31 @@ static void balloon_stats_get_all(Object *obj, Visi= tor *v, const char *name, > void *opaque, Error **errp) > { > VirtIOBalloon *s =3D VIRTIO_BALLOON(obj); > - bool ok =3D false; > - int i; > + VirtioBalloonStats stats =3D { > + .stat_swap_in =3D s->stats[VIRTIO_BALLOON_S_SWAP_IN], > + .stat_swap_out =3D s->stats[VIRTIO_BALLOON_S_SWAP_OUT], > + .stat_major_faults =3D s->stats[VIRTIO_BALLOON_S_MAJFLT], > + .stat_minor_faults =3D s->stats[VIRTIO_BALLOON_S_MINFLT], > + .stat_free_memory =3D s->stats[VIRTIO_BALLOON_S_MEMFREE], > + .stat_total_memory =3D s->stats[VIRTIO_BALLOON_S_MEMTOT], > + .stat_available_memory =3D s->stats[VIRTIO_BALLOON_S_AVAIL], > + .stat_disk_caches =3D s->stats[VIRTIO_BALLOON_S_CACHES], > + .stat_htlb_pgalloc =3D s->stats[VIRTIO_BALLOON_S_HTLB_PGALLOC], > + .stat_htlb_pgfail =3D s->stats[VIRTIO_BALLOON_S_HTLB_PGFAIL], > + .stat_oom_kills =3D s->stats[VIRTIO_BALLOON_S_OOM_KILL], > + .stat_alloc_stalls =3D s->stats[VIRTIO_BALLOON_S_ALLOC_STALL], > + .stat_async_scans =3D s->stats[VIRTIO_BALLOON_S_ASYNC_SCAN], > + .stat_direct_scans =3D s->stats[VIRTIO_BALLOON_S_DIRECT_SCAN], > + .stat_async_reclaims =3D s->stats[VIRTIO_BALLOON_S_ASYNC_RECLAIM= ], > + .stat_direct_reclaims =3D s->stats[VIRTIO_BALLOON_S_DIRECT_RECLA= IM], > + }; ... putting it here? This initializer also needs to be updated when stats are added. Another review note... The kernel gives us an array of stats with well-known indexes. Since we "improved" on that interface, we need to manually convert to struct. Oh well. > + VirtioBalloonGuestStats guest_stats =3D { > + .last_update =3D s->stats_last_update, > + .stats =3D &stats, > + }; > + VirtioBalloonGuestStats *argp =3D &guest_stats; >=20=20 > - if (!visit_start_struct(v, name, NULL, 0, errp)) { > - return; > - } > - if (!visit_type_int(v, "last-update", &s->stats_last_update, errp)) { > - goto out_end; > - } > - > - if (!visit_start_struct(v, "stats", NULL, 0, errp)) { > - goto out_end; > - } > - for (i =3D 0; i < VIRTIO_BALLOON_S_NR; i++) { > - if (!visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], e= rrp)) { > - goto out_nested; > - } > - } > - ok =3D visit_check_struct(v, errp); > -out_nested: > - visit_end_struct(v, NULL); > - > - if (ok) { > - visit_check_struct(v, errp); > - } > -out_end: > - visit_end_struct(v, NULL); > + visit_type_VirtioBalloonGuestStats(v, name, &argp, errp); > } >=20=20 > static void balloon_stats_get_poll_interval(Object *obj, Visitor *v, > @@ -1037,7 +1012,8 @@ static void virtio_balloon_instance_init(Object *ob= j) > s->free_page_hint_cmd_id =3D VIRTIO_BALLOON_FREE_PAGE_HINT_CMD_ID_MI= N; > s->free_page_hint_notify.notify =3D virtio_balloon_free_page_hint_no= tify; >=20=20 > - object_property_add(obj, "guest-stats", "guest statistics", > + object_property_add_qapi(obj, "guest-stats", > + &VirtioBalloonGuestStats_type_info, > balloon_stats_get_all, NULL, NULL, NULL); The @type reported by qom-list & friends changes from "guest statistics" to "VirtioBalloonStats". Such changes can be compatibility breaks. We accept this one, because: 1. What "guest statistics" might mean was never documented. ObjectPropertyInfo's (flawed) comment covers only "primitive", child and link types, which misleadingly suggests values like this one don't exist. 2. We are not aware of anything relying on it, and we can't quite see why anything would want to rely on it. 3. @type is a bad interface we ought to deprecate in favor of @qapi-type. All this needs to be noted in the commit message. Perhaps even in release notes. I guess the latter will be easier if we do deprecate @type. >=20=20 > object_property_add_qapi(obj, "guest-stats-polling-interval", &int_t= ype_info, > diff --git a/qapi/machine.json b/qapi/machine.json > index 9b145c635baf..34be3156270c 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1126,6 +1126,98 @@ > ## > { 'struct': 'BalloonInfo', 'data': {'actual': 'int' } } >=20=20 > +## > +# @VirtioBalloonStats: > +# > +# VirtIO balloon device guest memory statistics. > +# > +# If the guest doesn't support a particular stat, or if it couldn't > +# be retrieved, its value will be -1. Please use the revised syntax for introductions: ## # @name: introduction, best to keep it brief, but you may continue it # like this if necessary. If your text need to refer to arguments # / members, it's almost certainly not brief introduction anymore. # # Additional text can go here. But do consider putting it further # down. I apologize for the inconvenience. We need this to eliminate certain syntactic ambiguities that plagued the doc generator. We'll need it some more for the doc inliner. > +# > +# @stat-swap-in: The amount of memory that has been > +# swapped in (in bytes). Wrap lines at column 70, please. > +# > +# @stat-swap-out: The amount of memory that has been > +# swapped out to disk (in bytes). > +# > +# @stat-major-faults: The number of major page faults > +# that have occurred. > +# > +# @stat-minor-faults: The number of minor page faults > +# that have occurred. > +# > +# @stat-free-memory: The amount of memory not being used > +# for any purpose (in bytes). > +# > +# @stat-total-memory: The total amount of memory available > +# (in bytes). > +# > +# @stat-available-memory: An estimate of how much memory is available > +# (in bytes) for starting new applications, without pushing the > +# system to swap. > +# > +# @stat-disk-caches: The amount of memory, in bytes, that can be > +# quickly reclaimed without additional I/O. Typically these pages > +# are used for caching files from disk. > +# > +# @stat-htlb-pgalloc: The number of successful hugetlb page > +# allocations > +# > +# @stat-htlb-pgfail: The number of failed hugetlb page > +# allocations > +# > +# @stat-oom-kills: The number of OOM killer invocations > +# > +# @stat-alloc-stalls: The number of memory allocation stalls > +# > +# @stat-async-scans: The number of memory scanned asynchronously > +# > +# @stat-direct-scans: The number of memory scanned directly > +# > +# @stat-async-reclaims: The number of memory reclaimed asynchronously > +# > +# @stat-direct-reclaims: The number of memory reclaimed directly > +# > +# Since: 11.2 The actual interface, namely the device property, is much older (1.4?). Do we care? I'm not sure; this isn't part of QMP proper, it's for use with QOM, where documentation has always been rather weak. > +## > +{ 'struct': 'VirtioBalloonStats', > + 'data': { > + 'stat-swap-in': 'uint64', > + 'stat-swap-out': 'uint64', > + 'stat-major-faults': 'uint64', > + 'stat-minor-faults': 'uint64', > + 'stat-free-memory': 'uint64', > + 'stat-total-memory': 'uint64', > + 'stat-available-memory': 'uint64', > + 'stat-disk-caches': 'uint64', > + 'stat-htlb-pgalloc': 'uint64', > + 'stat-htlb-pgfail': 'uint64', > + 'stat-oom-kills': 'uint64', > + 'stat-alloc-stalls': 'uint64', > + 'stat-async-scans': 'uint64', > + 'stat-direct-scans': 'uint64', > + 'stat-async-reclaims': 'uint64', > + 'stat-direct-reclaims': 'uint64' } } I don't like the "stat-" prefix, but we can't change that now. > + > +## > +# @VirtioBalloonGuestStats: > +# > +# Guest statistics from the VirtIO balloon device. Please use the revised syntax for introductions. > +# > +# @last-update: timestamp in seconds of the last stats > +# update from the guest (since this timestamp is generated > +# by the host, a buggy guest can't influence its value), > +# or 0 if no update has been received yet. The original text is=20 * A key named last-update, which contains the last stats update timestamp in seconds. Since this timestamp is generated by the host, a buggy guest can't influence its value. The value is 0 if the guest has not updated the stats (yet). Threee sentences instead of one with a parenthesis. I find this easier to read. > +# > +# @stats: balloon memory statistics > +# > +# Since: 11.2 > +## > +{ 'struct': 'VirtioBalloonGuestStats', > + 'data': { > + 'last-update': 'int', > + 'stats': 'VirtioBalloonStats' } } > + > ## > # @query-balloon: > # Return information about the balloon device.