block_hotplug: Refectoring block_hotplug

Update the order of importing modules by PEP8.

Wrap the codes to function:
  1. Update function find_disk and rename it.
  2. Update function run_sub_test.
  3. Wrap the part of waiting plugging disks completely to
     function wait_plug_disks.
  4. Wrap the part of creating block devices to function
     create_block_devices.
  5. Wrap the part of getting block devices to function
     get_block_devices.
  6. Wrap the part of hotplug or unplug block devices to
     function plug_block_devices.
  7. Wrap the part of format windows disk to function format_disk_win.
  8. Wrap the part of doing io testing on disk to function
     run_io_test.
  9. Update function get_disk_size.
Signed-off-by: NYongxue Hong <yhong@redhat.com>

qemu/tests/block_hotplug.py

@@ -2,12 +2,13 @@ import logging
 import re
 
 from virttest import error_context
-from virttest import utils_misc
-from virttest import utils_test
-from virttest.qemu_devices import qdevices
 from virttest import utils_disk
+from virttest import utils_misc
 from virttest import utils_numeric
+from virttest import utils_test
+
 from virttest.qemu_capabilities import Flags
+from virttest.qemu_devices import qdevices
 
 
 @error_context.context_aware
@@ -24,48 +25,90 @@ def run(test, params, env):
     :param params: Dictionary with the test parameters.
     :param env:    Dictionary with test environment.
     """
-    def find_disk(vm, cmd):
-        """
-        Find all disks in guest.
-        """
-        if params.get("os_type") == "linux":
-            pattern = params.get("get_disk_pattern", "^/dev/vd[a-z]*$")
-        elif params.get("os_type") == "windows":
-            pattern = r"^\d+"
-            cmd = params.get("get_disk_index", "wmic diskdrive get index")
+    def _find_all_disks(session):
+        """ Find all disks in guest. """
+        global all_disks
+        if windows:
+            all_disks = set(session.cmd('wmic diskdrive get index').split()[1:])
         else:
-            test.cancel("Unsupported OS type '%s'" % params.get("os_type"))
-
+            all_disks = utils_misc.list_linux_guest_disks(session)
+        return all_disks
+
+    def run_sub_test(test_name):
+        """ Run subtest before/after hotplug/unplug device. """
+        error_context.context("Running sub test '%s'." % test_name, logging.info)
+        utils_test.run_virt_sub_test(test, params, env, test_name)
+
+    def wait_plug_disks(session, action, disks_before_plug, excepted_num):
+        """ Wait plug disks completely. """
+        if not utils_misc.wait_for(lambda: len(disks_before_plug ^ _find_all_disks(
+                session)) == excepted_num, 60, step=1.5):
+            disks_info_win = ('wmic logicaldisk get drivetype,name,description '
+                              '& wmic diskdrive list brief /format:list')
+            disks_info_linux = 'lsblk -a'
+            disks_info = session.cmd(disks_info_win if windows else disks_info_linux)
+            logging.debug("The details of disks:\n %s" % disks_info)
+            test.fail("Failed to {0} devices from guest, need to {0}: {1}, "
+                      "actual {0}: {2}".format(action, excepted_num,
+                                               len(disks_before_plug ^ all_disks)))
+        return disks_before_plug ^ all_disks
+
+    def create_block_devices(image):
+        """ Create block devices. """
+        return vm.devices.images_define_by_params(
+            image, params.object_params(image), 'disk')
+
+    def get_block_devices(objs):
+        """ Get block devices. """
+        if isinstance(objs, str):
+            return [dev for dev in vm.devices if dev.get_param("id") == objs]
+        dtype = qdevices.QBlockdevNode if vm.check_capability(
+            Flags.BLOCKDEV) else qdevices.QDrive
+        return [dev for dev in objs if not isinstance(dev, dtype)]
+
+    def plug_block_devices(action, plug_devices):
+        """ Plug block devices. """
+        error_context.context("%s block device (iteration %d)" %
+                              (action.capitalize(), iteration), logging.info)
         session = vm.wait_for_login(timeout=timeout)
-        output = session.cmd_output_safe(cmd)
-        disks = re.findall(pattern, output, re.M)
+        disks_before_plug = _find_all_disks(session)
+        plug_devices = plug_devices if action == 'hotplug' else plug_devices[::-1]
+        for device in plug_devices:
+            if not getattr(vm.devices, 'simple_%s' % action)(device, vm.monitor)[1]:
+                test.fail("Failed to %s device '%s'." % (action, device))
+
+        num = 1 if action == 'hotplug' else len(data_imgs)
+        plugged_disks = wait_plug_disks(session, action, disks_before_plug, num)
         session.close()
-        return disks
+        return plugged_disks
 
-    def get_new_disk(disk1, disk2):
-        """
-        Get the different disk between disk1 and disk2.
-        """
-        disk = list(set(disk2).difference(set(disk1)))
-        return disk
-
-    def run_sub_test(params, plug_tag):
-        """
-        Run subtest before/after hotplug/unplug device.
+    def format_disk_win():
+        """ Format disk in windows. """
+        error_context.context("Format disk %s in windows." % new_disk, logging.info)
+        session = vm.wait_for_login(timeout=timeout)
+        if disk_index is None and disk_letter is None:
+            drive_letters.append(
+                utils_disk.configure_empty_windows_disk(
+                    session, new_disk, params['image_size_%s' % img])[0])
+        elif disk_index and disk_letter:
+            utils_misc.format_windows_disk(
+                session, disk_index[index], disk_letter[index])
+            drive_letters.append(disk_letter[index])
+        session.close()
 
