[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 11/14] block/core: add generic infrastructure for x-blockd
From: |
Max Reitz |
Subject: |
Re: [PATCH v6 11/14] block/core: add generic infrastructure for x-blockdev-amend qmp command |
Date: |
Thu, 14 May 2020 17:47:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
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 <address@hidden>
> Reviewed-by: Daniel P. Berrangé <address@hidden>
> ---
> 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
- [PATCH v6 10/14] iotests: qemu-img tests for luks key management, (continued)
[PATCH v6 12/14] block/crypto: implement blockdev-amend, Maxim Levitsky, 2020/05/10
[PATCH v6 06/14] block/crypto: rename two functions, Maxim Levitsky, 2020/05/10
[PATCH v6 11/14] block/core: add generic infrastructure for x-blockdev-amend qmp command, Maxim Levitsky, 2020/05/10
- Re: [PATCH v6 11/14] block/core: add generic infrastructure for x-blockdev-amend qmp command,
Max Reitz <=
[PATCH v6 13/14] block/qcow2: implement blockdev-amend, Maxim Levitsky, 2020/05/10
[PATCH v6 14/14] iotests: add tests for blockdev-amend, Maxim Levitsky, 2020/05/10
Re: [PATCH v6 00/14] LUKS: encryption slot management using amend interface, no-reply, 2020/05/10
Re: [PATCH v6 00/14] LUKS: encryption slot management using amend interface, no-reply, 2020/05/10