| Message ID | 53BA499B.5090206@huawei.com |
|---|---|
| State | RFC |
| Headers | show |
On Mon, 2014-07-07 at 15:17 +0800, hujianyang wrote: > Add a new ioctl to dump EC header and VID header to user space. This > ioctl is called without lock so I think users *should not* run ubidump > when volume is mounted. > > Struct ubi_ebdump_req is used to transfer data between user space and > kernel space. Can I find a way to set buffer length from '64' to > UBI_VID_HDR_SIZE and UBI_EC_HDR_SIZE? > > > Signed-off-by: hujianyang <hujianyang@huawei.com> I've never seen an ioctl for retrieving an on-the-media data structure, do you have any examples? I think the traditional way is that the user-space reads all the information from the disk itself. Do you really need to dump UBI headers? Could you please drop this functionality for now, and provide an ubidump which only dumps UBIFS data structures for now. Then we can talk and think about the UBI-level headers dumping.
Hi Artem, Thank you for reviewing these patches. > > Do you really need to dump UBI headers? > Yes, because I think ec_counter in ec_hdr and copy_flag, sqnum in vid_hdr are useful. Current ->read() can just read data from data_offset where the leb located. And reading UBI headers by mtd functionality are very complicated, we don't know pnum either. Maybe we can try other way instead of this ioctl, by adding an new function which can read the hole peb specified by lnum to user space.
On Wed, 2014-07-16 at 16:47 +0800, hujianyang wrote: > Hi Artem, > > Thank you for reviewing these patches. > > > > > Do you really need to dump UBI headers? > > > > Yes, because I think ec_counter in ec_hdr and copy_flag, sqnum > in vid_hdr are useful. > > Current ->read() can just read data from data_offset where the > leb located. And reading UBI headers by mtd functionality are > very complicated, we don't know pnum either. > > Maybe we can try other way instead of this ioctl, by adding an > new function which can read the hole peb specified by lnum to > user space. Yes, this is a lot better approach. But I'd still suggest to start with a smaller step - ubidump without the UBI headers dumping support. Then add the UBI headers dumping support as a separate step.
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c index 7646220..5b94323 100644 --- a/drivers/mtd/ubi/cdev.c +++ b/drivers/mtd/ubi/cdev.c @@ -581,6 +581,56 @@ static long vol_cdev_ioctl(struct file *file, unsigned int cmd, break; } + case UBI_IOCEBDUMP: + { + int pnum; + struct ubi_ebdump_req req; + struct ubi_ec_hdr *ec_hdr; + struct ubi_vid_hdr *vid_hdr; + + err = copy_from_user(&req, argp, + sizeof(struct ubi_ebdump_req)); + if (err) { + err = -EFAULT; + break; + } + + err = ubi_is_mapped(desc, req.lnum); + if (err <= 0) + break; + pnum = vol->eba_tbl[req.lnum]; + req.pnum = pnum; + + /* + * No need to check return value of ubi_io_read(), + * we will confirm the date in user space. + */ + ec_hdr = kzalloc(ubi->ec_hdr_alsize, GFP_NOFS); + if (!ec_hdr) { + err = -ENOMEM; + break; + } + ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0); + memcpy(req.ec_hdr, ec_hdr, UBI_EC_HDR_SIZE); + kfree(ec_hdr); + + vid_hdr = ubi_zalloc_vid_hdr(ubi, GFP_NOFS); + if (!vid_hdr) { + err = -ENOMEM; + break; + } + ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 1); + memcpy(req.vid_hdr, vid_hdr, UBI_VID_HDR_SIZE); + ubi_free_vid_hdr(ubi, vid_hdr); + + err = copy_to_user(argp, &req, + sizeof(struct ubi_ebdump_req)); + if (err) + err = -EFAULT; + + break; + } + default: err = -ENOTTY; break; diff --git a/include/uapi/mtd/ubi-user.h b/include/uapi/mtd/ubi-user.h index 1927b0d..76ff035 100644 --- a/include/uapi/mtd/ubi-user.h +++ b/include/uapi/mtd/ubi-user.h @@ -205,6 +205,8 @@ #define UBI_IOCVOLCRBLK _IOW(UBI_VOL_IOC_MAGIC, 7, struct ubi_blkcreate_req) /* Remove the R/O block device */ #define UBI_IOCVOLRMBLK _IO(UBI_VOL_IOC_MAGIC, 8) +/* Dump LEB header */ +#define UBI_IOCEBDUMP _IOW(UBI_VOL_IOC_MAGIC, 9, struct ubi_ebdump_req) /* Maximum MTD device name length supported by UBI */ #define MAX_UBI_MTD_NAME_LEN 127 @@ -442,4 +444,17 @@ struct ubi_blkcreate_req { __s8 padding[128]; } __packed; +/** + * struct ubi_ebdump_req - a data structure used in dump eraseblock header. + * @lnum: logical eraseblock num to dump + * @ec_hdr: ec_hdr to set + * @vid_hdr: vid_hdr to set + */ +struct ubi_ebdump_req { + __s32 lnum; + __s32 pnum; + char ec_hdr[64]; + char vid_hdr[64]; +} __packed; + #endif /* __UBI_USER_H__ */
Add a new ioctl to dump EC header and VID header to user space. This ioctl is called without lock so I think users *should not* run ubidump when volume is mounted. Struct ubi_ebdump_req is used to transfer data between user space and kernel space. Can I find a way to set buffer length from '64' to UBI_VID_HDR_SIZE and UBI_EC_HDR_SIZE? Signed-off-by: hujianyang <hujianyang@huawei.com> --- drivers/mtd/ubi/cdev.c | 50 +++++++++++++++++++++++++++++++++++++++++++++ include/uapi/mtd/ubi-user.h | 15 ++++++++++++++ 2 files changed, 65 insertions(+)