[libvirt PATCH] qemu: qemuDomainSetLaunchSecurityState: do not lookup qemuCaps in cache

Ján Tomko posted 1 patch 2 years, 3 months ago
Failed in applying to current master (apply log)
src/qemu/qemu_driver.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
[libvirt PATCH] qemu: qemuDomainSetLaunchSecurityState: do not lookup qemuCaps in cache
Posted by Ján Tomko 2 years, 3 months ago
Any active domain has a copy in the privateData, filled in
qemuProcessInit.

Move the qemu capability check below the activeness check and remove
the extra lookup.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_driver.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cbd17c10ae..acaa6f7629 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19975,12 +19975,12 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain,
     virDomainObj *vm;
     int ret = -1;
     int rc;
-    g_autoptr(virQEMUCaps) qemucaps = NULL;
     const char *secrethdr = NULL;
     const char *secret = NULL;
     unsigned long long setaddr = 0;
     bool hasSetaddr = false;
     int state;
+    qemuDomainObjPrivate *priv;
 
     virCheckFlags(0, -1);
     if (virTypedParamsValidate(params, nparams,
@@ -19996,6 +19996,8 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain,
     if (!(vm = qemuDomainObjFromDomain(domain)))
         goto cleanup;
 
+    priv = vm->privateData;
+
     if (virDomainSetLaunchSecurityStateEnsureACL(domain->conn, vm->def) < 0)
         goto cleanup;
 
@@ -20007,17 +20009,6 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain,
         goto cleanup;
     }
 
-    if (!(qemucaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache,
-                                                   NULL, NULL, NULL, NULL,
-                                                   NULL, NULL, NULL)))
-        goto cleanup;
-
-    if (!virQEMUCapsGet(qemucaps, QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET)) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                       _("QEMU does not support setting a launch secret"));
-        goto cleanup;
-    }
-
     if (virTypedParamsGetString(params, nparams,
                                 VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_HEADER,
                                 &secrethdr) < 0 ||
@@ -20050,6 +20041,12 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain,
         goto endjob;
     }
 
+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("QEMU does not support setting a launch secret"));
+        goto cleanup;
+    }
+
     qemuDomainObjEnterMonitor(driver, vm);
     rc = qemuMonitorSetLaunchSecurityState(QEMU_DOMAIN_PRIVATE(vm)->mon,
                                            secrethdr, secret, setaddr, hasSetaddr);
-- 
2.34.1

Re: [libvirt PATCH] qemu: qemuDomainSetLaunchSecurityState: do not lookup qemuCaps in cache
Posted by Peter Krempa 2 years, 3 months ago
On Mon, Jan 31, 2022 at 13:40:34 +0100, Ján Tomko wrote:
> Any active domain has a copy in the privateData, filled in
> qemuProcessInit.
> 
> Move the qemu capability check below the activeness check and remove
> the extra lookup.

What's worse is that you can get caps for a different emulator binary
since the code didn't even bother passing the path to the lookup
function.

> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  src/qemu/qemu_driver.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Re: [libvirt PATCH] qemu: qemuDomainSetLaunchSecurityState: do not lookup qemuCaps in cache
Posted by Ján Tomko 2 years, 3 months ago
On a Monday in 2022, Peter Krempa wrote:
>On Mon, Jan 31, 2022 at 13:40:34 +0100, Ján Tomko wrote:
>> Any active domain has a copy in the privateData, filled in
>> qemuProcessInit.
>>
>> Move the qemu capability check below the activeness check and remove
>> the extra lookup.
>
>What's worse is that you can get caps for a different emulator binary
>since the code didn't even bother passing the path to the lookup
>function.
>
>>
>> Signed-off-by: Ján Tomko <jtomko@redhat.com>
>> ---
>>  src/qemu/qemu_driver.c | 21 +++++++++------------
>>  1 file changed, 9 insertions(+), 12 deletions(-)
>
>Reviewed-by: Peter Krempa <pkrempa@redhat.com>
>

Thanks, I will squash this in before pushing:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 57f286b343..698f57f00e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20044,7 +20044,7 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain,
      if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET)) {
          virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                         _("QEMU does not support setting a launch secret"));
-        goto cleanup;
+        goto endjob;
      }

      qemuDomainObjEnterMonitor(driver, vm);

Jano