Re: [Libvirt-cim] [PATCH] [TEST] Adding new tc to verify Libvirt-CIM profile advertisement via slp

>> + cmd = "%s findsrvs service:wbem | grep -w http | "\ >> + "grep -e '%s'" % (slp_path, server) >> + rc, out = run_remote(server, cmd) >> + if rc != 0: >> + logger.error("Failed to find slp service on '%s'", server) >> + return status, slp_attrs > > Instead of calling findsrvs, why not call findattrs directly? I call findsrvs to get all the ports which could be used to find the attrs.

> >> + >> + attrs_in = out.rsplit(",") >> + slp_query = attrs_in[0] >> + cmd = "%s findattrs %s" % (slp_path, slp_query) >> + rc, slp_attrs = run_remote(server, cmd) >> + if len(slp_attrs) == 0: >> + # Getting the query and the port details >> + slp_query_info = slp_query.rsplit(":", 1) >> + slp_service_query = slp_query_info[0] >> + port_used = slp_query_info[1] >> + logger.info("Did not get the profile information for '%s'", slp_query) >> + logger.info("Trying with ports other than '%s'....", port_used) >> + >> + # Remove the already used port information + ports = >> attrs_in.remove(slp_query) >> + for port in attrs_in: >> + new_slp_query = slp_service_query + ":" + port >> + cmd = "%s findattrs %s" % (slp_path, new_slp_query) >> + rc, slp_attrs = run_remote(server, cmd) >> + if len(slp_attrs) != 0: + break > > If I force the test to execute this block of code, I get the following > error: > > ERROR - ValueError : list.remove(x): x not in list > Traceback (most recent call last): > File "./lib/XenKvmLib/const.py", line 140, in do_try > File "04_verify_libvirt_cim_slp_profiles.py", line 167, in main > status, slp_attrs = get_slp_attrs(ip_addr) > File "04_verify_libvirt_cim_slp_profiles.py", line 96, in get_slp_attrs > ports = attrs_in.remove(slp_query) > ValueError: list.remove(x): x not in list > > I think it's okay to remove this bit of code. We have to enable http > connections with the CIMOM in order to run cimtest. If we want cimtest > to work with just https, we'd need to do a lot of cleanup. That means > if the CIMOM is advertising via slp correctly, then the advertisement > will occur with the http port (otherwise its an error with the > advertisement). > > You can put a comment here indicating the test only works when the > CIMOM is enabled for http. That's fine for now. > I am not using https at all. I am parsing the port associated with the http record returned by the slptool findsrvs.. Let me know if I am wrong in understanding what you mean here.

> >> + >> + if len(slp_attrs) != 0: >> + status = PASS >> + >> + return status, slp_attrs >> + >> +def filter_reg_name_from_slp(slp_attrs): >> + slp_profile_list = [] >> + >> + for line in slp_attrs.split('\n'): >> + lines=line.split("RegisteredProfilesSupported") > > Spaces around the = needed here. > >> + dmtf_profiles=lines[1].split("DMTF") > > And here. > > >> + >> +@do_main(sup_types) >> +def main(): >> + options = main.options >> + server = options.ip >> + virt = options.virt >> + status = FAIL >> + >> + # Making sure that the server information passed is + # hostname or >> ip address >> + if server == "localhost": >> + logger.error("Please specify hostname or ip address, than '%s'", + >> server) >> + return SKIP > > Why not use gethostname() to get the hostname? > >> + + status = get_slp_info(server) >> + if status != PASS: >> + return status + >> + ip_addr = gethostbyaddr(server)[2][0] > > You can use gethostbyname(), which is a little cleaner. You can pass > it the output from gethostname(). > I tried supplying the hostname to the slp command, but it does not seem to work. Hence I choose gethostbyaddr() to get the ip from the hostname. Let me know if we can access the attrs while passing the hostname.

>> + status, slp_attrs = get_slp_attrs(ip_addr) >> + if status != PASS: >> + logger.error("Failed to get slp attributes on %s", server) >> + return status >> + >> + slp_profile_list = filter_reg_name_from_slp(slp_attrs) >> + >> + status, libvirt_cim_reg_list = get_libvirt_cim_profile_info(server, >> virt) >> + if status != PASS: >> + logger.error("Failed to enumerate profile information on %s", server) >> + return status >> + >> + # Make sure all the Libvirt-CIM profiles are advertised via slp >> + if (libvirt_cim_reg_list) <= (slp_profile_list): >> + logger.info("Successfully verified the Libvirt-CIM profiles") >> + return PASS >> + else: > > No need for the else here. Don't we want to print an error message if we dont get the expected Libvirt-CIM profiles ?

> >> + logger.error("Mismatch in the profiles registered") >> + logger.error("Slp returned profile --> %s,\n Libvirt-CIM expected " >> + "profiles %s", slp_profile_list, libvirt_cim_reg_list) >> + return FAIL >> + >> +if __name__=="__main__": >> + sys.exit(main()) >> + >> >