| Message ID | 20250212205448.2107005-2-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | enable bs > ps device testing | expand |
On 2/12/25 12:54 PM, Luis Chamberlain wrote: > We want to help capture error messages with _xfs_mkfs_and_mount() on > $FULL, to do that we should avoid spamming error messages for things > which we know are not fatal. Such is the case of when we try to > mkfs a filesystem but before that try to umount the target path. > The first umount is just for sanity, so ignore the error messages from > it. > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > common/xfs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/xfs b/common/xfs > index 569770fecd53..67a3b8a97391 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -15,7 +15,7 @@ _xfs_mkfs_and_mount() { > local mount_dir=$2 > > mkdir -p "${mount_dir}" > - umount "${mount_dir}" > + umount "${mount_dir}" >/dev/null 2>&1 > mkfs.xfs -l size=64m -f "${bdev}" || return $? > mount "${bdev}" "${mount_dir}" > } Shouldn't ">/dev/null 2>&1" be added to the _xfs_mkfs_and_mount() caller rather than inside the _xfs_mkfs_and_mount() implementation? The above patch makes it impossible for any caller to capture the stdout output of umount. Thanks, Bart.
On Wed, Feb 12, 2025 at 12:58:36PM -0800, Bart Van Assche wrote: > On 2/12/25 12:54 PM, Luis Chamberlain wrote: > > We want to help capture error messages with _xfs_mkfs_and_mount() on > > $FULL, to do that we should avoid spamming error messages for things > > which we know are not fatal. Such is the case of when we try to > > mkfs a filesystem but before that try to umount the target path. > > The first umount is just for sanity, so ignore the error messages from > > it. > > > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > > --- > > common/xfs | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/common/xfs b/common/xfs > > index 569770fecd53..67a3b8a97391 100644 > > --- a/common/xfs > > +++ b/common/xfs > > @@ -15,7 +15,7 @@ _xfs_mkfs_and_mount() { > > local mount_dir=$2 > > mkdir -p "${mount_dir}" > > - umount "${mount_dir}" > > + umount "${mount_dir}" >/dev/null 2>&1 > > mkfs.xfs -l size=64m -f "${bdev}" || return $? > > mount "${bdev}" "${mount_dir}" > > } > > Shouldn't ">/dev/null 2>&1" be added to the _xfs_mkfs_and_mount() > caller rather than inside the _xfs_mkfs_and_mount() implementation? > The above patch makes it impossible for any caller to capture the > stdout output of umount. That is the point. In this case the umount *can* fail, we do it for sanity purposes. We don't care if umount failed. Luis
On Feb 12, 2025 / 13:07, Luis Chamberlain wrote: > On Wed, Feb 12, 2025 at 12:58:36PM -0800, Bart Van Assche wrote: > > On 2/12/25 12:54 PM, Luis Chamberlain wrote: > > > We want to help capture error messages with _xfs_mkfs_and_mount() on > > > $FULL, to do that we should avoid spamming error messages for things > > > which we know are not fatal. Such is the case of when we try to > > > mkfs a filesystem but before that try to umount the target path. > > > The first umount is just for sanity, so ignore the error messages from > > > it. > > > > > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > > > --- > > > common/xfs | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/common/xfs b/common/xfs > > > index 569770fecd53..67a3b8a97391 100644 > > > --- a/common/xfs > > > +++ b/common/xfs > > > @@ -15,7 +15,7 @@ _xfs_mkfs_and_mount() { > > > local mount_dir=$2 > > > mkdir -p "${mount_dir}" > > > - umount "${mount_dir}" > > > + umount "${mount_dir}" >/dev/null 2>&1 > > > mkfs.xfs -l size=64m -f "${bdev}" || return $? > > > mount "${bdev}" "${mount_dir}" > > > } > > > > Shouldn't ">/dev/null 2>&1" be added to the _xfs_mkfs_and_mount() > > caller rather than inside the _xfs_mkfs_and_mount() implementation? > > The above patch makes it impossible for any caller to capture the > > stdout output of umount. > > That is the point. In this case the umount *can* fail, we do it for > sanity purposes. We don't care if umount failed. > > Luis IMO, umount --quiet option will help instead of redirect to /dev/null. In most cases, the umount command just ensures that nothing is mounted at ${mount_dir}, and prints the "not mounted" error message. The --quiet option will suppress it, and I guess it will address the problem Luis faces. With this, still we can keep other error messages printed when anything unexpected happens.
diff --git a/common/xfs b/common/xfs index 569770fecd53..67a3b8a97391 100644 --- a/common/xfs +++ b/common/xfs @@ -15,7 +15,7 @@ _xfs_mkfs_and_mount() { local mount_dir=$2 mkdir -p "${mount_dir}" - umount "${mount_dir}" + umount "${mount_dir}" >/dev/null 2>&1 mkfs.xfs -l size=64m -f "${bdev}" || return $? mount "${bdev}" "${mount_dir}" }
We want to help capture error messages with _xfs_mkfs_and_mount() on $FULL, to do that we should avoid spamming error messages for things which we know are not fatal. Such is the case of when we try to mkfs a filesystem but before that try to umount the target path. The first umount is just for sanity, so ignore the error messages from it. Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- common/xfs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)