Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automate blockresize cases #5585

Merged
merged 1 commit into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions libvirt/tests/cfg/virtual_disks/virtual_disks_blockresize.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
- virtual_disks.blockresize:
type = virtual_disks_blockresize
take_regular_screendumps = "no"
start_vm = "no"
target_format = "raw"
type_name = "block"
driver_type = 'raw'
device_type = "disk"
target_dev = "sdb"
status_error = "no"
define_error = "no"
func_supported_since_libvirt_ver = (10, 0, 0)
offset = "512"
size_in_mb = "100"
block_size_in_bytes = "104857600"
disk_slice_attrs = "{'slice_type': 'storage', 'slice_offset': '0', 'slice_size': '512'}"
variants test_scenario:
- capacity:
resize_value = "--capacity"
- specific:
resize_value = "104857600B"
- save_restore:
only coldplug..virtio
resize_value = "--capacity"
vm_save = "/var/lib/libvirt/images/%s.save"
- not_equal_actual_size:
only coldplug..virtio
resize_value = "1024B"
status_error = "yes"
status_error_msg = "Operation not supported: block device backed disk must be resized to its actual size"
- not_zero_offset:
only coldplug..virtio
disk_slice_attrs = "{'slice_type': 'storage', 'slice_offset': '512', 'slice_size': '1024'}"
status_error = "yes"
resize_value = "--capacity"
status_error_msg = "Operation not supported: resize of a disk with storage slice with non-zero .*offset.*"
- non_raw_block_device:
only coldplug..virtio
overlay_source_file_path = "/var/lib/libvirt/images/qcow2"
driver_type = 'qcow2'
target_format = "qcow2"
status_error = "yes"
resize_value = "--capacity"
status_error_msg = "Operation not supported: block resize to full capacity supported only with.*raw.*local block-based disks"
variants target_bus:
- virtio:
- sata:
only coldplug
Copy link
Contributor

@smitterl smitterl Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add no s390-virtio for 'sata'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No aarch64, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

no s390-virtio,aarch64
- scsi:
variants:
- coldplug:
virt_device_hotplug = "no"
- hotplug:
virt_device_hotplug = "yes"
234 changes: 234 additions & 0 deletions libvirt/tests/src/virtual_disks/virtual_disks_blockresize.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
#
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#
# Copyright Red Hat
#
# SPDX-License-Identifier: GPL-2.0

# Author: Chunfu Wen <chwen@redhat.com>
#

import logging
import os
import time

from virttest import libvirt_version
from virttest import virsh
from virttest import virt_vm
from virttest import utils_misc

from virttest.libvirt_xml import vm_xml, xcepts
from virttest.utils_libvirt import libvirt_disk
from virttest.utils_test import libvirt

from avocado.utils import process


LOG = logging.getLogger('avocado.' + __name__)
cleanup_files = []


def setup_scsi_debug_block_device():
"""
Setup one scsi_debug block device

:return: device name
"""
source_disk = libvirt.create_scsi_disk(scsi_option="",
scsi_size="100")
return source_disk


def check_image_virtual_size(image_file, expected_size, test):
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

matched_size -> expected_size?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Check whether image virtual size queried by qemu-img is matched

:param image_file: image file path
:param expected_size: expected size
:param test: test case itself
"""
disk_info_dict = utils_misc.get_image_info(image_file)
actual_size_in_bytes = str(disk_info_dict.get("vsize"))
if actual_size_in_bytes != expected_size:
test.fail(f"actual size of block disk is {actual_size_in_bytes}, but expected is : {expected_size}")


def create_customized_disk(params, test):
"""
Create one customized disk with related attributes

:param params: dict wrapped with params
:param test: test case itself
"""
type_name = params.get("type_name")
disk_device = params.get("device_type")
device_target = params.get("target_dev")
device_bus = params.get("target_bus")
device_format = params.get("target_format")
source_file_path = setup_scsi_debug_block_device()
overlay_source_file_path = params.get("overlay_source_file_path")
expected_size = params.get("block_size_in_bytes")
source_dict = {}

# check block size
check_image_virtual_size(source_file_path, expected_size, test)

if source_file_path:
source_dict.update({"dev": source_file_path})

if overlay_source_file_path:
libvirt.create_local_disk("file", overlay_source_file_path, "100M", device_format)
cleanup_files.append(overlay_source_file_path)
convert_cmd = "qemu-img convert -f qcow2 -O qcow2 %s %s" % (overlay_source_file_path, source_file_path)
process.run(convert_cmd, shell=True, verbose=True, ignore_status=False)

disk_src_dict = {"attrs": source_dict}

customized_disk = libvirt_disk.create_primitive_disk_xml(
type_name, disk_device,
device_target, device_bus,
device_format, disk_src_dict, None)

# Sometimes, slice size can be gotten by command (du -b source_file_path), but here not necessary
disk_slice_attrs = params.get('disk_slice_attrs')
if disk_slice_attrs:
disk_source = customized_disk.source
disk_source.slices = customized_disk.new_slices(**eval(disk_slice_attrs))
customized_disk.source = disk_source

LOG.debug("create customized xml: %s", customized_disk)
return customized_disk


def check_slice_within_vm(vm, new_disk, slice_value):
"""
Check disk slices attributes in guest internal

