Re: [Autotest] [PATCH] KVM test: Update pci_hotplug to suit for new qemu

[Yiqiao Pu <ypu@redhat.com>]

  On 08/27/2010 11:17 AM, Lucas Meneghel Rodrigues wrote:
> On Fri, 2010-08-06 at 10:23 +0800, Yiqiao Pu wrote:
>> In the new version of qemu in RHEL 6.0 device_add is instead of pci_add. So
>> modify the scripts to suit for the new version of qemu.
>> There is still one problem here. Block device htoplug only supported in qmp
>> command line, but qmp don't implement pci query in the current version which
>> is needed by the verification of device hotplug. So the block device
>> hotplug will report a failed rignt now.
>>
>> Signed-off-by: Yiqiao Pu<ypu@redhat.com>
>> ---
>>   client/tests/kvm/tests/pci_hotplug.py |   64 ++++++++++++++++++++++++---------
>>   1 files changed, 47 insertions(+), 17 deletions(-)
>>
>> diff --git a/client/tests/kvm/tests/pci_hotplug.py b/client/tests/kvm/tests/pci_hotplug.py
>> index 2c459d7..5202cf6 100644
>> --- a/client/tests/kvm/tests/pci_hotplug.py
>> +++ b/client/tests/kvm/tests/pci_hotplug.py
>> @@ -1,4 +1,4 @@
>> -import logging, os
>> +import logging, os, commands, re
>>   from autotest_lib.client.common_lib import error
>>   import kvm_subprocess, kvm_test_utils, kvm_utils, kvm_vm
>>
>> @@ -35,26 +35,56 @@ def run_pci_hotplug(test, params, env):
>>
>>       tested_model = params.get("pci_model")
>>       test_type = params.get("pci_type")
>> -
>> -    if test_type == "nic":
>> -        pci_add_cmd = "pci_add pci_addr=auto nic model=%s" % tested_model
>> -    elif test_type == "block":
>> -        image_params = kvm_utils.get_sub_dict(params, "stg")
>> -        image_filename = kvm_vm.get_image_filename(image_params, test.bindir)
>> -        pci_add_cmd = ("pci_add pci_addr=auto storage file=%s,if=%s" %
>> -                       (image_filename, tested_model))
>> -
>> -    # Execute pci_add (should be replaced by a proper monitor method call)
>> -    add_output = vm.monitor.cmd(pci_add_cmd)
>> -    if not "OK domain" in add_output:
>> -        raise error.TestFail("Add device failed. Hypervisor command is: %s. "
>> -                             "Output: %r" % (pci_add_cmd, add_output))
>> -    after_add = vm.monitor.info("pci")
>> +    s, o = commands.getstatusoutput("uname -r")
>> +    if len(re.findall("el6", o))>  0:
> ^ This check is not good for upstream code, after all we might be
> running on qemu-kvm upstream, generic versions that already support the
> new syntax. Asking the monitor the set of supported commands seems the
> most sensible solution here. Please resolve this issue and re-send the
> patch.
>
>> +        cmd_type = "device"
>> +    else:
>> +        cmd_type = "pci"
>> +
>> +    if cmd_type == "pci":
>> +        if test_type == "nic":
>> +            pci_add_cmd = "pci_add pci_addr=auto nic model=%s" % tested_model
>> +        elif test_type == "block":
>> +            image_params = kvm_utils.get_sub_dict(params, "stg")
>> +            image_filename = kvm_vm.get_image_filename(image_params,
>> +                                                        test.bindir)
>> +            pci_add_cmd = ("pci_add pci_addr=auto storage file=%s,if=%s" %
>> +                           (image_filename, tested_model))
>> +        # Execute pci_add (should be replaced by a proper monitor method call)
>> +        add_output = vm.monitor.cmd(pci_add_cmd)
> ^ You might want to wrap pci device addition on a new monitor method, as
> the TODO comment on this above line suggests. This comment was made by
> Michael on his monitor patchset, and since you are working on this test,
> it's a good opportunity to get this implemented.
>
>> +        if not "OK domain" in add_output:
>> +            raise error.TestFail("Add device failed. Hypervisor command is: %s"
>> +                                  ". Output: %r" % (pci_add_cmd, add_output))
>> +        after_add = vm.monitor.info("pci")
>> +    elif cmd_type == "device":
>> +        # block device hotplug is not support rignt now
> ^ Typo, "supported"
>
>> +        id = test_type + "-" + kvm_utils.generate_random_string(4)
>> +        if test_type == "nic":
>> +            if tested_model == "virtio":
>> +                tested_model = "virtio-net-pci"
>> +            pci_add_cmd = "device_add id=%s,driver=%s" % (id, tested_model)
>> +        elif test_type == "block":
>> +            if tested_model == "virtio":
>> +                tested_model = "virtio-blk-pci"
>> +            if tested_model == "scsi":
>> +                tested_model = "scsi-disk"
>> +            pci_add_cmd = "device_add %s,id=%s,driver=%s" % (image_filename,
>> +                           id, tested_model)
>> +
>> +            raise error.TestError("Do not support in current version")
> ^ Here, we should verify whether the monitor does support pci query
> command (at some point, this *will* be implemented). If it doesn't,
> throw a error.TestNA with the message "PCI block hotplug is not
> supported by the monitor".
>
>> +        add_output = vm.monitor.cmd(pci_add_cmd)
>> +        after_add = vm.monitor.info("pci")
>> +        if not id in after_add:
>> +            raise error.TestFail("Add device failed. Hypervisor command is: %s"
>> +                                 ". Output: %r" % (pci_add_cmd, add_output))
>>
>>       # Define a helper function to delete the device
>>       def pci_del(ignore_failure=False):
>> -        slot_id = "0" + add_output.split(",")[2].split()[1]
>> -        cmd = "pci_del pci_addr=%s" % slot_id
>> +        if cmd_type == "pci":
>> +            slot_id = "0" + add_output.split(",")[2].split()[1]
>> +            cmd = "pci_del pci_addr=%s" % slot_id
>> +        elif cmd_type == "device":
>> +            cmd = "device_del id=%s" % id
>>           # This should be replaced by a proper monitor method call
>>           vm.monitor.cmd(cmd)
>>
Hi Lucas,
Sorry that I didn't see your reply until now, because my bad filter for 
emails. Now we find that the block devices also supported by monitor 
command line(virtio blocks), so we update the patch. I will modify that 
patch according your comments and resend it soon. Thank you.

Best Regards
Yiqiao Pu