[PATCH 1/1] qemu_tpm: Start swtpm(8) daemon with --terminate switch

[Michal Prívozník]

On 9/14/21 7:34 PM, Nick Chevsky wrote:
> Hi Michal,
> 
>> The patch is correct, but what we already have is qemuExtDevicesStop()
>> being called from qemuProcessStop(). The former will eventually call
>> qemuTPMEmulatorStop() which should kill the swtmp process, shouldn't it?
> 
> I'd missed this and that's great to hear, because it means the
> upstream bug (i.e. swtpm behaving as though --terminate was given even
> when it wasn't) can be fixed now, without breaking libvirt.
> 
>> Or this patch is there to kill swtmp earlier, i.e. as soon as it sees
>> HUP on the socket?
> 
> That's the thing; this is already (incorrectly) happening right now:
> swtpm is self-terminating as soon as it sees HUP on the socket, even
> though --terminate wasn't given. In other words, we're currently
> seeing --terminate behavior *without* --terminate. Due to this
> upstream bug, I suspect swtpm is already shutting itself down by the
> time libvirt executes qemuExtDevicesStop().
> 
> Once the upstream bug is fixed, swtpm will no longer self-terminate on
> HUP *unless* --terminate was given. If libvirt doesn't add
> --terminate, swtpm will then stay up (as it should) and be killed by
> qemuExtDevicesStop() instead. Does that make sense?
> 
> While the end result will ultimately be the same, I'd still argue that
> it's a good idea for libvirt to start invoking swtpm with --terminate
> since (a) it's free, (b) it's more graceful, (c) it takes some
> responsibility off libvirt, and (d) it adds an extra layer of
> redundancy by allowing swtpm to try to shut itself down first and, if
> that fails, have libvirt's qemuExtDevicesStop() kill it.

Just to be clear, I am not against this patch, in fact the opposite.
Just wanted to make clear I understand what's going on. It's good to
have redundancy.

Looking at git blame of swtpm.c I can see that --terminate was there
since the initial commit so we don't need any additional machinery to
determine whether the argument is supported or not. That's good.

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

and pushed. Congratulations on your first libvirt contribution!

Michal