From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35164) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UkVCH-00031P-3J for qemu-devel@nongnu.org; Thu, 06 Jun 2013 04:05:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UkVC9-0002cd-6l for qemu-devel@nongnu.org; Thu, 06 Jun 2013 04:05:25 -0400 Received: from mail-we0-x232.google.com ([2a00:1450:400c:c03::232]:60307) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UkVC9-0002cV-1E for qemu-devel@nongnu.org; Thu, 06 Jun 2013 04:05:17 -0400 Received: by mail-we0-f178.google.com with SMTP id u53so1775171wes.37 for ; Thu, 06 Jun 2013 01:05:16 -0700 (PDT) Date: Thu, 6 Jun 2013 10:05:13 +0200 From: Stefan Hajnoczi Message-ID: <20130606080513.GA13466@stefanha-thinkpad.redhat.com> References: <1369917299-5725-1-git-send-email-stefanha@redhat.com> <1369917299-5725-4-git-send-email-stefanha@redhat.com> <20130606035618.GA24375@localhost.nay.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130606035618.GA24375@localhost.nay.redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 03/11] block: add basic backup support to block driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org, Kevin Wolf , dietmar@proxmox.com, imain@redhat.com, Paolo Bonzini , xiawenc@linux.vnet.ibm.com, Eric Blake On Thu, Jun 06, 2013 at 11:56:18AM +0800, Fam Zheng wrote: > On Thu, 05/30 14:34, Stefan Hajnoczi wrote: > > + > > +static int coroutine_fn backup_before_write_notify( > > + NotifierWithReturn *notifier, > > + void *opaque) > > +{ > > + BdrvTrackedRequest *req = opaque; > > + > > + return backup_do_cow(req->bs, req->sector_num, req->nb_sectors, NULL); > > +} > > I'm wondering if we can see the logic here with a backing hd > relationship? req->bs is a backing file of job->target, but guest is > going to write to it, so we need to COW down the data to job->target > before overwritting (i.e. cluster is not allocated in child). > > I think if we do this in block layer, there's not much necessity for a > before-write notifier here (although it may be useful for other cases): > > in bdrv_write: > for child in req->bs->open_children > if not child->is_allocated(req->sectors) > do COW to child > > The advantage of this is that we won't need to start block-backup job in > sync mode "none" to do point-in-time snapshot (image fleecing), and we > get writable snapshot (possibility to open backing file writable and > write to it safely) as a by-product. > > But we will need to keep track of parent<->child of block states, and we > still need to take care of overlapping writing between block job and > guest request. There's one catch here: bs->target may not support backing files, it can be a raw file, for example. We'll only use backing files for point-in-time snapshots but other use cases might not. raw doesn't really implement is_allocated(), so the whole concept would have to change a little: bs->open_children becomes independent of backing files - any BlockDriverState can be added to this list. ->is_allocated() basically becomes the bitmap that we keep in the block job. In the end I'm not sure there is much advantage since we need backup_do_cow() and the overlapping request code anyway for the block job. Stefan