Re: [libvirt] [PATCH] Fix sparse volume allocation reporting

[Cole Robinson <crobinso redhat com>]

Daniel P. Berrange wrote:
> On Thu, Mar 26, 2009 at 05:55:00PM +0100, Daniel Veillard wrote:
>> On Thu, Mar 26, 2009 at 01:19:12PM +0000, Daniel P. Berrange wrote:
>>> On Tue, Mar 24, 2009 at 03:29:21PM -0400, Cole Robinson wrote:
>> [...]
>>> Having read the man page again, I'm inclined to say using st_blksize
>>> is always wrong no matter what, because it is quite clear that 'st_blocks'
>>> is always in 512 byte units. So perhaps we might be better of doing
>>>
>>>    #ifndef DEV_BSIZE
>>>    #define DEV_BSIZE 512
>>>    #endif
>>>
>>> And then always using DEV_BSIZE. 
>>   In those kind of cases I go down to the spec and it states (in the
>> informative section though):
>>
>> -----------------------------------------
>> http://www.opengroup.org/onlinepubs/000095399/basedefs/sys/stat.h.html
>> "The unit for the st_blocks member of the stat structure is not defined
>> within IEEE Std 1003.1-2001. In some implementations it is 512 bytes. It
>> may differ on a file system basis. There is no correlation between
>> values of the st_blocks and st_blksize, and the f_bsize (from
>> <sys/statvfs.h>) structure members.
>>
>> Traditionally, some implementations defined the multiplier for st_blocks
>> in <sys/param.h> as the symbol DEV_BSIZE."
>> -----------------------------------------
>>
>>   So I agree with Dan, we need to drop st_blksize in any volume size
>> computation, and fallback to 512 if not defined, apparently only
>> src/storage_backend.c referenced it :-)
>>
>>   I still find the "It may differ on a file system basis" to be a bit
>> frightening considering the sandard doesn't seems to indicate how to
>> extract that information from the filesystem :-( , oh well ...
> 
> I guess if we come across a filesystem where it is not 512, then someone
> will have created an ioctl() or equivalent to let us find out the true
> value. Failing that, relying on 512 seems like a good plan.
> 
> Daniel

Okay, updated patch attached.

Thanks,
Cole

commit 3f289f7c1ea04cb5d14bf125959ba41b13b64443
Author: Cole Robinson <crobinso redhat com>
Date:   Wed Apr 1 16:00:58 2009 -0400

    Fix sparse volume allocation reporting.

diff --git a/src/storage_backend.c b/src/storage_backend.c
index 71b8020..79c070c 100644
--- a/src/storage_backend.c
+++ b/src/storage_backend.c
@@ -36,6 +36,7 @@
 #include <fcntl.h>
 #include <stdint.h>
 #include <sys/stat.h>
+#include <sys/param.h>
 #include <dirent.h>
 
 #if HAVE_SELINUX
@@ -64,6 +65,9 @@
 #include "storage_backend_fs.h"
 #endif
 
+#ifndef DEV_BSIZE
+#define DEV_BSIZE 512
+#endif
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
@@ -211,7 +215,7 @@ virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn,
         if (S_ISREG(sb.st_mode)) {
 #ifndef __MINGW32__
             *allocation = (unsigned long long)sb.st_blocks *
-                (unsigned long long)sb.st_blksize;
+                          (unsigned long long)DEV_BSIZE;
 #else
             *allocation = sb.st_size;
 #endif