Only create 1MiB-aligned disk images #6754
Conversation
kubevirt-bot
added
release-note
|
/cc @rmohr |
|
I think we also have this place: kubevirt/pkg/host-disk/host-disk.go Line 143 in 9b1ac82 It is used by host-disks and PVCs (PVCs are mapped to hostdisks in virt-launcher). I think that this is also the main place which causes the issue. Can that be? |
Oh, good catch, will add |
|
/lgtm |
kubevirt-bot
added
lgtm
kubevirt-bot
removed
lgtm
|
The CI failures aren't all flakes - can you run |
Thank you, sorry I missed that comment |
|
/approve @maya-r leaving the lgtm to you once you are satisfied. |
kubevirt-bot
added
the
approved
|
/retest |
kubevirt-bot
added
the
needs-rebase
kubevirt-bot
removed
the
needs-rebase
|
/retest |
|
/retest |
jean-edouard
changed the title
jean-edouard
changed the title
|
Added a commit to align on 1MiB instead of 4KiB |
|
/lgtm |
|
@awels: once the present PR merges, I will cherry-pick it on top of release-v0.49 in a new PR and assign it to you. In response to this:
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/test-infra repository. |
Signed-off-by: Jed Lejosne <jed@redhat.com>
|
/retest |
| size = util2.AlignImageSizeTo1MiB(size, log.Log.With("image", imagePath)) | ||
| if size == 0 { | ||
| return fmt.Errorf("%s must be at least 1MiB", imagePath) | ||
| } | ||
| cmd := exec.Command("/usr/bin/qemu-img", "resize", preallocateFlag, imagePath, strconv.FormatInt(size, 10)) |
There was a problem hiding this comment.
I think we need to add a check here that does the following:
- Get the current virtual size of the disk.img
- Make sure that aligned size >= current virtual size
- Do the actual resize.
I think right now in some edge cases it is possible that when we align to 1Mi we will try to shrink an existing disk.img and that will fail.
There was a problem hiding this comment.
I think this is already taken care of by shouldExpandOffline(), which calls possibleGuestSize(), which accounts for alignment.
Am I missing something?
There was a problem hiding this comment.
No you are right, I was seeing ghosts.
|
/lgtm |
|
/hold cancel |
kubevirt-bot
removed
the
do-not-merge/hold
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mhenriks, rmohr The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
1 similar comment
|
/retest |
|
/retest |
|
@awels: cannot checkout In response to this:
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/test-infra repository. |
|
/cherrypick release-0.49 |
|
@aglitke: new pull request created: #7247 In response to this:
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/test-infra repository. |
What this PR does / why we need it:
Recent libvirt/qemu now refuse to start if any disk is not 1MiB-aligned.
This PR ensures disk image creation/resize will always produce 1MiB-aligned images by rounding the size down to the nearest 1MiB boundary.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #6712
Special notes for your reviewer:
2G (2000000000) happens to be 1k-aligned (!), so I used 1G in the test to ensure mis-alignment. (1k used to be fine, and might still be in some cases)
Release note: