Best Regards,
Konstantin Kostiuk.
On Mon, Feb 26, 2024 at 7:02 PM Andrey Drobyshev <
andrey.drobys...@virtuozzo.com> wrote:
> Since the commit 25b5ff1a86 ("qga: add mountpoint usage info to
> GuestFilesystemInfo") we have 2 values reported in guest-get-fsinfo:
> used = (f_blocks - f_bfree), total = (f_blocks - f_bfree + f_bavail).
> These calculations might be obscure for the end user and require one to
> actually get into QGA source to understand how they're obtained. Let's
> just report the values f_blocks, f_bfree, f_bavail (in bytes) from
> statvfs() as they are, letting the user decide how to process them further.
>
> Originally-by: Yuri Pudgorodskiy <y...@virtuozzo.com>
> Signed-off-by: Andrey Drobyshev <andrey.drobys...@virtuozzo.com>
> ---
> qga/commands-posix.c | 16 +++++++---------
> qga/qapi-schema.json | 11 +++++++----
> 2 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 26008db497..752ef509d0 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1554,8 +1554,7 @@ static GuestFilesystemInfo
> *build_guest_fsinfo(struct FsMount *mount,
> Error **errp)
> {
> GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
> - struct statvfs buf;
> - unsigned long used, nonroot_total, fr_size;
> + struct statvfs st;
> char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
> mount->devmajor, mount->devminor);
>
> @@ -1563,15 +1562,14 @@ static GuestFilesystemInfo
> *build_guest_fsinfo(struct FsMount *mount,
> fs->type = g_strdup(mount->devtype);
> build_guest_fsinfo_for_device(devpath, fs, errp);
>
> - if (statvfs(fs->mountpoint, &buf) == 0) {
> - fr_size = buf.f_frsize;
> - used = buf.f_blocks - buf.f_bfree;
> - nonroot_total = used + buf.f_bavail;
> - fs->used_bytes = used * fr_size;
> - fs->total_bytes = nonroot_total * fr_size;
> + if (statvfs(fs->mountpoint, &st) == 0) {
> + fs->total_bytes = st.f_blocks * st.f_frsize;
> + fs->free_bytes = st.f_bfree * st.f_frsize;
> + fs->avail_bytes = st.f_bavail * st.f_frsize;
>
> fs->has_total_bytes = true;
> - fs->has_used_bytes = true;
> + fs->has_free_bytes = true;
> + fs->has_avail_bytes = true;
> }
>
> g_free(devpath);
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index b8efe31897..1cce3c1df5 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1030,9 +1030,12 @@
> #
> # @type: file system type string
> #
> -# @used-bytes: file system used bytes (since 3.0)
> +# @total-bytes: total file system size in bytes (since 8.3)
> #
> -# @total-bytes: non-root file system total bytes (since 3.0)
> +# @free-bytes: amount of free space in file system in bytes (since 8.3)
>
I don't agree with this as it breaks backward compatibility. If we want to
get
these changes we should release a new version with both old and new fields
and mark old as deprecated to get a time for everyone who uses this
API updates its solutions.
A similar thing was with replacing the 'blacklist' command line.
https://gitlab.com/qemu-project/qemu/-/commit/582a098e6ca00dd42f317dad8affd13e5a20bc42
Currently, we support both 'blacklist' and 'block-rpcs' command line options
but the first one wrote a warning.
@Marc-André Lureau <marcandre.lur...@redhat.com> @Philippe Mathieu-Daudé
<phi...@linaro.org>
What do you think about this?
> +#
> +# @avail-bytes: amount of free space in file system for unprivileged
> +# users in bytes (since 8.3)
> #
> # @disk: an array of disk hardware information that the volume lies
> # on, which may be empty if the disk type is not supported
> @@ -1041,8 +1044,8 @@
> ##
> { 'struct': 'GuestFilesystemInfo',
> 'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
> - '*used-bytes': 'uint64', '*total-bytes': 'uint64',
> - 'disk': ['GuestDiskAddress']} }
> + '*total-bytes': 'uint64', '*free-bytes': 'uint64',
> + '*avail-bytes': 'uint64', 'disk': ['GuestDiskAddress']} }
>
> ##
> # @guest-get-fsinfo:
> --
> 2.39.3
>
>
>
