Re: [pve-devel] [PATCH qemu-server v3 3/5] await and kill lingering KVM thread when VM start reaches timeout

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

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 :)