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@xxxxxxxxxx>
> Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx>
> ---
> 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
>
