[PATCH] btrfs: trivial fix, a potential memory leak in btrfs_parse_early_options()

2011-09-13 Thread Jeff Liu

Signed-off-by: Jie Liu jeff@oracle.com

---
 fs/btrfs/super.c |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 15634d4..16f31e1 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char 
*options, fmode_t flags,

 u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices)
 {
 substring_t args[MAX_OPT_ARGS];
-char *opts, *orig, *p;
+char *device_name, *opts, *orig, *p;
 int error = 0;
 int intarg;

@@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char 
*options, fmode_t flags,

 }
 break;
 case Opt_device:
-error = btrfs_scan_one_device(match_strdup(args[0]),
+device_name = match_strdup(args[0]);
+if (!device_name) {
+error = -ENOMEM;
+goto out_free_opts;
+}
+error = btrfs_scan_one_device(device_name,
 flags, holder, fs_devices);
+kfree(device_name);
 if (error)
 goto out_free_opts;
 break;
--
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] xfstests: add new getdents test

2011-09-13 Thread Christoph Hellwig
On Mon, Sep 12, 2011 at 03:19:07AM +0300, Grazvydas Ignotas wrote:
 The test checks if no duplicate d_off values are returned and that
 those values are seekable to the right inodes.
 
 Signed-off-by: Grazvydas Ignotas nota...@gmail.com

Thanks a lot!  I've applied it locally and will push it out as soon
as kernel.org is back up.

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Honest timeline for btrfsck

2011-09-13 Thread Jeff Putney
Isn't it about time to make some hard decisions about btrfsck?  Three
years is enough time to go without this type of functionality in a
modern filesystem, especially given btrfs's fragility in the face of
power failures.

Given the lack of progress, and the inability to provide remotely
realistic time lines, I'd say it is inappropriate for development to
remain in secret.  I am aware of the type of noise that could be
created by releasing this code out to the public, but that noise will
help drive fixes and development of the tool.  We have seen months
worth of wasted effort spent on stop gap measures that could have been
funneled into the real utility, if people just had access to the code.

If we can't get btrfsck checked into btrfs-progs-unstable, should we
just cut our losses and start a recovery tool from scratch.  There was
a btrfs_rescue utility mentioned at
http://article.gmane.org/gmane.comp.file-systems.btrfs/8309/match=btrfs_rescue,
but the source doesn't seem to be available anymore.

I had started to work on my own repair tool/script/hack.  I was
looking into some of the current btrfs progs code, and the biggest
issue I had was all of the data validation that disk_io.c and other
code does that makes sure the FS is in a good state before you can do
anything with it.  This makes those utility methods pretty much
useless for exploring a broken FS.  So, understanding how to
incrementally read the FS without doing all of the validations
beforehand was the biggest hurdle I encountered.


On Sat, Sep 10, 2011 at 5:09 AM, Martin Steigerwald mar...@lichtvoll.de wrote:
 Am Donnerstag, 1. September 2011 schrieb Hugo Mills:
 On Thu, Sep 01, 2011 at 03:24:28PM -0500, Michael Cronenworth wrote:
  On 09/01/2011 03:20 PM, Hugo Mills wrote:
      You may have missed the on vacation bit.
 
  I did read the on vacation bit. Not that it is any of my business,
  but how long is that vacation?

    Your guess is as good as mine. It's only been two weeks...

      The canonical place to look for btrfsck updates is the relevant
      FAQ
  
  item on the btrfs wiki.
 
  I know this. No need to repeat it. I'm not a complete idiot.

    I never claimed you were. Many people either don't realise that
 that is the right place, or don't realise that it really is updated
 pretty much as soon as possible with all the information that's been
 made public on the subject.

 Well, I do think that a release of the fsck for BTRFS is important enough
 to announce it on this mailinglist, too ;).

 Fortunately I didn´t have any major problems so far. I once had to use
 btrfs-zero-log. And I am still not using BTRFS for my main machine´s home
 directory. For this I intend to wait till it is marked stable in kernel
 sources + fsck available.

 --
 Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
 GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7
 --
 To unsubscribe from this list: send the line unsubscribe linux-btrfs in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] Btrfs: don't make a file partly checksummed through file clone

2011-09-13 Thread Li Zefan
To reproduce the bug:

  # mount /dev/sda7 /mnt
  # dd if=/dev/zero of=/mnt/src bs=4K count=1
  # umount /mnt

  # mount -o nodatasum /dev/sda7 /mnt
  # dd if=/dev/zero of=/mnt/dst bs=4K count=1
  # clone_range -s 4K -l 4K /mnt/src /mnt/dst

  # echo 3  /proc/sys/vm/drop_caches
  # cat /mnt/dst
  # dmesg
  ...
  btrfs no csum found for inode 258 start 0
  btrfs csum failed ino 258 off 0 csum 2566472073 private 0

It's because part of the file is checksummed and the other part is not,
and then btrfs will complain checksum is not found when we read the file.

Disallow file clone if src and dst file have different checksum flag,
so we ensure a file is completely checksummed or unchecksummed.

Signed-off-by: Li Zefan l...@cn.fujitsu.com
---
 fs/btrfs/ioctl.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 970977a..dc82bbb 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2177,6 +2177,11 @@ static noinline long btrfs_ioctl_clone(struct file 
*file, unsigned long srcfd,
if (!(src_file-f_mode  FMODE_READ))
goto out_fput;
 
+   /* don't make the dst file partly checksummed */
+   if ((BTRFS_I(src)-flags  BTRFS_INODE_NODATASUM) !=
+   (BTRFS_I(inode)-flags  BTRFS_INODE_NODATASUM))
+   goto out_fput;
+
ret = -EISDIR;
if (S_ISDIR(src-i_mode) || S_ISDIR(inode-i_mode))
goto out_fput;
-- 1.7.3.1 
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] Btrfs: don't change inode flag of the dest clone file

2011-09-13 Thread Li Zefan
The dst file will have the same inode flags with dst file after
file clone, and I think it's unexpected.

For example, the dst file will suddenly become immutable after
getting some share of data with src file, if the src is immutable.

Signed-off-by: Li Zefan l...@cn.fujitsu.com
---
 fs/btrfs/ioctl.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index dc82bbb..a401514 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2434,7 +2434,6 @@ static noinline long btrfs_ioctl_clone(struct file *file, 
unsigned long srcfd,
if (endoff  inode-i_size)
btrfs_i_size_write(inode, endoff);
 
-   BTRFS_I(inode)-flags = BTRFS_I(src)-flags;
ret = btrfs_update_inode(trans, root, inode);
BUG_ON(ret);
btrfs_end_transaction(trans, root);
-- 1.7.3.1 
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] btrfs: trivial fix, a potential memory leak in btrfs_parse_early_options()

2011-09-13 Thread Li Zefan
14:06, Jeff Liu wrote:
 Signed-off-by: Jie Liu jeff@oracle.com
 
 ---
  fs/btrfs/super.c |   10 --
  1 files changed, 8 insertions(+), 2 deletions(-)
 
 diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
 index 15634d4..16f31e1 100644
 --- a/fs/btrfs/super.c
 +++ b/fs/btrfs/super.c
 @@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char *options, 
 fmode_t flags,
  u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices)
  {
  substring_t args[MAX_OPT_ARGS];
 -char *opts, *orig, *p;
 +char *device_name, *opts, *orig, *p;

Seems your email client replaced tabs with spaces.

Please read Documentation/email-clients.txt

  int error = 0;
  int intarg;
 
 @@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char 
 *options, fmode_t flags,
  }
  break;
  case Opt_device:
 -error = btrfs_scan_one_device(match_strdup(args[0]),
 +device_name = match_strdup(args[0]);
 +if (!device_name) {
 +error = -ENOMEM;
 +goto out_free_opts;
 +}
 +error = btrfs_scan_one_device(device_name,
  flags, holder, fs_devices);
 +kfree(device_name);
  if (error)
  goto out_free_opts;
  break;
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html