From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41433) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGZcp-0006LV-6N for qemu-devel@nongnu.org; Tue, 25 Sep 2012 14:12:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGZcn-0003tr-V6 for qemu-devel@nongnu.org; Tue, 25 Sep 2012 14:12:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42550) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGZcn-0003tn-MM for qemu-devel@nongnu.org; Tue, 25 Sep 2012 14:12:49 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8PICmKU031221 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 25 Sep 2012 14:12:49 -0400 Message-ID: <5061F41F.4080502@redhat.com> Date: Tue, 25 Sep 2012 12:12:47 -0600 From: Eric Blake MIME-Version: 1.0 References: <1efc2c8c9b7c215b787507afa5a34c0175f7f4f0.1348589526.git.jcody@redhat.com> In-Reply-To: <1efc2c8c9b7c215b787507afa5a34c0175f7f4f0.1348589526.git.jcody@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig551D90172279BD51A00AF099" Subject: Re: [Qemu-devel] [PATCH v2 2/7] block: add live block commit functionality List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig551D90172279BD51A00AF099 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 09/25/2012 10:29 AM, Jeff Cody wrote: > This adds the live commit coroutine. This iteration focuses on the > commit only below the active layer, and not the active layer itself. >=20 > The behaviour is similar to block streaming; the sectors are walked > through, and anything that exists above 'base' is committed back down > into base. At the end, intermediate images are deleted, and the > chain stitched together. Images are restored to their original open > flags upon completion. >=20 > Signed-off-by: Jeff Cody > +void commit_start(BlockDriverState *bs, BlockDriverState *base, > + BlockDriverState *top, int64_t speed, > + BlockErrorAction on_error, BlockDriverCompletionFunc = *cb, > + void *opaque, Error **errp) > +{ > + CommitBlockJob *s; > + BlockReopenQueue *reopen_queue =3D NULL; > + int orig_overlay_flags; > + int orig_base_flags; > + BlockDriverState *overlay_bs; > + Error *local_err =3D NULL; > + > + if ((on_error =3D=3D BLOCK_ERR_STOP_ANY || > + on_error =3D=3D BLOCK_ERR_STOP_ENOSPC) && > + !bdrv_iostatus_is_enabled(bs)) { > + error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > + return; > + } > + > + overlay_bs =3D bdrv_find_overlay(bs, top); > + > + if (overlay_bs =3D=3D NULL) { > + error_setg(errp, "Could not find overlay image for %s:", top->= filename); > + return; > + } > + > + orig_base_flags =3D bdrv_get_flags(base); > + orig_overlay_flags =3D bdrv_get_flags(overlay_bs); I think you are missing a check here that base is on the backing chain of top. See also my comments to 5/7. > + > + /* convert base_bs & overlay_bs to r/w, if necessary */ > + if (!(orig_base_flags & BDRV_O_RDWR)) { > + reopen_queue =3D bdrv_reopen_queue(reopen_queue, base, > + orig_base_flags | BDRV_O_RDWR= ); > + } > + if (!(orig_overlay_flags & BDRV_O_RDWR)) { > + reopen_queue =3D bdrv_reopen_queue(reopen_queue, overlay_bs, > + orig_overlay_flags | BDRV_O_R= DWR); > + } > + if (reopen_queue) { > + bdrv_reopen_multiple(reopen_queue, &local_err); Is it valid to make a no-op call, such as: { "execute":"block-commit", "arguments":{ "device":"drive0", "top":"base", "base":"base" }} If so, should we do an early exit here, rather than temporarily changing base to R/W just to change it back to R/O? If not, should we be rejecting it up front (again, back to the question of failing if 'base' is not a backing file of 'top', even if both 'top' and 'base' are backing files of 'device'). --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig551D90172279BD51A00AF099 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQEcBAEBCAAGBQJQYfQfAAoJEKeha0olJ0NqJLAH/3BdXNpxHvR8+RAnXKsFDol7 rXJKXX86MR9G9Dz/GMIISUznkw7DDAxBvg3M4BBDu8X6BsZ/VcssoW2IEzeNIzNC fXfsODcwh0KCxSYy+PmqlFTzD2KH1GnJqp2AWBRgb94kPSTNEqjPGODdeFd6rBw3 BXyRS7Jb2xdwEwfjuMQOrSV7/FeK+l3vJTVyXVNm0Sou18FCf6uswJVb3VtCmYk3 aYGbx5kGxVSPNgMusyy7rm70szItrSKXn2/xu/VwA1vrK20wxAygTTVkmH+XUKm+ FhrQvtb1aRZSMMev1fytSc7rz3YXpk6QGKd1t7OZRDnG8e5qp+5ptKOzSFWhZSw= =LOpb -----END PGP SIGNATURE----- --------------enig551D90172279BD51A00AF099--