Re: [Libvirt-cim] [RFC] Patches need no testcase

On 01/16/2013 12:46 AM, Wenchao Xia wrote: First off - let me introduce myself... I'm (relatively) new to Red Hat having joined in Dec 2012 from HP where I worked on Virtualization products at HP. Primarily I was responsible for the API/CLI space of the HPVM product. I also was part of a project that created what HP calls VirtProvider which was a DSP2013 CIM Provider. Since I knew how to spell CIM and had some history, I was asked to take a look at failures that Yanbing Du posted questions about back in October: https://www.redhat.com/archives/libvirt-cim/2012-October/msg00000.html Sorry in advance for the length of the rest of this... While this won't be a full time endeavour, putting some cycles on libvirt-cim has been added to my list of things to do. I still need a bit of help - I'm not quite sure of the "proper procedure" to use when testing a local libvirt-cim build. The only way I was able to make things work was remove the libvirt-cim package (rpm -e) and then reinstall (yum localinstall from build rpm in a 'make rpm'). I tried the install/postinstall process with stopping/starting tog-pegasus, but couldn't quite get things to work. Getting "up to speed" was a bit of a challenge as the libvirt CIM web pages seem to be a bit out of date and perhaps assume a few things along the way. I've taken some notes, but was eventually able to get things installed. I'm still working through some of the "kinks". Setting up a CIMserver with tog_pegasus was not something I did at HP, so it was a bit of an adventure. Eventually I worked my way through things with lots of trial and error. Following the README instructions I ran into two issues during the post installation phase, which I'm somewhat curious if long time users would also hit (not that many are active any more): #1: The commit id '22022870d' to Makefile.am which essentially added "$(top_srcdir)/" in front of the "schema" directory path caused the 'make postinstall' to fail (at least on my fedora18 system). The problem is that the path ends up as ".//usr/..." the "./" being the problem with respect to the ability to "find" the files. Stripping that off allows finding files to be found again. The fix is to modify the various "$(subst schema" and replace them with "$(subst $(top_srcdir)/schema" - this allowed me to continue. #2: After that was solved, as things were scrolling past I noted that there was some sort of issue with "schema/SwitchService.registration". In looking at the file, it seems the blank line at the end of the file caused some sort of "issue", so I removed it and the registration was happier. After those changes I was able to run through the 'cimtest' using the most recent libvirt (what is looking to be 1.0.3). I didn't see all the same errors, but did trip across a few. Figuring that perhaps some of those were fixed by patches that I knew were out there, I went back to start looking, applying, and running cimtest again for tests that failed for me. I downloaded and applied the Dec 2012 patches for libvirt-cim (v3 01 -> 11 and v4 DevicePool) and libcmpiutil (v3 1&2) as well as the Nov 2012 patch submitted by Jirka. I believe that gets me "up to date". Although it was a bit confusing as what I should have done for the v4 patches on 12/25/12 was to "replace" the 12/18/12 patches 6 and 10. Luckily it was a quick retrace of my steps. Unfortunately it didn't seem I was "using" the changes when I ran "make", "make preinstall", "make install", & "make postinstall". So, I figured I'd try to make a new distribution to see if that would work. I got a bit further; however, the 'make distcheck' failed for me much in the same way the 'make postinstall' did above. So I adjusted commit id '9f5e204f' to rather than "$(subst ./schema," use "$(subst $(top_srcdir)/schema" which resolve the issue I was running into. Once I did the 'rpm -e libvirt-cim', I was able to install the rpm I build and test the changes. The (partial) good news is between the fixes provided in the December patches and some of my own I can get a majority of cimtest to succeed. I've had to make other adjustments, but I figured that'd be par for the course anyway. So some comments about your changes. Now that I have a little more experience I'll look at the other changes more in depth, but these two popped out due to me running into "issues" with them. Patch 01/11: * The change for 'bridge' networks and not allowing scripts is only for qemu domains. Apparently Xen domains would still support the script. So while your change does effectively remove it for Qemu, doing so for Xen could be a regression (I have no way to test). See the following for my logic: http://www.redhat.com/archives/libvir-list/2012-January/msg00240.html Before I had applied your patches I had made a similar change except that I passed along the dominfo->type to the bridge_net_to_xml() function and then test in there whether or not the "script" should be added based on type of DOMAIN_XENPV or DOMAIN_XENFV. Patch 09/11 * This change caused the 'make distcheck" to fail. The 'wait_for_event()' routine has a "ret" variable that is unused. Some changes I've made to 'cimtest' in order to get things to work * The test makes use of setting a MAC in quite a few places, unfortunately the MAC chosen for most (99:aa:bb:cc:ee:ff) is a Multicast Address. We know this because the low bit of the first byte (e.g. "99") is set (in hex 0x10011001). Changing those references to "88" works. * ComputerSystemIndication and RASDIndications fail for me. After a long pause, I got a Python stack trace where it looks like it's trying to print testsuite results. What that turns out to be is the 'os_status' being passed along is the errno value for 'ETIMEDOUT' (eg. 110). Using 110 to index into the rc[] record in Reporter.py causes a KeyError exception. I "fixed" that by checking if os_status was outside of the valid range and if so, set it to FAIL, but log that I did so. * Profile/04_verify_libvirt_cim_slp_profiles.py has a bug. It only checks for "slp" when it should check "slp=true". * Looking for the NFS Server at /sbin/init.d/nfs doesn't work any more with the usage of systemd in order to start/stop daemons. I adjusted the code accordingly * The ResourcePoolConfigurationService tests 08, 09, & 15 try to create a pool pointing at /var/lib/libvirt/images which causes a failure because cimtest-diskpool already exists and uses it. Not sure if this is a new check or not (remember I haven't been here that long). My current results have 173 of 192 tests passing. There are 4 FAIL, 4 XFAIL, and 11 SKIP (mostly migration which I haven't figured out yet). 2 of the SKIP's are because I already had a domains and storage pools defined on my system. John