add block size to unikernel_info, also add the possibility to change arguments in restart #195
Conversation
|
I added the needed backwards-compatibility code. This is ready for review. So, old clients can talk to new servers. There's no attempt to make new clients talk to old servers (they'd need to send old commands, so figure out which version the server has before sending the initial command, or re-connect on failure and try with an older version). |
| type arguments = { | ||
| fail_behaviour : fail_behaviour; | ||
| cpuid : int ; | ||
| memory : int ; | ||
| block_devices : (string * string option * int option) list ; | ||
| bridges : (string * string option * Macaddr.t option) list ; | ||
| argv : string list option ; | ||
| } |
There was a problem hiding this comment.
that's the new record for restart with arguments.
| type block_info = { | ||
| unikernel_device : string ; | ||
| host_device : string ; | ||
| sector_size : int ; | ||
| size : int ; | ||
| } | ||
|
|
||
| type net_info = { | ||
| unikernel_device : string ; | ||
| host_device : string ; | ||
| mac : Macaddr.t ; | ||
| } | ||
|
|
||
| type info = { | ||
| typ : typ ; | ||
| fail_behaviour : fail_behaviour; | ||
| cpuid : int ; | ||
| memory : int ; | ||
| block_devices : (string * string option * int option) list ; | ||
| bridges : (string * string option * Macaddr.t option) list ; | ||
| block_devices : block_info list ; | ||
| bridges : net_info list ; | ||
| argv : string list option ; | ||
| digest : string ; | ||
| started : Ptime.t ; | ||
| } |
There was a problem hiding this comment.
I solved some confusion (hopefully) by moving to records for block and network devices -- they also no longer have any optional fields, but are completely filled.
|
Maybe to elaborate a bit: we're not concerned about API versioning / compatibility here. We just upgrade the API as we want. ;) What we are concerned are the clients and servers that are deployed by different parties (i.e. not under our control), so esp. an old client should not be punished by our server. In this specific PR, the old client will get reasonable (the same, expected) answers from the new server. This means that in our serialisation (vmm_asn.ml), we do some extra work to keep track of the old wire formats. In addition the For the restart, it is slightly simpler, since the old command did result in a generic reply - not a specific one (i.e. a success or an error only) -- here we can just translate the old wire format to I guess reading the diff is sometimes confusing (as is writing the code) - and we should add tests for the different versions. But I've been too lazy for writing these tests. |
|
rebased on main, @reynir would you mind to look over this PR? |
|
I will merge #207 then rebase so we can have CI pass. |
this requires some additional work to re-introduce backward compatibility
TODO: this needs a bit more thought in respect to backwards compatibility
A new client talking to an old server will still not be understood
We want to fail early without killing the running instance rather than kill and then fail with an error without actually restarting.
As a dummy.
Co-authored-by: Reynir Björnsson <[email protected]>
CHANGES: * albatross-daemon: on startup backup the state file (robur-coop/albatross#206 @hannesm, fixes robur-coop/albatross#203) * Add block size to info command, adapt types slightly (robur-coop/albatross#195 @hannesm) * Allow the arguments in restart to be changed (robur-coop/albatross#195 @hannesm) * Update to ocaml-solo5-elftool 0.4.0 - bump OCaml lower bound to 4.14 (robur-coop/albatross#207 @reynir, robur-coop/albatross#208) * BUGFIX: restart command kills the unikernel first (robur-coop/albatross#205 @reynir) * Print manifest on mismatch (robur-coop/albatross#196 @reynir) * Albatross-console: use physical equality for file descriptors (robur-coop/albatross#200 @reynir) * Albatross-console: allow multiple subscribers (robur-coop/albatross#194 @reynir)
//cc @PizieDust @reynir
There's still more work needed to retain backwards compatibility.
The main question I have is about the
albatross-client.. Now therestartsubcommand can replace the arguments, but this is guarded by an additional flag "--replace-arguments". I came there since just checking whether the arguments are provided or not does not suffice (due to default values, and also maybe a user wants to restart their "hello world" this time with providing no argv at all. -- this wouldn't easily be possible without such a flag. Still it makes me wonder whether we want a separate (sub)command for it -- but then I struggle with naming. Any ideas?Also, the restart-with-arguments-replaced can't check the manifest on the client side (since there's no image available)... I'm hesitating on what to do here -- I'd like to avoid someone to do a restart and end up with "sorry, your arguments didn't specify the required devices". Any ideas?