Message ID | 166553912788.422450.6797363004980943410.stgit@magnolia (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fstests: support external logs when caching prepopulated images | expand |
On Tue, Oct 11, 2022 at 06:45:27PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Not sure why this hasn't been broken all along, but we should be > exporting this variable so that it shows up in subshells.... May I ask where's the subshell which uses $POPULATE_METADUMP? > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > common/populate | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > > diff --git a/common/populate b/common/populate > index cfdaf766f0..b501c2fe45 100644 > --- a/common/populate > +++ b/common/populate > @@ -868,9 +868,9 @@ _scratch_populate_cached() { > local meta_tag="$(echo "${meta_descr}" | md5sum - | cut -d ' ' -f 1)" > local metadump_stem="${TEST_DIR}/__populate.${FSTYP}.${meta_tag}" > > - # These variables are shared outside this function > - POPULATE_METADUMP="${metadump_stem}.metadump" > - POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > + # This variable is shared outside this function > + export POPULATE_METADUMP="${metadump_stem}.metadump" > + local POPULATE_METADUMP_DESCR="${metadump_stem}.txt" If the POPULATE_METADUMP_DESCR is not shared outside anymore, how about change it to lower-case? > > # Don't keep metadata images cached for more 48 hours... > rm -rf "$(find "${POPULATE_METADUMP}" -mtime +2 2>/dev/null)" >
On Thu, Oct 13, 2022 at 10:55:15PM +0800, Zorro Lang wrote: > On Tue, Oct 11, 2022 at 06:45:27PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Not sure why this hasn't been broken all along, but we should be > > exporting this variable so that it shows up in subshells.... > > May I ask where's the subshell which uses $POPULATE_METADUMP? _scratch_xfs_fuzz_metadata does this: echo "${fields}" | while read field; do echo "${verbs}" | while read fuzzverb; do __scratch_xfs_fuzz_mdrestore _xfs_mdrestore "${POPULATE_METADUMP}" The (nested) echo piped to while starts subshells. > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > common/populate | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/common/populate b/common/populate > > index cfdaf766f0..b501c2fe45 100644 > > --- a/common/populate > > +++ b/common/populate > > @@ -868,9 +868,9 @@ _scratch_populate_cached() { > > local meta_tag="$(echo "${meta_descr}" | md5sum - | cut -d ' ' -f 1)" > > local metadump_stem="${TEST_DIR}/__populate.${FSTYP}.${meta_tag}" > > > > - # These variables are shared outside this function > > - POPULATE_METADUMP="${metadump_stem}.metadump" > > - POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > > + # This variable is shared outside this function > > + export POPULATE_METADUMP="${metadump_stem}.metadump" > > + local POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > > If the POPULATE_METADUMP_DESCR is not shared outside anymore, how about change > it to lower-case? Ok. --D > > > > # Don't keep metadata images cached for more 48 hours... > > rm -rf "$(find "${POPULATE_METADUMP}" -mtime +2 2>/dev/null)" > > >
On Thu, Oct 13, 2022 at 08:54:35AM -0700, Darrick J. Wong wrote: > On Thu, Oct 13, 2022 at 10:55:15PM +0800, Zorro Lang wrote: > > On Tue, Oct 11, 2022 at 06:45:27PM -0700, Darrick J. Wong wrote: > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > Not sure why this hasn't been broken all along, but we should be > > > exporting this variable so that it shows up in subshells.... > > > > May I ask where's the subshell which uses $POPULATE_METADUMP? > > _scratch_xfs_fuzz_metadata does this: > > echo "${fields}" | while read field; do > echo "${verbs}" | while read fuzzverb; do > __scratch_xfs_fuzz_mdrestore > _xfs_mdrestore "${POPULATE_METADUMP}" > > The (nested) echo piped to while starts subshells. I'm not so familar with this part, so I didn't a simple test[1], and looks like the PARAM can be seen, even it's not exported. Do I misunderstand something? Thanks, Zorro [1] $ echo "$list" a b cc $ PARAM="This's a test" $ echo "$list"|while read c1;do echo "$list"|while read c2;do echo $PARAM;done; done This's a test This's a test This's a test This's a test This's a test This's a test This's a test This's a test This's a test > > > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > --- > > > common/populate | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > diff --git a/common/populate b/common/populate > > > index cfdaf766f0..b501c2fe45 100644 > > > --- a/common/populate > > > +++ b/common/populate > > > @@ -868,9 +868,9 @@ _scratch_populate_cached() { > > > local meta_tag="$(echo "${meta_descr}" | md5sum - | cut -d ' ' -f 1)" > > > local metadump_stem="${TEST_DIR}/__populate.${FSTYP}.${meta_tag}" > > > > > > - # These variables are shared outside this function > > > - POPULATE_METADUMP="${metadump_stem}.metadump" > > > - POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > > > + # This variable is shared outside this function > > > + export POPULATE_METADUMP="${metadump_stem}.metadump" > > > + local POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > > > > If the POPULATE_METADUMP_DESCR is not shared outside anymore, how about change > > it to lower-case? > > Ok. > > --D > > > > > > > # Don't keep metadata images cached for more 48 hours... > > > rm -rf "$(find "${POPULATE_METADUMP}" -mtime +2 2>/dev/null)" > > > > > >
On Fri, Oct 14, 2022 at 12:28:26AM +0800, Zorro Lang wrote: > On Thu, Oct 13, 2022 at 08:54:35AM -0700, Darrick J. Wong wrote: > > On Thu, Oct 13, 2022 at 10:55:15PM +0800, Zorro Lang wrote: > > > On Tue, Oct 11, 2022 at 06:45:27PM -0700, Darrick J. Wong wrote: > > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > > > Not sure why this hasn't been broken all along, but we should be > > > > exporting this variable so that it shows up in subshells.... > > > > > > May I ask where's the subshell which uses $POPULATE_METADUMP? > > > > _scratch_xfs_fuzz_metadata does this: > > > > echo "${fields}" | while read field; do > > echo "${verbs}" | while read fuzzverb; do > > __scratch_xfs_fuzz_mdrestore > > _xfs_mdrestore "${POPULATE_METADUMP}" > > > > The (nested) echo piped to while starts subshells. > > I'm not so familar with this part, so I didn't a simple test[1], and looks like > the PARAM can be seen, even it's not exported. Do I misunderstand something? > > Thanks, > Zorro > > [1] > $ echo "$list" > a > b > cc > $ PARAM="This's a test" > $ echo "$list"|while read c1;do echo "$list"|while read c2;do echo $PARAM;done; done > This's a test > This's a test > This's a test > This's a test > This's a test > This's a test > This's a test > This's a test > This's a test Hmm. I can't figure out why I needed the export here. It was late one night, something was broken, and exporting the variable made it work. Now I can't recall exactly what that was and it seems fine without it...? I guess I'll put it back and rerun the entire fuzz suite to see what pops out... --D > > > > > > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > > --- > > > > common/populate | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/common/populate b/common/populate > > > > index cfdaf766f0..b501c2fe45 100644 > > > > --- a/common/populate > > > > +++ b/common/populate > > > > @@ -868,9 +868,9 @@ _scratch_populate_cached() { > > > > local meta_tag="$(echo "${meta_descr}" | md5sum - | cut -d ' ' -f 1)" > > > > local metadump_stem="${TEST_DIR}/__populate.${FSTYP}.${meta_tag}" > > > > > > > > - # These variables are shared outside this function > > > > - POPULATE_METADUMP="${metadump_stem}.metadump" > > > > - POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > > > > + # This variable is shared outside this function > > > > + export POPULATE_METADUMP="${metadump_stem}.metadump" > > > > + local POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > > > > > > If the POPULATE_METADUMP_DESCR is not shared outside anymore, how about change > > > it to lower-case? > > > > Ok. > > > > --D > > > > > > > > > > # Don't keep metadata images cached for more 48 hours... > > > > rm -rf "$(find "${POPULATE_METADUMP}" -mtime +2 2>/dev/null)" > > > > > > > > > >
On Thu, Oct 13, 2022 at 12:12:19PM -0700, Darrick J. Wong wrote: > On Fri, Oct 14, 2022 at 12:28:26AM +0800, Zorro Lang wrote: > > On Thu, Oct 13, 2022 at 08:54:35AM -0700, Darrick J. Wong wrote: > > > On Thu, Oct 13, 2022 at 10:55:15PM +0800, Zorro Lang wrote: > > > > On Tue, Oct 11, 2022 at 06:45:27PM -0700, Darrick J. Wong wrote: > > > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > > > > > Not sure why this hasn't been broken all along, but we should be > > > > > exporting this variable so that it shows up in subshells.... > > > > > > > > May I ask where's the subshell which uses $POPULATE_METADUMP? > > > > > > _scratch_xfs_fuzz_metadata does this: > > > > > > echo "${fields}" | while read field; do > > > echo "${verbs}" | while read fuzzverb; do > > > __scratch_xfs_fuzz_mdrestore > > > _xfs_mdrestore "${POPULATE_METADUMP}" > > > > > > The (nested) echo piped to while starts subshells. > > > > I'm not so familar with this part, so I didn't a simple test[1], and looks like > > the PARAM can be seen, even it's not exported. Do I misunderstand something? > > > > Thanks, > > Zorro > > > > [1] > > $ echo "$list" > > a > > b > > cc > > $ PARAM="This's a test" > > $ echo "$list"|while read c1;do echo "$list"|while read c2;do echo $PARAM;done; done > > This's a test > > This's a test > > This's a test > > This's a test > > This's a test > > This's a test > > This's a test > > This's a test > > This's a test > > Hmm. I can't figure out why I needed the export here. It was late one > night, something was broken, and exporting the variable made it work. > Now I can't recall exactly what that was and it seems fine without > it...? > > I guess I'll put it back and rerun the entire fuzz suite to see what > pops out... Sure, I don't have objection on this patch, so you can have the RVB when you change "local POPULATE_METADUMP_DESCR" to lower-case, and show at least one example about why POPULATE_METADUMP need to be exported in next version. No push:) Reviewed-by: Zorro Lang <zlang@redhat.com> Thanks, Zorro > > --D > > > > > > > > > > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > > > --- > > > > > common/populate | 6 +++--- > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > diff --git a/common/populate b/common/populate > > > > > index cfdaf766f0..b501c2fe45 100644 > > > > > --- a/common/populate > > > > > +++ b/common/populate > > > > > @@ -868,9 +868,9 @@ _scratch_populate_cached() { > > > > > local meta_tag="$(echo "${meta_descr}" | md5sum - | cut -d ' ' -f 1)" > > > > > local metadump_stem="${TEST_DIR}/__populate.${FSTYP}.${meta_tag}" > > > > > > > > > > - # These variables are shared outside this function > > > > > - POPULATE_METADUMP="${metadump_stem}.metadump" > > > > > - POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > > > > > + # This variable is shared outside this function > > > > > + export POPULATE_METADUMP="${metadump_stem}.metadump" > > > > > + local POPULATE_METADUMP_DESCR="${metadump_stem}.txt" > > > > > > > > If the POPULATE_METADUMP_DESCR is not shared outside anymore, how about change > > > > it to lower-case? > > > > > > Ok. > > > > > > --D > > > > > > > > > > > > > # Don't keep metadata images cached for more 48 hours... > > > > > rm -rf "$(find "${POPULATE_METADUMP}" -mtime +2 2>/dev/null)" > > > > > > > > > > > > > > >
diff --git a/common/populate b/common/populate index cfdaf766f0..b501c2fe45 100644 --- a/common/populate +++ b/common/populate @@ -868,9 +868,9 @@ _scratch_populate_cached() { local meta_tag="$(echo "${meta_descr}" | md5sum - | cut -d ' ' -f 1)" local metadump_stem="${TEST_DIR}/__populate.${FSTYP}.${meta_tag}" - # These variables are shared outside this function - POPULATE_METADUMP="${metadump_stem}.metadump" - POPULATE_METADUMP_DESCR="${metadump_stem}.txt" + # This variable is shared outside this function + export POPULATE_METADUMP="${metadump_stem}.metadump" + local POPULATE_METADUMP_DESCR="${metadump_stem}.txt" # Don't keep metadata images cached for more 48 hours... rm -rf "$(find "${POPULATE_METADUMP}" -mtime +2 2>/dev/null)"