[pve-devel] applied: [PATCH qemu-server] qm agent : check if qga service is running

[Alexandre DERUMIER]

>>No need to generate extra work for you, it's just a simple change.
>>I applied the patch and followed up with adding and using the $silent param.

Thanks !

----- Mail original -----
De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
À: "pve-devel" <pve-devel at pve.proxmox.com>, "Alexandre Derumier" <aderumier at odiso.com>
Envoyé: Mercredi 30 Mai 2018 09:04:25
Objet: applied: [pve-devel] [PATCH qemu-server] qm agent : check if qga service is running

On 5/30/18 8:27 AM, Alexandre DERUMIER wrote: 
>>> 
>>> That's clear, I understood that. 
>>> Just though that if a user wants to do a guest-ping it could be a bit weird 
>>> that we do not execute it directly but check first with the same command, i.e., 
>>> ping, if we QGA is running - I meant to just if it makes sense to add a exception 
>> >from this test if the command itself is ping too, but not to important. 
> 
> ah ok. sorry. yes, indeed, we could skip it ;) 
> 
>> return if !PVE::QemuServer::qga_check_running($vmid); 
> 
>>> I actually want to keep $@ as it makes sense for the other places where we call 
>>> qga_check_running. I'll do a follow up. 
> 
> ok I understand. I can rebase my patch and add the $silent parameter if you want. 
> 

No need to generate extra work for you, it's just a simple change. 
I applied the patch and followed up with adding and using the $silent param. 

> 
> ----- Mail original ----- 
> De: "Thomas Lamprecht" <t.lamprecht at proxmox.com> 
> À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com> 
> Envoyé: Mercredi 30 Mai 2018 08:18:04 
> Objet: Re: [pve-devel] [PATCH qemu-server] qm agent : check if qga service is running 
> 
> On 5/30/18 8:04 AM, Alexandre DERUMIER wrote: 
>>>> check running uses the ping command, so maybe we should exempt it here and always 
>>>> to it - may not make sense to check with ping if we can do ping, or what do you mean? 
>> 
>> We need to check if qga is running with a small timeout (guest-ping) before doing another command with a bigger timeout (like freeze-fs / 1h timeout). 
>> The only wait to test is to send a command to qga (can be a ping or another command) or retreive qmp event but we don't have the framework to do it currently. 
>> 
> 
> That's clear, I understood that. 
> Just though that if a user wants to do a guest-ping it could be a bit weird 
> that we do not execute it directly but check first with the same command, i.e., 
> ping, if we QGA is running - I meant to just if it makes sense to add a exception 
> from this test if the command itself is ping too, but not to important. 
> 
>> 
>> 
>>>> So I actually get: 
>>>> 
>>>> # pvesh create nodes/dev5/qemu/12312321/agent/fstrim 
>>>> Qemu Guest Agent is not running - VM 12312321 qmp command 'guest-ping' failed - got timeout 
>>>> Qemu Guest Agent is not running 
>>>> 
>>>> We could add a $silent parameter to qga_check_running to avoid that. 
>> 
>> maybe could we simply change the warn in 
>> 
>> sub qga_check_running { 
>> my ($vmid) = @_; 
>> 
>> eval { vm_mon_cmd($vmid, "guest-ping", timeout => 3); }; 
>> if ($@) { 
>> warn "Qemu Guest Agent is not running - $@"; 
>> return 0; 
>> } 
>> return 1; 
>> } 
>> 
>> 
>> -> warn "Qemu Guest Agent is not running"; 
>> 
>> 
>> 
>> and in Agent.pm, 
>> 
>> return if !PVE::QemuServer::qga_check_running($vmid); 
>> 
>> ? 
>> 
> 
> I actually want to keep $@ as it makes sense for the other places where we call 
> qga_check_running. I'll do a follow up. 
> 
>> 
>> 
>> 
>> ----- Mail original ----- 
>> De: "Thomas Lamprecht" <t.lamprecht at proxmox.com> 
>> À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com> 
>> Envoyé: Mercredi 30 Mai 2018 07:49:00 
>> Objet: Re: [pve-devel] [PATCH qemu-server] qm agent : check if qga service is running 
>> 
>> I'll agree with the patches intent, two notes inline 
>> 
>> On 5/28/18 5:36 PM, Alexandre Derumier wrote: 
>>> --- 
>>> PVE/API2/Qemu/Agent.pm | 1 + 
>>> 1 file changed, 1 insertion(+) 
>>> 
>>> diff --git a/PVE/API2/Qemu/Agent.pm b/PVE/API2/Qemu/Agent.pm 
>>> index 9af5d5f..fbc8105 100644 
>>> --- a/PVE/API2/Qemu/Agent.pm 
>>> +++ b/PVE/API2/Qemu/Agent.pm 
>>> @@ -174,6 +174,7 @@ sub register_command { 
>>> 
>>> die "No Qemu Guest Agent\n" if !defined($conf->{agent}); 
>>> die "VM $vmid is not running\n" if !PVE::QemuServer::check_running($vmid); 
>>> + die "Qemu Guest Agent is not running\n" if !PVE::QemuServer::qga_check_running($vmid); 
>> 
>> check running uses the ping command, so maybe we should exempt it here and always 
>> to it - may not make sense to check with ping if we can do ping, or what do you mean? 
>> 
>> Further, qga_check_running always does a: warn "Qemu Guest Agent is not running - $@"; 
>> 
>> So I actually get: 
>> 
>> # pvesh create nodes/dev5/qemu/12312321/agent/fstrim 
>> Qemu Guest Agent is not running - VM 12312321 qmp command 'guest-ping' failed - got timeout 
>> Qemu Guest Agent is not running 
>> 
>> We could add a $silent parameter to qga_check_running to avoid that. 
>> 
>>> 
>>> my $cmd = $param->{command} // $command; 
>>> my $res = PVE::QemuServer::vm_mon_cmd($vmid, "guest-$cmd"); 
>>> 
>> 
>> _______________________________________________ 
>> pve-devel mailing list 
>> pve-devel at pve.proxmox.com 
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
>> 
> 
> _______________________________________________ 
> pve-devel mailing list 
> pve-devel at pve.proxmox.com 
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
>