[PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'

Peter Krempa via Devel posted 1 patch 2 weeks, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/b4f624c1cffd7c1ec5deeb8558cf5f40c0188d4c.1763971647.git.pkrempa@redhat.com
libvirt.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'
Posted by Peter Krempa via Devel 2 weeks, 4 days ago
From: Peter Krempa <pkrempa@redhat.com>

On Fedora 43 and newer the 'fuse-zfs' package was removed. Commit
bd30147e740 added an 'Obsoletes' directive so that the storage driver
core package will update properly but hardcoded the obsoleted version
as 11.4 (when the change was comitted) similarly to the old sheepdog/rbd
packages.

Unfortunately that doesn't work if the sub-package removal happened
based on an external dependancy, rather than complete removal of said
module in libvirt. If users have e.g. virt-preview repos enabled they'll
get libvirt-11.9 currently installed, but the obsoleted version is just
11.4, thus upgrades to Fedora 43 with virt-preview are broken.

Fix this by obsoleting everything up to the current build.

Fixes: bd30147e740d49fdb5844160e480ca34611f75e5
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 libvirt.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8314fbeb34..f1bfd5e652 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -668,7 +668,7 @@ Obsoletes: libvirt-daemon-driver-storage-rbd < 5.2.0
     %endif
 Obsoletes: libvirt-daemon-driver-storage-sheepdog < 8.8.0
     %if !%{with_storage_zfs}
-Obsoletes: libvirt-daemon-driver-storage-zfs < 11.4.0
+Obsoletes: libvirt-daemon-driver-storage-zfs <= %{version}-%{release}
     %endif

 %description daemon-driver-storage-core
-- 
2.51.1
Re: [PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'
Posted by Ján Tomko via Devel 2 weeks, 4 days ago
On a Monday in 2025, Peter Krempa via Devel wrote:
>From: Peter Krempa <pkrempa@redhat.com>
>
>On Fedora 43 and newer the 'fuse-zfs' package was removed. Commit
>bd30147e740 added an 'Obsoletes' directive so that the storage driver
>core package will update properly but hardcoded the obsoleted version
>as 11.4 (when the change was comitted) similarly to the old sheepdog/rbd
>packages.
>
>Unfortunately that doesn't work if the sub-package removal happened
>based on an external dependancy, rather than complete removal of said

*dependency

>module in libvirt. If users have e.g. virt-preview repos enabled they'll
>get libvirt-11.9 currently installed, but the obsoleted version is just
>11.4, thus upgrades to Fedora 43 with virt-preview are broken.
>

Isn't that the expected behavior?

>Fix this by obsoleting everything up to the current build.
>
>Fixes: bd30147e740d49fdb5844160e480ca34611f75e5
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> libvirt.spec.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/libvirt.spec.in b/libvirt.spec.in
>index 8314fbeb34..f1bfd5e652 100644
>--- a/libvirt.spec.in
>+++ b/libvirt.spec.in
>@@ -668,7 +668,7 @@ Obsoletes: libvirt-daemon-driver-storage-rbd < 5.2.0
>     %endif
> Obsoletes: libvirt-daemon-driver-storage-sheepdog < 8.8.0
>     %if !%{with_storage_zfs}
>-Obsoletes: libvirt-daemon-driver-storage-zfs < 11.4.0
>+Obsoletes: libvirt-daemon-driver-storage-zfs <= %{version}-%{release}
>     %endif
>

Either way,
Reviewed-by: Ján Tomko <jtomko@redhat.com>

It won't stay there for too long.

Jano
Re: [PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'
Posted by Peter Krempa via Devel 2 weeks, 4 days ago
On Mon, Nov 24, 2025 at 09:32:52 +0100, Ján Tomko wrote:
> On a Monday in 2025, Peter Krempa via Devel wrote:
> > From: Peter Krempa <pkrempa@redhat.com>
> > 
> > On Fedora 43 and newer the 'fuse-zfs' package was removed. Commit
> > bd30147e740 added an 'Obsoletes' directive so that the storage driver
> > core package will update properly but hardcoded the obsoleted version
> > as 11.4 (when the change was comitted) similarly to the old sheepdog/rbd
> > packages.
> > 
> > Unfortunately that doesn't work if the sub-package removal happened
> > based on an external dependancy, rather than complete removal of said
> 
> *dependency
> 
> > module in libvirt. If users have e.g. virt-preview repos enabled they'll
> > get libvirt-11.9 currently installed, but the obsoleted version is just
> > 11.4, thus upgrades to Fedora 43 with virt-preview are broken.
> > 
> 
> Isn't that the expected behavior?

No. While it doesn't break for users not using virt-preview since
they're on 11.0.0-5.fc42 it is expected that updates from any version
actually work properly.

> > Fix this by obsoleting everything up to the current build.
> > 
> > Fixes: bd30147e740d49fdb5844160e480ca34611f75e5
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > libvirt.spec.in | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index 8314fbeb34..f1bfd5e652 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -668,7 +668,7 @@ Obsoletes: libvirt-daemon-driver-storage-rbd < 5.2.0
> >     %endif
> > Obsoletes: libvirt-daemon-driver-storage-sheepdog < 8.8.0
> >     %if !%{with_storage_zfs}
> > -Obsoletes: libvirt-daemon-driver-storage-zfs < 11.4.0
> > +Obsoletes: libvirt-daemon-driver-storage-zfs <= %{version}-%{release}
> >     %endif

Looking at it now ... I wonder if this perhaps shouldn't be just '<'
given that it uses also %{release}


> 
> Either way,
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> 
> It won't stay there for too long.

The Obsoletes line will be kept "forever" similarly to e.g. the sheepdog
driver line.
Re: [PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'
Posted by Daniel P. Berrangé via Devel 2 weeks, 4 days ago
On Mon, Nov 24, 2025 at 09:44:11AM +0100, Peter Krempa via Devel wrote:
> On Mon, Nov 24, 2025 at 09:32:52 +0100, Ján Tomko wrote:
> > On a Monday in 2025, Peter Krempa via Devel wrote:
> > > From: Peter Krempa <pkrempa@redhat.com>
> > > 
> > > On Fedora 43 and newer the 'fuse-zfs' package was removed. Commit
> > > bd30147e740 added an 'Obsoletes' directive so that the storage driver
> > > core package will update properly but hardcoded the obsoleted version
> > > as 11.4 (when the change was comitted) similarly to the old sheepdog/rbd
> > > packages.
> > > 
> > > Unfortunately that doesn't work if the sub-package removal happened
> > > based on an external dependancy, rather than complete removal of said
> > 
> > *dependency
> > 
> > > module in libvirt. If users have e.g. virt-preview repos enabled they'll
> > > get libvirt-11.9 currently installed, but the obsoleted version is just
> > > 11.4, thus upgrades to Fedora 43 with virt-preview are broken.
> > > 
> > 
> > Isn't that the expected behavior?
> 
> No. While it doesn't break for users not using virt-preview since
> they're on 11.0.0-5.fc42 it is expected that updates from any version
> actually work properly.

IMHO if someone has virt-preview enabled and wants to upgrade to
newer Fedora, they must first disable virt-preview and then do
a  "dnf distro-sync" to remove the virt-preview packages.
We don't guarantee an upgrade path from virt-preview -> Fedora

> 
> > > Fix this by obsoleting everything up to the current build.
> > > 
> > > Fixes: bd30147e740d49fdb5844160e480ca34611f75e5
> > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > ---
> > > libvirt.spec.in | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > > index 8314fbeb34..f1bfd5e652 100644
> > > --- a/libvirt.spec.in
> > > +++ b/libvirt.spec.in
> > > @@ -668,7 +668,7 @@ Obsoletes: libvirt-daemon-driver-storage-rbd < 5.2.0
> > >     %endif
> > > Obsoletes: libvirt-daemon-driver-storage-sheepdog < 8.8.0
> > >     %if !%{with_storage_zfs}
> > > -Obsoletes: libvirt-daemon-driver-storage-zfs < 11.4.0
> > > +Obsoletes: libvirt-daemon-driver-storage-zfs <= %{version}-%{release}
> > >     %endif
> 
> Looking at it now ... I wonder if this perhaps shouldn't be just '<'
> given that it uses also %{release}

Using  "%{version}" in an Obsoletes is contrary to Fedora guidelines
which want us to explicitly use the last version which had the feature
available.

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 :|

Re: [PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'
Posted by Peter Krempa via Devel 2 weeks, 4 days ago
On Mon, Nov 24, 2025 at 10:00:34 +0000, Daniel P. Berrangé wrote:
> On Mon, Nov 24, 2025 at 09:44:11AM +0100, Peter Krempa via Devel wrote:
> > On Mon, Nov 24, 2025 at 09:32:52 +0100, Ján Tomko wrote:
> > > On a Monday in 2025, Peter Krempa via Devel wrote:
> > > > From: Peter Krempa <pkrempa@redhat.com>
> > > > 
> > > > On Fedora 43 and newer the 'fuse-zfs' package was removed. Commit
> > > > bd30147e740 added an 'Obsoletes' directive so that the storage driver
> > > > core package will update properly but hardcoded the obsoleted version
> > > > as 11.4 (when the change was comitted) similarly to the old sheepdog/rbd
> > > > packages.
> > > > 
> > > > Unfortunately that doesn't work if the sub-package removal happened
> > > > based on an external dependancy, rather than complete removal of said
> > > 
> > > *dependency
> > > 
> > > > module in libvirt. If users have e.g. virt-preview repos enabled they'll
> > > > get libvirt-11.9 currently installed, but the obsoleted version is just
> > > > 11.4, thus upgrades to Fedora 43 with virt-preview are broken.
> > > > 
> > > 
> > > Isn't that the expected behavior?
> > 
> > No. While it doesn't break for users not using virt-preview since
> > they're on 11.0.0-5.fc42 it is expected that updates from any version
> > actually work properly.
> 
> IMHO if someone has virt-preview enabled and wants to upgrade to
> newer Fedora, they must first disable virt-preview and then do
> a  "dnf distro-sync" to remove the virt-preview packages.
> We don't guarantee an upgrade path from virt-preview -> Fedora

Technically I'm updating from virt-preview on 42  to virt-preview on 43.

While I we likely don't claim any responsibility for any of the
virt-preview packages I don't see a logical reason why that shouldn't
work (and it does unless stuff like this happens).

> > > > Fix this by obsoleting everything up to the current build.
> > > > 
> > > > Fixes: bd30147e740d49fdb5844160e480ca34611f75e5
> > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > > ---
> > > > libvirt.spec.in | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > > > index 8314fbeb34..f1bfd5e652 100644
> > > > --- a/libvirt.spec.in
> > > > +++ b/libvirt.spec.in
> > > > @@ -668,7 +668,7 @@ Obsoletes: libvirt-daemon-driver-storage-rbd < 5.2.0
> > > >     %endif
> > > > Obsoletes: libvirt-daemon-driver-storage-sheepdog < 8.8.0
> > > >     %if !%{with_storage_zfs}
> > > > -Obsoletes: libvirt-daemon-driver-storage-zfs < 11.4.0
> > > > +Obsoletes: libvirt-daemon-driver-storage-zfs <= %{version}-%{release}
> > > >     %endif
> > 
> > Looking at it now ... I wonder if this perhaps shouldn't be just '<'
> > given that it uses also %{release}
> 
> Using  "%{version}" in an Obsoletes is contrary to Fedora guidelines
> which want us to explicitly use the last version which had the feature
> available.

Well that works well when the package itself removes the feature, but if
the package removal is caused by a dependancy it breaks if anyone build
something else than the official build.

I guess the package guidelines will explicitly not support that, but
still it's terrible UX if someone uses anything else than an official
build. In addition the hardcoded version will also work for everyone who
used any other custom build package, just not the "later" ones.

Finally the '11.4.0' doesn't even match what Fedora 42 ships,
(11.0.0-5.fc42) so I don't really understand why we can't use the latest
version macro in this case.
Re: [PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'
Posted by Daniel P. Berrangé via Devel 2 weeks, 4 days ago
On Mon, Nov 24, 2025 at 11:19:11AM +0100, Peter Krempa wrote:
> On Mon, Nov 24, 2025 at 10:00:34 +0000, Daniel P. Berrangé wrote:
> > On Mon, Nov 24, 2025 at 09:44:11AM +0100, Peter Krempa via Devel wrote:
> > > On Mon, Nov 24, 2025 at 09:32:52 +0100, Ján Tomko wrote:
> > > > On a Monday in 2025, Peter Krempa via Devel wrote:
> > > > > From: Peter Krempa <pkrempa@redhat.com>
> > > > > 
> > > > > On Fedora 43 and newer the 'fuse-zfs' package was removed. Commit
> > > > > bd30147e740 added an 'Obsoletes' directive so that the storage driver
> > > > > core package will update properly but hardcoded the obsoleted version
> > > > > as 11.4 (when the change was comitted) similarly to the old sheepdog/rbd
> > > > > packages.
> > > > > 
> > > > > Unfortunately that doesn't work if the sub-package removal happened
> > > > > based on an external dependancy, rather than complete removal of said
> > > > 
> > > > *dependency
> > > > 
> > > > > module in libvirt. If users have e.g. virt-preview repos enabled they'll
> > > > > get libvirt-11.9 currently installed, but the obsoleted version is just
> > > > > 11.4, thus upgrades to Fedora 43 with virt-preview are broken.
> > > > > 
> > > > 
> > > > Isn't that the expected behavior?
> > > 
> > > No. While it doesn't break for users not using virt-preview since
> > > they're on 11.0.0-5.fc42 it is expected that updates from any version
> > > actually work properly.
> > 
> > IMHO if someone has virt-preview enabled and wants to upgrade to
> > newer Fedora, they must first disable virt-preview and then do
> > a  "dnf distro-sync" to remove the virt-preview packages.
> > We don't guarantee an upgrade path from virt-preview -> Fedora
> 
> Technically I'm updating from virt-preview on 42  to virt-preview on 43.
> 
> While I we likely don't claim any responsibility for any of the
> virt-preview packages I don't see a logical reason why that shouldn't
> work (and it does unless stuff like this happens).
> 
> > > > > Fix this by obsoleting everything up to the current build.
> > > > > 
> > > > > Fixes: bd30147e740d49fdb5844160e480ca34611f75e5
> > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > > > ---
> > > > > libvirt.spec.in | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > > > > index 8314fbeb34..f1bfd5e652 100644
> > > > > --- a/libvirt.spec.in
> > > > > +++ b/libvirt.spec.in
> > > > > @@ -668,7 +668,7 @@ Obsoletes: libvirt-daemon-driver-storage-rbd < 5.2.0
> > > > >     %endif
> > > > > Obsoletes: libvirt-daemon-driver-storage-sheepdog < 8.8.0
> > > > >     %if !%{with_storage_zfs}
> > > > > -Obsoletes: libvirt-daemon-driver-storage-zfs < 11.4.0
> > > > > +Obsoletes: libvirt-daemon-driver-storage-zfs <= %{version}-%{release}
> > > > >     %endif
> > > 
> > > Looking at it now ... I wonder if this perhaps shouldn't be just '<'
> > > given that it uses also %{release}
> > 
> > Using  "%{version}" in an Obsoletes is contrary to Fedora guidelines
> > which want us to explicitly use the last version which had the feature
> > available.
> 
> Well that works well when the package itself removes the feature, but if
> the package removal is caused by a dependancy it breaks if anyone build
> something else than the official build.
> 
> I guess the package guidelines will explicitly not support that, but
> still it's terrible UX if someone uses anything else than an official
> build. In addition the hardcoded version will also work for everyone who
> used any other custom build package, just not the "later" ones.
> 
> Finally the '11.4.0' doesn't even match what Fedora 42 ships,
> (11.0.0-5.fc42) so I don't really understand why we can't use the latest
> version macro in this case.

Actually now I've had coffee, I remember that we had a user complaining
that we should not have added the Obsoletes at all in this case.

While ZFS was removed from Fedora, it is still possible to get the ZFS
tools and use them on Fedora with libvirt.

We have precedent here with enabling cloud hypervisor and virtualbox
both of which require 3rd party tools to be installed.

Sheepdog was different  because the sheepdog project is dead and the
code was culled from libvirt/qemu entirely.

IOW, we should just remove this "Obsoletes" line for ZFS, and re-enable
the zfs sub-RPM.

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 :|

Re: [PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'
Posted by Peter Krempa via Devel 2 weeks, 3 days ago
On Mon, Nov 24, 2025 at 10:22:57 +0000, Daniel P. Berrangé wrote:
> On Mon, Nov 24, 2025 at 11:19:11AM +0100, Peter Krempa wrote:
> > On Mon, Nov 24, 2025 at 10:00:34 +0000, Daniel P. Berrangé wrote:
> > > On Mon, Nov 24, 2025 at 09:44:11AM +0100, Peter Krempa via Devel wrote:

[...]

> > Well that works well when the package itself removes the feature, but if
> > the package removal is caused by a dependancy it breaks if anyone build
> > something else than the official build.
> > 
> > I guess the package guidelines will explicitly not support that, but
> > still it's terrible UX if someone uses anything else than an official
> > build. In addition the hardcoded version will also work for everyone who
> > used any other custom build package, just not the "later" ones.
> > 
> > Finally the '11.4.0' doesn't even match what Fedora 42 ships,
> > (11.0.0-5.fc42) so I don't really understand why we can't use the latest
> > version macro in this case.
> 
> Actually now I've had coffee, I remember that we had a user complaining
> that we should not have added the Obsoletes at all in this case.
> 
> While ZFS was removed from Fedora, it is still possible to get the ZFS
> tools and use them on Fedora with libvirt.
> 
> We have precedent here with enabling cloud hypervisor and virtualbox
> both of which require 3rd party tools to be installed.

Agreed; In such case though IMO we need to also change that
'daemon-driver-storage' no longer 'Requires'
libvirt-daemon-driver-storage-zfs, but rather 'Recommends' or drop it
completely since it's useless without extra installation.

I vote for completely dropping it from 'daemon-driver-storage'
dependency on Fedora 43 and later.

> Sheepdog was different  because the sheepdog project is dead and the
> code was culled from libvirt/qemu entirely.

Yes; my point was that the 'Obsoletes' is fine with a specific version
if the code was removed from the package itself. But if it depends on
the build environment it IMO ought to use a rolling version because
otherwise you can get into a situation where it will break.

> IOW, we should just remove this "Obsoletes" line for ZFS, and re-enable
> the zfs sub-RPM.
Re: [PATCH] spec: Fix 'obsoletes' for 'libvirt-daemon-driver-storage-zfs'
Posted by Daniel P. Berrangé via Devel 2 weeks, 3 days ago
On Mon, Nov 24, 2025 at 01:35:07PM +0100, Peter Krempa wrote:
> On Mon, Nov 24, 2025 at 10:22:57 +0000, Daniel P. Berrangé wrote:
> > On Mon, Nov 24, 2025 at 11:19:11AM +0100, Peter Krempa wrote:
> > > On Mon, Nov 24, 2025 at 10:00:34 +0000, Daniel P. Berrangé wrote:
> > > > On Mon, Nov 24, 2025 at 09:44:11AM +0100, Peter Krempa via Devel wrote:
> 
> [...]
> 
> > > Well that works well when the package itself removes the feature, but if
> > > the package removal is caused by a dependancy it breaks if anyone build
> > > something else than the official build.
> > > 
> > > I guess the package guidelines will explicitly not support that, but
> > > still it's terrible UX if someone uses anything else than an official
> > > build. In addition the hardcoded version will also work for everyone who
> > > used any other custom build package, just not the "later" ones.
> > > 
> > > Finally the '11.4.0' doesn't even match what Fedora 42 ships,
> > > (11.0.0-5.fc42) so I don't really understand why we can't use the latest
> > > version macro in this case.
> > 
> > Actually now I've had coffee, I remember that we had a user complaining
> > that we should not have added the Obsoletes at all in this case.
> > 
> > While ZFS was removed from Fedora, it is still possible to get the ZFS
> > tools and use them on Fedora with libvirt.
> > 
> > We have precedent here with enabling cloud hypervisor and virtualbox
> > both of which require 3rd party tools to be installed.
> 
> Agreed; In such case though IMO we need to also change that
> 'daemon-driver-storage' no longer 'Requires'
> libvirt-daemon-driver-storage-zfs, but rather 'Recommends' or drop it
> completely since it's useless without extra installation.
> 
> I vote for completely dropping it from 'daemon-driver-storage'
> dependency on Fedora 43 and later.

Yep, lets just drop the Requires dep entirely on new enough Fedora, as
we can't assume RPM package names for anything outside Fedora repos.

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 :|