On Tue, Aug 25, 2020 at 3:55 AM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > On Tue, Aug 25, 2020 at 12:51:55AM +0300, Amir Goldstein wrote: > > > Ok, I am wondering why are we concerned about older kernels. I mean, > > > if we introduce new features, we don't provide compatibility with > > > older kernels. Say "metacopy", "redirect_dir". If you mount with > > > older kernel, they will see something which you don't expect. > > > > > > > True. We missed the opportunity to do the work/incompat trick > > with metacopy etc. > > > > > So why "volatile" is different. We seem to be bending backward and > > > using an unrelated behavior of overlay to provide this. > > > > > > Why not simply drop a file $workdir/volatile for volatile mounts > > > and check for presence of this file when mounting? > > > > > > > That's an option. > > But what's the problem with > > $workdir/work/incompat/volatile/dirty > > compared to: > > $workdir/volatile > > > > It's not more complicated to implement is it? > > So we get some extra protection with little extra cost. No? > > Ok, I will look into it. > > > > I don't feel strongly about it. > > > > But I must say, according to Giuseppe's description of the use case: > > "mount volatile overlay+umount overlay+syncfs upper dir..." > > looks like what he is looking for is "volatile,sync=shutdown", is it not? > > > > And if this is the case, I think it would be much preferred to implement > > "volatile,sync=shutdown", over documenting how to make a "volatile" > > overlay mountable from outside overlay. Don't you guys agree? > > When it comes to requirements, to me it felt that Giuseppe seemed > to have two requirements. For running containers, he did not care > seem to care about syncing upper to disk at all. For building > images he probably wanted to sync upper to disk. > You know, I am not sure that building images requires syncing to disk. Why is syncfs() needed between unmount and copying/tar'ing the layers? Why is it needed before mounting again? It is not. It is only needed "before crash", so whether or not "dirty volatile" overlay can be mounted is a decision better be made by userspace. The only problem with this approach is that it is a bit harder to document the filesystem behavior, but I think that we need to. > From overlayfs perspective, "volatile,sync=shutdown" seems like > a nicer interface because overlay will take care of removing > "dirty" file and until and unless crash happens, user does > not have to step in and there is less confusion about syncing > upper and removing dirty file etc. > > Last time Miklos seemed to prefer to implement just "volatile" > for now and drop "sync=shutdown". > > https://lore.kernel.org/linux-unionfs/CAJfpegt2k=r6TRok57tKPcLyUhCBOcBAV7bgLSPrQYXsPoPkpQ@xxxxxxxxxxxxxx/ > > I personally think that "volatile,sync=shutdown" is first good step > because it is less error prone and overlayfs manages dirty file > and it will provide lot of benefits in terms of not having to > do very frequent sync. > > And if this does not prove to be enough for certain use cases, > then one can extend this to also implement "volatile,sync=none". > > But frankly speaking, there has been so much of back and forth > on this patch, that I am fine with any of the option which is > acceptable to Miklos. > I agree. Miklos accepted $workdir/work/incompat/volatile/dirty. I assume the name 'dirty'/'donotremove' is not an issue. It's simple. Let's go with that. Thanks, Amir.
