From 140b1324e0966a429c53ec4550117513384b4dec Mon Sep 17 00:00:00 2001 Message-Id: <140b1324e0966a429c53ec4550117513384b4dec.1334770230.git.minovotn@redhat.com> In-Reply-To: <5e4659718c6d6ee9ab11b269d929a292a71b3ab0.1334770230.git.minovotn@redhat.com> References: <5e4659718c6d6ee9ab11b269d929a292a71b3ab0.1334770230.git.minovotn@redhat.com> From: Paolo Bonzini Date: Fri, 13 Apr 2012 16:27:25 +0200 Subject: [PATCH 17/18] block: rewrite drive-mirror for mirror job RH-Author: Paolo Bonzini Message-id: <1334334446-31987-16-git-send-email-pbonzini@redhat.com> Patchwork-id: 39227 O-Subject: [RHEL 6.3 qemu-kvm PATCH 15/16] block: rewrite drive-mirror for mirror job Bugzilla: 806432 RH-Acked-by: Kevin Wolf RH-Acked-by: Laszlo Ersek RH-Acked-by: Jeffrey Cody Bugzilla: 806432 Upstream status: submitted as part of the mirroring forward-port, with non-transactionable drive-mirror. This *is* ugly, which is why upstream I didn't make drive-mirror a transactionable command. --- blockdev.c | 148 ++++++++++++++++++++++++++++++++++++++---------------- hmp.c | 7 ++- qapi-schema.json | 14 ++++-- qemu-monitor.hx | 9 +++- trace-events | 2 +- 5 files changed, 125 insertions(+), 55 deletions(-) Signed-off-by: Michal Novotny --- blockdev.c | 148 ++++++++++++++++++++++++++++++++++++++---------------- hmp.c | 7 +-- qapi-schema.json | 14 +++-- qemu-monitor.hx | 9 +++- trace-events | 2 +- 5 files changed, 125 insertions(+), 55 deletions(-) diff --git a/blockdev.c b/blockdev.c index 9fd29d7..ae8ee6c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -21,6 +21,7 @@ struct drivelist drives = QTAILQ_HEAD_INITIALIZER(drives); DriveInfo *extboot_drive = NULL; +static void block_job_cb(void *opaque, int ret); static const char *const if_name[IF_COUNT] = { [IF_NONE] = "none", @@ -801,6 +802,7 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file, #ifdef CONFIG_LIVE_SNAPSHOTS void qmp___com_redhat_drive_mirror(const char *device, const char *target, bool has_format, const char *format, + bool has_full, bool full, bool has_mode, enum NewImageMode mode, Error **errp) { BlockdevMirror mirror = { @@ -810,12 +812,15 @@ void qmp___com_redhat_drive_mirror(const char *device, const char *target, .format = (char *) format, .has_mode = has_mode, .mode = mode, + .has_full = has_full, + .full = full, }; blockdev_do_action(BLOCKDEV_ACTION_KIND___COM_REDHAT_DRIVE_MIRROR, &mirror, errp); } /* New and old BlockDriverState structs for group snapshots */ typedef struct BlkTransactionStates { + enum BlockdevActionKind kind; BlockDriverState *old_bs; BlockDriverState *new_bs; QSIMPLEQ_ENTRY(BlkTransactionStates) entry; @@ -831,7 +836,6 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) int ret = 0; BlockdevActionList *dev_entry = dev_list; BlkTransactionStates *states, *next; - char *new_source = NULL; QSIMPLEQ_HEAD(snap_bdrv_states, BlkTransactionStates) snap_bdrv_states; QSIMPLEQ_INIT(&snap_bdrv_states); @@ -842,21 +846,23 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) /* We don't do anything in this loop that commits us to the snapshot */ while (NULL != dev_entry) { BlockdevAction *dev_info = NULL; + BlockDriverState *source; BlockDriver *proto_drv; - BlockDriver *target_drv; BlockDriver *drv = NULL; int flags; enum NewImageMode mode; const char *new_image_file; const char *device; - const char *format = "qcow2"; + const char *format = NULL; uint64_t size; + bool full; dev_info = dev_entry->value; dev_entry = dev_entry->next; states = g_malloc0(sizeof(BlkTransactionStates)); QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, states, entry); + states->kind = dev_info->kind; switch (dev_info->kind) { case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC: @@ -869,12 +875,12 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) format = dev_info->blockdev_snapshot_sync->format; } mode = dev_info->blockdev_snapshot_sync->mode; - new_source = g_strdup(new_image_file); + source = states->old_bs; + full = false; break; case BLOCKDEV_ACTION_KIND___COM_REDHAT_DRIVE_MIRROR: device = dev_info->__com_redhat_drive_mirror->device; - drv = bdrv_find_format("blkmirror"); if (!dev_info->__com_redhat_drive_mirror->has_mode) { dev_info->__com_redhat_drive_mirror->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; } @@ -883,21 +889,23 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) format = dev_info->__com_redhat_drive_mirror->format; } mode = dev_info->__com_redhat_drive_mirror->mode; - new_source = g_strdup_printf("blkmirror:%s:%s", format, - dev_info->__com_redhat_drive_mirror->target); + full = dev_info->__com_redhat_drive_mirror->has_full + && dev_info->__com_redhat_drive_mirror->full; break; default: abort(); } - target_drv = bdrv_find_format(format); - if (!target_drv) { - error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); - goto delete_and_fail; + if (!format && mode != NEW_IMAGE_MODE_EXISTING) { + format = "qcow2"; } - if (!drv) { - drv = target_drv; + if (format) { + drv = bdrv_find_format(format); + if (!drv) { + error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); + goto delete_and_fail; + } } states->old_bs = bdrv_find(device); @@ -916,16 +924,24 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) goto delete_and_fail; } - if (!bdrv_is_read_only(states->old_bs)) { - if (bdrv_flush(states->old_bs)) { - error_set(errp, QERR_IO_ERROR); - goto delete_and_fail; + if (dev_info->kind == BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) { + if (!bdrv_is_read_only(states->old_bs)) { + if (bdrv_flush(states->old_bs)) { + error_set(errp, QERR_IO_ERROR); + goto delete_and_fail; + } } - } + source = states->old_bs; + } else { + source = states->old_bs->backing_hd; + if (!source) { + full = true; + } + } flags = states->old_bs->open_flags; - proto_drv = bdrv_find_protocol(new_source); + proto_drv = bdrv_find_protocol(new_image_file); if (!proto_drv) { error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); goto delete_and_fail; @@ -936,26 +952,28 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) goto delete_and_fail; } - /* create new image w/backing file */ - switch (mode) { + if (full && mode != NEW_IMAGE_MODE_EXISTING) { + assert(format && drv); + bdrv_get_geometry(states->old_bs, &size); + size *= 512; + ret = bdrv_img_create(new_image_file, format, + NULL, NULL, NULL, size, flags); + } else { + /* create new image w/backing file */ + switch (mode) { case NEW_IMAGE_MODE_EXISTING: ret = 0; break; case NEW_IMAGE_MODE_ABSOLUTE_PATHS: ret = bdrv_img_create(new_image_file, format, - states->old_bs->filename, - states->old_bs->drv->format_name, + source->filename, + source->drv->format_name, NULL, -1, flags); break; - case NEW_IMAGE_MODE_NO_BACKING_FILE: - bdrv_get_geometry(states->old_bs, &size); - size *= 512; - ret = bdrv_img_create(new_image_file, format, - NULL, NULL, NULL, size, flags); - break; default: ret = -1; break; + } } if (ret) { @@ -964,23 +982,53 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) } /* We will manually add the backing_hd field to the bs later */ - states->new_bs = bdrv_new(""); - ret = bdrv_open(states->new_bs, new_source, - flags | BDRV_O_NO_BACKING, drv); + switch (dev_info->kind) { + case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC: + states->new_bs = bdrv_new(""); + ret = bdrv_open(states->new_bs, new_image_file, + flags | BDRV_O_NO_BACKING, drv); + break; + + case BLOCKDEV_ACTION_KIND___COM_REDHAT_DRIVE_MIRROR: + /* Grab a reference so hotplug does not delete the BlockDriverState + * from underneath us. + */ + drive_get_ref(drive_get_by_blockdev(states->old_bs)); + ret = mirror_start(states->old_bs, new_image_file, drv, flags, + block_job_cb, states->old_bs, full); + if (ret == 0) { + /* A marker for the abort action */ + states->new_bs = states->old_bs; + } + break; + + default: + abort(); + } + if (ret != 0) { - error_set(errp, QERR_OPEN_FILE_FAILED, new_source); + error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file); goto delete_and_fail; } - g_free(new_source); - new_source = NULL; } /* Now we are going to do the actual pivot. Everything up to this point * is reversible, but we are committed at this point */ QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) { - /* This removes our old bs from the bdrv_states, and adds the new bs */ - bdrv_append(states->new_bs, states->old_bs); + switch (states->kind) { + case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC: + /* This removes our old bs from the bdrv_states, and adds the new bs */ + bdrv_append(states->new_bs, states->old_bs); + break; + + case BLOCKDEV_ACTION_KIND___COM_REDHAT_DRIVE_MIRROR: + mirror_commit(states->old_bs); + break; + + default: + abort(); + } } /* success */ @@ -992,15 +1040,29 @@ delete_and_fail: * the original bs for all images */ QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) { - if (states->new_bs) { - bdrv_delete(states->new_bs); + switch (states->kind) { + case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC: + if (states->new_bs) { + bdrv_delete(states->new_bs); + } + break; + + case BLOCKDEV_ACTION_KIND___COM_REDHAT_DRIVE_MIRROR: + /* This will still invoke the callback and release the + * reference. */ + if (states->new_bs) { + mirror_abort(states->old_bs); + } + break; + + default: + abort(); } } exit: QSIMPLEQ_FOREACH_SAFE(states, &snap_bdrv_states, entry, next) { g_free(states); } - g_free(new_source); return; } #endif @@ -1183,12 +1245,12 @@ static QObject *qobject_from_block_job(BlockJob *job) job->speed); } -static void block_stream_cb(void *opaque, int ret) +static void block_job_cb(void *opaque, int ret) { BlockDriverState *bs = opaque; QObject *obj; - trace_block_stream_cb(bs, bs->job, ret); + trace_block_job_cb(bs, bs->job, ret); assert(bs->job); obj = qobject_from_block_job(bs->job); @@ -1229,7 +1291,7 @@ int do_block_stream(Monitor *mon, const QDict *params, QObject **ret_data) } } - ret = stream_start(bs, base_bs, base, block_stream_cb, bs); + ret = stream_start(bs, base_bs, base, block_job_cb, bs); if (ret < 0) { switch (ret) { case -EBUSY: diff --git a/hmp.c b/hmp.c index b9e0ad3..c5757d3 100644 --- a/hmp.c +++ b/hmp.c @@ -29,7 +29,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) const char *filename = qdict_get_try_str(qdict, "target"); const char *format = qdict_get_try_str(qdict, "format"); int reuse = qdict_get_try_bool(qdict, "reuse", 0); - int no_backing = qdict_get_try_bool(qdict, "no-backing", 0); + int full = qdict_get_try_bool(qdict, "full", 0); enum NewImageMode mode; Error *errp = NULL; @@ -41,13 +41,12 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) if (reuse) { mode = NEW_IMAGE_MODE_EXISTING; - } else if (no_backing) { - mode = NEW_IMAGE_MODE_NO_BACKING_FILE; } else { mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; } - qmp___com_redhat_drive_mirror(device, filename, !!format, format, true, mode, &errp); + qmp___com_redhat_drive_mirror(device, filename, !!format, format, + true, full, true, mode, &errp); hmp_handle_error(mon, &errp); } diff --git a/qapi-schema.json b/qapi-schema.json index 5e939dd..22f770c 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -14,12 +14,10 @@ # @absolute-paths: QEMU should create a new image with absolute paths # for the backing file. # -# @no-backing-file: QEMU should create a new image with no backing file. -# # Since: 1.1 ## { 'enum': 'NewImageMode' - 'data': [ 'existing', 'absolute-paths', 'no-backing-file' ] } + 'data': [ 'existing', 'absolute-paths' ] } ## # @BlockdevSnapshot @@ -49,10 +47,13 @@ # # @mode: #optional whether and how QEMU should create a new image, default is # 'absolute-paths'. +# +# @full: whether the whole disk should be copied to the destination, or +# only the topmost image. ## { 'type': 'BlockdevMirror', 'data': { 'device': 'str', 'target': 'str', '*format': 'str', - '*mode': 'NewImageMode' } } + '*full': 'bool', '*mode': 'NewImageMode' } } ## # @BlockdevAction @@ -166,6 +167,9 @@ # @mode: #optional whether and how QEMU should create a new image, default is # 'absolute-paths'. # +# @full: whether the whole disk should be copied to the destination, or +# only the topmost image. +# # Returns: nothing on success # If @device is not a valid block device, DeviceNotFound # If @target can't be opened, OpenFileFailed @@ -175,5 +179,5 @@ ## { 'command': '__com.redhat_drive-mirror', 'data': { 'device': 'str', 'target': 'str', '*format': 'str', - '*mode': 'NewImageMode'} } + '*full': 'bool', '*mode': 'NewImageMode'} } #endif diff --git a/qemu-monitor.hx b/qemu-monitor.hx index bafd0ae..8889e6f 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -1533,6 +1533,8 @@ actions array: - "device": device name to snapshot (json-string) - "target": name of destination image file (json-string) - "format": format of new image (json-string, optional) + - "full": whether the mirror should include all images or + just the topmost (json-bool) - "mode": how QEMU should look for an existing image file (NewImageMode, optional, default "absolute-paths") @@ -1577,6 +1579,8 @@ Arguments: - "snapshot-file": name of new image file (json-string) - "mode": whether and how QEMU should create the snapshot file (NewImageMode, optional, default "absolute-paths") +- "full": whether the whole disk should be copied to the destination, + or only the topmost image (json-bool) - "format": format of new image (json-string, optional) Example: @@ -1584,6 +1588,7 @@ Example: -> { "execute": "blockdev-snapshot", "arguments": { "device": "ide-hd0", "snapshot-file": "/some/place/my-image", + "full": false, "format": "qcow2" } } <- { "return": {} } @@ -1592,7 +1597,7 @@ EQMP #ifdef CONFIG_LIVE_SNAPSHOTS { .name = "__com.redhat_drive-mirror", - .args_type = "device:B,target:s,mode:s?,format:s?", + .args_type = "full:-f,device:B,target:s,mode:s?,format:s?", .params = "device destination-image-file [mode] [format]", .user_print = monitor_user_noop, .mhandler.cmd_new = qmp_marshal_input___com_redhat_drive_mirror, @@ -1607,7 +1612,7 @@ Start mirroring a block device's writes to a new destination. target specifies the target of the new image. If the file exists, or if it is a device, it will be used as the new destination for writes. If does not exist, a new file will be created. format specifies the format of the -mirror image, default is qcow2. +mirror image, default is to probe if mode='existing', else qcow2. Arguments: diff --git a/trace-events b/trace-events index 5a616a8..9a9d234 100644 --- a/trace-events +++ b/trace-events @@ -73,7 +73,7 @@ disable stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs # blockdev.c disable do_block_job_cancel(void *job) "job %p" -disable block_stream_cb(void *bs, void *job, int ret) "bs %p job %p ret %d" +disable block_job_cb(void *bs, void *job, int ret) "bs %p job %p ret %d" disable do_block_stream(void *bs, void *job) "bs %p job %p" # hw/virtio-blk.c -- 1.7.7.6