-        :param plug_tag: identify when to run subtest,
-                         ex, before_hotplug.
-        :return: whether vm was successfully shut-down
-                 if needed
-        """
-        sub_type = params.get("sub_type_%s" % plug_tag)
-        if sub_type:
-            error_context.context(context_msg % (sub_type, plug_tag),
-                                  logging.info)
-            utils_test.run_virt_sub_test(test, params, env, sub_type)
-            if sub_type == "shutdown" and vm.is_dead():
-                return True
-        return None
+    def run_io_test():
+        """ Run io test on the hot plugged disks. """
+        error_context.context(
+            "Run io test on the hot plugged disks.", logging.info)
+        session = vm.wait_for_login(timeout=timeout)
+        if windows:
+            drive_letter = drive_letters[index]
+            test_cmd = disk_op_cmd % (drive_letter, drive_letter)
+            test_cmd = utils_misc.set_winutils_letter(session, test_cmd)
+        else:
+            test_cmd = disk_op_cmd % (new_disk, new_disk)
+        session.cmd(test_cmd, timeout=disk_op_timeout)
+        session.close()
 
     def get_disk_size(did):
         """
@@ -74,15 +117,17 @@ def run(test, params, env):
         :param did: the disk of id, e.g. sdb,sda for linux, 1, 2 for windows
         :return: the disk size
         """
-        if params['os_type'] == 'linux':
-            size = utils_disk.get_linux_disks(session)[did][1].strip()
-        else:
+        session = vm.wait_for_login(timeout=timeout)
+        if windows:
             script = '{}_{}'.format("disk", utils_misc.generate_random_string(6))
             cmd = "echo %s > {0} && diskpart /s {0} && del /f {0}".format(script)
-            p = r'Disk\s+%s\s+[A-Z]+\s+\d+\s+[A-Z]+\s+(?P<free>\d+\s+[A-Z]+)'
+            p = r'Disk\s+%s\s+[A-Z]+\s+(?P<size>\d+\s+[A-Z]+)\s+'
             disk_info = session.cmd(cmd % 'list disk')
-            size = re.search(p % did, disk_info, re.I | re.M).groupdict()['free'].strip()
-        logging.info('The size of disk[%s] is %s' % (did, size))
+            size = re.search(p % did, disk_info, re.I | re.M).groupdict()['size'].strip()
+        else:
+            size = utils_disk.get_linux_disks(session)[did][1].strip()
+        logging.info('The size of disk %s is %s' % (did, size))
+        session.close()
         return size
 
     def check_disk_size(did, excepted_size):
@@ -97,109 +142,60 @@ def run(test, params, env):
             'excepted size(%s).' % (did, excepted_size), logging.info)
         value, unit = re.search(r"(\d+\.?\d*)\s*(\w?)", excepted_size).groups()
         if utils_numeric.normalize_data_size(get_disk_size(did), unit) != value:
-            test.fail('The size of [%s] is not equal to excepted size(%s).'
+            test.fail('The size of disk %s is not equal to excepted size(%s).'
                       % (did, excepted_size))
 
-    img_list = params.get("images").split()
-    #sometimes, ppc can't get new plugged disk in 5s, so time to 10s
-    pause = float(params.get("virtio_block_pause", 10.0))
-    blk_num = int(params.get("blk_num", 1))
-    repeat_times = int(params.get("repeat_times", 3))
-    timeout = int(params.get("login_timeout", 360))
-    disk_op_timeout = int(params.get("disk_op_timeout", 360))
-    get_disk_cmd = params.get("get_disk_cmd")
-    context_msg = "Running sub test '%s' %s"
+    data_imgs = params.get("images").split()[1:]
     disk_index = params.objects("disk_index")
     disk_letter = params.objects("disk_letter")
+    disk_op_cmd = params.get("disk_op_cmd")
+    disk_op_timeout = int(params.get("disk_op_timeout", 360))
+    timeout = int(params.get("login_timeout", 360))
+    windows = params["os_type"] == 'windows'
+
+    sub_test_after_plug = params.get("sub_type_after_plug")
+    sub_test_after_unplug = params.get("sub_type_after_unplug")
+    sub_test_before_unplug = params.get("sub_type_before_unplug")
+    shutdown_after_plug = sub_test_after_plug == 'shutdown'
+    need_plug = params.get("need_plug", 'no') == "yes"
+    need_check_disk_size = params.get('check_disk_size', 'no') == 'yes'
+
+    drive_letters = []
+    unplug_devs = []
+    global all_disks
+    all_disks = set()
 
     vm = env.get_vm(params["main_vm"])
     vm.verify_alive()
 
-    for iteration in range(repeat_times):
-        device_list = []
-        error_context.context("Hotplug block device (iteration %d)" % iteration,
-                              logging.info)
-
-        plug_tag = "before_plug"
-        run_sub_test(params, plug_tag)
-
-        for num in range(blk_num):
-            image_name = img_list[num + 1]
-            image_params = params.object_params(image_name)
-            if params.get("need_plug") == "yes":
-                disks_before_plug = find_disk(vm, get_disk_cmd)
-                devs = vm.devices.images_define_by_params(image_name,
-                                                          image_params, 'disk')
-                for dev in devs:
-                    ret = vm.devices.simple_hotplug(dev, vm.monitor)
-                    if ret[1] is False:
-                        test.fail("Failed to hotplug device '%s'."
-                                  "Output:\n%s" % (dev, ret[0]))
-                plug_disks = utils_misc.wait_for(lambda: get_new_disk(disks_before_plug,
-                                                 find_disk(vm, get_disk_cmd)), pause)
-                if not plug_disks:
-                    test.fail("Failed to hotplug device to guest")
-                disk = plug_disks[0]
-
-                session = vm.wait_for_login(timeout=timeout)
-                if params.get('check_disk_size', 'no') == 'yes':
-                    did = disk_index[num] if params['os_type'] == 'windows' else disk[5:]
-                    check_disk_size(did, image_params['image_size'])
-                if params.get("os_type") == "windows":
+    for iteration in range(int(params.get("repeat_times", 3))):
+        data_imgs_devs = {img: create_block_devices(img) for img in data_imgs}
+        for index, img in enumerate(data_imgs_devs):
+            data_devs = data_imgs_devs[img]
+            if need_plug:
+                new_disk = plug_block_devices('hotplug', data_devs).pop()
+
+                if windows:
                     if iteration == 0:
-                        error_context.context("Format disk", logging.info)
-                        utils_misc.format_windows_disk(session, disk_index[num],
-                                                       mountpoint=disk_letter[num])
-                error_context.context("Check block device after hotplug.",
-                                      logging.info)
-                if params.get("disk_op_cmd"):
-                    if params.get("os_type") == "linux":
-                        test_cmd = params.get("disk_op_cmd") % (disk, disk)
-                    elif params.get("os_type") == "windows":
-                        test_cmd = params.get("disk_op_cmd") % (disk_letter[num],
-                                                                disk_letter[num])
-                        test_cmd = utils_misc.set_winutils_letter(session, test_cmd)
-                    else:
-                        test.cancel("Unsupported OS type '%s'" % params.get("os_type"))
-
-                    status, output = session.cmd_status_output(test_cmd,
-                                                               timeout=disk_op_timeout)
-                    if status:
-                        test.fail("Check for block device failed."
-                                  "Output: %s" % output)
-                session.close()
-
-                dtype = qdevices.QBlockdevNode if vm.check_capability(
-                    Flags.BLOCKDEV) else qdevices.QDrive
-                devs = [dev for dev in devs if not isinstance(dev, dtype)]
-                device_list.extend(devs)
-            else:
-                for device in vm.devices:
-                    if device.get_param("id") == img_list[num + 1]:
-                        device_list.append(device)
-
-        plug_tag = "after_plug"
-        vm_switched_off = run_sub_test(params, plug_tag)
-        if vm_switched_off:
+                        format_disk_win()
+                if need_check_disk_size:
+                    check_disk_size(new_disk if windows else new_disk[5:],
+                                    params['image_size_%s' % img])
+
+                if disk_op_cmd:
+                    run_io_test()
+            unplug_devs.extend(get_block_devices(
+                data_devs) if need_plug else get_block_devices(img))
+        if sub_test_after_plug:
+            run_sub_test(sub_test_after_plug)
+        if shutdown_after_plug:
             return
 
-        plug_tag = "before_unplug"
-        run_sub_test(params, plug_tag)
-
-        error_context.context("Unplug block device (iteration %d)" % iteration,
-                              logging.info)
-        disks_before_unplug = find_disk(vm, get_disk_cmd)
-        for device in reversed(device_list):
-            ret = vm.devices.simple_unplug(device, vm.monitor)
-            if ret[1] is False:
-                test.fail("Failed to unplug device '%s'."
-                          "Output:\n%s" % (device, ret[0]))
-
-        unplug_disks = utils_misc.wait_for(lambda: get_new_disk(find_disk(vm, get_disk_cmd),
-                                           disks_before_unplug), pause)
-        if len(unplug_disks) != blk_num:
-            test.fail("Failed to unplug devices from guest, need to unplug: %d,"
-                      "actual unplug: %d" % (blk_num, len(unplug_disks)))
-
-        plug_tag = "after_unplug"
-        run_sub_test(params, plug_tag)
+        if sub_test_before_unplug:
+            run_sub_test(sub_test_before_unplug)
+
+        plug_block_devices('unplug', unplug_devs)
+        unplug_devs.clear()
+
+        if sub_test_after_unplug:
+            run_sub_test(sub_test_after_unplug)

qemu/tests/cfg/block_hotplug.cfg

@@ -23,12 +23,10 @@
 
     variants:
         - one_pci:
-            blk_num = 1
             repeat_times = 1
         - multi_pci:
             q35, arm64-pci:
                 pcie_extra_root_port = 2
-            blk_num = 2
             repeat_times = 1
             images += " stg1"
             boot_drive_stg1 = no