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 <m...@tls.msk.ru>
---
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
