linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
To: "Pali Rohár" <pali@kernel.org>
Cc: "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dsterba@suse.cz" <dsterba@suse.cz>,
	"aaptel@suse.com" <aaptel@suse.com>,
	"willy@infradead.org" <willy@infradead.org>,
	"rdunlap@infradead.org" <rdunlap@infradead.org>,
	"joe@perches.com" <joe@perches.com>,
	"mark@harmstone.com" <mark@harmstone.com>,
	"nborisov@suse.com" <nborisov@suse.com>,
	"linux-ntfs-dev@lists.sourceforge.net" 
	<linux-ntfs-dev@lists.sourceforge.net>,
	"anton@tuxera.com" <anton@tuxera.com>
Subject: RE: [PATCH v11 00/10] NTFS read-write driver GPL implementation by Paragon Software
Date: Fri, 30 Oct 2020 15:51:10 +0000	[thread overview]
Message-ID: <5313baaad14c40d09738bf63e4659ac9@paragon-software.com> (raw)
In-Reply-To: <20201030152450.77mtzkxjove36qfd@pali>

From: Pali Rohár <pali@kernel.org>
Sent: Friday, October 30, 2020 6:25 PM
> To: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> Cc: linux-fsdevel@vger.kernel.org; viro@zeniv.linux.org.uk; linux-kernel@vger.kernel.org; dsterba@suse.cz; aaptel@suse.com;
> willy@infradead.org; rdunlap@infradead.org; joe@perches.com; mark@harmstone.com; nborisov@suse.com; linux-ntfs-
> dev@lists.sourceforge.net; anton@tuxera.com
> Subject: Re: [PATCH v11 00/10] NTFS read-write driver GPL implementation by Paragon Software
> 
> Hello and thanks for update!
> 
> I have just two comments for the last v11 version.
> 
> I really do not like nls_alt mount option and I do not think we should
> merge this mount option into ntfs kernel driver. Details I described in:
> https://lore.kernel.org/linux-fsdevel/20201009154734.andv4es3azkkskm5@pali/
> 
> tl;dr it is not systematic solution and is incompatible with existing
> in-kernel ntfs driver, also incompatible with in-kernel vfat, udf and
> ext4 (with UNICODE support) drivers. In my opinion, all kernel fs
> drivers which deals with UNICODE should handle it in similar way.
> 

Hello Pali! First of all, apologies for not providing a feedback on your previous
message regarding the 'nls_alt'. We had internal discussions on the topic and
overall conclusion is that: we do not want to compromise Kernel standards with
our submission. So we will remove the 'nls_alt' option in the next version.

However, there are still few points we have on the topic, please read below.

> It would be really bad if userspace application need to behave
> differently for this new ntfs driver and differently for all other
> UNICODE drivers.
> 

The option does not anyhow affect userspace applications. For the "default" example
of unzip/tar:
1 - if this option is not applied (e.g. "vfat case"), trying to unzip an archive with, e.g. CP-1251,
names inside to the target fs volume, will return error, and issued file(s) won't be unzipped;
2 - if this option is applied and "nls_alt" is set, the above case will result in unzipping all the files;

Also, this issue in general only applies to "non-native" filesystems. I.e. ext4 is not affected by it
in any case, as it just stores the name as bytes, no matter what those bytes are. The above case
won't give an unzip error on ext4. The only symptom of this would be, maybe, "incorrect encoding"
marking within the listing of such files (in File Manager or Terminal, e.g. in Ubuntu), but there won't
be an unzip process termination with incomplete unarchived fileset, unlike it is for vfat/exfat/ntfs
without "nls_alt".

> Second comment is simplification of usage nls_load() with UTF-8 parameter
> which I described in older email:
> https://lore.kernel.org/linux-fsdevel/948ac894450d494ea15496c2e5b8c906@paragon-software.com/
> 
> You wrote that you have applied it, but seems it was lost (maybe during
> rebase?) as it is not present in the last v11 version.
> 
> I suggested to not use nls_load() with UTF-8 at all. Your version of
> ntfs driver does not use kernel's nls utf8 module for UTF-8 support, so
> trying to load it should be avoided. Also kernel can be compiled without
> utf8 nls module (which is moreover broken) and with my above suggestion,
> ntfs driver would work correctly. Without that suggestion, mounting
> would fail.

Thanks for pointing that out. It is likely the "nls_load()" fixes were lost during rebase.
Will recheck it and return them to the v12.

Best regards!

  reply	other threads:[~2020-10-30 15:51 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-30 15:02 Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 01/10] fs/ntfs3: Add headers and misc files Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 02/10] fs/ntfs3: Add initialization of super block Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 03/10] fs/ntfs3: Add bitmap Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 04/10] fs/ntfs3: Add file operations and implementation Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 05/10] fs/ntfs3: Add attrib operations Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 06/10] fs/ntfs3: Add compression Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 07/10] fs/ntfs3: Add NTFS journal Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 08/10] fs/ntfs3: Add Kconfig, Makefile and doc Konstantin Komarov
2020-10-30 15:02 ` [PATCH v11 09/10] fs/ntfs3: Add NTFS3 in fs/Kconfig and fs/Makefile Konstantin Komarov
2020-10-31  1:23   ` kernel test robot
2020-11-02  8:36   ` [kbuild] " Dan Carpenter
2020-11-03  3:06   ` kernel test robot
2020-10-30 15:02 ` [PATCH v11 10/10] fs/ntfs3: Add MAINTAINERS Konstantin Komarov
2020-10-30 15:24 ` [PATCH v11 00/10] NTFS read-write driver GPL implementation by Paragon Software Pali Rohár
2020-10-30 15:51   ` Konstantin Komarov [this message]
2020-10-30 16:24     ` Pali Rohár
2020-10-30 16:41 ` Pali Rohár
2020-10-31  8:51   ` Christoph Hellwig
2020-10-31  2:42 ` Eric Biggers
2020-10-31  2:48   ` Eric Biggers

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=5313baaad14c40d09738bf63e4659ac9@paragon-software.com \
    --to=almaz.alexandrovich@paragon-software.com \
    --cc=aaptel@suse.com \
    --cc=anton@tuxera.com \
    --cc=dsterba@suse.cz \
    --cc=joe@perches.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-ntfs-dev@lists.sourceforge.net \
    --cc=mark@harmstone.com \
    --cc=nborisov@suse.com \
    --cc=pali@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --subject='RE: [PATCH v11 00/10] NTFS read-write driver GPL implementation by Paragon Software' \
    /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
on how to clone and mirror all data and code used for this inbox