Re: [Xen-devel] [PATCH 1/4] Add an option to exclude devices from search results.

[Jordan Uggla]

On Dec 13, 2013 6:28 AM, "Vladimir 'φ-coder/phcoder' Serbinenko"
<address@hidden> wrote:
>
> On 12.12.2013 16:37, Colin Watson wrote:
> > * grub-core/commands/search.c (struct search_ctx): Add excludes and
> > nexcludes.
> > (iterate_device): Ignore devices listed in ctx->excludes.
> > (FUNC_NAME): Accept excludes and nexcludes parameters.
> > (grub_cmd_do_search): Pass empty excludes.
> > * grub-core/commands/search_wrap.c (options): Add --exclude option.
> > (grub_cmd_search): Handle --exclude.
> > * include/grub/search.h (grub_search_fs_file): Update prototype.
> > (grub_search_fs_uuid): Likewise.
> > (grub_search_label): Likewise.
> > * docs/grub.texi (search): Document --exclude.
> > ---
> >  ChangeLog                        | 16 ++++++++++++++++
> >  docs/grub.texi                   |  7 ++++++-
> >  grub-core/commands/search.c      | 15 +++++++++++++--
> >  grub-core/commands/search_wrap.c | 27 ++++++++++++++++++++++-----
> >  include/grub/search.h            |  9 ++++++---
> >  5 files changed, insertions(+), 11 deletions(-)
> >
> > diff --git a/ChangeLog b/ChangeLog
> > index 9cec63f..766fe4b 100644
> > --- a/ChangeLog
> > +++ b/ChangeLog
> > @@ -1,3 +1,19 @@
> > +2013-12-12  Colin Watson  <address@hidden>
> > +
> > +     Add an option to exclude devices from search results.
> > +
> > +     * grub-core/commands/search.c (struct search_ctx): Add excludes and
> > +     nexcludes.
> > +     (iterate_device): Ignore devices listed in ctx->excludes.
> > +     (FUNC_NAME): Accept excludes and nexcludes parameters.
> > +     (grub_cmd_do_search): Pass empty excludes.
> > +     * grub-core/commands/search_wrap.c (options): Add --exclude option.
> > +     (grub_cmd_search): Handle --exclude.
> > +     * include/grub/search.h (grub_search_fs_file): Update prototype.
> > +     (grub_search_fs_uuid): Likewise.
> > +     (grub_search_label): Likewise.
> > +     * docs/grub.texi (search): Document --exclude.
> > +
> >  2013-12-11  Vladimir Serbinenko  <address@hidden>
> >
> >       * grub-core/normal/charset.c: Fix premature line wrap and crash.
> > diff --git a/docs/grub.texi b/docs/grub.texi
> > index 91fa1de..4c53665 100644
> > --- a/docs/grub.texi
> > +++ b/docs/grub.texi
> > @@ -4731,7 +4731,8 @@ unbootable. @xref{Using digital signatures}, for more 
> > information.
> >
> > address@hidden Command search @
> >   address@hidden|@option{--label}|@option{--fs-uuid}] @
> > - address@hidden [var]] address@hidden name
> > + address@hidden [var]] address@hidden @
> > + address@hidden @var{name} ...] name
> >  Search devices by file (@option{-f}, @option{--file}), filesystem label
> >  (@option{-l}, @option{--label}), or filesystem UUID (@option{-u},
> > address@hidden).
> > @@ -4743,6 +4744,10 @@ value of environment variable @var{var}.  The 
> > default variable is
> >  The @option{--no-floppy} option prevents searching floppy devices, which 
> > can
> >  be slow.
> >
> > +The @option{--exclude} option excludes any matches of the given GRUB device
> > +name.  For example, this may be used to search for any file named
> > address@hidden/boot/grub/grub.cfg} other than the one in a memdisk.
> > +
> >  The @samp{search.file}, @samp{search.fs_label}, and @samp{search.fs_uuid}
> >  commands are aliases for @samp{search --file}, @samp{search --label}, and
> > address@hidden --fs-uuid} respectively.
> > diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
> > index 16143a3..03cfea1 100644
> > --- a/grub-core/commands/search.c
> > +++ b/grub-core/commands/search.c
> > @@ -50,6 +50,8 @@ struct search_ctx
> >    int no_floppy;
> >    char **hints;
> >    unsigned nhints;
> > +  char **excludes;
> > +  unsigned nexcludes;
> >    int count;
> >    int is_cache;
> >  };
> > @@ -59,8 +61,15 @@ static int
> >  iterate_device (const char *name, void *data)
> >  {
> >    struct search_ctx *ctx = data;
> > +  unsigned i;
> >    int found = 0;
> >
> > +  for (i = 0; i < ctx->nexcludes; i++)
> > +    {
> > +      if (grub_strcmp (name, ctx->excludes[i]) == 0)
> > +     return 0;
> > +    }
> > +
> This makes behaviour strongly dependent on the unstable names. In case
> of memdisk it's not a problem because its name is stable but outside
> this specific use it's easily misusable command. E.g.
> search --exclude=hd0,1
> would be heavily dependent on disk naming and won't work because this
> check considers hd0,1 and hd0,msdos1 as distict. The only way to sanely
> use this interface is to do sth like
> search --exclude $boot
> to find next configfile, a bit like include_next directive but it
> creates a problem that the same disk may have several names. E.g. LVM
> have names base on name and UUID.
> This doesn't handle some configs like 2 autodetecting scripts including
> each other.
> This is complex problem and I'm unsure how to solve it.

How about excluding by uuid (or uuid-file)?

For example, the following (untested) grub.cfg provides what I expect
would be a nice interface for Xen users, using existing grub features
only:

# Search all devices for grub.cfg files to use, presenting a menu to
chose among them if
# more than  one is found.

timeout=5

probe --set=this_uuid --fs-uuid "(${root})"

insmod regexp

found_a_cfg=false
found_multiple_cfgs=false

for cfg in (*)/boot/grub/grub.cfg (*)/grub/grub.cfg (*)/boot/grub2/grub.cfg \
           (*)/grub2/grub.cfg; do
  regexp --set=1:cfg_device '^\((.*)\)/' "$cfg"
  probe --set=cfg_device_uuid --fs-uuid "(${cfg_device})"

  if [ "$this_uuid" != "$cfg_device_uuid" ]; then
    if [ "$found_a_cfg" = true ]; then found_multiple_cfgs=true; fi

    found_a_cfg=true

    menuentry "$cfg" "$cfg_device" "$cfg" {
      cfg_device="$2"
      cfg="$3"

      root="$cfg_device"
      configfile "$cfg"
    }
  fi
done

if [ "$cfg_found" != true ]; then
  echo $"Error: Could not find any grub.cfg files."
else if [ "$multiple_cfg_found" != true ]; then # We found exactly one grub.cfg
  timeout=0 # Don't bother showing the menu, just use the grub.cfg we found.
fi

One obvious downside of the above is that all devices (including
possibly slow ones) are searched rather than stopping at the first
grub.cfg found. To fix that properly would require a way to exclude
devices with certain UUIDs (or UUID-files) from being searched by the
search command.

-- 
Jordan Uggla (Jordan_U on irc.freenode.net)