Daniel Lezcano [dlezcano@xxxxxxxxxx] wrote: >> +find_cinit_pid(const char *name) >> { >> + struct lxc_command command = { >> + .request = { .type = LXC_COMMAND_CINIT_PID }, >> + }; >> + >> + int ret, stopped; >> + >> + ret = lxc_command(name, &command, &stopped); >> + if (ret < 0) { >> + ERROR("failed to send command"); >> + return -1; >> + } >> + >> + ERROR("find_cinit_pid %d\n", command.answer.ret); >> + >> + return command.answer.ret; >> +} > > I just committed the same function for the lxc_attach command. I think > you can reuse it. Ok, will try to reuse it. > >> +int lxc_checkpoint(const char *name, const char *statefile, int lxc_flags) >> +{ >> + int ret; >> + int pid; >> + int flags; >> + struct stat statbuf; >> + struct app_checkpoint_args crargs; >> + >> + if (access(statefile, F_OK) == 0) { >> + ret = stat(statefile, &statbuf); >> + if (ret < 0) { >> + ERROR("stat(%s): %s\n", statefile, strerror(errno)); >> + return -1; >> + } >> + >> + if (S_ISDIR(statbuf.st_mode)) { >> + ERROR("--directory option not implemented"); >> + return -1; >> + } else { >> + ERROR("Checkpoint image file %s exists\n", statefile); >> + return -1; >> + } >> + } > > For the checkpoint, you don't need to check if it's a directory or a > file (but it should be done at restart time). I need to open() the statefile and pass its fd to app_checkpoint(). If I don't check for directory, the open() below would fail, only because of the O_RDWR flag. By checking, we could give a more useful error message ? > I am not sure 'access' is > really necessary because the O_EXCL is set in the open below. Agree. How about just the stat() and the error message ? Maybe later we could bury this code under say #ifdef USERCR so if LXC is configured without USERCR, the current behavior is preserved ? > >> + >> + pid = find_cinit_pid(name); >> + if (pid < 0) { >> + ERROR("Unable to find cinit pid"); >> + return -1; >> + } >> + >> + memset(&crargs, 0, sizeof(crargs)); >> + >> + ret = open(statefile, O_CREAT|O_RDWR|O_EXCL, 0644); >> + if (ret < 0) { >> + ERROR("open(%s) failed\n", statefile); >> + return -1; >> + } > > As the statefile may contain sensible data, it would be preferable to > set it 0600, no ? Agree. _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers