From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ua1-f47.google.com (mail-ua1-f47.google.com [209.85.222.47]) by mx.groups.io with SMTP id smtpd.web09.5009.1631248527716023142 for ; Thu, 09 Sep 2021 21:35:27 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=TTDi+uOl; spf=pass (domain: gmail.com, ip: 209.85.222.47, mailfrom: pedro.falcato@gmail.com) Received: by mail-ua1-f47.google.com with SMTP id g2so440197uad.4 for ; Thu, 09 Sep 2021 21:35:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=2QU7r4lNuOrgvXRUWx8T+HrxtBC8yqGJ2In6L+xaWRI=; b=TTDi+uOlSJx69MW8dU5QUv87Jir2r5AeCKe30/jozkS5ATjFTEb9kTudMiL2o4+vBf s7gqH+YNK1keI5x8pg6uUtlpMDPkRPD/vz/KfYBitIGwV6TqcdmgogKQJPhyWQC29Lk6 MGpNYs3F4muVSWuyffENvykbCmD25aOgwkxUZ9e8c4t100jJNWvtv2UPTPuCLXUt3drT W62B6kHCZSorUllf+C4M0arQIO+0fNaUNeQPi9udkGsaRURIkX+RYKK76H70G4bOcqQf +j4ZCWcdsxqlWDcffmQSoKIaLkcFdkRzTUmkoDUOr3bc5kEzj5HuAqEmXM1+4o//75h9 /D8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=2QU7r4lNuOrgvXRUWx8T+HrxtBC8yqGJ2In6L+xaWRI=; b=XlMix4+LrUuxInuWYjY+VaEj0sPFIZLN7VqCi5ZKNJ7SESSl8tw/egcCvzWJ6C0zWY upsiSqWrU2aR3a6tOLMQQSjIaQLRJJlM+k6RsRLHz0qPp2P69ntr4iU683Eqiqiq58G7 vdeMUW/L8wuvNgL6jaEwnLxZ1bZizdZ8gPuxfI+kIP684wAuPrsjOl620JsaUSyKacXs b6ckHGah1qoCu/5cuBBx38yK9G/dtrU+nvVZwAQduIhMv7SjIcF7YXRnx/+Ho+7iLWrw 4Kp8aWZWtVQKIDpTscrluaCk0F1rpI+ReTW9BmRT2bqEhoyjOZ7UvP3pvL/f2OJMIY+V NIdw== X-Gm-Message-State: AOAM5302857pt1NwnFLeSFMfBkUm7ar81PXfeecfYE1zgoVusaFCD+uo 5SSIcUvFpaIrPuN1qJrWKYPrB2LeGjOLzzYooW8= X-Google-Smtp-Source: ABdhPJy56tJx4Y8KolW8m0jlVw0nNAcs9hyqKbTX8mPi7MXAabGEK2vJ5vQnX4KbHmwRLE+4Tr7HYM9asZsnzEung2w= X-Received: by 2002:a9f:3210:: with SMTP id x16mr4377861uad.135.1631248526822; Thu, 09 Sep 2021 21:35:26 -0700 (PDT) MIME-Version: 1.0 References: <36024a6bf15f115313e6fa134ee44dc6002de740.1631219610.git.jbrasen@nvidia.com> In-Reply-To: <36024a6bf15f115313e6fa134ee44dc6002de740.1631219610.git.jbrasen@nvidia.com> From: "Pedro Falcato" Date: Fri, 10 Sep 2021 05:35:16 +0100 Message-ID: Subject: Re: [PATCH 2/2] Ext4Pkg: Support non-cleanlty unmounted filesystems To: Jeff Brasen Cc: edk2-devel-groups-io Content-Type: text/plain; charset="UTF-8" Comments below. The patch itself also looks good. Commit message issue: typo on "non-cleanlty". On Thu, Sep 9, 2021 at 9:41 PM Jeff Brasen wrote: > > Support for uncleanly mounted filesystems, if there is a recovery > journal mark filesystem as read-only. > > Signed-off-by: Jeff Brasen > --- > Features/Ext4Pkg/Ext4Dxe/Superblock.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/Ext4Dxe/Superblock.c > index 1f8cdd3705..444dd3ca97 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c > +++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c > @@ -18,7 +18,7 @@ STATIC CONST UINT32 gSupportedIncompatFeat = > EXT4_FEATURE_INCOMPAT_64BIT | EXT4_FEATURE_INCOMPAT_DIRDATA | > EXT4_FEATURE_INCOMPAT_FLEX_BG | EXT4_FEATURE_INCOMPAT_FILETYPE | > EXT4_FEATURE_INCOMPAT_EXTENTS | EXT4_FEATURE_INCOMPAT_LARGEDIR | > - EXT4_FEATURE_INCOMPAT_MMP; > + EXT4_FEATURE_INCOMPAT_MMP | EXT4_FEATURE_INCOMPAT_RECOVER; > > // Future features that may be nice additions in the future: > // 1) Btree support: Required for write support and would speed up lookups in large directories. > @@ -88,10 +88,8 @@ Ext4SuperblockValidate ( > return FALSE; > } > > - // Is this correct behaviour? Imagine the power cuts out, should the system fail to boot because > - // we're scared of touching something corrupt? > if ((Sb->s_state & EXT4_FS_STATE_UNMOUNTED) == 0) { > - return FALSE; > + DEBUG ((DEBUG_WARN, "[ext4] filesystem was not unmounted cleanly\n")); Nitpick: filesystem should be capitalized. > } > > return TRUE; > @@ -214,6 +212,11 @@ Ext4OpenSuperblock ( > return EFI_UNSUPPORTED; > } > > + if ((Partition->FeaturesIncompat & EXT4_FEATURE_INCOMPAT_RECOVER) == EXT4_FEATURE_INCOMPAT_RECOVER) { New code that wants to test for features/feature-sets should use EXT4_HAS_INCOMPAT, EXT4_HAS_COMPAT, EXT4_HAS_RO_COMPAT. The code in this function that's testing for features using i.e: FeaturesIncompat & FEATURE manually should likely be replaced by the macros. > + DEBUG ((DEBUG_WARN, "[ext4] Needs journal recovery mount read-only\n")); The debug message looks poorly worded; something like "[ext4] Needs journal recovery, mounting read-only\n" sounds good? > + Partition->ReadOnly = TRUE; > + } > + > // At the time of writing, it's the only supported checksum. > if (Partition->FeaturesCompat & EXT4_FEATURE_RO_COMPAT_METADATA_CSUM && > Sb->s_checksum_type != EXT4_CHECKSUM_CRC32C) { > -- > 2.17.1 > -- Pedro Falcato