Do not recurse scan for images in standard image directory

Fix GNS3/gns3-gui#1680

gns3server/compute/base_manager.py

@@ -39,7 +39,7 @@
 from .nios.nio_udp import NIOUDP
 from .nios.nio_tap import NIOTAP
 from .nios.nio_ethernet import NIOEthernet
-from ..utils.images import md5sum, remove_checksum, images_directories
+from ..utils.images import md5sum, remove_checksum, images_directories, default_images_directory, list_images
 from .error import NodeError, ImageMissingError
 
 
@@ -477,27 +477,14 @@ def list_images(self):
         :returns: Array of hash
         """
 
-        images = []
-        img_dir = self.get_images_directory()
-        for root, dirs, files in os.walk(img_dir):
-            for filename in files:
-                if filename[0] != "." and not filename.endswith(".md5sum"):
-                    path = os.path.relpath(os.path.join(root, filename), img_dir)
-                    try:
-                        images.append({
-                            "filename": filename,
-                            "path": path,
-                            "md5sum": md5sum(os.path.join(root, filename)),
-                            "filesize": os.stat(os.path.join(root, filename)).st_size})
-                    except OSError as e:
-                        log.warn("Can't add image {}: {}".format(path, str(e)))
-        return images
+        return list_images(self._NODE_TYPE)
 
     def get_images_directory(self):
         """
         Get the image directory on disk
         """
-
+        if hasattr(self, "_NODE_TYPE"):
+            return default_images_directory(self._NODE_TYPE)
         raise NotImplementedError
 
     @asyncio.coroutine

gns3server/compute/dynamips/__init__.py

@@ -678,44 +678,3 @@ def auto_idlepc(self, vm):
             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")
-
-    @asyncio.coroutine
-    def list_images(self):
-        """
-        Return the list of available IOS images.
-
-        :returns: Array of hash
-        """
-
-        image_dir = self.get_images_directory()
-        if not os.path.exists(image_dir):
-            return []
-        try:
-            files = os.listdir(image_dir)
-        except OSError as e:
-            raise DynamipsError("Can not list {}: {}".format(image_dir, str(e)))
-        files.sort()
-        images = []
-        for filename in files:
-            if filename[0] != "." and not filename.endswith(".md5sum"):
-                try:
-                    path = os.path.join(image_dir, filename)
-                    with open(path, "rb") as f:
-                        # read the first 7 bytes of the file.
-                        elf_header_start = f.read(7)
-                except OSError as e:
-                    continue
-                # valid IOS images must start with the ELF magic number, be 32-bit, big endian and have an ELF version of 1
-                if elf_header_start == b'\x7fELF\x01\x02\x01':
-                    images.append({"filename": filename,
-                                   "path": os.path.relpath(path, image_dir),
-                                   "md5sum": md5sum(path),
-                                   "filesize": os.stat(path).st_size
-                                   })
-        return images

gns3server/compute/iou/__init__.py

@@ -95,9 +95,3 @@ def get_legacy_vm_workdir(legacy_vm_id, name):
         """
 
         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/compute/qemu/__init__.py

@@ -229,12 +229,6 @@ def get_legacy_vm_workdir(legacy_vm_id, name):
 
         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")
-
     @asyncio.coroutine
     def create_disk(self, qemu_img, path, options):
         """

gns3server/controller/compute.py

@@ -26,7 +26,7 @@
 import io
 
 from ..utils import parse_version
-from ..utils.images import scan_for_images, md5sum
+from ..utils.images import list_images, md5sum
 from ..utils.asyncio import locked_coroutine
 from ..controller.controller_error import ControllerError
 from ..config import Config
@@ -563,18 +563,10 @@ def images(self, type):
         res = yield from self.http_query("GET", "/{}/images".format(type), timeout=None)
         images = res.json
 
-        try:
-            if type in ["qemu", "dynamips", "iou"]:
-                for path in scan_for_images(type):
-                    image = os.path.basename(path)
-                    if image not in [i['filename'] for i in images]:
-                        images.append({"filename": image,
-                                       "path": image,
-                                       "md5sum": md5sum(path),
-                                       "filesize": os.stat(path).st_size
-                                       })
-        except OSError as e:
-            raise aiohttp.web.HTTPConflict(text="Can't scan for images: {}".format(str(e)))
+        if type in ["qemu", "dynamips", "iou"]:
+            for local_image in list_images(type):
+                if local_image['filename'] not in [i['filename'] for i in images]:
+                    images.append(local_image)
         return images
 
     @asyncio.coroutine

gns3server/controller/topology.py

@@ -22,6 +22,7 @@
 import shutil
 import zipfile
 import aiohttp
+import platform
 import jsonschema
 
 

gns3server/utils/images.py

@@ -26,28 +26,96 @@
 log = logging.getLogger(__name__)
 
 
-def scan_for_images(type):
+def list_images(type):
     """
     Scan directories for available image for a type
 
     :param type: emulator type (dynamips, qemu, iou)
     """
     files = set()
-    paths = []
+    images = []
+
+    server_config = Config.instance().get_section_config("Server")
+    general_images_directory = os.path.expanduser(server_config.get("images_path", "~/GNS3/images"))
+
+    # Subfolder of the general_images_directory specific to this VM type
+    default_directory = default_images_directory(type)
+
     for directory in images_directories(type):
+
+        # We limit recursion to path outside the default images directory
+        # the reason is in the default directory manage file organization and
+        # it should be flatten to keep things simple
+        recurse = True
+        if os.path.commonprefix([directory, general_images_directory]) == general_images_directory:
+            recurse = False
+
         directory = os.path.normpath(directory)
-        for root, _, filenames in os.walk(directory):
-            for file in filenames:
-                path = os.path.join(root, file)
-                if file not in files:
-                    if file.endswith(".md5sum") or file.startswith("."):
+        for root, _, filenames in _os_walk(directory, recurse=recurse):
+            for filename in filenames:
+                path = os.path.join(root, filename)
+                if filename not in files:
+                    if filename.endswith(".md5sum") or filename.startswith("."):
                         continue
-                    elif (file.endswith(".image") and type == "dynamips") \
-                            or (file.endswith(".bin") and type == "iou") \
-                            or (not file.endswith(".bin") and not file.endswith(".image") and type == "qemu"):
-                        files.add(file)
-                        paths.append(force_unix_path(path))
-    return paths
+                    elif ((filename.endswith(".image") or filename.endswith(".bin")) and type == "dynamips") \
+                            or (filename.endswith(".bin") and type == "iou") \
+                            or (not filename.endswith(".bin") and not filename.endswith(".image") and type == "qemu"):
+                        files.add(filename)
+
+                        # It the image is located in the standard directory the path is relative
+                        if os.path.commonprefix([root, default_directory]) != default_directory:
+                            path = os.path.join(root, filename)
+                        else:
+                            path = os.path.relpath(os.path.join(root, filename), default_directory)
+
+                        try:
+                            if type in ["dynamips", "iou"]:
+                                with open(os.path.join(root, filename), "rb") as f:
+                                    # read the first 7 bytes of the file.
+                                    elf_header_start = f.read(7)
+                                # valid IOS images must start with the ELF magic number, be 32-bit, big endian and have an ELF version of 1
+                                if not elf_header_start == b'\x7fELF\x01\x02\x01' and not elf_header_start == b'\x7fELF\x01\x01\x01':
+                                    continue
+
+                            images.append({
+                                "filename": filename,
+                                "path": path,
+                                "md5sum": md5sum(os.path.join(root, filename)),
+                                "filesize": os.stat(os.path.join(root, filename)).st_size})
+                        except OSError as e:
+                            log.warn("Can't add image {}: {}".format(path, str(e)))
+    return images
+
+
+def _os_walk(directory, recurse=True, **kwargs):
+    """
+    Work like os.walk but if recurse is False just list current directory
+    """
+    if recurse:
+        for root, dirs, files in os.walk(directory, **kwargs):
+            yield root, dirs, files
+    else:
+        files = []
+        for filename in os.listdir(directory):
+            if os.path.isfile(os.path.join(directory, filename)):
+                files.append(filename)
+        yield directory, [], files
+
+
+def default_images_directory(type):
+    """
+    :returns: Return the default directory for a node type
+    """
+    server_config = Config.instance().get_section_config("Server")
+    img_dir = os.path.expanduser(server_config.get("images_path", "~/GNS3/images"))
+    if type == "qemu":
+        return os.path.join(img_dir, "QEMU")
+    elif type == "iou":
+        return os.path.join(img_dir, "IOU")
+    elif type == "dynamips":
+        return os.path.join(img_dir, "IOS")
+    else:
+        raise NotImplementedError("%s node type is not supported", type)
 
 
 def images_directories(type):
@@ -61,14 +129,7 @@ def images_directories(type):
 
     paths = []
     img_dir = os.path.expanduser(server_config.get("images_path", "~/GNS3/images"))
-    if type == "qemu":
-        type_img_directory = os.path.join(img_dir, "QEMU")
-    elif type == "iou":
-        type_img_directory = os.path.join(img_dir, "IOU")
-    elif type == "dynamips":
-        type_img_directory = os.path.join(img_dir, "IOS")
-    else:
-        raise NotImplementedError("%s is not supported", type)
+    type_img_directory = default_images_directory(type)
     os.makedirs(type_img_directory, exist_ok=True)
     paths.append(type_img_directory)
     for directory in server_config.get("additional_images_path", "").split(";"):

tests/compute/test_manager.py

@@ -222,35 +222,36 @@ def test_get_relative_image_path_ova(qemu, tmpdir, config):
 
 def test_list_images(loop, qemu, tmpdir):
 
-    fake_images = ["a.bin", "b.bin", ".blu.bin", "a.bin.md5sum"]
+    fake_images = ["a.qcow2", "b.qcow2", ".blu.qcow2", "a.qcow2.md5sum"]
     for image in fake_images:
         with open(str(tmpdir / image), "w+") as f:
             f.write("1")
 
-    with patch("gns3server.compute.Qemu.get_images_directory", return_value=str(tmpdir)):
+    with patch("gns3server.utils.images.default_images_directory", return_value=str(tmpdir)):
         assert loop.run_until_complete(qemu.list_images()) == [
-            {"filename": "a.bin", "path": "a.bin", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1},
-            {"filename": "b.bin", "path": "b.bin", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}
+            {"filename": "a.qcow2", "path": "a.qcow2", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1},
+            {"filename": "b.qcow2", "path": "b.qcow2", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}
         ]
 
 
 def test_list_images_recursives(loop, qemu, tmpdir):
 
-    fake_images = ["a.bin", "b.bin", ".blu.bin", "a.bin.md5sum"]
+    fake_images = ["a.qcow2", "b.qcow2", ".blu.qcow2", "a.qcow2.md5sum"]
     for image in fake_images:
         with open(str(tmpdir / image), "w+") as f:
             f.write("1")
     os.makedirs(str(tmpdir / "c"))
-    fake_images = ["c.bin", "c.bin.md5sum"]
+    fake_images = ["c.qcow2", "c.qcow2.md5sum"]
     for image in fake_images:
         with open(str(tmpdir / "c" / image), "w+") as f:
             f.write("1")
 
-    with patch("gns3server.compute.Qemu.get_images_directory", return_value=str(tmpdir)):
+    with patch("gns3server.utils.images.default_images_directory", return_value=str(tmpdir)):
+
         assert loop.run_until_complete(qemu.list_images()) == [
-            {"filename": "a.bin", "path": "a.bin", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1},
-            {"filename": "b.bin", "path": "b.bin", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1},
-            {"filename": "c.bin", "path": os.path.sep.join(["c", "c.bin"]), "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}
+            {"filename": "a.qcow2", "path": "a.qcow2", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1},
+            {"filename": "b.qcow2", "path": "b.qcow2", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1},
+            {"filename": "c.qcow2", "path": os.path.sep.join(["c", "c.qcow2"]), "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}
         ]
 
 

tests/controller/test_compute.py

@@ -355,7 +355,7 @@ def test_images(compute, async_run, images_dir):
         "path": "linux.qcow2",
         "md5sum": "d41d8cd98f00b204e9800998ecf8427e",
         "filesize": 0}]).encode())
-    open(os.path.join(images_dir, "asa.qcow2"), "w+").close()
+    open(os.path.join(images_dir, "QEMU", "asa.qcow2"), "w+").close()
     with asyncio_patch("aiohttp.ClientSession.request", return_value=response) as mock:
         images = async_run(compute.images("qemu"))
         mock.assert_called_with("GET", "https://example.com:84/v2/compute/qemu/images", auth=None, data=None, headers={'content-type': 'application/json'}, chunked=False, timeout=None)

tests/handlers/api/compute/test_dynamips.py

@@ -132,7 +132,6 @@
 @pytest.fixture
 def fake_dynamips(tmpdir):
     """Create a fake Dynamips image on disk"""
-
     path = str(tmpdir / "7200.bin")
     with open(path, "wb+") as f:
         f.write(b'\x7fELF\x01\x02\x01')
@@ -153,7 +152,7 @@ def fake_file(tmpdir):
 
 def test_images(http_compute, tmpdir, fake_dynamips, fake_file):
 
-    with patch("gns3server.compute.Dynamips.get_images_directory", return_value=str(tmpdir), example=True):
+    with patch("gns3server.utils.images.default_images_directory", return_value=str(tmpdir)):
         response = http_compute.get("/dynamips/images")
     assert response.status == 200
     assert response.json == [{"filename": "7200.bin",
@@ -163,24 +162,24 @@ def test_images(http_compute, tmpdir, fake_dynamips, fake_file):
                               }]
 
 
-def test_upload_image(http_compute, tmpdir):
-    with patch("gns3server.compute.Dynamips.get_images_directory", return_value=str(tmpdir),):
-        response = http_compute.post("/dynamips/images/test2", body="TEST", raw=True)
-        assert response.status == 204
+def test_upload_image(http_compute, tmpdir, images_dir):
+    response = http_compute.post("/dynamips/images/test2", body="TEST", raw=True)
+    assert response.status == 204
 
-    with open(str(tmpdir / "test2")) as f:
+    with open(os.path.join(images_dir, "IOS", "test2")) as f:
         assert f.read() == "TEST"
 
-    with open(str(tmpdir / "test2.md5sum")) as f:
+    with open(os.path.join(images_dir, "IOS", "test2.md5sum")) as f:
         checksum = f.read()
         assert checksum == "033bd94b1168d7e4f0d644c3c95e35bf"
 
 
-def test_upload_image_permission_denied(http_compute, tmpdir):
-    with open(str(tmpdir / "test2.tmp"), "w+") as f:
+def test_upload_image_permission_denied(http_compute, tmpdir, images_dir):
+    os.makedirs(os.path.join(images_dir, "IOS"), exist_ok=True)
+    with open(os.path.join(images_dir, "IOS", "test2.tmp"), "w+") as f:
         f.write("")
-    os.chmod(str(tmpdir / "test2.tmp"), 0)
+    os.chmod(os.path.join(images_dir, "IOS", "test2.tmp"), 0)
 
-    with patch("gns3server.compute.Dynamips.get_images_directory", return_value=str(tmpdir),):
+    with patch("gns3server.utils.images.default_images_directory", return_value=str(tmpdir)):
         response = http_compute.post("/dynamips/images/test2", body="TEST", raw=True)
         assert response.status == 409