On Mon, Feb 20, 2023 at 11:13:45AM +0100, Juraj Linkeš wrote: > Thanks for the comments, Bruce. > > On Fri, Feb 17, 2023 at 6:26 PM Bruce Richardson > <bruce.richard...@intel.com> wrote: > > > > On Mon, Feb 13, 2023 at 04:28:36PM +0100, Juraj Linkeš wrote: > > > Add code needed to run the HelloWorld testcase which just runs the hello > > > world dpdk application. > > > > > > The patchset currently heavily refactors this original DTS code needed > > > to run the testcase: > > > * The whole architecture has been redone into more sensible class > > > hierarchy > > > * DPDK build on the System under Test > > > * DPDK eal args construction, app running and shutting down > > > * Optional SUT hugepage memory configuration > > > * Test runner > > > * Test results > > > * TestSuite class > > > * Test runner parts interfacing with TestSuite > > > * The HelloWorld testsuite itself > > > > > > The code is divided into sub-packages, some of which are divided > > > further. > > > > > > There patch may need to be divided into smaller chunks. If so, proposals > > > on where exactly to split it would be very helpful. > > > > > > v3: > > > Finished refactoring everything in this patch, with test suite and test > > > results being the last parts. > > > Also changed the directory structure. It's now simplified and the > > > imports look much better. > > > I've also many many minor changes such as renaming variables here and > > > there. > > > > > > v4: > > > Made hugepage config optional, users may now specify that in the main > > > config file. > > > Removed HelloWorld test plan and incorporated parts of it into the test > > > suite python file. > > > Updated documentation. > > > > > Hi, > > > > just trying this out by reading the docs and trying to follow along. Couple > > of high-level comments thus far without getting into the patches: > > > > * In the "configuring DTS" section, I think it would be good to: > > - say that the config file should be named conf.yaml by default. It's in > > the next section, but I think it should be called out earlier. > > - say that there is a template conf.yaml file in the dts directory > > already. On my first reading I actually thought that the sample config > > file was dts/framework/config/conf_yaml_schema.json (and I was going > > to comment on the name being weird! :-)). Only when I opened it did I > > realise my mistake. Therefore, downplan the schema, and put more > > emphasis on where to find the simple conf example to start with. > > Good points, I'll rewrite that a bit. > > > - if hugepage config is now optional, as you say above, remove that from > > the sample and the docs. > > > > The optional part is just that users may choose between DTS either > configuring the hugepages or not, but hugepages still must be > configured (if not by DTS, then beforehand). I'll document this a bit > more, but I'd like to leave it in the sample config with a note saying > it's optional. > > > * The code thus far seems to imply that you are always going to use root. > > When I configured it to log on to bruce@localhost, it timed out waiting > > for a prompt, I believe because it was looking for "#" which is the > > default only for root prompts. > > > > True, I'll add this to docs. This will not be a requirement in the > future though - we want to do passwordless sudo. > > > * When running as root, things progressed further but I hit an error when > > DTS was trying to get the CPU config. No idea what is happening here, > > because running the same commands manually over ssh seemed to work fine. > > Below is the error. Any hints as to what is the problem appreciated. > > > > I remember running into the same issue as well. I think it's related > to the bracketed paste feature of some terminal emulators: > https://askubuntu.com/questions/662222/why-bracketed-paste-mode-is-enabled-sporadically-in-my-terminal-screen > Please try disabling it and see whether that helps. > I haven't gone to great lengths to harden this part of SSH > implementation as we'll be moving to Fabric (from pexpect) after this > patch (which uses a mature Python SSH implementation instead of > expect). > Ok, thanks for the explanation, I'll try that out.
/Bruce
