On 9/5/19 12:42 PM, Thomas Lamprecht wrote:
On 05.09.19 12:27, Dominik Csapak wrote:
On 9/5/19 11:47 AM, Thomas Lamprecht wrote:
On 23.08.19 10:55, Dominik Csapak wrote:
Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
+ open(my $fh, '>', "/run/qemu-server/$vmid.reboot")
+ or die "failed to create reboot trigger file: $!\n";
maybe indentation issue for or
what do you mean?
sorry, misinterpreted the tabulator in my MTA, ignore that
open[..]
<tab>or die[..]
is our indentation scheme?
+ close($fh);
+}
+
+sub remove_reboot_trigger {
maybe clear_reboot_request?
ok
+ my ($vmid) = @_;
+ return unlink("/run/qemu-server/$vmid.reboot");
FYI, unlink can also fail for other reasons that ENOENT,
which you use as return value, see `man 2 unlink`
Maybe do a bit more and actively check if the file is here first?
(This will be always racy and thus should only be done in locked
context anyway)
Or explicitly check if the error was ENOENT ?
was also copied from container, but yeah it makes to check if
it exists and log an error in case we cannot remove it
I nowhere saw anything mentioning that this all is just copied over from CT.
It makes sense that it's there too, but maybe we could add the common code
then to guest-common? (and fix it for both)
yeah i only briefly mentioned that its inspired by pve-container in 3/5
but yes makes sense to fix it for both and put it into
PVE::GuestHelpers in guest-common
i would also put the trigger/request creation code there
it would only reboot it in case we can remove the file though,
otherwise we risk that the user cannot shutdown the vm...
yes, all else is an error, but I would warn and say that reboot was ignored
because of "blah".
Another possible alternative, in combination with the separate vm_reboot
method from my 3/5 review, could be to remove this early in the locked context,
before doing the stop, and in the case of a error on removal just abort and
do nothing - then the VM would keep running, maybe a better option?
As a reboot always has the end goal to have a running machine again, if you
stop it in case of error the goal is violated, if you do nothing it isn't fully
violated (as it may still be outdated on pending changes)
but we have to somehow detect a reboot in the qm cleanup step
in which the vm is already stopped so that we can start it again.
i am not sure how this should work if we remove the file before
stopping? or am i missing something here?
+}
+
# bash completion helper
sub complete_backup_archives {
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
