Am 16.07.2014 um 20:07 hat Eric Blake geschrieben: > On 07/16/2014 09:48 AM, Kevin Wolf wrote: > > @@ -1154,7 +1167,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, > > BlockDriverState *backing_hd) > > bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, > > bs->backing_blocker); > > out: > > - bdrv_refresh_limits(bs); > > + bdrv_refresh_limits(bs, NULL); > > } > > > > /* > > @@ -1778,7 +1791,7 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) > > BDRV_O_CACHE_WB); > > reopen_state->bs->read_only = !(reopen_state->flags & BDRV_O_RDWR); > > > > - bdrv_refresh_limits(reopen_state->bs); > > + bdrv_refresh_limits(reopen_state->bs, NULL); > > > +++ b/block/stream.c > > @@ -76,7 +76,7 @@ static void close_unused_images(BlockDriverState *top, > > BlockDriverState *base, > > bdrv_unref(unused); > > } > > > > - bdrv_refresh_limits(top); > > + bdrv_refresh_limits(top, NULL); > > } > > > > Should these three callers be concerned about failure? If so, would > &error_abort be better than NULL? But as for this patch, you are > preserving existing semantics, so you could save it for a later patch.
They probably should, but I couldn't figure out what they should to on failure. Aborting qemu because a single block device fails isn't nice. &error_abort has similar semantics as assert() for me ("This can't ever happen"), and this is definitely not the case here as I/O errors can happen anytime. If anything, it's similar to a qcow2 image that has detected corruption and therefore made the BDS unusable. But setting bs->drv = NULL within a single block driver is already tricky (and so it was buggy at first), doing it in block.c with BDSes of any driver sounds even worse. This is the reason why I kept NULL here, even if it's not completely right. Kevin
pgphNT7w32VvT.pgp
Description: PGP signature