Rewrote image search

This code is more generic and support all cases. Previously
we had bug where the user lost his image path if the image
was not located in image directory.

gns3server/modules/base_manager.py

@@ -371,3 +371,40 @@ class BaseManager:
             nio = NIOGenericEthernet(nio_settings["ethernet_device"])
         assert nio is not None
         return nio
+
+    def get_abs_image_path(self, path):
+        """
+        Get the absolute path of an image
+
+        :param path: file path
+        :return: file path
+        """
+
+        img_directory = self.get_images_directory()
+
+        if not os.path.isabs(path):
+            s = os.path.split(path)
+            return os.path.normpath(os.path.join(img_directory, *s))
+        return path
+
+    def get_relative_image_path(self, path):
+        """
+        Get a path relative to images directory path
+        or an abspath if the path is not located inside
+        image directory
+
+        :param path: file path
+        :return: file path
+        """
+
+        img_directory = self.get_images_directory()
+        path = self.get_abs_image_path(path)
+        if os.path.dirname(path) == img_directory:
+            return os.path.basename(path)
+        return path
+
+    def get_images_directory(self):
+        """
+        Get the image directory on disk
+        """
+        raise NotImplementedError

gns3server/modules/dynamips/__init__.py

@@ -616,3 +616,9 @@ class Dynamips(BaseManager):
             if was_auto_started:
                 yield from vm.stop()
         return validated_idlepc
+
+    def get_images_directory(self):
+        """
+        Return the full path of the images directory on disk
+        """
+        return os.path.join(os.path.expanduser(self.config.get_section_config("Server").get("images_path", "~/GNS3/images")), "IOS")

gns3server/modules/dynamips/nodes/router.py

@@ -151,10 +151,7 @@ class Router(BaseVM):
                        "system_id": self._system_id}
 
         # return the relative path if the IOS image is in the images_path directory
-        server_config = self.manager.config.get_section_config("Server")
-        relative_image = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "IOS", self._image)
-        if os.path.exists(relative_image):
-            router_info["image"] = os.path.basename(self._image)
+        router_info["image"] = self.manager.get_relative_image_path(self._image)
 
         # add the slots
         slot_number = 0
@@ -427,9 +424,7 @@ class Router(BaseVM):
         :param image: path to IOS image file
         """
 
-        if not os.path.isabs(image):
-            server_config = self.manager.config.get_section_config("Server")
-            image = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "IOS", image)
+        image = self.manager.get_abs_image_path(image)
 
         if not os.path.isfile(image):
             raise DynamipsError("IOS image '{}' is not accessible".format(image))

gns3server/modules/iou/__init__.py

@@ -91,3 +91,9 @@ class IOU(BaseManager):
         """
 
         return os.path.join("iou", "device-{}".format(legacy_vm_id))
+
+    def get_images_directory(self):
+        """
+        Return the full path of the images directory on disk
+        """
+        return os.path.join(os.path.expanduser(self.config.get_section_config("Server").get("images_path", "~/GNS3/images")), "IOU")

gns3server/modules/iou/iou_vm.py

@@ -145,14 +145,7 @@ class IOUVM(BaseVM):
         :param path: path to the IOU image executable
         """
 
-        if not os.path.isabs(path):
-            server_config = self.manager.config.get_section_config("Server")
-            relative_path = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), path)
-            if not os.path.exists(relative_path):
-                relative_path = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "IOU", path)
-            path = relative_path
-
-        self._path = path
+        self._path = self.manager.get_abs_image_path(path)
 
         # In 1.2 users uploaded images to the images roots
         # after the migration their images are inside images/IOU
@@ -240,11 +233,7 @@ class IOUVM(BaseVM):
                        "iourc_path": self.iourc_path}
 
         # return the relative path if the IOU image is in the images_path directory
-        server_config = self.manager.config.get_section_config("Server")
-        relative_image = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "IOU", self.path)
-        if os.path.exists(relative_image):
-            iou_vm_info["path"] = os.path.basename(self.path)
-
+        iou_vm_info["path"] = self.manager.get_relative_image_path(self.path)
         return iou_vm_info
 
     @property

gns3server/modules/qemu/__init__.py

@@ -112,3 +112,9 @@ class Qemu(BaseManager):
         """
 
         return os.path.join("qemu", "vm-{}".format(legacy_vm_id))
+
+    def get_images_directory(self):
+        """
+        Return the full path of the images directory on disk
+        """
+        return os.path.join(os.path.expanduser(self.config.get_section_config("Server").get("images_path", "~/GNS3/images")), "QEMU")

gns3server/modules/qemu/qemu_vm.py

