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

[Eric Van Hensbergen <ericvh@gmail.com>]

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.

> - Multiple virtio-9p devices.

This should be pretty straightforward.

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

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

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

         -eric