From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58094) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLc6k-00063A-7N for qemu-devel@nongnu.org; Wed, 11 Feb 2015 13:33:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLc6g-0003TU-Rj for qemu-devel@nongnu.org; Wed, 11 Feb 2015 13:33:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49323) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLc6g-0003TP-Jn for qemu-devel@nongnu.org; Wed, 11 Feb 2015 13:33:50 -0500 Message-ID: <54DBA08A.90901@redhat.com> Date: Wed, 11 Feb 2015 13:33:46 -0500 From: Max Reitz MIME-Version: 1.0 References: <1423532117-14490-1-git-send-email-jsnow@redhat.com> <1423532117-14490-8-git-send-email-jsnow@redhat.com> <54DB95CC.5050205@redhat.com> <54DB974C.902@redhat.com> <54DB9D06.3060209@redhat.com> <54DB9FE8.6070806@redhat.com> In-Reply-To: <54DB9FE8.6070806@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v12 07/17] qmp: Add support of "dirty-bitmap" sync mode for drive-backup List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, armbru@redhat.com, vsementsov@parallels.com, stefanha@redhat.com On 2015-02-11 at 13:31, John Snow wrote: > > > On 02/11/2015 01:18 PM, Max Reitz wrote: >> On 2015-02-11 at 12:54, John Snow wrote: >>> >>> On 02/11/2015 12:47 PM, Max Reitz wrote: >>>> Looks good to me in general, now I need to find out what the successor >>>> bitmap is used for; but I guess I'll find that out by reviewing the >>>> rest >>>> of this series. >>>> >>>> Max >>> >>> They don't really come up again, actually. >>> >>> The basic idea is this: While the backup is going on, reads and writes >>> may occur (albeit delayed) and we want to track those writes in a >>> separate bitmap for the duration of the backup operation. >> >> Yes, I thought as much; but where are writes to the named bitmap being >> redirected to its successor? bdrv_set_dirty() doesn't do that, as far as >> I can see. >> > > bdrv_dirty_bitmap_create_successor calls bdrv_create_dirty_bitmap, > which installs it in the bitmap chain attached to a BDS: > > QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list); > > which is read by bdrv_set_dirty. Oooh, clever. Right. > > bdrv_set_dirty operates on all bitmaps attached to a BDS, while > bdrv_set_dirty_bitmap operates on a single specific instance. > >>> If the backup operation fails, we use the dirty sector tracking info >>> in the successor to know what has changed since we started the backup, >>> and we merge this bitmap back into the originating bitmap; then if an >>> incremental backup is tried again, it includes all of the original >>> data plus any data changed while we failed to do a backup. >>> >>> If the backup operation succeeds, the originating bitmap is deleted >>> and the successor is installed in its place. >>> >>> It's a namespace trick: by having an anonymous bitmap as a child of >>> the "real" bitmap, the real bitmap can be frozen and prohibited from >>> being moved, renamed, deleted, etc. This prevents the user from adding >>> a new bitmap with the same name or similar while the backup is in >>> progress. >> >> Hm, if it's just for that, wouldn't disabling the bitmap suffice? >> >> Max >> > > Kind of? We still want to track writes while it's disabled. Right, I was assuming here that writes are not tracked in the successor. Thanks for pointing out that they are! Max > > If we try to use a single bitmap, we have no real way to know which > bits to clear after the operation succeeds. I think two bitmaps is a > requirement to accommodate both failure and success cases. > > A distinction is made between a disabled bitmap (which is just > read-only: it can be deleted) and a frozen bitmap (which is in-use by > an operation, implicitly disabled, and cannot be enabled, disabled, > deleted, cleared, set or reset.) > >>> A previous approach was to immediately take the bitmap off of the BDS, >>> but in the error case here, the logic becomes more complicated when we >>> need to re-install the bitmap but the user has already installed a new >>> bitmap with the same name, etc. >>> >>> So the general lifetime is this: >>> >>> (1) A backup is started. the block/backup routine calls >>> create_successor. >>> (2) If the backup fails to start, the block/backup routine will call >>> the "reclaim" method, which will merge the (empty) successor back into >>> the original bitmap, unfreezing it. >>> (3) If the backup starts, and then fails, the bitmap is "reclaim"ed >>> (merged back into one bitmap.) >>> (4) If the backup succeeds, the bitmap "abdicates" to the successor. >>> (The parent bitmap is erased and the successor is installed in its >>> place.) >> >> Yes, see the graph at the whiteboard behind me. :-) >> >> Max