| Message ID | 1491413638-9590-1-git-send-email-zlang@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 4/5/17 12:33 PM, Zorro Lang wrote: > Due to xfs_repair uses direct IO, sometimes it can't read superblock > from an image file has smaller sector size than host filesystem. > Especially that superblock doesn't align with host filesystem's > sector size. > > Fortunately, xfsprogs already has code to do "isa_file" check in > xfs_repair.c, it turns off O_DIRECT after phase1 if the sector > size is less than the host filesystem's geometry. So move it upward > phase1 directly. Hm, I think the problem with this is that the code you moved reads the primary superblock into the psb structure: rval = get_sb(&psb, 0, XFS_MAX_SECTORSIZE, 0); but that's now tested before it's read: if (psb.sb_sectsize < geom.sectsize) { In other words, we rely on the primary superblock geometry to make a determination about turning off O_DIRECT; with your patch it sure looks like it's testing an uninitialized variable, no? -Eric > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- > > Hi, > > V2 did below changes: > 1) remove all changes in V1. > 2) move "isa_file" double check over than phase1. > > Thanks, > Zorro > > repair/xfs_repair.c | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c > index b07567b..0525c6d 100644 > --- a/repair/xfs_repair.c > +++ b/repair/xfs_repair.c > @@ -657,24 +657,6 @@ main(int argc, char **argv) > timestamp(PHASE_START, 0, NULL); > timestamp(PHASE_END, 0, NULL); > > - /* do phase1 to make sure we have a superblock */ > - phase1(temp_mp); > - timestamp(PHASE_END, 1, NULL); > - > - if (no_modify && primary_sb_modified) { > - do_warn(_("Primary superblock would have been modified.\n" > - "Cannot proceed further in no_modify mode.\n" > - "Exiting now.\n")); > - exit(1); > - } > - > - rval = get_sb(&psb, 0, XFS_MAX_SECTORSIZE, 0); > - if (rval != XR_OK) { > - do_warn(_("Primary superblock bad after phase 1!\n" > - "Exiting now.\n")); > - exit(1); > - } > - > /* -f forces this, but let's be nice and autodetect it, as well. */ > if (!isa_file) { > int fd = libxfs_device_to_fd(x.ddev); > @@ -717,6 +699,24 @@ main(int argc, char **argv) > } > } > > + /* do phase1 to make sure we have a superblock */ > + phase1(temp_mp); > + timestamp(PHASE_END, 1, NULL); > + > + if (no_modify && primary_sb_modified) { > + do_warn(_("Primary superblock would have been modified.\n" > + "Cannot proceed further in no_modify mode.\n" > + "Exiting now.\n")); > + exit(1); > + } > + > + rval = get_sb(&psb, 0, XFS_MAX_SECTORSIZE, 0); > + if (rval != XR_OK) { > + do_warn(_("Primary superblock bad after phase 1!\n" > + "Exiting now.\n")); > + exit(1); > + } > + > /* > * Prepare the mount structure. Point the log reference to our local > * copy so it's available to the various phases. The log bits are > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c index b07567b..0525c6d 100644 --- a/repair/xfs_repair.c +++ b/repair/xfs_repair.c @@ -657,24 +657,6 @@ main(int argc, char **argv) timestamp(PHASE_START, 0, NULL); timestamp(PHASE_END, 0, NULL); - /* do phase1 to make sure we have a superblock */ - phase1(temp_mp); - timestamp(PHASE_END, 1, NULL); - - if (no_modify && primary_sb_modified) { - do_warn(_("Primary superblock would have been modified.\n" - "Cannot proceed further in no_modify mode.\n" - "Exiting now.\n")); - exit(1); - } - - rval = get_sb(&psb, 0, XFS_MAX_SECTORSIZE, 0); - if (rval != XR_OK) { - do_warn(_("Primary superblock bad after phase 1!\n" - "Exiting now.\n")); - exit(1); - } - /* -f forces this, but let's be nice and autodetect it, as well. */ if (!isa_file) { int fd = libxfs_device_to_fd(x.ddev); @@ -717,6 +699,24 @@ main(int argc, char **argv) } } + /* do phase1 to make sure we have a superblock */ + phase1(temp_mp); + timestamp(PHASE_END, 1, NULL); + + if (no_modify && primary_sb_modified) { + do_warn(_("Primary superblock would have been modified.\n" + "Cannot proceed further in no_modify mode.\n" + "Exiting now.\n")); + exit(1); + } + + rval = get_sb(&psb, 0, XFS_MAX_SECTORSIZE, 0); + if (rval != XR_OK) { + do_warn(_("Primary superblock bad after phase 1!\n" + "Exiting now.\n")); + exit(1); + } + /* * Prepare the mount structure. Point the log reference to our local * copy so it's available to the various phases. The log bits are
Due to xfs_repair uses direct IO, sometimes it can't read superblock from an image file has smaller sector size than host filesystem. Especially that superblock doesn't align with host filesystem's sector size. Fortunately, xfsprogs already has code to do "isa_file" check in xfs_repair.c, it turns off O_DIRECT after phase1 if the sector size is less than the host filesystem's geometry. So move it upward phase1 directly. Signed-off-by: Zorro Lang <zlang@redhat.com> --- Hi, V2 did below changes: 1) remove all changes in V1. 2) move "isa_file" double check over than phase1. Thanks, Zorro repair/xfs_repair.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-)