From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57918) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RlNIG-00065y-Ly for qemu-devel@nongnu.org; Thu, 12 Jan 2012 11:14:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RlNIA-0005U9-V6 for qemu-devel@nongnu.org; Thu, 12 Jan 2012 11:14:24 -0500 Received: from mail-wi0-f173.google.com ([209.85.212.173]:38994) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RlNIA-0005Tp-Oc for qemu-devel@nongnu.org; Thu, 12 Jan 2012 11:14:18 -0500 Received: by wibhm11 with SMTP id hm11so1605708wib.4 for ; Thu, 12 Jan 2012 08:14:18 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <4F0ED544.9050602@redhat.com> References: <1325858501-25741-1-git-send-email-stefanha@linux.vnet.ibm.com> <1325858501-25741-14-git-send-email-stefanha@linux.vnet.ibm.com> <4F0ED544.9050602@redhat.com> Date: Thu, 12 Jan 2012 16:14:17 +0000 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 13/15] block stream: add support for partial streaming List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Luiz Capitulino , Marcelo Tosatti , Stefan Hajnoczi , qemu-devel@nongnu.org On Thu, Jan 12, 2012 at 12:42 PM, Kevin Wolf wrote: > Am 06.01.2012 15:01, schrieb Stefan Hajnoczi: >> From: Marcelo Tosatti >> >> Add support for streaming data from an intermediate section of the >> image chain (see patch and documentation for details). >> >> Signed-off-by: Marcelo Tosatti >> Signed-off-by: Stefan Hajnoczi >> --- >> =A0block.c =A0 =A0 =A0 =A0| =A0 64 +++++++++++++++++++++++++++++++++++++= +++++++++++++++++++ >> =A0block.h =A0 =A0 =A0 =A0| =A0 =A04 +++ >> =A0block/stream.c | =A0 28 +++++++++++++++++++++--- >> =A0block_int.h =A0 =A0| =A0 =A03 +- >> =A0blockdev.c =A0 =A0 | =A0 11 ++++++--- >> =A05 files changed, 101 insertions(+), 9 deletions(-) >> >> diff --git a/block.c b/block.c >> index 9b688a0..d2143b1 100644 >> --- a/block.c >> +++ b/block.c >> @@ -2263,6 +2263,70 @@ int bdrv_is_allocated(BlockDriverState *bs, int64= _t sector_num, int nb_sectors, >> =A0 =A0 =A0return data.ret; >> =A0} >> >> +/* >> + * Given an image chain: [BASE] -> [INTER1] -> [INTER2] -> [TOP] >> + * >> + * Return true if the given sector is allocated in top or base. >> + * Return false if the given sector is allocated in intermediate images= . > > This description is inexact. A sector could be allocated both in base in > an intermediate image. > > Also initially I thought that we not only need to check whether the > sector is allocated in BASE, but also in any parents of BASE. You don't > do this: Clusters that are completely unallocated all through the chain > are reported as allocated. > > After reading all of the patch, I believe this provides the right > semantics: "Normal" image streaming would copy them into the topmost > file, but if you keep a backing file, you want to copy as little as > possible and keep using the backing file whenever possible. > > So I suggest to fix the description rather than the implementation. > > Maybe we should also rename the function. With this semantics it's not a > generic is_allocated function any more, but something quite specific to > streaming with a base file. I have moved the function into block/stream.c and renamed it to just is_allocated_base(). The description is updated. This makes it clearer that it's a special-case is_allocated-like function. Stefan