From: "Denis V. Lunev" <den@openvz.org>
To: qemu-block@nongnu.org, qemu-devel@nongnu.org
Cc: den@openvz.org, Pavel Butsykin <pbutsykin@virtuozzo.com>,
Jeff Cody <jcody@redhat.com>,
Markus Armbruster <armbru@redhat.com>,
Eric Blake <eblake@redhat.com>, John Snow <jsnow@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
Kevin Wolf <kwolf@redhat.com>
Subject: [Qemu-devel] [PATCH v6 11/16] block: simplify drive-backup
Date: Thu, 21 Jul 2016 22:40:14 +0300 [thread overview]
Message-ID: <1469130019-18497-12-git-send-email-den@openvz.org> (raw)
In-Reply-To: <1469130019-18497-1-git-send-email-den@openvz.org>
From: Pavel Butsykin <pbutsykin@virtuozzo.com>
Now that we can support boxed commands, use it to greatly reduce the
number of parameters (and likelihood of getting out of sync) when
adjusting drive-backup parameters..
Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 115 +++++++++++++++++----------------------------------
hmp.c | 29 ++++++++-----
qapi/block-core.json | 3 +-
3 files changed, 58 insertions(+), 89 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index eafeba9..e29147a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1838,17 +1838,8 @@ typedef struct DriveBackupState {
BlockJob *job;
} DriveBackupState;
-static void do_drive_backup(const char *job_id, const char *device,
- const char *target, bool has_format,
- const char *format, enum MirrorSyncMode sync,
- bool has_mode, enum NewImageMode mode,
- bool has_speed, int64_t speed,
- bool has_bitmap, const char *bitmap,
- bool has_on_source_error,
- BlockdevOnError on_source_error,
- bool has_on_target_error,
- BlockdevOnError on_target_error,
- BlockJobTxn *txn, Error **errp);
+static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
+ Error **errp);
static void drive_backup_prepare(BlkActionState *common, Error **errp)
{
@@ -1878,16 +1869,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
bdrv_drained_begin(blk_bs(blk));
state->bs = blk_bs(blk);
- do_drive_backup(backup->has_job_id ? backup->job_id : NULL,
- backup->device, backup->target,
- backup->has_format, backup->format,
- backup->sync,
- backup->has_mode, backup->mode,
- backup->has_speed, backup->speed,
- backup->has_bitmap, backup->bitmap,
- backup->has_on_source_error, backup->on_source_error,
- backup->has_on_target_error, backup->on_target_error,
- common->block_job_txn, &local_err);
+ do_drive_backup(backup, common->block_job_txn, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -3155,17 +3137,7 @@ out:
aio_context_release(aio_context);
}
-static void do_drive_backup(const char *job_id, const char *device,
- const char *target, bool has_format,
- const char *format, enum MirrorSyncMode sync,
- bool has_mode, enum NewImageMode mode,
- bool has_speed, int64_t speed,
- bool has_bitmap, const char *bitmap,
- bool has_on_source_error,
- BlockdevOnError on_source_error,
- bool has_on_target_error,
- BlockdevOnError on_target_error,
- BlockJobTxn *txn, Error **errp)
+static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp)
{
BlockBackend *blk;
BlockDriverState *bs;
@@ -3178,23 +3150,26 @@ static void do_drive_backup(const char *job_id, const char *device,
int flags;
int64_t size;
- if (!has_speed) {
- speed = 0;
+ if (!backup->has_speed) {
+ backup->speed = 0;
}
- if (!has_on_source_error) {
- on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+ if (!backup->has_on_source_error) {
+ backup->on_source_error = BLOCKDEV_ON_ERROR_REPORT;
}
- if (!has_on_target_error) {
- on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+ if (!backup->has_on_target_error) {
+ backup->on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+ if (!backup->has_mode) {
+ backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
}
- if (!has_mode) {
- mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+ if (!backup->has_job_id) {
+ backup->job_id = NULL;
}
- blk = blk_by_name(device);
+ blk = blk_by_name(backup->device);
if (!blk) {
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
- "Device '%s' not found", device);
+ "Device '%s' not found", backup->device);
return;
}
@@ -3204,13 +3179,14 @@ static void do_drive_backup(const char *job_id, const char *device,
/* 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);
+ error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, backup->device);
goto out;
}
bs = blk_bs(blk);
- if (!has_format) {
- format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
+ if (!backup->has_format) {
+ backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
+ NULL : (char *)bs->drv->format_name;
}
/* Early check to avoid creating target */
@@ -3222,13 +3198,13 @@ static void do_drive_backup(const char *job_id, const char *device,
/* See if we have a backing HD we can use to create our new image
* on top of. */
- if (sync == MIRROR_SYNC_MODE_TOP) {
+ if (backup->sync == MIRROR_SYNC_MODE_TOP) {
source = backing_bs(bs);
if (!source) {
- sync = MIRROR_SYNC_MODE_FULL;
+ backup->sync = MIRROR_SYNC_MODE_FULL;
}
}
- if (sync == MIRROR_SYNC_MODE_NONE) {
+ if (backup->sync == MIRROR_SYNC_MODE_NONE) {
source = bs;
}
@@ -3238,14 +3214,14 @@ static void do_drive_backup(const char *job_id, const char *device,
goto out;
}
- if (mode != NEW_IMAGE_MODE_EXISTING) {
- assert(format);
+ if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
+ assert(backup->format);
if (source) {
- bdrv_img_create(target, format, source->filename,
+ bdrv_img_create(backup->target, backup->format, source->filename,
source->drv->format_name, NULL,
size, flags, &local_err, false);
} else {
- bdrv_img_create(target, format, NULL, NULL, NULL,
+ bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL,
size, flags, &local_err, false);
}
}
@@ -3255,29 +3231,29 @@ static void do_drive_backup(const char *job_id, const char *device,
goto out;
}
- if (format) {
+ if (backup->format) {
options = qdict_new();
- qdict_put(options, "driver", qstring_from_str(format));
+ qdict_put(options, "driver", qstring_from_str(backup->format));
}
- target_bs = bdrv_open(target, NULL, options, flags, errp);
+ target_bs = bdrv_open(backup->target, NULL, options, flags, errp);
if (!target_bs) {
goto out;
}
bdrv_set_aio_context(target_bs, aio_context);
- if (has_bitmap) {
- bmap = bdrv_find_dirty_bitmap(bs, bitmap);
+ if (backup->has_bitmap) {
+ bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
if (!bmap) {
- error_setg(errp, "Bitmap '%s' could not be found", bitmap);
+ error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
bdrv_unref(target_bs);
goto out;
}
}
- backup_start(job_id, bs, target_bs, speed, sync, bmap,
- on_source_error, on_target_error,
+ backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync,
+ bmap, backup->on_source_error, backup->on_target_error,
block_job_cb, bs, txn, &local_err);
bdrv_unref(target_bs);
if (local_err != NULL) {
@@ -3289,24 +3265,9 @@ out:
aio_context_release(aio_context);
}
-void qmp_drive_backup(bool has_job_id, const char *job_id,
- const char *device, const char *target,
- bool has_format, const char *format,
- enum MirrorSyncMode sync,
- bool has_mode, enum NewImageMode mode,
- bool has_speed, int64_t speed,
- bool has_bitmap, const char *bitmap,
- bool has_on_source_error, BlockdevOnError on_source_error,
- bool has_on_target_error, BlockdevOnError on_target_error,
- Error **errp)
+void qmp_drive_backup(DriveBackup *arg, Error **errp)
{
- return do_drive_backup(has_job_id ? job_id : NULL, device, target,
- has_format, format, sync,
- has_mode, mode, has_speed, speed,
- has_bitmap, bitmap,
- has_on_source_error, on_source_error,
- has_on_target_error, on_target_error,
- NULL, errp);
+ return do_drive_backup(arg, NULL, errp);
}
BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
diff --git a/hmp.c b/hmp.c
index cc2056e..9ac49b2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1109,8 +1109,24 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
const char *format = qdict_get_try_str(qdict, "format");
bool reuse = qdict_get_try_bool(qdict, "reuse", false);
bool full = qdict_get_try_bool(qdict, "full", false);
- enum NewImageMode mode;
Error *err = NULL;
+ DriveBackup backup = {
+ .device = (char *)device,
+ .target = (char *)filename,
+ .has_format = !!format,
+ .format = (char *)format,
+ .sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
+ .has_mode = true,
+ .mode = reuse ? NEW_IMAGE_MODE_EXISTING : NEW_IMAGE_MODE_ABSOLUTE_PATHS,
+ .has_speed = false,
+ .speed = 0,
+ .has_bitmap = false,
+ .bitmap = NULL,
+ .has_on_source_error = false,
+ .on_source_error = 0,
+ .has_on_target_error = false,
+ .on_target_error = 0,
+ };
if (!filename) {
error_setg(&err, QERR_MISSING_PARAMETER, "target");
@@ -1118,16 +1134,7 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
return;
}
- if (reuse) {
- mode = NEW_IMAGE_MODE_EXISTING;
- } else {
- mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
- }
-
- qmp_drive_backup(false, NULL, device, filename, !!format, format,
- full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
- true, mode, false, 0, false, NULL,
- false, 0, false, 0, &err);
+ qmp_drive_backup(&backup, &err);
hmp_handle_error(mon, &err);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f462345..121a540 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1086,7 +1086,8 @@
#
# Since 1.6
##
-{ 'command': 'drive-backup', 'data': 'DriveBackup' }
+{ 'command': 'drive-backup', 'boxed': true,
+ 'data': 'DriveBackup' }
##
# @blockdev-backup
--
2.5.0
next prev parent reply other threads:[~2016-07-21 19:40 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 01/16] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
2016-07-21 23:08 ` Eric Blake
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 02/16] block: Convert bdrv_pwrite_compressed() to BdrvChild Denis V. Lunev
2016-07-21 23:10 ` Eric Blake
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 03/16] block/io: reuse bdrv_co_pwritev() for write compressed Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 04/16] qcow2: add qcow2_co_pwritev_compressed Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 05/16] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 06/16] vmdk: add vmdk_co_pwritev_compressed Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 07/16] qcow: add qcow_co_pwritev_compressed Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 08/16] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 09/16] block: remove BlockDriver.bdrv_write_compressed Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 10/16] block/io: turn on dirty_bitmaps for the compressed writes Denis V. Lunev
2016-07-21 19:40 ` Denis V. Lunev [this message]
2016-07-21 23:21 ` [Qemu-devel] [PATCH v6 11/16] block: simplify drive-backup Eric Blake
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 12/16] block: simplify blockdev-backup Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 13/16] drive-backup: added support for data compression Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 14/16] blockdev-backup: " Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 15/16] qemu-iotests: test backup compression in 055 Denis V. Lunev
2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 16/16] qemu-iotests: add vmdk for " Denis V. Lunev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1469130019-18497-12-git-send-email-den@openvz.org \
--to=den@openvz.org \
--cc=armbru@redhat.com \
--cc=eblake@redhat.com \
--cc=jcody@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=pbutsykin@virtuozzo.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.