Re: [PATCH] Add me_mountroot to mount_entry for linux machines

[Pádraig Brady]

On 28/08/15 21:41, Dave Chiluk wrote:
> Add me_mountroot to mount_entry so linux machines based on
> /proc/self/mountinfo can distinguish between bind mounts and original
> mounts. In reality bind mounts aren't treated any different than
> mountroot=/ mounts by the kernel, but this is still confusing to the
> user, and a change in behavior

Thanks, I think this makes sense,
and shouldn't introduce API issues since gnulib
isn't released as a separate lib.
More notes below...

> More information is available here
> https://bugs.launchpad.net/ubuntu/+source/coreutils/+bug/1432871
> ---
>  lib/mountlist.c | 29 ++++++++++++++++++++++++++---
>  lib/mountlist.h |  2 ++
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/mountlist.c b/lib/mountlist.c
> index 6f04f55..4e4c129 100644
> --- a/lib/mountlist.c
> +++ b/lib/mountlist.c
> @@ -480,25 +480,32 @@ read_file_system_list (bool need_fs_type)
>              unsigned int devmaj, devmin;
>              int target_s, target_e, type_s, type_e, source_s, source_e;
>              char test;
> -            char *dash;
> +            char *dash, *mountroot;
>              int rc;
>  
>              rc = sscanf(line, "%*u "        /* id - discarded  */
>                                "%*u "        /* parent - discarded */
>                                "%u:%u "      /* dev major:minor  */
> -                              "%*s "        /* mountroot - discarded  */
> +                              "%ms "        /* mountroot */

Better to use the mntroot_s, mntroot_e ... unescape_tab()
mechanism used for other parsed paths.

>                                "%n%*s%n"     /* target, start and end  */
>                                "%c",         /* more data...  */
>                                &devmaj, &devmin,
> +                              &mountroot,
>                                &target_s, &target_e,
>                                &test);
> -            if (rc != 3 && rc != 5)  /* 5 if %n included in count.  */
> +            if (rc != 4 && rc != 6)  /* 6 if %n included in count.  */
> +            {
> +              free(mountroot);
>                continue;
> +            }
>  
>              /* skip optional fields, terminated by " - "  */
>              dash = strstr (line + target_e, " - ");
>              if (! dash)
> +            {
> +              free(mountroot);
>                continue;
> +            }
>  
>              rc = sscanf(dash, " - "
>                                "%n%*s%n "    /* FS type, start and end  */
> @@ -508,7 +515,10 @@ read_file_system_list (bool need_fs_type)
>                                &source_s, &source_e,
>                                &test);
>              if (rc != 1 && rc != 5)  /* 5 if %n included in count.  */
> +            {
> +              free(mountroot);
>                continue;
> +            }
>  
>              /* manipulate the sub-strings in place.  */
>              line[target_e] = '\0';
> @@ -521,6 +531,7 @@ read_file_system_list (bool need_fs_type)
>  
>              me->me_devname = xstrdup (dash + source_s);
>              me->me_mountdir = xstrdup (line + target_s);
> +            me->me_mountroot = mountroot;
>              me->me_type = xstrdup (dash + type_s);
>              me->me_type_malloced = 1;
>              me->me_dev = makedev (devmaj, devmin);
> @@ -566,6 +577,7 @@ read_file_system_list (bool need_fs_type)
>              me = xmalloc (sizeof *me);
>              me->me_devname = xstrdup (mnt->mnt_fsname);
>              me->me_mountdir = xstrdup (mnt->mnt_dir);
> +            me->me_mountroot = NULL;
>              me->me_type = xstrdup (mnt->mnt_type);
>              me->me_type_malloced = 1;
>              me->me_dummy = ME_DUMMY (me->me_devname, me->me_type, bind);
> @@ -598,6 +610,7 @@ read_file_system_list (bool need_fs_type)
>          me = xmalloc (sizeof *me);
>          me->me_devname = xstrdup (fsp->f_mntfromname);
>          me->me_mountdir = xstrdup (fsp->f_mntonname);
> +        me->me_mountroot = NULL;
>          me->me_type = fs_type;
>          me->me_type_malloced = 0;
>          me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
> @@ -624,6 +637,7 @@ read_file_system_list (bool need_fs_type)
>          me = xmalloc (sizeof *me);
>          me->me_devname = xstrdup (fsp->f_mntfromname);
>          me->me_mountdir = xstrdup (fsp->f_mntonname);
> +        me->me_mountroot = NULL;
>          me->me_type = xstrdup (fsp->f_fstypename);
>          me->me_type_malloced = 1;
>          me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
> @@ -650,6 +664,7 @@ read_file_system_list (bool need_fs_type)
>          me = xmalloc (sizeof *me);
>          me->me_devname = xstrdup (fsd.fd_req.devname);
>          me->me_mountdir = xstrdup (fsd.fd_req.path);
> +        me->me_mountroot = NULL;
>          me->me_type = gt_names[fsd.fd_req.fstype];
>          me->me_type_malloced = 0;
>          me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
> @@ -748,6 +763,7 @@ read_file_system_list (bool need_fs_type)
>            me->me_devname = xstrdup (fi.device_name[0] != '\0'
>                                      ? fi.device_name : fi.fsh_name);
>            me->me_mountdir = xstrdup (re != NULL ? re->name : fi.fsh_name);
> +          me->mountroot = NULL ;
>            me->me_type = xstrdup (fi.fsh_name);
>            me->me_type_malloced = 1;
>            me->me_dev = fi.dev;
> @@ -797,6 +813,7 @@ read_file_system_list (bool need_fs_type)
>          me = xmalloc (sizeof *me);
>          me->me_devname = xstrdup (stats[counter].f_mntfromname);
>          me->me_mountdir = xstrdup (stats[counter].f_mntonname);
> +        me->me_mountroot = NULL;
>          me->me_type = xstrdup (FS_TYPE (stats[counter]));
>          me->me_type_malloced = 1;
>          me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
> @@ -833,6 +850,7 @@ read_file_system_list (bool need_fs_type)
>          strcpy (me->me_devname + 5, mnt.mt_dev);
>  # endif
>          me->me_mountdir = xstrdup (mnt.mt_filsys);
> +        me->me_mountroot = NULL;
>          me->me_dev = (dev_t) -1;        /* Magic; means not known yet. */
>          me->me_type = "";
>          me->me_type_malloced = 0;
> @@ -880,6 +898,7 @@ read_file_system_list (bool need_fs_type)
>          me = xmalloc (sizeof *me);
>          me->me_devname = xstrdup ((*ent)->mt_resource);
>          me->me_mountdir = xstrdup ((*ent)->mt_directory);
> +        me->me_mountroot = NULL;
>          me->me_type = xstrdup ((*ent)->mt_fstype);
>          me->me_type_malloced = 1;
>          me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
> @@ -942,6 +961,7 @@ read_file_system_list (bool need_fs_type)
>              me = xmalloc (sizeof *me);
>              me->me_devname = xstrdup (mnt.mnt_special);
>              me->me_mountdir = xstrdup (mnt.mnt_mountp);
> +            me->me_mountroot = NULL;
>              me->me_type = xstrdup (mnt.mnt_fstype);
>              me->me_type_malloced = 1;
>              me->me_dummy = MNT_IGNORE (&mnt) != 0;
> @@ -1020,6 +1040,7 @@ read_file_system_list (bool need_fs_type)
>                                        vmp->vmt_data[VMT_OBJECT].vmt_off);
>            }
>          me->me_mountdir = xstrdup (thisent + 
> vmp->vmt_data[VMT_STUB].vmt_off);
> +        me->me_mountroot = NULL;
>          me->me_type = xstrdup (fstype_to_string (vmp->vmt_gfstype));
>          me->me_type_malloced = 1;
>          options = thisent + vmp->vmt_data[VMT_ARGS].vmt_off;
> @@ -1063,6 +1084,7 @@ read_file_system_list (bool need_fs_type)
>              me = xmalloc (sizeof *me);
>              me->me_devname = xstrdup (dev.f_mntfromname);
>              me->me_mountdir = xstrdup (dev.f_mntonname);
> +            me->me_mountroot = NULL;
>              me->me_type = xstrdup (dev.f_fstypename);
>              me->me_type_malloced = 1;
>              me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
> @@ -1105,6 +1127,7 @@ void free_mount_entry (struct mount_entry *me)
>  {
>    free (me->me_devname);
>    free (me->me_mountdir);
> +  free (me->me_mountroot);
>    if (me->me_type_malloced)
>      free (me->me_type);
>    free (me);
> diff --git a/lib/mountlist.h b/lib/mountlist.h
> index 735776b..38a4ccf 100644
> --- a/lib/mountlist.h
> +++ b/lib/mountlist.h
> @@ -27,6 +27,8 @@ struct mount_entry
>  {
>    char *me_devname;             /* Device node name, including "/dev/". */
>    char *me_mountdir;            /* Mount point directory name. */
> +  char *me_mountroot;           /* Directory on filesystem of device used */
> +                                /* as root for the mount*/

s/mount/(bind) mount. /

>    char *me_type;                /* "nfs", "4.2", etc. */
>    dev_t me_dev;                 /* Device number of me_mountdir. */
>    unsigned int me_dummy : 1;    /* Nonzero for dummy file systems. */
> 

thanks,
Pádraig