HI Mathias, hi Lorenzo. Of course I do not expect any replies during a weekend. Added to that: Take your time. As I did, cause life has been busy.
@Lorenzo: below there is also one question for you. Feel free to answer privately to me. Or reply to me on my reply to your review. Mathias Gibbens - 13.10.24, 18:57:21 MEZ: > On Sat, 12 Oct 2024 16:59:17 +0200 Martin Steigerwald > <mar...@lichtvoll.de> wrote: […] > I would be more than happy to accept MRs/patches that enable the use > of Incus with additional inits. Please feel free to proceed. I have received a thorough review of my current approach from Lorenzo. I do have a few remarks and questions for you, Mathias: I will be using apache-2.0 license so that is compatible with your packaging. According to Lorenzo restarting Incus during upgrade appears to be destructive to running containers. So I would switch it off if that is the case. However, Lorenzo: I still wonder about it. Cause on my server containers seem to survive "sv restart incus" and "sv restart incus-user" just fine, with exactly the runit service dir setup I sent to you for review. Also I did not notice anything odd on upgrading Incus 6.0.0 to 6.0.1 to 6.0.2 on my server. All Alpine and that one Devuan based container appear to work just fine after a service restart. What do you mean by destructive? The Systemd unit depends on a incus-startup service that calls "/usr/ libexec/incus/incusd activateifneeded" at startup and "/usr/libexec/incus/ incusd shutdown". Lorenzo has been testing what these do already. The first did nothing on his system and the second was more destructive to running containers as restarting Incus service. What are these to supposed to do? So far I have just ignored them. Regarding shutdown of containers during a reboot or shutdown there is an issue: By default runit would send SIGTERM to all services, but that does not work for Incus. sv would block shutdown for 7 seconds, then timeout and running images are not stopped. I did not even notice it cause I use Incus on a hosted server and I do not look on the VM console during reboots. "incus stop --all" would do the trick. I will be testing whether overriding sending the TERM signal with "incus stop --all" and then sending the TERM signal will work nicely. If you have any additional insight on this, please share. I have forked Incus git on Salsa and set up pbuilder on my private machine. The package appears to be maintained with gbp. I still have to figure out how to have the build dependencies installed automatically on "gbp buildpackage" as I am used to with my fio package as currently the build fails with "W: Unmet build-dependency in source". But I am sure I can figure it out or install them manually. Maybe it is some pbuilder/ whatever option I have set on my work machine. Do you prefer a complete history of commits in the merge request when developing runit support or just the commits with the end result? How would you prefer commits to be split? So far I have just been editing the runit service dir on the server directly. My idea would be to incorporate the changes suggested by Lorenzo and then setup a merge request branch with the result of that. I'd add Lorenzo as a reviewer to my merge request. It has taken a long time to finally start about this. Life has been busy. So it will be ready when it will be ready. I hope to bring this into shape for Debian 13 aka Trixie, but of course you do not need to wait on it, Mathias. Have a great weekend, -- Martin
