vm-creator: More reliable post-installation setup

We need to make sure that post-installation setup is only done once.
App.add_domain() gets called twice (assuming libvirt events are working)
at domain creation time. Then we have the case of this function being
called for existing domains when Boxes starts-up. This function adds the
new machine to collection, that emits 'item_added' signal and ultimately
that trigered the post_installation setup.

We no longer listen to the 'item_added' signal on the collection but
launch the post-installation setup (or hook it to domain shutdown)
where/when its appropriate.

This fixes the recent regression of live VMs failing on first launch. It
also re-adds the launch of post-installation setup for existing domains
on boxes startup.

https://bugzilla.gnome.org/show_bug.cgi?id=678894

3 files changed, 39 insertions, 33 deletions

diff --git a/src/app.vala b/src/app.vala
index 6c40220..980eda6 100644
--- a/src/app.vala
+++ b/src/app.vala
@@ -184,7 +184,8 @@ private class Boxes.App: Boxes.UI {
         return machine;
     }
 
-    private void try_add_domain (CollectionSource source, GVir.Connection connection, GVir.Domain domain) {
+    // New == Added after Boxes launch
+    private void try_add_new_domain (CollectionSource source, GVir.Connection connection, GVir.Domain domain) {
         try {
             add_domain (source, connection, domain);
         } catch (GLib.Error error) {
@@ -192,6 +193,21 @@ private class Boxes.App: Boxes.UI {
         }
     }
 
+    // Existing == Existed before Boxes was launched
+    private void try_add_existing_domain (CollectionSource source, GVir.Connection connection, GVir.Domain domain) {
+        try {
+            var machine = add_domain (source, connection, domain);
+            var config = machine.domain_config;
+
+            if (VMConfigurator.is_install_config (config) || VMConfigurator.is_live_config (config)) {
+                debug ("Continuing installation/live session for '%s', ..", machine.name);
+                new VMCreator.for_install_completion (machine); // This instance will take care of its own lifecycle
+            }
+        } catch (GLib.Error error) {
+            warning ("Failed to create source '%s': %s", source.name, error.message);
+        }
+    }
+
     public void delete_machine (Machine machine, bool by_user = true) {
         machine.delete (by_user);         // Will also delete associated storage volume if by_user is 'true'
         collection.remove_item (machine);
@@ -223,7 +239,7 @@ private class Boxes.App: Boxes.UI {
         }
 
         foreach (var domain in connection.get_domains ())
-            try_add_domain (source, connection, domain);
+            try_add_existing_domain (source, connection, domain);
 
         connection.domain_removed.connect ((connection, domain) => {
             var machine = domain.get_data<LibvirtMachine> ("machine");
@@ -235,7 +251,7 @@ private class Boxes.App: Boxes.UI {
 
         connection.domain_added.connect ((connection, domain) => {
             debug ("New domain '%s'", domain.get_name ());
-            try_add_domain (source, connection, domain);
+            try_add_new_domain (source, connection, domain);
         });
     }
 
diff --git a/src/libvirt-machine.vala b/src/libvirt-machine.vala
index 69256d3..0bf7b9e 100644
--- a/src/libvirt-machine.vala
+++ b/src/libvirt-machine.vala
@@ -8,6 +8,8 @@ private class Boxes.LibvirtMachine: Boxes.Machine {
     public GVir.Connection connection;
     private string? storage_volume_path;
 
+    public VMCreator? vm_creator; // Under installation if this is set to non-null
+
     public bool save_on_quit {
         get { return source.get_boolean ("source", "save-on-quit"); }
         set { source.set_boolean ("source", "save-on-quit", value); }
diff --git a/src/vm-creator.vala b/src/vm-creator.vala
index c217dee..300ed16 100644
--- a/src/vm-creator.vala
+++ b/src/vm-creator.vala
@@ -4,41 +4,20 @@ using Osinfo;
 using GVir;
 
 private class Boxes.VMCreator {
-    public InstallerMedia install_media { get; private set; }
+    public InstallerMedia? install_media { get; private set; }
 
     private Connection? connection { get { return App.app.default_connection; } }
-    private ulong stopped_id;
+    private ulong state_changed_id;
 
     public VMCreator (InstallerMedia install_media) {
         this.install_media = install_media;
-
-        App.app.collection.item_added.connect (on_item_added);
     }
 
-    private void on_item_added (Collection collection, CollectionItem item) {
-        if (!(item is LibvirtMachine))
-            return;
-
-        var machine = item as LibvirtMachine;
-        if (machine.connection != connection)
-            return;
-
-        try {
-            var config = machine.domain.get_config (0);
-            if (!VMConfigurator.is_install_config (config) && !VMConfigurator.is_live_config (config)) {
-                debug ("'%s' does not need post-installation setup", machine.name);
-                return;
-            }
+    public VMCreator.for_install_completion (LibvirtMachine machine) {
+        state_changed_id = machine.notify["state"].connect (on_machine_state_changed);
+        machine.vm_creator = this;
 
-            var state = machine.domain.get_info ().state;
-            if (state == DomainState.SHUTOFF || state == DomainState.CRASHED || state == DomainState.NONE)
-                on_domain_stopped (machine);
-            else {
-                stopped_id = machine.domain.stopped.connect (() => { on_domain_stopped (machine); });
-            }
-        } catch (GLib.Error error) {
-            warning ("Failed to get information on domain '%s': %s", machine.domain.get_name (), error.message);
-        }
+        on_machine_state_changed (machine);
     }
 
     public async LibvirtMachine create_vm (Cancellable? cancellable) throws GLib.Error {
@@ -88,9 +67,17 @@ private class Boxes.VMCreator {
                 return;
             });
         }
+
+        state_changed_id = machine.notify["state"].connect (on_machine_state_changed);
+        machine.vm_creator = this;
     }
 
-    private void on_domain_stopped (LibvirtMachine machine) {
+    private void on_machine_state_changed (GLib.Object object, GLib.ParamSpec? pspec = null) {
+        var machine = object as LibvirtMachine;
+
+        if (machine.state != Machine.MachineState.STOPPED && machine.state != Machine.MachineState.UNKNOWN)
+            return;
+
         var domain = machine.domain;
 
         if (machine.deleted) {
@@ -113,7 +100,8 @@ private class Boxes.VMCreator {
             } catch (GLib.Error error) {
                 warning ("Failed to start domain '%s': %s", domain.get_name (), error.message);
             }
-            domain.disconnect (stopped_id);
+            machine.disconnect (state_changed_id);
+            machine.vm_creator = null;
         } else {
             try {
                 var config = domain.get_config (0);
@@ -122,7 +110,7 @@ private class Boxes.VMCreator {
                     return;
 
                 // No installation during live session, so lets delete the VM
-                domain.disconnect (stopped_id);
+                machine.disconnect (state_changed_id);
                 App.app.delete_machine (machine);
             } catch (GLib.Error error) {
                 warning ("Failed to delete domain '%s' or its volume: %s", domain.get_name (), error.message);