<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Aug 13, 2014 at 12:43 PM, Andrew Vagin <span dir="ltr"><<a href="mailto:avagin@parallels.com" target="_blank">avagin@parallels.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">On Wed, Aug 13, 2014 at 11:59:33AM -0700, <a href="mailto:gbellack@google.com">gbellack@google.com</a> wrote:<br>
> From: Garrison Bellack <<a href="mailto:gbellack@google.com">gbellack@google.com</a>><br>
><br>
> Because different kernel versions have different cgroup properties, criu<br>
> shouldn't crash just because the properties statically listed aren't exact.<br>
> Instead, during dump, ignore properties the kernel doesn't have and continue.<br>
> In addition, during restore, in the event of migration to a kernel that is<br>
> missing a property that was dumped, print an error but continue.<br>
<br>
</div>I am not sure that we can continue in this case. Why do you think that<br>
it's safe? I think we need to ask a user about this. So can we add a new<br>
option to allow continuing in this case?</blockquote><div><br></div><div>I'm a little confused about your concerns of safety. Are you referring to missing properties on the dump or restore side?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div class="h5">
><br>
> Change-Id: I5a8b93d6a8a3a9664914f10cf8e2110340dd8b31<br>
> Signed-off-by: Garrison Bellack <<a href="mailto:gbellack@google.com">gbellack@google.com</a>><br>
> ---<br>
> cgroup.c | 40 ++++++++++++++++++++++++++--------------<br>
> 1 file changed, 26 insertions(+), 14 deletions(-)<br>
><br>
> diff --git a/cgroup.c b/cgroup.c<br>
> index f8dbbde..b727a50 100644<br>
> --- a/cgroup.c<br>
> +++ b/cgroup.c<br>
> @@ -284,35 +284,30 @@ static int find_dir(const char *path, struct list_head *dirs, struct cgroup_dir<br>
> * Currently this function only supports properties that have 1 value, under 100<br>
> * chars<br>
> */<br>
> -static int read_cgroup_prop(struct cgroup_prop *property, const char *fpath)<br>
> +static int read_cgroup_prop(struct cgroup_prop *property, const char *fullpath)<br>
> {<br>
> - char pbuf[PATH_MAX], buf[100];<br>
> + char buf[100];<br>
> FILE *f;<br>
> char *endptr;<br>
><br>
> - if (snprintf(pbuf, PATH_MAX, "%s/%s", fpath, property->name) >= PATH_MAX) {<br>
> - pr_err("snprintf output was truncated");<br>
> - return -1;<br>
> - }<br>
> -<br>
> - f = fopen(pbuf, "r");<br>
> + f = fopen(fullpath, "r");<br>
> if (!f) {<br>
> - pr_err("Failed opening %s\n", pbuf);<br>
> property->value = NULL;<br>
> + pr_perror("Failed opening %s\n", fullpath);<br>
> return -1;<br>
> }<br>
><br>
> memset(buf, 0, sizeof(buf));<br>
> if (fread(buf, sizeof(buf), 1, f) != 1) {<br>
> if (!feof(f)) {<br>
> - pr_err("Failed scanning %s\n", pbuf);<br>
> + pr_err("Failed scanning %s\n", fullpath);<br>
> fclose(f);<br>
> return -1;<br>
> }<br>
> }<br>
><br>
> if (fclose(f) != 0) {<br>
> - pr_err("Failed closing %s\n", pbuf);<br>
> + pr_err("Failed closing %s\n", fullpath);<br>
> return -1;<br>
> }<br>
><br>
> @@ -390,6 +385,7 @@ static int add_cgroup_properties(const char *fpath, struct cgroup_dir *ncd,<br>
> struct cg_controller *controller)<br>
> {<br>
> int i, j;<br>
> + char buf[PATH_MAX];<br>
> struct cgroup_prop *prop;<br>
><br>
> for (i = 0; i < controller->n_controllers; ++i) {<br>
> @@ -397,16 +393,28 @@ static int add_cgroup_properties(const char *fpath, struct cgroup_dir *ncd,<br>
> const char **prop_arr = get_known_properties(controller->controllers[i]);<br>
><br>
> for (j = 0; prop_arr != NULL && prop_arr[j] != NULL; ++j) {<br>
> + if (snprintf(buf, PATH_MAX, "%s/%s", fpath, prop_arr[j]) >= PATH_MAX) {<br>
> + pr_err("snprintf output was truncated");<br>
> + return -1;<br>
> + }<br>
> +<br>
> + if (access(buf, F_OK) < 0 && errno == ENOENT) {<br>
> + pr_info("Couldn't open %s. This cgroup property may not exist on this kernel\n", buf);<br>
> + continue;<br>
> + }<br>
> +<br>
> prop = create_cgroup_prop(prop_arr[j]);<br>
> if (!prop) {<br>
> free_all_cgroup_props(ncd);<br>
> return -1;<br>
> }<br>
> - if (read_cgroup_prop(prop, fpath) < 0) {<br>
> +<br>
> + if (read_cgroup_prop(prop, buf) < 0) {<br>
> free_cgroup_prop(prop);<br>
> free_all_cgroup_props(ncd);<br>
> return -1;<br>
> }<br>
> +<br>
> pr_info("Dumping value %s from %s/%s\n", prop->value, fpath, prop->name);<br>
> list_add_tail(&prop->list, &ncd->properties);<br>
> ncd->n_properties++;<br>
> @@ -924,8 +932,12 @@ static int restore_cgroup_prop(const CgroupPropEntry * cg_prop_entry_p,<br>
><br>
> f = fopenat(cg, path, "w+");<br>
> if (!f) {<br>
> - pr_perror("Failed opening %s for writing\n", path);<br>
> - return -1;<br>
> + if (errno != ENOENT) {<br>
> + pr_perror("Failed opening %s\n", path);<br>
> + return -1;<br>
> + }<br>
> + pr_perror("Couldn't open %s. This cgroup property may not exist on this kernel\n", path);<br>
> + return 0;<br>
> }<br>
><br>
> if (fprintf(f, "%s", cg_prop_entry_p->value) < 0) {<br>
> --<br>
> 2.1.0.rc2.206.gedb03e5<br>
><br>
</div></div>> _______________________________________________<br>
> CRIU mailing list<br>
> <a href="mailto:CRIU@openvz.org">CRIU@openvz.org</a><br>
> <a href="https://lists.openvz.org/mailman/listinfo/criu" target="_blank">https://lists.openvz.org/mailman/listinfo/criu</a><br>
</blockquote></div><br></div></div>