On 05/04/24 7:58 pm, Fabiano Rosas wrote:
!-------------------------------------------------------------------|
CAUTION: External Email
|-------------------------------------------------------------------!
Het Gala<het.g...@nutanix.com> writes:
On 27/03/24 2:37 am, Fabiano Rosas wrote:
Het Gala<het.g...@nutanix.com> writes:
Some comments, mostly just thinking out loud...
For <test-type> --> migrate
/<test-type>/<migration-mode>/<method>/<transport>/<invocation>/
<compression>/<encryption>/O:<others>/...
For <test-type> --> validate
/<test-type>/<validate-variable>/O:<transport>/O:<invocation>/
<validate-test-result>/O:<test-reason>/O:<others>/...
Do we need an optional 'capability' element? I'm not sure how practical
is to leave that as 'others', because that puts it at the end of the
string. We'd want the element that's more important/with more variants
to be towards the start of the string so we can run all tests of the
same kind with the -r option.
While also looking at different functions for figuring out the transport
and invocation, my observation was that, there might be many capabilities
added to the same test, while it might not be important also.
Ex: /migrate/multifd/tcp/plain
1. multifd is defined as a migration mode.
2. It is also a capability, and comes in 2 parts [multifd, multifd-channels]
though one is a capability and another is parameter
Similarly in other examples of compression, there are many capabilities
and parameters added, but it might be not important to mention that ?
Secondly, there are multiple migration capabilities IIRC (> 15). And a test
requiring multiple capabilities, the overall string would be too long, and
not that important also to mention all capabilities.
Just thinking out of mind - Can we have selective list of capabilities ?
1. multifd 2. compress (again, there might be confusion with multifd
compression methods like zstd, zlib and just 'compress') 3. zero-page
(This will have sub capabilities ?)
I was thinking of keeping that part more open-ended. So not specifying
capabilities one by one, but more like "if you're testing a capability,
it comes here".
About multifd, it's a bit special since it cannot be seen as just a
"feature" anymore. It's a core part of the migration code. I wouldn't
classify it as capability for the purposes of the tests.
Ack, got it.
test-type :: migrate | validate
We could alternatively drop migration|migrate|validate. They are kind of
superfluous.
I agree with the above comment. 'migrate' and 'validate' have a different
set of variables required, some necessary, while other optional. IMO this
will help is in streamlining the design further.
migration-mode
a. migrate --> :: precopy | postcopy | multifd
b. validate --> :: (what to validate)
methods :: preempt | recovery | reboot | suspend | simple
I want some inputs here.
1. is there a better variable name rather than 'methods'
Does this fall into the "mode" terminology that Steven introduced?
Yes, as we decided that we don't want 'migration-mode' key-value pair,
naming 'mode' would be a better term.
In cases, where multiple modes are to be used ex: postcopy_preempt_recovery
I feel it might be a good idea to separate multiple modes by '-'
For example - .../preempty-recovery/...
Similarly for other keys too if required
2. 'simple' does not fit perfect here IMO.
Can we go without it?
You mean omit the key itself in case of a no-op ?
transport :: tcp | fd | unix | file
invocation :: uri | channels | both
CompressionType :: zlib | zstd | none
s/none/nocomp/ ? We're already familiar with that.
Ack. Will change that.
encryptionType :: tls | plain
s/plain/notls/ ?
What if there is another encryption technique in future ?
Or maybe we simply omit the noop options. It would make the string way
shorter in most cases.
This might be a better approach. Can have some keys/variables as optional
while some necessary. For ex: for 'migrate' - transport and invocation
might be necessary while it might not be necessary for 'validate' qtests
Yep
Ack, will do that!
validate-test-result :: success | failure
others :: other comments/capability that needs to be
addressed. Can be multiple
(more than one applicable, separated by using '-' in between)
O: optional
Signed-off-by: Het Gala<het.g...@nutanix.com>
Suggested-by: Fabiano Rosas<faro...@suse.de>
---
tests/qtest/migration-test.c | 143 ++++++++++++++++++-----------------
1 file changed, 72 insertions(+), 71 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index bd9f4b9dbb..bf4d000b76 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
Regards,
Het Gala
I'm wondering whether we should leave the existing tests untouched and
require the new format only for new tests. Going through a git bisection
with a change in the middle that alters test names would be infuriating.
Hmm yup. I had this doubt on, how would we be enforcing the new design
for any new qtests that gets added from now on ?
Can we have this design started for validation tests maybe for now, the
number is low and might get some feedback to improve this ?
Regards,
Het Gala
