Petr Uzel wrote: > Since kernel 2.6.26, kernel allows partitions on loopback devices. > Implement support for this feature in parted. > > * libparted/arch/linux.c (_sysfs_int_entry_from_dev): New function. > (_loop_get_partition_range): New function. > (_device_get_partition_range): Add special handling for loop devices. > * NEWS: Mention this change.
Thanks for doing this. > Signed-off-by: Petr Uzel <[email protected]> Please omit Signed-off-by lines from the log in the future. When you're the author, that's just redundant. To do that just for parted, run this in your parted clone: git config format.signoff false > +** New features > + > + parted has improved support for partitionable loopback devices Thanks for adding a NEWS entry. Would you please also add a test to exercise this feature? > diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c > index adaf89a..2c0619a 100644 > --- a/libparted/arch/linux.c > +++ b/libparted/arch/linux.c > @@ -2419,32 +2419,74 @@ _blkpg_remove_partition (PedDisk* disk, int n) > BLKPG_DEL_PARTITION); > } > > -/* > - * The number of partitions that a device can have depends on the kernel. > - * If we don't find this value in /sys/block/DEV/range, we will use our own > - * value. > - */ > -static unsigned int > -_device_get_partition_range(PedDevice* dev) Please add a brief comment for each of these new functions, e.g., /* Read the integer from /sys/block/DEV_BASE/ENTRY and set *VAL to that value, where DEV_BASE is the last component of DEV->path. Upon success, return true. Otherwise return false. */ ...which (along with that "bool ok") suggests that we can make the return type be "bool" rather than "int": > +static int > +_sysfs_int_entry_from_dev(PedDevice *dev, const char *entry, int *val) > { > - int range, r; > + int r; > char path[128]; > FILE* fp; > bool ok; > > - r = snprintf(path, sizeof(path), "/sys/block/%s/range", > - last_component(dev->path)); > + r = snprintf(path, sizeof(path), "/sys/block/%s/%s", > + last_component(dev->path), entry); > if (r < 0 || r >= sizeof(path)) > - return MAX_NUM_PARTS; > + return 0; s/0/false/ > > fp = fopen(path, "r"); > if (!fp) > - return MAX_NUM_PARTS; > + return 0; s/0/false/ > - ok = fscanf(fp, "%d", &range) == 1; > + ok = fscanf(fp, "%d", val) == 1; > fclose(fp); > > - /* (range <= 0) is none sense.*/ > + return ok; > +} > + > +static unsigned int > +_loop_get_partition_range(PedDevice* dev) > +{ > + int max_part; > + FILE* fp; Please move this declaration down to the first use: > + bool ok = 0; > + > + /* max_part module param is exported since kernel 3.0 */ > + fp = fopen("/sys/module/loop/parameters/max_part", "r"); FILE *fp = fopen... > + if (fp) { > + ok = fscanf(fp, "%d", &max_part) == 1; > + fclose(fp); > + } > + > + if (ok) > + return max_part > 0 ? max_part : 0; > + > + /* > + * max_part is not exported - check ext_range; > + * device supports partitions if ext_range > 1 > + */ > + int range; > + ok = _sysfs_int_entry_from_dev(dev, "range", &range); > + > + return ok && range > 1 ? range : 0; > + > +} > + > +/* > + * The number of partitions that a device can have depends on the kernel. > + * If we don't find this value in /sys/block/DEV/range, we will use our own > + * value. > + */ > +static unsigned int > +_device_get_partition_range(PedDevice* dev) This parameter seems like it should be "const", along with the two others, above. If possible, please adjust: _device_get_partition_range(PedDevice const* dev) > +{ > + int range; > + bool ok; Please move both of these declarations down: > + > + /* loop handling is special */ > + if (dev->type == PED_DEVICE_LOOP) > + return _loop_get_partition_range(dev); > + > + ok = _sysfs_int_entry_from_dev(dev, "range", &range); int range; bool ok = _sysfs_int_entry_from_dev(dev, "range", &range); > return ok && range > 0 ? range : MAX_NUM_PARTS; _______________________________________________ parted-devel mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/parted-devel
