From patchwork Mon Sep 2 13:52:37 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 271950 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)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 87D352C0097 for ; Mon, 2 Sep 2013 23:53:00 +1000 (EST) Received: from localhost ([::1]:40026 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGUYr-0003V0-EH for incoming@patchwork.ozlabs.org; Mon, 02 Sep 2013 09:52:57 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54136) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGUYZ-0003Uj-Ls for qemu-devel@nongnu.org; Mon, 02 Sep 2013 09:52:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VGUYY-00037U-4M for qemu-devel@nongnu.org; Mon, 02 Sep 2013 09:52:39 -0400 Received: from mail-qa0-x229.google.com ([2607:f8b0:400d:c00::229]:39766) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGUYX-00037K-VD for qemu-devel@nongnu.org; Mon, 02 Sep 2013 09:52:38 -0400 Received: by mail-qa0-f41.google.com with SMTP id hu16so941381qab.7 for ; Mon, 02 Sep 2013 06:52:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=wWTK46ya0uzwzww0eGMd700qt58vStHh80t0DOQIMjo=; b=LM50Y2Oh6LzuQwALkXaEjuAFREcUo8sInDY98RyzjBZPZxXqnkXd5GhNW60Ptm4nNs zruSRYrv5ULY3rOWdFrSMoCc8DOS8rmwVjW+W0443yOsh0ZVIciYDttgAj2Mq+Ocv4xH /6nvHjJkA4AEVqyL7Cai4fpWQcInNyGImXQoj9ddoD5mzzBF5rAfTMVOGK/Sul7J1sQV gkd2AMazPMqRsVsUti+NvS6Kshpph5P5Mw/YAjxpYRE16Dv/ZPq2wkAbKsxKLNw29BdM j8/C6NeMsXXnfeNa3w0jpSdDxyIydAvow7aXdTlByvtxhsEByOQx2lpI1w/4r4uI76l4 ouZA== MIME-Version: 1.0 X-Received: by 10.49.81.237 with SMTP id d13mr28343916qey.44.1378129957445; Mon, 02 Sep 2013 06:52:37 -0700 (PDT) Received: by 10.49.60.98 with HTTP; Mon, 2 Sep 2013 06:52:37 -0700 (PDT) In-Reply-To: <1378111792-20436-23-git-send-email-kwolf@redhat.com> References: <1378111792-20436-1-git-send-email-kwolf@redhat.com> <1378111792-20436-23-git-send-email-kwolf@redhat.com> Date: Mon, 2 Sep 2013 15:52:37 +0200 Message-ID: From: Stefan Hajnoczi To: Max Reitz X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:400d:c00::229 Cc: Kevin Wolf , qemu-devel , Anthony Liguori Subject: Re: [Qemu-devel] [PULL v2 22/26] qcow2-refcount: Move OFLAG_COPIED checks 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 On Mon, Sep 2, 2013 at 10:49 AM, Kevin Wolf wrote: > From: Max Reitz > > Move the OFLAG_COPIED checks out of check_refcounts_l1 and > check_refcounts_l2 and after the actual refcount checks/fixes (since the > refcounts might actually change there). > > Signed-off-by: Max Reitz > Signed-off-by: Kevin Wolf > --- > block/qcow2-refcount.c | 115 +++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 82 insertions(+), 33 deletions(-) This patch breaks qemu-iotests 039 as follows: $ ./check -qcow2 039 QEMU -- qemu QEMU_IMG -- qemu-img QEMU_IO -- qemu-io IMGFMT -- qcow2 (compat=1.1) IMGPROTO -- file PLATFORM -- Linux/x86_64 3.10.9-200.fc19.x86_64 039 2s ... - output mismatch (see 039.out.bad) Failures: 039 Failed 1 of 1 tests > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > index 8ee2f13..aa4b98d 100644 > --- a/block/qcow2-refcount.c > +++ b/block/qcow2-refcount.c > @@ -1053,7 +1053,7 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, > BDRVQcowState *s = bs->opaque; > uint64_t *l2_table, l2_entry; > uint64_t next_contiguous_offset = 0; > - int i, l2_size, nb_csectors, refcount; > + int i, l2_size, nb_csectors; > > /* Read L2 table from disk */ > l2_size = s->l2_size * sizeof(uint64_t); > @@ -1105,23 +1105,8 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, > > case QCOW2_CLUSTER_NORMAL: > { > - /* QCOW_OFLAG_COPIED must be set iff refcount == 1 */ > uint64_t offset = l2_entry & L2E_OFFSET_MASK; > > - if (flags & CHECK_OFLAG_COPIED) { > - refcount = get_refcount(bs, offset >> s->cluster_bits); > - if (refcount < 0) { > - fprintf(stderr, "Can't get refcount for offset %" > - PRIx64 ": %s\n", l2_entry, strerror(-refcount)); > - goto fail; > - } > - if ((refcount == 1) != ((l2_entry & QCOW_OFLAG_COPIED) != 0)) { > - fprintf(stderr, "ERROR OFLAG_COPIED: offset=%" > - PRIx64 " refcount=%d\n", l2_entry, refcount); > - res->corruptions++; > - } > - } > - > if (flags & CHECK_FRAG_INFO) { > res->bfi.allocated_clusters++; > if (next_contiguous_offset && > @@ -1178,7 +1163,7 @@ static int check_refcounts_l1(BlockDriverState *bs, > { > BDRVQcowState *s = bs->opaque; > uint64_t *l1_table, l2_offset, l1_size2; > - int i, refcount, ret; > + int i, ret; > > l1_size2 = l1_size * sizeof(uint64_t); > > @@ -1202,22 +1187,6 @@ static int check_refcounts_l1(BlockDriverState *bs, > for(i = 0; i < l1_size; i++) { > l2_offset = l1_table[i]; > if (l2_offset) { > - /* QCOW_OFLAG_COPIED must be set iff refcount == 1 */ > - if (flags & CHECK_OFLAG_COPIED) { > - refcount = get_refcount(bs, (l2_offset & ~QCOW_OFLAG_COPIED) > - >> s->cluster_bits); > - if (refcount < 0) { > - fprintf(stderr, "Can't get refcount for l2_offset %" > - PRIx64 ": %s\n", l2_offset, strerror(-refcount)); > - goto fail; > - } > - if ((refcount == 1) != ((l2_offset & QCOW_OFLAG_COPIED) != 0)) { > - fprintf(stderr, "ERROR OFLAG_COPIED: l2_offset=%" PRIx64 > - " refcount=%d\n", l2_offset, refcount); > - res->corruptions++; > - } > - } > - > /* Mark L2 table as used */ > l2_offset &= L1E_OFFSET_MASK; > inc_refcounts(bs, res, refcount_table, refcount_table_size, > @@ -1249,6 +1218,80 @@ fail: > } > > /* > + * Checks the OFLAG_COPIED flag for all L1 and L2 entries. > + * > + * This function does not print an error message nor does it increment > + * check_errors if get_refcount fails (this is because such an error will have > + * been already detected and sufficiently signaled by the calling function > + * (qcow2_check_refcounts) by the time this function is called). > + */ > +static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res) > +{ > + BDRVQcowState *s = bs->opaque; > + uint64_t *l2_table = qemu_blockalign(bs, s->cluster_size); > + int ret; > + int refcount; > + int i, j; > + > + for (i = 0; i < s->l1_size; i++) { > + uint64_t l1_entry = s->l1_table[i]; > + uint64_t l2_offset = l1_entry & L1E_OFFSET_MASK; > + > + if (!l2_offset) { > + continue; > + } > + > + refcount = get_refcount(bs, l2_offset >> s->cluster_bits); > + if (refcount < 0) { > + /* don't print message nor increment check_errors */ > + continue; > + } > + if ((refcount == 1) != ((l1_entry & QCOW_OFLAG_COPIED) != 0)) { > + fprintf(stderr, "ERROR OFLAG_COPIED L2 cluster: l1_index=%d " > + "l1_entry=%" PRIx64 " refcount=%d\n", > + i, l1_entry, refcount); > + res->corruptions++; > + } > + > + ret = bdrv_pread(bs->file, l2_offset, l2_table, > + s->l2_size * sizeof(uint64_t)); > + if (ret < 0) { > + fprintf(stderr, "ERROR: Could not read L2 table: %s\n", > + strerror(-ret)); > + res->check_errors++; > + goto fail; > + } > + > + for (j = 0; j < s->l2_size; j++) { > + uint64_t l2_entry = be64_to_cpu(l2_table[j]); > + uint64_t data_offset = l2_entry & L2E_OFFSET_MASK; > + int cluster_type = qcow2_get_cluster_type(l2_entry); > + > + if ((cluster_type == QCOW2_CLUSTER_NORMAL) || > + ((cluster_type == QCOW2_CLUSTER_ZERO) && (data_offset != 0))) { > + refcount = get_refcount(bs, data_offset >> s->cluster_bits); > + if (refcount < 0) { > + /* don't print message nor increment check_errors */ > + continue; > + } > + if ((refcount == 1) != ((l2_entry & QCOW_OFLAG_COPIED) != 0)) { > + fprintf(stderr, "ERROR OFLAG_COPIED data cluster: " > + "l2_entry=%" PRIx64 " refcount=%d\n", > + l2_entry, refcount); > + res->corruptions++; > + } > + } > + } > + } > + > + ret = 0; > + > +fail: > + qemu_vfree(l2_table); > + return ret; > +} > + > +/* > * Checks an image for refcount consistency. > * > * Returns 0 if no errors are found, the number of errors in case the image is > @@ -1383,6 +1426,12 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, > } > } > > + /* check OFLAG_COPIED */ > + ret = check_oflag_copied(bs, res); > + if (ret < 0) { > + goto fail; > + } > + > res->image_end_offset = (highest_cluster + 1) * s->cluster_size; > ret = 0; > > -- > 1.8.1.4 > > --- 039.out 2013-09-02 13:31:26.134178859 +0200 +++ 039.out.bad 2013-09-02 15:48:00.103063126 +0200 @@ -12,8 +12,8 @@ wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) incompatible_features 0x1 -ERROR OFLAG_COPIED: offset=8000000000050000 refcount=0 ERROR cluster 5 refcount=0 reference=1 +ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0 2 errors were found on the image. Data may be corrupted, or further writes to the image may corrupt it. @@ -24,7 +24,6 @@ incompatible_features 0x1 == Repairing the image file must succeed == -ERROR OFLAG_COPIED: offset=8000000000050000 refcount=0 Repairing cluster 5 refcount=0 reference=1 The following inconsistencies were found and repaired: @@ -44,7 +43,6 @@ wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) incompatible_features 0x1 -ERROR OFLAG_COPIED: offset=8000000000050000 refcount=0 Repairing cluster 5 refcount=0 reference=1 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)