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@xxxxxxxxxx> and pushed. Congratulations on your first libvirt contribution! Michal
