On Thu, May 9, 2019 at 8:29 AM Jeffle Xu <jefflexu@xxxxxxxxxxxxxxxxx> wrote: > > We got "shutdown: Inappropriate ioctl for device" when running > "./check -overlay generic/530". The error message is due to the > XFS_IOC_GOINGDOWN ioctl used in generic/530. > > Though _require_scratch_shutdown() is called to test whether the > tested filesystem supports XFS_IOC_GOINGDOWN ioctl or not, a piece > of C code snippet in src/t_open_tmpfiles.c is used to shutdown the > filesysetm, rather than calling the corresponding helper function > _scratch_shutdown(). > > Let me explain it clearly: > > 1. suppose the config is > > ``` > export TEST_DEV=/dev/vdb > export TEST_DIR=/mnt/test > export SCRATCH_DEV=/dev/vdc > export SCRATCH_MNT=/mnt/scratch > ``` > > 2. When running "./check -overlay generic/530", the scratch device, > /dev/vdc is mounted on /mnt/scratch, and the overlay filesystem is > mounted on /mnt/scratch/mnt using the follwing command > > ``` > mount -t overlay -o lowerdir=/mnt/scratch/ovl-lower \ > -o upperdir=/mnt/scratch/ovl-upper/ -o workdir=/mnt/scratch/ovl-work/ \ > overlay /mnt/scratch/ovl-mnt > ``` > > 3. In this case, _require_scratch_shutdown() will inspect whether the > underlying upper system, that is **/mnt/scratch/**, supports shutdown > feature or not. In my test, the underlying system of overlay (that is > /dev/vdc) is ext4, and thus it passes the _require_scratch_shutdown > test. > > 4. However, the C code executing the shutdown action in t_open_tmpfiles.c > actually execute XFS_IOC_GOINGDOWN ioctl on the mount point of overlay > filesystem, that is **/mnt/scratch/ovl-mnt**. Since overlay doesn't support > XFS_IOC_GOINGDOWN ioctl, it fails naturally. > > 5. So we should use _scratch_shutdown() to shutdown if we have used > _require_scratch_shutdown > > As for the solution to fix the failure, I temporarily move the shutdown > action from t_open_tmpfiles into generic/530 as the workaround. How would > you think @Darrick? Maybe there is a better solution but I'm not sure. > I think this is the correct solution (and not a workaround) FWIW there are other operations (e.g. umount) when done directly and not via common helpers would not have the same result when running check -overlay. I suggest that you remove the option 'shutdown' from t_open_tmpfiles if shutdown can be done by test, so future tests won't use it. There are 2 generic tests that use src/godown directly and not via common shutdown helper. Those tests are skipped with -overlay due to other unmet requirements, but as a general practice, using src/godown directly should be avoided if possible. Thanks, Amir.
