On 11/25/2015 12:39 AM, Fam Zheng wrote: > The "flags" bit mask is expanded to two booleans, "data" and "zero"; > "bs" is replaced with "filename" string. > > Signed-off-by: Fam Zheng <f...@redhat.com> > --- > qapi/block-core.json | 28 ++++++++++++++++++++++++++++ > qemu-img.c | 48 ++++++++++++++++++++++-------------------------- > 2 files changed, 50 insertions(+), 26 deletions(-) >
> +## > + > +{ 'struct': 'MapEntry', Blank line is not typical here. > + 'data': {'start': 'int', 'length': 'int', 'data': 'bool', > + 'zero': 'bool', 'depth': 'int', '*offset': 'int', > + '*filename': 'str' } } Struct looks fine. > > - if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == > BDRV_BLOCK_DATA) { > + if (e->data && !e->zero) { > printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n", > - e->start, e->length, e->offset, e->bs->filename); > + e->start, e->length, e->offset, > + e->has_filename ? e->filename : ""); > } This blindly prints e->offset, even though...[1] > case OFORMAT_JSON: > - printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64", \"depth\": > %d," > + printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64"," > + " \"depth\": %ld," %ld is wrong; it needs to be PRId64. > " \"zero\": %s, \"data\": %s", Worth joining the two short lines into one? > @@ -2219,10 +2208,15 @@ static int get_block_status(BlockDriverState *bs, > int64_t sector_num, > > e->start = sector_num * BDRV_SECTOR_SIZE; > e->length = nb_sectors * BDRV_SECTOR_SIZE; > - e->flags = ret & ~BDRV_BLOCK_OFFSET_MASK; > + e->data = !!(ret & BDRV_BLOCK_DATA); > + e->zero = !!(ret & BDRV_BLOCK_ZERO); > e->offset = ret & BDRV_BLOCK_OFFSET_MASK; > + e->has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID); [1]... offset might be garbage if has_offset is false. > e->depth = depth; > - e->bs = bs; > + if (e->has_offset) { > + e->has_filename = true; > + e->filename = bs->filename; > + } > return 0; > } Are we guaranteed that bs->filename is non-NULL when offset is set? Or does this need to be if (e->has_offset && bs->filename)? > > @@ -2307,9 +2301,11 @@ static int img_map(int argc, char **argv) > goto out; > } > > - if (curr.length != 0 && curr.flags == next.flags && > + if (curr.length != 0 && curr.zero == next.zero && > + curr.data == next.data && > curr.depth == next.depth && > - ((curr.flags & BDRV_BLOCK_OFFSET_VALID) == 0 || > + !strcmp(curr.filename, next.filename) && Is this strcmp valid even when !has_filename? > + (!curr.has_offset || > curr.offset + curr.length == next.offset)) { > curr.length += next.length; > continue; > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature