[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] qemu-img: return allocated size for block devic
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH] qemu-img: return allocated size for block device with qcow2 format |
Date: |
Wed, 2 May 2018 16:37:20 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
Hi,
[replying to this version because the previous mail doesn't seem to have
made it to the mailing lists for whatever reason]
On 2018-05-02 15:34, Ivan Ren wrote:
> qemu-img info with a block device which has a qcow2 format always
> return 0 for disk size, and this can not reflect the qcow2 size
> and the used space of the block device. This patch return the
> allocated size of qcow2 as the disk size.
I'm not quite sure whether you really need this information for block
devices (I tend to agree with Eric that wr_highest_cluster is the more
important information there), but I can imagine it just being nice to have.
So the basic idea makes sense to me, but I think the implementation can
be simplified and the reporting in qemu-img should be done differently.
>
> Signed-off-by: Ivan Ren <address@hidden>
> ---
> block/qcow2-bitmap.c | 69 +++++++++++++++++
> block/qcow2.c | 212
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> block/qcow2.h | 42 ++++++++++
> 3 files changed, 323 insertions(+)
The whole implementation reminds me a lot of qcow2's check function,
which basically just recalculates the refcounts. So I'm wondering
whether you could just count how many clusters with non-0 refcount there
are and thus simplify the implementation dramatically.
[...]
> +static int64_t qcow2_get_allocated_file_size(BlockDriverState *bs)
> +{
> + struct stat st;
> + if (stat(bs->filename, &st) < 0 || !S_ISBLK(st.st_mode)) {
> + goto get_file_size;
> + }
This definitely doesn't work because nobody guarantees that bs->filename
is something that stat() can work with. I'm aware that you need to do
the S_ISBLK() check somewhere, but the qcow2 driver is not the right place.
I don't really have a good way around this, though. These things come
to mind:
(1) We could let file-posix report an error for S_ISBLK because the
information is known to be usually useless -- but I think that is not
quite the right thing to do because maybe some block devices do report
useful information there, I don't know.
(2) Or we introduce a new field in qemu-img info (and thus in ImageInfo,
too, I suppose?). qemu-img info (or rather bdrv_query_image_info())
could detect whether the format layer supports
bdrv_get_allocated_file_size, and if so, it emits that information
separately from the allocated size of bs->file->bs. But that would
break vmdk...
(3) As a hack, qcow2_get_allocated_file_size() could first always call
bdrv_get_allocated_file_size(bs->file->bs), and if that returns 0 (which
is absolutely impossible for qcow2 files because they have an image
header that takes up some space), we fall back to
qcow2_get_block_allocated_size(). While I consider it a hack, I can't
come up with a scenario where it wouldn't work.
Max
> +
> + return qcow2_get_block_allocated_size(bs);
> +
> +get_file_size:
> + if (bs->file) {
> + return bdrv_get_allocated_file_size(bs->file->bs);
> + }
> + return -ENOTSUP;
> +}
> +
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH] qemu-img: return allocated size for block device with qcow2 format, 叶残风, 2018/05/03