Re: [PATCH v4 1/7] qga: guest-get-fsinfo: add optional 'total-bytes-privileged' field
Hi Andrey,
I am ok with these changes.
No need new iteration with the version change, I will update before merging.
I will merge it after release.
Best Regards,
Konstantin Kostiuk.
On Mon, Apr 1, 2024 at 5:49 PM Andrey Drobyshev <
andrey.drobys...@virtuozzo.com> wrote:
> On 3/22/24 15:17, Andrey Drobyshev wrote:
> > On 3/22/24 12:39, Daniel P. Berrangé wrote:
> >> On Wed, Mar 20, 2024 at 06:16:42PM +0200, Andrey Drobyshev 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) as
> >>> returned by statvfs(3). While on Windows guests that's all we can get
> >>> with GetDiskFreeSpaceExA(), on POSIX guests we might also be
> interested in
> >>> total file system size, as it's visible for root user. Let's add an
> >>> optional field 'total-bytes-privileged' to GuestFilesystemInfo struct,
> >>> which'd only be reported on POSIX and represent f_blocks value as
> returned
> >>> by statvfs(3).
> >>>
> >>> While here, also tweak the docs to reflect better where those values
> >>> come from.
> >>>
> >>> Signed-off-by: Andrey Drobyshev
> >>> ---
> >>> qga/commands-posix.c | 2 ++
> >>> qga/commands-win32.c | 1 +
> >>> qga/qapi-schema.json | 7 +--
> >>> 3 files changed, 8 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> >>> index 26008db497..7df2d72e9f 100644
> >>> --- a/qga/commands-posix.c
> >>> +++ b/qga/commands-posix.c
> >>> @@ -1569,8 +1569,10 @@ static GuestFilesystemInfo
> *build_guest_fsinfo(struct FsMount *mount,
> >>> nonroot_total = used + buf.f_bavail;
> >>> fs->used_bytes = used * fr_size;
> >>> fs->total_bytes = nonroot_total * fr_size;
> >>> +fs->total_bytes_privileged = buf.f_blocks * fr_size;
> >>>
> >>> fs->has_total_bytes = true;
> >>> +fs->has_total_bytes_privileged = true;
> >>> fs->has_used_bytes = true;
> >>> }
> >>>
> >>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> >>> index 6242737b00..6fee0e1e6f 100644
> >>> --- a/qga/commands-win32.c
> >>> +++ b/qga/commands-win32.c
> >>> @@ -1143,6 +1143,7 @@ static GuestFilesystemInfo
> *build_guest_fsinfo(char *guid, Error **errp)
> >>> fs = g_malloc(sizeof(*fs));
> >>> fs->name = g_strdup(guid);
> >>> fs->has_total_bytes = false;
> >>> +fs->has_total_bytes_privileged = false;
> >>> fs->has_used_bytes = false;
> >>> if (len == 0) {
> >>> fs->mountpoint = g_strdup("System Reserved");
> >>> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> >>> index 9554b566a7..dcc469b268 100644
> >>> --- a/qga/qapi-schema.json
> >>> +++ b/qga/qapi-schema.json
> >>> @@ -1026,7 +1026,10 @@
> >>> #
> >>> # @used-bytes: file system used bytes (since 3.0)
> >>> #
> >>> -# @total-bytes: non-root file system total bytes (since 3.0)
> >>> +# @total-bytes: filesystem capacity in bytes for unprivileged users
> (since 3.0)
> >>> +#
> >>> +# @total-bytes-privileged: filesystem capacity in bytes for
> privileged users
> >>> +# (since 9.0)
> >>
> >> Will need to be '9.1', not '9.0', since we don't accept new features
> >> during freeze periods.
> >>
> >>> #
> >>> # @disk: an array of disk hardware information that the volume lies
> >>> # on, which may be empty if the disk type is not supported
> >>> @@ -1036,7 +1039,7 @@
> >>> { 'struct': 'GuestFilesystemInfo',
> >>>'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
> >>> '*used-bytes': 'uint64', '*total-bytes': 'uint64',
> >>> - 'disk': ['GuestDiskAddress']} }
> >>> + '*total-bytes-privileged': 'uint64', 'disk':
> ['GuestDiskAddress']} }
> >>>
> >>> ##
> >>> # @guest-get-fsinfo:
> >>
> >> Assuming the version is changed:
> >>
> >> Reviewed-by: Daniel P. Berrangé
> >>
> >> With regards,
> >> Daniel
> >
> > Thanks, Daniel.
> >
> > @kkostiuk do I need to send one more iteration with the version change?
> > Whatever is most convenient for you.
> >
> > Andrey
>
> Ping
>
>
Re: [PATCH v4 1/7] qga: guest-get-fsinfo: add optional 'total-bytes-privileged' field
On 3/22/24 15:17, Andrey Drobyshev wrote:
> On 3/22/24 12:39, Daniel P. Berrangé wrote:
>> On Wed, Mar 20, 2024 at 06:16:42PM +0200, Andrey Drobyshev 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) as
>>> returned by statvfs(3). While on Windows guests that's all we can get
>>> with GetDiskFreeSpaceExA(), on POSIX guests we might also be interested in
>>> total file system size, as it's visible for root user. Let's add an
>>> optional field 'total-bytes-privileged' to GuestFilesystemInfo struct,
>>> which'd only be reported on POSIX and represent f_blocks value as returned
>>> by statvfs(3).
>>>
>>> While here, also tweak the docs to reflect better where those values
>>> come from.
>>>
>>> Signed-off-by: Andrey Drobyshev
>>> ---
>>> qga/commands-posix.c | 2 ++
>>> qga/commands-win32.c | 1 +
>>> qga/qapi-schema.json | 7 +--
>>> 3 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>>> index 26008db497..7df2d72e9f 100644
>>> --- a/qga/commands-posix.c
>>> +++ b/qga/commands-posix.c
>>> @@ -1569,8 +1569,10 @@ static GuestFilesystemInfo
>>> *build_guest_fsinfo(struct FsMount *mount,
>>> nonroot_total = used + buf.f_bavail;
>>> fs->used_bytes = used * fr_size;
>>> fs->total_bytes = nonroot_total * fr_size;
>>> +fs->total_bytes_privileged = buf.f_blocks * fr_size;
>>>
>>> fs->has_total_bytes = true;
>>> +fs->has_total_bytes_privileged = true;
>>> fs->has_used_bytes = true;
>>> }
>>>
>>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>>> index 6242737b00..6fee0e1e6f 100644
>>> --- a/qga/commands-win32.c
>>> +++ b/qga/commands-win32.c
>>> @@ -1143,6 +1143,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char
>>> *guid, Error **errp)
>>> fs = g_malloc(sizeof(*fs));
>>> fs->name = g_strdup(guid);
>>> fs->has_total_bytes = false;
>>> +fs->has_total_bytes_privileged = false;
>>> fs->has_used_bytes = false;
>>> if (len == 0) {
>>> fs->mountpoint = g_strdup("System Reserved");
>>> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
>>> index 9554b566a7..dcc469b268 100644
>>> --- a/qga/qapi-schema.json
>>> +++ b/qga/qapi-schema.json
>>> @@ -1026,7 +1026,10 @@
>>> #
>>> # @used-bytes: file system used bytes (since 3.0)
>>> #
>>> -# @total-bytes: non-root file system total bytes (since 3.0)
>>> +# @total-bytes: filesystem capacity in bytes for unprivileged users (since
>>> 3.0)
>>> +#
>>> +# @total-bytes-privileged: filesystem capacity in bytes for privileged
>>> users
>>> +# (since 9.0)
>>
>> Will need to be '9.1', not '9.0', since we don't accept new features
>> during freeze periods.
>>
>>> #
>>> # @disk: an array of disk hardware information that the volume lies
>>> # on, which may be empty if the disk type is not supported
>>> @@ -1036,7 +1039,7 @@
>>> { 'struct': 'GuestFilesystemInfo',
>>>'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
>>> '*used-bytes': 'uint64', '*total-bytes': 'uint64',
>>> - 'disk': ['GuestDiskAddress']} }
>>> + '*total-bytes-privileged': 'uint64', 'disk':
>>> ['GuestDiskAddress']} }
>>>
>>> ##
>>> # @guest-get-fsinfo:
>>
>> Assuming the version is changed:
>>
>> Reviewed-by: Daniel P. Berrangé
>>
>> With regards,
>> Daniel
>
> Thanks, Daniel.
>
> @kkostiuk do I need to send one more iteration with the version change?
> Whatever is most convenient for you.
>
> Andrey
Ping
Re: [PATCH v4 1/7] qga: guest-get-fsinfo: add optional 'total-bytes-privileged' field
On 3/22/24 12:39, Daniel P. Berrangé wrote:
> On Wed, Mar 20, 2024 at 06:16:42PM +0200, Andrey Drobyshev 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) as
>> returned by statvfs(3). While on Windows guests that's all we can get
>> with GetDiskFreeSpaceExA(), on POSIX guests we might also be interested in
>> total file system size, as it's visible for root user. Let's add an
>> optional field 'total-bytes-privileged' to GuestFilesystemInfo struct,
>> which'd only be reported on POSIX and represent f_blocks value as returned
>> by statvfs(3).
>>
>> While here, also tweak the docs to reflect better where those values
>> come from.
>>
>> Signed-off-by: Andrey Drobyshev
>> ---
>> qga/commands-posix.c | 2 ++
>> qga/commands-win32.c | 1 +
>> qga/qapi-schema.json | 7 +--
>> 3 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index 26008db497..7df2d72e9f 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -1569,8 +1569,10 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct
>> FsMount *mount,
>> nonroot_total = used + buf.f_bavail;
>> fs->used_bytes = used * fr_size;
>> fs->total_bytes = nonroot_total * fr_size;
>> +fs->total_bytes_privileged = buf.f_blocks * fr_size;
>>
>> fs->has_total_bytes = true;
>> +fs->has_total_bytes_privileged = true;
>> fs->has_used_bytes = true;
>> }
>>
>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>> index 6242737b00..6fee0e1e6f 100644
>> --- a/qga/commands-win32.c
>> +++ b/qga/commands-win32.c
>> @@ -1143,6 +1143,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char
>> *guid, Error **errp)
>> fs = g_malloc(sizeof(*fs));
>> fs->name = g_strdup(guid);
>> fs->has_total_bytes = false;
>> +fs->has_total_bytes_privileged = false;
>> fs->has_used_bytes = false;
>> if (len == 0) {
>> fs->mountpoint = g_strdup("System Reserved");
>> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
>> index 9554b566a7..dcc469b268 100644
>> --- a/qga/qapi-schema.json
>> +++ b/qga/qapi-schema.json
>> @@ -1026,7 +1026,10 @@
>> #
>> # @used-bytes: file system used bytes (since 3.0)
>> #
>> -# @total-bytes: non-root file system total bytes (since 3.0)
>> +# @total-bytes: filesystem capacity in bytes for unprivileged users (since
>> 3.0)
>> +#
>> +# @total-bytes-privileged: filesystem capacity in bytes for privileged users
>> +# (since 9.0)
>
> Will need to be '9.1', not '9.0', since we don't accept new features
> during freeze periods.
>
>> #
>> # @disk: an array of disk hardware information that the volume lies
>> # on, which may be empty if the disk type is not supported
>> @@ -1036,7 +1039,7 @@
>> { 'struct': 'GuestFilesystemInfo',
>>'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
>> '*used-bytes': 'uint64', '*total-bytes': 'uint64',
>> - 'disk': ['GuestDiskAddress']} }
>> + '*total-bytes-privileged': 'uint64', 'disk':
>> ['GuestDiskAddress']} }
>>
>> ##
>> # @guest-get-fsinfo:
>
> Assuming the version is changed:
>
> Reviewed-by: Daniel P. Berrangé
>
> With regards,
> Daniel
Thanks, Daniel.
@kkostiuk do I need to send one more iteration with the version change?
Whatever is most convenient for you.
Andrey
Re: [PATCH v4 1/7] qga: guest-get-fsinfo: add optional 'total-bytes-privileged' field
On Wed, Mar 20, 2024 at 06:16:42PM +0200, Andrey Drobyshev 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) as
> returned by statvfs(3). While on Windows guests that's all we can get
> with GetDiskFreeSpaceExA(), on POSIX guests we might also be interested in
> total file system size, as it's visible for root user. Let's add an
> optional field 'total-bytes-privileged' to GuestFilesystemInfo struct,
> which'd only be reported on POSIX and represent f_blocks value as returned
> by statvfs(3).
>
> While here, also tweak the docs to reflect better where those values
> come from.
>
> Signed-off-by: Andrey Drobyshev
> ---
> qga/commands-posix.c | 2 ++
> qga/commands-win32.c | 1 +
> qga/qapi-schema.json | 7 +--
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 26008db497..7df2d72e9f 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1569,8 +1569,10 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct
> FsMount *mount,
> nonroot_total = used + buf.f_bavail;
> fs->used_bytes = used * fr_size;
> fs->total_bytes = nonroot_total * fr_size;
> +fs->total_bytes_privileged = buf.f_blocks * fr_size;
>
> fs->has_total_bytes = true;
> +fs->has_total_bytes_privileged = true;
> fs->has_used_bytes = true;
> }
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 6242737b00..6fee0e1e6f 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1143,6 +1143,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char
> *guid, Error **errp)
> fs = g_malloc(sizeof(*fs));
> fs->name = g_strdup(guid);
> fs->has_total_bytes = false;
> +fs->has_total_bytes_privileged = false;
> fs->has_used_bytes = false;
> if (len == 0) {
> fs->mountpoint = g_strdup("System Reserved");
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 9554b566a7..dcc469b268 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1026,7 +1026,10 @@
> #
> # @used-bytes: file system used bytes (since 3.0)
> #
> -# @total-bytes: non-root file system total bytes (since 3.0)
> +# @total-bytes: filesystem capacity in bytes for unprivileged users (since
> 3.0)
> +#
> +# @total-bytes-privileged: filesystem capacity in bytes for privileged users
> +# (since 9.0)
Will need to be '9.1', not '9.0', since we don't accept new features
during freeze periods.
> #
> # @disk: an array of disk hardware information that the volume lies
> # on, which may be empty if the disk type is not supported
> @@ -1036,7 +1039,7 @@
> { 'struct': 'GuestFilesystemInfo',
>'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
> '*used-bytes': 'uint64', '*total-bytes': 'uint64',
> - 'disk': ['GuestDiskAddress']} }
> + '*total-bytes-privileged': 'uint64', 'disk':
> ['GuestDiskAddress']} }
>
> ##
> # @guest-get-fsinfo:
Assuming the version is changed:
Reviewed-by: Daniel P. Berrangé
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v4 1/7] qga: guest-get-fsinfo: add optional 'total-bytes-privileged' field
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) as
returned by statvfs(3). While on Windows guests that's all we can get
with GetDiskFreeSpaceExA(), on POSIX guests we might also be interested in
total file system size, as it's visible for root user. Let's add an
optional field 'total-bytes-privileged' to GuestFilesystemInfo struct,
which'd only be reported on POSIX and represent f_blocks value as returned
by statvfs(3).
While here, also tweak the docs to reflect better where those values
come from.
Signed-off-by: Andrey Drobyshev
---
qga/commands-posix.c | 2 ++
qga/commands-win32.c | 1 +
qga/qapi-schema.json | 7 +--
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 26008db497..7df2d72e9f 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1569,8 +1569,10 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct
FsMount *mount,
nonroot_total = used + buf.f_bavail;
fs->used_bytes = used * fr_size;
fs->total_bytes = nonroot_total * fr_size;
+fs->total_bytes_privileged = buf.f_blocks * fr_size;
fs->has_total_bytes = true;
+fs->has_total_bytes_privileged = true;
fs->has_used_bytes = true;
}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 6242737b00..6fee0e1e6f 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1143,6 +1143,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char
*guid, Error **errp)
fs = g_malloc(sizeof(*fs));
fs->name = g_strdup(guid);
fs->has_total_bytes = false;
+fs->has_total_bytes_privileged = false;
fs->has_used_bytes = false;
if (len == 0) {
fs->mountpoint = g_strdup("System Reserved");
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 9554b566a7..dcc469b268 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1026,7 +1026,10 @@
#
# @used-bytes: file system used bytes (since 3.0)
#
-# @total-bytes: non-root file system total bytes (since 3.0)
+# @total-bytes: filesystem capacity in bytes for unprivileged users (since 3.0)
+#
+# @total-bytes-privileged: filesystem capacity in bytes for privileged users
+# (since 9.0)
#
# @disk: an array of disk hardware information that the volume lies
# on, which may be empty if the disk type is not supported
@@ -1036,7 +1039,7 @@
{ 'struct': 'GuestFilesystemInfo',
'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
'*used-bytes': 'uint64', '*total-bytes': 'uint64',
- 'disk': ['GuestDiskAddress']} }
+ '*total-bytes-privileged': 'uint64', 'disk': ['GuestDiskAddress']} }
##
# @guest-get-fsinfo:
--
2.39.3
