[CRIU] [PATCH 0/7] CRiu Image Tool(crit), v4
Ruslan Kuprieiev
kupruser at gmail.com
Tue Jan 13 06:19:11 PST 2015
On 01/13/2015 03:29 PM, Pavel Emelyanov wrote:
> On 01/12/2015 07:42 PM, Ruslan Kuprieiev wrote:
>> On 01/12/2015 05:06 PM, Pavel Emelyanov wrote:
>>> OK, I like what is there in the set :) Let's do a set of incremental changes
>>> and I'll apply the set. Or, if you prefer, just rework the whole set. So.
>>>
>>> 1. In order to print the contents of the file on the screen in
>>> human readable format the CLI I should use is
>>>
>>> # ./crit convert -i test/dump/static/env00/4300/1/reg-files.img
>>>
>>> This is strange, as "convert" generally means converting from one
>>> format _to_ _another_ and in this case we always convert to json and
>>> no other format.
>>>
>>> From my perspective we should either rename action name from convert to
>>> "show", "decode" or smth like this or require the target format to be
>>> explicitly specified. The former is more preferable from my perspective.
>>>
>> But we discussed previously that we should use stdin and stdout as
>> defaults. We can use "show" name to print contents to the stdout, but
>> "show" will have no sense when converting back.
>>
>> Maybe you meant that "show" should be an alias for "convert -i"?
>> If so, that sounds good.
>>
>> "Decode"/"encode" doesn't fit here, as we have a format detection
>> implemented.
>>
>> Specifying target format looks redundant, as we have only 2 formats:
>> binary and json(i'm not sure we need to convert to any other type, as
>> there are a hell lot of tools to convert json to anything =)). So we
>> can just convert input to the opposite format.
> :) Yes, I understand. But I still want to see some CLI that, well, "makes
> sense" in a certain meaning.
>
> How about the following?
>
> # crit decode [-i protobuf file, stdin by default] [-o output file, stdout by default]
>
> takes input file, checks it being the criu PB one and decodes one into json.
>
> # crit encode [-i json stream, stdin by default] [-o output file, stdount by default]
>
> takes input json stream, checking being such and encodes one back to criu PB.
Yes, after this time looking at decode/encode I agree that it makes
perfect sense.
Thanks! Will fix.
>
>> b) About payload and extra fields. I think we need them, as they
>> show the structure of criu images according to one that is
>> described in our Images wiki page:
>>
>> All criu-specific image files begin with 32-bit magic cookie. Images in
>> PB format are followed by zero or more entries of the same type (not
>> size!), each entry is preceded with 32-bit entry size value (not
>> including this 32-bit value itself). Optionally each entry may be
>> followed by extra payload which depends on the entry type.
>>
>> IOW protocol-buffers image files look like
>>
>> IMAGE_FILE ::= MAGIC { ENTRY }
>> ENTRY ::= SIZE PAYLOAD [ EXTRA ]
>> PAYLOAD ::= "message encoded in ProtocolBuffer format"
>> EXTRA ::= "arbitrary blob, depends on the PAYLOAD contents"
>>
>> MAGIC ::= "32 bit integer"
>> SIZE ::= "32 bit integer, equals the PAYLOAD length"
>>
>> So the current structure with "payload" and "extra" fields
>> makes perfect sense to me. The linear structure that
>> we've used previously in our text-format confuses the user,
>> just the way I was confused about pagemap\pipes-data image
>> structure.
> Hm... The wiki page is not a rule written in stone. Let's make the
> json representation look human-readable and fix the wiki page. Like
> this:
>
> IMAGE_FILE ::= MAGIC { SIZE ENTRY [PAYLOAD] }
> ENTRY ::= "message encoded in PB"
> PAYLOAD ::= "arbitrary blob"
>
> :)
>
> Too many braces and extra fields make things harder to understand.
As I said before, linear { SIZE ENTRY [PAYLOAD] } structure is confusing.
I like the one on wiki better, especially when describing pipes-data and
pagemap images. I think that even though there are extra fields and braces
involved, current structure looks a lot better and is self-describing.
And the crit sources look a lot better when sticking to wiki description.
I would really like to stick with wiki description.
>> c) I wrap every value in "" as it isn't that easy to convert
>> from pb to json, as theirs supported types do not match,
>> and wrapping everything into strings is just the easiest
>> way to get everything work. I'm currently thinking on
>> a way to improve that, but for now it works just fine.
> OK. Can we at least keep strings as string, not as string with string?
> I mean this:
>
> "path": "/foo/bar"
>
> instead of
>
> "path": "\"/foo/bar\""
Yes, we can, but it is a next step, i'm working on it in whole context
of impoving pb<->json convertation.
>>> 3. Let's now look at reg-files.img
>>>
>>> {
>>> "magic": "REG_FILES",
>>> "entries": [
>>> {
>>> "payload": {
>>> "fown": {
>>> "pid_type": "0",
>>> "signum": "0",
>>> "pid": "0",
>>> "uid": "0",
>>> "euid": "0"
>>> },
>>> "flags": "32770",
>>> "id": "1",
>>> "name": "\"/dev/null\"",
>>> "pos": "0"
>>> }
>>> },
>>> {
>>> "payload": {
>>> "name": "\"/\"",
>>> "fown": {
>>> "pid_type": "0",
>>> "signum": "0",
>>> "pid": "0",
>>> "uid": "0",
>>> "euid": "0"
>>> },
>>> "pos": "0",
>>> "flags": "33793",
>>> "id": "2",
>>> "size": "0"
>>> }
>>> },
>>>
>>> Other than those from inventory, the concerns here are:
>>>
>>> a) The order of fields differs from entry to entry.
>> It is normal for json =).
> Yes, but not for human eyes :)
>> I thought about it and concluded that it doesn't harm us in any way,
>> as we only care about the order in "repeated" fields and we treat
>> them properly by converting to an array, so the order is saved as
>> the order of elements in json arrays is preserved.
> How hard would it be to keep names in the order they are met in pb file?
Well, json specification says that the order shouldn't be preserved.
Same thing with dicts, as the names are hashable, used to find
items and are tossed around .
Hm... But what if we just store protobuf-text format string inside json?
It would solve all those problems with convertation and saving the names
order, but whould brake integrity check(which is still weak compared to
protobuf)...
I was also considering packing image into one protobuf message,
similar to current json, when it looks like:
message criu_image {
required string magic = 1;
repeated entry entries = 2;
}
message criu_entry {
required criu_payload payload = 1;
optional criu_extra extra = 2;
}
message criu_payload {
required string name = 1; // for example "core"
optional core_entry core = 1;
....
}
message criu_extra {
required string name = 1; // for example "pagemaps"
repeated pagemap_entry pagemaps = 2;
...
}
And it now looks like it fits better, even though it requires
a bit more code to write(actually, it can be autogenerated =) ).
This way it will be still readable, easy to parse from file
and will preserve the names order.
What do you think?
>>> b) The name of the 2nd entry is just /, but in the output it takes 7 characters to print one.
>>>
>> Yep, that is a side-effect of that simplified pb<->json convertation.
>>
>>> 4. Next the pagemap.img
>>>
>>> {
>>> "magic": "PAGEMAP",
>>> "entries": [
>>> {
>>> "payload": {
>>> "pages_id": "1"
>>> },
>>> "extra": [
>>> {
>>> "nr_pages": "1",
>>> "vaddr": "4194304"
>>> },
>>>
>>> Similar to the inventory, field "extra" is obfuscating.
>>>
>> See 1.
> Disagree. The pagemap_entry-s are not "extra". They are entries w/o extra payload.
But wiki says that " Images in PB format are followed by zero or more
entries of the _SAME TYPE_",
which makes perfect sense to me. As I mentioned before, wiki description
looks a
lot better than linear.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20150113/f933465a/attachment.html>
More information about the CRIU
mailing list