Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190821173742.24574-1-vgoyal@redhat.com> <CAJfpegvPTxkaNhXWhiQSprSJqyW1cLXeZEz6x_f0PxCd-yzHQg@mail.gmail.com>
 <20190903041507-mutt-send-email-mst@kernel.org>
In-Reply-To: <20190903041507-mutt-send-email-mst@kernel.org>
From:   Miklos Szeredi <miklos@szeredi.hu>
Date:   Tue, 3 Sep 2019 11:17:35 +0200
Message-ID: <CAJfpeguB6fFhghuFS420ZQ+JuQvTLc5TgsGjoB_RvFrSVf+v5w@mail.gmail.com>
Subject: Re: [PATCH v3 00/13] virtio-fs: shared file system for virtual machines
To:     "Michael S. Tsirkin" <mst@redhat.com>
Cc:     Vivek Goyal <vgoyal@redhat.com>, Jason Wang <jasowang@redhat.com>,
        virtualization@lists.linux-foundation.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        virtio-fs@redhat.com, Stefan Hajnoczi <stefanha@redhat.com>,
        "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Sep 3, 2019 at 10:31 AM Michael S. Tsirkin <mst@redhat.com> wrote:

> Can the patches themselves be posted to the relevant list(s) please?
> If possible, please also include "v3" in all patches so they are
> easier to find.

I'll post a v4.

> I poked at
> https://lwn.net/ml/linux-kernel/20190821173742.24574-1-vgoyal@redhat.com/
> specifically
> https://lwn.net/ml/linux-kernel/20190821173742.24574-12-vgoyal@redhat.com/
> and things like:
> +       /* TODO lock */
> give me pause.
>
> Cleanup generally seems broken to me - what pauses the FS
>
> What about the rest of TODOs in that file?

AFAICS, some of those are QOI issues, and some are long term
performance items.  I see no blockers or things that would pose a
security concern.

That said, it would be nice to get rid of them at some point.

> use of usleep is hacky - can't we do better e.g. with a
> completion?

Yes.  I have a different implementation, but was planning to leave
this one in until the dust settles.

> Some typos - e.g. "reuests".

Fixed.

> > >  fs/fuse/Kconfig                 |   11 +
> > >  fs/fuse/Makefile                |    1 +
> > >  fs/fuse/control.c               |    4 +-
> > >  fs/fuse/cuse.c                  |    4 +-
> > >  fs/fuse/dev.c                   |   89 ++-
> > >  fs/fuse/dir.c                   |   26 +-
> > >  fs/fuse/file.c                  |   15 +-
> > >  fs/fuse/fuse_i.h                |  120 +++-
> > >  fs/fuse/inode.c                 |  203 +++---
> > >  fs/fuse/virtio_fs.c             | 1061 +++++++++++++++++++++++++++++++
> > >  fs/splice.c                     |    3 +-
> > >  include/linux/fs.h              |    2 +
> > >  include/uapi/linux/virtio_fs.h  |   41 ++
> > >  include/uapi/linux/virtio_ids.h |    1 +
> > >  init/do_mounts.c                |   10 +
> > >  15 files changed, 1462 insertions(+), 129 deletions(-)
> > >  create mode 100644 fs/fuse/virtio_fs.c
> > >  create mode 100644 include/uapi/linux/virtio_fs.h
>
> Don't the new files need a MAINTAINERS entry?
> I think we want virtualization@lists.linux-foundation.org to be
> copied.

Yep.

Stefan, do you want to formally maintain this file?

Thanks,
Miklos