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

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.

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

On 04/02/2015 11:47 AM, Pino Toscano wrote:
> 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?
I'll try it for sure. I am having problems accessing the system at this
moment (lab downtime/maintenance) but as soon as I am able I'll try
to follow your checklist and see if I can make the skipped test work
with ibm-powerkvm too.

I'll update this thread when I have news about it.


Thanks!

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