Am 10.05.2010 22:12, schrieb Stefan Weil:
> The fix is based on a patch from Kevin Wolf. Here his comment:
>
> "The number of blocks needs to be rounded up to cover all of the virtual hard
> disk. Without this fix, we can't even open our own images if their size is not
> a multiple of the block size."
>
> While Kevin's patch addressed vdi_create, my modification also fixes
> vdi_open which now accepts images with odd disk sizes as well as
> images created with old versions of qemu-img.
>
> Cc: Kevin Wolf <kw...@redhat.com>
> Cc: François Revol <re...@free.fr>
> Signed-off-by: Stefan Weil <w...@mail.berlios.de>
> ---
> block/vdi.c | 29 +++++++++++++++++++++--------
> 1 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/block/vdi.c b/block/vdi.c
> index 1ce18d5..362c898 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -405,19 +405,12 @@ static int vdi_open(BlockDriverState *bs, int flags)
> /* We only support data blocks which start on a sector boundary. */
> logout("unsupported data offset 0x%x B\n", header.offset_data);
> goto fail;
> - } else if (header.disk_size % SECTOR_SIZE != 0) {
> - logout("unsupported disk size %" PRIu64 " B\n", header.disk_size);
> - goto fail;
> } else if (header.sector_size != SECTOR_SIZE) {
> logout("unsupported sector size %u B\n", header.sector_size);
> goto fail;
> } else if (header.block_size != 1 * MiB) {
> logout("unsupported block size %u B\n", header.block_size);
> goto fail;
> - } else if ((header.disk_size + header.block_size - 1) /
> header.block_size !=
> - (uint64_t)header.blocks_in_image) {
> - logout("unexpected block number %u B\n", header.blocks_in_image);
> - goto fail;
> } else if (!uuid_is_null(header.uuid_link)) {
> logout("link uuid != 0, unsupported\n");
> goto fail;
> @@ -426,6 +419,23 @@ static int vdi_open(BlockDriverState *bs, int flags)
> goto fail;
> }
>
> + if (header.disk_size % SECTOR_SIZE != 0) {
> + /* 'VBoxManage convertfromraw' can create images with odd disk sizes.
> + We accept them but round the disk size to the next multiple of
> + SECTOR_SIZE. */
> + logout("odd disk size %" PRIu64 " B, round up\n", header.disk_size);
> + header.disk_size += SECTOR_SIZE - 1;
> + header.disk_size &= ~(SECTOR_SIZE - 1);
> + }
> +
> + if (header.disk_size >
> + (uint64_t)header.blocks_in_image * header.block_size) {
> + /* Old versions of qemu-img could create images with too large
> + disk sizes. We accept them but truncate the disk size. */
> + logout("large disk size %" PRIu64 " B, truncated\n",
> header.disk_size);
> + header.disk_size = (uint64_t)header.blocks_in_image *
> header.block_size;
> + }
I don't think this is useful behaviour. Such images are broken and
should not be opened. While it's true that qemu-img could create such
images, qemu could never open them afterwards, so nobody can have used
them anyway. So I think a goto fail; is the right thing to do.
Otherwise the patch looks good to me now.
Kevin