@@ -148,14 +148,10 @@ class QemuVM(BaseVM):
         :param hda_disk_image: QEMU hda disk image path
         """
 
-        if not os.path.isabs(hda_disk_image):
-            server_config = self.manager.config.get_section_config("Server")
-            hda_disk_image = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "QEMU", hda_disk_image)
-
+        self._hda_disk_image = self.manager.get_abs_image_path(hda_disk_image)
         log.info('QEMU VM "{name}" [{id}] has set the QEMU hda disk image path to {disk_image}'.format(name=self._name,
                                                                                                        id=self._id,
-                                                                                                       disk_image=hda_disk_image))
-        self._hda_disk_image = hda_disk_image
+                                                                                                       disk_image=self._hda_disk_image))
 
     @property
     def hdb_disk_image(self):
@@ -175,14 +171,10 @@ class QemuVM(BaseVM):
         :param hdb_disk_image: QEMU hdb disk image path
         """
 
-        if not os.path.isabs(hdb_disk_image):
-            server_config = self.manager.config.get_section_config("Server")
-            hdb_disk_image = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "QEMU", hdb_disk_image)
-
+        self._hdb_disk_image = self.manager.get_abs_image_path(hdb_disk_image)
         log.info('QEMU VM "{name}" [{id}] has set the QEMU hdb disk image path to {disk_image}'.format(name=self._name,
                                                                                                        id=self._id,
-                                                                                                       disk_image=hdb_disk_image))
-        self._hdb_disk_image = hdb_disk_image
+                                                                                                       disk_image=self._hdb_disk_image))
 
     @property
     def hdc_disk_image(self):
@@ -202,14 +194,10 @@ class QemuVM(BaseVM):
         :param hdc_disk_image: QEMU hdc disk image path
         """
 
-        if not os.path.isabs(hdc_disk_image):
-            server_config = self.manager.config.get_section_config("Server")
-            hdc_disk_image = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "QEMU", hdc_disk_image)
-
+        self._hdc_disk_image = self.manager.get_abs_image_path(hdc_disk_image)
         log.info('QEMU VM "{name}" [{id}] has set the QEMU hdc disk image path to {disk_image}'.format(name=self._name,
                                                                                                        id=self._id,
-                                                                                                       disk_image=hdc_disk_image))
-        self._hdc_disk_image = hdc_disk_image
+                                                                                                       disk_image=self._hdc_disk_image))
 
     @property
     def hdd_disk_image(self):
@@ -229,14 +217,11 @@ class QemuVM(BaseVM):
         :param hdd_disk_image: QEMU hdd disk image path
         """
 
-        if not os.path.isabs(hdd_disk_image):
-            server_config = self.manager.config.get_section_config("Server")
-            hdd_disk_image = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "QEMU", hdd_disk_image)
-
+        self._hdd_disk_image = hdd_disk_image
+        self._hdd_disk_image = self.manager.get_abs_image_path(hdd_disk_image)
         log.info('QEMU VM "{name}" [{id}] has set the QEMU hdd disk image path to {disk_image}'.format(name=self._name,
                                                                                                        id=self._id,
-                                                                                                       disk_image=hdd_disk_image))
-        self._hdd_disk_image = hdd_disk_image
+                                                                                                       disk_image=self._hdd_disk_image))
 
     @property
     def adapters(self):
@@ -778,9 +763,9 @@ class QemuVM(BaseVM):
 
         adapter.add_nio(0, nio)
         log.info('QEMU VM "{name}" [{id}]: {nio} added to adapter {adapter_number}'.format(name=self._name,
-                                                                                          id=self._id,
-                                                                                          nio=nio,
-                                                                                          adapter_number=adapter_number))
+                                                                                           id=self._id,
+                                                                                           nio=nio,
+                                                                                           adapter_number=adapter_number))
 
     @asyncio.coroutine
     def adapter_remove_nio_binding(self, adapter_number):
@@ -1068,23 +1053,6 @@ class QemuVM(BaseVM):
         command.extend(self._graphic())
         return command
 
-    def _get_relative_disk_image_path(self, disk_image):
-        """
-        Returns a relative image path if the disk image is in the images directory.
-
-        :param disk_image: path to the disk image
-
-        :returns: relative or full path
-        """
-
-        if disk_image:
-            # return the relative path if the disk image is in the images_path directory
-            server_config = self.manager.config.get_section_config("Server")
-            relative_image = os.path.join(os.path.expanduser(server_config.get("images_path", "~/GNS3/images")), "QEMU", disk_image)
-            if os.path.exists(relative_image):
-                return os.path.basename(disk_image)
-        return disk_image
-
     def __json__(self):
         answer = {
             "project_id": self.project.id,
@@ -1095,11 +1063,11 @@ class QemuVM(BaseVM):
             if field not in answer:
                 answer[field] = getattr(self, field)
 
-        answer["hda_disk_image"] = self._get_relative_disk_image_path(self._hda_disk_image)
-        answer["hdb_disk_image"] = self._get_relative_disk_image_path(self._hdb_disk_image)
-        answer["hdc_disk_image"] = self._get_relative_disk_image_path(self._hdc_disk_image)
-        answer["hdd_disk_image"] = self._get_relative_disk_image_path(self._hdd_disk_image)
-        answer["initrd"] = self._get_relative_disk_image_path(self._initrd)
-        answer["kernel_image"] = self._get_relative_disk_image_path(self._kernel_image)
+        answer["hda_disk_image"] = self.manager.get_relative_image_path(self._hda_disk_image)
+        answer["hdb_disk_image"] = self.manager.get_relative_image_path(self._hdb_disk_image)
+        answer["hdc_disk_image"] = self.manager.get_relative_image_path(self._hdc_disk_image)
+        answer["hdd_disk_image"] = self.manager.get_relative_image_path(self._hdd_disk_image)
+        answer["initrd"] = self.manager.get_relative_image_path(self._initrd)
+        answer["kernel_image"] = self.manager.get_relative_image_path(self._kernel_image)
 
         return answer

tests/modules/iou/test_iou_manager.py

@@ -17,7 +17,9 @@
 
 
 import pytest
+from unittest.mock import patch
 import uuid
+import os
 
 
 from gns3server.modules.iou import IOU
@@ -25,6 +27,15 @@ from gns3server.modules.iou.iou_error import IOUError
 from gns3server.modules.project_manager import ProjectManager
 
 
+@pytest.fixture(scope="function")
+def iou(port_manager):
+    # Cleanup the IOU object
+    IOU._instance = None
+    iou = IOU.instance()
+    iou.port_manager = port_manager
+    return iou
+
+
 def test_get_application_id(loop, project, port_manager):
     # Cleanup the IOU object
     IOU._instance = None
@@ -71,3 +82,8 @@ def test_get_application_id_no_id_available(loop, project, port_manager):
             vm_id = str(uuid.uuid4())
             loop.run_until_complete(iou.create_vm("PC {}".format(i), project.id, vm_id))
             assert iou.get_application_id(vm_id) == i
+
+
+def test_get_images_directory(iou, tmpdir):
+    with patch("gns3server.config.Config.get_section_config", return_value={"images_path": str(tmpdir)}):
+        assert iou.get_images_directory() == str(tmpdir / "IOU")

tests/modules/test_manager.py

@@ -22,6 +22,7 @@ from unittest.mock import patch
 
 
 from gns3server.modules.vpcs import VPCS
+from gns3server.modules.iou import IOU
 
 
 def test_create_vm_new_topology(loop, project, port_manager):
@@ -82,3 +83,33 @@ def test_create_vm_old_topology(loop, project, tmpdir, port_manager):
         vm_dir = os.path.join(project_dir, "project-files", "vpcs", vm.id)
         with open(os.path.join(vm_dir, "startup.vpc")) as f:
             assert f.read() == "1"
+
+
+def test_get_abs_image_path(iou, tmpdir):
+    os.makedirs(str(tmpdir / "IOU"))
+    path1 = str(tmpdir / "test1.bin")
+    open(path1, 'w+').close()
+
+    path2 = str(tmpdir / "IOU" / "test2.bin")
+    open(path2, 'w+').close()
+
+    with patch("gns3server.config.Config.get_section_config", return_value={"images_path": str(tmpdir)}):
+        assert iou.get_abs_image_path(path1) == path1
+        assert iou.get_abs_image_path(path2) == path2
+        assert iou.get_abs_image_path("test2.bin") == path2
+        assert iou.get_abs_image_path("../test1.bin") == path1
+
+
+def test_get_relative_image_path(iou, tmpdir):
+    os.makedirs(str(tmpdir / "IOU"))
+    path1 = str(tmpdir / "test1.bin")
+    open(path1, 'w+').close()
+
+    path2 = str(tmpdir / "IOU" / "test2.bin")
+    open(path2, 'w+').close()
+
+    with patch("gns3server.config.Config.get_section_config", return_value={"images_path": str(tmpdir)}):
+        assert iou.get_relative_image_path(path1) == path1
+        assert iou.get_relative_image_path(path2) == "test2.bin"
+        assert iou.get_relative_image_path("test2.bin") == "test2.bin"
+        assert iou.get_relative_image_path("../test1.bin") == path1