On 02/23/2012 04:23 PM, tangchen wrote:
> Hi~
>
> Thanks for the comments.:)
>
> In my optinion, the functions in VM class should act exactly the same as 
> virsh command.
> All the check work should be done in test cases.
As I said, It's okay if a caller/test case will do these checkpoint.

However, the patches set are better to instead of just to wrapper some 
virsh commands
separately, IMHO, they should be committed together like a unit test, 
for instance,

1. wrapper virsh cmdline with internal API
2. implement test cases to verify the internal API
     2.1 add test cases in configuration file
     2.2 add test cases for the interanl API

Regards,
Alex
> We are also doing libvirt test recently. In our {at,de}tach_device tests, 
> just like you said,
> we also login VM and do some more checks. But we put all this checks in our 
> test cases, not in
> class VM.
>
> Maybe virsh command returns 0, and class VM API returns True. But this 
> doesn't mean that the test
> passes. In the test cases, if we find something wrong, the test should fail, 
> and we can still
> find out what is wrong.
>
> And, class VM API should act exactly the same as virsh command. So that if we 
> are sure our test
> case is correct, but the result is FAIL, then we find a bug.
>
> Thanks. :)
>
> On 02/23/2012 03:53 PM, Alex Jia wrote:
>> On 02/23/2012 01:42 PM, tangchen wrote:
>>> This patch adds 2 functions to libvirt_vm.
>>>       1) attach-device()
>>>       2) detach-device()
>>>
>> If each test cases just check virsh command return value, in other words,
>> it only wrappers virsh command in autotest, I assume you haven't other
>> function will call the virsh_{at,de}tach_device then enhance checkpoint
>> in caller, if so, you will push check work to qemu/kvm, I'm not sure it's a
>> good idea, for example, if libvirt makes a mistake, a actual result is 
>> failed,
>> but virsh cmdline return 0, how to debug it?
>>
>> We do the same test on libvirt, as usual, we will check pci device driver 
>> change,
>> for instance, if attaching device is successful, the pci device will be 
>> separated
>> from original driver and put into pci-stub driver for kvm hypervisor, 
>> meanwhile,
>> we also log in guest to check whether the pci device works well etc.
>>
>> Regards,
>> Alex
>>> Signed-off-by: Tang Chen<[email protected]>
>>> ---
>>>    client/virt/libvirt_vm.py |   34 ++++++++++++++++++++++++++++++++++
>>>    1 files changed, 34 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py
>>> index 4b391c1..d7eb7b0 100644
>>> --- a/client/virt/libvirt_vm.py
>>> +++ b/client/virt/libvirt_vm.py
>>> @@ -374,6 +374,29 @@ def virsh_migrate(options, name, dest_uri, extra, uri 
>>> = ""):
>>>            return False
>>>        return True
>>>
>>> +def virsh_attach_device(name, xml_file, extra = "", uri = ""):
>>> +    """
>>> +    Attach a device to VM.
>>> +    """
>>> +    cmd = "attach-device --domain %s --file %s %s" % (name, xml_file, 
>>> extra)
>>> +    try:
>>> +        virsh_cmd(cmd, uri)
>>> +        return True
>>> +    except error.CmdError:
>>> +        logging.error("Attaching device to VM %s failed." % name)
>>> +        return False
>>> +
>>> +def virsh_detach_device(name, xml_file, extra = "", uri = ""):
>>> +    """
>>> +    Detach a device from VM.
>>> +    """
>>> +    cmd = "detach-device --domain %s --file %s %s" % (name, xml_file, 
>>> extra)
>>> +    try:
>>> +        virsh_cmd(cmd, uri)
>>> +        return True
>>> +    except error.CmdError:
>>> +        logging.error("Detaching device from VM %s failed." % name)
>>> +        return False
>>>
>>>    class VM(virt_vm.BaseVM):
>>>        """
>>> @@ -1126,6 +1149,17 @@ class VM(virt_vm.BaseVM):
>>>                self.connect_uri = dest_uri
>>>            return result
>>>
>>> +    def attach_device(self, xml_file, extra = ""):
>>> +        """
>>> +        Attach a device to VM.
>>> +        """
>>> +        return virsh_attach_device(self.name, xml_file, extra, 
>>> self.connect_uri)
>>> +
>>> +    def detach_device(self, xml_file, extra = ""):
>>> +        """
>>> +        Detach a device from VM.
>>> +        """
>>> +        return virsh_detach_device(self.name, xml_file, extra, 
>>> self.connect_uri)
>>>
>>>        def destroy(self, gracefully=True, free_mac_addresses=True):
>>>            """
>>

_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to