Message ID | 1320731717-903-1-git-send-email-david@gibson.dropbear.id.au |
---|---|
State | New |
Headers | show |
On Tue, 8 Nov 2011 16:55:17 +1100, David Gibson <david@gibson.dropbear.id.au> wrote: > From: Timothy Rule <trule@linux.vnet.ibm.com> > > The 9P spec states that for the stat message the "stat[n]" structure shall be > encoded at offset 7 in the 9P message (see §13.9 message Rstat). > > The existing code is encoding a 2 byte value (hard coded 0 value) at > offset 7 of the 9P message, and then follows with the "stat[n]" structure > at offset 9 of the 9P message. > > This patch removes the encoding of the 2 byte value which has the effect > of moving the "stat[n]" structure from offset 9 to offset 7 in the 9P > message Rstat. > > Signed-off-by: Timothy Rule <trule@linux.vnet.ibm.com> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/9pfs/virtio-9p.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c > index 01cf337..35d8851 100644 > --- a/hw/9pfs/virtio-9p.c > +++ b/hw/9pfs/virtio-9p.c > @@ -1279,7 +1279,7 @@ static void v9fs_stat(void *opaque) > if (err < 0) { > goto out; > } > - offset += pdu_marshal(pdu, offset, "wS", 0, &v9stat); > + offset += pdu_marshal(pdu, offset, "S", &v9stat); > err = offset; > trace_v9fs_stat_return(pdu->tag, pdu->id, v9stat.mode, > v9stat.atime, v9stat.mtime, v9stat.length); The reason for that "w" is explained in the 9p2000 protocol RFC. Towards the end of "wstat" message para we have BUGS 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. Also on the client side we do in p9_client_stat err = p9pdu_readf(req->rc, clnt->proto_version, "wS", &ignored, ret); if (err) { So the above change will break existing client. Any reason why you need to make the above change ? -aneesh
On 11/08/2011 02:39 PM, Aneesh Kumar K.V wrote: > On Tue, 8 Nov 2011 16:55:17 +1100, David Gibson<david@gibson.dropbear.id.au> wrote: >> From: Timothy Rule<trule@linux.vnet.ibm.com> >> >> The 9P spec states that for the stat message the "stat[n]" structure shall be >> encoded at offset 7 in the 9P message (see §13.9 message Rstat). >> >> The existing code is encoding a 2 byte value (hard coded 0 value) at >> offset 7 of the 9P message, and then follows with the "stat[n]" structure >> at offset 9 of the 9P message. >> >> This patch removes the encoding of the 2 byte value which has the effect >> of moving the "stat[n]" structure from offset 9 to offset 7 in the 9P >> message Rstat. >> >> Signed-off-by: Timothy Rule<trule@linux.vnet.ibm.com> >> Signed-off-by: David Gibson<david@gibson.dropbear.id.au> >> --- >> hw/9pfs/virtio-9p.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c >> index 01cf337..35d8851 100644 >> --- a/hw/9pfs/virtio-9p.c >> +++ b/hw/9pfs/virtio-9p.c >> @@ -1279,7 +1279,7 @@ static void v9fs_stat(void *opaque) >> if (err< 0) { >> goto out; >> } >> - offset += pdu_marshal(pdu, offset, "wS", 0,&v9stat); >> + offset += pdu_marshal(pdu, offset, "S",&v9stat); >> err = offset; >> trace_v9fs_stat_return(pdu->tag, pdu->id, v9stat.mode, >> v9stat.atime, v9stat.mtime, v9stat.length); > The reason for that "w" is explained in the 9p2000 protocol RFC. Towards > the end of "wstat" message para we have > > BUGS > > 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. > > Also on the client side we do in p9_client_stat > > err = p9pdu_readf(req->rc, clnt->proto_version, "wS",&ignored, ret); > if (err) { > > So the above change will break existing client. > > Any reason why you need to make the above change ? > > -aneesh It would be fine for us to implement our solution that way. I apologise for the inconvenience, I wrote this particular function against the 9P2000.u specification which does not mention the Bug. Tim.
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index 01cf337..35d8851 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -1279,7 +1279,7 @@ static void v9fs_stat(void *opaque) if (err < 0) { goto out; } - offset += pdu_marshal(pdu, offset, "wS", 0, &v9stat); + offset += pdu_marshal(pdu, offset, "S", &v9stat); err = offset; trace_v9fs_stat_return(pdu->tag, pdu->id, v9stat.mode, v9stat.atime, v9stat.mtime, v9stat.length);