Skip to content

utils_disk: check the mount point by findmnt cmd#4098

Merged
YongxueHong merged 1 commit intoavocado-framework:masterfrom
leidwang:issue3444
Jul 24, 2025
Merged

utils_disk: check the mount point by findmnt cmd#4098
YongxueHong merged 1 commit intoavocado-framework:masterfrom
leidwang:issue3444

Conversation

@leidwang
Copy link
Copy Markdown
Contributor

@leidwang leidwang commented Apr 2, 2025

Currently, if we run the test in image mode, the mount path check will not match because the directory is redirected,
so use findmnt to solve this problem.

ID: 3444

@leidwang leidwang marked this pull request as draft April 2, 2025 08:31
@leidwang leidwang marked this pull request as ready for review April 7, 2025 01:58
@leidwang
Copy link
Copy Markdown
Contributor Author

leidwang commented Apr 7, 2025

Hi @hellohellenmao Could you please check if this MR can fix the error that you encountered, thanks.

@hellohellenmao
Copy link
Copy Markdown
Contributor

Hi @hellohellenmao Could you please check if this MR can fix the error that you encountered, thanks.

I do not think so, for my case is invoking the umount https://github.com/avocado-framework/avocado-vt/blob/9f599feb7a75c756727e5f874f67ecb0e38bcc00/virttest/utils_disk.py#L158C36-L158C39 here to do the umounting, so could you please update the code here either?
Thanks

@leidwang
Copy link
Copy Markdown
Contributor Author

leidwang commented Apr 7, 2025

Hi @hellohellenmao Could you please check if this MR can fix the error that you encountered, thanks.

I do not think so, for my case is invoking the umount https://github.com/avocado-framework/avocado-vt/blob/9f599feb7a75c756727e5f874f67ecb0e38bcc00/virttest/utils_disk.py#L158C36-L158C39 here to do the umounting, so could you please update the code here either? Thanks

Updated the code, could you please add your logs to the issue, thanks.

@hellohellenmao
Copy link
Copy Markdown
Contributor

Still hit the issue with the patch , does it work from your side @leidwang

(1/1) Host_RHEL.m9.u6.ovmf.qcow2.virtio_scsi.up.virtio_net.bootc.Guest.RHEL.9.6.0.x86_64.io-github-autotest-qemu.virtio_fs_sandbox.none.q35: ERROR: Failed to remove directory rm -rf /mnt/myfs from remote machine: Shell command failed: 'rm -rf /mnt/myfs' (status: 1, output: "rm: cannot remove '/mnt/myfs': Device or resource busy\n")

@leidwang leidwang force-pushed the issue3444 branch 2 times, most recently from 5d6c80f to 6919eca Compare April 22, 2025 08:55
@leidwang
Copy link
Copy Markdown
Contributor Author

Hi @PaulYuuu Could you please review this MR? Thanks.

@leidwang leidwang changed the title utils_disk: get the realpath when umount utils_disk: check the moint point by findmnt cmd Apr 24, 2025
@leidwang
Copy link
Copy Markdown
Contributor Author

Hi @PaulYuuu @hellohellenmao Would you please help review this MR, thanks.

Copy link
Copy Markdown
Contributor

@YvanY0 YvanY0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost LGTM, but you should rebase the branch to fix the CI issue.

Comment thread virttest/utils_disk.py
@leidwang
Copy link
Copy Markdown
Contributor Author

Hi @PaulYuuu Could you please help involve others to review this PR?Thanks.

@YvanY0
Copy link
Copy Markdown
Contributor

YvanY0 commented May 27, 2025

Hi @PaulYuuu Could you please help involve others to review this PR?Thanks.

As this is related to block feature, I think @qingwangrh @YongxueHong should be the right persons to review. And, not only us use this, we should also include libvirt people to review if needed.

@YvanY0 YvanY0 requested a review from YongxueHong May 27, 2025 02:40
@qingwangrh
Copy link
Copy Markdown
Contributor

hi, @leidwang , if we want fewer changes, how about changing "cat /proc/mount" to "mount"
or " mount|grep %s" % mount_str to get precise result. that is one line changed. Could you please help to try it?

@leidwang
Copy link
Copy Markdown
Contributor Author

hi, @leidwang , if we want fewer changes, how about changing "cat /proc/mount" to "mount" or " mount|grep %s" % mount_str to get precise result. that is one line changed. Could you please help to try it?

Hi @qingwangrh findmnt provides a more structured output that can display mount information in a tree format, table format, or JSON format(-J). Its output is easier to read and parse. I would more prefer to use findmnt to check mount info and update currently code accordingly, feel free to add any comments about that? @PaulYuuu @YongxueHong

@YvanY0
Copy link
Copy Markdown
Contributor

YvanY0 commented May 27, 2025

hi, @leidwang , if we want fewer changes, how about changing "cat /proc/mount" to "mount" or " mount|grep %s" % mount_str to get precise result. that is one line changed. Could you please help to try it?

Hi @qingwangrh findmnt provides a more structured output that can display mount information in a tree format, table format, or JSON format(-J). Its output is easier to read and parse. I would more prefer to use findmnt to check mount info and update currently code accordingly, feel free to add any comments about that? @PaulYuuu @YongxueHong

Agree. And mount | grep can only check the target path, and cannot grep the soft link.

@YongxueHong
Copy link
Copy Markdown

Hi @leidwang
Where does it redirect to? Could you give more? Thanks.

CC @nickzhq
Please help check if the vt_utils.filesystem module needs to be updated.
Thanks.

@qingwangrh
Copy link
Copy Markdown
Contributor

qingwangrh commented May 27, 2025

It makes sense to use findmnt if we want to support softlink.

mount | grep $(readlink -f xxx) also may support softlink

The findmnt may get better readability output,
Both methods LGTM

@leidwang
Copy link
Copy Markdown
Contributor Author

Hi @leidwang Where does it redirect to? Could you give more? Thanks.

Sure, for example, if we mount nfs to /home/mount_test, in bootc host, it will shown as /var/home/mount_test instead of /home/mount_test.

# mkdir mount_test
# mount ***:/kvmqeauto/iso /home/mount_test/
# cat /proc/mounts
......
***:/kvmqeauto/iso /var/home/mount_test nfs ro,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,mountaddr=***,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=*** 0 0

CC @nickzhq Please help check if the vt_utils.filesystem module needs to be updated. Thanks.

@YongxueHong
Copy link
Copy Markdown

Hi @leidwang Where does it redirect to? Could you give more? Thanks.

Sure, for example, if we mount nfs to /home/mount_test, in bootc host, it will shown as /var/home/mount_test instead of /home/mount_test.

# mkdir mount_test
# mount ***:/kvmqeauto/iso /home/mount_test/
# cat /proc/mounts
......
***:/kvmqeauto/iso /var/home/mount_test nfs ro,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,mountaddr=***,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=*** 0 0

Hi @leidwang
Thanks for illustrating. I am just curious why the mount point will be redirected to another?
Did it just happen with the image mode?
Would you know more?
Thanks.

CC @nickzhq Please help check if the vt_utils.filesystem module needs to be updated. Thanks.

@leidwang
Copy link
Copy Markdown
Contributor Author

Hi @leidwang Where does it redirect to? Could you give more? Thanks.

Sure, for example, if we mount nfs to /home/mount_test, in bootc host, it will shown as /var/home/mount_test instead of /home/mount_test.

# mkdir mount_test
# mount ***:/kvmqeauto/iso /home/mount_test/
# cat /proc/mounts
......
***:/kvmqeauto/iso /var/home/mount_test nfs ro,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,mountaddr=***,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=*** 0 0

Hi @leidwang Thanks for illustrating. I am just curious why the mount point will be redirected to another? Did it just happen with the image mode? Would you know more? Thanks.

Yes, it only happens in image mode OS, it should be related to image mode os file system is read-only, only /var and /etc are writable.

CC @nickzhq Please help check if the vt_utils.filesystem module needs to be updated. Thanks.

