From: Markus Armbruster <armbru@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: peterx@redhat.com,  stefanb@linux.vnet.ibm.com,  farosas@suse.de,
	qemu-devel@nongnu.org,  berrange@redhat.com
Subject: Re: [PATCH v3 2/4] tmp_emulator: fix unset errp on error path
Date: Mon, 27 Oct 2025 16:06:39 +0100	[thread overview]
Message-ID: <87jz0gl6yo.fsf@pond.sub.org> (raw)
In-Reply-To: <ad68df68-bd35-4ebe-9577-88cc98630aac@yandex-team.ru> (Vladimir Sementsov-Ogievskiy's message of "Mon, 27 Oct 2025 17:33:20 +0300")
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.
next prev parent reply	other threads:[~2025-10-27 15:08 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-25 20:26 [PATCH v3 0/4] migration: vmsd errp handlers: return bool Vladimir Sementsov-Ogievskiy
2025-10-25 20:26 ` [PATCH v3 1/4] migration: vmstate_save_state_v(): fix error path Vladimir Sementsov-Ogievskiy
2025-10-27 14:24   ` Stefan Berger
2025-10-25 20:26 ` [PATCH v3 2/4] tmp_emulator: fix unset errp on " Vladimir Sementsov-Ogievskiy
2025-10-27 10:16   ` Markus Armbruster
2025-10-27 14:33     ` Vladimir Sementsov-Ogievskiy
2025-10-27 15:06       ` Markus Armbruster [this message]
2025-10-25 20:26 ` [PATCH v3 3/4] migration/vmstate: stop reporting error number for new _errp APIs Vladimir Sementsov-Ogievskiy
2025-10-27 10:23   ` Markus Armbruster
2025-10-27 20:28     ` Vladimir Sementsov-Ogievskiy
2025-10-27 14:38   ` Stefan Berger
2025-10-27 14:55     ` Vladimir Sementsov-Ogievskiy
2025-10-25 20:26 ` [PATCH v3 4/4] migration: vmsd errp handlers: return bool Vladimir Sementsov-Ogievskiy
2025-10-27 10:38   ` Markus Armbruster
2025-10-27 14:58     ` Vladimir Sementsov-Ogievskiy
2025-10-27 16:30       ` Arun Menon
2025-10-27 17:06         ` Vladimir Sementsov-Ogievskiy
2025-10-28 15:12           ` Peter Xu
2025-10-28 16:26             ` Vladimir Sementsov-Ogievskiy
2025-10-28 16:42               ` Peter Xu
2025-10-27 20:33         ` Vladimir Sementsov-Ogievskiy
2025-10-28  5:08           ` Arun Menon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox
  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):
  git send-email \
    --in-reply-to=87jz0gl6yo.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=farosas@suse.de \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanb@linux.vnet.ibm.com \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY
  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
  Be sure your reply has a Subject: header at the top and a blank line
  before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).