Re: [PATCH] tools/kvm/9p: Add encode/decode routines for protocol data

Aneesh Kumar K.V
2011-06-28T06:39:44Z

On Fri, 24 Jun 2011 19:26:48 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Fri, 2011-06-24 at 21:30 +0530, Aneesh Kumar K.V wrote:
> > On Fri, 24 Jun 2011 09:47:18 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> > > On Tue, 2011-06-21 at 15:50 +0530, Aneesh Kumar K.V wrote:
> > > > +static void virtio_p9_pdu_read(struct p9_pdu *pdu, void *data, size_t size)
> > > > +{
> > > > + size_t len;
> > > > + int i, copied = 0;
> > > > + u16 iov_cnt = pdu->out_iov_cnt;
> > > > + size_t offset = pdu->read_offset;
> > > > + struct iovec *iov = pdu->out_iov;
> > > > +
> > > > + for (i = 0; i < iov_cnt && size; i++) {
> > > > + if (offset >= iov[i].iov_len) {
> > > > + offset -= iov[i].iov_len;
> > > > + continue;
> > > > + } else {
> > > > + len = MIN(iov[i].iov_len - offset, size);
> > > > + memcpy(data, iov[i].iov_base + offset, len);
> > > > + size -= len;
> > > > + data += len;
> > > > + offset = 0;
> > > > + copied += len;
> > > > + }
> > > > + }
> > > > + pdu->read_offset += copied;
> > > > +}
> > > > +
> > > > +static void virtio_p9_pdu_write(struct p9_pdu *pdu,
> > > > + const void *data, size_t size)
> > > > +{
> > > > + size_t len;
> > > > + int i, copied = 0;
> > > > + u16 iov_cnt = pdu->in_iov_cnt;
> > > > + size_t offset = pdu->write_offset;
> > > > + struct iovec *iov = pdu->in_iov;
> > > > +
> > > > + for (i = 0; i < iov_cnt && size; i++) {
> > > > + if (offset >= iov[i].iov_len) {
> > > > + offset -= iov[i].iov_len;
> > > > + continue;
> > > > + } else {
> > > > + len = MIN(iov[i].iov_len - offset, size);
> > > > + memcpy(iov[i].iov_base + offset, data, len);
> > > > + size -= len;
> > > > + data += len;
> > > > + offset = 0;
> > > > + copied += len;
> > > > + }
> > > > + }
> > > > + pdu->write_offset += copied;
> > > > +}
> > > > +
> > >
> > > Just wondering here, can we use pipes (read/write/vmsplice) instead of a
> > > scatter-gather list?
> >
> > Can you elaborate on this. The function is to encode/decode data out of
> > protocol buffer.
>
> I was just thinking about storing the data in a pipe instead of a
> scatter-gather list.

We are building that scatter-gather list from vring. I am not clear
how to push that to pipe ? ie we have iovec in vring and how do i
store that to a pipe ?

>
> This would turn virtio_p9_pdu_read() into a simple wrapper for read()
> from a pipe and virtio_p9_pdu_write() into a wrapper for write().
>
> Something like this:
>
> void virtio_9p__init(struct kvm *kvm, const char *root, const char
> *tag_name)
> {
> [...]
> pipe(p9_pipe);
> [...]
> }
>
> static void virtio_p9_pdu_read(struct p9_pdu *pdu, void *data, size_t
> size)
> {
> read(p9_pipe[0], data, size);
> }
>
> static void virtio_p9_pdu_write(struct p9_pdu *pdu, const void *data,
> size_t size)
> {
> write(p9_pipe[1], data, size);
> }
>

But why ? what we currently have is a simple memcpy, why replace that
with syscall read/write ?

-aneesh