On 11/20/2014 05:44 AM, Max Reitz wrote: > Add an option to qmp_change_blockdev() which allows changing the > read-only status of the block device to be changed. > > Some drives do not have a inherently fixed read-only status; for > instance, floppy disks can be set read-only or writable independently of > the drive. Some users may find it useful to be able to therefore change > the read-only status of a block device when changing the medium. > > Signed-off-by: Max Reitz <[email protected]> > --- > blockdev.c | 41 ++++++++++++++++++++++++++++++++++++++--- > include/sysemu/blockdev.h | 3 ++- > qapi-schema.json | 20 ++++++++++++++++++++ > qmp.c | 3 ++- > 4 files changed, 62 insertions(+), 5 deletions(-) >
>
> - qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
> + qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err);
> +
> + if (err) {
> + if (read_only == BLOCKDEV_CHANGE_READ_ONLY_MODE_AUTO) {
> + error_free(err);
> + err = NULL;
> +
> + /* RDWR did not work, try RO now */
> + bdrv_flags &= ~BDRV_O_RDWR;
> + qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL,
> errp);
> + } else {
> + error_propagate(errp, err);
> + }
Umm, why are you propagating the error here manually, when it was
previously propagated as part of the fall-through into the out: label?
Particularly since the second open_encrypted call still relies on
fallthrough for propagating the error? I think this should be
simplified to:
if (err && read_only == BLOCKDEV_CHANGE_READ_ONLY_MODE_AUTO) {
error_free(err);
err = NULL;
/* RDWR did not work, try RO now */
bdrv_flags &= ~BDRV_O_RDWR;
qmp_bdrv_open_encrypted(...);
}
Otherwise, looks okay to me.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
