[CRIU] [PATCH 4/5] Move error reporting to inside open_proc and
friends
Kir Kolyshkin
kir at openvz.org
Thu Feb 16 16:39:35 EST 2012
...and make it correctly print the file name we were unable to open.
Also, error from fdopen[dir]() is now reported with file name as well.
Note that open_proc() and friends need to be macros in order for
pr_perror() to show actual file name and line number where error occured.
Historical note: the original version of this patch was way more radical,
changing openat() to open() and thus removing pid_dir (replacing with pid
when needed) and open_proc_dir(), changing openat() to open(). The word
from Pavel is he wants to keep the openat/pid_dir optimization because
it saves two dentry lookups in kernel code for each open(). Because of
this optimization (and desire to print correct file name in case
of error) we have to carry both pid and pid_dir everywhere.
Signed-off-by: Kir Kolyshkin <kir at openvz.org>
---
cr-dump.c | 71 ++++++++++++++++++-------------------------------
include/proc_parse.h | 2 +-
include/util.h | 56 ++++++++++++++++++++++++++++++++++++---
parasite-syscall.c | 2 +-
proc_parse.c | 28 +++++++------------
util.c | 46 +++++---------------------------
6 files changed, 97 insertions(+), 108 deletions(-)
diff --git a/cr-dump.c b/cr-dump.c
index c39bbce..9aaaf13 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -138,29 +138,25 @@ err:
return ret;
}
-static int dump_task_special_files(int pid_dir, struct cr_fdset *cr_fdset)
+static int dump_task_special_files(pid_t pid, int pid_dir, struct cr_fdset *cr_fdset)
{
struct fd_parms params;
int fd, ret;
/* Dump /proc/pid/cwd */
params = (struct fd_parms){ .fd_name = FDINFO_CWD, };
- fd = open_proc(pid_dir, "cwd");
- if (fd < 0) {
- pr_perror("Failed to openat cwd");
+ fd = open_proc(pid, pid_dir, "cwd");
+ if (fd < 0)
return -1;
- }
ret = dump_one_reg_file(FDINFO_FD, ¶ms, fd, cr_fdset, 1);
if (ret)
return ret;
/* Dump /proc/pid/exe */
params = (struct fd_parms){ .fd_name = FDINFO_EXE, };
- fd = open_proc(pid_dir, "exe");
- if (fd < 0) {
- pr_perror("Failed to openat exe");
+ fd = open_proc(pid, pid_dir, "exe");
+ if (fd < 0)
return -1;
- }
ret = dump_one_reg_file(FDINFO_FD, ¶ms, fd, cr_fdset, 1);
return ret;
@@ -311,11 +307,9 @@ static int read_fd_params(pid_t pid, int pid_dir, char *fd, struct fd_parms *p)
FILE *file;
int ret;
- file = fopen_proc(pid_dir, "fdinfo/%s", fd);
- if (!file) {
- pr_perror("Can't open %d's %s fdinfo", pid, fd);
+ file = fopen_proc(pid, pid_dir, "fdinfo/%s", fd);
+ if (!file)
return -1;
- }
p->fd_name = atoi(fd);
ret = fscanf(file, "pos:\t%li\nflags:\t%o\nid:\t%s\n", &p->pos, &p->flags, p->id);
@@ -348,16 +342,14 @@ static int dump_task_files(pid_t pid, int pid_dir, struct cr_fdset *cr_fdset)
* to re-read them in restorer, so better to make it
* fast.
*/
- if (dump_task_special_files(pid_dir, cr_fdset)) {
+ if (dump_task_special_files(pid, pid_dir, cr_fdset)) {
pr_err("Can't dump special files\n");
return -1;
}
- fd_dir = opendir_proc(pid_dir, "fd");
- if (!fd_dir) {
- pr_perror("Can't open %d's fd", pid);
+ fd_dir = opendir_proc(pid, pid_dir, "fd");
+ if (!fd_dir)
return -1;
- }
while ((de = readdir(fd_dir))) {
char id[FD_ID_SIZE];
@@ -455,7 +447,7 @@ static int dump_task_creds(pid_t pid, int pid_dir,
pr_info("Dumping creds for %d)\n", pid);
pr_info("----------------------------------------\n");
- ret = parse_pid_status(pid_dir, &cr);
+ ret = parse_pid_status(pid, pid_dir, &cr);
if (ret < 0)
return ret;
@@ -497,11 +489,9 @@ static int get_task_sigmask(pid_t pid, int pid_dir, u64 *task_sigset)
/*
* Now signals.
*/
- file = fopen_proc(pid_dir, "status");
- if (!file) {
- pr_perror("Can't open %d status", pid);
+ file = fopen_proc(pid, pid_dir, "status");
+ if (!file)
goto err;
- }
while (fgets(loc_buf, sizeof(loc_buf), file)) {
if (!strncmp(loc_buf, "SigBlk:", 7)) {
@@ -519,13 +509,11 @@ err:
static int get_task_auxv(pid_t pid, int pid_dir, struct core_entry *core)
{
- int fd = open_proc(pid_dir, "auxv");
+ int fd = open_proc(pid, pid_dir, "auxv");
int ret, i;
- if (fd < 0) {
- pr_perror("Can't open %d's auxv", pid);
+ if (fd < 0)
return -1;
- }
for (i = 0; i < AT_VECTOR_SIZE; i++) {
ret = read(fd, &core->tc.mm_saved_auxv[i],
@@ -552,11 +540,9 @@ static int get_task_personality(pid_t pid, int pid_dir, u32 *personality)
FILE *file = NULL;
int ret = -1;
- file = fopen_proc(pid_dir, "personality");
- if (!file) {
- pr_perror("Can't open %d personality", pid);
+ file = fopen_proc(pid, pid_dir, "personality");
+ if (!file)
goto err;
- }
if (!fgets(loc_buf, sizeof(loc_buf), file)) {
perror("Can't read task personality");
@@ -738,18 +724,16 @@ err:
return ret;
}
-static int parse_threads(struct pstree_item *item, int pid_dir)
+static int parse_threads(struct pstree_item *item, pid_t pid, int pid_dir)
{
struct dirent *de;
DIR *dir;
u32 *t = NULL;
int nr = 1;
- dir = opendir_proc(pid_dir, "task");
- if (!dir) {
- pr_perror("Can't open %d/task", item->pid);
+ dir = opendir_proc(pid, pid_dir, "task");
+ if (!dir)
return -1;
- }
while ((de = readdir(dir))) {
u32 *tmp;
@@ -776,7 +760,7 @@ static int parse_threads(struct pstree_item *item, int pid_dir)
return 0;
}
-static int parse_children(struct pstree_item *item, int pid_dir)
+static int parse_children(struct pstree_item *item, pid_t pid, int pid_dir)
{
FILE *file;
char *tok;
@@ -785,12 +769,9 @@ static int parse_children(struct pstree_item *item, int pid_dir)
for (i = 0; i < item->nr_threads; i++) {
- file = fopen_proc(pid_dir, "task/%d/children", item->threads[i]);
- if (!file) {
- pr_perror("Can't open %d children %d",
- item->pid, item->threads[i]);
+ file = fopen_proc(pid, pid_dir, "task/%d/children", item->threads[i]);
+ if (!file)
goto err;
- }
if (!(fgets(loc_buf, sizeof(loc_buf), file)))
loc_buf[0] = 0;
@@ -918,7 +899,7 @@ static struct pstree_item *collect_task(pid_t pid, struct list_head *list)
item->state = TASK_DEAD;
}
- ret = parse_threads(item, pid_dir);
+ ret = parse_threads(item, pid, pid_dir);
if (ret < 0)
goto err_close;
@@ -926,7 +907,7 @@ static struct pstree_item *collect_task(pid_t pid, struct list_head *list)
if (ret < 0)
goto err_close;
- ret = parse_children(item, pid_dir);
+ ret = parse_children(item, pid, pid_dir);
if (ret < 0)
goto err_close;
@@ -1286,7 +1267,7 @@ int cr_dump_tasks(pid_t pid, struct cr_options *opts)
LIST_HEAD(pstree_list);
struct cr_fdset *cr_fdset = NULL;
struct pstree_item *item;
- int i, ret = -1, pid_dir;
+ int i, ret = -1;
pr_info("========================================\n");
if (!opts->leader_only)
diff --git a/include/proc_parse.h b/include/proc_parse.h
index d7c637f..788200e 100644
--- a/include/proc_parse.h
+++ b/include/proc_parse.h
@@ -80,6 +80,6 @@ struct proc_status_creds {
extern int parse_pid_stat(pid_t pid, int pid_dir, struct proc_pid_stat *s);
extern int parse_pid_stat_small(pid_t pid, int pid_dir, struct proc_pid_stat_small *s);
extern int parse_maps(pid_t pid, int pid_dir, struct list_head *vma_area_list, bool use_map_files);
-extern int parse_pid_status(int pid_dir, struct proc_status_creds *);
+extern int parse_pid_status(pid_t pid, int pid_dir, struct proc_status_creds *);
#endif /* PROC_PARSE_H__ */
diff --git a/include/util.h b/include/util.h
index 5d43dd3..212d380 100644
--- a/include/util.h
+++ b/include/util.h
@@ -205,10 +205,58 @@ extern int reopen_fd_as_safe(int new_fd, int old_fd, bool allow_reuse_fd);
extern void hex_dump(void *addr, unsigned long len);
int open_pid_proc(pid_t pid);
-int open_proc(int pid_dir_fd, char *fmt, ...);
-int open_proc_rw(int pid_dir_fd, char *fmt, ...)
-DIR *opendir_proc(int pid_dir_fd, char *fmt, ...);
-FILE *fopen_proc(int pid_dir_fd, char *fmt, ...);
+int do_open_proc(int pid_dir, int flags, const char *fmt, ...);
+
+#define __open_proc(pid, pid_dir, flags, fmt, ...) \
+ ({ \
+ int __fd = do_open_proc(pid_dir, flags, \
+ fmt, ##__VA_ARGS__); \
+ if (__fd < 0) \
+ pr_perror("Can't open /proc/%d/" fmt, \
+ pid, ##__VA_ARGS__); \
+ \
+ __fd; \
+ })
+
+/* int open_proc(pid_t pid, int pid_dir, const char *fmt, ...); */
+#define open_proc(pid, pid_dir, fmt, ...) \
+ __open_proc(pid, pid_dir, O_RDONLY, fmt, ##__VA_ARGS__)
+
+/* int open_proc_rw(pid_t pid, int pid_dir, const char *fmt, ...); */
+#define open_proc_rw(pid, pid_dir, fmt, ...) \
+ __open_proc(pid, pid_dir, O_RDWR, fmt, ##__VA_ARGS__)
+
+/* DIR *opendir_proc(pid_t pid, int pid_dir, const char *fmt, ...); */
+#define opendir_proc(pid, pid_dir, fmt, ...) \
+ ({ \
+ int __fd = open_proc(pid, pid_dir, fmt, ##__VA_ARGS__); \
+ DIR *__d = NULL; \
+ \
+ if (__fd >= 0) \
+ __d = fdopendir(__fd); \
+ if (__d == NULL) \
+ pr_perror("Can't fdopendir %d " \
+ "(/proc/%d/" fmt ")", \
+ __fd, pid, ##__VA_ARGS__); \
+ \
+ __d; \
+ })
+
+/* FILE *fopen_proc(pid_t pid, int pid_dir, const char *fmt, ...); */
+#define fopen_proc(pid, pid_dir, fmt, ...) \
+ ({ \
+ int __fd = open_proc(pid, pid_dir, fmt, ##__VA_ARGS__); \
+ FILE *__f = NULL; \
+ \
+ if (__fd >= 0) \
+ __f = fdopen(__fd, "r"); \
+ if (__f == NULL) \
+ pr_perror("Can't fdopen %d " \
+ "(/proc/%d/" fmt ")", \
+ __fd, pid, ##__VA_ARGS__); \
+ \
+ __f; \
+ })
#define __xalloc(op, size, ...) \
({ \
diff --git a/parasite-syscall.c b/parasite-syscall.c
index 7e25722..83646a8 100644
--- a/parasite-syscall.c
+++ b/parasite-syscall.c
@@ -620,7 +620,7 @@ struct parasite_ctl *parasite_infect_seized(pid_t pid, int pid_dir, struct list_
ctl->map_length = round_up(parasite_size, PAGE_SIZE);
- fd = open_proc_rw(pid_dir, "map_files/%p-%p",
+ fd = open_proc_rw(pid, pid_dir, "map_files/%p-%p",
ctl->remote_map, ctl->remote_map + ctl->map_length);
if (fd < 0) {
pr_perror("Can't open remote parasite map");
diff --git a/proc_parse.c b/proc_parse.c
index 8c607de..6c5620f 100644
--- a/proc_parse.c
+++ b/proc_parse.c
@@ -28,18 +28,14 @@ int parse_maps(pid_t pid, int pid_dir, struct list_head *vma_area_list, bool use
DIR *map_files_dir = NULL;
FILE *maps = NULL;
- maps = fopen_proc(pid_dir, "maps");
- if (!maps) {
- pr_perror("Can't open %d's maps", pid);
+ maps = fopen_proc(pid, pid_dir, "maps");
+ if (!maps)
goto err;
- }
if (use_map_files) {
- map_files_dir = opendir_proc(pid_dir, "map_files");
- if (!map_files_dir) {
- pr_perror("Can't open %d's map_files (old kernel?)\n", pid);
+ map_files_dir = opendir_proc(pid, pid_dir, "map_files");
+ if (!map_files_dir) /* old kernel? */
goto err;
- }
}
while (fgets(big_buffer, sizeof(big_buffer), maps)) {
@@ -189,11 +185,9 @@ int parse_pid_stat_small(pid_t pid, int pid_dir, struct proc_pid_stat_small *s)
char *tok;
int n;
- f = fopen_proc(pid_dir, "stat");
- if (f == NULL) {
- pr_perror("Can't open %d's stat", pid);
+ f = fopen_proc(pid, pid_dir, "stat");
+ if (f == NULL)
return -1;
- }
memset(s, 0, sizeof(*s));
n = fscanf(f, "%d " PROC_TASK_COMM_LEN_FMT " %c",
@@ -219,11 +213,9 @@ int parse_pid_stat(pid_t pid, int pid_dir, struct proc_pid_stat *s)
char *tok;
int n;
- f = fopen_proc(pid_dir, "stat");
- if (f == NULL) {
- pr_perror("Can't open %d's stat", pid);
+ f = fopen_proc(pid, pid_dir, "stat");
+ if (f == NULL)
return -1;
- }
memset(s, 0, sizeof(*s));
n = fscanf(f,
@@ -326,13 +318,13 @@ static int cap_parse(char *str, unsigned int *res)
return 0;
}
-int parse_pid_status(int pid_dir, struct proc_status_creds *cr)
+int parse_pid_status(pid_t pid, int pid_dir, struct proc_status_creds *cr)
{
int done = 0;
FILE *f;
char str[64];
- f = fopen_proc(pid_dir, "status");
+ f = fopen_proc(pid, pid_dir, "status");
if (f == NULL) {
pr_perror("Can't open proc status");
return -1;
diff --git a/util.c b/util.c
index dfbcb98..bad52e4 100644
--- a/util.c
+++ b/util.c
@@ -217,46 +217,14 @@ int open_pid_proc(pid_t pid)
return fd;
}
-#define do_open_proc(pid_dir_fd, fmt, flags) \
- ({ \
- char fname[64]; \
- va_list args; \
- \
- va_start(args, fmt); \
- vsnprintf(fname, sizeof(fname), fmt, args); \
- va_end(args); \
- \
- openat(pid_dir_fd, fname, flags); \
- })
-
-int open_proc(int pid_dir_fd, char *fmt, ...)
+int do_open_proc(int dirfd, int flags, const char *fmt, ...)
{
- return do_open_proc(pid_dir_fd, fmt, O_RDONLY);
-}
-
-int open_proc_rw(int pid_dir_fd, char *fmt, ...)
-{
- return do_open_proc(pid_dir_fd, fmt, O_RDWR);
-}
-
-DIR *opendir_proc(int pid_dir_fd, char *fmt, ...)
-{
- int dirfd;
-
- dirfd = do_open_proc(pid_dir_fd, fmt);
- if (dirfd >= 0)
- return fdopendir(dirfd);
-
- return NULL;
-}
-
-FILE *fopen_proc(int pid_dir_fd, char *fmt, ...)
-{
- int fd;
+ char path[128];
+ va_list args;
- fd = do_open_proc(pid_dir_fd, fmt);
- if (fd >= 0)
- return fdopen(fd, "r");
+ va_start(args, fmt);
+ vsnprintf(path, sizeof(path), fmt, args);
+ va_end(args);
- return NULL;
+ return openat(dirfd, path, flags);
}
--
1.7.7.6
More information about the CRIU
mailing list