| Message ID | 20180601165750.25420-3-chen_han_xiao@126.com |
|---|---|
| State | New |
| Headers | show |
| Series | qga: report the usage of fs in guests | expand |
On 06/01/2018 11:57 AM, Chen Hanxiao wrote: > From: Chen Hanxiao <chenhanxiao@gmail.com> > > This patch adds support for getting the usage of > windows driver path. > The usage of fs stored as used_bytes and total_bytes. > > Cc: Michael Roth <mdroth@linux.vnet.ibm.com> > Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> > --- > v2: > unpack usage as used-bytes and total-bytes > > @@ -699,10 +700,21 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp) > fs_name[sizeof(fs_name) - 1] = 0; > fs = g_malloc(sizeof(*fs)); > fs->name = g_strdup(guid); > + fs->has_total_bytes = false; > + fs->has_used_bytes = false; Dead assignments. > if (len == 0) { > fs->mountpoint = g_strdup("System Reserved"); > } else { > fs->mountpoint = g_strndup(mnt_point, len); > + if (GetDiskFreeSpaceEx(fs->mountpoint, > + (PULARGE_INTEGER) & i64FreeBytesToCaller, > + (PULARGE_INTEGER) & i64TotalBytes, > + (PULARGE_INTEGER) & i64FreeBytes)) { > + fs->used_bytes = i64TotalBytes - i64FreeBytes; > + fs->total_bytes = i64TotalBytes; > + fs->has_total_bytes = true; > + fs->has_used_bytes = true; > + } I'm less familiar with Windows API, so I won't give this R-b. But now that I read this patch, I do have a question: since both statvfs() and GetDiskFreeSpaceEx() give you free bytes rather than used bytes, and you had to perform a subtraction to report 'used-bytes' over QGA, would it make any more sense to instead report 'free-bytes' over QGA for less subtraction work on your end and so that the QGA interface is more similar to existing interfaces?
At 2018-06-02 01:15:14, "Eric Blake" <eblake@redhat.com> wrote: >On 06/01/2018 11:57 AM, Chen Hanxiao wrote: >> From: Chen Hanxiao <chenhanxiao@gmail.com> ... >> fs->mountpoint = g_strdup("System Reserved"); >> } else { >> fs->mountpoint = g_strndup(mnt_point, len); >> + if (GetDiskFreeSpaceEx(fs->mountpoint, >> + (PULARGE_INTEGER) & i64FreeBytesToCaller, >> + (PULARGE_INTEGER) & i64TotalBytes, >> + (PULARGE_INTEGER) & i64FreeBytes)) { >> + fs->used_bytes = i64TotalBytes - i64FreeBytes; >> + fs->total_bytes = i64TotalBytes; >> + fs->has_total_bytes = true; >> + fs->has_used_bytes = true; >> + } > >I'm less familiar with Windows API, so I won't give this R-b. But now >that I read this patch, I do have a question: since both statvfs() and >GetDiskFreeSpaceEx() give you free bytes rather than used bytes, and you >had to perform a subtraction to report 'used-bytes' over QGA, would it >make any more sense to instead report 'free-bytes' over QGA for less >subtraction work on your end and so that the QGA interface is more >similar to existing interfaces? > All build-in tools report used-bytes for linux and windows. For windows, GetDiskFreeSpaceEx would tell total and free bytes, used bytes is total - free, which is exactly same from the view of disk properties. But for linux, df(1) do some calculation for showing used-bytes and free-bytes. df(1) takes f_blocks - f_bfree for 'Used' , which take 'Used' + 'Available'(f_bavail) for non-root total, which is different from 'Size'(f_blocks); Then calculate 'Use%' by Used / non-root-total. If we want get what df(3) outputs, we must report more members of struct statvfs from STATVFS(3) , which windows does not need it. And client had to do some calculation with if-else dealing with data from windows/linux. Customs usually take 'Use%' to monitor their disks, so I think we should report nums that can easily get the right percentage of usage from it, for both linux and windows. Regards, - Chen
diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 2d48394748..eff26c177e 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -670,6 +670,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp) char fs_name[32]; char vol_info[MAX_PATH+1]; size_t len; + uint64_t i64FreeBytesToCaller, i64TotalBytes, i64FreeBytes; GuestFilesystemInfo *fs = NULL; GetVolumePathNamesForVolumeName(guid, (LPCH)&mnt, 0, &info_size); @@ -699,10 +700,21 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp) fs_name[sizeof(fs_name) - 1] = 0; fs = g_malloc(sizeof(*fs)); fs->name = g_strdup(guid); + fs->has_total_bytes = false; + fs->has_used_bytes = false; if (len == 0) { fs->mountpoint = g_strdup("System Reserved"); } else { fs->mountpoint = g_strndup(mnt_point, len); + if (GetDiskFreeSpaceEx(fs->mountpoint, + (PULARGE_INTEGER) & i64FreeBytesToCaller, + (PULARGE_INTEGER) & i64TotalBytes, + (PULARGE_INTEGER) & i64FreeBytes)) { + fs->used_bytes = i64TotalBytes - i64FreeBytes; + fs->total_bytes = i64TotalBytes; + fs->has_total_bytes = true; + fs->has_used_bytes = true; + } } fs->type = g_strdup(fs_name); fs->disk = build_guest_disk_info(guid, errp);