All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>,
	xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH] xfsprogs: handle symlinks etc in fs_table_initialise_mounts()
Date: Tue, 15 Oct 2013 07:46:17 +1100	[thread overview]
Message-ID: <20131014204617.GG5663@dastard> (raw)
In-Reply-To: <5249AE5F.30305@redhat.com>

On Mon, Sep 30, 2013 at 12:01:19PM -0500, Eric Sandeen wrote:
> Commit:
> 
> 6a23747d xfs_quota: support relative path as `path' arguments
> 
> used realpath() on the supplied pathname to handle things like
> relative pathnames and pathnames ending in "/" which otherwise
> caused the getmntent scanning to fail.
> 
> However, this regressed cases where a path in mtab was a symlink;
> realpath() resolves this to the target, and so no match is found.
> 
> This causes i.e.:
> 
> # xfs_quota -x -c report /dev/mapper/testvg-testlv
> 
> to fail with:
> 
> xfs_quota: cannot setup path for mount /dev/mapper/testvg-testlv: No such device or address
> 
> because the scanning looks for /dev/dm-3, but the long symlink
> name is what exists in mtab, and no match is found.
> 
> Fix this, but keep the intended enhancements, by testing *both* the
> user-specified path (which might be relative, or contain a trailing
> slash on a mountpoint) and the realpath-resolved path (which turns
> a relative mountpoint into a full path, and removes trailing slashes),
> to determine whether the user-specified path is an xfs mountpoint or
> device.
> 
> While we're at it, add a few comments, and go back to the testing
> of "path" not "rpath"; whether or not path is passed to the function
> is what determines control flow.  If path is specified, and realpath
> succeeds, we're guaranteed to have rpath as well, so there is no need
> to retest that.  rpath is initialized to NULL, so an unconditional
> free(rpath) is safe as well.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Looks good. There's a bunch of minor cleanups that could be done to
this code, but save that for a rainy day...

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2013-10-14 20:46 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-30 17:01 [PATCH] xfsprogs: handle symlinks etc in fs_table_initialise_mounts() Eric Sandeen
2013-10-14 20:46 ` Dave Chinner [this message]
2013-10-18 17:18 ` Rich Johnston

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131014204617.GG5663@dastard \
    --to=david@fromorbit.com \
    --cc=sandeen@redhat.com \
    --cc=takeuchi_satoru@jp.fujitsu.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.