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

LoadSnapshot #395

Closed
wants to merge 6 commits into from
Closed

LoadSnapshot #395

wants to merge 6 commits into from

Conversation

kzys
Copy link
Contributor

@kzys kzys commented Mar 7, 2022

Signed-off-by: Kazuyoshi Kato katokazu@amazon.com

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Gabriel Ionescu and others added 5 commits October 22, 2021 15:34
This commit adds implementation for the LoadSnapshot API call.

Signed-off-by: Gabriel Ionescu <gbi@amazon.com>
Signed-off-by: Gabriel Ionescu <plp.github@gmail.com>
By adding functional options to Start(), we can call Start() with the
WithSnapshot parameter to load a snapshot before starting.

Signed-off-by: Gabriel Ionescu <gbi@amazon.com>
Signed-off-by: Gabriel Ionescu <plp.github@gmail.com>
Test the LoadSnapshot functionality by creating a snapshot and loading
it immediately.

Signed-off-by: Gabriel Ionescu <gbi@amazon.com>
Signed-off-by: Gabriel Ionescu <plp.github@gmail.com>
Signed-off-by: Gabriel Ionescu <gbi@amazon.com>
Signed-off-by: Gabriel Ionescu <plp.github@gmail.com>
This commit merges firecracker-microvm#348 from Gabriel Ionescu as is.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
We should not expose LoadSnapshot, since it is allowed only before boot.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
@alexellis
Copy link

Great to see this PR up @kzys. One thing I really liked about the current SDK is how it configures CNI during the initial setup of the VM, I started poking around with the unix socket and manual HTTP requests to try to load a snapshot, but this fails due to the missing network. What sorts of requirements are there on the networking tap/veth devices for a restore from a snapshot? The VMID is used or a random UID for the network namespace, is that required for rehydrating the snapshot?

Happy to do some early testing, what I'd look for is a simple script / main.go that shows:

Create VM with CNI bridge / host-local and tc-tap-redirect and stable VMID
Pauses, snapshots, kills process
A new process is started, loads the snapshot, and networking in the guest still works

@ustiugov
Copy link

hi @kzys, it is great to see progress on snapshot support in the SDK. I wonder if you have a different implementation in mind, compared to our implementation apart from the fact that we implemented it in the firecracker-containerd directly.

@alexellis
Copy link

Hi @kzys, this is what I got when trying to add the following to the existing "create VM" method I have:

	if action == "load" {
		fcCfg.Snapshot = firecracker.SnapshotConfig{
			MemFilePath:         path.Join(cwd, "mem"),
			SnapshotPath:        path.Join(cwd, "disk"),
			ResumeVM:            true,
			EnableDiffSnapshots: false,
		}
	}
INFO[0000] Called startVMM(), setting up a VMM on firecracker.sock 
INFO[0000] VMM logging disabled.                        
INFO[0000] VMM metrics disabled.                        
INFO[0000] Attaching drive out.img, slot 1, root true.  
2022-03-14T20:44:08.903778314 [anonymous-instance:fc_api:ERROR:src/api_server/src/parsed_request.rs:174] Received Error. Status code: 400 Bad Request. Message: The requested operation is not supported after starting the microVM.
ERRO[0000] Attach drive failed: out.img: [PUT /drives/{drive_id}][400] putGuestDriveByIdBadRequest  &{FaultMessage:The requested operation is not supported after starting the microVM.} 
ERRO[0000] While attaching drive out.img, got error [PUT /drives/{drive_id}][400] putGuestDriveByIdBadRequest  &{FaultMessage:The requested operation is not supported after starting the microVM.} 
WARN[0000] Failed handler "fcinit.AttachDrives": [PUT /drives/{drive_id}][400] putGuestDriveByIdBadRequest  &{FaultMessage:The requested operation is not supported after starting the microVM.} 

2022-03-14T20:44:08.909516283 [anonymous-instance:main:WARN:src/devices/src/virtio/net/event_handler.rs:62] Received unknown event: ERROR from source: 30
failed to start the vm: [PUT /drives/{drive_id}][400] putGuestDriveByIdBadRequest  &{FaultMessage:The requested operation is not supported after starting the microVM.}
alex@nuc7:~$ 2022-03-14T20:44:13.072828504 [anonymous-instance:main:ERROR:src/devices/src/virtio/net/device.rs:438] Failed to write to tap: Os { code: 77, kind: Other, message: "File descriptor in bad state" }

2022-03-14T20:44:25.133210665 [anonymous-instance:main:ERROR:src/devices/src/virtio/net/device.rs:438] Failed to write to tap: Os { code: 77, kind: Other, message: "File descriptor in bad state" }
2022-03-14T20:44:28.432145169 [anonymous-instance:main:ERROR:src/devices/src/virtio/net/device.rs:438] Failed to write to tap: Os { code: 77, kind: Other, message: "File descriptor in bad state" }

A CNI adapter is being added, the snapshot is just saved to the CWD, and when running commands via curl, a similar approach worked for me.

A more complete sample:

