Re: [PATCH 2/2] kvm tools: Add virtio-9p

[Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>]

On 05/18/2011 02:05 AM, Sasha Levin wrote:
> On Tue, 2011-05-17 at 20:18 -0500, Eric Van Hensbergen wrote:
>> On Tue, May 17, 2011 at 3:27 PM, Sasha Levin<levinsasha928@gmail.com>  wrote:
>>> On Tue, 2011-05-17 at 22:08 +0300, Sasha Levin wrote:
>>>> 'kvm_9p' isn't created as a device under /dev, it's just a name used
>>>> internally by 9pnet_virtio (and located under sysfs).
>>>>
>>>> I couldn't figure out which params the kernel would expect to boot using
>>>> 9p over virtio (theres no device name to begin with).
>>>>
>>>> I've also couldn't find anything that suggested it's possible to boot
>>>> using virtio-9p as rootfs.
>>> Ignore that.
>>>
>>> Naming the virtio transport "/dev/root" and passing proper params to the
>>> kernel makes it work:
>>>
>>> [    1.844983] VFS: Mounted root (9p filesystem) on device 0:11.
>>>
>>> I'll make some changes to the virtio-9p patch to make it easier for the
>>> user to do that.
>>>
>> This is really sweet.  Thanks for beating me to the punch of porting
>> the 9p support to kvm tools.
> Clear RFC and good source code to refer to within 9p modules made this
> easy (and fun) :)
>
>>> - Multiple virtio-9p devices.
>> This should be pretty straightforward.
> Yes, Most of the work here is within the kvm tool.
>
>>> - Ugly hack in virtio_p9_stat() (See desc in code).
>>>        /*
>>> +        * HACK: For some reason the p9 virtio transport reads a u16 and discards
>>> +        * it before reading the p9_rstat struct. I couldn't find a logical reason for
>>> +        * that, so we just add an extra u16 before the struct.
>>> +        */
>> This is part of the protocol spec (from
>> http://ericvh.github.com/9p-rfc/rfc9p2000.html#anchor32):
>> "To make the contents of a directory, such as returned by read(5),
>> easy to parse, each directory entry begins with a size field. For
>> consistency, the entries in Twstat and Rstat messages also contain
>> their size, which means the size appears twice. For example, the Rstat
>> message is formatted as ``(4+1+2+2+n)[4] Rstat tag[2] n[2] (n-2)[2]
>> type[2] dev[4]...,'' where n is the value returned by convD2M."
>> It's appropriate to duplicate the size.  I think the Linux client
>> ignores it, but others implementations may complain.
> Thanks for the explanation!
> Yes, Linux implementation just throws it away - which was what confused
> me initially.
> Why not add a u16 to the beginning of 'struct p9_rstat'?
>
>>> - Update atime/mtime in p9_wstat, not really needed.
>> The underlying storage may handle this for you, I think 9p avoids
>> updating atime by default, at least in caching scenarios -- too much
>> unnecessary protocol traffic.
> My assumption was that the storage I read/write to will take care of it
> for me, and unless it bothers anyone in the future I'll assume it's
> doing a good job at it.
>
>>> - Pass usernames in p9_stat, not really needed and not really sure how p9 expects to handle them.
>> The username, group name issue is one of the principle reasons behind
>> the extended protocol operations (.u and .L) -- of course, if there
>> was a Plan 9 or Inferno guest they would be quite happy with the
>> usernames, but Linux (and other UNIX variants) will want the ids.  To
>> really keep things simple we could add a client option that would let
>> you pass the various ids as strings.  Although no doubt folks will
>> want the other extensions (symlinks, links, device nodes, etc.) before
>> long.  When we built the qemu server for .L, the team tried to keep
>> everything in a library, but there is some entanglement with the qemu
>> APIs -- it'd be nice if we could reuse that code here, maybe we need
>> an abstract glue layer so that the core code can be used by both the
>> kvm tool and qemu.  I'm copy the lead of that team on this message
>> just so he's aware how far you've come.
> I'd prefer using a tested lib which also implements .L over what we have
> now, assuming it's not tangled into qemu too hard.
>
Very interesting to see we have 9p in the KVM tool which we can use as 
root dev.
I think this is an excellent start.

We have been working on adding 9P2000.L support to both kernel and QEMU.
QEMU code is not tangled and it can be easily reused. Well easy to say. :)
The current mainline QEMU code is written in the state machine model we 
are working
on a series of patches to make use of coroutines which make the 
functions look
lot logical. You may look at the current dev branch here:
http://repo.or.cz/w/qemu/aliguori/jvrao.git/shortlog/refs/heads/9p-coroutine-bh

Thanks,
JV


>>           -eric
>