On Mon, Jan 02, 2023 at 10:26:13AM -0500, Andrea Bolognani wrote:
On Fri, Dec 23, 2022 at 11:56:03AM -0700, Jim Fehlig wrote:
%package daemon-qemu Summary: Server side daemon & driver required to run QEMU guests
+ %if %{with_modular_daemons} +Requires: libvirt-daemon-common = %{version}-%{release} +Requires: libvirt-daemon-log = %{version}-%{release} +Requires: libvirt-daemon-lock = %{version}-%{release} +Requires: libvirt-daemon-plugin-lockd = %{version}-%{release} + %else Requires: libvirt-daemon = %{version}-%{release} + %endif
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}
I don't think we should special case libvirt-daemon-proxy in this way. The libvirt-daemon-kvm package is intended to give consuming apps a guaranteed set of packages for KVM usage.
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;
That rational applies to almost all the deps in libvirt-daemon-kvm. If someone wants a minimal KVM install, then don't install libvirt-daemon-kvm, pick the individual pieces you want instead. Making bits of libvirt-daemon-kvm optional, means that apps requesting it no longer have a guarantee that all the pieces they want are always present, as an admin could have uninstalled bits.
* 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.
Dropping libvirt-daemon-plugin-lockd breaks the upgrade path for people using virtlockd, as nothing will pull in the plugin for talking to virtlockd
Thoughts?
I think this patch is correct as-is. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|