Daniel Lezcano [[email protected]] 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
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel
