qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [PATCH 50/51] 9pfs-proxy: remove one half of redundrand cod


From: Michael Tokarev
Subject: [Qemu-devel] [PATCH 50/51] 9pfs-proxy: remove one half of redundrand code
Date: Fri, 6 Mar 2015 23:19:35 +0300

The 9pfs-proxy code is actually terrible: it does the same
things again and again, ignoring function arguments and
re-inventing the same values again, and a lot of code must
be consistent with each other.

In particular, lots of filesystem methods use common v9fs_request()
function and pass all method args together with pack string to it.
However, v9fs_request() just ignores this, pop the values out of
stack and sets pack string once more.  This is sort of absurd.

This patch removes per-request-type argument marshalling from
v9fs_request() and keeps it only in the individual request
handling methods.

What's left is to do something similar with receiving response,
v9fs_receive_response() function.

Signed-off-by: Michael Tokarev <address@hidden>
---
 hw/9pfs/virtio-9p-proxy.c | 275 +++-------------------------------------------
 1 file changed, 14 insertions(+), 261 deletions(-)

diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index c187d31..aa9659a 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -291,22 +291,13 @@ static int v9fs_receive_status(V9fsProxy *proxy,
 /*
  * Proxy->header and proxy->request written to socket by QEMU process.
  * This request read by proxy helper process
- * returns 0 on success and -errno on error
+ * returns 0 on success and -1 (setting errno) on error
  */
 static int v9fs_request(V9fsProxy *proxy, int type,
                         void *response, const char *fmt, ...)
 {
-    dev_t rdev;
     va_list ap;
-    int size = 0;
-    int retval = 0, err;
-    uint64_t offset;
-    ProxyHeader header = { 0, 0};
-    struct timespec spec[2];
-    int flags, mode, uid, gid;
-    V9fsString *name, *value;
-    V9fsString *path, *oldpath;
-    struct iovec *iovec = NULL, *reply = NULL;
+    int retval, err;
 
     qemu_mutex_lock(&proxy->mutex);
 
@@ -315,218 +306,8 @@ static int v9fs_request(V9fsProxy *proxy, int type,
         goto out;
     }
 
-    iovec = &proxy->out_iovec;
-    reply = &proxy->in_iovec;
     va_start(ap, fmt);
-    switch (type) {
-    case T_OPEN:
-        path = va_arg(ap, V9fsString *);
-        flags = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sd", path, flags);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_OPEN;
-        }
-        break;
-    case T_CREATE:
-        path = va_arg(ap, V9fsString *);
-        flags = va_arg(ap, int);
-        mode = va_arg(ap, int);
-        uid = va_arg(ap, int);
-        gid = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sdddd", path,
-                                    flags, mode, uid, gid);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_CREATE;
-        }
-        break;
-    case T_MKNOD:
-        path = va_arg(ap, V9fsString *);
-        mode = va_arg(ap, int);
-        rdev = va_arg(ap, long int);
-        uid = va_arg(ap, int);
-        gid = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "ddsdq",
-                                    uid, gid, path, mode, rdev);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_MKNOD;
-        }
-        break;
-    case T_MKDIR:
-        path = va_arg(ap, V9fsString *);
-        mode = va_arg(ap, int);
-        uid = va_arg(ap, int);
-        gid = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "ddsd",
-                                    uid, gid, path, mode);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_MKDIR;
-        }
-        break;
-    case T_SYMLINK:
-        oldpath = va_arg(ap, V9fsString *);
-        path = va_arg(ap, V9fsString *);
-        uid = va_arg(ap, int);
-        gid = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "ddss",
-                                    uid, gid, oldpath, path);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_SYMLINK;
-        }
-        break;
-    case T_LINK:
-        oldpath = va_arg(ap, V9fsString *);
-        path = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "ss",
-                                    oldpath, path);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_LINK;
-        }
-        break;
-    case T_LSTAT:
-        path = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "s", path);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_LSTAT;
-        }
-        break;
-    case T_READLINK:
-        path = va_arg(ap, V9fsString *);
-        size = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sd", path, size);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_READLINK;
-        }
-        break;
-    case T_STATFS:
-        path = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "s", path);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_STATFS;
-        }
-        break;
-    case T_CHMOD:
-        path = va_arg(ap, V9fsString *);
-        mode = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sd", path, mode);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_CHMOD;
-        }
-        break;
-    case T_CHOWN:
-        path = va_arg(ap, V9fsString *);
-        uid = va_arg(ap, int);
-        gid = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sdd", path, uid, gid);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_CHOWN;
-        }
-        break;
-    case T_TRUNCATE:
-        path = va_arg(ap, V9fsString *);
-        offset = va_arg(ap, uint64_t);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sq", path, offset);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_TRUNCATE;
-        }
-        break;
-    case T_UTIME:
-        path = va_arg(ap, V9fsString *);
-        spec[0].tv_sec = va_arg(ap, long);
-        spec[0].tv_nsec = va_arg(ap, long);
-        spec[1].tv_sec = va_arg(ap, long);
-        spec[1].tv_nsec = va_arg(ap, long);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sqqqq", path,
-                                    spec[0].tv_sec, spec[1].tv_nsec,
-                                    spec[1].tv_sec, spec[1].tv_nsec);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_UTIME;
-        }
-        break;
-    case T_RENAME:
-        oldpath = va_arg(ap, V9fsString *);
-        path = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "ss", oldpath, path);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_RENAME;
-        }
-        break;
-    case T_REMOVE:
-        path = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "s", path);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_REMOVE;
-        }
-        break;
-    case T_LGETXATTR:
-        size = va_arg(ap, int);
-        path = va_arg(ap, V9fsString *);
-        name = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ,
-                                    "dss", size, path, name);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_LGETXATTR;
-        }
-        break;
-    case T_LLISTXATTR:
-        size = va_arg(ap, int);
-        path = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "ds", size, path);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_LLISTXATTR;
-        }
-        break;
-    case T_LSETXATTR:
-        path = va_arg(ap, V9fsString *);
-        name = va_arg(ap, V9fsString *);
-        value = va_arg(ap, V9fsString *);
-        size = va_arg(ap, int);
-        flags = va_arg(ap, int);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sssdd",
-                                    path, name, value, size, flags);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_LSETXATTR;
-        }
-        break;
-    case T_LREMOVEXATTR:
-        path = va_arg(ap, V9fsString *);
-        name = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "ss", path, name);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_LREMOVEXATTR;
-        }
-        break;
-    case T_GETVERSION:
-        path = va_arg(ap, V9fsString *);
-        retval = proxy_marshal(iovec, PROXY_HDR_SZ, "s", path);
-        if (retval > 0) {
-            header.size = retval;
-            header.type = T_GETVERSION;
-        }
-        break;
-    default:
-        error_report("Invalid type %d", type);
-        retval = -EINVAL;
-        break;
-    }
+    retval = v9fs_marshal(&proxy->out_iovec, 1, PROXY_HDR_SZ, 0, fmt, ap);
     va_end(ap);
 
     if (retval < 0) {
@@ -534,51 +315,23 @@ static int v9fs_request(V9fsProxy *proxy, int type,
     }
 
     /* marshal the header details */
-    proxy_marshal(iovec, 0, "dd", header.type, header.size);
-    header.size += PROXY_HDR_SZ;
+    v9fs_marshal(&proxy->out_iovec, 1, 0, 0, "dd", type, retval);
+    retval += PROXY_HDR_SZ;
 
-    err = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size);
-    if (err != header.size) {
+    err = qemu_write_full(proxy->sockfd, proxy->out_iovec.iov_base, retval);
+    if (err != retval) {
         goto close_error;
     }
 
-    switch (type) {
-    case T_OPEN:
-    case T_CREATE:
-        /*
-         * A file descriptor is returned as response for
+    if (type == T_OPEN || type == T_CREATE) {
+        /* A file descriptor is returned as response for
          * T_OPEN,T_CREATE on success
          */
         err = v9fs_receivefd(proxy->sockfd, &retval);
-        break;
-    case T_MKNOD:
-    case T_MKDIR:
-    case T_SYMLINK:
-    case T_LINK:
-    case T_CHMOD:
-    case T_CHOWN:
-    case T_RENAME:
-    case T_TRUNCATE:
-    case T_UTIME:
-    case T_REMOVE:
-    case T_LSETXATTR:
-    case T_LREMOVEXATTR:
-        err = v9fs_receive_status(proxy, reply, &retval);
-        break;
-    case T_LSTAT:
-    case T_READLINK:
-    case T_STATFS:
-    case T_GETVERSION:
+    } else if (response) { 
         err = v9fs_receive_response(proxy, type, &retval, response);
-        break;
-    case T_LGETXATTR:
-    case T_LLISTXATTR:
-        if (!size) {
-            err = v9fs_receive_status(proxy, reply, &retval);
-        } else {
-            err = v9fs_receive_response(proxy, type, &retval, response);
-        }
-        break;
+    } else {
+        err = v9fs_receive_status(proxy, &proxy->in_iovec, &retval);
     }
 
     if (err < 0) {
@@ -912,7 +665,7 @@ static ssize_t proxy_lgetxattr(FsContext *ctx, V9fsPath 
*fs_path,
 
     v9fs_string_init(&xname);
     v9fs_string_sprintf(&xname, "%s", name);
-    retval = v9fs_request(ctx->private, T_LGETXATTR, value, "dss", size,
+    retval = v9fs_request(ctx->private, T_LGETXATTR, size ? value : NULL, 
"dss", size,
                           fs_path, &xname);
     v9fs_string_free(&xname);
     return retval;
@@ -922,7 +675,7 @@ static ssize_t proxy_llistxattr(FsContext *ctx, V9fsPath 
*fs_path,
                                 void *value, size_t size)
 {
     int retval;
-    retval = v9fs_request(ctx->private, T_LLISTXATTR, value, "ds", size,
+    retval = v9fs_request(ctx->private, T_LLISTXATTR, size ? value : NULL, 
"ds", size,
                           fs_path);
     return retval;
 }
-- 
2.1.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]