[Qemu-devel] [PATCH -V5 28/28] virtio-9p: Change handling of flags in open() path for 9P2000.L

["Venkateswararao Jujjuri (JV)" <jvrao@linux.vnet.ibm.com>]

From: Sripathi Kodi <sripathik@in.ibm.com>

This patch applies on top of 9P2000.L patches that we have on the list.
I took a look at how 9P server is handling open() flags in 9P2000.L path.
I think we can do away with the valid_flags() function and simplify the
code. The reasoning is as follows:

O_NOCTTY: (If the file is a terminal, don't make it the controlling
terminal of the process even though the process does not have a controlling
terminal) By the time the control reaches 9P client it is clear that what
we have is not a terminal device. Hence it does not matter what we do with
this flag. In any case 9P server can filter this flag out before making the
syscall.

O_NONBLOCK: (Don't block if i) Can't read/write to the file ii) Can't get
locks) This has an impact on FIFOs, but also on file locks. Hence we can
pass it down to the system call.

O_ASYNC: From the manpage:

   O_ASYNC
          Enable signal-driven I/O: generate a signal (SIGIO by default,  but
          this  can be changed via fcntl(2)) when input or output becomes pos-
          sible on this file descriptor.  This feature is only available  for
          terminals,  pseudo-terminals,  sockets,  and (since Linux 2.6) pipes
          and FIFOs.  See fcntl(2) for further details.

Again, this does not make any impact on regular files handled by 9P. Also,
we don't want 9P server to receive SIGIO. Hence I think 9P server can
filter this flag out before making the syscall.

O_CLOEXEC: This flag makes sense only on the client. If guest user space
sets this flag the guest VFS will take care of calling close() on the fd if
an exec() happens. Hence 9P client need not be bothered with this flag.
Also I think QEMU will not do an exec, but if it does, it makes sense to
close these fds. Hence we can pass this flag down to the syscall.

O_CREAT: Since we are in open() path it means we have confirmed that the file
exists. Hence there is no need to pass O_CREAT flag down to the system. In fact
on some versions of glibc this causes problems, because we pass O_CREAT flag,
but don't have permission bits. Hence we can just mask this flag out.

So in summary:

Mask out:
O_NOCTTY
O_ASYNC
O_CREAT

Pass-through:
O_NONBLOCK
O_CLOEXEC

Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com>
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
 hw/virtio-9p.c |   14 +-------------
 1 files changed, 1 insertions(+), 13 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 4b15ce7..32fa3bc 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1684,15 +1684,6 @@ out:
     qemu_free(vs);
 }
 
-static inline int valid_flags(int flag)
-{
-    if (flag & O_NOCTTY || flag & O_NONBLOCK || flag & O_ASYNC ||
-            flag & O_CLOEXEC)
-        return 0;
-    else
-        return 1;
-}
-
 static void v9fs_open_post_lstat(V9fsState *s, V9fsOpenState *vs, int err)
 {
     int flags;
@@ -1709,11 +1700,8 @@ static void v9fs_open_post_lstat(V9fsState *s, V9fsOpenState *vs, int err)
         v9fs_open_post_opendir(s, vs, err);
     } else {
         if (s->proto_version == V9FS_PROTO_2000L) {
-            if (!valid_flags(vs->mode)) {
-                err = -EINVAL;
-                goto out;
-            }
             flags = vs->mode;
+            flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
         } else {
             flags = omode_to_uflags(vs->mode);
         }
-- 
1.6.5.2