[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 31/37] btrfs: grub2-btrfs-04-grub2-install
From: |
Michael Chang |
Subject: |
Re: [PATCH v1 31/37] btrfs: grub2-btrfs-04-grub2-install |
Date: |
Tue, 8 Oct 2024 16:07:44 +0800 |
On Tue, Oct 08, 2024 at 10:14:59AM GMT, Vladimir 'phcoder' Serbinenko wrote:
> Le mar. 8 oct. 2024, 09:53, Michael Chang via Grub-devel <grub-devel@gnu.org>
> a écrit :
>
> > On Tue, Oct 08, 2024 at 08:07:17AM GMT, Vladimir 'phcoder' Serbinenko
> > wrote:
> > > Again, what do you try to achieve? Why aren't absolute paths enough?
> >
> > The absolute path does not align with the default subvolume. As a
> > result, after running btrfs set-default <SUBVOLID>, the system does not
> > boot the new default subvolume as expected. Instead, it continues to
> > boot from a fixed subvolume.
> >
> Yes, this is the intended behavior. When you change mount points or mount
> options they are not propagated either. We can have a command to retrieve
> default subvolume:
> btrfs_default_subvolume --set=subvol /
> linux /$subvol/boot/vmlinuz
> This way you can still e.g. do operations with different subvols like:
> if cmp /$subvol/grub.cfg /vol2/grub.cfg; then
> ...
> fi
I think I did try the approach above, but in the end, I gave up due to
some problem I can't recall right now. Anyway, I think I will revisit it
to see if it can replace the relative path scheme.
One of the bigger issues or challenges that comes to mind is that
grub-mkrelpath can only output a 'path,' and for it to output
/$subvol/boot/vmlinuz, it would need to output a 'relative path' with a
/$subvol prefix. The /$subvol handling is currently missing in
grub2-mkconfig and all relevant tools: for example, grub2-mkimage -p ...
or early load.cfg may only reference a path without a defined $subvol,
if they aimed to follow default is easy to go wrong.
Thanks,
Michael
>
> >
> > Thanks,
> > Michael
> >
> > >
> > > Le lun. 7 oct. 2024, 21:23, Leo Sandoval <lsandova@redhat.com> a écrit :
> > >
> > > > From: Michael Chang <mchang@suse.com>
> > > >
> > > > Signed-off-by: Michael Chang <mchang@suse.com>
> > > > Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> > > > ---
> > > > grub-core/osdep/linux/getroot.c | 7 +++++++
> > > > grub-core/osdep/unix/config.c | 17 +++++++++++++++--
> > > > include/grub/emu/config.h | 1 +
> > > > util/config.c | 10 ++++++++++
> > > > util/grub-install.c | 14 ++++++++++++++
> > > > util/grub-mkrelpath.c | 6 ++++++
> > > > 6 files changed, 53 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/grub-core/osdep/linux/getroot.c
> > > > b/grub-core/osdep/linux/getroot.c
> > > > index 7dd775d2a..7c29b3523 100644
> > > > --- a/grub-core/osdep/linux/getroot.c
> > > > +++ b/grub-core/osdep/linux/getroot.c
> > > > @@ -373,6 +373,7 @@ get_btrfs_fs_prefix (const char *mount_path)
> > > > return NULL;
> > > > }
> > > >
> > > > +int use_relative_path_on_btrfs = 0;
> > > >
> > > > char **
> > > > grub_find_root_devices_from_mountinfo (const char *dir, char
> > **relroot)
> > > > @@ -516,6 +517,12 @@ again:
> > > > {
> > > > ret = grub_find_root_devices_from_btrfs (dir);
> > > > fs_prefix = get_btrfs_fs_prefix (entries[i].enc_path);
> > > > + if (use_relative_path_on_btrfs)
> > > > + {
> > > > + if (fs_prefix)
> > > > + free (fs_prefix);
> > > > + fs_prefix = xstrdup ("/");
> > > > + }
> > > > }
> > > > else if (!retry && grub_strcmp (entries[i].fstype, "autofs") ==
> > 0)
> > > > {
> > > > diff --git a/grub-core/osdep/unix/config.c
> > b/grub-core/osdep/unix/config.c
> > > > index 0b1f7618d..0ce0e309a 100644
> > > > --- a/grub-core/osdep/unix/config.c
> > > > +++ b/grub-core/osdep/unix/config.c
> > > > @@ -82,6 +82,19 @@ grub_util_load_config (struct grub_util_config *cfg)
> > > > if (v)
> > > > cfg->grub_distributor = xstrdup (v);
> > > >
> > > > + v = getenv ("SUSE_BTRFS_SNAPSHOT_BOOTING");
> > > > + if (v)
> > > > + {
> > > > + if (grub_strncmp(v, "true", sizeof ("true") - 1) == 0)
> > > > + {
> > > > + cfg->is_suse_btrfs_snapshot_enabled = 1;
> > > > + }
> > > > + else
> > > > + {
> > > > + cfg->is_suse_btrfs_snapshot_enabled = 0;
> > > > + }
> > > > + }
> > > > +
> > > > cfgfile = grub_util_get_config_filename ();
> > > > if (!grub_util_is_regular (cfgfile))
> > > > return;
> > > > @@ -105,8 +118,8 @@ grub_util_load_config (struct grub_util_config
> > *cfg)
> > > > *ptr++ = *iptr;
> > > > }
> > > >
> > > > - strcpy (ptr, "'; printf
> > > > \"GRUB_ENABLE_CRYPTODISK=%s\\nGRUB_DISTRIBUTOR=%s\\n\" "
> > > > - "\"$GRUB_ENABLE_CRYPTODISK\" \"$GRUB_DISTRIBUTOR\"");
> > > > + strcpy (ptr, "'; printf
> > > >
> > \"GRUB_ENABLE_CRYPTODISK=%s\\nGRUB_DISTRIBUTOR=%s\\nSUSE_BTRFS_SNAPSHOT_BOOTING=%s\\n\"
> > > > "
> > > > + "\"$GRUB_ENABLE_CRYPTODISK\" \"$GRUB_DISTRIBUTOR\"
> > > > \"$SUSE_BTRFS_SNAPSHOT_BOOTING\"");
> > > >
> > > > argv[2] = script;
> > > > argv[3] = '\0';
> > > > diff --git a/include/grub/emu/config.h b/include/grub/emu/config.h
> > > > index 875d5896c..c9a7e5f4a 100644
> > > > --- a/include/grub/emu/config.h
> > > > +++ b/include/grub/emu/config.h
> > > > @@ -37,6 +37,7 @@ struct grub_util_config
> > > > {
> > > > int is_cryptodisk_enabled;
> > > > char *grub_distributor;
> > > > + int is_suse_btrfs_snapshot_enabled;
> > > > };
> > > >
> > > > void
> > > > diff --git a/util/config.c b/util/config.c
> > > > index ebcdd8f5e..f044a880a 100644
> > > > --- a/util/config.c
> > > > +++ b/util/config.c
> > > > @@ -42,6 +42,16 @@ grub_util_parse_config (FILE *f, struct
> > > > grub_util_config *cfg, int simple)
> > > > cfg->is_cryptodisk_enabled = 1;
> > > > continue;
> > > > }
> > > > + if (grub_strncmp (ptr, "SUSE_BTRFS_SNAPSHOT_BOOTING=",
> > > > + sizeof ("SUSE_BTRFS_SNAPSHOT_BOOTING=") - 1)
> > == 0)
> > > > + {
> > > > + ptr += sizeof ("SUSE_BTRFS_SNAPSHOT_BOOTING=") - 1;
> > > > + if (*ptr == '"' || *ptr == '\'')
> > > > + ptr++;
> > > > + if (grub_strncmp(ptr, "true", sizeof ("true") - 1) == 0)
> > > > + cfg->is_suse_btrfs_snapshot_enabled = 1;
> > > > + continue;
> > > > + }
> > > > if (grub_strncmp (ptr, "GRUB_DISTRIBUTOR=",
> > > > sizeof ("GRUB_DISTRIBUTOR=") - 1) == 0)
> > > > {
> > > > diff --git a/util/grub-install.c b/util/grub-install.c
> > > > index 7dc5657bb..ec3ed4967 100644
> > > > --- a/util/grub-install.c
> > > > +++ b/util/grub-install.c
> > > > @@ -843,6 +843,8 @@ try_open (const char *path)
> > > > }
> > > > #endif
> > > >
> > > > +extern int use_relative_path_on_btrfs;
> > > > +
> > > > int
> > > > main (int argc, char *argv[])
> > > > {
> > > > @@ -876,6 +878,9 @@ main (int argc, char *argv[])
> > > >
> > > > grub_util_load_config (&config);
> > > >
> > > > + if (config.is_suse_btrfs_snapshot_enabled)
> > > > + use_relative_path_on_btrfs = 1;
> > > > +
> > > > if (!bootloader_id && config.grub_distributor)
> > > > {
> > > > char *ptr;
> > > > @@ -1366,6 +1371,15 @@ main (int argc, char *argv[])
> > > > relative_grubdir = xstrdup ("/");
> > > > }
> > > >
> > > > + if (config.is_suse_btrfs_snapshot_enabled
> > > > + && grub_strncmp(grub_fs->name, "btrfs", sizeof ("btrfs") - 1)
> > == 0)
> > > > + {
> > > > + if (!load_cfg_f)
> > > > + load_cfg_f = grub_util_fopen (load_cfg, "wb");
> > > > + have_load_cfg = 1;
> > > > + fprintf (load_cfg_f, "set btrfs_relative_path='y'\n");
> > > > + }
> > > > +
> > > > char *prefix_drive = NULL;
> > > > char *install_drive = NULL;
> > > >
> > > > diff --git a/util/grub-mkrelpath.c b/util/grub-mkrelpath.c
> > > > index 47a241a39..5db7a9a7d 100644
> > > > --- a/util/grub-mkrelpath.c
> > > > +++ b/util/grub-mkrelpath.c
> > > > @@ -40,9 +40,12 @@ struct arguments
> > > > };
> > > >
> > > > static struct argp_option options[] = {
> > > > + {"relative", 'r', 0, 0, "use relative path on btrfs", 0},
> > > > { 0, 0, 0, 0, 0, 0 }
> > > > };
> > > >
> > > > +extern int use_relative_path_on_btrfs;
> > > > +
> > > > static error_t
> > > > argp_parser (int key, char *arg, struct argp_state *state)
> > > > {
> > > > @@ -52,6 +55,9 @@ argp_parser (int key, char *arg, struct argp_state
> > > > *state)
> > > >
> > > > switch (key)
> > > > {
> > > > + case 'r':
> > > > + use_relative_path_on_btrfs = 1;
> > > > + break;
> > > > case ARGP_KEY_ARG:
> > > > if (state->arg_num == 0)
> > > > arguments->pathname = xstrdup (arg);
> > > > --
> > > > 2.46.2
> > > >
> > > >
> > > > _______________________________________________
> > > > Grub-devel mailing list
> > > > Grub-devel@gnu.org
> > > > https://lists.gnu.org/mailman/listinfo/grub-devel
> > > >
> >
> > > _______________________________________________
> > > Grub-devel mailing list
> > > Grub-devel@gnu.org
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
- Re: [PATCH v1 25/37] grub.texi: Make our info pages say "grub2" where appropriate., (continued)
[PATCH v1 36/37] efi: Add grub_efi_allocate_pool() and grub_efi_free_pool() wrappers., Leo Sandoval, 2024/10/07
[PATCH v1 20/37] net/tcp: add window scaling support, Leo Sandoval, 2024/10/07
[PATCH v1 32/37] btdfs: grub2-btrfs-05-grub2-mkconfig, Leo Sandoval, 2024/10/07
[PATCH v1 34/37] btrfs: Fallback to old subvol name scheme to support old snapshot config, Leo Sandoval, 2024/10/07
[PATCH v1 29/37] btrfs: export btrfs_subvol and btrfs_subvolid, Leo Sandoval, 2024/10/07
[PATCH v1 35/37] btrfs: Grub not working correctly with btrfs snapshots (bsc#1026511), Leo Sandoval, 2024/10/07