[CRIU] [RFC PATCH 3/6] introduce mnt_entry->fsname for FSTYPE__AUTO filesystems

[Oleg Nesterov]

On 03/30, Pavel Emelyanov wrote:
>
> On 03/30/2015 06:12 PM, Oleg Nesterov wrote:
> > On 03/30, Pavel Emelyanov wrote:
> >>
> >> On 03/27/2015 08:55 PM, Oleg Nesterov wrote:
> >>> A separate change for documentation purposes.
> >>>
> >>> This patch adds mnt_entry->fsname to dump/restore the ->fsname of the
> >>> FSTYPE__AUTO filesystems. This is ugly, we need to dump and restore
> >>> the list of FSTYPE__AUTO names once,
> >>
> >> I don't understand this :( What's wrong with how it's done in this patch?
> >
> > Well, I do not like the new mnt_entry->fsname field. In fact I do not
> > want to touch this mnt.proto at all. We can avoid this afaics, see below.
>
> Why don't we want to touch it? For me adding optional fsname field for
> FSTYPE__AUTO sounds OK :)

The less changes the better ;)

> > This is suboptimal although we do not really care.
> >
> > More importantly, this doesn't look clean. Perhaps even wrong in the long
> > term.
> >
> > prepare_mnt_ns() calls collect_mntinfo() before collect_mnt_from_image(),
> > and collect_mntinfo() does parse_mountinfo() too. So unless the user passes
> > the same --xxx argument to restore (and I think this arg should be nacked
> > if "restore"), find_fstype_by_name() will return FSTYPE__UNSUPPORTED while
> > this fs was dumped as FSTYPE__AUTO.
> >
> > Probably (but I am not really sure) this doesn't matter today correctness
> > wise, but this doesn't look good anyway.
> >
> > So. Can't we just dump/restore an additional string?
>
> From my perspective -- we can :)

OK. good.

> > To simplify the discussion, lets suppose that we want to dump/restore that
> > --xxx argument (we do not actually want this).
> >
> > How can we do this?
> >
> > Can I add .xxx to "struct inventory_entry" ? check_img_inventory() is called
> > early, and this is what we need on restore.
> >
> > But unfortunately write_img_inventory() is called early too, and this doesn't
> > allow to dump the list of actually used FSTYPE__AUTO names after dump_namespaces.
> >
> > Can I move write_img_inventory() down to make it the last "dump"? Or is there
> > a better solution?
>
> Do I get you correctly that you want to de-duplicate fsname-s for many entries?

No. Well yes, de-duplication is good too, but this is minor.

The main problem is that (I think) we need (or at least want) to re-create the
FSTYPE__AUTO entries asap, before collect_mnt_from_image().

See above, parse_mountinfo() is called before this stage. Again, iiuc currently
this doesn't matter correctness wise, but this looks unsafe. At least inconsistent.
It would be better to ensure that "restore" never sees (say) hugetlbfs as
FSTYPE__UNSUPPORTED if it was dumped as FSTYPE__AUTO. Even if (iiuc) collect_mntinfo()
is called to umount everything and thus FSTYPE__UNSUPPORTED is fine.

OK. If we can abuse inventory_entry, I'll try to do this. We will see if this
looks better or not. If not, we can return to mnt_entry->fsname.

Oleg.