Qemu Agent Enhancement #1053
Qemu Agent Enhancement #1053
Conversation
…come online. This makes ip information queries using the qemu_agent more reliable
|
Checked this PR locally and it works! Upstream code with @dmacvicar this should be merged I guess |
|
@dmacvicar, any chance of getting this merged ? |
There was a problem hiding this comment.
Thanks for the PR 🙏
I think it makes sense to merge some kind of check. However, I'd need to understand the level of defensiveness of the code (eg. checking for qemu_agent inside and outside of the function, and running the same block twice). This code smells like "just in case".
| @@ -602,6 +602,18 @@ func resourceLibvirtDomainCreate(ctx context.Context, d *schema.ResourceData, me | |||
| d.SetId(id) | |||
| log.Printf("[INFO] Domain ID: %s", d.Id()) | |||
|
|
|||
| if d.Get("qemu_agent").(bool) && d.Get("running").(bool) { | |||
There was a problem hiding this comment.
Why does the code check for d.Get("qemu_agent") if then waitingForAgentRunning checks again?
| @@ -769,6 +781,18 @@ func resourceLibvirtDomainUpdate(ctx context.Context, d *schema.ResourceData, me | |||
| } | |||
| } | |||
|
|
|||
| if d.Get("qemu_agent").(bool) && d.Get("running").(bool) { | |||
There was a problem hiding this comment.
Why is this wait executed twice?
|
@dmacvicar, I replied to your comments. As of the level of defensiveness (eg. checking for qemu_agent inside and outside of the function), it's just my style of programming. But you are right. what do you prefer ? keeping the check inside or outside the function ? as for your comment about running the same block twice, as I explained in the comment, those blocks happened to be inside different functions |
The idea of this PR is to make sure that the guest qemu agent is running by explicitly query it. This will make ip retrieval for example more reliable. let me know what you think
Please make sure you read the contributor documentation before opening a Pull Request.