[CRIU] [RFC PATCH 0/5] introduce --enable-fs cli option

[Pavel Emelyanov]

On 04/10/2015 04:20 PM, Oleg Nesterov wrote:
> On 04/10, Pavel Emelyanov wrote:
>>
>> On 04/10/2015 03:19 PM, Oleg Nesterov wrote:
>>>>> 	   Why do we expose "enum fstype" to protobufs at all? Please
>>>>> 	   note that typeof(mnt_entry->fstype) is uint32, not "fstype".
>>>>
>>>> Yes, we have this fact mentioned on the "what's bad with v1 images" page.
>>>> We want it to be the "fstype".
>>>
>>> Yes, and thus it is not clear what could I do if I wanted to implement
>>> "dump/restore auto fs names once".
>>>
>>> Because a single FSTYPE__AUTO code can't work in this case. This is trivial,
>>> just we need
>>>
>>>
>>> 	#define FSTYPE__AUTO		1024	// random big value
>>> 	static int fstype_auto_cur = FSTYPE__AUTO;
>>>
>>> 	fscode_is_auto(code)
>>> 	{
>>> 		return code >= FSTYPE__AUTO;
>>> 	}
>>
>> I don't get it -- why would we need more AUTO codes than just one? If you
>> keep the fsname in the image anyway.
> 
> If we do not add ->fsname into mnt_entry, then we obviously can't use
> a single FSTYPE__AUTO in mnt_entry->fstype.
> 
> What we need to do is:
> 
> 	1. generate the new AUTO code (just fstype_auto_cur++) every
> 	   time we add the new AUTO entry.
> 
> 	2. Move write_img_inventory() down in cr_dump_tasks() to make
> 	   it the last "dump".
> 
> 	3. Save the (say) comma-separated list of the used auto fsnames
> 	   in write_img_inventory(). This needs the new member, say,
> 	   inventory_entry->auto_names.
> 
> Now "restore" should just restore the auto entries in check_img_inventory()
> from he->auto_names, decode_fstype(me->fstype) will work just fine.
> 
> But again, this means that we need the "typeless" mnt_entry->fstype.

Sure, but we keep the fsname in mnt_entry (in the image) and this is nice :)

>>> Not sure I understand... or perhaps you misunderstood me. I meant that
>>> even if we turn fstypes[] into list_head, we still need fstypes_bultin[]
>>> and constructor which walks this array and adds everything into the list.
>>
>> But you don't have to turn fstypes into list-head. The current implementation
>> (seems to) works pretty well -- you just keep a string of "auto" fsnames and
>> scan it without any need to add more than one entry in the fstypes[] array.
> 
> Ah! So I misunderstood you.
> 
> Yes, I thought about a single "auto" entry too. Firstly this doesn't look
> clean to me because for example mounts_equal() checks c->fstype != mi->fstype.
> Yes, yes, this doesn't really matter, it will work anyway. Still doesn't
> look nice, I'd like to keep the "different fstypes should differ" property.
> 
> But this is minor. The main problem is: what should "dump" write into
> ->fsname? With this series dump_one_mountpoint() simply does
> 
> 	if (me.fstype == FSTYPE__AUTO)
> 		me.fsname = pm->fstype->name;
> 
> If we make a single "auto" entry, we forget the actual fsname immediately
> after find_fstype_by_name(). So we need the new mount_info->fsname member
> initialized by parse_mountinfo_ent(). Doesn't look very nice.

OK, agreed. I will then apply the existing set and we'll make the rest on top.

-- Pavel