On Fri, 05/15 11:04, Alberto Garcia wrote: > On Fri 15 May 2015 04:33:19 AM CEST, Fam Zheng wrote: > > >> + /* Make sure that the image is opened in read-write mode */ > >> + orig_bs_flags = bdrv_get_flags(bs); > >> + if (!(orig_bs_flags & BDRV_O_RDWR)) { > >> + if (bdrv_reopen(bs, orig_bs_flags | BDRV_O_RDWR, errp) != 0) { > > > > I don't think all bdrv_reopen implementation will guarantee to set errp > > on error :(. For example: > > > > static int qcow2_reopen_prepare(BDRVReopenState *state, > > BlockReopenQueue *queue, Error **errp) > > { > > bdrv_reopen_prepare() already sets a generic error if the implementation > (qcow2_reopen_prepare() in this case) doesn't provide one: > > ret = drv->bdrv_reopen_prepare(reopen_state, queue, &local_err); > if (ret) { > if (local_err != NULL) { > error_propagate(errp, local_err); > } else { > error_setg(errp, "failed while preparing to reopen image '%s'", > reopen_state->bs->filename); > } > goto error; > } >
You're right, I missed that. So, Reviewed-by: Fam Zheng <f...@redhat.com>