Re: [Qemu-block] [PATCH] pr-manager-helper: fix pr process been killed when reconectting

[Michal Privoznik]

On 5/29/19 11:34 AM, Paolo Bonzini wrote:
> On 29/05/19 09:33, Michal Privoznik wrote:
> > On 5/28/19 7:45 PM, Paolo Bonzini wrote:
> > > On 28/05/19 15:06, Jie Wang wrote:
> > > > if pr-helper been killed and qemu send disconnect event to libvirt
> > > > and libvirt started a new pr-helper process, the new pr-heleper
> > > > been killed again when qemu is connectting to the new pr-helper,
> > > > qemu will never send disconnect to libvirt, and libvirt will never
> > > > receive connected event.
> > >
> > > I think this would let a guest "spam" events just by sending a lot PR
> > > commands.  Also, as you said, in this case QEMU has never sent a
> > > "connected" event, so I'm not sure why it should send a disconnection
> > > event.
> >
> > So pr manager is initialized on the first PR command and not when qemu
> > is starting?
>
> It is initialized when QEMU is started, but if it dies, that's not
> detected until the first PR command.  The command is retried for 5
> seconds, which should give libvirt ample time to restart the PR manager
> (and it's an exceptional situation anyway).

Ah yes, I recall vaguelly testing this whilst writing patches for libvirt.

> > If a user inside the guest could somehow kill pr-helper process in the
> > host then yes, they could spam libvirt/qemu. But if a user from inside a
> > guest can kill a process in the host that is much bigger problem than
> > spaming libvirt.
>
> This is true.
>
> > > Does libvirt monitor at all the pr-helper to check if it dies?  Or does
> > > it rely exclusively on QEMU's events?
> >
> > Libvirt relies solely on QEMU's events. Just like with qemu process
> > itself, libvirt can't rely on SIGCHILD because the daemon might be
> > restarted which would reparent all qemu and pr-helper processes
> > rendering libvirt wait for SIGCHILD useless.
> >
> > But there is an exception to this: when libvirt is spawning pr-helper it
> > does so by following these steps:
> >
> > 1) Try to acquire (lock) pidfile
> > 2) unlink(socket)
> > 3) spawn pr-helper process (this yields child's PID)
> > 4) wait some time until socket is created
> > 5) some follow up work (move child's PID into same cgroup as qemu's main
> > thread, relabel the socket so that qemu can access it)
> >
> > If any of these steps fails then child is killed. However, the PID is
> > not recorded anywhere and thus is forgotten once control jumps out of
> > the function.
>
> Note that qemu-pr-helper supports the systemd socket activation
> protocol.  Would it help if libvirt used it?

Thing is, libvirt creates a mount namespace for domains (one namespace 
for one domain). In this namespace a dummy /dev is mounted and only 
nodes that qemu is configured to have are created. For instance, you 
won't see /dev/sda there unless your domain has it as a disk. Then, 
libvirt moves pr-helper process into the same cgroups as the qemu's main 
thread. This is all done so that pr-helper has the same view of the 
system as qemu. I don't think that he same result can be achieved using 
socket activation.
Also, libvirt spawns one pr-helper per domain (so that the socket can be 
private and share seclabel with qemu process it's attached to).

Michal