From d8c56f70efa21a3b3ee7e17377a6cf0f7e040134 Mon Sep 17 00:00:00 2001 From: Tomohiro Kusumi Date: Wed, 15 Apr 2015 23:21:38 +0900 Subject: [PATCH] sys/vfs/hammer: Try to fix hammer_ioc_pfs_iterate() [1/2] - The ioctl HAMMERIOC_PFS_ITERATE added in commit 29d31c2d has several design issues. This commit and the next one try to fix them. Since it's been exposed to userspace, some of them are left as it is with comments. - The name hammer_ioc_pfs_iterate::pos (pi->pos) is misleading. It should have been 'pfs_id'. Users will have no idea what the 'pos' is supposed to be. - pi->pos is unsigned. - Don't shift pi->pos (Don't ip localize pi->pos). Copy it to a local variable and then shift that. - cursor.key_end fields are necessary only when iterating btree. This function has 'iterate' in its name but all it does is a single btree lookup. - Don't |= HAMMER_PFSD_DELETED with pi->head.flags. pi->head.flags is for HAMMER_IOC_XXX macros, but not for ondisk pfs status (it was lucky PFSD_DELETED does not conflict with those values). Users get pfs status from cursor.data->pfsd.mirror_flags copied to userspace. libhammer code that is based on this wrong flag checking is fixed accordingly. - Check if pi->ondisk is allocated. - Copy ondisk pfs data regardless of ondisk pfs delete flag bit. The delete flag doesn't mean the data is gone. --- lib/libhammer/info.c | 3 +-- sys/vfs/hammer/hammer_ioctl.c | 38 ++++++++++++++++++++--------------- sys/vfs/hammer/hammer_ioctl.h | 2 +- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/lib/libhammer/info.c b/lib/libhammer/info.c index 769219283b77..77600d852633 100644 --- a/lib/libhammer/info.c +++ b/lib/libhammer/info.c @@ -82,8 +82,7 @@ libhammer_get_fsinfo(const char *path) while(error == 0) { error = ioctl(fd, HAMMERIOC_PFS_ITERATE, &pi); if (error == 0 && - ((pi.head.flags & HAMMER_PFSD_DELETED) == 0)) { - + (pi.ondisk->mirror_flags & HAMMER_PFSD_DELETED) == 0) { pip = _libhammer_malloc(sizeof(*pip)); pfs_od = pi.ondisk; pip->ismaster = diff --git a/sys/vfs/hammer/hammer_ioctl.c b/sys/vfs/hammer/hammer_ioctl.c index 8fc990442017..a8301260fb11 100644 --- a/sys/vfs/hammer/hammer_ioctl.c +++ b/sys/vfs/hammer/hammer_ioctl.c @@ -1031,6 +1031,7 @@ hammer_ioc_pfs_iterate(hammer_transaction_t trans, { struct hammer_cursor cursor; hammer_inode_t ip; + uint32_t localization; int error; ip = hammer_get_inode(trans, NULL, HAMMER_OBJID_ROOT, HAMMER_MAX_TID, @@ -1041,8 +1042,6 @@ hammer_ioc_pfs_iterate(hammer_transaction_t trans, if (error) goto out; - pi->head.flags &= ~HAMMER_PFSD_DELETED; - cursor.key_beg.localization = HAMMER_DEF_LOCALIZATION + HAMMER_LOCALIZE_MISC; cursor.key_beg.obj_id = HAMMER_OBJID_ROOT; @@ -1050,27 +1049,34 @@ hammer_ioc_pfs_iterate(hammer_transaction_t trans, cursor.key_beg.delete_tid = 0; cursor.key_beg.rec_type = HAMMER_RECTYPE_PFS; cursor.key_beg.obj_type = 0; - cursor.key_end = cursor.key_beg; - cursor.key_end.key = HAMMER_MAX_KEY; cursor.asof = HAMMER_MAX_TID; cursor.flags |= HAMMER_CURSOR_ASOF; - if (pi->pos < 0) /* Sanity check */ - pi->pos = 0; - - pi->pos <<= 16; - cursor.key_beg.key = pi->pos; + /* + * This pi->pos is about PFS id and ip localization. + * The name is misleading, but it's been exposed to userspace header.. + */ + localization = pi->pos; + if (localization >= HAMMER_MAX_PFS) { + error = EINVAL; + goto out; + } + localization <<= 16; + cursor.key_beg.key = localization; error = hammer_ip_lookup(&cursor); if (error == 0) { error = hammer_ip_resolve_data(&cursor); - if (error) - goto out; - if (cursor.data->pfsd.mirror_flags & HAMMER_PFSD_DELETED) - pi->head.flags |= HAMMER_PFSD_DELETED; - else - copyout(cursor.data, pi->ondisk, cursor.leaf->data_len); - pi->pos = (u_int32_t)(cursor.leaf->base.key >> 16); + if (error == 0) { + if (pi->ondisk) + copyout(cursor.data, pi->ondisk, cursor.leaf->data_len); + localization = cursor.leaf->base.key; + pi->pos = localization >> 16; + /* + * Caller needs to increment pi->pos each time calling + * this ioctl. This ioctl only restores current PFS id. + */ + } } out: diff --git a/sys/vfs/hammer/hammer_ioctl.h b/sys/vfs/hammer/hammer_ioctl.h index 6491f793c1e4..58c1808f7b37 100644 --- a/sys/vfs/hammer/hammer_ioctl.h +++ b/sys/vfs/hammer/hammer_ioctl.h @@ -246,7 +246,7 @@ struct hammer_ioc_info { */ struct hammer_ioc_pfs_iterate { struct hammer_ioc_head head; - uint32_t pos; + uint32_t pos; /* set PFS id here */ struct hammer_pseudofs_data *ondisk; };