On Wed, Apr 05, 2017 at 02:28:44PM -0400, Jeff Cody wrote: Minor comments but:
Reviewed-by: Stefan Hajnoczi <[email protected]> > diff --git a/block.c b/block.c > index 7b4c7ef..f60d5ea 100644 > --- a/block.c > +++ b/block.c > @@ -192,9 +192,17 @@ void path_combine(char *dest, int dest_size, > } > } > > -void bdrv_set_read_only(BlockDriverState *bs, bool read_only) > +int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp) > { > + /* Do not set read_only if copy_on_read is enabled */ > + if (bs->copy_on_read && read_only) { > + error_setg(errp, "Cannot set node '%s' to r/o while COW enabled", Users might be puzzled by "COR". The -drive option is called "copy-on-read" so spelling it out is clearer than using an acronym. > @@ -1173,7 +1171,18 @@ static int vvfat_open(BlockDriverState *bs, QDict > *options, int flags, > if (ret < 0) { > goto fail; > } > - bdrv_set_read_only(bs, false); > + ret = bdrv_set_read_only(bs, false, &local_err); > + if (ret < 0) { > + error_propagate(errp, local_err); > + goto fail; > + } read_only = false by default. There's no need to set it now that you've moved the bdrv_set_read_only(bs, true) call.
signature.asc
Description: PGP signature
