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

Add 'zfs rename -u' to rename without remounting #10839

Merged
merged 1 commit into from
Sep 1, 2020

Conversation

ghost
Copy link

@ghost ghost commented Aug 28, 2020

Motivation and Context

A motivating example is to allow a boot environment to be renamed while active.

Description

Ported the change from FreeBSD to OpenZFS. On Linux we can use sb->s_op->show_devname to
keep the device name current.

Original commit message:

Allow to rename file systems without remounting if it is possible.
It is possible for file systems with 'mountpoint' preperty set to
'legacy' or 'none' - we don't have to change mount directory for them.
Currently such file systems are unmounted on rename and not even
mounted back.

This introduces layering violation, as we need to update
'f_mntfromname' field in statfs structure related to mountpoint (for
the dataset we are renaming and all its children).

In my opinion it is worth it, as it allow to update FreeBSD in even
cleaner way - in ZFS-only configuration root file system is ZFS file
system with 'mountpoint' property set to 'legacy'. If root dataset is
named system/rootfs, we can snapshot it (system/rootfs@upgrade), clone
it (system/oldrootfs), update FreeBSD and if it doesn't boot we can
boot back from system/oldrootfs and rename it back to system/rootfs
while it is mounted as /. Before it was not possible, because
unmounting / was not possible.

Authored by: Pawel Jakub Dawidek pjd@FreeBSD.org
Ported by: Matt Macy mmacy@freebsd.org
Signed-off-by: Ryan Moeller ryan@ixsystems.com

How Has This Been Tested?

Ran the added test on FreeBSD and Linux. Manually confirmed on Linux I can cd into a mounted dataset, then zfs rename fails and zfs rename -u succeeds. Also visually confirmed the mount command shows the expected output after renaming.

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

@codecov
Copy link

codecov bot commented Aug 29, 2020

Codecov Report

Merging #10839 into master will increase coverage by 0.05%.
The diff coverage is 91.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10839      +/-   ##
==========================================
+ Coverage   79.63%   79.69%   +0.05%     
==========================================
  Files         395      395              
  Lines      125066   125097      +31     
==========================================
+ Hits        99601    99699      +98     
+ Misses      25465    25398      -67     
Flag Coverage Δ
#kernel 80.42% <100.00%> (-0.03%) ⬇️
#user 65.63% <86.84%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/libzpool/kernel.c 64.54% <0.00%> (-0.15%) ⬇️
cmd/zfs/zfs_main.c 82.79% <81.25%> (-0.03%) ⬇️
lib/libzfs/libzfs_changelist.c 85.18% <100.00%> (+0.91%) ⬆️
lib/libzfs/libzfs_dataset.c 77.05% <100.00%> (+0.09%) ⬆️
module/os/linux/zfs/zfs_vfsops.c 78.49% <100.00%> (+0.02%) ⬆️
module/os/linux/zfs/zpl_super.c 85.08% <100.00%> (+1.12%) ⬆️
module/zfs/dsl_dir.c 89.30% <100.00%> (-0.10%) ⬇️
module/zfs/zfs_ioctl.c 86.43% <100.00%> (+0.04%) ⬆️
module/zcommon/zfs_fletcher.c 68.09% <0.00%> (-7.57%) ⬇️
module/zfs/vdev_rebuild.c 93.69% <0.00%> (-3.70%) ⬇️
... and 61 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b07c5a...c1c41ec. Read the comment docs.

module/zfs/dsl_dir.c Outdated Show resolved Hide resolved
@ghost ghost force-pushed the projects/zfs_renameflags branch from b43f7b3 to a5d969f Compare August 29, 2020 16:51
Allow to rename file systems without remounting if it is possible.
It is possible for file systems with 'mountpoint' preperty set to
'legacy' or 'none' - we don't have to change mount directory for them.
Currently such file systems are unmounted on rename and not even
mounted back.

This introduces layering violation, as we need to update
'f_mntfromname' field in statfs structure related to mountpoint (for
the dataset we are renaming and all its children).

In my opinion it is worth it, as it allow to update FreeBSD in even
cleaner way - in ZFS-only configuration root file system is ZFS file
system with 'mountpoint' property set to 'legacy'. If root dataset is
named system/rootfs, we can snapshot it (system/rootfs@upgrade), clone
it (system/oldrootfs), update FreeBSD and if it doesn't boot we can
boot back from system/oldrootfs and rename it back to system/rootfs
while it is mounted as /. Before it was not possible, because
unmounting / was not possible.

