containerized-data-importer
containerized-data-importer copied to clipboard
kubevirt
Bugfix/qemu img convert lose sparseness
What this PR does / why we need it:
This PR updates the qemu-img convert command by conditionally adding the --target-is-zero and -n flags when preallocation is not requested. Since we assume that the target block device is already zeroed, the flag allows qemu-img to skip writing zero blocks—reducing unnecessary I/O and speeding up image conversion. The -n flag (which is used to skip target image creation) is required together with --target-is-zero, so it is automatically added when preallocation is disabled.
Which issue(s) this PR fixes: Fixes #3614
Special notes for your reviewer:
Release note:
NONE
Hi @noamasu. Thanks for your PR.
PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign mhenriks for approval. For more information see the Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@noamasu following offline discussions, should we hold this for now? I guess we're not in the wrong to not use this flag but rather device mapper devices return ENOSUPP on the appropriate ioctl, where other storage providers don't?
Hey @akalenyu, I guess we should hold this for now, at least until we know whats going on with the ENOSUPP when using thin lv, not sure if this is the only one with ENOSUPP.
but worth mentioning that using --target-is zero will always reduce I/O and amount of sectors written to the block device. in fact, it will write the minimum amount of I/O and sectors possible which is always what we want. on an ideal scenario, if we knew 100% that the target device is zeroed out, it is worth adding this flag.
but yes, looks like most devices actually do preserve sparseness even when not using --target-is-zero (except that thin lv ENOSUPP), in that case not adding the flag will just mean more I/O and sectors written but at least the target block device conversion result is reliable (results in the same sha256sum disk signature even if the disk is not zeroed).
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
/lifecycle stale
Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
/lifecycle rotten
Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
/close
@kubevirt-bot: Closed this PR.
In response to this:
Rotten issues close after 30d of inactivity. Reopen the issue with
/reopen. Mark the issue as fresh with/remove-lifecycle rotten./close
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.