On 08/29/2013 08:00 AM, Paolo Bonzini wrote: Subject line mentions bdrv_co_is_allocated, but patch body deals with bdrv_is_allocated.
> Some bdrv_is_allocated callers do not expect errors, but the fallback > in qcow2.c might make other callers trip on assertion failures or > infinite loops. > > Fix the callers to always look for errors. > > Cc: [email protected] > Reviewed-by: Eric Blake <[email protected]> > Signed-off-by: Paolo Bonzini <[email protected]> > --- > v4: also fix bdrv_commit, cow_read, img_convert, alloc_f Hmm - the v4 changelog implies things changed since my review. Thankfully, this patch still looks sane when looking at just this patch (what you have is good). But your comment made me grep the rest of the source code for ALL bdrv_is_allocated callers (since that's the harder task - ensuring we didn't forget anything): block-migration.c uses !bdrv_is_allocated as a condition for a while loop; should that check for errors? block/vvfat.c contains an if (bdrv_is_allocated(...)); should that handle errors? If you can justify that those don't need changes, then I'm okay with: Reviewed-by: Eric Blake <[email protected]> > block.c | 7 +++++-- > block/cow.c | 6 +++++- > block/qcow2.c | 4 +--- > block/stream.c | 2 +- > qemu-img.c | 16 ++++++++++++++-- > qemu-io-cmds.c | 4 ++++ > 6 files changed, 30 insertions(+), 9 deletions(-) > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
