Merge remote-tracking branch 'origin/pr/188'

* origin/pr/188:
  updater: make pylint happy
  updater: better restarting logic

qui/updater/summary_page.py

@@ -131,9 +131,9 @@ def refresh_buttons(self):
     def on_header_toggled(self, _emitter):
         """Handles clicking on header checkbox.
 
-        Cycle between selection from appvms which templates was updated :
-         <1> only sys-vms
-         <2> sys-vms + other appvms but without excluded in settings
+        Cycle between selection from appvms which templates was updated:
+         <1> only servicevms
+         <2> servicevms + other appvms but without excluded in settings
          <3> all appvms
          <4> no vm. (nothing)
 
@@ -210,14 +210,14 @@ def populate_restart_list(self, restart, vm_updated, settings):
                     vm.klass != 'DispVM' or not vm.auto_cleanup):
                 self.list_store.append_vm(vm)
 
-        if settings.restart_system_vms:
-            self.head_checkbox.allow_sys()
+        if settings.restart_service_vms:
+            self.head_checkbox.allow_service_vms()
         if settings.restart_other_vms:
-            self.head_checkbox.allow_non_sys()
+            self.head_checkbox.allow_non_service_vms()
         if not restart:
             self.head_checkbox.state = HeaderCheckbox.NONE
         else:
-            if settings.restart_system_vms:
+            if settings.restart_service_vms:
                 self.head_checkbox.state = HeaderCheckbox.SAFE
             if settings.restart_other_vms:
                 self.head_checkbox.state = HeaderCheckbox.EXTENDED
@@ -226,13 +226,13 @@ def populate_restart_list(self, restart, vm_updated, settings):
     def select_rows(self):
         for row in self.list_store:
             row.selected = (
-                    row.is_sys_qube
+                    row.is_service_qube
                     and not row.is_excluded
-                    and AppVMType.SYS in self.head_checkbox.allowed
+                    and AppVMType.SERVICEVM in self.head_checkbox.allowed
                     or
-                    not row.is_sys_qube
+                    not row.is_service_qube
                     and not row.is_excluded
-                    and AppVMType.NON_SYS in self.head_checkbox.allowed
+                    and AppVMType.NON_SERVICEVM in self.head_checkbox.allowed
                     or
                     AppVMType.EXCLUDED in self.head_checkbox.allowed
             )
@@ -254,7 +254,7 @@ def restart_selected_vms(self):
             # show wainting dialog
             spinner = Gtk.Spinner()
             spinner.start()
-            dialog = show_dialog(None, l("Restarting qubes"), l(
+            dialog = show_dialog(None, l("Applying updates to qubes"), l(
                 "Waiting for qubes to be restarted/shutdown."),
                                  {}, spinner)
             dialog.set_deletable(False)
@@ -282,11 +282,11 @@ def perform_restart(self):
         to_restart = [qube_row.vm
                       for qube_row in self.list_store
                       if qube_row.selected
-                      and qube_row.is_sys_qube]
+                      and qube_row.is_service_qube]
         to_shutdown = [qube_row.vm
                        for qube_row in self.list_store
                        if qube_row.selected
-                       and not qube_row.is_sys_qube]
+                       and not qube_row.is_service_qube]
 
         if not any([tmpls_to_shutdown, to_restart, to_shutdown]):
             self.status = RestartStatus.NOTHING_TO_DO
@@ -381,9 +381,9 @@ def selected(self, value):
 
     def refresh_additional_info(self):
         self.raw_row[RestartRowWrapper._ADDITIONAL_INFO] = ''
-        if self.selected and not self.is_sys_qube:
+        if self.selected and not self.is_service_qube:
             self.raw_row[RestartRowWrapper._ADDITIONAL_INFO] = \
-                'Restarting an app qube will shut down all running applications'
+                'This qube and all running applications within will be shutdown'
         if self.selected and self.is_excluded:
             self.raw_row[RestartRowWrapper._ADDITIONAL_INFO] = \
                 '<span foreground="red">This qube has been explicitly ' \
@@ -406,17 +406,17 @@ def additional_info(self):
         return self.raw_row[self._ADDITIONAL_INFO]
 
     @property
-    def is_sys_qube(self):
-        return str(self.vm.name).startswith("sys-")
+    def is_service_qube(self):
+        return get_boolean_feature(self.vm, 'servicevm', False)
 
     @property
     def is_excluded(self):
         return not get_boolean_feature(self.vm, 'restart-after-update', True)
 
 
 class AppVMType:
-    SYS = 0
-    NON_SYS = 1
+    SERVICEVM = 0
+    NON_SERVICEVM = 1
     EXCLUDED = 2
 
 
@@ -433,22 +433,22 @@ def __init__(self, checkbox_column_button, next_button):
                          [None, None, AppVMType.EXCLUDED])
         self.next_button = next_button
 
-    def allow_sys(self, value=True):
+    def allow_service_vms(self, value=True):
         if value:
-            self._allowed[0] = AppVMType.SYS
+            self._allowed[0] = AppVMType.SERVICEVM
         else:
             self._allowed[0] = None
 
-    def allow_non_sys(self, value=True):
+    def allow_non_service_vms(self, value=True):
         if value:
-            self._allowed[1] = AppVMType.NON_SYS
+            self._allowed[1] = AppVMType.NON_SERVICEVM
         else:
             self._allowed[1] = None
 
     # pylint: disable=arguments-differ
     def all_action(self, num, *args, **kwargs):
-        text = ngettext("_Finish and restart %(num)d qube",
-                        "_Finish and restart %(num)d qubes",
+        text = ngettext("_Finish and restart/shutdown %(num)d qube",
+                        "_Finish and restart/shutdown %(num)d qubes",
                         num) % {'num': num}
         self.next_button.set_label(text)
 

qui/updater/tests/conftest.py

@@ -230,6 +230,7 @@ def remove(self, idx):
             self.raw_rows.remove(idx)
 
         def set_sort_func(self, _col, _sort_func, _data):
+            """not used in tests"""
             pass
 
     return MockListStore()
@@ -240,7 +241,7 @@ def mock_settings():
     class MockSettings:
         def __init__(self):
             self.update_if_stale = 7
-            self.restart_system_vms = True
+            self.restart_service_vms = True
             self.restart_other_vms = True
             self.max_concurrency = None
 

qui/updater/tests/test_summary_page.py

@@ -78,14 +78,14 @@ def test_on_header_toggled(
 
     sut.list_store = appvms_list
     all_num = len(appvms_list)
-    sut.head_checkbox._allowed[0] = AppVMType.SYS
-    sys_num = 3
-    sut.head_checkbox._allowed[1] = AppVMType.NON_SYS
+    sut.head_checkbox._allowed[0] = AppVMType.SERVICEVM
+    service_num = 3
+    sut.head_checkbox._allowed[1] = AppVMType.NON_SERVICEVM
     non_excluded_num = 6
 
     sut.head_checkbox.state = HeaderCheckbox.NONE
 
-    for expected in (0, sys_num, non_excluded_num, all_num, 0):
+    for expected in (0, service_num, non_excluded_num, all_num, 0):
         selected_num = len([row for row in sut.list_store if row.selected])
         assert selected_num == expected
         assert sut.head_checkbox_button.get_inconsistent() \
@@ -109,8 +109,8 @@ def test_on_checkbox_toggled(
     )
 
     sut.list_store = appvms_list
-    sut.head_checkbox._allowed[0] = AppVMType.SYS
-    sut.head_checkbox._allowed[1] = AppVMType.NON_SYS
+    sut.head_checkbox._allowed[0] = AppVMType.SERVICEVM
+    sut.head_checkbox._allowed[1] = AppVMType.NON_SERVICEVM
 
     sut.head_checkbox.state = HeaderCheckbox.NONE
     sut.head_checkbox.set_buttons()
@@ -153,30 +153,30 @@ def test_on_checkbox_toggled(
 
 # expected data based on test_qapp setup
 UP_VMS = 7
-UP_SYS_VMS = 3
+UP_SERVICE_VMS = 3
 UP_APP_VMS = 4
 
 
 @pytest.mark.parametrize(
-    "restart_system_vms, restart_other_vms, excluded, expected",
+    "restart_service_vms, restart_other_vms, excluded, expected",
     (
         pytest.param(True, True, (), UP_VMS),
-        pytest.param(True, False, (), UP_SYS_VMS),
+        pytest.param(True, False, (), UP_SERVICE_VMS),
         pytest.param(False, False, (), 0),
         pytest.param(False, True, (), UP_APP_VMS),
         pytest.param(True, True, ("test-blue",), UP_VMS - 1),
-        pytest.param(True, False, ("test-blue",), UP_SYS_VMS),
+        pytest.param(True, False, ("test-blue",), UP_SERVICE_VMS),
         pytest.param(False, True, ("sys-usb",), UP_APP_VMS),
-        pytest.param(True, False, ("sys-usb",), UP_SYS_VMS - 1),
+        pytest.param(True, False, ("sys-usb",), UP_SERVICE_VMS - 1),
     ),
 )
 def test_populate_restart_list(
-        restart_system_vms, restart_other_vms, excluded, expected,
+        restart_service_vms, restart_other_vms, excluded, expected,
         real_builder, test_qapp, updatable_vms_list,
         mock_next_button, mock_cancel_button, mock_settings, mock_tree_view
 ):
     mock_settings.restart_other_vms = restart_other_vms
-    mock_settings.restart_system_vms = restart_system_vms
+    mock_settings.restart_service_vms = restart_service_vms
     for exclude in excluded:
         test_qapp.expected_calls[
             (exclude, "admin.vm.feature.Get", 'restart-after-update', None)

qui/updater/tests/test_updater_settings.py

@@ -31,7 +31,7 @@
 
 def init_features(test_qapp):
     test_qapp.expected_calls[
-        ('dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-system', None)
+        ('dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-servicevms', None)
     ] = b"0\x00" + str(1).encode()
     test_qapp.expected_calls[
         ('dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-other', None)
@@ -66,24 +66,24 @@ def test_update_if_stale(test_qapp):
     assert sut.update_if_stale == 7
 
 
-def test_restart_system_vms(test_qapp):
+def test_restart_service_vms(test_qapp):
     mock_log = Mock()
     sut = Settings(Gtk.Window(), test_qapp, mock_log, lambda *args: None)
     test_qapp.expected_calls[
-        ('dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-system', None)
+        ('dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-servicevms', None)
     ] = b'2\x00QubesFeatureNotFoundError\x00\x00' \
-        + b'qubes-vm-update-restart-system' + b'\x00'
-    assert sut.restart_system_vms
+        + b'qubes-vm-update-restart-servicevms' + b'\x00'
+    assert sut.restart_service_vms
     test_qapp.expected_calls[
-        ('dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-system', None)
+        ('dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-servicevms', None)
     ] = b"0\x00" + ''.encode()
-    assert not sut.restart_system_vms
+    assert not sut.restart_service_vms
     test_qapp.expected_calls[
         (
-        'dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-system', None)
+        'dom0', 'admin.vm.feature.Get', 'qubes-vm-update-restart-servicevms', None)
     ] = b'2\x00QubesFeatureNotFoundError\x00\x00' \
-            + b'qubes-vm-update-restart-system' + b'\x00'
-    assert sut.restart_system_vms
+            + b'qubes-vm-update-restart-servicevms' + b'\x00'
+    assert sut.restart_service_vms
 
 
 def test_restart_other_vms(test_qapp):
@@ -143,8 +143,8 @@ def __call__(self, value):
     (
         pytest.param("update-if-stale", Settings.DEFAULT_UPDATE_IF_STALE, 30,
                      "days_without_update_button"),
-        pytest.param("restart-system", Settings.DEFAULT_RESTART_SYSTEM_VMS,
-                     False, "restart_system_checkbox"),
+        pytest.param("restart-servicevms", Settings.DEFAULT_RESTART_SERVICEVMS,
+                     False, "restart_servicevms_checkbox"),
         pytest.param("restart-other", Settings.DEFAULT_RESTART_OTHER_VMS,
                      True, "restart_other_checkbox"),
     ),
@@ -222,9 +222,9 @@ def test_save(feature, default_value, new_value, test_qapp, button_name):
 
 def test_limit_concurrency(test_qapp):
     dom0_set_max_concurrency = ('dom0', 'admin.vm.feature.Set',
-                                f'qubes-vm-update-max-concurrency',)
+                                'qubes-vm-update-max-concurrency',)
     dom0_get_max_concurrency = ('dom0', 'admin.vm.feature.Get',
-                                f'qubes-vm-update-max-concurrency', None)
+                                'qubes-vm-update-max-concurrency', None)
 
     mock_log = Mock()
     sut = Settings(Gtk.Window(), test_qapp, mock_log, lambda *args: None)

qui/updater/updater_settings.py

@@ -49,7 +49,7 @@ class Settings:
     MAX_CONCURRENCY = 16
     DEFAULT_UPDATE_IF_STALE = 7
     MAX_UPDATE_IF_STALE = 99
-    DEFAULT_RESTART_SYSTEM_VMS = True
+    DEFAULT_RESTART_SERVICEVMS = True
     DEFAULT_RESTART_OTHER_VMS = False
 
     def __init__(
@@ -93,9 +93,9 @@ def __init__(
         )
         self.days_without_update_button.configure(adj, 1, 0)
 
-        self.restart_system_checkbox: Gtk.CheckButton = self.builder.get_object(
-            "restart_system")
-        self.restart_system_checkbox.connect(
+        self.restart_servicevms_checkbox: Gtk.CheckButton = \
+            self.builder.get_object("restart_servicevms")
+        self.restart_servicevms_checkbox.connect(
             "toggled", self._show_restart_exceptions)
 
         self.restart_other_checkbox: Gtk.CheckButton = self.builder.get_object(
@@ -129,7 +129,7 @@ def __init__(
         self.max_concurrency_button.configure(adj, 1, 0)
 
         self._init_update_if_stale: Optional[int] = None
-        self._init_restart_system_vms: Optional[bool] = None
+        self._init_restart_servicevms: Optional[bool] = None
         self._init_restart_other_vms: Optional[bool] = None
         self._init_limit_concurrency: Optional[bool] = None
         self._init_max_concurrency: Optional[int] = None
@@ -143,13 +143,24 @@ def update_if_stale(self) -> int:
                                Settings.DEFAULT_UPDATE_IF_STALE))
 
     @property
-    def restart_system_vms(self) -> bool:
+    def restart_service_vms(self) -> bool:
         """Return the current (set by this window or manually) option value."""
         if self.overrides.restart is not None:
             return self.overrides.restart
-        return get_boolean_feature(
-            self.vm, "qubes-vm-update-restart-system",
-            Settings.DEFAULT_RESTART_SYSTEM_VMS)
+
+        result = get_boolean_feature(
+            self.vm, "qubes-vm-update-restart-servicevms",
+            None)
+        # TODO
+        #  If not set, try to use a deprecated flag instead of
+        #  the default value. This is only for backward compatibility
+        #  and should be removed in future (e.g. Qubes 5.0 or later)
+        if result is None:
+            result = get_boolean_feature(
+                self.vm, "qubes-vm-update-restart-system",
+                Settings.DEFAULT_RESTART_SERVICEVMS)
+
+        return result
 
     @property
     def restart_other_vms(self) -> bool:
@@ -176,10 +187,12 @@ def load_settings(self):
         self.days_without_update_button.set_sensitive(
             not self.overrides.update_if_stale)
 
-        self._init_restart_system_vms = self.restart_system_vms
+        self._init_restart_servicevms = self.restart_service_vms
         self._init_restart_other_vms = self.restart_other_vms
-        self.restart_system_checkbox.set_sensitive(not self.overrides.restart)
-        self.restart_system_checkbox.set_active(self._init_restart_system_vms)
+        self.restart_servicevms_checkbox.set_sensitive(
+            not self.overrides.restart)
+        self.restart_servicevms_checkbox.set_active(
+            self._init_restart_servicevms)
         self.restart_other_checkbox.set_active(self._init_restart_other_vms)
         self.restart_other_checkbox.set_sensitive(not self.overrides.restart)
 
@@ -226,11 +239,15 @@ def save_and_close(self, _emitter):
         )
 
         self._save_option(
-            name="restart-system",
-            value=self.restart_system_checkbox.get_active(),
-            init=self._init_restart_system_vms,
-            default=Settings.DEFAULT_RESTART_SYSTEM_VMS
+            name="restart-servicevms",
+            value=self.restart_servicevms_checkbox.get_active(),
+            init=self._init_restart_servicevms,
+            default=Settings.DEFAULT_RESTART_SERVICEVMS
         )
+        # TODO
+        #  Make sure that the deprecated flag is unset.
+        #  Should be removed in future (e.g. Qubes 5.0 or later)
+        apply_feature_change(self.vm, "qubes-vm-update-restart-system", None)
 
         self._save_option(
             name="restart-other",

qui/updater_settings.glade

@@ -174,8 +174,8 @@
                       </packing>
                     </child>
                     <child>
-                      <object class="GtkCheckButton" id="restart_system">
-                        <property name="label" translatable="yes">Restart all _system qubes after update by default</property>
+                      <object class="GtkCheckButton" id="restart_servicevms">
+                        <property name="label" translatable="yes">Restart all _service qubes after update by default</property>
                         <property name="visible">True</property>
                         <property name="can-focus">True</property>
                         <property name="receives-default">False</property>