Return correct disk free/available size when FS is mounted with size = 0

Return correct disk free/available size when FS is mounted with size = 0

When tmpfs is mounted with size = 0 (i.e. without any limit), then statvfs will return 0 for f_bfree, f_bavail, and f_blocks. Catch this case and return max_int64 instead.

Also switch to using statfs() instead of statvfs() as f_type is not exposed via latter.

BUG=628710
Committed: https://crrev.com/9bbda42930813a781a034c378e1bcfbe90d503f8
Cr-Commit-Position: refs/heads/master@{#409358}

[Sriram, 2016-08-01 20:43:07]

sriramsr@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2016-08-01 21:30:55]

https://codereview.chromium.org/2198283002/diff/1/base/sys_info_posix.cc
File base/sys_info_posix.cc (right):

https://codereview.chromium.org/2198283002/diff/1/base/sys_info_posix.cc#newc...
base/sys_info_posix.cc:98: stats.f_blocks == 0 &&
IsStatsZeroIfUnlimited(stats.f_fsid);
Are you sure f_fsid is the right field to check? Locally I ran this program:

#include <stdio.h>
#include <sys/statvfs.h>

int main() {
  struct statvfs stats;
  statvfs("/run/shm/foo", &stats);
  printf("%lu\n", stats.f_fsid);
  return 0;
}

and it returned 0, for a file foo that exists on /run/shm, which is tmpfs.

[Sriram, 2016-08-02 00:00:03]

The CQ bit was checked by sriramsr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 00:00:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sriram, 2016-08-02 00:00:54]

Description was changed from

==========
Return correct disk free/available size when FS is mounted with size = 0

When tmpfs is mounted with size = 0 (i.e. without any limit), then statvfs
will return 0 for f_bfree, f_bavail, and f_blocks. Catch this case and return
max_int64 instead.

BUG=628710
==========

to

==========
Return correct disk free/available size when FS is mounted with size = 0

When tmpfs is mounted with size = 0 (i.e. without any limit), then statvfs will
return 0 for f_bfree, f_bavail, and f_blocks. Catch this case and return
max_int64 instead.

Also switch to using statfs() instead of statvfs() as f_type is not exposed via
latter.

BUG=628710
==========

[Sriram, 2016-08-02 00:03:24]

https://codereview.chromium.org/2198283002/diff/1/base/sys_info_posix.cc
File base/sys_info_posix.cc (right):

https://codereview.chromium.org/2198283002/diff/1/base/sys_info_posix.cc#newc...
base/sys_info_posix.cc:98: stats.f_blocks == 0 &&
IsStatsZeroIfUnlimited(stats.f_fsid);
On 2016/08/01 21:30:55, Lei Zhang wrote:
> Are you sure f_fsid is the right field to check? Locally I ran this program:
> 
> #include <stdio.h>
> #include <sys/statvfs.h>
> 
> int main() {
>   struct statvfs stats;
>   statvfs("/run/shm/foo", &stats);
>   printf("%lu\n", stats.f_fsid);
>   return 0;
> }
> 
> and it returned 0, for a file foo that exists on /run/shm, which is tmpfs.

You are right - I misunderstood the man page and f_fsid is not the same as
f_type and f_type is not exposed via statvfs() call. Switched to statfs() as all
values needed here is available.

[commit-bot: I haz the power, 2016-08-02 00:09:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 00:09:02]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Lei Zhang, 2016-08-02 00:11:48]

You've got red bots.

https://codereview.chromium.org/2198283002/diff/20001/base/sys_info_posix.cc
File base/sys_info_posix.cc (left):

https://codereview.chromium.org/2198283002/diff/20001/base/sys_info_posix.cc#...
base/sys_info_posix.cc:24: #if defined(OS_ANDROID)
Ok, hope this works.

https://codereview.chromium.org/2198283002/diff/20001/base/sys_info_posix.cc
File base/sys_info_posix.cc (right):

https://codereview.chromium.org/2198283002/diff/20001/base/sys_info_posix.cc#...
base/sys_info_posix.cc:25: #include <linux/magic.h>
Now that you have removed the #if, move these up to the line 7 block, and keep
them in alphabetical order.

[Sriram, 2016-08-02 00:17:03]

The CQ bit was checked by sriramsr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 00:17:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 00:27:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 00:27:46]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Lei Zhang, 2016-08-02 19:15:19]

Now I suspect the reason statvfs was used is because of Mac. I think it's
cleaner to keep statvfs(), and do an extra statfs() call in the Linux-only code.
I don't think we ask for the free disk space that often, so the slight
inefficiency is ok.

[Sriram, 2016-08-02 19:32:43]

The CQ bit was checked by sriramsr@chromium.org to run a CQ dry run

[Sriram, 2016-08-02 19:32:45]

On 2016/08/02 19:15:19, Lei Zhang wrote:
> Now I suspect the reason statvfs was used is because of Mac. I think it's
> cleaner to keep statvfs(), and do an extra statfs() call in the Linux-only
code.
> I don't think we ask for the free disk space that often, so the slight
> inefficiency is ok.

Agreed and done.

[commit-bot: I haz the power, 2016-08-02 19:33:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-08-02 19:35:41]

lgtm

https://codereview.chromium.org/2198283002/diff/50001/base/sys_info_posix.cc
File base/sys_info_posix.cc (right):

https://codereview.chromium.org/2198283002/diff/50001/base/sys_info_posix.cc#...
base/sys_info_posix.cc:33: #include <linux/magic.h>
alphabetical order

[commit-bot: I haz the power, 2016-08-02 19:55:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 19:55:07]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[Sriram, 2016-08-02 20:51:04]

The CQ bit was checked by sriramsr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 20:51:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sriram, 2016-08-02 20:51:56]

https://codereview.chromium.org/2198283002/diff/20001/base/sys_info_posix.cc
File base/sys_info_posix.cc (left):

https://codereview.chromium.org/2198283002/diff/20001/base/sys_info_posix.cc#...
base/sys_info_posix.cc:24: #if defined(OS_ANDROID)
On 2016/08/02 00:11:48, Lei Zhang wrote:
> Ok, hope this works.

Acknowledged.

[Lei Zhang, 2016-08-02 20:53:26]

++lgtm

[commit-bot: I haz the power, 2016-08-02 21:45:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 21:45:52]

Dry run: This issue passed the CQ dry run.

[Sriram, 2016-08-02 22:37:53]

The CQ bit was checked by sriramsr@chromium.org

[commit-bot: I haz the power, 2016-08-02 22:38:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 22:42:47]

Description was changed from

==========
Return correct disk free/available size when FS is mounted with size = 0

When tmpfs is mounted with size = 0 (i.e. without any limit), then statvfs will
return 0 for f_bfree, f_bavail, and f_blocks. Catch this case and return
max_int64 instead.

Also switch to using statfs() instead of statvfs() as f_type is not exposed via
latter.

BUG=628710
==========

to

==========
Return correct disk free/available size when FS is mounted with size = 0

When tmpfs is mounted with size = 0 (i.e. without any limit), then statvfs will
return 0 for f_bfree, f_bavail, and f_blocks. Catch this case and return
max_int64 instead.

Also switch to using statfs() instead of statvfs() as f_type is not exposed via
latter.

BUG=628710
==========

[commit-bot: I haz the power, 2016-08-02 22:42:50]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2016-08-02 22:46:01]

Description was changed from

==========
Return correct disk free/available size when FS is mounted with size = 0

When tmpfs is mounted with size = 0 (i.e. without any limit), then statvfs will
return 0 for f_bfree, f_bavail, and f_blocks. Catch this case and return
max_int64 instead.

Also switch to using statfs() instead of statvfs() as f_type is not exposed via
latter.

BUG=628710
==========

to

==========
Return correct disk free/available size when FS is mounted with size = 0

When tmpfs is mounted with size = 0 (i.e. without any limit), then statvfs will
return 0 for f_bfree, f_bavail, and f_blocks. Catch this case and return
max_int64 instead.

Also switch to using statfs() instead of statvfs() as f_type is not exposed via
latter.

BUG=628710

Committed: https://crrev.com/9bbda42930813a781a034c378e1bcfbe90d503f8
Cr-Commit-Position: refs/heads/master@{#409358}
==========

[commit-bot: I haz the power, 2016-08-02 22:46:02]

Patchset 5 (id:??) landed as
https://crrev.com/9bbda42930813a781a034c378e1bcfbe90d503f8
Cr-Commit-Position: refs/heads/master@{#409358}