lkml.org 
[lkml]   [2007]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] add_partition silently ignored errors
    On Tue, Oct 30, 2007 at 06:31:12PM +0100, Cornelia Huck wrote:
    > On Tue, 30 Oct 2007 09:56:08 -0700,
    > Dirk Hohndel <hohndel@linux.intel.com> wrote:
    >
    > > > > IIRC, Al recently vetoed a similar patch. As far as I'm concerned, with
    > > > > the correct return values, the patch then looks fine to me.

    So Al, are you ok with this one?

    > > > We need some kind of check concerning the kobject to avoid mysterious
    > > > errors (especially checking for the failed kobject_add() is needed).
    > > > Whether we want just to inform the user of the failure instead of
    > > > failing the function is another question.
    > >
    > > What are you suggesting? I'd love to make the behaviour consistent everywhere
    > > (and am willing to go through things in order to make that happen), but what is
    > > the consistent behaviour that we'd want?
    >
    > I'd be fine with just propagating the error after cleanup (that is what
    > for example the driver core usually does), but I don't know the
    > surrounding code well enough for a definitive answer.

    Ok, I think I have it consistent now. I also ran it through checkpatch.pl :-)

    /D


    [FILESYSTEM] add_partition ignores errors

    Signed-off-by: Dirk Hohndel <hohndel@linux.intel.com>

    ---
    block/ioctl.c | 9 +++++++--
    fs/partitions/check.c | 36 +++++++++++++++++++++++++++++-------
    include/linux/genhd.h | 2 +-
    3 files changed, 37 insertions(+), 10 deletions(-)

    diff --git a/block/ioctl.c b/block/ioctl.c
    index 52d6385..662ec18 100644
    --- a/block/ioctl.c
    +++ b/block/ioctl.c
    @@ -16,7 +16,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
    struct blkpg_partition p;
    long long start, length;
    int part;
    - int i;
    + int i, err;

    if (!capable(CAP_SYS_ADMIN))
    return -EACCES;
    @@ -61,7 +61,12 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
    }
    }
    /* all seems OK */
    - add_partition(disk, part, start, length, ADDPART_FLAG_NONE);
    + err = add_partition(disk, part, start, length,
    + ADDPART_FLAG_NONE)
    + if (err) {
    + mutex_unlock(&bdev->bd_mutex);
    + return err;
    + }
    mutex_unlock(&bdev->bd_mutex);
    return 0;
    case BLKPG_DEL_PARTITION:
    diff --git a/fs/partitions/check.c b/fs/partitions/check.c
    index 722e12e..dea79bd 100644
    --- a/fs/partitions/check.c
    +++ b/fs/partitions/check.c
    @@ -368,13 +368,14 @@ void delete_partition(struct gendisk *disk, int part)
    kobject_put(&p->kobj);
    }

    -void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, int flags)
    +int add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, int flags)
    {
    struct hd_struct *p;
    + int err = 0;

    p = kzalloc(sizeof(*p), GFP_KERNEL);
    if (!p)
    - return;
    + return -ENOMEM;

    p->start_sect = start;
    p->nr_sects = len;
    @@ -390,20 +391,38 @@ void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len,
    p->kobj.parent = &disk->kobj;
    p->kobj.ktype = &ktype_part;
    kobject_init(&p->kobj);
    - kobject_add(&p->kobj);
    + err = kobject_add(&p->kobj);
    + if (err)
    + goto out_put;
    if (!disk->part_uevent_suppress)
    kobject_uevent(&p->kobj, KOBJ_ADD);
    - sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem");
    + err = sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem");
    + if (err)
    + goto out_cleanup;
    if (flags & ADDPART_FLAG_WHOLEDISK) {
    static struct attribute addpartattr = {
    .name = "whole_disk",
    .mode = S_IRUSR | S_IRGRP | S_IROTH,
    };

    - sysfs_create_file(&p->kobj, &addpartattr);
    + err = sysfs_create_file(&p->kobj, &addpartattr);
    + if (err) {
    + sysfs_remove_link(&p->kobj, "subsystem");
    + goto out_cleanup;
    + }
    }
    partition_sysfs_add_subdir(p);
    disk->part[part-1] = p;
    +
    + return 0;
    +
    +out_cleanup:
    + if (!disk->part_uevent_suppress)
    + kobject_uevent(&p->kobj, KOBJ_REMOVE);
    + kobject_del(&p->kobj);
    +out_put:
    + kobject_put(&p->kobj);
    + return err;
    }

    static char *make_block_name(struct gendisk *disk)
    @@ -530,7 +549,7 @@ exit:
    int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
    {
    struct parsed_partitions *state;
    - int p, res;
    + int p, res, err;

    if (bdev->bd_part_count)
    return -EBUSY;
    @@ -554,8 +573,11 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
    if (from + size > get_capacity(disk)) {
    printk(" %s: p%d exceeds device capacity\n",
    disk->disk_name, p);
    + return -EBUSY;
    }
    - add_partition(disk, p, from, size, state->parts[p].flags);
    + err = add_partition(disk, p, from, size, state->parts[p].flags);
    + if (err)
    + return err;
    #ifdef CONFIG_BLK_DEV_MD
    if (state->parts[p].flags & ADDPART_FLAG_RAID)
    md_autodetect_dev(bdev->bd_dev+p);
    diff --git a/include/linux/genhd.h b/include/linux/genhd.h
    index a47b802..ae6af2c 100644
    --- a/include/linux/genhd.h
    +++ b/include/linux/genhd.h
    @@ -414,7 +414,7 @@ struct unixware_disklabel {
    char *disk_name (struct gendisk *hd, int part, char *buf);

    extern int rescan_partitions(struct gendisk *disk, struct block_device *bdev);
    -extern void add_partition(struct gendisk *, int, sector_t, sector_t, int);
    +extern int add_partition(struct gendisk *, int, sector_t, sector_t, int);
    extern void delete_partition(struct gendisk *, int);
    extern void printk_all_partitions(void);

    --
    gitgui.0.8.4.g8d863
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-10-30 23:59    [W:0.040 / U:30.200 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site