containers / virtcontainers

A Go package for building hardware virtualized container runtimes
Apache License 2.0
139 stars 43 forks source link

qemu: Add support for ARM #614

Closed devimc closed 6 years ago

devimc commented 6 years ago

This patch introduces a new interface called qemuArch that contains the functions needed to create qemu specific architecture implementations. This patch also includes three qemu specific architecture implementations: qemuArchBase, qemuAmd64 and qemuArm64, since all architectures use similar qemu command lines, the base structure is qemuArchBase that is an agnostic implementation to start virtual machines, qemuAmd64 and qemuArm64 inherit and override (if needed) the functions of qemuArchBase, therefore architecture specific optimizations are in qemuAmd64 and qemuArm64. For example the appendImage function defined by qemuArchBase uses a block device to provide the image to the VM, qemuArm64 inherits and doesn't overwrite it, but qemuAmd64 inherits and overwites it to provide the image throught NVDIMM and DAX.

fixes #608 fixes #610 fixes #611 fixes #612

Signed-off-by: Julio Montes julio.montes@intel.com

jodh-intel commented 6 years ago

Nice PR.

Looks like it needs a rebase and a re-vendor of govmm for qemu.Machine.Options.

devimc commented 6 years ago

depends-on https://github.com/containers/virtcontainers/pull/625

devimc commented 6 years ago

@sameo @jodh-intel changes applied, thanks

devimc commented 6 years ago

@sboeuf changes applied, thanks

sboeuf commented 6 years ago

Thanks @devimc, I'll do the review on Tuesday (Monday being a public holiday in the US).

devimc commented 6 years ago

@jodh-intel changes applied, thanks

sboeuf commented 6 years ago

LGTM I'll wait for @jodh-intel to give his approval before to merge this.

jodh-intel commented 6 years ago

I still wonder if we need some rootflags for qemu_arm64.go, but for now...

lgtm

Great piece of work by the way @devimc !!

Approved with PullApprove Approved with PullApprove

sameo commented 6 years ago

Thanks @devimc LGTM

Approved with PullApprove Approved with PullApprove