On 2/16/23 17:35, Laine Stump wrote: > On 2/16/23 8:32 AM, Michal Privoznik wrote: >> This is a v2 of: >> >> https://listman.redhat.com/archives/libvir-list/2023-February/237731.html >> >> diff to v1: >> - Merged patches that were ACKed in v1, >> - Dropped 4/4 from the original series (the one that sets --foreground), >> and implemented a different approach >> >> Michal Prívozník (5): >> qemu_passt: Avoid double daemonizing passt >> qemu_passt: Report passt's error on failed start >> qemu_passt: Make passt report errors to stderr whenever possible >> qemu_passt: Deduplicate passt killing code >> qemu_passt: Let passt write the PID file > > This is everything that was in the patch I sent last week, with the > following additions > > 1) adding NULLSTR() around the reference to errbuf in patch 2/5 > > 2) adding "--stderr" to the commandline in patch 3/5 (which I found to > be unnecessary in my testing - as Stefano says everything goes to stderr > until passt has completed its init anyway) > > 3) the other bit of patch 3/5 which adds an extra message telling the > user to look into the designated logfile for the error - this is > unnecessary (and actually now counter-productive, as it forces you to > look elsewhere for the error when you wouldn't have needed to) because > of patches I've sent to passt. > > 4) patch 4/5 that is a cleanup de-duplicating code > > 5) patch 5 changes additional code (that I didn't touch in my patch) to > use virPidFileReadPath() instead of virPidFileReadPathIfLocked(), and > virProcessKillPainfully() instead of the higher level > virPidFileForceCleanupPath(). > > So it all seems fine (except the error reporting stuff), but why revert > a patch only to push back the same changes in a deconstructed fashion > plus some fixups, rather than just posting a followup or two? Yeah, I realized this too and I'm sorry. My original intention was to fix this in a completely different way (as my last patch from v1 demonstrates) and that was incompatible with yours. Michal