Did you do some manual changes in ~/.local/share/gnome-boxes ?

(In reply to comment #1)
> Did you do some manual changes in ~/.local/share/gnome-boxes ?

 ~/.local/share/gnome-boxes folder doesn't exist.

My question was if you had manually removed it, .. ?

(In reply to comment #3)
> My question was if you had manually removed it, .. ?

Yes, I removed the folder.

(In reply to comment #4)
> (In reply to comment #3)
> > My question was if you had manually removed it, .. ?
> 
> Yes, I removed the folder.

Thanks for the honesty. You are not supposed to do that. If you really want to manipulate storage pools, please do so using he virsh commands. Try `virsh pool-destroy gnome-boxes; virsh pool-undefine gnome-boxes` and then create the VM in Boxes.

(In reply to comment #5)
> Thanks for the honesty. You are not supposed to do that. If you really want to
> manipulate storage pools, please do so using he virsh commands. Try `virsh
> pool-destroy gnome-boxes; virsh pool-undefine gnome-boxes` and then create the
> VM in Boxes.

Sorry, but this is not an invalid bug. People are doing this anyway, rm -rf in ~ is a common way of freeing some space so we have to cope with it.
We can't expect people to be aware of magic commands they have to run to avoid breaking boxes totally simply by messing with their home dir.

(In reply to comment #6)
> (In reply to comment #5)
> > Thanks for the honesty. You are not supposed to do that. If you really want to
> > manipulate storage pools, please do so using he virsh commands. Try `virsh
> > pool-destroy gnome-boxes; virsh pool-undefine gnome-boxes` and then create the
> > VM in Boxes.
> 
> Sorry, but this is not an invalid bug. People are doing this anyway,

That does not at all make it a valid usecase.

> We can't expect people to be aware of magic commands they have to run to avoid
> breaking boxes totally simply by messing with their home dir.

Not at all but:

1. If they are typical end-users that we mainly target, they are not at all expected to even know or touch hidden directories directly, let alone do `rm -rf` on them.
2 If they are advanced/technical users then expecting them to use virsh instead is not at all too much to ask.

Knowing you, I bet you want Boxes to handle all possible scenarios and there is no way I can convince you out of this so feel free to provide a patch.

Until then this bug remains closed as INVALID.

(In reply to comment #7)
> > Sorry, but this is not an invalid bug. People are doing this anyway,
> 
> That does not at all make it a valid usecase.

Nope, but something we need to cope with.


> 2 If they are advanced/technical users then expecting them to use virsh instead
> is not at all too much to ask.

It is, they can be advanced/technical users without knowing anything about virt, and you can't expect them to realize they need to read up about libvirt before rm -rf'ing something to try to reset their config.

Should I be required to read up on libgphoto before tring to remove anything Photos related from my home dir? Then I'll read about libmtp before removing manually any Music files.

This bug is about writing a robust application that handles gracefully errors rather than writing an application that will never startup again if the user presses the wrong key at the wrong time, so let's keep that open.

(In reply to comment #8)
> (In reply to comment #7)
> > > Sorry, but this is not an invalid bug. People are doing this anyway,
> > 
> > That does not at all make it a valid usecase.
> 
> Nope, but something we need to cope with.
> 
> 
> > 2 If they are advanced/technical users then expecting them to use virsh instead
> > is not at all too much to ask.
> 
> It is, they can be advanced/technical users without knowing anything about
> virt, and you can't expect them to realize they need to read up about libvirt
> before rm -rf'ing something to try to reset their config.

If they are advanced users they should know that they are not supposed to mingle with *hidden* *project-specific* directories. Things should not break if anything in .cache is deleted but .config or .local  is supposed to keep important info that one should only touch when he/she knows exactly what he/she is doing.

> Should I be required to read up on libgphoto before tring to remove anything
> Photos related from my home dir? Then I'll read about libmtp before removing
> manually any Music files.

No, that is a very different case. Photos is not a hidden directory nor it is project-specific. A good example would be .local/share/gphoto.

> This bug is about writing a robust application that handles gracefully errors
> rather than writing an application that will never startup again if the user
> presses the wrong key at the wrong time, so let's keep that open.

No, that is you generalizing to something that this bug is not at all about. I do completely agree with what you say about being robust and Boxes should break just because user clicked/pressed wrong button/key at the wrong time but that is totally different from user doing things he/she is not supposed to do. With wrong generalization, you arrive at a wrong conclusion.

Anyways, I'm assigning the bug to you cause I don't want this to appear on my list of open bugs that I need to worry about.

(In reply to comment #9)
> Anyways, I'm assigning the bug to you cause I don't want this to appear on my
> list of open bugs that I need to worry about.

That's fine with me, and a much more acceptable answer than "Feel free to provide a patch. Until then this bug remains closed as INVALID."
Don't expect a quick patch though has that aforementioned behaviour has unsurprisingly had some 'stop energy' effect.
For what it's worth, I had sent https://www.redhat.com/archives/libvir-list/2013-March/msg01088.html before this bug was opened with exactly the intent of improving our handling of this...

*** Bug 698405 has been marked as a duplicate of this bug. ***

I'd expect user-created files to be within the user's remit. If I want to delete it, it shouldn't stop other applications from starting up.

Yeah,

Created attachment 260021 [details] [review]
Fix VM creation if userdatadir is missing

Review of attachment 260021 [details] [review]:

::: src/vm-creator.vala
@@ +196,2 @@
     protected async StoragePool get_storage_pool () throws GLib.Error {
+        // Workaround for https://bugzilla.gnome.org/show_bug.cgi?id=696360

Instead/apart from referring to the bug, I'd add a small explaination here why this is done. Something like "Ensure pool directory exists in case user deleted it after pool creation" is enough.

@@ +196,3 @@
     protected async StoragePool get_storage_pool () throws GLib.Error {
+        // Workaround for https://bugzilla.gnome.org/show_bug.cgi?id=696360
+        DirUtils.create_with_parents (GLib.Path.build_filename (get_user_pkgdata (), "images", null), 0755);

This breaks the pool creation case below under if (pool == null).

(In reply to comment #15)
> 
> This breaks the pool creation case below under if (pool == null).

I think we should not break that....

Created attachment 260043 [details] [review]
Fix VM creation if userdatadir is missing

Created attachment 260044 [details] [review]
Fix VM creation if userdatadir is missing

Review of attachment 260044 [details] [review]:

ack w/ that changed

::: src/vm-creator.vala
@@ +202,3 @@
+        }
+
+        if (pool.get_info ().state == StoragePoolState.INACTIVE) {

since this is unneeded when pool does not exist, lets put this 'if' as 'else if' to previous block.

But we still need to call start_async and refresh_async after the pool is created (it will be INACTIVE), so an else doesn't work. I admit it's a bit confusing as written; the alternative is to duplicate those calls in both blocks, which I didn't want to do, but that would be more clear.  So whichever version you prefer.

(In reply to comment #20)
> But we still need to call start_async and refresh_async after the pool is
> created (it will be INACTIVE), so an else doesn't work.

If that would have been the case, we'd hit this issue on first Boxes use, not just when user delets the directory. '(pool == null)' means pool has not yet been created, in which case the code you are adding is redundant.

In the patch I attached above, you really do have to go into that conditional after creating a new storage pool, otherwise the pool will never be started, since I moved the start_async and refresh_async calls into the new conditional. It's a bit tricky to see that from looking at just the patch.

Here's another version where those calls are present in both conditionals, so we can use else.  I think this second patch is more clear; the cost is two lines of code duplication.

Created attachment 260062 [details] [review]
Fix VM creation if userdatadir is missing

Created attachment 260063 [details] [review]
Fix VM creation if userdatadir is missing

Whoops, attached the old one again....

Review of attachment 260063 [details] [review]:

yeah, its clearer this way. ACK but please do test both cases before pushing. :)

I did a 'virsh pool-destroy gnome-boxes' and everything seems to work fine. Thanks for helping.

Attachment 260063 [details] pushed as f5bfb35 - Fix VM creation if userdatadir is missing

(In reply to comment #26)
> I did a 'virsh pool-destroy gnome-boxes' and everything seems to work fine.

You also want to try with `virsh pool-destroy gnome-boxes; virsh pool-undefine gnome-boxes`, which should simulate the first Boxes run in terms of storage pool. I assume you also tested the case this bug is about: user deleting the pool diretory manually after storage pool was setup by boxes.

(In reply to comment #28)
> (In reply to comment #26)
> > I did a 'virsh pool-destroy gnome-boxes' and everything seems to work fine.
> 
> You also want to try with `virsh pool-destroy gnome-boxes; virsh pool-undefine
> gnome-boxes`, which should simulate the first Boxes run in terms of storage
> pool.

Still works, yay.

> I assume you also tested the case this bug is about: user deleting the
> pool diretory manually after storage pool was setup by boxes.

Of course.