Add case to cover start virtlogd with specified configuration file #5188
Conversation
|
cleaning libvirtd logs... Command 'virsh list' finished with 0 after 0.037828258s cleaning libvirtd logs... DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file) DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file) cleaning libvirtd logs... cleaning libvirtd logs... Command 'virsh list' finished with 0 after 0.026679640s cleaning libvirtd logs... DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file) DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file) |
chunfuwen
force-pushed
the
add_virtlogd_config_specfic_file
branch
5 times, most recently
from
46a5de0
to
86dc1dc
Compare
chunfuwen
force-pushed
the
add_virtlogd_config_specfic_file
branch
4 times, most recently
from
d860eae
to
42f909b
Compare
| cmd = "chcon system_u:object_r:virtlogd_etc_t:s0 %s" % virtlogd_config_file_new | ||
| process.run(cmd, ignore_status=True, shell=True) | ||
| # restart virtlogd | ||
| process.run("systemctl restart virtlogd", ignore_status=True, shell=True) |
There was a problem hiding this comment.
To operate a service, we do not hardcode the command. I suggest we use
service.Factory.create_service("virtlogd").restart()
| if matched_message not in cmd_result: | ||
| test.fail("Check virtlogd is started with config :%s failed in output:%s" | ||
| % (matched_message, cmd_result)) | ||
| new_virtlogd_log_file = "/var/log/libvirt/virtlogd-new.log" |
There was a problem hiding this comment.
For the log file name, I sugguest we get it from predefined parameter, like virtlogd_config_file_new, instead of hardcoding here.
| fd.write('VIRTLOGD_ARGS="--timeout=30"') | ||
| fd.write('\n') | ||
| # restart virtlogd | ||
| process.run("systemctl restart virtlogd", ignore_status=True, shell=True) |
There was a problem hiding this comment.
To operate a service, we do not hardcode the command. I suggest we use
service.Factory.create_service("virtlogd").restart()
| cmd = ("systemctl status virtlogd | grep 'Active: active'") | ||
| ret = process.run(cmd, ignore_status=True, shell=True, verbose=True) | ||
| if ret.exit_status != 0: | ||
| test.fail("virtlogd is not in active from log:%s" % ret.stdout_text.strip()) |
There was a problem hiding this comment.
To get service status, I suggest we use
service.Factory.create_service("virtlogd").status('Active: active')
See below definition:
def status(self, regex=r"active \(running\)"):
There was a problem hiding this comment.
As validated, service.status() method can not accept parameter
| if ret.exit_status != 0: | ||
| test.fail("virtlogd is not in active from log:%s" % ret.stdout_text.strip()) | ||
| action = params.get("action") | ||
| process.run("systemctl %s virtlogd" % action, ignore_status=True, shell=True) |
There was a problem hiding this comment.
How about
service.Factory.create_service("virtlogd").action()
We'd better to add reload() in service.py
There was a problem hiding this comment.
look like not so urgent, so will address in another PR, but not this time
| cmd = ("systemctl status virtlogd | grep 'Active: failed'") | ||
| result = process.run(cmd, ignore_status=True, shell=True, verbose=True) | ||
| if result.exit_status != 0: | ||
| test.fail("virtlogd is not in failed state from output: %s" % result.stdout_text.strip()) |
There was a problem hiding this comment.
As validated, service.status() method can not accept parameter
| """ | ||
| vm.start() | ||
| check_file_exist(guest_log_file) | ||
| # remove the qemu log file |
There was a problem hiding this comment.
It is obviously , so no need this comment line.
| os.remove(virtlogd_config_file_new) | ||
| if virtlogd_config_bak_file: | ||
| os.rename(virtlogd_config_bak_file, virtlogd_config_file) | ||
| process.run("systemctl restart virtlogd", ignore_status=True, shell=True) |
There was a problem hiding this comment.
service.Factory.create_service("virtlogd").restart()
| process.run("systemctl restart virtlogd", ignore_status=True, shell=True) | ||
| if expected_result == "invalid_virtlogd_conf": | ||
| virtlogd_config.restore() | ||
| process.run("systemctl restart virtlogd", ignore_status=True, shell=True) |
There was a problem hiding this comment.
service.Factory.create_service("virtlogd").restart()
| if virtlogd_config_bak_file: | ||
| os.rename(virtlogd_config_bak_file, virtlogd_config_file) | ||
| process.run("systemctl restart virtlogd", ignore_status=True, shell=True) | ||
| if expected_result == "invalid_virtlogd_conf": |
There was a problem hiding this comment.
I think it is better to use elif
There was a problem hiding this comment.
not updated, but I am ok with it, too.
chunfuwen
force-pushed
the
add_virtlogd_config_specfic_file
branch
2 times, most recently
from
3e82d6b
to
74067bd
Compare
| try: | ||
| vm.start() | ||
| except virt_vm.VMStartError as detail: | ||
| logging.info("VM failed to start." | ||
| "Error: %s" % str(detail)) | ||
| crash_information = params.get("crash_information") | ||
| if not libvirt.check_logfile(crash_information, guest_log_file): | ||
| test.fail("Check expected message log:%s failed in log file:%s" | ||
| % (crash_information, guest_log_file)) |
There was a problem hiding this comment.
Just confirm, When the guest start successfully ,Will we miss the check point?
There was a problem hiding this comment.
Here Vm will fail to start, but if we don't catch exception, the whole program will be terminated immediately. In this way we can not do later check_logfile.
In terms of logging mechanism, virtlogd will output log information into specificed log file whether VM will be started successfully or not.
chunfuwen
force-pushed
the
add_virtlogd_config_specfic_file
branch
2 times, most recently
from
2572767
to
72f6246
Compare
VIRT-xx: virtlogd can be started with specified config by configuring VIRTLOGD_ARGS="--config /etc/libvirt/virtlogd-new.conf" in /etc/sysconfig/virtlogd VIRT-xx: [virtlogd][-t | --timeout] Start virtlogd with --timeout <SECONDS> and without active guests VIRT-xx: [virtlogd] [Reload] [negative] Virtlogd.service reload failed with invalid configuration VIRT-xx: [virtlogd][Stop] Stop virtlogd service when guest is running VIRT-xx: [virtlogd]Check qemu log when guest fails to start due to "qemu process exits" error VIRT-xx: [virtlogd] Start virtlogd with default max_size and max_backups VIRT-xx: [virtlogd]Remove the qemu log file when guest is running VIRT-xx: [virtlogd] [start] [negative] Virtlogd.serivce start failed with invalid configuration VIRT-xx: [virtlogd] Check opened FDs of qemu log file when guest is running and destroyed VIRT-xx: [virtlogd] Check qemu log when guest shuts down VIRT-xx: [virtlogd] Check qemu log when guest starts for the second time VIRT-xx: [virtlogd]Check the qemu log when save&restore guest. Signed-off-by: chunfuwen <[email protected]>
chunfuwen
force-pushed
the
add_virtlogd_config_specfic_file
branch
from
47738b7
to
712d5c6
Compare
|
After comments are addressed, rerun impacted cases, result follow as: PASS 11-type_specific.io-github-autotest-libvirt.conf_file.qemu_conf.set_virtlogd.positive_test.specific_config_file |
VIRT-xx: [virtlogd]Remove the qemu log file when guest is running
VIRT-xx: [virtlogd] [start] [negative] Virtlogd.serivce start failed with invalid configuration
VIRT-xx: [virtlogd] Check opened FDs of qemu log file when guest is running and destroyed