files-reg: Prepare for sysfs entries mode change

Submitted by Kirill Gorkunov on Jan. 15, 2018, 10:51 p.m.

Details

Message ID 20180115225131.GC13633@uranus.lan
State New
Series "files-reg: Prepare for sysfs entries mode change"
Headers show

Commit Message

Kirill Gorkunov Jan. 15, 2018, 10:51 p.m.
The kernel virtualize access to proc/sys/ entries in
lightweight way -- if entry is opened from inside of
veX then it's not allowed to be written.

Still we're dumping files in ve0 environment so the
mode for such files may no match on restore, because
restore itself is running inside veX.

 | (00.701122)    111: Error (criu/files-reg.c:1976): File proc/sys/vm/overcommit_kbytes has bad mode 0100444 (expect 0100644)
 | (00.701496)    334: Error (criu/cr-restore.c:1353): 111 exited, status=1

so i think we can simply skip such testing inside ve criu
instance since it's kernel specific.

Simply print out a warning for refernce and continue

 | (00.827328)    111: Error (criu/files-reg.c:1977): File proc/sys/vm/overcommit_kbytes has bad mode 0100444 (expect 0100644)
 | (00.827332)    111: Warn  (criu/files-reg.c:1985):      Expecting in VE environment. Ignore.

https://jira.sw.ru/browse/PSBM-80585

Signed-off-by: Cyrill Gorcunov <gorcunov@virtuozzo.com>
---
 criu/files-reg.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/criu/files-reg.c b/criu/files-reg.c
index 27e4cf48d..0cc6a5e19 100644
--- a/criu/files-reg.c
+++ b/criu/files-reg.c
@@ -48,6 +48,7 @@ 
 #include "plugin.h"
 
 #define ATOP_ACCT_FILE "tmp/atop.d/atop.acct"
+#define PROCFS_SYSDIR	"proc/sys/"
 
 int setfsuid(uid_t fsuid);
 int setfsgid(gid_t fsuid);
@@ -1974,7 +1975,16 @@  int open_path(struct file_desc *d,
 				pr_err("File %s has bad mode 0%o (expect 0%o)\n",
 				       rfi->path, (int)st.st_mode,
 				       rfi->rfe->mode);
-				return -1;
+				/*
+				 * When we're restoring proc/sysfs entry the
+				 * file modes are virtualized by kernel and
+				 * 'write' bit is dropped when opening inside
+				 * veX. So don't fail in such case.
+				 */
+				if (!strncmp(rfi->path, PROCFS_SYSDIR, strlen(PROCFS_SYSDIR))) {
+					pr_warn("\tExpecting in VE environment. Ignore.\n");
+				} else
+					return -1;
 			}
 		}
 

Comments

Kirill Gorkunov Jan. 16, 2018, 8:32 a.m.
On Tue, Jan 16, 2018 at 11:03:26AM +0300, gremlin@gremlin.ru wrote:
> On 2018-01-16 01:51:31 +0300, Cyrill Gorcunov wrote:
> 
>  > The kernel virtualize access to proc/sys/ entries in lightweight
>  > way -- if entry is opened from inside of veX then it's not allowed
>  > to be written.
> 
> In general, the "ugo-w" permissions don't seem to be a good solution
> for that: returning EROFS or simply EACCES on open() for writing
> regardless of permissions would be much better.

Might be. Moreover we may rework this aspect of virtualization one
day, so then this patch will be dropped off from the criu.

>  > Still we're dumping files in ve0 environment so the mode for such
>  > files may no match on restore, because restore itself is running
>  > inside veX.
> 
> If the /proc/sys entries are not allowed to be written from inside of
> VE - then, possibly, they may be safely skipped on restore...

No. If files have been opened by container for any reason we must restore
them in opened state as well.

> 
>  > so i think we can simply skip such testing inside ve criu instance
>  > since it's kernel specific. Simply print out a warning for refernce
>  > and continue
> 
> These warnings may be annoying and spoil other messages. Adding an
> option for them (or using common -v -vv -vvv or -q) would be wise.

It's already controlled by -v option. Currently we run criu with
debug level turned on by default because we need as much information
as possible in case of error. A customer may setup -v0 and zap everything
except error messages.