Remove img_handlers config parameter usage

Now that the only disk code requiring host filesystem
mounts is the LXC filesystem setup, the img_handlers
config parameter is an solution looking for a purpose.
For any disk image format we can categorically say
whether it should be mounted using loop or qemu-nbd
mount impls. There is no desire to use libguestfs FUSE
for mounting LXC disk images, since users of LXC do
not want a huge KVM process alongside every container!
Move the mount class choice logic into the mount API,
by creating new methods

  nova.virt.disk.mount.API.instance_for_format()
  nova.virt.disk.mount.API.instance_for_device()

and then call these from nova.virt.disk.API code

blueprint: virt-disk-api-refactoring
Change-Id: I088b5debdf62d4a9a6b4521eee68cd4757f2ff42
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

5 files changed, 44 insertions, 61 deletions

diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py
index 2c5dd4734..0cede2197 100644
--- a/nova/tests/test_virt.py
+++ b/nova/tests/test_virt.py
@@ -107,7 +107,6 @@ class TestVirtDisk(test.TestCase):
                 '/mnt/loop/part': '/dev/mapper/loop0p1',
                 '/mnt/nbd/nopart': '/dev/nbd15',
                 '/mnt/nbd/part': '/dev/mapper/nbd15p1',
-                '/mnt/guestfs': 'guestmount',
             }
             return mount_points[mount_point]
 
@@ -141,11 +140,4 @@ class TestVirtDisk(test.TestCase):
                               ('qemu-nbd', '-d', '/dev/nbd15'),
                              ]
 
-        disk_api.destroy_container('/mnt/guestfs')
-        expected_commands += [
-                              ('fusermount', '-u', '/mnt/guestfs'),
-                             ]
-        # It's not worth trying to match the last timeout command
-        self.executes.pop()
-
         self.assertEqual(self.executes, expected_commands)
diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py
index 33773e59c..9d9d672d7 100644
--- a/nova/virt/disk/api.py
+++ b/nova/virt/disk/api.py
@@ -37,9 +37,7 @@ from nova.openstack.common import cfg
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
 from nova import utils
-from nova.virt.disk.mount import guestfs
-from nova.virt.disk.mount import loop
-from nova.virt.disk.mount import nbd
+from nova.virt.disk.mount import api as mount
 from nova.virt.disk.vfs import api as vfs
 from nova.virt import images
 
