On 03/27/2012 05:42 PM, Martin Kletzander wrote:
On 03/27/2012 10:59 AM, Guannan Ren wrote:
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.
We both probably talk about something else, let me clarify.
My apologies first, because I misunderstood that you wanted to use the
native approach. I thought you meant implementing the function in
connectAPI whether you meant checking on the machine without using
libvirt. That said, there is no point in talking about lookupByName
implementation for now.
About the actual checking if the domain was created. Leaving it as it is
now, any misconfiguration will make this check fail (meaning the
configuration of sshd, libvirt, etc.). It will work for now as we are
the only ones using that right now (I guess), I'm just trying to think
ahead and I don't see that big problem with checking the domain being
created using libvirt again, but that's just my opinion.
About the re-implementation of the API, I was just looking into the
connectAPI class for now, but what I see there is only constructor that
saves the connection from libvirt and then for each method of libvirts
connection, there is connectAPI method that does the same, just using
different method names (and raising different exception). Unfortunately
these aren't even consistent. For example:
libvirts openAuth is encapsulated as openAuth
libvirts isEncrypted is encapsulated as isEncrypted
but
libvirts getCapabilities is encapsulated as get_caps
libvirts getHostname is encapsulated as get_host_name
and so on.
Don't get me wrong, I'm not trying to complain or anything, I'm just
trying to understand because for me it doesn't make any sense to use
this class.
Ok, let me keep it temporarily, because it doesn't affect testing.
After some practice, it proves to be useless, we can remove them
anytime.
Is that okay?
I missed few other answers in my previous mail, so just to inform you
that I acknowledge them:
try ... except ...finally is new syntax since 2.5,
But we need to support 2.4, so should use
try:
try:
except:
finally
I didn't know that, good point, Python 2.4 is still used somewhere probably.
- if target_machine:
- REMOVE_SSH = "ssh %s \"rm -rf /root/.ssh/*\"" % (target_machine)
- logger.info("remove ssh key on remote machine")
- status, ret = util.exec_cmd(REMOVE_SSH, shell=True)
- if status:
- logger.error("failed to remove ssh key")
-
- REMOVE_LOCAL_SSH = "rm -rf /root/.ssh/*"
Never remove or change the local ssh directory like this.
The autotest have ssh configuration file stored here.
This is a code that what was already in the test before, however I
probably copy-pasted it into the utils as it is. Better approach would
definitely be generating the keys somewhere, then pasting them into ssh
parameter '-i' and backing up the keys on the remote, while putting them
back after the test. However the _best_ option in this case is not using
keys (we have to use pexpect and connect there with password anyway) at all.
Yep, I just want to give a hint about that autotest view ssh
as critical thing.
It's my hint. no comment on you.
I agree with your ideas.
If we agree on need for this, then we can do that, however since we got
into the deep conversation how to do what, I'd rather wait till more
things are decided.
Thanks for the patience,
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list