On 6/7/23 13:25, Vitaliy Gusev wrote:
Hi Corvin,
On 6 Jun 2023, at 15:59, Corvin Köhne <[email protected]> wrote:
...
We may have different version of the format from the same produce.
IMHO, it makes sense to have a dedicated IDENT and VERSION field to
easily figure out
1) if the producer of the image is known
2) if we support that version of the producer
Even if you allocated a huge amount of free space, someone would need
more. So, what do you think about this format:
+---------------------------------------------------------------------+
| IDENT - 16 BYTES |
+-------------------+-----------------------+-------------------------+
| VERSION - 4 BYTES | NVLIST SIZE - 4 BYTES | NVLIST OFFSET - 8 BYTES |
+-------------------+-----------------------+-------------------------+
| POSSIBLE FREE SPACE (e.g. for custom data, alignment etc.) |
+---------------------------------------------------------------------+
| NVLIST DATA |
+---------------------------------------------------------------------+
| POSSIBLE FREE SPACE (for whatever reason) |
+---------------------------------------------------------------------+
| SNAPSHOT DATA |
+——————————————————————————————————+
I agree. The only values that need to exist in the initial header would
be the identity and version. This could be as simple as "BHYVE" which is
similar to how common network protocols identify themselves ...
SSH-protoversion-softwareversion SP comments CR LF
Note the use of a carriage return + line feed to signify the identity +
version string. It avoids the need to reserve a specific number of bytes
for this purpose and offers more free form expression of values. If you
want to include a stream subtype, you could add another field resulting
in something like ...
IDENT-SUBTYPE-VERSION CR LF
I think it would also be helpful to borrow the concept of using generic
data stream section headers. Instead of expecting a fixed number bytes
after the identity + version, you can append section headers that define
the type and the length of each. For example ...
+-----------------------------------+
| bhyve-checkpoint-1.0\n\r |
+-----------------------------------+
| TYPE - 2 BYTES | LENGTH - 8 BYTES |
+-----------------------------------+
| DATA ... |
+-----------------------------------+
| TYPE - 2 BYTES | LENGTH - 8 BYTES |
+-----------------------------------+
| DATA ... |
+-----------------------------------+
Example section types:
0x0001 = NVLIST A
0x0002 = NVLIST B
0x0003 = MEMORY
0x0004 = ...
0xFFFF = CHECKSUM
Any number of sections could be included in a single file. Sections can
be added or without breaking the stream format. Required sections for
the subtype could be accounted for and simple integrity checks could be
performed based on section length. Checksum validation can be performed
by only parsing the trailing checksum section and the rest as raw input.
The same basic structure could also be used in a network stream so there
is potential for code reuse.
Note, simple string "BHYVE CHECKPOINT IMAGE” has 22 bytes. So 16 bytes seems
too small.
So I would not to complicate a header first.
I would rather describe ideas, conditions and then solutions:
1. Need to distinguish snapshot image file from other files.
_Solution_: Header should have "magic id”.
> 2. Need a barrier for resuming if image "is not ours”. Idea is not to
allow to resume images from other producers.
The reason to have it and use it instead of header versioning:
Imagine that mainstream has its own implementation and company’s
fork repo has its own implementation. How to *ensure* that the
*versions* in an image file are ours and not somebody’s else?
_Solution_: Header should have “Producer id” string.
Example: Snapshot image file has empty Producer string , but bhyve
has current Producer as “MYCOMPANY”. Strings are not equal, resume
must fail.
It would appear that you'd like the provider to be a string. I'm not
sure why this couldn't be added by the provider as a value in an nvlist
section. The consumer can filter on that value. Why should it be part of
the stream format?
3. The Rule above does not restrict getting/decoding data from an image
file. It should be possible to look at an image file and analyse
internals, to get/decode values, etc.
_Solution_: Have additional option either in bhyve or bhyvectl to
get into image file.
See my final comments regarding encoding.
4. Following nvlist header data should have a short information about
image file and its internals.
_Solution_: NV HEADER can have several sections: “config”, “kernel”,
“devices”, “memory”, …
See my final comments regarding encoding.
5. Versioning of NV HEADER. Idea is to have an information in
advance whether it is possible to be resumed or not. In other words,
before do resume, get information about ability to resume.
_Solution_: Each Section should have “version” and “subversion”.
While “version” is responsible for both types of compatibility:
backward and forward, “subversion” is for forward compatibility only.
_Rules for check_:
If bh_version == version && bh_subversion >= subversion then
Bhyve able to resume the Section
Else
Bhyve cannot resume the Section
Endif
_
Example 1_: Section in image has “version=1", “subversion=5”, Bhyve
has “version=1", “subversion=6". That means, bhyve can resume the
Section.
_Example 2_: The same image Section, but bhyve has “version=1",
“subversion=4". Bhyve cannot resume the Section.
Example 3: The same image Section, but bhyve has “version=2",
“subversion=5”. Bhyve cannot resume the Section.
_Rules for increasing versions_:
- If during code-change “backward” compatibility is broken,
“version” should be increased and “subversion” is set to 0.
- If during code-change “forward” compatibility is broken,
“subversion” should be increased.
More version values could be included I suppose, but it seems overly
complicated to me. I see most of the change over time being in nvlist
value sections. There's a lot of flexibility and validation that can
occur there before pushing version numbers down into individual stream
sections, but I guess I could be convinced otherwise.
In any case, we need to be careful to not rely too much on version
checks. Otherwise it will be difficult to snapshot/restore/migrate
between different versions of bhyve.
6. Other versioning in HEADER is redundant. If something is changed in
the format, “magic id” can be changed appropriately.
_Solution_: “magic id” should be stable and not changed for a long time.
I think that's what the major version would be used for.
As result I would suggest to give at least 32 bytes for "magic id” /
ident and 32 bytes for “producer id”.
> Format of entire image file can be:
+-----------------------------------------------------------------+
| HEADER MAGIC ID - 32 BYTES |
+-----------------------------------------------------------------+
| HEADER PRODUCER ID - 32 BYTES |
+———————————————----------------————————--—-----------------------+
| NVLIST HEADER SIZE - 4 BYTES |
+-----------------------------------------------------------------+
| NVLIST HEADER DATA (SECTIONS) |
+-----------------------------------------------------------------+
| SNAPSHOT DATA |
+-----------------------------------------------------------------+
_MAGIC ID_: should be hardcoded: "BHYVE CHECKPOINT IMAGE”.
_PRODUCER ID_: can be empty and supported by producer, i.e. reserved.
_
_
_NVLIST HEADER SIZE_: has enough dimension, but in general size is less
than 4KB
_NVLIST HEADER DATA_: Packed nvlist data, contains Sections: “config”,
“kernel”, “devices”, “memory”, … :
[config]
offset = 0x1000 (4096)
size = 0x1f6 (502)
type = text
vers = 1
subvers = 5
[kernel]
offset = 0x11f6 (4598)
size = 0x19a7 (6567)
type = nvlist
vers = 1
subvers = 0
[devices]
offset = 0x2b9d (11165)
size = 0x10145ba (16860602)
type = nvlist
vers = 2
subvers = 1
[memory]
offset = 0x1200000 (18874368)
size = 0x3ce00000 (1021313024)
type = pages
vers = 1
subvers = 0
I see a need to define a format for bhyve so it's possible to mix
different sections and encodings inside a unified stream. But all the
data in your nvlist example above can be easily be represented as text.
We already have JSON, YAML, XML, etc ... By adopting an preexisting
format, we could retain the snapshot structure instead of lifting it up
into the stream format. Even if we decide to break the structure up into
different nvlist stream sections, using a common format would allow
other tools to more easily parse and validate the structure inside these
sections. Isn't that a good thing? Is there a reason we're trying to
reinvent the wheel here?
Thanks,
-Matthew
