Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-123279 / 23.10 / Improve the handling of mountpoint, sharesmb and sharenfs properties #167

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

usaleem-ix
Copy link

Motivation and Context

There are some inconsistencies in the handling of mountpoint, sharesmb and sharenfs
properties.

If mountpoint property is set when dataset is unmounted, this would update the mountpoint
property. The mountpoint could be valid or invalid in this case. Setting the mountpoint property
would result in success and dataset would still be unmounted here.

On the other hand, if dataset is mounted and mountpoint property is updated to something
invalid where mount cannot be successful, for example, setting the mountpoint inside a readonly
directory. This would unmount the dataset, set the mountpoint property to requested value and
tries to mount the dataset. The mount operation returns error and this error is treated as overall
failure of setting the property while the property is actually set.

For sharesmb and sharenfs properties, the status of setting the property is tied with whether
we succeed to share the dataset or not. In case sharing the dataset is not successful, this is
treated as overall failure of setting the property. In this case, if we check the property after the
failure, it is set to on.

For sharenfs property, if access list is provided, the syntax errors in access list/host addresses
are not validated until after setting the property during postfix phase while trying to share the
dataset. This is not correct, since the property has already been set when we reach there.

For TrueNAS SCALE, zfs-share.service executes zfs share -a on every boot and fails to
share over SMB or NFS if sharesmb and/or sharenfs are set, as we are handling shares in
middleware differently.

Description

For mountpoint property, to make the behavior consistent in case dataset is mounted or
unmounted, we should try to mount the dataset whenever mountpoint property is updated. This
would result in mounting the datasets if canmount property is set to on, regardless if the dataset
was previously unmounted.

The failure in mount operation while setting the mountpoint property should not be treated as
failure, since the property is actually set now to user requested value.

Similarly, for sharenfs and sharesmb properties, the status of setting the share properties
should not be returned as failure, when we fail to share the dataset.

For sharenfs property, syntax errors in access list/host addresses are validated while validating
the property list, before setting the property and failure is returned to user in this case when there
are errors in access list.

zfs-share.service is now disabled since middleware is responsible for sharing over SMB and
NFS.

How Has This Been Tested?

Manually tested by setting the properties and ran ZTS.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

zfs-share.service executes `zfs share` on every boot to share any
filesystem/s, that are shared over SMB and/or NFS using the
sharesmb and sharenfs properties.

Since we do not rely on these properties to share over SMB and NFS
and the service fails on boot on TrueNAS if sharesmb and/or
sharenfs properties are set, and we rely on middleware to control
the SMB and NFS shares, zfs-share.service should be disabled for
TrueNAS SCALE.

Signed-off-by: Umer Saleem <[email protected]>
There are some inconsistencies in the handling of mountpoint
property. This commit updates the behavior and makes it
consistent.

If mountpoint property is set when dataset is unmounted, this
would update the mountpoint property. The mountpoint could be
valid or invalid in this case. Setting the mountpoint property
would result in success in this case. Dataset would still be
unmounted here.

On the other hand, if dataset is mounted and mountpoint
property is updated to something invalid where mount cannot be
successful, for example, setting the mountpoint inside a readonly
directory. This would unmount the dataset, set the mountpoint
property to requested value and tries to mount the dataset. The
mount operation returns error and this error is treated as
overall failure of setting the property while the property is
actually set.

To make the behavior consistent in case dataset is mounted or
unmounted, we should try to mount the dataset whenever mountpoint
property is updated. This would result in mounting the datasets
if canmount property is set to on, regardless if the dataset was
previously unmounted.

The failure in mount operation while setting the mountpoint
property should not be treated as failure, since the property is
actually set now to user requested value.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Ameer Hamza <[email protected]>
Signed-off-by: Umer Saleem <[email protected]>
Closes openzfs#15240
For sharesmb and sharenfs properties, the status of setting the
property is tied with whether we succeed to share the dataset or
not. In case sharing the dataset is not successful, this is
treated as overall failure of setting the property. In this case,
if we check the property after the failure, it is set to on.

This commit updates this behavior and the status of setting the
share properties is not returned as failure, when we fail to
share the dataset.

For sharenfs property, if access list is provided, the syntax
errors in access list/host adresses are not validated until after
setting the property during postfix phase while trying to
share the dataset. This is not correct, since the property has
already been set when we reach there.

Syntax errors in access list/host addresses are validated while
validating the property list, before setting the property and
failure is returned to user in this case when there are errors
in access list.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Ameer Hamza <[email protected]>
Signed-off-by: Umer Saleem <[email protected]>
Closes openzfs#15240
@bugclerk
Copy link

@usaleem-ix usaleem-ix merged commit 22da5ae into truenas/zfs-2.2-release Sep 20, 2023
16 of 18 checks passed
@usaleem-ix usaleem-ix deleted the NAS-123279 branch September 20, 2023 14:05
@bugclerk
Copy link

Not updating JIRA ticket https://ixsystems.atlassian.net/browse/NAS-123279 target versions as no JIRA version corresponds to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants