On 1/3/23 12:02, Andrea Bolognani wrote:
On Tue, Jan 03, 2023 at 10:57:21AM -0700, Jim Fehlig wrote:
On 1/3/23 10:27, Jim Fehlig wrote:
On 1/2/23 08:26, Andrea Bolognani wrote:
This version is fine, but as explained elsewhere I think it would be
better to have
Requires: libvirt-daemon-common = %{version}-%{release}
Requires: libvirt-daemon-log = %{version}-%{release}
Recommends: libvirt-daemon-proxy = %{version}-%{release}
and no dependency at all on the locking part.
Rationale:
* virtproxyd being present allows clients that are older than ~2
years to connect, so it should be there by default while still
making it possible for the admin to opt out, which can be done by
simply uninstalling the corresponding package;
Agree. virtproxyd was created to help transition from monolithic to
modular daemons so makes sense to include the weak dependency.
* storage locking is not the default behavior and needs to be
turned on explicitly, so it's not a big deal if part of the setup
involves installing a couple extra packages in addition to
editing some configuration files, and everyone else gets a leaner
installation.
I'm fine dropping the daemon-lock dependency. I do seem to recall an old
discussion about enabling lockd by default, but I guess it's no longer
necessary with qemu locking image files these days.
BTW, I've made these changes to a local branch and pushed to my fork
https://gitlab.com/jfehlig/libvirt/-/tree/spec-modular-daemons-v2
and enabled the CI pipeline when pushing
https://gitlab.com/jfehlig/libvirt/-/pipelines/737202454
Let me know if a V6 is needed. This patch is the last one without a R-B.
I'd post what you have on the branch as v6. It looks good, so I'll
give my R-b to the patch that's missing it, but I'd prefer if Dan had
a chance to look over everything one last time before merging the
series.
Sounds good. Thanks for the reviews and help with this series!
Regards,
Jim
