Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

Fix typo in lxc.cgroup config breaking lxc >= 4.0.9 #1827

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pabloyoyoista
Copy link

Closes #1801

LXC 4.0.9 is a LTS and should not have any breaking change in behavior. I believe that the issue in #1801 is due to a typo in the original configuration. At least since version 2.0.0 (I checked the docs in lxc for the corresponding tag), lxc.group.devices has not been a valid configuration, but lxc.cgroup.devices instead. I guess there was some bug that silently ignored that invalid configuration previously to 4.0.9

@Atemu
Copy link

Atemu commented Jun 7, 2021

Have you verified this works? I tried playing around with this config flag's name too but it didn't make a difference. I was on 4.0.8 though, not 4.0.9.

@pabloyoyoista
Copy link
Author

pabloyoyoista commented Jun 7, 2021

No, I have not been able to test it. Very similar patches have been proposed in the linked issue and alpine linux, which are reported to work.

@Atemu
Copy link

Atemu commented Jun 8, 2021

Neither of those are working for me; might be my setup though at this point.

@ahmubashshir
Copy link
Contributor

ahmubashshir commented Jun 12, 2021

@pablofsf, you didn't set lxc.cgroup2.devices... alpine is setting them to empty string.

--- a/src/anbox/container/lxc_container.cpp
+++ b/src/anbox/container/lxc_container.cpp
@@ -343,8 +343,10 @@ void LxcContainer::start(const Configura
   set_config_item(lxc_config_tty_max_key, "0");
   set_config_item(lxc_config_uts_name_key, "anbox");
 
-  set_config_item("lxc.group.devices.deny", "");
-  set_config_item("lxc.group.devices.allow", "");
+  set_config_item("lxc.cgroup.devices.deny", "");
+  set_config_item("lxc.cgroup.devices.allow", "");
+  set_config_item("lxc.cgroup2.devices.deny", "");
+  set_config_item("lxc.cgroup2.devices.allow", "");
 
   // We can't move bind-mounts, so don't use /dev/lxc/
   set_config_item(lxc_config_tty_dir_key, "");

@pabloyoyoista
Copy link
Author

pabloyoyoista commented Jun 12, 2021

@ahmubashshir, the reason was that it felt like doing two things in the same commit (fix typo and add cgroupv2 support), but being such a small change should be fine, so I added it.

Just in case somebody wonders why lxc.group.devices.allow would fail and lxc.cgroup.devices.allow works even if the system does not support the legacy cgroup hierarchy. Quoting from lxc.containers.conf manual page: "Note that LXC will ignore lxc.cgroup. settings on systems that only use the unified hierarchy. Conversely, it will ignore lxc.cgroup2. options on systems that only use legacy hierachies."

@JuniorJPDJ
Copy link

anyone tested newer version if it's working?

@ahmubashshir
Copy link
Contributor

ahmubashshir commented Jun 24, 2021 via email

@JuniorJPDJ
Copy link

Well, so we are just waiting for this to get merged.. ;/

Copy link
Member

@morphis morphis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Once CI approves we can get this in.

@JuniorJPDJ
Copy link

Wohoo :D

@JuniorJPDJ
Copy link

Hey, it passed almost a week ago.

@JuniorJPDJ
Copy link

@morphis could you merge it? Patching it myself is annoying.

@JuniorJPDJ
Copy link

What the hell? Still not merged?

@Shatur Shatur mentioned this pull request Aug 2, 2021
@Fuseteam
Copy link

Fuseteam commented Aug 3, 2021

@morphis did the CI approve?

@JuniorJPDJ
Copy link

Bump? :X

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the decaying label Apr 16, 2022
@Fuseteam
Copy link

Still approved 🤔

@stale stale bot removed the decaying label Apr 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anbox Couldn't work on lxc 4.0.8
6 participants