[libvirt PATCH] qemu: fix response timeout for agent guest-sync

Jonathon Jongsma posted 1 patch 4 years, 1 month ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200320222810.3288-1-jjongsma@redhat.com
src/qemu/qemu_agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt PATCH] qemu: fix response timeout for agent guest-sync
Posted by Jonathon Jongsma 4 years, 1 month ago
The agent 'guest-sync' command historically had a 5s response timeout
which was different from other agent commands, which waited forever.
When we added the ability to customize the response timeout for guest
agent commands, we intended to continue to use 5s for 'guest-sync' when
the user specified a response timeout greater than 5s, and use the
user-specified timeout if it was below 5s. Unfortunately, when
attempting to determine whether the user-specified timeout was less than
5s, we were comparing against an enum value of
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT (which is -1) rather than against
the actual time value that it represented (5).

This change makes it so that 'guest-sync' now uses the user-specified
tiemout if it is less than 5s.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/qemu/qemu_agent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 6ff5b11374..a10ef0728a 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -904,7 +904,7 @@ qemuAgentGuestSync(qemuAgentPtr agent)
 
     /* if user specified a custom agent timeout that is lower than the
      * default timeout, use the shorter timeout instead */
-    if ((agent->timeout >= 0) && (agent->timeout < timeout))
+    if ((agent->timeout >= 0) && (agent->timeout < QEMU_AGENT_WAIT_TIME))
         timeout = agent->timeout;
 
     memset(&sync_msg, 0, sizeof(sync_msg));
-- 
2.21.1

Re: [libvirt PATCH] qemu: fix response timeout for agent guest-sync
Posted by Daniel Henrique Barboza 4 years, 1 month ago

On 3/20/20 7:28 PM, Jonathon Jongsma wrote:
> The agent 'guest-sync' command historically had a 5s response timeout
> which was different from other agent commands, which waited forever.
> When we added the ability to customize the response timeout for guest
> agent commands, we intended to continue to use 5s for 'guest-sync' when
> the user specified a response timeout greater than 5s, and use the
> user-specified timeout if it was below 5s. Unfortunately, when
> attempting to determine whether the user-specified timeout was less than
> 5s, we were comparing against an enum value of
> VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT (which is -1) rather than against
> the actual time value that it represented (5).
> 
> This change makes it so that 'guest-sync' now uses the user-specified
> tiemout if it is less than 5s.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Re: [libvirt PATCH] qemu: fix response timeout for agent guest-sync
Posted by Michal Prívozník 4 years, 1 month ago
On 23. 3. 2020 12:33, Daniel Henrique Barboza wrote:
> 
> 
> On 3/20/20 7:28 PM, Jonathon Jongsma wrote:
>> The agent 'guest-sync' command historically had a 5s response timeout
>> which was different from other agent commands, which waited forever.
>> When we added the ability to customize the response timeout for guest
>> agent commands, we intended to continue to use 5s for 'guest-sync' when
>> the user specified a response timeout greater than 5s, and use the
>> user-specified timeout if it was below 5s. Unfortunately, when
>> attempting to determine whether the user-specified timeout was less than
>> 5s, we were comparing against an enum value of
>> VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT (which is -1) rather than against
>> the actual time value that it represented (5).
>>
>> This change makes it so that 'guest-sync' now uses the user-specified
>> tiemout if it is less than 5s.
>>
>> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
>> ---
> 
> 
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 

And puhsed.

Michal