diff mbox series

[v4,14/32] tools/xen-9pfsd: add 9pfs read request support

Message ID 20240205105001.24171-15-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series tools: enable xenstore-stubdom to use 9pfs | expand

Commit Message

Jürgen Groß Feb. 5, 2024, 10:49 a.m. UTC
Add the read request of the 9pfs protocol.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
---
V2:
- make error check more readable (Jason Andryuk)
V4:
- add directory read support
---
 tools/xen-9pfsd/io.c | 90 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

Comments

Jason Andryuk Feb. 8, 2024, 1:28 a.m. UTC | #1
On Mon, Feb 5, 2024 at 5:51 AM Juergen Gross <jgross@suse.com> wrote:
>
> Add the read request of the 9pfs protocol.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> V2:
> - make error check more readable (Jason Andryuk)
> V4:
> - add directory read support
> ---
>  tools/xen-9pfsd/io.c | 90 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>
> diff --git a/tools/xen-9pfsd/io.c b/tools/xen-9pfsd/io.c
> index b763e3d8d9..cfbf3bef59 100644
> --- a/tools/xen-9pfsd/io.c
> +++ b/tools/xen-9pfsd/io.c

> +
> +    len = count;
> +    buf = ring->buffer + sizeof(*hdr) + sizeof(uint32_t);
> +
> +    if ( fidp->isdir )
> +    {
> +        struct dirent *dirent;
> +        struct stat st;
> +        struct p9_stat p9s;
> +

"""
For directories, read returns an integral number of direc- tory
entries exactly as in stat (see stat(5)), one for each member of the
directory. The read request message must have offset equal to zero or
the value of offset in the previous read on the directory, plus the
number of bytes returned in the previous read. In other words, seeking
other than to the beginning is illegal in a directory (see seek(2)).
"""

I think you need to check `off`.  Looks like QEMU only checks for off
== 0 and rewinds in that case.  QEMU ignores other values.

Regards,
Jason

> +        while ( len != 0 )
> +        {
> +            errno = 0;
> +            dirent = readdir(fidp->data);
> +            if ( !dirent )
> +            {
> +                if ( errno )
> +                    goto err;
> +                break;
> +            }
> +            if ( fstatat(fidp->fd, dirent->d_name, &st, 0) < 0 )
> +                goto err;
> +            fill_p9_stat(device, &p9s, &st, dirent->d_name);
> +            if ( p9s.size + sizeof(p9s.size) > len )
> +            {
> +                seekdir(fidp->data, dirent->d_off);
> +                break;
> +            }
> +            fill_buffer_at(&buf, "s", &p9s);
> +            len -= p9s.size + sizeof(p9s.size);
> +        }
> +    }
> +    else
> +    {
> +        while ( len != 0 )
> +        {
> +            ret = pread(fidp->fd, buf, len, off);
> +            if ( ret <= 0 )
> +                break;
> +            len -= ret;
> +            buf += ret;
> +            off += ret;
> +        }
> +        if ( ret < 0 && len == count )
> +            goto err;
> +    }
> +
> +    buf = ring->buffer + sizeof(*hdr) + sizeof(uint32_t);
> +    len = count - len;
> +    fill_buffer(ring, hdr->cmd + 1, hdr->tag, "D", &len, buf);
> +
> + out:
> +    free_fid(device, fidp);
> +
> +    return;
> +
> + err:
> +    p9_error(ring, hdr->tag, errno);
> +    goto out;
> +}
> +
Jürgen Groß Feb. 8, 2024, 9:04 a.m. UTC | #2
On 08.02.24 02:28, Jason Andryuk wrote:
> On Mon, Feb 5, 2024 at 5:51 AM Juergen Gross <jgross@suse.com> wrote:
>>
>> Add the read request of the 9pfs protocol.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
>> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>> V2:
>> - make error check more readable (Jason Andryuk)
>> V4:
>> - add directory read support
>> ---
>>   tools/xen-9pfsd/io.c | 90 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 90 insertions(+)
>>
>> diff --git a/tools/xen-9pfsd/io.c b/tools/xen-9pfsd/io.c
>> index b763e3d8d9..cfbf3bef59 100644
>> --- a/tools/xen-9pfsd/io.c
>> +++ b/tools/xen-9pfsd/io.c
> 
>> +
>> +    len = count;
>> +    buf = ring->buffer + sizeof(*hdr) + sizeof(uint32_t);
>> +
>> +    if ( fidp->isdir )
>> +    {
>> +        struct dirent *dirent;
>> +        struct stat st;
>> +        struct p9_stat p9s;
>> +
> 
> """
> For directories, read returns an integral number of direc- tory
> entries exactly as in stat (see stat(5)), one for each member of the
> directory. The read request message must have offset equal to zero or
> the value of offset in the previous read on the directory, plus the
> number of bytes returned in the previous read. In other words, seeking
> other than to the beginning is illegal in a directory (see seek(2)).
> """
> 
> I think you need to check `off`.  Looks like QEMU only checks for off
> == 0 and rewinds in that case.  QEMU ignores other values.

Oh, indeed. Thanks for noticing. I'll use the same approach as qemu.


Juergen
diff mbox series

Patch

diff --git a/tools/xen-9pfsd/io.c b/tools/xen-9pfsd/io.c
index b763e3d8d9..cfbf3bef59 100644
--- a/tools/xen-9pfsd/io.c
+++ b/tools/xen-9pfsd/io.c
@@ -33,6 +33,7 @@ 
 #define P9_CMD_WALK       110
 #define P9_CMD_OPEN       112
 #define P9_CMD_CREATE     114
+#define P9_CMD_READ       116
 #define P9_CMD_WRITE      118
 #define P9_CMD_CLUNK      120
 #define P9_CMD_STAT       124
@@ -1247,6 +1248,91 @@  static void p9_stat(struct ring *ring, struct p9_header *hdr)
     free_fid(device, fidp);
 }
 
