[CRIU] Re: [PATCH 8/8] protobuf: Convert IPC entries to PB format
Stanislav Kinsbursky
skinsbursky at parallels.com
Wed Jul 18 06:16:55 EDT 2012
Kirill, please, answer to several comment below.
Thanks.
17.07.2012 12:52, Cyrill Gorcunov пишет:
> Signed-off-by: Cyrill Gorcunov<gorcunov at openvz.org>
> ---
> include/image.h | 50 --------
> ipc_ns.c | 290 ++++++++++++++++++++++++++++-------------------
> protobuf/Makefile | 5 +
> protobuf/ipc-desc.proto | 9 ++
> protobuf/ipc-msg.proto | 12 ++
> protobuf/ipc-sem.proto | 6 +
> protobuf/ipc-shm.proto | 6 +
> protobuf/ipc-var.proto | 14 +++
> 8 files changed, 227 insertions(+), 165 deletions(-)
> create mode 100644 protobuf/ipc-desc.proto
> create mode 100644 protobuf/ipc-msg.proto
> create mode 100644 protobuf/ipc-sem.proto
> create mode 100644 protobuf/ipc-shm.proto
> create mode 100644 protobuf/ipc-var.proto
>
>
> 0008-protobuf-Convert-IPC-entries-to-PB-format.patch
>
>
> diff --git a/include/image.h b/include/image.h
> index 80292b7..852cac0 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -80,56 +80,6 @@ struct vma_entry {
> s64 fd;
> } __packed;
>
> -struct ipc_var_entry {
> - u32 sem_ctls[4];
> - u32 msg_ctlmax;
> - u32 msg_ctlmnb;
> - u32 msg_ctlmni;
> - u32 auto_msgmni;
> - u64 shm_ctlmax;
> - u64 shm_ctlall;
> - u32 shm_ctlmni;
> - u32 shm_rmid_forced;
> - u32 mq_queues_max;
> - u32 mq_msg_max;
> - u32 mq_msgsize_max;
> -} __packed;
> -
> -struct ipc_desc_entry {
> - u32 key;
> - u32 uid;
> - u32 gid;
> - u32 cuid;
> - u32 cgid;
> - u32 mode;
> - u32 id;
> - u8 pad[4];
> -} __packed;
> -
> -struct ipc_shm_entry {
> - struct ipc_desc_entry desc;
> - u64 size;
> -} __packed;
> -
> -struct ipc_msg {
> - u64 mtype;
> - u32 msize;
> - u8 pad[4];
> -} __packed;
> -
> -struct ipc_msg_entry {
> - struct ipc_desc_entry desc;
> - u16 qbytes;
> - u16 qnum;
> - u8 pad[4];
> -} __packed;
> -
> -struct ipc_sem_entry {
> - struct ipc_desc_entry desc;
> - u16 nsems;
> - u8 pad[6];
> -} __packed;
> -
> #define VMA_AREA_NONE (0 << 0)
> #define VMA_AREA_REGULAR (1 << 0) /* Dumpable area */
> #define VMA_AREA_STACK (1 << 1)
> diff --git a/ipc_ns.c b/ipc_ns.c
> index 6dfe8f0..d653f55 100644
> --- a/ipc_ns.c
> +++ b/ipc_ns.c
> @@ -13,6 +13,13 @@
> #include "namespaces.h"
> #include "sysctl.h"
>
> +#include "protobuf.h"
> +#include "protobuf/ipc-var.pb-c.h"
> +#include "protobuf/ipc-desc.pb-c.h"
Does this include really required?IpcMsg msg = IPC_MSG__IN
> +#include "protobuf/ipc-shm.pb-c.h"
> +#include "protobuf/ipc-sem.pb-c.h"
> +#include "protobuf/ipc-msg.pb-c.h"
> +
> #if defined (__GLIBC__) && __GLIBC__ >= 2
> #define KEY __key
> #else
> @@ -51,7 +58,7 @@ struct msgbuf_a {
> #define SEM_SET 20
> #endif
>
> -static void pr_ipc_desc_entry(unsigned int loglevel, const struct ipc_desc_entry *desc)
> +static void pr_ipc_desc_entry(unsigned int loglevel, const IpcDescEntry *desc)
> {
> print_on_level(loglevel, "id: %-10d key: 0x%08x ", desc->id, desc->key);
> print_on_level(loglevel, "uid: %-10d gid: %-10d ", desc->uid, desc->gid);
> @@ -59,8 +66,7 @@ static void pr_ipc_desc_entry(unsigned int loglevel, const struct ipc_desc_entry
> print_on_level(loglevel, "mode: %-10o ", desc->mode);
> }
>
> -static void fill_ipc_desc(int id, struct ipc_desc_entry *desc,
> - const struct ipc_perm *ipcp)
> +static void fill_ipc_desc(int id, IpcDescEntry *desc, const struct ipc_perm *ipcp)
> {
> desc->id = id;
> desc->key = ipcp->KEY;
> @@ -81,16 +87,16 @@ static void pr_ipc_sem_array(unsigned int loglevel, int nr, u16 *values)
> #define pr_info_ipc_sem_array(nr, values) pr_ipc_sem_array(LOG_INFO, nr, values)
> #define pr_msg_ipc_sem_array(nr, values) pr_ipc_sem_array(LOG_MSG, nr, values)
>
> -static void pr_ipc_sem_entry(unsigned int loglevel, const struct ipc_sem_entry *sem)
> +static void pr_ipc_sem_entry(unsigned int loglevel, const IpcSemEntry *sem)
> {
> - pr_ipc_desc_entry(loglevel, &sem->desc);
> + pr_ipc_desc_entry(loglevel, sem->desc);
> print_on_level(loglevel, "nsems: %-10d\n", sem->nsems);
> }
>
> #define pr_info_ipc_sem_entry(sem) pr_ipc_sem_entry(LOG_INFO, sem)
> #define pr_msg_ipc_sem_entry(sem) pr_ipc_sem_entry(LOG_MSG, sem)
>
> -static int dump_ipc_sem_set(int fd, const struct ipc_sem_entry *entry)
> +static int dump_ipc_sem_set(int fd, const IpcSemEntry *entry)
> {
> int ret, size;
> u16 *values;
> @@ -102,7 +108,7 @@ static int dump_ipc_sem_set(int fd, const struct ipc_sem_entry *entry)
> ret = -ENOMEM;
> goto out;
> }
> - ret = semctl(entry->desc.id, 0, GETALL, values);
> + ret = semctl(entry->desc->id, 0, GETALL, values);
> if (ret < 0) {
> pr_perror("Failed to get semaphore set values");
> ret = -errno;
> @@ -122,14 +128,17 @@ out:
>
> static int dump_ipc_sem_desc(int fd, int id, const struct semid_ds *ds)
> {
> - struct ipc_sem_entry sem;
> + IpcSemEntry sem = IPC_SEM_ENTRY__INIT;
> + IpcDescEntry desc = IPC_DESC_ENTRY__INIT;
> int ret;
>
> - fill_ipc_desc(id, &sem.desc, &ds->sem_perm);
> + sem.desc = &desc;
> sem.nsems = ds->sem_nsems;
> +
> + fill_ipc_desc(id, sem.desc, &ds->sem_perm);
> pr_info_ipc_sem_entry(&sem);
>
> - ret = write_img(fd, &sem);
> + ret = pb_write(fd, &sem, ipc_sem_entry);
> if (ret < 0) {
> pr_err("Failed to write IPC semaphores set\n");
> return ret;
> @@ -172,7 +181,7 @@ static int dump_ipc_sem(int fd)
> return info.semusz;
> }
>
> -static void pr_ipc_msg(unsigned int loglevel, int nr, const struct ipc_msg *msg)
> +static void pr_ipc_msg(unsigned int loglevel, int nr, const IpcMsg *msg)
> {
> print_on_level(loglevel, " %-5d: type: %-20ld size: %-10d\n",
> nr++, msg->mtype, msg->msize);
> @@ -181,9 +190,9 @@ static void pr_ipc_msg(unsigned int loglevel, int nr, const struct ipc_msg *msg)
> #define pr_info_ipc_msg(nr, msg) pr_ipc_msg(LOG_INFO, nr, msg)
> #define pr_msg_ipc_msg(nr, msg) pr_ipc_msg(LOG_MSG, nr, msg)
>
> -static void pr_ipc_msg_entry(unsigned int loglevel, const struct ipc_msg_entry *msg)
> +static void pr_ipc_msg_entry(unsigned int loglevel, const IpcMsgEntry *msg)
> {
> - pr_ipc_desc_entry(loglevel, &msg->desc);
> + pr_ipc_desc_entry(loglevel, msg->desc);
> print_on_level(loglevel, "qbytes: %-10d qnum: %-10d\n",
> msg->qbytes, msg->qnum);
> }
> @@ -191,7 +200,7 @@ static void pr_ipc_msg_entry(unsigned int loglevel, const struct ipc_msg_entry *
> #define pr_info_ipc_msg_entry(msg) pr_ipc_msg_entry(LOG_INFO, msg)
> #define pr_msg_ipc_msg_entry(msg) pr_ipc_msg_entry(LOG_MSG, msg)
>
> -static int dump_ipc_msg_queue_messages(int fd, const struct ipc_msg_entry *entry, size_t cbytes)
> +static int dump_ipc_msg_queue_messages(int fd, const IpcMsgEntry *entry, size_t cbytes)
> {
> void *msg_array, *ptr;
> size_t array_size;
> @@ -208,7 +217,7 @@ static int dump_ipc_msg_queue_messages(int fd, const struct ipc_msg_entry *entry
> return -ENOMEM;
> }
>
> - ret = msgrcv(entry->desc.id, msg_array, array_size, 0, IPC_NOWAIT | MSG_STEAL);
> + ret = msgrcv(entry->desc->id, msg_array, array_size, 0, IPC_NOWAIT | MSG_STEAL);
> if (ret < 0) {
> pr_perror("Failed to receive IPC messages array");
> goto err;
> @@ -216,14 +225,14 @@ static int dump_ipc_msg_queue_messages(int fd, const struct ipc_msg_entry *entry
>
> while (msg_nr < entry->qnum) {
> struct msgbuf_a *data = ptr;
> - struct ipc_msg msg;
> + IpcMsg msg = IPC_MSG__INIT;
>
> msg.msize = data->msize;
> msg.mtype = data->mtype;
>
> pr_info_ipc_msg(msg_nr, &msg);
>
> - ret = write_img(fd, &msg);
> + ret = pb_write(fd, &msg, ipc_msg);
> if (ret < 0) {
> pr_err("Failed to write IPC message header\n");
> break;
> @@ -244,15 +253,18 @@ err:
>
> static int dump_ipc_msg_queue(int fd, int id, const struct msqid_ds *ds)
> {
> - struct ipc_msg_entry msg;
> + IpcMsgEntry msg = IPC_MSG_ENTRY__INIT;
> + IpcDescEntry desc = IPC_DESC_ENTRY__INIT;
> int ret;
>
> - fill_ipc_desc(id, &msg.desc, &ds->msg_perm);
> + msg.desc = &desc;
> + fill_ipc_desc(id, msg.desc, &ds->msg_perm);
> msg.qbytes = ds->msg_qbytes;
> msg.qnum = ds->msg_qnum;
> +
> pr_info_ipc_msg_entry(&msg);
>
> - ret = write_img(fd, &msg);
> + ret = pb_write(fd, &msg, ipc_msg_entry);
> if (ret < 0) {
> pr_err("Failed to write IPC message queue\n");
> return ret;
> @@ -295,19 +307,19 @@ static int dump_ipc_msg(int fd)
> return info.msgpool;
> }
>
> -static void pr_ipc_shm(unsigned int loglevel, const struct ipc_shm_entry *shm)
> +static void pr_ipc_shm(unsigned int loglevel, const IpcShmEntry *shm)
> {
> - pr_ipc_desc_entry(loglevel, &shm->desc);
> + pr_ipc_desc_entry(loglevel, shm->desc);
> print_on_level(loglevel, "size: %-10lu\n", shm->size);
> }
>
> #define pr_info_ipc_shm(shm) pr_ipc_shm(LOG_INFO, shm)
> #define pr_msg_ipc_shm(shm) pr_ipc_shm(LOG_MSG, shm)
>
> -static int ipc_sysctl_req(struct ipc_var_entry *e, int op)
> +static int ipc_sysctl_req(IpcVarEntry *e, int op)
> {
> struct sysctl_req req[] = {
> - { "kernel/sem", e->sem_ctls, CTL_U32A(4) },
> + { "kernel/sem", e->sem_ctls, CTL_U32A(e->n_sem_ctls) },
> { "kernel/msgmax", &e->msg_ctlmax, CTL_U32 },
> { "kernel/msgmnb", &e->msg_ctlmnb, CTL_U32 },
> { "kernel/msgmni", &e->msg_ctlmni, CTL_U32 },
> @@ -329,12 +341,12 @@ static int ipc_sysctl_req(struct ipc_var_entry *e, int op)
> * TODO: Function below should be later improved to locate and dump only dirty
> * pages via updated sys_mincore().
> */
> -static int dump_ipc_shm_pages(int fd, const struct ipc_shm_entry *shm)
> +static int dump_ipc_shm_pages(int fd, const IpcShmEntry *shm)
> {
> void *data;
> int ret;
>
> - data = shmat(shm->desc.id, NULL, SHM_RDONLY);
> + data = shmat(shm->desc->id, NULL, SHM_RDONLY);
> if (data == (void *)-1) {
> pr_perror("Failed to attach IPC shared memory");
> return -errno;
> @@ -353,14 +365,16 @@ static int dump_ipc_shm_pages(int fd, const struct ipc_shm_entry *shm)
>
> static int dump_ipc_shm_seg(int fd, int id, const struct shmid_ds *ds)
> {
> - struct ipc_shm_entry shm;
> + IpcShmEntry shm = IPC_SHM_ENTRY__INIT;
> + IpcDescEntry desc = IPC_DESC_ENTRY__INIT;
> int ret;
>
> - fill_ipc_desc(id, &shm.desc, &ds->shm_perm);
> + shm.desc = &desc;
> shm.size = ds->shm_segsz;
> + fill_ipc_desc(id, shm.desc, &ds->shm_perm);
> pr_info_ipc_shm(&shm);
>
> - ret = write_img(fd, &shm);
> + ret = pb_write(fd, &shm, ipc_shm_entry);
> if (ret < 0) {
> pr_err("Failed to write IPC shared memory segment\n");
> return ret;
> @@ -407,21 +421,29 @@ static int dump_ipc_shm(int fd)
>
> static int dump_ipc_var(int fd)
> {
> - int ret;
> - struct ipc_var_entry var;
> + IpcVarEntry var = IPC_VAR_ENTRY__INIT;
> + int ret = -1;
> +
> + var.n_sem_ctls = 4;
> + var.sem_ctls = xmalloc(pb_repeated_size(&var, sem_ctls));
Off-topic: is "pb_repeated_size" our helper or generic one?
> + if (!var.sem_ctls)
> + goto err;
>
> ret = ipc_sysctl_req(&var, CTL_READ);
> if (ret < 0) {
> pr_err("Failed to read IPC variables\n");
> - return ret;
> + goto err;
> }
>
> - ret = write_img(fd, &var);
> + ret = pb_write(fd, &var, ipc_var_entry);
> if (ret < 0) {
> pr_err("Failed to write IPC variables\n");
> - return ret;
> + goto err;
> }
> - return 0;
> +
> +err:
> + xfree(var.sem_ctls);
> + return ret;
> }
>
> static int dump_ipc_data(const struct cr_fdset *fdset)
> @@ -461,23 +483,34 @@ int dump_ipc_ns(int ns_pid, const struct cr_fdset *fdset)
>
> static void show_ipc_sem_entries(int fd)
> {
> + IpcSemEntry *entry;
> + u16 *values;
> +
> pr_msg("\nSemaphores sets:\n");
> while (1) {
> int size;
> - struct ipc_sem_entry entry;
> - u16 *values;
>
> - if (read_img_eof(fd, &entry) <= 0)
> - return;
> - pr_msg_ipc_sem_entry(&entry);
> - size = sizeof(u16) * entry.nsems;
> + entry = NULL;
Looks like this assignment is redundant.
> + values = NULL;
> +
> + if (pb_read_eof(fd, &entry, ipc_sem_entry) <= 0)
> + break;
> + pr_msg_ipc_sem_entry(entry);
> + size = sizeof(u16) * entry->nsems;
> values = xmalloc(size);
> if (values == NULL)
> - return;
> + break;
> if (read_img_buf(fd, values, round_up(size, sizeof(u64))) <= 0)
Probably, "value" should be a repeated field in sem protobuf description?
Binary, for example.
> - return;
> - pr_msg_ipc_sem_array(entry.nsems, values);
> + break;
> + pr_msg_ipc_sem_array(entry->nsems, values);
> +
> + ipc_sem_entry__free_unpacked(entry, NULL);
> + xfree(values);
> }
> +
> + xfree(values);
> + if (entry)
> + ipc_sem_entry__free_unpacked(entry, NULL);
Is this an optimization? I.e. allocating this entry only once and releasing it
at the end?
> }
>
> void show_ipc_sem(int fd, struct cr_options *o)
> @@ -492,29 +525,36 @@ static void show_ipc_msg_entries(int fd)
> pr_msg("\nMessage queues:\n");
> while (1) {
> int ret;
> - struct ipc_msg_entry entry;
> + IpcMsgEntry *entry;
> int msg_nr = 0;
>
> - ret = read_img_eof(fd, &entry);
> + ret = pb_read_eof(fd, &entry, ipc_msg_entry);
> if (ret <= 0)
> return;
>
> - pr_msg_ipc_msg_entry(&entry);
> + pr_msg_ipc_msg_entry(entry);
>
> - while (msg_nr < entry.qnum) {
> - struct ipc_msg msg;
> + while (msg_nr < entry->qnum) {
> + IpcMsg *msg;
>
> - ret = read_img(fd, &msg);
> + ret = pb_read(fd, &msg, ipc_msg);
> if (ret <= 0)
> - return;
> + break;
>
> - pr_msg_ipc_msg(msg_nr, &msg);
> + pr_msg_ipc_msg(msg_nr, msg);
>
> - if (lseek(fd, round_up(msg.msize, sizeof(u64)),
> - SEEK_CUR) == (off_t) -1)
> - return;
> + if (lseek(fd, round_up(msg->msize, sizeof(u64)), SEEK_CUR) == (off_t) -1)
> + ret = -1;
> + ipc_msg__free_unpacked(msg, NULL);
> msg_nr++;
> +
> + if (ret < 0)
> + break;
> }
> +
> + ipc_msg_entry__free_unpacked(entry, NULL);
> + if (ret < 0)
> + break;
> }
> }
>
> @@ -530,16 +570,20 @@ static void show_ipc_shm_entries(int fd)
> pr_msg("\nShared memory segments:\n");
> while (1) {
> int ret;
> - struct ipc_shm_entry shm;
> + IpcShmEntry *shm;
>
> - ret = read_img_eof(fd, &shm);
> + ret = pb_read_eof(fd, &shm, ipc_shm_entry);
> if (ret <= 0)
> return;
>
> - pr_msg_ipc_shm(&shm);
> + pr_msg_ipc_shm(shm);
>
> - if (lseek(fd, round_up(shm.size, sizeof(u32)), SEEK_CUR) == (off_t) -1)
> - return;
> + if (lseek(fd, round_up(shm->size, sizeof(u32)), SEEK_CUR) == (off_t) -1)
> + ret = -1;
> +
> + ipc_shm_entry__free_unpacked(shm, NULL);
Maybe we should use one way of managing PB unpacked entries? I.e. either alloc
and free only once per image or once per message.
> + if (ret < 0)
> + break;
> }
> }
>
> @@ -553,12 +597,14 @@ void show_ipc_shm(int fd, struct cr_options *o)
> static void show_ipc_var_entry(int fd)
> {
> int ret;
> - struct ipc_var_entry var;
> + IpcVarEntry *var;
>
> - ret = read_img_eof(fd, &var);
> + ret = pb_read_eof(fd, &var, ipc_var_entry);
> if (ret <= 0)
> return;
> - ipc_sysctl_req(&var, CTL_SHOW);
> + ipc_sysctl_req(var, CTL_SHOW);
> +
> + ipc_var_entry__free_unpacked(var, NULL);
> }
>
> void show_ipc_var(int fd, struct cr_options *o)
> @@ -568,7 +614,7 @@ void show_ipc_var(int fd, struct cr_options *o)
> pr_img_tail(CR_FD_IPCNS);
> }
>
> -static int prepare_ipc_sem_values(int fd, const struct ipc_sem_entry *entry)
> +static int prepare_ipc_sem_values(int fd, const IpcSemEntry *entry)
> {
> int ret, size;
> u16 *values;
> @@ -590,7 +636,7 @@ static int prepare_ipc_sem_values(int fd, const struct ipc_sem_entry *entry)
>
> pr_info_ipc_sem_array(entry->nsems, values);
>
> - ret = semctl(entry->desc.id, 0, SETALL, values);
> + ret = semctl(entry->desc->id, 0, SETALL, values);
> if (ret < 0) {
> pr_perror("Failed to set semaphores set values");
> ret = -errno;
> @@ -600,21 +646,21 @@ out:
> return ret;
> }
>
> -static int prepare_ipc_sem_desc(int fd, const struct ipc_sem_entry *entry)
> +static int prepare_ipc_sem_desc(int fd, const IpcSemEntry *entry)
> {
> int ret, id;
> struct semid_ds ds;
>
> - id = semget(entry->desc.id, entry->nsems,
> - entry->desc.mode | IPC_CREAT | IPC_EXCL | IPC_PRESET);
> + id = semget(entry->desc->id, entry->nsems,
> + entry->desc->mode | IPC_CREAT | IPC_EXCL | IPC_PRESET);
> if (id == -1) {
> pr_perror("Failed to create sem set");
> return -errno;
> }
>
> - if (id != entry->desc.id) {
> + if (id != entry->desc->id) {
> pr_err("Failed to preset id (%d instead of %d)\n",
> - id, entry->desc.id);
> + id, entry->desc->id);
> return -EFAULT;
> }
>
> @@ -624,7 +670,7 @@ static int prepare_ipc_sem_desc(int fd, const struct ipc_sem_entry *entry)
> return -errno;
> }
>
> - ds.sem_perm.KEY = entry->desc.key;
> + ds.sem_perm.KEY = entry->desc->key;
> ret = semctl(id, 0, SEM_SET, &ds);
> if (ret < 0) {
> pr_perror("Failed to update sem key");
> @@ -649,17 +695,19 @@ static int prepare_ipc_sem(int pid)
>
> while (1) {
> int ret;
> - struct ipc_sem_entry entry;
> + IpcSemEntry *entry;
>
> - ret = read_img_eof(fd, &entry);
> + ret = pb_read_eof(fd, &entry, ipc_sem_entry);
> if (ret < 0)
> return -EIO;
> if (ret == 0)
> break;
>
> - pr_info_ipc_sem_entry(&entry);
> + pr_info_ipc_sem_entry(entry);
> +
> + ret = prepare_ipc_sem_desc(fd, entry);
> + ipc_sem_entry__free_unpacked(entry, NULL);
>
Ditto.
> - ret = prepare_ipc_sem_desc(fd, &entry);
> if (ret < 0) {
> pr_err("Failed to prepare semaphores set\n");
> return ret;
> @@ -668,62 +716,68 @@ static int prepare_ipc_sem(int pid)
> return close_safe(&fd);
> }
>
> -static int prepare_ipc_msg_queue_messages(int fd, const struct ipc_msg_entry *entry)
> +static int prepare_ipc_msg_queue_messages(int fd, const IpcMsgEntry *entry)
> {
> + IpcMsg *msg = NULL;
> int msg_nr = 0;
> + int ret = 0;
>
> while (msg_nr < entry->qnum) {
> struct msgbuf {
> long mtype;
> char mtext[MSGMAX];
> } data;
> - struct ipc_msg msg;
> - int ret;
>
> - ret = read_img(fd, &msg);
> + ret = pb_read(fd, &msg, ipc_msg);
> if (ret <= 0)
> return -EIO;
>
> - pr_info_ipc_msg(msg_nr, &msg);
> + pr_info_ipc_msg(msg_nr, msg);
>
> - if (msg.msize > MSGMAX) {
> + if (msg->msize > MSGMAX) {
> + ret = -1;
> pr_err("Unsupported message size: %d (MAX: %d)\n",
> - msg.msize, MSGMAX);
> - return ret;
> + msg->msize, MSGMAX);
> + goto out;
"out" label is redundant. Break is enough. This gives us minus 1 line.
> }
>
> - ret = read_img_buf(fd, data.mtext, round_up(msg.msize, sizeof(u64)));
> + ret = read_img_buf(fd, data.mtext, round_up(msg->msize, sizeof(u64)));
> if (ret < 0) {
> pr_err("Failed to read IPC message data\n");
> - return ret;
> + goto out;
> }
>
> - data.mtype = msg.mtype;
> - ret = msgsnd(entry->desc.id, &data, msg.msize, IPC_NOWAIT);
> + data.mtype = msg->mtype;
> + ret = msgsnd(entry->desc->id, &data, msg->msize, IPC_NOWAIT);
> if (ret < 0) {
> pr_perror("Failed to send IPC message");
> - return -errno;
> + ret = -errno;
> + goto out;
> }
> msg_nr++;
> }
> - return 0;
> +
> +out:
> + if (msg)
> + ipc_msg__free_unpacked(msg, NULL);
> + return ret;
> }
>
> -static int prepare_ipc_msg_queue(int fd, const struct ipc_msg_entry *entry)
> +static int prepare_ipc_msg_queue(int fd, const IpcMsgEntry *entry)
> {
> int ret, id;
> struct msqid_ds ds;
>
> - id = msgget(entry->desc.id,
> - entry->desc.mode | IPC_CREAT | IPC_EXCL | IPC_PRESET);
> + id = msgget(entry->desc->id,
> + entry->desc->mode | IPC_CREAT | IPC_EXCL | IPC_PRESET);
> if (id == -1) {
> pr_perror("Failed to create message queue");
> return -errno;
> }
>
> - if (id != entry->desc.id) {
> + if (id != entry->desc->id) {
> pr_err("Failed to preset id (%d instead of %d)\n",
> - id, entry->desc.id);
> + id, entry->desc->id);
> return -EFAULT;
> }
>
> @@ -733,7 +787,7 @@ static int prepare_ipc_msg_queue(int fd, const struct ipc_msg_entry *entry)
> return -errno;
> }
>
> - ds.msg_perm.KEY = entry->desc.key;
> + ds.msg_perm.KEY = entry->desc->key;
> ds.msg_qbytes = entry->qbytes;
> ret = msgctl(id, MSG_SET, &ds);
> if (ret < 0) {
> @@ -759,9 +813,9 @@ static int prepare_ipc_msg(int pid)
>
> while (1) {
> int ret;
> - struct ipc_msg_entry entry;
> + IpcMsgEntry *entry;
>
> - ret = read_img_eof(fd, &entry);
> + ret = pb_read_eof(fd, &entry, ipc_msg_entry);
> if (ret < 0) {
> pr_err("Failed to read IPC messages queue\n");
> return -EIO;
> @@ -769,9 +823,11 @@ static int prepare_ipc_msg(int pid)
> if (ret == 0)
> break;
>
> - pr_info_ipc_msg_entry(&entry);
> + pr_info_ipc_msg_entry(entry);
> +
> + ret = prepare_ipc_msg_queue(fd, entry);
> + ipc_msg_entry__free_unpacked(entry, NULL);
>
> - ret = prepare_ipc_msg_queue(fd, &entry);
> if (ret < 0) {
> pr_err("Failed to prepare messages queue\n");
> return ret;
> @@ -780,12 +836,12 @@ static int prepare_ipc_msg(int pid)
> return close_safe(&fd);
> }
>
> -static int prepare_ipc_shm_pages(int fd, const struct ipc_shm_entry *shm)
> +static int prepare_ipc_shm_pages(int fd, const IpcShmEntry *shm)
> {
> int ret;
> void *data;
>
> - data = shmat(shm->desc.id, NULL, 0);
> + data = shmat(shm->desc->id, NULL, 0);
> if (data == (void *)-1) {
> pr_perror("Failed to attach IPC shared memory");
> return -errno;
> @@ -802,21 +858,21 @@ static int prepare_ipc_shm_pages(int fd, const struct ipc_shm_entry *shm)
> return 0;
> }
>
> -static int prepare_ipc_shm_seg(int fd, const struct ipc_shm_entry *shm)
> +static int prepare_ipc_shm_seg(int fd, const IpcShmEntry *shm)
> {
> int ret, id;
> struct shmid_ds ds;
>
> - id = shmget(shm->desc.id, shm->size,
> - shm->desc.mode | IPC_CREAT | IPC_EXCL | IPC_PRESET);
> + id = shmget(shm->desc->id, shm->size,
> + shm->desc->mode | IPC_CREAT | IPC_EXCL | IPC_PRESET);
> if (id == -1) {
> pr_perror("Failed to create shm segment");
> return -errno;
> }
>
> - if (id != shm->desc.id) {
> + if (id != shm->desc->id) {
> pr_err("Failed to preset id (%d instead of %d)\n",
> - id, shm->desc.id);
> + id, shm->desc->id);
> return -EFAULT;
> }
>
> @@ -826,7 +882,7 @@ static int prepare_ipc_shm_seg(int fd, const struct ipc_shm_entry *shm)
> return -errno;
> }
>
> - ds.shm_perm.KEY = shm->desc.key;
> + ds.shm_perm.KEY = shm->desc->key;
> ret = shmctl(id, SHM_SET, &ds);
> if (ret < 0) {
> pr_perror("Failed to update shm key");
> @@ -851,9 +907,9 @@ static int prepare_ipc_shm(int pid)
>
> while (1) {
> int ret;
> - struct ipc_shm_entry shm;
> + IpcShmEntry *shm;
>
> - ret = read_img_eof(fd, &shm);
> + ret = pb_read_eof(fd, &shm, ipc_shm_entry);
> if (ret < 0) {
> pr_err("Failed to read IPC shared memory segment\n");
> return -EIO;
> @@ -861,9 +917,11 @@ static int prepare_ipc_shm(int pid)
> if (ret == 0)
> break;
>
> - pr_info_ipc_shm(&shm);
> + pr_info_ipc_shm(shm);
> +
> + ret = prepare_ipc_shm_seg(fd, shm);
> + ipc_shm_entry__free_unpacked(shm, NULL);
>
> - ret = prepare_ipc_shm_seg(fd, &shm);
> if (ret < 0) {
> pr_err("Failed to prepare shm segment\n");
> return ret;
> @@ -875,22 +933,24 @@ static int prepare_ipc_shm(int pid)
> static int prepare_ipc_var(int pid)
> {
> int fd, ret;
> - struct ipc_var_entry var;
> + IpcVarEntry *var;
>
> pr_info("Restoring IPC variables\n");
> fd = open_image_ro(CR_FD_IPCNS_VAR, pid);
> if (fd < 0)
> return -1;
>
> - ret = read_img(fd, &var);
> + ret = pb_read(fd, &var, ipc_var_entry);
> if (ret <= 0) {
> pr_err("Failed to read IPC namespace variables\n");
> return -EFAULT;
> }
>
> - ipc_sysctl_req(&var, CTL_PRINT);
> + ipc_sysctl_req(var, CTL_PRINT);
> +
> + ret = ipc_sysctl_req(var, CTL_WRITE);
> + ipc_var_entry__free_unpacked(var, NULL);
>
> - ret = ipc_sysctl_req(&var, CTL_WRITE);
> if (ret < 0) {
> pr_err("Failed to prepare IPC namespace variables\n");
> return -EFAULT;
> diff --git a/protobuf/Makefile b/protobuf/Makefile
> index a0d19ce..9ed70a7 100644
> --- a/protobuf/Makefile
> +++ b/protobuf/Makefile
> @@ -39,6 +39,11 @@ PROTO_FILES += sk-opts.proto
> PROTO_FILES += sk-unix.proto
> PROTO_FILES += sk-inet.proto
> PROTO_FILES += mnt.proto
> +PROTO_FILES += ipc-var.proto
> +PROTO_FILES += ipc-desc.proto
Do we really need to compile this one?
> +PROTO_FILES += ipc-shm.proto
> +PROTO_FILES += ipc-msg.proto
> +PROTO_FILES += ipc-sem.proto
>
> HDRS := $(patsubst %.proto,%.pb-c.h,$(PROTO_FILES))
> SRCS := $(patsubst %.proto,%.pb-c.c,$(PROTO_FILES))
> diff --git a/protobuf/ipc-desc.proto b/protobuf/ipc-desc.proto
> new file mode 100644
> index 0000000..2a705ac
> --- /dev/null
> +++ b/protobuf/ipc-desc.proto
> @@ -0,0 +1,9 @@
> +message ipc_desc_entry {
> + required uint32 key = 1;
> + required uint32 uid = 2;
> + required uint32 gid = 3;
> + required uint32 cuid = 4;
> + required uint32 cgid = 5;
> + required uint32 mode = 6;
> + required uint32 id = 7;
> +}
> diff --git a/protobuf/ipc-msg.proto b/protobuf/ipc-msg.proto
> new file mode 100644
> index 0000000..6e42f9b
> --- /dev/null
> +++ b/protobuf/ipc-msg.proto
> @@ -0,0 +1,12 @@
> +import "ipc-desc.proto";
> +
> +message ipc_msg {
> + required uint64 mtype = 1;
> + required uint32 msize = 2;
> +}
> +
> +message ipc_msg_entry {
> + required ipc_desc_entry desc = 1;
> + required uint32 qbytes = 2;
> + required uint32 qnum = 3;
> +}
> diff --git a/protobuf/ipc-sem.proto b/protobuf/ipc-sem.proto
> new file mode 100644
> index 0000000..4ec65be
> --- /dev/null
> +++ b/protobuf/ipc-sem.proto
> @@ -0,0 +1,6 @@
> +import "ipc-desc.proto";
> +
> +message ipc_sem_entry {
> + required ipc_desc_entry desc = 1;
> + required uint32 nsems = 2;
> +}
> diff --git a/protobuf/ipc-shm.proto b/protobuf/ipc-shm.proto
> new file mode 100644
> index 0000000..b402e8e
> --- /dev/null
> +++ b/protobuf/ipc-shm.proto
> @@ -0,0 +1,6 @@
> +import "ipc-desc.proto";
> +
> +message ipc_shm_entry {
> + required ipc_desc_entry desc = 1;
> + required uint64 size = 2;
> +}
> diff --git a/protobuf/ipc-var.proto b/protobuf/ipc-var.proto
> new file mode 100644
> index 0000000..4acda69
> --- /dev/null
> +++ b/protobuf/ipc-var.proto
> @@ -0,0 +1,14 @@
> +message ipc_var_entry {
> + repeated uint32 sem_ctls = 1;
> + required uint32 msg_ctlmax = 2;
> + required uint32 msg_ctlmnb = 3;
> + required uint32 msg_ctlmni = 4;
> + required uint32 auto_msgmni = 5;
> + required uint64 shm_ctlmax = 6;
> + required uint64 shm_ctlall = 7;
> + required uint32 shm_ctlmni = 8;
> + required uint32 shm_rmid_forced = 9;
> + required uint32 mq_queues_max = 10;
> + required uint32 mq_msg_max = 11;
> + required uint32 mq_msgsize_max = 12;
> +}
>
--
Best regards,
Stanislav Kinsbursky
More information about the CRIU
mailing list