Michal,
On Fri, 17 Feb 2023 13:51:42 +0100
Michal Prívozník <mprivozn(a)redhat.com> 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,
--
Stefano