[Qemu-devel] [PATCH] qga: add mountpoint usage to GuestFilesystemInfo

[Qemu-devel] [PATCH] qga: add mountpoint usage to GuestFilesystemInfo

[Chen Hanxiao]

From: Chen Hanxiao <chenhanxiao@gmail.com>

This patch adds support for getting the usage of mounted
filesystem.
It's very useful when we try to monitor guest's filesystem.
Use df of coreutils for reference.

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
---
 qga/commands-posix.c | 17 +++++++++++++++++
 qga/qapi-schema.json |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..0d93c47a5d 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -46,6 +46,7 @@ extern char **environ;
 #include <arpa/inet.h>
 #include <sys/socket.h>
 #include <net/if.h>
+#include <sys/statvfs.h>
 
 #ifdef FIFREEZE
 #define CONFIG_FSFREEZE
@@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount,
                                                Error **errp)
 {
     GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
+    struct statvfs buf;
+    unsigned long u100, used, nonroot_total;
+    int usage;
     char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
                                     mount->devmajor, mount->devminor);
 
@@ -1079,7 +1083,20 @@ 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)) {
+        error_setg_errno(errp, errno, "Failed to get statvfs");
+        return NULL;
+    }
+
+    used = buf.f_blocks - buf.f_bfree;
+    u100 = 100 * used;
+    nonroot_total = used + buf.f_bavail;
+    usage = u100 / nonroot_total + (u100 % nonroot_total != 0);
+
+    fs->usage = usage;
+
     g_free(devpath);
+
     return fs;
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..22c22d8a62 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -846,13 +846,14 @@
 # @name: disk name
 # @mountpoint: mount point path
 # @type: file system type string
+# @usage: file system usage
 # @disk: an array of disk hardware information that the volume lies on,
 #        which may be empty if the disk type is not supported
 #
 # Since: 2.2
 ##
 { 'struct': 'GuestFilesystemInfo',
-  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
+  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str', 'usage': 'int',
            'disk': ['GuestDiskAddress']} }
 
 ##
-- 
2.17.0

Re: [Qemu-devel] [PATCH] qga: add mountpoint usage to GuestFilesystemInfo

[Eric Blake]

On 05/29/2018 10:01 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@gmail.com>
> 
> This patch adds support for getting the usage of mounted
> filesystem.
> It's very useful when we try to monitor guest's filesystem.
> Use df of coreutils for reference.
> 
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
> ---

> @@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount,
>                                                  Error **errp)
>   {
>       GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
> +    struct statvfs buf;
> +    unsigned long u100, used, nonroot_total;
> +    int usage;
>       char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
>                                       mount->devmajor, mount->devminor);
>   
> @@ -1079,7 +1083,20 @@ 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)) {
> +        error_setg_errno(errp, errno, "Failed to get statvfs");
> +        return NULL;
> +    }
> +
> +    used = buf.f_blocks - buf.f_bfree;
> +    u100 = 100 * used;
> +    nonroot_total = used + buf.f_bavail;
> +    usage = u100 / nonroot_total + (u100 % nonroot_total != 0);

Why integral instead of floating point?

> +++ b/qga/qapi-schema.json
> @@ -846,13 +846,14 @@
>   # @name: disk name
>   # @mountpoint: mount point path
>   # @type: file system type string
> +# @usage: file system usage

Needs more details.  As written, it is an integer between 0 and 100; but 
if you use floating point, a better description would be a fraction 
between 0 and 1.

>   # @disk: an array of disk hardware information that the volume lies on,
>   #        which may be empty if the disk type is not supported
>   #
>   # Since: 2.2
>   ##
>   { 'struct': 'GuestFilesystemInfo',
> -  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
> +  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str', 'usage': 'int',
>              'disk': ['GuestDiskAddress']} }
>   
>   ##
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] qga: add mountpoint usage to GuestFilesystemInfo

[Eric Blake]

On 05/29/2018 10:19 PM, Eric Blake wrote:

>> @@ -846,13 +846,14 @@
>>   # @name: disk name
>>   # @mountpoint: mount point path
>>   # @type: file system type string
>> +# @usage: file system usage
> 
> Needs more details.  As written, it is an integer between 0 and 100; but 
> if you use floating point, a better description would be a fraction 
> between 0 and 1.

Also, missing a '(since 3.0)' designation.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] qga: add mountpoint usage to GuestFilesystemInfo

[Chen Hanxiao]

At 2018-05-30 11:19:27, "Eric Blake" <eblake@redhat.com> wrote:
>On 05/29/2018 10:01 PM, Chen Hanxiao wrote:
>> From: Chen Hanxiao <chenhanxiao@gmail.com>
>> 
>> This patch adds support for getting the usage of mounted
>> filesystem.
>> It's very useful when we try to monitor guest's filesystem.
>> Use df of coreutils for reference.
>> 
>> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
>> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
>> ---
>
>> @@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount,
>>                                                  Error **errp)
>>   {
>>       GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
>> +    struct statvfs buf;
>> +    unsigned long u100, used, nonroot_total;
>> +    int usage;
>>       char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
>>                                       mount->devmajor, mount->devminor);
>>   
>> @@ -1079,7 +1083,20 @@ 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)) {
>> +        error_setg_errno(errp, errno, "Failed to get statvfs");
>> +        return NULL;
>> +    }
>> +
>> +    used = buf.f_blocks - buf.f_bfree;
>> +    u100 = 100 * used;
>> +    nonroot_total = used + buf.f_bavail;
>> +    usage = u100 / nonroot_total + (u100 % nonroot_total != 0);
>
>Why integral instead of floating point?

I followed the style of df from coreutils.
As the percentage already multiplied by 100,
I think it has enough precision.

>
>> +++ b/qga/qapi-schema.json
>> @@ -846,13 +846,14 @@
>>   # @name: disk name
>>   # @mountpoint: mount point path
>>   # @type: file system type string
>> +# @usage: file system usage
>
>Needs more details.  As written, it is an integer between 0 and 100; but 
>if you use floating point, a better description would be a fraction 
>between 0 and 1.

Will be updated in the following patch.

Regards,
- Chen

>
>>   # @disk: an array of disk hardware information that the volume lies on,
>>   #        which may be empty if the disk type is not supported
>>   #
>>   # Since: 2.2
>>   ##
>>   { 'struct': 'GuestFilesystemInfo',
>> -  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
>> +  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str', 'usage': 'int',
>>              'disk': ['GuestDiskAddress']} }
>>   
>>   ##
>> 
>
>-- 
>Eric Blake, Principal Software Engineer
>Red Hat, Inc.           +1-919-301-3266
>Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] qga: add mountpoint usage to GuestFilesystemInfo

[Eric Blake]

On 05/30/2018 01:55 AM, Chen Hanxiao wrote:

>>> +    usage = u100 / nonroot_total + (u100 % nonroot_total != 0);
>>
>> Why integral instead of floating point?
> 
> I followed the style of df from coreutils.
> As the percentage already multiplied by 100,
> I think it has enough precision.

You're making an arbitrary policy decision.  Give the client a floating 
point, and they can still truncate to print out a 2-digit percentage. 
But give them a percentage, and for a 2T disk image, they are forced to 
guess whether they are 20G high or low on usage, which starts to take a 
non-trivial amount of time to copy that much data.  In other words, I 
think an int is wrong for being too imprecise.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org