Hi,
On 08/26/2009 03:16 PM, Joel Granados wrote:
On Wed, Aug 26, 2009 at 09:13:34AM +0200, Hans de Goede wrote:
Well, the thing you are expressing here is the difference between
commit_to_os and commit_to_dev. one tells the kernel what parted has in
memory and the other writes to disk.
Erm, no, these are both commit to os. The difference is BLKPG is modifying the kernels
where does ped_disk_commit_to_dev call BLKPG or BLKRRPART? I see it
getting to BLKFLSBUF, but not to the others. So ped_disk_commit_to_dev
does not commit to os. it simply wirtes to dev.
As discussed on irc I'm only talking about commit_to_os, to quote myself 5 lines above:
"no, these are both commit to os"
partitiontable to represent parted's in memory view. Where as BLKRRPART tells the kernel
to re-read the table from disk and use that, since on a normal boot the kernel reads the
table from disk, telling it to re-read it seems best to me, as that leads to the kernel
actually using the same view of the table as will be used during a normal boot of the
system (although I admit the 2 views should not differ, currently they can due to multiple
issues with parted's BLKPG code).
I think a much better solution would be a commit_kernel_read_dev
function, that implements just what you suggested.
IMHO that is what commit_to_os is supposed to do, and what it does in some cases. Currently
parted is not very consistend in some scenario's it will use BLKPG and ignore ebusy errors
Ack, not consistent...
and in others it will use BLKRRPART and report EBUSY errors. It seems more consistent to
me to just always use BLKRRPART.
See the attached patch which I'm currently using for testing.
Regards,
Hans
diff -up parted-1.9.0/libparted/arch/linux.c~ parted-1.9.0/libparted/arch/linux.c
--- parted-1.9.0/libparted/arch/linux.c~ 2009-08-26 06:40:11.000000000 +0200
+++ parted-1.9.0/libparted/arch/linux.c 2009-08-26 06:51:21.000000000 +0200
@@ -41,7 +41,6 @@
#include<libdevmapper.h>
#endif
-#include "blkpg.h"
#include "../architecture.h"
#include "dirname.h"
@@ -587,22 +586,6 @@ _get_linux_version ()
return kver = KERNEL_VERSION (major, minor, teeny);
}
-static int
-_have_devfs ()
-{
- static int have_devfs = -1;
- struct stat sb;
...
@@ -2621,19 +2421,6 @@ linux_disk_commit (PedDisk* disk)
return _dm_reread_part_table (disk);
#endif
if (disk->dev->type != PED_DEVICE_FILE) {
- /* The ioctl() command BLKPG_ADD_PARTITION does not notify
- * the devfs system; consequently, /proc/partitions will not
- * be up to date, and the proper links in /dev are not
- * created. Therefore, if using DevFS, we must get the kernel
- * to re-read and grok the partition table.
- */
- /* Work around kernel dasd problem so we really do BLKRRPART */
- if (disk->dev->type != PED_DEVICE_DASD&&
- _have_blkpg ()&& !_have_devfs ()) {
- if (_disk_sync_part_table (disk))
- return 1;
- }
-
return _kernel_reread_part_table (disk->dev);
}
Instead of removing a whole bunch of code I prefer addint an additional
function and deprecating these that you erased. what do you think?
I think the current behaviour is broken and inconsistent, so just kill the BLKPG code
and be done with it much easier then adding another function. The only possible regression
is that people might get EBUSY errors in cases where before they got lucky and things
happened to end up correct in the end. But users should be prepared for EBUSY errors
anyways as in certain cases libparted will already return those.
Regards,
Hans
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
