On Thu, Jan 12, 2012 at 12:42 PM, Kevin Wolf <kw...@redhat.com> wrote: > Am 06.01.2012 15:01, schrieb Stefan Hajnoczi: >> From: Marcelo Tosatti <mtosa...@redhat.com> >> >> Add support for streaming data from an intermediate section of the >> image chain (see patch and documentation for details). >> >> Signed-off-by: Marcelo Tosatti <mtosa...@redhat.com> >> Signed-off-by: Stefan Hajnoczi <stefa...@linux.vnet.ibm.com> >> --- >> block.c | 64 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> block.h | 4 +++ >> block/stream.c | 28 +++++++++++++++++++++--- >> block_int.h | 3 +- >> blockdev.c | 11 ++++++--- >> 5 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, >> return data.ret; >> } >> >> +/* >> + * 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
