Received-SPF: softfail (google.com: domain of transitioning linux-nfs-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <929b0a83-7a10-8a26-941f-3819c957ba7a@redhat.com>
Date:   Mon, 2 May 2022 18:46:59 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH 0/5] nfs-utils: Improving NFS re-exports
Content-Language: en-US
To:     "J. Bruce Fields" <bfields@fieldses.org>,
        Richard Weinberger <richard@nod.at>
Cc:     linux-nfs@vger.kernel.org, david@sigma-star.at,
        luis.turcitu@appsbroker.com, david.young@appsbroker.com,
        david.oberhollenzer@sigma-star.at, trond.myklebust@hammerspace.com,
        anna.schumaker@netapp.com, chris.chilvers@appsbroker.com
References: <20220502085045.13038-1-richard@nod.at>
 <20220502161713.GI30550@fieldses.org>
From:   Steve Dickson <steved@redhat.com>
In-Reply-To: <20220502161713.GI30550@fieldses.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 5/2/22 12:17 PM, J. Bruce Fields wrote:
> On Mon, May 02, 2022 at 10:50:40AM +0200, Richard Weinberger wrote:
>> This is the first non-RFC iteration of the NFS re-export
>> improvement series for nfs-utils.
>> While the kernel side[0] didn't change at all and is still small,
>> the userspace side saw much more changes.
>>
>> The core idea is adding new export option: reeport=
>> Using reexport= it is possible to mark an export entry in the exports
>> file explicitly as NFS re-export and select a strategy how unique
>> identifiers should be provided.
>> Currently two strategies are supported, "auto-fsidnum" and
>> "predefined-fsidnum", both use a SQLite database as backend to keep
>> track of generated ids.
>> For a more detailed description see patch "exports: Implement new export option reexport=".
>> I choose SQLite because nfs-utils already uses it and using SQL ids can nicely
>> generated and maintained. It will also scale for large setups where the amount
>> of subvolumes is high.
>>
>> Beside of id generation this series also addresses the reboot problem.
>> If the re-exporting NFS server reboots, uncovered NFS subvolumes are not yet
>> mounted and file handles become stale.
>> Now mountd/exportd keeps track of uncovered subvolumes and makes sure they get
>> uncovered while nfsd starts.
>>
>> The whole set of features is currently opt-in via --enable-reexport.
> 
> Can we remove that option before upstreaming?
> 
> For testing purposes it may makes sense to be able to turn the new code
> on and off quickly.  But for something we're really going to support,
> it's just another hurdle for users to jump through, and another case we
> probably won't remember to test.  The export options themselves should
> be enough configuration.
> 
> Anyway, basically sounds reasonable to me.  I'll try to give it a proper
> review sometime, feel free to bug me if I don't get to it in a week or
> so.
How about --disable-reexport? So it is on by default to help testing
but if there are issues we can turn it off...

steved.

> 
> --b.
> 
> 
>> I'm also not sure about the rearrangement of the reexport code,
>> currently it is a helper library.
>>
>> A typical export entry on a re-exporting server looks like:
>> 	/nfs *(rw,no_root_squash,no_subtree_check,crossmnt,reexport=auto-fsidnum)
>> reexport=auto-fsidnum will automatically assign an fsid= to /nfs and all
>> uncovered subvolumes.
>>
>> Richard Weinberger (5):
>>    Implement reexport helper library
>>    exports: Implement new export option reexport=
>>    export: Implement logic behind reexport=
>>    export: Avoid fsid= conflicts
>>    reexport: Make state database location configurable
>>
>> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/log/?h=nfs_reexport_clean
>>
>>   configure.ac                   |  12 ++
>>   nfs.conf                       |   3 +
>>   support/Makefile.am            |   4 +
>>   support/export/Makefile.am     |   2 +
>>   support/export/cache.c         |  71 ++++++-
>>   support/export/export.c        |  27 ++-
>>   support/include/nfslib.h       |   1 +
>>   support/nfs/Makefile.am        |   1 +
>>   support/nfs/exports.c          |  68 +++++++
>>   support/reexport/Makefile.am   |   6 +
>>   support/reexport/reexport.c    | 354 +++++++++++++++++++++++++++++++++
>>   support/reexport/reexport.h    |  39 ++++
>>   systemd/Makefile.am            |   4 +
>>   systemd/nfs-server-generator.c |  14 +-
>>   systemd/nfs.conf.man           |   6 +
>>   utils/exportd/Makefile.am      |   8 +-
>>   utils/exportd/exportd.c        |   5 +
>>   utils/exportfs/Makefile.am     |   6 +
>>   utils/exportfs/exportfs.c      |  21 +-
>>   utils/exportfs/exports.man     |  31 +++
>>   utils/mount/Makefile.am        |   7 +
>>   utils/mountd/Makefile.am       |   6 +
>>   utils/mountd/mountd.c          |   1 +
>>   utils/mountd/svc_run.c         |   6 +
>>   24 files changed, 690 insertions(+), 13 deletions(-)
>>   create mode 100644 support/reexport/Makefile.am
>>   create mode 100644 support/reexport/reexport.c
>>   create mode 100644 support/reexport/reexport.h
>>
>> -- 
>> 2.31.1
>