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

chunfuwen · 2024-10-06T12:51:34Z

startuppolicy is set to optional

xx-38693 - [disk-change]Check the disk-change events when startuppolicy is optional
xx-112414 - start a guest with block type cdrom --bz1471225
xx-32763 - [audit log] Audit log related to guest CDROM

chunfuwen · 2024-10-06T12:51:53Z

DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file)
DATA (filename=stdout.expected) => NOT FOUND (data sources: variant, test, file)
DATA (filename=stderr.expected) => NOT FOUND (data sources: variant, test, file)
PASS 1-type_specific.io-github-autotest-libvirt.virtual_disks.cdrom_device.coldplug.dropped_changed_events_startuppolicy_backend.positive_test

DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file)
DATA (filename=stdout.expected) => NOT FOUND (data sources: variant, test, file)
DATA (filename=stderr.expected) => NOT FOUND (data sources: variant, test, file)
PASS 1-type_specific.io-github-autotest-libvirt.virtual_disks.cdrom_device.coldplug.block_cdrom_tainted.positive_test

DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file)
DATA (filename=stdout.expected) => NOT FOUND (data sources: variant, test, file)
DATA (filename=stderr.expected) => NOT FOUND (data sources: variant, test, file)
PASS 1-type_specific.io-github-autotest-libvirt.virtual_disks.cdrom_device.coldplug.disconnect_audit_cdrom_backend.positive_test

startuppolicy is set to optional, and no cdrom:pass-through log when using block type cdrom xx-38693 - [disk-change]Check the disk-change events when startuppolicy is optional xx-112414 - start a guest with block type cdrom --bz1471225 xx-32763 - [audit log] Audit log related to guest CDROM Signed-off-by: chunfuwen <[email protected]>

meinaLi · 2024-10-23T02:10:31Z

libvirt/tests/cfg/virtual_disks/virtual_disks_cdrom_device.cfg

@@ -129,6 +129,29 @@
            target_dev = "sdd"
            backend_device = "empty_source_cdrom_backend"
            only hotplug..positive_test
+        - dropped_changed_events_startuppolicy_backend:


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

meinaLi · 2024-10-23T02:21:44Z

libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py

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


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

meinaLi · 2024-10-23T02:25:10Z

libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py

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


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?

meinaLi · 2024-10-23T02:30:16Z

libvirt/tests/cfg/virtual_disks/virtual_disks_cdrom_device.cfg

+            startupPolicy="optional"
+            backend_device = "dropped_changed_events_startuppolicy_backend"
+            only coldplug..positive_test
+        - block_cdrom_tainted:


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.

meinaLi · 2024-10-23T02:32:12Z

libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py

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


Better to have a blank line above.

meinaLi · 2024-10-23T02:35:35Z

libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py

+    return block_cdrom_disk
+
+
+def check_block_cdrom_tainted(vm, params, test):


check_block_cdrom_log maybe more better to understand.

meinaLi · 2024-10-23T02:37:50Z

libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py

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


Better to have a blank line above.

meinaLi · 2024-10-23T02:39:38Z

libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py

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


Better to have a blank line above.

meinaLi · 2024-10-23T02:44:17Z

libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py

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


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

meinaLi · 2024-10-23T02:47:02Z

libvirt/tests/src/virtual_disks/virtual_disks_cdrom_device.py

+    service_name = 'auditd'
+    service_mgr = service.ServiceManager()
+    status = service_mgr.status(service_name)
+    LOG.debug('Service status is %s', status)


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

chunfuwen force-pushed the automate_dropped_changed_event_case branch 2 times, most recently from f0efc32 to aa017a2 Compare October 8, 2024 05:58

chunfuwen force-pushed the automate_dropped_changed_event_case branch from aa017a2 to b1729c4 Compare October 8, 2024 06:09

meinaLi reviewed Oct 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

chunfuwen commented Oct 6, 2024 •

edited

Loading

chunfuwen commented Oct 6, 2024 •

edited

Loading

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

meinaLi Oct 23, 2024

		return block_cdrom_disk


		def check_block_cdrom_tainted(vm, params, test):

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

Are you sure you want to change the base?

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

Conversation

chunfuwen commented Oct 6, 2024 • edited Loading

chunfuwen commented Oct 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunfuwen commented Oct 6, 2024 •

edited

Loading

chunfuwen commented Oct 6, 2024 •

edited

Loading