On Fri, 2 Sep 2011 18:06:13 -0300
Luiz Capitulino <lcapitul...@redhat.com> wrote:
> On Fri, 2 Sep 2011 12:34:56 -0500
> Anthony Liguori <aligu...@us.ibm.com> wrote:
>
> > A new QMP only command to change the blockdev associated with a block
> > device.
> > The semantics of change right now are just plain scary. This command
> > introduces
> > sane semantics. For more details, see the documentation in the schema file.
> >
> > Signed-off-by: Anthony Liguori <aligu...@us.ibm.com>
> > ---
> > v1 -> v2
> > - Rename command to drive-change (Kevin)
> > ---
> > blockdev.c | 108
> > +++++++++++++++++++++++++++++++++++++++++++++++++++---
> > qapi-schema.json | 30 +++++++++++++++
> > qmp-commands.hx | 8 ++++
> > 3 files changed, 140 insertions(+), 6 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 07eafce..cd338ed 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -688,12 +688,101 @@ void qmp_block_passwd(const char *device, const char
> > *password, Error **err)
> > qmp_set_blockdev_password(device, password, err);
> > }
> >
> > +static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char
> > *filename,
> > + int bdrv_flags, BlockDriver *drv,
> > + const char *password, Error **errp)
> > +{
> > + if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> > + error_set(errp, QERR_OPEN_FILE_FAILED, filename);
> > + return;
> > + }
> > +
> > + if (bdrv_key_required(bs)) {
> > + if (password) {
> > + if (bdrv_set_key(bs, password) < 0) {
> > + error_set(errp, QERR_INVALID_PASSWORD);
> > + }
> > + } else {
> > + error_set(errp, QERR_DEVICE_ENCRYPTED,
> > bdrv_get_device_name(bs),
> > + bdrv_get_encrypted_filename(bs));
> > + }
> > + } else if (password) {
> > + error_set(errp, QERR_DEVICE_NOT_ENCRYPTED,
> > bdrv_get_device_name(bs));
> > + }
> > +}
> > +
> > +void qmp_drive_change(const char *device, const char *filename,
> > + bool has_format, const char *format,
> > + bool has_password, const char *password,
> > + Error **errp)
> > +{
> > + BlockDriverState *bs, *bs1;
> > + BlockDriver *drv = NULL;
> > + int bdrv_flags;
> > + Error *err = NULL;
> > + bool probed_raw = false;
> > +
> > + bs = bdrv_find(device);
> > + if (!bs) {
> > + error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> > + return;
> > + }
> > +
> > + if (has_format) {
> > + drv = bdrv_find_whitelisted_format(format);
> > + if (!drv) {
> > + error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> > + return;
> > + }
> > + }
> > +
> > + bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
> > + bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> > +
> > + if (!has_password) {
> > + password = NULL;
> > + }
> > +
> > + /* Try to open once with a temporary block device to make sure that
> > + * the disk isn't encrypted and we lack a key. This also helps keep
> > + * this operation as a transaction. That is, if we fail, we try very
> > + * hard to make sure that the state is the same as before the operation
> > + * was started.
> > + */
> > + bs1 = bdrv_new("");
> > + qmp_bdrv_open_encrypted(bs1, filename, bdrv_flags, drv, password,
> > &err);
> > + if (!has_format && bs1->drv->unsafe_probe) {
> > + probed_raw = true;
> > + }
> > + bdrv_delete(bs1);
> > +
> > + if (err) {
> > + error_propagate(errp, err);
> > + return;
> > + }
> > +
> > + if (probed_raw) {
> > + /* We will not auto probe raw files. */
> > + error_set(errp, QERR_MISSING_PARAMETER, "format");
> > + return;
> > + }
>
> This has to be noted in the documentation.
>
> > +
> > + eject_device(bs, 0, &err);
> > + if (err) {
> > + error_propagate(errp, err);
> > + return;
> > + }
>
> Hm. I'm not wondering if this command isn't actually overloading two
s/not/now
> distinct operations: eject and insert. Maybe what we want here is
> drive-insert?
>
> > +
> > + qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, password, errp);
> > +}
> > +
> > int do_change_block(Monitor *mon, const char *device,
> > const char *filename, const char *fmt)
> > {
> > BlockDriverState *bs;
> > BlockDriver *drv = NULL;
> > int bdrv_flags;
> > + Error *err = NULL;
> >
> > bs = bdrv_find(device);
> > if (!bs) {
> > @@ -707,16 +796,23 @@ int do_change_block(Monitor *mon, const char *device,
> > return -1;
> > }
> > }
> > - if (eject_device(bs, 0, NULL) < 0) {
> > - return -1;
> > - }
> > +
> > bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
> > bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> > - if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> > - qerror_report(QERR_OPEN_FILE_FAILED, filename);
> > +
> > + eject_device(bs, 0, &err);
> > + if (err) {
> > + qerror_report_err(err);
> > + return -1;
> > + }
> > +
> > + qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err);
> > + if (err) {
> > + qerror_report_err(err);
> > return -1;
> > }
> > - return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
> > +
> > + return 0;
> > }
> >
> > int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 0c6c9b8..cbb5bf1 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -124,3 +124,33 @@
> > # settings may change after executing this command.
> > ##
> > { 'command': 'change-vnc-listen', 'data': {'target': 'str'} }
> > +
> > +##
> > +# @drive-change:
> > +#
> > +# This is the preferred interface for changing a block device.
> > +#
> > +# @device: The block device name.
> > +#
> > +# @filename: The new filename for the block device. This may contain
> > options
> > +# encoded in a format specified by @format.
> > +#
> > +# @format: #optional The format to use open the block device
> > +#
> > +# @password: #optional The password to use if the block device is encrypted
> > +#
> > +# Returns: Nothing on success.
> > +# If @device is not a valid block device, DeviceNotFound
> > +# If @format is not a valid block format, InvalidBlockFormat
> > +# If @filename is encrypted and @password isn't supplied,
> > +# DeviceEncrypted. The call should be repeated with @password
> > +# supplied.
> > +# If @format is not specified and @filename is a format that
> > cannot
> > +# be safely probed, MissingParameter.
> > +# If @filename cannot be opened, OpenFileFailed
> > +#
> > +# Since: 1.0
> > +##
> > +{ 'command': 'drive-change',
> > + 'data': {'device': 'str', 'filename': 'str', '*format': 'str',
> > + '*password': 'str'} }
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 5cab212..623f158 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -121,6 +121,14 @@ EQMP
> > .mhandler.cmd_new = do_change,
> > },
> >
> > + {
> > + .name = "drive-change",
> > + .args_type = "device:B,filename:F,format:s?password:s?",
> > + .params = "device filename [format] [password]",
> > + .help = "change a removable medium, optional format",
> > + .mhandler.cmd_new = qmp_marshal_input_drive_change,
> > + },
> > +
> > SQMP
> > change
> > ------
>
