From b9bd6aa501828b9454e05103662a56ddfb3314a4 Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Fri, 27 Jan 2017 10:41:39 +0100 Subject: [PATCH 1/9] Do not try to start the GNS3 VM if the name is none Fix #881 --- gns3server/controller/gns3vm/__init__.py | 3 +++ tests/controller/test_controller.py | 15 ++++++++++----- tests/controller/test_gns3vm.py | 5 +++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/gns3server/controller/gns3vm/__init__.py b/gns3server/controller/gns3vm/__init__.py index 88b3f5da..dec8a8c3 100644 --- a/gns3server/controller/gns3vm/__init__.py +++ b/gns3server/controller/gns3vm/__init__.py @@ -260,6 +260,9 @@ class GNS3VM: """ engine = self.current_engine() if not engine.running: + if self._settings["vmname"] is None: + return + log.info("Start the GNS3 VM") engine.vmname = self._settings["vmname"] engine.ram = self._settings["ram"] diff --git a/tests/controller/test_controller.py b/tests/controller/test_controller.py index d7cac05e..3227c54f 100644 --- a/tests/controller/test_controller.py +++ b/tests/controller/test_controller.py @@ -376,7 +376,8 @@ def test_getProject(controller, async_run): def test_start(controller, async_run): controller.gns3vm.settings = { "enable": False, - "engine": "vmware" + "engine": "vmware", + "vmname": "GNS3 VM" } with asyncio_patch("gns3server.controller.compute.Compute.connect") as mock: async_run(controller.start()) @@ -390,7 +391,8 @@ def test_start_vm(controller, async_run): """ controller.gns3vm.settings = { "enable": True, - "engine": "vmware" + "engine": "vmware", + "vmname": "GNS3 VM" } with asyncio_patch("gns3server.controller.gns3vm.vmware_gns3_vm.VMwareGNS3VM.start") as mock: with asyncio_patch("gns3server.controller.compute.Compute.connect") as mock_connect: @@ -415,7 +417,8 @@ def test_stop_vm(controller, async_run): controller.gns3vm.settings = { "enable": True, "engine": "vmware", - "when_exit": "stop" + "when_exit": "stop", + "vmname": "GNS3 VM" } controller.gns3vm.current_engine().running = True with asyncio_patch("gns3server.controller.gns3vm.vmware_gns3_vm.VMwareGNS3VM.stop") as mock: @@ -430,7 +433,8 @@ def test_suspend_vm(controller, async_run): controller.gns3vm.settings = { "enable": True, "engine": "vmware", - "when_exit": "suspend" + "when_exit": "suspend", + "vmname": "GNS3 VM" } controller.gns3vm.current_engine().running = True with asyncio_patch("gns3server.controller.gns3vm.vmware_gns3_vm.VMwareGNS3VM.suspend") as mock: @@ -445,7 +449,8 @@ def test_keep_vm(controller, async_run): controller.gns3vm.settings = { "enable": True, "engine": "vmware", - "when_exit": "keep" + "when_exit": "keep", + "vmname": "GNS3 VM" } controller.gns3vm.current_engine().running = True with asyncio_patch("gns3server.controller.gns3vm.vmware_gns3_vm.VMwareGNS3VM.suspend") as mock: diff --git a/tests/controller/test_gns3vm.py b/tests/controller/test_gns3vm.py index cbeeded9..d99a558f 100644 --- a/tests/controller/test_gns3vm.py +++ b/tests/controller/test_gns3vm.py @@ -21,6 +21,7 @@ from tests.utils import asyncio_patch, AsyncioMagicMock from gns3server.controller.gns3vm import GNS3VM from gns3server.controller.gns3vm.gns3_vm_error import GNS3VMError + @pytest.fixture def dummy_engine(): engine = AsyncioMagicMock() @@ -65,7 +66,8 @@ def test_update_settings(controller, async_run): vm = GNS3VM(controller) vm.settings = { "enable": True, - "engine": "vmware" + "engine": "vmware", + "vmname": "GNS3 VM" } with asyncio_patch("gns3server.controller.gns3vm.vmware_gns3_vm.VMwareGNS3VM.start"): async_run(vm.auto_start_vm()) @@ -94,4 +96,3 @@ def test_auto_start_with_error(async_run, controller, dummy_gns3vm, dummy_engine async_run(dummy_gns3vm.auto_start_vm()) assert dummy_engine.start.called assert controller.computes["vm"].name == "GNS3 VM (Test VM)" - From d99ec92210657eb2237d5ce7e5bd12fec0ab8439 Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Fri, 27 Jan 2017 10:48:07 +0100 Subject: [PATCH 2/9] Fix a rare race condition when exporting debug informations Fix #880 --- gns3server/handlers/api/controller/server_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gns3server/handlers/api/controller/server_handler.py b/gns3server/handlers/api/controller/server_handler.py index 07f7528f..b66f2e83 100644 --- a/gns3server/handlers/api/controller/server_handler.py +++ b/gns3server/handlers/api/controller/server_handler.py @@ -150,7 +150,7 @@ class ServerHandler: # If something is wrong we log the info to the log and we hope the log will be include correctly to the debug export log.error("Could not copy VMware VMX file {}".format(e), exc_info=1) - for compute in Controller.instance().computes.values(): + for compute in list(Controller.instance().computes.values()): try: r = yield from compute.get("/debug", raw=True) data = r.body.decode("utf-8") From 8ff7670031ef4c86f57e8eb4fbdcaf6deb6ed78c Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Fri, 27 Jan 2017 10:49:57 +0100 Subject: [PATCH 3/9] Fix a crash when you broke permission on your file system Fix #879 --- gns3server/ubridge/hypervisor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gns3server/ubridge/hypervisor.py b/gns3server/ubridge/hypervisor.py index 579ebf7c..452e2524 100644 --- a/gns3server/ubridge/hypervisor.py +++ b/gns3server/ubridge/hypervisor.py @@ -171,7 +171,7 @@ class Hypervisor(UBridgeHypervisor): env=env) log.info("ubridge started PID={}".format(self._process.pid)) - except (OSError, subprocess.SubprocessError) as e: + except (OSError, PermissionError, subprocess.SubprocessError) as e: ubridge_stdout = self.read_stdout() log.error("Could not start ubridge: {}\n{}".format(e, ubridge_stdout)) raise UBridgeHypervisor("Could not start ubridge: {}\n{}".format(e, ubridge_stdout)) From 267a5ae3a8cb6e56884ae23f5fb7eecad6d62bbf Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Fri, 27 Jan 2017 10:52:17 +0100 Subject: [PATCH 4/9] Do not crash when you broke permission on your file system during execution Fix #878 --- gns3server/controller/project.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index 24614236..4debcb73 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -643,8 +643,11 @@ class Project: # We don't care if a compute is down at this step except (ComputeError, aiohttp.web.HTTPNotFound, aiohttp.web.HTTPConflict): pass - if os.path.exists(path + ".backup"): - shutil.copy(path + ".backup", path) + try: + if os.path.exists(path + ".backup"): + shutil.copy(path + ".backup", path) + except (PermissionError, OSError): + pass self._status = "closed" self._loading = False raise e From 2da20177a28e335de6fe5cdbd45ee4fa0c741dfa Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Fri, 27 Jan 2017 10:56:48 +0100 Subject: [PATCH 5/9] Avoid crash when you broke your system permissions Fix #877 --- gns3server/utils/images.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gns3server/utils/images.py b/gns3server/utils/images.py index 03bb3d16..fd867aa6 100644 --- a/gns3server/utils/images.py +++ b/gns3server/utils/images.py @@ -130,8 +130,11 @@ def images_directories(type): paths = [] img_dir = os.path.expanduser(server_config.get("images_path", "~/GNS3/images")) type_img_directory = default_images_directory(type) - os.makedirs(type_img_directory, exist_ok=True) - paths.append(type_img_directory) + try: + os.makedirs(type_img_directory, exist_ok=True) + paths.append(type_img_directory) + except (OSError, PermissionError): + pass for directory in server_config.get("additional_images_path", "").split(";"): paths.append(directory) # Compatibility with old topologies we look in parent directory From 580693b1ecf53ea209e05980b70d9513d2c01fe7 Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Mon, 30 Jan 2017 15:19:46 +0100 Subject: [PATCH 6/9] Prevent renaming of a running VirtualBox linked VM Fix https://github.com/GNS3/gns3-gui/issues/1816 --- gns3server/compute/virtualbox/virtualbox_vm.py | 2 ++ tests/compute/virtualbox/test_virtualbox_vm.py | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/gns3server/compute/virtualbox/virtualbox_vm.py b/gns3server/compute/virtualbox/virtualbox_vm.py index 7cd121ce..93391b8d 100644 --- a/gns3server/compute/virtualbox/virtualbox_vm.py +++ b/gns3server/compute/virtualbox/virtualbox_vm.py @@ -604,6 +604,8 @@ class VirtualBoxVM(BaseNode): """ if self.linked_clone: + if self.status == "started": + raise VirtualBoxError("You can't change the name of running VM {}".format(self._name)) yield from self._modify_vm('--name "{}"'.format(vmname)) log.info("VirtualBox VM '{name}' [{id}] has set the VM name to '{vmname}'".format(name=self.name, id=self.id, vmname=vmname)) diff --git a/tests/compute/virtualbox/test_virtualbox_vm.py b/tests/compute/virtualbox/test_virtualbox_vm.py index 9a5543ed..7614167c 100644 --- a/tests/compute/virtualbox/test_virtualbox_vm.py +++ b/tests/compute/virtualbox/test_virtualbox_vm.py @@ -44,6 +44,17 @@ def test_vm(project, manager): assert vm.vmname == "test" +def test_rename_vmname(project, manager, async_run): + """ + Rename a VM is not allowed when using linked clone + """ + vm = VirtualBoxVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", project, manager, "test", False) + vm._node_status = "started" + vm._linked_clone = True + with pytest.raises(VirtualBoxError): + async_run(vm.set_vmname("toto")) + + def test_vm_valid_virtualbox_api_version(loop, project, manager): with asyncio_patch("gns3server.compute.virtualbox.VirtualBox.execute", return_value=["API version: 4_3"]): vm = VirtualBoxVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", project, manager, "test", False) From bfbc6ff0bec62e2ba7945c44f4caac8b6be01762 Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Tue, 31 Jan 2017 13:43:05 +0100 Subject: [PATCH 7/9] Fix rare race condition when stopping ubridge Fix #887 --- gns3server/utils/asyncio/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gns3server/utils/asyncio/__init__.py b/gns3server/utils/asyncio/__init__.py index 524903ad..2046f6a6 100644 --- a/gns3server/utils/asyncio/__init__.py +++ b/gns3server/utils/asyncio/__init__.py @@ -77,8 +77,11 @@ def wait_for_process_termination(process, timeout=10): :param timeout: Timeout in seconds """ - if sys.version_info >= (3,5): - yield from asyncio.wait_for(process.wait(), timeout=timeout) + if sys.version_info >= (3, 5): + try: + yield from asyncio.wait_for(process.wait(), timeout=timeout) + except ProcessLookupError: + return else: while timeout > 0: if process.returncode is not None: From 27a10898063631da33900d124d01ec1b36e72c97 Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Tue, 31 Jan 2017 15:16:05 +0100 Subject: [PATCH 8/9] Fix creation of qemu img Fix https://github.com/GNS3/gns3-gui/issues/1826 --- gns3server/web/route.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/gns3server/web/route.py b/gns3server/web/route.py index ce527239..3f5a153c 100644 --- a/gns3server/web/route.py +++ b/gns3server/web/route.py @@ -41,16 +41,15 @@ from ..config import Config def parse_request(request, input_schema, raw): """Parse body of request and raise HTTP errors in case of problems""" - content_length = request.content_length - if content_length is not None and content_length > 0 and not raw: + request.json = {} + if not raw: body = yield from request.read() - try: - request.json = json.loads(body.decode('utf-8')) - except ValueError as e: - request.json = {"malformed_json": body.decode('utf-8')} - raise aiohttp.web.HTTPBadRequest(text="Invalid JSON {}".format(e)) - else: - request.json = {} + if body: + try: + request.json = json.loads(body.decode('utf-8')) + except ValueError as e: + request.json = {"malformed_json": body.decode('utf-8')} + raise aiohttp.web.HTTPBadRequest(text="Invalid JSON {}".format(e)) # Parse the query string if len(request.query_string) > 0: From f0ff035c0bb9568c5dfc9afa950ad1e5cca51967 Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Tue, 31 Jan 2017 18:58:43 +0100 Subject: [PATCH 9/9] Prevent corruption of VM in VirtualBox when using linked clone Fix https://github.com/GNS3/gns3-gui/issues/1821 --- gns3server/compute/virtualbox/__init__.py | 4 +-- .../compute/virtualbox/virtualbox_vm.py | 7 ++++++ .../api/compute/virtualbox_handler.py | 11 +++++--- .../compute/virtualbox/test_virtualbox_vm.py | 25 ++++++++++++++++--- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/gns3server/compute/virtualbox/__init__.py b/gns3server/compute/virtualbox/__init__.py index acfd9735..f9b403a1 100644 --- a/gns3server/compute/virtualbox/__init__.py +++ b/gns3server/compute/virtualbox/__init__.py @@ -161,7 +161,7 @@ class VirtualBox(BaseManager): continue @asyncio.coroutine - def list_vms(self): + def list_vms(self, allow_clone=False): """ Gets VirtualBox VM list. """ @@ -176,7 +176,7 @@ class VirtualBox(BaseManager): if vmname == "": continue # ignore inaccessible VMs extra_data = yield from self.execute("getextradata", [vmname, "GNS3/Clone"]) - if len(extra_data) == 0 or not extra_data[0].strip() == "Value: yes": + if allow_clone or len(extra_data) == 0 or not extra_data[0].strip() == "Value: yes": # get the amount of RAM info_results = yield from self.execute("showvminfo", [vmname, "--machinereadable"]) ram = 0 diff --git a/gns3server/compute/virtualbox/virtualbox_vm.py b/gns3server/compute/virtualbox/virtualbox_vm.py index 93391b8d..801988e2 100644 --- a/gns3server/compute/virtualbox/virtualbox_vm.py +++ b/gns3server/compute/virtualbox/virtualbox_vm.py @@ -603,9 +603,16 @@ class VirtualBoxVM(BaseNode): :param vmname: VirtualBox VM name """ + if vmname == self._vmname: + return + if self.linked_clone: if self.status == "started": raise VirtualBoxError("You can't change the name of running VM {}".format(self._name)) + # We can't rename a VM to name that already exists + vms = yield from self.manager.list_vms(allow_clone=True) + if vmname in [vm["vmname"] for vm in vms]: + raise VirtualBoxError("You can't change the name to {} it's already use in VirtualBox".format(vmname)) yield from self._modify_vm('--name "{}"'.format(vmname)) log.info("VirtualBox VM '{name}' [{id}] has set the VM name to '{vmname}'".format(name=self.name, id=self.id, vmname=vmname)) diff --git a/gns3server/handlers/api/compute/virtualbox_handler.py b/gns3server/handlers/api/compute/virtualbox_handler.py index 50ef5bad..f7ee4b45 100644 --- a/gns3server/handlers/api/compute/virtualbox_handler.py +++ b/gns3server/handlers/api/compute/virtualbox_handler.py @@ -112,10 +112,13 @@ class VirtualBoxHandler: vbox_manager = VirtualBox.instance() vm = vbox_manager.get_node(request.match_info["node_id"], project_id=request.match_info["project_id"]) - if "vmname" in request.json: - vmname = request.json.pop("vmname") - if vmname != vm.vmname: - yield from vm.set_vmname(vmname) + if "name" in request.json: + name = request.json.pop("name") + vmname = request.json.pop("vmname", None) + if name != vm.name: + vm.name = name + if vm.linked_clone: + yield from vm.set_vmname(vm.name) if "adapters" in request.json: adapters = int(request.json.pop("adapters")) diff --git a/tests/compute/virtualbox/test_virtualbox_vm.py b/tests/compute/virtualbox/test_virtualbox_vm.py index 7614167c..ef617fe4 100644 --- a/tests/compute/virtualbox/test_virtualbox_vm.py +++ b/tests/compute/virtualbox/test_virtualbox_vm.py @@ -18,7 +18,7 @@ import os import pytest import asyncio -from tests.utils import asyncio_patch +from tests.utils import asyncio_patch, AsyncioMagicMock from gns3server.compute.virtualbox.virtualbox_vm import VirtualBoxVM from gns3server.compute.virtualbox.virtualbox_error import VirtualBoxError @@ -46,13 +46,30 @@ def test_vm(project, manager): def test_rename_vmname(project, manager, async_run): """ - Rename a VM is not allowed when using linked clone + Rename a VM is not allowed when using a running linked clone + or if the vm already exists in Vbox """ vm = VirtualBoxVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", project, manager, "test", False) - vm._node_status = "started" + vm.manager.list_vms = AsyncioMagicMock(return_value=[{"vmname": "Debian"}]) vm._linked_clone = True + vm._modify_vm = AsyncioMagicMock() + + # Vm is running + vm._node_status = "started" with pytest.raises(VirtualBoxError): - async_run(vm.set_vmname("toto")) + async_run(vm.set_vmname("Arch")) + assert not vm._modify_vm.called + + vm._node_status = "stopped" + + # Name already use + with pytest.raises(VirtualBoxError): + async_run(vm.set_vmname("Debian")) + assert not vm._modify_vm.called + + # Work + async_run(vm.set_vmname("Arch")) + assert vm._modify_vm.called def test_vm_valid_virtualbox_api_version(loop, project, manager):