diff mbox

[12/12] Add disk_size field to BlockDriverState structure

Message ID BANLkTi=GKQo724CK2ULgjych2OP3hr6cyQ@mail.gmail.com
State New
Headers show

Commit Message

Feiran Zheng June 4, 2011, 12:44 a.m. UTC
The `qemu-img info` results for mono flat image are no longer
accurate, as the "disk size" was the length of bs->file, which is not
the case for multi file images (such as vmdk images with multiple
files).
The new field disk_size in BlockDriverState can be used by block
driver to tell the exact disk size of block image (only valid if the
field is set to non-zero).

Signed-off-by: Fam Zheng <famcool@gmail.com>
---
 block/vmdk.c |    1 +
 block_int.h  |    1 +
 qemu-img.c   |    6 +++++-
 3 files changed, 7 insertions(+), 1 deletions(-)

Comments

Stefan Hajnoczi June 18, 2011, 6:35 p.m. UTC | #1
On Sat, Jun 4, 2011 at 1:44 AM, Fam Zheng <famcool@gmail.com> wrote:
> The `qemu-img info` results for mono flat image are no longer
> accurate, as the "disk size" was the length of bs->file, which is not
> the case for multi file images (such as vmdk images with multiple
> files).
> The new field disk_size in BlockDriverState can be used by block
> driver to tell the exact disk size of block image (only valid if the
> field is set to non-zero).
>
> Signed-off-by: Fam Zheng <famcool@gmail.com>
> ---
>  block/vmdk.c |    1 +
>  block_int.h  |    1 +
>  qemu-img.c   |    6 +++++-
>  3 files changed, 7 insertions(+), 1 deletions(-)

Instead of introducing a variable please add a new BlockDriver
.bdrv_get_allocated_file_size() function pointer and move
get_allocated_file_size() from qemu-img.c into block/raw-posix.c and
block/raw-win32.c.

qemu-img.c should call bdrv_get_allocated_file_size() and
bdrv_get_allocated_file_size() could look like this:

int bdrv_get_allocated_file_size(BlockDriverState *bs, int64_t *size)
{
    BlockDriver *drv = bs->drv;

    if (!drv) {
        return -ENOMEDIUM;
    }

    if (drv->bdrv_get_allocated_file_size) {
        return drv->bdrv_get_allocated_file_size(bs, size);
    }

    if (bs->file) {
        return bdrv_get_allocated_file_size(bs->file, size);
    }
    return -ENOTSUP;
}

> @@ -607,6 +607,7 @@ static int vmdk_open_desc_file(BlockDriverState
> *bs, int flags)
>     extent = s->extents;
>     vmdk_parse_extents(buf, s->extents, bs->file->filename);
>     bs->total_sectors = extent->sectors;
> +    bs->disk_size = bdrv_getlength(bs->file) + bdrv_getlength(extent->file);

This should call bdrv_get_allocated_file_size() so add together the
actually allocated amount, not what bdrv_getlength() returns.

Stefan
Feiran Zheng June 20, 2011, 5:37 a.m. UTC | #2
Is there any difference between bdrv_getlength and
bdrv_get_allocated_file_size for bs-file? If not, I can simplify it by
reusing it in two raw devices.

