Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed restore from checkpoint (was always failing) #65

Merged
merged 2 commits into from Aug 3, 2016

Conversation

quadrifoglio
Copy link
Contributor

if (!c->is_defined(c)) {
    fprintf(stderr, "%s is not defined\n", my_args.name);
    lxc_container_put(c);
    exit(1);
}

Based on this code from the lxc-checkpoint command, I think the container should be defined in order to process a restore from a checkpoint.

@caglar10ur
Copy link
Member

LGTM, @tych0 any objections changing this behavior?

@tych0
Copy link
Member

tych0 commented Aug 1, 2016

Sorry about the delay, I was on vacation. I think actually we can just drop the defined check entirely from go-lxc. In LXD, we don't "define" containers in the liblxc sense, since their config files never exist on disk, but only in memory. So we can't merge this as is, because it will break LXD's migration.

The check in lxc-checkpoint is basically just to print a user friendly error message, since lxc-checkpoint relies on the container being configured on disk, as that's where it tries to load the config from.

However, at the API level, there's nothing that requires a container to be "defined" in the liblxc sense, so we can just drop the check, I think.

@caglar10ur
Copy link
Member

[I remember replying this but the comment is gone, so trying again]

@quadrifoglio do you want to drop refresh this PR and drop the isDefined part?

@quadrifoglio
Copy link
Contributor Author

quadrifoglio commented Aug 3, 2016

Done, I removed the isDefined check.

@tych0
Copy link
Member

tych0 commented Aug 3, 2016

LGTM

@caglar10ur caglar10ur merged commit f8a6938 into lxc:v2 Aug 3, 2016
@caglar10ur
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants