From nobody Mon Feb 9 16:50:44 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+69029+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+69029+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1608154995; cv=none; d=zohomail.com; s=zohoarc; b=Y7T7D7/0ITj9eBeT+f3EieaINOtO6euqBg7eTF8jtHVfDVqFffuWy2fZbELoAHEgkeau9DvuySM+P6/P722uECUl4AqDrykC7KnQrR5tABFPDaZWa+g/qa7C30mBB8/Vwg4PWKut2UZaoWiohV/6Hv634H4lpquiXeIaAo5YR3M= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1608154995; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=SUouXpIEa9hfJ2V35njxyKU/mCto7glsOSX0e8E+qyk=; b=XHEI4ApVrGB3ozh+eRQW+nBNOYYFdvMQ/G1ESjUPKWTUjobyiuTtULt6kPc6maKPtWhWgtNRFiPSxTtRHbcJ4pGrkPZQ6gvIB7RBDClO6uS2PEHecXJoVUIlwxYlBg8VWIaXOvwRd9S/gRh+pvupM+myw42xov8ca6zHA97RqbY= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+69029+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1608154995148393.4398791829932; Wed, 16 Dec 2020 13:43:15 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id ohLbYY1788612xdc2bLLrz0G; Wed, 16 Dec 2020 13:43:14 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web10.4075.1608154992662794590 for ; Wed, 16 Dec 2020 13:43:12 -0800 X-Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-312-IKvaRMSgO4Gm1u0IXq1lLg-1; Wed, 16 Dec 2020 16:43:04 -0500 X-MC-Unique: IKvaRMSgO4Gm1u0IXq1lLg-1 X-Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CB6B8800D55; Wed, 16 Dec 2020 21:43:03 +0000 (UTC) X-Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-152.ams2.redhat.com [10.36.114.152]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3690860C15; Wed, 16 Dec 2020 21:42:58 +0000 (UTC) From: "Laszlo Ersek" To: devel@edk2.groups.io, virtio-fs@redhat.com, lersek@redhat.com Cc: Ard Biesheuvel , Jordan Justen , =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Subject: [edk2-devel] [edk2 PATCH 15/48] OvmfPkg/VirtioFsDxe: flush, sync, release and forget in Close() / Delete() Date: Wed, 16 Dec 2020 22:10:52 +0100 Message-Id: <20201216211125.19496-16-lersek@redhat.com> In-Reply-To: <20201216211125.19496-1-lersek@redhat.com> References: <20201216211125.19496-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com X-Gm-Message-State: AEPOf1T4BGQBH5jUbM5reZfwx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1608154994; bh=SUouXpIEa9hfJ2V35njxyKU/mCto7glsOSX0e8E+qyk=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=AvJRClskvYpn7L6ks4CGcYidzEMZdtt3ZC243muiWiVTbEkP7VEWnUCHycTx6h4WFcr j0b1mNAYm4IG4iCzuNeXigi3vb8fp/YQqnirW6XEXF3AxTZ7tLV8W+VbHo2CtNDpn0LD7 EYseWMuOkcTNlHVcNb2wqVKDI3YM4qnNe6U= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" The two member functions that free the EFI_FILE_PROTOCOL object are Close() and Delete(). Before we create VIRTIO_FS_FILE objects with EFI_FILE_PROTOCOL.Open() later in this patch series, extend each of these "destructor" functions to get rid of the FuseHandle and NodeId resources properly -- in a way that matches each function's own purpose. For the time being, VirtioFsSimpleFileDelete() only gets a reminder about its core task (namely, removing the file), as the needed machinery will become only later. But we can already outline the "task list", and deal with the FuseHandle and NodeId resources. The "task list" of VirtioFsSimpleFileDelete() is different from that of VirtioFsSimpleFileClose(), thus both destructors diverge at this point. Cc: Ard Biesheuvel Cc: Jordan Justen Cc: Philippe Mathieu-Daud=C3=A9 Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3097 Signed-off-by: Laszlo Ersek --- OvmfPkg/VirtioFsDxe/VirtioFsDxe.h | 1 + OvmfPkg/VirtioFsDxe/SimpleFsClose.c | 34 ++++++++---- OvmfPkg/VirtioFsDxe/SimpleFsDelete.c | 55 +++++++++++++++++--- OvmfPkg/VirtioFsDxe/SimpleFsOpenVolume.c | 1 + 4 files changed, 74 insertions(+), 17 deletions(-) diff --git a/OvmfPkg/VirtioFsDxe/VirtioFsDxe.h b/OvmfPkg/VirtioFsDxe/Virtio= FsDxe.h index 7151a62bb42b..1cbd27d8fb52 100644 --- a/OvmfPkg/VirtioFsDxe/VirtioFsDxe.h +++ b/OvmfPkg/VirtioFsDxe/VirtioFsDxe.h @@ -105,16 +105,17 @@ typedef struct { // // Private context structure that exposes EFI_FILE_PROTOCOL on top of an o= pen // FUSE file reference. // typedef struct { UINT64 Signature; EFI_FILE_PROTOCOL SimpleFile; BOOLEAN IsDirectory; + BOOLEAN IsOpenForWriting; VIRTIO_FS *OwnerFs; LIST_ENTRY OpenFilesEntry; // // In the FUSE wire protocol, every request except FUSE_INIT refers to a // file, namely by the "VIRTIO_FS_FUSE_REQUEST.NodeId" field; that is, b= y the // inode number of the file. However, some of the FUSE requests that we = need // for some of the EFI_FILE_PROTOCOL member functions require an open fi= le // handle *in addition* to the inode number. For simplicity, whenever a diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsClose.c b/OvmfPkg/VirtioFsDxe/Simp= leFsClose.c index 01bbeae21473..bc91ad726b2c 100644 --- a/OvmfPkg/VirtioFsDxe/SimpleFsClose.c +++ b/OvmfPkg/VirtioFsDxe/SimpleFsClose.c @@ -19,30 +19,46 @@ VirtioFsSimpleFileClose ( { VIRTIO_FS_FILE *VirtioFsFile; VIRTIO_FS *VirtioFs; =20 VirtioFsFile =3D VIRTIO_FS_FILE_FROM_SIMPLE_FILE (This); VirtioFs =3D VirtioFsFile->OwnerFs; =20 // - // 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 unconditional= ly, + // and to return EFI_SUCCESS unconditionally. // - ASSERT (VirtioFsFile->IsDirectory); - ASSERT (VirtioFsFile->NodeId =3D=3D 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); =20 + // + // VirtioFsFile->FuseHandle is gone at this point, but VirtioFsFile->Nod= eId + // is still valid. If we've known VirtioFsFile->NodeId from a lookup, th= en + // now we should ask the server to forget it *once*. + // + if (VirtioFsFile->NodeId !=3D VIRTIO_FS_FUSE_ROOT_DIR_NODE_ID) { + VirtioFsFuseForget (VirtioFs, VirtioFsFile->NodeId); + } + // // One fewer file left open for the owner filesystem. // RemoveEntryList (&VirtioFsFile->OpenFilesEntry); =20 FreePool (VirtioFsFile); return EFI_SUCCESS; } diff --git a/OvmfPkg/VirtioFsDxe/SimpleFsDelete.c b/OvmfPkg/VirtioFsDxe/Sim= pleFsDelete.c index 3209923d1e49..bbad64bf7886 100644 --- a/OvmfPkg/VirtioFsDxe/SimpleFsDelete.c +++ b/OvmfPkg/VirtioFsDxe/SimpleFsDelete.c @@ -1,29 +1,68 @@ /** @file EFI_FILE_PROTOCOL.Delete() member function for the Virtio Filesystem dri= ver. =20 Copyright (C) 2020, Red Hat, Inc. =20 SPDX-License-Identifier: BSD-2-Clause-Patent **/ =20 +#include // RemoveEntryList() +#include // FreePool() + #include "VirtioFsDxe.h" =20 EFI_STATUS EFIAPI VirtioFsSimpleFileDelete ( IN EFI_FILE_PROTOCOL *This ) { + VIRTIO_FS_FILE *VirtioFsFile; + VIRTIO_FS *VirtioFs; + EFI_STATUS Status; + + VirtioFsFile =3D VIRTIO_FS_FILE_FROM_SIMPLE_FILE (This); + VirtioFs =3D 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 g= oing - // 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->Nod= eId + // is still valid. Continue with removing the file or directory. The res= ult + // of this operation determines the return status of the function. + // + // TODO + // + Status =3D EFI_WARN_DELETE_FAILURE; + + // + // Finally, if we've known VirtioFsFile->NodeId from a lookup, then we s= hould + // also ask the server to forget it *once*. + // + if (VirtioFsFile->NodeId !=3D 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 8c1457a68aad..67d2deb6bdf2 100644 --- a/OvmfPkg/VirtioFsDxe/SimpleFsOpenVolume.c +++ b/OvmfPkg/VirtioFsDxe/SimpleFsOpenVolume.c @@ -57,16 +57,17 @@ VirtioFsOpenVolume ( VirtioFsFile->SimpleFile.Read =3D VirtioFsSimpleFileRead; VirtioFsFile->SimpleFile.Write =3D VirtioFsSimpleFileWrite; VirtioFsFile->SimpleFile.GetPosition =3D VirtioFsSimpleFileGetPosition; VirtioFsFile->SimpleFile.SetPosition =3D VirtioFsSimpleFileSetPosition; VirtioFsFile->SimpleFile.GetInfo =3D VirtioFsSimpleFileGetInfo; VirtioFsFile->SimpleFile.SetInfo =3D VirtioFsSimpleFileSetInfo; VirtioFsFile->SimpleFile.Flush =3D VirtioFsSimpleFileFlush; VirtioFsFile->IsDirectory =3D TRUE; + VirtioFsFile->IsOpenForWriting =3D FALSE; VirtioFsFile->OwnerFs =3D VirtioFs; VirtioFsFile->NodeId =3D VIRTIO_FS_FUSE_ROOT_DIR_NODE_ID; VirtioFsFile->FuseHandle =3D RootDirHandle; =20 // // One more file open for the filesystem. // InsertTailList (&VirtioFs->OpenFiles, &VirtioFsFile->OpenFilesEntry); --=20 2.19.1.3.g30247aa5d201 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69029): https://edk2.groups.io/g/devel/message/69029 Mute This Topic: https://groups.io/mt/79023154/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-