From patchwork Thu Jul 11 15:33:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Filipe Manana X-Patchwork-Id: 2826490 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A07249F7D6 for ; Thu, 11 Jul 2013 15:34:06 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 489BA2026D for ; Thu, 11 Jul 2013 15:34:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 048CE2026C for ; Thu, 11 Jul 2013 15:34:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756189Ab3GKPeA (ORCPT ); Thu, 11 Jul 2013 11:34:00 -0400 Received: from mail-we0-f179.google.com ([74.125.82.179]:48635 "EHLO mail-we0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755885Ab3GKPd7 (ORCPT ); Thu, 11 Jul 2013 11:33:59 -0400 Received: by mail-we0-f179.google.com with SMTP id w59so6970362wes.24 for ; Thu, 11 Jul 2013 08:33:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=poqE+VzUMOy3IO3dXGnZ7VRWwu990/gJ/rO0WXNzMBg=; b=Cb+HlCy6rVHlxmDhzLU8B+/yfuCNY4MufDZ25Omp9zcTg2wymLZ+G4qMgqs7YztTD3 qaxAH2PJi9d9fokQQYfK4xAaPfR8L6w05sgsMmSzb+FdBygln3sim9Kne5+4xn8qSJre LMMiDW4k96pPtX3CtniG2RoKLOSGtGz/8v7WwvYeVKUSp4TK7dgKYDoQhOyZLFSb5X0s DD+kq4zvb/qni06GrdI0ZEiRe+We0F/Sf+RsovMkmlvU4X6nzOZ3h3C4F8zxz8mOE8Gq Axo6FpikoKZkPIPzN523SfSAS2ZZr2pJxO24WuMTLLALAGx8lytC5PEHwEJoa921CIVI 4wPA== X-Received: by 10.180.208.7 with SMTP id ma7mr38308316wic.11.1373556837971; Thu, 11 Jul 2013 08:33:57 -0700 (PDT) Received: from storm-desktop.lan (bl10-255-22.dsl.telepac.pt. [85.243.255.22]) by mx.google.com with ESMTPSA id eu1sm74189413wib.8.2013.07.11.08.33.56 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 11 Jul 2013 08:33:57 -0700 (PDT) From: Filipe David Borba Manana To: linux-btrfs@vger.kernel.org Cc: Filipe David Borba Manana Subject: [PATCH v5] Btrfs-progs: fix restore command leaving corrupted files Date: Thu, 11 Jul 2013 16:33:40 +0100 Message-Id: <1373556820-30293-1-git-send-email-fdmanana@gmail.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1373475413-23563-1-git-send-email-fdmanana@gmail.com> References: <1373475413-23563-1-git-send-email-fdmanana@gmail.com> Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When there are files that have parts shared with snapshots, the restore command was incorrectly restoring them, as it was not taking into account the offset and number of bytes fields from the file extent item. Besides leaving the recovered file corrupt, it was also inneficient as it read and wrote more data than needed (with each extent copy overwriting portions of the one previously written). The following steps show how to reproduce this corruption issue: $ mkfs.btrfs -f /dev/sdb3 $ mount /dev/sdb3 /mnt/btrfs $ perl -e '$d = "\x41" . ("\x00" x (1024*1024+349)); open($f,">","/mnt/btrfs/foobar"); print $f $d; close($f);' $ du -b /mnt/btrfs/foobar 1048926 /mnt/btrfs/foobar $ md5sum /mnt/btrfs/foobar f9f778f3a7410c40e4ed104a3a63c3c4 /mnt/btrfs/foobar $ btrfs subvolume snapshot /mnt/btrfs /mnt/btrfs/my_snap $ perl -e 'open($f, "+<", "/dev/btrfs/foobar"); seek($f, 4096, 0); print $f "\xff"; close($f);' $ md5sum /mnt/btrfs/foobar b983fcefd4622a03a78936484c40272b /mnt/btrfs/foobar $ umount /mnt/btrfs $ btrfs restore /dev/sdb3 /tmp/copy $ du -b /tmp/copy/foobar 1048926 /tmp/copy/foobar $ md5sum /tmp/copy/foobar 88db338cbc1c44dfabae083f1ce642d5 /tmp/copy/foobar $ od -t x1 -j 8192 -N 4 /tmp/copy/foobar 0020000 41 00 00 00 0020004 $ mount /dev/sdb3 /mnt/btrfs $ od -t x1 -j 8192 -N 4 /mnt/btrfs/foobar 0020000 00 00 00 00 0020004 $ md5sum /mnt/btrfs/foobar b983fcefd4622a03a78936484c40272b /mnt/btrfs/foobar Tested this change with zlib, lzo compression and file sizes larger than 1GiB, and found no regression or other corruption issues (so far at least). Signed-off-by: Filipe David Borba Manana --- V2: updated commit message to include the C preprocessor macros in the C program. V3: updated commit message again to reflect the file size used in the example in the C program macro. V4: fixed wrong path in commit message in the perl command line. V5: updated commit message with simpler steps to reproduce the issue. cmds-restore.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cmds-restore.c b/cmds-restore.c index e48df40..9688599 100644 --- a/cmds-restore.c +++ b/cmds-restore.c @@ -272,6 +272,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd, u64 bytenr; u64 ram_size; u64 disk_size; + u64 num_bytes; u64 length; u64 size_left; u64 dev_bytenr; @@ -288,7 +289,9 @@ static int copy_one_extent(struct btrfs_root *root, int fd, disk_size = btrfs_file_extent_disk_num_bytes(leaf, fi); ram_size = btrfs_file_extent_ram_bytes(leaf, fi); offset = btrfs_file_extent_offset(leaf, fi); - size_left = disk_size; + num_bytes = btrfs_file_extent_num_bytes(leaf, fi); + size_left = num_bytes; + bytenr += offset; if (offset) printf("offset is %Lu\n", offset); @@ -296,7 +299,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd, if (disk_size == 0) return 0; - inbuf = malloc(disk_size); + inbuf = malloc(size_left); if (!inbuf) { fprintf(stderr, "No memory\n"); return -1; @@ -351,8 +354,8 @@ again: goto again; if (compress == BTRFS_COMPRESS_NONE) { - while (total < ram_size) { - done = pwrite(fd, inbuf+total, ram_size-total, + while (total < num_bytes) { + done = pwrite(fd, inbuf+total, num_bytes-total, pos+total); if (done < 0) { ret = -1; @@ -365,7 +368,7 @@ again: goto out; } - ret = decompress(inbuf, outbuf, disk_size, &ram_size, compress); + ret = decompress(inbuf, outbuf, num_bytes, &ram_size, compress); if (ret) { num_copies = btrfs_num_copies(&root->fs_info->mapping_tree, bytenr, length);