Merge remote-tracking branch 'qubesos/pr/55'

* qubesos/pr/55:
  restore.py: add template-related debug log messages
  restory.py: restore template_for_dispvms property early
  restore.py: factor out _restore_property() helper
  restore.py: deal with DispVM templates
  restore.py: sort into three tiers in _templates_first()
  restore.py: don't try to restore dispid property
  qvm-backup: let backup core handle default VM selection
  qvm-backup-restore: really pass options
  qvm-backup-restore: remove orphaned --replace-template option

doc/manpages/qvm-backup-restore.rst

@@ -53,11 +53,6 @@ Options
 
    Restore VMs that are already present on the host under different names
 
-.. option:: --replace-template=REPLACE_TEMPLATE
-
-    Restore VMs using another template, syntax:
-    ``old-template-name:new-template-name`` (might be repeated)
-
 .. option:: --exclude=EXCLUDE, -x EXCLUDE
 
     Skip restore of specified VM (might be repeated)

qubesadmin/backup/restore.py

@@ -1472,26 +1472,50 @@ class BackupRestore(object):
 
             # check template
             if vm_info.template:
-                template_name = vm_info.template
-                try:
-                    host_template = self.app.domains[template_name]
-                except KeyError:
-                    host_template = None
-                present_on_host = (host_template and
-                    host_template.klass == 'TemplateVM')
-                present_in_backup = (template_name in restore_info.keys() and
-                    restore_info[template_name].good_to_go and
-                    restore_info[template_name].vm.klass ==
-                    'TemplateVM')
+                present_on_host = False
+                if vm_info.template in self.app.domains:
+                    host_tpl = self.app.domains[vm_info.template]
+                    if vm_info.vm.klass == 'DispVM':
+                        present_on_host = (
+                            getattr(host_tpl, 'template_for_dispvms', False))
+                    else:
+                        present_on_host = host_tpl.klass == 'TemplateVM'
+
+                present_in_backup = False
+                if vm_info.template in restore_info:
+                    bak_tpl = restore_info[vm_info.template]
+                    if bak_tpl.good_to_go:
+                        if vm_info.vm.klass == 'DispVM':
+                            present_in_backup = (
+                                bak_tpl.vm.properties.get(
+                                    'template_for_dispvms', False))
+                        else:
+                            present_in_backup = (
+                                bak_tpl.vm.klass == 'TemplateVM')
+
+                self.log.debug(
+                    "vm=%s template=%s on_host=%s in_backup=%s",
+                    vm_info.name, vm_info.template,
+                    present_on_host, present_in_backup)
+
                 if not present_on_host and not present_in_backup:
-                    if self.options.use_default_template and \
-                            self.app.default_template:
+                    if vm_info.vm.klass == 'DispVM':
+                        default_template = self.app.default_dispvm
+                    else:
+                        default_template = self.app.default_template
+
+                    if (self.options.use_default_template
+                            and default_template is not None):
                         if vm_info.orig_template is None:
-                            vm_info.orig_template = template_name
-                        vm_info.template = self.app.default_template.name
+                            vm_info.orig_template = vm_info.template
+                        vm_info.template = default_template.name
+
+                        self.log.debug(
+                            "vm=%s orig_template=%s -> default_template=%s",
+                            vm_info.name, vm_info.orig_template,
+                            default_template.name)
                     else:
-                        vm_info.problems.add(
-                            self.VMToRestore.MISSING_TEMPLATE)
+                        vm_info.problems.add(self.VMToRestore.MISSING_TEMPLATE)
 
             # check netvm
             if vm_info.vm.properties.get('netvm', None) is not None:
@@ -1664,18 +1688,19 @@ class BackupRestore(object):
 
     @staticmethod
     def _templates_first(vms):
-        '''Sort templates befor other VM types (AppVM etc)'''
+        '''Sort templates before other VM types'''
         def key_function(instance):
             '''Key function for :py:func:`sorted`'''
             if isinstance(instance, BackupVM):
-                return instance.klass == 'TemplateVM'
+                if instance.klass == 'TemplateVM':
+                    return 0
+                elif instance.properties.get('template_for_dispvms', False):
+                    return 1
+                return 2
             elif hasattr(instance, 'vm'):
                 return key_function(instance.vm)
-            return 0
-        return sorted(vms,
-            key=key_function,
-            reverse=True)
-
+            return 9
+        return sorted(vms, key=key_function)
 
     def _handle_dom0(self, stream):
         '''Extract dom0 home'''
