From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54227) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bqLZ2-0004Tw-3X for qemu-devel@nongnu.org; Sat, 01 Oct 2016 10:46:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bqLYz-00046E-Ru for qemu-devel@nongnu.org; Sat, 01 Oct 2016 10:46:55 -0400 References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-7-git-send-email-vsementsov@virtuozzo.com> From: Max Reitz Message-ID: <617c1183-a483-7fb6-5e00-8d6fa5e7394f@redhat.com> Date: Sat, 1 Oct 2016 16:46:40 +0200 MIME-Version: 1.0 In-Reply-To: <1475232808-4852-7-git-send-email-vsementsov@virtuozzo.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ptgUn16U5Xj2vCHdndesPee9ApitvJgA0" Subject: Re: [Qemu-devel] [PATCH 06/22] qcow2: add dirty bitmaps extension List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --ptgUn16U5Xj2vCHdndesPee9ApitvJgA0 From: Max Reitz To: Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com Message-ID: <617c1183-a483-7fb6-5e00-8d6fa5e7394f@redhat.com> Subject: Re: [PATCH 06/22] qcow2: add dirty bitmaps extension References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-7-git-send-email-vsementsov@virtuozzo.com> In-Reply-To: <1475232808-4852-7-git-send-email-vsementsov@virtuozzo.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: > Add dirty bitmap extension as specified in docs/specs/qcow2.txt. > For now, just mirror extension header into Qcow2 state and check > constraints. >=20 > For now, disable image resize if it has bitmaps. It will be fixed later= =2E >=20 > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > block/qcow2.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++ > block/qcow2.h | 4 +++ > 2 files changed, 87 insertions(+) >=20 > diff --git a/block/qcow2.c b/block/qcow2.c > index c079aa8..08c4ef9 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c [...] > @@ -162,6 +164,62 @@ static int qcow2_read_extensions(BlockDriverState = *bs, uint64_t start_offset, > } > break; > =20 > + case QCOW2_EXT_MAGIC_DIRTY_BITMAPS: > + ret =3D bdrv_pread(bs->file, offset, &bitmaps_ext, ext.len= ); Overflows with ext.len > sizeof(bitmaps_ext). (ext.len < sizeof(bitmaps_ext) is also wrong, but less dramatically so.) > + if (ret < 0) { > + error_setg_errno(errp, -ret, "ERROR: bitmaps_ext: " > + "Could not read ext header"); > + return ret; > + } > + > + if (bitmaps_ext.reserved32 !=3D 0) { > + error_setg_errno(errp, -ret, "ERROR: bitmaps_ext: " > + "Reserved field is not zero."); Please drop the full stop at the end. > + return -EINVAL; > + } > + > + be32_to_cpus(&bitmaps_ext.nb_bitmaps); > + be64_to_cpus(&bitmaps_ext.bitmap_directory_size); > + be64_to_cpus(&bitmaps_ext.bitmap_directory_offset); > + > + if (bitmaps_ext.nb_bitmaps > QCOW_MAX_DIRTY_BITMAPS) { > + error_setg(errp, "ERROR: bitmaps_ext: " > + "too many dirty bitmaps"); > + return -EINVAL; > + } > + > + if (bitmaps_ext.nb_bitmaps =3D=3D 0) { > + error_setg(errp, "ERROR: bitmaps_ext: " > + "found bitmaps extension with zero bi= tmaps"); > + return -EINVAL; > + } > + > + if (bitmaps_ext.bitmap_directory_offset & (s->cluster_size= - 1)) { > + error_setg(errp, "ERROR: bitmaps_ext: " > + "wrong dirty bitmap directory offset"= ); s/wrong/invalid/ > + return -EINVAL; > + } > + > + if (bitmaps_ext.bitmap_directory_size > > + QCOW_MAX_DIRTY_BITMAP_DIRECTORY_SIZE) { > + error_setg(errp, "ERROR: bitmaps_ext: " > + "too large dirty bitmap directory"); > + return -EINVAL; > + } > + > + s->nb_bitmaps =3D bitmaps_ext.nb_bitmaps; > + s->bitmap_directory_offset =3D > + bitmaps_ext.bitmap_directory_offset; > + s->bitmap_directory_size =3D > + bitmaps_ext.bitmap_directory_size; > + > +#ifdef DEBUG_EXT > + printf("Qcow2: Got dirty bitmaps extension:" > + " offset=3D%" PRIu64 " nb_bitmaps=3D%" PRIu32 "\n",= > + s->bitmap_directory_offset, s->nb_bitmaps); > +#endif > + break; > + > default: > /* unknown magic - save it in case we need to rewrite the = header */ > { [...] > @@ -2509,6 +2585,13 @@ static int qcow2_truncate(BlockDriverState *bs, = int64_t offset) > return -ENOTSUP; > } > =20 > + /* cannot proceed if image has bitmaps */ > + if (s->nb_bitmaps) { > + /* FIXME */ I'd call it a TODO, but that's probably a matter of taste. > + error_report("Can't resize an image which has bitmaps"); > + return -ENOTSUP; > + } > + > /* shrinking is currently not supported */ > if (offset < bs->total_sectors * 512) { > error_report("qcow2 doesn't support shrinking images yet"); Max --ptgUn16U5Xj2vCHdndesPee9ApitvJgA0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEvBAEBCAAZBQJX78xQEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRD0B9sAYdXPQArh CACOSybwUSvkNedxi2TNvU5pC0Hy84QNU2y/Y0bFiYSUj8PKA/9LW2Wxie3uSvyl DF3uX17p1Tq73VTzhSYzLDOMyrzsJYlZCaHCSTNfXuaUN0q6gnt9r+mcddjyoB8n cHx8TIaVbzlIGmwh2Ij8dQs0o2hSpR7hmv3crD+DtXWw+rxbW2bui16KYeRvAv+m v1iFVsnYOOPrTbnOFOoASJC1YiCFbt/1mpogtIXTAxgsaHdzeMdY1vqXQT0w13J6 2brzvmwP7lu2PRosE9S8rmo/pdhZGHkZYPq120PHpl/ovHQ5sfjuhZj5kio8PfUQ w3yUwjAYns4YNm5lJ0F5T5FM =49fs -----END PGP SIGNATURE----- --ptgUn16U5Xj2vCHdndesPee9ApitvJgA0--