-
Notifications
You must be signed in to change notification settings - Fork 401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(url-lib.sh): nfs_already_mounted() with trailing slash in nfs path #2182
fix(url-lib.sh): nfs_already_mounted() with trailing slash in nfs path #2182
Conversation
e38faa5
to
c87feac
Compare
@mmatsuya Can you please check the commit message on your latest commit. Perhaps you saw it, but it is easy to miss..
|
f624999
to
4a6bee5
Compare
@mmatsuya you can run the test locally with e.g. docker fairly easily - https://github.com/dracutdevs/dracut/wiki/Dracut-development#running-tests-with-docker Also, the goal is to make sure that the code you changing gets executed by the test, it does not need to replicate your exact scenario. |
fb67160
to
d91d84d
Compare
Oh, sorry. I missed your comment. I will stop test it here. |
Not a problem, just was not sure if this workflow is efficient for you. Use whatever tool makes most sense for you. |
Was it enough to run nfs_already_mounted() and check the result without replicating the problem? |
d91d84d
to
128b3ec
Compare
nfs_already_mounted() doesn't work when the installation ISO and kickstart file on a same NFS share are specified with inst.repo and inst.ks boot parameter as below. inst.repo=nfs:192.168.1.1:/home/data/rhel9.iso inst.ks=nfs:192.168.1.1:/home/data/ks.cfg NOTE: /home/data is configured for nfs share on 192.168.1.1 One problem is a file (not a directory) was passed into nfs_already_mounted(). nfs_already_mounted() is the function to judge if the given directory is already mounted. So, filepath should be passed in nfs_fetch_url(). The other problem is about the trailing slash in the nfs path in /proc/mounts. The /proc/mounts has an entry after nfs mount of inst.repo. 192.168.1.1:/data/ /run/install/isodir nfs ro,relatime,<snip> In this case, nfs_already_mounted() returns "/run/install/isodir//home/data/ks.cfg" wrongly. The following is from the log. [ 14.556279] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@156(nfs_fetch_url): nfs_already_mounted 192.168.122.1 /home/data/ks.cfg [ 14.556279] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@137(nfs_already_mounted): local server=192.168.122.1 path=/home/data/ks.cfg s= p= ... [ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@140(nfs_already_mounted): '[' 192.168.122.1 = 192.168.122.1 ']' [ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@141(nfs_already_mounted): '[' /home/data/ks.cfg = /home/data/ ']' [ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@143(nfs_already_mounted): str_starts /home/data/ks.cfg /home/data/ [ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/dracut-lib.sh@51(str_starts): '[' ks.cfg '!=' /home/data/ks.cfg ']' [ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@144(nfs_already_mounted): echo /run/install/isodir//home/data/ks.cfg ... [ 14.658069] localhost.localdomain dracut-initqueue[934]: //lib/url-lib.sh@156(nfs_fetch_url): mntdir=/run/install/isodir//home/data/ks.cfg This function doesn't expect the trailiing slash of the nfs path in /proc/mounts, so it should be removed before processing it.
This is to check the behavior of nfs_fetch_url() in nfs-lib.sh. nfs_fetch_url() calls nfs_already_mounted() internally. A file /nfs/client/root/fetchfile is on NFS server, which is fetched from clients for testing with nfs_fetch_url().
6d697cd
to
3cf092d
Compare
As a result, I added a test to check the behavior of nfs_fetch_url() into TEST-20-NFS. nfs_already_mounted() is called in it. I didn't make any additional nfs mounts, but I just used the existing mount for the nfs root. /nfs/client/root/fetchfile was located on the nfs server. And if a nfs client can fetch the file with nfs_already_mounted(), the result is written into marker2.img. And, the result in the marker2.img is checked in client_test(). Hopefully, this meets what you expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks !
nfs_already_mounted() doesn't work when the installation ISO and kickstart file on a same NFS share are specified with inst.repo and inst.ks boot parameter as below.
inst.repo=nfs:192.168.1.1:/home/data/rhel9.iso inst.ks=nfs:192.168.1.1:/home/data/ks.cfg
NOTE: /home/data is configured for nfs share on 192.168.1.1
One problem is a file (not a directory) was passed into nfs_already_mounted(). nfs_already_mounted() is the function to judge if the given directory is already mounted. So, filepath should be passed in nfs_fetch_url().
The other problem is about the trailing slash in the nfs path in /proc/mounts.
The /proc/mounts has an entry after nfs mount of inst.repo.
192.168.1.1:/data/ /run/install/isodir nfs ro,relatime,
In this case, nfs_already_mounted() returns "/run/install/isodir//home/data/ks.cfg" wrongly. The following is from the log.
[ 14.556279] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@156(nfs_fetch_url): nfs_already_mounted 192.168.122.1 /home/data/ks.cfg
[ 14.556279] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@137(nfs_already_mounted): local server=192.168.122.1 path=/home/data/ks.cfg s= p=
...
[ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@140(nfs_already_mounted): '[' 192.168.122.1 = 192.168.122.1 ']'
[ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@141(nfs_already_mounted): '[' /home/data/ks.cfg = /home/data/ ']'
[ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@143(nfs_already_mounted): str_starts /home/data/ks.cfg /home/data/
[ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/dracut-lib.sh@51(str_starts): '[' ks.cfg '!=' /home/data/ks.cfg ']'
[ 14.654966] localhost.localdomain dracut-initqueue[1282]: ///lib/url-lib.sh@144(nfs_already_mounted): echo /run/install/isodir//home/data/ks.cfg
...
[ 14.658069] localhost.localdomain dracut-initqueue[934]: //lib/url-lib.sh@156(nfs_fetch_url): mntdir=/run/install/isodir//home/data/ks.cfg
This function doesn't expect the trailiing slash of the nfs path in /proc/mounts, so it should be removed before processing it.
This pull request changes...
Changes
Checklist
Fixes #