On Fri, Oct 14, 2011 at 09:04:28AM +0900, Tsutomu Itoh wrote:
> (2011/10/14 2:11), Josef Bacik wrote:
> > Recently I changed the xattr stuff to unconditionally set the xattr first in
> > case the xattr didn't exist yet.  This has introduced a regression when 
> > setting
> > an xattr that already exists with a large value.  If we find the key we are
> > looking for split_leaf will assume that we're extending that item.  The 
> > problem
> > is the size we pass down to btrfs_search_slot includes the size of the item
> > already, so if we have the largest xattr we can possibly have plus the size 
> > of
> > the xattr item plus the xattr item that btrfs_search_slot we'd overflow the
> > leaf.  Thankfully this is not what we're doing, but split_leaf doesn't know 
> > this
> > so it just returns EOVERFLOW.  So in the xattr code we need to check and 
> > see if
> > we got back EOVERFLOW and treat it like EEXIST since that's really what
> > happened.  Thanks,
> > 
> > Signed-off-by: Josef Bacik <jo...@redhat.com>
> > ---
> >  fs/btrfs/xattr.c |   13 ++++++++++++-
> >  1 files changed, 12 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
> > index 69565e5..5bd7877 100644
> > --- a/fs/btrfs/xattr.c
> > +++ b/fs/btrfs/xattr.c
> > @@ -127,7 +127,18 @@ static int do_setxattr(struct btrfs_trans_handle 
> > *trans,
> >  again:
> >     ret = btrfs_insert_xattr_item(trans, root, path, btrfs_ino(inode),
> >                                   name, name_len, value, size);
> > -   if (ret == -EEXIST) {
> > +   /*
> > +    * If we're setting an xattr to a new value but the new value is say
> > +    * exactly BTRFS_MAX_XATTR_SIZE, we could end up with EOVERFLOW getting
> > +    * back from split_leaf.  This is because it thinks we'll be extending
> > +    * the existing item size, but we're asking for enough space to add the
> > +    * item itself.  So if we get EOVERFLOW just set ret to EEXIST and let
> > +    * the rest of the function figure it out.
> > +    */
> > +   if (ret == -EOVERFLOW)
> > +           ret = -EEXIST;
> > +
> > +   if (ret == -EEXIST || ret == -EOVERFLOW) {
> 
> Why tested again EOVERFLOW?
> 

Oops thats my fault, I had thought to check for eoverflow but then thought
better to just set it to eexist and didn't fix the first thought.  I'll send out
a fix.  Thanks,

Josef
--
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

Reply via email to