From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48465 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OBkGy-000646-6Z for qemu-devel@nongnu.org; Tue, 11 May 2010 03:53:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OBkGt-0007jf-C0 for qemu-devel@nongnu.org; Tue, 11 May 2010 03:53:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5660) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OBkGs-0007jI-Tl for qemu-devel@nongnu.org; Tue, 11 May 2010 03:52:55 -0400 Message-ID: <4BE90CB6.50709@redhat.com> Date: Tue, 11 May 2010 09:52:22 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <4BE7BA2B.103@redhat.com> <1273522353-12851-1-git-send-email-weil@mail.berlios.de> In-Reply-To: <1273522353-12851-1-git-send-email-weil@mail.berlios.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCH] vdi: Fix image opening and creation for odd disk sizes List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: =?UTF-8?B?RnJhbsOnb2lzIFJldm9s?= , qemu-devel@nongnu.org Am 10.05.2010 22:12, schrieb Stefan Weil: > The fix is based on a patch from Kevin Wolf. Here his comment: >=20 > "The number of blocks needs to be rounded up to cover all of the virtua= l hard > disk. Without this fix, we can't even open our own images if their size= is not > a multiple of the block size." >=20 > 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. >=20 > Cc: Kevin Wolf > Cc: Fran=C3=A7ois Revol > Signed-off-by: Stefan Weil > --- > block/vdi.c | 29 +++++++++++++++++++++-------- > 1 files changed, 21 insertions(+), 8 deletions(-) >=20 > 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 fla= gs) > /* We only support data blocks which start on a sector boundar= y. */ > logout("unsupported data offset 0x%x B\n", header.offset_data)= ; > goto fail; > - } else if (header.disk_size % SECTOR_SIZE !=3D 0) { > - logout("unsupported disk size %" PRIu64 " B\n", header.disk_si= ze); > - goto fail; > } else if (header.sector_size !=3D SECTOR_SIZE) { > logout("unsupported sector size %u B\n", header.sector_size); > goto fail; > } else if (header.block_size !=3D 1 * MiB) { > logout("unsupported block size %u B\n", header.block_size); > goto fail; > - } else if ((header.disk_size + header.block_size - 1) / header.blo= ck_size !=3D > - (uint64_t)header.blocks_in_image) { > - logout("unexpected block number %u B\n", header.blocks_in_imag= e); > - goto fail; > } else if (!uuid_is_null(header.uuid_link)) { > logout("link uuid !=3D 0, unsupported\n"); > goto fail; > @@ -426,6 +419,23 @@ static int vdi_open(BlockDriverState *bs, int flag= s) > goto fail; > } > =20 > + if (header.disk_size % SECTOR_SIZE !=3D 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 +=3D SECTOR_SIZE - 1; > + header.disk_size &=3D ~(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.di= sk_size); > + header.disk_size =3D (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