From b812f6719c21921a819709098dc018ed151c999b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 21 Jan 2013 17:09:43 +0100 Subject: mirror: perform COW if the cluster size is bigger than the granularity When mirroring runs, the backing files for the target may not yet be ready. However, this means that a copy-on-write operation on the target would fill the missing sectors with zeros. Copy-on-write only happens if the granularity of the dirty bitmap is smaller than the cluster size (and only for clusters that are allocated in the source after the job has started copying). So far, the granularity was fixed to 1MB; to avoid the problem we detected the situation and required the backing files to be available in that case only. However, we want to lower the granularity for efficiency, so we need a better solution. The solution is to always copy a whole cluster the first time it is touched. The code keeps a bitmap of clusters that have already been allocated by the mirroring job, and only does "manual" copy-on-write if the chunk being copied is zero in the bitmap. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- blockdev.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 9126587c45..1eb62b637c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1197,7 +1197,6 @@ void qmp_drive_mirror(const char *device, const char *target, bool has_on_target_error, BlockdevOnError on_target_error, Error **errp) { - BlockDriverInfo bdi; BlockDriverState *bs; BlockDriverState *source, *target_bs; BlockDriver *proto_drv; @@ -1288,6 +1287,9 @@ void qmp_drive_mirror(const char *device, const char *target, return; } + /* Mirroring takes care of copy-on-write using the source's backing + * file. + */ target_bs = bdrv_new(""); ret = bdrv_open(target_bs, target, flags | BDRV_O_NO_BACKING, drv); @@ -1297,17 +1299,6 @@ void qmp_drive_mirror(const char *device, const char *target, return; } - /* We need a backing file if we will copy parts of a cluster. */ - if (bdrv_get_info(target_bs, &bdi) >= 0 && bdi.cluster_size != 0 && - bdi.cluster_size >= BDRV_SECTORS_PER_DIRTY_CHUNK * 512) { - ret = bdrv_open_backing_file(target_bs); - if (ret < 0) { - bdrv_delete(target_bs); - error_set(errp, QERR_OPEN_FILE_FAILED, target); - return; - } - } - mirror_start(bs, target_bs, speed, sync, on_source_error, on_target_error, block_job_cb, bs, &local_err); if (local_err != NULL) { -- cgit v1.2.3 From eee13dfe302833944d1176677d12a6ea421a94ea Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 21 Jan 2013 17:09:46 +0100 Subject: mirror: allow customizing the granularity The desired granularity may be very different depending on the kind of operation (e.g. continuous replication vs. collapse-to-raw) and whether the VM is expected to perform lots of I/O while mirroring is in progress. Allow the user to customize it, while providing a sane default so that in general there will be no extra allocated space in the target compared to the source. Reviewed-by: Eric Blake Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- blockdev.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 1eb62b637c..07fd3273ed 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1193,6 +1193,7 @@ void qmp_drive_mirror(const char *device, const char *target, enum MirrorSyncMode sync, bool has_mode, enum NewImageMode mode, bool has_speed, int64_t speed, + bool has_granularity, uint32_t granularity, bool has_on_source_error, BlockdevOnError on_source_error, bool has_on_target_error, BlockdevOnError on_target_error, Error **errp) @@ -1218,6 +1219,17 @@ void qmp_drive_mirror(const char *device, const char *target, if (!has_mode) { mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; } + if (!has_granularity) { + granularity = 0; + } + if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) { + error_set(errp, QERR_INVALID_PARAMETER, device); + return; + } + if (granularity & (granularity - 1)) { + error_set(errp, QERR_INVALID_PARAMETER, device); + return; + } bs = bdrv_find(device); if (!bs) { @@ -1299,7 +1311,8 @@ void qmp_drive_mirror(const char *device, const char *target, return; } - mirror_start(bs, target_bs, speed, sync, on_source_error, on_target_error, + mirror_start(bs, target_bs, speed, granularity, sync, + on_source_error, on_target_error, block_job_cb, bs, &local_err); if (local_err != NULL) { bdrv_delete(target_bs); -- cgit v1.2.3 From 08e4ed6cdeeee7912072cf14aa8ab6c60dacb4fb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 22 Jan 2013 09:03:13 +0100 Subject: mirror: add buf-size argument to drive-mirror This makes sense when the next commit starts using the extra buffer space to perform many I/O operations asynchronously. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- blockdev.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 07fd3273ed..ad25b9b86e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1188,12 +1188,15 @@ void qmp_block_commit(const char *device, drive_get_ref(drive_get_by_blockdev(bs)); } +#define DEFAULT_MIRROR_BUF_SIZE (10 << 20) + void qmp_drive_mirror(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_granularity, uint32_t granularity, + bool has_buf_size, int64_t buf_size, bool has_on_source_error, BlockdevOnError on_source_error, bool has_on_target_error, BlockdevOnError on_target_error, Error **errp) @@ -1222,6 +1225,10 @@ void qmp_drive_mirror(const char *device, const char *target, if (!has_granularity) { granularity = 0; } + if (!has_buf_size) { + buf_size = DEFAULT_MIRROR_BUF_SIZE; + } + if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) { error_set(errp, QERR_INVALID_PARAMETER, device); return; @@ -1311,7 +1318,7 @@ void qmp_drive_mirror(const char *device, const char *target, return; } - mirror_start(bs, target_bs, speed, granularity, sync, + mirror_start(bs, target_bs, speed, granularity, buf_size, sync, on_source_error, on_target_error, block_job_cb, bs, &local_err); if (local_err != NULL) { -- cgit v1.2.3 From 02582abd48aa3d860015e9a8fcd0d7ec1c34ec62 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Thu, 17 Jan 2013 21:45:24 +0100 Subject: block: Add special error code for wrong format The block drivers need a special error code for "wrong format". From the available error codes EMEDIUMTYPE fits best. It is not available on all platforms, so a definition in qemu-common.h and a specific error report are needed. Signed-off-by: Stefan Weil Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- blockdev.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index ad25b9b86e..ac396f3c94 100644 --- a/blockdev.c +++ b/blockdev.c @@ -617,8 +617,13 @@ DriveInfo *drive_init(QemuOpts *opts, BlockInterfaceType block_default_type) ret = bdrv_open(dinfo->bdrv, file, bdrv_flags, drv); if (ret < 0) { - error_report("could not open disk image %s: %s", - file, strerror(-ret)); + if (ret == -EMEDIUMTYPE) { + error_report("could not open disk image %s: not in %s format", + file, drv->format_name); + } else { + error_report("could not open disk image %s: %s", + file, strerror(-ret)); + } goto err; } -- cgit v1.2.3 From 8689907266b649b757c2203d9652cbe928a3ae0b Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 24 Jan 2013 10:00:40 -0800 Subject: block: Create proper size file for disk mirror The qmp monitor command to mirror a disk was passing -1 for size along with the disk's backing file. This size of the resulting disk is the size of the backing file, which is incorrect if the disk has been resized. Therefore we should always pass in the size of the current disk. Signed-off-by: Vishvananda Ishaya Signed-off-by: Kevin Wolf --- blockdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index ac396f3c94..fdc573f519 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1282,11 +1282,11 @@ void qmp_drive_mirror(const char *device, const char *target, return; } + bdrv_get_geometry(bs, &size); + size *= 512; if (sync == MIRROR_SYNC_MODE_FULL && mode != NEW_IMAGE_MODE_EXISTING) { /* create new image w/o backing file */ assert(format && drv); - bdrv_get_geometry(bs, &size); - size *= 512; bdrv_img_create(target, format, NULL, NULL, NULL, size, flags, &local_err); } else { @@ -1299,7 +1299,7 @@ void qmp_drive_mirror(const char *device, const char *target, bdrv_img_create(target, format, source->filename, source->drv->format_name, - NULL, -1, flags, &local_err); + NULL, size, flags, &local_err); break; default: abort(); -- cgit v1.2.3