From patchwork Wed Jul 10 16:56:53 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Filipe Manana X-Patchwork-Id: 2825832 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id BBAB7C0AB2 for ; Wed, 10 Jul 2013 16:57:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 65E142015E for ; Wed, 10 Jul 2013 16:57:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5E2CD2014B for ; Wed, 10 Jul 2013 16:57:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754714Ab3GJQ5G (ORCPT ); Wed, 10 Jul 2013 12:57:06 -0400 Received: from mail-wg0-f51.google.com ([74.125.82.51]:49352 "EHLO mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754474Ab3GJQ5F (ORCPT ); Wed, 10 Jul 2013 12:57:05 -0400 Received: by mail-wg0-f51.google.com with SMTP id e11so6034843wgh.18 for ; Wed, 10 Jul 2013 09:57:03 -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=u6ce8r5fZnSgYCXC1Y0QgZlUAd/QWvK8jMb4hkvJpO4=; b=MVJKUECsf2LOdrWUlQo/ICzTaIFVbuPAX3v+FPHRgEqjeFDnQ1xl80urNeNYe5NGYA aUnd2lQ+oilDc+a8vl0qJPGR3JaaSIL27dkiNJplxLmDpeTHHTYdB8dCLQK7IZwiC701 p1x60Pc4AdSs3RrmFjf7x5ph/sDtxB8FNDzuwO84zzoYK2t8I1HRRdY8ZhH8keYDJqf5 0LVQdRvoquEB/cblT2UIa/Fl+oxlorH1skNHv6n+ZaZPF6J/jiiVFEdmkUXWLi/kQjTY oFxBSvn5V1vmXgjZwrjOTHQjM9h1WFDT08W0zvQt/57WwmxyBGEMMzuvXeEyy37a4CkK /O4Q== X-Received: by 10.180.100.35 with SMTP id ev3mr35303810wib.12.1373475423238; Wed, 10 Jul 2013 09:57:03 -0700 (PDT) Received: from storm-desktop.lan (bl10-255-22.dsl.telepac.pt. [85.243.255.22]) by mx.google.com with ESMTPSA id w4sm37297689wia.9.2013.07.10.09.57.02 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 10 Jul 2013 09:57:02 -0700 (PDT) From: Filipe David Borba Manana To: linux-btrfs@vger.kernel.org Cc: Filipe David Borba Manana Subject: [PATCH v4] Btrfs-progs: fix restore command leaving corrupted files Date: Wed, 10 Jul 2013 17:56:53 +0100 Message-Id: <1373475413-23563-1-git-send-email-fdmanana@gmail.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1373473052-22240-1-git-send-email-fdmanana@gmail.com> References: <1373473052-22240-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 and small C program show how to reproduce this corruption issue: $ mkfs.btrfs -f /dev/sdb3 $ mount /dev/sdb3 /mnt/btrfs $ ./write_file /mnt/btrfs/foobar $ 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, "+<", "/mnt/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 $ cat write_file.c: #include #include #include #include #include #include #include #include #define BUF_SIZE (1 * 1024 * 1024 + 350) int main(int argc, char *argv[]) { int fd; unsigned char *buf = malloc(BUF_SIZE); assert(buf != NULL); if (argc < 2) { fprintf(stderr, "Use: %s filepath\n", argv[0]); return 1; } fd = open(argv[1], O_CREAT | O_WRONLY | O_TRUNC, S_IRWXU); assert(fd >= 0); memset(buf, 0, BUF_SIZE); buf[0] = 65; assert(write(fd, buf, BUF_SIZE) == BUF_SIZE); assert(close(fd) == 0); return 0; } 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. 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);