Page MenuHomeFreeBSD

Bhyve - Using JSON format for saving and restoring the state
Needs ReviewPublic

Authored by ionut.mihalache1506_gmail.com on Mar 14 2021, 5:37 PM.
Tags
Referenced Files
Unknown Object (File)
Jan 10 2024, 3:45 AM
Unknown Object (File)
Dec 28 2023, 1:53 AM
Unknown Object (File)
Dec 11 2023, 12:39 AM
Unknown Object (File)
Nov 20 2023, 7:34 AM
Unknown Object (File)
Nov 16 2023, 9:58 PM
Unknown Object (File)
Nov 13 2023, 3:51 AM
Unknown Object (File)
Oct 15 2023, 8:55 PM
Unknown Object (File)
Oct 14 2023, 3:58 AM

Details

Reviewers
jhb
Group Reviewers
bhyve
Summary

I made the necessary modification in order for the hypervisor to save and restore the state of the guest using two files (.ckp for RAM and .meta for the state of each device) instead of three files (.ckp for RAM, .kern for the data of each device, .meta for the necessary information to extract the data from the .kern file). The feature is useful in order to make the interpretation of the data that represents a state easier for a human, the binary data is hard to understand and debug when it is needed.

In order to store the data I used base64 encoding for the array types and I kept the actual type if it can be easy to understand(ex: int type). I used the encoding in order to keep the file type "text" and not write pure binary data(the data from the frame buffer for example).
The usual types, that are human friendly, such as int, long, etc, I just used the corresponding format(%d, %ld, etc.) in order to avoid doing more computations and make the file harder to read.

Here is an example of the file.ckp.meta file generated with the implementation:

