Implement bookmark copying

This feature allows copying existing bookmarks using

    zfs bookmark fs#target fs#newbookmark

There are some niche use cases for such functionality,
e.g. when using bookmarks as markers for replication progress.

Copying redaction bookmarks produces a normal bookmark that
cannot be used for redacted send (we are not duplicating
the redaction object).

ZCP support for bookmarking (both creation and copying) will be
implemented in a separate patch based on this work.

Overview:

- Terminology:
    - source = existing snapshot or bookmark
    - new/bmark = new bookmark
- Implement bookmark copying in `dsl_bookmark.c`
  - create new bookmark node
  - copy source's `zbn_phys` to new's `zbn_phys`
  - zero-out redaction object id in copy
- Extend existing bookmark ioctl nvlist schema to accept
  bookmarks as sources
  - => `dsl_bookmark_create_nvl_validate` is authoritative
- use `dsl_dataset_is_before` check for both snapshot
  and bookmark sources
- Adjust CLI
  - refactor shortname expansion logic in `zfs_do_bookmark`
- Update man pages
  - warn about redaction bookmark handling
- Add test cases
  - CLI
  - pyyzfs libzfs_core bindings

Reviewed-by: Matt Ahrens <[email protected]>
Reviewed-by: Paul Dagnelie <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Christian Schwarz <[email protected]>
Closes #9571

5 files changed, 113 insertions, 25 deletions

diff --git a/contrib/pyzfs/libzfs_core/_constants.py b/contrib/pyzfs/libzfs_core/_constants.py
index 55de55d42..16057e9b3 100644
--- a/contrib/pyzfs/libzfs_core/_constants.py
+++ b/contrib/pyzfs/libzfs_core/_constants.py
@@ -22,11 +22,15 @@ from __future__ import absolute_import, division, print_function
 
 
 # https://stackoverflow.com/a/1695250
-def enum(*sequential, **named):
-    enums = dict(((b, a) for a, b in enumerate(sequential)), **named)
+def enum_with_offset(offset, sequential, named):
+    enums = dict(((b, a + offset) for a, b in enumerate(sequential)), **named)
     return type('Enum', (), enums)
 
 
+def enum(*sequential, **named):
+    return enum_with_offset(0, sequential, named)
+
+
 #: Maximum length of any ZFS name.
 MAXNAMELEN = 255
 #: Default channel program limits
@@ -60,12 +64,34 @@ zio_encrypt = enum(
     'ZIO_CRYPT_AES_256_GCM'
 )
 # ZFS-specific error codes
-ZFS_ERR_CHECKPOINT_EXISTS = 1024
-ZFS_ERR_DISCARDING_CHECKPOINT = 1025
-ZFS_ERR_NO_CHECKPOINT = 1026
-ZFS_ERR_DEVRM_IN_PROGRESS = 1027
-ZFS_ERR_VDEV_TOO_BIG = 1028
-ZFS_ERR_WRONG_PARENT = 1033
+zfs_errno = enum_with_offset(1024, [
+        'ZFS_ERR_CHECKPOINT_EXISTS',
+        'ZFS_ERR_DISCARDING_CHECKPOINT',
+        'ZFS_ERR_NO_CHECKPOINT',
+        'ZFS_ERR_DEVRM_IN_PROGRESS',
+        'ZFS_ERR_VDEV_TOO_BIG',
+        'ZFS_ERR_IOC_CMD_UNAVAIL',
+        'ZFS_ERR_IOC_ARG_UNAVAIL',
+        'ZFS_ERR_IOC_ARG_REQUIRED',
+        'ZFS_ERR_IOC_ARG_BADTYPE',
+        'ZFS_ERR_WRONG_PARENT',
+        'ZFS_ERR_FROM_IVSET_GUID_MISSING',
+        'ZFS_ERR_FROM_IVSET_GUID_MISMATCH',
+        'ZFS_ERR_SPILL_BLOCK_FLAG_MISSING',
+        'ZFS_ERR_UNKNOWN_SEND_STREAM_FEATURE',
+        'ZFS_ERR_EXPORT_IN_PROGRESS',
+        'ZFS_ERR_BOOKMARK_SOURCE_NOT_ANCESTOR',
+    ],
+    {}
+)
+# compat before we used the enum helper for these values
+ZFS_ERR_CHECKPOINT_EXISTS = zfs_errno.ZFS_ERR_CHECKPOINT_EXISTS
+assert(ZFS_ERR_CHECKPOINT_EXISTS == 1024)
+ZFS_ERR_DISCARDING_CHECKPOINT = zfs_errno.ZFS_ERR_DISCARDING_CHECKPOINT
+ZFS_ERR_NO_CHECKPOINT = zfs_errno.ZFS_ERR_NO_CHECKPOINT
+ZFS_ERR_DEVRM_IN_PROGRESS = zfs_errno.ZFS_ERR_DEVRM_IN_PROGRESS
+ZFS_ERR_VDEV_TOO_BIG = zfs_errno.ZFS_ERR_VDEV_TOO_BIG
+ZFS_ERR_WRONG_PARENT = zfs_errno.ZFS_ERR_WRONG_PARENT
 
 
 # vim: softtabstop=4 tabstop=4 expandtab shiftwidth=4
