On Mon, 23 Sep 2019 16:06:13 +0200 Christian Schoenebeck <qemu_...@crudebyte.com> wrote:
> On Montag, 23. September 2019 14:56:11 CEST Greg Kurz wrote: > > On Mon, 23 Sep 2019 11:50:46 +0200 > > > > Christian Schoenebeck <qemu_...@crudebyte.com> wrote: > > > On Freitag, 13. September 2019 19:01:57 CEST Greg Kurz wrote: > > > > So I did some changes in 1/3 and pushed everything to 9p-next. > > > > > > I've reviewed your changes. Some notes: > > > > > > Patch 1: > > > https://github.com/gkurz/qemu/commit/9295011c5a961603959b966c8aa6ad9840fe6 > > > db2> > > > * Typo 1: > > > error_append_hint(&local_err, "Valide options are: multidevs=" > > > > > > Valide -> Valid > > > > > > * Typo 2 in log comment: > > > [groug: - Moved "multidevs" parsing the local backend. > > > -> > > > [groug: - Moved "multidevs" parsing to the local backend. > > > > Fixed. > > Thanks! Saw it, looks fine now. > > > > > I'll do some > > > > more manual testing and issue a PR when I'm confident enough. > > > > > > That would be highly appreciated! So far I am the only one ever having > > > tested this patch set at all! > > > > Just to clarify, I won't thoroughly test it. My main concern is that it > > doesn't break things. > > So in other words you are only going to test the default behaviour > --multidevs=warn? > This I've already done, along with multidevs=forbid. Now I plan to run the PJD test suite from Tuxera with a simple cross-device setup and --multidevs=remap. And that's it. > If yes, and since that would mean I was the only person ever having tested > the > actual fix, shouldn't --multidevs=remap|forbid better be marked as > experimental (docs and runtime warning) for now? Maybe that would also > anticipate receiving feedback from people actually using it later on. > Makes sense. I don't think it is worth having a runtime warning, but I'll turn remap to x-remap and amend the docs. > > I usually rely on this: > > > > https://www.tuxera.com/community/posix-test-suite/ > > Well, that website is far from being too verbose of what that test suite > actually encompasses. > Yeah... this does a variety of tests I've never had time to audit, but it exercices many paths of the 9pfs code. > > > > It would be nice to have some sort of automated test for that in 'make > > > > check'. My first thought is to simulate a cross-device setup with the > > > > synth > > > > backend, because it might be difficult to do this on a real filesystem > > > > without requiring elevated privileges. > > > > > > Hmm, since I neither haven't used the synth backend before, nor added qemu > > > test cases so far, I am yet missing the complete picture here. My initial > > > suggested approach would have been using loopback devices for simulating > > > two file systems, but yes that's probably not viable due to required > > > permissions. How would the synth backend help here? I mean you would need > > > to simulate specific inode numbers and device numbers in some way for the > > > test cases. > > The synth backend allows to simulate anything you want, provided you > > code it of course :) > > > > It is currently used to run some 9p protocol conformance tests. Have a > > look at the backend code to get the idea. > > > > hw/9pfs/9p-synth.h > > hw/9pfs/9p-synth.c > > > > and the test program: > > > > tests/virtio-9p-test.c > > > > It currently doesn't care for st_dev/st_ino at all, but I guess > > it shouldn't be that hard to add the necessary bits. > > I see. Well, I will look at it, but that will definitely not be one of my > current high priority tasks that would happen in the next few weeks (simply > due to my work load). > Same here :)
