Re: [libvirt] [PATCH 19/41] remote: introduce virtproxyd daemon to handle IP connectivity

On Mon, Jul 29, 2019 at 01:30:42PM +0100, Daniel P. Berrangé wrote: > On Sun, Jul 28, 2019 at 04:42:52PM +0200, Andrea Bolognani wrote: > > On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote: > > [...] > > > - We can make virtproxyd and the virtXXXd per-driver daemons all > > > have "Conflicts: libvirtd.service" in their systemd unit files. > > > This will guarantee that libvirtd is never started at the same > > > time, as this would result in two daemons running the same driver. > > > Fortunately drivers use locking to protect themselves, but it is > > > better to avoid starting a daemon we know will conflict. > > > > I feel like this will need to be tested extensively to make sure > > we're always doing the right thing, including on non-systemd hosts. > > Testing is quite easy - just try to start the two units and make > sure only one ends up running. Similarly for non-systemd hosts, > start both daemons & see that only one succeeds - the others > fail with lock conflict. > > > > > +++ b/src/remote/virtproxyd.service.in > > > @@ -0,0 +1,24 @@ > > > +[Unit] > > > +Description=Virtualization daemon > > > +Conflicts=libvirtd.service > > > +Requires=virtproxyd.socket > > > +Requires=virtproxyd-ro.socket > > > +Requires=virtproxyd-admin.socket > > > +After=network.target > > > +After=dbus.service > > > +After=apparmor.service > > > +After=local-fs.target > > > +After=remote-fs.target > > > +Documentation=man:libvirtd(8) > > > +Documentation=https://libvirt.org > > > > There are a few non-obvious changes between libvirtd.service.in and > > this file: > > > > -Requires=virtlogd.socket > > -Requires=virtlockd.socket > > -Wants=systemd-machined.service > > -Before=libvirt-guests.service > > -After=iscsid.service > > -After=systemd-logind.service > > -After=systemd-machined.service > > > > I can see why we'd move the relationships with iscsid and virtlockd > > to virtstoraged, except looking ahead to patch 23 I see you haven't > > actually done that; either way, I'm not so convinced about the > > remaining changes. Care to explain the rationale behind them? > > virtproxdy contains no drivers, so it doesn't need to depend > on any of these services. > > virtdstoraged/qemud/lxcd should have gained some of these though. I should have killed dbus.service and remote-fs.service too.