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
qemu_guest_agent: add new guest agent case #1032
Conversation
qemu/tests/qemu_guest_agent.py
Outdated
:param env: Dictionary with test environment. | ||
""" | ||
vm = env.get_vm(params["main_vm"]) | ||
if not self.vm.is_alive(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have get vm in above line, so this line could be:
if not vm.is_alive():
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed self.
if not vm.is_alive():
qemu/tests/qemu_guest_agent.py
Outdated
try: | ||
self.gagent.shutdown(self.gagent.SHUTDOWN_MODE_POWERDOWN) | ||
except Exception: | ||
self.test.fail("guest-shutdown command returned a wrong message") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
param 'test' is passed to this function, so could remove self.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed self.
except Exception:
test.fail("guest-shutdown command returned a wrong message")
qemu/tests/qemu_guest_agent.py
Outdated
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_FROZEN) | ||
|
||
if vm.monitor.cmd("system_reset"): | ||
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_THAWED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need an else
here to fail the case if above command does not perform as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add else
if vm.monitor.cmd("system_reset"):
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_THAWED)
else:
test.fail("system_reset command fail,please check qemu process")
qemu/tests/qemu_guest_agent.py
Outdated
|
||
""" | ||
Execute "system_reset" and send "shutdown,reboot" command to guest agent after FS fsreezed | ||
:param test: kvm test objecti |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/objecti/object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/fsreezed/freeze
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
qemu/tests/qemu_guest_agent.py
Outdated
@@ -722,6 +722,40 @@ def _action_after_fsfreeze(self, *args): | |||
" write file") | |||
|
|||
@error.context_aware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better remove all the @error.context_aware and remove from autotest.client.shared import error
use test.error instead of error.TestError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirm, Do I need remove all @error.context_aware and use test.error instead of error.TestError in this qemu_guest_agent.py?
I always @error.context_aware works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you remove all the @error.context_aware and use test.error will be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed all the @error.context_aware and from autotest.client.shared import error. and use test.error instead of error.TestError in this qemu_guest_agent.py
qemu/tests/qemu_guest_agent.py
Outdated
try: | ||
self.gagent.shutdown(self.gagent.SHUTDOWN_MODE_REBOOT) | ||
except Exception: | ||
self.test.fail("guest-reboot command returned a wrong message") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test.fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed self.
except Exception:
test.fail("guest-reboot command returned a wrong message")
qemu/tests/qemu_guest_agent.py
Outdated
if vm.monitor.cmd("system_reset"): | ||
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_THAWED) | ||
|
||
@error.context_aware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I only need remove " @error.context_aware" or remove from 755~758 lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@error.context_aware only
logging.info("guest FS is still freeze status") | ||
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_FROZEN) | ||
|
||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless try, better to raise original exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed try. use 'if' to instead of try.
if self.gagent.shutdown(self.gagent.SHUTDOWN_MODE_POWERDOWN):
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_FROZEN)
if self.gagent.shutdown(self.gagent.SHUTDOWN_MODE_REBOOT):
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_FROZEN)
if vm.monitor.cmd("system_reset"):
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_THAWED)
qemu/tests/qemu_guest_agent.py
Outdated
|
||
if vm.monitor.cmd("system_reset"): | ||
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_THAWED) | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part will not be execute, because monitor.cmd() always return c,md output .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have also discover this problem. monitor.cmd() can properly deal with cmd return. removed else part.
if vm.monitor.cmd("system_reset"):
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_THAWED)
Why two commits? |
the configure file is a little long, do we need to seperate it? |
@suqinhuang It is currently acceptable. As all test cases are very similar for this feature. the confiure file is more clear. I will consider to seperate when a large number of test case are added. |
aa1eab5
to
f03fce1
Compare
qemu/tests/qemu_guest_agent.py
Outdated
def gagent_install(self, params, vm, *args): | ||
if args and isinstance(args, tuple): | ||
gagent_install_cmd = args[0] | ||
else: | ||
raise error.TestError("Missing config 'gagent_install_cmd'") | ||
raise test.error("Missing config 'gagent_install_cmd'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not need 'raise', remember to modify all in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modified the whole file
qemu/tests/qemu_guest_agent.py
Outdated
@@ -137,14 +134,13 @@ def gagent_start(self, params, vm, *args): | |||
raise error.TestFail("Could not start qemu-guest-agent in VM" | |||
" '%s', detail: '%s'" % (vm.name, o)) | |||
|
|||
@error.context_aware | |||
def gagent_create(self, params, vm, *args): | |||
if self.gagent: | |||
return self.gagent | |||
|
|||
error.context("Create a QemuAgent object.", logging.info) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you remove "from autotest.client.shared import error", can this file still be execute successfully?
error.context should not be run correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use logging.info instead of error.context
qemu/tests/qemu_guest_agent.py
Outdated
""" | ||
vm = env.get_vm(params["main_vm"]) | ||
if not vm.is_alive(): | ||
test.skip("VM died,can't continue the test loop.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test.cancel should be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use test.cancel instead of test.skip
qemu/tests/qemu_guest_agent.py
Outdated
@@ -582,7 +562,7 @@ def get_allocation_bitmap(): | |||
bitmap = get_allocation_bitmap() | |||
if bitmap: | |||
logging.debug("block allocation bitmap: %s" % bitmap) | |||
raise error.TestError("block allocation bitmap" | |||
raise test.error("block allocation bitmap" | |||
" not empty before test.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is failed checked by "inspekt style".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I resoved inspekt style's problem already
qemu/tests/cfg/qemu_guest_agent.cfg
Outdated
@@ -86,6 +86,9 @@ | |||
no Windows | |||
gagent_check_type = fsfreeze | |||
gagent_fs_test_cmd = "rm -f /tmp/foo; echo foo > /tmp/foo" | |||
- check_reset_reboot_shutdown_fsfreeze: | |||
no Windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked polarion case https://polarion.engineering.redhat.com/polarion/#/project/RedHatEnterpriseLinux7/workitem?id=RHEL7-8350
This case is linux only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, this case supports windows guest. remove 'no windows' line
qemu/tests/qemu_guest_agent.py
Outdated
if not vm.is_alive(): | ||
test.skip("VM died,can't continue the test loop.") | ||
self.gagent.fsfreeze() | ||
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_FROZEN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a status verify command in fsfreeze() function 'self.verify_fsfreeze_status(self.FSFREEZE_STATUS_FROZEN)'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think this line can be deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right, fsfreeze() function verified guest file system status after fsfreeze command. I haven't to verify it again. deleted this line.
qemu/tests/qemu_guest_agent.py
Outdated
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_FROZEN) | ||
|
||
if vm.monitor.cmd("system_reset"): | ||
self.gagent.verify_fsfreeze_status(self.gagent.FSFREEZE_STATUS_THAWED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to check 'vm.is_alive()' again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all guest agent command need agent service response in guest. if send agent command to vm but vm is dead. I am afraid that agent command will get error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for explaining.
ab288dd
to
73fc57d
Compare
smp_4.2048m.repeat1.pre_test.Host_RHEL.m7.u4.qcow2.virtio_blk.up.virtio_net.RHEL.7.4.x86_64.myprovider.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_native: PASS (1699.28 s) smp_4.2048m.repeat1.pre_test.Host_RHEL.m7.u4.qcow2.virtio_blk.up.virtio_net.RHEL.7.4.x86_64.myprovider.rh_kernel_update: PASS (957.87 s) smp_4.2048m.repeat1.run_test.Host_RHEL.m7.u4.qcow2.virtio_blk.up.virtio_net.RHEL.7.4.x86_64.myprovider.qemu_guest_agent.virtio_serial.check_reset_reboot_shutdown_fsfreeze: PASS (426.97 s) Acked-by: Sitong Liu siliu@redhat.com |
73fc57d
to
d45ca9b
Compare
Thanks Sitong, (1/1) smp_2.512m.repeat1.run_test.Host_RHEL.m7.u4.qcow2.virtio_blk.up.virtio_net.RHEL.7.4.x86_64.myprovider.qemu_guest_agent.virtio_serial.check_reset_reboot_shutdown_fsfreeze: PASS |
qemu/tests/qemu_guest_agent.py
Outdated
session = self._get_session(params, vm) | ||
s, o = session.cmd_status_output(gagent_install_cmd) | ||
if s: | ||
raise error.TestFail("Could not install qemu-guest-agent package" | ||
" in VM '%s', detail: '%s'" % (vm.name, o)) | ||
error.TestFail("Could not install qemu-guest-agent package in VM '%s'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test.fail()
qemu/tests/qemu_guest_agent.py
Outdated
|
||
if not gagent_start_cmd: | ||
return | ||
|
||
error.context("Try to start 'qemu-guest-agent'.", logging.info) | ||
logging.info("Try to start 'qemu-guest-agent'.") | ||
session = self._get_session(params, vm) | ||
s, o = self._session_cmd_close(session, gagent_start_cmd) | ||
if s and "already been started" not in o: | ||
raise error.TestFail("Could not start qemu-guest-agent in VM" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test.fail(), please check again to make sure all error module replaced.
qemu/tests/qemu_guest_agent.py
Outdated
rebooted = self.__gagent_check_serial_output(pattern) | ||
if not rebooted: | ||
raise error.TestFail("Could not reboot VM via guest agent") | ||
error.context("Try to re-login to guest after reboot") | ||
logging.info("Try to re-login to guest after reboot") | ||
try: | ||
session = self._get_session(self.params, None) | ||
session.close() | ||
except Exception, detail: | ||
raise error.TestFail("Could not login to guest," |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test.fail()
qemu/tests/qemu_guest_agent.py
Outdated
@@ -386,7 +366,6 @@ def gagent_check_set_vcpus(self, test, params, env): | |||
if vcpus_info[vcpus_num - 1]["online"] is not False: | |||
raise error.TestFail("the vcpu status is not changed as expected") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
qemu/tests/qemu_guest_agent.py
Outdated
% int(guest_time), logging.info) | ||
test.error("can't get the guest time for contrast") | ||
logging.info("the time get inside guest by shell cmd is '%d' " | ||
% int(guest_time)) | ||
delta = abs(int(guest_time) - nanoseconds_time / 1000000000) | ||
if delta > 3: | ||
raise error.TestFail("the time get by guest agent is not the same " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
qemu/tests/qemu_guest_agent.py
Outdated
error.context("the time after being moved back into past is '%d' " | ||
% int(guest_time_after), logging.info) | ||
logging.info("the time after being moved back into past is '%d' " | ||
% int(guest_time_after)) | ||
delta = abs(int(guest_time_after) - target_time / 1000000000) | ||
if delta > 3: | ||
raise error.TestFail("the time set for guest is not the same " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
@lijinlijin For debug log. As system_reset command is success. and check guest's status is correct. don't need wait guest boot up fully. It is normal debug log. |
(1/2) smp_8.8192m.repeat1.run_test.Host_RHEL.m7.u3.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.7.4.x86_64.myprovider.qemu_guest_agent.virtio_serial.check_reset_reboot_shutdown_fsfreeze: PASS (93.19 s) |
qemu/tests/qemu_guest_agent.py
Outdated
test.fail("agent shutdown command shouldn't succeed for freeze FS") | ||
except guest_agent.VAgentCmdError as detail: | ||
if not re.search('guest-shutdown has been disabled', str(detail)): | ||
test.fail("qemu monitor return error msg") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
show "detail" is better:
test.fail(detail)
6814abd
to
8ddea06
Compare
qemu/tests/qemu_guest_agent.py
Outdated
gagent.verify_fsfreeze_status(gagent.FSFREEZE_STATUS_FROZEN) | ||
|
||
if not vm.monitor.cmd("system_reset"): | ||
time.sleep(120) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need to sleep 120s ?
a29f154
to
91c5fb8
Compare
Acked-by: Li Jin lijin@redhat.com |
@luckyh @hereischen Hi Hanxu, Haotong, please give suggestion to this pr since it has been pending for more than 100 days, Thanks. |
This patch depends on avocado-framework/avocado-vt#1391. |
@hereischen I have updated avocado-vt patch 1391. and re-tested this 2 patches. (1/2) Host_RHEL.m7.u6.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.7.6.x86_64.io-github-autotest-qemu.qemu_guest_agent.virtio_serial.check_reset_reboot_shutdown_fsfreeze: PASS (120.34 s) |
qemu/tests/qemu_guest_agent.py
Outdated
test.fail("agent shutdown command shouldn't succeed for freeze FS") | ||
except guest_agent.VAgentCmdError as detail: | ||
if not re.search('guest-shutdown has been disabled', str(detail)): | ||
test.fail(str(detail)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per avocado-framework/avocado-vt#1391 You dropped return True
, so should not judge it with if gagent.shutdow()
. Here should be:
try:
gagent.shutdown(mode)
except guest_agent.VAgentCmdError as detail:
if not re.search('guest-shutdown has been disabled', str(detail)):
test.fail(str(detail))
else:
test.fail("agent shutdown command shouldn't succeed for freeze FS")
91c5fb8
to
ef06f85
Compare
@vivianQizhu updated (1/4) Host_RHEL.m7.u6.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.7.6.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads: PASS (2186.04 s) |
@luckyh Would you help review it again? Thanks. |
9a3c396
to
f8fbf3d
Compare
1.frozen guest FS 2.send shutdown agent cmd to guest 3.send reboot agent cmd to guest 4.send fsthaw cmd to guest Signed-off-by: Xiangchun Fu <xfu@redhat.com>
f8fbf3d
to
37c5f88
Compare
updated, and This is the latest testing result. (1/2) Host_RHEL.m8.u0.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.0.x86_64.io-github-autotest-qemu.qemu_guest_agent.virtio_serial.check_reboot_shutdown_fsfreeze: PASS (56.43 s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
Thanks all. |
1.frozen guest FS
2.send shutdown agent cmd to guest
3.send reboot agent cmd to guest
4.send fsthaw cmd to guest
id: 1468530, 1476648
Signed-off-by: Xiangchun Fu xfu@redhat.com