From a5e07cf5068aaa71a906c4b54b94e98c6aeef97e Mon Sep 17 00:00:00 2001
From: Christian Kellner <christian@kellner.me>
Date: Mon, 18 Oct 2021 15:50:30 +0200
Subject: [PATCH] devices: introduce new device manager class

Introduce a new class to manage devices, `DeviceManger` and move the
code to open devices from the `Device` here. The main insight of why
the logic should be place here is that certain information is needed
to open the devices, independently of specific type: the path to the
device node directory, `devpath`, the actual `tree` and the service
manager instance to start the actual service. Instead of passing all
this information again and again to the `Device` class, we now have
a specialized (service) manager class for devices that has all the
needed information all the time. Additionally, the special handling
of parent devices is moved from the pipeline to the service manager,
which is where it belongs.
This will make even more sense for mounts, where the `DeviceManger`
can then be passed to access the individual devices.
Port the test to use the `DeviceManager`.
---
 osbuild/devices.py       | 32 +++++++++++++++++++++++++++-----
 osbuild/pipeline.py      | 11 ++++-------
 test/run/test_devices.py |  3 ++-
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/osbuild/devices.py b/osbuild/devices.py
index 1e40160f..09225bbf 100644
--- a/osbuild/devices.py
+++ b/osbuild/devices.py
@@ -44,21 +44,43 @@ class Device:
         m.update(json.dumps(self.options, sort_keys=True).encode())
         return m.hexdigest()
 
-    def open(self, mgr: host.ServiceManager, dev: str, parent: str, tree: str) -> Dict:
+
+class DeviceManager:
+    """Manager for Devices
+
+    Uses a `host.ServiceManager` to open `Device` instances.
+    """
+
+    def __init__(self, mgr: host.ServiceManager, devpath: str, tree: str) -> Dict:
+        self.service_manager = mgr
+        self.devpath = devpath
+        self.tree = tree
+        self.devices = {}
+
+    def open(self, dev: Device) -> Dict:
+
+        if dev.parent:
+            parent = self.devices[dev.parent.name]["path"]
+        else:
+            parent = None
+
         args = {
             # global options
-            "dev": dev,
-            "tree": tree,
+            "dev": self.devpath,
+            "tree": self.tree,
 
             "parent": parent,
 
             # per device options
-            "options": self.options,
+            "options": dev.options,
         }
 
-        client = mgr.start(f"device/{self.name}", self.info.path)
+        mgr = self.service_manager
+
+        client = mgr.start(f"device/{dev.name}", dev.info.path)
         res = client.call("open", args)
 
+        self.devices[dev.name] = res
         return res
 
 
diff --git a/osbuild/pipeline.py b/osbuild/pipeline.py
index 507f529a..3533cb02 100644
--- a/osbuild/pipeline.py
+++ b/osbuild/pipeline.py
@@ -10,7 +10,7 @@ from . import buildroot
 from . import host
 from . import objectstore
 from . import remoteloop
-from .devices import Device
+from .devices import Device, DeviceManager
 from .inputs import Input
 from .mounts import Mount
 from .sources import Source
@@ -164,12 +164,9 @@ class Stage:
                 data = ip.map(mgr, storeapi, inputs_tmpdir)
                 inputs[key] = data
 
-            for key, dev in self.devices.items():
-                parent = None
-                if dev.parent:
-                    parent = devices[dev.parent.name]["path"]
-                reply = dev.open(mgr, build_root.dev, parent, tree)
-                devices[key] = reply
+            devmgr = DeviceManager(mgr, build_root.dev, tree)
+            for name, dev in self.devices.items():
+                devices[name] = devmgr.open(dev)
 
             for key, mount in self.mounts.items():
                 relpath = devices[mount.device.name]["path"]
diff --git a/test/run/test_devices.py b/test/run/test_devices.py
index 80b25c89..2293867b 100755
--- a/test/run/test_devices.py
+++ b/test/run/test_devices.py
@@ -48,7 +48,8 @@ def test_loopback_basic(tmpdir):
     dev = devices.Device("loop", info, None, options)
 
     with host.ServiceManager() as mgr:
-        reply = dev.open(mgr, devpath, None, tree)
+        devmgr = devices.DeviceManager(mgr, devpath, tree)
+        reply = devmgr.open(dev)
         assert reply
         assert reply["path"]