Suggested changes

included changes as suggested by @marmarek.

ci/pylintrc

@@ -27,6 +27,7 @@ disable=
   duplicate-code,
   file-ignored,
   fixme,
+  inconsistent-return-statements,
   locally-disabled,
   locally-enabled,
   logging-format-interpolation,

qubesmanager/backup.py

@@ -46,11 +46,11 @@ import time
 
 class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
 
-    def __init__(self, app, qvm_collection, parent=None):
+    def __init__(self, qt_app, qubes_app, parent=None):
         super(BackupVMsWindow, self).__init__(parent)
 
-        self.app = app
+        self.qt_app = qt_app
-        self.qvm_collection = qvm_collection
+        self.qubes_app = qubes_app
         self.backup_settings = QtCore.QSettings()
 
         self.selected_vms = []
@@ -98,9 +98,9 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
 
         self.target_vm_list, self.target_vm_idx = utils.prepare_vm_choice(
             self.appvm_combobox,
-            self.qvm_collection,
+            self.qubes_app,
             None,
-            self.qvm_collection.domains['dom0'],
+            self.qubes_app.domains['dom0'],
             (lambda vm: vm.klass != 'TemplateVM' and vm.is_running()),
             allow_internal=False,
             allow_default=False,
@@ -111,6 +111,13 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
         self.__fill_vms_list__(selected)
 
     def load_settings(self):
+        """
+        Helper function that tries to load existing backup profile
+        (default path: /etc/qubes/backup/qubes-manager-backup.conf )
+        and then apply its contents to the Backup window.
+        :return: list of vms to include in backup, if it exists in the profile,
+        or None if it does not
+        """
         try:
             profile_data = backup_utils.load_backup_profile()
         except FileNotFoundError:
@@ -147,6 +154,13 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
         return None
 
     def save_settings(self, use_temp):
+        """
+        Helper function that saves backup profile to either
+        /etc/qubes/backup/qubes-manager-backup.conf or
+        /etc/qubes/backup/qubes-manager-backup-tmp.conf
+        :param use_temp: whether to use temporary profile (True) or the default
+         backup profile (False)
+        """
         settings = {'destination_vm': self.appvm_combobox.currentText(),
                     'destination_path': self.dir_line_edit.text(),
                     'include': [vm.name for vm in self.selected_vms],
@@ -169,7 +183,7 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
                 vm.name + " (" + admin_utils.size_to_human(self.size) + ")")
 
     def __fill_vms_list__(self, selected=None):
-        for vm in self.qvm_collection.domains:
+        for vm in self.qubes_app.domains:
             if vm.features.get('internal', False):
                 continue
 
@@ -248,11 +262,11 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
         msg = []
 
         try:
-            vm = self.qvm_collection.domains[
+            vm = self.qubes_app.domains[
                 self.appvm_combobox.currentText()]
             if not vm.is_running():
                 vm.start()
-            self.qvm_collection.qubesd_call(
+            self.qubes_app.qubesd_call(
                 'dom0', 'admin.backup.Execute',
                 backup_utils.get_profile_name(True))
         except Exception as ex:  # pylint: disable=broad-except
@@ -263,13 +277,19 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
 
         t_monitor.set_finished()
 
+    @staticmethod
+    def cleanup_temporary_files():
+        try:
+            os.remove(backup_utils.get_profile_path(use_temp=True))
+        except FileNotFoundError:
+            pass
 
     def current_page_changed(self, page_id): # pylint: disable=unused-argument
         old_sigchld_handler = signal.signal(signal.SIGCHLD, signal.SIG_DFL)
         if self.currentPage() is self.confirm_page:
 
-            self.save_settings(True)
+            self.save_settings(use_temp=True)
-            backup_summary = self.qvm_collection.qubesd_call(
+            backup_summary = self.qubes_app.qubesd_call(
                 'dom0', 'admin.backup.Info',
                 backup_utils.get_profile_name(True))
 
@@ -280,7 +300,7 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
         elif self.currentPage() is self.commit_page:
 
             if self.save_profile_checkbox.isChecked():
-                self.save_settings(False)
+                self.save_settings(use_temp=False)
 
             self.button(self.FinishButton).setDisabled(True)
             self.showFileDialog.setEnabled(
@@ -296,7 +316,7 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
             thread.start()
 
             while not self.thread_monitor.is_finished():
-                self.app.processEvents()
+                self.qt_app.processEvents()
                 time.sleep(0.1)
 
             if not self.thread_monitor.success:
@@ -325,18 +345,21 @@ class BackupVMsWindow(ui_backupdlg.Ui_Backup, multiselectwidget.QtGui.QWizard):
             self.button(self.CancelButton).setEnabled(False)
             self.button(self.FinishButton).setEnabled(True)
             self.showFileDialog.setEnabled(False)
+            self.cleanup_temporary_files()
         signal.signal(signal.SIGCHLD, old_sigchld_handler)
 
     def reject(self):
         if self.currentPage() is self.commit_page:
             self.canceled = True
-            self.qvm_collection.qubesd_call(
+            self.qubes_app.qubesd_call(
                 'dom0', 'admin.backup.Cancel',
                 backup_utils.get_profile_name(True))
             self.progress_bar.setMaximum(100)
             self.progress_bar.setValue(0)
             self.button(self.CancelButton).setDisabled(True)
+            self.cleanup_temporary_files()
         else:
+            self.cleanup_temporary_files()
             self.done(0)
 
     def has_selected_vms(self):
@@ -374,21 +397,21 @@ def handle_exception(exc_type, exc_value, exc_traceback):
 
 def main():
 
-    qtapp = QtGui.QApplication(sys.argv)
+    qt_app = QtGui.QApplication(sys.argv)
-    qtapp.setOrganizationName("The Qubes Project")
+    qt_app.setOrganizationName("The Qubes Project")
-    qtapp.setOrganizationDomain("http://qubes-os.org")
+    qt_app.setOrganizationDomain("http://qubes-os.org")
-    qtapp.setApplicationName("Qubes Backup VMs")
+    qt_app.setApplicationName("Qubes Backup VMs")
 
     sys.excepthook = handle_exception
 
     app = Qubes()
 
-    backup_window = BackupVMsWindow(qtapp, app)
+    backup_window = BackupVMsWindow(qt_app, app)
 
     backup_window.show()
 
-    qtapp.exec_()
+    qt_app.exec_()
-    qtapp.exit()
+    qt_app.exit()
 
 
 if __name__ == "__main__":

qubesmanager/backup_utils.py

@@ -31,15 +31,18 @@ path_max_len = 512
 
 
 def fill_appvms_list(dialog):
+    """
+    Helper function, designed to fill the destination vm combobox in both backup
+    and restore GUI tools.
+    :param dialog: QtGui.QWizard with a combobox called appvm_combobox
+    """
     dialog.appvm_combobox.clear()
     dialog.appvm_combobox.addItem("dom0")
 
     dialog.appvm_combobox.setCurrentIndex(0)  # current selected is null ""
 
-    for vm in dialog.qvm_collection.domains:
+    for vm in dialog.qubes_app.domains:
-        if vm.klass == 'AppVM' and vm.features.get('internal', False):
+        if vm.features.get('internal', False) or vm.klass == 'TemplateVM':
-            continue
-        if vm.klass == 'TemplateVM' and vm.installed_by_rpm:
             continue
 
         if vm.is_running() and vm.qid != 0:
@@ -52,6 +55,17 @@ def enable_dir_line_edit(dialog, boolean):
 
 
 def select_path_button_clicked(dialog, select_file=False, read_only=False):
+    """
+    Helper function that displays a file/directory selection wizard. Used by
+    backup and restore GUI tools.
+    :param dialog: QtGui.QWizard with a dir_line_edit text box that wants to
+    receive a file/directory path and appvm_combobox with VM to use
+    :param select_file: True: select file dialog; False: select directory
+    dialog
+    :param read_only: should the dir_line_edit be changed after selecting a file
+    or directory
+    :return:
+    """
     backup_location = str(dialog.dir_line_edit.text())
     file_dialog = QtGui.QFileDialog()
     file_dialog.setReadOnly(True)
@@ -59,7 +73,7 @@ def select_path_button_clicked(dialog, select_file=False, read_only=False):
     new_path = None
 
     new_appvm = str(dialog.appvm_combobox.currentText())
-    vm = dialog.qvm_collection.domains[new_appvm]
+    vm = dialog.qubes_app.domains[new_appvm]
     try:
         new_path = utils.get_path_from_vm(
             vm,
@@ -81,6 +95,18 @@ def select_path_button_clicked(dialog, select_file=False, read_only=False):
         dialog.select_dir_page.emit(QtCore.SIGNAL("completeChanged()"))
 
 
+def get_profile_name(use_temp):
+    backup_profile_name = 'qubes-manager-backup'
+    temp_backup_profile_name = 'qubes-manager-backup-tmp'
+
+    return temp_backup_profile_name if use_temp else backup_profile_name
+
+
+def get_profile_path(use_temp):
+    path = '/etc/qubes/backup/' + get_profile_name(use_temp) + '.conf'
+    return path
+
+
 def load_backup_profile(use_temp=False):
 
     path = get_profile_path(use_temp)
@@ -102,15 +128,3 @@ def write_backup_profile(args, use_temp=False):
 
     with open(path, 'w') as profile_file:
         yaml.safe_dump(profile_data, profile_file)
-
-
-def get_profile_name(use_temp):
-    backup_profile_name = 'qubes-manager-backup'
-    temp_backup_profile_name = 'qubes-manager-backup-tmp'
-
-    return temp_backup_profile_name if use_temp else backup_profile_name
-
-
-def get_profile_path(use_temp):
-    path = '/etc/qubes/backup/' + get_profile_name(use_temp) + '.conf'
-    return path

qubesmanager/restore.py

@@ -28,6 +28,8 @@ import time
 import os
 import os.path
 import traceback
+import logging
+import logging.handlers
 
 import signal
 
@@ -46,15 +48,22 @@ from qubesadmin.backup import restore
 
 class RestoreVMsWindow(ui_restoredlg.Ui_Restore, QtGui.QWizard):
 
-    def __init__(self, app, qvm_collection, parent=None):
+    def __init__(self, qt_app, qubes_app, parent=None):
         super(RestoreVMsWindow, self).__init__(parent)
 
-        self.app = app
+        self.qt_app = qt_app
-        self.qvm_collection = qvm_collection
+        self.qubes_app = qubes_app
 
         self.vms_to_restore = None
         self.func_output = []
+
+        # Set up logging
         self.feedback_queue = Queue()
+        handler = logging.handlers.QueueHandler(self.feedback_queue)
+        logger = logging.getLogger('qubesadmin.backup')
+        logger.addHandler(handler)
+        logger.setLevel(logging.INFO)
+
         self.canceled = False
         self.error_detected = Event()
         self.thread_monitor = None
@@ -104,12 +113,12 @@ class RestoreVMsWindow(ui_restoredlg.Ui_Restore, QtGui.QWizard):
 
         self.target_appvm = None
         if self.appvm_combobox.currentIndex() != 0:   # An existing appvm chosen
-            self.target_appvm = self.qvm_collection.domains[
+            self.target_appvm = self.qubes_app.domains[
                 str(self.appvm_combobox.currentText())]
 
         try:
             self.backup_restore = restore.BackupRestore(
-                self.qvm_collection,
+                self.qubes_app,
                 self.dir_line_edit.text(),
                 self.target_appvm,
                 self.passphrase_line_edit.text()
@@ -138,13 +147,6 @@ class RestoreVMsWindow(ui_restoredlg.Ui_Restore, QtGui.QWizard):
     def append_output(self, text):
         self.commit_text_edit.append(text)
 
-    def restore_error_output(self, text):
-        self.error_detected.set()
-        self.append_output(u'<font color="red">{0}</font>'.format(text))
-
-    def restore_output(self, text):
-        self.append_output(u'<font color="black">{0}</font>'.format(text))
-
     def __do_restore__(self, t_monitor):
         err_msg = []
         try:
@@ -212,14 +214,20 @@ class RestoreVMsWindow(ui_restoredlg.Ui_Restore, QtGui.QWizard):
                                       args=(self.thread_monitor,))
             thread.daemon = True
             thread.start()
-
             while not self.thread_monitor.is_finished():
-                self.app.processEvents()
+                self.qt_app.processEvents()
                 time.sleep(0.1)
                 try:
-                    for (signal_to_emit, data) in iter(
+                    log_record = self.feedback_queue.get_nowait()
-                            self.feedback_queue.get_nowait, None):
+                    while log_record:
-                        self.emit(signal_to_emit, data)
+                        if log_record.levelno == logging.ERROR or\
+                                        log_record.levelno == logging.CRITICAL:
+                            output = '<font color="red">{0}</font>'.format(
+                                log_record.getMessage())
+                        else:
+                            output = log_record.getMessage()
+                        self.append_output(output)
+                        log_record = self.feedback_queue.get_nowait()
                 except Empty:
                     pass
 
@@ -238,7 +246,7 @@ class RestoreVMsWindow(ui_restoredlg.Ui_Restore, QtGui.QWizard):
                     '<b><font color="black">{0}</font></b>'.format(
                         self.tr("Please unmount your backup volume and cancel "
                                 "the file selection dialog.")))
-                self.app.processEvents()
+                self.qt_app.processEvents()
                 backup_utils.select_path_button_clicked(self, False, True)
 
             self.button(self.FinishButton).setEnabled(True)
@@ -305,21 +313,21 @@ def handle_exception(exc_type, exc_value, exc_traceback):
 
 def main():
 
-    qtapp = QtGui.QApplication(sys.argv)
+    qt_app = QtGui.QApplication(sys.argv)
-    qtapp.setOrganizationName("The Qubes Project")
+    qt_app.setOrganizationName("The Qubes Project")
-    qtapp.setOrganizationDomain("http://qubes-os.org")
+    qt_app.setOrganizationDomain("http://qubes-os.org")
-    qtapp.setApplicationName("Qubes Restore VMs")
+    qt_app.setApplicationName("Qubes Restore VMs")
 
     sys.excepthook = handle_exception
 
-    app = Qubes()
+    qubes_app = Qubes()
 
-    restore_window = RestoreVMsWindow(qtapp, app)
+    restore_window = RestoreVMsWindow(qt_app, qubes_app)
 
     restore_window.show()
 
-    qtapp.exec_()
+    qt_app.exec_()
-    qtapp.exit()
+    qt_app.exit()
 
 
 if __name__ == "__main__":

test-packages/qubesadmin/exc.py

@@ -1,4 +1,6 @@
-# pylint: disable=unused-variable### mock qubesadmin.exc module
+### mock qubesadmin.exc module
+# pylint: disable=unused-variable
+
 
 class QubesException(BaseException):
     pass