Bug 1989514

Fedora Rawhide package:
https://koji.fedoraproject.org/koji/taskinfo?taskID=73378833

Yongkui can you consider QA ACKing this bug please?

Basic verification is to check that libguestfs can still be
installed and still passes basic sanity checks.

There will be a new subpackage ("libguestfs-appliance") which
contains the supermin appliance (/usr/lib64/guestfs/supermin.d/)
and the dependencies required by the appliance, and the main
package (libguestfs) should pull in libguestfs-appliance.  The
main package no longer depends directly on all the appliance
packages.

(In reply to Richard W.M. Jones from comment #2) 
> Basic verification is to check that libguestfs can still be
> installed and still passes basic sanity checks.
> 
> There will be a new subpackage ("libguestfs-appliance") which
> contains the supermin appliance (/usr/lib64/guestfs/supermin.d/)
> and the dependencies required by the appliance, and the main
> package (libguestfs) should pull in libguestfs-appliance.  The
> main package no longer depends directly on all the appliance
> packages.

Got it. Thanks for the details.

Rich,

you seem to have gone with the libguestfs/libguestfs-(no-)appliance
approach rather than the libguestfs/libguestfs-runtime approach I had
proposed.

Is there some advantage to the former that I'm not seeing? AFAICT it
achieves the same results (installing the libguestfs package brings
in the same contents as before, but there's now a way to avoid
installing supermin and its dependencies if you are looking to save
disk space and are willing to bring your own appliance); however, the
implementation you've merged requires you to also create an
additional, possibly dummy libguestfs-no-appliance package instead of
simply dropping the pre-created appliance on the filesystem, which
means more work for the libguestfs consumer.

Can you please explain the rationale for picking one approach over
the other?

Thanks in advance :)

I don't want to have a situation where on a working, normal Fedora
system libguestfs.so.0 is installed but won't work.  The way
I chose gives me that.

Working around broken packaging systems (like bazel) isn't the
job of the Fedora package, so people who want to use them need
to put in the very minimal extra work of creating the Provides:
libguestfs-noappliance.

(In reply to Richard W.M. Jones from comment #7)
> I don't want to have a situation where on a working, normal Fedora
> system libguestfs.so.0 is installed but won't work.  The way
> I chose gives me that.
> 
> Working around broken packaging systems (like bazel) isn't the
> job of the Fedora package, so people who want to use them need
> to put in the very minimal extra work of creating the Provides:
> libguestfs-noappliance.

I'm not familiar with the internals of libguestfs, so please bear
with me if my assumptions are incorrect :)

The way I understand it, the first time libguestfs needs to use an
appliance it will call supermin and ask it to create one in a
per-user temporary directory; the appliance will be created based on
the configuration files in /usr/lib64/guestfs/supermin.d/, and will
contain mostly binaries taken from the host plus a number of custom
configuration files and binaries (notably the init process and
guestfsd daemon). Subsequent uses of libguestfs by the same user
during the same boot will be able to reuse the appliance.

However, libguestfs also has explicit support for the BYOA scenario
through the libguestfs-make-fixed-appliance command, where you
prepare the appliance ahead of time (possibly on a separate machine)
and then simply stash it on the filesystem for libguestfs to use,
which requires explicitly pointing to it using the LIBGUESTFS_PATH
environment variable. IIUC, this is what Alice used to implement
libguestfs support in KubeVirt.

Assuming the above is correct, I will suggest an alternative
implementation strategy: replace the current

  Requires: supermin >= 5.1.18

in libguestfs with

  Requires: (supermin >= 5.1.18 or libguestfs-fixed-appliance)

This looks like it describes the scenario more accurately: in order
for libguestfs to work, you need either supermin (to dynamically
create appliances) or a fixed appliance that you've prepared ahead of
time. It also avoids introducing a new binary package for libguestfs,
and the honestly a bit awkward "libguestfs-noappliance" package name.

Alice would still need to create a simple RPM container around the
fixed appliance instead of shipping it as a tarball, but that would
hopefully not be too big of a problem.

How does this proposal sound?

