Re: Where is the testing? (was: Re: [PATCH] fs/xfs: Avoid unreadble filesystem if V4 superblock)
Le mer. 8 sept. 2021 à 03:24, Glenn Washburn a écrit : > On Thu, 2 Sep 2021 10:56:49 +0200 > Carlos Maiolino wrote: > > > On Wed, Sep 01, 2021 at 02:40:57PM +0200, Daniel Kiper wrote: > > > CC-ing Javier... > > > > > > On Mon, Aug 30, 2021 at 01:48:50PM +0200, Carlos Maiolino wrote: > > > > Hi. > > > > On Mon, Aug 30, 2021 at 11:18:31AM +0200, Erwan Velu wrote: > > > > >Good day list, > > > > >Le jeu. 26 août 2021 à 15:26, Carlos Maiolino > > > > > <[1]cmaiol...@redhat.com> a écrit : > > > > > > > > > > [..] > > > > > Thanks for spotting this! > > > > > > > > > >I'm adding the maintainers in CC. Carlos who commit the > > > > > patch I'm fixing, agreed on the content. > > > > > > > > I didn't test the patch itself yet, but I've reproduced the > > > > issue. I was quite sure I had tested this patch on a V4 fs, but > > > > looks like I miscalculated the sizing. Thanks again. I'll try to > > > > test the patch here asap. > > > > > > Did you test this patch? If yes may I add your Tested-by to it? > > > > Yup, patch works fine, just finished testing it, I was just trying to > > understand where/why I miscalculated the inode size on V4 > > filesystems, and the reason was the same why Erwan split the > > last/first members of inode v2/v3 in two different unused structs. > > > > Feel free to add to the patch: > > > > Tested-by: Carlos Maiolino > > It looks like the xfs_test test succeeds with tag grub-2.06-rc1a, fails > with tag grub-2.06, and succeeds with current master. Yes, as expected. > However, what this tells me is that no "make check" tests were done > before finalizing the 2.06 release. I was under the impression that that > was part of the release procedure. If its not, it would seem that we're > not using the tests at a time when they would have the most impact. > > It is my understanding that we have travis-ci tests that get run (at > some point?), however they are only build tests and so would not have > caught this. It was precisely this scenario that I hoped to avoid by > doing more thorough continuous integration, which runs the extensive > array of "make check" tests, when I submitted the Gitlab-CI patch > series (which would've caught this automatically if it had been merged). > To me this scenario is the poster child for taking action on this > front. Can we make this a priority? > > That also triggers to me the question of the difficulty to make a release. I'm pretty new to this project but from my other experiences, I'm missing what prevents doing a stable branch per release to perform critical updates on them. I'm a big fan of how the Linux kernel manages this: once the commit is merged in master, it can be backported into the stable branches if the patch makes sense. Then, the stable branch can be released whenever it makes sense. This increases the maintenance burden (but can be delegated to dedicated people) but gives users the insurance of having an up to date & secured product. This topic was probably already addressed or questioned by the past soplease receive this as a simple questioning and not a straight criticism of the project maintenance. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] [PATCH] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons
The test for the ability to decompress zisofs encoded files is supposed
to fail due to the lack of this ability in GRUB. But it fails early with
xorriso : FAILURE : -volid: Text too long (1650 > 32)
because "ziso9660" is not in the list of filesystems which accept at most
32 bytes in their FSLABEL. If this is fixed, the test returns false success
because the xorriso run does not produce any zisofs compressed files. A bug
in xorriso causes a distracting warning about FSLABEL being too long for
Joliet. Shortcommings of Joliet cause warnings about symbolic links.
So add "ziso9660" to the 32-byte FSLABEL list. Fix the xorriso run to
produce compressed files which for now cause righteous failure of the test.
Remove the option to produce a Joliet filesystem tree in order to
concentrate on testing zisofs decompression.
Signed-off-by: Thomas Schmitt
---
tests/util/grub-fs-tester.in | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 4f581b638..a28e07295 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -318,7 +318,8 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
FSLABEL="grub_;/testéтi
u😁莽茝кириrewfceniuewruevrewnuuireurevueurnievrewfne";;
# FS LIMITATION: afs and iso9660 label is at most 32 UTF-8
characters
x"afs" | xiso9660 | xrockridge | xrockridge_joliet\
- | xiso9660_1999 | xrockridge_1999 |
xrockridge_joliet_1999)
+| xiso9660_1999 | xrockridge_1999\
+| xrockridge_joliet_1999 | xziso9660)
FSLABEL="gr_;/é莭莽😁кирит u";;
# FS LIMITATION: bfs label is at most 32 UTF-8 characters
# OS LIMITATION: bfs label can't contain ; or /
@@ -1024,7 +1025,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
"$MAXLOGSECSIZE" 1); do
(cd "$MASTER"; find . | cpio -o -H "$(echo ${fs} | sed
's@^cpio_@@')" > "${FSIMAGEP}0.img" ) ;;
x"ziso9660")
FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
- xorriso -compliance rec_mtime -set_filter_r --zisofs --
-zisofs default -as mkisofs $XORRISOFS_CHARSET -iso-level 3 -graft-points -R -J
-joliet-long -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed
's/-//g;') -o "${FSIMAGEP}0.img" -- -set_filter_r --zisofs -- -zisofs default
-add /="$MASTER" ;;
+ xorriso -compliance rec_mtime -as mkisofs
$XORRISOFS_CHARSET -iso-level 3 -graft-points -R -V "$FSLABEL"
--modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" --
-add /="$MASTER" -- -zisofs default -set_filter_r --zisofs / -- ;;
x"iso9660")
FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
xorriso --rockridge off -compliance rec_mtime -as mkisofs
$XORRISOFS_CHARSET -iso-level 3 -graft-points -V "$FSLABEL"
--modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img"
/="$MASTER" ;;
--
2.20.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] [PATCH] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons
Hi, sorry for messing up the mail subject by "[PATCH] [PATCH]". (I added one of them already to the local commit message.) Shall i send new mail with better subject ? Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Where is the testing? (was: Re: [PATCH] fs/xfs: Avoid unreadble filesystem if V4 superblock)
On Wed, Sep 08, 2021 at 01:22:20AM +, Glenn Washburn wrote: > On Thu, 2 Sep 2021 10:56:49 +0200 > Carlos Maiolino wrote: > > > On Wed, Sep 01, 2021 at 02:40:57PM +0200, Daniel Kiper wrote: > > > CC-ing Javier... > > > > > > On Mon, Aug 30, 2021 at 01:48:50PM +0200, Carlos Maiolino wrote: > > > > Hi. > > > > On Mon, Aug 30, 2021 at 11:18:31AM +0200, Erwan Velu wrote: > > > > >Good day list, > > > > >Le jeu. 26 août 2021 à 15:26, Carlos Maiolino > > > > > <[1]cmaiol...@redhat.com> a écrit : > > > > > > > > > > [..] > > > > > Thanks for spotting this! > > > > > > > > > >I'm adding the maintainers in CC. Carlos who commit the > > > > > patch I'm fixing, agreed on the content. > > > > > > > > I didn't test the patch itself yet, but I've reproduced the > > > > issue. I was quite sure I had tested this patch on a V4 fs, but > > > > looks like I miscalculated the sizing. Thanks again. I'll try to > > > > test the patch here asap. > > > > > > Did you test this patch? If yes may I add your Tested-by to it? > > > > Yup, patch works fine, just finished testing it, I was just trying to > > understand where/why I miscalculated the inode size on V4 > > filesystems, and the reason was the same why Erwan split the > > last/first members of inode v2/v3 in two different unused structs. > > > > Feel free to add to the patch: > > > > Tested-by: Carlos Maiolino > > It looks like the xfs_test test succeeds with tag grub-2.06-rc1a, fails > with tag grub-2.06, and succeeds with current master. Yes, as expected. > However, what this tells me is that no "make check" tests were done > before finalizing the 2.06 release. I was under the impression that that > was part of the release procedure. If its not, it would seem that we're > not using the tests at a time when they would have the most impact. Currently I run build tests for all architectures and platforms and Coverity for x86_64 EFI before every push and release. I know this is not enough but I tried "make check" at least once and got the impression that the tests are not reliable. Sadly I have not time to dive deeper and fix this and that. If someone, you?, wants to verify all tests and fix broken ones I will be more than happy to start using it (it seems to me your "[PATCH v2 0/8] Various fixes/improvements for tests" patch set fixes some issues but I am not sure if all of them). > It is my understanding that we have travis-ci tests that get run (at > some point?), however they are only build tests and so would not have > caught this. It was precisely this scenario that I hoped to avoid by > doing more thorough continuous integration, which runs the extensive > array of "make check" tests, when I submitted the Gitlab-CI patch > series (which would've caught this automatically if it had been merged). > To me this scenario is the poster child for taking action on this > front. Can we make this a priority? I think I can take a closer look at patch set mentioned above in a week or two. If "make check" works as expected and can be run locally then we can think about automating the testing. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Where is the testing? (was: Re: [PATCH] fs/xfs: Avoid unreadble filesystem if V4 superblock)
On Wed, Sep 08, 2021 at 11:20:08AM +0200, Erwan Velu wrote: > Le mer. 8 sept. 2021 à 03:24, Glenn Washburn > a écrit : > > > On Thu, 2 Sep 2021 10:56:49 +0200 > > Carlos Maiolino wrote: > > > > > On Wed, Sep 01, 2021 at 02:40:57PM +0200, Daniel Kiper wrote: > > > > CC-ing Javier... > > > > > > > > On Mon, Aug 30, 2021 at 01:48:50PM +0200, Carlos Maiolino wrote: > > > > > Hi. > > > > > On Mon, Aug 30, 2021 at 11:18:31AM +0200, Erwan Velu wrote: > > > > > >Good day list, > > > > > >Le jeu. 26 août 2021 à 15:26, Carlos Maiolino > > > > > > <[1]cmaiol...@redhat.com> a écrit : > > > > > > > > > > > > [..] > > > > > > Thanks for spotting this! > > > > > > > > > > > >I'm adding the maintainers in CC. Carlos who commit the > > > > > > patch I'm fixing, agreed on the content. > > > > > > > > > > I didn't test the patch itself yet, but I've reproduced the > > > > > issue. I was quite sure I had tested this patch on a V4 fs, but > > > > > looks like I miscalculated the sizing. Thanks again. I'll try to > > > > > test the patch here asap. > > > > > > > > Did you test this patch? If yes may I add your Tested-by to it? > > > > > > Yup, patch works fine, just finished testing it, I was just trying to > > > understand where/why I miscalculated the inode size on V4 > > > filesystems, and the reason was the same why Erwan split the > > > last/first members of inode v2/v3 in two different unused structs. > > > > > > Feel free to add to the patch: > > > > > > Tested-by: Carlos Maiolino > > > > It looks like the xfs_test test succeeds with tag grub-2.06-rc1a, fails > > with tag grub-2.06, and succeeds with current master. Yes, as expected. > > However, what this tells me is that no "make check" tests were done > > before finalizing the 2.06 release. I was under the impression that that > > was part of the release procedure. If its not, it would seem that we're > > not using the tests at a time when they would have the most impact. > > > > It is my understanding that we have travis-ci tests that get run (at > > some point?), however they are only build tests and so would not have > > caught this. It was precisely this scenario that I hoped to avoid by > > doing more thorough continuous integration, which runs the extensive > > array of "make check" tests, when I submitted the Gitlab-CI patch > > series (which would've caught this automatically if it had been merged). > > To me this scenario is the poster child for taking action on this > > front. Can we make this a priority? > > > > > That also triggers to me the question of the difficulty to make a release. > I'm pretty new to this project but from my other experiences, I'm missing > what prevents doing a stable branch per release to perform critical updates > on them. > I'm a big fan of how the Linux kernel manages this: once the commit is > merged in master, it can be backported into the stable branches if the > patch makes sense. > Then, the stable branch can be released whenever it makes sense. > This increases the maintenance burden (but can be delegated to dedicated > people) but gives users the insurance of having an up to date & secured > product. > > This topic was probably already addressed or questioned by the past > soplease receive this as a simple questioning and not a straight criticism > of the project maintenance. The Linux project is more mature in many areas than the GRUB right now. Additionally, due to various reasons development of the GRUB almost stopped for years. Currently we are clearing the backlog and trying to regain the control on the project. So, we have to focus on most important things due to limited resources. Though I think at some point we change and improve the release process too. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Where is the testing? (was: Re: [PATCH] fs/xfs: Avoid unreadble filesystem if V4 superblock)
When this topic will be out, I'd would be glad to help. Cheers, Erwan, Le mer. 8 sept. 2021 à 18:19, Daniel Kiper a écrit : > On Wed, Sep 08, 2021 at 11:20:08AM +0200, Erwan Velu wrote: > > Le mer. 8 sept. 2021 à 03:24, Glenn Washburn < > developm...@efficientek.com> > > a écrit : > > > > > On Thu, 2 Sep 2021 10:56:49 +0200 > > > Carlos Maiolino wrote: > > > > > > > On Wed, Sep 01, 2021 at 02:40:57PM +0200, Daniel Kiper wrote: > > > > > CC-ing Javier... > > > > > > > > > > On Mon, Aug 30, 2021 at 01:48:50PM +0200, Carlos Maiolino wrote: > > > > > > Hi. > > > > > > On Mon, Aug 30, 2021 at 11:18:31AM +0200, Erwan Velu wrote: > > > > > > >Good day list, > > > > > > >Le jeu. 26 août 2021 à 15:26, Carlos Maiolino > > > > > > > <[1]cmaiol...@redhat.com> a écrit : > > > > > > > > > > > > > > [..] > > > > > > > Thanks for spotting this! > > > > > > > > > > > > > >I'm adding the maintainers in CC. Carlos who commit the > > > > > > > patch I'm fixing, agreed on the content. > > > > > > > > > > > > I didn't test the patch itself yet, but I've reproduced the > > > > > > issue. I was quite sure I had tested this patch on a V4 fs, but > > > > > > looks like I miscalculated the sizing. Thanks again. I'll try to > > > > > > test the patch here asap. > > > > > > > > > > Did you test this patch? If yes may I add your Tested-by to it? > > > > > > > > Yup, patch works fine, just finished testing it, I was just trying to > > > > understand where/why I miscalculated the inode size on V4 > > > > filesystems, and the reason was the same why Erwan split the > > > > last/first members of inode v2/v3 in two different unused structs. > > > > > > > > Feel free to add to the patch: > > > > > > > > Tested-by: Carlos Maiolino > > > > > > It looks like the xfs_test test succeeds with tag grub-2.06-rc1a, fails > > > with tag grub-2.06, and succeeds with current master. Yes, as expected. > > > However, what this tells me is that no "make check" tests were done > > > before finalizing the 2.06 release. I was under the impression that > that > > > was part of the release procedure. If its not, it would seem that we're > > > not using the tests at a time when they would have the most impact. > > > > > > It is my understanding that we have travis-ci tests that get run (at > > > some point?), however they are only build tests and so would not have > > > caught this. It was precisely this scenario that I hoped to avoid by > > > doing more thorough continuous integration, which runs the extensive > > > array of "make check" tests, when I submitted the Gitlab-CI patch > > > series (which would've caught this automatically if it had been > merged). > > > To me this scenario is the poster child for taking action on this > > > front. Can we make this a priority? > > > > > > > > That also triggers to me the question of the difficulty to make a > release. > > I'm pretty new to this project but from my other experiences, I'm missing > > what prevents doing a stable branch per release to perform critical > updates > > on them. > > I'm a big fan of how the Linux kernel manages this: once the commit is > > merged in master, it can be backported into the stable branches if the > > patch makes sense. > > Then, the stable branch can be released whenever it makes sense. > > This increases the maintenance burden (but can be delegated to dedicated > > people) but gives users the insurance of having an up to date & secured > > product. > > > > This topic was probably already addressed or questioned by the past > > soplease receive this as a simple questioning and not a straight > criticism > > of the project maintenance. > > The Linux project is more mature in many areas than the GRUB right now. > Additionally, due to various reasons development of the GRUB almost > stopped for years. Currently we are clearing the backlog and trying to > regain the control on the project. So, we have to focus on most > important things due to limited resources. Though I think at some point > we change and improve the release process too. > > Daniel > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] [PATCH] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons
On Wed, Sep 08, 2021 at 01:10:55PM +0200, Thomas Schmitt wrote:
> The test for the ability to decompress zisofs encoded files is supposed
> to fail due to the lack of this ability in GRUB. But it fails early with
> xorriso : FAILURE : -volid: Text too long (1650 > 32)
> because "ziso9660" is not in the list of filesystems which accept at most
> 32 bytes in their FSLABEL. If this is fixed, the test returns false success
> because the xorriso run does not produce any zisofs compressed files. A bug
> in xorriso causes a distracting warning about FSLABEL being too long for
> Joliet. Shortcommings of Joliet cause warnings about symbolic links.
s/links./links too./?
> So add "ziso9660" to the 32-byte FSLABEL list. Fix the xorriso run to
I have an itching to ask you to split this thing into two patches.
Though I am not really convinced doing it. So, feel free to do what
is better here in your opinion...
> produce compressed files which for now cause righteous failure of the test.
> Remove the option to produce a Joliet filesystem tree in order to
> concentrate on testing zisofs decompression.
>
> Signed-off-by: Thomas Schmitt
> ---
> tests/util/grub-fs-tester.in | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> index 4f581b638..a28e07295 100644
> --- a/tests/util/grub-fs-tester.in
> +++ b/tests/util/grub-fs-tester.in
> @@ -318,7 +318,8 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
> "$MAXLOGSECSIZE" 1); do
> FSLABEL="grub_;/testéтi
> u😁莽茝кириrewfceniuewruevrewnuuireurevueurnievrewfne";;
> # FS LIMITATION: afs and iso9660 label is at most 32 UTF-8
> characters
> x"afs" | xiso9660 | xrockridge | xrockridge_joliet\
> - | xiso9660_1999 | xrockridge_1999 |
> xrockridge_joliet_1999)
> + | xiso9660_1999 | xrockridge_1999\
> + | xrockridge_joliet_1999 | xziso9660)
>FSLABEL="gr_;/é莭莽😁кирит u";;
> # FS LIMITATION: bfs label is at most 32 UTF-8 characters
> # OS LIMITATION: bfs label can't contain ; or /
> @@ -1024,7 +1025,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
> "$MAXLOGSECSIZE" 1); do
> (cd "$MASTER"; find . | cpio -o -H "$(echo ${fs} | sed
> 's@^cpio_@@')" > "${FSIMAGEP}0.img" ) ;;
> x"ziso9660")
> FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
> - xorriso -compliance rec_mtime -set_filter_r --zisofs --
> -zisofs default -as mkisofs $XORRISOFS_CHARSET -iso-level 3 -graft-points -R
> -J -joliet-long -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed
> 's/-//g;') -o "${FSIMAGEP}0.img" -- -set_filter_r --zisofs -- -zisofs
> default -add /="$MASTER" ;;
> + xorriso -compliance rec_mtime -as mkisofs
> $XORRISOFS_CHARSET -iso-level 3 -graft-points -R -V "$FSLABEL"
> --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img"
> -- -add /="$MASTER" -- -zisofs default -set_filter_r --zisofs / -- ;;
You described everything in the commit message what you are doing here
except of "-set_filter_r --zisofs -- -zisofs default" games. Could you
explain that too? And if you could not change the order of above mentioned
options it would be nice. Or explain why you change the order.
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] tests: Let xorriso fixely assume UTF-8 as local character set
On Tue, Sep 07, 2021 at 09:15:58PM +, Glenn Washburn wrote: > On Wed, 1 Sep 2021 15:11:38 +0200 > Daniel Kiper wrote: > > > On Fri, Aug 27, 2021 at 11:05:06PM +0200, Thomas Schmitt wrote: > > > iso9660_test fails if the effective locale is not UTF-8. This > > > happens because xorriso needs to convert file names and FSLABEL to > > > UCS-2 when preparing a Joliet tree. grub-fs-tester obviously > > > intends to use UTF-8 as character set, but xorriso assumes by > > > default the result of nl_langinfo(3) with item CODESET. > > > So override the result of nl_langinfo(CODESET) by options of > > > xorriso -as mkisofs. > > > > > > Signed-off-by: Thomas Schmitt > > > > Thomas, Glenn, thank you for doing such deep investigation! > > > > Reviewed-by: Daniel Kiper > > > > Glenn, did you test this patch? > > Coming to this late, had some issues which kept me away. I notice that No worries... > the patch is already in master, but for what its worth, I have now > tested with the patch and LANG unset and the test now succeeds. Thanks Thank you for doing test. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: submenu fails to see variables
On Tue, Sep 07, 2021 at 11:54:13AM +0100, Dimitri John Ledkov wrote:
> On Tue, Sep 7, 2021 at 10:30 AM Olaf Hering wrote:
> >
> > On Mon, Sep 06, Vladimir 'phcoder' Serbinenko wrote:
> >
> > > Le lun. 6 sept. 2021 à 12:49, Olaf Hering a écrit :
> > > For some reason global variables are not seen in a submenu {} section.
> > > Does anyone happen to know why this behavior is useful?
> > > You need to export variable to make it visible in submenu
> >
> > Thanks. This was less than obvious. I did not expect a command named
> > 'export' in the context of grub.
> >
> > The documentation needs to be updated to state what the difference between
> > 'set key=val', 'export key=val' and plain 'key=val' actually is.
>
> I have seen somewhere that some distros applied a patch to simply
> export all variables always; or like not to create new contexts for
> submenus, to keep the variable space the same.
>
> Imho keeping variable space global, and exported by default seems to
> lead to least surprises. But it is a behaviour change/break.
I would prefer to keep behavior as is and document it.
Dimitri, Olaf, could one of you do it?
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] i386-pc: build btrfs zstd support into separate module
On Fri, Sep 03, 2021 at 09:21:39AM +0800, Michael Chang via Grub-devel wrote: > On Thu, Sep 02, 2021 at 02:12:52PM +0200, Daniel Kiper wrote: > > On Thu, Sep 02, 2021 at 01:48:30PM +0800, Michael Chang via Grub-devel > > wrote: > > > On Wed, Sep 01, 2021 at 06:38:22PM +0200, Daniel Kiper wrote: > > > > On Tue, Aug 31, 2021 at 03:12:28PM +0800, Michael Chang via Grub-devel > > > > wrote: > > > > > The zstd support in btrfs brings significant size increment to the > > > > > on-disk image that it can no longer fit into btrfs bootloader area and > > > > > short mbr gap. > > > > > > > > > > In order to support grub update on outstanding i386-pc setup with > > > > > these > > > > > size constraints remain in place, here we build the zstd suppprt of > > > > > btrfs into a separate module, named btrfs_zstd, to alleviate the size > > > > > change. Please note this only makes it's way to i386-pc, other > > > > > architecture is not affected. > > > > > > > > I am OK with extracting zstd code from btrfs code. However, I want that > > > > be > > > > done for all architectures and platforms. No exceptions. > > > > > > May I ask for more background about this decision ? > > > > Yes, I want the same code and behavior for all architectures and > > platforms if possible. > > > > > Given that btrfs compression is per file property and can be > > > recompressed on the fly, there is no way to detect it beforehand thus > > > the safest bet is to assume that it is always needed. In other words if > > > the platform has no known limitation on the size of image, the zstd code > > > should be indivisible to btrfs.mo. > > > > Wait, you said this in the commit message too: > > > > Therefore if the system has enough space of embedding area for grub then > > zstd support for btrfs will be enabled automatically in the process of > > running grub-install through inserting btrfs_zstd module to the on-disk > > image, otherwise a warning will be logged on screen to indicate user > > that zstd support for btrfs is disabled due to the size limit. > > > > So, I thought this may work in the same way in all cases. If this is not > > true you have to fix this. Otherwise I cannot take this patch. > > I literally don't like to propagate this (somewhat awkward) fix to other > architectures that do not have sensible requirement to it, though it > looks possible to do so. > > If this doesn't meet the expection then I'm sorry. > > > TBH, I should reject this patch immediately because this is "small MBR > > gap stuff". And as I said a few months ago, I want to stop to pay > > attention to "small MBR gap stuff" in upstream. Sorry for being blunt > > but it is full can of worms, i.e., each patch like that one adds more > > cruft which we have to take care because a few folks in the wild could > > not spend a few hours to repartition their systems. I think distros > > should start putting more effort in educating their users WRT to small > > MBR gap and problems related to it instead of pushing again and again > > upstream patches which make the GRUB code more complicated. > > Well, I think the short mbr gap issue is all well received been > discussed several times. The reason I'm poking this beehive again is Cool! Thanks! > just that I can't resist to fix problem from our users who opted to use > "btrfs partition" which differs to "short mbr gap" with a lot more user > base and sensible usecases. > > FWIW we want to address this problem, because mbr gap is adjustable via > re-partitioning but btrfs bootloader area is not. Huh! The "btrfs partition" sounds like not resizable MBR gap! I am not happy with it just at the beginning. Anyway, could you explain your use case in more details including example commands and why core.img seems landing in the "btrfs partition". I am especially curious about the latter because I think the "btrfs partition" and things like that was designed for something else, e.g., storing grubenv data. And why this solution is i386 specific? Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Where is the testing? (was: Re: [PATCH] fs/xfs: Avoid unreadble filesystem if V4 superblock)
On Wed, 8 Sep 2021 18:03:50 +0200 Daniel Kiper wrote: > On Wed, Sep 08, 2021 at 01:22:20AM +, Glenn Washburn wrote: > > It looks like the xfs_test test succeeds with tag grub-2.06-rc1a, > > fails with tag grub-2.06, and succeeds with current master. Yes, as > > expected. However, what this tells me is that no "make check" tests > > were done before finalizing the 2.06 release. I was under the > > impression that that was part of the release procedure. If its not, > > it would seem that we're not using the tests at a time when they > > would have the most impact. > > Currently I run build tests for all architectures and platforms and > Coverity for x86_64 EFI before every push and release. I know this is > not enough but I tried "make check" at least once and got the > impression that the tests are not reliable. Sadly I have not time to > dive deeper and fix this and that. If someone, you?, wants to verify > all tests and fix broken ones I will be more than happy to start > using it (it seems to me your "[PATCH v2 0/8] Various > fixes/improvements for tests" patch set fixes some issues but I am > not sure if all of them). I suspect the problem you're running into with the tests is more a matter of setting up the expected environment than the tests themselves. From my experience, most of the tests work once everything is setup right. My gitlab patch shows how to do this on Ubuntu 20.04. Due to not being able to load kernel modules on the shared gitlab runners, I was not able to run all the tests either, and there are some tests that fail. The failures that come to mind are on Sparc and some of the functional tests, and I've notified the list, but no one came forward to fix them. So what I've done is have a list of known failing tests and ignore those in determining whether the test suite as a whole has succeeded. I'm not convinced that that behavior is something we should include in the test framework itself. So to my mind, I have verified (nearly) all tests on most platforms. I have fixed the ones I could. I think the biggest low hanging fruit (for me), is to get the tests running on a system with all the needed kernel fs drivers loaded. Perhaps to your point, the filesystem tests failing due to not having a kernel module loaded should be marked as skipped because the test isn't really run. Here is an example of a successfully completed pipeline https://gitlab.com/gwashburn/grub/-/pipelines/258561619 And here is the raw log for the x86_64-efi build and test (search for "Testsuite summary" and look at the lines above for individual test pass/fail): https://storage.googleapis.com/gitlab-gprd-artifacts/0e/0d/0e0d2ccc27a1e9a121b3d4ef831c56b5fd81cc8b565a3dbc5fa09b9a58edbcb7/2021_02_19/1042905700/1137549157/job.log?response-content-type=text%2Fplain%3B%20charset%3Dutf-8&response-content-disposition=inline&GoogleAccessId=gitlab-object-storage-...@gitlab-production.iam.gserviceaccount.com&Signature=ewpRWBqWWorOXak6hFkb5kUEfefU1biAtu6xY2Rtyds3%2BduM6q0kiFIKe4A5%0A8wS%2FPbd8Al3AwF45Q22KcpYyZ87UBkryQHjispAj%2B3tBQrnnyOYSRKGNV%2Bbz%0A4iaKAdYn4onIcJM9Ro4RkJ%2FCAY2tBxWmmLoEZ%2FQzWLvbhH%2BJSmuNqtEZXfuD%0AI1tXD1ZKgoLcYCCLNz8iaMFpgB32NfR2W6sDDuFb24ZFSy0X7H02KRVAMIor%0Akhj5QXdXGqoqFFXXMM9r8ZGv34Kh4GdDbVjMmJ%2FaDhvLPgmZF3UKnhcDYJoB%0AiMj%2BbimYNoiQW0SLg4hHA11PkKNn4KPAFqhLVscsCw%3D%3D&Expires=1631130462 This is all to show that the tests are (mostly) working. There's some post-processing on the pass/fail status of some tests to ignore expected failures (eg. zfs_test). Occasionally there are spurious test failures, I think due to the VM getting starved at times (eg. why grub_cmd_sleep fails and is ignored). No as far as you personally using the tests, based on the above what do we need to get that to happen? I don't know the setup that you plan on doing the tests on (is it debian based?). Do you want a script that sets up the environment for you? Perhaps a docker or lxd setup (probably better in the long run)? A word about why I haven't verified the fs tests that require kernel modules not loaded in gitlab shared runner, I could run these on my system. However, I've had some strange experiences in the past with the fs tests not cleaning up well and causing some issues with the rest of the system. So I'm not keen on running all the fs tests on my daily driver. Perhaps in a container (a virtual machine would work, but I suspect really slow, because many of the tests use virtual machines for testing, so there would be VMs in a VM). I'd be curious to know what specific issues you had with the test, if you can remember or reproduce them. > > It is my understanding that we have travis-ci tests that get run (at > > some point?), however they are only build tests and so would not > > have caught this. It was precisely this scenario that I hoped to > > avoid by doing more thorough continuous integration, which runs the > > extensive array of "make check" tests, when I submitted the > > Gitlab-CI patch series (which woul
Re: [PATCH] [PATCH] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons
Hi, i am working on the requested changes. In my patch message i wrote: > > A bug > > in xorriso causes a distracting warning about FSLABEL being too long for > > Joliet. Shortcommings of Joliet cause warnings about symbolic links. Daniel Kiper wrote: > s/links./links too./? Rather not. Both warnings have very different reasons. The xorriso bug was that the warning came if more than 16 bytes of volume id are submitted. The correct test is to count the UCS-2 characters after conversion from the local character set (meanwhile surely UTF-8). The FSLABEL which is prepared by grub-fs-tester for ISO 9660 tests has only 15 characters but 32 UTF-8 bytes. The shortcomming of Joliet is in its specs. It was invented by Microsoft Inc. for presenting Microsoft filesystem names. At least back in 1995 no symbolic links were desired for their version of ISO 9660. Other than the xorriso bug i cannot fix this. Both warnings could deceive the reader of the test report, especially since the test is expected to fail. Thus my patch disables Joliet to silence both for all xorriso versions. > I have an itching to ask you to split this thing into two patches. I decided against this because the test would indicate false success after the early failure because of the FSLABEL length is fixed. If i fix the xorriso command sequence problem first, then this first patch does not have any effect at run time. So i rather consider the ziso9660 test as untested sketch which i replace by a working test with proper failure as long as no zisofs decompression is implemented. > You described everything in the commit message what you are doing here > except of "-set_filter_r --zisofs -- -zisofs default" games. Could you > explain that too? Will try without copying a whole paragraph from man xorriso. Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2] tests: Keep grub-fs-tester ziso9660 from failing for wrong reasons
The test for the ability to decompress zisofs encoded files is supposed
to fail due to the lack of this ability in GRUB. But it fails early with
xorriso : FAILURE : -volid: Text too long (1650 > 32)
because "ziso9660" is not in the list of filesystems which accept at most
32 bytes in their FSLABEL.
If this is fixed, the test returns false success because the xorriso run
does not produce any zisofs compressed files. The problem is in the
sequence of native xorriso commands used. The command -set_filter_r
applies only to the files which are already inserted into the emerging
ISO filesystem. In the current sequence no files have been inserted yet
by command -add when the last of two -set_filter_r commands is executed.
After this is corrected, xorriso refuses to work because the global
settings of command -zisofs can be made only before command -set_filter_r
has attached zisofs filters to the data files in the emerging ISO.
Further: A bug in xorriso causes a false warning about FSLABEL being
too long for Joliet. Shortcommings of Joliet cause warnings about
symbolic links. Such warnings might distract from the actual reason why
the test is expected to fail.
So add "ziso9660" to the 32-byte FSLABEL list.
Fix the xorriso run to produce compressed files which for now cause
righteous failure of the test. Do this by removing a surplus group of
-set_filter_r and -zisofs commands, by moving the other such group
behind -add, and by swapping -set_filter_r and -zisofs.
Remove the -as mkisofs options which produce a Joliet filesystem tree.
Signed-off-by: Thomas Schmitt
---
tests/util/grub-fs-tester.in | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 4f581b638..a28e07295 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -318,7 +318,8 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
FSLABEL="grub_;/testéтi
u😁莽茝кириrewfceniuewruevrewnuuireurevueurnievrewfne";;
# FS LIMITATION: afs and iso9660 label is at most 32 UTF-8
characters
x"afs" | xiso9660 | xrockridge | xrockridge_joliet\
- | xiso9660_1999 | xrockridge_1999 |
xrockridge_joliet_1999)
+| xiso9660_1999 | xrockridge_1999\
+| xrockridge_joliet_1999 | xziso9660)
FSLABEL="gr_;/é莭莽😁кирит u";;
# FS LIMITATION: bfs label is at most 32 UTF-8 characters
# OS LIMITATION: bfs label can't contain ; or /
@@ -1024,7 +1025,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
"$MAXLOGSECSIZE" 1); do
(cd "$MASTER"; find . | cpio -o -H "$(echo ${fs} | sed
's@^cpio_@@')" > "${FSIMAGEP}0.img" ) ;;
x"ziso9660")
FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
- xorriso -compliance rec_mtime -set_filter_r --zisofs --
-zisofs default -as mkisofs $XORRISOFS_CHARSET -iso-level 3 -graft-points -R -J
-joliet-long -V "$FSLABEL" --modification-date=$(echo ${FSUUID} | sed
's/-//g;') -o "${FSIMAGEP}0.img" -- -set_filter_r --zisofs -- -zisofs default
-add /="$MASTER" ;;
+ xorriso -compliance rec_mtime -as mkisofs
$XORRISOFS_CHARSET -iso-level 3 -graft-points -R -V "$FSLABEL"
--modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img" --
-add /="$MASTER" -- -zisofs default -set_filter_r --zisofs / -- ;;
x"iso9660")
FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
xorriso --rockridge off -compliance rec_mtime -as mkisofs
$XORRISOFS_CHARSET -iso-level 3 -graft-points -V "$FSLABEL"
--modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o "${FSIMAGEP}0.img"
/="$MASTER" ;;
--
2.20.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
