From d46bf2aa8e512c9cca396e936f42f89c75fce212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marta=20Marczykowska-G=C3=B3recka?= Date: Wed, 18 Jul 2018 23:51:37 +0200 Subject: [PATCH 1/4] Improved communication on Remove VM in Qube Manager When a user tries to remove a VM that's used by other VMs or Global Settings from Qube Manager, they get information about where the VM is used. Depends on https://github.com/QubesOS/qubes-core-admin-client/pull/73 --- qubesmanager/qube_manager.py | 41 +++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/qubesmanager/qube_manager.py b/qubesmanager/qube_manager.py index 5d6d88f..afc59ce 100644 --- a/qubesmanager/qube_manager.py +++ b/qubesmanager/qube_manager.py @@ -35,6 +35,7 @@ from pydbus import SessionBus from qubesadmin import Qubes from qubesadmin import exc +from qubesadmin import utils from PyQt4 import QtGui # pylint: disable=import-error from PyQt4 import QtCore # pylint: disable=import-error @@ -685,21 +686,31 @@ class VmManagerWindow(ui_qubemanager.Ui_VmManagerWindow, QtGui.QMainWindow): vm = self.get_selected_vm() - if vm.klass == 'TemplateVM': - dependent_vms = 0 - for single_vm in self.qubes_app.domains: - if getattr(single_vm, 'template', None) == vm: - dependent_vms += 1 - if dependent_vms > 0: - QtGui.QMessageBox.warning( - None, self.tr("Warning!"), - self.tr("This Template Qube cannot be removed, " - "because there is at least one Qube that is based " - "on it.
If you want to remove this " - "Template Qube and all the Qubes based on it, you " - "should first remove each individual Qube that " - "uses this template.")) - return + dependencies = utils.vm_usage(self.qubes_app, vm) + + if dependencies: + list_text = "
" + for (holder, prop) in dependencies: + if holder is None: + list_text += "- Global property {}
".format(prop) + else: + list_text += "- {} for qube {}
".format( + prop, holder.name) + list_text += "
" + + info_dialog = QtGui.QMessageBox(self) + info_dialog.setWindowTitle(self.tr("Warning!")) + info_dialog.setText( + self.tr("This qube cannot be removed. It is used as:" + "
" + list_text + "If you want to " + " remove this qube, you should remove or change " + " settings of each qube or setting that uses" + " it.")) + info_dialog.setModal(False) + info_dialog.show() + self.qt_app.processEvents() + + return (requested_name, ok) = QtGui.QInputDialog.getText( None, self.tr("Qube Removal Confirmation"), From b2fdbb950a67b38f179f213713df562cc7785c04 Mon Sep 17 00:00:00 2001 From: Christopher Laprise Date: Fri, 20 Jul 2018 00:03:37 +0200 Subject: [PATCH 2/4] Handling VM dependencies in GUI tools Added the following reactions ot VM dependencies in GUI tools: - Qube Manager will inform the user why they cannot delete a VM (which properties of which VMS [or global] are using the VM) - Settings window will try to intelligently rename VMs (change properties to the new name, if possible, and inform the user what went wrong if not) - Settings window will inform the user why they cannot delete a VM Also, renaming VM from Settings launched from Qube Manager will refresh the VM list through a horrible hack, to be replaced by a neater Admin API solution in some near future. depends on https://github.com/QubesOS/qubes-core-admin-client/pull/73/commits/ca848ca7bd07dfb6c2135c4767b0e6536cd9a430 fixes QubesOS/qubes-issues#3992 fixes QubesOS/qubes-issues#3993 fixes QubesOS/qubes-issues#3892 fixes QubesOS/qubes-issues#3499 --- qubesmanager/qube_manager.py | 45 ++++++++++++++++-------- qubesmanager/settings.py | 67 +++++++++++++++++++++++++++++++++--- qubesmanager/utils.py | 15 ++++++++ 3 files changed, 108 insertions(+), 19 deletions(-) diff --git a/qubesmanager/qube_manager.py b/qubesmanager/qube_manager.py index afc59ce..17e9e62 100644 --- a/qubesmanager/qube_manager.py +++ b/qubesmanager/qube_manager.py @@ -50,6 +50,7 @@ from . import global_settings from . import restore from . import backup from . import log_dialog +from . import utils as manager_utils class SearchBox(QtGui.QLineEdit): @@ -413,7 +414,11 @@ class VmManagerWindow(ui_qubemanager.Ui_VmManagerWindow, QtGui.QMainWindow): def check_updates(self): for vm in self.qubes_app.domains: if vm.klass in {'TemplateVM', 'StandaloneVM'}: - self.vms_in_table[vm.qid].update() + try: + self.vms_in_table[vm.qid].update() + except exc.QubesException: + # the VM might have vanished in the meantime + pass def on_domain_added(self, _, domain): #needs to clear cache @@ -686,26 +691,20 @@ class VmManagerWindow(ui_qubemanager.Ui_VmManagerWindow, QtGui.QMainWindow): vm = self.get_selected_vm() - dependencies = utils.vm_usage(self.qubes_app, vm) + dependencies = utils.vm_dependencies(self.qubes_app, vm) if dependencies: - list_text = "
" - for (holder, prop) in dependencies: - if holder is None: - list_text += "- Global property {}
".format(prop) - else: - list_text += "- {} for qube {}
".format( - prop, holder.name) - list_text += "
" + list_text = "
" + \ + manager_utils.format_dependencies_list(dependencies) + \ + "
" info_dialog = QtGui.QMessageBox(self) info_dialog.setWindowTitle(self.tr("Warning!")) info_dialog.setText( self.tr("This qube cannot be removed. It is used as:" - "
" + list_text + "If you want to " - " remove this qube, you should remove or change " - " settings of each qube or setting that uses" - " it.")) + "
{} If you want to remove this qube, " + "you should remove or change settings of each qube " + "or setting that uses it.".format(list_text))) info_dialog.setModal(False) info_dialog.show() self.qt_app.processEvents() @@ -1001,8 +1000,24 @@ class VmManagerWindow(ui_qubemanager.Ui_VmManagerWindow, QtGui.QMainWindow): settings_window = settings.VMSettingsWindow( vm, self.qt_app, "basic") settings_window.exec_() - self.vms_in_table[vm.qid].update() + vm_deleted = False + + try: + # the VM might not exist after running Settings - it might + # have been cloned or removed + self.vms_in_table[vm.qid].update() + except exc.QubesException: + # TODO: this will be replaced by proper signal handling once + # settings are migrated to AdminAPI + vm_deleted = True + + if vm_deleted: + for row in self.vms_in_table: + try: + self.vms_in_table[row].update() + except exc.QubesException: + pass # noinspection PyArgumentList @QtCore.pyqtSlot(name='on_action_appmenus_triggered') diff --git a/qubesmanager/settings.py b/qubesmanager/settings.py index adcf21c..8c1d705 100755 --- a/qubesmanager/settings.py +++ b/qubesmanager/settings.py @@ -34,6 +34,7 @@ import traceback import sys from qubesadmin.tools import QubesArgumentParser from qubesadmin import devices +from qubesadmin import utils as admin_utils import qubesadmin.exc from . import utils @@ -481,10 +482,35 @@ class VMSettingsWindow(ui_settingsdlg.Ui_SettingsDialog, QtGui.QDialog): return False return True - def _rename_vm(self, t_monitor, name): + def _rename_vm(self, t_monitor, name, dependencies): try: - self.vm.app.clone_vm(self.vm, name) - del self.vm.app.domains[self.vm.name] + new_vm = self.vm.app.clone_vm(self.vm, name) + + failed_props = [] + + for (holder, prop) in dependencies: + try: + if holder is None: + setattr(self.vm.app, prop, new_vm) + else: + setattr(holder, prop, new_vm) + except qubesadmin.exc.QubesException as qex: + failed_props += (holder, prop) + + if not failed_props: + del self.vm.app.domains[self.vm.name] + else: + list_text = utils.format_dependencies_list(failed_props) + + QtGui.QMessageBox.warning( + self, + self.tr("Warning: rename partially unsuccessful"), + self.tr("Some properties could not be changed to the new " + "name. The system has now both {} and {} qubes. " + "To resolve this, please check and change the " + "following properties and remove the qube {} " + "manually.
".format( + self.vm.name, name, self.vm.name) + list_text)) except qubesadmin.exc.QubesException as qex: t_monitor.set_error_msg(str(qex)) @@ -494,13 +520,31 @@ class VMSettingsWindow(ui_settingsdlg.Ui_SettingsDialog, QtGui.QDialog): t_monitor.set_finished() def rename_vm(self): + + dependencies = admin_utils.vm_dependencies(self.vm.app, self.vm) + + running_dependencies = [vm.name for (vm, prop) in dependencies + if vm and prop == 'template' + and vm.is_running()] + + if running_dependencies: + QtGui.QMessageBox.warning( + self, + self.tr("Qube cannot be renamed!"), + self.tr( + "The following qubes using this qube as a template are " + "running:
{}.
In order to rename this qube, you " + "must first shut them down.".format( + ", ".join(running_dependencies)))) + return + new_vm_name, ok = QtGui.QInputDialog.getText( self, self.tr('Rename qube'), self.tr('New name: (WARNING: all other changes will be discarded)')) if ok: - if self._run_in_thread(self._rename_vm, new_vm_name): + if self._run_in_thread(self._rename_vm, new_vm_name, dependencies): self.done(0) def _remove_vm(self, t_monitor): @@ -516,6 +560,21 @@ class VMSettingsWindow(ui_settingsdlg.Ui_SettingsDialog, QtGui.QDialog): def remove_vm(self): + dependencies = admin_utils.vm_dependencies(self.vm.app, self.vm) + + if dependencies: + list_text = utils.format_dependencies_list(dependencies) + QtGui.QMessageBox.warning( + self, + self.tr("Qube cannot be removed!"), + self.tr("This qube cannot be removed. It is used as:" + "
{} If you want to remove this qube, " + "you should remove or change settings of each qube " + "or setting that uses it.".format(list_text))) + + return + + answer, ok = QtGui.QInputDialog.getText( self, self.tr('Delete qube'), diff --git a/qubesmanager/utils.py b/qubesmanager/utils.py index 01913b8..80d9b40 100644 --- a/qubesmanager/utils.py +++ b/qubesmanager/utils.py @@ -172,3 +172,18 @@ def get_path_from_vm(vm, service_name): assert '\0' not in untrusted_path return untrusted_path.strip() raise ValueError('Unexpected characters in path.') + + +def format_dependencies_list(dependencies): + """Given a list of tuples representing properties, formats them in + a readable list.""" + + list_text = "" + for (holder, prop) in dependencies: + if holder is None: + list_text += "- Global property {}
".format(prop) + else: + list_text += "- {} for qube {}
".format( + prop, holder.name) + + return list_text From 027abd8ac3ca7921f2e24a9708a0bf122cd87c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marta=20Marczykowska-G=C3=B3recka?= Date: Fri, 20 Jul 2018 00:25:27 +0200 Subject: [PATCH 3/4] Pylint corrections Pylint 2.0 was complaining. Cosmetic fixes to make it stop. --- qubesmanager/restore.py | 3 ++- test-packages/qubesadmin/utils.py | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/qubesmanager/restore.py b/qubesmanager/restore.py index b36ec52..18e628c 100644 --- a/qubesmanager/restore.py +++ b/qubesmanager/restore.py @@ -134,6 +134,7 @@ class RestoreVMsWindow(ui_restoredlg.Ui_Restore, QtGui.QWizard): if self.verify_only.isChecked(): self.backup_restore.options.verify_only = True + # pylint: disable=assignment-from-no-return self.vms_to_restore = self.backup_restore.get_restore_info() for vmname in self.vms_to_restore: @@ -182,7 +183,7 @@ class RestoreVMsWindow(ui_restoredlg.Ui_Restore, QtGui.QWizard): self.__fill_vms_list__() elif self.currentPage() is self.confirm_page: - + # pylint: disable=assignment-from-no-return self.vms_to_restore = self.backup_restore.get_restore_info() for i in range(self.select_vms_widget.available_list.count()): diff --git a/test-packages/qubesadmin/utils.py b/test-packages/qubesadmin/utils.py index 6915104..bd109e0 100644 --- a/test-packages/qubesadmin/utils.py +++ b/test-packages/qubesadmin/utils.py @@ -8,3 +8,6 @@ def updates_vms_status(*args, **kwargs): def size_to_human(*args, **kwargs): return args[0] + +def vm_dependencies(*args): + return args[0] From 5e52b16fd37d3f06967a30447de7eca4e749050a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marta=20Marczykowska-G=C3=B3recka?= Date: Fri, 20 Jul 2018 17:48:39 +0200 Subject: [PATCH 4/4] Minor changes as requested Minor changes as requested by @marmarek --- qubesmanager/qube_manager.py | 2 +- qubesmanager/settings.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/qubesmanager/qube_manager.py b/qubesmanager/qube_manager.py index 17e9e62..04ab7ca 100644 --- a/qubesmanager/qube_manager.py +++ b/qubesmanager/qube_manager.py @@ -704,7 +704,7 @@ class VmManagerWindow(ui_qubemanager.Ui_VmManagerWindow, QtGui.QMainWindow): self.tr("This qube cannot be removed. It is used as:" "
{} If you want to remove this qube, " "you should remove or change settings of each qube " - "or setting that uses it.".format(list_text))) + "or setting that uses it.").format(list_text)) info_dialog.setModal(False) info_dialog.show() self.qt_app.processEvents() diff --git a/qubesmanager/settings.py b/qubesmanager/settings.py index 8c1d705..c73e0b7 100755 --- a/qubesmanager/settings.py +++ b/qubesmanager/settings.py @@ -495,7 +495,7 @@ class VMSettingsWindow(ui_settingsdlg.Ui_SettingsDialog, QtGui.QDialog): else: setattr(holder, prop, new_vm) except qubesadmin.exc.QubesException as qex: - failed_props += (holder, prop) + failed_props += [(holder, prop)] if not failed_props: del self.vm.app.domains[self.vm.name] @@ -509,8 +509,8 @@ class VMSettingsWindow(ui_settingsdlg.Ui_SettingsDialog, QtGui.QDialog): "name. The system has now both {} and {} qubes. " "To resolve this, please check and change the " "following properties and remove the qube {} " - "manually.
".format( - self.vm.name, name, self.vm.name) + list_text)) + "manually.
").format( + self.vm.name, name, self.vm.name) + list_text) except qubesadmin.exc.QubesException as qex: t_monitor.set_error_msg(str(qex)) @@ -534,8 +534,8 @@ class VMSettingsWindow(ui_settingsdlg.Ui_SettingsDialog, QtGui.QDialog): self.tr( "The following qubes using this qube as a template are " "running:
{}.
In order to rename this qube, you " - "must first shut them down.".format( - ", ".join(running_dependencies)))) + "must first shut them down.").format( + ", ".join(running_dependencies))) return new_vm_name, ok = QtGui.QInputDialog.getText( @@ -570,7 +570,7 @@ class VMSettingsWindow(ui_settingsdlg.Ui_SettingsDialog, QtGui.QDialog): self.tr("This qube cannot be removed. It is used as:" "
{} If you want to remove this qube, " "you should remove or change settings of each qube " - "or setting that uses it.".format(list_text))) + "or setting that uses it.").format(list_text)) return