* [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
@ 2021-05-19 15:22 Lukas Herbolt
2021-05-20 15:23 ` Darrick J. Wong
2022-06-03 16:14 ` Eric Sandeen
0 siblings, 2 replies; 9+ messages in thread
From: Lukas Herbolt @ 2021-05-19 15:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Darrick J. Wong, linux-xfs, 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
2021-05-19 15:22 [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events Lukas Herbolt
@ 2021-05-20 15:23 ` Darrick J. Wong
2021-05-21 9:03 ` lukas
2022-06-03 16:14 ` Eric Sandeen
1 sibling, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2021-05-20 15:23 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: Christoph Hellwig, linux-xfs
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
2021-05-20 15:23 ` Darrick J. Wong
@ 2021-05-21 9:03 ` lukas
2021-05-21 16:06 ` Darrick J. Wong
0 siblings, 1 reply; 9+ messages in thread
From: lukas @ 2021-05-21 9:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
> 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
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
2021-05-21 9:03 ` lukas
@ 2021-05-21 16:06 ` Darrick J. Wong
2021-05-21 16:11 ` Eric Sandeen
2021-05-21 17:13 ` Lukas Herbolt
0 siblings, 2 replies; 9+ messages in thread
From: Darrick J. Wong @ 2021-05-21 16:06 UTC (permalink / raw)
To: lukas; +Cc: Christoph Hellwig, linux-xfs
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
> > >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
2021-05-21 16:06 ` Darrick J. Wong
@ 2021-05-21 16:11 ` Eric Sandeen
2021-05-21 17:13 ` Lukas Herbolt
1 sibling, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2021-05-21 16:11 UTC (permalink / raw)
To: Darrick J. Wong, lukas; +Cc: Christoph Hellwig, linux-xfs
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
2021-05-21 16:06 ` Darrick J. Wong
2021-05-21 16:11 ` Eric Sandeen
@ 2021-05-21 17:13 ` Lukas Herbolt
1 sibling, 0 replies; 9+ messages in thread
From: Lukas Herbolt @ 2021-05-21 17:13 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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
>> > >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
2021-05-19 15:22 [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events Lukas Herbolt
2021-05-20 15:23 ` Darrick J. Wong
@ 2022-06-03 16:14 ` Eric Sandeen
2022-06-04 0:34 ` Darrick J. Wong
1 sibling, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2022-06-03 16:14 UTC (permalink / raw)
To: Lukas Herbolt, Christoph Hellwig; +Cc: Darrick J. Wong, linux-xfs
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);
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
2022-06-03 16:14 ` Eric Sandeen
@ 2022-06-04 0:34 ` Darrick J. Wong
2022-06-06 14:05 ` Eric Sandeen
0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2022-06-04 0:34 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Lukas Herbolt, Christoph Hellwig, linux-xfs
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);
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events.
2022-06-04 0:34 ` Darrick J. Wong
@ 2022-06-06 14:05 ` Eric Sandeen
0 siblings, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2022-06-06 14:05 UTC (permalink / raw)
To: Darrick J. Wong, Eric Sandeen; +Cc: Lukas Herbolt, Christoph Hellwig, linux-xfs
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);
>>>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-06-06 14:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 15:22 [PATCH RFC v2] xfs: Print XFS UUID on mount and umount events Lukas Herbolt
2021-05-20 15:23 ` Darrick J. Wong
2021-05-21 9:03 ` lukas
2021-05-21 16:06 ` Darrick J. Wong
2021-05-21 16:11 ` Eric Sandeen
2021-05-21 17:13 ` Lukas Herbolt
2022-06-03 16:14 ` Eric Sandeen
2022-06-04 0:34 ` Darrick J. Wong
2022-06-06 14:05 ` Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.