Message ID | 20221104044149.655317-1-alessandro.carminati@gmail.com |
---|---|
Headers | show |
Series | Fix tst_find_backing_dev adding BTRFS support and /dev/root missing case | expand |
Hello, Alessandro Carminati <alessandro.carminati@gmail.com> writes: > Hello Richard, > > Thanks for the detailed review. > If in the future I want to contribute more to the LTP project, I need > to provide myself with a CI pipeline like yours. If you fork the project in Github and create a new branch then the CI will run on the commits you push to GH. > I appreciated the review that was very detailed, but I couldn't address > a single comment. > >>> + if (!ioctl(fd, BTRFS_IOC_FS_INFO, &args)) { >>What happens if the test author allows this function to be called on > tmpfs, rootfs, etc.? Or if the FS is valid, but has the same issue as > BTRFS. > > I have gone thorough all the file systems supported by LTP at this stage, > and I noticed that BTRFS is the only file system that owns this > singularity. tmpfs doesn't have a backing device and we support it? E.g. 40 60 0:38 / /tmp rw,nosuid,nodev shared:19 - tmpfs tmpfs rw,nr_inodes=1048576,inode64 So this function shouldn't be called on it and it is not in the test currently effected[1]. However if the test author does it by accident (99% chance of happening) then we want a sensible error message. > In addition to this, I also dared to assume that if device major number > is == 0 then the test is facing the BTRFS. > This assumption might not be true in general, but I tested it to be > true in the test supported file system. > Is your comment referring to this? I think it is absolutely correct that we shouldn't design for requirements we presently don't have. However: 1. It's highly probable this function will be misused. 2. It's relatively easy to guard against. > > > Alessandro Carminati (2): > tst_find_backing_dev: Get dev name from /sys/dev/block/*/uevent > c-test-api: Documentation updated > > doc/c-test-api.txt | 7 ++-- > lib/tst_device.c | 87 ++++++++++++++++++++++++++++++---------------- > 2 files changed, 63 insertions(+), 31 deletions(-) [1]: The test skips it .skip_filesystems = (const char *const []) { "tmpfs", "overlayfs", NULL }, Also note that you can run LTP on any filesystem. You just need to set the appropriate env vars.