[CRIU] [PATCH 0/7] CRiu Image Tool(crit), v4

[Pavel Emelyanov]

>> Is it technically possible to make the json text look like this:
>>
>>   {
>>        "magic": "REG_FILES",
>>        "entries": [
>>            {
>>                "foo": "1",
>>                "bar": "BAR",
>>                "extra": {
>>                      /* entry-specific stuff here */
>>                }
>>            },
>>
>> ?
>>
>> And how bad would the code look like for this?
> 
> You mean like add "extra" field to the payload, right?
> Yes, it does look better.

Not exactly. I mean no "payload" keyword in the output, just
the "extra" one for those with extra data (pipe-data, IPC).

>>>>> 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.
>> This step should be accomplished till v1.5 (beginning of March). Can we?
> 
> Yes, definitely.
> But I'll try to fix it asap.

No hurry :) This is why I asked to cleanup the stuff incrementally
AFTER which I will apply all the set.

>>> Hm... But what if we just store protobuf-text format string inside json?
>> I don't understand this. How would the text look like?
> 
> Here is how reg-files.img would look like:
> 
> 
> {
>       "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"
>           },
> 
> 
> So the payload is a string that contains pb entry in protobuf text format.

Ah, I see. No, this is not good. Let's choose only ONE format and
print everything in it. Be it PB-test, JSON or even YAML doesn't
matter. But the whole output should be in one format.

>>> 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;
>>>      ...
>>> }
>> PB docs say, that the format is good for encoding "small" messages (without
>> specification of how small the small is). According to this, keeping all
>> that CRIU generates in one entry (with sub-entries) is not good idea.
> 
> Yes, but we are using it just to print the images. I mean, we will read them
> from *.img, pack in criu_image structure and print using pb text format.
> And our images don't usually contain large amounts of data.

Ah I see. Well, the temporary internal representation of data being
de-/encoded is not really important, at least at this point.

>>>>>> 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_",
>> Wiki is wrong here :) The EXTRA is stated to be "arbitrary blob", but it's
>> not such in this case.
> 
> Why? It fits perfectly if we state that pagemap_entry's are the extra 
> for pagemap_head.

Well, _if_ -- yes. But these are individual entries. So it's more
correct just to treat this file of the image containing objects of
different types. Which is kinda unique to how we planned to do images,
but somehow unavoidable.

>>> which makes perfect sense to me. As I mentioned before, wiki description looks a
>>> lot better than linear.
>> The images we generate are not nice, yes. We even have a page describing how
>> exactly they are not nice. But this is what we have, and starting developing
>> new format is good task, but too early. Let's try to make crit decode produce
>> as nice-looking output as can possibly be with the current images set.
> 
> Those messages (criu_image, criu_payload etc) are not for changing our 
> images =).
> They are just to contain and treat images within crit, just to keep them 
> sorted and
> nice-looking and be able to just parse human readable image using standart
> ParseFromString protobuf method=).
> 
>> Thanks,
>> Pavel
>>
> 
> .
>