[lxc-devel] [PATCH] Add support for checkpoint and restore via CRIU

[Tycho Andersen]

On Fri, Aug 22, 2014 at 04:36:37AM +0000, Serge Hallyn wrote:
> Quoting Tycho Andersen (tycho.andersen at canonical.com):
> 
> Thanks, Tycho.  Just one remaining request below.  Other than that,
> 
> Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> 
> > +static bool lxcapi_checkpoint(struct lxc_container *c, char *directory, bool stop, bool verbose)
> > +{
> > +	int netnr, status;
> > +	struct lxc_list *it;
> > +	bool error = false;
> > +	pid_t pid;
> > +
> > +	if (!criu_ok(c))
> > +		return false;
> > +
> > +	if (mkdir(directory, 0700) < 0 && errno != EEXIST)
> > +		return false;
> > +
> > +	netnr = 0;
> > +	lxc_list_for_each(it, &c->lxc_conf->network) {
> > +		char *veth = NULL, *bridge = NULL, veth_path[PATH_MAX], eth[128];
> > +		struct lxc_netdev *n = it->elem;
> > +		int pret;
> > +
> > +		pret = snprintf(veth_path, PATH_MAX, "lxc.network.%d.veth.pair", netnr);
> > +		if (pret < 0 || pret >= PATH_MAX) {
> > +			error = true;
> > +			goto out;
> > +		}
> > +
> > +		veth = lxcapi_get_running_config_item(c, veth_path);
> > +		if (!veth) {
> > +			/* criu_ok() checks that all interfaces are
> > +			 * LXC_NET{VETH,NONE}, and VETHs should have this
> > +			 * config */
> > +			assert(n->type == LXC_NET_NONE);
> > +			break;
> > +		}
> > +
> > +		pret = snprintf(veth_path, PATH_MAX, "lxc.network.%d.link", netnr);
> > +		if (pret < 0 || pret >= PATH_MAX) {
> > +			error = true;
> > +			goto out;
> > +		}
> > +
> > +		bridge = lxcapi_get_running_config_item(c, veth_path);
> > +		if (!bridge) {
> > +			error = true;
> > +			goto out;
> > +		}
> > +
> > +		pret = snprintf(veth_path, PATH_MAX, "%s/veth%d", directory, netnr);
> > +		if (pret < 0 || pret >= PATH_MAX || print_to_file(veth_path, veth) < 0) {
> > +			error = true;
> > +			goto out;
> > +		}
> > +
> > +		pret = snprintf(veth_path, PATH_MAX, "%s/bridge%d", directory, netnr);
> > +		if (pret < 0 || pret >= PATH_MAX || print_to_file(veth_path, bridge) < 0) {
> > +			error = true;
> > +			goto out;
> > +		}
> > +
> > +		if (n->name) {
> > +			assert(strlen(n->name) <= 127);
> > +			strncpy(eth, n->name, 128);
> 
> This again is an API function so I don't want asserts there if we can help
> it.  I was thinking
> 
> 	strncpy(eth, n->name, 127);
> 
> Is there a good reason to do it this way?

Well, I guess we could also test if it is 127 or less, I was just
thinking that if it is over 127, you're going to get a non-working
container, so best to bail vs. continuing on knowing things will
break. You're right though, assert is probably the wrong thing here.
Maybe just a test and then return false?

Tycho

> > +		} else
> > +			sprintf(eth, "eth%d", netnr);
> > +
> > +		pret = snprintf(veth_path, PATH_MAX, "%s/eth%d", directory, netnr);
> > +		if (pret < 0 || pret >= PATH_MAX || print_to_file(veth_path, eth) < 0)
> > +			error = true;
> > +
> > +out:
> > +		free(veth);
> > +		free(bridge);
> > +		if (error)
> > +			return false;
> > +	}
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel