[libvirt] [PATCH 0/4] Wrong allocation size displayed for NFS volumes
https://bugzilla.redhat.com/show_bug.cgi?id=1077068 The virsh vol-info (and virsh domblkinfo) commands return an incorrect size if the target device is an NFS volume. These patches will add a check for the target being NFS and rather than calculate the size based on "st_blocks" and "DEV_BSIZE" (512) use the "st_size" value instead. John Ferlan (4): virfile: Split virFileGetFSFtype virfile: Introduce virFileIsNFSFSType vol-info: Check for NFS FS type domblkinfo: Check for NFS FS Type src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 7 ++++-- src/storage/storage_backend.c | 7 ++++-- src/util/virfile.c | 53 +++++++++++++++++++++++++++++++++++-------- src/util/virfile.h | 1 + 5 files changed, 55 insertions(+), 14 deletions(-) -- 1.9.3
Split out the portion of virFileGetFSFtype that makes the statfs call in order to return the "f_type" field Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virfile.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 9863fd0..7612007 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2841,14 +2841,16 @@ int virFilePrintf(FILE *fp, const char *msg, ...) # define HUGETLBFS_MAGIC 0x958458f6 # endif -int -virFileIsSharedFSType(const char *path, - int fstypes) +static int +virFileGetFSFtype(const char *path, + long long int *ret_ftype) { char *dirpath, *p; struct statfs sb; int statfs_ret; + *ret_ftype = 0; + if (VIR_STRDUP(dirpath, path) < 0) return -1; @@ -2886,28 +2888,40 @@ virFileIsSharedFSType(const char *path, path); return -1; } + *ret_ftype = sb.f_type; + return 0; +} + +int +virFileIsSharedFSType(const char *path, + int fstypes) +{ + long long int f_type; + + if (virFileGetFSFtype(path, &f_type) < 0) + return -1; VIR_DEBUG("Check if path %s with FS magic %lld is shared", - path, (long long int)sb.f_type); + path, f_type); if ((fstypes & VIR_FILE_SHFS_NFS) && - (sb.f_type == NFS_SUPER_MAGIC)) + (f_type == NFS_SUPER_MAGIC)) return 1; if ((fstypes & VIR_FILE_SHFS_GFS2) && - (sb.f_type == GFS2_MAGIC)) + (f_type == GFS2_MAGIC)) return 1; if ((fstypes & VIR_FILE_SHFS_OCFS) && - (sb.f_type == OCFS2_SUPER_MAGIC)) + (f_type == OCFS2_SUPER_MAGIC)) return 1; if ((fstypes & VIR_FILE_SHFS_AFS) && - (sb.f_type == AFS_FS_MAGIC)) + (f_type == AFS_FS_MAGIC)) return 1; if ((fstypes & VIR_FILE_SHFS_SMB) && - (sb.f_type == SMB_SUPER_MAGIC)) + (f_type == SMB_SUPER_MAGIC)) return 1; if ((fstypes & VIR_FILE_SHFS_CIFS) && - (sb.f_type == CIFS_SUPER_MAGIC)) + (f_type == CIFS_SUPER_MAGIC)) return 1; return 0; -- 1.9.3
Use the virFileGetFSFtype() in order to compare the returned f_type for the NFS super magic number Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 19 +++++++++++++++++++ src/util/virfile.h | 1 + 3 files changed, 21 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 08111d4..121e578 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1291,6 +1291,7 @@ virFileIsDir; virFileIsExecutable; virFileIsLink; virFileIsMountPoint; +virFileIsNFSFSType; virFileIsSharedFS; virFileIsSharedFSType; virFileLinkPointsTo; diff --git a/src/util/virfile.c b/src/util/virfile.c index 7612007..e6c767d 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2927,6 +2927,18 @@ virFileIsSharedFSType(const char *path, return 0; } +bool +virFileIsNFSFSType(const char *path) +{ + long long int f_type; + + if ((virFileGetFSFtype(path, &f_type) == 0) && + (f_type == NFS_SUPER_MAGIC)) + return true; + + return false; +} + int virFileGetHugepageSize(const char *path, unsigned long long *size) @@ -3060,6 +3072,13 @@ int virFileIsSharedFSType(const char *path ATTRIBUTE_UNUSED, } int +virFileIsNFSFSType(const char *path ATTRIBUTE_UNUSED) +{ + /* XXX implement me :-) */ + return false; +} + +int virFileGetHugepageSize(const char *path ATTRIBUTE_UNUSED, unsigned long long *size ATTRIBUTE_UNUSED) { diff --git a/src/util/virfile.h b/src/util/virfile.h index 403d0ba..cc07c53 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -191,6 +191,7 @@ enum { }; int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1); +bool virFileIsNFSFSType(const char *path); int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1); int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1); -- 1.9.3
On 08/05/2014 04:38 PM, John Ferlan wrote:
Use the virFileGetFSFtype() in order to compare the returned f_type for the NFS super magic number
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 19 +++++++++++++++++++ src/util/virfile.h | 1 + 3 files changed, 21 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 08111d4..121e578 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1291,6 +1291,7 @@ virFileIsDir; virFileIsExecutable; virFileIsLink; virFileIsMountPoint; +virFileIsNFSFSType; virFileIsSharedFS; virFileIsSharedFSType; virFileLinkPointsTo; diff --git a/src/util/virfile.c b/src/util/virfile.c index 7612007..e6c767d 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2927,6 +2927,18 @@ virFileIsSharedFSType(const char *path, return 0; }
+bool +virFileIsNFSFSType(const char *path) +{ + long long int f_type; + + if ((virFileGetFSFtype(path, &f_type) == 0) && + (f_type == NFS_SUPER_MAGIC)) + return true; + + return false; +} + int virFileGetHugepageSize(const char *path, unsigned long long *size) @@ -3060,6 +3072,13 @@ int virFileIsSharedFSType(const char *path ATTRIBUTE_UNUSED, }
int +virFileIsNFSFSType(const char *path ATTRIBUTE_UNUSED)
This doesn't match the prototype in virfile.h. Also, I wonder if virFileIsNFSFSType(path) is that much more readable than virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 1.
+{ + /* XXX implement me :-) */ + return false; +} + +int virFileGetHugepageSize(const char *path ATTRIBUTE_UNUSED, unsigned long long *size ATTRIBUTE_UNUSED) { diff --git a/src/util/virfile.h b/src/util/virfile.h index 403d0ba..cc07c53 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -191,6 +191,7 @@ enum { };
int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1); +bool virFileIsNFSFSType(const char *path);
ATTRIBUTE_NONNULL(1);
int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1); int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1);
ACK
On 08/08/2014 07:07 AM, Ján Tomko wrote:
On 08/05/2014 04:38 PM, John Ferlan wrote:
Use the virFileGetFSFtype() in order to compare the returned f_type for the NFS super magic number
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 19 +++++++++++++++++++ src/util/virfile.h | 1 + 3 files changed, 21 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 08111d4..121e578 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1291,6 +1291,7 @@ virFileIsDir; virFileIsExecutable; virFileIsLink; virFileIsMountPoint; +virFileIsNFSFSType; virFileIsSharedFS; virFileIsSharedFSType; virFileLinkPointsTo; diff --git a/src/util/virfile.c b/src/util/virfile.c index 7612007..e6c767d 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2927,6 +2927,18 @@ virFileIsSharedFSType(const char *path, return 0; }
+bool +virFileIsNFSFSType(const char *path) +{ + long long int f_type; + + if ((virFileGetFSFtype(path, &f_type) == 0) && + (f_type == NFS_SUPER_MAGIC)) + return true; + + return false; +} + int virFileGetHugepageSize(const char *path, unsigned long long *size) @@ -3060,6 +3072,13 @@ int virFileIsSharedFSType(const char *path ATTRIBUTE_UNUSED, }
int +virFileIsNFSFSType(const char *path ATTRIBUTE_UNUSED)
This doesn't match the prototype in virfile.h.
Drat - boy I hate making the same mistake twice - changed the function late, but forgot to change the #else...
Also, I wonder if virFileIsNFSFSType(path) is that much more readable than virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 1.
hmm... guess that makes patch 1 & 2 unnecessary. Guess I was more hyperfocused on how to get at NFS_SUPER_MAGIC that I missed the SHFS enum I'll rework... John
+{ + /* XXX implement me :-) */ + return false; +} + +int virFileGetHugepageSize(const char *path ATTRIBUTE_UNUSED, unsigned long long *size ATTRIBUTE_UNUSED) { diff --git a/src/util/virfile.h b/src/util/virfile.h index 403d0ba..cc07c53 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -191,6 +191,7 @@ enum { };
int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1); +bool virFileIsNFSFSType(const char *path);
ATTRIBUTE_NONNULL(1);
int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1); int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1);
ACK
https://bugzilla.redhat.com/show_bug.cgi?id=1077068 Check for the NFS FS type being true for a "local" stat of the file to force usage of the 'st_size' value rather than calculating the size using the 'st_blocks' and 'st_blksize'. As described in the stat(2) man page: "Use of the st_blocks and st_blksize fields may be less portable." experimentation shows a 10M file could get the following output from stat: st_size=10485760 st_blocks=88 st_blksize=1048576 resulting in a "44 KiB" value being displayed as the allocation value. While this value does match the "du -s" value of the same file, the "du -b" value shows the "st_size" field. Similarly a long listing of the file shows the 10M size. Prior to the change: Name: test-vol1 Type: file Capacity: 10.00 MiB Allocation: 44.00 KiB After the change: Name: test-vol1 Type: file Capacity: 10.00 MiB Allocation: 10.00 MiB Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5cada39..bbc992e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1520,8 +1520,11 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target, if (S_ISREG(sb->st_mode)) { #ifndef WIN32 - target->allocation = (unsigned long long)sb->st_blocks * - (unsigned long long)DEV_BSIZE; + if (virFileIsNFSFSType(target->path)) + target->allocation = sb->st_size; + else + target->allocation = (unsigned long long)sb->st_blocks * + (unsigned long long)DEV_BSIZE; #else target->allocation = sb->st_size; #endif -- 1.9.3
On 08/05/2014 04:38 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1077068
Check for the NFS FS type being true for a "local" stat of the file to force usage of the 'st_size' value rather than calculating the size using the 'st_blocks' and 'st_blksize'. As described in the stat(2) man page:
"Use of the st_blocks and st_blksize fields may be less portable."
experimentation shows a 10M file could get the following output from stat:
st_size=10485760 st_blocks=88 st_blksize=1048576
resulting in a "44 KiB" value being displayed as the allocation value. While this value does match the "du -s" value of the same file, the "du -b" value shows the "st_size" field. Similarly a long listing of the file shows the 10M size.
Capacity should be the apparent size (what du -b shows, or st_size), while allocation should track the on-disk usage (du, st_blocks * 512). It looks to me that the values are correct, it's just that posix_fallocate does neither work nor fail on NFS. Jan
On 08/08/2014 07:07 AM, Ján Tomko wrote:
On 08/05/2014 04:38 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1077068
Check for the NFS FS type being true for a "local" stat of the file to force usage of the 'st_size' value rather than calculating the size using the 'st_blocks' and 'st_blksize'. As described in the stat(2) man page:
"Use of the st_blocks and st_blksize fields may be less portable."
experimentation shows a 10M file could get the following output from stat:
st_size=10485760 st_blocks=88 st_blksize=1048576
resulting in a "44 KiB" value being displayed as the allocation value. While this value does match the "du -s" value of the same file, the "du -b" value shows the "st_size" field. Similarly a long listing of the file shows the 10M size.
Capacity should be the apparent size (what du -b shows, or st_size), while allocation should track the on-disk usage (du, st_blocks * 512).
It looks to me that the values are correct, it's just that posix_fallocate does neither work nor fail on NFS.
Ah yes - this is exactly where I went back and forth on... Digging through 'old' google results on posix_fallocate() and wondering whether it was incorrectly returning success or whether stat() was as pointed out in its man page not getting a reliable st_blocks value. However, if posix_fallocate() isn't working as specified for NFS and not producing any error message, then how does one really determine that? I also had some code that reworked the two callers/users in order to force the "allocation" paths to go through the slower lseek/safewrite calls. Is it worth resurrecting that and going with it? John
On 08/08/2014 07:07 AM, Ján Tomko wrote:
On 08/05/2014 04:38 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1077068
Check for the NFS FS type being true for a "local" stat of the file to force usage of the 'st_size' value rather than calculating the size using the 'st_blocks' and 'st_blksize'. As described in the stat(2) man page:
"Use of the st_blocks and st_blksize fields may be less portable."
experimentation shows a 10M file could get the following output from stat:
st_size=10485760 st_blocks=88 st_blksize=1048576
resulting in a "44 KiB" value being displayed as the allocation value. While this value does match the "du -s" value of the same file, the "du -b" value shows the "st_size" field. Similarly a long listing of the file shows the 10M size.
Capacity should be the apparent size (what du -b shows, or st_size), while allocation should track the on-disk usage (du, st_blocks * 512).
It looks to me that the values are correct, it's just that posix_fallocate does neither work nor fail on NFS.
Jan
Testing seems to indicate that posix_fallocate() either doesn't work as expected on the target or using the target.path is incorrect... Before posix_fallocate stat st_blocks=0 st_blksize=1048576 st_size=10485760 lseek end=10485760 posix_fallocate of 10485760 bytes on /home/nfs_pool/target/test-vol1 After posix_fallocate stat st_blocks=88 st_blksize=1048576 st_size=10485760 lseek end=10485760 Hmm... would going at the target be correct in this instance? Same test but use the source path: Before posix_fallocate stat st_blocks=0 st_blksize=4096 st_size=10485760 lseek end=10485760 posix_fallocate of 10485760 bytes on /home/nfs_pool/nfs-export/test-vol1 After posix_fallocate stat st_blocks=20480 st_blksize=4096 st_size=10485760 lseek end=10485760 ... hmm.... 20480 * 512 = 10485760 # df ... localhost:/home/nfs_pool/nfs-export 140979200 35521536 98273280 27% /home/nfs_pool/target # John
On 08/08/2014 11:26 AM, John Ferlan wrote:
On 08/08/2014 07:07 AM, Ján Tomko wrote:
On 08/05/2014 04:38 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1077068
Check for the NFS FS type being true for a "local" stat of the file to force usage of the 'st_size' value rather than calculating the size using the 'st_blocks' and 'st_blksize'. As described in the stat(2) man page:
"Use of the st_blocks and st_blksize fields may be less portable."
experimentation shows a 10M file could get the following output from stat:
st_size=10485760 st_blocks=88 st_blksize=1048576
resulting in a "44 KiB" value being displayed as the allocation value. While this value does match the "du -s" value of the same file, the "du -b" value shows the "st_size" field. Similarly a long listing of the file shows the 10M size.
Capacity should be the apparent size (what du -b shows, or st_size), while allocation should track the on-disk usage (du, st_blocks * 512).
It looks to me that the values are correct, it's just that posix_fallocate does neither work nor fail on NFS.
Jan
Testing seems to indicate that posix_fallocate() either doesn't work as expected on the target or using the target.path is incorrect...
Before posix_fallocate stat st_blocks=0 st_blksize=1048576 st_size=10485760 lseek end=10485760
posix_fallocate of 10485760 bytes on /home/nfs_pool/target/test-vol1
After posix_fallocate stat st_blocks=88 st_blksize=1048576 st_size=10485760 lseek end=10485760
Hmm... would going at the target be correct in this instance? Same test but use the source path:
Before posix_fallocate stat st_blocks=0 st_blksize=4096 st_size=10485760 lseek end=10485760
posix_fallocate of 10485760 bytes on /home/nfs_pool/nfs-export/test-vol1
After posix_fallocate stat st_blocks=20480 st_blksize=4096 st_size=10485760 lseek end=10485760
...
hmm.... 20480 * 512 = 10485760
# df ... localhost:/home/nfs_pool/nfs-export 140979200 35521536 98273280 27% /home/nfs_pool/target #
Well it's a tangled web that's being weaved... The blksize of the target volume comes from the 'wsize' value in the mount: localhost:/home/nfs_pool/nfs-export on /home/nfs_pool/target type nfs4 (rw,relatime,vers=4.0,rsize=1048576,wsize=1048576,...) Further testing shows if I change the wsize to 4096, then I get what I expect; however, starting at 8192 I'll get decreasingly smaller allocations. So this is a "math problem". So going back to writing via posix_fallocate() to the "/home/nfs_pool/target/test-vol" the "issue" is the blksize of the "source" (nfs-export) is 4096 while the blksize of the "target" (target) is 1048576 (as a result of the nfs mount settings). What "seems" to happen is the posix_fallocate() makes 11 "writes" - I assume because blksize*10 < desired_size (10485760) (instead of <=...). Thus 11 * 4096 = 45056 (bytes) / 1024 = 44 KiB which was displayed. Why all this happens I'm not sure. Bug in posix_fallocate()? Bug in configuration? I have to assume that when this code was first added NFS probably was still using smaller block sizes. Whether anyone has noticed or not beyond the virt-test which discovered the issue - I'm not sure. In any case, does anyone have feedback/thoughts for next steps? I can put together something that avoids posix_fallocate() for the create-as and resize paths. John
On 08/09/2014 12:28 AM, John Ferlan wrote:
Testing seems to indicate that posix_fallocate() either doesn't work as expected on the target or using the target.path is incorrect...
Before posix_fallocate stat st_blocks=0 st_blksize=1048576 st_size=10485760 lseek end=10485760
posix_fallocate of 10485760 bytes on /home/nfs_pool/target/test-vol1
After posix_fallocate stat st_blocks=88 st_blksize=1048576 st_size=10485760 lseek end=10485760
Hmm... would going at the target be correct in this instance? Same test but use the source path:
Using the source path would only work if the NFS export is on the localhost :)
Before posix_fallocate stat st_blocks=0 st_blksize=4096 st_size=10485760 lseek end=10485760
posix_fallocate of 10485760 bytes on /home/nfs_pool/nfs-export/test-vol1
After posix_fallocate stat st_blocks=20480 st_blksize=4096 st_size=10485760 lseek end=10485760
...
hmm.... 20480 * 512 = 10485760
# df ... localhost:/home/nfs_pool/nfs-export 140979200 35521536 98273280 27% /home/nfs_pool/target #
Well it's a tangled web that's being weaved... The blksize of the target volume comes from the 'wsize' value in the mount:
localhost:/home/nfs_pool/nfs-export on /home/nfs_pool/target type nfs4 (rw,relatime,vers=4.0,rsize=1048576,wsize=1048576,...)
Further testing shows if I change the wsize to 4096, then I get what I expect; however, starting at 8192 I'll get decreasingly smaller allocations. So this is a "math problem".
So going back to writing via posix_fallocate() to the "/home/nfs_pool/target/test-vol" the "issue" is the blksize of the "source" (nfs-export) is 4096 while the blksize of the "target" (target) is 1048576 (as a result of the nfs mount settings). What "seems" to happen is the posix_fallocate() makes 11 "writes" - I assume because blksize*10 < desired_size (10485760) (instead of <=...).
Thus 11 * 4096 = 45056 (bytes) / 1024 = 44 KiB which was displayed.
Why all this happens I'm not sure. Bug in posix_fallocate()? Bug in configuration? I have to assume that when this code was first added NFS probably was still using smaller block sizes.
The code was introduced in 2013. Maybe it wasn't tested on NFS at all? It looks like a posix_fallocate bug to me. The other method we have (syscall(SYS_fallocate)) gives me EOPNOTSUPP - Operation not supported on transport endpoint.
Whether anyone has noticed or not beyond the virt-test which discovered the issue - I'm not sure. In any case, does anyone have feedback/thoughts for next steps? I can put together something that avoids posix_fallocate() for the create-as and resize paths.
I think reporting an error when preallocate is requested for a NFS pool makes sense, but that might be pretty annoying if something supplies the preallocate flag by default. Jan
<...snip...>
Why all this happens I'm not sure. Bug in posix_fallocate()? Bug in configuration? I have to assume that when this code was first added NFS probably was still using smaller block sizes.
The code was introduced in 2013. Maybe it wasn't tested on NFS at all? It looks like a posix_fallocate bug to me.
Technically it was before that - the safezero function and it's supporting cast were moved into virfile.c in 2013; however, they were in [vir]util.c before that. It was first introduced in 2009 in commit id 'c29d0929' and beyond some minor changes/reword has stayed relatively unchanged. The one downside I see with it is that it's a build related either/or setup. That is the "HAVE_POSIX_FALLOCATE" dictates the usage and provides no fallback in the event that perhaps the result isn't expected or if for some reason posix_fallocate fails. "Theoretically" the safewrite option should/could be the fallback in the event it's determined that the posix_fallocate() didn't generate the right allocation size.
The other method we have (syscall(SYS_fallocate)) gives me EOPNOTSUPP - Operation not supported on transport endpoint.
Right - I saw that too.... The resize code followed the safezero() code at least with respect to the either/or with the HAVE_POSIX_FALLOCATE build conditional. It was introduced in commit id 'aa2a4cff'. It assumes that the "SYS_fallocate" syscall is present "and" will work. It is missing an option to safewrite() from current allocation to the new end of the file (eg, a call to safezero passing the the current offset).
Whether anyone has noticed or not beyond the virt-test which discovered the issue - I'm not sure. In any case, does anyone have feedback/thoughts for next steps? I can put together something that avoids posix_fallocate() for the create-as and resize paths.
I think reporting an error when preallocate is requested for a NFS pool makes sense, but that might be pretty annoying if something supplies the preallocate flag by default.
My comment was more geared towards my findings of the posix_fallocate() introduction in 2009. While I agree there's something flaky with posix_fallocate, I don't think an error is necessarily the best path. Maybe better "fall back code" - we could check the posix_fallocate() results (using stat) perhaps VIR_WARN that something is wrong before falling back to the mmap/safewrite options. John
Similar to the vol-info code, check if the target device for the blkinfo fetch is an NFS volume and return the st_size value rather than calculating the size based on st_blksize and DEV_BSIZE. Prior to the change a 40M target device would display: Capacity: 41943040 Allocation: 126976 Physical: 126976 With the change it will display: Capacity: 41943040 Allocation: 41943040 Physical: 41943040 Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8eba8e8..f5d31f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10432,8 +10432,11 @@ qemuDomainGetBlockInfo(virDomainPtr dom, /* Get info for normal formats */ if (S_ISREG(sb.st_mode) || fd == -1) { #ifndef WIN32 - info->physical = (unsigned long long)sb.st_blocks * - (unsigned long long)DEV_BSIZE; + if (virFileIsNFSFSType(disk->src->path)) + info->physical = sb.st_size; + else + info->physical = (unsigned long long)sb.st_blocks * + (unsigned long long)DEV_BSIZE; #else info->physical = sb.st_size; #endif -- 1.9.3
participants (2)
-
John Ferlan -
Ján Tomko