Note, that called tpm_emulator_startup_tpm_resume() does error_report()
failure paths, which could be turned into error_setg() to passthough an
error. But, not on all error paths. Not saying about
tpm_emulator_startup_tpm_resume() return -1 on failure and we mix it
with -errno from tpm_emulator_set_state_blobs(). So, it all needs deeper
refactoring, which is out of scope of this small fix.

Fixes: 42e556fa3f7ac
    "backends/tpm: Propagate vTPM error on migration failure"
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 backends/tpm/tpm_emulator.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index dacfca5ab7..aa69eb606f 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -961,6 +961,7 @@ static int tpm_emulator_post_load(void *opaque, int version_id, Error **errp)
     }
 
     if (tpm_emulator_startup_tpm_resume(tb, 0, true) < 0) {
+        error_setg(errp, "Failed to resume tpm");
         return -EIO;
     }
 
-- 
2.48.1

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Note, that called tpm_emulator_startup_tpm_resume() does error_report()
> failure paths, which could be turned into error_setg() to passthough an
> error. But, not on all error paths. Not saying about
> tpm_emulator_startup_tpm_resume() return -1 on failure and we mix it
> with -errno from tpm_emulator_set_state_blobs(). So, it all needs deeper
> refactoring, which is out of scope of this small fix.
>
> Fixes: 42e556fa3f7ac
>     "backends/tpm: Propagate vTPM error on migration failure"
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  backends/tpm/tpm_emulator.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index dacfca5ab7..aa69eb606f 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -961,6 +961,7 @@ static int tpm_emulator_post_load(void *opaque, int version_id, Error **errp)
>      }
>  
>      if (tpm_emulator_startup_tpm_resume(tb, 0, true) < 0) {
> +        error_setg(errp, "Failed to resume tpm");
>          return -EIO;
>      }

Anti-pattern: we call error_report() (via
tpm_emulator_startup_tpm_resume(), tpm_emulator_set_buffer_size, ...)
from within an Error-setting function.  You need to convert the entire
nest of functions to Error.

On 27.10.25 13:16, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> Note, that called tpm_emulator_startup_tpm_resume() does error_report()
>> failure paths, which could be turned into error_setg() to passthough an
>> error. But, not on all error paths. Not saying about
>> tpm_emulator_startup_tpm_resume() return -1 on failure and we mix it
>> with -errno from tpm_emulator_set_state_blobs(). So, it all needs deeper
>> refactoring, which is out of scope of this small fix.
>>
>> Fixes: 42e556fa3f7ac
>>      "backends/tpm: Propagate vTPM error on migration failure"
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   backends/tpm/tpm_emulator.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
>> index dacfca5ab7..aa69eb606f 100644
>> --- a/backends/tpm/tpm_emulator.c
>> +++ b/backends/tpm/tpm_emulator.c
>> @@ -961,6 +961,7 @@ static int tpm_emulator_post_load(void *opaque, int version_id, Error **errp)
>>       }
>>   
>>       if (tpm_emulator_startup_tpm_resume(tb, 0, true) < 0) {
>> +        error_setg(errp, "Failed to resume tpm");
>>           return -EIO;
>>       }
> 
> Anti-pattern: we call error_report() (via
> tpm_emulator_startup_tpm_resume(), tpm_emulator_set_buffer_size, ...)
> from within an Error-setting function.  You need to convert the entire
> nest of functions to Error.
> 

Is it a show-stopper for bug-fix? I'm afraid, I have no time to convert the whole
tpm_emulator.c.


-- 
Best regards,
Vladimir

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 27.10.25 13:16, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>> 
>>> Note, that called tpm_emulator_startup_tpm_resume() does error_report()
>>> failure paths, which could be turned into error_setg() to passthough an
>>> error. But, not on all error paths. Not saying about
>>> tpm_emulator_startup_tpm_resume() return -1 on failure and we mix it
>>> with -errno from tpm_emulator_set_state_blobs(). So, it all needs deeper
>>> refactoring, which is out of scope of this small fix.
>>>
>>> Fixes: 42e556fa3f7ac
>>>      "backends/tpm: Propagate vTPM error on migration failure"
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>> ---
>>>   backends/tpm/tpm_emulator.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
>>> index dacfca5ab7..aa69eb606f 100644
>>> --- a/backends/tpm/tpm_emulator.c
>>> +++ b/backends/tpm/tpm_emulator.c
>>> @@ -961,6 +961,7 @@ static int tpm_emulator_post_load(void *opaque, int version_id, Error **errp)
>>>      }
>>>          if (tpm_emulator_startup_tpm_resume(tb, 0, true) < 0) {
>>> +        error_setg(errp, "Failed to resume tpm");
>>>          return -EIO;
>>>      }
>>
>> Anti-pattern: we call error_report() (via
>> tpm_emulator_startup_tpm_resume(), tpm_emulator_set_buffer_size, ...)
>> from within an Error-setting function.  You need to convert the entire
>> nest of functions to Error.
>> 
>
> Is it a show-stopper for bug-fix? I'm afraid, I have no time to convert the whole
> tpm_emulator.c.

Fair.  Throw in a FIXME comment, and I'll give my R-by.