Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

YarBor
Copy link

@YarBor YarBor commented Feb 26, 2024

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

When many duḿps failed in checkpoint-restore#2348, it created a bunch of empty directories

```txt
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>
@adrianreber
Copy link
Member

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
Copy link
Author

YarBor commented Feb 26, 2024

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
Copy link
Member

Snorch commented Feb 27, 2024

@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.

Copy link

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

@YarBor YarBor closed this Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

temporary directory littering
3 participants