[CRIU] [PATCH] mount: detect the newinstance option for devpts (v2)

[Andrey Wagin]

2014-04-02 15:54 GMT+04:00 Pavel Emelyanov <xemul at parallels.com>:
> On 04/02/2014 11:12 AM, Andrey Vagin wrote:
>> The devpts instance was mounted w/o the newinstance option if,
>> the device number is equal to the root /dev/pts.
>>
>> I think this condition is strong enough to not mount devpts in a
>> temporary place.
>>
>> v2: move the host.bla-bla-bla in kerndat.c
>> Signed-off-by: Andrey Vagin <avagin at openvz.org>
>> ---
>>  include/fs-magic.h |  4 ++++
>>  include/kerndat.h  |  3 +++
>>  kerndat.c          | 27 +++++++++++++++++++++++++++
>>  mount.c            | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 76 insertions(+)
>>
>> diff --git a/include/fs-magic.h b/include/fs-magic.h
>> index d31a939..9db3ee9 100644
>> --- a/include/fs-magic.h
>> +++ b/include/fs-magic.h
>> @@ -29,4 +29,8 @@
>>  # define SOCKFS_MAGIC                0x534f434b
>>  #endif
>>
>> +#ifndef DEVPTS_SUPER_MAGIC
>> +#define DEVPTS_SUPER_MAGIC   0x1cd1
>> +#endif
>> +
>>  #endif /* __CR_FS_MAGIC_H__ */
>> diff --git a/include/kerndat.h b/include/kerndat.h
>> index 2b3856c..6b9947f 100644
>> --- a/include/kerndat.h
>> +++ b/include/kerndat.h
>> @@ -21,4 +21,7 @@ extern int tcp_max_rshare;
>>  extern int kern_last_cap;
>>  extern u64 zero_page_pfn;
>>
>> +struct stat;
>> +extern struct stat *kerndat_get_devpts_stat(void);
>> +
>>  #endif /* __CR_KERNDAT_H__ */
>> diff --git a/kerndat.c b/kerndat.c
>> index 00eda2e..2ba1edb 100644
>> --- a/kerndat.c
>> +++ b/kerndat.c
>> @@ -9,6 +9,7 @@
>>  #include "log.h"
>>  #include "bug.h"
>>  #include "kerndat.h"
>> +#include "fs-magic.h"
>>  #include "mem.h"
>>  #include "compiler.h"
>>  #include "sysctl.h"
>> @@ -52,6 +53,32 @@ static int kerndat_get_shmemdev(void)
>>       return 0;
>>  }
>>
>> +struct stat *kerndat_get_devpts_stat()
>> +{
>> +     static struct stat st = {};
>> +     struct statfs fst;
>> +
>> +     if (st.st_dev != 0)
>> +             return &st;
>> +
>> +     if (statfs("/dev/pts", &fst)) {
>> +             pr_perror("Unable to statefs /dev/pts");
>> +             return NULL;
>> +     }
>> +     if (fst.f_type != DEVPTS_SUPER_MAGIC) {
>> +             pr_err("devpts isn't mount on the host\n");
>> +             return NULL;
>> +     }
>> +
>> +     /* The root /dev/pts is mounted w/o newinstance, isn't it? */
>> +     if (stat("/dev/pts", &st)) {
>> +             pr_perror("Unable to stat /dev/pts");
>> +             return NULL;
>> +     }
>> +
>> +     return &st;
>> +}
>> +
>>  /*
>>   * Check whether pagemap reports soft dirty bit. Kernel has
>>   * this functionality under CONFIG_MEM_SOFT_DIRTY option.
>> diff --git a/mount.c b/mount.c
>> index f88ac52..f999c39 100644
>> --- a/mount.c
>> +++ b/mount.c
>> @@ -24,6 +24,7 @@
>>  #include "image.h"
>>  #include "namespaces.h"
>>  #include "protobuf.h"
>> +#include "kerndat.h"
>>  #include "protobuf/mnt.pb-c.h"
>>
>>  /*
>> @@ -593,6 +594,46 @@ out:
>>       return NULL;
>>  }
>>
>> +/* Is it mounted w or w/o the newinstance option */
>> +static int devpts_dump(struct mount_info *pm)
>> +{
>> +     static const char newinstance[] = ",newinstance";
>> +     struct stat *host_st;
>> +     struct stat st;
>> +     DIR *fdir = NULL;
>> +     char *buf;
>> +     int len;
>> +
>> +     host_st = kerndat_get_devpts_stat();
>> +     if (host_st == NULL)
>> +             return -1;
>> +
>> +     fdir = open_mountpoint(pm);
>> +     if (fdir == NULL)
>> +             return -1;
>> +
>> +     if (fstat(dirfd(fdir), &st)) {
>> +             pr_perror("Unable to statfs %d:%s",
>> +                             pm->mnt_id, pm->mountpoint);
>> +             close_mountpoint(fdir);
>> +             return -1;
>> +     }
>> +     close_mountpoint(fdir);
>> +
>> +     if (host_st->st_dev == st.st_dev)
>> +             return 0;
>> +
>> +     len = strlen(pm->options);
>> +     buf = xrealloc(pm->options, len + sizeof(newinstance));
>> +     if (buf == NULL)
>> +             return -1;
>> +     memcpy(buf, newinstance, sizeof(newinstance));
>
> Are we OK to call mount() with options equals ",newinstance", i.e.
> with a comma as the first character?

The kernel doesn't worry about this comma.