@@ -1805,6 +1830,14 @@ class BackupRestore(object):
             self.log.info("-> Please install updates for all the restored "
                           "templates.")
 
+    def _restore_property(self, vm, prop, value):
+        '''Restore a single VM property, logging exceptions'''
+        try:
+            setattr(vm, prop, value)
+        except Exception as err:  # pylint: disable=broad-except
+            self.log.error('Error setting %s.%s to %s: %s',
+                vm.name, prop, value, err)
+
     def _restore_vms_metadata(self, restore_info):
         '''Restore VM metadata
 
@@ -1860,6 +1893,12 @@ class BackupRestore(object):
                         del self.app.domains[new_vm.name]
                     continue
 
+            # restore this property early to be ready for dependent DispVMs
+            prop = 'template_for_dispvms'
+            value = vm.properties.get(prop, None)
+            if value is not None:
+                self._restore_property(new_vm, prop, value)
+
             restore_info[vm.name].restored_vm = new_vm
 
         for vm in vms.values():
@@ -1872,15 +1911,14 @@ class BackupRestore(object):
                 continue
 
             for prop, value in vm.properties.items():
+                # can't reset the first; already handled the second
+                if prop in ['dispid', 'template_for_dispvms']:
+                    continue
                 # exclude VM references - handled manually according to
                 # restore options
                 if prop in ['template', 'netvm', 'default_dispvm']:
                     continue
-                try:
-                    setattr(new_vm, prop, value)
-                except Exception as err:  # pylint: disable=broad-except
-                    self.log.error('Error setting %s.%s to %s: %s',
-                        vm.name, prop, value, err)
+                self._restore_property(new_vm, prop, value)
 
             for feature, value in vm.features.items():
                 try:

qubesadmin/tests/tools/qvm_backup.py

@@ -33,10 +33,7 @@ class TC_00_qvm_backup(qubesadmin.tests.QubesTestCase):
         profile = io.StringIO()
         qvm_backup.write_backup_profile(profile, args)
         expected_profile = (
-            'destination_path: /var/tmp\n'
-            'destination_vm: dom0\n'
-            'include: [\'$type:AppVM\', \'$type:TemplateVM\', '
-            '\'$type:StandaloneVM\']\n'
+            '{destination_path: /var/tmp, destination_vm: dom0, include: null}\n'
         )
         self.assertEqual(profile.getvalue(), expected_profile)
 
@@ -67,8 +64,7 @@ class TC_00_qvm_backup(qubesadmin.tests.QubesTestCase):
             'destination_path: /var/tmp\n'
             'destination_vm: dom0\n'
             'exclude: [vm1, vm2]\n'
-            'include: [\'$type:AppVM\', \'$type:TemplateVM\', '
-            '\'$type:StandaloneVM\']\n'
+            'include: null\n'
         )
         self.assertEqual(profile.getvalue(), expected_profile)
 
@@ -77,11 +73,7 @@ class TC_00_qvm_backup(qubesadmin.tests.QubesTestCase):
         profile = io.StringIO()
         qvm_backup.write_backup_profile(profile, args, passphrase='test123')
         expected_profile = (
-            'destination_path: /var/tmp\n'
-            'destination_vm: dom0\n'
-            'include: [\'$type:AppVM\', \'$type:TemplateVM\', '
-            '\'$type:StandaloneVM\']\n'
-            'passphrase_text: test123\n'
+            '{destination_path: /var/tmp, destination_vm: dom0, include: null, passphrase_text: test123}\n'
         )
         self.assertEqual(profile.getvalue(), expected_profile)
 
@@ -101,10 +93,7 @@ class TC_00_qvm_backup(qubesadmin.tests.QubesTestCase):
             qvm_backup.main(['--save-profile', 'test-profile', '/var/tmp'],
                 app=self.app)
             expected_profile = (
-                'destination_path: /var/tmp\n'
-                'destination_vm: dom0\n'
-                'include: [\'$type:AppVM\', \'$type:TemplateVM\', '
-                '\'$type:StandaloneVM\']\n'
+                '{destination_path: /var/tmp, destination_vm: dom0, include: null}\n'
             )
             with open(profile_path) as f:
                 self.assertEqual(expected_profile, f.read())
@@ -130,11 +119,8 @@ class TC_00_qvm_backup(qubesadmin.tests.QubesTestCase):
             qvm_backup.main(['--save-profile', 'test-profile', '/var/tmp'],
                 app=self.app)
             expected_profile = (
-                'destination_path: /var/tmp\n'
-                'destination_vm: dom0\n'
-                'include: [\'$type:AppVM\', \'$type:TemplateVM\', '
-                '\'$type:StandaloneVM\']\n'
-                'passphrase_text: some password\n'
+                '{destination_path: /var/tmp, destination_vm: dom0, include: null, passphrase_text: some\n'
+                '    password}\n'
             )
             with open(profile_path) as f:
                 self.assertEqual(expected_profile, f.read())
@@ -194,8 +180,7 @@ class TC_00_qvm_backup(qubesadmin.tests.QubesTestCase):
             'destination_path: /var/tmp\n'
             'destination_vm: dom0\n'
             'exclude: [vm1]\n'
-            'include: [\'$type:AppVM\', \'$type:TemplateVM\', '
-            '\'$type:StandaloneVM\']\n'
+            'include: null\n'
             '# specify backup passphrase below\n'
             'passphrase_text: ...\n'
         )
@@ -225,11 +210,8 @@ class TC_00_qvm_backup(qubesadmin.tests.QubesTestCase):
                     'test-profile', '/var/tmp'],
                     app=self.app)
             expected_profile = (
-                'destination_path: /var/tmp\n'
-                'destination_vm: dom0\n'
-                'include: [\'$type:AppVM\', \'$type:TemplateVM\', '
-                '\'$type:StandaloneVM\']\n'
-                'passphrase_text: other passphrase\n'
+                '{destination_path: /var/tmp, destination_vm: dom0, include: null, passphrase_text: other\n'
+                '    passphrase}\n'
             )
             with open(profile_path) as f:
                 self.assertEqual(expected_profile, f.read())

qubesadmin/tests/tools/qvm_backup_restore.py

@@ -93,7 +93,7 @@ class TC_00_qvm_backup_restore(qubesadmin.tests.QubesTestCase):
             app=self.app)
         mock_backup.assert_called_once_with(
             self.app, '/some/path', None, 'testpass')
-        self.assertEqual(exclude_list, ['test-vm2'])
+        self.assertEqual(mock_backup.return_value.options.exclude, ['test-vm2'])
         self.assertAllCalled()
 
     def test_010_handle_broken_no_problems(self):

qubesadmin/tools/qvm_backup.py

@@ -94,11 +94,7 @@ def write_backup_profile(output_stream, args, passphrase=None):
     '''
 
     profile_data = {}
