[libvirt] [PATCH 0/2] A potential deadlock fix for SEV and a tiny cleanup

*** BLURB HERE *** Erik Skultety (2): qemu: sev: Use EnterMonitor instead of EnterMonitorAsync qemu: sev: Don't jump to endjob if SEV measurement retrieval fails src/qemu/qemu_driver.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) -- 2.14.4

Since it's being called with QEMU_ASYNC_JOB_NONE which is what qemuDomainObjEnterMonitor is going to use with the internal helper, let's use that one instead. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 42069ee617..fd25eb1b0b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21511,9 +21511,7 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) return -1; - if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) - goto endjob; - + qemuDomainObjEnterMonitor(driver, vm); tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); if (tmp == NULL) goto endjob; -- 2.14.4

On Mon, Jun 18, 2018 at 09:20 AM +0200, Erik Skultety <eskultet@redhat.com> wrote:
Since it's being called with QEMU_ASYNC_JOB_NONE which is what qemuDomainObjEnterMonitor is going to use with the internal helper, let's use that one instead.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 42069ee617..fd25eb1b0b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21511,9 +21511,7 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) return -1;
- if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) - goto endjob; - + qemuDomainObjEnterMonitor(driver, vm); tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); if (tmp == NULL) goto endjob; -- 2.14.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

If measurement retrieval fails we'd forget to call ExitMonitor to unlock the monitor. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reported-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fd25eb1b0b..d71956988f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21513,10 +21513,8 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); - if (tmp == NULL) - goto endjob; - if (qemuDomainObjExitMonitor(driver, vm) < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !tmp) goto endjob; if (virTypedParamsAddString(params, nparams, &maxpar, -- 2.14.4

On Mon, Jun 18, 2018 at 09:20 AM +0200, Erik Skultety <eskultet@redhat.com> wrote:
If measurement retrieval fails we'd forget to call ExitMonitor to unlock the monitor.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reported-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fd25eb1b0b..d71956988f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21513,10 +21513,8 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm); tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); - if (tmp == NULL) - goto endjob;
- if (qemuDomainObjExitMonitor(driver, vm) < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !tmp) goto endjob;
if (virTypedParamsAddString(params, nparams, &maxpar, -- 2.14.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Small nit: I would probably just move the “if-tmp-block” behind the “if-…ExitMonitor()-block”. But I have no strong opinion about that :) Anyway Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Tue, Jun 19, 2018 at 03:52:34PM +0200, Marc Hartmayer wrote:
On Mon, Jun 18, 2018 at 09:20 AM +0200, Erik Skultety <eskultet@redhat.com> wrote:
If measurement retrieval fails we'd forget to call ExitMonitor to unlock the monitor.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reported-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fd25eb1b0b..d71956988f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21513,10 +21513,8 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm); tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); - if (tmp == NULL) - goto endjob;
- if (qemuDomainObjExitMonitor(driver, vm) < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !tmp) goto endjob;
if (virTypedParamsAddString(params, nparams, &maxpar, -- 2.14.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Small nit: I would probably just move the “if-tmp-block” behind the “if-…ExitMonitor()-block”. But I have no strong opinion about that :)
I'll make the change...we should be more consistent here though :). Thanks, Erik
participants (2)
-
Erik Skultety
-
Marc Hartmayer