Remove acl-restore.sh, and add user to kvm group instead#2580
Conversation
GoogleCuttlefishTesterBot
removed
the
kokoro:force-run
GoogleCuttlefishTesterBot
removed
the
kokoro:force-run
GoogleCuttlefishTesterBot
removed
the
kokoro:force-run
0405ysj
force-pushed
the
setfacl
branch
6 times, most recently
from
593edcf to
79d27c1
Compare
| for unique_group in "${unique_groups[@]}"; do | ||
| sudo usermod -aG "$unique_group" "$username" | ||
| done | ||
| echo "Warning: Please reboot the machine (or log out and log back in) before using podcvd." |
There was a problem hiding this comment.
The whole point of having the restore acls service is that logging out and back in is not necessary. Also, the groups for the device nodes can change, in fact cuttlefish-base changes them, so depending on those can be risky.
There was a problem hiding this comment.
I believe the change around cuttlefish-base is #2589, but it's fine as it doesn't introduce any new group in overall.(Needed kvm and cvdnetwork before, but will need kvm only) Though I agree that groups for device nodes could be changed, I don't think doing chown on device nodes is a common behavior.
The most critical problem I faced at was using setfacl could be problematic for some machines. Machine details are described at b/514558958. While creating Cuttlefish instances, udev change action on /dev/kvm is triggered, and it flushes existing ACL but not recover it.
I think we can make a conversation around acl-restore.sh later but resolving the critical problem is urgent, because it's highly coupled with deployment of its skill. So I'd separate this PR into two, one for doing sudo usermod -aG "$unique_group" "$username" (#2590) and the other for removing acl-restore.sh. I'm targeting to merge former one within today, but please feel free to leave comments afterwards.
0405ysj
force-pushed
the
setfacl
branch
3 times, most recently
from
f4f62a1 to
e2b36d1
Compare
0405ysj
force-pushed
the
setfacl
branch
2 times, most recently
from
55b823f to
a27f06c
Compare
GoogleCuttlefishTesterBot
removed
the
kokoro:force-run
| echo "Configuring device permissions..." | ||
| sudo setfacl -m "u:$username:rw" /dev/kvm | ||
| sudo setfacl -m "u:$username:rw" /dev/vhost-net | ||
| sudo setfacl -m "u:$username:rw" /dev/vhost-vsock |
There was a problem hiding this comment.
I understand adding the user to the kvm group eliminates the need for restore-acls. But why detele these? without them the user will have to create a new session.
There was a problem hiding this comment.
Because creating a new session may be necessary though setting ACL via setfacl. Those settings could be flushed by triggering udev change action while creating Cuttlefish instance.
There was a problem hiding this comment.
triggering udev change action while creating Cuttlefish instance
I am missing something here. Creating a cuttlefish instance is done without root privileges, how can that trigger changes in device nodes?
There was a problem hiding this comment.
- While creating cuttlefish instance, it triggers udev change action. I don't know the exact mechanism how udev change action is triggered, but I'm speculating that an event is triggered while interacting with those devices.
- From udev rule for
/dev/kvm,uaccessis set. When it's set, it flushes current ACL setting and sets ACL dynamically depending on 'seat'. Physical workstations were okay, but it mattered on virtual environments.
0405ysj
force-pushed
the
setfacl
branch
4 times, most recently
from
007e7d2 to
4342f95
Compare
GoogleCuttlefishTesterBot
removed
the
kokoro:force-run
3 participants
Context: b/514558958