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. Thanks, Miklos -- 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
