env_process: allow creating the dir while it exists

[hellohellenmao]

set exist_ok=True for os.makedirs function

ID: 3442

Summary by CodeRabbit

• Bug Fixes
  • Setup now tolerates pre-existing directories for mounted filesystem sources, preventing unnecessary errors during environment preparation.
  • Avoids failures when the target directory already exists, improving reliability of repeated runs.
  • Reduces manual cleanup and reruns by making directory creation idempotent.
  • Improves stability for repeated and concurrent workflows while preserving existing error handling in other scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adjusts directory creation in preprocess_fs_source for fs_type "mount" when fs_source_user_config == "no" to call os.makedirs(fs_source, exist_ok=True), making creation tolerant of an already-existing directory; surrounding removal/recreation logic is unchanged.

Changes

Cohort / File(s)	Summary of Changes
Filesystem source preprocessing virttest/env_process.py	Replaced os.makedirs(fs_source) with os.makedirs(fs_source, exist_ok=True) in the "mount" branch when fs_source_user_config == "no"; control flow and error handling remain unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "env_process: allow creating the dir while it exists" directly references the primary change in the pull request by naming the module and explaining that it enables directory creation when the directory already exists, which closely matches the implementation of setting exist_ok=True for os.makedirs without extra detail.
Description Check	✅ Passed	The description “set exist_ok=True for os.makedirs function” succinctly describes the key modification made in the changeset and is clearly related to the updated behavior in the env_process module.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d9b070f and 507c8fa.

📒 Files selected for processing (1)

• virttest/env_process.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• virttest/env_process.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Static checks

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[hellohellenmao]

@YongxueHong Could you please help to take a review here? Thanks

[YongxueHong]

Hi @hellohellenmao
Could you clarify more about this PR in the commit message, such as the motivation and approach?
Thanks.

Write a good commit message, pointing motivation, issues that you're
addressing. Usually you should try to explain 3 points in the commit message:
motivation, approach and effects

[hellohellenmao]

Hi @hellohellenmao Could you clarify more about this PR in the commit message, such as the motivation and approach? Thanks.

Write a good commit message, pointing motivation, issues that you're
addressing. Usually you should try to explain 3 points in the commit message:
motivation, approach and effects

Sure, updated. Review again, please.

[YongxueHong]

LGTM.

[YongxueHong]

Hi @xiagao @fbq815
Could you help review it from your platform? Thanks in advance!

[fbq815]

LGTM:
(1/3)unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.s390-virtio: PASS (377.62 s)
(2/3) virtio_fs_multi_vms.with_multi_fs_sources.s390-virtio: PASS (49.11 s)
(3/3) virtio_fs_multi_vms.share_fs_source.s390-virtio: PASS (49.16 s)

re-run:

[xiagao]

If there are existing files in the test folder, they may affect the test results. Could you please take this into account? @hellohellenmao

[hellohellenmao]

If there are existing files in the test folder, they may affect the test results. Could you please take this into account? @hellohellenmao

Hi @xiagao , with the changes here, creating the dir will success even though there are files in the test folder. And I believe that if the user are calling create_fs_source function that their intention should be creating a clean new dir. so overwriting the original dir/files should be fine. What your points here ?

[hellohellenmao]

If there are existing files in the test folder, they may affect the test results. Could you please take this into account? @hellohellenmao

Hi @xiagao , with the changes here, creating the dir will success even though there are files in the test folder. And I believe that if the user are calling create_fs_source function that their intention should be creating a clean new dir. so overwriting the original dir/files should be fine. What your points here ?

@xiagao Could you please help to check out here? Thanks

[xiagao]

If there are existing files in the test folder, they may affect the test results. Could you please take this into account? @hellohellenmao

Hi @xiagao , with the changes here, creating the dir will success even though there are files in the test folder. And I believe that if the user are calling create_fs_source function that their intention should be creating a clean new dir. so overwriting the original dir/files should be fine. What your points here ?

@xiagao Could you please help to check out here? Thanks

I mean if the directory already exists, its contents will remain unchanged. os.makedirs() never deletes or overwrites files.
In such situation, it would impact the test result due to the existed content in directory.

[hellohellenmao]

If there are existing files in the test folder, they may affect the test results. Could you please take this into account? @hellohellenmao

Hi @xiagao , with the changes here, creating the dir will success even though there are files in the test folder. And I believe that if the user are calling create_fs_source function that their intention should be creating a clean new dir. so overwriting the original dir/files should be fine. What your points here ?

@xiagao Could you please help to check out here? Thanks

I mean if the directory already exists, its contents will remain unchanged. os.makedirs() never deletes or overwrites files. In such situation, it would impact the test result due to the existed content in directory.

So my patch here is to resolve this issue, with exist_ok=True the dir will be created again even though the directory already exists

[xiagao]

If there are existing files in the test folder, they may affect the test results. Could you please take this into account? @hellohellenmao

Hi @xiagao , with the changes here, creating the dir will success even though there are files in the test folder. And I believe that if the user are calling create_fs_source function that their intention should be creating a clean new dir. so overwriting the original dir/files should be fine. What your points here ?

@xiagao Could you please help to check out here? Thanks

I mean if the directory already exists, its contents will remain unchanged. os.makedirs() never deletes or overwrites files. In such situation, it would impact the test result due to the existed content in directory.

So my patch here is to resolve this issue, with exist_ok=True the dir will be created again even though the directory already exists

Hi, tingting
If the dir exist, exist_ok=True will skip creating the dir. The existed dir might be not clean which might lead to test fail.
So if the dir existed, we should re-create it.

[hellohellenmao]

If there are existing files in the test folder, they may affect the test results. Could you please take this into account? @hellohellenmao

Hi @xiagao , with the changes here, creating the dir will success even though there are files in the test folder. And I believe that if the user are calling create_fs_source function that their intention should be creating a clean new dir. so overwriting the original dir/files should be fine. What your points here ?

@xiagao Could you please help to check out here? Thanks

I mean if the directory already exists, its contents will remain unchanged. os.makedirs() never deletes or overwrites files. In such situation, it would impact the test result due to the existed content in directory.

So my patch here is to resolve this issue, with exist_ok=True the dir will be created again even though the directory already exists

Hi, tingting If the dir exist, exist_ok=True will skip creating the dir. The existed dir might be not clean which might lead to test fail. So if the dir existed, we should re-create it.

Yeah, you're right. Thanks for pointing this out. And I remember that shutil.rmtree(fs_source) fails in this case is because that the fs_source is mounted. So we might solve this at first, I am going to umount before shutil.rmtree(fs_source). What your points on my new fix plan?

[xiagao]

Hi, tingting If the dir exist, exist_ok=True will skip creating the dir. The existed dir might be not clean which might lead to test fail. So if the dir existed, we should re-create it.

Yeah, you're right. Thanks for pointing this out. And I remember that shutil.rmtree(fs_source) fails in this case is because that the fs_source is mounted. So we might solve this at first, I am going to umount before shutil.rmtree(fs_source). What your points on my new fix plan?

It looks good to me. And I don't know why ignore_errors=True here, is there risk to remove it?
if create_fs_source:
if os.path.exists(fs_source):
shutil.rmtree(fs_source, ignore_errors=True)
LOG.info("Create filesystem source %s." % fs_source)