[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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
  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 '' in the
tests =)

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]