On Mon, 20 Feb 2023 09:38:05 +0100 Stefano Brivio <sbrivio@xxxxxxxxxx> wrote: > Michal, > > On Fri, 17 Feb 2023 13:51:42 +0100 > Michal Prívozník <mprivozn@xxxxxxxxxx> wrote: > > > 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. > > How do you want to proceed? Laine's series to improve error reporting in > passt is included in: > > - passt version 2023_02_16.4663ccc > - Debian and Ubuntu packages 0.0~git20230216.4663ccc, pending upload > - Fedora and CentOS Stream packages passt-0^20230216.g4663ccc-1 > (stable: fc38, fc39, eln126, testing: fc36, fc37) > > I guess either an updated patch from Laine with your refinements or an > updated series from you would work...? Thanks, Ah, I see you just pushed your series... -- Stefano