* [PATCHSET 0/3] populate: fix a few bugs with fs pre-population
@ 2021-03-23 4:19 Darrick J. Wong
2021-03-23 4:19 ` [PATCH 1/3] populate: create block devices when pre-populating filesystems Darrick J. Wong
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Darrick J. Wong @ 2021-03-23 4:19 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
Hi all,
While I was auditing the efficacy of the xfs repair tools I noticed a
few deficiencies in the code that populates filesystems, so I fixed
them. Most notable are the fact that we didn't create fifos, messed up
blockdev creation, and while the cache tags should record the size of
external devices, the actual device names aren't exciting.
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This is an extraordinary way to destroy everything. Enjoy!
Comments and questions are, as always, welcome.
--D
kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes
xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes
fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
---
common/populate | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] populate: create block devices when pre-populating filesystems
2021-03-23 4:19 [PATCHSET 0/3] populate: fix a few bugs with fs pre-population Darrick J. Wong
@ 2021-03-23 4:19 ` Darrick J. Wong
2021-03-24 18:03 ` Christoph Hellwig
2021-03-23 4:19 ` [PATCH 2/3] common/populate: create fifos " Darrick J. Wong
2021-03-23 4:19 ` [PATCH 3/3] common/populate: change how we describe cached populated images Darrick J. Wong
2 siblings, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2021-03-23 4:19 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
I just noticed that the fs population helper creates a chardev file
"S_IFBLK" on the scratch filesystem. This seems bogus (particularly
since we actually also create a chardev named S_IFCHR) so fix up the
mknod calls.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
common/populate | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/populate b/common/populate
index 4135d89d..8f42a528 100644
--- a/common/populate
+++ b/common/populate
@@ -230,7 +230,7 @@ _scratch_xfs_populate() {
# Char & block
echo "+ special"
mknod "${SCRATCH_MNT}/S_IFCHR" c 1 1
- mknod "${SCRATCH_MNT}/S_IFBLK" c 1 1
+ mknod "${SCRATCH_MNT}/S_IFBLK" b 1 1
# special file with an xattr
setfacl -P -m u:nobody:r ${SCRATCH_MNT}/S_IFCHR
@@ -402,7 +402,7 @@ _scratch_ext4_populate() {
# Char & block
echo "+ special"
mknod "${SCRATCH_MNT}/S_IFCHR" c 1 1
- mknod "${SCRATCH_MNT}/S_IFBLK" c 1 1
+ mknod "${SCRATCH_MNT}/S_IFBLK" b 1 1
# special file with an xattr
setfacl -P -m u:nobody:r ${SCRATCH_MNT}/S_IFCHR
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] common/populate: create fifos when pre-populating filesystems
2021-03-23 4:19 [PATCHSET 0/3] populate: fix a few bugs with fs pre-population Darrick J. Wong
2021-03-23 4:19 ` [PATCH 1/3] populate: create block devices when pre-populating filesystems Darrick J. Wong
@ 2021-03-23 4:19 ` Darrick J. Wong
2021-03-24 18:05 ` Christoph Hellwig
2021-03-23 4:19 ` [PATCH 3/3] common/populate: change how we describe cached populated images Darrick J. Wong
2 siblings, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2021-03-23 4:19 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
Create fifos when populating the scratch filesystem for completeness.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
common/populate | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/common/populate b/common/populate
index 8f42a528..c01b7e0e 100644
--- a/common/populate
+++ b/common/populate
@@ -231,6 +231,7 @@ _scratch_xfs_populate() {
echo "+ special"
mknod "${SCRATCH_MNT}/S_IFCHR" c 1 1
mknod "${SCRATCH_MNT}/S_IFBLK" b 1 1
+ mknod "${SCRATCH_MNT}/S_IFIFO" p
# special file with an xattr
setfacl -P -m u:nobody:r ${SCRATCH_MNT}/S_IFCHR
@@ -403,6 +404,7 @@ _scratch_ext4_populate() {
echo "+ special"
mknod "${SCRATCH_MNT}/S_IFCHR" c 1 1
mknod "${SCRATCH_MNT}/S_IFBLK" b 1 1
+ mknod "${SCRATCH_MNT}/S_IFIFO" p
# special file with an xattr
setfacl -P -m u:nobody:r ${SCRATCH_MNT}/S_IFCHR
@@ -580,6 +582,7 @@ _scratch_xfs_populate_check() {
extents_slink="$(__populate_find_inode "${SCRATCH_MNT}/S_IFLNK.FMT_EXTENTS")"
bdev="$(__populate_find_inode "${SCRATCH_MNT}/S_IFBLK")"
cdev="$(__populate_find_inode "${SCRATCH_MNT}/S_IFCHR")"
+ fifo="$(__populate_find_inode "${SCRATCH_MNT}/S_IFIFO")"
local_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_LOCAL")"
leaf_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_LEAF")"
node_attr="$(__populate_find_inode "${SCRATCH_MNT}/ATTR.FMT_NODE")"
@@ -605,6 +608,7 @@ _scratch_xfs_populate_check() {
__populate_check_xfs_dformat "${btree_dir}" "btree"
__populate_check_xfs_dformat "${bdev}" "dev"
__populate_check_xfs_dformat "${cdev}" "dev"
+ __populate_check_xfs_dformat "${fifo}" "dev"
__populate_check_xfs_attr "${local_attr}" "local"
__populate_check_xfs_attr "${leaf_attr}" "leaf"
__populate_check_xfs_attr "${node_attr}" "node"
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] common/populate: change how we describe cached populated images
2021-03-23 4:19 [PATCHSET 0/3] populate: fix a few bugs with fs pre-population Darrick J. Wong
2021-03-23 4:19 ` [PATCH 1/3] populate: create block devices when pre-populating filesystems Darrick J. Wong
2021-03-23 4:19 ` [PATCH 2/3] common/populate: create fifos " Darrick J. Wong
@ 2021-03-23 4:19 ` Darrick J. Wong
2021-03-24 18:11 ` Christoph Hellwig
2021-03-24 18:17 ` [PATCH v1.1 " Darrick J. Wong
2 siblings, 2 replies; 10+ messages in thread
From: Darrick J. Wong @ 2021-03-23 4:19 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
The device name of a secondary storage device isn't all that important,
but the size is.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
common/populate | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/common/populate b/common/populate
index c01b7e0e..94bf5ce9 100644
--- a/common/populate
+++ b/common/populate
@@ -808,13 +808,23 @@ _fill_fs()
_scratch_populate_cache_tag() {
local extra_descr=""
local size="$(blockdev --getsz "${SCRATCH_DEV}")"
+ local logdev="none"
+ local rtdev="none"
+
+ if [ "${USE_EXTERNAL}" = "yes" ] && [ -n "${SCRATCH_LOGDEV}" ]; then
+ logdev="$(blockdev --getsz "${SCRATCH_LOGDEV}")"
+ fi
+
+ if [ "${USE_EXTERNAL}" = "yes" ] && [ -n "${SCRATCH_RTDEV}" ]; then
+ rtdev="$(blockdev --getsz "${SCRATCH_RTDEV}")"
+ fi
case "${FSTYP}" in
"ext4")
- extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL}"
+ extra_descr="LOGDEV_SIZE ${logdev}"
;;
"xfs")
- extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}"
+ extra_descr="LOGDEV_SIZE ${logdev} RTDEV_SIZE ${rtdev}"
_populate_xfs_qmount_option
if echo "${MOUNT_OPTIONS}" | grep -q 'usrquota'; then
extra_descr="${extra_descr} QUOTAS"
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] populate: create block devices when pre-populating filesystems
2021-03-23 4:19 ` [PATCH 1/3] populate: create block devices when pre-populating filesystems Darrick J. Wong
@ 2021-03-24 18:03 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2021-03-24 18:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
On Mon, Mar 22, 2021 at 09:19:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> I just noticed that the fs population helper creates a chardev file
> "S_IFBLK" on the scratch filesystem. This seems bogus (particularly
> since we actually also create a chardev named S_IFCHR) so fix up the
> mknod calls.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] common/populate: create fifos when pre-populating filesystems
2021-03-23 4:19 ` [PATCH 2/3] common/populate: create fifos " Darrick J. Wong
@ 2021-03-24 18:05 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2021-03-24 18:05 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
On Mon, Mar 22, 2021 at 09:19:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Create fifos when populating the scratch filesystem for completeness.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] common/populate: change how we describe cached populated images
2021-03-23 4:19 ` [PATCH 3/3] common/populate: change how we describe cached populated images Darrick J. Wong
@ 2021-03-24 18:11 ` Christoph Hellwig
2021-03-24 18:15 ` Darrick J. Wong
2021-03-24 18:17 ` [PATCH v1.1 " Darrick J. Wong
1 sibling, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2021-03-24 18:11 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
On Mon, Mar 22, 2021 at 09:19:59PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> The device name of a secondary storage device isn't all that important,
> but the size is.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> common/populate | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
>
> diff --git a/common/populate b/common/populate
> index c01b7e0e..94bf5ce9 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -808,13 +808,23 @@ _fill_fs()
> _scratch_populate_cache_tag() {
> local extra_descr=""
> local size="$(blockdev --getsz "${SCRATCH_DEV}")"
> + local logdev="none"
> + local rtdev="none"
> +
> + if [ "${USE_EXTERNAL}" = "yes" ] && [ -n "${SCRATCH_LOGDEV}" ]; then
> + logdev="$(blockdev --getsz "${SCRATCH_LOGDEV}")"
> + fi
> +
> + if [ "${USE_EXTERNAL}" = "yes" ] && [ -n "${SCRATCH_RTDEV}" ]; then
> + rtdev="$(blockdev --getsz "${SCRATCH_RTDEV}")"
Shouldn't these variables be called LOGDEV_SIZE and RTDEV_SIZE?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] common/populate: change how we describe cached populated images
2021-03-24 18:11 ` Christoph Hellwig
@ 2021-03-24 18:15 ` Darrick J. Wong
0 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2021-03-24 18:15 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: guaneryu, linux-xfs, fstests, guan
On Wed, Mar 24, 2021 at 06:11:57PM +0000, Christoph Hellwig wrote:
> On Mon, Mar 22, 2021 at 09:19:59PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > The device name of a secondary storage device isn't all that important,
> > but the size is.
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > common/populate | 14 ++++++++++++--
> > 1 file changed, 12 insertions(+), 2 deletions(-)
> >
> >
> > diff --git a/common/populate b/common/populate
> > index c01b7e0e..94bf5ce9 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -808,13 +808,23 @@ _fill_fs()
> > _scratch_populate_cache_tag() {
> > local extra_descr=""
> > local size="$(blockdev --getsz "${SCRATCH_DEV}")"
> > + local logdev="none"
> > + local rtdev="none"
> > +
> > + if [ "${USE_EXTERNAL}" = "yes" ] && [ -n "${SCRATCH_LOGDEV}" ]; then
> > + logdev="$(blockdev --getsz "${SCRATCH_LOGDEV}")"
> > + fi
> > +
> > + if [ "${USE_EXTERNAL}" = "yes" ] && [ -n "${SCRATCH_RTDEV}" ]; then
> > + rtdev="$(blockdev --getsz "${SCRATCH_RTDEV}")"
>
> Shouldn't these variables be called LOGDEV_SIZE and RTDEV_SIZE?
Oops, yeah.
--D
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1.1 3/3] common/populate: change how we describe cached populated images
2021-03-23 4:19 ` [PATCH 3/3] common/populate: change how we describe cached populated images Darrick J. Wong
2021-03-24 18:11 ` Christoph Hellwig
@ 2021-03-24 18:17 ` Darrick J. Wong
2021-03-24 18:22 ` Christoph Hellwig
1 sibling, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2021-03-24 18:17 UTC (permalink / raw)
To: guaneryu; +Cc: linux-xfs, fstests, guan, hch
From: Darrick J. Wong <djwong@kernel.org>
The device name of a secondary storage device isn't all that important,
but the size is.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
v1.1: fix variable names
---
common/populate | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/common/populate b/common/populate
index c01b7e0e..d484866a 100644
--- a/common/populate
+++ b/common/populate
@@ -808,13 +808,23 @@ _fill_fs()
_scratch_populate_cache_tag() {
local extra_descr=""
local size="$(blockdev --getsz "${SCRATCH_DEV}")"
+ local logdev_sz="none"
+ local rtdev_sz="none"
+
+ if [ "${USE_EXTERNAL}" = "yes" ] && [ -n "${SCRATCH_LOGDEV}" ]; then
+ logdev_sz="$(blockdev --getsz "${SCRATCH_LOGDEV}")"
+ fi
+
+ if [ "${USE_EXTERNAL}" = "yes" ] && [ -n "${SCRATCH_RTDEV}" ]; then
+ rtdev_sz="$(blockdev --getsz "${SCRATCH_RTDEV}")"
+ fi
case "${FSTYP}" in
"ext4")
- extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL}"
+ extra_descr="LOGDEV_SIZE ${logdev_sz}"
;;
"xfs")
- extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}"
+ extra_descr="LOGDEV_SIZE ${logdev_sz} RTDEV_SIZE ${rtdev_sz}"
_populate_xfs_qmount_option
if echo "${MOUNT_OPTIONS}" | grep -q 'usrquota'; then
extra_descr="${extra_descr} QUOTAS"
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1.1 3/3] common/populate: change how we describe cached populated images
2021-03-24 18:17 ` [PATCH v1.1 " Darrick J. Wong
@ 2021-03-24 18:22 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2021-03-24 18:22 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan, hch
On Wed, Mar 24, 2021 at 11:17:48AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> The device name of a secondary storage device isn't all that important,
> but the size is.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-24 18:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 4:19 [PATCHSET 0/3] populate: fix a few bugs with fs pre-population Darrick J. Wong
2021-03-23 4:19 ` [PATCH 1/3] populate: create block devices when pre-populating filesystems Darrick J. Wong
2021-03-24 18:03 ` Christoph Hellwig
2021-03-23 4:19 ` [PATCH 2/3] common/populate: create fifos " Darrick J. Wong
2021-03-24 18:05 ` Christoph Hellwig
2021-03-23 4:19 ` [PATCH 3/3] common/populate: change how we describe cached populated images Darrick J. Wong
2021-03-24 18:11 ` Christoph Hellwig
2021-03-24 18:15 ` Darrick J. Wong
2021-03-24 18:17 ` [PATCH v1.1 " Darrick J. Wong
2021-03-24 18:22 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).