Re: [Qemu-devel] virtio-9p

[Linda <lindaj@jma3.com>]

On 8/10/2015 4:10 AM, Stefan Hajnoczi wrote:
> On Fri, Aug 07, 2015 at 10:21:47AM -0600, Linda wrote:
>>      As background, for the backend, I have been looking at the code, 
>> written
>> by Anthony Liguori, and maintained by Aneesh Kumar (who I sent this 
>> email
>> to, earlier).  It looks to me (please correct me if I'm wrong, on 
>> this or
>> any other point, below) as if Anthony wrote not just a backend transport
>> layer, but the server as well.  AFAICT, there is no other Linux 9p 
>> server.
> There are other Linux 9P servers.  At least there is diod:
> https://github.com/chaos/diod
Thank you.  I will look into that.
> Anthony Liguori didn't write all of the virtio-9p code in QEMU.  Aneesh
> Kumar, JV Rao, M. Mohan Kumar, and Harsh Prateek Bora did a lot of the
> 9P server development in QEMU.
>
> Take a look at git shortlog -nse hw/9pfs
>
>>      virtio-9p.c contains a lot of this server code, the rest spread 
>> between
>> 13 other files which handle all file access operations, converting 
>> them from
>> 9p to Linux file system calls.
>>      virtio-9p.c also contains some virtio-specific code (although 
>> most of
>> that is in virtio-device.c).
>>
>> The problems I am encountering are the following:
>>
>> 1.  In the virio-9p.h is a struct V9fsPDU that contains an element 
>> (in the
>> middle of the struct) of type VirtQueueElement. Every 9p I/O command
>> handler, as well as co-routines and support functions that go with them
>> (i.e., a large part of the server), passes a parameter that is a struct
>> V9fsPDU.   Almost all of these use only the variable that defines state
>> information, and never touch the VirtQueueElement member.
>>      The easiest fix for this is to have a separate header file with a
>> #define GENERIC_9P_SERVER
>>      Then I could modify the virtio-9p.h with:
>>              #ifdef GENERIC_9P_SERVER
>>                     a union of a void *, a char * (what I use), and a
>> VirtQueueElement (guaranteeing the size is unchanged)
>>              #else
>>                      VirtQueueElement    elem;
>>              #endif
>>
>>      It's not my favorite construct, but it would involve the least 
>> amount of
>> changes to the code.   Before I modify a header file, that code, I'm not
>> touching, is dependent on, I wanted to know if this is an OK way.  If 
>> not,
>> is there another way (short of copying fourteen files, and changing the
>> names of all the functions in them, as well as the file names), that you
>> would prefer?
> What is the goal of your project?
>
> If you just want a Linux 9P server, use diod.  You might be able to find
> other servers that suit your needs better too (e.g. programming
> language, features, etc).
>
> An #ifdef is ugly and if you are going to submit code upstream then a
> cleaner solution should be used.  Either separate a VirtIO9fsPDU struct
> that contains the generic 9pfsPDU as a field (so that container_of() can
> be used to go from 9pfsPDU back to VirtIO9fsPDU).  Or add a void* into
> the generic 9pfsPDU so transports can correlate the generic struct with
> a transport-specific struct.
I agree about ifdefs being ugly.  I guess I was just trying to save 
space - all I did was add a void * and a pointer to a function
>
>>      2.  The other problem, is that most of the "server" functions 
>> described
>> above, end by calling complete_pdu.   Complete_pdu (which is defined in
>> virtio-9p.c) does many things that are generic, and also a few virito
>> specific operations (pushing to the virtqueue, etc.).
>>      Again, I can use a similar mechanism to the above.  Or is there 
>> some
>> other way you'd prefer? I'm trying to find a way that has the least 
>> impact
>> on virtio/qemu maintainers.
> The generic PDU struct could have a .complete() function pointer. This
> is how the SCSI subsystem works, for example.  scsi_req_complete() calls
> req->bus->info->complete(req, req->status, req->resid) so that the
> bus-specific completion behavior is invoked.
It has a function pointer to a function complete that returns a pointer 
to a Coroutine.   But it uses (in dozens of places) a straight call to a 
complete function.

I will look into diod.   If there are any problems with it, I'll take 
your suggestions above.

Thanks.

Linda
>
> Stefan