Created attachment 300145 [details] [review]
Don't try to call get_mode() on null GVirConfigDomainCpu

Review of attachment 300145 [details] [review]:

* I don't see any usecase of this but since we can handle the case without complicating the code, I don't see why not.

* Shorter shortlog please and add a prefix as per conventions. I'd go for "vm-configurator: Handle "no cpu" case gracefully".

* Empty line before the pasted critical error please.

::: src/vm-configurator.vala
@@ +243,3 @@
+                    var capabilities = yield connection.get_capabilities_async (null);
+                    set_cpu_config (domain, capabilities);
+                }

Since null cpu is more of a corner/erroneous case here, I'd rather avoid further indenting/nesting the code for it. i-e:

if (cpu != null &&
    cpu.get_mode () == .. &&
    cpu.get_mode () ==)

Created attachment 300147 [details] [review]
vm-configurator: Handle "no cpu" case gracefully

A domain config can contain no CPU node (for example if it was removed
using virsh edit). Such a domain triggers a warning on Boxes startup:

(gnome-boxes:15373): Libvirt.GConfig-CRITICAL **: gvir_config_domain_cpu_get_mode: assertion 'GVIR_CONFIG_IS_DOMAIN_CPU(cpu)' failed

This commit makes sure we got a non-null GVirConfigDomainCpu before
attempting to call get_mode() on it.

Review of attachment 300147 [details] [review]:

::: src/vm-configurator.vala
@@ +241,1 @@
                 is_boxes_installed (domain)) {

No idea about the right indentation here.

Review of attachment 300147 [details] [review]:

ack

::: src/vm-configurator.vala
@@ +241,1 @@
                 is_boxes_installed (domain)) {

seems right to me.

Attachment 300147 [details] pushed as a6970ac - vm-configurator: Handle "no cpu" case gracefully