From patchwork Mon Feb 29 20:08:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Cody X-Patchwork-Id: 590151 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id F1C44140783 for ; Tue, 1 Mar 2016 07:11:53 +1100 (AEDT) Received: from localhost ([::1]:38905 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaUAZ-0003pN-Tc for incoming@patchwork.ozlabs.org; Mon, 29 Feb 2016 15:11:51 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52956) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaU8B-0007jh-Jv for qemu-devel@nongnu.org; Mon, 29 Feb 2016 15:09:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaU8A-0006Qa-HW for qemu-devel@nongnu.org; Mon, 29 Feb 2016 15:09:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38889) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaU84-0006Nr-Eo; Mon, 29 Feb 2016 15:09:16 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 1218963178; Mon, 29 Feb 2016 20:09:16 +0000 (UTC) Received: from localhost (ovpn-112-65.phx2.redhat.com [10.3.112.65]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1TK9EPE008125 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO); Mon, 29 Feb 2016 15:09:15 -0500 From: Jeff Cody To: qemu-block@nongnu.org Date: Mon, 29 Feb 2016 15:08:51 -0500 Message-Id: <1456776532-1008-12-git-send-email-jcody@redhat.com> In-Reply-To: <1456776532-1008-1-git-send-email-jcody@redhat.com> References: <1456776532-1008-1-git-send-email-jcody@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 29 Feb 2016 20:09:16 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Cc: peter.maydell@linaro.org, jcody@redhat.com, famz@redhat.com, qemu-devel@nongnu.org Subject: [Qemu-devel] [PULL 11/12] block/backup: avoid copying less than full target clusters X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: John Snow During incremental backups, if the target has a cluster size that is larger than the backup cluster size and we are backing up to a target that cannot (for whichever reason) pull clusters up from a backing image, we may inadvertantly create unusable incremental backup images. For example: If the bitmap tracks changes at a 64KB granularity and we transmit 64KB of data at a time but the target uses a 128KB cluster size, it is possible that only half of a target cluster will be recognized as dirty by the backup block job. When the cluster is allocated on the target image but only half populated with data, we lose the ability to distinguish between zero padding and uninitialized data. This does not happen if the target image has a backing file that points to the last known good backup. Even if we have a backing file, though, it's likely going to be faster to just buffer the redundant data ourselves from the live image than fetching it from the backing file, so let's just always round up to the target granularity. The same logic applies to backup modes top, none, and full. Copying fractional clusters without the guarantee of COW is dangerous, but even if we can rely on COW, it's likely better to just re-copy the data. Reported-by: Fam Zheng Signed-off-by: John Snow Reviewed-by: Fam Zheng Message-id: 1456433911-24718-3-git-send-email-jsnow@redhat.com Signed-off-by: Jeff Cody --- block/backup.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/block/backup.c b/block/backup.c index 76addef..0f1b1bc 100644 --- a/block/backup.c +++ b/block/backup.c @@ -501,6 +501,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, BlockJobTxn *txn, Error **errp) { int64_t len; + BlockDriverInfo bdi; + int ret; assert(bs); assert(target); @@ -570,15 +572,32 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, goto error; } - bdrv_op_block_all(target, job->common.blocker); - job->on_source_error = on_source_error; job->on_target_error = on_target_error; job->target = target; job->sync_mode = sync_mode; job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ? sync_bitmap : NULL; - job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT; + + /* If there is no backing file on the target, we cannot rely on COW if our + * backup cluster size is smaller than the target cluster size. Even for + * targets with a backing file, try to avoid COW if possible. */ + ret = bdrv_get_info(job->target, &bdi); + if (ret < 0 && !target->backing) { + error_setg_errno(errp, -ret, + "Couldn't determine the cluster size of the target image, " + "which has no backing file"); + error_append_hint(errp, + "Aborting, since this may create an unusable destination image\n"); + goto error; + } else if (ret < 0 && target->backing) { + /* Not fatal; just trudge on ahead. */ + job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT; + } else { + job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size); + } + + bdrv_op_block_all(target, job->common.blocker); job->common.len = len; job->common.co = qemu_coroutine_create(backup_run); block_job_txn_add_job(txn, &job->common);