[v1] https://listman.redhat.com/archives/libvir-list/2022-January/msg00008.html
[v2] https://listman.redhat.com/archives/libvir-list/2022-January/msg00582.html

As suggesed in the review comments:
- dropped virFileGetLockOwner;
- simplified lock validation by using VIR_AUTOCLOSE and just trying to
lock the file;
- introduced virPidFileReadPathIfLocked to preserve the existing
behaviour of virPidFileReadPathIfAlive.

Vasiliy Ulyanov (3):
  virpidfile: Add virPidFileReadPathIfLocked func
  qemu: tpm: Get swtpm pid without binary validation
  qemu: gpu: Get pid without binary validation

 src/libvirt_private.syms       |  1 +
 src/qemu/qemu_tpm.c            | 40 +++++++++++++++++++++-------------
 src/qemu/qemu_vhost_user_gpu.c | 11 +++++-----
 src/util/virpidfile.c          | 34 +++++++++++++++++++++++++++++
 src/util/virpidfile.h          |  2 ++
 5 files changed, 67 insertions(+), 21 deletions(-)

-- 
2.34.1

On 2/2/22 17:28, Vasiliy Ulyanov wrote:
> [v1] https://listman.redhat.com/archives/libvir-list/2022-January/msg00008.html
> [v2] https://listman.redhat.com/archives/libvir-list/2022-January/msg00582.html
> 
> As suggesed in the review comments:
> - dropped virFileGetLockOwner;
> - simplified lock validation by using VIR_AUTOCLOSE and just trying to
> lock the file;
> - introduced virPidFileReadPathIfLocked to preserve the existing
> behaviour of virPidFileReadPathIfAlive.
> 
> Vasiliy Ulyanov (3):
>   virpidfile: Add virPidFileReadPathIfLocked func
>   qemu: tpm: Get swtpm pid without binary validation
>   qemu: gpu: Get pid without binary validation
> 
>  src/libvirt_private.syms       |  1 +
>  src/qemu/qemu_tpm.c            | 40 +++++++++++++++++++++-------------
>  src/qemu/qemu_vhost_user_gpu.c | 11 +++++-----
>  src/util/virpidfile.c          | 34 +++++++++++++++++++++++++++++
>  src/util/virpidfile.h          |  2 ++
>  5 files changed, 67 insertions(+), 21 deletions(-)
> 

Apart from patch 2/3 this looks good. So here's what we are going to do.
I've uploaded fixup commits onto my gitlab:

  https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virpidfile/

Please take a look and if you agree I will squash those fixup commits
and merge.

Michal

Hi Michal,

On 03/02/2022 18:09, Michal Prívozník wrote:
> 
> Apart from patch 2/3 this looks good. So here's what we are going to do.
> I've uploaded fixup commits onto my gitlab:
> 
>   https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virpidfile/
> 
> Please take a look and if you agree I will squash those fixup commits
> and merge.
> 

Thank you for the detailed explanation in 2/3. Makes sense. Probably I need to
dive a bit deeper into the code the next time. And yeah, I missed that redundant
include initially and noticed it only after sending the patches. I agree on
squashing the fixups and merging.

-- 
Vasily Ulyanov <vulyanov@suse.de>
Software Engineer, SUSE Labs Core

On 2/3/22 18:56, Vasily Ulyanov wrote:
> Hi Michal,
> 
> On 03/02/2022 18:09, Michal Prívozník wrote:
>>
>> Apart from patch 2/3 this looks good. So here's what we are going to do.
>> I've uploaded fixup commits onto my gitlab:
>>
>>   https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virpidfile/
>>
>> Please take a look and if you agree I will squash those fixup commits
>> and merge.
>>
> 
> Thank you for the detailed explanation in 2/3. Makes sense. Probably I need to
> dive a bit deeper into the code the next time. And yeah, I missed that redundant
> include initially and noticed it only after sending the patches. I agree on
> squashing the fixups and merging.
> 

Perfect, fixed and merged.

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

Michal