:param vm: vm object
:param new_disk: newly vm disk
:param slice_value: slice value
"""
session = None
try:
session = vm.wait_for_login()
cmd = ("lsblk |grep {0}"
.format(new_disk))
status, output = session.cmd_status_output(cmd)
LOG.debug("Disk operation in VM:\nexit code:\n%s\noutput:\n%s",
status, output)
return slice_value in output
except Exception as err:
LOG.debug("Error happens when check slices in vm: %s", str(err))
return False
finally:
if session:
session.close()


def check_vm_dumpxml(params, test, expected_attribute=True):
"""
Common method to check source in cdrom device

:param params: one collective object representing wrapped parameters
:param test: test object
:param expected_attribute: bool indicating whether expected attribute exists or not
"""
vm_name = params.get("main_vm")
disk_vmxml = vm_xml.VMXML.new_from_dumpxml(vm_name)
target_dev = params.get('target_dev')
disk = disk_vmxml.get_disk_all()[target_dev]
slices = disk.find('source').find('slices')
if not expected_attribute:
if slices is not None:
test.fail("unexpected slices appear in vm disk xml")
else:
if slices is None:
test.fail("slices can not be found in vm disk xml")


def run(test, params, env):
"""
Test resize disk with slices.

1.Prepare a vm with block disk
2.Attach block disk with slice attribute to the vm
3.Start vm
4.Resize the block disk
5.Check resize operations
6.Check status in VM internal
7.Detach device
"""
vm_name = params.get("main_vm")
vm = env.get_vm(vm_name)
virsh_dargs = {'debug': True}

hotplug = "yes" == params.get("virt_device_hotplug")
part_path = "/dev/%s"

# Skip test if version not match expected one
libvirt_version.is_libvirt_feature_supported(params)

# Get disk partitions info before hot/cold plug virtual disk
if vm.is_alive():
vm.destroy(gracefully=False)

# Backup vm xml
vmxml_backup = vm_xml.VMXML.new_from_inactive_dumpxml(vm_name)
vmxml = vmxml_backup.copy()

test_scenario = params.get("test_scenario")
target_bus = params.get("target_bus")
target_dev = params.get('target_dev')
status_error = "yes" == params.get("status_error")
try:
device_obj = create_customized_disk(params, test)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could reuse setup_attrs if you want

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor Author

@chunfuwen chunfuwen May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue has been discussed multiple round before. Whether use setup_attrs or previously base utils. Setup_attrs need input lots of long and nested config information as raw in cfg file, sometimes it look not easily to understand clearly. Base utils build up from some disperse parameter, and those parameter can share across variants in cfg file. Finally, I would like to say it is real balance here which one your prefer. I would like author choose his or her preferable ways.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'm fine with it. I was recently requested to use a dictionary in the cfg though, but I agree the author should be able to decide. #5287 (comment)

if not hotplug:
vmxml.add_device(device_obj)
vmxml.sync()
vm.start()
vm.wait_for_login()
if hotplug:
virsh.attach_device(vm_name, device_obj.xml,
ignore_status=False, debug=True)
except xcepts.LibvirtXMLError as xml_error:
test.fail("Failed to define VM:\n%s" % str(xml_error))
except virt_vm.VMStartError as details:
test.fail("VM failed to start."
"Error: %s" % str(details))
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if does this correspond to? Is this a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is try exception...else ..finally mechanism

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for clarifying!

session = vm.wait_for_login()
time.sleep(20)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to wait here , or could we use utils_misc.wait_for(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wait is to allow VM to show up disk ,and following call new_disk, _ = libvirt_disk.get_non_root_disk_name(session) can succeed.
So here it is for guest, and hard to find matched pattern for utils_misc.wait_for().

new_disk, _ = libvirt_disk.get_non_root_disk_name(session)
session.close()
check_vm_dumpxml(params, test, True)

slice_value = params.get("offset")
check_slice_within_vm(vm, new_disk, slice_value)

if test_scenario in ["blockresize_save_restore"]:
vm_save = params.get("vm_save")
cleanup_files.append(vm_save)
virsh.save(vm_name, vm_save)
virsh.restore(vm_save)
vm.wait_for_login().close()

resize_value = params.get("resize_value")
result = virsh.blockresize(vm_name, target_dev,
resize_value, **virsh_dargs)
libvirt.check_exit_status(result, status_error)

if not status_error:
check_vm_dumpxml(params, test, False)
slice_value = params.get("size_in_mb")
check_slice_within_vm(vm, new_disk, slice_value)
if hotplug:
virsh.detach_device(vm_name, device_obj.xml, flagstr="--live",
debug=True, ignore_status=False)
finally:
if vm.is_alive():
vm.destroy(gracefully=False)
vmxml_backup.sync()
libvirt.delete_scsi_disk()
for file_path in cleanup_files:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, the two comments are not necessary the xml.sync and file_path in cleanup_file already conveys what's done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove those two comments

if os.path.exists(file_path):
os.remove(file_path)
Loading