From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id E4F8C1FF183 for ; Wed, 10 Sep 2025 16:32:52 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C7C0B25006; Wed, 10 Sep 2025 16:32:54 +0200 (CEST) Date: Wed, 10 Sep 2025 16:32:48 +0200 From: Wolfgang Bumiller To: Filip Schauer Message-ID: References: <20250908150224.155373-1-f.schauer@proxmox.com> <20250908150224.155373-6-f.schauer@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250908150224.155373-6-f.schauer@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1757514767875 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.075 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: Re: [pve-devel] [PATCH lxc v4 05/15] lxc: conf: split `lxc.environment` into `runtime` and `hooks` X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Cc: pve-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Please open a PR against upstream with this. At first glance it looks okay. Except that you seem to be using all-space indentation in some places where there should be tabs, so fix those first ;-) On Mon, Sep 08, 2025 at 05:02:08PM +0200, Filip Schauer wrote: > Introduce `lxc.environment.runtime` to set environment variables only > for the container init process and `lxc.environment.hooks` to set > environement variables only for hooks. Leave the original > `lxc.environment` unchanged. It still applies to everything. > > This will be needed by containers created from OCI images with custom > environment variables that could interfere with hooks. > > Signed-off-by: Filip Schauer > --- > Introduced in v4 > > ...c.environment-into-runtime-and-hooks.patch | 324 ++++++++++++++++++ > debian/patches/series | 1 + > 2 files changed, 325 insertions(+) > create mode 100644 debian/patches/pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch > > diff --git a/debian/patches/pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch b/debian/patches/pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch > new file mode 100644 > index 0000000..93e81f3 > --- /dev/null > +++ b/debian/patches/pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch > @@ -0,0 +1,324 @@ > +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > +From: Filip Schauer > +Date: Mon, 8 Sep 2025 11:11:31 +0200 > +Subject: [PATCH] PVE: conf: split `lxc.environment` into `runtime` and `hooks` > + > +Introduce `lxc.environment.runtime` to set environment variables only > +for the container init process and `lxc.environment.hooks` to set > +environement variables only for hooks. Leave the original > +`lxc.environment` unchanged. It still applies to everything. > + > +Signed-off-by: Filip Schauer > +--- > + src/lxc/attach.c | 6 +++- > + src/lxc/conf.c | 14 +++++---- > + src/lxc/conf.h | 15 +++++++--- > + src/lxc/confile.c | 72 +++++++++++++++++++++++++++++++++++++++++------ > + src/lxc/start.c | 12 ++++++-- > + 5 files changed, 98 insertions(+), 21 deletions(-) > + > +diff --git a/src/lxc/attach.c b/src/lxc/attach.c > +index 8f2f7a37c..f22f83f0d 100644 > +--- a/src/lxc/attach.c > ++++ b/src/lxc/attach.c > +@@ -879,7 +879,11 @@ static int lxc_attach_set_environment(struct attach_context *ctx, > + > + /* Set container environment variables.*/ > + if (ctx->container->lxc_conf) { > +- ret = lxc_set_environment(ctx->container->lxc_conf); > ++ ret = lxc_set_environment(&ctx->container->lxc_conf->environment); > ++ if (ret < 0) > ++ return -1; > ++ > ++ ret = lxc_set_environment(&ctx->container->lxc_conf->environment_runtime); > + if (ret < 0) > + return -1; > + } > +diff --git a/src/lxc/conf.c b/src/lxc/conf.c > +index 1899b2806..7533e2830 100644 > +--- a/src/lxc/conf.c > ++++ b/src/lxc/conf.c > +@@ -3209,6 +3209,8 @@ struct lxc_conf *lxc_conf_init(void) > + new->root_nsuid_map = NULL; > + new->root_nsgid_map = NULL; > + INIT_LIST_HEAD(&new->environment); > ++ INIT_LIST_HEAD(&new->environment_runtime); > ++ INIT_LIST_HEAD(&new->environment_hooks); > + INIT_LIST_HEAD(&new->limits); > + INIT_LIST_HEAD(&new->sysctls); > + INIT_LIST_HEAD(&new->procs); > +@@ -4239,18 +4241,18 @@ int lxc_clear_groups(struct lxc_conf *c) > + return 0; > + } > + > +-int lxc_clear_environment(struct lxc_conf *c) > ++int lxc_clear_environment(struct list_head *environment) > + { > + struct environment_entry *env, *nenv; > + > +- list_for_each_entry_safe(env, nenv, &c->environment, head) { > ++ list_for_each_entry_safe(env, nenv, environment, head) { > + list_del(&env->head); > + free(env->key); > + free(env->val); > + free(env); > + } > + > +- INIT_LIST_HEAD(&c->environment); > ++ INIT_LIST_HEAD(environment); > + return 0; > + } > + > +@@ -4359,7 +4361,7 @@ void lxc_conf_free(struct lxc_conf *conf) > + lxc_clear_mount_entries(conf); > + lxc_clear_idmaps(conf); > + lxc_clear_groups(conf); > +- lxc_clear_environment(conf); > ++ lxc_clear_environment(&conf->environment); > + lxc_clear_limits(conf, "lxc.prlimit"); > + lxc_clear_sysctls(conf, "lxc.sysctl"); > + lxc_clear_procs(conf, "lxc.proc"); > +@@ -5210,11 +5212,11 @@ void suggest_default_idmap(void) > + ERROR("lxc.idmap = g 0 %u %u", gid, grange); > + } > + > +-int lxc_set_environment(const struct lxc_conf *conf) > ++int lxc_set_environment(const struct list_head *environment) > + { > + struct environment_entry *env; > + > +- list_for_each_entry(env, &conf->environment, head) { > ++ list_for_each_entry(env, environment, head) { > + int ret; > + > + ret = setenv(env->key, env->val, 1); > +diff --git a/src/lxc/conf.h b/src/lxc/conf.h > +index 31cd39e3f..d16564f10 100644 > +--- a/src/lxc/conf.h > ++++ b/src/lxc/conf.h > +@@ -506,10 +506,17 @@ struct lxc_conf { > + unsigned int monitor_unshare; > + unsigned int monitor_signal_pdeath; > + > +- /* list of environment variables we'll add to the container when > +- * started */ > ++ /* list of environment variables to provide to both the container's init > ++ * process and hooks */ > + struct list_head environment; > + > ++ /* list of environment variables to provide to the container's init > ++ * process */ > ++ struct list_head environment_runtime; > ++ > ++ /* list of environment variables to provide to container hooks */ > ++ struct list_head environment_hooks; > ++ > + /* text representation of the config file */ > + char *unexpanded_config; > + size_t unexpanded_len; > +@@ -599,7 +606,7 @@ __hidden extern int lxc_clear_automounts(struct lxc_conf *c); > + __hidden extern int lxc_clear_hooks(struct lxc_conf *c, const char *key); > + __hidden extern int lxc_clear_idmaps(struct lxc_conf *c); > + __hidden extern int lxc_clear_groups(struct lxc_conf *c); > +-__hidden extern int lxc_clear_environment(struct lxc_conf *c); > ++__hidden extern int lxc_clear_environment(struct list_head *environment); > + __hidden extern int lxc_clear_limits(struct lxc_conf *c, const char *key); > + __hidden extern int lxc_delete_autodev(struct lxc_handler *handler); > + __hidden extern int lxc_clear_autodev_tmpfs_size(struct lxc_conf *c); > +@@ -710,7 +717,7 @@ static inline int lxc_personality(personality_t persona) > + return personality(persona); > + } > + > +-__hidden extern int lxc_set_environment(const struct lxc_conf *conf); > ++__hidden extern int lxc_set_environment(const struct list_head *environment); > + __hidden extern int parse_cap(const char *cap_name, __u32 *cap); > + > + #endif /* __LXC_CONF_H */ > +diff --git a/src/lxc/confile.c b/src/lxc/confile.c > +index 5045741bb..875418792 100644 > +--- a/src/lxc/confile.c > ++++ b/src/lxc/confile.c > +@@ -80,6 +80,8 @@ lxc_config_define(console_rotate); > + lxc_config_define(console_size); > + lxc_config_define(unsupported_key); > + lxc_config_define(environment); > ++lxc_config_define(environment_runtime); > ++lxc_config_define(environment_hooks); > + lxc_config_define(ephemeral); > + lxc_config_define(execute_cmd); > + lxc_config_define(group); > +@@ -211,6 +213,8 @@ static struct lxc_config_t config_jump_table[] = { > + { "lxc.console.rotate", true, set_config_console_rotate, get_config_console_rotate, clr_config_console_rotate, }, > + { "lxc.console.size", true, set_config_console_size, get_config_console_size, clr_config_console_size, }, > + { "lxc.sched.core", true, set_config_sched_core, get_config_sched_core, clr_config_sched_core, }, > ++ { "lxc.environment.runtime", true, set_config_environment_runtime, get_config_environment_runtime, clr_config_environment_runtime }, > ++ { "lxc.environment.hooks", true, set_config_environment_hooks, get_config_environment_hooks, clr_config_environment_hooks }, > + { "lxc.environment", true, set_config_environment, get_config_environment, clr_config_environment, }, > + { "lxc.ephemeral", true, set_config_ephemeral, get_config_ephemeral, clr_config_ephemeral, }, > + { "lxc.execute.cmd", true, set_config_execute_cmd, get_config_execute_cmd, clr_config_execute_cmd, }, > +@@ -1574,15 +1578,15 @@ static int set_config_group(const char *key, const char *value, > + return 0; > + } > + > +-static int set_config_environment(const char *key, const char *value, > +- struct lxc_conf *lxc_conf, void *data) > ++static int set_config_environment_impl(const char *value, > ++ struct list_head *environment) > + { > + __do_free char *dup = NULL, *val = NULL; > + __do_free struct environment_entry *new_env = NULL; > + char *env_val; > + > + if (lxc_config_value_empty(value)) > +- return lxc_clear_environment(lxc_conf); > ++ return lxc_clear_environment(environment); > + > + new_env = zalloc(sizeof(struct environment_entry)); > + if (!new_env) > +@@ -1609,12 +1613,30 @@ static int set_config_environment(const char *key, const char *value, > + new_env->key = move_ptr(dup); > + new_env->val = move_ptr(val); > + > +- list_add_tail(&new_env->head, &lxc_conf->environment); > ++ list_add_tail(&new_env->head, environment); > + move_ptr(new_env); > + > + return 0; > + } > + > ++static int set_config_environment(const char *key, const char *value, > ++ struct lxc_conf *lxc_conf, void *data) > ++{ > ++ return set_config_environment_impl(value, &lxc_conf->environment); > ++} > ++ > ++static int set_config_environment_runtime(const char *key, const char* value, > ++ struct lxc_conf *lxc_conf, void *data) > ++{ > ++ return set_config_environment_impl(value, &lxc_conf->environment_runtime); > ++} > ++ > ++static int set_config_environment_hooks(const char *key, const char* value, > ++ struct lxc_conf *lxc_conf, void *data) > ++{ > ++ return set_config_environment_impl(value, &lxc_conf->environment_hooks); > ++} > ++ > + static int set_config_tty_max(const char *key, const char *value, > + struct lxc_conf *lxc_conf, void *data) > + { > +@@ -4473,8 +4495,8 @@ static int get_config_group(const char *key, char *retv, int inlen, > + return fulllen; > + } > + > +-static int get_config_environment(const char *key, char *retv, int inlen, > +- struct lxc_conf *c, void *data) > ++static int get_config_environment_impl(char *retv, int inlen, > ++ struct list_head *environment) > + { > + int len, fulllen = 0; > + struct environment_entry *env; > +@@ -4484,13 +4506,32 @@ static int get_config_environment(const char *key, char *retv, int inlen, > + else > + memset(retv, 0, inlen); > + > +- list_for_each_entry(env, &c->environment, head) { > ++ list_for_each_entry(env, environment, head) { > + strprint(retv, inlen, "%s=%s\n", env->key, env->val); > + } > + > + return fulllen; > + } > + > ++static int get_config_environment(const char *key, char *retv, int inlen, > ++ struct lxc_conf *c, void *data) > ++{ > ++ return get_config_environment_impl(retv, inlen, &c->environment); > ++} > ++ > ++static int get_config_environment_runtime(const char *key, char *retv, > ++ int inlen, struct lxc_conf *c, > ++ void *data) > ++{ > ++ return get_config_environment_impl(retv, inlen, &c->environment_runtime); > ++} > ++ > ++static int get_config_environment_hooks(const char *key, char *retv, int inlen, > ++ struct lxc_conf *c, void *data) > ++{ > ++ return get_config_environment_impl(retv, inlen, &c->environment_hooks); > ++} > ++ > + static int get_config_execute_cmd(const char *key, char *retv, int inlen, > + struct lxc_conf *c, void *data) > + { > +@@ -5211,7 +5252,19 @@ static inline int clr_config_group(const char *key, struct lxc_conf *c, > + static inline int clr_config_environment(const char *key, struct lxc_conf *c, > + void *data) > + { > +- return lxc_clear_environment(c); > ++ return lxc_clear_environment(&c->environment); > ++} > ++ > ++static inline int clr_config_environment_runtime(const char *key, > ++ struct lxc_conf *c, void *data) > ++{ > ++ return lxc_clear_environment(&c->environment_runtime); > ++} > ++ > ++static inline int clr_config_environment_hooks(const char *key, > ++ struct lxc_conf *c, void *data) > ++{ > ++ return lxc_clear_environment(&c->environment_hooks); > + } > + > + static inline int clr_config_execute_cmd(const char *key, struct lxc_conf *c, > +@@ -6607,6 +6660,9 @@ int lxc_list_subkeys(struct lxc_conf *conf, const char *key, char *retv, > + } else if (strequal(key, "lxc.console")) { > + strprint(retv, inlen, "logfile\n"); > + strprint(retv, inlen, "path\n"); > ++ } else if (strequal(key, "lxc.environment")) { > ++ strprint(retv, inlen, "runtime\n"); > ++ strprint(retv, inlen, "hooks\n"); > + } else if (strequal(key, "lxc.seccomp")) { > + strprint(retv, inlen, "profile\n"); > + } else if (strequal(key, "lxc.signal")) { > +diff --git a/src/lxc/start.c b/src/lxc/start.c > +index 3160459b4..fc6ca147a 100644 > +--- a/src/lxc/start.c > ++++ b/src/lxc/start.c > +@@ -1260,10 +1260,14 @@ static int do_start(void *data) > + * to allow them to be used by the various hooks, such as the start > + * hook below. > + */ > +- ret = lxc_set_environment(handler->conf); > ++ ret = lxc_set_environment(&handler->conf->environment); > + if (ret < 0) > + goto out_warn_father; > + > ++ ret = lxc_set_environment(&handler->conf->environment_hooks); > ++ if (ret < 0) > ++ goto out_warn_father; > ++ > + if (!lxc_sync_wait_parent(handler, START_SYNC_POST_CONFIGURE)) > + goto out_warn_father; > + > +@@ -1361,10 +1365,14 @@ static int do_start(void *data) > + if (ret < 0) > + SYSERROR("Failed to clear environment."); > + > +- ret = lxc_set_environment(handler->conf); > ++ ret = lxc_set_environment(&handler->conf->environment); > + if (ret < 0) > + goto out_warn_father; > + > ++ ret = lxc_set_environment(&handler->conf->environment_runtime); > ++ if (ret < 0) > ++ goto out_warn_father; > ++ > + ret = putenv("container=lxc"); > + if (ret < 0) { > + SYSERROR("Failed to set environment variable: container=lxc"); > +-- > +2.47.2 > + > diff --git a/debian/patches/series b/debian/patches/series > index 5f3f0b6..b1353eb 100644 > --- a/debian/patches/series > +++ b/debian/patches/series > @@ -2,3 +2,4 @@ apparmor/0001-apparmor-allow-lxc-start-to-create-user-namespaces.patch > apparmor/0002-apparmor-use-abi-directive-in-apparmor-profiles.patch > pve/0001-PVE-Config-deny-rw-mounting-of-sys-and-proc.patch > pve/0002-PVE-Config-attach-always-use-getent.patch > +pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch > -- > 2.47.2 > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel