open-iscsi / targetd

Remote configuration of a LIO-based storage appliance
GNU General Public License v3.0
71 stars 28 forks source link

NFS export parsing needs work #65

Closed tasleson closed 4 years ago

tasleson commented 4 years ago

From looking at the code it's apparent that the NFS export parsing needs more testing and corrections for handling all the NFS export file syntax.

tasleson commented 4 years ago

The sample NFS export file from man 5 exports raises an exception ValueError("Both RO & RW set")

       # sample /etc/exports file
       /               master(rw) trusty(rw,no_root_squash)
       /projects       proj*.local.domain(rw)
       /usr            *.local.domain(ro) @trusted(rw)
       /home/joe       pc001(rw,all_squash,anonuid=150,anongid=100)
       /pub            *(ro,insecure,all_squash)
       /srv/www        -sync,rw server @trusted @external(ro)
       /foo            2001:db8:9:e54::/64(rw) 192.0.2.0/24(rw)
       /build          buildhost[0-9].local.domain(rw)
tasleson commented 4 years ago

OK, so the problem with this example is:

/srv/www -sync,rw server @trusted @external(ro)

The -sync,rw are global options. The entry @external(ro) specifies read only. In the code we concat the global with the entry specific ro flag which causes us to raise an error. In this case I believe the ro overrides the rw but the logic is calling this out as an error as they seem to conflict.

I'm wondering if we should do any verification here and just let the nfs tools sort stuff out instead.

It's surprisingly difficult to understand code you wrote yourself 7+ years ago :-) Should be a hint to add comments which explain intent.