Authored by: Pawel Jakub Dawidek <pjd@FreeBSD.org>
Ported by: Matt Macy <mmacy@freebsd.org>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
@ghost ghost force-pushed the projects/zfs_renameflags branch from a5d969f to c1c41ec Compare August 29, 2020 16:53
@ghost
Copy link
Author

ghost commented Aug 29, 2020

  • Move include/os/{freebsd,linux}/zfs/sys/zfs_vfsops.h to include/os/{freebsd,linux}/zfs/sys/zfs_vfsops_os.h
  • Add include/sys/zfs_vfsops.h so libzpool can get a prototype for zfsvfs_update_fromname
  • Add a dummy zfsvfs_update_fromname in lib/libzpool/kernel.c
  • Remove ifdef _KERNEL around zfs_vfsops.h includes and call to zfsvfs_update_fromname

@behlendorf behlendorf requested a review from ahrens August 30, 2020 21:49
Copy link
Contributor

@allanjude allanjude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed By: Allan Jude allanjude@freebsd.org

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 1, 2020
@behlendorf behlendorf merged commit 7b4e272 into openzfs:master Sep 1, 2020
@ghost ghost deleted the projects/zfs_renameflags branch September 2, 2020 01:52
behlendorf pushed a commit that referenced this pull request Sep 3, 2020
Allow to rename file systems without remounting if it is possible.
It is possible for file systems with 'mountpoint' property set to
'legacy' or 'none' - we don't have to change mount directory for them.
Currently such file systems are unmounted on rename and not even
mounted back.

This introduces layering violation, as we need to update
'f_mntfromname' field in statfs structure related to mountpoint (for
the dataset we are renaming and all its children).

In my opinion it is worth it, as it allow to update FreeBSD in even
cleaner way - in ZFS-only configuration root file system is ZFS file
system with 'mountpoint' property set to 'legacy'. If root dataset is
named system/rootfs, we can snapshot it (system/rootfs@upgrade), clone
it (system/oldrootfs), update FreeBSD and if it doesn't boot we can
boot back from system/oldrootfs and rename it back to system/rootfs
while it is mounted as /. Before it was not possible, because
unmounting / was not possible.

Authored by: Pawel Jakub Dawidek <pjd@FreeBSD.org>
Reviewed-by: Allan Jude <allan@klarasystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Ported by: Matt Macy <mmacy@freebsd.org>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes #10839
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Allow to rename file systems without remounting if it is possible.
It is possible for file systems with 'mountpoint' property set to
'legacy' or 'none' - we don't have to change mount directory for them.
Currently such file systems are unmounted on rename and not even
mounted back.

This introduces layering violation, as we need to update
'f_mntfromname' field in statfs structure related to mountpoint (for
the dataset we are renaming and all its children).

In my opinion it is worth it, as it allow to update FreeBSD in even
cleaner way - in ZFS-only configuration root file system is ZFS file
system with 'mountpoint' property set to 'legacy'. If root dataset is
named system/rootfs, we can snapshot it (system/rootfs@upgrade), clone
it (system/oldrootfs), update FreeBSD and if it doesn't boot we can
boot back from system/oldrootfs and rename it back to system/rootfs
while it is mounted as /. Before it was not possible, because
unmounting / was not possible.

Authored by: Pawel Jakub Dawidek <pjd@FreeBSD.org>
Reviewed-by: Allan Jude <allan@klarasystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Ported by: Matt Macy <mmacy@freebsd.org>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10839
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
Allow to rename file systems without remounting if it is possible.
It is possible for file systems with 'mountpoint' property set to
'legacy' or 'none' - we don't have to change mount directory for them.
Currently such file systems are unmounted on rename and not even
mounted back.

This introduces layering violation, as we need to update
'f_mntfromname' field in statfs structure related to mountpoint (for
the dataset we are renaming and all its children).

In my opinion it is worth it, as it allow to update FreeBSD in even
cleaner way - in ZFS-only configuration root file system is ZFS file
system with 'mountpoint' property set to 'legacy'. If root dataset is
named system/rootfs, we can snapshot it (system/rootfs@upgrade), clone
it (system/oldrootfs), update FreeBSD and if it doesn't boot we can
boot back from system/oldrootfs and rename it back to system/rootfs
while it is mounted as /. Before it was not possible, because
unmounting / was not possible.

Authored by: Pawel Jakub Dawidek <pjd@FreeBSD.org>
Reviewed-by: Allan Jude <allan@klarasystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Ported by: Matt Macy <mmacy@freebsd.org>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants