Re: [PATCH] ZFS/other CoW FS save_env support

[Michael Chang]

On Mon, Feb 24, 2020 at 10:30:36AM -0800, Paul Dagnelie wrote:
> On Mon, Feb 24, 2020 at 3:03 AM Daniel Kiper <address@hidden> wrote:
> >
> >
> > Why "root" not "boot"?
> That was a typo on my part; the code uses grub_guess_root_device to
> find the devices backing the default grub directory, but in most
> configurations, this should attempt to locate the boot filesystem
> instead of the root. I was uncertain of a better way to consistently
> determine the boot filesystem, and this portion of the code was copied
> from another GRUB patch
> (https://build.opensuse.org/package/view_file/openSUSE:Factory/grub2/grub2-grubenv-in-btrfs-header.patch?expand=1).

I used to have the same confusion as well. But I tend to interpret that
as "root device/filesystem for grub" and thus not from the viewpoint of
a linux system. Essentially we have been using "root" as the name for
the environment variable pointing to the grub device in which /boot
resides and the concept just carried over from it to the description. 

So in my opinion it certainly not a typo but rather a common expression
mentioned by the source code. Although it would become misleading at
times when people have to take in with the linux system, I still prefer
to keep it as it wasn't that much a mistake by itself. :)  

Thanks.
Michael

> 
> >
> > Yes, please split the patch into smaller patches. Please do one logical
> > change per patch.
> >
> > I quickly went through the patch and pointed things which I spotted at
> > first sight. I will provide more comments when you split the patch into
> > separate patches.
> >
> > Next time please CC following people too: address@hidden,
> > address@hidden and address@hidden.
> Understood! I will post an updated version hopefully today or tomorrow.
> 
> >
> > I think that you can drop parenthesis here. And please use NULL instead of 
> > 0.
> Will do. In general, this was one of my questions about writing new
> code in this code base. There are several things where I decided to go
> with consistency with surrounding code instead of what would commonly
> be preferred in modern coding standards or by the style guide (see
> also, the block comment style you mentioned further down). Is there a
> preference in this codebase against consistency when other
> considerations are also relevant?
> 
> 
> 
> -- 
> Paul Dagnelie
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel