From acd5c992ba37e4bb258d02d7ba0b5b6f37fa5dc4 Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Wed, 7 Oct 2015 11:34:27 +0200 Subject: [PATCH 1/2] Fix Dynamips identifier is already used by another router Fix #327 --- gns3server/modules/dynamips/__init__.py | 53 ++++++++++++++++--- gns3server/modules/dynamips/nodes/router.py | 19 ++----- .../modules/dynamips/test_dynamips_manager.py | 51 ++++++++++++++++++ 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/gns3server/modules/dynamips/__init__.py b/gns3server/modules/dynamips/__init__.py index 2b00849b..f69d6641 100644 --- a/gns3server/modules/dynamips/__init__.py +++ b/gns3server/modules/dynamips/__init__.py @@ -116,6 +116,43 @@ class Dynamips(BaseManager): self._devices = {} self._ghost_files = set() self._dynamips_path = None + self._dynamips_ids = {} + + def get_dynamips_id(self, project_id): + """ + :param project_id: UUID of the project + :returns: a free dynamips id + """ + self._dynamips_ids.setdefault(project_id, set()) + dynamips_id = 0 + for dynamips_id in range(1, 4097): + if dynamips_id not in self._dynamips_ids[project_id]: + self._dynamips_ids[project_id].add(dynamips_id) + return dynamips_id + raise DynamipsError("Maximum number of Dynamips instances reached") + + def take_dynamips_id(self, project_id, dynamips_id): + """ + Reserve a dynamips id or raise an error + + :param project_id: UUID of the project + :param dynamips_id: Asked id + """ + self._dynamips_ids.setdefault(project_id, set()) + if dynamips_id in self._dynamips_ids[project_id]: + raise DynamipsError("Dynamips identifier {} is already used by another router".format(dynamips_id)) + self._dynamips_ids[project_id].add(dynamips_id) + + def release_dynamips_id(self, project_id, dynamips_id): + """ + A dynamips id can be reused by another vm + + :param project_id: UUID of the project + :param dynamips_id: Asked id + """ + self._dynamips_ids.setdefault(project_id, set()) + if dynamips_id in self._dynamips_ids[project_id]: + self._dynamips_ids[project_id].remove(dynamips_id) @asyncio.coroutine def unload(self): @@ -165,15 +202,14 @@ class Dynamips(BaseManager): :param project: Project instance """ - yield from super().project_closed(project) # delete useless Dynamips files project_dir = project.module_working_path(self.module_name.lower()) - files = glob.glob(glob_escape(os.path.join(project_dir, "*.ghost"))) - files += glob.glob(glob_escape(os.path.join(project_dir, "*_lock"))) - files += glob.glob(glob_escape(os.path.join(project_dir, "ilt_*"))) - files += glob.glob(glob_escape(os.path.join(project_dir, "c[0-9][0-9][0-9][0-9]_i[0-9]*_rommon_vars"))) - files += glob.glob(glob_escape(os.path.join(project_dir, "c[0-9][0-9][0-9][0-9]_i[0-9]*_log.txt"))) + files = glob.glob(os.path.join(glob_escape(project_dir), "*.ghost")) + files += glob.glob(os.path.join(glob_escape(project_dir), "*_lock")) + files += glob.glob(os.path.join(glob_escape(project_dir), "ilt_*")) + files += glob.glob(os.path.join(glob_escape(project_dir), "c[0-9][0-9][0-9][0-9]_i[0-9]*_rommon_vars")) + files += glob.glob(os.path.join(glob_escape(project_dir), "c[0-9][0-9][0-9][0-9]_i[0-9]*_log.txt")) for file in files: try: log.debug("Deleting file {}".format(file)) @@ -184,6 +220,11 @@ class Dynamips(BaseManager): log.warn("Could not delete file {}: {}".format(file, e)) continue + # Release the dynamips ids if we want to reload the same project + # later + if project.id in self._dynamips_ids: + del self._dynamips_ids[project.id] + @asyncio.coroutine def project_moved(self, project): """ diff --git a/gns3server/modules/dynamips/nodes/router.py b/gns3server/modules/dynamips/nodes/router.py index bf25b6a6..b896e488 100644 --- a/gns3server/modules/dynamips/nodes/router.py +++ b/gns3server/modules/dynamips/nodes/router.py @@ -53,7 +53,6 @@ class Router(BaseVM): :param platform: Platform of this router """ - _dynamips_ids = {} _status = {0: "inactive", 1: "shutting down", 2: "running", @@ -95,20 +94,11 @@ class Router(BaseVM): self._ghost_flag = ghost_flag if not ghost_flag: - self._dynamips_ids.setdefault(project.id, list()) if not dynamips_id: - # find a Dynamips ID if none is provided (0 < id <= 4096) - self._dynamips_id = 0 - for identifier in range(1, 4097): - if identifier not in self._dynamips_ids[project.id]: - self._dynamips_id = identifier - break - if self._dynamips_id == 0: - raise DynamipsError("Maximum number of Dynamips instances reached") + self._dynamips_id = manager.get_dynamips_id(project.id) else: - if dynamips_id in self._dynamips_ids[project.id]: - raise DynamipsError("Dynamips identifier {} is already used by another router".format(dynamips_id)) - self._dynamips_ids[project.id].append(self._dynamips_id) + self._dynamips_id = dynamips_id + manager.take_dynamips_id(project.id, dynamips_id) if self._aux is not None: self._aux = self._manager.port_manager.reserve_tcp_port(self._aux, self._project) @@ -1608,8 +1598,7 @@ class Router(BaseVM): log.warn("Could not delete file {}: {}".format(file, e)) continue - if self._dynamips_id in self._dynamips_ids[self._project.id]: - self._dynamips_ids[self._project.id].remove(self._dynamips_id) + self.manager.release_dynamips_id(self._project.id, self._dynamips_id) @asyncio.coroutine def clean_delete(self): diff --git a/tests/modules/dynamips/test_dynamips_manager.py b/tests/modules/dynamips/test_dynamips_manager.py index 00d48150..c744b29d 100644 --- a/tests/modules/dynamips/test_dynamips_manager.py +++ b/tests/modules/dynamips/test_dynamips_manager.py @@ -19,6 +19,9 @@ import pytest import tempfile import sys +import uuid +import os +import asyncio from gns3server.modules.dynamips import Dynamips from gns3server.modules.dynamips.dynamips_error import DynamipsError @@ -37,9 +40,57 @@ def test_vm_invalid_dynamips_path(manager): with pytest.raises(DynamipsError): manager.find_dynamips() + @pytest.mark.skipif(sys.platform.startswith("win"), reason="Not supported by Windows") def test_vm_non_executable_dynamips_path(manager): tmpfile = tempfile.NamedTemporaryFile() with patch("gns3server.config.Config.get_section_config", return_value={"dynamips_path": tmpfile.name}): with pytest.raises(DynamipsError): manager.find_dynamips() + + +def test_get_dynamips_id(manager): + project_1 = str(uuid.uuid4()) + project_2 = str(uuid.uuid4()) + project_3 = str(uuid.uuid4()) + + assert manager.get_dynamips_id(project_1) == 1 + assert manager.get_dynamips_id(project_1) == 2 + assert manager.get_dynamips_id(project_2) == 1 + with pytest.raises(DynamipsError): + for dynamips_id in range(1, 4098): + manager.get_dynamips_id(project_3) + + +def test_take_dynamips_id(manager): + project_1 = str(uuid.uuid4()) + + manager.take_dynamips_id(project_1, 1) + assert manager.get_dynamips_id(project_1) == 2 + with pytest.raises(DynamipsError): + manager.take_dynamips_id(project_1, 1) + + +def test_release_dynamips_id(manager): + project_1 = str(uuid.uuid4()) + project_2 = str(uuid.uuid4()) + + manager.take_dynamips_id(project_1, 1) + manager.release_dynamips_id(project_1, 1) + assert manager.get_dynamips_id(project_1) == 1 + # Should not crash for 0 id + manager.release_dynamips_id(project_2, 0) + + +def test_project_closed(manager, project, loop): + + manager._dynamips_ids[project.id] = set([1, 2, 3]) + + project_dir = project.module_working_path(manager.module_name.lower()) + os.makedirs(project_dir) + open(os.path.join(project_dir, "test.ghost"), "w+").close() + + loop.run_until_complete(asyncio.async(manager.project_closed(project))) + + assert not os.path.exists(os.path.join(project_dir, "test.ghost")) + assert project.id not in manager._dynamips_ids From 98ac295e2e432a7ca4d4eea77bc12f70227c005f Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Wed, 7 Oct 2015 16:42:34 +0200 Subject: [PATCH 2/2] Escape other usage of glob Fix #332 --- gns3server/modules/dynamips/nodes/router.py | 15 ++++++++------- gns3server/modules/iou/iou_vm.py | 5 +++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/gns3server/modules/dynamips/nodes/router.py b/gns3server/modules/dynamips/nodes/router.py index b896e488..a8e61a6b 100644 --- a/gns3server/modules/dynamips/nodes/router.py +++ b/gns3server/modules/dynamips/nodes/router.py @@ -34,6 +34,7 @@ log = logging.getLogger(__name__) from ...base_vm import BaseVM from ..dynamips_error import DynamipsError from ..nios.nio_udp import NIOUDP +from ....utils.glob import glob_escape from gns3server.utils.asyncio import wait_run_in_executor @@ -337,13 +338,13 @@ class Router(BaseVM): if self._auto_delete_disks: # delete nvram and disk files project_dir = os.path.join(self.project.module_working_directory(self.manager.module_name.lower())) - files = glob.glob(os.path.join(project_dir, "{}_i{}_disk[0-1]".format(self.platform, self.dynamips_id))) - files += glob.glob(os.path.join(project_dir, "{}_i{}_slot[0-1]".format(self.platform, self.dynamips_id))) - files += glob.glob(os.path.join(project_dir, "{}_i{}_nvram".format(self.platform, self.dynamips_id))) - files += glob.glob(os.path.join(project_dir, "{}_i{}_flash[0-1]".format(self.platform, self.dynamips_id))) - files += glob.glob(os.path.join(project_dir, "{}_i{}_rom".format(self.platform, self.dynamips_id))) - files += glob.glob(os.path.join(project_dir, "{}_i{}_bootflash".format(self.platform, self.dynamips_id))) - files += glob.glob(os.path.join(project_dir, "{}_i{}_ssa").format(self.platform, self.dynamips_id)) + files = glob.glob(os.path.join(glob_escape(project_dir), "{}_i{}_disk[0-1]".format(self.platform, self.dynamips_id))) + files += glob.glob(os.path.join(glob_escape(project_dir), "{}_i{}_slot[0-1]".format(self.platform, self.dynamips_id))) + files += glob.glob(os.path.join(glob_escape(project_dir), "{}_i{}_nvram".format(self.platform, self.dynamips_id))) + files += glob.glob(os.path.join(glob_escape(project_dir), "{}_i{}_flash[0-1]".format(self.platform, self.dynamips_id))) + files += glob.glob(os.path.join(glob_escape(project_dir), "{}_i{}_rom".format(self.platform, self.dynamips_id))) + files += glob.glob(os.path.join(glob_escape(project_dir), "{}_i{}_bootflash".format(self.platform, self.dynamips_id))) + files += glob.glob(os.path.join(glob_escape(project_dir), "{}_i{}_ssa").format(self.platform, self.dynamips_id)) for file in files: try: log.debug("Deleting file {}".format(file)) diff --git a/gns3server/modules/iou/iou_vm.py b/gns3server/modules/iou/iou_vm.py index 9237dcaf..98a75b5a 100644 --- a/gns3server/modules/iou/iou_vm.py +++ b/gns3server/modules/iou/iou_vm.py @@ -41,6 +41,7 @@ from ..nios.nio_udp import NIOUDP from ..nios.nio_tap import NIOTAP from ..nios.nio_generic_ethernet import NIOGenericEthernet from ..base_vm import BaseVM +from ...utils.glob import glob_escape from .ioucon import start_ioucon import gns3server.utils.asyncio @@ -486,10 +487,10 @@ class IOUVM(BaseVM): """ destination = os.path.join(self.working_dir, "nvram_{:05d}".format(self.application_id)) - for file_path in glob.glob(os.path.join(self.working_dir, "nvram_*")): + for file_path in glob.glob(os.path.join(glob_escape(self.working_dir), "nvram_*")): shutil.move(file_path, destination) destination = os.path.join(self.working_dir, "vlan.dat-{:05d}".format(self.application_id)) - for file_path in glob.glob(os.path.join(self.working_dir, "vlan.dat-*")): + for file_path in glob.glob(os.path.join(glob_escape(self.working_dir), "vlan.dat-*")): shutil.move(file_path, destination) @asyncio.coroutine