Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 19 Oct 2021 18:01:52 +0200
From:   Jan Kara <jack@suse.cz>
To:     Gabriel Krisman Bertazi <krisman@collabora.com>
Cc:     jack@suse.com, amir73il@gmail.com, djwong@kernel.org,
        tytso@mit.edu, david@fromorbit.com, dhowells@redhat.com,
        khazhy@google.com, linux-fsdevel@vger.kernel.org,
        linux-ext4@vger.kernel.org, linux-api@vger.kernel.org,
        kernel@collabora.com
Subject: Re: [PATCH v8 30/32] ext4: Send notifications on error
Message-ID: <20211019160152.GT3255@quack2.suse.cz>
References: <20211019000015.1666608-1-krisman@collabora.com>
 <20211019000015.1666608-31-krisman@collabora.com>
 <20211019154426.GR3255@quack2.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211019154426.GR3255@quack2.suse.cz>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Tue 19-10-21 17:44:26, Jan Kara wrote:
> On Mon 18-10-21 21:00:13, Gabriel Krisman Bertazi wrote:
> > Send a FS_ERROR message via fsnotify to a userspace monitoring tool
> > whenever a ext4 error condition is triggered.  This follows the existing
> > error conditions in ext4, so it is hooked to the ext4_error* functions.
> > 
> > It also follows the current dmesg reporting in the format.  The
> > filesystem message is composed mostly by the string that would be
> > otherwise printed in dmesg.
> > 
> > A new ext4 specific record format is exposed in the uapi, such that a
> > monitoring tool knows what to expect when listening errors of an ext4
> > filesystem.
> > 
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > Reviewed-by: Theodore Ts'o <tytso@mit.edu>
> > Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> 
> Looks good to me. Feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Hum, I actually retract this because the code doesn't match what is written
in the documentation and I'm not 100% sure what is correct. In particular:

> > @@ -759,6 +760,8 @@ void __ext4_error(struct super_block *sb, const char *function,
> >  		       sb->s_id, function, line, current->comm, &vaf);
> >  		va_end(args);
> >  	}
> > +	fsnotify_sb_error(sb, NULL, error);
> > +

E.g. here you pass the 'error' to fsnotify. This will be just standard
'errno' number, not ext4 error code as described in the documentation. Also
note that frequently 'error' will be 0 which gets magically transformed to
EFSCORRUPTED in save_error_info() in the ext4 error handling below. So
there's clearly some more work to do...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR