Differential D21795

ZFS: add bookmark renaming
ClosedPublic
Actions

Authored by avg on Sep 25 2019, 8:24 PM.

Details

Reviewers

mahrens

Group Reviewers

manpages

Commits

rS353037: ZFS: add bookmark renaming

Summary

The feature is implemented as an extension of the existing ZFS_IOC_RENAME
ioctl. Both the userland and the DSL interfaces support renaming only a
single bookmark at a time. As of now, there is no ZCP interface to the new
functionality. I am going to add it once the DSL interface passes a test of
time.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 26792
Build 25130: arc lint + arc unit

Event Timeline

avg created this revision.Sep 25 2019, 8:24 PM

Herald added a reviewer: manpages. · View Herald TranscriptSep 25 2019, 8:24 PM

Herald added subscribers: delphij, imp. · View Herald Transcript

Harbormaster completed remote builds in B26702: Diff 62570.Sep 25 2019, 8:24 PM

allanjude added a subscriber: allanjude.Sep 25 2019, 8:28 PM

the interfaces look good to me.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_bookmark.c
552–562	I think we should do this in dsl_bookmark_rename_check (assuming it's necessary - I didn't see similar namecheck() calls for renaming snapshots, although maybe that's a bug?). This will help when making the channel program function, which would also want to do the same checks (name length at least).

OK from manpages.

avg marked an inline comment as done.Sep 30 2019, 2:08 PM

avg added inline comments.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_bookmark.c
552–562	I added some of these checks before adding ENTITY_NAME check because there was nothing to ensure name validity back then. I think that now we can be sure that the dataset name and the old bookmark name must be valid. So, I think that only the new bookmark name and its full length need to be checked now. I will move the needed checks to the check function and drop the redundant checks. Thanks!