Re: [Libguestfs] [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/b2b0f29efb537161df0286f4a9d... >> 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,