(In reply to Richard W.M. Jones from comment #7)
> I don't want to have a situation where on a working, normal Fedora
> system libguestfs.so.0 is installed but won't work.  The way
> I chose gives me that.

Note that the original proposal of splitting off the runtime
functionality into a separate binary package should still be on the
table IMO: only consumers who want to opt into the BYOA scenario
would install libguestfs-runtime instead of the full libguestfs
package.

But I understand that you're reluctant to adopt this solution because
a package might depend on libguestfs.so.0 directly instead of the
libguestfs package, and such a package would stop working as a result
of such a change.

(In reply to Andrea Bolognani from comment #8)
> (In reply to Richard W.M. Jones from comment #7)
> > I don't want to have a situation where on a working, normal Fedora
> > system libguestfs.so.0 is installed but won't work.  The way
> > I chose gives me that.
> > 
> > Working around broken packaging systems (like bazel) isn't the
> > job of the Fedora package, so people who want to use them need
> > to put in the very minimal extra work of creating the Provides:
> > libguestfs-noappliance.
> 
> I'm not familiar with the internals of libguestfs, so please bear
> with me if my assumptions are incorrect :)
> 
> The way I understand it, the first time libguestfs needs to use an
> appliance it will call supermin and ask it to create one in a
> per-user temporary directory; the appliance will be created based on
> the configuration files in /usr/lib64/guestfs/supermin.d/, and will
> contain mostly binaries taken from the host plus a number of custom
> configuration files and binaries (notably the init process and
> guestfsd daemon).

This is correct.

> Subsequent uses of libguestfs by the same user
> during the same boot will be able to reuse the appliance.

Any changes to rpmdb will also trigger a rebuild, eg. if a system
package is installed or updated.

> However, libguestfs also has explicit support for the BYOA scenario
> through the libguestfs-make-fixed-appliance command, where you
> prepare the appliance ahead of time (possibly on a separate machine)
> and then simply stash it on the filesystem for libguestfs to use,
> which requires explicitly pointing to it using the LIBGUESTFS_PATH
> environment variable. IIUC, this is what Alice used to implement
> libguestfs support in KubeVirt.

Right.

> Assuming the above is correct, I will suggest an alternative
> implementation strategy: replace the current
> 
>   Requires: supermin >= 5.1.18
> 
> in libguestfs with
> 
>   Requires: (supermin >= 5.1.18 or libguestfs-fixed-appliance)
>
> This looks like it describes the scenario more accurately: in order
> for libguestfs to work, you need either supermin (to dynamically
> create appliances) or a fixed appliance that you've prepared ahead of
> time. It also avoids introducing a new binary package for libguestfs,
> and the honestly a bit awkward "libguestfs-noappliance" package name.

This only solves a fraction of the problem.  The problem isn't really the
supermin dependency which consumes only 1.5MB of disk.  The problem
are the automatically added dependencies of all the other packages
that are copied into the appliance, things like:

$ rpm -qR libguestfs-appliance 
acl
attr
augeas-libs
bash
binutils
btrfs-progs
bzip2
coreutils
cpio
cryptsetup
dhcp-client
diffutils
dosfstools
e2fsprogs
file
findutils
gawk
gdisk
[etc]

If libguestfs were still to depend on these you've not saved any space at all.

See also:

$ rpm -ql supermin-devel
/usr/lib/rpm/fileattrs/supermin.attr
/usr/lib/rpm/supermin-find-requires

(In reply to Richard W.M. Jones from comment #10)
> (In reply to Andrea Bolognani from comment #8)
> > Assuming the above is correct, I will suggest an alternative
> > implementation strategy: replace the current
> > 
> >   Requires: supermin >= 5.1.18
> > 
> > in libguestfs with
> > 
> >   Requires: (supermin >= 5.1.18 or libguestfs-fixed-appliance)
> >
> > This looks like it describes the scenario more accurately: in order
> > for libguestfs to work, you need either supermin (to dynamically
> > create appliances) or a fixed appliance that you've prepared ahead of
> > time. It also avoids introducing a new binary package for libguestfs,
> > and the honestly a bit awkward "libguestfs-noappliance" package name.
> 
> This only solves a fraction of the problem.  The problem isn't really the
> supermin dependency which consumes only 1.5MB of disk.  The problem
> are the automatically added dependencies of all the other packages
> that are copied into the appliance, things like:
> 
> $ rpm -qR libguestfs-appliance 
> acl
> attr
> augeas-libs
> bash
> binutils
> btrfs-progs
> bzip2
> coreutils
> cpio
> cryptsetup
> dhcp-client
> diffutils
> dosfstools
> e2fsprogs
> file
> findutils
> gawk
> gdisk
> [etc]
> 
> If libguestfs were still to depend on these you've not saved any space at
> all.

Right. The kernel is a particularly bad offender here.

> See also:
> 
> $ rpm -ql supermin-devel
> /usr/lib/rpm/fileattrs/supermin.attr
> /usr/lib/rpm/supermin-find-requires

Oh, so you have some RPM-level magic that generates dependencies at
build time based on the contents of supermin.d? That's clever :)

Note that if you were to define libguestfs-runtime as the subset that
doesn't include supermin you'd also leave out the supermin.d files,
so that split would work too.

Will you at least consider renaming the alternative from
libguestfs-noappliance to libguestfs-fixed-appliance? I think the
former is quite confusing.

Also, is there a way to set the equivalent of LIBGUESTFS_PATH through
a configuration file instead of the environment? That would allow the
RPM package containing a fixed appliance to also configure libguestfs
so that it uses it at the same time.

> Right. The kernel is a particularly bad offender here.

Indeed, and since I missed it in the ellipsis above:

$ rpm -qR libguestfs | grep kernel
$ rpm -qR libguestfs-appliance | grep kernel
kernel

So by providing libguestfs-noappliance you should be avoiding
the kernel dependency.

> Will you at least consider renaming the alternative from
> libguestfs-noappliance to libguestfs-fixed-appliance? I think the
> former is quite confusing.

Yeah it's in RHEL now.  I'll see.

> Also, is there a way to set the equivalent of LIBGUESTFS_PATH through
> a configuration file instead of the environment? That would allow the
> RPM package containing a fixed appliance to also configure libguestfs
> so that it uses it at the same time.

This is a good idea.

Verified this bug with package:
libguestfs-1.45.6-11.el9.x86_64

Steps:

1. The installation and upgradation are no problem.
2. The comprehensive rhel9 compose test passed.
The test run: https://polarion.engineering.redhat.com/polarion/#/project/RedHatEnterpriseLinux7/testrun?id=libguestfs-1_45_6-11_el9%20RHEL-9_0_0-20210809_6%20x86_64%202021-08-10%2018-03-55

(In reply to Richard W.M. Jones from comment #12)
> > Will you at least consider renaming the alternative from
> > libguestfs-noappliance to libguestfs-fixed-appliance? I think the
> > former is quite confusing.
> 
> Yeah it's in RHEL now.  I'll see.
> 
> > Also, is there a way to set the equivalent of LIBGUESTFS_PATH through
> > a configuration file instead of the environment? That would allow the
> > RPM package containing a fixed appliance to also configure libguestfs
> > so that it uses it at the same time.
> 
> This is a good idea.

Hi Rich,

any update on these two items?

I don't mean to annoy you, I'm just trying to make sure they don't
fall between the cracks :)

Do you want me to file a separate RFE for the second one? I feel like
it doesn't necessarily belong with this bug, despite being somewhat
related, and also it can be attacked at any time. I'm more concerned
about the name of the new package becoming crystallized and
impossible or simply too bothersome to change later on.

Thanks!

The second one first: It's going to need an RFE because it's
upstream development work to do it.  I'm also not convinced
that using a configuration file is a good idea.  Although we
have one now (/etc/libguestfs-tools.conf) it's neglected.
But yeah, maybe.

I think changing the package name now might require an exception ...

(In reply to Richard W.M. Jones from comment #15)
> The second one first: It's going to need an RFE because it's
> upstream development work to do it.  I'm also not convinced
> that using a configuration file is a good idea.  Although we
> have one now (/etc/libguestfs-tools.conf) it's neglected.
> But yeah, maybe.

Done: Bug 2001639.

> I think changing the package name now might require an exception ...

It's even later now, so I leave it up to you whether it's even worth
pursuing at this point. I still think the current name is somewhat
unfortunate, but then again this is probably going to be a fairly
niche feature so it might not be a big deal either way.

Rich,

I'm working on an update to virtualization packages in KubeVirt[1]
and, as a result of this change, the following packages no longer
need to be included in the guestfs container image:

  attr-0__2.4.48-3.el8.x86_64
  bind-export-libs-32__9.11.36-2.el8.x86_64
  binutils-0__2.30-113.el8.x86_64
  cpio-0__2.12-11.el8.x86_64
  cryptsetup-0__2.3.7-1.el8.x86_64
  dhcp-client-12__4.3.6-47.el8.0.1.x86_64
  dhcp-common-12__4.3.6-47.el8.0.1.x86_64
  dhcp-libs-12__4.3.6-47.el8.0.1.x86_64
  diffutils-0__3.6-6.el8.x86_64
  dnf-0__4.7.0-7.el8.x86_64
  dnf-plugins-core-0__4.0.21-10.el8.x86_64
  dosfstools-0__4.1-6.el8.x86_64
  e2fsprogs-0__1.45.6-3.el8.x86_64
  e2fsprogs-libs-0__1.45.6-3.el8.x86_64
  file-0__5.33-20.el8.x86_64
  file-libs-0__5.33-20.el8.x86_64
  findutils-1__4.6.0-20.el8.x86_64
  gdisk-0__1.0.3-8.el8.x86_64
  genisoimage-0__1.1.11-39.el8.x86_64
  ipcalc-0__0.2.4-4.el8.x86_64
  iputils-0__20180629-9.el8.x86_64
  libreport-filesystem-0__2.9.5-15.el8.x86_64
  libselinux-utils-0__2.9-5.el8.x86_64
  libss-0__1.45.6-3.el8.x86_64
  libusal-0__1.1.11-39.el8.x86_64
  lsscsi-0__0.32-3.el8.x86_64
  mdadm-0__4.2-1.el8.x86_64
  mtools-0__4.0.18-14.el8.x86_64
  parted-0__3.2-39.el8.x86_64
  policycoreutils-0__2.9-18.el8.x86_64
  procps-ng-0__3.3.15-6.el8.x86_64
  scrub-0__2.5.2-16.el8.x86_64
  squashfs-tools-0__4.3-20.el8.x86_64
  supermin-0__5.2.1-1.module_el8.6.0__plus__983__plus__a7505f3f.x86_64
  syslinux-0__6.04-5.el8.x86_64
  syslinux-extlinux-0__6.04-5.el8.x86_64
  syslinux-extlinux-nonlinux-0__6.04-5.el8.x86_64
  syslinux-nonlinux-0__6.04-5.el8.x86_64
  systemd-udev-0__239-58.el8.x86_64
  tar-2__1.30-5.el8.x86_64

This is great news, as it significantly reduces the size of the
container image :) So, for the most part, things are working exactly
as intended.

We have however noticed that some of these packages are still used at
runtime by libguestfs, even in the noappliance scenario. More
specifically, I have found "file" to be such a package while testing
basic usage, and after digging deeper Alice suggested[2] that at
least "findutils" needs to be present as well.

I think we need to add such runtime dependencies explicitly to the
main package. Before the appliance split, this was not necessary
because whether a package was needed just to build the appliance or
also to use it didn't make any difference because it would be
installed anyway; now that the noappliance scenario is supported, we
have to spell out the latter.

Is my understanding of the situation roughly correct? Please let me
know if I have gotten anything wrong!


[1] https://github.com/kubevirt/kubevirt/pull/7277
[2] https://github.com/kubevirt/kubevirt/pull/7277#issuecomment-1050629506

We actually stopped using "file" upstream[1], but I suspect you're
correct that there are others that we miss out.  I cannot see where
we are using findutils (did you mean coreutils?).  From a quick look
at the library I think we need to make sure the main spec file
specifies:

qemu-img
supermin
(uml_mkcow - not used in RHEL)
(umask - not used in RHEL >= 8)
cp
ls
tar
grep
rm
touch
guestunmount

[1] https://github.com/libguestfs/libguestfs/commit/278d0d3226f4bdb7c6586986ca46d0a25c976fe4

Fedora change: https://src.fedoraproject.org/rpms/libguestfs/c/419bada2844bf2dc8e0200b858a0d0ca77964f45?branch=rawhide

Andrea, I suggested keeping findutils for the KubeVirt container but it could be that libguestfs doesn't strictly need it

(In reply to Richard W.M. Jones from comment #20)
> We actually stopped using "file" upstream[1]

That change is in 1.45.4, and the version currently in CentOS Stream
8 is 1.44.0. So while it's great to know that we'll be able to drop
yet another dependency in the future, for the time being we still
have to add this manually.

> From a quick look
> at the library I think we need to make sure the main spec file
> specifies:
> 
> qemu-img
> supermin
> (uml_mkcow - not used in RHEL)
> (umask - not used in RHEL >= 8)
> cp
> ls
> tar
> grep
> rm
> touch
> guestunmount

From the list above it looks like we only need to add tar, which we
would have needed anyway because it's used by the wrapper script to
unpack the prebuilt appliance.

(In reply to Richard W.M. Jones from comment #21)
> Fedora change:
> https://src.fedoraproject.org/rpms/libguestfs/c/419bada2844bf2dc8e0200b858a0d0ca77964f45?branch=rawhide

Great stuff! Can I expect this to also land in CentOS Stream 9 at
some point? What about CentOS Stream 8? Note that I don't really need
either as the workaround in KubeVirt is simple enough so it's not a
big deal to keep it there for a while :)

(In reply to Alice Frosi from comment #22)
> Andrea, I suggested keeping findutils for the KubeVirt container but it
> could be that libguestfs doesn't strictly need it

Since it doesn't look like it's needed based on the list Rich
produced, I'm going to leave it out after all.

I'm going to synchronise the spec files in RHEL 9.1 (bug 2059285),
which should happen in the next few weeks.  Mainly waiting for a good
time to do an upstream stable release (1.48).

I mean: synchronise the spec files with Fedora.