On Sun, Jun 19, 2011 at 2:35 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Sat, Jun 4, 2011 at 1:44 AM, Fam Zheng <famcool@gmail.com> wrote:
>> The `qemu-img info` results for mono flat image are no longer
>> accurate, as the "disk size" was the length of bs->file, which is not
>> the case for multi file images (such as vmdk images with multiple
>> files).
>> The new field disk_size in BlockDriverState can be used by block
>> driver to tell the exact disk size of block image (only valid if the
>> field is set to non-zero).
>>
>> Signed-off-by: Fam Zheng <famcool@gmail.com>
>> ---
>>  block/vmdk.c |    1 +
>>  block_int.h  |    1 +
>>  qemu-img.c   |    6 +++++-
>>  3 files changed, 7 insertions(+), 1 deletions(-)
>
> Instead of introducing a variable please add a new BlockDriver
> .bdrv_get_allocated_file_size() function pointer and move
> get_allocated_file_size() from qemu-img.c into block/raw-posix.c and
> block/raw-win32.c.
>
> qemu-img.c should call bdrv_get_allocated_file_size() and
> bdrv_get_allocated_file_size() could look like this:
>
> int bdrv_get_allocated_file_size(BlockDriverState *bs, int64_t *size)
> {
>    BlockDriver *drv = bs->drv;
>
>    if (!drv) {
>        return -ENOMEDIUM;
>    }
>
>    if (drv->bdrv_get_allocated_file_size) {
>        return drv->bdrv_get_allocated_file_size(bs, size);
>    }
>
>    if (bs->file) {
>        return bdrv_get_allocated_file_size(bs->file, size);
>    }
>    return -ENOTSUP;
> }
>
>> @@ -607,6 +607,7 @@ static int vmdk_open_desc_file(BlockDriverState
>> *bs, int flags)
>>     extent = s->extents;
>>     vmdk_parse_extents(buf, s->extents, bs->file->filename);
>>     bs->total_sectors = extent->sectors;
>> +    bs->disk_size = bdrv_getlength(bs->file) + bdrv_getlength(extent->file);
>
> This should call bdrv_get_allocated_file_size() so add together the
> actually allocated amount, not what bdrv_getlength() returns.
>
> Stefan
>
Stefan Hajnoczi June 20, 2011, 6:08 a.m. UTC | #3
On Mon, Jun 20, 2011 at 6:37 AM, Fam Zheng <famcool@gmail.com> wrote:
> Is there any difference between bdrv_getlength and
> bdrv_get_allocated_file_size for bs-file? If not, I can simplify it by
> reusing it in two raw devices.

Yes, the two functions are different:

POSIX sparse files (files with holes) take up less space on disk than
their file size.  For example a 1 GB file where you've only written
the last byte and never touched any other blocks will only take up one
block - the rest will be unallocated.  So bdrv_getlength() == 1 GB and
bdrv_get_allocated_file_size() == 4 KB (or whatever the file system
block size is).

You can look at this using the stat(1) command and dd(1) to only write
the last byte of a file.

Stefan
diff mbox

Patch

diff --git a/block/vmdk.c b/block/vmdk.c
index 43b47e2..39c553c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -607,6 +607,7 @@  static int vmdk_open_desc_file(BlockDriverState
*bs, int flags)
     extent = s->extents;
     vmdk_parse_extents(buf, s->extents, bs->file->filename);
     bs->total_sectors = extent->sectors;
+    bs->disk_size = bdrv_getlength(bs->file) + bdrv_getlength(extent->file);

     // try to open parent images, if exist
     if (vmdk_parent_open(bs) != 0) {
diff --git a/block_int.h b/block_int.h
index dd8f8cb..4c58d6d 100644
--- a/block_int.h
+++ b/block_int.h
@@ -144,6 +144,7 @@  struct BlockDriver {
 struct BlockDriverState {
     int64_t total_sectors; /* if we are reading a disk image, give its
                               size in sectors */
+    int64_t disk_size; /* if non-zero, holds the disk size of image */
     int read_only; /* if true, the media is read only */
     int keep_read_only; /* if true, the media was requested to stay
read only */
     int open_flags; /* flags used to open the file, re-used for re-open */
diff --git a/qemu-img.c b/qemu-img.c
index 4f162d1..7ea8faf 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1067,7 +1067,11 @@  static int img_info(int argc, char **argv)
     bdrv_get_format(bs, fmt_name, sizeof(fmt_name));
     bdrv_get_geometry(bs, &total_sectors);
     get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
-    allocated_size = get_allocated_file_size(filename);
+    if (bs->disk_size) {
+        allocated_size = bs->disk_size;
+    } else {
+        allocated_size = get_allocated_file_size(filename);
+    }
     if (allocated_size < 0) {
         snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
     } else {