LVM-nvmet: Use nvmetcli as library instead of CLI

The nvmet target was using the nvmetcli command to manage the NVMe-oF
targets, but the command has a big limitation when it comes to
controlling it's behaviour through the command line, as it can only
restore all its configuration and small parts, such as ports or
subsystems, cannot be directly modified without entering into the
interactive mode.

Due to this limitation the nvmet target would:

- Save current nvmet configuration
- Make changes to the json data
- Restore the updated configuration

The problem with this process, besides being slow because it runs a CLI
command and uses temporary files, is that the restoration completely
deletes EVERYTHING, before recreating it again.  This means that all
hosts that are connected to volumes will suddenly experience a
disconnect to the volumes (because the namespace and subsystems have
disappeared) and will keep retrying to connect.  The reconnect succeeds
after the configuration has been restored by nvmet, but that's 10 to 20
seconds that hosts cannot access volumes (this may block things in VMs)
and will present nnvme kernel log error messages.

To fix all these issues, speed and disconnect, this patch stops using
the nvmetcli as a CLI and uses it as a Python library, since that's the
most feature rich functionality of nvmetcli.

Querying the nvmet system can be done directly with the library, but to
make changes (create/destroy ports, subsystems, namespaces) it requires
privileges, so this patch adds a privsep wrapper for the operations that
we use and cannot be done as a normal user.

The nvmet wrapper doesn't provide privsep support for ALL operations,
only for those that we currently use.

Due to the client-server architecture of privsep and nvmet using non
primitive instances as parameters, the privsep wrapper needs custom
serialization code to pass these instances.

As a side effect of the refactoring we also fix a bug were we tried to
create the port over and over again on each create_export call which
resulted in nvme kernel warning logs.

Closes-Bug: #1964391
Closes-Bug: #1964394
Change-Id: Icae9802713867fa148bc041c86beb010086dacc9

3 files changed, 209 insertions, 56 deletions

diff --git a/cinder/privsep/nvmcli.py b/cinder/privsep/nvmcli.py
deleted file mode 100644
index 95f5b1c56..000000000
--- a/cinder/privsep/nvmcli.py
+++ /dev/null
@@ -1,41 +0,0 @@
-# Copyright 2018 Red Hat, Inc
-# Copyright 2017 Rackspace Australia
-# Copyright 2018 Michael Still and Aptira
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""
-Helpers for nvmetcli related routines.
-"""
-
-from oslo_concurrency import processutils
-
-import cinder.privsep
-
-
-@cinder.privsep.sys_admin_pctxt.entrypoint
-def restore(path):
-    cmd = [
-        'nvmetcli',
-        'restore',
-        path]
-    return processutils.execute(*cmd)
-
-
-@cinder.privsep.sys_admin_pctxt.entrypoint
-def save(path):
-    cmd = [
-        'nvmetcli',
-        'save',
-        path]
-    return processutils.execute(*cmd)
diff --git a/cinder/privsep/path.py b/cinder/privsep/path.py
index 8ab98e54c..6fd7405fd 100644
--- a/cinder/privsep/path.py
+++ b/cinder/privsep/path.py
@@ -25,21 +25,6 @@ import cinder.privsep
 
 
 @cinder.privsep.sys_admin_pctxt.entrypoint
-def readfile(path):
-    if not os.path.exists(path):
-        raise exception.FileNotFound(file_path=path)
-    with open(path, 'r') as f:
-        return f.read()
-
-
-@cinder.privsep.sys_admin_pctxt.entrypoint
-def removefile(path):
-    if not os.path.exists(path):
-        raise exception.FileNotFound(file_path=path)
-    os.unlink(path)
-
-
-@cinder.privsep.sys_admin_pctxt.entrypoint
 def touch(path):
     if os.path.exists(path):
         os.utime(path, None)
diff --git a/cinder/privsep/targets/nvmet.py b/cinder/privsep/targets/nvmet.py
new file mode 100644
index 000000000..9b2f5c8f7
--- /dev/null
+++ b/cinder/privsep/targets/nvmet.py
@@ -0,0 +1,209 @@
+# Copyright 2022 Red Hat, Inc
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+"""NVMet Python Interface using privsep
+
+This file adds the privsep support to the nvmet package so it can be easily
+consumed by Cinder nvmet target.
+
+It also:
+
+- Adds some methods to the Root class to be able to get a specific subsystem or
+  port directly without having to go through all the existing ones.
+
+- Presents the CFSNotFound exception as a NotFound exception which is easier to
+  consume.
+"""
+import os
+
+import nvmet
+from oslo_log import log as logging
+
+from cinder import exception
+from cinder import privsep
+
+
+LOG = logging.getLogger(__name__)
+
+
+###################
+# Helper methods to serialize/deserialize parameters to be sent through privsep
+# and to do the instance/class calls on the privsep side.
+
+def serialize(instance):
+    """Serialize parameters, specially nvmet instances.
+
+    The idea is to be able to pass an nvmet instance to privsep methods, since
+    they are sometimes required as parameters (ie: port.setup) and also to pass
+    the instance where do_privsep_call has to call a specific method.
+
+    Instances are passed as a tuple, with the name of the class as the first
+    element, and in the second element the kwargs necessary to instantiate the
+    instance of that class.
+
+    To differentiate nvmet instances from tuples there is a 'tuple' value that
+    can be passed in the first element of the tuple to differentiate them.
+
+    All other instances as passed unaltered.
+    """
+    if isinstance(instance, nvmet.Root):
+        return ('Root', {})
+
+    if isinstance(instance, (nvmet.Subsystem, nvmet.Host)):
+        return (type(instance).__name__, {'nqn': instance.nqn,
+                                          'mode': 'lookup'})
+
+    if isinstance(instance, nvmet.Namespace):
+        return ('Namespace', {'nsid': instance.nsid,
+                              'subsystem': serialize(instance.subsystem),
+                              'mode': 'lookup'})
+
+    if isinstance(instance, nvmet.Port):
+        return ('Port', {'portid': instance.portid, 'mode': 'lookup'})
+
+    if isinstance(instance, nvmet.Referral):
+        return ('Referral', {'name': instance.name,
+                             'port': serialize(instance.port),
+                             'mode': 'lookup'})
+
+    if isinstance(instance, nvmet.ANAGroup):
+        return ('ANAGroup', {'grpid': instance.grpid,
+                             'port': serialize(instance.port),
+                             'mode': 'lookup'})
+
+    if isinstance(instance, tuple):
+        return ('tuple', instance)
+
+    return instance
+
+
+def deserialize(data):
+    """Deserialize an instance, specially nvmet instances.
+
+    Reverse operation of the serialize method.  Converts an nvmet instance
+    serialized in a tuple into an actual nvmet instance.
+    """
+    if not isinstance(data, tuple):
+        return data
+
+    cls_name, cls_params = data
+    if cls_name == 'tuple':
+        return cls_params
+
+    # Parameters for the instantiation of the class can be nvmet objects
+    # themselves.
+    params = {name: deserialize(value) for name, value in cls_params.items()}
+    # We don't want the classes from the nvmet method but ours instead
+    instance = getattr(nvmet, cls_name)(**params)
+    return instance
+
+
+def deserialize_params(args, kwargs):
+    """Deserialize function arguments using deserialize method."""
+    args = [deserialize(arg) for arg in args]
+    kwargs = {key: deserialize(value) for key, value in kwargs.items()}
+    return args, kwargs
+
+
+def _nvmet_setup_failure(message):
+    """Simple error method to use when calling nvmet setup methods."""
+    LOG.error(message)
+    raise exception.CinderException(message)
+
+
+@privsep.sys_admin_pctxt.entrypoint
+def do_privsep_call(instance, method_name, *args, **kwargs):
+    """General privsep method for instance calls.
+
+    Handle privsep method calls by deserializing the instance where we want to
+    call a given method with the deserialized parameters.
+    """
+    LOG.debug('Calling %s on %s with %s - %s',
+              method_name, instance, args, kwargs)
+    instance = deserialize(instance)
+    method = getattr(instance, method_name)
+    args, kwargs = deserialize_params(args, kwargs)
+    # NOTE: No returning nvmet objects support. If needed add serialization on
+    #       the result and deserialization decorator before the entrypoint.
+    return method(*args, **kwargs)
+
+
+@privsep.sys_admin_pctxt.entrypoint
+def privsep_setup(cls_name, *args, **kwargs):
+    """Special privsep method for nvmet setup method calls.
+
+    The setup method is a special case because it's a class method (which
+    privsep cannot handle) and also requires a function for the error handling.
+
+    This method accepts a class name and reconstructs it, then calls the
+    class' setup method passing our own error function.
+    """
+    LOG.debug('Setup %s with %s - %s', cls_name, args, kwargs)
+    cls = getattr(nvmet, cls_name)
+    args, kwargs = deserialize_params(args, kwargs)
+    kwargs['err_func'] = _nvmet_setup_failure
+    cls.setup(*args, **kwargs)
+
+
+###################
+# Classes that don't currently have privsep support
+
+Namespace = nvmet.Namespace
+Host = nvmet.Host
+Referral = nvmet.Referral
+ANAGroup = nvmet.ANAGroup
+
+
+###################
+# Custom classes that divert privileges calls to privsep
+# Support in these classes is limited to what's needed by the nvmet target.
+
+# Convenience error class link to nvmet's
+NotFound = nvmet.nvme.CFSNotFound
+
+
+class Subsystem(nvmet.Subsystem):
+    def __init__(self, nqn=None, mode='lookup'):
+        super().__init__(nqn=nqn, mode=mode)
+
+    @classmethod
+    def setup(cls, t):
+        privsep_setup(cls.__name__, t)
+
+    def delete(self):
+        do_privsep_call(serialize(self), 'delete')
+
+
+class Port(nvmet.Port):
+    def __init__(self, portid, mode='lookup'):
+        super().__init__(portid=portid, mode=mode)
+
+    @classmethod
+    def setup(cls, root, n):
+        privsep_setup(cls.__name__, serialize(root), n)
+
+    def add_subsystem(self, nqn):
+        do_privsep_call(serialize(self), 'add_subsystem', nqn)
+
+    def remove_subsystem(self, nqn):
+        do_privsep_call(serialize(self), 'remove_subsystem', nqn)
+
+    def delete(self):
+        do_privsep_call(serialize(self), 'delete')
+
+
+class Root(nvmet.Root):
+    @property
+    def ports(self):
+        for d in os.listdir(self.path + '/ports/'):
+            yield Port(os.path.basename(d))