GNOME Bugzilla – Bug 784923
Delete a Virtual Machine on Gnome-Boxes
Last modified: 2018-01-09 11:23:43 UTC
If you delete a virtual machine on gnome-boxes it will give you a message to undo the process. If you wait it and do not click on the undo button, gnome-boxes will start the process to finally delete this virtual machine. If you click on undo it will bring it back and cancel de process of delete. The problem here is when you choose to delete a vm and close the gnome-boxes straight away. I've tried to close the gnome-boxes using 2 approachs: 1) By killing gnome-boxes using killall gnome-boxes - In this case when I open gnome-boxes the virtual machine I asked to delete will be there, listed and not deleted. 2) By clicking on the close buttom - In this case the process will keep running on the bg consuming 100% of the CPU and so far, the only way I've found to stop it is to kill the process. Before I kill the process I waited 10 minutes and nothing happened. If both cases when I lauch the gnome-boxes the virtual macine I asked to delete will be there.
Which version of Boxes is this? I fixed such an issue in bug#761479 (Boxes version: 3.20) a while ago.
(In reply to Zeeshan Ali (Khattak) from comment #1) > Which version of Boxes is this? I fixed such an issue in bug#761479 (Boxes > version: 3.20) a while ago. Hi Zeeshan. Thanks for the reply. 5 [remote "origin"] 6 url = git://git.gnome.org/gnome-boxes 7 fetch = +refs/heads/*:refs/remotes/origin/* 8 [branch "master"] 9 remote = origin 10 merge = refs/heads/master 11 [submodule "libgd"] 12 active = true 13 url = git://git.gnome.org/libgd I'm on the streets right now but I'll have a check (read the thread) at the bug#761479 as soon as I'm back to the computer. Are you at the IRC #boxes channel?
(In reply to Thiago Mendes from comment #2) > (In reply to Zeeshan Ali (Khattak) from comment #1) > > Which version of Boxes is this? I fixed such an issue in bug#761479 (Boxes > > version: 3.20) a while ago. > > Hi Zeeshan. Thanks for the reply. > > 5 [remote "origin"] > 6 url = git://git.gnome.org/gnome-boxes > 7 fetch = +refs/heads/*:refs/remotes/origin/* > 8 [branch "master"] > 9 remote = origin > 10 merge = refs/heads/master > 11 [submodule "libgd"] > 12 active = true > 13 url = git://git.gnome.org/libgd > > I'm on the streets right now but I'll have a check (read the thread) at the > bug#761479 as soon as I'm back to the computer. Sure. > Are you at the IRC #boxes > channel? When I am on IRC, yes. :)
(In reply to Thiago Mendes from comment #0) > 1) By killing gnome-boxes using killall gnome-boxes > - In this case when I open gnome-boxes the virtual machine I asked to > delete will be there, listed and not deleted. That's an expected outcome. We cannot guarantee the integrity of the data if the user is playing the system under the hood. > 2) By clicking on the close buttom > - In this case the process will keep running on the bg consuming 100% of > the CPU Did you benchmark your CPU after deleting a machine AND dismissing the notification? I wonder if the consumption has anything to do with when the deletion truly occurs. > If both cases when I lauch the gnome-boxes the virtual macine I asked to > delete will be there. If the machine still persists, that's a bug. I am unable to reproduce it so far (deleting a machine is really deleting for me). Can you supply us with more info regarding how to reproduce the issue? Is anybody else experiencing the "non deletion"?
Sure I can :) > That's an expected outcome. We cannot guarantee the integrity of the data if the > user is playing the system under the hood. Ok :D > Did you benchmark your CPU after deleting a machine AND dismissing the notification? I wonder if the consumption has anything to do with when the deletion truly occurs. The only thing I did was to check the top output and there I saw gnome-boxes using 100% my cpu even after the main window is gone. The process still had something to do but I waited for a long period nothing happened. After killing it my CPU was back to the normal. > If the machine still persists, that's a bug. I am unable to reproduce it so far > (deleting a machine is really deleting for me). > Can you supply us with more info regarding how to reproduce the issue? Is anybody else experiencing the "non deletion"? My assumptions here are: When you delete a domain using the gnome-boxes interface it will hold that command until a timer expire or until the user click on the X button at the UNDO notification. In case of none of the options become true the domain will stay there and show up at the next time I launch gnome-boxes. The first scenario is ok because a Signalkill will not give the sotware time to handle the timeout or anything else. The second scenario it'll be up to gnome-boxes 'cause it will have time to decide what it will do before finish. In that case, if I'm right the way you reproduce the issue is: -> Right click in a domain -> Click on the delete option -> Close gnome-boxes straightaway -> check if the gnome-boxes still running using htop I had success reproducing the bug here but it could have something else that I don't know. I'll check it again as soon as I'm back to my computer. I haven't checked the code yet to understand where, when and how it happens but I wanna do it soon. As soon as I have more info (If I do have it) about it I share it with you. :D
Created attachment 355794 [details] [review] Fix Removing the notification from the active_notification GList before call the object.destroy. This will stop gnome-boxes from consuming 100% of the cpu and run the rest of the shutdown function.
Review of attachment 355794 [details] [review]: While I'll let Felipe do the actual review of the patch, I'll point out the formatting issues: The subject line is supposed to be a very short summary. If you haven't seen yet, you want to follow this guideline: https://wiki.gnome.org/Git/CommitMessages/ ::: src/notificationbar.vala @@ +79,3 @@ // We destroy all active notifications, which will cause them to be dismissed while (active_notifications != null) { + foreach (var notification in active_notifications) { too much indentation: needs to be indented by 4 chars only.
Created attachment 355829 [details] [review] Fix: Using 4 chars
Created attachment 355880 [details] [review] dismiss all notification Fix indent style and the commit message.
*** Bug 781515 has been marked as a duplicate of this bug. ***
Review of attachment 355880 [details] [review]: ::: src/libvirt-machine.vala @@ -495,3 @@ try { storage_volume.delete (0); - } catch (GLib.Error err) { nothing changed here, please drop this from the patch. ::: src/notificationbar.vala @@ +82,3 @@ + active_notifications.remove(notification); + notification.destroy(); + } Have you seen Bug 695122?
Ah, it is worth mentioning that the desired behavior here includes having the box deleted when you click Delete and close Boxes right away.
(In reply to Felipe Borges from comment #11) > Review of attachment 355880 [details] [review] [review]: > > ::: src/libvirt-machine.vala > @@ -495,3 @@ > try { > storage_volume.delete (0); > - } catch (GLib.Error err) { > > nothing changed here, please drop this from the patch. > > ::: src/notificationbar.vala > @@ +82,3 @@ > + active_notifications.remove(notification); > + notification.destroy(); > + } > > Have you seen Bug 695122? Felipe. Seeing it now! It seems like the patch for the bug#695122 do the same thing in a better way :D :D :D and the patch for the bug#695122 should also fix this one here.
(In reply to Felipe Borges from comment #12) > Ah, it is worth mentioning that the desired behavior here includes having > the box deleted when you click Delete and close Boxes right away. It does it already. The problem is that boxes get stucked at this part of the code (before it really do the delete thing) and you will have to kill the process and in that case the delete will not happen! But with this patch or the patch from the bug#695122 it should work and delete it. I'm gonna test it with the patch from the bug#695122 as soon as I'm back to my computer and I update you here.
(In reply to Thiago Mendes from comment #14) > (In reply to Felipe Borges from comment #12) > > Ah, it is worth mentioning that the desired behavior here includes having > > the box deleted when you click Delete and close Boxes right away. > > It does it already. The problem is that boxes get stucked at this part of > the code (before it really do the delete thing) and you will have to kill > the process and in that case the delete will not happen! But with this patch > or the patch from the bug#695122 it should work and delete it. > > I'm gonna test it with the patch from the bug#695122 as soon as I'm back to > my computer and I update you here. Sorry Felipe. My mistake. It does not delete like I said before! Gonna work here to have the desired behavior!
(In reply to Felipe Borges from comment #12) > Ah, it is worth mentioning that the desired behavior here includes having > the box deleted when you click Delete and close Boxes right away. Felipe. I have a patch but I'm wondering here for the best approach and I tell you why. I went for the second approach but the first one is shorter and light (I would prefer the first one). When boxes remove a domain as we know it puts on wait but one of the firts thing it does is remove this domain from the collection list. So!! If the user wait for the timeout or click on the close button it removes everything and this domain will never go back to the collection. The domain will only go back to the collection if the user clicks on "undo". Buuut: If the user quit boxes before the timeout and not clicking at the close button I won't have this domain at the collection to check any attribute from it when the app is shutting down. Without it on the collection I can't know a thing about that "removed" domain. I could: 1) create a new collection like "waiting_for_actions" where I will "move" all the machines that are still waiting for any action and I would check that list and take an action when boxes quit. (I vote for this one) 2) create a new attribute for Machine and set it as true or false it latter but in that case I can't remove the Machine from the collection before it is really gone! (I have this patch) What is the problem here with the second one? The thumb will not disappear as soon as the user click on delete :(. If we choose this way I would write a message "deleting" as the Machine status. (not sure if I'm clear here) and as soon this domain is really delete it would be removed from the window list.
Created attachment 356386 [details] [review] dismiss all notifications Will dismiss all the notifications when boxes is closed
Created attachment 356387 [details] [review] Delete virtual machines when quitting boxes This is the firts approach. Just let me know if you like the second on better so I send you another patch!
*** Bug 789520 has been marked as a duplicate of this bug. ***
Review of attachment 356386 [details] [review]: That's not related to the issue.
Review of attachment 356387 [details] [review]: .
I could debug the issue and it seems that everything below the dismissal of notifications in the App shutdown method doesn't get executed.
Created attachment 366501 [details] [review] notificationbar: Call each notification dismiss() at cleanup Each Notification object wraps in a GLib.Timeout in order to schedule the execution of the callback. Boxes gets stuck if it is closed when a notification hasn't finished yet. The solution for that is to make sure that each Notification's dimiss() method gets called when the GtkApplication shutdown method is invoked. This way we can ensure that all the sources are removed from the default main context.
Attachment 366501 [details] pushed as 81722d6 - notificationbar: Call each notification dismiss() at cleanup