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

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4585 bytes --]

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, 63 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  <cjwatson@ubuntu.com>
> +
> +	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  <phcoder@gmail.com>
>  
>  	* 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.
>  
>  @deffn Command search @
>   [@option{--file}|@option{--label}|@option{--fs-uuid}] @
> - [@option{--set} [var]] [@option{--no-floppy}] name
> + [@option{--set} [var]] [@option{--no-floppy}] @
> + [@option{--exclude} @var{name} ...] name
>  Search devices by file (@option{-f}, @option{--file}), filesystem label
>  (@option{-l}, @option{--label}), or filesystem UUID (@option{-u},
>  @option{--fs-uuid}).
> @@ -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
> +@file{/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
>  @samp{search --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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]