[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [V2 06/25] [virtio-9p] coroutines for readlink
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [V2 06/25] [virtio-9p] coroutines for readlink |
Date: |
Wed, 18 May 2011 10:43:38 +0100 |
On Tue, May 17, 2011 at 8:43 PM, Venkateswararao Jujjuri (JV)
<address@hidden> wrote:
> Signed-off-by: Venkateswararao Jujjuri "<address@hidden>
> ---
> Makefile.objs | 2 +-
> hw/9pfs/cofs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> hw/9pfs/virtio-9p-coth.h | 1 +
> hw/9pfs/virtio-9p.c | 27 ++++-----------------------
> hw/9pfs/virtio-9p.h | 3 ++-
> 5 files changed, 50 insertions(+), 25 deletions(-)
> create mode 100644 hw/9pfs/cofs.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 96f6a24..36005bb 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
> 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o
> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o
> 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o
> virtio-9p-posix-acl.o
> -9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o
> +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o
>
> hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
> $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
> new file mode 100644
> index 0000000..6d94673
> --- /dev/null
> +++ b/hw/9pfs/cofs.c
> @@ -0,0 +1,42 @@
> +
> +/*
> + * Virtio 9p backend
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + * Aneesh Kumar K.V <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "fsdev/qemu-fsdev.h"
> +#include "qemu-thread.h"
> +#include "qemu-coroutine.h"
> +#include "virtio-9p-coth.h"
> +
> +int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf)
> +{
> + int err;
> + ssize_t len;
> + V9fsString tbuf;
> +
> + tbuf.data = qemu_malloc(PATH_MAX);
Why introduce tbuf when the buf is available? You end up having to
copy back fields at the end of the function and load from an
uninitialized address (tbuf.size) in the error case.
> @@ -299,7 +282,7 @@ static void v9fs_string_init(V9fsString *str)
> str->size = 0;
> }
>
> -static void v9fs_string_free(V9fsString *str)
> +void v9fs_string_free(V9fsString *str)
> {
> qemu_free(str->data);
> str->data = NULL;
> @@ -421,7 +404,7 @@ v9fs_string_sprintf(V9fsString *str, const char *fmt, ...)
> str->size = err;
> }
>
> -static void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs)
> +void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs)
> {
> v9fs_string_free(lhs);
> v9fs_string_sprintf(lhs, "%s", rhs->data);
Spurious changes? I don't see any users here.
> @@ -1057,8 +1040,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString
> *name,
> v9fs_string_null(&v9stat->extension);
>
> if (v9stat->mode & P9_STAT_MODE_SYMLINK) {
> - ssize_t symlink_len;
> - err = v9fs_do_readlink(s, name, &v9stat->extension, &symlink_len);
> + err = v9fs_co_readlink(s, name, &v9stat->extension);
> if (err < 0) {
> return err;
> }
> @@ -3591,7 +3573,6 @@ static void v9fs_readlink(void *opaque)
> int32_t fid;
> int err = 0;
> V9fsFidState *fidp;
> - ssize_t symlink_len;
>
> pdu_unmarshal(pdu, offset, "d", &fid);
> fidp = lookup_fid(pdu->s, fid);
> @@ -3601,7 +3582,7 @@ static void v9fs_readlink(void *opaque)
> }
>
> v9fs_string_init(&target);
> - err = v9fs_do_readlink(pdu->s, &fidp->path, &target, &symlink_len);
> + err = v9fs_co_readlink(pdu->s, &fidp->path, &target);
> if (err < 0) {
> goto out;
> }
Ah, the unused length argument from patch 5 has been removed. It
would be nice to do fixes earlier rather than later in the patch
series, otherwise time is spent reviewing and commenting on code which
gets removed.
Stefan
- [Qemu-devel] [V2 11/25] hw/9pfs: Add yield support to statfs coroutine, (continued)
- [Qemu-devel] [V2 11/25] hw/9pfs: Add yield support to statfs coroutine, Venkateswararao Jujjuri (JV), 2011/05/17
- [Qemu-devel] [V2 16/25] hw/9pfs: Update v9fs_setattr to use coroutines, Venkateswararao Jujjuri (JV), 2011/05/17
- [Qemu-devel] [V2 13/25] hw/9pfs: Add yield support to lstat coroutine, Venkateswararao Jujjuri (JV), 2011/05/17
- [Qemu-devel] [V2 18/25] hw/9pfs: Update v9fs_xattrwalk to coroutines, Venkateswararao Jujjuri (JV), 2011/05/17
- [Qemu-devel] [V2 07/25] [virtio-9p] Remove post functions for v9fs_mkdir., Venkateswararao Jujjuri (JV), 2011/05/17
- [Qemu-devel] [V2 15/25] hw/9pfs: Add yield support to setattr related coroutines, Venkateswararao Jujjuri (JV), 2011/05/17
- [Qemu-devel] [V2 10/25] hw/9pfs: Update v9fs_readdir to use coroutines, Venkateswararao Jujjuri (JV), 2011/05/17
- [Qemu-devel] [V2 19/25] hw/9pfs: Update v9fs_xattrcreate to use coroutines, Venkateswararao Jujjuri (JV), 2011/05/17
- [Qemu-devel] [V2 06/25] [virtio-9p] coroutines for readlink, Venkateswararao Jujjuri (JV), 2011/05/17
- Re: [Qemu-devel] [V2 06/25] [virtio-9p] coroutines for readlink,
Stefan Hajnoczi <=
[Qemu-devel] [V2 02/25] [virtio-9p] Change all pdu handlers to coroutines., Venkateswararao Jujjuri (JV), 2011/05/17
[Qemu-devel] [V2 17/25] hw/9pfs: Add yield support to xattr related coroutine, Venkateswararao Jujjuri (JV), 2011/05/17
[Qemu-devel] [V2 22/25] [virtio-9p] coroutine and threading for mkdir, Venkateswararao Jujjuri (JV), 2011/05/17
[Qemu-devel] [V2 21/25] hw/9pfs: Update v9fs_mknod to use coroutines, Venkateswararao Jujjuri (JV), 2011/05/17
[Qemu-devel] [V2 04/25] [virtio-9p] clean up v9fs_readlink., Venkateswararao Jujjuri (JV), 2011/05/17
[Qemu-devel] [V2 23/25] [virtio-9p] Remove post functions for v9fs_remove, Venkateswararao Jujjuri (JV), 2011/05/17