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.

Sign up for GitHub

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

Jump to bottom

Automate Check the disk-change events for cdrom and disk when #5932

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Automate Check the disk-change events for cdrom and disk when #5932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 23 additions & 0 deletions libvirt/tests/cfg/virtual_disks/virtual_disks_cdrom_device.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,29 @@
target_dev = "sdd"
backend_device = "empty_source_cdrom_backend"
only hotplug..positive_test
- dropped_changed_events_startuppolicy_backend:
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I think this pattern is a little unclear. Maybe events_check_with_optional_startuppolicy is better?

type_name = "file"
target_format = "raw"
virt_disk_device_source_second = "/var/lib/libvirt/images/none.qcow2"
virt_disk_device_source = "/var/lib/libvirt/images/none"
target_dev = "sdc"
startupPolicy="optional"
backend_device = "dropped_changed_events_startuppolicy_backend"
only coldplug..positive_test
- block_cdrom_tainted:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a positive case which we don't want to find the tainted log. But this pattern seems a negative case. I suggest we can directly to use block_cdrom_bankend to help us better understand the case when we debug.

only coldplug..positive_test
target_format = "raw"
type_name = "block"
target_dev = "sdb"
virt_disk_device_source = "/dev/sdb"
backend_device = "block_cdrom_tainted"
- disconnect_audit_cdrom_backend:
type_name = "file"
target_format = "raw"
virt_disk_device_source = "/var/lib/libvirt/images/disconnect.iso"
target_dev = "sdb"
backend_device = "disconnect_audit_cdrom_backend"
only coldplug..positive_test
variants:
- hotplug:
virt_device_hotplug = "yes"
Expand Down
139 changes: 138 additions & 1 deletion libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,23 @@

from avocado.utils import linux_modules
from avocado.utils import process
from avocado.utils import service

from virttest import libvirt_version
from virttest import virt_vm, utils_misc
from virttest import virsh
from virttest import utils_split_daemons
from virttest import utils_libvirtd

from virttest.libvirt_xml import vm_xml, xcepts

from virttest.utils_test import libvirt
from virttest.utils_libvirt import libvirt_disk

from virttest.utils_config import LibvirtdConfig
from virttest.utils_config import VirtQemudConfig


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

Expand Down Expand Up @@ -512,6 +518,112 @@ def check_empty_source_cdrom(vm, params, test):
check_source_in_cdrom_device(vm, None, test)


def check_dropped_changed_events_startuppolicy_backend(vm, params, virsh_session, test):
"""
Check changed event for cdrom, and dropped event for disk

:param vm: one object representing VM
:param params: wrapped parameters in dictionary format
:param virsh_session: virsh session
:param test: test assert object
"""
# check tray-change event
virsh_session.send_ctrl("^C")
ret_output = virsh_session.get_stripped_output().replace("\n", "").strip()
LOG.debug("dropped and changed events:\n%s", ret_output)

change_event_matched = r"event 'disk-change' for domain '%s' disk .*changed" % vm.name
if not re.search(change_event_matched, ret_output):
test.fail("Can not find matched event:%s from event output: %s" % (change_event_matched, ret_output))
dropped_event_matched = r"event 'disk-change' for domain '%s' disk .*dropped" % vm.name
if not re.search(dropped_event_matched, ret_output):
test.fail("Can not find matched event:%s from event output: %s" % (dropped_event_matched, ret_output))
Copy link
Contributor

Choose a reason for hiding this comment

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

The above codes are little repetitive, I suggest we can merge them together. For example, use for statement to check events:

events_to_check = ["changed", "dropped"]
for event in events_to_check:
    event_matched = r"event 'disk-change' for domain '%s' disk .*%s" % (vm.name, event)
    if not re.search(event_matched, ret_output):
        test.fail("Can not find matched event:%s from event output: %s" % (event_matched, ret_output))



def create_block_cdrom_disk(params):
"""
Create one block cdrom device
:param params: dict wrapped with parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to have a blank line above.

"""
source_disk = libvirt.create_scsi_disk(scsi_option="",
scsi_size="100")
params.update({'virt_disk_device_source': source_disk})

block_cdrom_disk = create_customized_disk(params)
return block_cdrom_disk


