[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 18/30] backup: Make sure that source and target size match
From: |
Kevin Wolf |
Subject: |
[PULL 18/30] backup: Make sure that source and target size match |
Date: |
Fri, 8 May 2020 14:41:23 +0200 |
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: address@hidden
Signed-off-by: Kevin Wolf <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
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 3b50c06e2c..79b268e6dc 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -148,8 +148,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 */
@@ -159,7 +161,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);
}
}
@@ -192,11 +194,13 @@ 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;
bool appended = false;
+ 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 c4c3b8cd46..4f13bb20a5 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -340,7 +340,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;
@@ -405,6 +405,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;
--
2.25.3
- [PULL 11/30] vmdk: Fix zero cluster allocation, (continued)
- [PULL 11/30] vmdk: Fix zero cluster allocation, Kevin Wolf, 2020/05/08
- [PULL 09/30] qcow2: Avoid integer wraparound in qcow2_co_truncate(), Kevin Wolf, 2020/05/08
- [PULL 14/30] vmdk: Flush only once in vmdk_L2update(), Kevin Wolf, 2020/05/08
- [PULL 16/30] iotests/283: Use consistent size for source and target, Kevin Wolf, 2020/05/08
- [PULL 15/30] iotests: vmdk: Enable zeroed_grained=on by default, Kevin Wolf, 2020/05/08
- [PULL 21/30] qcow2: Fix preallocation on block devices, Kevin Wolf, 2020/05/08
- [PULL 27/30] ssh: Support BDRV_REQ_ZERO_WRITE for truncate, Kevin Wolf, 2020/05/08
- [PULL 22/30] gluster: Drop useless has_zero_init callback, Kevin Wolf, 2020/05/08
- [PULL 17/30] backup: Improve error for bdrv_getlength() failure, Kevin Wolf, 2020/05/08
- [PULL 20/30] iotests/055: Use cache.no-flush for vmdk target, Kevin Wolf, 2020/05/08
- [PULL 18/30] backup: Make sure that source and target size match,
Kevin Wolf <=
- [PULL 29/30] vhdx: Rework truncation logic, Kevin Wolf, 2020/05/08
- [PULL 19/30] iotests: Backup with different source/target size, Kevin Wolf, 2020/05/08
- [PULL 24/30] nfs: Support BDRV_REQ_ZERO_WRITE for truncate, Kevin Wolf, 2020/05/08
- [PULL 25/30] rbd: Support BDRV_REQ_ZERO_WRITE for truncate, Kevin Wolf, 2020/05/08
- [PULL 23/30] file-win32: Support BDRV_REQ_ZERO_WRITE for truncate, Kevin Wolf, 2020/05/08
- [PULL 28/30] parallels: Rework truncation logic, Kevin Wolf, 2020/05/08
- [PULL 30/30] block: Drop unused .bdrv_has_zero_init_truncate, Kevin Wolf, 2020/05/08
- [PULL 26/30] sheepdog: Support BDRV_REQ_ZERO_WRITE for truncate, Kevin Wolf, 2020/05/08
- Re: [PULL 00/30] Block layer patches, Peter Maydell, 2020/05/08