[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 03/11] block: Accept node-name for blockdev-b
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v4 03/11] block: Accept node-name for blockdev-backup |
Date: |
Mon, 18 Jul 2016 15:59:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
On 14.07.2016 15:28, Kevin Wolf wrote:
> In order to remove the necessity to use BlockBackend names in the
> external API, we want to allow node-names everywhere. This converts
> blockdev-backup and the corresponding transaction action to accept a
> node-name without lifting the restriction that we're operating at a root
> node.
>
> In case of an invalid device name, the command returns the GenericError
> error class now instead of DeviceNotFound, because this is what
> qmp_get_root_bs() returns.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> blockdev.c | 35 ++++++++++-------------------------
> qapi/block-core.json | 2 +-
> qmp-commands.hx | 2 +-
> 3 files changed, 12 insertions(+), 27 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 22bf03d..e5a5642 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1954,38 +1954,31 @@ static void blockdev_backup_prepare(BlkActionState
> *common, Error **errp)
> {
> BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common,
> common);
> BlockdevBackup *backup;
> - BlockBackend *blk, *target;
> + BlockDriverState *bs, *target;
> Error *local_err = NULL;
>
> assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
> backup = common->action->u.blockdev_backup.data;
>
> - blk = blk_by_name(backup->device);
> - if (!blk) {
> - error_setg(errp, "Device '%s' not found", backup->device);
> - return;
> - }
> -
> - if (!blk_is_available(blk)) {
> - error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, backup->device);
> + bs = qmp_get_root_bs(backup->device, errp);
> + if (!bs) {
> return;
> }
>
> - target = blk_by_name(backup->target);
> + target = qmp_get_root_bs(backup->target, errp);
As of recently (0d978913), do_blockdev_backup() uses bdrv_lookup_bs()
for the target. While I think it was a mistake for that commit not to
change this place to bdrv_lookup_bs() as well, I think this patch should
either not touch this place at all, or it should fix it by using
bdrv_lookup_bs().
Max
> if (!target) {
> - error_setg(errp, "Device '%s' not found", backup->target);
> return;
> }
>
> /* AioContext is released in .clean() */
> - state->aio_context = blk_get_aio_context(blk);
> - if (state->aio_context != blk_get_aio_context(target)) {
> + state->aio_context = bdrv_get_aio_context(bs);
> + if (state->aio_context != bdrv_get_aio_context(target)) {
> state->aio_context = NULL;
> error_setg(errp, "Backup between two IO threads is not implemented");
> return;
> }
> aio_context_acquire(state->aio_context);
> - state->bs = blk_bs(blk);
> + state->bs = bs;
> bdrv_drained_begin(state->bs);
>
> do_blockdev_backup(backup->has_job_id ? backup->job_id : NULL,
> @@ -3361,7 +3354,6 @@ void do_blockdev_backup(const char *job_id, const char
> *device,
> BlockdevOnError on_target_error,
> BlockJobTxn *txn, Error **errp)
> {
> - BlockBackend *blk;
> BlockDriverState *bs;
> BlockDriverState *target_bs;
> Error *local_err = NULL;
> @@ -3377,21 +3369,14 @@ void do_blockdev_backup(const char *job_id, const
> char *device,
> on_target_error = BLOCKDEV_ON_ERROR_REPORT;
> }
>
> - blk = blk_by_name(device);
> - if (!blk) {
> - error_setg(errp, "Device '%s' not found", device);
> + bs = qmp_get_root_bs(device, errp);
> + if (!bs) {
> return;
> }
>
> - aio_context = blk_get_aio_context(blk);
> + aio_context = bdrv_get_aio_context(bs);
> aio_context_acquire(aio_context);
>
> - if (!blk_is_available(blk)) {
> - error_setg(errp, "Device '%s' has no medium", device);
> - goto out;
> - }
> - bs = blk_bs(blk);
> -
> target_bs = bdrv_lookup_bs(target, target, errp);
> if (!target_bs) {
> goto out;
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 61f8fd7..728b5f7 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -921,7 +921,7 @@
> # @job-id: #optional identifier for the newly-created block job. If
> # omitted, the device name will be used. (Since 2.7)
> #
> -# @device: the name of the device which should be copied.
> +# @device: the device name or node-name of a root node which should be
> copied.
> #
> # @target: the name of the backup target device.
> #
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 047b6f1..e7608f2 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1288,7 +1288,7 @@ Arguments:
>
> - "job-id": Identifier for the newly-created block job. If omitted,
> the device name will be used. (json-string, optional)
> -- "device": the name of the device which should be copied.
> +- "device": the device name or node-name of a root node which should be
> copied.
> (json-string)
> - "target": the name of the backup target device. (json-string)
> - "sync": what parts of the disk image should be copied to the destination;
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v4 00/11] block: Accept node-name in all node level QMP commands, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 02/11] block: Accept node-name for block-commit, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 04/11] block: Accept node-name for blockdev-mirror, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 03/11] block: Accept node-name for blockdev-backup, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 05/11] block: Accept node-name for blockdev-snapshot-delete-internal-sync, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 07/11] block: Accept node-name for change-backing-file, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 08/11] block: Accept node-name for drive-backup, Kevin Wolf, 2016/07/14
- [Qemu-devel] [PATCH v4 06/11] block: Accept node-name for blockdev-snapshot-internal-sync, Kevin Wolf, 2016/07/14