Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
libvirt.spec.in | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f6d644a3ae..d5243e859b 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -441,6 +441,7 @@ Summary: Server side daemon and supporting files for libvirt library
# The client side, i.e. shared libs are in a subpackage
Requires: libvirt-libs = %{version}-%{release}
Requires: libvirt-daemon-lock = %{version}-%{release}
+Requires: libvirt-daemon-plugin-lockd = %{version}-%{release}
Requires: libvirt-daemon-log = %{version}-%{release}
Requires: libvirt-daemon-proxy = %{version}-%{release}
@@ -504,6 +505,13 @@ Requires: libvirt-libs = %{version}-%{release}
Server side daemon used to manage locks held against virtual machine
resources
+%package daemon-plugin-lockd
+Plugin for virtlockd
+Requires: libvirt-libs = %{version}-%{release}
+
+%description daemon-plugin-lockd
+A plugin for integrating with virtlockd
+
%package daemon-log
Summary: Server side daemon for managing logs
Requires: libvirt-libs = %{version}-%{release}
@@ -1866,8 +1874,6 @@ exit 0
%dir %attr(0755, root, root) %{_libdir}/libvirt/
%dir %attr(0755, root, root) %{_libdir}/libvirt/connection-driver/
-%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
-%attr(0755, root, root) %{_libdir}/libvirt/lock-driver/lockd.so
%{_datadir}/augeas/lenses/libvirtd.aug
%{_datadir}/augeas/lenses/tests/test_libvirtd.aug
@@ -1909,6 +1915,10 @@ exit 0
%attr(0755, root, root) %{_sbindir}/virtlockd
%{_mandir}/man8/virtlockd.8*
+%files daemon-plugin-lockd
+%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
+%attr(0755, root, root) %{_libdir}/libvirt/lock-driver/lockd.so
+
%files daemon-log
%{_unitdir}/virtlogd.service
%{_unitdir}/virtlogd.socket
--
2.38.1On Fri, Dec 02, 2022 at 05:17:35PM -0700, Jim Fehlig wrote:
> +%package daemon-plugin-lockd
> +Plugin for virtlockd
> +Requires: libvirt-libs = %{version}-%{release}
Maybe libvirt-daemon-lock-plugin-lockd? A bit verbose, but would help
better differenciate it from other loadable drivers.
Alternatively we could follow the example set by the storage drivers
and go with libvirt-daemon-driver-lock-lockd. Pretty ugly, and also
kind of inaccurate because, unlike the storage driver, the lock
functionality can't be loaded into the monolithic daemon and always
lives, by design, in a separate daemon.
Either way, we should take the existing libvirt-lock-sanlock package
and convert it to the new naming convention for consistency.
Both packages should depend on libvirt-daemon-lock too, instead of
just the libraries.
> +%files daemon-plugin-lockd
> +%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
I believe this directory belongs to either the libvirt-daemon-lock
package (more likely) or possibly the libvirt-daemon-common package.
--
Andrea Bolognani / Red Hat / Virtualization
On Sun, Dec 11, 2022 at 10:22:00AM -0800, Andrea Bolognani wrote:
> On Fri, Dec 02, 2022 at 05:17:35PM -0700, Jim Fehlig wrote:
> > +%package daemon-plugin-lockd
> > +Plugin for virtlockd
> > +Requires: libvirt-libs = %{version}-%{release}
>
> Maybe libvirt-daemon-lock-plugin-lockd? A bit verbose, but would help
> better differenciate it from other loadable drivers.
The other loadable drivers are in libvirt-dameon-driver-XXX
packages, so IMHO it is already easily distinguished by
being in a libvirt-daemon-plugin-XXX package. So lets
keep it more concise as Jim has it named here.
> Either way, we should take the existing libvirt-lock-sanlock package
> and convert it to the new naming convention for consistency.
Yes.
> Both packages should depend on libvirt-daemon-lock too, instead of
> just the libraries.
Nope, they shouldn't - that's the virtlockd server, which is completely
separate from these plugins.
>
> > +%files daemon-plugin-lockd
> > +%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
>
> I believe this directory belongs to either the libvirt-daemon-lock
> package (more likely) or possibly the libvirt-daemon-common package.
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
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 :|
On Tue, Dec 13, 2022 at 09:57:31AM +0000, Daniel P. Berrangé wrote:
> On Sun, Dec 11, 2022 at 10:22:00AM -0800, Andrea Bolognani wrote:
> > Both packages should depend on libvirt-daemon-lock too, instead of
> > just the libraries.
>
> Nope, they shouldn't - that's the virtlockd server, which is completely
> separate from these plugins.
Okay, thanks for explaining this. I was clearly providing bad advice
based on my flawed understanding of the situation O:-)
> > > +%package daemon-plugin-lockd
> > > +Plugin for virtlockd
> > > +Requires: libvirt-libs = %{version}-%{release}
> >
> > Maybe libvirt-daemon-lock-plugin-lockd? A bit verbose, but would help
> > better differenciate it from other loadable drivers.
>
> The other loadable drivers are in libvirt-dameon-driver-XXX
> packages, so IMHO it is already easily distinguished by
> being in a libvirt-daemon-plugin-XXX package. So lets
> keep it more concise as Jim has it named here.
I think the distinction is not as obvious to someone who's not
intimately familiar with the inner workings of libvirt. And the files
in question are stored in a directory called "lock-driver", not
"lock-plugin", so I'd say we're not entirely consistent about it
internally either :)
There's another naming question that I've been thinking about: the
libvirt-daemon-driver-foo convention made perfect sense when the
(monolithic) daemon would load the various drivers, but as we
progress further towards a fully modular future it starts to become
awkward.
For example, you will have a system where libvirt-daemon-driver-qemu
is installed but libvirt-daemon is not. That feels weird.
I don't have a great solution for this. My instinct would be to have
a libvirt-daemon-foo for each foo:// that libvirt supports, and then
probably libvirt-daemon-foo-bar or libvirt-daemon-foo-plugin-bar for
each bar backend/implementation of foo://. But for the various
hypervisor drivers, those names are already taken...
Any ideas?
--
Andrea Bolognani / Red Hat / Virtualization
On 12/11/22 11:22, Andrea Bolognani wrote:
> On Fri, Dec 02, 2022 at 05:17:35PM -0700, Jim Fehlig wrote:
>> +%package daemon-plugin-lockd
>> +Plugin for virtlockd
>> +Requires: libvirt-libs = %{version}-%{release}
>
> Maybe libvirt-daemon-lock-plugin-lockd? A bit verbose, but would help
> better differenciate it from other loadable drivers.
>
> Alternatively we could follow the example set by the storage drivers
> and go with libvirt-daemon-driver-lock-lockd. Pretty ugly, and also
> kind of inaccurate because, unlike the storage driver, the lock
> functionality can't be loaded into the monolithic daemon and always
> lives, by design, in a separate daemon.
I slightly prefer libvirt-daemon-lock-plugin-lockd. Yes it's verbose, but it
does a better job of describing the thing.
> Either way, we should take the existing libvirt-lock-sanlock package
> and convert it to the new naming convention for consistency.
>
> Both packages should depend on libvirt-daemon-lock too, instead of
> just the libraries.
>
>> +%files daemon-plugin-lockd
>> +%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
>
> I believe this directory belongs to either the libvirt-daemon-lock
> package (more likely) or possibly the libvirt-daemon-common package.
Yep, I think libvirt-daemon-lock is correct. Thanks a lot for the review and
comments!
Regards,
Jim
On Mon, Dec 12, 2022 at 03:39:44PM -0700, Jim Fehlig wrote:
> On 12/11/22 11:22, Andrea Bolognani wrote:
> > On Fri, Dec 02, 2022 at 05:17:35PM -0700, Jim Fehlig wrote:
> > > +%package daemon-plugin-lockd
> > > +Plugin for virtlockd
> > > +Requires: libvirt-libs = %{version}-%{release}
> >
> > Maybe libvirt-daemon-lock-plugin-lockd? A bit verbose, but would help
> > better differenciate it from other loadable drivers.
> >
> > Alternatively we could follow the example set by the storage drivers
> > and go with libvirt-daemon-driver-lock-lockd. Pretty ugly, and also
> > kind of inaccurate because, unlike the storage driver, the lock
> > functionality can't be loaded into the monolithic daemon and always
> > lives, by design, in a separate daemon.
>
> I slightly prefer libvirt-daemon-lock-plugin-lockd. Yes it's verbose, but it
> does a better job of describing the thing.
>
> > Either way, we should take the existing libvirt-lock-sanlock package
> > and convert it to the new naming convention for consistency.
> >
> > Both packages should depend on libvirt-daemon-lock too, instead of
> > just the libraries.
> >
> > > +%files daemon-plugin-lockd
> > > +%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
> >
> > I believe this directory belongs to either the libvirt-daemon-lock
> > package (more likely) or possibly the libvirt-daemon-common package.
>
> Yep, I think libvirt-daemon-lock is correct. Thanks a lot for the review and
> comments!
libvirt-daemon-lock isn't the right place, as that's the home
for virtlockd.
This directory is what contains either lockd.so or sanlock.so,
which are both client side plugins for other daemons.
It could go in libvirt-daemon-common, or it can just be
duplicated in both the daemon-plugin-lockd and daemon-lock-sanlock
packages - its fine to have multiple RPMs own the same dir, as long
as permissions/user/group match
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 :|
On Tue, Dec 13, 2022 at 09:55:19AM +0000, Daniel P. Berrangé wrote:
> On Mon, Dec 12, 2022 at 03:39:44PM -0700, Jim Fehlig wrote:
> > > > +%files daemon-plugin-lockd
> > > > +%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
> > >
> > > I believe this directory belongs to either the libvirt-daemon-lock
> > > package (more likely) or possibly the libvirt-daemon-common package.
> >
> > Yep, I think libvirt-daemon-lock is correct. Thanks a lot for the review and
> > comments!
>
> libvirt-daemon-lock isn't the right place, as that's the home
> for virtlockd.
>
> This directory is what contains either lockd.so or sanlock.so,
> which are both client side plugins for other daemons.
virlockd is the one that's going to load the plugins, so I don't
think it's necessarily wrong for its package to own the directory.
> It could go in libvirt-daemon-common, or it can just be
> duplicated in both the daemon-plugin-lockd and daemon-lock-sanlock
> packages - its fine to have multiple RPMs own the same dir, as long
> as permissions/user/group match
libvirt-daemon-common sounds good. It is already going to own the
connection-driver directory. But libvirt-daemon-lock will need a
dependency on it, which otherwise it wouldn't have.
--
Andrea Bolognani / Red Hat / Virtualization
On Tue, Dec 13, 2022 at 02:07:12AM -0800, Andrea Bolognani wrote:
> On Tue, Dec 13, 2022 at 09:55:19AM +0000, Daniel P. Berrangé wrote:
> > On Mon, Dec 12, 2022 at 03:39:44PM -0700, Jim Fehlig wrote:
> > > > > +%files daemon-plugin-lockd
> > > > > +%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
> > > >
> > > > I believe this directory belongs to either the libvirt-daemon-lock
> > > > package (more likely) or possibly the libvirt-daemon-common package.
> > >
> > > Yep, I think libvirt-daemon-lock is correct. Thanks a lot for the review and
> > > comments!
> >
> > libvirt-daemon-lock isn't the right place, as that's the home
> > for virtlockd.
> >
> > This directory is what contains either lockd.so or sanlock.so,
> > which are both client side plugins for other daemons.
>
> virlockd is the one that's going to load the plugins, so I don't
> think it's necessarily wrong for its package to own the directory.
No it is not. virtqemud locks the lockd.so plugin, as lockd.so
provides the client impl to talk to virtlockd. Think of lockd.so
as being equiv of libvirt.so
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 :|
On 12/2/22 17:17, Jim Fehlig wrote:
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> libvirt.spec.in | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index f6d644a3ae..d5243e859b 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -441,6 +441,7 @@ Summary: Server side daemon and supporting files for libvirt library
> # The client side, i.e. shared libs are in a subpackage
> Requires: libvirt-libs = %{version}-%{release}
> Requires: libvirt-daemon-lock = %{version}-%{release}
> +Requires: libvirt-daemon-plugin-lockd = %{version}-%{release}
> Requires: libvirt-daemon-log = %{version}-%{release}
> Requires: libvirt-daemon-proxy = %{version}-%{release}
>
> @@ -504,6 +505,13 @@ Requires: libvirt-libs = %{version}-%{release}
> Server side daemon used to manage locks held against virtual machine
> resources
>
> +%package daemon-plugin-lockd
> +Plugin for virtlockd
That no workie. I've squashed the below into my local branch.
Regards,
Jim
diff --git a/libvirt.spec.in b/libvirt.spec.in
index d5243e859b..b3f8b5d325 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -506,7 +506,7 @@ Server side daemon used to manage locks held against virtual
machine
resources
%package daemon-plugin-lockd
-Plugin for virtlockd
+Summary: Plugin for virtlockd
Requires: libvirt-libs = %{version}-%{release}
%description daemon-plugin-lockd
© 2016 - 2026 Red Hat, Inc.