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 <mre...@redhat.com> > --- > 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