Re: xfs_repair deleting realtime files.

[Anand Tiwari <tiwarikanand@xxxxxxxxx>]

On Wed, Sep 26, 2012 at 12:17 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:

On Tue, Sep 25, 2012 at 09:45:07PM -0600, Anand Tiwari wrote:
> thanks Dave for prompt reply, I meant to implement option 2 as you
> explained (option 3).  I will start working on it tomorrow. In the mean
> time, I also had  to put something in xfs_repair for the files which
> already exists on the disk. Would you guys willing to review/comment on
> that ?

Sure.

Cheers,

Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx

following are my changes for xfs_repair. my goal  is to keep changes minimum as they may not included in upstream. I had to do these changes as we already have files with extent map not properly aligned.

As we know, this happens only when we are growing a file in a realtime volume. by keeping this in mind, I am checking if start of a record in extent map is not aligned, check previous record and if they are contiguous, we can skip that part of record. 

let me know if you any issues with this or if someone has better approach. I would like to use pointers for prev and irec but again, I wanted to keep changes minimum

thanks

anand

XFS currently can have records in extent map, which starts from unaligned block w.r.t rextsize.

--------------------------------------

xfs_repair considers this as a bug (multiple claims for a one real time extent) and deletes the file.

This patch addresses the issue, by comparing current and previous records and make sure they are

contiguous and not overlapped.

diff --git a/repair/dinode.c b/repair/dinode.c

index 5a2da39..5537f1c 100644

--- a/repair/dinode.c

+++ b/repair/dinode.c

@@ -406,6 +406,7 @@ verify_agbno(xfs_mount_t *mp,

 static int

 process_rt_rec(

  xfs_mount_t *mp,

+ xfs_bmbt_irec_t *prev,

  xfs_bmbt_irec_t *irec,

  xfs_ino_t ino,

  xfs_drfsbno_t *tot,

@@ -413,8 +414,11 @@ process_rt_rec(

 {

  xfs_dfsbno_t b;

  xfs_drtbno_t ext;

+ xfs_drtbno_t start_block;

+ xfs_filblks_t block_count;

  int state;

  int pwe; /* partially-written extent */

+ int rtext_remainder; /* start block is not aligned w.r.t rextsize */

 

  /*

  * check numeric validity of the extent

@@ -461,12 +465,32 @@ _("malformed rt inode extent [%" PRIu64 " %" PRIu64 "] (fs rtext size = %u)\n"),

  return 1;

  }

 

+ /* If we have start of record unaligned w.r.t to rextsize, see

+ * if we are sharing this realtime extent with previous record. sharing is only

+ * allowed with previous extent. fail otherwise.

+ * Also, if above condition is true, align start block and block count

+ */

+ rtext_remainder = irec->br_startblock % mp->m_sb.sb_rextsize;

+ if (rtext_remainder) {

+ do_warn(

+_("data fork in rt ino %" PRIu64 " has unalinged start block %"PRIu64 "\n"),

+ ino,

+ irec->br_startblock);

+ if ((prev->br_startoff + prev->br_blockcount == irec->br_startoff) &&

+    (prev->br_startblock + prev->br_blockcount == irec->br_startblock)) {

+ start_block = irec->br_startblock + (mp->m_sb.sb_rextsize - rtext_remainder);

+ block_count = irec->br_blockcount - (mp->m_sb.sb_rextsize - rtext_remainder);

+ }

+ } else {

+ start_block = irec->br_startblock;

+ block_count = irec->br_blockcount;

+ }

+

  /*

  * set the appropriate number of extents

  * this iterates block by block, this can be optimised using extents

  */

- for (b = irec->br_startblock; b < irec->br_startblock +

- irec->br_blockcount; b += mp->m_sb.sb_rextsize)  {

+ for (b = start_block; b < start_block + block_count; b += mp->m_sb.sb_rextsize)  {

  ext = (xfs_drtbno_t) b / mp->m_sb.sb_rextsize;

  pwe = xfs_sb_version_hasextflgbit(&mp->m_sb) &&

  irec->br_state == XFS_EXT_UNWRITTEN &&

@@ -548,6 +572,7 @@ process_bmbt_reclist_int(

  int check_dups,

  int whichfork)

 {

+ xfs_bmbt_irec_t prev;

  xfs_bmbt_irec_t irec;

  xfs_dfilblks_t cp = 0; /* prev count */

  xfs_dfsbno_t sp = 0; /* prev start */

@@ -574,6 +599,11 @@ process_bmbt_reclist_int(

  else

  ftype = _("regular");

 

+ prev.br_startoff = 0;

+ prev.br_blockcount = 0;

+ prev.br_startblock = 0;

+ prev.br_state = 0;

+

  for (i = 0; i < *numrecs; i++) {

  libxfs_bmbt_disk_get_all(rp + i, &irec);

  if (i == 0)

@@ -610,12 +640,13 @@ _("zero length extent (off = %" PRIu64 ", fsbno = %" PRIu64 ") in ino %" PRIu64

  * realtime bitmaps don't use AG locks, so returning

  * immediately is fine for this code path.

  */

- if (process_rt_rec(mp, &irec, ino, tot, check_dups))

+ if (process_rt_rec(mp, &prev, &irec, ino, tot, check_dups))

  return 1;

  /*

  * skip rest of loop processing since that'irec.br_startblock

  * all for regular file forks and attr forks

  */

+ memcpy(&prev, &irec, sizeof(xfs_bmbt_irec_t));

  continue;

  }