[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions |
Date: |
Mon, 23 Sep 2019 16:46:53 +0200 |
On Mon, 23 Sep 2019 16:06:13 +0200
Christian Schoenebeck <address@hidden> wrote:
> On Montag, 23. September 2019 14:56:11 CEST Greg Kurz wrote:
> > On Mon, 23 Sep 2019 11:50:46 +0200
> >
> > Christian Schoenebeck <address@hidden> 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 :)
- [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions, Christian Schoenebeck, 2019/09/05
- [Qemu-devel] [PATCH v7 1/3] 9p: Added virtfs option 'multidevs=remap|forbid|warn', Christian Schoenebeck, 2019/09/05
- [Qemu-devel] [PATCH v7 3/3] 9p: Use variable length suffixes for inode remapping, Christian Schoenebeck, 2019/09/05
- [Qemu-devel] [PATCH v7 2/3] 9p: stat_to_qid: implement slow path, Christian Schoenebeck, 2019/09/05
- Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions, Greg Kurz, 2019/09/13
- Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions, Christian Schoenebeck, 2019/09/23
- Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions, Greg Kurz, 2019/09/23
- Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions, Christian Schoenebeck, 2019/09/23
- Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions,
Greg Kurz <=
- Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions, Christian Schoenebeck, 2019/09/23
- Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions, Greg Kurz, 2019/09/23
- Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions, Christian Schoenebeck, 2019/09/24