GNOME Bugzilla – Bug 693134
Be more robust when restarting installations
Last modified: 2016-03-31 13:59:20 UTC
When Boxes is restarted, it will automatically restart domains that were doing an unattended installation. While doing that, the code assumes the ISO image used to start the installation will still be present in the domain XML configuration. However, this is not necessarily true as it's possible that the installation was completed outside of Boxes (using virt-viewer for example), and that the user then ejected the ISO image using virsh or virt-manager. In such a situation, Boxes will crash at startup, which should never happen. This commit adds null checks here and there and fixes the issue, but hopefully a more elegant solution can be found...
Created attachment 235135 [details] [review] Be more robust when restarting installations
Not at all keen on supporting the "use case" of user mingling with VMs outside of Boxes, especially while they are still under installation. If they want to do that, they *must* also be smart enough to reset the installation state in the configuration.
Regardless of what the user does, Boxes shouldn't crash mysteriously during startup. I'm only asking for 'not crash' here, I'm not pushing for some kind of 'perfect' behaviour when the VM state has changed behind our back.
(In reply to comment #3) > Regardless of what the user does, Boxes shouldn't crash mysteriously during > startup. I'm only asking for 'not crash' here, I'm not pushing for some kind of > 'perfect' behaviour when the VM state has changed behind our back. Not against null checks and ensuring we don't crash. :) I got the wrong impression from "but hopefully a more elegant solution can be found..." that you would like to implement a proper solution for this use case.
Review of attachment 235135 [details] [review]: Rest looks good. ::: src/vm-creator.vala @@ +82,3 @@ var name = machine.domain.get_name (); + + if (install_media == null) { Can we be certain that installation is complete if there is no installer media anymore? In either case, a debug would be nice here.
(In reply to comment #4) > "hopefully a more elegant solution can be found..." that you would like to > implement a proper solution for this use case. A more elegant way of avoiding this crash code-wise, I don't really like randomly adding null checks here and there in some code that was not designed to cope with a missing InstallerMedia.
Review of attachment 235135 [details] [review]: ::: src/vm-creator.vala @@ +82,3 @@ var name = machine.domain.get_name (); + + if (install_media == null) { Nope,we cannot be certain at all, but I'm not really sure what we can do in this case. We can only make a guess, which will be true sometimes, false sometimes, but I don't think we can always guess right. Since the next thing this method do is call an install_media method to continue the install, it seemed better to exit early, and give up on unattended install magic since we no longer now in which state the VM is.
Review of attachment 235135 [details] [review]: ::: src/vm-creator.vala @@ +82,3 @@ var name = machine.domain.get_name (); + + if (install_media == null) { ah ok, just add a debug then and we are good.
Attachment 235135 [details] pushed as 4bc7402 - Be more robust when restarting installations