As of now a common 'Invalid argument' return code is for a set of 5 errors, that is misleading from the end-user context. Albeit the proposed return error codes are the closest they might not be the perfect though. Since the idea is at least if we use 'Invalid argument' only when no btrfs SB found, that will solve the bug in this context. Any other simpler fix ? or should we go for ioctl arg V2 and have room for return error string ? Thanks. -Anand
/dev/sdc does not contain btrfs SB at all.. --- # btrfs dev scan /dev/sdc Scanning for Btrfs filesystems in '/dev/sdc' ERROR: unable to scan the device '/dev/sdc' - Invalid argument --- here appropriate error is something like no btrfs found on dev However btrfs_scan_one_device (kernel) returns -EINVAL for other errors too Does the below fix sound reasonable ?
It's too bad that ioctl arg doesn't have room to pass back any other status. A rev of the ioctl (BTRFS_IOC_SCAN_DEV_V2), which allows passing status flags back out, would solve the problem nicely I think.
-------------- diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 6b9cff4..d6deae0 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -808,7 +808,7 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder, struct block_device *bdev; struct page *page; void *p; - int ret = -EINVAL; + int ret = 0; u64 devid; u64 transid; u64 total_devices; @@ -833,24 +833,32 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder, } /* make sure our super fits in the device */ - if (bytenr + PAGE_CACHE_SIZE >= i_size_read(bdev->bd_inode)) + if (bytenr + PAGE_CACHE_SIZE >= i_size_read(bdev->bd_inode)) { + ret = -ENOSPC; goto error_bdev_put; + } /* make sure our super fits in the page */ - if (sizeof(*disk_super) > PAGE_CACHE_SIZE) + if (sizeof(*disk_super) > PAGE_CACHE_SIZE) { + ret = -ENOMEM; goto error_bdev_put; + } /* make sure our super doesn't straddle pages on disk */ index = bytenr >> PAGE_CACHE_SHIFT; - if ((bytenr + sizeof(*disk_super) - 1) >> PAGE_CACHE_SHIFT != index) + if ((bytenr + sizeof(*disk_super) - 1) >> PAGE_CACHE_SHIFT != index) { + ret = -ENOMEM; goto error_bdev_put; + } /* pull in the page with our super */ page = read_cache_page_gfp(bdev->bd_inode->i_mapping, index, GFP_NOFS); - if (IS_ERR_OR_NULL(page)) + if (IS_ERR_OR_NULL(page)) { + ret = -EIO; goto error_bdev_put; + } p = kmap(page); @@ -858,8 +866,10 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder, disk_super = p + (bytenr & ~PAGE_CACHE_MASK); if (btrfs_super_bytenr(disk_super) != bytenr || - disk_super->magic != cpu_to_le64(BTRFS_MAGIC)) + disk_super->magic != cpu_to_le64(BTRFS_MAGIC)) { + ret = -EINVAL; goto error_unmap; + } devid = btrfs_stack_device_id(&disk_super->dev_item); transid = btrfs_super_generation(disk_super); -------------------
-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
