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

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,

On 04/02/2015 01:20 PM, Daniel Henrique Barboza wrote:
>
>
> 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.
I've changed tests/test-harder.sh and now make checks passes it 8/8 in my
ibm_powerkvm test system:

=========================================Testsuite summary for supermin 5.1.12
=========================================# TOTAL: 8
# PASS:  8
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
=========================================
I've double checked 'supermin --list-drivers' to see if the
detection
was working as
intended and it is.

The patch that adds ibm-powerkvm in test-harder.sh is attached in this 
email.


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

On Thursday 02 April 2015 19:10:39 Daniel Henrique Barboza
wrote:
> 
> On 04/02/2015 01:20 PM, Daniel Henrique Barboza wrote:
> >
> >
> > 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.
> 
> I've changed tests/test-harder.sh and now make checks passes it 8/8 in
my
> ibm_powerkvm test system:
> 
> =========================================> Testsuite summary for
supermin 5.1.12
> =========================================> # TOTAL: 8
> # PASS:  8
> # SKIP:  0
> # XFAIL: 0
> # FAIL:  0
> # XPASS: 0
> # ERROR: 0
> =========================================> 
> I've double checked 'supermin --list-drivers' to see if the
detection
> was working as
> intended and it is.
> 
> The patch that adds ibm-powerkvm in test-harder.sh is attached in this 
> email.
Looks good to me, pushed.

Thanks,
-- 
Pino Toscano