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

Clarify documentation of zfs destroy on snapshots #17021

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

mnrx
Copy link
Contributor

@mnrx mnrx commented Feb 3, 2025

Motivation and Context

The current documentation of zfs destroy in application to snapshots is particularly difficult to understand.

Description

The current manual page reads as follows:

zfs destroy [-Rdnprv] filesystem|volume@snap[%snap[,snap[%snap]]]…

The given snapshots are destroyed immediately if and only if the zfs destroy command without the -d option would have destroyed it. Such immediate destruction would occur, for example, if the snapshot had no clones and the user-initiated reference count were zero.

If a snapshot does not qualify for immediate destruction, it is marked for deferred deletion. In this state, it exists as a usable, visible snapshot until both of the preconditions listed above are met, at which point it is destroyed.

...

-d: Destroy immediately. If a snapshot cannot be destroyed now, mark it for deferred destruction.

I think there are a few problems:

  • The first sentence attempts to document the default operation of zfs destroy by referring to the "zfs destroy command without the -d option". This creates some kind of circular reference, made doubly confusing by the poor description of that option.
  • The presence of "for example" in the second sentence implies that the list of reasons is not complete, but it is, as far as I can tell. If there are other reasons, they should also be listed.
  • No mention is made of properties defer_destroy and userrefs. This may be intentional, but to point them out seems like useful information. If defer_destroy can be unset, that too could be mentioned.
  • "Destroy immediately" is particularly confusing. Immediate destruction is always enabled—this option alters the behaviour of zfs destroy when destruction is not possible. I would guess that -d originally stood for defer, but the current description makes no mention of that.

This PR introduces the following replacement:

zfs destroy [-Rdnprv] filesystem|volume@snap[%snap[,snap[%snap]]]…

Attempts to destroy the given snapshot(s). This will fail if any clones of the snapshot exist or if the snapshot is held. In this case, by default, zfs destroy will have no effect and exit in error. If the -d option is applied, the command will instead mark the given snapshot for automatic destruction as soon as it becomes eligible. While marked for destruction, a snapshot remains visible, and the user may create new clones from it and place new holds on it.

The read-only snapshot properties defer_destroy and userrefs are used by zfs destroy to determine eligibility and marked status.

...

-d: Rather than exiting if the given snapshot is ineligible for destruction, mark it for deferred, automatic destruction once it becomes eligible.

How Has This Been Tested?

Reviewed modified documentation with mandoc.

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:

man/man8/zfs-destroy.8 Outdated Show resolved Hide resolved
@amotin amotin added the Status: Code Review Needed Ready for review and testing label Feb 3, 2025
The current documentation of `zfs destroy` in application to snapshots
is particularly difficult to understand. The following changes are made:

- Remove circular reference to `zfs destroy` in the documentation of
  that command.
- Remove use of "for example", which implies there are more,
  undocumented reasons that ZFS may fail to destroy a snapshot
  immediately.
- Mention properties `defer_destroy` and `userrefs`.
- Add `zfsprops(8)` to "SEE ALSO" list.
- Clarify meaning of `-d` option.

Signed-off-by: mnrx <[email protected]>
Co-authored-by: Alexander Motin <[email protected]>
Requires-builders: none
@tonyhutter tonyhutter merged commit 0be1da2 into openzfs:master Feb 5, 2025
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants