Libguestfs - Apr 2015 - Re: [PATCH] Adding ibm-powerkvm distro detection (the right one)

Hi Pino,


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


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.


Daniel

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

-- 
Pino Toscano

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.

Trying to find out how I managed to not do something simple as intalling 
rpm-devel, the
reasoning is that I've run yum-builddep with an older supermin.spec file 
prior to building
supermin upstream code. This spec file I've used didn't include librpm 
as a build
dependency and the project compiled without errors.