On 03/19/2012 12:49 AM, tangchen wrote:
>
> This patch adds 3 API for libvirt_vm.
> 1) vm.define()
> 2) vm.attach_interface()
> 3) vm.detach_interface()
>
> Signed-off-by: Tang Chen<[email protected]>
> ---
> client/virt/libvirt_vm.py | 52
> +++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py
> index 903f03f..9a93880 100644
> --- a/client/virt/libvirt_vm.py
> +++ b/client/virt/libvirt_vm.py
> @@ -431,6 +431,39 @@ def virsh_migrate(options, name, dest_uri, extra, uri =
> ""):
> return False
> return True
>
> +def virsh_attach_interface(name, type, extra = "", uri = ""):
> + """
> + Attach a nic to VM.
> + """
> + cmd = "attach-interface --domain %s " % name
> + if type not in ('bridge', 'network', 'user', 'ethernet', 'direct', \
> + 'hostdev', 'mcast', 'server', 'client', ''):
^ You don't need the \ at the end of the first line. Some other comments:
* Default argument passing is not being done according to PEP8, ie,
you're putting spaces around the default argument;
* The part of the code that checks if interface type is not on the
list of known types can be factored to an auxiliary function. Also, you
can do a better check for empty string;
* You're not honoring the 2 newlines between each function rule of the
coding style.
* Even with separate functions to build the attach command, you don't
need much code duplication.
Summarizing my suggestions for the API, I have the following:
def _check_interface_type(type):
if type and type not in ('bridge', 'network', 'user', 'ethernet',
'direct',
'hostdev', 'mcast', 'server', 'client'):
logging.error("Unknown interface type %s", type)
return False
def _build_run_attach_cmd(cmd):
if not _check_interface_type(type):
return False
if "attach" in cmd:
verb = "attach"
elif "detach" in cmd:
verb = "detach"
if type:
cmd += " --type %s" % type
if extra:
cmd += " %s" % extra
try:
virsh_cmd(cmd, uri)
return True
except error.CmdError, detail:
logging.error(Interface %s on domain %s failed:\n%s",
verb, name, detail)
return False
def virsh_attach_interface(name, type, extra="", uri=""):
"""
Attach a nic to VM.
"""
cmd = "attach-interface --domain %s " % name
return _build_run_attach_cmd(cmd)
def virsh_detach_interface(name, type, extra="", uri=""):
"""
Attach a nic to VM.
"""
cmd = "detach-interface --domain %s " % name
return _build_run_attach_cmd(cmd)
Of course some pylint check and verification is necessary.
> + logging.error("Unknown interface type.")
> + return False
> + cmd += "--type %s %s" % (type, extra)
> + try:
> + virsh_cmd(cmd, uri)
> + return True
> + except error.CmdError, detail:
> + logging.error("Attaching interface to %s failed.\n%s" % (name,
> detail))
> + return False
> +
> +def virsh_detach_interface(name, type, extra = "", uri = ""):
> + """
> + Detach a nic from VM.
> + """
> + cmd = "detach-interface --domain %s " % name
> + if type not in ('bridge', 'network', 'user', 'ethernet', 'direct', \
> + 'hostdev', 'mcast', 'server', 'client', ''):
> + logging.error("Unknown interface type.")
> + return False
> + cmd += "--type %s %s" % (type, extra)
> + try:
> + virsh_cmd(cmd, uri)
> + return True
> + except error.CmdError, detail:
> + logging.error("Detaching interface from %s failed.\n%s" % (name,
> detail))
> + return False
>
> def virsh_attach_device(name, xml_file, extra = "", uri = ""):
> """
> @@ -555,6 +588,14 @@ class VM(virt_vm.BaseVM):
> else:
> return False
>
> + def define(self, path):
> + """
> + Define the VM.
> + """
> + if not os.path.exists(path):
> + logging.error("File %s not found." % path)
> + return False
> + return virsh_define(path, self.connect_uri)
>
> def undefine(self):
> """
> @@ -1212,6 +1253,17 @@ class VM(virt_vm.BaseVM):
> self.connect_uri = dest_uri
> return result
>
> + def attach_interface(self, type, extra = ""):
> + """
> + Attach a nic to VM.
> + """
> + return virsh_attach_interface(self.name, type, extra,
> self.connect_uri)
> +
> + def detach_interface(self, type, extra = ""):
> + """
> + Detach a nic from VM.
> + """
> + return virsh_detach_interface(self.name, type, extra,
> self.connect_uri)
>
> def attach_device(self, xml_file, extra = ""):
> """
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
