125 lines
4.9 KiB
Diff
125 lines
4.9 KiB
Diff
From e56abd782be8bb41bb07c0317d008f95ec9a8ee5 Mon Sep 17 00:00:00 2001
|
|
From: Kevin Wolf <kwolf@redhat.com>
|
|
Date: Wed, 3 Jun 2020 16:03:20 +0100
|
|
Subject: [PATCH 21/26] backup: Make sure that source and target size match
|
|
|
|
RH-Author: Kevin Wolf <kwolf@redhat.com>
|
|
Message-id: <20200603160325.67506-7-kwolf@redhat.com>
|
|
Patchwork-id: 97107
|
|
O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH v2 06/11] backup: Make sure that source and target size match
|
|
Bugzilla: 1778593
|
|
RH-Acked-by: Eric Blake <eblake@redhat.com>
|
|
RH-Acked-by: Max Reitz <mreitz@redhat.com>
|
|
RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>
|
|
|
|
Since the introduction of a backup filter node in commit 00e30f05d, the
|
|
backup block job crashes when the target image is smaller than the
|
|
source image because it will try to write after the end of the target
|
|
node without having BLK_PERM_RESIZE. (Previously, the BlockBackend layer
|
|
would have caught this and errored out gracefully.)
|
|
|
|
We can fix this and even do better than the old behaviour: Check that
|
|
source and target have the same image size at the start of the block job
|
|
and unshare BLK_PERM_RESIZE. (This permission was already unshared
|
|
before the same commit 00e30f05d, but the BlockBackend that was used to
|
|
make the restriction was removed without a replacement.) This will
|
|
immediately error out when starting the job instead of only when writing
|
|
to a block that doesn't exist in the target.
|
|
|
|
Longer target than source would technically work because we would never
|
|
write to blocks that don't exist, but semantically these are invalid,
|
|
too, because a backup is supposed to create a copy, not just an image
|
|
that starts with a copy.
|
|
|
|
Fixes: 00e30f05de1d19586345ec373970ef4c192c6270
|
|
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1778593
|
|
Cc: qemu-stable@nongnu.org
|
|
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
Message-Id: <20200430142755.315494-4-kwolf@redhat.com>
|
|
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
|
|
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
(cherry picked from commit 958a04bd32af18d9a207bcc78046e56a202aebc2)
|
|
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
|
|
---
|
|
block/backup-top.c | 14 +++++++++-----
|
|
block/backup.c | 14 +++++++++++++-
|
|
2 files changed, 22 insertions(+), 6 deletions(-)
|
|
|
|
diff --git a/block/backup-top.c b/block/backup-top.c
|
|
index b8d863f..6756091 100644
|
|
--- a/block/backup-top.c
|
|
+++ b/block/backup-top.c
|
|
@@ -143,8 +143,10 @@ static void backup_top_child_perm(BlockDriverState *bs, BdrvChild *c,
|
|
*
|
|
* Share write to target (child_file), to not interfere
|
|
* with guest writes to its disk which may be in target backing chain.
|
|
+ * Can't resize during a backup block job because we check the size
|
|
+ * only upfront.
|
|
*/
|
|
- *nshared = BLK_PERM_ALL;
|
|
+ *nshared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
|
|
*nperm = BLK_PERM_WRITE;
|
|
} else {
|
|
/* Source child */
|
|
@@ -154,7 +156,7 @@ static void backup_top_child_perm(BlockDriverState *bs, BdrvChild *c,
|
|
if (perm & BLK_PERM_WRITE) {
|
|
*nperm = *nperm | BLK_PERM_CONSISTENT_READ;
|
|
}
|
|
- *nshared &= ~BLK_PERM_WRITE;
|
|
+ *nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
|
|
}
|
|
}
|
|
|
|
@@ -187,10 +189,12 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
|
|
{
|
|
Error *local_err = NULL;
|
|
BDRVBackupTopState *state;
|
|
- BlockDriverState *top = bdrv_new_open_driver(&bdrv_backup_top_filter,
|
|
- filter_node_name,
|
|
- BDRV_O_RDWR, errp);
|
|
+ BlockDriverState *top;
|
|
+
|
|
+ assert(source->total_sectors == target->total_sectors);
|
|
|
|
+ top = bdrv_new_open_driver(&bdrv_backup_top_filter, filter_node_name,
|
|
+ BDRV_O_RDWR, errp);
|
|
if (!top) {
|
|
return NULL;
|
|
}
|
|
diff --git a/block/backup.c b/block/backup.c
|
|
index 7c6ddd2..821c9fb 100644
|
|
--- a/block/backup.c
|
|
+++ b/block/backup.c
|
|
@@ -348,7 +348,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
|
|
BlockCompletionFunc *cb, void *opaque,
|
|
JobTxn *txn, Error **errp)
|
|
{
|
|
- int64_t len;
|
|
+ int64_t len, target_len;
|
|
BackupBlockJob *job = NULL;
|
|
int64_t cluster_size;
|
|
BdrvRequestFlags write_flags;
|
|
@@ -413,6 +413,18 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
|
|
goto error;
|
|
}
|
|
|
|
+ target_len = bdrv_getlength(target);
|
|
+ if (target_len < 0) {
|
|
+ error_setg_errno(errp, -target_len, "Unable to get length for '%s'",
|
|
+ bdrv_get_device_or_node_name(bs));
|
|
+ goto error;
|
|
+ }
|
|
+
|
|
+ if (target_len != len) {
|
|
+ error_setg(errp, "Source and target image have different sizes");
|
|
+ goto error;
|
|
+ }
|
|
+
|
|
cluster_size = backup_calculate_cluster_size(target, errp);
|
|
if (cluster_size < 0) {
|
|
goto error;
|
|
--
|
|
1.8.3.1
|
|
|