+static void p9_read(struct ring *ring, struct p9_header *hdr)
+{
+    device *device = ring->device;
+    uint32_t fid;
+    uint64_t off;
+    unsigned int len;
+    uint32_t count;
+    void *buf;
+    struct p9_fid *fidp;
+    int ret;
+
+    ret = fill_data(ring, "ULU", &fid, &off, &count);
+    if ( ret != 3 )
+    {
+        p9_error(ring, hdr->tag, EINVAL);
+        return;
+    }
+
+    fidp = get_fid_ref(device, fid);
+    if ( !fidp || !fidp->opened )
+    {
+        errno = EBADF;
+        goto err;
+    }
+
+    len = count;
+    buf = ring->buffer + sizeof(*hdr) + sizeof(uint32_t);
+
+    if ( fidp->isdir )
+    {
+        struct dirent *dirent;
+        struct stat st;
+        struct p9_stat p9s;
+
+        while ( len != 0 )
+        {
+            errno = 0;
+            dirent = readdir(fidp->data);
+            if ( !dirent )
+            {
+                if ( errno )
+                    goto err;
+                break;
+            }
+            if ( fstatat(fidp->fd, dirent->d_name, &st, 0) < 0 )
+                goto err;
+            fill_p9_stat(device, &p9s, &st, dirent->d_name);
+            if ( p9s.size + sizeof(p9s.size) > len )
+            {
+                seekdir(fidp->data, dirent->d_off);
+                break;
+            }
+            fill_buffer_at(&buf, "s", &p9s);
+            len -= p9s.size + sizeof(p9s.size);
+        }
+    }
+    else
+    {
+        while ( len != 0 )
+        {
+            ret = pread(fidp->fd, buf, len, off);
+            if ( ret <= 0 )
+                break;
+            len -= ret;
+            buf += ret;
+            off += ret;
+        }
+        if ( ret < 0 && len == count )
+            goto err;
+    }
+
+    buf = ring->buffer + sizeof(*hdr) + sizeof(uint32_t);
+    len = count - len;
+    fill_buffer(ring, hdr->cmd + 1, hdr->tag, "D", &len, buf);
+
+ out:
+    free_fid(device, fidp);
+
+    return;
+
+ err:
+    p9_error(ring, hdr->tag, errno);
+    goto out;
+}
+
 static void p9_write(struct ring *ring, struct p9_header *hdr)
 {
     device *device = ring->device;
@@ -1371,6 +1457,10 @@  void *io_thread(void *arg)
                 p9_create(ring, &hdr);
                 break;
 
+            case P9_CMD_READ:
+                p9_read(ring, &hdr);
+                break;
+
             case P9_CMD_WRITE:
                 p9_write(ring, &hdr);
                 break;