From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YqNCO-0006Tz-TE for qemu-devel@nongnu.org; Thu, 07 May 2015 10:54:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YqNCN-0007nQ-T5 for qemu-devel@nongnu.org; Thu, 07 May 2015 10:54:52 -0400 Date: Thu, 7 May 2015 15:54:40 +0100 From: Stefan Hajnoczi Message-ID: <20150507145440.GN13985@stefanha-thinkpad.redhat.com> References: <1429747493-24397-1-git-send-email-jsnow@redhat.com> <1429747493-24397-2-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="SgT04PEqo/+yUDw3" Content-Disposition: inline In-Reply-To: <1429747493-24397-2-git-send-email-jsnow@redhat.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v3 01/10] qapi: Add transaction support to block-dirty-bitmap operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: famz@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com, vsementsov@parallels.com, stefanha@redhat.com --SgT04PEqo/+yUDw3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Apr 22, 2015 at 08:04:44PM -0400, John Snow wrote: > +static void block_dirty_bitmap_clear_prepare(BlkTransactionState *common, > + Error **errp) > +{ > + BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, > + common, common); > + BlockDirtyBitmap *action; > + > + action = common->action->block_dirty_bitmap_clear; > + state->bitmap = block_dirty_bitmap_lookup(action->node, > + action->name, > + &state->bs, > + &state->aio_context, > + errp); > + if (!state->bitmap) { > + return; > + } > + > + if (bdrv_dirty_bitmap_frozen(state->bitmap)) { > + error_setg(errp, "Cannot modify a frozen bitmap"); > + return; > + } else if (!bdrv_dirty_bitmap_enabled(state->bitmap)) { > + error_setg(errp, "Cannot clear a disabled bitmap"); > + return; > + } > + > + /* AioContext is released in .clean() */ > +} > + > +static void block_dirty_bitmap_clear_commit(BlkTransactionState *common) > +{ > + BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, > + common, common); > + bdrv_clear_dirty_bitmap(state->bitmap); > +} These semantics don't work in this example: [block-dirty-bitmap-clear, drive-backup] Since drive-backup starts the blockjob in .prepare() but block-dirty-bitmap-clear only clears the bitmap in .commit() the order is wrong. .prepare() has to do something non-destructive, like stashing away the HBitmap and replacing it with an empty one. Then .commit() can discard the old bitmap while .abort() can move the old bitmap back to undo the operation. Stefan --SgT04PEqo/+yUDw3 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVS3ywAAoJEJykq7OBq3PICt8H+gNsTodNqGmufkwg0YcVR/Yu quemvnQvnMDuR6EvkK+UlS06b5bbd7a97vBMdDg0fOmWuZtWLWhHl8jv7l2NmxjI HsmpWHZk/ymYxWpmdvAJgZRxgLiJsv4K5JXdt5Y9rW8i/oAOq49AdrF3to3T64WS SslVFv8TjDof9AlnxmC9odv/hjPkDPUKsTpbITGihEQ4ZXIuysAg+b4EIEfvj4Sj 9KZnvBm3B2Z+aAJW7gFOLoYmzJaWkubn0SqQrXCA5baZyW/RDTX7Srf+ydpMmsef EOa5A4zMcEQqzbfgy2L2wpaJWpB46tSsXrt+FBZEuwqkrQa5fz4J4l1lXXaaJfI= =oO9M -----END PGP SIGNATURE----- --SgT04PEqo/+yUDw3--