diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsClose.c b/OvmfPkg/VirtioFsDxe/SimpleFsClose.c index 01bbeae214..bc91ad726b 100644 --- a/OvmfPkg/VirtioFsDxe/SimpleFsClose.c +++ b/OvmfPkg/VirtioFsDxe/SimpleFsClose.c @@ -24,20 +24,36 @@ VirtioFsSimpleFileClose ( VirtioFs = VirtioFsFile->OwnerFs; // - // At this point, the implementation is only suitable for closing the - // VIRTIO_FS_FILE that was created by VirtioFsOpenVolume(). + // All actions in this function are "best effort"; the UEFI spec requires + // EFI_FILE_PROTOCOL.Close() to sync all data to the device, but it also + // requires EFI_FILE_PROTOCOL.Close() to release resources unconditionally, + // and to return EFI_SUCCESS unconditionally. // - ASSERT (VirtioFsFile->IsDirectory); - ASSERT (VirtioFsFile->NodeId == VIRTIO_FS_FUSE_ROOT_DIR_NODE_ID); - // - // Close the root directory. - // - // Ignore any errors, because EFI_FILE_PROTOCOL.Close() is required to - // release the EFI_FILE_PROTOCOL object unconditionally. + // Flush, sync, release, and (if needed) forget. If any action fails, we + // still try the others. // + if (VirtioFsFile->IsOpenForWriting) { + if (!VirtioFsFile->IsDirectory) { + VirtioFsFuseFlush (VirtioFs, VirtioFsFile->NodeId, + VirtioFsFile->FuseHandle); + } + + VirtioFsFuseFsyncFileOrDir (VirtioFs, VirtioFsFile->NodeId, + VirtioFsFile->FuseHandle, VirtioFsFile->IsDirectory); + } + VirtioFsFuseReleaseFileOrDir (VirtioFs, VirtioFsFile->NodeId, VirtioFsFile->FuseHandle, VirtioFsFile->IsDirectory); + // + // VirtioFsFile->FuseHandle is gone at this point, but VirtioFsFile->NodeId + // is still valid. If we've known VirtioFsFile->NodeId from a lookup, then + // now we should ask the server to forget it *once*. + // + if (VirtioFsFile->NodeId != VIRTIO_FS_FUSE_ROOT_DIR_NODE_ID) { + VirtioFsFuseForget (VirtioFs, VirtioFsFile->NodeId); + } + // // One fewer file left open for the owner filesystem. // diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsDelete.c b/OvmfPkg/VirtioFsDxe/SimpleFsDelete.c index 3209923d1e..bbad64bf78 100644 --- a/OvmfPkg/VirtioFsDxe/SimpleFsDelete.c +++ b/OvmfPkg/VirtioFsDxe/SimpleFsDelete.c @@ -6,6 +6,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ +#include // RemoveEntryList() +#include // FreePool() + #include "VirtioFsDxe.h" EFI_STATUS @@ -14,16 +17,52 @@ VirtioFsSimpleFileDelete ( IN EFI_FILE_PROTOCOL *This ) { + VIRTIO_FS_FILE *VirtioFsFile; + VIRTIO_FS *VirtioFs; + EFI_STATUS Status; + + VirtioFsFile = VIRTIO_FS_FILE_FROM_SIMPLE_FILE (This); + VirtioFs = VirtioFsFile->OwnerFs; + // - // At this point, the implementation is only suitable for closing the - // VIRTIO_FS_FILE that was created by VirtioFsOpenVolume(). + // All actions in this function are "best effort"; the UEFI spec requires + // EFI_FILE_PROTOCOL.Delete() to release resources unconditionally. If a step + // related to removing the file fails, it's only reflected in the return + // status (EFI_WARN_DELETE_FAILURE rather than EFI_SUCCESS). // - // Actually deleting the root directory is not possible, so we're only going - // to release resources, and return EFI_WARN_DELETE_FAILURE. + // Release, remove, and (if needed) forget. We don't waste time flushing and + // syncing; if the EFI_FILE_PROTOCOL user cares enough, they should keep the + // parent directory open until after this function call returns, and then + // force a sync on *that* EFI_FILE_PROTOCOL instance, using either the + // Flush() member function, or the Close() member function. // - // In order to release resources, VirtioFsSimpleFileClose() is just right - // here. + // If any action fails below, we still try the others. // - VirtioFsSimpleFileClose (This); - return EFI_WARN_DELETE_FAILURE; + VirtioFsFuseReleaseFileOrDir (VirtioFs, VirtioFsFile->NodeId, + VirtioFsFile->FuseHandle, VirtioFsFile->IsDirectory); + + // + // VirtioFsFile->FuseHandle is gone at this point, but VirtioFsFile->NodeId + // is still valid. Continue with removing the file or directory. The result + // of this operation determines the return status of the function. + // + // TODO + // + Status = EFI_WARN_DELETE_FAILURE; + + // + // Finally, if we've known VirtioFsFile->NodeId from a lookup, then we should + // also ask the server to forget it *once*. + // + if (VirtioFsFile->NodeId != VIRTIO_FS_FUSE_ROOT_DIR_NODE_ID) { + VirtioFsFuseForget (VirtioFs, VirtioFsFile->NodeId); + } + + // + // One fewer file left open for the owner filesystem. + // + RemoveEntryList (&VirtioFsFile->OpenFilesEntry); + + FreePool (VirtioFsFile); + return Status; } diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsOpenVolume.c b/OvmfPkg/VirtioFsDxe/SimpleFsOpenVolume.c index 8c1457a68a..67d2deb6bd 100644 --- a/OvmfPkg/VirtioFsDxe/SimpleFsOpenVolume.c +++ b/OvmfPkg/VirtioFsDxe/SimpleFsOpenVolume.c @@ -62,6 +62,7 @@ VirtioFsOpenVolume ( VirtioFsFile->SimpleFile.SetInfo = VirtioFsSimpleFileSetInfo; VirtioFsFile->SimpleFile.Flush = VirtioFsSimpleFileFlush; VirtioFsFile->IsDirectory = TRUE; + VirtioFsFile->IsOpenForWriting = FALSE; VirtioFsFile->OwnerFs = VirtioFs; VirtioFsFile->NodeId = VIRTIO_FS_FUSE_ROOT_DIR_NODE_ID; VirtioFsFile->FuseHandle = RootDirHandle; diff --git a/OvmfPkg/VirtioFsDxe/VirtioFsDxe.h b/OvmfPkg/VirtioFsDxe/VirtioFsDxe.h index 7151a62bb4..1cbd27d8fb 100644 --- a/OvmfPkg/VirtioFsDxe/VirtioFsDxe.h +++ b/OvmfPkg/VirtioFsDxe/VirtioFsDxe.h @@ -110,6 +110,7 @@ typedef struct { UINT64 Signature; EFI_FILE_PROTOCOL SimpleFile; BOOLEAN IsDirectory; + BOOLEAN IsOpenForWriting; VIRTIO_FS *OwnerFs; LIST_ENTRY OpenFilesEntry; //