[PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[Lukas Herbolt]

As of now only device names are printed out over __xfs_printk().
The device names are not persistent across reboots which in case
of searching for origin of corruption brings another task to properly
identify the devices. This patch add XFS UUID upon every mount/umount
event which will make the identification much easier.

Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
---
V2: Drop void casts and fix long lines

 fs/xfs/xfs_log.c   | 10 ++++++----
 fs/xfs/xfs_super.c |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 06041834daa31..8f4f671fd80d5 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -570,12 +570,14 @@ xfs_log_mount(
 	int		min_logfsbs;
 
 	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
-		xfs_notice(mp, "Mounting V%d Filesystem",
-			   XFS_SB_VERSION_NUM(&mp->m_sb));
+		xfs_notice(mp, "Mounting V%d Filesystem %pU",
+			   XFS_SB_VERSION_NUM(&mp->m_sb),
+			   &mp->m_sb.sb_uuid);
 	} else {
 		xfs_notice(mp,
-"Mounting V%d filesystem in no-recovery mode. Filesystem will be inconsistent.",
-			   XFS_SB_VERSION_NUM(&mp->m_sb));
+"Mounting V%d filesystem %pU in no-recovery mode. Filesystem will be inconsistent.",
+			   XFS_SB_VERSION_NUM(&mp->m_sb),
+			   &mp->m_sb.sb_uuid);
 		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
 	}
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index e5e0713bebcd8..a4b8a5ad8039f 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1043,7 +1043,7 @@ xfs_fs_put_super(
 	if (!sb->s_fs_info)
 		return;
 
-	xfs_notice(mp, "Unmounting Filesystem");
+	xfs_notice(mp, "Unmounting Filesystem %pU", &mp->m_sb.sb_uuid);
 	xfs_filestream_unmount(mp);
 	xfs_unmountfs(mp);
 
-- 
2.31.1

Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[Darrick J. Wong]

On Wed, May 19, 2021 at 05:22:48PM +0200, Lukas Herbolt wrote:
> As of now only device names are printed out over __xfs_printk().
> The device names are not persistent across reboots which in case
> of searching for origin of corruption brings another task to properly
> identify the devices. This patch add XFS UUID upon every mount/umount
> event which will make the identification much easier.

A few questions....

Are you going to wire up fs uuid logging for the other filesystems that
support them?

What happens w.r.t. uuid disambiguation if someone uses a nouuid mount
to mount a filesystem with the same uuid as an already-mounted xfs?

The changes themselves look ok, but I'm wondering what the use case is
here.

--D

> 
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
> V2: Drop void casts and fix long lines
> 
>  fs/xfs/xfs_log.c   | 10 ++++++----
>  fs/xfs/xfs_super.c |  2 +-
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 06041834daa31..8f4f671fd80d5 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -570,12 +570,14 @@ xfs_log_mount(
>  	int		min_logfsbs;
>  
>  	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
> -		xfs_notice(mp, "Mounting V%d Filesystem",
> -			   XFS_SB_VERSION_NUM(&mp->m_sb));
> +		xfs_notice(mp, "Mounting V%d Filesystem %pU",
> +			   XFS_SB_VERSION_NUM(&mp->m_sb),
> +			   &mp->m_sb.sb_uuid);
>  	} else {
>  		xfs_notice(mp,
> -"Mounting V%d filesystem in no-recovery mode. Filesystem will be inconsistent.",
> -			   XFS_SB_VERSION_NUM(&mp->m_sb));
> +"Mounting V%d filesystem %pU in no-recovery mode. Filesystem will be inconsistent.",
> +			   XFS_SB_VERSION_NUM(&mp->m_sb),
> +			   &mp->m_sb.sb_uuid);
>  		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
>  	}
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e5e0713bebcd8..a4b8a5ad8039f 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1043,7 +1043,7 @@ xfs_fs_put_super(
>  	if (!sb->s_fs_info)
>  		return;
>  
> -	xfs_notice(mp, "Unmounting Filesystem");
> +	xfs_notice(mp, "Unmounting Filesystem %pU", &mp->m_sb.sb_uuid);
>  	xfs_filestream_unmount(mp);
>  	xfs_unmountfs(mp);
>  
> -- 
> 2.31.1
> 

Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[lukas]

> Are you going to wire up fs uuid logging for the other filesystems that
> support them?
Well, I wasn't planning to but I can take a look on other FS as well
Ext4 and Btrfs for start.

> What happens w.r.t. uuid disambiguation if someone uses a nouuid mount
> to mount a filesystem with the same uuid as an already-mounted xfs?

I a not sure I understand the "nouuid mount". I don't think there can
be XFS with empty uuid value in SB. And printing the message is 
independent
on the mount method (mount UUID="" ...; mount /dev/sdX ...;).


On 20.05.2021 17:23, Darrick J. Wong wrote:
> On Wed, May 19, 2021 at 05:22:48PM +0200, Lukas Herbolt wrote:
>> As of now only device names are printed out over __xfs_printk().
>> The device names are not persistent across reboots which in case
>> of searching for origin of corruption brings another task to properly
>> identify the devices. This patch add XFS UUID upon every mount/umount
>> event which will make the identification much easier.
> 
> A few questions....
> 
> Are you going to wire up fs uuid logging for the other filesystems that
> support them?
> 
> What happens w.r.t. uuid disambiguation if someone uses a nouuid mount
> to mount a filesystem with the same uuid as an already-mounted xfs?
> 
> The changes themselves look ok, but I'm wondering what the use case is
> here.
> 
> --D
> 
>> 
>> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
>> ---
>> V2: Drop void casts and fix long lines
>> 
>>  fs/xfs/xfs_log.c   | 10 ++++++----
>>  fs/xfs/xfs_super.c |  2 +-
>>  2 files changed, 7 insertions(+), 5 deletions(-)
>> 
>> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
>> index 06041834daa31..8f4f671fd80d5 100644
>> --- a/fs/xfs/xfs_log.c
>> +++ b/fs/xfs/xfs_log.c
>> @@ -570,12 +570,14 @@ xfs_log_mount(
>>  	int		min_logfsbs;
>> 
>>  	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
>> -		xfs_notice(mp, "Mounting V%d Filesystem",
>> -			   XFS_SB_VERSION_NUM(&mp->m_sb));
>> +		xfs_notice(mp, "Mounting V%d Filesystem %pU",
>> +			   XFS_SB_VERSION_NUM(&mp->m_sb),
>> +			   &mp->m_sb.sb_uuid);
>>  	} else {
>>  		xfs_notice(mp,
>> -"Mounting V%d filesystem in no-recovery mode. Filesystem will be 
>> inconsistent.",
>> -			   XFS_SB_VERSION_NUM(&mp->m_sb));
>> +"Mounting V%d filesystem %pU in no-recovery mode. Filesystem will be 
>> inconsistent.",
>> +			   XFS_SB_VERSION_NUM(&mp->m_sb),
>> +			   &mp->m_sb.sb_uuid);
>>  		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
>>  	}
>> 
>> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
>> index e5e0713bebcd8..a4b8a5ad8039f 100644
>> --- a/fs/xfs/xfs_super.c
>> +++ b/fs/xfs/xfs_super.c
>> @@ -1043,7 +1043,7 @@ xfs_fs_put_super(
>>  	if (!sb->s_fs_info)
>>  		return;
>> 
>> -	xfs_notice(mp, "Unmounting Filesystem");
>> +	xfs_notice(mp, "Unmounting Filesystem %pU", &mp->m_sb.sb_uuid);
>>  	xfs_filestream_unmount(mp);
>>  	xfs_unmountfs(mp);
>> 
>> --
>> 2.31.1
>> 

Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[Darrick J. Wong]

On Fri, May 21, 2021 at 11:03:11AM +0200, lukas@herbolt.com wrote:
> > Are you going to wire up fs uuid logging for the other filesystems that
> > support them?
> Well, I wasn't planning to but I can take a look on other FS as well
> Ext4 and Btrfs for start.
> 
> > What happens w.r.t. uuid disambiguation if someone uses a nouuid mount
> > to mount a filesystem with the same uuid as an already-mounted xfs?
> 
> I a not sure I understand the "nouuid mount". I don't think there can
> be XFS with empty uuid value in SB. And printing the message is independent
> on the mount method (mount UUID="" ...; mount /dev/sdX ...;).

I meant specifically:

mount /dev/mapper/fubar /mnt
<snapshot fubar to fubar.bak>

Oh no, I deleted something in fubar, let's retrieve it from fubar.bak!

mount /dev/mapper/fubar.bak /opt	# fails because same uuid as fubar
mount /dev/mapper/fubar.bak /opt -o nouuid

--D

> 
> 
> On 20.05.2021 17:23, Darrick J. Wong wrote:
> > On Wed, May 19, 2021 at 05:22:48PM +0200, Lukas Herbolt wrote:
> > > As of now only device names are printed out over __xfs_printk().
> > > The device names are not persistent across reboots which in case
> > > of searching for origin of corruption brings another task to properly
> > > identify the devices. This patch add XFS UUID upon every mount/umount
> > > event which will make the identification much easier.
> > 
> > A few questions....
> > 
> > Are you going to wire up fs uuid logging for the other filesystems that
> > support them?
> > 
> > What happens w.r.t. uuid disambiguation if someone uses a nouuid mount
> > to mount a filesystem with the same uuid as an already-mounted xfs?
> > 
> > The changes themselves look ok, but I'm wondering what the use case is
> > here.
> > 
> > --D
> > 
> > > 
> > > Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> > > ---
> > > V2: Drop void casts and fix long lines
> > > 
> > >  fs/xfs/xfs_log.c   | 10 ++++++----
> > >  fs/xfs/xfs_super.c |  2 +-
> > >  2 files changed, 7 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > > index 06041834daa31..8f4f671fd80d5 100644
> > > --- a/fs/xfs/xfs_log.c
> > > +++ b/fs/xfs/xfs_log.c
> > > @@ -570,12 +570,14 @@ xfs_log_mount(
> > >  	int		min_logfsbs;
> > > 
> > >  	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
> > > -		xfs_notice(mp, "Mounting V%d Filesystem",
> > > -			   XFS_SB_VERSION_NUM(&mp->m_sb));
> > > +		xfs_notice(mp, "Mounting V%d Filesystem %pU",
> > > +			   XFS_SB_VERSION_NUM(&mp->m_sb),
> > > +			   &mp->m_sb.sb_uuid);
> > >  	} else {
> > >  		xfs_notice(mp,
> > > -"Mounting V%d filesystem in no-recovery mode. Filesystem will be
> > > inconsistent.",
> > > -			   XFS_SB_VERSION_NUM(&mp->m_sb));
> > > +"Mounting V%d filesystem %pU in no-recovery mode. Filesystem will
> > > be inconsistent.",
> > > +			   XFS_SB_VERSION_NUM(&mp->m_sb),
> > > +			   &mp->m_sb.sb_uuid);
> > >  		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
> > >  	}
> > > 
> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > index e5e0713bebcd8..a4b8a5ad8039f 100644
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -1043,7 +1043,7 @@ xfs_fs_put_super(
> > >  	if (!sb->s_fs_info)
> > >  		return;
> > > 
> > > -	xfs_notice(mp, "Unmounting Filesystem");
> > > +	xfs_notice(mp, "Unmounting Filesystem %pU", &mp->m_sb.sb_uuid);
> > >  	xfs_filestream_unmount(mp);
> > >  	xfs_unmountfs(mp);
> > > 
> > > --
> > > 2.31.1
> > > 

Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[Eric Sandeen]

On 5/21/21 11:06 AM, Darrick J. Wong wrote:
> On Fri, May 21, 2021 at 11:03:11AM +0200, lukas@herbolt.com wrote:
>>> Are you going to wire up fs uuid logging for the other filesystems that
>>> support them?
>> Well, I wasn't planning to but I can take a look on other FS as well
>> Ext4 and Btrfs for start.
>>
>>> What happens w.r.t. uuid disambiguation if someone uses a nouuid mount
>>> to mount a filesystem with the same uuid as an already-mounted xfs?
>>
>> I a not sure I understand the "nouuid mount". I don't think there can
>> be XFS with empty uuid value in SB. And printing the message is independent
>> on the mount method (mount UUID="" ...; mount /dev/sdX ...;).
> 
> I meant specifically:
> 
> mount /dev/mapper/fubar /mnt
> <snapshot fubar to fubar.bak>
> 
> Oh no, I deleted something in fubar, let's retrieve it from fubar.bak!
> 
> mount /dev/mapper/fubar.bak /opt	# fails because same uuid as fubar
> mount /dev/mapper/fubar.bak /opt -o nouuid

While it is possible to end up with 2 mounted devices with the same UUID,
that's still useful information I think; to me, the -o nouuid case doesn't
really argue against this patch.

In fact, it could be /very/ useful information if someone force-mounted
2 paths to the same underlying device by using -o nouuid.

-Eric

Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[Lukas Herbolt]

On 21.05.2021 18:06, Darrick J. Wong wrote:
> On Fri, May 21, 2021 at 11:03:11AM +0200, lukas@herbolt.com wrote:
>> > Are you going to wire up fs uuid logging for the other filesystems that
>> > support them?
>> Well, I wasn't planning to but I can take a look on other FS as well
>> Ext4 and Btrfs for start.
>> 
>> > What happens w.r.t. uuid disambiguation if someone uses a nouuid mount
>> > to mount a filesystem with the same uuid as an already-mounted xfs?
>> 
>> I a not sure I understand the "nouuid mount". I don't think there can
>> be XFS with empty uuid value in SB. And printing the message is 
>> independent
>> on the mount method (mount UUID="" ...; mount /dev/sdX ...;).
> 
> I meant specifically:
> 
> mount /dev/mapper/fubar /mnt
> <snapshot fubar to fubar.bak>
> 
> Oh no, I deleted something in fubar, let's retrieve it from fubar.bak!
> 
> mount /dev/mapper/fubar.bak /opt	# fails because same uuid as fubar
> mount /dev/mapper/fubar.bak /opt -o nouuid
> 
> --D
> 

Ah, right. Well using -o nouuid might have it's own info/notice in the 
logs
to make it clear.

>> 
>> 
>> On 20.05.2021 17:23, Darrick J. Wong wrote:
>> > On Wed, May 19, 2021 at 05:22:48PM +0200, Lukas Herbolt wrote:
>> > > As of now only device names are printed out over __xfs_printk().
>> > > The device names are not persistent across reboots which in case
>> > > of searching for origin of corruption brings another task to properly
>> > > identify the devices. This patch add XFS UUID upon every mount/umount
>> > > event which will make the identification much easier.
>> >
>> > A few questions....
>> >
>> > Are you going to wire up fs uuid logging for the other filesystems that
>> > support them?
>> >
>> > What happens w.r.t. uuid disambiguation if someone uses a nouuid mount
>> > to mount a filesystem with the same uuid as an already-mounted xfs?
>> >
>> > The changes themselves look ok, but I'm wondering what the use case is
>> > here.
>> >
>> > --D
>> >
>> > >
>> > > Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
>> > > ---
>> > > V2: Drop void casts and fix long lines
>> > >
>> > >  fs/xfs/xfs_log.c   | 10 ++++++----
>> > >  fs/xfs/xfs_super.c |  2 +-
>> > >  2 files changed, 7 insertions(+), 5 deletions(-)
>> > >
>> > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
>> > > index 06041834daa31..8f4f671fd80d5 100644
>> > > --- a/fs/xfs/xfs_log.c
>> > > +++ b/fs/xfs/xfs_log.c
>> > > @@ -570,12 +570,14 @@ xfs_log_mount(
>> > >  	int		min_logfsbs;
>> > >
>> > >  	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
>> > > -		xfs_notice(mp, "Mounting V%d Filesystem",
>> > > -			   XFS_SB_VERSION_NUM(&mp->m_sb));
>> > > +		xfs_notice(mp, "Mounting V%d Filesystem %pU",
>> > > +			   XFS_SB_VERSION_NUM(&mp->m_sb),
>> > > +			   &mp->m_sb.sb_uuid);
>> > >  	} else {
>> > >  		xfs_notice(mp,
>> > > -"Mounting V%d filesystem in no-recovery mode. Filesystem will be
>> > > inconsistent.",
>> > > -			   XFS_SB_VERSION_NUM(&mp->m_sb));
>> > > +"Mounting V%d filesystem %pU in no-recovery mode. Filesystem will
>> > > be inconsistent.",
>> > > +			   XFS_SB_VERSION_NUM(&mp->m_sb),
>> > > +			   &mp->m_sb.sb_uuid);
>> > >  		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
>> > >  	}
>> > >
>> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
>> > > index e5e0713bebcd8..a4b8a5ad8039f 100644
>> > > --- a/fs/xfs/xfs_super.c
>> > > +++ b/fs/xfs/xfs_super.c
>> > > @@ -1043,7 +1043,7 @@ xfs_fs_put_super(
>> > >  	if (!sb->s_fs_info)
>> > >  		return;
>> > >
>> > > -	xfs_notice(mp, "Unmounting Filesystem");
>> > > +	xfs_notice(mp, "Unmounting Filesystem %pU", &mp->m_sb.sb_uuid);
>> > >  	xfs_filestream_unmount(mp);
>> > >  	xfs_unmountfs(mp);
>> > >
>> > > --
>> > > 2.31.1
>> > >

Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[Eric Sandeen]

On 5/19/21 10:22 AM, Lukas Herbolt wrote:
> As of now only device names are printed out over __xfs_printk().
> The device names are not persistent across reboots which in case
> of searching for origin of corruption brings another task to properly
> identify the devices. This patch add XFS UUID upon every mount/umount
> event which will make the identification much easier.
> 
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
> V2: Drop void casts and fix long lines

Can we revisit this? I think it's a nice enhancement.

The "nouuid" concern raised in the thread doesn't seem like a problem;
if someone mounts with "-o nouuid" then you'll see 2 different devices
mounted with the same uuid printed. I don't think that's an argument
against the patch. Printing the uuid still provides more info than not.

I, uh, also don't think the submitter should be required to do a tree-wide
change for an xfs printk enhancement. Sure, it'd be nice to have ext4
and btrfs and and and but we have no other requirements that mount-time
messages must be consistent across all filesystems....

Thanks,
-Eric

> 
>  fs/xfs/xfs_log.c   | 10 ++++++----
>  fs/xfs/xfs_super.c |  2 +-
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 06041834daa31..8f4f671fd80d5 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -570,12 +570,14 @@ xfs_log_mount(
>  	int		min_logfsbs;
>  
>  	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
> -		xfs_notice(mp, "Mounting V%d Filesystem",
> -			   XFS_SB_VERSION_NUM(&mp->m_sb));
> +		xfs_notice(mp, "Mounting V%d Filesystem %pU",
> +			   XFS_SB_VERSION_NUM(&mp->m_sb),
> +			   &mp->m_sb.sb_uuid);
>  	} else {
>  		xfs_notice(mp,
> -"Mounting V%d filesystem in no-recovery mode. Filesystem will be inconsistent.",
> -			   XFS_SB_VERSION_NUM(&mp->m_sb));
> +"Mounting V%d filesystem %pU in no-recovery mode. Filesystem will be inconsistent.",
> +			   XFS_SB_VERSION_NUM(&mp->m_sb),
> +			   &mp->m_sb.sb_uuid);
>  		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
>  	}
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e5e0713bebcd8..a4b8a5ad8039f 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1043,7 +1043,7 @@ xfs_fs_put_super(
>  	if (!sb->s_fs_info)
>  		return;
>  
> -	xfs_notice(mp, "Unmounting Filesystem");
> +	xfs_notice(mp, "Unmounting Filesystem %pU", &mp->m_sb.sb_uuid);
>  	xfs_filestream_unmount(mp);
>  	xfs_unmountfs(mp);
>  

Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[Darrick J. Wong]

On Fri, Jun 03, 2022 at 11:14:13AM -0500, Eric Sandeen wrote:
> On 5/19/21 10:22 AM, Lukas Herbolt wrote:
> > As of now only device names are printed out over __xfs_printk().
> > The device names are not persistent across reboots which in case
> > of searching for origin of corruption brings another task to properly
> > identify the devices. This patch add XFS UUID upon every mount/umount
> > event which will make the identification much easier.
> > 
> > Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> > ---
> > V2: Drop void casts and fix long lines
> 
> Can we revisit this? I think it's a nice enhancement.
> 
> The "nouuid" concern raised in the thread doesn't seem like a problem;
> if someone mounts with "-o nouuid" then you'll see 2 different devices
> mounted with the same uuid printed. I don't think that's an argument
> against the patch. Printing the uuid still provides more info than not.

Ok fair.

> I, uh, also don't think the submitter should be required to do a tree-wide
> change for an xfs printk enhancement. Sure, it'd be nice to have ext4
> and btrfs and and and but we have no other requirements that mount-time
> messages must be consistent across all filesystems....

As you pointed out on irc, btrfs already prints its own uuids.  So that
leaves ext4 -- are you all planning to send a patch for that?

(Otherwise, I don't mind this patch, if it helps support perform
forensics on systems with a lot of filesystem activity.)

> Thanks,
> -Eric
> 
> > 
> >  fs/xfs/xfs_log.c   | 10 ++++++----
> >  fs/xfs/xfs_super.c |  2 +-
> >  2 files changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > index 06041834daa31..8f4f671fd80d5 100644
> > --- a/fs/xfs/xfs_log.c
> > +++ b/fs/xfs/xfs_log.c
> > @@ -570,12 +570,14 @@ xfs_log_mount(
> >  	int		min_logfsbs;
> >  
> >  	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
> > -		xfs_notice(mp, "Mounting V%d Filesystem",
> > -			   XFS_SB_VERSION_NUM(&mp->m_sb));
> > +		xfs_notice(mp, "Mounting V%d Filesystem %pU",
> > +			   XFS_SB_VERSION_NUM(&mp->m_sb),
> > +			   &mp->m_sb.sb_uuid);
> >  	} else {
> >  		xfs_notice(mp,
> > -"Mounting V%d filesystem in no-recovery mode. Filesystem will be inconsistent.",
> > -			   XFS_SB_VERSION_NUM(&mp->m_sb));
> > +"Mounting V%d filesystem %pU in no-recovery mode. Filesystem will be inconsistent.",
> > +			   XFS_SB_VERSION_NUM(&mp->m_sb),
> > +			   &mp->m_sb.sb_uuid);

sb_uuid is the uuid that the user can set, not the one that's encoded
identically in all the cloud vm images, right?

--D

> >  		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
> >  	}
> >  
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index e5e0713bebcd8..a4b8a5ad8039f 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -1043,7 +1043,7 @@ xfs_fs_put_super(
> >  	if (!sb->s_fs_info)
> >  		return;
> >  
> > -	xfs_notice(mp, "Unmounting Filesystem");
> > +	xfs_notice(mp, "Unmounting Filesystem %pU", &mp->m_sb.sb_uuid);
> >  	xfs_filestream_unmount(mp);
> >  	xfs_unmountfs(mp);
> >  

Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.

[Eric Sandeen]

On 6/3/22 7:34 PM, Darrick J. Wong wrote:
> On Fri, Jun 03, 2022 at 11:14:13AM -0500, Eric Sandeen wrote:
>> On 5/19/21 10:22 AM, Lukas Herbolt wrote:
>>> As of now only device names are printed out over __xfs_printk().
>>> The device names are not persistent across reboots which in case
>>> of searching for origin of corruption brings another task to properly
>>> identify the devices. This patch add XFS UUID upon every mount/umount
>>> event which will make the identification much easier.
>>>
>>> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
>>> ---
>>> V2: Drop void casts and fix long lines
>>
>> Can we revisit this? I think it's a nice enhancement.
>>
>> The "nouuid" concern raised in the thread doesn't seem like a problem;
>> if someone mounts with "-o nouuid" then you'll see 2 different devices
>> mounted with the same uuid printed. I don't think that's an argument
>> against the patch. Printing the uuid still provides more info than not.
> 
> Ok fair.
> 
>> I, uh, also don't think the submitter should be required to do a tree-wide
>> change for an xfs printk enhancement. Sure, it'd be nice to have ext4
>> and btrfs and and and but we have no other requirements that mount-time
>> messages must be consistent across all filesystems....
> 
> As you pointed out on irc, btrfs already prints its own uuids.  So that
> leaves ext4 -- are you all planning to send a patch for that?

I threw one together, need to actually test and send it, but sure.

> (Otherwise, I don't mind this patch, if it helps support perform
> forensics on systems with a lot of filesystem activity.)

Cool, thanks.

>> Thanks,
>> -Eric
>>
>>>
>>>  fs/xfs/xfs_log.c   | 10 ++++++----
>>>  fs/xfs/xfs_super.c |  2 +-
>>>  2 files changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
>>> index 06041834daa31..8f4f671fd80d5 100644
>>> --- a/fs/xfs/xfs_log.c
>>> +++ b/fs/xfs/xfs_log.c
>>> @@ -570,12 +570,14 @@ xfs_log_mount(
>>>  	int		min_logfsbs;
>>>  
>>>  	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
>>> -		xfs_notice(mp, "Mounting V%d Filesystem",
>>> -			   XFS_SB_VERSION_NUM(&mp->m_sb));
>>> +		xfs_notice(mp, "Mounting V%d Filesystem %pU",
>>> +			   XFS_SB_VERSION_NUM(&mp->m_sb),
>>> +			   &mp->m_sb.sb_uuid);
>>>  	} else {
>>>  		xfs_notice(mp,
>>> -"Mounting V%d filesystem in no-recovery mode. Filesystem will be inconsistent.",
>>> -			   XFS_SB_VERSION_NUM(&mp->m_sb));
>>> +"Mounting V%d filesystem %pU in no-recovery mode. Filesystem will be inconsistent.",
>>> +			   XFS_SB_VERSION_NUM(&mp->m_sb),
>>> +			   &mp->m_sb.sb_uuid);
> 
> sb_uuid is the uuid that the user can set, not the one that's encoded
> identically in all the cloud vm images, right?

Uhhh yeah I think so, and "sb_meta_uuid" is the internal one that remains
unchanged but I will double check because even though I wrote that, it's
un-intuitive. :(

Thanks,
-Eric

> --D
> 
>>>  		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
>>>  	}
>>>  
>>> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
>>> index e5e0713bebcd8..a4b8a5ad8039f 100644
>>> --- a/fs/xfs/xfs_super.c
>>> +++ b/fs/xfs/xfs_super.c
>>> @@ -1043,7 +1043,7 @@ xfs_fs_put_super(
>>>  	if (!sb->s_fs_info)
>>>  		return;
>>>  
>>> -	xfs_notice(mp, "Unmounting Filesystem");
>>> +	xfs_notice(mp, "Unmounting Filesystem %pU", &mp->m_sb.sb_uuid);
>>>  	xfs_filestream_unmount(mp);
>>>  	xfs_unmountfs(mp);
>>>  
> 
>