On Fri, Apr 12, 2024 at 11:58:43AM -0300, Fabiano Rosas wrote: > Peter Xu <pet...@redhat.com> writes: > > > On Thu, Apr 11, 2024 at 04:41:16PM -0300, Fabiano Rosas wrote: > >> Peter Xu <pet...@redhat.com> writes: > >> > >> > On Thu, Apr 11, 2024 at 11:31:08PM +0530, Het Gala wrote: > >> >> I just wanted to highlight couple of pointers: > >> >> 1. though we are using 'channels' in the precopy tests for 'migrate' > >> >> QAPI, > >> >> we > >> >> use the old uri for 'migrate-incoming' QAPI. > >> >> 2. We do not cover other 'channels' abi, only have tcp path tested. > >> >> > >> >> So, the TO-DOs could be: > >> >> 1. Omit the 4th patch here, which introduced postcopy qtests with > >> >> 'channels' > >> >> interface OR have 'channels' interface with other than tcp transport > >> >> (file, exec, vsock, etc) so as to cover different code paths. > >> >> 2. Extend channels interface to migrate-incoming QAPI for precopy qtests > >> > > >> > You can see whether Fabiano has anything to say, but what you proposed > >> > looks good to me. > >> > >> Ok, so what about we convert some of the 'plain' tests into channels to > >> cover all transports? > >> > >> - tcp: test_multifd_tcp_none (this one we already did) > >> - file: test_precopy_file > >> - unix: test_precopy_unix_plain > >> - exec: test_analyze_script > >> - fd: test_migrate_precopy_fd_socket > >> > >> Those^, plus the validate_uri that's already in next should cover > >> everything. > >> > >> We don't need to do this at once, by the way. > >> > >> Moreover: > >> > >> - leave all test strings untouched to preserve bisecting; > >> > >> - let's not bother adding "channels" and "uri" to the test string > >> anymore. The channels API should be taken for granted at this point, I > >> don't expect we start hitting bugs that will require us to run either > >> foo/uri/plain or foo/channels/plain, so there's not much point in > >> making the distinction. > > > > Do you mean we can put "uri:" aside? Maybe I misunderstood.. > > I mean the test name does not need to specify "channels" vs. "uri" > because that should never be broken to the point that we actually need > to go fetch those tests by name. We'd still have at least 1 test for > each transport with channels and (existing) at least 1 test for each > transport with uri. > > > > > The matrix previously was (I think.. when this series posted): > > > > [tcp, unix, file, exec, fd] x [uri, channels] x [precopy, postcopy] > > > > Drop postcopy as doesn't seem to have any special paths: > > > > [tcp, unix, file, exec, fd] x [uri, channels] > > > > So logically we should still cover these, right? > > Right, I'm just suggesting we convert some tests to use channels, one > for each transport, to test the channels API in full. The rest of the > existing tests as well as future tests need not have a uri (or channel) > variant. Just one of them is enough.
Ah so that's the "test string"; sounds all good. -- Peter Xu
