qemuDomainPostParseDataAlloc: Don't reset error if looking up caps fails

[PATCH] qemuDomainPostParseDataAlloc: Don't reset error if looking up caps fails

Posted by Michal Privoznik 3 years, 9 months ago

When starting the QEMU driver we load all domain XMLs. This
means, that post parse callbacks are run for each XML, but they
are allowed to fail because we will run them again when starting
a domain. In the case I am fixing, we were unable to look up QEMU
capabilities (in both post parse runs) and reported appropriate
error, sort of. It can be found in the logs, but the caller
doesn't get it because after 5331c4804f4 it is reset. Therefore,
as reported here [1], if we are unable to start QEMU for caps
probing the following will happen:

  virsh start fedora
  error: Failed to start domain fedora
  error: An error occurred, but the cause is unknown

1: https://www.redhat.com/archives/libvir-list/2020-July/msg00673.html

Fixes: 5331c4804f4

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_domain.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c365d92ae0..f31690c50a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5307,7 +5307,6 @@ qemuDomainPostParseDataAlloc(const virDomainDef *def,
 
     if (!(*parseOpaque = virQEMUCapsCacheLookup(driver->qemuCapsCache,
                                                 def->emulator))) {
-        virResetLastError();
         return 1;
     }
 
-- 
2.26.2

Re: [PATCH] qemuDomainPostParseDataAlloc: Don't reset error if looking up caps fails

Posted by Peter Krempa 3 years, 9 months ago

On Fri, Jul 17, 2020 at 09:27:54 +0200, Michal Privoznik wrote:
> When starting the QEMU driver we load all domain XMLs. This
> means, that post parse callbacks are run for each XML, but they
> are allowed to fail because we will run them again when starting
> a domain. In the case I am fixing, we were unable to look up QEMU
> capabilities (in both post parse runs) and reported appropriate
> error, sort of. It can be found in the logs, but the caller
> doesn't get it because after 5331c4804f4 it is reset. Therefore,
> as reported here [1], if we are unable to start QEMU for caps
> probing the following will happen:
> 
>   virsh start fedora
>   error: Failed to start domain fedora
>   error: An error occurred, but the cause is unknown
> 
> 1: https://www.redhat.com/archives/libvir-list/2020-July/msg00673.html
> 
> Fixes: 5331c4804f4

FYI Daniel already posted his version of the fix which does a bit more:

https://www.redhat.com/archives/libvir-list/2020-July/msg00804.html

Re: [PATCH] qemuDomainPostParseDataAlloc: Don't reset error if looking up caps fails

Posted by Michal Prívozník 3 years, 9 months ago

On 7/17/20 9:36 AM, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 09:27:54 +0200, Michal Privoznik wrote:
>> When starting the QEMU driver we load all domain XMLs. This
>> means, that post parse callbacks are run for each XML, but they
>> are allowed to fail because we will run them again when starting
>> a domain. In the case I am fixing, we were unable to look up QEMU
>> capabilities (in both post parse runs) and reported appropriate
>> error, sort of. It can be found in the logs, but the caller
>> doesn't get it because after 5331c4804f4 it is reset. Therefore,
>> as reported here [1], if we are unable to start QEMU for caps
>> probing the following will happen:
>>
>>    virsh start fedora
>>    error: Failed to start domain fedora
>>    error: An error occurred, but the cause is unknown
>>
>> 1: https://www.redhat.com/archives/libvir-list/2020-July/msg00673.html
>>
>> Fixes: 5331c4804f4
> 
> FYI Daniel already posted his version of the fix which does a bit more:
> 
> https://www.redhat.com/archives/libvir-list/2020-July/msg00804.html
> 

Ah, should have read the list first. Self-NACK then :-)

Michal