[CRIU] [PATCH 01/10] parasite: Refactor code to follow calling
convention
Cyrill Gorcunov
gorcunov at openvz.org
Mon Mar 12 16:55:14 EDT 2012
It happened some routines in parasite service code
were not following calling convention so I fixed the
callers and added a comment about adding new code here.
At the same time the 3 error code madness is dropped
as being requested by Pavel -- now we return one error
code only.
The PARASITE_ERR_ codes were dropped as well due to
become redundant.
The status of parasite service routine is set via
SET_PARASITE helpers
- SET_PARASITE_ERR, if you have some error code
to return
- SET_PARASITE_RET, if you have some value to return,
but you don't know if it's error or not (basically
it's simply semanatic wrapper over SET_PARASITE_ERR,
should be used in places where _ERR version migh confuse
the reader)
- SET_PARASITE_FAIL, if it's a generic error
- SET_PARASITE_PASS, if routine finished without errors
Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
include/parasite.h | 42 ++++++------
parasite-syscall.c | 10 +--
parasite.c | 192 ++++++++++++++++++++++++++++++----------------------
3 files changed, 137 insertions(+), 107 deletions(-)
diff --git a/include/parasite.h b/include/parasite.h
index 2a67647..2dd5b8d 100644
--- a/include/parasite.h
+++ b/include/parasite.h
@@ -16,19 +16,6 @@
#define PARASITE_MAX_SIZE (64 << 10)
-/* we need own error code for diagnostics */
-#define PARASITE_ERR_FAIL -1024
-#define PARASITE_ERR_OPEN -1025
-#define PARASITE_ERR_MMAP -1026
-#define PARASITE_ERR_MINCORE -1027
-#define PARASITE_ERR_MUNMAP -1028
-#define PARASITE_ERR_CLOSE -1029
-#define PARASITE_ERR_WRITE -1030
-#define PARASITE_ERR_MPROTECT -1031
-#define PARASITE_ERR_SIGACTION -1032
-#define PARASITE_ERR_GETITIMER -1033
-#define PARASITE_ERR_IOCTL -1034
-
enum {
PARASITE_CMD_PINGME,
PARASITE_CMD_INIT,
@@ -49,16 +36,29 @@ enum {
};
typedef struct {
- long ret; /* custom ret code */
- long sys_ret; /* syscall ret code */
- long line; /* where we're failed */
+ long ret; /* ret code */
+ long line; /* where we're failed at */
} parasite_status_t;
-#define SET_PARASITE_STATUS(st, ret_code, sys_ret_code) \
- do { \
- (st)->ret = ret_code, \
- (st)->sys_ret = sys_ret_code, \
- (st)->line = __LINE__; \
+#define SET_PARASITE_ERR(st, err) \
+ do { \
+ (st)->ret = err, \
+ (st)->line = __LINE__; \
+ } while (0)
+
+#define SET_PARASITE_RET(st, ret) \
+ SET_PARASITE_ERR(st, ret)
+
+#define SET_PARASITE_FAIL(st) \
+ do { \
+ (st)->ret = -1, \
+ (st)->line = __LINE__; \
+ } while (0)
+
+#define SET_PARASITE_PASS(st) \
+ do { \
+ (st)->ret = 0, \
+ (st)->line = __LINE__; \
} while (0)
struct parasite_init_args {
diff --git a/parasite-syscall.c b/parasite-syscall.c
index d1d63a8..731ba80 100644
--- a/parasite-syscall.c
+++ b/parasite-syscall.c
@@ -259,8 +259,8 @@ static int parasite_execute_by_pid(unsigned long cmd, struct parasite_ctl *ctl,
ret = args->ret;
if (ret)
- pr_err("Parasite exited with %d ret (%li at %li)\n",
- ret, args->sys_ret, args->line);
+ pr_err("Parasite exited with %d ret (at %li)\n",
+ ret, args->line);
if (ctl->pid != pid)
if (ptrace(PTRACE_SETREGS, pid, NULL, ®s_orig)) {
@@ -542,9 +542,8 @@ int parasite_dump_pages_seized(struct parasite_ctl *ctl, struct list_head *vma_a
ret = parasite_execute(PARASITE_CMD_DUMPPAGES_INIT, ctl, st, sizeof(*st));
if (ret < 0) {
- pr_err("Dumping pages failed with %li (%li) at %li\n",
+ pr_err("Dumping pages failed with %li at %li\n",
parasite_dumppages.status.ret,
- parasite_dumppages.status.sys_ret,
parasite_dumppages.status.line);
goto out;
}
@@ -582,9 +581,8 @@ int parasite_dump_pages_seized(struct parasite_ctl *ctl, struct list_head *vma_a
(parasite_status_t *) ¶site_dumppages,
sizeof(parasite_dumppages));
if (ret) {
- pr_err("Dumping pages failed with %li (%li) at %li\n",
+ pr_err("Dumping pages failed with %li at %li\n",
parasite_dumppages.status.ret,
- parasite_dumppages.status.sys_ret,
parasite_dumppages.status.line);
goto out;
diff --git a/parasite.c b/parasite.c
index b6c6bba..6334f4a 100644
--- a/parasite.c
+++ b/parasite.c
@@ -9,6 +9,22 @@
#include "util.h"
#include "util-net.h"
+/*
+ * Some notes on parasite code overall. There are a few
+ * calling convention specfics the caller must follow
+ *
+ * - on success, 0 must be returned, anything else
+ * treated as error
+ *
+ * - every routine which takes arguments and called from
+ * parasite_head
+ * parasite_service
+ * must provide parasite_status_t argument either via
+ * plain pointer or as first member of an embedding
+ * structure so service routine will pass error code
+ * there
+ */
+
#ifdef CONFIG_X86_64
static void *brk_start, *brk_end, *brk_tail;
@@ -112,21 +128,23 @@ static int fd_pages[2] = { -1, -1 };
static int dump_pages_init(parasite_status_t *st)
{
- fd_pages[PG_PRIV] = recv_fd(tsock);
- if (fd_pages[PG_PRIV] < 0)
+ int ret;
+
+ ret = fd_pages[PG_PRIV] = recv_fd(tsock);
+ if (ret < 0)
goto err;
- fd_pages[PG_SHARED] = recv_fd(tsock);
- if (fd_pages[PG_SHARED] < 0)
+ ret = fd_pages[PG_SHARED] = recv_fd(tsock);
+ if (ret < 0)
goto err_s;
- SET_PARASITE_STATUS(st, 0, 0);
+ SET_PARASITE_PASS(st);
return 0;
err_s:
sys_close(fd_pages[PG_PRIV]);
err:
- SET_PARASITE_STATUS(st, PARASITE_ERR_FAIL, -1);
+ SET_PARASITE_ERR(st, ret);
return -1;
}
@@ -140,7 +158,7 @@ static int dump_pages(struct parasite_dump_pages_args *args)
unsigned long nrpages, pfn, length;
unsigned long prot_old, prot_new;
unsigned char *map;
- int ret = PARASITE_ERR_FAIL, fd;
+ int ret = -1, fd;
args->nrpages_dumped = 0;
prot_old = prot_new = 0;
@@ -159,8 +177,7 @@ static int dump_pages(struct parasite_dump_pages_args *args)
*/
map = brk_alloc(nrpages);
if (!map) {
- SET_PARASITE_STATUS(st, PARASITE_ERR_MMAP, (long)map);
- ret = st->ret;
+ SET_PARASITE_ERR(st, -ENOMEM);
goto err;
}
@@ -176,8 +193,7 @@ static int dump_pages(struct parasite_dump_pages_args *args)
prot_new);
if (ret) {
sys_write_msg("sys_mprotect failed\n");
- SET_PARASITE_STATUS(st, PARASITE_ERR_MPROTECT, ret);
- ret = st->ret;
+ SET_PARASITE_ERR(st, ret);
goto err_free;
}
}
@@ -190,8 +206,7 @@ static int dump_pages(struct parasite_dump_pages_args *args)
ret = sys_mincore((unsigned long)args->vma_entry.start, length, map);
if (ret) {
sys_write_msg("sys_mincore failed\n");
- SET_PARASITE_STATUS(st, PARASITE_ERR_MINCORE, ret);
- ret = st->ret;
+ SET_PARASITE_ERR(st, ret);
goto err_free;
}
@@ -210,8 +225,8 @@ static int dump_pages(struct parasite_dump_pages_args *args)
written += sys_write(fd, &vaddr, sizeof(vaddr));
written += sys_write(fd, (void *)vaddr, PAGE_SIZE);
if (written != sizeof(vaddr) + PAGE_SIZE) {
- SET_PARASITE_STATUS(st, PARASITE_ERR_WRITE, written);
- ret = st->ret;
+ ret = -1;
+ SET_PARASITE_ERR(st, -EIO);
goto err_free;
}
@@ -228,25 +243,25 @@ static int dump_pages(struct parasite_dump_pages_args *args)
prot_old);
if (ret) {
sys_write_msg("PANIC: Ouch! sys_mprotect failed on restore\n");
- SET_PARASITE_STATUS(st, PARASITE_ERR_MPROTECT, ret);
- ret = st->ret;
+ SET_PARASITE_ERR(st, ret);
goto err_free;
}
}
- /* on success ret = 0 */
- SET_PARASITE_STATUS(st, ret, ret);
-
+ ret = 0;
+ SET_PARASITE_PASS(st);
err_free:
brk_free(nrpages);
err:
return ret;
}
-static int dump_pages_fini(void)
+static int dump_pages_fini(parasite_status_t *st)
{
sys_close(fd_pages[PG_PRIV]);
sys_close(fd_pages[PG_SHARED]);
+
+ SET_PARASITE_PASS(st);
return 0;
}
@@ -255,12 +270,13 @@ static int dump_sigact(parasite_status_t *st)
rt_sigaction_t act;
struct sa_entry e;
int fd, sig;
-
- int ret = PARASITE_ERR_FAIL;
+ int ret;
fd = recv_fd(tsock);
- if (fd < 0)
+ if (fd < 0) {
+ SET_PARASITE_ERR(st, fd);
return fd;
+ }
sys_lseek(fd, MAGIC_OFFSET, SEEK_SET);
@@ -271,8 +287,7 @@ static int dump_sigact(parasite_status_t *st)
ret = sys_sigaction(sig, NULL, &act);
if (ret < 0) {
sys_write_msg("sys_sigaction failed\n");
- SET_PARASITE_STATUS(st, PARASITE_ERR_SIGACTION, ret);
- ret = st->ret;
+ SET_PARASITE_ERR(st, ret);
goto err_close;
}
@@ -284,15 +299,13 @@ static int dump_sigact(parasite_status_t *st)
ret = sys_write(fd, &e, sizeof(e));
if (ret != sizeof(e)) {
sys_write_msg("sys_write failed\n");
- SET_PARASITE_STATUS(st, PARASITE_ERR_WRITE, ret);
- ret = st->ret;
+ SET_PARASITE_ERR(st, -EIO);
goto err_close;
}
}
ret = 0;
- SET_PARASITE_STATUS(st, 0, ret);
-
+ SET_PARASITE_PASS(st);
err_close:
sys_close(fd);
return ret;
@@ -307,8 +320,8 @@ static int dump_itimer(int which, int fd, parasite_status_t *st)
ret = sys_getitimer(which, &val);
if (ret < 0) {
sys_write_msg("getitimer failed\n");
- SET_PARASITE_STATUS(st, PARASITE_ERR_GETITIMER, ret);
- return st->ret;
+ SET_PARASITE_ERR(st, ret);
+ return ret;
}
ie.isec = val.it_interval.tv_sec;
@@ -319,10 +332,11 @@ static int dump_itimer(int which, int fd, parasite_status_t *st)
ret = sys_write(fd, &ie, sizeof(ie));
if (ret != sizeof(ie)) {
sys_write_msg("sys_write failed\n");
- SET_PARASITE_STATUS(st, PARASITE_ERR_WRITE, ret);
- return st->ret;
+ SET_PARASITE_ERR(st, -EIO);
+ return -1;
}
+ SET_PARASITE_PASS(st);
return 0;
}
@@ -331,12 +345,13 @@ static int dump_itimers(parasite_status_t *st)
rt_sigaction_t act;
struct sa_entry e;
int fd, sig;
-
- int ret = PARASITE_ERR_FAIL;
+ int ret = -1;
fd = recv_fd(tsock);
- if (fd < 0)
+ if (fd < 0) {
+ SET_PARASITE_ERR(st, fd);
return fd;
+ }
sys_lseek(fd, MAGIC_OFFSET, SEEK_SET);
@@ -352,9 +367,6 @@ static int dump_itimers(parasite_status_t *st)
if (ret < 0)
goto err_close;
- ret = 0;
- SET_PARASITE_STATUS(st, 0, ret);
-
err_close:
sys_close(fd);
return ret;
@@ -371,7 +383,7 @@ static int dump_misc(struct parasite_dump_misc *args)
args->brk = sys_brk(0);
args->blocked = old_blocked;
- SET_PARASITE_STATUS(st, 0, 0);
+ SET_PARASITE_PASS(st);
return 0;
}
@@ -381,12 +393,16 @@ static int dump_tid_addr(struct parasite_dump_tid_addr *args)
int ret;
ret = sys_prctl(PR_GET_TID_ADDR, (unsigned long) &args->tid_addr, 0, 0, 0);
+ if (ret) {
+ SET_PARASITE_ERR(st, ret);
+ return ret;
+ }
- SET_PARASITE_STATUS(st, 0, ret);
+ SET_PARASITE_PASS(st);
return 0;
}
-static int dump_socket_queue(int img_fd, struct sk_queue_item *item, int *err)
+static int dump_socket_queue(int img_fd, struct sk_queue_item *item)
{
struct sk_packet_entry *pe;
unsigned long size;
@@ -400,8 +416,7 @@ static int dump_socket_queue(int img_fd, struct sk_queue_item *item, int *err)
ret = sys_getsockopt(sock_fd, SOL_SOCKET, SO_PEEK_OFF, &orig_peek_off, &tmp);
if (ret < 0) {
sys_write_msg("getsockopt failed\n");
- *err = ret;
- return PARASITE_ERR_FAIL;
+ return ret;
}
/*
* Discover max DGRAM size
@@ -409,8 +424,7 @@ static int dump_socket_queue(int img_fd, struct sk_queue_item *item, int *err)
ret = sys_getsockopt(sock_fd, SOL_SOCKET, SO_SNDBUF, &ret, &tmp);
if (ret < 0) {
sys_write_msg("getsockopt failed\n");
- *err = ret;
- return PARASITE_ERR_FAIL;
+ return ret;
}
/*
* Note: 32 bytes will be used by kernel for protocol header.
@@ -423,17 +437,14 @@ static int dump_socket_queue(int img_fd, struct sk_queue_item *item, int *err)
pe = brk_alloc(size + sizeof(struct sk_packet_entry));
if (!pe) {
sys_write_msg("not enough mem for skb\n");
- *err = -ENOMEM;
- return PARASITE_ERR_MMAP;
+ return -ENOMEM;
}
/*
* Enable peek offset incrementation.
*/
- ret = 0;
- *err = sys_setsockopt(sock_fd, SOL_SOCKET, SO_PEEK_OFF, &ret, sizeof(int));
- if (*err < 0) {
+ ret = sys_setsockopt(sock_fd, SOL_SOCKET, SO_PEEK_OFF, &ret, sizeof(int));
+ if (ret < 0) {
sys_write_msg("setsockopt fail\n");
- ret = PARASITE_ERR_FAIL;
goto err_brk;
}
@@ -449,12 +460,11 @@ static int dump_socket_queue(int img_fd, struct sk_queue_item *item, int *err)
.msg_iovlen = 1,
};
- *err = pe->length = sys_recvmsg(sock_fd, &msg, MSG_DONTWAIT | MSG_PEEK);
- if (*err < 0) {
- if (*err == -EAGAIN)
+ ret = pe->length = sys_recvmsg(sock_fd, &msg, MSG_DONTWAIT | MSG_PEEK);
+ if (ret < 0) {
+ if (ret == -EAGAIN)
break; /* we're done */
sys_write_msg("sys_recvmsg fail: error\n");
- ret = PARASITE_ERR_FAIL;
goto err_set_sock;
}
if (msg.msg_flags & MSG_TRUNC) {
@@ -463,23 +473,25 @@ static int dump_socket_queue(int img_fd, struct sk_queue_item *item, int *err)
* to check...
*/
sys_write_msg("sys_recvmsg failed: truncated\n");
- ret = PARASITE_ERR_FAIL;
- *err = -E2BIG;
+ ret = -E2BIG;
goto err_set_sock;
}
- *err = sys_write(img_fd, pe, sizeof(pe) + pe->length);
- if (*err != sizeof(pe) + pe->length) {
+ ret = sys_write(img_fd, pe, sizeof(pe) + pe->length);
+ if (ret != sizeof(pe) + pe->length) {
sys_write_msg("sys_write failed\n");
- ret = PARASITE_ERR_WRITE;
+ ret = -EIO;
goto err_set_sock;
}
}
ret = 0;
+
err_set_sock:
/*
* Restore original peek offset.
*/
- sys_setsockopt(sock_fd, SOL_SOCKET, SO_PEEK_OFF, &orig_peek_off, sizeof(int));
+ ret = sys_setsockopt(sock_fd, SOL_SOCKET, SO_PEEK_OFF, &orig_peek_off, sizeof(int));
+ if (ret < 0)
+ sys_write_msg("setsockopt failed on restore\n");
err_brk:
brk_free(size + sizeof(struct sk_packet_entry));
return ret;
@@ -491,21 +503,21 @@ static int dump_skqueues(struct parasite_dump_sk_queues *args)
int img_fd, i, ret = -1;
img_fd = recv_fd(tsock);
- if (img_fd < 0)
+ if (img_fd < 0) {
+ SET_PARASITE_ERR(st, img_fd);
return img_fd;
+ }
for (i = 0; i < args->nr_items; i++) {
- int err;
-
- ret = dump_socket_queue(img_fd, &args->items[i], &err);
+ ret = dump_socket_queue(img_fd, &args->items[i]);
if (ret < 0) {
- SET_PARASITE_STATUS(st, ret, err);
+ SET_PARASITE_RET(st, ret);
goto err_dmp;
}
}
ret = 0;
- SET_PARASITE_STATUS(st, 0, 0);
+ SET_PARASITE_PASS(st);
err_dmp:
sys_close(img_fd);
return ret;
@@ -513,19 +525,25 @@ err_dmp:
static int init(struct parasite_init_args *args)
{
- int ret;
+ parasite_status_t *st = &args->status;
k_rtsigset_t to_block;
+ int ret;
- if (brk_init() < 0)
+ ret = brk_init();
+ if (ret) {
+ SET_PARASITE_ERR(st, ret);
return -1;
+ }
tsock = sys_socket(PF_UNIX, SOCK_DGRAM, 0);
if (tsock < 0) {
+ SET_PARASITE_ERR(st, tsock);
return -1;
}
ret = sys_bind(tsock, (struct sockaddr *) &args->saddr, args->sun_len);
if (ret < 0) {
+ SET_PARASITE_ERR(st, ret);
return -1;
}
@@ -536,23 +554,33 @@ static int init(struct parasite_init_args *args)
else
reset_blocked = 1;
- SET_PARASITE_STATUS(&args->status, ret, ret);
+ SET_PARASITE_RET(st, ret);
return ret;
}
-static int parasite_set_logfd(void)
+static int parasite_set_logfd(parasite_status_t *st)
{
- logfd = recv_fd(tsock);
- return logfd;
+ int ret;
+
+ ret = recv_fd(tsock);
+ if (ret >= 0) {
+ logfd = ret;
+ ret = 0;
+ }
+
+ SET_PARASITE_RET(st, ret);
+ return ret;
}
-static int fini(void)
+static int fini(parasite_status_t *st)
{
if (reset_blocked == 1)
sys_sigprocmask(SIG_SETMASK, &old_blocked, NULL);
sys_close(logfd);
sys_close(tsock);
brk_fini();
+
+ SET_PARASITE_PASS(st);
return 0;
}
@@ -568,13 +596,13 @@ static int __used parasite_service(unsigned long cmd, void *args)
case PARASITE_CMD_INIT:
return init((struct parasite_init_args *) args);
case PARASITE_CMD_FINI:
- return fini();
+ return fini((parasite_status_t *)args);
case PARASITE_CMD_SET_LOGFD:
- return parasite_set_logfd();
+ return parasite_set_logfd((parasite_status_t *)args);
case PARASITE_CMD_DUMPPAGES_INIT:
return dump_pages_init((parasite_status_t *) args);
case PARASITE_CMD_DUMPPAGES_FINI:
- return dump_pages_fini();
+ return dump_pages_fini((parasite_status_t *)args);
case PARASITE_CMD_DUMPPAGES:
return dump_pages((struct parasite_dump_pages_args *)args);
case PARASITE_CMD_DUMP_SIGACTS:
@@ -588,7 +616,11 @@ static int __used parasite_service(unsigned long cmd, void *args)
case PARASITE_CMD_DUMP_SK_QUEUES:
return dump_skqueues((struct parasite_dump_sk_queues *)args);
default:
- sys_write_msg("Unknown command to parasite\n");
+ {
+ parasite_status_t *st = (parasite_status_t *)args;
+ SET_PARASITE_ERR(st, -EINVAL);
+ sys_write_msg("Unknown command to parasite\n");
+ }
break;
}
--
1.7.7.6
More information about the CRIU
mailing list