bugGNU GRUB - Bugs: bug #32931, LVM write support, patch included

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #32931: LVM write support, patch included

Submitted by:  Valentin Hilbig <tino>
Submitted on:  Sun 27 Mar 2011 10:16:44 PM UTC  
 
Category: Disk & PartitionSeverity: Major
Priority: 5 - NormalItem Group: Feature Request
Status: Wont FixPrivacy: Public
Assigned to: NoneOriginator Name: 
Open/Closed: ClosedRelease: 
Release: 1.98Reproducibility: Every Time
Planned Release: None

Wed 30 Mar 2011 08:01:23 PM UTC, comment #3:

Allowing only when user specifically selects so sounds good in theory, in practice, though, people will just force it without thinking of consequences on their system. Also LVM would try to repair itself and so will conflict with GRUB. If nothing else, if it stays non-official it clearly says that it can damage the data.

> OTOH not having LVM write support is a complete desaster

There is a difference between considering something low-quality and actively damaging it. I've tested LVM mirror only once for testing GRUB in such conditions so I can't say anything about its quality. But even without lvm mirror, if you have snapshots then probably some blocks are shared and writing to them will change the data in snapshots as well.
The LVM write is a no-go unless a strategy is proposed which will avoid corrupting data no matter what (this would need to be discussed at grub-devel@gnu.org)
The consensus reached in last LVM discussion was to use embedding zone. Same consensus was reached for other similar situations: mdraid, zfs and btrfs.
It would be desirable to ask LVM maintainers to allow us to use some space for such things. E.g. we could use the space after the last usable block. It would be helpful if one could tell to have at least e.g. 64K there. This would need a small revival discussion at grub-devel@gnu.org. None of this was done yet.
Since LVM write probably will never be accepted, marking it as wontfix. For embedding area please discuss upstream.

Vladimir Serbinenko <phcoder>
Project Administrator
Mon 28 Mar 2011 12:08:37 AM UTC, comment #2:

Thanks, good point, I missed that completely.

Lession learned, having write support in LVM is probably evil. OTOH not having LVM write support is a complete desaster (see below). So I hope a suitable fix for the time being (until somebody fixes it 101%) can be to allow users to enable write support if they really want it.

In the new patch (lvm-write.2.patch) this is controlled by environment variable "lvm_write_enabled". It increases lvm.mod by 560 bytes.

Example:

Boot grub
c
insmod lvm
set prefix=(VGNAME-boot)/grub
load_env
set saved_entry=2
save_env saved_entry
# This fails now
set lvm_write_enabled=true
save_env saved_enty lvm_write_enabled
# now worked
reboot

Some rationale on this patch:

At my side Linux is installed into LVM and the GRUB config must be on LVM too. My setup is Multiboot and Windows breaks if GRUB does not automatically boot into it after hibernation.

So a not working save_env is a complete showstopper at my side. And I don't think I am alone with this!

As I cannot provide a separate /boot partition in my environment either, enabling LVM writes are the only way to get my setup working here.

I hope, to disable writes by default and enable them on user's request is the best of both worlds for now.

The downside is that it does not work perfectly on mirrored LVM. However I would say that LVM mirror support is extremely dangerous anyway, so the trouble GRUB adds does not count much. My recommendation on mirroring is: Use MD instead, it just works.

Note:

The patch does not need to make it into dist soon, as you can see, patching yourself is not much effort.

(file #23033)

Valentin Hilbig <tino>
Sun 27 Mar 2011 10:30:17 PM UTC, comment #1:

I suppose that you didn't follow the discussion on LVM writing. Basically the problem is safety: any bug in LVM writing code would result in disk corruption. Your patch actually perfectly illustrates one of the points which were discussed: in case of multiple copies (e.g. stripped LVM) only one is needed for read but you need to update all of them when you write. Your patch will surely corrupt any mirrored LVM. I recommend to discuss the approach on -unavailable- before proceding.

Vladimir Serbinenko <phcoder>
Project Administrator
Sun 27 Mar 2011 10:16:44 PM UTC, original submission:

Here is a patch for grub_lvm_write() based on grub_lvm_read() to add write support on LVM. This makes save_env and GRUB_DEFAULT=saved work as expected for LVM volumes on GNU/Linux, too.

This also should fix
https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/625234

Hope I did not mess too much with your coding standards.
Note that the diff is against Debian Squeeze i386 version of GRUB (1.98.something).
lvm.mod increases by 332 bytes, it was tested to work with Squeeze, as follows:

apt-get source grub2
cd grub2-*/
patch -p1 <../lvm-write.patch
mkdir OBJ
cd OBJ
../configure
make -kj2
# ignore the zfs.c error note, hence the -k
sudo cp --backup=t lvm.mod /usr/lib/grub/i386-pc/lvm.mod
sudo grub-install --no-floppy /dev/sdYOURDRIVE

The patch applies (with offset -1) against the current trunk as well, but I did not test the result:

bzr branch bzr://bzr.savannah.gnu.org/grub/trunk/grub
cd grub/grub-core
patch -p1 <../../lvm-write.patch
cd ..
./autogen.sh
mkdir OBJ
cd OBJ
../configure
make -j2
ls -al grub-core/lvm.mod /usr/lib/grub/i386-pc/lvm.mod

Example use (GRUB 1.98):

Boot into GRUB
c
# next 2 lines are redundant if you already use lvm in core.img
insmod lvm
set prefix=(VGNAME-boot)/grub
set debug=lvm
list_env
load_env
set saved_entry=2
save_env saved_entry
set debug=
list_env
reboot

Valentin Hilbig <tino>

 

Attached Files
file #23033:  lvm-write.2.patch added by tino (4KiB - application/octet-stream)
file #23031:  lvm-write.patch added by tino (3KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by phcoder (Posted a comment)
  • -unavailable- added by tino (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 30 Mar 2011 08:01:23 PM UTCphcoderStatusNone=>Wont Fix
      Open/ClosedOpen=>Closed
    Mon 28 Mar 2011 12:08:37 AM UTCtinoAttached File-=>Added lvm-write.2.patch, #23033
    Sun 27 Mar 2011 10:16:44 PM UTCtinoAttached File-=>Added lvm-write.patch, #23031

    Back to the top


    Powered by Savane 3.1-cleanup1