@@ -50,9 +48,6 @@ disk_opts = [
     cfg.StrOpt('injected_network_template',
                default='$pybasedir/nova/virt/interfaces.template',
                help='Template file for injected network'),
-    cfg.ListOpt('img_handlers',
-                default=['loop', 'nbd', 'guestfs'],
-                help='Order of methods used to mount disk images'),
 
     # NOTE(yamahata): ListOpt won't work because the command may include a
     #                 comma. For example:
@@ -174,25 +169,14 @@ class _DiskImage(object):
         self.image = image
         self.partition = partition
         self.mount_dir = mount_dir
+        self.use_cow = use_cow
 
         # Internal
         self._mkdir = False
         self._mounter = None
         self._errors = []
 
-        # As a performance tweak, don't bother trying to
-        # directly loopback mount a cow image.
-        self.handlers = CONF.img_handlers[:]
-        if use_cow and 'loop' in self.handlers:
-            self.handlers.remove('loop')
-
-        if not self.handlers:
-            msg = _('no capable image handler configured')
-            raise exception.NovaException(msg)
-
         if mount_dir:
-            # Note the os.path.ismount() shortcut doesn't
-            # work with libguestfs due to permissions issues.
             device = self._device_for_path(mount_dir)
             if device:
                 self._reset(device)
@@ -211,12 +195,10 @@ class _DiskImage(object):
 
     def _reset(self, device):
         """Reset internal state for a previously mounted directory."""
-        mounter_cls = self._handler_class(device=device)
-        mounter = mounter_cls(image=self.image,
-                              partition=self.partition,
-                              mount_dir=self.mount_dir,
-                              device=device)
-        self._mounter = mounter
+        self._mounter = mount.Mount.instance_for_device(self.image,
+                                                        self.mount_dir,
+                                                        self.partition,
+                                                        device)
 
         mount_name = os.path.basename(self.mount_dir or '')
         self._mkdir = mount_name.startswith(self.tmp_prefix)
@@ -226,17 +208,6 @@ class _DiskImage(object):
         """Return the collated errors from all operations."""
         return '\n--\n'.join([''] + self._errors)
 
-    @staticmethod
-    def _handler_class(mode=None, device=None):
-        """Look up the appropriate class to use based on MODE or DEVICE."""
-        for cls in (loop.LoopMount, nbd.NbdMount, guestfs.GuestFSMount):
-            if mode and cls.mode == mode:
-                return cls
-            elif device and cls.device_id_string in device:
-                return cls
-        msg = _("no disk image handler for: %s") % mode or device
-        raise exception.NovaException(msg)
-
     def mount(self):
         """Mount a disk image, using the object attributes.
 
@@ -252,21 +223,19 @@ class _DiskImage(object):
             self.mount_dir = tempfile.mkdtemp(prefix=self.tmp_prefix)
             self._mkdir = True
 
-        try:
-            for h in self.handlers:
-                mounter_cls = self._handler_class(h)
-                mounter = mounter_cls(image=self.image,
-                                      partition=self.partition,
-                                      mount_dir=self.mount_dir)
-                if mounter.do_mount():
-                    self._mounter = mounter
-                    break
-                else:
-                    LOG.debug(mounter.error)
-                    self._errors.append(mounter.error)
-        finally:
-            if not self._mounter:
-                self.umount()  # rmdir
+        imgfmt = "raw"
+        if self.use_cow:
+            imgfmt = "qcow2"
+
+        mounter = mount.Mount.instance_for_format(self.image,
+                                                  self.mount_dir,
+                                                  self.partition,
+                                                  imgfmt)
+        if mounter.do_mount():
+            self._mounter = mounter
+        else:
+            LOG.debug(mounter.error)
+            self._errors.append(mounter.error)
 
         return bool(self._mounter)
 
@@ -275,6 +244,7 @@ class _DiskImage(object):
         try:
             if self._mounter:
                 self._mounter.do_umount()
+                self._mounter = None
         finally:
             if self._mkdir:
                 os.rmdir(self.mount_dir)
diff --git a/nova/virt/disk/mount/api.py b/nova/virt/disk/mount/api.py
index e683658d2..b63aa3300 100644
--- a/nova/virt/disk/mount/api.py
+++ b/nova/virt/disk/mount/api.py
@@ -17,6 +17,7 @@
 
 import os
 
+from nova.openstack.common import importutils
 from nova.openstack.common import log as logging
 from nova import utils
 
@@ -30,7 +31,29 @@ class Mount(object):
     to be called in that order.
     """
 
-    mode = device_id_string = None  # to be overridden in subclasses
+    mode = None  # to be overridden in subclasses
+
+    @staticmethod
+    def instance_for_format(imgfile, mountdir, partition, imgfmt):
+        if imgfmt == "raw":
+            return importutils.import_object(
+                "nova.virt.disk.mount.loop.LoopMount",
+                imgfile, mountdir, partition)
+        else:
+            return importutils.import_object(
+                "nova.virt.disk.mount.nbd.NbdMount",
+                imgfile, mountdir, partition)
+
+    @staticmethod
+    def instance_for_device(imgfile, mountdir, partition, device):
+        if "loop" in device:
+            return importutils.import_object(
+                "nova.virt.disk.mount.loop.LoopMount",
+                imgfile, mountdir, partition, device)
+        else:
+            return importutils.import_object(
+                "nova.virt.disk.mount.nbd.NbdMount",
+                imgfile, mountdir, partition, device)
 
     def __init__(self, image, mount_dir, partition=None, device=None):
 
diff --git a/nova/virt/disk/mount/loop.py b/nova/virt/disk/mount/loop.py
index 9b87b6df5..d6c53cbce 100644
--- a/nova/virt/disk/mount/loop.py
+++ b/nova/virt/disk/mount/loop.py
@@ -22,7 +22,6 @@ from nova.virt.disk.mount import api
 class LoopMount(api.Mount):
     """loop back support for raw images."""
     mode = 'loop'
-    device_id_string = mode
 
     def get_dev(self):
         out, err = utils.trycmd('losetup', '--find', '--show', self.image,
diff --git a/nova/virt/disk/mount/nbd.py b/nova/virt/disk/mount/nbd.py
index 01d8b66b5..4a27d4f75 100644
--- a/nova/virt/disk/mount/nbd.py
+++ b/nova/virt/disk/mount/nbd.py
@@ -39,7 +39,6 @@ CONF.register_opts(nbd_opts)
 class NbdMount(api.Mount):
     """qemu-nbd support disk images."""
     mode = 'nbd'
-    device_id_string = mode
 
     # NOTE(padraig): There are three issues with this nbd device handling
     #  1. max_nbd_devices should be inferred (#861504)