"device_params": [
  {
    "vcpus": [
      {
        "vcpus@0": [
          {
            "x2apic_state$uint32": 0,
            "exitintinfo$uint64": "0",
            "exc_vector$int32": 0,
            "exc_errcode_valid$int32": 0,
            "exc_errcode$uint32": 0,
            "guest_xcr0$uint64": "7",
            "exitinfo$b64": "FQAAAAAAAACWXQiB/////wCwHDsBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAPYSsBAAAAAAAAAAAAwAAAAMAAAAAAAAAiYGAAwAAMcBdw2YuDx+EDwZIAAgBAAQAAAAAAAIAAAAtAAAALQAAAIADAAAAAAAAAAAAAAAAAAABAIkBAAAAAA==",
            "nextrip$uint64": "ffffffff81085d96",
            "tsc_offset$uint64": "15063e03a9f0"
          }
        ],
Test Plan

Started the guest vm with: bhyve -c 1 -m 1024M -H -A -P -s 0:0,hostbridge -s 1:0,lpc -l bootrom,/usr/local/share/uefi-firmware/BHYVE_UEFI.fd -s 2:0,virtio-net,tap0 -s 3:0,ahci-hd,main.img -l com1,stdio freebsd_guest

For testing the functionality I opened two processes at the same time on the guest vm:

  • one copy process of a 500MB file
  • one ping process

While the processes mentioned above are running:

  • I suspended the guest with the command: bhyvectl --suspend file.ckp --vm=freebsd_guestand
  • then I restored its state with the command: bhyve -c 1 -m 1024M -H -A -P -s 0:0,hostbridge -s 1:0,lpc -l bootrom,/usr/local/share/uefi-firmware/BHYVE_UEFI.fd -s 2:0,virtio-net,tap0 -s 3:0,ahci-hd,main.img -l com1,stdio -r file.ckp freebsd_guest.

After the restore command, both processes should finish successfully and the guest can be used without errors.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

lwhsu added inline comments.
usr.sbin/bhyve/kern_snapshot.c
5

typo in the year?

usr.sbin/bhyve/kern_snapshot.c
11

Usual practice is to leave this (All rights reserved) out of new files. It doesn't cause any harm but is not needed.

24

Cut and paste leftovers? (NETAPP)

41

I don't see anything in the file other than headers conditioned on WITHOUT_CAPSICUM, can these headers just be removed?

I rebased the code with a recent of the code.
I applied the feedback for kern_snapshot.c: I updated the copyright and I remove the unnecessary headers.
I tested again to make sure that the suspend/restore functionalities are working (I applied the same test plan).

What is the real purpose for this review? If debugging, I think this is not right way by adding huge amount of new code. I would suggest to remove all json things, remove all SNAPSHOT_VAR_OR_LEAVE and its variations and move basis to nvlist.

Using nvlist is more elegant and simpler, brings ability to extract (dump) all information for debugging, and gives to snapshot/resume dealing with optional parameters, removing some parameters, adding new parameters without breaking "resume" with previously created snapshots.

What is the real purpose for this review? If debugging, I think this is not right way by adding huge amount of new code. I would suggest to remove all json things, remove all SNAPSHOT_VAR_OR_LEAVE and its variations and move basis to nvlist.

Using nvlist is more elegant and simpler, brings ability to extract (dump) all information for debugging, and gives to snapshot/resume dealing with optional parameters, removing some parameters, adding new parameters without breaking "resume" with previously created snapshots.

I've been funding work in collaboration with Mihai Carabas at UPB since 2016. The goal was to attract more developers to the bhyve project and implement suspend/resume with live migration. After the initial patch landed thanks to JHB, we spoke to project leaders about what needed to be done to move the suspend/resume code out from under #ifdef so it could become a generally useful feature. Mihai and I prioritized which projects should receive scholarship funding based on that feedback and engaged students that were willing to further these goals. One of the blocking items was to improve the state file format. This patch was the result of a student project to work on that item. Sadly, there has been little to no review or feedback. As a result, these considerable efforts are wasted, students eventually get frustrated and loose interest in working on FreeBSD projects.

We had a few discussions related to what file format to use. One of the options discussed was to use a format similar to the one used for the basic VM config file. Maybe Mihai can recall more specific details. I just remember that using json was the result of a consensus. If there was a new discussion to reach a new consensus, we'd be thrilled. That would at least be progress.

What is the real purpose for this review? If debugging, I think this is not right way by adding huge amount of new code. I would suggest to remove all json things, remove all SNAPSHOT_VAR_OR_LEAVE and its variations and move basis to nvlist.

Using nvlist is more elegant and simpler, brings ability to extract (dump) all information for debugging, and gives to snapshot/resume dealing with optional parameters, removing some parameters, adding new parameters without breaking "resume" with previously created snapshots.

I've been funding work in collaboration with Mihai Carabas at UPB since 2016. The goal was to attract more developers to the bhyve project and implement suspend/resume with live migration. After the initial patch landed thanks to JHB, we spoke to project leaders about what needed to be done to move the suspend/resume code out from under #ifdef so it could become a generally useful feature. Mihai and I prioritized which projects should receive scholarship funding based on that feedback and engaged students that were willing to further these goals. One of the blocking items was to improve the state file format. This patch was the result of a student project to work on that item. Sadly, there has been little to no review or feedback. As a result, these considerable efforts are wasted, students eventually get frustrated and loose interest in working on FreeBSD projects.

We had a few discussions related to what file format to use. One of the options discussed was to use a format similar to the one used for the basic VM config file. Maybe Mihai can recall more specific details. I just remember that using json was the result of a consensus. If there was a new discussion to reach a new consensus, we'd be thrilled. That would at least be progress.

What is the real purpose for this review? If debugging, I think this is not right way by adding huge amount of new code. I would suggest to remove all json things, remove all SNAPSHOT_VAR_OR_LEAVE and its variations and move basis to nvlist.

Using nvlist is more elegant and simpler, brings ability to extract (dump) all information for debugging, and gives to snapshot/resume dealing with optional parameters, removing some parameters, adding new parameters without breaking "resume" with previously created snapshots.

I've been funding work in collaboration with Mihai Carabas at UPB since 2016. The goal was to attract more developers to the bhyve project and implement suspend/resume with live migration. After the initial patch landed thanks to JHB, we spoke to project leaders about what needed to be done to move the suspend/resume code out from under #ifdef so it could become a generally useful feature. Mihai and I prioritized which projects should receive scholarship funding based on that feedback and engaged students that were willing to further these goals. One of the blocking items was to improve the state file format. This patch was the result of a student project to work on that item. Sadly, there has been little to no review or feedback. As a result, these considerable efforts are wasted, students eventually get frustrated and loose interest in working on FreeBSD projects.

We had a few discussions related to what file format to use. One of the options discussed was to use a format similar to the one used for the basic VM config file. Maybe Mihai can recall more specific details. I just remember that using json was the result of a consensus. If there was a new discussion to reach a new consensus, we'd be thrilled. That would at least be progress.

Let me help in this. I would open several reviews with moving from json to using plain bhyve config format and using nvlist in saving/restoring devices state. It also can help for live migration.

Thank you for explaining!