[CRIU] [PATCH 27/32] tools: cpt2 -- Add "write" routines for virtual memory areas

[Pavel Emelyanov]

On 04/01/2013 02:51 PM, Cyrill Gorcunov wrote:
> On Mon, Apr 01, 2013 at 02:25:40PM +0400, Pavel Emelyanov wrote:
>>> +	list_for_each_entry(vma, &mm->vma_list, list) {
>>> +
>>> +		if (!vma_is(vma, VMA_AREA_REGULAR))
>>> +			continue;
>>> +
>>> +		if (vma_is(vma, VMA_AREA_SYSVIPC))
>>> +			continue;
>>
>> Why?
> 
> sysvipc was not addressed neither it was yet tested
> (while crtools is ready to restore this zone i desided
>  to not include it until anything else tested).

Then fail convert.

>>
>>> +
>>
>>> +		if (vma_is_shared(vma))
>>
>> This define is in patch #19 and ...
>>
>>> +			e.flags = MAP_SHARED;
>>> +		else
>>> +			e.flags = MAP_PRIVATE;
>>> +
>>> +		if (vma_is(vma, VMA_ANON_SHARED | VMA_ANON_PRIVATE))
>>
>> ... looks exactly like this one. Such tricks do not improve code readability.
> 
> Thanks, will update!
> 
>>
>>> +			e.flags |= MAP_ANONYMOUS;
>>> +
>>
>>> +
>>> +#define write_socket_map(ctx, vma, pid)		0
>>
>> WTF???
> 
> not covered yet, but to not fail I zapped it.

You must fail in this case, not ignore.

>>> +	for (; start < end; start += u.h.cpt_next) {
>>> +		if (read_obj_cpt(ctx->fd, OBJ_ANY, &u.h, start)) {
>>> +			pr_err("Can't read page header at %li\n", (long)start);
>>> +			goto err;
>>> +		}
>>> +
>>> +		/*
>>> +		 * Skip any non regular pages.
>>> +		 */
>>> +		switch (u.h.cpt_object) {
>>> +		case CPT_OBJ_PAGES:
>>> +			break;
>>> +		case CPT_OBJ_REMAPPAGES:
>>> +		case CPT_OBJ_COPYPAGES:
>>> +		case CPT_OBJ_LAZYPAGES:
>>> +		case CPT_OBJ_ITERPAGES:
>>> +		case CPT_OBJ_ITERYOUNGPAGES:
>>> +			continue;
>>
>> Bail out with error.
> 
> ok
> 
>>> +		for (i = 0; i < nr_pages; i++) {
>>> +			if (__read(ctx->fd, page, sizeof(page))) {
>>> +				pr_err("Can't read page at %li\n",
>>> +				       (long)start);
>>> +					goto err;
>>> +				}
>>> +			if (__write(page_fd, page, sizeof(page))) {
>>> +				pr_err("Can't write page at %li\n",
>>> +				       (long)start);
>>> +				goto err;
>>> +			}
>>
>> Plz, use sendfile for this.
> 
> I don't think it worth, we will be teaching crtools to work with
> cpt pages directly so this is for simplicity.

OK

> .
>