From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49100) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z9afJ-00064b-R5 for qemu-devel@nongnu.org; Mon, 29 Jun 2015 11:08:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z9afG-0008Eq-JJ for qemu-devel@nongnu.org; Mon, 29 Jun 2015 11:08:09 -0400 Received: from mail-wg0-x229.google.com ([2a00:1450:400c:c00::229]:36745) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z9afG-0008Ei-90 for qemu-devel@nongnu.org; Mon, 29 Jun 2015 11:08:06 -0400 Received: by wguu7 with SMTP id u7so144001627wgu.3 for ; Mon, 29 Jun 2015 08:08:05 -0700 (PDT) Date: Mon, 29 Jun 2015 16:08:02 +0100 From: Stefan Hajnoczi Message-ID: <20150629150802.GM32151@stefanha-thinkpad.redhat.com> References: <1435234332-581-1-git-send-email-stefanha@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="1hVIwB4NpNcOOTEe" Content-Disposition: inline In-Reply-To: <1435234332-581-1-git-send-email-stefanha@redhat.com> Subject: Re: [Qemu-devel] [PATCH 00/10] block: incremental backup transactions using BlockJobTxn List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , famz@redhat.com, Jeff Cody , qemu-devel@nongnu.org, mreitz@redhat.com, vsementsov@parallels.com, John Snow --1hVIwB4NpNcOOTEe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 25, 2015 at 01:12:02PM +0100, Stefan Hajnoczi wrote: > This series is based on my block branch > (https://github.com/stefanha/qemu/commits/block). >=20 > It uses patches from John Snow's "[PATCH v6 00/10] block: incremental bac= kup > transactions" series but implements the feature with a new transaction > mechanism for blockjobs called BlockJobTxn. >=20 > Recap: motivation for block job transactions > -------------------------------------------- > If an incremental backup block job fails then we reclaim the bitmap so > the job can be retried. The problem comes when multiple jobs are started= as > part of a qmp 'transaction' command. We need to group these jobs in a > transaction so that either all jobs complete successfully or all bitmaps = are > reclaimed. >=20 > Without transactions, there is a case where some jobs complete successful= ly and > throw away their bitmaps, making it impossible to retry the backup by rer= unning > the command if one of the jobs fails. >=20 > How does this implementation work? > ---------------------------------- > These patches add a BlockJobTxn object with the following API: >=20 > txn =3D block_job_txn_new(); > block_job_txn_add_job(txn, job1); > block_job_txn_add_job(txn, job2); > block_job_txn_begin(); >=20 > The jobs either both complete successfully or they both fail/cancel. If = the > user cancels job1 then job2 will also be cancelled and vice versa. >=20 > Jobs stay alive waiting for other jobs to complete. They can be cancelle= d by > the user during this time. Job blockers are still in effect and no other= block > job can run on this device in the meantime (since QEMU currently only all= ows 1 > job per device). This is the main drawback to this approach but reasonab= le > since you probably don't want to run other jobs/operations until you're s= ure > the backup was successful (you won't be able to retry a failed backup if > there's a new job running). >=20 > Adding transaction support to the backup job is very easy. It just needs= to > make a call before throwing away the bitmap and returning from its corout= ine: >=20 > block_job_txn_prepare_to_complete(job->txn, job, ret); >=20 > if (job->sync_bitmap) { > BdrvDirtyBitmap *bm; > if (ret < 0 || block_job_is_cancelled(&job->common)) { > ... >=20 > John Snow (4): > qapi: Add transaction support to block-dirty-bitmap operations > iotests: add transactional incremental backup test > block: rename BlkTransactionState and BdrvActionOps > iotests: 124 - transactional failure test >=20 > Kashyap Chamarthy (1): > qmp-commands.hx: Update the supported 'transaction' operations >=20 > Stefan Hajnoczi (5): > block: keep bitmap if incremental backup job is cancelled > block: add block job transactions > blockdev: make BlockJobTxn available to qmp 'transaction' > block/backup: support block job transactions > tests: add BlockJobTxn unit test >=20 > block.c | 19 ++- > block/backup.c | 9 +- > blockdev.c | 298 +++++++++++++++++++++++++++++++++++----= ------ > blockjob.c | 160 ++++++++++++++++++++++++ > docs/bitmaps.md | 6 +- > include/block/block.h | 2 +- > include/block/block_int.h | 6 +- > include/block/blockjob.h | 49 ++++++++ > qapi-schema.json | 6 +- > qmp-commands.hx | 21 +++- > tests/Makefile | 3 + > tests/qemu-iotests/124 | 180 ++++++++++++++++++++++++++- > tests/qemu-iotests/124.out | 4 +- > tests/test-blockjob-txn.c | 191 +++++++++++++++++++++++++++++ > trace-events | 4 + > 15 files changed, 873 insertions(+), 85 deletions(-) > create mode 100644 tests/test-blockjob-txn.c John: Do you want to review this series? --1hVIwB4NpNcOOTEe Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVkV9SAAoJEJykq7OBq3PIDbAIAKMYqLNlY0Q2KXGBOc7uTId+ dOlje/iwQesiXVnLe5yhoHdwwNyr0RhB8+IsKkw6+hYLv4z8yrOIa3yi4Zl3j/aM KWLYGWd0oj2olOh9GjzVSv6kY1FlHH2LwUrqF5P2MregyI3Fx1lPe7FL/YayEBMv 3tneNc5ubu+AwkqeRIhHL6dvbk/MXizZn+VxAXYbZDXYXCaEmiCYQ09sP5TIXjjL gTlY1FnjQ+ONvzmiSIEO3oROS+Iy8SVKBYjNAZ1ZobMddD2AvIml4+aFH8zO6Qdt wEqOpsxzhQ/wj6MeTINMSKiwEnFZ17n1csUA5+EVakhjKOe2VHGOFy2tPOEc8Ig= =Xi2g -----END PGP SIGNATURE----- --1hVIwB4NpNcOOTEe--