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@xxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
