From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>

Unmap stats are useful for debugging how efficient qemu is in freeing host
disk space.

Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
---
 src/qemu/qemu_driver.c       | 3 +++
 src/qemu/qemu_monitor.h      | 3 +++
 src/qemu/qemu_monitor_json.c | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5ee15bab7a..6725bddff9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17451,6 +17451,9 @@ qemuDomainGetStatsBlockExportFrontend(const char *frontendname,
         virTypedParamListAddULLong(par, en->wr_req, "block.%zu.wr.reqs", idx) < 0 ||
         virTypedParamListAddULLong(par, en->wr_bytes, "block.%zu.wr.bytes", idx) < 0 ||
         virTypedParamListAddULLong(par, en->wr_total_times, "block.%zu.wr.times", idx) < 0 ||
+        virTypedParamListAddULLong(par, en->unmap_req, "block.%zu.un.reqs", idx) < 0 ||
+        virTypedParamListAddULLong(par, en->unmap_bytes, "block.%zu.un.bytes", idx) < 0 ||
+        virTypedParamListAddULLong(par, en->unmap_total_times, "block.%zu.un.times", idx) < 0 ||
         virTypedParamListAddULLong(par, en->flush_req, "block.%zu.fl.reqs", idx) < 0 ||
         virTypedParamListAddULLong(par, en->flush_total_times, "block.%zu.fl.times", idx) < 0)
         return -1;
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 72db0c0838..0c34319ea2 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -667,6 +667,9 @@ struct _qemuBlockStats {
     unsigned long long wr_total_times;
     unsigned long long flush_req;
     unsigned long long flush_total_times;
+    unsigned long long unmap_req;
+    unsigned long long unmap_bytes;
+    unsigned long long unmap_total_times;
     unsigned long long capacity;
     unsigned long long physical;
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index b2c0b20a11..c0fe4a7554 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2280,6 +2280,9 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValue *dev,
     QEMU_MONITOR_BLOCK_STAT_GET("wr_total_time_ns", bstats->wr_total_times, false);
     QEMU_MONITOR_BLOCK_STAT_GET("flush_operations", bstats->flush_req, false);
     QEMU_MONITOR_BLOCK_STAT_GET("flush_total_time_ns", bstats->flush_total_times, false);
+    QEMU_MONITOR_BLOCK_STAT_GET("unmap_operations", bstats->unmap_req, false);
+    QEMU_MONITOR_BLOCK_STAT_GET("unmap_bytes", bstats->unmap_bytes, false);
+    QEMU_MONITOR_BLOCK_STAT_GET("unmap_total_time_ns", bstats->unmap_total_times, false);
 #undef QEMU_MONITOR_BLOCK_STAT_GET
 
     if ((parent = virJSONValueObjectGetObject(dev, "parent")) &&
-- 
2.40.1

On Thu, Jun 01, 2023 at 12:44:19 +0600, Oleg Vasilev wrote:
> From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> 
> Unmap stats are useful for debugging how efficient qemu is in freeing host
> disk space.

"debugging [..] qemu" seems a bit too weak of a justification. Do you
have any better usage for this data?

> 
> Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>

Missing sign-off by original author. (See 'From' field above)

> ---
>  src/qemu/qemu_driver.c       | 3 +++
>  src/qemu/qemu_monitor.h      | 3 +++
>  src/qemu/qemu_monitor_json.c | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5ee15bab7a..6725bddff9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17451,6 +17451,9 @@ qemuDomainGetStatsBlockExportFrontend(const char *frontendname,
>          virTypedParamListAddULLong(par, en->wr_req, "block.%zu.wr.reqs", idx) < 0 ||
>          virTypedParamListAddULLong(par, en->wr_bytes, "block.%zu.wr.bytes", idx) < 0 ||
>          virTypedParamListAddULLong(par, en->wr_total_times, "block.%zu.wr.times", idx) < 0 ||
> +        virTypedParamListAddULLong(par, en->unmap_req, "block.%zu.un.reqs", idx) < 0 ||
> +        virTypedParamListAddULLong(par, en->unmap_bytes, "block.%zu.un.bytes", idx) < 0 ||
> +        virTypedParamListAddULLong(par, en->unmap_total_times, "block.%zu.un.times", idx) < 0 ||

Missing documentation for the new fields in:

docs/manpages/virsh.rst - section for 'domstats' has:

 * ``block.<num>.rd.times`` - total time (ns) spent on reads
 * ``block.<num>.wr.reqs`` - number of write requests
 * ``block.<num>.wr.bytes`` - number of written bytes
 * ``block.<num>.wr.times`` - total time (ns) spent on writes

src/libvirt-domain.c - function comment for 'virConnectGetAllDomainStats'

 *     "block.<num>.wr.reqs" - number of write requests as unsigned long long.
 *     "block.<num>.wr.bytes" - number of written bytes as unsigned long long.
 *     "block.<num>.wr.times" - total time (ns) spent on writes as
 *                              unsigned long long.

also, both internally and in qemu it's spelled 'unmap' so consider
spelling it in full here too.

Good thing is that the stats are reported since qemu-4.2 which is the
minimum supported version so you don't need to add any special handling
for the case if it weren't present.