Re: [Qemu-devel] [PATCH v7 0/3] 9p: Fix file ID collisions

[Greg Kurz <groug@kaod.org>]

On Mon, 23 Sep 2019 11:50:46 +0200
Christian Schoenebeck <qemu_oss@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/9295011c5a961603959b966c8aa6ad9840fe6db2
> 
> * 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.

> > 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. I usually rely on this:

https://www.tuxera.com/community/posix-test-suite/

> > 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.

> 
>