Add options for specifying integrity metadata pre-allocations

Signed-off-by: mulhern <[email protected]>

Author: mulkieran

docs/stratis.txt

@@ -36,8 +36,10 @@ GLOBAL OPTIONS
 
 COMMANDS
 --------
-pool create [--key-desc <key_desc>] [--clevis <(nbde|tang|tpm2)> [--tang-url <tang_url>] [<(--thumbprint <thp> | --trust-url)>] [--no-overprovision] <pool_name> <blockdev> [<blockdev>..]::
+pool create [--key-desc <key_desc>] [--clevis <(nbde|tang|tpm2)> [--tang-url <tang_url>] [<(--thumbprint <thp> | --trust-url)>] [--no-overprovision] [--integrity <(no,pre-allocate)>] [--journal-size <journal_size>] [--tag-spec <tag_spec>] <pool_name> <blockdev> [<blockdev>..]::
      Create a pool from one or more block devices, with the given pool name.
+     The --tag-spec and --journal-size options are used to configure the amount
+     of space to reserve for integrity metadata.
 pool stop <(--uuid <uuid> |--name <name>)>::
      Stop a pool, specifying the pool by its UUID or by its name. Tear down
      the storage stack but leave all metadata intact.

src/stratis_cli/_actions/_introspect.py

@@ -13,6 +13,9 @@
       <arg name="devices" type="as" direction="in" />
       <arg name="key_desc" type="(bs)" direction="in" />
       <arg name="clevis_info" type="(b(ss))" direction="in" />
+      <arg name="journal_size" type="(bt)" direction="in" />
+      <arg name="tag_spec" type="(bs)" direction="in" />
+      <arg name="allocate_superblock" type="(bb)" direction="in" />
       <arg name="result" type="(b(oao))" direction="out" />
       <arg name="return_code" type="q" direction="out" />
       <arg name="return_string" type="s" direction="out" />

src/stratis_cli/_actions/_pool.py

@@ -25,7 +25,7 @@
 # isort: THIRDPARTY
 from justbytes import Range
 
-from .._constants import Id, IdType, UnlockMethod
+from .._constants import Id, IdType, IntegrityOption, IntegrityTagSpec, UnlockMethod
 from .._error_codes import PoolErrorCode
 from .._errors import (
     StratisCliEngineError,
@@ -180,6 +180,16 @@ def create_pool(namespace):  # pylint: disable=too-many-locals
             None if namespace.clevis is None else ClevisInfo.get_info(namespace.clevis)
         )
 
+        (journal_size, tag_spec, allocate_superblock) = (
+            ((True, 0), (True, IntegrityTagSpec.B0), (True, False))
+            if namespace.integrity.integrity is IntegrityOption.NO
+            else (
+                (True, namespace.integrity.journal_size.magnitude.numerator),
+                (True, namespace.integrity.tag_spec),
+                (True, True),
+            )
+        )
+
         (
             (changed, (pool_object_path, _)),
             return_code,
@@ -199,10 +209,13 @@ def create_pool(namespace):  # pylint: disable=too-many-locals
                     if clevis_info is None
                     else (True, (clevis_info.pin, json.dumps(clevis_info.config)))
                 ),
+                "journal_size": journal_size,
+                "tag_spec": tag_spec,
+                "allocate_superblock": allocate_superblock,
             },
         )
 
-        if return_code != StratisdErrors.OK:  # pragma: no cover
+        if return_code != StratisdErrors.OK:
             raise StratisCliEngineError(return_code, message)
 
         if not changed:  # pragma: no cover

src/stratis_cli/_constants.py

@@ -116,3 +116,28 @@ class Clevis(Enum):
 
     def __str__(self):
         return self.value
+
+
+class IntegrityTagSpec(Enum):
+    """
+    Options for specifying integrity tag size.
+    """
+
+    B0 = "0b"
+    B32 = "32b"
+    B512 = "512b"
+
+    def __str__(self):
+        return self.value
+
+
+class IntegrityOption(Enum):
+    """
+    Options for specifying integrity choices on create.
+    """
+
+    NO = "no"
+    PRE_ALLOCATE = "pre-allocate"
+
+    def __str__(self):
+        return self.value

src/stratis_cli/_parser/_pool.py

@@ -20,12 +20,22 @@
 from argparse import SUPPRESS, ArgumentTypeError
 from uuid import UUID
 
+# isort: THIRDPARTY
+from justbytes import MiB, Range
+
 from .._actions import BindActions, PoolActions
-from .._constants import Clevis, EncryptionMethod, UnlockMethod, YesOrNo
+from .._constants import (
+    Clevis,
+    EncryptionMethod,
+    IntegrityOption,
+    IntegrityTagSpec,
+    UnlockMethod,
+    YesOrNo,
+)
 from .._error_codes import PoolErrorCode
 from ._bind import BIND_SUBCMDS, REBIND_SUBCMDS
 from ._debug import POOL_DEBUG_SUBCMDS
-from ._range import RejectAction
+from ._range import DefaultAction, RejectAction, parse_range
 
 
 class ClevisEncryptionOptions:  # pylint: disable=too-few-public-methods
@@ -81,6 +91,62 @@ def verify(self, namespace, parser):
         namespace.clevis = None if self.clevis is None else self
 
 
+class IntegrityOptions:  # pylint: disable=too-few-public-methods
+    """
+    Gathers and verifies integrity options.
+    """
+
+    def __init__(self, namespace):
+        self.integrity = copy.copy(namespace.integrity)
+        del namespace.integrity
+
+        self.journal_size = copy.copy(namespace.journal_size)
+        del namespace.journal_size
+
+        self.journal_size_default = getattr(namespace, "journal_size_default", True)
+
+        self.tag_spec = copy.copy(namespace.tag_spec)
+        del namespace.tag_spec
+
+        self.tag_spec_default = getattr(namespace, "tag_spec_default", True)
+
+    def verify(self, namespace, parser):
+        """
+        Do supplementary parsing of conditional arguments.
+        """
+
+        if self.integrity is IntegrityOption.NO and not self.journal_size_default:
+            parser.error(
+                f'--integrity value "{IntegrityOption.NO}" and --journal-size '
+                "option can not be specified together"
+            )
+
+        if self.integrity is IntegrityOption.NO and not self.tag_spec_default:
+            parser.error(
+                f'--integrity value "{IntegrityOption.NO}" and --tag-spec '
+                "option can not be specified together"
+            )
+
+        namespace.integrity = self
+
+
+class CreateOptions:  # pylint: disable=too-few-public-methods
+    """
+    Gathers and verifies options specified on pool create.
+    """
+
+    def __init__(self, namespace):
+        self.clevis_encryption_options = ClevisEncryptionOptions(namespace)
+        self.integrity_options = IntegrityOptions(namespace)
+
+    def verify(self, namespace, parser):
+        """
+        Verify that create command-line is formed correctly.
+        """
+        self.clevis_encryption_options.verify(namespace, parser)
+        self.integrity_options.verify(namespace, parser)
+
+
 def _ensure_nat(arg):
     """
     Raise error if argument is not an natural number.
@@ -110,7 +176,7 @@ def _ensure_nat(arg):
                                 "--post-parser",
                                 {
                                     "action": RejectAction,
-                                    "default": ClevisEncryptionOptions,
+                                    "default": CreateOptions,
                                     "help": SUPPRESS,
                                     "nargs": "?",
                                 },
@@ -163,7 +229,75 @@ def _ensure_nat(arg):
                             )
                         ],
                     },
-                )
+                ),
+                (
+                    "integrity",
+                    {
+                        "description": (
+                            "Optional parameters for configuring integrity "
+                            "metadata pre-allocation"
+                        ),
+                        "args": [
+                            (
+                                "--integrity",
+                                {
+                                    "help": (
+                                        "Integrity options for this pool. If "
+                                        f'"{IntegrityOption.NO}" no space will '
+                                        "be allocated for integrity metadata "
+                                        "and it will never be possible to turn "
+                                        "on integrity functionality for this "
+                                        "pool. "
+                                        f'If "{IntegrityOption.PRE_ALLOCATE}" '
+                                        "then space will be allocated for "
+                                        "integrity metadata and it will be "
+                                        "possible to switch on integrity "
+                                        "functionality in future. The default "
+                                        'is "%(default)s".'
+                                    ),
+                                    "choices": list(IntegrityOption),
+                                    "default": IntegrityOption.PRE_ALLOCATE,
+                                    "type": IntegrityOption,
+                                },
+                            ),
+                            (
+                                "--journal-size",
+                                {
+                                    "help": (
+                                        "Size of integrity device's journal. "
+                                        "Each block is written to this journal "
+                                        "before being written to its address. "
+                                        "The default is %(default)s."
+                                    ),
+                                    "action": DefaultAction,
+                                    "default": Range(128, MiB),
+                                    "type": parse_range,
+                                },
+                            ),
+                            (
+                                "--tag-spec",
+                                {
+                                    "help": (
+                                        "Integrity tag specification defining "
+                                        "the size of the tag used to store a "
+                                        "checksum or other value for each "
+                                        "block on a device. All size "
+                                        "specifications are in bits. The "
+                                        "default is %(default)s."
+                                    ),
+                                    "action": DefaultAction,
+                                    "default": IntegrityTagSpec.B512,
+                                    "choices": [
+                                        x
+                                        for x in list(IntegrityTagSpec)
+                                        if x is not IntegrityTagSpec.B0
+                                    ],
+                                    "type": IntegrityTagSpec,
+                                },
+                            ),
+                        ],
+                    },
+                ),
             ],
             "args": [
                 ("pool_name", {"help": "Name of new pool"}),

src/stratis_cli/_parser/_range.py

@@ -88,3 +88,13 @@ def __call__(self, parser, namespace, values, option_string=None):
         raise argparse.ArgumentError(
             self, f"Option {option_string} can not be assigned to or set."
         )
+
+
+class DefaultAction(argparse.Action):
+    """
+    Detect if the default value was set.
+    """
+
+    def __call__(self, parser, namespace, values, option_string=None):
+        setattr(namespace, self.dest, values)
+        setattr(namespace, self.dest + "_default", False)

tests/whitebox/integration/pool/test_create.py

@@ -18,6 +18,7 @@
 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
 from stratis_cli._errors import (
+    StratisCliEngineError,
     StratisCliInUseSameTierError,
     StratisCliNameConflictError,
 )
@@ -165,3 +166,22 @@ def test_no_overprovision(self):
             self._MENU + [self._POOLNAME] + self._DEVICES + ["--no-overprovision"]
         )
         TEST_RUNNER(command_line)
+
+
+class Create7TestCase(SimTestCase):
+    """
+    Test create with integrity options.
+    """
+
+    _MENU = ["--propagate", "pool", "create"]
+    _DEVICES = _DEVICE_STRATEGY()
+    _POOLNAME = "thispool"
+
+    def test_invalid_journal_size(self):
+        """
+        Test creating with an invalid journal size.
+        """
+        command_line = (
+            self._MENU + [self._POOLNAME] + self._DEVICES + ["--journal-size=131079KiB"]
+        )
+        self.check_error(StratisCliEngineError, command_line, _ERROR)

tests/whitebox/integration/test_parser.py

@@ -23,7 +23,7 @@
 _PARSE_ERROR = StratisCliErrorCodes.PARSE_ERROR
 
 
-class ParserTestCase(RunTestCase):
+class ParserTestCase(RunTestCase):  # pylint: disable=too-many-public-methods
     """
     Test parser behavior. The behavior should be identical, regardless of
     whether the "--propagate" flag is set. That is, stratis should never produce
