Fix hypervisor integration tests(x86) - #1302
Conversation
![cubesandboxbot[bot]](https://avatars.githubusercontent.com/in/3653584?s=60&v=4){kind=small}
| // collision with other parallel snapshot tests that also pre-create a | ||
| // restore tap (ip tuntap add fails with EBUSY on TUNSETIFF when another | ||
| // test has already opened the same name). | ||
| let tap_name_restored = format!( |
There was a problem hiding this comment.
rtap{tmp_dir.file_name()} will very likely exceed the Linux interface-name limit (IFNAMSIZ-1 = 15 chars). Guest::new creates its temp dir via TempDir::new_with_prefix("/tmp/ch"), and vmm-sys-util 0.12.1 names that directory ch<epoch_secs>_<subsec_nanos> (e.g. ch1712345678_123456789, ~22 chars). The derived tap name is therefore ~26 chars, and both net_util::open_named (via build_terminated_if_name, which rejects names > 15 with InvalidIfname) and the sudo ip tuntap add pre-create below would fail — so test_snapshot_from_release_restore_on_head and test_snapshot_restore_from_config (same pattern further down) would fail at tap creation instead of colliding. Use a short per-test suffix (e.g. the VM id / a counter) instead of the whole temp-dir name.
Review: Fix hypervisor integration tests (x86) — PR #1302AI-generated review — not a human approval. SummaryThis PR makes the x86_64 integration suite pass on PVM hosts. The changes fall into three categories:
Findings1. 18 tests are
|
|
@aichitudou123 Pls make sure cubesandboxbot is satisfied first. |
lisongqian
left a comment
lisongqian
left a comment
There was a problem hiding this comment.
Thank you for your work on fixing the integration test. Some commits need to be confirmed as necessary.
| CONFIG_NETDEVICES=y | ||
| CONFIG_NET_CORE=y | ||
| # CONFIG_BONDING is not set | ||
| CONFIG_DUMMY=y |
8756fc7 to
b3105ed
Compare
b3105ed to
82ffeac
Compare
|
All test cases have passed. |
|
Detail test results with |
| let start_addr = MemoryManager::start_addr(self.guest_memory.memory().last_addr(), true)?; | ||
|
|
||
| if start_addr.checked_add(size.try_into().unwrap()).unwrap() >= self.end_of_ram_area { | ||
| if start_addr |
There was a problem hiding this comment.
(size - 1) underflows when size == 0. 0 passes the earlier size % (128 << 20) != 0 check (0 is a multiple of 128 MiB), so the size validation doesn't exclude it. The current caller (resize ACPI path) guarantees size > 0 via desired_ram > self.current_ram, but the old code at least wouldn't panic here (it would fall through and create a zero-size region). A defensive size.checked_sub(1) / saturating_sub(1) would keep this function robust to a future zero-size call. (The off-by-one fix itself is correct given end_of_ram_area is the inclusive last address — start_of_device_area - 1.)
a55772f to
bad7a61
Compare
| } | ||
|
|
||
| #[test] | ||
| #[cfg(target_arch = "x86_64")] |
There was a problem hiding this comment.
These #[ignore] attributes are unconditional, so they also disable these tests on non-PVM x86_64 hosts — including the public GitHub Actions CI, where test_focal_hypervisor_fw is part of the quick-mode PRIORITY1 smoke suite (.github/workflows/hypervisor-integration.yaml runs dev_cli.sh tests --integration --quick, whose PRIORITY1_TESTS includes test_focal_hypervisor_fw). Ignored tests pass silently, so that CI will stop exercising the hypervisor-fw/OVMF boot path entirely on a normal KVM host. Consider gating the skip on a PVM-specific condition (e.g. an env var / runtime check in the test, or a cfg flag) instead of unconditionally ignoring, so non-PVM CI still runs these. Same applies to the other #[ignore]s added in this PR (vhdx, watchdog, vfio-user, ovs-dpdk, noacpi, live-migration watchdog/ovs-dpdk).
| # destination use the same binary, so the cross-version upgrade path | ||
| # is not exercised on PVM. | ||
| CH_RELEASE_NAME="cloud-hypervisor-static" | ||
| cp -f target/$BUILD_TARGET/release/cube-hypervisor "$WORKLOADS_DIR"/"$CH_RELEASE_NAME" || exit 1 |
There was a problem hiding this comment.
With cloud-hypervisor-static now being a copy of the just-built cube-hypervisor, test_live_upgrade_* start the source and destination with the same binary. The cross-version upgrade path is therefore silently not exercised — the test passes without testing what its name claims. Since upstream v26 genuinely can't boot the PVM guest kernel, it would be more honest to #[ignore] the upgrade tests on PVM (with this reason) than to run them as no-ops, so the coverage loss is explicit and can be revisited when a bootable older binary is available. (Also note: cloud_hypervisor_release_path() is shared with setup_ovs_dpdk_guests, so this substitution affects OVS-DPDK release-mode runs too.)
| // Read vectored | ||
| file.read_vectored(slices.as_mut_slice()) | ||
| .map_err(AsyncIoError::ReadVectored)? | ||
| let mut r = 0; |
There was a problem hiding this comment.
This manual loop replaces the std read_vectored/write_vectored default. For the only two AsyncAdaptor implementors (QcowFile, Vhdx), the std default is already a per-buffer loop, so the behavior is mostly equivalent — but there are subtle differences that are hard to see without a comment explaining what this works around:
- The std default retries on
ErrorKind::Interrupted;map_errhere turns EINTR into a hard I/O failure. - The loop advances past a partial read/write of a buffer and continues with the next slice. If this trait is ever implemented over a type that can short-read/short-write mid-buffer (a raw
Fileon NFS/FUSE, or any partial write), data is silently misplaced/lost. The proper pattern for "fill all slices" is to loop within each buffer until it's full (or useread_exact/write_allsemantics), not to sum onereadper slice.
Could you add a comment explaining which failure this fixes (e.g. a filesystem returning EINVAL for readv/writev on the PVM host)? That would make the intent reviewable.
| if let Some(balloon) = &self.balloon { | ||
| let mut ram_size = self.memory.size; | ||
|
|
||
| if let Some(hotplugged_size) = &self.memory.hotplugged_size { |
There was a problem hiding this comment.
MemoryConfig::total_size() (vm_config.rs:902) already computes exactly this accumulation — size + hotplugged_size + Σ(zone.size + zone.hotplugged_size) — and additionally includes each zone's hotplugged_size, which this manual addition misses. A virtio-mem zone's pre-hotplugged memory would still be excluded from ram_size here, under-counting the real RAM and potentially rejecting a valid balloon size. Suggest replacing this block with let ram_size = self.memory.total_size();.
| let start_addr = MemoryManager::start_addr(self.guest_memory.memory().last_addr(), true)?; | ||
|
|
||
| if start_addr.checked_add(size.try_into().unwrap()).unwrap() >= self.end_of_ram_area { | ||
| if start_addr |
There was a problem hiding this comment.
size - 1 underflows (panic in debug builds) when size == 0. It's not reachable today — the ACPI hotplug caller only invokes hotplug_ram_region when desired_ram > current_ram, and the modulus check above rejects non-128MiB multiples — but the old code (start_addr.checked_add(size)...) was safe for size == 0, so this is a new (if latent) panic path. A size == 0 guard or size.checked_sub(1) would keep it defensive. (The boundary change itself — allowing a region whose last byte is exactly end_of_ram_area — is correct.)
| let mut ifname: String = String::new(); | ||
| let vnet_hdr_size = vnet_hdr_len() as i32; | ||
| // Check if the given interface exists before we create it. | ||
| let tap_existed = if_name.map_or(false, |n| { |
There was a problem hiding this comment.
Behavioral note on the IP/mask skip: once a pre-existing interface's IP is not overwritten, a stale tap left over from a previous VM (with a different IP than the current --net ip=/mask= values) will silently leave the guest unable to reach the host at the configured address — and the warn! is the only signal. The test collision that motivated this (explicit vmtap0 vs. auto-assigned vmtap%d) is worked around by renaming the test tap to src-tap0, but this changes production reuse semantics too. Consider logging at info!/including the configured IP in the warning so the mismatch is diagnosable.
Fixes cases: - `test_virtio_vsock` - `test_virtio_vsock_hotplug` Both cases failed with `EpollWaitTimeout` because the PVM guest kernel had `CONFIG_VHOST_VSOCK` disabled, so the guest side of the vsock link never came up. Turn it on in `deploy/pvm/configs/pvm_guest`; the required `CONFIG_VHOST` / `CONFIG_VHOST_IOTLB` deps are pulled in automatically by `make olddefconfig` during the guest build. Signed-off-by: Jiahui Xu <clayxu@tencent.com>
The rate_limiter, windows, vfio (test_vfio/test_nvidia_*) and sgx test
groups are exercised by their own dedicated CI scripts
(run_integration_tests_{rate_limiter,windows_*,vfio,sgx}.sh), so they no
longer need to be ignored here. Drop the corresponding #[ignore]
attributes that were previously added for them, restoring those cases to
their original runnable state.
The remaining 17 ignores are kept, because the PVM host/guest cannot run
them (firmware boot chain, vhdx toolchain, CONFIG_OPENVSWITCH
disabled in the PVM guest kernel, no virtio-watchdog driver, no vfio-user
support):
- test_bionic_hypervisor_fw, test_focal_hypervisor_fw,
test_bionic_ovmf, test_focal_ovmf:
PVM host does not support hypervisor-fw/OVMF firmware boot chain
- test_virtio_block_vhdx:
vhdx toolchain/firmware compatibility
- test_virtio_block_direct_and_firmware:
PVM host does not support firmware boot
- test_ovs_dpdk, test_live_migration_ovs_dpdk,
test_live_migration_ovs_dpdk_local, test_live_upgrade_ovs_dpdk,
test_live_upgrade_ovs_dpdk_local:
PVM guest kernel has CONFIG_OPENVSWITCH disabled
- test_watchdog, test_live_migration_watchdog,
test_live_migration_watchdog_local, test_live_upgrade_watchdog,
test_live_upgrade_watchdog_local:
PVM guest kernel has no virtio-watchdog driver (CONFIG_VIRTIO_WDT
not enabled)
- test_vfio_user:
PVM host/guest kernel does not support vfio-user (no VFIO support)
Signed-off-by: Jiahui Xu <clayxu@tencent.com>
upstream commit: 0e9513f2b7c72556ff499789420815aacc2070a9 When a tap name is explicitly set, device_manager dropped the IP/netmask, leaving the tap without a host address and breaking test_snapshot_restore_from_config on PVM. Forward the values so the tap is configured correctly, matching the internal CubeHypervisor. Also update net_util open_tap to not overwrite IP configuration of existing TAP interfaces. Also rename tap vmtap0 -> src-tap0 to avoid EBUSY with parallel tests. Signed-off-by: Jiahui Xu <clayxu@tencent.com>
Fixes case: `test_snapshot_restore_hotplug_virtiomem` (and any
virtio-mem / ACPI memory hotplug case that hits the boundary address).
The end address of a hotplug region is inclusive, but three places
treated it as exclusive, rejecting the last valid page:
- `virtio-devices/src/mem.rs::is_valid_range`: use `>` instead of `>=`
- `vmm/src/memory_manager.rs`: compare `(size - 1) > end_of_ram_area`
- `vmm/src/config.rs`: include `hotplugged_size` when validating
balloon size against the hotpluggable region
Signed-off-by: Jiahui Xu <clayxu@tencent.com>
Two fixes for test_snapshot_restore_native_virtiofs_with_deleted_backing_file: 1. Guard the restore polling loop with Path::exists() on the event monitor file. On PVM the restore VM boots slower and the event file may not exist on the first poll, causing check_latest_events_exact to panic on fs::read().unwrap(). 2. Drop guest dentry cache (echo 2 > /proc/sys/vm/drop_caches) before the test -e probe. With cache=always the guest retains dentries from before snapshot, so test -e would still report the deleted file as present. Flushing the cache forces a fresh FUSE LOOKUP that correctly returns ENOENT. Signed-off-by: Jiahui Xu <clayxu@tencent.com>
Fixes case: `test_vhost_user_net` (multi-NIC vhost-user-net path).
The PVM guest kernel had three config options that prevented the
test from passing:
- CONFIG_DUMMY=y: kernel auto-creates a `dummy0` interface at boot,
making `ip -o link | wc -l` return 3 instead of the expected 2.
Disabling it removes the extra interface with no side effect—
`dummy0` is not referenced by any test or cloud-init rule.
- CONFIG_IPV6_SIT=y: kernel auto-creates `sit0` and `ip6tnl0`
tunnel interfaces, also inflating the interface count. Disabling
SIT removes these; the PVM guest does not use IPv6-in-IPv4
tunnelling.
- CONFIG_VFAT_FS was disabled, causing `/boot/efi` mount failure
at boot → systemd emergency mode → SSH unreachable → test
timeout. Enabling VFAT_FS allows the EFI partition to mount.
No additional NIC or cloud-init network-config entry is needed:
`test_vhost_user_net` only creates a single vhost-user-net device
and asserts exactly 2 interfaces (lo + ens4).
Signed-off-by: Jiahui Xu <clayxu@tencent.com>
Fixes cases (live_migration / upgrade group): - test_live_migration_basic, _local - test_live_migration_balloon, _balloon_local - test_live_migration_numa, _numa_local - test_live_migration_watchdog, _watchdog_local - test_live_upgrade_watchdog, _watchdog_local - (also fixes the source-VM path in setup_ovs_dpdk_guests) This fork builds the VMM as `cube-hypervisor`, but the live_migration helpers still spawned `cloud-hypervisor` via clh_command(), so GuestCommand::spawn() failed with NotFound (test_infra/src/lib.rs:1394) and every migration case died before booting. Point the 5 call sites at the real binary name. Signed-off-by: Jiahui Xu <clayxu@tencent.com>
Fixes case: common_parallel::test_multi_cpu
The x86_64 assertion read dmesg as the `cloud` user, but the PVM
guest kernel enables CONFIG_SECURITY_DMESG_RESTRICT=y, so non-root
dmesg returns empty and the strict assert_eq!() against the full
"smpboot: Allowing 4 CPUs, 2 hotplug CPUs" line fails (left: "").
Two small adjustments, both aligned with the inner-network tree:
- prefix the dmesg command with `sudo` (the `cloud` user already
has passwordless sudo, see sudo journalctl in test_watchdog);
- switch to a loose match (grep -o "Allowing.*hotplug CPU[s]*" +
contains("2 hotplug CPU")), matching the inner tree, so the
assertion is robust to timestamp/prefix formatting changes.
Signed-off-by: Jiahui Xu <clayxu@tencent.com>
In PVM container environment (non init_net namespace), devtmpfs does not auto-create /dev/tap<ifindex> char device node when creating a macvtap interface, while the macvtap net_device itself is created successfully. The test relies on /dev/tap<ifindex> to chown and open the tap fd, so it failed at the chown assertion. Read major:minor from /sys/class/macvtap/tap<ifindex>/dev and mknod the node manually if it does not exist. On bare metal (init_net) the node already exists and the workaround is not triggered, keeping behavior aligned with the intranet. Fixes: common_parallel::test_macvtap, common_parallel::test_macvtap_hotplug Signed-off-by: Jiahui Xu <clayxu@tencent.com>
PVM guest kernel disables TSC deadline timer via setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER) for safety. With acpi=off, no clockevent source works: - TSC deadline timer: disabled by PVM kernel policy - HPET/ACPI PM-Timer: unavailable (ACPI tables not parsed) - PIT IRQ 0 through 8259 PIC: cloud-hypervisor does not emulate 8259 As a result, calibrate_APIC_clock() fails verification, no clockevent device is available, and the kernel hangs. This is expected behavior for the PVM guest configuration. Signed-off-by: Jiahui Xu <clayxu@tencent.com>
1. Use 'sudo poweroff' instead of 'sudo reboot' because the PVM guest kernel does not support reboot (CMOS reset triggers KVM internal error on PVM host, which makes VmShutdown event never delivered). 2. Optimize the event receive wait: use recv_timeout instead of the blocking recv() so the test fails fast instead of hanging forever. Signed-off-by: Jiahui Xu <clayxu@tencent.com>
Upstream cloud-hypervisor-static v26 lacks PVM CPUID support, causing test_live_upgrade_* to fail. Copy the locally-built binary after cargo build using relative paths with error checking. Both sides use the same binary, so cross-version upgrade is not tested on PVM. Signed-off-by: Jiahui Xu <clayxu@tencent.com>
(cherry picked from commit b2f40afc6913eaa62d846283ef4b34dc7f579beb) The original code relied on the default `read_vectored` or `write_vectored` implementations from the standard library. The default implementation of those functions only uses the first non-empty buffer. That is not correct when there are more than one buffers. Fixes: #6876 Signed-off-by: Jiahui Xu <clayxu@tencent.com>
bad7a61 to
a49c701
Compare
3 participants
Fix hypervisor integration tests(x86) for full tests
We fix the test cases related to the following commands:
./scripts/dev_cli.sh tests --integration
./scripts/dev_cli.sh tests --integration-live-migration