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

[Thomas Lamprecht]

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
>