[pve-devel] [PATCH qemu-server v2 1/5] add create and remove reboot trigger subs

[Thomas Lamprecht]

On 23.08.19 10:55, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> new in v2

trigger is normally something which is done actively, i.e., gets triggered
like "pulling the trigger" says.

Maybe call it a flag and the file "$vmid.reboot-request"?

>  PVE/QemuServer.pm | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 9f5bf56..d1767a9 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -7369,6 +7369,18 @@ sub nbd_stop {
>      vm_mon_cmd($vmid, 'nbd-server-stop');
>  }
>  
> +sub create_reboot_trigger {

create_reboot_request?

> +    my ($vmid) = @_;

please add a newline here

> +    open(my $fh, '>', "/run/qemu-server/$vmid.reboot")
> +	or die "failed to create reboot trigger file: $!\n";

maybe indentation issue for or

> +    close($fh);
> +}
> +
> +sub remove_reboot_trigger {

maybe clear_reboot_request?

> +    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 ?

> +}
> +
>  # bash completion helper
>  
>  sub complete_backup_archives {
>