From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:34070) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hL2HL-0002RJ-Hu for qemu-devel@nongnu.org; Mon, 29 Apr 2019 05:08:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hL2HJ-0003vW-Jv for qemu-devel@nongnu.org; Mon, 29 Apr 2019 05:08:51 -0400 From: Vladimir Sementsov-Ogievskiy Date: Mon, 29 Apr 2019 12:08:40 +0300 Message-Id: <20190429090842.57910-4-vsementsov@virtuozzo.com> In-Reply-To: <20190429090842.57910-1-vsementsov@virtuozzo.com> References: <20190429090842.57910-1-vsementsov@virtuozzo.com> Subject: [Qemu-devel] [PATCH v7 3/5] block/backup: refactor and tolerate unallocated cluster skipping List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: mreitz@redhat.com, kwolf@redhat.com, jsnow@redhat.com, vsementsov@virtuozzo.com, den@openvz.org Split allocation checking to separate function and reduce nesting. Consider bdrv_is_allocated() fail as allocated area, as copying more than needed is not wrong (and we do it anyway) and seems better than fail the whole job. And, most probably we will fail on the next read, if there are real problem with source. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/backup.c | 60 +++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/block/backup.c b/block/backup.c index 510fc54f98..298e85f1a9 100644 --- a/block/backup.c +++ b/block/backup.c @@ -377,6 +377,22 @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job) return false; } +static bool bdrv_is_unallocated_range(BlockDriverState *bs, + int64_t offset, int64_t bytes) +{ + int64_t end = offset + bytes; + + while (offset < end && !bdrv_is_allocated(bs, offset, bytes, &bytes)) { + if (bytes == 0) { + return true; + } + offset += bytes; + bytes = end - offset; + } + + return offset >= end; +} + static int coroutine_fn backup_run_incremental(BackupBlockJob *job) { int ret; @@ -462,49 +478,19 @@ static int coroutine_fn backup_run(Job *job, Error **errp) for (offset = 0; offset < s->len; offset += s->cluster_size) { bool error_is_read; - int alloced = 0; if (yield_and_check(s)) { break; } - if (s->sync_mode == MIRROR_SYNC_MODE_TOP) { - int i; - int64_t n; - - /* Check to see if these blocks are already in the - * backing file. */ - - for (i = 0; i < s->cluster_size;) { - /* bdrv_is_allocated() only returns true/false based - * on the first set of sectors it comes across that - * are are all in the same state. - * For that reason we must verify each sector in the - * backup cluster length. We end up copying more than - * needed but at some point that is always the case. */ - alloced = - bdrv_is_allocated(bs, offset + i, - s->cluster_size - i, &n); - i += n; - - if (alloced || n == 0) { - break; - } - } - - /* If the above loop never found any sectors that are in - * the topmost image, skip this backup. */ - if (alloced == 0) { - continue; - } - } - /* FULL sync mode we copy the whole drive. */ - if (alloced < 0) { - ret = alloced; - } else { - ret = backup_do_cow(s, offset, s->cluster_size, - &error_is_read, false); + if (s->sync_mode == MIRROR_SYNC_MODE_TOP && + bdrv_is_unallocated_range(bs, offset, s->cluster_size)) + { + continue; } + + ret = backup_do_cow(s, offset, s->cluster_size, + &error_is_read, false); if (ret < 0) { /* Depending on error action, fail now or retry cluster */ BlockErrorAction action = -- 2.18.0 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.9 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4F524C43219 for ; Mon, 29 Apr 2019 09:18:20 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 20F3D20578 for ; Mon, 29 Apr 2019 09:18:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 20F3D20578 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=virtuozzo.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([127.0.0.1]:54505 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hL2QV-0001oB-Bv for qemu-devel@archiver.kernel.org; Mon, 29 Apr 2019 05:18:19 -0400 Received: from eggs.gnu.org ([209.51.188.92]:34070) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hL2HL-0002RJ-Hu for qemu-devel@nongnu.org; Mon, 29 Apr 2019 05:08:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hL2HJ-0003vW-Jv for qemu-devel@nongnu.org; Mon, 29 Apr 2019 05:08:51 -0400 Received: from relay.sw.ru ([185.231.240.75]:32884) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hL2HH-0003sP-LY; Mon, 29 Apr 2019 05:08:49 -0400 Received: from [10.28.8.145] (helo=kvm.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1hL2HE-0002jV-10; Mon, 29 Apr 2019 12:08:44 +0300 From: Vladimir Sementsov-Ogievskiy To: qemu-devel@nongnu.org, qemu-block@nongnu.org Date: Mon, 29 Apr 2019 12:08:40 +0300 Message-Id: <20190429090842.57910-4-vsementsov@virtuozzo.com> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20190429090842.57910-1-vsementsov@virtuozzo.com> References: <20190429090842.57910-1-vsementsov@virtuozzo.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 185.231.240.75 Subject: [Qemu-devel] [PATCH v7 3/5] block/backup: refactor and tolerate unallocated cluster skipping X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, den@openvz.org, vsementsov@virtuozzo.com, jsnow@redhat.com, mreitz@redhat.com Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="UTF-8" Message-ID: <20190429090840.vVuoRFSU7Ve8v9aplAWsskWjIa3jduC7TQBpjIKl5h0@z> Split allocation checking to separate function and reduce nesting. Consider bdrv_is_allocated() fail as allocated area, as copying more than needed is not wrong (and we do it anyway) and seems better than fail the whole job. And, most probably we will fail on the next read, if there are real problem with source. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/backup.c | 60 +++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/block/backup.c b/block/backup.c index 510fc54f98..298e85f1a9 100644 --- a/block/backup.c +++ b/block/backup.c @@ -377,6 +377,22 @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job) return false; } +static bool bdrv_is_unallocated_range(BlockDriverState *bs, + int64_t offset, int64_t bytes) +{ + int64_t end = offset + bytes; + + while (offset < end && !bdrv_is_allocated(bs, offset, bytes, &bytes)) { + if (bytes == 0) { + return true; + } + offset += bytes; + bytes = end - offset; + } + + return offset >= end; +} + static int coroutine_fn backup_run_incremental(BackupBlockJob *job) { int ret; @@ -462,49 +478,19 @@ static int coroutine_fn backup_run(Job *job, Error **errp) for (offset = 0; offset < s->len; offset += s->cluster_size) { bool error_is_read; - int alloced = 0; if (yield_and_check(s)) { break; } - if (s->sync_mode == MIRROR_SYNC_MODE_TOP) { - int i; - int64_t n; - - /* Check to see if these blocks are already in the - * backing file. */ - - for (i = 0; i < s->cluster_size;) { - /* bdrv_is_allocated() only returns true/false based - * on the first set of sectors it comes across that - * are are all in the same state. - * For that reason we must verify each sector in the - * backup cluster length. We end up copying more than - * needed but at some point that is always the case. */ - alloced = - bdrv_is_allocated(bs, offset + i, - s->cluster_size - i, &n); - i += n; - - if (alloced || n == 0) { - break; - } - } - - /* If the above loop never found any sectors that are in - * the topmost image, skip this backup. */ - if (alloced == 0) { - continue; - } - } - /* FULL sync mode we copy the whole drive. */ - if (alloced < 0) { - ret = alloced; - } else { - ret = backup_do_cow(s, offset, s->cluster_size, - &error_is_read, false); + if (s->sync_mode == MIRROR_SYNC_MODE_TOP && + bdrv_is_unallocated_range(bs, offset, s->cluster_size)) + { + continue; } + + ret = backup_do_cow(s, offset, s->cluster_size, + &error_is_read, false); if (ret < 0) { /* Depending on error action, fail now or retry cluster */ BlockErrorAction action = -- 2.18.0