[CRIU] [PATCH 2/2] Make lacking cgroup properties non-fatal

[Tycho Andersen]

On Wed, Aug 13, 2014 at 01:32:01PM -0700, Garrison Bellack wrote:
> On Wed, Aug 13, 2014 at 12:43 PM, Andrew Vagin <avagin at parallels.com> wrote:
> 
> > On Wed, Aug 13, 2014 at 11:59:33AM -0700, gbellack at google.com wrote:
> > > From: Garrison Bellack <gbellack at google.com>
> > >
> > > Because different kernel versions have different cgroup properties, criu
> > > shouldn't crash just because the properties statically listed aren't
> > exact.
> > > Instead, during dump, ignore properties the kernel doesn't have and
> > continue.
> > > In addition, during restore, in the event of migration to a kernel that
> > is
> > > missing a property that was dumped, print an error but continue.
> >
> > I am not sure that we can continue in this case. Why do you think that
> > it's safe? I think we need to ask a user about this. So can we add a new
> > option to allow continuing in this case?
> 
> 
> I'm a little confused about your concerns of safety. Are you referring to
> missing properties on the dump or restore side?

I think restore side. I also wondered the same thing -- it seems that
missing properties on restore should be an error (perhaps as Andrew
says with an --allow-missing-cg-props flag on the restore side to
allow you to override this error).

Tycho

> 
> > >
> > > Change-Id: I5a8b93d6a8a3a9664914f10cf8e2110340dd8b31
> > > Signed-off-by: Garrison Bellack <gbellack at google.com>
> > > ---
> > >  cgroup.c | 40 ++++++++++++++++++++++++++--------------
> > >  1 file changed, 26 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/cgroup.c b/cgroup.c
> > > index f8dbbde..b727a50 100644
> > > --- a/cgroup.c
> > > +++ b/cgroup.c
> > > @@ -284,35 +284,30 @@ static int find_dir(const char *path, struct
> > list_head *dirs, struct cgroup_dir
> > >   * Currently this function only supports properties that have 1 value,
> > under 100
> > >   * chars
> > >   */
> > > -static int read_cgroup_prop(struct cgroup_prop *property, const char
> > *fpath)
> > > +static int read_cgroup_prop(struct cgroup_prop *property, const char
> > *fullpath)
> > >  {
> > > -     char pbuf[PATH_MAX], buf[100];
> > > +     char buf[100];
> > >       FILE *f;
> > >       char *endptr;
> > >
> > > -     if (snprintf(pbuf, PATH_MAX, "%s/%s", fpath, property->name) >=
> > PATH_MAX) {
> > > -             pr_err("snprintf output was truncated");
> > > -             return -1;
> > > -     }
> > > -
> > > -     f = fopen(pbuf, "r");
> > > +     f = fopen(fullpath, "r");
> > >       if (!f) {
> > > -             pr_err("Failed opening %s\n", pbuf);
> > >               property->value = NULL;
> > > +             pr_perror("Failed opening %s\n", fullpath);
> > >               return -1;
> > >       }
> > >
> > >       memset(buf, 0, sizeof(buf));
> > >       if (fread(buf, sizeof(buf), 1, f) != 1) {
> > >               if (!feof(f)) {
> > > -                     pr_err("Failed scanning %s\n", pbuf);
> > > +                     pr_err("Failed scanning %s\n", fullpath);
> > >                       fclose(f);
> > >                       return -1;
> > >               }
> > >       }
> > >
> > >       if (fclose(f) != 0) {
> > > -             pr_err("Failed closing %s\n", pbuf);
> > > +             pr_err("Failed closing %s\n", fullpath);
> > >               return -1;
> > >       }
> > >
> > > @@ -390,6 +385,7 @@ static int add_cgroup_properties(const char *fpath,
> > struct cgroup_dir *ncd,
> > >                                struct cg_controller *controller)
> > >  {
> > >       int i, j;
> > > +     char buf[PATH_MAX];
> > >       struct cgroup_prop *prop;
> > >
> > >       for (i = 0; i < controller->n_controllers; ++i) {
> > > @@ -397,16 +393,28 @@ static int add_cgroup_properties(const char
> > *fpath, struct cgroup_dir *ncd,
> > >               const char **prop_arr =
> > get_known_properties(controller->controllers[i]);
> > >
> > >               for (j = 0; prop_arr != NULL && prop_arr[j] != NULL; ++j) {
> > > +                     if (snprintf(buf, PATH_MAX, "%s/%s", fpath,
> > prop_arr[j]) >= PATH_MAX) {
> > > +                             pr_err("snprintf output was truncated");
> > > +                             return -1;
> > > +                     }
> > > +
> > > +                     if (access(buf, F_OK) < 0 && errno == ENOENT) {
> > > +                             pr_info("Couldn't open %s. This cgroup
> > property may not exist on this kernel\n", buf);
> > > +                             continue;
> > > +                     }
> > > +
> > >                       prop = create_cgroup_prop(prop_arr[j]);
> > >                       if (!prop) {
> > >                               free_all_cgroup_props(ncd);
> > >                               return -1;
> > >                       }
> > > -                     if (read_cgroup_prop(prop, fpath) < 0) {
> > > +
> > > +                     if (read_cgroup_prop(prop, buf) < 0) {
> > >                               free_cgroup_prop(prop);
> > >                               free_all_cgroup_props(ncd);
> > >                               return -1;
> > >                       }
> > > +
> > >                       pr_info("Dumping value %s from %s/%s\n",
> > prop->value, fpath, prop->name);
> > >                       list_add_tail(&prop->list, &ncd->properties);
> > >                       ncd->n_properties++;
> > > @@ -924,8 +932,12 @@ static int restore_cgroup_prop(const
> > CgroupPropEntry * cg_prop_entry_p,
> > >
> > >       f = fopenat(cg, path, "w+");
> > >       if (!f) {
> > > -             pr_perror("Failed opening %s for writing\n", path);
> > > -             return -1;
> > > +             if (errno != ENOENT) {
> > > +                     pr_perror("Failed opening %s\n", path);
> > > +                     return -1;
> > > +             }
> > > +             pr_perror("Couldn't open %s. This cgroup property may not
> > exist on this kernel\n", path);
> > > +             return 0;
> > >       }
> > >
> > >       if (fprintf(f, "%s", cg_prop_entry_p->value) < 0) {
> > > --
> > > 2.1.0.rc2.206.gedb03e5
> > >
> > > _______________________________________________
> > > CRIU mailing list
> > > CRIU at openvz.org
> > > https://lists.openvz.org/mailman/listinfo/criu
> >

> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu