From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57564) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLc46-00053O-Fq for qemu-devel@nongnu.org; Wed, 11 Feb 2015 13:31:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLc43-0002MF-7R for qemu-devel@nongnu.org; Wed, 11 Feb 2015 13:31:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51126) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLc42-0002M9-WE for qemu-devel@nongnu.org; Wed, 11 Feb 2015 13:31:07 -0500 Message-ID: <54DB9FE8.6070806@redhat.com> Date: Wed, 11 Feb 2015 13:31:04 -0500 From: John Snow 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> In-Reply-To: <54DB9D06.3060209@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: Max Reitz , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, armbru@redhat.com, vsementsov@parallels.com, stefanha@redhat.com 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. 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. 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