[PATCH] qemu: use proper job type for qemuDomainAuthorizedSSHKeysSet()

Roman Bogorodskiy posted 1 patch 1 week, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260613050120.61453-1-bogorodskiy@gmail.com
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] qemu: use proper job type for qemuDomainAuthorizedSSHKeysSet()
Posted by Roman Bogorodskiy 1 week, 4 days ago
As the qemuDomainAuthorizedSSHKeysSet() call modifies the domain,
use the VIR_AGENT_JOB_MODIFY job type, not VIR_AGENT_JOB_QUERY.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 544955ecf9..c3eb209f70 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20375,7 +20375,7 @@ qemuDomainAuthorizedSSHKeysSet(virDomainPtr dom,
     if (virDomainAuthorizedSshKeysSetEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
-    if (virDomainObjBeginAgentJob(vm, VIR_AGENT_JOB_QUERY) < 0)
+    if (virDomainObjBeginAgentJob(vm, VIR_AGENT_JOB_MODIFY) < 0)
         goto cleanup;
 
     if (!qemuDomainAgentAvailable(vm, true))
-- 
2.52.0
Re: [PATCH] qemu: use proper job type for qemuDomainAuthorizedSSHKeysSet()
Posted by Peter Krempa via Devel 1 week, 2 days ago
On Sat, Jun 13, 2026 at 07:01:20 +0200, Roman Bogorodskiy wrote:
> As the qemuDomainAuthorizedSSHKeysSet() call modifies the domain,
> use the VIR_AGENT_JOB_MODIFY job type, not VIR_AGENT_JOB_QUERY.

Technically the call doesn't modify anything of the libvirt-stored state
of the domain so the _QUERY job is sufficient. Changing it to _MODIFY
will thus restrict when the API can run.

I agree though that semantically a _MODIFY job makes sense based on
*what* the API does.

> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> ---
>  src/qemu/qemu_driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 544955ecf9..c3eb209f70 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20375,7 +20375,7 @@ qemuDomainAuthorizedSSHKeysSet(virDomainPtr dom,
>      if (virDomainAuthorizedSshKeysSetEnsureACL(dom->conn, vm->def) < 0)
>          goto cleanup;
>  
> -    if (virDomainObjBeginAgentJob(vm, VIR_AGENT_JOB_QUERY) < 0)
> +    if (virDomainObjBeginAgentJob(vm, VIR_AGENT_JOB_MODIFY) < 0)
>          goto cleanup;

I'm not opposed to this patch, but the commit message ought to capture
that _QUERY was not actually a technical problem.

Also leave some time for anyone else to object, e.g. since with this
patch you can no longer do this while e.g. migrating the VM. I don't
think it's too much of a problem though.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Re: [PATCH] qemu: use proper job type for qemuDomainAuthorizedSSHKeysSet()
Posted by Roman Bogorodskiy 1 week, 2 days ago
  Peter Krempa wrote:

> On Sat, Jun 13, 2026 at 07:01:20 +0200, Roman Bogorodskiy wrote:
> > As the qemuDomainAuthorizedSSHKeysSet() call modifies the domain,
> > use the VIR_AGENT_JOB_MODIFY job type, not VIR_AGENT_JOB_QUERY.
> 
> Technically the call doesn't modify anything of the libvirt-stored state
> of the domain so the _QUERY job is sufficient.

Yes, initially I wasn't sure about that. But the similar calls use
_MODIFY, specifically, qemuDomainSetUserPassword() and
qemuDomainSetTime().

There are some more, like qemuDomainSetGuestVcpus(), but they are more
invasive, so not sure if that is applicable for this case.