On 05/24/2012 10:30 PM, Chris Evich wrote:
> 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<tangc...@cn.fujitsu.com>
>> ---
>>    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.
>
IMHO, we should reuse virsh_dumpxml() then give a optional 'to_file'
parameter rather than defining many similar function, for example,

def virsh_dumpxml(name, to_file="",  uri="")
      if to_file:
          cmd = "dumpxml %s>  %s" % (name, to_file)
      else:
          cmd = "dumpxml %s" % name

      return virsh_cmd(cmd, uri)


And then I saw each virsh cmd need to deal with exception case,
we should do it in virsh_cmd() to simplify codes.

>> +
>>    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):
>>            """
>

_______________________________________________
Autotest mailing list
Autotest@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to