-    if args.vms:
-        profile_data['include'] = args.vms
-    else:
-        profile_data['include'] = [
-            '$type:AppVM', '$type:TemplateVM', '$type:StandaloneVM']
+    profile_data['include'] = args.vms or None
     if args.exclude_list:
         profile_data['exclude'] = args.exclude_list
     if passphrase:

qubesadmin/tools/qvm_backup_restore.py

@@ -55,12 +55,6 @@ parser.add_argument("--rename-conflicting", action="store_true",
     help="Restore VMs that are already present on the host "
          "under different names")
 
-parser.add_argument("--replace-template", action="append",
-    dest="replace_template", default=[],
-    help="Restore VMs using another TemplateVM; syntax: "
-         "old-template-name:new-template-name (may be "
-         "repeated)")
-
 parser.add_argument("-x", "--exclude", action="append", dest="exclude",
     default=[],
     help="Skip restore of specified VM (may be repeated)")
@@ -229,20 +223,12 @@ def main(args=None, app=None):
     if args.ignore_missing:
         backup.options.use_default_template = True
         backup.options.use_default_netvm = True
-    if args.replace_template:
-        backup.options.replace_template = args.replace_template
-    if args.rename_conflicting:
-        backup.options.rename_conflicting = True
-    if not args.dom0_home:
-        backup.options.dom0_home = False
-    if args.ignore_username_mismatch:
-        backup.options.ignore_username_mismatch = True
-    if args.ignore_size_limit:
-        backup.options.ignore_size_limit = True
-    if args.exclude:
-        backup.options.exclude = args.exclude
-    if args.verify_only:
-        backup.options.verify_only = True
+    backup.options.rename_conflicting = args.rename_conflicting
+    backup.options.dom0_home = args.dom0_home
+    backup.options.ignore_username_mismatch = args.ignore_username_mismatch
+    backup.options.ignore_size_limit = args.ignore_size_limit
+    backup.options.exclude = args.exclude
+    backup.options.verify_only = args.verify_only
 
     restore_info = None
     try: