[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Multiple issues with commit_to_os (Linux)

From: Hans de Goede
Subject: Multiple issues with commit_to_os (Linux)
Date: Wed, 26 Aug 2009 16:23:44 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20090806 Fedora/3.0-3.7.b3.fc12 Thunderbird/3.0b3

Hi All,

Short intro: I'm an anaconda (The Fedora installer) developer, and
anaconda uses parted for its partitioning.

While testing partitionable mdraid I noticed that the kernels
view of the partition table never changes even though I was successfully
making commit_to_os() calls.

This has let to me diving into libparted's commit_to_os() code for Linux
and there are multiple issues hiding in there:

1) Parted reads /sys/block/foo/range to determine how many partitions
   the device type supports and then makes BLKPG ioctl's to update the
   kernels view of the partition table for partitions which fall into
   this range. However for example /sys/block/sda/range contains 16,
   there are 2 issue with libparted using this number:
   1) scsi major's only support 15 partitions, 1 of the range of 16
      is reserved for the whole device, yet libparted will try
      to notify the kernel about 16 partitions if present
   2) If the major's partition minor's run out, the kernel will switch
      to the mdp major for the other partitions, iow range no longer limits
      the number of partitions.

2) libparted assumes the user knows what he is doing, and will ignore
   -ebusy errors for partitions, assuming that the user is smart enough
   to only change unused partitions. Parted does this without checking
   if the partitions which return ebusy actually are unchanged causing
   REAL errors to get unreported (BAD, really really BAD)

3) because of 1) libparted will only sync 1 partition on /dev/md# devices
   (would be 0 if not for the of by 1 bug as all md#p# partitions use the
    mdp major), and it fails to even do that without reporting an error.


1) we can fix by simply not checking /sys/block/foo/range, but instead
   just syncing max partitions.

2) is more troublesome, we could just make -EBUSY n error,
   but that may annoy / bug some users. OTOH in certain cases libparted already
   falls back to BLKRRPART which will return EBUSY so users should already be
   prepared to handle EBUSY

3) Could be fixed by making libparted recognize mdraid as a device type
   and except mdraid from using BLKPG, like it already is doing with
   DASD, but it might be better to just get rid of using BLKPG al together.
   See below.

An even bigger problem IMHO is the use of the BLKPG ioctl instead of BLKRRPART
at all. What this does is tell the kernel parted's view of the partition table
and make it use that, instead of telling the kernel to reread the partition 
According to the parted sources this is done for the case where the kernel does
not know the disklabel type. However as soon as the system is rebooted, the 
will be using the kernel's view. So IMHO it would be much better to always use 
kernels view and just always call BLKRRPART in commit_to_os(), this would solve 
of the above issues, *and* make the way the system views the partition table 
between just after running parted and after a reboot.

I've attached a patch which removes the use of the BLKPG ioctl, notice that 
this also
removes a lot of special case code and workarounds, which existence to me 
indicates that using the BLKPG ioctl is a bad idea.



Attachment: parted-1.9.0-no-BLKPG.patch
Description: Text document

reply via email to

[Prev in Thread] Current Thread [Next in Thread]