Clean up 'VM Restore' functionality.

Actually catch error messages. Don't check save file header magic when
libvirt already does it for us.

src/virtManager/connection.py

@@ -36,6 +36,10 @@ from virtManager.network import vmmNetwork
 from virtManager.netdev import vmmNetDevice
 from virtManager.storagepool import vmmStoragePool
 
+XEN_SAVE_MAGIC = "LinuxGuestRecord"
+QEMU_SAVE_MAGIC = "LibvirtQemudSave"
+TEST_SAVE_MAGIC = "TestGuestMagic"
+
 def get_local_hostname():
     try:
         return gethostbyaddr(gethostname())[0]
@@ -296,6 +300,32 @@ class vmmConnection(gobject.GObject):
     def get_pool(self, uuid):
         return self.pools[uuid]
 
+    def is_valid_saved_image(self, path):
+        # FIXME: Not really sure why we are even doing this check? If libvirt
+        # isn't exporting this information, seems like we shouldn't be duping
+        # the validation. Maintain existing behavior until someone insists
+        # otherwise I suppose.
+        magic = ""
+
+        # If running on PolKit or remote, we may not be able to access
+        if not self.is_remote() and os.access(path, os.R_OK):
+            try:
+                f = open(path, "r")
+                magic = f.read(16)
+            except:
+                logging.exception("Reading save image file header failed.")
+                return False
+        else:
+            return True
+
+        driver = self.get_driver()
+        if driver == "xen" and not magic.startswith(XEN_SAVE_MAGIC):
+            return False
+
+        # Libvirt should validate the magic for other drivers
+        return True
+
+
     def open(self):
         if self.state != self.STATE_DISCONNECTED:
             return
@@ -584,10 +614,15 @@ class vmmConnection(gobject.GObject):
         self.vmm.defineXML(xml)
 
     def restore(self, frm):
-        status = self.vmm.restore(frm)
-        if(status == 0):
+        self.vmm.restore(frm)
+        try:
+            # FIXME: This isn't correct in the remote case. Why do we even
+            #        do this? Seems like we should provide an option for this
+            #        to the user.
             os.remove(frm)
-        return status
+        except:
+            logging.debug("Couldn't remove save file '%s'used for restore." %
+                          frm)
 
     def _update_nets(self):
         """Return lists of start/stopped/new networks"""

src/virtManager/manager.py

@@ -320,7 +320,8 @@ class vmmManager(gobject.GObject):
             self.enable_polling(None, None, init_val, typ)
 
         # store any error message from the restore-domain callback
-        self.domain_restore_error = ""
+        self.restore_err = ""
+        self.restore_err_details = ""
 
         self.window.get_widget("menu_file_restore_saved").set_sensitive(False)
 
@@ -383,7 +384,7 @@ class vmmManager(gobject.GObject):
                                  self.config.get_default_save_dir(conn))
 
         if path:
-            if self.is_valid_saved_image(path):
+            if conn.is_valid_saved_image(path):
                 progWin = vmmAsyncJob(self.config,
                                       self.restore_saved_callback,
                                       [path],
@@ -394,24 +395,21 @@ class vmmManager(gobject.GObject):
                                    "valid saved machine image") % path)
                 return
 
-        if self.domain_restore_error != "":
-            self.err.val_err(self.domain_restore_error)
-            self.domain_restore_error = ""
-
-    def is_valid_saved_image(self, savfile):
-        try:
-            f = open(savfile, "r")
-            magic = f.read(16)
-            if magic != "LinuxGuestRecord" and magic != "LibvirtQemudSave":
-                return False
-            return True
-        except:
-            return False
+        if self.restore_err != "":
+            self.err.show_err(self.restore_err, self.restore_err_details,
+                              title=_("Error restoring domain"))
+            self.restore_err = ""
+            self.restore_details = ""
 
     def restore_saved_callback(self, file_to_load, ignore1=None):
-        status = self.current_connection().restore(file_to_load)
-        if(status != 0):
-            self.domain_restore_error = _("Error restoring domain '%s'. Is the domain already running?") % file_to_load
+        try:
+            self.current_connection().restore(file_to_load)
+        except Exception, e:
+            self.restore_err = (_("Error restoring domain '%s': %s") %
+                                  (file_to_load, str(e)))
+            self.restore_err_details = "".join(traceback.format_exc())
+            return
+
 
     def vm_view_changed(self, src):
         vmlist = self.window.get_widget("vm-list")