[CRIU] [PATCH 1/2] mount: Use path resolving to find mount points

[Pavel Emelyanov]

On 12/06/2013 03:09 PM, Cyrill Gorcunov wrote:
> On Fri, Dec 06, 2013 at 02:57:15PM +0400, Pavel Emelyanov wrote:
>> On 12/06/2013 02:53 PM, Cyrill Gorcunov wrote:
>>> On Fri, Dec 06, 2013 at 02:39:37PM +0400, Pavel Emelyanov wrote:
>>>>> @@ -270,7 +270,7 @@ struct file_remap *lookup_ghost_remap(u32 dev, u32 ino)
>>>>>  
>>>>>  	mutex_lock(ghost_file_mutex);
>>>>>  	list_for_each_entry(gf, &ghost_files, list) {
>>>>> -		if (phys_stat_dev_match(gf->dev, dev) && gf->ino == ino) {
>>>>> +		if (gf->ino == ino && phys_stat_dev_match(gf->dev, dev, gf->remap.path)) {
>>>>
>>>> The gf->dev value came from the image. It's the device we _saw_ at dump time.
>>>> Thus we should do the name resolution _then_ and write in the image the phys
>>>> device ID instead of volume id.
>>>
>>> Sticktly speaking -- yes, we should. But on the other hands I don't see much point
>>> in it. gf->dev migh be some subvolume and then gf->dev != dev but path-resolve
>>> should find that @dev is subvolume of some mount point where gf->remap.path
>>> is resides. No?
>>
>> No. Currently you may dump tasks on btrfs, them move them to another box with
>> changed devices. All will work, since on restore we don't stat paths. With your
>> patch things would break.
> 
> And again gf->dev != dev then we take a slow walk and resolve gf->remap.path
> thus @dev must be some (new!) subvolume of mount point @gf->remap.path lays on.
> If gf->dev != dev we don't care about gf->dev anymore but check if @dev is
> subvolume of mount point where gf->remap.path resides. So I don't get how it
> is breaking here. What I'm missing?

You check the dev against dev-s obtained from destination node's /proc/mountinfo.
The devs in there may differ from those in the image, if you don't take mount
namespace with you (and read one from the /proc on restore).

IOW the path-based dev vs proc dev matching should be done in one environment. In
this place gf->dev is from the image (i.e. -- from dump stage) but path-based dev
is from the place we restore images.

> Look the function
> 
> bool phys_stat_dev_match(dev_t st_dev, dev_t phys_dev, const char *path)
> {
> 	struct mount_info *m;
> 
> 	if (st_dev == phys_dev)
> 		return true;
> 
> 	m = mount_resolve_path(path);
> 
> 	if (!strcmp(m->fstype->name, "btrfs"))
> 		return m->s_dev == phys_dev;
> 	return false;
> }
> 
> st_dev here is @gf->dev, phys_dev is @dev from inotify|unix-socket if we have
> found that gf->remap.path is on btrfs we check if @phys_dev == m->s_dev.
> .
>