block: Accept node-name for drive-backup
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts drive-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 <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
parent
7b5dca3f02
commit
b7e4fa2242
36
blockdev.c
36
blockdev.c
@ -1856,30 +1856,23 @@ static void do_drive_backup(const char *job_id, const char *device,
|
|||||||
static void drive_backup_prepare(BlkActionState *common, Error **errp)
|
static void drive_backup_prepare(BlkActionState *common, Error **errp)
|
||||||
{
|
{
|
||||||
DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
|
DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
|
||||||
BlockBackend *blk;
|
BlockDriverState *bs;
|
||||||
DriveBackup *backup;
|
DriveBackup *backup;
|
||||||
Error *local_err = NULL;
|
Error *local_err = NULL;
|
||||||
|
|
||||||
assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
|
assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
|
||||||
backup = common->action->u.drive_backup.data;
|
backup = common->action->u.drive_backup.data;
|
||||||
|
|
||||||
blk = blk_by_name(backup->device);
|
bs = qmp_get_root_bs(backup->device, errp);
|
||||||
if (!blk) {
|
if (!bs) {
|
||||||
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
|
|
||||||
"Device '%s' not found", backup->device);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!blk_is_available(blk)) {
|
|
||||||
error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, backup->device);
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* AioContext is released in .clean() */
|
/* AioContext is released in .clean() */
|
||||||
state->aio_context = blk_get_aio_context(blk);
|
state->aio_context = bdrv_get_aio_context(bs);
|
||||||
aio_context_acquire(state->aio_context);
|
aio_context_acquire(state->aio_context);
|
||||||
bdrv_drained_begin(blk_bs(blk));
|
bdrv_drained_begin(bs);
|
||||||
state->bs = blk_bs(blk);
|
state->bs = bs;
|
||||||
|
|
||||||
do_drive_backup(backup->has_job_id ? backup->job_id : NULL,
|
do_drive_backup(backup->has_job_id ? backup->job_id : NULL,
|
||||||
backup->device, backup->target,
|
backup->device, backup->target,
|
||||||
@ -3153,7 +3146,6 @@ static void do_drive_backup(const char *job_id, const char *device,
|
|||||||
BlockdevOnError on_target_error,
|
BlockdevOnError on_target_error,
|
||||||
BlockJobTxn *txn, Error **errp)
|
BlockJobTxn *txn, Error **errp)
|
||||||
{
|
{
|
||||||
BlockBackend *blk;
|
|
||||||
BlockDriverState *bs;
|
BlockDriverState *bs;
|
||||||
BlockDriverState *target_bs;
|
BlockDriverState *target_bs;
|
||||||
BlockDriverState *source = NULL;
|
BlockDriverState *source = NULL;
|
||||||
@ -3177,24 +3169,14 @@ static void do_drive_backup(const char *job_id, const char *device,
|
|||||||
mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
|
mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
|
||||||
}
|
}
|
||||||
|
|
||||||
blk = blk_by_name(device);
|
bs = qmp_get_root_bs(device, errp);
|
||||||
if (!blk) {
|
if (!bs) {
|
||||||
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
|
|
||||||
"Device '%s' not found", device);
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
aio_context = blk_get_aio_context(blk);
|
aio_context = bdrv_get_aio_context(bs);
|
||||||
aio_context_acquire(aio_context);
|
aio_context_acquire(aio_context);
|
||||||
|
|
||||||
/* Although backup_run has this check too, we need to use bs->drv below, so
|
|
||||||
* do an early check redundantly. */
|
|
||||||
if (!blk_is_available(blk)) {
|
|
||||||
error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
bs = blk_bs(blk);
|
|
||||||
|
|
||||||
if (!has_format) {
|
if (!has_format) {
|
||||||
format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
|
format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
|
||||||
}
|
}
|
||||||
|
@ -876,7 +876,7 @@
|
|||||||
# @job-id: #optional identifier for the newly-created block job. If
|
# @job-id: #optional identifier for the newly-created block job. If
|
||||||
# omitted, the device name will be used. (Since 2.7)
|
# 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 target of the new image. If the file exists, or if it
|
# @target: the target of the new image. If the file exists, or if it
|
||||||
# is a device, the existing file/device will be used as the new
|
# is a device, the existing file/device will be used as the new
|
||||||
@ -1087,7 +1087,7 @@
|
|||||||
# For the arguments, see the documentation of DriveBackup.
|
# For the arguments, see the documentation of DriveBackup.
|
||||||
#
|
#
|
||||||
# Returns: nothing on success
|
# Returns: nothing on success
|
||||||
# If @device is not a valid block device, DeviceNotFound
|
# If @device is not a valid block device, GenericError
|
||||||
#
|
#
|
||||||
# Since 1.6
|
# Since 1.6
|
||||||
##
|
##
|
||||||
|
@ -1235,7 +1235,7 @@ Arguments:
|
|||||||
|
|
||||||
- "job-id": Identifier for the newly-created block job. If omitted,
|
- "job-id": Identifier for the newly-created block job. If omitted,
|
||||||
the device name will be used. (json-string, optional)
|
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)
|
(json-string)
|
||||||
- "target": the target of the new image. If the file exists, or if it is a
|
- "target": the target of the new image. If the file exists, or if it is a
|
||||||
device, the existing file/device will be used as the new
|
device, the existing file/device will be used as the new
|
||||||
|
@ -134,9 +134,6 @@ class TestSingleDrive(iotests.QMPTestCase):
|
|||||||
|
|
||||||
def do_test_device_not_found(self, cmd, **args):
|
def do_test_device_not_found(self, cmd, **args):
|
||||||
result = self.vm.qmp(cmd, **args)
|
result = self.vm.qmp(cmd, **args)
|
||||||
if cmd == 'drive-backup':
|
|
||||||
self.assert_qmp(result, 'error/class', 'DeviceNotFound')
|
|
||||||
else:
|
|
||||||
self.assert_qmp(result, 'error/class', 'GenericError')
|
self.assert_qmp(result, 'error/class', 'GenericError')
|
||||||
|
|
||||||
def test_device_not_found(self):
|
def test_device_not_found(self):
|
||||||
@ -371,7 +368,7 @@ class TestSingleTransaction(iotests.QMPTestCase):
|
|||||||
'sync': 'full' },
|
'sync': 'full' },
|
||||||
}
|
}
|
||||||
])
|
])
|
||||||
self.assert_qmp(result, 'error/class', 'DeviceNotFound')
|
self.assert_qmp(result, 'error/class', 'GenericError')
|
||||||
|
|
||||||
result = self.vm.qmp('transaction', actions=[{
|
result = self.vm.qmp('transaction', actions=[{
|
||||||
'type': 'blockdev-backup',
|
'type': 'blockdev-backup',
|
||||||
|
Loading…
Reference in New Issue
Block a user