@YvanY0
Copy link
Copy Markdown
Contributor

YvanY0 commented May 28, 2025

I don't think it's only related to image mode, no one can promise users will not create a softlink.

For instance, avocado will point the iso path to /root/avocado/data/avocado-vt/isos/, our internal usage will link it /home/kvm_autotest_root/iso/

@leidwang leidwang force-pushed the issue3444 branch 5 times, most recently from c5df768 to 4e536ed Compare June 25, 2025 01:43
Comment thread virttest/utils_disk.py
Comment thread virttest/utils_disk.py
Comment thread virttest/utils_disk.py Outdated
@leidwang leidwang force-pushed the issue3444 branch 3 times, most recently from c44f0d1 to b08a936 Compare July 1, 2025 07:08
@leidwang
Copy link
Copy Markdown
Contributor Author

leidwang commented Jul 1, 2025

Hi @PaulYuuu @YongxueHong I have updated the code according to our offline discussion, would you please check it again?Thanks.

@YongxueHong
Copy link
Copy Markdown

Hi @leidwang
The code is good for me.
Could you help run some test cases in non-image mode to verify if it will introduce a regression issue?
Thanks in advance.

@hellohellenmao @qingwangrh
Could you help review it? Since there are some updates with it.
Thanks.

Copy link
Copy Markdown

@YongxueHong YongxueHong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like mount_result = None is useless.

Comment thread virttest/utils_disk.py
@qingwangrh
Copy link
Copy Markdown
Contributor

change the commit message "check the moint point by findmnt cmd" -> check the mount point by findmnt cmd
Other LGTM

@leidwang
Copy link
Copy Markdown
Contributor Author

leidwang commented Jul 3, 2025

Hi @leidwang The code is good for me. Could you help run some test cases in non-image mode to verify if it will introduce a regression issue? Thanks in advance.

@hellohellenmao @qingwangrh Could you help review it? Since there are some updates with it. Thanks.

Run the installation test case as it will call this function.

JOB LOG : /root/avocado/job-results/job-2025-07-03T10.37-7bff2d1/job.log
(1/2) Host_RHEL.m10.u.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: STARTED
(1/2) Host_RHEL.m10.u.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win2022.x86_64.io-github-autotest-qemu.unattended_install.cdrom.extra_cdrom_ks.default_install.aio_threads.q35: PASS (631.99 s)

@leidwang leidwang marked this pull request as draft July 4, 2025 02:19
Currently, if we run the test in image mode, the mount path
check will not match because the directory is redirected,
so use findmnt to solve this problem.

Signed-off-by: Leidong Wang <leidwang@redhat.com>
@YvanY0 YvanY0 changed the title utils_disk: check the moint point by findmnt cmd utils_disk: check the mount point by findmnt cmd Jul 7, 2025
@leidwang leidwang marked this pull request as ready for review July 10, 2025 05:53
@leidwang
Copy link
Copy Markdown
Contributor Author

Tested this PR on image mode and package mode, all works well.

Copy link
Copy Markdown

@YongxueHong YongxueHong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@leidwang
Copy link
Copy Markdown
Contributor Author

Hi @YongxueHong @PaulYuuu If there is no more comments for this MR, would you please help merge it?Thanks.

@YongxueHong YongxueHong merged commit 9ea9b5c into avocado-framework:master Jul 24, 2025
50 checks passed
liang-cong-red-hat added a commit to liang-cong-red-hat/avocado-vt that referenced this pull request Jul 28, 2025
Since avocado-framework#4098, using findmnt to find mount points.
It is file type sensitive, so add nfs4 to support nfs4 type.

Signed-off-by: liang-cong-red-hat <lcong@redhat.com>
hholoubk pushed a commit to hholoubk/avocado-vt that referenced this pull request Oct 24, 2025
Since avocado-framework#4098, using findmnt to find mount points.
It is file type sensitive, so add nfs4 to support nfs4 type.

Signed-off-by: liang-cong-red-hat <lcong@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants