Skip to content

Commit

Permalink
[metalos][vm] remove arch from kernel name
Browse files Browse the repository at this point in the history
Summary:
It's hard to maintain when we have arch in kernel target name. It
needs to be passed around all the time downstream. In reality, there is only
one kernel per version for each arch (i.e., target platform). `select` and
`target_compatible_with` can perfectly describe the arch compatibility.

Thus remove arch from the name so we no longer need it at evaluation time. This
enables transparent aarch64 VM support based on kernel compatibility.
Meanwhile, getting rid of `arch` at evaluation time sidesteps autodeps
limitation so that we no longer need to disable it (separate patch).

The burden of setting compatibility properly rests on the owner that creates
the kernel target. There is no way for us to know if a specific kernel version
is supposed to exist or work on a specific arch when making up the VM targets.
For example, even if we built 5.12 kernels for aarch64, they simply wouldn't
boot. Meanwhile we don't dual build kernels yet. The creator of the kernel
target is at the best place to decide that and its compatibility will be
inherited by downstream targets automatically.

Test Plan:
This has to rely on CI to catch any missing places need to be
updated.

Reviewed By: aijayadams

Differential Revision: D51861953

fbshipit-source-id: 557f6fe64190056d0817946a8bad9c323a1e9916
  • Loading branch information
wujj123456 authored and facebook-github-bot committed Dec 12, 2023
1 parent f178b39 commit cbb2387
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 22 deletions.
2 changes: 1 addition & 1 deletion antlir/antlir2/bzl/platform.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def default_target_platform_kwargs():
"default_target_platform": config.get_platform_for_current_buildfile().target_platform,
}

def arch_select(aarch64: str, x86_64: str) -> Select:
def arch_select(aarch64, x86_64) -> Select:
"""Helper for any field that needs arch dependent select"""
return select({
"ovr_config//cpu:arm64": aarch64,
Expand Down
24 changes: 6 additions & 18 deletions antlir/antlir2/docs/docs/internals/vm-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ Notable benefits for VM test owners includes:
- Use of virtiofsd for file sharing with better performance
- All common benefits of antlir2, including faster builds and better cached
artifacts
- Enables automatic multi-arch testing

For developers, there are additional benefits:

- Antlir2 VM is written in Rust, and thus it's safer to iterate
- Buck2 elimated a lot of hacks in antlir1, like cleaner dependency tracking
- Data types are decoupled from buck, which makes it easier wrap a VM standalone
- Enables multi-arch testing (still WIP)

## General Note for Examples

Expand Down Expand Up @@ -355,7 +355,6 @@ vm.metalos_host(
# rootfs_layer = ...,
# root_disk = ...,
# extra_disks = ...,
# arch = ...,
# uname = ...,
# any other parameter that antlir's vm.host takes
)
Expand Down Expand Up @@ -433,9 +432,9 @@ separate and have to co-exist for now. The kernel artifact is the same even
though you might see a different kernel target.

All APIs under `metalos/vm/*/defs.bzl` supports one or multiple `get*()`
function that takes `arch` and kernel `uname`. So long as the arch and uname
combination in the `versions.bzl`, the target can be used. For example, this
turns the default VM into a different kernel.
function that takes kernel `uname`. So long as the uname combination in the
`versions.bzl`, the target can be used for compatible platform. For example,
this turns the default VM into a different kernel.

```
load("//antlir/antlir2/antlir2_vm/bzl:defs.bzl", "vm")
Expand All @@ -445,7 +444,6 @@ vm.host(
name = "disk-boot-5.19",
compatible_with = ["ovr_config//cpu:x86_64"],
disks = [simple_disk.get_boot_disk(
arch = "x86_64",
interface = "virtio-blk",
uname = "5.19",
)],
Expand All @@ -462,25 +460,15 @@ load("//metalos/vm/kernels:defs.bzl", "vm_kernel")
vm.host(
name = "nondisk-boot-5.19",
compatible_with = ["ovr_config//cpu:x86_64"],
disks = [simple_disk.get_control_disk(
arch = "x86_64",
interface = "virtio-blk",
uname = "5.19",
)],
initrd = initrd.get("x86_64", "5.19"),
kernel = vm_kernel.get("x86_64", "5.19").vmlinuz,
initrd = initrd.get("5.19"),
kernel = vm_kernel.get("5.19").vmlinuz,
)
```

Some of the details would change when we unify the kernel type later, and that
means the parameters we use to fill the fields could change. Of course they can
be customized to whatever buck target you want as well. In addition,
`compatible_with` might need to be set given the image content is arch specific.
This might look awkward now, but in the future when multi-arch support is fully
ready, the various `get*()` will return `select()` to avoid the need of setting
`compatible_with` or passing in `arch` entirely.

### Migrating from Antlir1 VM test

Make sure you've read the [MetalOS VM API](#metalos-vm-api) section first. That
Expand Down
2 changes: 1 addition & 1 deletion antlir/bzl/linux/boot/ble_build.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def ble_build(

features.extend([
feature.install(
src = initrd(arch = kernel.arch, uname = kernel.uname),
src = initrd(kernel.uname),
dst = "/initrd-{}.img".format(kernel.uname),
),
feature.install(
Expand Down
4 changes: 2 additions & 2 deletions antlir/vm/bzl/initrd.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

def initrd(arch: str, uname: str):
def initrd(uname: str):
""" Get the initrd target based on arch and uname.
Initrd data isn't really antlir's business, but some of our images need it.
Expand All @@ -12,4 +12,4 @@ def initrd(arch: str, uname: str):
initrd, or we set this as a REPO_CFG so it's easier to specify where initrd
targets live.
"""
return "//metalos/vm/initrd:vm-{}-{}-initrd".format(arch, uname)
return "//metalos/vm/initrd:vm-{}-initrd".format(uname)

0 comments on commit cbb2387

Please sign in to comment.