GNOME Bugzilla – Bug 696242
Allow rebooting saved VMs
Last modified: 2016-03-31 13:22:07 UTC
See comment#3.
Created attachment 239427 [details] [review] libvirt-machine-props: Micro refactor Instead of checking for machine being ON each time before calling notify_reboot_required(), better do this check in the function itself.
Created attachment 239428 [details] [review] libvirt-machine-props: Some more refactoring Put 'reboot' into a separate method. lambdas inside lambdas weren't make it easy to read this code.
Created attachment 239429 [details] [review] libvirt-machine-props: Allow rebooting saved VMs Currently, if user changes the properties of a saved VM that require reboot to take effect, we don't offer user the option to reboot. As a result the properties are not in effect when user restores the VM, which is contrary to what she/he will expect. This patch: * offers user the option to reboot saved VMs. * if user chooses to reboot, the VM is first restored and then rebooted.
These patches are based on fix in bug#696239.
Review of attachment 239429 [details] [review]: Haven't looked closely at the patches, but couldn't find the answer while looking at that patch. Is the timeout to switch from 'reboot' to 'force reboot' disabled when rebooting saved vms? I've been able to hit the 'force reboot' dialog when boxes tried to reboot an already running box, so I guess it will trigger every time when restarting a saved vm if it's not disabled.
Review of attachment 239429 [details] [review]: No its not disabled. If machine is in saved state, we simply start/resume it, hook-up to async start completing and return. When starting is done, 'reboot' is called for doing exactly what it did before this patch.
Review of attachment 239429 [details] [review]: Ah ok, sounds good to me, I was worried we would do start force-reboot timeout restore vm reboot rather than restore vm start force-reboot timeout reboot sounds good to me
Review of attachment 239427 [details] [review]: Failed to publish review: Undefined subroutine &extensions::splinter::lib::WSSplinter::ThrowCodeError called at /var/www/bugzilla.gnome.org/extensions/splinter/lib/WSSplinter.pm line 88.
What about rest of the patches?
Review of attachment 239428 [details] [review]: Diff's hard to read, but seems ok.
Review of attachment 239429 [details] [review]: Failed to publish review: Undefined subroutine &extensions::splinter::lib::WSSplinter::ThrowCodeError called at /var/www/bugzilla.gnome.org/extensions/splinter/lib/WSSplinter.pm line 88.