Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add json serialization/deserialization #46

Merged
merged 15 commits into from
Jun 26, 2023
Merged

Add json serialization/deserialization #46

merged 15 commits into from
Jun 26, 2023

Conversation

cfergeau
Copy link
Collaborator

This allows other projects to write a configuration file based on virtual
machine.

This is based on #43
These 2 commits could be merged in the initial one

   json: Fix segfault when json does not have a 'kind' field
   json: Fix 'MashallJSON()' typos

This improves on #43 by adding some unit tests, fixing a few bugs found by the
unit tests, and tightening error handling during unmarshalling.
I've also simplified the code/refactored it a bit.

baude and others added 5 commits June 23, 2023 12:14
Add custom JSON methods for Marshal and Unmarshal so other projects can
write a configuration file based on virtual machine.  the key issue is
the use of interfaces, for example devices, in virtual machine.  When
using JSON to marshal and unmarshal, we need to tag what kind of device
it is so the process can tag and ultimately re-identify the proper
device.

Signed-off-by: Brent Baude <[email protected]>
Signed-off-by: Christophe Fergeau <[email protected]>
This field is required, and when deserializing devices/bootloader, the
code assumes it's always present.

Signed-off-by: Christophe Fergeau <[email protected]>
No need to add 'timesync: null' to the JSON data.

Signed-off-by: Christophe Fergeau <[email protected]>
The deserialization code silently ignores json devices/bootloaders with
unknown (invalid) 'kind' fields, this commit returns an error instead.

Signed-off-by: Christophe Fergeau <[email protected]>
@cfergeau
Copy link
Collaborator Author

More json annotations to the struct used by vfkit could be useful, as now the json is not always looking great with respect to capitalization.

@cfergeau cfergeau force-pushed the json branch 2 times, most recently from ca3f49e to f8fb15a Compare June 23, 2023 15:30
cfergeau added 10 commits June 23, 2023 17:31
Signed-off-by: Christophe Fergeau <[email protected]>
This removes some code from VirtualMachine.UnmarshalJSON

Signed-off-by: Christophe Fergeau <[email protected]>
This removes some code from VirtualMachine.UnmarshalJSON

Signed-off-by: Christophe Fergeau <[email protected]>
This removes some code from VirtualMachine.UnmarshalJSON

Signed-off-by: Christophe Fergeau <[email protected]>
This will be useful in the next commits to simplify the unmarshalling
code.

Signed-off-by: Christophe Fergeau <[email protected]>
Unmarshalling a device from json is done in 2 steps:
1) unmarshal the device 'kind' to know the type to use
2) unmarshal the device

1) is currently done by parsing the json to a map of fields, and getting
'kind' from this map
2) is done by regenerating a json string from the previous map after
removing the 'kind' entry

1) can be done more directly by parsing the json data to the jsonKind
struct which was added before.
2) can also be simplified: golang json unmarshalling code will ignore
unknown fields, so we don't need to remove 'kind' and can directly
reuse the initial json data.

Signed-off-by: Christophe Fergeau <[email protected]>
This commit is doing the same simplification as was done in the previous
commit.

Signed-off-by: Christophe Fergeau <[email protected]>
Signed-off-by: Christophe Fergeau <[email protected]>
It was missing from the implementation.

Signed-off-by: Christophe Fergeau <[email protected]>
The goal is to return an error as soon as a parsing error occurs.
The current code would sometimes ignore parsing errors and try to parse
as much as it can, which would give a partial VM representation, which
is not desirable.

Signed-off-by: Christophe Fergeau <[email protected]>
@baude
Copy link
Collaborator

baude commented Jun 26, 2023

/approve]

@baude
Copy link
Collaborator

baude commented Jun 26, 2023

/approve

@baude baude merged commit 66d1e8c into crc-org:main Jun 26, 2023
@openshift-ci openshift-ci bot added the lgtm label Jun 26, 2023
@openshift-ci
Copy link

openshift-ci bot commented Jun 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cfergeau cfergeau deleted the json branch July 26, 2023 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants