Re: [Qemu-devel] [RFC/PATCH] virtio-9p: Add Read only support for 9p export.

[M. Mohan Kumar]

On Mon, May 23, 2011 at 11:08:34AM +0100, Stefan Hajnoczi wrote:
> On Mon, May 23, 2011 at 8:58 AM, M. Mohan Kumar <address@hidden> wrote:
> > A new fsdev parameter "access" is introduced to control accessing 9p export.
> > access=ro|rw can be used to specify the access type. By default rw access
> > is given to 9p export.
> 
> It would be consistent with -drive to use readonly=on|off (default:
> off).  Do you have any future access modes in mind that require more
> than readonly on|off?

You mean to use 'readonly=on|off' instead of 'access=ro|rw' ?

> 
> It seems the client does not know ahead of time that the mount is
> read-only.  I wonder if this exposes any weirdness compared to
> actually mounting ro on the client side (i.e. operations that appear
> to start okay but then fail with -EROFS at a different point than if
> you had mounted ro)?  It could cause apps to error in new ways.
> 

When a NFS share is exported as read-only, client will come to know about
that when it does some write operation. IMHO its not an issue.

> > +static inline int is_ro_export(FsContext *fs_ctx)
> > +{
> > +        if (fs_ctx->flags & FS_RDONLY) {
> > +                return 1;
> > +        }
> > +        return 0;
> > +}
> 
> Please use bool and fix indentation:
Ok
> 
> static inline bool is_ro_export(FsContext *fs_ctx)
> {
>     return fs_ctx->flags & FS_RDONLY;
> }
> 
> > +
> >  static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
> >  {
> >     V9fsString version;
> > @@ -1734,6 +1742,13 @@ static void v9fs_open_post_lstat(V9fsState *s, 
> > V9fsOpenState *vs, int err)
> >         vs->fidp->fs.dir = v9fs_do_opendir(s, &vs->fidp->path);
> >         v9fs_open_post_opendir(s, vs, err);
> >     } else {
> > +        if (is_ro_export(&s->ctx)) {
> > +            if (vs->mode & O_WRONLY || vs->mode & O_RDWR ||
> > +                            vs->mode & O_APPEND) {
> > +                err = -EROFS;
> > +                goto out;
> > +            }
> > +        }
> >         if (s->proto_version == V9FS_PROTO_2000L) {
> >             flags = vs->mode;
> >             flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
> > @@ -3606,6 +3621,33 @@ static pdu_handler_t *pdu_handlers[] = {
> >     [P9_TREMOVE] = v9fs_remove,
> >  };
> >
> > +static inline int is_read_only_op(int id)
> 
> bool
Ok