Skip to content

Commit

Permalink
config: fix the handling of lxc.hook and hwaddrs in unexpanded config
Browse files Browse the repository at this point in the history 
And add a testcase.

The code to update hwaddrs in a clone was walking through the container
configuration and re-printing all network entries.  However network
entries from an include file which should not be printed out were being
added to the unexpanded config.  With this patch, at clone we simply
update the hwaddr in-place in the unexpanded configuration file, making
sure to make the same update to the expanded network configuration.

The code to update out lxc.hook statements had the same problem.
We also update it in-place in the unexpanded configuration, though
we mirror the logic we use when updating the expanded configuration.
(Perhaps that should be changed, to simplify future updates)

This code isn't particularly easy to review, so testcases are added
to make sure that (1) extra lxc.network entries are not added (or
removed), even if they are present in an included file, (2) lxc.hook
entries are not added, (3) hwaddr entries are updated, and (4)
the lxc.hook entries are properly updated (only when they should be).

Reported-by: Stéphane Graber <stgraber@ubuntu.com>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>
  • Loading branch information
@hallyn @stgraber
hallyn authored and stgraber committed Sep 19, 2014
1 parent de9a4bf commit 67702c2
Show file tree
Hide file tree
Showing 5 changed files with 292 additions and 123 deletions.
231 changes: 144 additions & 87 deletions src/lxc/confile.c
View file
Expand Up @@ -36,6 +36,7 @@
#include <arpa/inet.h>
#include <netinet/in.h>
#include <net/if.h>
#include <time.h>

#include "parse.h"
#include "config.h"
Expand Down Expand Up @@ -2407,16 +2408,84 @@ void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_sub
}
}

