[CRIU] [PATCH v3 1/4] IPC: dump shared memory
Kinsbursky Stanislav
skinsbursky at openvz.org
Thu Feb 9 03:13:21 EST 2012
08.02.2012 22:48, Kir Kolyshkin пишет:
> On 02/08/2012 09:26 PM, Kinsbursky Stanislav wrote:
>> Signed-off-by: Stanislav Kinsbursky<skinsbursky at parallels.com>
>>
>> ---
>> crtools.c | 6 +++
>> include/crtools.h | 5 ++-
>> include/image.h | 17 +++++++++
>> ipc_ns.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>> 4 files changed, 123 insertions(+), 11 deletions(-)
>>
>> diff --git a/crtools.c b/crtools.c
>> index 6f6c254..c43eeab 100644
>> --- a/crtools.c
>> +++ b/crtools.c
>> @@ -116,6 +116,12 @@ struct cr_fd_desc_tmpl fdset_template[CR_FD_MAX] = {
>> .fmt = FMT_FNAME_IPCNS_VAR,
>> .magic = IPCNS_VAR_MAGIC,
>> },
>> +
>> + /* IPC namespace shared memory */
>> + [CR_FD_IPCNS_SHM] = {
>> + .fmt = FMT_FNAME_IPCNS_SHM,
>> + .magic = IPCNS_SHM_MAGIC,
>> + },
>> };
>>
>> static struct cr_fdset *alloc_cr_fdset(void)
>> diff --git a/include/crtools.h b/include/crtools.h
>> index 868ab38..cf7a46b 100644
>> --- a/include/crtools.h
>> +++ b/include/crtools.h
>> @@ -38,6 +38,7 @@ enum {
>> CR_FD_PSTREE,
>> CR_FD_UTSNS,
>> CR_FD_IPCNS_VAR,
>> + CR_FD_IPCNS_SHM,
>>
>> CR_FD_MAX
>> };
>> @@ -80,6 +81,7 @@ extern struct cr_fd_desc_tmpl fdset_template[CR_FD_MAX];
>> #define FMT_FNAME_CREDS "creds-%d.img"
>> #define FMT_FNAME_UTSNS "utsns-%d.img"
>> #define FMT_FNAME_IPCNS_VAR "ipcns-var-%d.img"
>> +#define FMT_FNAME_IPCNS_SHM "ipcns-shm-%d.img"
>>
>> extern int get_image_path(char *path, int size, const char *fmt, int pid);
>>
>> @@ -111,7 +113,8 @@ struct cr_fdset {
>> CR_FD_DESC_USE(CR_FD_CREDS) )
>> #define CR_FD_DESC_NS (\
>> CR_FD_DESC_USE(CR_FD_UTSNS) |\
>> - CR_FD_DESC_USE(CR_FD_IPCNS_VAR) )
>> + CR_FD_DESC_USE(CR_FD_IPCNS_VAR) |\
>> + CR_FD_DESC_USE(CR_FD_IPCNS_SHM) )
> If you move the ending bracket ) to a separate line, your next patches
> will be 1 line shorter and easier to read.
It's not so hard as is. But I'll take it into account.
>> #define CR_FD_DESC_NONE (0)
>>
>> int cr_dump_tasks(pid_t pid, struct cr_options *opts);
>> diff --git a/include/image.h b/include/image.h
>> index 84712a9..01a3b86 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -22,6 +22,7 @@
>> #define UTSNS_MAGIC 0x54473203 /* Smolensk */
>> #define CREDS_MAGIC 0x54023547 /* Kozelsk */
>> #define IPCNS_VAR_MAGIC 0x53115007 /* Samara */
>> +#define IPCNS_SHM_MAGIC 0x46283044 /* Odessa */
>>
>> #define PIPEFS_MAGIC 0x50495045
>>
>> @@ -125,6 +126,22 @@ struct ipc_var_entry {
>> u32 mq_msgsize_max;
>> } __packed;
>>
>> +struct ipc_seg {
>> + u32 key;
>> + u32 uid;
>> + u32 gid;
>> + u32 cuid;
>> + u32 cgid;
>> + u32 mode;
>> + u32 id;
>> + u8 pad[4];
>> +} __packed;
>> +
>> +struct ipc_shm_entry {
>> + struct ipc_seg seg;
>> + u64 size;
>> +} __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 de51715..877c16d 100644
>> --- a/ipc_ns.c
>> +++ b/ipc_ns.c
>> @@ -13,6 +13,36 @@
>> #include "namespaces.h"
>> #include "sysctl.h"
>>
>> +static void print_ipc_seg(const struct ipc_seg *seg)
>> +{
>> + pr_info("id: %-10d key: 0x%08x ", seg->id, seg->key);
>> + pr_info("uid: %-10d gid: %-10d ", seg->uid, seg->gid);
>> + pr_info("cuid: %-10d cgid: %-10d ", seg->cuid, seg->cgid);
>> + pr_info ("mode: %-10o ", seg->mode);
> An extra space
Yep. Thanks.
>> +}
>> +
>> +static void fill_ipc_seg(int id, struct ipc_seg *seg, const struct ipc_perm *ipcp)
>> +{
>> +#if defined (__GLIBC__)&& __GLIBC__>= 2
>> +#define KEY __key
>> +#else
>> +#define KEY key
>> +#endif
> I would add a comment here saying something like (if I get it right) we
> compile this file with and without glibc headers, and that struct
> ipc_perm is different for kernel and glibc; in particular, kernel
> version has item named 'key' while in glibc it is '__key', thus the define.
Kir, it's redundant. Really.
>> + seg->id = id;
>> + seg->key = ipcp->KEY;
>> + seg->uid = ipcp->uid;
>> + seg->gid = ipcp->gid;
>> + seg->cuid = ipcp->cuid;
>> + seg->cgid = ipcp->cgid;
>> + seg->mode = ipcp->mode;
>> +}
> Add
> #undef KEY
What for?
>> +
>> +static void print_ipc_shm(const struct ipc_shm_entry *shm)
>> +{
>> + print_ipc_seg(&shm->seg);
>> + pr_info ("size: %-10lu\n", shm->size);
> extra space
Yep. Thanks. (2)
>> +}
>> +
>> static int ipc_sysctl_req(struct ipc_var_entry *e, int op)
>> {
>> struct sysctl_req req[] = {
>> @@ -71,21 +101,77 @@ static int dump_ipc_sem(void *data)
>> return 0;
>> }
>>
>> -static int dump_ipc_shm(void *data)
>> +/*
>> + * 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)
>> {
>> - int fd;
>> + void *data;
>> int ret;
>> - struct shmid_ds shmid;
>>
>> - ret = shmctl(0, IPC_INFO,&shmid);
>> - if (ret< 0)
>> - pr_perror("semctl failed");
>> + data = shmat(shm->seg.id, NULL, SHM_RDONLY);
>> + if (data == (void *)-1) {
>> + pr_err("Failed to attach IPC shared memory\n");
> Use pr_perror() here.
Sounds reasonable. Thanks.
>> + return -EFAULT;
>> + }
>> + ret = write_img_buf(fd, data, round_up(shm->size, sizeof(u32)));
>> + if (ret< 0) {
>> + pr_err("Failed to write IPC shared memory data\n");
>> + return ret;
>> + }
>> + if (shmdt(data))
>> + pr_err("Failed to detach IPC shared memory\n");
> ditto
Sounds reasonable. Thanks. (2)
>> - if (ret) {
>> - pr_err("IPC shared memory migration is not supported yet\n");
>> - return -EINVAL;
>> + return 0;
>> +}
>> +
>> +static int dump_ipc_shm_seg(int fd, int id, const struct shmid_ds *ds)
>> +{
>> + struct ipc_shm_entry shm;
>> + int ret;
>> +
>> + fill_ipc_seg(id,&shm.seg,&ds->shm_perm);
>> + shm.size = ds->shm_segsz;
>> + print_ipc_shm(&shm);
>> +
>> + ret = write_img(fd,&shm);
>> + if (ret< 0) {
>> + pr_err("Failed to write IPC shared memory segment\n");
>> + return ret;
>> }
>> + return dump_ipc_shm_pages(fd,&shm);
>> +}
>>
>> +static int dump_ipc_shm(int fd)
>> +{
>> + int i, maxid, slot;
>> + struct shm_info info;
>> +
>> + maxid = shmctl(0, SHM_INFO, (void *)&info);
>> + if (maxid< 0) {
>> + pr_perror("shmctl(SHM_INFO) failed with %d\n", errno);
> no need for \n
OMG... This is confusing.
--
Best regards,
Stanislav Kinsbursky
More information about the CRIU
mailing list