@@ -221,6 +221,52 @@ def test_create_with_post_parser_set(self):
         for prefix in [[], ["--propagate"]]:
             self.check_system_exit(prefix + command_line, _PARSE_ERROR)
 
+    def test_create_with_bad_tag_value(self):
+        """
+        Verify that an unrecognized tag value causes an error.
+        """
+        command_line = [
+            "pool",
+            "create",
+            "pn",
+            "/dev/n",
+            "--tag-spec=512",
+        ]
+        for prefix in [[], ["--propagate"]]:
+            self.check_system_exit(prefix + command_line, _PARSE_ERROR)
+
+    def test_create_with_integrity_no_journal_size(self):
+        """
+        Verify that creating with integrity = no plus good journal-size
+        results in a parse error.
+        """
+        command_line = [
+            "pool",
+            "create",
+            "pn",
+            "/dev/n",
+            "--integrity=no",
+            "--journal-size=128MiB",
+        ]
+        for prefix in [[], ["--propagate"]]:
+            self.check_system_exit(prefix + command_line, _PARSE_ERROR)
+
+    def test_create_with_integrity_no_tag_spec(self):
+        """
+        Verify that creating with integrity = no plus good tag-size
+        results in a parse error.
+        """
+        command_line = [
+            "pool",
+            "create",
+            "pn",
+            "/dev/n",
+            "--integrity=no",
+            "--tag-spec=32b",
+        ]
+        for prefix in [[], ["--propagate"]]:
+            self.check_system_exit(prefix + command_line, _PARSE_ERROR)
+
     def test_stratis_list_filesystem_with_name_no_pool(self):
         """
         We want to get a parse error if filesystem UUID is specified but no