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. > >>> 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? > 2. 'simple' does not fit perfect here IMO. Can we go without it? >>> 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 >>> 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.
