checkpoint-restore / criu

Checkpoint/Restore tool
criu.org
Other
2.76k stars 560 forks source link

criu: fix tempdir recycle : Fixes: #2351 #2355

Closed YarBor closed 3 months ago

YarBor commented 4 months ago

As #2351 say When many duḿps failed in #2348, it created a bunch of empty directories

drwx------ 2 root   root   4096 Feb 12 23:48 .criu.temp-aa-policy.8uhTbO
drwx------ 2 root   root   4096 Feb 12 23:51 .criu.temp-aa-policy.90Vqt9
drwx------ 2 root   root   4096 Feb 12 23:29 .criu.temp-aa-policy.9an3IS
drwx------ 2 root   root   4096 Feb 12 23:51 .criu.temp-aa-policy.CBz1ud
drwx------ 2 root   root   4096 Feb 12 23:39 .criu.temp-aa-policy.eA0Kj6
drwx------ 2 root   root   4096 Feb 12 23:34 .criu.temp-aa-policy.GD04gM
drwx------ 2 root   root   4096 Feb 12 23:38 .criu.temp-aa-policy.ItnOKf
drwx------ 2 root   root   4096 Feb 12 23:57 .criu.temp-aa-policy.NuRSjL
drwx------ 2 root   root   4096 Feb 12 23:57 .criu.temp-aa-policy.nUX8Kq
drwx------ 2 root   root   4096 Feb 12 23:36 .criu.temp-aa-policy.PGi24L
drwx------ 2 root   root   4096 Feb 12 23:35 .criu.temp-aa-policy.QNuwGp
drwx------ 2 root   root   4096 Feb 12 23:28 .criu.temp-aa-policy.RyvBPt
drwx------ 2 root   root   4096 Feb 12 23:41 .criu.temp-aa-policy.TXQpQv
drwx------ 2 root   root   4096 Feb 12 23:41 .criu.temp-aa-policy.URYvi0
drwx------ 2 root   root   4096 Feb 12 23:34 .criu.temp-aa-policy.VJixh8
drwx------ 2 root   root   4096 Feb 12 23:48 .criu.temp-aa-policy.whT8gi

delete the directories after aborting

Signed-off-by: YarBor yarbor.ww@gmail.com

Fixes: #2351

adrianreber commented 4 months ago

Thanks for the PR. I just read the man page from mkdtemp() a couple of times and I am not sure this change helps:

DESCRIPTION
       The  mkdtemp() function generates a uniquely named temporary directory from template. 
       The last six characters of template must be XXXXXX and these are replaced with a string that
       makes the directory name unique.  The directory is then created with permissions 0700.
       Since it will be modified, template must not be a string constant, but should be declared as
       a character array.

RETURN VALUE
       The mkdtemp() function returns a pointer to the modified template string on success,
       and NULL on failure, in which case errno is set to indicate the error.

That sounds, to me, like your change does not make a difference. Because after your change tempdirname and policydir are pointing to the changed value of the template policydir.

Independent of your change this code only works once. The second time it runs policydir no longer contains XXXXXX and mkdtemp() should return EINVAL.

YarBor commented 4 months ago

Thanks for the PR. I just read the man page from mkdtemp() a couple of times and I am not sure this change helps:感谢您的公关。我刚刚阅读了几次 mkdtemp() 的手册页,我不确定此更改是否有帮助:

DESCRIPTION
       The  mkdtemp() function generates a uniquely named temporary directory from template. 
       The last six characters of template must be XXXXXX and these are replaced with a string that
       makes the directory name unique.  The directory is then created with permissions 0700.
       Since it will be modified, template must not be a string constant, but should be declared as
       a character array.

RETURN VALUE
       The mkdtemp() function returns a pointer to the modified template string on success,
       and NULL on failure, in which case errno is set to indicate the error.

That sounds, to me, like your change does not make a difference. Because after your change tempdirname and policydir are pointing to the changed value of the template policydir.在我看来,这听起来就像你的改变没有什么区别。因为更改后 tempdirnamepolicydir 指向模板 policydir 的更改值。

Independent of your change this code only works once. The second time it runs policydir no longer contains XXXXXX and mkdtemp() should return EINVAL.与您的更改无关,此代码仅有效一次。第二次运行时 policydir 不再包含 XXXXXX 并且 mkdtemp() 应返回 EINVAL。

Sorry, you're right. I oversimplified it. Let me take another look.

Snorch commented 4 months ago

@YarBor small nit: You can see in git log that we never put Fixes: #XXXX to the subject line of the commit, normally we put it on the separate line just before signoff.

github-actions[bot] commented 3 months ago

A friendly reminder that this PR had no activity for 30 days.