From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60018) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uymtr-0007Ha-8P for qemu-devel@nongnu.org; Mon, 15 Jul 2013 13:49:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uymto-00019T-5a for qemu-devel@nongnu.org; Mon, 15 Jul 2013 13:49:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12458) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uymtn-000193-VC for qemu-devel@nongnu.org; Mon, 15 Jul 2013 13:49:24 -0400 Date: Mon, 15 Jul 2013 10:49:18 -0700 From: Ian Main Message-ID: <20130715174918.GA15976@gate.mains.priv> References: <1372386525-23155-1-git-send-email-imain@redhat.com> <1372386525-23155-2-git-send-email-imain@redhat.com> <51D1730C.6030902@redhat.com> <20130708092150.GA11838@T430s.redhat.com> <51E3D3FF.1060309@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51E3D3FF.1060309@redhat.com> Subject: Re: [Qemu-devel] [PATCH V1 1/2] Implement sync modes for drive-backup. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Stefan Hajnoczi , famz@redhat.com, qemu-devel@nongnu.org On Mon, Jul 15, 2013 at 12:50:39PM +0200, Paolo Bonzini wrote: > Il 08/07/2013 11:21, Fam Zheng ha scritto: > > > Should the source be "bs" for MIRROR_SYNC_MODE_NONE? Also in this case > > > you may want to default the format to "qcow2" instead of bs's format. > > > > Maybe not. "source" only affects when sync=top below here. For reading > > the uncopied for target from source, we can't simply assign it to "bs" > > for sync=none and create the new image with with bs->filename as backing > > file, because that way we don't know how to open it with what we have > > now: the backing file is already opened RW (as "bs"). > > But if we do not set the source, how can you read at all from the > temporary backup destination? > > If I read the code correctly, target_bs should be opened with > BDRV_O_NO_BACKING, and then you should set target_bs->backing_hd = bs. > Which in turn would only work for a format that supports backing files > (such as qcow2). OK well, I'll explain here my understanding. I apologize if I explain more than needed but it might be good to get this out there anyway. When we do the create with: bdrv_img_create(target, format, source->filename, source->drv->format_name, NULL, size, flags, &local_err, false); We are creating a new target image using the same backing file as the original disk image. Then any new data that has been laid on top of it since creation is copied in the main backup_run() loop. There is an extra check in the 'TOP' case so that we don't bother to copy all the data of the backing file as it already exists in the target. This is where the bdrv_co_is_allocated() is used to determine if the data exists in the topmost layer or below. Also any new data being written is intercepted via the write_notifier hook which ends up calling backup_do_cow() to copy old data out before it gets overwritten. > I'm not sure how the "none" mode works with these patches. It's quite > possible I'm wrong, of course, but then the explanation should be in the > code or the commit message. It should be in the code or the commit > message even if my suggestions are applied. :) For mode 'NONE' we create the new target image and only copy in the original data from the disk image starting from the time the call was made. This preserves the point in time data by only copying the parts that are *going to change* to the target image. This way we can reconstruct the final image by checking to see if the given block exists in the new target image first, and if it does not, you can get it from the original image. This is basically an optimization allowing you to do point-in-time snapshots with low overhead vs the 'FULL' version. Since there is no old data to copy out the loop in backup_run() for the NONE case just calls qemu_coroutine_yield() which only wakes up after an event (usually cancel in this case). The rest is handled by the before_write notifier which again calls backup_do_cow() to write out the old data so it can be preserved. Does that help? Ian