def check_block_cdrom_tainted(vm, 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.

check_block_cdrom_log maybe more better to understand.

"""
Check matched information in vm.log

:param vm: one object representing VM
:param params: wrapped parameters in dictionary format
:param test: test assert object
"""
qemu_log = "/var/log/libvirt/qemu/%s.log" % vm.name
process.run("truncate -s 0 {}".format(qemu_log), shell=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

This step makes the checkpoint meaningless because the log has been cleared.

libvirt.check_logfile(" tainted: cdrom-passthrough", qemu_log, str_in_log=False)


def create_disconnect_audit_cdrom(params):
"""
Create one file cdrom device
:param params: dict wrapped with parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to have a blank line above.

"""
libvirtd_config = VirtQemudConfig() if utils_split_daemons.is_modular_daemon() else LibvirtdConfig()
libvirtd_config.audit_level = 1
libvirtd_config.audit_logging = 1
utils_libvirtd.Libvirtd('virtqemud').restart()

# Clean up audit message in log file
cmd = "truncate -s 0 /var/log/audit/audit.log*"
process.run(cmd, shell=True)

# ensure audit service is started
service_name = 'auditd'
service_mgr = service.ServiceManager()
status = service_mgr.status(service_name)
LOG.debug('Service status is %s', status)
Copy link
Contributor

Choose a reason for hiding this comment

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

I found this function Factory.create_service("auditd"). You can check if we can use this common function.


if not status:
service_mgr.start(service_name)

block_cdrom_disk = create_iso_cdrom_disk(params)
return block_cdrom_disk


def check_disconnect_audit_cdrom(params, test):
"""
check audit log when cdrom is disconnected
:param params: dict wrapped with parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to have a blank line above.

:param test: test assert object
"""
source_file_path = params.get("virt_disk_device_source")
if os.path.exists(source_file_path):
os.remove(source_file_path)

def _check_disk_message_from_audit_log():
"""
Check whether disk related message in /var/log/audit/audit.log
"""
cmd = 'ausearch --start today -m VIRT_RESOURCE -i | grep update'
return process.system(cmd, ignore_status=True, shell=True)

result = utils_misc.wait_for(lambda: _check_disk_message_from_audit_log(), timeout=30)
if not result:
test.fail("Failed to get expected messages: virt resource update from log file: /var/log/audit/audit.log.")


def restore_libvirtd_config():
"""
restore to previous libvirtd config
"""
libvirtd_config = VirtQemudConfig() if utils_split_daemons.is_modular_daemon() else LibvirtdConfig()
libvirtd_config.restore()
utils_libvirtd.Libvirtd('virtqemud').restart()


def run(test, params, env):
"""
Test attach cdrom device with option.
Expand Down Expand Up @@ -579,6 +691,23 @@ def run(test, params, env):
if backend_device == "empty_source_cdrom_backend":
device_obj = create_customized_disk(params)
params.update({'cdrom_xml': device_obj})
if backend_device == "dropped_changed_events_startuppolicy_backend":
# First create cdrom disk, then create one more file disk
device_obj1 = create_customized_disk(params)
vmxml.add_device(device_obj1)
vmxml.sync()
params.update({'virt_disk_device_source': params.get("virt_disk_device_source_second")})
params.update({'target_dev': "sdd"})
params.update({'device_type': 'disk'})
device_obj = create_customized_disk(params)
# start loop to wait for disk change event
virsh_session = aexpect.ShellSession(virsh.VIRSH_EXEC, auto_close=True)
event_cmd = "event --domain %s --event disk-change --loop" % vm.name
virsh_session.sendline(event_cmd)
if backend_device == "block_cdrom_tainted":
device_obj = create_block_cdrom_disk(params)
if backend_device == "disconnect_audit_cdrom_backend":
device_obj = create_disconnect_audit_cdrom(params)
if not hotplug:
# Sync VM xml.
vmxml.add_device(device_obj)
Expand Down Expand Up @@ -638,6 +767,12 @@ def run(test, params, env):
check_scsi_cdrom_hot_eject(vm, params, test)
elif backend_device == "empty_source_cdrom_backend":
check_empty_source_cdrom(vm, params, test)
elif backend_device == "dropped_changed_events_startuppolicy_backend":
check_dropped_changed_events_startuppolicy_backend(vm, params, virsh_session, test)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's also important to check the dumpxml after starting guest. But I didn't find this step. Did I miss this part of the code or is it not added in the script?

elif backend_device == "block_cdrom_tainted":
check_block_cdrom_tainted(vm, params, test)
elif backend_device == "disconnect_audit_cdrom_backend":
check_disconnect_audit_cdrom(params, test)
finally:
# Recover VM.
if vm.is_alive():
Expand All @@ -648,11 +783,13 @@ def run(test, params, env):
for file_path in CLEANUP_FILES:
if os.path.exists(file_path):
os.remove(file_path)
if backend_device == "change_startuppolicy_cdrom_backend":
if backend_device in ["change_startuppolicy_cdrom_backend", "block_cdrom_tainted"]:
# unload scsi_debug module if loaded
def _unload():
linux_modules.unload_module("scsi_debug")
return True
utils_misc.wait_for(_unload, timeout=20, ignore_errors=True)
if backend_device in ["disconnect_audit_cdrom_backend"]:
restore_libvirtd_config()
if backend_device == "block_lun_source":
process.run("losetup -D", shell=True)
Loading