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

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.