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

Isn't opt_default_exclude backwards? #127

Open
rzumer opened this issue Mar 19, 2024 · 1 comment · May be fixed by #128
Open

Isn't opt_default_exclude backwards? #127

rzumer opened this issue Mar 19, 2024 · 1 comment · May be fixed by #128

Comments

@rzumer
Copy link

rzumer commented Mar 19, 2024

if [ -n "$opt_default_exclude" ]
then
# Get a list of datasets for which snapshots are explicitly enabled.
CANDIDATES=$(echo "$ZFS_LIST" | awk -F '\t' \
'tolower($2) ~ /true/ || tolower($3) ~ /true/ {print $1}')
else
# Invert the NOAUTO list.
CANDIDATES=$(echo "$ZFS_LIST" | awk -F '\t' \
'tolower($2) !~ /false/ && tolower($3) !~ /false/ {print $1}')
fi

If com.sun:auto-snapshot is true, with opt_default_exclude enabled, then every label is enabled, even if com.sun:auto-snapshot:label is false.

If com.sun:auto-snapshot is false, with opt_default_exclude disabled, then even if com.sun:auto-snapshot:label is true, it will be disabled.

This contradicts the default behavior on Solaris as documented here (See: "Choose to only take snapshots under a given schedule for a dataset and all direct descendent datasets from the command line").

I'd expect the default behavior with opt_default_exclude disabled to enable a label if com.sun:auto-snapshot:label is not false and com.sun:auto-snapshot is not false or com.sun:auto-snapshot:label is true, even if com.sun:auto-snapshot is false. That second case is not handled right now, so if you want snapshots enabled by default, but have a label "whitelist" for a given dataset/pool, you have to explicitly disable every label you use instead of being able to disable snapshots globally and override that setting for some of the labels.

With it enabled it should probably disable every label unless both com.sun:auto-snapshot and com.sun:auto-snapshot:label are true.

@rzumer
Copy link
Author

rzumer commented Mar 19, 2024

Because of NOAUTO I think the behavior is correct for opt_default_exclude enabled, on second thought.

The default behavior is not exactly backwards but is missing the pattern where snapshots are disabled by default but specific labels are enabled explicitly, #128 should fix that. Hopefully this isn't something that was purposely excluded from this implementation.

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