Hi,
On Wed, 25 May 2016, Stefan Hundhammer wrote:
> Brainstorming approach #4: Add read-only mode to parted
> =======================================================
>
> "parted -l", which we are using in this situation, should already be a
> read-only operation. Unfortunately, strace shows that this is not the
> case: It starts with opening the disk device read-only, reads
> information, closes it - and then for whatever reason opens it again
> read-write which triggers the code that sends the ioctl() to make the
> kernel re-read the partition table.
That's of course stupid of parted.
> We consider that a bug
> (https://bugzilla.suse.com/show_bug.cgi?id=979275), but it does not seem
> to be easy to fix. Any contribution to that would be very welcome.
See attached, fixes the bug by always opening read-only and lazily
switching to read-write only when necessary (i.e. a write or flush
operation occurs). No libparted API changes, purely internal to the linux
"backend". I think I got all places where _ensure_read_write must be
called, if you hit problems it should be easy to diagnose, because a
forgotten call will lead to obvious errors for using a write on a
read-only FD, so should be easy to diagnose and add.
(also contains a local fix when not using blkid)
Ciao,
Michael.
Index: parted-3.1/libparted/arch/linux.c
===================================================================
--- parted-3.1.orig/libparted/arch/linux.c 2016-05-25 17:23:19.000000000 +0200
+++ parted-3.1/libparted/arch/linux.c 2016-05-25 18:18:01.000000000 +0200
@@ -440,6 +440,19 @@ _is_virtblk_major (int major)
return _major_type_in_devices (major, "virtblk");
}
+static int linux_close (PedDevice* dev);
+static void
+_ensure_read_write (PedDevice *dev)
+{
+ LinuxSpecific* arch_specific = LINUX_SPECIFIC (dev);
+ if (arch_specific->rw)
+ return;
+ if (!linux_close(dev))
+ return;
+ _device_open (dev, RW_MODE);
+ arch_specific->rw = 1;
+}
+
#ifdef ENABLE_DEVICE_MAPPER
static int
_is_dm_major (int major)
@@ -1513,6 +1526,7 @@ _flush_cache (PedDevice* dev)
if (dev->read_only)
return;
+ _ensure_read_write (dev);
dev->dirty = 0;
ioctl (arch_specific->fd, BLKFLSBUF);
@@ -1560,7 +1574,7 @@ _device_open_ro (PedDevice* dev)
static int
linux_open (PedDevice* dev)
{
- return _device_open (dev, RW_MODE);
+ return _device_open (dev, RD_MODE);
}
static int
@@ -1598,6 +1612,10 @@ retry:
}
} else {
dev->read_only = 0;
+ if (flags == WR_MODE || flags == RW_MODE)
+ arch_specific->rw = 1;
+ else
+ arch_specific->rw = 0;
}
/* With kernels < 2.6 flush cache for cache coherence issues */
@@ -1835,6 +1853,7 @@ _write_lastoddsector (PedDevice* dev, co
PED_ASSERT(dev != NULL);
PED_ASSERT(buffer != NULL);
+ _ensure_read_write (dev);
arch_specific = LINUX_SPECIFIC (dev);
@@ -1883,6 +1902,7 @@ linux_write (PedDevice* dev, const void*
return 1;
}
+ _ensure_read_write (dev);
if (_get_linux_version() < KERNEL_VERSION (2,6,0)) {
/* Kludge. This is necessary to read/write the last
block of an odd-sized disk, until Linux 2.5.x kernel fixes.
@@ -2377,6 +2397,7 @@ _blkpg_part_command (PedDevice* dev, str
LinuxSpecific* arch_specific = LINUX_SPECIFIC (dev);
struct blkpg_ioctl_arg ioctl_arg;
+ _ensure_read_write (dev);
ioctl_arg.op = op;
ioctl_arg.flags = 0;
ioctl_arg.datalen = sizeof (struct blkpg_partition);
Index: parted-3.1/libparted/arch/linux.h
===================================================================
--- parted-3.1.orig/libparted/arch/linux.h 2016-05-25 17:56:59.000000000 +0200
+++ parted-3.1/libparted/arch/linux.h 2016-05-25 17:57:14.000000000 +0200
@@ -30,6 +30,7 @@ struct _LinuxSpecific {
int fd;
int major;
int minor;
+ int rw;
char* dmtype; /**< device map target type */
#if defined __s390__ || defined __s390x__
unsigned int real_sector_size;
Index: parted-3.1/parted/parted.c
===================================================================
--- parted-3.1.orig/parted/parted.c 2016-05-25 17:23:19.000000000 +0200
+++ parted-3.1/parted/parted.c 2016-05-25 17:27:09.000000000 +0200
@@ -60,7 +60,9 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
+#if USE_BLKID
#include <blkid/blkid.h>
+#endif
#ifdef ENABLE_MTRACE
#include <mcheck.h>
@@ -659,7 +661,7 @@ _adjust_end_if_iec (PedSector* start, Pe
}
}
-#ifdef USE_BLKID
+#if USE_BLKID
static int
_wipe_signatures (PedDevice *dev, PedSector start, PedSector length)
{
