avocado-framework / avocado-vt

Avocado VT Plugin
https://avocado-vt.readthedocs.org/
Other
83 stars 241 forks source link

env_process: Refactor migration setup into a Setuper #3950

Closed bgartzi closed 1 month ago

bgartzi commented 1 month ago

Move the setup step in which it is the environment is set up for libvirt migration tests in env_process.{pre,post}process into a Setuper subclass. The env_process setup_manager will be in charge of running such the setup/cleanup steps from now on.

ID: 2435

bgartzi commented 1 month ago

Hi @YongxueHong,

Yet another env_process refactoring patch. Could you have a look please?

The logic is related to libvirt migration tests. @dzhengfy, @chunfuwen could you also have a look? Really appreciated!

YongxueHong commented 1 month ago

Hi, @bgartzi There is another migration setup for the power hosts(power8 on power9(compat mode) remote host), it may be the setup in the power host scenarios. How should we handle this part during this phase, or do you have another plan for refactoring it? Thanks.

bgartzi commented 1 month ago

Hi @YongxueHong,

I'm aware of it. My initial plan was to refactor it separately so as to stick to the original order in which setup steps are executed. That is, I was going to refactor it as another setuper and register it in the uppermost part of the setup_manager setuper registration stack.

However, now that I have a second look at it, I think we could just merge the two separate migration test environment setup steps into one. Let me double confirm that. I'll back in a bit.

bgartzi commented 1 month ago

Hi @YongxueHong,

I'm back with some more things to consider. IIUC after a couple of little discussions, moving the step turning smt off up/down during host preparation wouldn't be an issue at all.

The migration_setup step you mention is part of a bigger block that turns smt off both in the source and target hosts (if it is a migration test). Now, however, I agree with you on that the step configuring the target host is more related to migration than what it is to basic smt configuration.

Let me refactor that too. I'm marking this PR as draft while I work on it. Sorry for the inconveniences.

YongxueHong commented 1 month ago

Hi @YongxueHong,

I'm back with some more things to consider. IIUC after a couple of little discussions, moving the step turning smt off up/down during host preparation wouldn't be an issue at all.

The migration_setup step you mention is part of a bigger block that turns smt off both in the source and target hosts (if it is a migration test). Now, however, I agree with you on that the step configuring the target host is more related to migration than what it is to basic smt configuration.

Let me refactor that too. I'm marking this PR as draft while I work on it. Sorry for the inconveniences.

Hi @bgartzi No problem. And I checked the commit log of the related code:

There are no such requirements from tp-qemu side as you know it has not supported the multi-host migration yet. We could ask for more details about this part regarding tp-libvirt. CC @chunfuwen

dzhengfy commented 1 month ago

@cliping Could you help try it in libvirt migration test?

cliping commented 1 month ago

@dzhengfy It works well in libvirt migration test. Thanks.