On Sunday 15 March 2009 14:52:05 Bean wrote:
> On Sun, Mar 15, 2009 at 5:10 AM, Yoshinori K. Okuji <ok...@enbug.org> wrote:
> > On Friday 13 March 2009 21:23:19 phcoder wrote:
> >> Look at load_env/save_env commands and grub-editenv util
> >
> > Thanks. Now I really regret that I didn't find those additions earlier.
> >
> > I do not like this implementation for the following reasons:
> >
> > - The saved file is not plain text, unlike GRUB Legacy. This is a bad
> > choice. Please let me know the reason why it must be binary, if any.
>
> Hi,
>
> As the command need to write to disk using blocklist information,
> which is not always correct (such as tail packing, sparse block), I
> use a magic header for verification. The length field is used to
> indicate the length of the block. because the command can't expand
> file, otherwise it would need to update fs information, which is too
> much for grub.
I have read your code deeply, and have found the following:
- in reality, you don't deal with tail packing, but just refuse it, because of
this check in the hook function:
if ((offset != 0) || (length != GRUB_DISK_SECTOR_SIZE))
return;
- grub-editenv and save_env always write the magic at the beginning of a file,
thus the magic does not make sense (besides an extremely conservative sanity
check).
I would say that this is a regression from GRUB Legacy, which takes care of
partial sector allocations gracefully.
So, assuming that every filesystem driver calls a read hook correctly, we
should change it for:
- eliminating the magic header (although it could be kept for a safety guard
for accidental writes)
- refusing to write, only if any sparse blocks are in use (as GRUB may not
allocate new sectors physically)
- dealing with partial sectors - possibly due to tail packing - with some
complicated code
I will work towards this direction. I will first fix up the sector handling
and change the format to plain text. Naming changes are quite trivial, so
they can be done later.
Regards,
Okuji
>
> > - The command names are ugly. Why didn't anybody follow Pavel's advise
> > using "env"?
> >
> > - The utility name is also ugly. I like Pavel's suggestion "grub-env".
> >
> > If nobody stops me, I will rewrite it in one week, without caring about
> > backward compatibility.
>
> I have no objection for the rename, although there should be two env
> commands, one to load and one to save.
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
