On Fri, Jul 14, 2017 at 1:05 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > On Fri, Jul 14, 2017 at 8:51 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> On Thu, Jul 13, 2017 at 11:13 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >>> On Tue, Jul 11, 2017 at 2:58 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >>>> Just like on failure to create work dir or index dir, when >>>> index dir verification or cleanup fails, mount the overlay >>>> read-only with a warning instead of failing the mount. >>> >>> Why? >> >> First of all, why not? to me, it makes more sense to treat >> "cannot create indexdir" and "cannot use existing indexdir" the same way >> and provide slightly better user experience. > > Not sure. Failure is the best way to alert the user of a problem and > "cannot use existing indexdir" is a permanent error that can only be > fixed by removing the indexdir. > > The failure to create workdir case is possibly temporary error (we > could check for EROFS to make sure) and one that happens in real life. > It was an actual bug report that resulted in this behavior AFAIR. > >> But I agree that this patch doesn't fix any bug or correctness issue. >> >>> >>> The failure to create case was done to ensure we can mount the overlay >>> read-only with the same options if the underlying layers are read-only >>> as well. >>> >>> This, however, does not have such a use case. Or does it? >>> >> >> I can think of two corner use cases: >> 1. We have a bug that causes ovl_indexdir_cleanup() failure. >> 2. Trying to reuse workdir from previous mount with a new upperdir. >> >> The second use case happened to me both with unionmount-testsuite >> and with xfstests. >> I pushed a fix to unionmount-testsuite: >> d283eab - Use separate workdir per upper dir >> and posted a fix for xfstests. >> >> Of course I am not concerned about out of date testsuite failing tests, >> but it goes to show that if utilities *can* reuse workdir with new >> upperdir they may very well do so, just on account on being used >> to overlayfs cleaning workdir for them on mount. >> >> If such a utility exists, say to create a new overlay of /home/guest/ dir >> on every login and it happens to reuse the same workdir for every login, >> then upgrading to new kernel with CONFIG_OVERLAYFS_INDEX=y will >> result in failure to mount /home/guest on login. >> >> I claim, that we would be better off falling back to ro mount, which could, >> maybe slightly, alter the pitch of the voice of the shouting we get from >> admins. >> >> But maybe I am miss-reading the shouting chain. I have no experience >> interacting with downstream projects. > > Maybe better just add big warnings about back incompatibility of > CONFIG_OVERLAYFS_INDEX=y to make sure unsuspecting sysadmins don't > turn this on accidentally. > What? always warn on boot? on mount? I donno. To me the comment about backward compat in Kconfig seems sufficient and no sysadmin can mistakenly set index=on on mount, but as I said, don't know enough about downstream to say much. i can only say that your arguments sound just as convincing and i won't shed any tears if this patch is dropped. We can always apply it as stable fix if the need arise. Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
