[Libguestfs] [PATCH] Adding ibm-powerkvm distro detection (the right one)

[Pino Toscano]

On Thursday 02 April 2015 11:15:07 Daniel Henrique Barboza wrote:
> Hi Pino,
> 
> On 04/02/2015 09:51 AM, Pino Toscano wrote:
> > Hi Daniel,
> >
> > On Thursday 02 April 2015 09:34:17 Daniel Henrique Barboza wrote:
> >> On 04/02/2015 05:23 AM, Pino Toscano wrote:
> >>> Hi Daniel,
> >>>
> >>> On Wednesday 01 April 2015 16:37:26 Daniel Henrique Barboza wrote:
> >>>> The one that got upstream does not work in ibm-powerkvm due to the
> >>>> rpm_is_avaiable verification
> >>>> in the detection (I've attached the wrong version in bugzilla).
> >>> the new version of the patch is somehow confusing. supermin >= 5.1.12
> >>> uses librpm to query for rpm dependencies, file listing, provides, etc.
> >>> If rpm_is_available returns false, that means you built without librpm,
> >>> and that supermin will not really work. Did you tried running the test
> >>> suite (`make check`)?
> >>>
> >> These are the results of make check in the system I've used to test the
> >> patch. The system is
> >> in an internal isolated network, thus I believe some failures were
> >> expected to happen.
> >>
> >>
> >> PASS: test-basic.sh
> >> PASS: test-execstack.sh
> >> FAIL: test-build-bash.sh
> >> FAIL: test-binaries-exist.sh
> >> SKIP: test-harder.sh
> >> FAIL: test-build-bash-network.sh
> >> FAIL: test-binaries-exist-network.sh
> >> SKIP: test-harder-network.sh
> >> make[4]: Entering directory `/root/supermin/tests'
> >> make[4]: Nothing to be done for `all'.
> >> make[4]: Leaving directory `/root/supermin/tests'
> >> ============================================================================
> >> Testsuite summary for supermin 5.1.12
> >> ============================================================================
> >> # TOTAL: 8
> >> # PASS:  2
> >> # SKIP:  2
> >> # XFAIL: 0
> >> # FAIL:  4
> >> # XPASS: 0
> >> # ERROR: 0
> >>
> >> I've run the non-related network tests to see the failure cause. It is
> >> worth saying that supermin
> >> upstream code builds and runs fine as far as I can tell, thus I couldn't
> >> figure it out much reading
> >> these errors:
> >>
> >> # ./test-build-bash.sh
> >> ./test-build-bash.sh: line 34: 83872 Aborted ../src/supermin -v
> >> --prepare $USE_INSTALLED bash -o $d1
> >> # ./test-binaries-exist.sh
> >> ./test-binaries-exist.sh: line 29: 83886 Aborted ../src/supermin -v
> >> --prepare $USE_INSTALLED bash coreutils -o $d1
> > These errors tells me exactly what I was talking about: you are
> > building supermin on a rpm-based distribution without librpm support,
> > meaning that supermin is basically non-functional.
> >
> > The abort() come from src/librpm-c.c, in the else part of the
> > #ifdef HAVE_LIBRPM.
> >
> >> I will be honest and say that I didn't dig further understanding why the
> >> rpm_is_available
> >> check fails in ibm-powerkvm. The patch I sent is similar to an internal
> >> patch we used to add
> >> temporary support to the distro on a older version of supermin (the
> >> version shipped
> >> with RHEL 7.1 GA).
> >>
> >> I assumed that rpm_is_available failed because ibm-powerkvm does not
> >> have all the rpm features
> >> fedora and rhel have, although it uses rpm/yum. Perhaps in a later
> >> release of the OS, using
> >> a newer version of supermin, we should review this code and enhance it.
> > As I said, I'm pretty sure it's because you are building without
> > librpm, and you should have got in configure's output something like:
> >
> >    checking for LIBRPM... no
> >
> > Please try the following:
> > - install rpm-devel (being a Fedora-based distro, you should have it)
> > - get supermin 5.1.12
> > - apply your ibm-powerkvm patch
> > - readd the rpm_is_available check (basically reverting [1])
> > - run ./configure and check that you have "LIBRPM... yes"
> > - build and run the test suite
> >
> > If things work, you should get 6 tests passing and 2 skipped (we can
> > make these 2 working as well later).
> >
> > [1] https://github.com/libguestfs/supermin/commit/b2b0f29efb537161df0286f4a9d5cfe94206be18
> 
> You are right. I've executed all the steps you mentioned and make check 
> now passes
> (6 pass 2 skipped) and the original patch now detects ibm-powerkvm as well.

Nice to hear that. I've just reverted upstream the aforementioned
patch, following your confirmation.

Regarding the skipped test (the two occurrence are the same test, just
run in local-only and network mode): would it be possible to extend it
to recognize the ibm-powerkvm distro?
Basically it boils down to:
- set the proper "distro" variable, which in your case would be checking
  for /etc/ibm_powerkvm-release
- set few packages in "pkgs", possibly taking one simple/small with
  epoch (like tar, if ibm_powerkvm is based on fedora/rhel/centos)
- check that there are "key files" (like shared libraries and binaries)
  installed by the specified packages

This test helps checking that the result produced by supermin actually
do contain the files from packages you asked to "install".

Thanks,
-- 
Pino Toscano