func bootVM(ctx context.Context, rawImage, kernel, firecrackerBinary, metricsFifo, cpuTemplate string, smt bool, action string) {

	vmmCtx, vmmCancel := context.WithCancel(ctx)
	defer vmmCancel()

	mmid := uuid.New().String()
	devices := make([]models.Drive, 1)
	devices[0] = models.Drive{
		DriveID:      firecracker.String("1"),
		PathOnHost:   &rawImage,
		IsRootDevice: firecracker.Bool(true),
		IsReadOnly:   firecracker.Bool(false),
	}

	cwd, _ := os.Getwd()
	fcCfg := firecracker.Config{

		LogLevel:        "debug",
		SocketPath:      firecrackerSock,
		KernelImagePath: kernel,
		KernelArgs:      "console=ttyS0 reboot=k panic=1 acpi=off pci=off i8042.noaux i8042.nomux i8042.nopnp i8042.dumbkbd init=/sbin/init random.trust_cpu=on",
		Drives:          devices,
		MetricsFifo:     metricsFifo,
		VMID:            mmid,
		MachineCfg: models.MachineConfiguration{
			VcpuCount:  firecracker.Int64(1),
			Smt:        firecracker.Bool(smt),
			MemSizeMib: firecracker.Int64(512),
		},
		NetworkInterfaces: []firecracker.NetworkInterface{
			{
				CNIConfiguration: &firecracker.CNIConfiguration{
					NetworkName: "mvm",
					IfName:      "eth0",
					ConfDir:     "/etc/cni/net.d",
				},
			},
		},
	}
	if action == "load" {
		fcCfg.Snapshot = firecracker.SnapshotConfig{
			MemFilePath:         path.Join(cwd, "mem"),
			SnapshotPath:        path.Join(cwd, "disk"),
			ResumeVM:            true,
			EnableDiffSnapshots: false,
		}
	}

	machineOpts := []firecracker.Opt{}

	command := firecracker.VMCommandBuilder{}.
		WithBin(firecrackerBinary).
		WithSocketPath(fcCfg.SocketPath).
		WithStdin(os.Stdin).
		WithStdout(os.Stdout).
		WithStderr(os.Stderr).
		Build(ctx)

	machineOpts = append(machineOpts, firecracker.WithProcessRunner(command))

	m, err := firecracker.NewMachine(vmmCtx, fcCfg, machineOpts...)
	if err != nil {
		fmt.Printf("failed to start the vm: %+v\n", err)
		os.Exit(1)
	}

	if err := m.Start(vmmCtx); err != nil {
		fmt.Printf("failed to start the vm: %+v\n", err)
		os.Exit(1)
	}
	defer m.StopVMM()

	log.Printf("Machine ID: %s, action: %s", mmid, action)

	if err := m.Wait(vmmCtx); err != nil {
		fmt.Printf("failed to start the vm: %+v\n", err)
		os.Exit(1)
	}

	log.Print("Machine exitied.")

}

Does this look like the test case that you ran? Any suggestions for me to try?

Alex

@alexellis
Copy link

@ustiugov the challenge with your forked code (for me) is that it doesn't cover CNI, which this SDK should/will do. That's the value I see in this SDK over using HTTP requests. From an experiment I performed using curl, it seems that the name of the tap adapter can actually change between the initial boot and loading of a snapshot.

If @kzys is able to fix up the PR for the above scenario, it'd solve this workflow, however if that takes longer than expected, I wonder if the following flow might work: use the SDK to create the VM snapshot and kill it, then use HTTP requests to a new firecracker process to load it back. The TAP will need to be created manually using netlink, unless we can unpick all the CNI code here and call it out of band, that's not going to be trivial.

@kzys
Copy link
Contributor Author

kzys commented Jul 15, 2022

Superseded by #414. We can have the snapshotting feature finally!!!

@kzys kzys closed this Jul 15, 2022
@kzys
Copy link
Contributor Author

kzys commented Jul 15, 2022

@alexellis @ustiugov Sorry for the very late reply.

Our intern @sondavidb has been working on the snapshotting feature. The tests are using CNI. So I believe that CNI is working with his version.

@ustiugov
Copy link

@kzys @sondavidb great news! Thank you for letting me know. I can see that you restore the NI with the exact same configuration.

If I may ask, What backend storage do you assume for VM snapshots? Is the host expected to retrieve the whole set of entire files or would retrieving on-demand guest memory pages be possible somehow?

@sondavidb
Copy link
Contributor

Hey there! The current implementation in the Go SDK is the first thing you said — the snapshot is assembled via the files it creates (the guest memory file & the microVM state file) and reassembled with these same files. If I understand correctly, the latter implementation is supported in v1.1, while this implementation is on the path to deprecation as of firecracker v1.1. I'm unsure where our priorities are, but at some point we will be forced to switch to the newer implementation.

@ustiugov
Copy link

@sondavidb thank you for the details. I am a little confused, could you clarify what the new implementation is going to be like and which implementation is going to be deprecated?

@sondavidb
Copy link
Contributor

sondavidb commented Aug 1, 2022

@ustiugov Sure, the differences can be found in their snapshotting documentation, via the first two API call examples under the "Loading snapshots" header. The second example uses a snapshot_path and a mem_file_path variable, which is the implementation being used in the SDK today. This is on the path to deprecation in favor of the format used in the first example, which uses the same snapshot_path but uses mem_backend as its second variable. This allows the user to have the option to run it similar to the current implementation and allow the kernel to handle page faults, or provide a UFFD that can have page faults delegated to it.

@ustiugov
Copy link

ustiugov commented Aug 3, 2022

@sondavidb thanks for the clarification! this is good news that UFFD is supported (we published the work that showcases this approach advantages last year :) ).

@maggie-lou
Copy link

@kzys Are there any plans to add support for the new mem_backend field to enable using UFFD as a backend?

@kzys
Copy link
Contributor Author

kzys commented Jun 15, 2023

Sorry. I no longer work for Amazon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants