Thanks for adding this into libvirt code, this is much needed 
functionality.   It will come in handy for many other libvirt tests too.

On 05/24/2012 03:47 AM, tangchen wrote:
> Signed-off-by: Tang Chen<[email protected]>
> ---
>   client/virt/libvirt_vm.py |   30 ++++++++++++++++++++++++++++++
>   1 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py
> index ce594af..4a1409c 100644
> --- a/client/virt/libvirt_vm.py
> +++ b/client/virt/libvirt_vm.py
> @@ -188,6 +188,27 @@ def virsh_dumpxml(name, uri = ""):
>       return virsh_cmd("dumpxml %s" % name, uri)
>
>
> +def dumpxml_to_local_file(name, file_path, uri=""):
> +    """
> +    Dump the guest's xml to a file on localhost.
> +    @param name: VM name
> +    @param file_path: The full path to new xml file.
> +                      If the file exists, it will be cleaned.
> +    """
> +    try:
> +        f = open(file_path, 'w')
> +        xml = virsh_dumpxml(name, uri)
> +        f.write(xml)
> +        f.close()
> +        return True
> +    except error.CmdError, detail:
> +        logging.error("Failed to dump xmlfile of %s to %s.\n%s",
> +                      (name, file_path, detail))
> +        if not f.closed:
> +            f.close()
> +        return False

I think this dumpxml_to_local_file() code can be pushed up into 
vm.backup_xml().  It's so useful a function in connection with a 
particular vm instance, it's okay to make it a method.

If code needs to dump another vm by name, it can just call 
virsh_dumpxml() on it's own.  However, the code you have written here 
looks good, and it's okay to catch the error.CmdError exception like this.

This is different case from VM migration test, where it is much more 
complex and there are many different ways it can fail.  So I think if we 
ever need to do dumpxml error testing, we can use True/False return 
since operation is so simplistic.  i.e. there are only very small number 
of reasons dumpxml can fail.

So I think this approach here is fine.  Though if you want to change it 
to just pass error.CmdError up, I would be fine with that too.

> +
>   def virsh_is_alive(name, uri = ""):
>       """
>       Return True if the domain is started/alive.
> @@ -689,6 +710,15 @@ class VM(virt_vm.BaseVM):
>           return virsh_dumpxml(self.name, self.connect_uri)
>
>
> +    def backup_xml(self, file_path):
> +        """
> +        Backup the guest's xmlfile.
> +
> +        @param file_path: Full path to the backup file.
> +        """
> +        return dumpxml_to_local_file(self.name, file_path, self.connect_uri)
> +
> +
>       def clone(self, name=None, params=None, root_dir=None, 
> address_cache=None,
>                 copy_state=False):
>           """


-- 
Chris Evich, RHCA, RHCE, RHCDS, RHCSS
Quality Assurance Engineer
e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to