LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kari Argillander <kari.argillander@gmail.com>
To: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
Cc: "Pali Rohár" <pali@kernel.org>,
	ntfs3@lists.linux.dev, "Christoph Hellwig" <hch@lst.de>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	"Matthew Wilcox" <willy@infradead.org>,
	"Christian Brauner" <christian.brauner@ubuntu.com>,
	torvalds@linux-foundation.org
Subject: Re: [PATCH v4 7/9] fs/ntfs3: Add iocharset= mount option as alias for nls=
Date: Tue, 12 Oct 2021 01:05:24 +0300	[thread overview]
Message-ID: <20211011220524.azcaxs7y5zgd2uwz@kari-VirtualBox> (raw)
In-Reply-To: <204a5be9-f0a2-1c85-d3a8-3011578b9299@paragon-software.com>

On Mon, Oct 11, 2021 at 08:14:28PM +0300, Konstantin Komarov wrote:
> 
> 
> On 09.10.2021 17:33, Kari Argillander wrote:
> > Choose to add Linus to CC so that he also knows whats coming.
> > 
> > On Sat, Oct 09, 2021 at 01:42:52PM +0200, Pali Rohár wrote:
> >> Hello!
> >>
> >> This patch have not been applied yet:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ntfs3/super.c#n247
> >>
> >> What happened that in upstream tree is still only nls= option and not
> >> this iocharset=?
> > 
> > Very valid question. For some reason Konstantin has not sended pr to
> > Linus. I have also address my concern that pr is not yet sended and he
> > will make very massive "patch dumb" to rc6/rc7. See thread [1]. There is
> > about 50-70 patch already which he will send to rc6/rc7. I have get also
> > impression that patches which are not yet even applied to ntfs3 tree [2]
> > will be also sended to rc6/rc7. There is lot of refactoring and new
> > algorithms which imo are not rc material. I have sended many message to
> > Konstantin about this topic, but basically ignored.
> > 
> > Basically we do not have anything for next merge window and every patch
> > will be sended for 5.15.
> > 
> > [1] lore.kernel.org/lkml/20210925082823.fo2wm62xlcexhwvi@kari-VirtualBox
> > [2] https://github.com/Paragon-Software-Group/linux-ntfs3/commits/master
> > 
> >   Argillander
> > 
> 
> Hello.
> 
> I was planning to send pull request on Friday 08.10.
> But there is still one panic, that wasn't resolved [1].
> It seems to be tricky, so I'll be content even with quick band-aid [2].
> After confirming, that it works, I plan on sending pull request.
> I don't want for this panic to remain in 5.15.

Of course we won't want panic to remain 5.15, but that does not mean you
can't send pr for other patches. You can still send other pr later for
other rc's. If you will send pr late it might be that Linus will not
apply any of them because it will be too big pr for late rc. I'm sure he
will be resonable in first time because ntfs3 is new driver and will
accept pr in this case, but this cannot come habbit.

Normally rc cycle is for this kind of things. We fix things we broke
during merge window, but right now it is real danger that we introduce
many bugs because people cannot test ntfs3 because you hold on patches.
They belong to also to Linus tree not just to linux-next. Many user use
Linus tree and there are many who send good bug reports.

But do not take this too harsh. I'm just personally worried here. Also I
think you as maintainer had improved well in short time.

> [1]: https://lore.kernel.org/ntfs3/f9de5807-2311-7374-afb0-bc5dffb522c0@gmail.com/
> [2]: https://lore.kernel.org/ntfs3/7e5b8dc9-9989-0e8a-9e8d-ae26b6e74df4@paragon-software.com/
> 
> >>
> >> On Wednesday 08 September 2021 21:09:38 Pali Rohár wrote:
> >>> On Tuesday 07 September 2021 18:35:55 Kari Argillander wrote:
> >>>> Other fs drivers are using iocharset= mount option for specifying charset.
> >>>> So add it also for ntfs3 and mark old nls= mount option as deprecated.
> >>>>
> >>>> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> >>>
> >>> Reviewed-by: Pali Rohár <pali@kernel.org>
> >>>
> >>>> ---
> >>>>  Documentation/filesystems/ntfs3.rst |  4 ++--
> >>>>  fs/ntfs3/super.c                    | 18 +++++++++++-------
> >>>>  2 files changed, 13 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/filesystems/ntfs3.rst b/Documentation/filesystems/ntfs3.rst
> >>>> index af7158de6fde..ded706474825 100644
> >>>> --- a/Documentation/filesystems/ntfs3.rst
> >>>> +++ b/Documentation/filesystems/ntfs3.rst
> >>>> @@ -32,12 +32,12 @@ generic ones.
> >>>>  
> >>>>  ===============================================================================
> >>>>  
> >>>> -nls=name		This option informs the driver how to interpret path
> >>>> +iocharset=name		This option informs the driver how to interpret path
> >>>>  			strings and translate them to Unicode and back. If
> >>>>  			this option is not set, the default codepage will be
> >>>>  			used (CONFIG_NLS_DEFAULT).
> >>>>  			Examples:
> >>>> -				'nls=utf8'
> >>>> +				'iocharset=utf8'
> >>>>  
> >>>>  uid=
> >>>>  gid=
> >>>> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> >>>> index 729ead6f2fac..503e2e23f711 100644
> >>>> --- a/fs/ntfs3/super.c
> >>>> +++ b/fs/ntfs3/super.c
> >>>> @@ -226,7 +226,7 @@ enum Opt {
> >>>>  	Opt_nohidden,
> >>>>  	Opt_showmeta,
> >>>>  	Opt_acl,
> >>>> -	Opt_nls,
> >>>> +	Opt_iocharset,
> >>>>  	Opt_prealloc,
> >>>>  	Opt_no_acs_rules,
> >>>>  	Opt_err,
> >>>> @@ -245,9 +245,13 @@ static const struct fs_parameter_spec ntfs_fs_parameters[] = {
> >>>>  	fsparam_flag_no("hidden",		Opt_nohidden),
> >>>>  	fsparam_flag_no("acl",			Opt_acl),
> >>>>  	fsparam_flag_no("showmeta",		Opt_showmeta),
> >>>> -	fsparam_string("nls",			Opt_nls),
> >>>>  	fsparam_flag_no("prealloc",		Opt_prealloc),
> >>>>  	fsparam_flag("no_acs_rules",		Opt_no_acs_rules),
> >>>> +	fsparam_string("iocharset",		Opt_iocharset),
> >>>> +
> >>>> +	__fsparam(fs_param_is_string,
> >>>> +		  "nls", Opt_iocharset,
> >>>> +		  fs_param_deprecated, NULL),
> >>>>  	{}
> >>>>  };
> >>>>  
> >>>> @@ -346,7 +350,7 @@ static int ntfs_fs_parse_param(struct fs_context *fc,
> >>>>  	case Opt_showmeta:
> >>>>  		opts->showmeta = result.negated ? 0 : 1;
> >>>>  		break;
> >>>> -	case Opt_nls:
> >>>> +	case Opt_iocharset:
> >>>>  		kfree(opts->nls_name);
> >>>>  		opts->nls_name = param->string;
> >>>>  		param->string = NULL;
> >>>> @@ -380,11 +384,11 @@ static int ntfs_fs_reconfigure(struct fs_context *fc)
> >>>>  	new_opts->nls = ntfs_load_nls(new_opts->nls_name);
> >>>>  	if (IS_ERR(new_opts->nls)) {
> >>>>  		new_opts->nls = NULL;
> >>>> -		errorf(fc, "ntfs3: Cannot load nls %s", new_opts->nls_name);
> >>>> +		errorf(fc, "ntfs3: Cannot load iocharset %s", new_opts->nls_name);
> >>>>  		return -EINVAL;
> >>>>  	}
> >>>>  	if (new_opts->nls != sbi->options->nls)
> >>>> -		return invalf(fc, "ntfs3: Cannot use different nls when remounting!");
> >>>> +		return invalf(fc, "ntfs3: Cannot use different iocharset when remounting!");
> >>>>  
> >>>>  	sync_filesystem(sb);
> >>>>  
> >>>> @@ -528,9 +532,9 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
> >>>>  	if (opts->dmask)
> >>>>  		seq_printf(m, ",dmask=%04o", ~opts->fs_dmask_inv);
> >>>>  	if (opts->nls)
> >>>> -		seq_printf(m, ",nls=%s", opts->nls->charset);
> >>>> +		seq_printf(m, ",iocharset=%s", opts->nls->charset);
> >>>>  	else
> >>>> -		seq_puts(m, ",nls=utf8");
> >>>> +		seq_puts(m, ",iocharset=utf8");
> >>>>  	if (opts->sys_immutable)
> >>>>  		seq_puts(m, ",sys_immutable");
> >>>>  	if (opts->discard)
> >>>> -- 
> >>>> 2.25.1
> >>>>

  reply	other threads:[~2021-10-11 22:05 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-07 15:35 [PATCH v4 0/9] fs/ntfs3: Use new mount api and change some opts Kari Argillander
2021-09-07 15:35 ` [PATCH v4 1/9] fs/ntfs3: Remove unnecesarry mount option noatime Kari Argillander
2021-09-08 19:10   ` Pali Rohár
2021-09-07 15:35 ` [PATCH v4 2/9] fs/ntfs3: Remove unnecesarry remount flag handling Kari Argillander
2021-09-07 15:35 ` [PATCH v4 3/9] fs/ntfs3: Convert mount options to pointer in sbi Kari Argillander
2021-09-07 15:35 ` [PATCH v4 4/9] fs/ntfs3: Use new api for mounting Kari Argillander
2021-09-07 15:35 ` [PATCH v4 5/9] fs/ntfs3: Init spi more in init_fs_context than fill_super Kari Argillander
2021-09-07 15:35 ` [PATCH v4 6/9] fs/ntfs3: Make mount option nohidden more universal Kari Argillander
2021-09-08 19:11   ` Pali Rohár
2021-09-07 15:35 ` [PATCH v4 7/9] fs/ntfs3: Add iocharset= mount option as alias for nls= Kari Argillander
2021-09-08 19:09   ` Pali Rohár
2021-10-09 11:42     ` Pali Rohár
2021-10-09 14:33       ` Kari Argillander
2021-10-11 17:14         ` Konstantin Komarov
2021-10-11 22:05           ` Kari Argillander [this message]
2021-09-07 15:35 ` [PATCH v4 8/9] fs/ntfs3: Rename mount option no_acs_rules > (no)acsrules Kari Argillander
2021-09-07 15:35 ` [PATCH v4 9/9] fs/ntfs3: Show uid/gid always in show_options() Kari Argillander
2021-09-09 16:39 ` [PATCH v4 0/9] fs/ntfs3: Use new mount api and change some opts Konstantin Komarov

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=20211011220524.azcaxs7y5zgd2uwz@kari-VirtualBox \
    --to=kari.argillander@gmail.com \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ntfs3@lists.linux.dev \
    --cc=pali@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=willy@infradead.org \
    --subject='Re: [PATCH v4 7/9] fs/ntfs3: Add iocharset= mount option as alias for nls=' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).