After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 686329 - should do basic check if the disk is ready to boot
should do basic check if the disk is ready to boot
Status: RESOLVED WONTFIX
Product: gnome-boxes
Classification: Applications
Component: installer
unspecified
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks: 686781
 
 
Reported: 2012-10-17 19:44 UTC by Marc-Andre Lureau
Modified: 2016-03-31 13:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vm-creator: Delete under-install/live VM on media deletion (1.33 KB, patch)
2013-04-09 21:59 UTC, Zeeshan Ali
committed Details | Review

Description Marc-Andre Lureau 2012-10-17 19:44:01 UTC
I just started a manual install express of XP and quit early to reboot the VM, unfortunately, Boxes disconnected me, the state was still "Installing" and when starting the VM again, the install CD isn't present, so the machine just hangs.

It would be nice instead to detect that nothing happened on disk or the disk is not bootable and either:
- keep the CD mounted and changing state that VM isn't ready (Installing is fine then)
- delete the VM automatically
Comment 1 Zeeshan Ali 2013-04-09 21:59:22 UTC
Created attachment 241102 [details] [review]
vm-creator: Delete under-install/live VM on media deletion

Before continuing installation/live session on a machine, we now check
if the source installer media exists. If it doesn't, there is no point in
keeping around a useless machine so we automatically delete it.
Comment 2 Christophe Fergeau 2013-04-11 13:33:23 UTC
Review of attachment 241102 [details] [review]:

What was happening when someone did that before the patch? This does not look like this fixes what this bug is about, but this should make things more robust.

::: src/vm-creator.vala
@@ +89,3 @@
 
+        var file = GLib.File.new_for_path (install_media.device_file);
+        if (!file.query_exists ()) {

Why not g_file_test?
Comment 3 Zeeshan Ali 2013-04-11 13:48:22 UTC
Review of attachment 241102 [details] [review]:

Before this patch, we started the domain as if everything is normal and got the symptoms described in the bug description. But now that I read it again, it seems Marc-Andre didn't actually get to suspend/save the under-installation domain but rather installation continued while Boxes was not running? If so, Marc-Andre: was the installation complete before you deleted the ISO?

::: src/vm-creator.vala
@@ +89,3 @@
 
+        var file = GLib.File.new_for_path (install_media.device_file);
+        if (!file.query_exists ()) {

Didn't remember that api, I'll change this.
Comment 4 Christophe Fergeau 2013-04-11 14:27:14 UTC
Review of attachment 241102 [details] [review]:

I think that "the install CD isn't present" means that the install CD was no longer present in libvirt xml configuration for some reason, not that he deleted the ISO from the filesystem
Comment 5 Marc-Andre Lureau 2013-04-11 14:31:37 UTC
(In reply to comment #4)
> Review of attachment 241102 [details] [review]:
> 
> I think that "the install CD isn't present" means that the install CD was no
> longer present in libvirt xml configuration for some reason, not that he
> deleted the ISO from the filesystem

Yes. I guess it is easy to reproduce today: start a manual install, but cancel/quit/shutdown it early (via installaer interface). Start again the VM. Nowadays, it is not a big issue, as we can mount the ISO again. So perhaps we can ignore that now
Comment 6 Zeeshan Ali 2013-04-11 21:47:20 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Review of attachment 241102 [details] [review] [details]:
> > 
> > I think that "the install CD isn't present" means that the install CD was no
> > longer present in libvirt xml configuration for some reason, not that he
> > deleted the ISO from the filesystem
> 
> Yes. I guess it is easy to reproduce today: start a manual install, but
> cancel/quit/shutdown it early (via installaer interface).

Firstly this is not something that user is expected to do and when he/she does, all that happens is that they end up having a useless box. They can simply delete that and start over.

> Nowadays, it is not a big issue, as we can mount the ISO again. So perhaps we
> can ignore that now

Unless there is an easy way to detect if disk is bootable, I don't think its really an issue worth spending time on.
Comment 7 Zeeshan Ali 2013-04-11 21:50:11 UTC
Comment on attachment 241102 [details] [review]
vm-creator: Delete under-install/live VM on media deletion

Attachment 241102 [details] pushed as 786f195 - vm-creator: Delete under-install/live VM on media deletion
Comment 8 Marc-Andre Lureau 2013-04-12 02:18:36 UTC
(In reply to comment #6)
> > Nowadays, it is not a big issue, as we can mount the ISO again. So perhaps we
> > can ignore that now
> 
> Unless there is an easy way to detect if disk is bootable, I don't think its
> really an issue worth spending time on.

Not a big issue anymore, as I said. Still, it would be nice to leave the iso mounted. Perhaps do not unmount automatically in manual install would be more friendly.
Comment 9 Zeeshan Ali 2013-04-12 02:51:11 UTC
(In reply to comment #8)
> (In reply to comment #6)
> > > Nowadays, it is not a big issue, as we can mount the ISO again. So perhaps we
> > > can ignore that now
> > 
> > Unless there is an easy way to detect if disk is bootable, I don't think its
> > really an issue worth spending time on.
> 
> Not a big issue anymore, as I said. Still, it would be nice to leave the iso
> mounted. Perhaps do not unmount automatically in manual install would be more
> friendly.

Not really, in the typical usage user would want us to automatically eject the cdrom and I don't think installation being manual changes anything. In fact we didn't use to eject the cdrom until recently and we got requests for doing that. What we lack here is a more reliable method to detect if installation completed than counting number of reboots.
Comment 10 Zeeshan Ali 2013-04-13 14:42:06 UTC
Feel free to re-open this but if you do please also change the 'blocks' from bug#686781 to bug#696727 at the same time.