Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 4 Oct 2022 11:47:47 -0700
From:   "Darrick J. Wong" <djwong@kernel.org>
To:     Lukas Czerner <lczerner@redhat.com>
Cc:     linux-ext4@vger.kernel.org, tytso@mit.edu,
        linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] ext4: journal_path mount options should follow links
Message-ID: <Yzx/08kWtzX8HR5i@magnolia>
References: <20221004135803.32283-1-lczerner@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221004135803.32283-1-lczerner@redhat.com>
Precedence: bulk

On Tue, Oct 04, 2022 at 03:58:03PM +0200, Lukas Czerner wrote:
> Before the commit 461c3af045d3 ("ext4: Change handle_mount_opt() to use
> fs_parameter") ext4 mount option journal_path did follow links in the
> provided path.
> 
> Bring this behavior back by allowing to pass pathwalk flags to
> fs_lookup_param().
> 
> Fixes: 461c3af045d3 ("ext4: Change handle_mount_opt() to use fs_parameter")
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

This bit me recently when I was debugging an old system that used an
external journal on an LVM LV and I wanted to know if the bug I was
chasing also reproduced on upstream.  Thanks for fixing this before I
got around to making a patch:

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  Documentation/filesystems/mount_api.rst | 1 +
>  fs/ext4/super.c                         | 2 +-
>  fs/fs_parser.c                          | 3 ++-
>  include/linux/fs_parser.h               | 1 +
>  4 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/filesystems/mount_api.rst b/Documentation/filesystems/mount_api.rst
> index eb358a00be27..1d16787a00e9 100644
> --- a/Documentation/filesystems/mount_api.rst
> +++ b/Documentation/filesystems/mount_api.rst
> @@ -814,6 +814,7 @@ process the parameters it is given.
>         int fs_lookup_param(struct fs_context *fc,
>  			   struct fs_parameter *value,
>  			   bool want_bdev,
> +			   unsigned int flags,
>  			   struct path *_path);
>  
>       This takes a parameter that carries a string or filename type and attempts
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 9a66abcca1a8..4c1b3972d53f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2271,7 +2271,7 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  			return -EINVAL;
>  		}
>  
> -		error = fs_lookup_param(fc, param, 1, &path);
> +		error = fs_lookup_param(fc, param, 1, LOOKUP_FOLLOW, &path);
>  		if (error) {
>  			ext4_msg(NULL, KERN_ERR, "error: could not find "
>  				 "journal device path");
> diff --git a/fs/fs_parser.c b/fs/fs_parser.c
> index ed40ce5742fd..edb3712dcfa5 100644
> --- a/fs/fs_parser.c
> +++ b/fs/fs_parser.c
> @@ -138,15 +138,16 @@ EXPORT_SYMBOL(__fs_parse);
>   * @fc: The filesystem context to log errors through.
>   * @param: The parameter.
>   * @want_bdev: T if want a blockdev
> + * @flags: Pathwalk flags passed to filename_lookup()
>   * @_path: The result of the lookup
>   */
>  int fs_lookup_param(struct fs_context *fc,
>  		    struct fs_parameter *param,
>  		    bool want_bdev,
> +		    unsigned int flags,
>  		    struct path *_path)
>  {
>  	struct filename *f;
> -	unsigned int flags = 0;
>  	bool put_f;
>  	int ret;
>  
> diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h
> index f103c91139d4..01542c4b87a2 100644
> --- a/include/linux/fs_parser.h
> +++ b/include/linux/fs_parser.h
> @@ -76,6 +76,7 @@ static inline int fs_parse(struct fs_context *fc,
>  extern int fs_lookup_param(struct fs_context *fc,
>  			   struct fs_parameter *param,
>  			   bool want_bdev,
> +			   unsigned int flags,
>  			   struct path *_path);
>  
>  extern int lookup_constant(const struct constant_table tbl[], const char *name, int not_found);
> -- 
> 2.37.3
>