[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-9p: Make rpath function thread safe
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-9p: Make rpath function thread safe |
Date: |
Fri, 15 Apr 2011 10:42:21 +0100 |
On Fri, Apr 15, 2011 at 10:05 AM, M. Mohan Kumar <address@hidden> wrote:
> Use dynamically allocated memory to return concatenated path
> (fs_root and file path) instead of a static buffer. Caller has to free
> the memory.
>
> Signed-off-by: M. Mohan Kumar <address@hidden>
> ---
> This patch depends on my chroot patchset.
> http://patchwork.ozlabs.org/patch/89033/
>
> hw/9pfs/virtio-9p-local.c | 551
> ++++++++++++++++++++++++++-------------------
> hw/file-op-9p.h | 10 +-
> 2 files changed, 322 insertions(+), 239 deletions(-)
Would it be possible to build a const char *fs_ctx->path once in the
PDU handler for each operation that takes a path instead of pushing so
many rpath() calls down into virtio-9p-local.c? That way you don't
need to worry about constructing and freeing paths over and over.
I think it's time to kill errno once and for all in virtio-9p. The
errno global variable usage is getting out of hand; pretty much every
patch is adding boilerplate code to juggle around errno and avoid
trampling it. This new qemu_free() call means we need to juggle in
new places.
Please stop adding more errno juggling and just use -errno return
values. That change needs to happen before this patch.
> @@ -137,29 +164,10 @@ static int local_lstat(FsContext *fs_ctx, const char
> *path, struct stat *stbuf)
> }
> if (getxattr(rpath(fs_ctx, path), "user.virtfs.rdev", &tmp_dev,
> sizeof(dev_t)) > 0) {
Leak.
> tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
Leak.
> -static void local_rewinddir(FsContext *ctx, DIR *dir)
> +static void local_rewinddir(FsContext *fs_ctx, DIR *dir)
> {
> return rewinddir(dir);
> }
Please do this in a separate patch.
> err = local_set_xattr(rpath(fs_ctx, newpath), credp);
Leak.
Stefan