From a455917db5bd05f27c0b797054d1aaf4a3d30fb1 Mon Sep 17 00:00:00 2001 From: Alyssa Ross Date: Wed, 13 Jul 2022 09:10:11 +0000 Subject: [PATCH] vmm: fix missed API or debug events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, we were assuming that every time an eventfd notified us, there was only a single event waiting for us. This meant that if, while one API request was being processed, two more arrived, the second one would not be processed (until the next one arrived, when it would be processed instead of that event, and so on). To fix this, make sure we're processing the number of API and debug requests we've been told have arrived, rather than just one. This is easy to demonstrate by sending lots of API events and adding some sleeps to make sure multiple events can arrive while each is being processed. For other uses of eventfd, like the exit event, this doesn't matter — even if we've received multiple exit events in quick succession, we only need to exit once. So I've only made this change where receiving an event is non-idempotent, i.e. where it matters that we process the event the right number of times. Technically, reset requests are also non-idempotent — there's an observable difference between a VM resetting once, and a VM resetting once and then immediately resetting again. But I've left that alone for now because two resets in immediate succession doesn't sound like something anyone would ever want to me. Signed-off-by: Alyssa Ross --- vmm/src/lib.rs | 446 +++++++++++++++++++++++++------------------------ 1 file changed, 224 insertions(+), 222 deletions(-) diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index 17435016d..6bc9e5deb 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -1685,252 +1685,254 @@ impl Vmm { } } EpollDispatch::Api => { - // Consume the event. - self.api_evt.read().map_err(Error::EventFdRead)?; + // Consume the events. + for _ in 0..self.api_evt.read().map_err(Error::EventFdRead)? { + // Read from the API receiver channel + let api_request = api_receiver.recv().map_err(Error::ApiRequestRecv)?; - // Read from the API receiver channel - let api_request = api_receiver.recv().map_err(Error::ApiRequestRecv)?; + info!("API request event: {:?}", api_request); + match api_request { + ApiRequest::VmCreate(config, sender) => { + let response = self + .vm_create(config) + .map_err(ApiError::VmCreate) + .map(|_| ApiResponsePayload::Empty); - info!("API request event: {:?}", api_request); - match api_request { - ApiRequest::VmCreate(config, sender) => { - let response = self - .vm_create(config) - .map_err(ApiError::VmCreate) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmDelete(sender) => { + let response = self + .vm_delete() + .map_err(ApiError::VmDelete) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmDelete(sender) => { - let response = self - .vm_delete() - .map_err(ApiError::VmDelete) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmBoot(sender) => { + let response = self + .vm_boot() + .map_err(ApiError::VmBoot) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmBoot(sender) => { - let response = self - .vm_boot() - .map_err(ApiError::VmBoot) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmShutdown(sender) => { + let response = self + .vm_shutdown() + .map_err(ApiError::VmShutdown) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmShutdown(sender) => { - let response = self - .vm_shutdown() - .map_err(ApiError::VmShutdown) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmReboot(sender) => { + let response = self + .vm_reboot() + .map_err(ApiError::VmReboot) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmReboot(sender) => { - let response = self - .vm_reboot() - .map_err(ApiError::VmReboot) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmInfo(sender) => { + let response = self + .vm_info() + .map_err(ApiError::VmInfo) + .map(ApiResponsePayload::VmInfo); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmInfo(sender) => { - let response = self - .vm_info() - .map_err(ApiError::VmInfo) - .map(ApiResponsePayload::VmInfo); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmmPing(sender) => { + let response = ApiResponsePayload::VmmPing(self.vmm_ping()); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmmPing(sender) => { - let response = ApiResponsePayload::VmmPing(self.vmm_ping()); + sender.send(Ok(response)).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmPause(sender) => { + let response = self + .vm_pause() + .map_err(ApiError::VmPause) + .map(|_| ApiResponsePayload::Empty); - sender.send(Ok(response)).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmPause(sender) => { - let response = self - .vm_pause() - .map_err(ApiError::VmPause) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmResume(sender) => { + let response = self + .vm_resume() + .map_err(ApiError::VmResume) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmResume(sender) => { - let response = self - .vm_resume() - .map_err(ApiError::VmResume) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmSnapshot(snapshot_data, sender) => { + let response = self + .vm_snapshot(&snapshot_data.destination_url) + .map_err(ApiError::VmSnapshot) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmSnapshot(snapshot_data, sender) => { - let response = self - .vm_snapshot(&snapshot_data.destination_url) - .map_err(ApiError::VmSnapshot) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmRestore(restore_data, sender) => { + let response = self + .vm_restore(restore_data.as_ref().clone()) + .map_err(ApiError::VmRestore) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmRestore(restore_data, sender) => { - let response = self - .vm_restore(restore_data.as_ref().clone()) - .map_err(ApiError::VmRestore) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + #[cfg(feature = "guest_debug")] + ApiRequest::VmCoredump(coredump_data, sender) => { + let response = self + .vm_coredump(&coredump_data.destination_url) + .map_err(ApiError::VmCoredump) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - #[cfg(feature = "guest_debug")] - ApiRequest::VmCoredump(coredump_data, sender) => { - let response = self - .vm_coredump(&coredump_data.destination_url) - .map_err(ApiError::VmCoredump) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmmShutdown(sender) => { + let response = self + .vmm_shutdown() + .map_err(ApiError::VmmShutdown) + .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmmShutdown(sender) => { - let response = self - .vmm_shutdown() - .map_err(ApiError::VmmShutdown) - .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; - sender.send(response).map_err(Error::ApiResponseSend)?; + break 'outer; + } + ApiRequest::VmResize(resize_data, sender) => { + let response = self + .vm_resize( + resize_data.desired_vcpus, + resize_data.desired_ram, + resize_data.desired_balloon, + ) + .map_err(ApiError::VmResize) + .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmResizeZone(resize_zone_data, sender) => { + let response = self + .vm_resize_zone( + resize_zone_data.id.clone(), + resize_zone_data.desired_ram, + ) + .map_err(ApiError::VmResizeZone) + .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmAddDevice(add_device_data, sender) => { + let response = self + .vm_add_device(add_device_data.as_ref().clone()) + .map_err(ApiError::VmAddDevice) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmAddUserDevice(add_device_data, sender) => { + let response = self + .vm_add_user_device(add_device_data.as_ref().clone()) + .map_err(ApiError::VmAddUserDevice) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmRemoveDevice(remove_device_data, sender) => { + let response = self + .vm_remove_device(remove_device_data.id.clone()) + .map_err(ApiError::VmRemoveDevice) + .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmAddDisk(add_disk_data, sender) => { + let response = self + .vm_add_disk(add_disk_data.as_ref().clone()) + .map_err(ApiError::VmAddDisk) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmAddFs(add_fs_data, sender) => { + let response = self + .vm_add_fs(add_fs_data.as_ref().clone()) + .map_err(ApiError::VmAddFs) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmAddPmem(add_pmem_data, sender) => { + let response = self + .vm_add_pmem(add_pmem_data.as_ref().clone()) + .map_err(ApiError::VmAddPmem) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmAddNet(add_net_data, sender) => { + let response = self + .vm_add_net(add_net_data.as_ref().clone()) + .map_err(ApiError::VmAddNet) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmAddVdpa(add_vdpa_data, sender) => { + let response = self + .vm_add_vdpa(add_vdpa_data.as_ref().clone()) + .map_err(ApiError::VmAddVdpa) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmAddVsock(add_vsock_data, sender) => { + let response = self + .vm_add_vsock(add_vsock_data.as_ref().clone()) + .map_err(ApiError::VmAddVsock) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmCounters(sender) => { + let response = self + .vm_counters() + .map_err(ApiError::VmInfo) + .map(ApiResponsePayload::VmAction); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmReceiveMigration(receive_migration_data, sender) => { + let response = self + .vm_receive_migration( + receive_migration_data.as_ref().clone(), + ) + .map_err(ApiError::VmReceiveMigration) + .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmSendMigration(send_migration_data, sender) => { + let response = self + .vm_send_migration(send_migration_data.as_ref().clone()) + .map_err(ApiError::VmSendMigration) + .map(|_| ApiResponsePayload::Empty); + sender.send(response).map_err(Error::ApiResponseSend)?; + } + ApiRequest::VmPowerButton(sender) => { + let response = self + .vm_power_button() + .map_err(ApiError::VmPowerButton) + .map(|_| ApiResponsePayload::Empty); - break 'outer; - } - ApiRequest::VmResize(resize_data, sender) => { - let response = self - .vm_resize( - resize_data.desired_vcpus, - resize_data.desired_ram, - resize_data.desired_balloon, - ) - .map_err(ApiError::VmResize) - .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmResizeZone(resize_zone_data, sender) => { - let response = self - .vm_resize_zone( - resize_zone_data.id.clone(), - resize_zone_data.desired_ram, - ) - .map_err(ApiError::VmResizeZone) - .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmAddDevice(add_device_data, sender) => { - let response = self - .vm_add_device(add_device_data.as_ref().clone()) - .map_err(ApiError::VmAddDevice) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmAddUserDevice(add_device_data, sender) => { - let response = self - .vm_add_user_device(add_device_data.as_ref().clone()) - .map_err(ApiError::VmAddUserDevice) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmRemoveDevice(remove_device_data, sender) => { - let response = self - .vm_remove_device(remove_device_data.id.clone()) - .map_err(ApiError::VmRemoveDevice) - .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmAddDisk(add_disk_data, sender) => { - let response = self - .vm_add_disk(add_disk_data.as_ref().clone()) - .map_err(ApiError::VmAddDisk) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmAddFs(add_fs_data, sender) => { - let response = self - .vm_add_fs(add_fs_data.as_ref().clone()) - .map_err(ApiError::VmAddFs) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmAddPmem(add_pmem_data, sender) => { - let response = self - .vm_add_pmem(add_pmem_data.as_ref().clone()) - .map_err(ApiError::VmAddPmem) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmAddNet(add_net_data, sender) => { - let response = self - .vm_add_net(add_net_data.as_ref().clone()) - .map_err(ApiError::VmAddNet) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmAddVdpa(add_vdpa_data, sender) => { - let response = self - .vm_add_vdpa(add_vdpa_data.as_ref().clone()) - .map_err(ApiError::VmAddVdpa) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmAddVsock(add_vsock_data, sender) => { - let response = self - .vm_add_vsock(add_vsock_data.as_ref().clone()) - .map_err(ApiError::VmAddVsock) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmCounters(sender) => { - let response = self - .vm_counters() - .map_err(ApiError::VmInfo) - .map(ApiResponsePayload::VmAction); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmReceiveMigration(receive_migration_data, sender) => { - let response = self - .vm_receive_migration(receive_migration_data.as_ref().clone()) - .map_err(ApiError::VmReceiveMigration) - .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmSendMigration(send_migration_data, sender) => { - let response = self - .vm_send_migration(send_migration_data.as_ref().clone()) - .map_err(ApiError::VmSendMigration) - .map(|_| ApiResponsePayload::Empty); - sender.send(response).map_err(Error::ApiResponseSend)?; - } - ApiRequest::VmPowerButton(sender) => { - let response = self - .vm_power_button() - .map_err(ApiError::VmPowerButton) - .map(|_| ApiResponsePayload::Empty); - - sender.send(response).map_err(Error::ApiResponseSend)?; + sender.send(response).map_err(Error::ApiResponseSend)?; + } } } } #[cfg(feature = "gdb")] EpollDispatch::Debug => { - // Consume the event. - self.debug_evt.read().map_err(Error::EventFdRead)?; + // Consume the events. + for _ in 0..self.debug_evt.read().map_err(Error::EventFdRead)? { + // Read from the API receiver channel + let gdb_request = gdb_receiver.recv().map_err(Error::GdbRequestRecv)?; - // Read from the API receiver channel - let gdb_request = gdb_receiver.recv().map_err(Error::GdbRequestRecv)?; + let response = if let Some(ref mut vm) = self.vm { + vm.debug_request(&gdb_request.payload, gdb_request.cpu_id) + } else { + Err(VmError::VmNotRunning) + } + .map_err(gdb::Error::Vm); - let response = if let Some(ref mut vm) = self.vm { - vm.debug_request(&gdb_request.payload, gdb_request.cpu_id) - } else { - Err(VmError::VmNotRunning) + gdb_request + .sender + .send(response) + .map_err(Error::GdbResponseSend)?; } - .map_err(gdb::Error::Vm); - - gdb_request - .sender - .send(response) - .map_err(Error::GdbResponseSend)?; } #[cfg(not(feature = "gdb"))] EpollDispatch::Debug => {}