qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@gmail.com>
To: Linda <lindaj@jma3.com>
Cc: Julien Grall <julien.grall@citrix.com>,
	Wei Liu <wei.liu2@citrix.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] virtio-9p
Date: Mon, 10 Aug 2015 11:10:18 +0100	[thread overview]
Message-ID: <20150810101018.GC31433@stefanha-thinkpad.redhat.com> (raw)
In-Reply-To: <55C4DB1B.1030304@jma3.com>

[-- Attachment #1: Type: text/plain, Size: 3723 bytes --]

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

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.

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

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

  reply	other threads:[~2015-08-10 10:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-07 16:21 [Qemu-devel] virtio-9p Linda
2015-08-10 10:10 ` Stefan Hajnoczi [this message]
2015-08-10 13:20   ` Linda
  -- strict thread matches above, loose matches on Subject: below --
2016-03-30 12:10 [Qemu-devel] Virtio-9p Pradeep Kiruvale
2016-03-30 14:13 ` Greg Kurz
2016-03-30 14:27   ` Pradeep Kiruvale
2016-03-31 16:12     ` Greg Kurz
     [not found]       ` <CAJ2SuLm3U354OOKhaEEH-m_HuoAbPEK4ib-H0hvR7Pn296offA@mail.gmail.com>
2016-04-01  9:35         ` Greg Kurz
2016-04-04 14:39           ` Pradeep Kiruvale

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150810101018.GC31433@stefanha-thinkpad.redhat.com \
    --to=stefanha@gmail.com \
    --cc=julien.grall@citrix.com \
    --cc=lindaj@jma3.com \
    --cc=qemu-devel@nongnu.org \
    --cc=wei.liu2@citrix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).