On 10.05.20 15:40, Maxim Levitsky wrote: > blockdev-amend will be used similiar to blockdev-create > to allow on the fly changes of the structure of the format based block > devices. > > Current plan is to first support encryption keyslot management for luks > based formats (raw and embedded in qcow2) > > Signed-off-by: Maxim Levitsky <[email protected]> > Reviewed-by: Daniel P. Berrangé <[email protected]> > --- > block/Makefile.objs | 2 +- > block/amend.c | 108 ++++++++++++++++++++++++++++++++++++++ > include/block/block_int.h | 21 +++++--- > qapi/block-core.json | 42 +++++++++++++++ > qapi/job.json | 4 +- > 5 files changed, 169 insertions(+), 8 deletions(-) > create mode 100644 block/amend.c
[...]
> diff --git a/block/amend.c b/block/amend.c
> new file mode 100644
> index 0000000000..4840c0ffef
> --- /dev/null
> +++ b/block/amend.c
[...]
> +void qmp_x_blockdev_amend(const char *job_id,
> + const char *node_name,
> + BlockdevAmendOptions *options,
> + bool has_force,
> + bool force,
> + Error **errp)
> +{
> + BlockdevAmendJob *s;
> + const char *fmt = BlockdevDriver_str(options->driver);
> + BlockDriver *drv = bdrv_find_format(fmt);
> + BlockDriverState *bs = bdrv_find_node(node_name);
> +
> + /*
> + * If the driver is in the schema, we know that it exists.
I wonder why create.c then still checks whether drv == NULL.
I wouldn’t count on the schema. For example, with modularized block
driver I could imagine that a driver appears in the schema, but loading
the module fails, so that drv still ends up NULL.
> But it may not
> + * be whitelisted.
> + */
> + assert(drv);
> + if (bdrv_uses_whitelist() && !bdrv_is_whitelisted(drv, false)) {
> + error_setg(errp, "Driver is not whitelisted");
> + return;
> + }
[...]
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 0a71357b50..fdb0cdbcdd 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -133,12 +133,27 @@ struct BlockDriver {
> int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
> Error **errp);
> void (*bdrv_close)(BlockDriverState *bs);
> +
> +
> int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
> Error **errp);
> int coroutine_fn (*bdrv_co_create_opts)(BlockDriver *drv,
> const char *filename,
> QemuOpts *opts,
> Error **errp);
> +
> + int coroutine_fn (*bdrv_co_amend)(BlockDriverState *bs,
> + BlockdevAmendOptions *opts,
> + bool force,
> + Error **errp);
> +
> + int (*bdrv_amend_options)(BlockDriverState *bs,
> + QemuOpts *opts,
> + BlockDriverAmendStatusCB *status_cb,
> + void *cb_opaque,
> + bool force,
> + Error **errp);
> +
> int (*bdrv_make_empty)(BlockDriverState *bs);
>
> /*
> @@ -433,12 +448,6 @@ struct BlockDriver {
> BdrvCheckResult *result,
> BdrvCheckMode fix);
>
> - int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts,
> - BlockDriverAmendStatusCB *status_cb,
> - void *cb_opaque,
> - bool force,
> - Error **errp);
> -
No harm done, but why not just leave it where it was?
> void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
>
> /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 943df1926a..74db515414 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4649,6 +4649,48 @@
> 'data': { 'job-id': 'str',
> 'options': 'BlockdevCreateOptions' } }
>
> +##
> +# @BlockdevAmendOptions:
> +#
> +# Options for amending an image format
> +#
> +# @driver block driver that is suitable for the image
Missing colon after @driver. Also, what does “suitable” mean?
Shouldn’t it be exactly the node’s driver? (i.e. “Block driver of the
node to amend”)
Max
signature.asc
Description: OpenPGP digital signature
