From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54893) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XsqLx-0001fO-IS for qemu-devel@nongnu.org; Mon, 24 Nov 2014 04:54:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XsqLo-0005GE-GD for qemu-devel@nongnu.org; Mon, 24 Nov 2014 04:54:41 -0500 Received: from mail-wg0-x22e.google.com ([2a00:1450:400c:c00::22e]:53027) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XsqLo-0005Fy-9F for qemu-devel@nongnu.org; Mon, 24 Nov 2014 04:54:32 -0500 Received: by mail-wg0-f46.google.com with SMTP id x12so11957442wgg.5 for ; Mon, 24 Nov 2014 01:54:31 -0800 (PST) Sender: Paolo Bonzini Message-ID: <54730053.2000305@redhat.com> Date: Mon, 24 Nov 2014 10:54:27 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1414639364-4499-1-git-send-email-famz@redhat.com> <1414639364-4499-9-git-send-email-famz@redhat.com> <5458B279.8010300@redhat.com> <546FBB93.7000809@redhat.com> <5472EDE4.6050509@redhat.com> <5472FD4F.2090301@redhat.com> <5472FE91.7000402@redhat.com> In-Reply-To: <5472FE91.7000402@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , John Snow , Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Benoit Canet , Markus Armbruster , Luiz Capitulino , Stefan Hajnoczi , Jd , Vladimir Sementsov-Ogievskij On 24/11/2014 10:46, Max Reitz wrote: > On 2014-11-24 at 10:41, Paolo Bonzini wrote: >> >> On 24/11/2014 09:35, Max Reitz wrote: >>>>> But what if the dirty bitmap was enabled before so that this enabling >>>>> transaction was supposed to be a no-op? >>>> Maybe it's not a problem: The only case in which the enable/disable >>>> operation will fail is if it cannot find the device or bitmap -- in >>>> which case, the abort operation isn't going to be able to affect >>>> anything either. >>> Well, it's part of a transaction. As far as I understand it, one groups >>> several operations in one transaction and if any fails, all are aborted. >>> Therefore, the "enable the dirty bitmap" operation can trivially succeed >>> (because it was enabled before), but some different operation may fail, >>> which then results in invocation of this abort function. >> Would it work to do the actual enabling/disabling in the commit >> function, since it cannot fail? > > Too easy. :-) > > This patch could then no longer reuse qmp_block_dirty_bitmap_enable(), > but reimplementing it here should be fine (it's short enough). Ah, right: prepare: + bs = bdrv_find(device); + if (!bs) { + error_set(errp, QERR_DEVICE_NOT_FOUND, device); + return; + } + + bitmap = bdrv_find_dirty_bitmap(bs, name); + if (!bitmap) { + error_setg(errp, "Dirty bitmap not found: %s", name); + return; + } commit: + bdrv_enable_dirty_bitmap(bs, bitmap); But qmp_block_dirty_bitmap_enable can reuse qmp_transaction instead! :) Paolo