Couple of qemu-pr-helper fixes

[Qemu-devel] [PATCH 0/2] Couple of qemu-pr-helper fixes

Michal Privoznik posted 2 patches 6 years ago

Download series mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/cover.1522760575.git.mprivozn@redhat.com

Test checkpatch passed

Test docker-build@min-glib passed

Test docker-mingw@fedora passed

Test s390x passed

scsi/qemu-pr-helper.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)

Expand all Fold all

[Qemu-devel] [PATCH 0/2] Couple of qemu-pr-helper fixes

Posted by Michal Privoznik 6 years ago

The first one is trivial.

The second is slightly trickier. Libvirt has this virCommand subsystem
(found under src/util/vircommand.c) which it uses to spawn commands. The
subsystem allows libvirt to both daemonize a process and learn its PID.
The latter of course does not work if command daemonizes itself (hence
libvirt will not use qemu-pr-helper --daemon). BUT, we still need
qemu-pr-helper to lock the pidfile (so that when libvirt tries to lock
it it is denied). NB, libvirt uses fnctl(cmd = F_SETLK) to lock
pidfiles. So leaking locked FD into qemu-pr-helper is not an option
because that doesn't survive fork().

Another, trivial reason might be that if I run:

  qemu-pr-helper --pidfile /tmp/pr.pid

(without --daemon), I'd still expect qemu-pr-helper to write pidfile
because I told it to.

Michal Privoznik (2):
  qemu-pr-helper: Daemonize before dropping privileges
  qemu-pr-helper: Write pidfile more often

 scsi/qemu-pr-helper.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

-- 
2.16.1

Re: [Qemu-devel] [PATCH 0/2] Couple of qemu-pr-helper fixes

Posted by Fam Zheng 6 years ago

On Tue, 04/03 15:12, Michal Privoznik wrote:
> The first one is trivial.
> 
> The second is slightly trickier. Libvirt has this virCommand subsystem
> (found under src/util/vircommand.c) which it uses to spawn commands. The
> subsystem allows libvirt to both daemonize a process and learn its PID.
> The latter of course does not work if command daemonizes itself (hence
> libvirt will not use qemu-pr-helper --daemon). BUT, we still need
> qemu-pr-helper to lock the pidfile (so that when libvirt tries to lock
> it it is denied). NB, libvirt uses fnctl(cmd = F_SETLK) to lock
> pidfiles. So leaking locked FD into qemu-pr-helper is not an option
> because that doesn't survive fork().
> 
> Another, trivial reason might be that if I run:
> 
>   qemu-pr-helper --pidfile /tmp/pr.pid
> 
> (without --daemon), I'd still expect qemu-pr-helper to write pidfile
> because I told it to.
> 
> Michal Privoznik (2):
>   qemu-pr-helper: Daemonize before dropping privileges
>   qemu-pr-helper: Write pidfile more often

Reviewed-by: Fam Zheng <famz@redhat.com>