From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33321) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TdGpw-00037F-Gu for qemu-devel@nongnu.org; Tue, 27 Nov 2012 03:48:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TdGpu-0001fr-4a for qemu-devel@nongnu.org; Tue, 27 Nov 2012 03:48:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33303) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TdGpt-0001fj-Rb for qemu-devel@nongnu.org; Tue, 27 Nov 2012 03:48:10 -0500 Message-ID: <50B47E46.5020001@redhat.com> Date: Tue, 27 Nov 2012 09:48:06 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1353488464-82756-1-git-send-email-dietmar@proxmox.com> <50ACB184.5080204@redhat.com> <24E144B8C0207547AD09C467A8259F755782D8A1@lisa.maurer-it.com> <50ACCAEC.2030001@redhat.com> <24E144B8C0207547AD09C467A8259F755782F79B@lisa.maurer-it.com> <50AF3D23.2010909@redhat.com> <24E144B8C0207547AD09C467A8259F755782FA23@lisa.maurer-it.com> <24E144B8C0207547AD09C467A8259F755782FA71@lisa.maurer-it.com> <50AF5007.4030505@redhat.com> <24E144B8C0207547AD09C467A8259F7557831181@lisa.maurer-it.com> <50B35B80.2010605@redhat.com> <24E144B8C0207547AD09C467A8259F755783274A@lisa.maurer-it.com> <24E144B8C0207547AD09C467A8259F75578327EC@lisa.maurer-it.com> In-Reply-To: <24E144B8C0207547AD09C467A8259F75578327EC@lisa.maurer-it.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/5] RFC: Efficient VM backup for qemu (v1) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dietmar Maurer Cc: Paolo Bonzini , "qemu-devel@nongnu.org" Am 27.11.2012 08:15, schrieb Dietmar Maurer: >>> The only solution I came up with is to add before/after hooks in the >>> block job. I agree with the criticism, but I think it's general >>> enough and at the same time easy enough to implement. >>> >>>> IMHO, the current implementation is quite simple and easy to maintain. >>> >>> No, "if (bs->backup_info)" simply doesn't belong in bdrv_co_writev. >> >> I do not really understand that argument, because the current >> COPY_ON_READ implementation also works that way: >> >> if (bs->copy_on_read) { >> flags |= BDRV_REQ_COPY_ON_READ; >> } >> if (flags & BDRV_REQ_COPY_ON_READ) { >> bs->copy_on_read_in_flight++; >> } >> >> if (bs->copy_on_read_in_flight) { >> wait_for_overlapping_requests(bs, sector_num, nb_sectors); >> } >> >> tracked_request_begin(&req, bs, sector_num, nb_sectors, false); >> >> if (flags & BDRV_REQ_COPY_ON_READ) { ... >> >> Or do you also want to move that to block job hooks? > > Just tried to move that code, but copy on read feature is unrelated to block jobs, > i.e. one can open a bdrv with BDRV_O_COPY_ON_READ, and that does not create > a job. > > I already suggested to add those hooks to BDS instead - don't you think that would work? To which BDS? If it is the BDS that is being backed up, the problem is that you could only have one implementation per BDS, i.e. you couldn't use backup and copy on read or I/O throttling or whatever at the same time. Kevin