bool clone_update_unexp_hooks(struct lxc_conf *c)
bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
const char *newpath, const char *oldname, const char *newname)
{
struct lxc_list *it;
int i;

clear_unexp_config_line(c, "lxc.hook", true);
for (i=0; i<NUM_LXC_HOOKS; i++) {
lxc_list_for_each(it, &c->hooks[i]) {
if (!do_append_unexp_config_line(c, lxchook_names[i], (char *)it->elem))
const char *key = "lxc.hook";
int ret;
char *lstart = conf->unexpanded_config, *lend, *p;
size_t newdirlen = strlen(newpath) + strlen(newname) + 1;
size_t olddirlen = strlen(oldpath) + strlen(oldname) + 1;
char *olddir = alloca(olddirlen + 1);
char *newdir = alloca(newdirlen + 1);

ret = snprintf(olddir, olddirlen+1, "%s/%s", oldpath, oldname);
if (ret < 0 || ret >= olddirlen+1) {
ERROR("Bug in %s", __func__);
return false;
}
ret = snprintf(newdir, newdirlen+1, "%s/%s", newpath, newname);
if (ret < 0 || ret >= newdirlen+1) {
ERROR("Bug in %s", __func__);
return false;
}
if (!conf->unexpanded_config)
return true;
while (*lstart) {
lend = strchr(lstart, '\n');
if (!lend)
lend = lstart + strlen(lstart);
else
lend++;
if (strncmp(lstart, key, strlen(key)) != 0) {
lstart = lend;
continue;
}
p = strchr(lstart+strlen(key), '=');
if (!p) {
lstart = lend;
continue;
}
p++;
while (isblank(*p))
p++;
if (!*p)
return true;
if (strncmp(p, olddir, strlen(olddir)) != 0) {
lstart = lend;
continue;
}
/* replace the olddir with newdir */
if (olddirlen >= newdirlen) {
size_t diff = olddirlen - newdirlen;
memcpy(p, newdir, newdirlen);
if (olddirlen != newdirlen) {
memmove(lend-diff, lend, strlen(lend)+1);
lend -= diff;
conf->unexpanded_len -= diff;
}
lstart = lend;
} else {
char *new;
size_t diff = newdirlen - olddirlen;
size_t oldlen = conf->unexpanded_len;
size_t newlen = oldlen + diff;
size_t poffset = p - conf->unexpanded_config;
new = realloc(conf->unexpanded_config, newlen);
if (!new) {
ERROR("Out of memory");
return false;
}
conf->unexpanded_len = newlen;
new[newlen-1] = '\0';
lend = new + (lend - conf->unexpanded_config);
/* move over the remainder, /$hookname\n$rest */
memmove(new+poffset+newdirlen,
new+poffset+olddirlen,
oldlen-poffset-olddirlen);
conf->unexpanded_config = new;
memcpy(new+poffset, newdir, newdirlen);
lstart = lend + diff;
}
}
return true;
Expand All @@ -2429,93 +2498,81 @@ bool clone_update_unexp_hooks(struct lxc_conf *c)
} \
}

bool clone_update_unexp_network(struct lxc_conf *c)
static void new_hwaddr(char *hwaddr)
{
FILE *f;
f = fopen("/dev/urandom", "r");
if (f) {
unsigned int seed;
int ret = fread(&seed, sizeof(seed), 1, f);
if (ret != 1)
seed = time(NULL);
fclose(f);
srand(seed);
} else
srand(time(NULL));
snprintf(hwaddr, 18, "00:16:3e:%02x:%02x:%02x",
rand() % 255, rand() % 255, rand() % 255);
}

/*
* This is called only from clone.
* We wish to update all hwaddrs in the unexpanded config file. We
* can't/don't want to update any which come from lxc.includes (there
* shouldn't be any).
* We can't just walk the c->lxc-conf->network list because that includes
* netifs from the include files. So we update the ones which we find in
* the unexp config file, then find the original macaddr in the
* conf->network, and update that to the same value.
*/
bool network_new_hwaddrs(struct lxc_conf *conf)
{
struct lxc_list *it;

clear_unexp_config_line(c, "lxc.network", true);
const char *key = "lxc.network.hwaddr";
char *lstart = conf->unexpanded_config, *lend, *p, *p2;

lxc_list_for_each(it, &c->network) {
struct lxc_netdev *n = it->elem;
const char *t = lxc_net_type_to_str(n->type);
struct lxc_list *it2;
DO(do_append_unexp_config_line(c, "lxc.network.type", t ? t : "(invalid)"));
if (n->flags & IFF_UP)
DO(do_append_unexp_config_line(c, "lxc.network.flags", "up"));
if (n->link)
DO(do_append_unexp_config_line(c, "lxc.network.link", n->link));
if (n->name)
DO(do_append_unexp_config_line(c, "lxc.network.name", n->name));
if (n->type == LXC_NET_MACVLAN) {
const char *mode;
switch (n->priv.macvlan_attr.mode) {
case MACVLAN_MODE_PRIVATE: mode = "private"; break;
case MACVLAN_MODE_VEPA: mode = "vepa"; break;
case MACVLAN_MODE_BRIDGE: mode = "bridge"; break;
default: mode = "(invalid)"; break;
}
DO(do_append_unexp_config_line(c, "lxc.network.macvlan.mode", mode));
} else if (n->type == LXC_NET_VETH) {
if (n->priv.veth_attr.pair)
DO(do_append_unexp_config_line(c, "lxc.network.veth.pair", n->priv.veth_attr.pair));
} else if (n->type == LXC_NET_VLAN) {
char vid[20];
sprintf(vid, "%d", n->priv.vlan_attr.vid);
DO(do_append_unexp_config_line(c, "lxc.network.vlan.id", vid));
}
if (n->upscript)
DO(do_append_unexp_config_line(c, "lxc.network.script.up", n->upscript));
if (n->downscript)
DO(do_append_unexp_config_line(c, "lxc.network.script.down", n->downscript));
if (n->hwaddr)
DO(do_append_unexp_config_line(c, "lxc.network.hwaddr", n->hwaddr));
if (n->mtu)
DO(do_append_unexp_config_line(c, "lxc.network.mtu", n->mtu));
if (n->ipv4_gateway_auto) {
DO(do_append_unexp_config_line(c, "lxc.network.ipv4.gateway", "auto"));
} else if (n->ipv4_gateway) {
char buf[INET_ADDRSTRLEN];
inet_ntop(AF_INET, n->ipv4_gateway, buf, sizeof(buf));
DO(do_append_unexp_config_line(c, "lxc.network.ipv4.gateway", buf));
if (!conf->unexpanded_config)
return true;
while (*lstart) {
char newhwaddr[18], oldhwaddr[17];
lend = strchr(lstart, '\n');
if (!lend)
lend = lstart + strlen(lstart);
else
lend++;
if (strncmp(lstart, key, strlen(key)) != 0) {
lstart = lend;
continue;
}
lxc_list_for_each(it2, &n->ipv4) {
struct lxc_inetdev *i = it2->elem;
char buf[2*INET_ADDRSTRLEN+20], buf2[INET_ADDRSTRLEN], prefix[20];
inet_ntop(AF_INET, &i->addr, buf, INET_ADDRSTRLEN);

if (i->prefix) {
sprintf(prefix, "/%d", i->prefix);
strcat(buf, prefix);
}

if (i->bcast.s_addr != (i->addr.s_addr |
htonl(INADDR_BROADCAST >> i->prefix))) {

inet_ntop(AF_INET, &i->bcast, buf2, sizeof(buf2));
strcat(buf, " ");
strcat(buf, buf2);
}
DO(do_append_unexp_config_line(c, "lxc.network.ipv4", buf));
p = strchr(lstart+strlen(key), '=');
if (!p) {
lstart = lend;
continue;
}
if (n->ipv6_gateway_auto) {
DO(do_append_unexp_config_line(c, "lxc.network.ipv6.gateway", "auto"));
} else if (n->ipv6_gateway) {
char buf[INET6_ADDRSTRLEN];
inet_ntop(AF_INET6, n->ipv6_gateway, buf, sizeof(buf));
DO(do_append_unexp_config_line(c, "lxc.network.ipv6.gateway", buf));
p++;
while (isblank(*p))
p++;
if (!*p)
return true;
p2 = p;
while (*p2 && !isblank(*p2) && *p2 != '\n')
p2++;
if (p2-p != 17) {
WARN("Bad hwaddr entry");
lstart = lend;
continue;
}
lxc_list_for_each(it2, &n->ipv6) {
struct lxc_inet6dev *i = it2->elem;
char buf[INET6_ADDRSTRLEN + 20], prefix[20];
inet_ntop(AF_INET6, &i->addr, buf, INET6_ADDRSTRLEN);
if (i->prefix) {
sprintf(prefix, "/%d", i->prefix);
strcat(buf, prefix);
}
DO(do_append_unexp_config_line(c, "lxc.network.ipv6", buf));
memcpy(oldhwaddr, p, 17);
new_hwaddr(newhwaddr);
memcpy(p, newhwaddr, 17);
lxc_list_for_each(it, &conf->network) {
struct lxc_netdev *n = it->elem;
if (n->hwaddr && memcmp(oldhwaddr, n->hwaddr, 17) == 0)
memcpy(n->hwaddr, newhwaddr, 17);
}

lstart = lend;
}
lxc_list_for_each(it, &c->environment)
DO(do_append_unexp_config_line(c, "lxc.environment", (char *)it->elem));
return true;
}
5 changes: 3 additions & 2 deletions src/lxc/confile.h
View file
Expand Up @@ -59,6 +59,7 @@ extern bool do_append_unexp_config_line(struct lxc_conf *conf, const char *key,

/* These are used when cloning a container */
extern void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_subkeys);
extern bool clone_update_unexp_hooks(struct lxc_conf *c);
extern bool clone_update_unexp_network(struct lxc_conf *c);
extern bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
const char *newpath, const char *oldname, const char *newmame);
extern bool network_new_hwaddrs(struct lxc_conf *conf);
#endif
37 changes: 4 additions & 33 deletions src/lxc/lxccontainer.c
View file
Expand Up @@ -415,8 +415,6 @@ static bool load_config_locked(struct lxc_container *c, const char *fname)
return false;
if (lxc_config_read(fname, c->lxc_conf, false) != 0)
return false;
if (!clone_update_unexp_network(c->lxc_conf))
return false;
return true;
}

Expand Down Expand Up @@ -2434,41 +2432,15 @@ static int copyhooks(struct lxc_container *oldc, struct lxc_container *c)
}
}

if (!clone_update_unexp_hooks(c->lxc_conf)) {
if (!clone_update_unexp_hooks(c->lxc_conf, oldc->config_path,
c->config_path, oldc->name, c->name)) {
ERROR("Error saving new hooks in clone");
return -1;
}
c->save_config(c, NULL);
return 0;
}

static void new_hwaddr(char *hwaddr)
{
FILE *f;
f = fopen("/dev/urandom", "r");
if (f) {
unsigned int seed;
int ret = fread(&seed, sizeof(seed), 1, f);
if (ret != 1)
seed = time(NULL);
fclose(f);
srand(seed);
} else
srand(time(NULL));
snprintf(hwaddr, 18, "00:16:3e:%02x:%02x:%02x",
rand() % 255, rand() % 255, rand() % 255);
}

static void network_new_hwaddrs(struct lxc_container *c)
{
struct lxc_list *it;

lxc_list_for_each(it, &c->lxc_conf->network) {
struct lxc_netdev *n = it->elem;
if (n->hwaddr)
new_hwaddr(n->hwaddr);
}
}

static int copy_fstab(struct lxc_container *oldc, struct lxc_container *c)
{
Expand Down Expand Up @@ -2844,9 +2816,8 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n

// update macaddrs
if (!(flags & LXC_CLONE_KEEPMACADDR)) {
network_new_hwaddrs(c2);
if (!clone_update_unexp_network(c2->lxc_conf)) {
ERROR("Error updating network for clone");
if (!network_new_hwaddrs(c2->lxc_conf)) {
ERROR("Error updating mac addresses");
goto out;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/tests/Makefile.am
View file
Expand Up @@ -48,7 +48,7 @@ bin_PROGRAMS = lxc-test-containertests lxc-test-locktests lxc-test-startone \
lxc-test-reboot lxc-test-list lxc-test-attach lxc-test-device-add-remove \
lxc-test-apparmor

bin_SCRIPTS = lxc-test-autostart
bin_SCRIPTS = lxc-test-autostart lxc-test-cloneconfig

if DISTRO_UBUNTU
bin_SCRIPTS += \
Expand Down Expand Up @@ -76,6 +76,7 @@ EXTRA_DIST = \
lxcpath.c \
lxc-test-autostart \
lxc-test-checkpoint-restore \
lxc-test-cloneconfig \
lxc-test-ubuntu \
lxc-test-unpriv \
lxc-test-usernic \
Expand Down

0 comments on commit 67702c2

Please sign in to comment.