[libvirt] [test-API PATCH 5/6] Cleanup and fix of domain/define test

[Guannan Ren]

On 03/27/2012 03:57 PM, Martin Kletzander wrote:
>>           It's better not to use libvirt API to check the result of one
>> another API.
>>           We should use native approach to do the checking. So I insist
>> on the original checking.
>>
>
> There was no lookupByName function, but I agree it's better to use the
> same approach, so I'll add this method to the connection API.
> One more question, whilst on this subject, though. I still didn't get
> why we encapsulate libvirt API into one more class where we don't make
> use of any persistence, inheritance nor any other OOP approach. It would
> help me to understand so I don't make future mistakes.

         I think you don't need to add  lookupByName() in connectAPI.py
     The APIs is domain related, so we suggest to make use of it in 
domainAPI.py
     only. The ideas is that all of hypervisor related APIs go to 
connectAPI.py.
         The relationship between classes domain, network, storage, 
nodedev is
     loose. If we had a virtnetwork subclass, It would be better to make 
virtnetwork inherite
     network for better OOP.
         The encapsulated API files in lib directory is easy to manage 
and use. For example
     If we want to write a domain testcases, we probably don't need to 
import network
     and storage module in lib.


>
>>>    def define(params):
>>>        """Define a domain from xml"""
>>> @@ -169,41 +125,10 @@ def define(params):
>>>        logger = params['logger']
>>>        guestname = params['guestname']
>>>        guesttype = params['guesttype']
>>> +    uri = params['uri']
>>               If uri is not None, we need to get the IP or hostname of
>> target machine from the uri
>>               Use that IP or hostname to perform ssh operation.
>>
> I dropped the part of the code that connected to the machine by ssh in
> order to check for the domain to be created. It was very limited and in
> case any other tests needs the IP of the target machine,

          These are two way to get the IP of target machine for remote 
testing.
          1) The $defaulturi global setting in env.cfg(if patch 5/6, 6/6 
got ACK ),
               then extract the IP or hostname part.
          2) pass in explicitly in case config file like the define.py 
does right now

          we can use either depending on testing requirement.


> there are 2
> ways it can get it:
>   1) have another parameter for the address (as the number of tests that
> could make use of the target IP address will be minimal, I'd prefer this
> option)
>   2) implement URI parsing the same way as it is done in libvirt code
> (libxml2.xmlParseURI or something like that)
>
> I think this is still better than having hardcoded '127.0.0.1' in the
> tests =)
>