On December 22, 2022 1:58 pm, Daniel Tschlatscher wrote:
>>>
>>> - my $exitcode = run_command($cmd, %run_params);
>>> - if ($exitcode) {
>>> - if ($tpmpid) {
>>> - warn "stopping swtpm instance (pid $tpmpid) due to QEMU
>>> startup error\n";
>>> - kill 'TERM', $tpmpid;
>>> + eval {
>>> + my $exitcode = run_command($cmd, %run_params);
>>> +
>>> + if ($exitcode) {
>>> + if ($tpmpid) {
>>> + log_warn "stopping swtpm instance (pid $tpmpid) due to
>>> QEMU startup
>> error\n";
>>
>> this warn -> log_warn change kind of slipped in, it's not really part of this
>> patch?
>
> Because I changed this line anyway, I changed it to log_warn as it is
> imported already and, as I understood, the preferable alternative
> to calling 'warn'.
> Sourcing this in it's own patch seems overkill to me, or would you
> rather suggest something like this should be handled in, e.g. a
> file-encompassing refactoring?
ideally it could be sent as cleanup patch up-front (then it can be applied even
if the rest needs another round ;)) or at least mentioned somewhere (e.g., in
the patch notes). seemingly unrelated changes in a patch always make me wary
that
the patch was generated from some unclean tree/more or less than intended was
`git add`ed. in this case my guess was that you just changed that (wrapped) call
site to match your newly introduced ones, but it could also have been an
unintentional search+replace result, for example, so I'd rather ask :)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
