[CRIU] [PATCH 2/3] dump: Add support to flock&posix file locks' dump

[Gu Zheng]

On 2012/11/9 1:56, Pavel Emelyanov wrote:
>> +static int dump_task_file_locks(struct parasite_ctl *ctl,
......
>> +       int maj, min;
>> +       unsigned long i_no;
>> +       long long start;
>> +       char end[32];
>> +
>> +       int num = 0, ret = 0;
>> +       bool is_block;
>> +
>> +       fp_locks = fopen("/proc/locks", "r");
>> +       if (fp_locks == NULL)
>> +               return -1;
>> +
>> +       while (fgets(buf, sizeof(buf), fp_locks)) {
> 
> The proc parsing stuff should be in proc_parse.c file.

Yes, this process should be in proc_parse.c file, I'll fix it, thanks! 
> 
>> +               file_lock_entry__init(&fe);
>> +               is_block = false;
>> +
>> +               if (strstr(buf, "->")) {
>> +                       is_block = true;
>> +                       pr_info("There is a blocked lock!\n");
>> +
>> +                       num = sscanf(buf,
......
>> +               if (ret) {
>> +                       pr_err("Dump file locks (pid: %d) failed with %d\n",
>> +                                       pid, ret);
>> +                       goto err_cure;
>> +               }
> 
> You parse the full /proc/locks file for every task. This is not optimal, please,
> collect locks from proc after all tasks are frozen and then get data from that
> image.

Yes, collect locks from proc after all tasks are frozen and then get data from that
image seems optimized , I'll follow your suggestion and modify the code, thanks!  

> 
> Besides, in the "else" branch you should check that task is holding a lock and
> abort dumping in that case.

Yeah, the "else" branch's logic is not precise, it has need to check the task is holding a lock and
abort dumping in that case. Thank you! :)


> 
>> +       }
>> +
>>         ret = parasite_dump_pages_seized(parasite_ctl, &vma_area_list, cr_fdset);
>>         if (ret) {
>>                 pr_err("Can't dump pages (pid: %d) with parasite\n", pid);
> 
>> @@ -61,6 +61,7 @@
>>  #define NETDEV_MAGIC           0x57373951 /* Yaroslavl */
>>  #define TTY_MAGIC              0x59433025 /* Pushkin */
>>  #define TTY_INFO_MAGIC         0x59453036 /* Kolpino */
>> +#define FILE_LOCKS_MAGIC       0x87654321 /* Town on the Mars */
> 
> Please, follow the rules of images' magic generation.

Yes, but can you provide one ? I do not know well about towns of Russia, thanks! 
 
> 
>>  #define IFADDR_MAGIC           RAW_IMAGE_MAGIC
>>  #define ROUTE_MAGIC            RAW_IMAGE_MAGIC
> 
> .
>