Avoid running blockdev command when DV is Filesystem volumeMode #2174
Conversation
kubevirt-bot
added
release-note
|
Hi @nijinashok. Thanks for your PR. I'm waiting for a kubevirt member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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/test-infra repository. |
nijinashok
changed the title
|
/ok-to-test |
kubevirt-bot
added
ok-to-test
kubevirt-bot
added
dco-signoff: no
kubevirt-bot
added
dco-signoff: yes
|
/test pull-containerized-data-importer-e2e-k8s-1.22-hpp |
|
/test pull-containerized-data-importer-e2e-k8s-1.22-nfs |
pkg/util/util.go
Outdated
| // Check if device exists. | ||
| info, err := os.Stat(deviceName) | ||
| // Check if the file exists and is a device file. | ||
| info, err := os.Lstat(deviceName) |
There was a problem hiding this comment.
So why change to lstat here? According to the documentation lstat doesn't follow symlinks while stat does. If the file we are looking at is a symlink I think we want to follow it.
There was a problem hiding this comment.
yes, lstat change is not needed. Reverted.
As of now, it is passing "/data/disk.img" to GetAvailableSpaceBlock in Filesystem volumeMode and hence it always run blockdev command. With this change, the blockdev command will be only executed if it's a device file. Signed-off-by: Nijin Ashok <nashok@redhat.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: awels 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 |
kubevirt-bot
added
the
approved
|
/test pull-containerized-data-importer-e2e-k8s-1.22-hpp-istio |
|
/test pull-containerized-data-importer-e2e-k8s-1.22-hpp |
|
/retest |
1 similar comment
|
/retest |
|
/override pull-containerized-data-importer-e2e-k8s-1.21-hpp |
|
@awels: Overrode contexts on behalf of awels: pull-containerized-data-importer-e2e-k8s-1.21-hpp, pull-containerized-data-importer-e2e-k8s-1.22-upg 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. |
Needed after kubevirt#2174 Signed-off-by: Maya Rashish <mrashish@redhat.com>
Needed after kubevirt#2174 Signed-off-by: Maya Rashish <mrashish@redhat.com>
Needed after kubevirt#2174 Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Retire ember LVM code, unused (Rationale: avoid having to change more things for changing the base image) Signed-off-by: Maya Rashish <mrashish@redhat.com> * Remove unreferenced files from WORKSPACE Signed-off-by: Maya Rashish <mrashish@redhat.com> * Switch to centos:stream9 as a base image. It has a significantly longer support cycle than Fedora releases, and supposedly offers vulnerability scans. Add a tinyCore.vdi to the repo instead of generating it. The centos qemu-img has read-only VDI support, so we can't generate it. Generate it using my system and add to the file-host. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Use full names for pulls from dockerhub CentOS doesn't like short tags Signed-off-by: Maya Rashish <mrashish@redhat.com> * Avoid specifying checksum for CentOS images. They expire faster than we can update checksums, this is unfortunate but perhaps they will soon publish images at a lower rate allowing us to keep up. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Bump number of open file limit to avoid bazel crashes Signed-off-by: Maya Rashish <mrashish@redhat.com> * Update builder to include #2087, builder based on centos stream9 Signed-off-by: Maya Rashish <mrashish@redhat.com> * Update checksums that seem wrong Signed-off-by: Maya Rashish <mrashish@redhat.com> * Update ovirt links: old ones were removed Signed-off-by: Maya Rashish <mrashish@redhat.com> * Remove unused RPMs Noticed due to: duplicated checksum but no problem in testsuite, lack of aarch64 equivalent. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Put nbdkit-vddk-plugin back for amd64. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Use quay.io instead of dockerhub. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Install util-linux-core for /usr/sbin/blockdev Needed after #2174 Signed-off-by: Maya Rashish <mrashish@redhat.com> * Update nbdkit/libnbd/nginx/ovirt versions to the latest The previous version we were using can't be fetched any more Signed-off-by: Maya Rashish <mrashish@redhat.com> * Generate our own CentOS stream9 image using RPMs Now updating the dependencies can be done by running `make rpm-deps` and committing the change, like kubevirt. This creates a small complication that we need to run update-ca-trust to trust root CAs. Do this on the pod, using the entrypoint to do so. Use a single image with all the dependencies for the test tools, we don't benefit from making them minimal and it saved some trouble in the conversion. Signed-off-by: Maya Rashish <mrashish@redhat.com> * Fixup imageio test container Run update-ca-trust and update-crypto-policies before running ovirt-imageio, to stop error messages. Signed-off-by: Maya Rashish <mrashish@redhat.com>
What this PR does / why we need it:
As of now, it is passing "/data/disk.img" to GetAvailableSpaceBlock in Filesystem volumeMode, and hence it always runs blockdev command. With this change, the blockdev command will be only executed if it's a device file.
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 #
Special notes for your reviewer:
Release note: