mirage / mirage-qubes

Mirage support for writing QubesOS AppVM unikernels
BSD 2-Clause "Simplified" License
63 stars 11 forks source link

Addition of common code from unikernels? #70

Closed palainp closed 1 month ago

palainp commented 3 months ago

Dear developpers,

Thinking about the code that can be factored between qubes-mirage-firewall and qubes-miragevpn (see https://github.com/robur-coop/qubes-miragevpn/issues/9), maybe the following can be written here:

Something in lib/misc.ml like that?:

let check_memory () =
  let fraction_free stats =
    let { Xen_os.Memory.free_words; heap_words; _ } = stats in
    float free_words /. float heap_words
  in
  let stats = Xen_os.Memory.stat () in
  if fraction_free stats > 0.4 then `Ok
  else (
    Gc.full_major ();
    let stats = Xen_os.Memory.quick_stat () in
    if fraction_free stats < 0.4 then `Memory_critical
    else `Ok
  )

let shutdown =
  let* value = Xen_os.Lifecycle.await_shutdown_request () in
  match value with `Poweroff | `Reboot -> Lwt.return_unit in

Now, when I look into the previous unikernels, they wait for:

Defining these is probably more complex, and I'm not confident enough to make a proposal. To me, they should permit to define the callback function that have to handle the incoming packets (and activate or not the check_memory test). I'm unsure how to deal with the netvm update function so far. It really adds complexity to the code.

Thank you for any advice :)

hannesm commented 3 months ago

I've not looked to deep, but that sounds like a good plan (to have some sort of Common or Misc (or directly Mirage_qubes.shutdown etc.). I think that starting with the "simple" common functionality is the path forward, we can always add more functionality once the (desired) API is clear :)

About "Ocaml5+ wants us to trigger the collections manually", all I know is that compactions need to be done manually.

In respect to memory usage, the no-cstruct work should be much nicer for the GC and our hope is we have to do less manual calling Gc.full_major ().

palainp commented 1 month ago

This has been done in #71 :)