| Message ID | 20201102100120.660443-1-hsiangkao@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] xfsdump: fix handling bind mount targets | expand |
On Mon, Nov 02, 2020 at 06:01:20PM +0800, Gao Xiang wrote: > Sometimes, it's not true that the root directory is always > the first result from calling bulkstat with lastino == 0 > assumed by xfsdump. > > Recently XFS_BULK_IREQ_SPECIAL_ROOT was introduced last year, > yet that doesn't exist in old kernels. > > Alternatively, we can also use bulkstat to walk through > all dirs and find the exact dir whose ino # of ".." is > itself by getdents, and that should be considered as the > root dir. > > Fixes: 25195ebf107d ("xfsdump: handle bind mount targets") > Cc: Eric Sandeen <sandeen@redhat.com> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > --- > preliminary test with the original testcase is done... I think that way is feasible, yet if my own understanding on xfsdump codebase is correct, xfsdump itself won't record ".." dirent for all its dir records, so I'm not sure how to find the root inode # for already broken images easily (maybe we might use union-find algorithm to scan all dirs, and then deduce the real root dir via the result of the algorithm, still looking into that if it's possible and easy to implement ...) Thanks, Gao Xiang
On Mon, Nov 02, 2020 at 06:01:20PM +0800, Gao Xiang wrote: > Sometimes, it's not true that the root directory is always > the first result from calling bulkstat with lastino == 0 > assumed by xfsdump. > > Recently XFS_BULK_IREQ_SPECIAL_ROOT was introduced last year, > yet that doesn't exist in old kernels. Is this patch supposed to make xfsrestore start using that? > Alternatively, we can also use bulkstat to walk through > all dirs and find the exact dir whose ino # of ".." is > itself by getdents, and that should be considered as the > root dir. > > Fixes: 25195ebf107d ("xfsdump: handle bind mount targets") > Cc: Eric Sandeen <sandeen@redhat.com> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > --- > preliminary test with the original testcase is done... > > dump/content.c | 168 ++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 140 insertions(+), 28 deletions(-) > > diff --git a/dump/content.c b/dump/content.c > index 30232d4..653d4eb 100644 > --- a/dump/content.c > +++ b/dump/content.c > @@ -511,6 +511,132 @@ static bool_t create_inv_session( > ix_t subtreecnt, > size_t strmix); > > +struct fixrootino_context { > + xfs_ino_t rootino; > + struct dirent *gdp; > + size_t gdsz; > +}; > + > +static bool_t > +scan_rootinode(int fd, > + xfs_ino_t ino, > + struct dirent *gdp, > + size_t gdsz) > +{ > + while (1) { > + struct dirent *p; > + int nread; > + > + nread = getdents_wrap(fd, (char *)gdp, gdsz); > + /* > + * negative count indicates something very bad happened; > + * try to gracefully end this dir. > + */ > + if (nread < 0) { > + mlog(MLOG_NORMAL | MLOG_WARNING, > +_("unable to read dirents for directory ino %llu: %s\n"), > + ino, strerror(errno)); > + /* !!! curtis looked at this, and pointed out that > + * we could take some recovery action here. if the > + * errno is appropriate, lseek64 to the value of > + * doff field of the last dirent successfully > + * obtained, and contiue the loop. > + */ > + nread = 0; /* pretend we are done */ > + } > + > + /* no more directory entries: break; */ > + if (!nread) > + break; > + > + for (p = gdp; nread > 0; > + nread -= (int)p->d_reclen, > + assert(nread >= 0), > + p = (struct dirent *)((char *)p + p->d_reclen)) { > + if (!strcmp(p->d_name, "..") && p->d_ino == ino) { Doesn't this fail on bind mounted directories? The VFS remaps '..' on a bind mount to the directory on the parent mount, which means... # ls -di / 128 drwxr-xr-x 20 root root 317 May 30 10:05 // # ls -ila /home/ total 16 128 drwxr-xr-x 9 root root 90 Apr 28 2018 ./ 128 drwxr-xr-x 20 root root 317 May 30 10:05 ../ that reading '..' won't get you to the root of /home, it'll get you to the root of the root filesystem. --D > + mlog(MLOG_NITTY, "FOUND: name %s d_ino %llu\n", > + p->d_name, ino); > + return BOOL_TRUE; > + } > + } > + } > + return BOOL_FALSE; > +} > + > +/* ARGSUSED */ > +static int > +cb_find_root_inode(void *arg1, > + jdm_fshandle_t *fshandlep, > + int fsfd, > + struct xfs_bstat *statp) > +{ > + int fd; > + struct fixrootino_context *ctx = arg1; > + > + /* open the directory named by statp*/ > + fd = jdm_open(fshandlep, statp, O_RDONLY); > + if (fd < 0) { > + mlog(MLOG_NORMAL | MLOG_WARNING, _( > + "unable to open directory: ino %llu: %s\n"), > + statp->bs_ino, strerror(errno)); > + return RV_OK; /* continue anyway */ > + } > + > + if (scan_rootinode(fd, statp->bs_ino, ctx->gdp, ctx->gdsz)) { > + ctx->rootino = statp->bs_ino; > + return RV_NOMORE; > + } > + return RV_OK; > +} > + > +static bool_t > +fix_root_inode( > + stat64_t *rootstat, > + struct xfs_bstat *sc_rootxfsstatp) > +{ > + struct xfs_bstat *bstatbufp; > + int rval; > + struct fixrootino_context ctx; > + rv_t rv = RV_OK; > + > + ctx.gdsz = sizeof(struct dirent) + NAME_MAX + 1; > + if (ctx.gdsz < GETDENTSBUF_SZ_MIN) > + ctx.gdsz = GETDENTSBUF_SZ_MIN; > + > + ctx.gdp = (struct dirent *)calloc(1, ctx.gdsz); > + assert(ctx.gdp); > + > + /* already a root dir */ > + if (scan_rootinode(sc_fsfd, rootstat->st_ino, ctx.gdp, ctx.gdsz)) > + goto out; > + > + /* allocate a buffer for use by bigstat_iter */ > + bstatbufp = (struct xfs_bstat *)calloc(BSTATBUFLEN, > + sizeof(struct xfs_bstat)); > + assert(bstatbufp); > + > + rval = bigstat_iter(sc_fshandlep, sc_fsfd, BIGSTAT_ITER_DIR, > + (xfs_ino_t)0, cb_find_root_inode, > + &ctx, NULL, NULL, (int *)&rv, preemptchk, > + bstatbufp, BSTATBUFLEN); > + if (rval) > + return BOOL_FALSE; > + > + if (rv != RV_NOMORE && rv != RV_OK) > + return BOOL_FALSE; > + > + mlog(MLOG_NORMAL | MLOG_NOTE, > + "fix up rootino %lld, bind mount?\n", ctx.rootino); > + rootstat->st_ino = ctx.rootino; > +out: > + if (bigstat_one(sc_fsfd, rootstat->st_ino, sc_rootxfsstatp) < 0) { > + mlog( MLOG_ERROR, > + _("failed to get bulkstat information for root inode\n")); > + return BOOL_FALSE; > + } > + return BOOL_TRUE; > +} > + > bool_t > content_init(int argc, > char *argv[], > @@ -1381,6 +1507,18 @@ baseuuidbypass: > return BOOL_FALSE; > } > > + /* alloc a file system handle, to be used with the jdm_open() > + * functions. > + */ > + sc_fshandlep = jdm_getfshandle(mntpnt); > + if (!sc_fshandlep) { > + mlog(MLOG_NORMAL, _( > + "unable to construct a file system handle for %s: %s\n"), > + mntpnt, > + strerror(errno)); > + return BOOL_FALSE; > + } > + > /* figure out the ino for the root directory of the fs > * and get its struct xfs_bstat for inomap_build(). This could > * be a bind mount; don't ask for the mount point inode, > @@ -1388,9 +1526,6 @@ baseuuidbypass: > */ > { > stat64_t rootstat; > - xfs_ino_t lastino = 0; > - int ocount = 0; > - struct xfs_fsop_bulkreq bulkreq; > > /* Get the inode of the mount point */ > rval = fstat64(sc_fsfd, &rootstat); > @@ -1404,33 +1539,10 @@ baseuuidbypass: > (struct xfs_bstat *)calloc(1, sizeof(struct xfs_bstat)); > assert(sc_rootxfsstatp); > > - /* Get the first valid (i.e. root) inode in this fs */ > - bulkreq.lastip = (__u64 *)&lastino; > - bulkreq.icount = 1; > - bulkreq.ubuffer = sc_rootxfsstatp; > - bulkreq.ocount = &ocount; > - if (ioctl(sc_fsfd, XFS_IOC_FSBULKSTAT, &bulkreq) < 0) { > - mlog(MLOG_ERROR, > - _("failed to get bulkstat information for root inode\n")); > + if (!fix_root_inode(&rootstat, sc_rootxfsstatp)) { > + mlog(MLOG_ERROR, _("failed to fix root inode\n")); > return BOOL_FALSE; > } > - > - if (sc_rootxfsstatp->bs_ino != rootstat.st_ino) > - mlog (MLOG_NORMAL | MLOG_NOTE, > - _("root ino %lld differs from mount dir ino %lld, bind mount?\n"), > - sc_rootxfsstatp->bs_ino, rootstat.st_ino); > - } > - > - /* alloc a file system handle, to be used with the jdm_open() > - * functions. > - */ > - sc_fshandlep = jdm_getfshandle(mntpnt); > - if (!sc_fshandlep) { > - mlog(MLOG_NORMAL, _( > - "unable to construct a file system handle for %s: %s\n"), > - mntpnt, > - strerror(errno)); > - return BOOL_FALSE; > } > > if (preemptchk(PREEMPT_FULL)) { > -- > 2.18.1 >
diff --git a/dump/content.c b/dump/content.c index 30232d4..653d4eb 100644 --- a/dump/content.c +++ b/dump/content.c @@ -511,6 +511,132 @@ static bool_t create_inv_session( ix_t subtreecnt, size_t strmix); +struct fixrootino_context { + xfs_ino_t rootino; + struct dirent *gdp; + size_t gdsz; +}; + +static bool_t +scan_rootinode(int fd, + xfs_ino_t ino, + struct dirent *gdp, + size_t gdsz) +{ + while (1) { + struct dirent *p; + int nread; + + nread = getdents_wrap(fd, (char *)gdp, gdsz); + /* + * negative count indicates something very bad happened; + * try to gracefully end this dir. + */ + if (nread < 0) { + mlog(MLOG_NORMAL | MLOG_WARNING, +_("unable to read dirents for directory ino %llu: %s\n"), + ino, strerror(errno)); + /* !!! curtis looked at this, and pointed out that + * we could take some recovery action here. if the + * errno is appropriate, lseek64 to the value of + * doff field of the last dirent successfully + * obtained, and contiue the loop. + */ + nread = 0; /* pretend we are done */ + } + + /* no more directory entries: break; */ + if (!nread) + break; + + for (p = gdp; nread > 0; + nread -= (int)p->d_reclen, + assert(nread >= 0), + p = (struct dirent *)((char *)p + p->d_reclen)) { + if (!strcmp(p->d_name, "..") && p->d_ino == ino) { + mlog(MLOG_NITTY, "FOUND: name %s d_ino %llu\n", + p->d_name, ino); + return BOOL_TRUE; + } + } + } + return BOOL_FALSE; +} + +/* ARGSUSED */ +static int +cb_find_root_inode(void *arg1, + jdm_fshandle_t *fshandlep, + int fsfd, + struct xfs_bstat *statp) +{ + int fd; + struct fixrootino_context *ctx = arg1; + + /* open the directory named by statp*/ + fd = jdm_open(fshandlep, statp, O_RDONLY); + if (fd < 0) { + mlog(MLOG_NORMAL | MLOG_WARNING, _( + "unable to open directory: ino %llu: %s\n"), + statp->bs_ino, strerror(errno)); + return RV_OK; /* continue anyway */ + } + + if (scan_rootinode(fd, statp->bs_ino, ctx->gdp, ctx->gdsz)) { + ctx->rootino = statp->bs_ino; + return RV_NOMORE; + } + return RV_OK; +} + +static bool_t +fix_root_inode( + stat64_t *rootstat, + struct xfs_bstat *sc_rootxfsstatp) +{ + struct xfs_bstat *bstatbufp; + int rval; + struct fixrootino_context ctx; + rv_t rv = RV_OK; + + ctx.gdsz = sizeof(struct dirent) + NAME_MAX + 1; + if (ctx.gdsz < GETDENTSBUF_SZ_MIN) + ctx.gdsz = GETDENTSBUF_SZ_MIN; + + ctx.gdp = (struct dirent *)calloc(1, ctx.gdsz); + assert(ctx.gdp); + + /* already a root dir */ + if (scan_rootinode(sc_fsfd, rootstat->st_ino, ctx.gdp, ctx.gdsz)) + goto out; + + /* allocate a buffer for use by bigstat_iter */ + bstatbufp = (struct xfs_bstat *)calloc(BSTATBUFLEN, + sizeof(struct xfs_bstat)); + assert(bstatbufp); + + rval = bigstat_iter(sc_fshandlep, sc_fsfd, BIGSTAT_ITER_DIR, + (xfs_ino_t)0, cb_find_root_inode, + &ctx, NULL, NULL, (int *)&rv, preemptchk, + bstatbufp, BSTATBUFLEN); + if (rval) + return BOOL_FALSE; + + if (rv != RV_NOMORE && rv != RV_OK) + return BOOL_FALSE; + + mlog(MLOG_NORMAL | MLOG_NOTE, + "fix up rootino %lld, bind mount?\n", ctx.rootino); + rootstat->st_ino = ctx.rootino; +out: + if (bigstat_one(sc_fsfd, rootstat->st_ino, sc_rootxfsstatp) < 0) { + mlog( MLOG_ERROR, + _("failed to get bulkstat information for root inode\n")); + return BOOL_FALSE; + } + return BOOL_TRUE; +} + bool_t content_init(int argc, char *argv[], @@ -1381,6 +1507,18 @@ baseuuidbypass: return BOOL_FALSE; } + /* alloc a file system handle, to be used with the jdm_open() + * functions. + */ + sc_fshandlep = jdm_getfshandle(mntpnt); + if (!sc_fshandlep) { + mlog(MLOG_NORMAL, _( + "unable to construct a file system handle for %s: %s\n"), + mntpnt, + strerror(errno)); + return BOOL_FALSE; + } + /* figure out the ino for the root directory of the fs * and get its struct xfs_bstat for inomap_build(). This could * be a bind mount; don't ask for the mount point inode, @@ -1388,9 +1526,6 @@ baseuuidbypass: */ { stat64_t rootstat; - xfs_ino_t lastino = 0; - int ocount = 0; - struct xfs_fsop_bulkreq bulkreq; /* Get the inode of the mount point */ rval = fstat64(sc_fsfd, &rootstat); @@ -1404,33 +1539,10 @@ baseuuidbypass: (struct xfs_bstat *)calloc(1, sizeof(struct xfs_bstat)); assert(sc_rootxfsstatp); - /* Get the first valid (i.e. root) inode in this fs */ - bulkreq.lastip = (__u64 *)&lastino; - bulkreq.icount = 1; - bulkreq.ubuffer = sc_rootxfsstatp; - bulkreq.ocount = &ocount; - if (ioctl(sc_fsfd, XFS_IOC_FSBULKSTAT, &bulkreq) < 0) { - mlog(MLOG_ERROR, - _("failed to get bulkstat information for root inode\n")); + if (!fix_root_inode(&rootstat, sc_rootxfsstatp)) { + mlog(MLOG_ERROR, _("failed to fix root inode\n")); return BOOL_FALSE; } - - if (sc_rootxfsstatp->bs_ino != rootstat.st_ino) - mlog (MLOG_NORMAL | MLOG_NOTE, - _("root ino %lld differs from mount dir ino %lld, bind mount?\n"), - sc_rootxfsstatp->bs_ino, rootstat.st_ino); - } - - /* alloc a file system handle, to be used with the jdm_open() - * functions. - */ - sc_fshandlep = jdm_getfshandle(mntpnt); - if (!sc_fshandlep) { - mlog(MLOG_NORMAL, _( - "unable to construct a file system handle for %s: %s\n"), - mntpnt, - strerror(errno)); - return BOOL_FALSE; } if (preemptchk(PREEMPT_FULL)) {
Sometimes, it's not true that the root directory is always the first result from calling bulkstat with lastino == 0 assumed by xfsdump. Recently XFS_BULK_IREQ_SPECIAL_ROOT was introduced last year, yet that doesn't exist in old kernels. Alternatively, we can also use bulkstat to walk through all dirs and find the exact dir whose ino # of ".." is itself by getdents, and that should be considered as the root dir. Fixes: 25195ebf107d ("xfsdump: handle bind mount targets") Cc: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Gao Xiang <hsiangkao@redhat.com> --- preliminary test with the original testcase is done... dump/content.c | 168 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 140 insertions(+), 28 deletions(-)