diff --git a/contrib/pyzfs/libzfs_core/_error_translation.py b/contrib/pyzfs/libzfs_core/_error_translation.py
index cf52ac918..b97bdd89a 100644
--- a/contrib/pyzfs/libzfs_core/_error_translation.py
+++ b/contrib/pyzfs/libzfs_core/_error_translation.py
@@ -39,7 +39,8 @@ from ._constants import (
     ZFS_ERR_NO_CHECKPOINT,
     ZFS_ERR_DEVRM_IN_PROGRESS,
     ZFS_ERR_VDEV_TOO_BIG,
-    ZFS_ERR_WRONG_PARENT
+    ZFS_ERR_WRONG_PARENT,
+    zfs_errno
 )
 
 
@@ -147,21 +148,36 @@ def lzc_destroy_snaps_translate_errors(ret, errlist, snaps, defer):
 
 
 def lzc_bookmark_translate_errors(ret, errlist, bookmarks):
+
     if ret == 0:
         return
 
     def _map(ret, name):
+        source = bookmarks[name]
         if ret == errno.EINVAL:
             if name:
-                snap = bookmarks[name]
                 pool_names = map(_pool_name, bookmarks.keys())
-                if not _is_valid_bmark_name(name):
-                    return lzc_exc.BookmarkNameInvalid(name)
-                elif not _is_valid_snap_name(snap):
-                    return lzc_exc.SnapshotNameInvalid(snap)
-                elif _fs_name(name) != _fs_name(snap):
-                    return lzc_exc.BookmarkMismatch(name)
-                elif any(x != _pool_name(name) for x in pool_names):
+
+                # use _validate* functions for MAXNAMELEN check
+                try:
+                    _validate_bmark_name(name)
+                except lzc_exc.ZFSError as e:
+                    return e
+
+                try:
+                    _validate_snap_name(source)
+                    source_is_snap = True
+                except lzc_exc.ZFSError:
+                    source_is_snap = False
+                try:
+                    _validate_bmark_name(source)
+                    source_is_bmark = True
+                except lzc_exc.ZFSError:
+                    source_is_bmark = False
+                if not source_is_snap and not source_is_bmark:
+                    return lzc_exc.BookmarkSourceInvalid(source)
+
+                if any(x != _pool_name(name) for x in pool_names):
                     return lzc_exc.PoolsDiffer(name)
             else:
                 invalid_names = [
@@ -174,6 +190,8 @@ def lzc_bookmark_translate_errors(ret, errlist, bookmarks):
             return lzc_exc.SnapshotNotFound(name)
         if ret == errno.ENOTSUP:
             return lzc_exc.BookmarkNotSupported(name)
+        if ret == zfs_errno.ZFS_ERR_BOOKMARK_SOURCE_NOT_ANCESTOR:
+            return lzc_exc.BookmarkMismatch(source)
         return _generic_exception(ret, name, "Failed to create bookmark")
 
     _handle_err_list(
diff --git a/contrib/pyzfs/libzfs_core/_libzfs_core.py b/contrib/pyzfs/libzfs_core/_libzfs_core.py
index 06797b0f3..fcfa5be31 100644
--- a/contrib/pyzfs/libzfs_core/_libzfs_core.py
+++ b/contrib/pyzfs/libzfs_core/_libzfs_core.py
@@ -319,14 +319,15 @@ def lzc_bookmark(bookmarks):
     Create bookmarks.
 
     :param bookmarks: a dict that maps names of wanted bookmarks to names of
-        existing snapshots.
+        existing snapshots or bookmarks.
     :type bookmarks: dict of bytes to bytes
     :raises BookmarkFailure: if any of the bookmarks can not be created for any
         reason.
 
     The bookmarks `dict` maps from name of the bookmark
     (e.g. :file:`{pool}/{fs}#{bmark}`) to the name of the snapshot
-    (e.g. :file:`{pool}/{fs}@{snap}`).  All the bookmarks and snapshots must
+    (e.g. :file:`{pool}/{fs}@{snap}`) or existint bookmark
+    :file:`{pool}/{fs}@{snap}`. All the bookmarks and snapshots must
     be in the same pool.
     '''
     errlist = {}
diff --git a/contrib/pyzfs/libzfs_core/exceptions.py b/contrib/pyzfs/libzfs_core/exceptions.py
index f8a775433..2206c2f2c 100644
--- a/contrib/pyzfs/libzfs_core/exceptions.py
+++ b/contrib/pyzfs/libzfs_core/exceptions.py
@@ -227,7 +227,15 @@ class BookmarkNotFound(ZFSError):
 
 class BookmarkMismatch(ZFSError):
     errno = errno.EINVAL
-    message = "Bookmark is not in snapshot's filesystem"
+    message = "source is not an ancestor of the new bookmark's dataset"
+
+    def __init__(self, name):
+        self.name = name
+
+
+class BookmarkSourceInvalid(ZFSError):
+    errno = errno.EINVAL
+    message = "Bookmark source is not a valid snapshot or existing bookmark"
 
     def __init__(self, name):
         self.name = name
diff --git a/contrib/pyzfs/libzfs_core/test/test_libzfs_core.py b/contrib/pyzfs/libzfs_core/test/test_libzfs_core.py
index 613d5eccd..f47583b83 100644
--- a/contrib/pyzfs/libzfs_core/test/test_libzfs_core.py
+++ b/contrib/pyzfs/libzfs_core/test/test_libzfs_core.py
@@ -1032,17 +1032,37 @@ class ZFSTest(unittest.TestCase):
         bmarks = [ZFSTest.pool.makeName(
             b'fs1#bmark1'), ZFSTest.pool.makeName(b'fs2#bmark1')]
         bmark_dict = {x: y for x, y in zip(bmarks, snaps)}
-
         lzc.lzc_snapshot(snaps)
         lzc.lzc_bookmark(bmark_dict)
         lzc.lzc_destroy_snaps(snaps, defer=False)
 
     @skipUnlessBookmarksSupported
+    def test_bookmark_copying(self):
+        snaps = [ZFSTest.pool.makeName(s) for s in [
+            b'fs1@snap1', b'fs1@snap2', b'fs2@snap1']]
+        bmarks = [ZFSTest.pool.makeName(x) for x in [
+            b'fs1#bmark1', b'fs1#bmark2', b'fs2#bmark1']]
+        bmarks_copies = [ZFSTest.pool.makeName(x) for x in [
+            b'fs1#bmark1_copy', b'fs1#bmark2_copy', b'fs2#bmark1_copy']]
+        bmark_dict = {x: y for x, y in zip(bmarks, snaps)}
+        bmark_copies_dict = {x: y for x, y in zip(bmarks_copies, bmarks)}
+
+        for snap in snaps:
+            lzc.lzc_snapshot([snap])
+        lzc.lzc_bookmark(bmark_dict)
+
+        lzc.lzc_bookmark(bmark_copies_dict)
+        lzc.lzc_destroy_bookmarks(bmarks_copies)
+
+        lzc.lzc_destroy_bookmarks(bmarks)
+        lzc.lzc_destroy_snaps(snaps, defer=False)
+
+    @skipUnlessBookmarksSupported
     def test_bookmarks_empty(self):
         lzc.lzc_bookmark({})
 
     @skipUnlessBookmarksSupported
-    def test_bookmarks_mismatching_name(self):
+    def test_bookmarks_foregin_source(self):
         snaps = [ZFSTest.pool.makeName(b'fs1@snap1')]
         bmarks = [ZFSTest.pool.makeName(b'fs2#bmark1')]
         bmark_dict = {x: y for x, y in zip(bmarks, snaps)}
@@ -1107,7 +1127,7 @@ class ZFSTest(unittest.TestCase):
             self.assertIsInstance(e, lzc_exc.NameTooLong)
 
     @skipUnlessBookmarksSupported
-    def test_bookmarks_mismatching_names(self):
+    def test_bookmarks_foreign_sources(self):
         snaps = [ZFSTest.pool.makeName(
             b'fs1@snap1'), ZFSTest.pool.makeName(b'fs2@snap1')]
         bmarks = [ZFSTest.pool.makeName(
@@ -1122,7 +1142,7 @@ class ZFSTest(unittest.TestCase):
             self.assertIsInstance(e, lzc_exc.BookmarkMismatch)
 
     @skipUnlessBookmarksSupported
-    def test_bookmarks_partially_mismatching_names(self):
+    def test_bookmarks_partially_foreign_sources(self):
         snaps = [ZFSTest.pool.makeName(
             b'fs1@snap1'), ZFSTest.pool.makeName(b'fs2@snap1')]
         bmarks = [ZFSTest.pool.makeName(
@@ -1154,33 +1174,48 @@ class ZFSTest(unittest.TestCase):
 
     @skipUnlessBookmarksSupported
     def test_bookmarks_missing_snap(self):
+        fss = [ZFSTest.pool.makeName(b'fs1'), ZFSTest.pool.makeName(b'fs2')]
         snaps = [ZFSTest.pool.makeName(
             b'fs1@snap1'), ZFSTest.pool.makeName(b'fs2@snap1')]
         bmarks = [ZFSTest.pool.makeName(
             b'fs1#bmark1'), ZFSTest.pool.makeName(b'fs2#bmark1')]
         bmark_dict = {x: y for x, y in zip(bmarks, snaps)}
 
-        lzc.lzc_snapshot(snaps[0:1])
+        lzc.lzc_snapshot(snaps[0:1])  # only create fs1@snap1
+
         with self.assertRaises(lzc_exc.BookmarkFailure) as ctx:
             lzc.lzc_bookmark(bmark_dict)
 
         for e in ctx.exception.errors:
             self.assertIsInstance(e, lzc_exc.SnapshotNotFound)
 
+        # no new bookmarks are created if one or more sources do not exist
+        for fs in fss:
+            fsbmarks = lzc.lzc_get_bookmarks(fs)
+            self.assertEqual(len(fsbmarks), 0)
+
     @skipUnlessBookmarksSupported
     def test_bookmarks_missing_snaps(self):
+        fss = [ZFSTest.pool.makeName(b'fs1'), ZFSTest.pool.makeName(b'fs2')]
         snaps = [ZFSTest.pool.makeName(
             b'fs1@snap1'), ZFSTest.pool.makeName(b'fs2@snap1')]
         bmarks = [ZFSTest.pool.makeName(
             b'fs1#bmark1'), ZFSTest.pool.makeName(b'fs2#bmark1')]
         bmark_dict = {x: y for x, y in zip(bmarks, snaps)}
 
+        # do not create any snapshots
+
         with self.assertRaises(lzc_exc.BookmarkFailure) as ctx:
             lzc.lzc_bookmark(bmark_dict)
 
         for e in ctx.exception.errors:
             self.assertIsInstance(e, lzc_exc.SnapshotNotFound)
 
+        # no new bookmarks are created if one or more sources do not exist
+        for fs in fss:
+            fsbmarks = lzc.lzc_get_bookmarks(fs)
+            self.assertEqual(len(fsbmarks), 0)
+
     @skipUnlessBookmarksSupported
     def test_bookmarks_for_the_same_snap(self):
         snap = ZFSTest.pool.makeName(b'fs1@snap1')