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

* Re: [Autotest] [PATCH] KVM test: Update pci_hotplug to suit for new qemu
       [not found] <1281061418-3938-1-git-send-email-ypu@redhat.com>
@ 2010-08-27 11:17 ` Lucas Meneghel Rodrigues
  2010-09-02 16:53   ` Yiqiao Pu
  0 siblings, 1 reply; 2+ messages in thread
From: Lucas Meneghel Rodrigues @ 2010-08-27 11:17 UTC (permalink / raw)
  To: Yiqiao Pu; +Cc: autotest, KVM mailing list

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)
>  

^ permalink raw reply	[flat|nested] 2+ messages in thread