From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yiqiao Pu Subject: Re: [Autotest] [PATCH] KVM test: Update pci_hotplug to suit for new qemu Date: Thu, 02 Sep 2010 16:53:52 +0000 Message-ID: <4C7FD6A0.1080605@redhat.com> References: <1281061418-3938-1-git-send-email-ypu@redhat.com> <1282907824.2924.21.camel@freedom> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: autotest@test.kernel.org, KVM mailing list To: Lucas Meneghel Rodrigues Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33966 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750799Ab0IBIxx (ORCPT ); Thu, 2 Sep 2010 04:53:53 -0400 In-Reply-To: <1282907824.2924.21.camel@freedom> Sender: kvm-owner@vger.kernel.org List-ID: 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 >> --- >> 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