* [PATCH 0/4] xfs: old lost patches
@ 2013-10-11 19:07 Eric Sandeen
2013-10-11 19:09 ` [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings Eric Sandeen
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Eric Sandeen @ 2013-10-11 19:07 UTC (permalink / raw)
To: xfs-oss
4 patches I found in patchworks in various state of review, but no merges.
Resending.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings 2013-10-11 19:07 [PATCH 0/4] xfs: old lost patches Eric Sandeen @ 2013-10-11 19:09 ` Eric Sandeen 2013-10-11 21:59 ` Mark Tinguely 2013-10-12 1:59 ` [PATCH 1/4 V2] xfs: remove newlines from strings passed to __xfs_printk Eric Sandeen 2013-10-11 19:11 ` [PATCH 2/4] xfs: reject completely bogus remount options Eric Sandeen ` (3 subsequent siblings) 4 siblings, 2 replies; 19+ messages in thread From: Eric Sandeen @ 2013-10-11 19:09 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs-oss xfs_alert_tag passes the format string to __xfs_printk, which adds its own "\n". Having it in the original string leads to unintentional blank lines from these messages. Most format strings have no newline, but these 3 do, leading to i.e.: [ 7347.119911] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 [ 7347.119911] [ 7347.119919] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 [ 7347.119919] Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> --- diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c index f47e65c..e6b4202 100644 --- a/fs/xfs/xfs_bmap.c +++ b/fs/xfs/xfs_bmap.c @@ -1482,7 +1482,7 @@ xfs_bmap_search_extents( xfs_alert_tag(ip->i_mount, XFS_PTAG_FSBLOCK_ZERO, "Access to block zero in inode %llu " "start_block: %llx start_off: %llx " - "blkcnt: %llx extent-state: %x lastx: %x\n", + "blkcnt: %llx extent-state: %x lastx: %x", (unsigned long long)ip->i_ino, (unsigned long long)gotp->br_startblock, (unsigned long long)gotp->br_startoff, diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c index 1123d93..40f2985 100644 --- a/fs/xfs/xfs_error.c +++ b/fs/xfs/xfs_error.c @@ -159,7 +159,7 @@ xfs_error_report( { if (level <= xfs_error_level) { xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT, - "Internal error %s at line %d of file %s. Caller 0x%p\n", + "Internal error %s at line %d of file %s. Caller 0x%p", tag, linenum, filename, ra); xfs_stack_trace(); diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 8d4d49b..43e31b0 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -110,7 +110,7 @@ xfs_alert_fsblock_zero( xfs_alert_tag(ip->i_mount, XFS_PTAG_FSBLOCK_ZERO, "Access to block zero in inode %llu " "start_block: %llx start_off: %llx " - "blkcnt: %llx extent-state: %x\n", + "blkcnt: %llx extent-state: %x", (unsigned long long)ip->i_ino, (unsigned long long)imap->br_startblock, (unsigned long long)imap->br_startoff, _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings 2013-10-11 19:09 ` [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings Eric Sandeen @ 2013-10-11 21:59 ` Mark Tinguely 2013-10-12 1:45 ` Eric Sandeen 2013-10-12 1:59 ` [PATCH 1/4 V2] xfs: remove newlines from strings passed to __xfs_printk Eric Sandeen 1 sibling, 1 reply; 19+ messages in thread From: Mark Tinguely @ 2013-10-11 21:59 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On 10/11/13 14:09, Eric Sandeen wrote: > xfs_alert_tag passes the format string to __xfs_printk, > which adds its own "\n". Having it in the original string > leads to unintentional blank lines from these messages. > > Most format strings have no newline, but these 3 do, leading to > i.e.: > > [ 7347.119911] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 > [ 7347.119911] > [ 7347.119919] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 > [ 7347.119919] > > Signed-off-by: Eric Sandeen<sandeen@redhat.com> > Reviewed-by: Carlos Maiolino<cmaiolino@redhat.com> > --- Is this true of xfs_alert() too? ie the newline in xfs_alert in xfs_dir2_leafn_rebalance(). The newline in xfs_alert() in xlog_unpack_data_crc() looks intentional. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings 2013-10-11 21:59 ` Mark Tinguely @ 2013-10-12 1:45 ` Eric Sandeen 0 siblings, 0 replies; 19+ messages in thread From: Eric Sandeen @ 2013-10-12 1:45 UTC (permalink / raw) To: Mark Tinguely; +Cc: Eric Sandeen, xfs-oss On 10/11/13 4:59 PM, Mark Tinguely wrote: > On 10/11/13 14:09, Eric Sandeen wrote: >> xfs_alert_tag passes the format string to __xfs_printk, >> which adds its own "\n". Having it in the original string >> leads to unintentional blank lines from these messages. >> >> Most format strings have no newline, but these 3 do, leading to >> i.e.: >> >> [ 7347.119911] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 >> [ 7347.119911] >> [ 7347.119919] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 >> [ 7347.119919] >> >> Signed-off-by: Eric Sandeen<sandeen@redhat.com> >> Reviewed-by: Carlos Maiolino<cmaiolino@redhat.com> >> --- > > Is this true of xfs_alert() too? ie the newline in xfs_alert in xfs_dir2_leafn_rebalance(). The newline in xfs_alert() in xlog_unpack_data_crc() looks intentional. All of these: define_xfs_printk_level(xfs_emerg, KERN_EMERG); define_xfs_printk_level(xfs_alert, KERN_ALERT); define_xfs_printk_level(xfs_crit, KERN_CRIT); define_xfs_printk_level(xfs_err, KERN_ERR); define_xfs_printk_level(xfs_warn, KERN_WARNING); define_xfs_printk_level(xfs_notice, KERN_NOTICE); define_xfs_printk_level(xfs_info, KERN_INFO); #ifdef DEBUG define_xfs_printk_level(xfs_debug, KERN_DEBUG); #endif go through __xfs_printk(), which adds a newline... seems like more fixes are in order, yeah. -Eric > --Mark. > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/4 V2] xfs: remove newlines from strings passed to __xfs_printk 2013-10-11 19:09 ` [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings Eric Sandeen 2013-10-11 21:59 ` Mark Tinguely @ 2013-10-12 1:59 ` Eric Sandeen 2013-10-12 21:07 ` Mark Tinguely 1 sibling, 1 reply; 19+ messages in thread From: Eric Sandeen @ 2013-10-12 1:59 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs-oss __xfs_printk adds its own "\n". Having it in the original string leads to unintentional blank lines from these messages. Most format strings have no newline, but a few do, leading to i.e.: [ 7347.119911] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 [ 7347.119911] [ 7347.119919] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 [ 7347.119919] Fix them all. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c index f47e65c..e6b4202 100644 --- a/fs/xfs/xfs_bmap.c +++ b/fs/xfs/xfs_bmap.c @@ -1482,7 +1482,7 @@ xfs_bmap_search_extents( xfs_alert_tag(ip->i_mount, XFS_PTAG_FSBLOCK_ZERO, "Access to block zero in inode %llu " "start_block: %llx start_off: %llx " - "blkcnt: %llx extent-state: %x lastx: %x\n", + "blkcnt: %llx extent-state: %x lastx: %x", (unsigned long long)ip->i_ino, (unsigned long long)gotp->br_startblock, (unsigned long long)gotp->br_startoff, diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 2634700..75e4ea7 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -590,7 +590,7 @@ found: error = _xfs_buf_map_pages(bp, flags); if (unlikely(error)) { xfs_warn(target->bt_mount, - "%s: failed to map pages\n", __func__); + "%s: failed to map pagesn", __func__); xfs_buf_relse(bp); return NULL; } @@ -809,7 +809,7 @@ xfs_buf_get_uncached( error = _xfs_buf_map_pages(bp, 0); if (unlikely(error)) { xfs_warn(target->bt_mount, - "%s: failed to map pages\n", __func__); + "%s: failed to map pages", __func__); goto fail_free_mem; } @@ -1618,7 +1618,7 @@ xfs_setsize_buftarg_flags( bdevname(btp->bt_bdev, name); xfs_warn(btp->bt_mount, - "Cannot set_blocksize to %u on device %s\n", + "Cannot set_blocksize to %u on device %s", sectorsize, name); return EINVAL; } diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c index 4c3dba7..34df052 100644 --- a/fs/xfs/xfs_dir2_node.c +++ b/fs/xfs/xfs_dir2_node.c @@ -1101,7 +1101,7 @@ xfs_dir2_leafn_rebalance( state->inleaf = 1; blk2->index = 0; xfs_alert(args->dp->i_mount, - "%s: picked the wrong leaf? reverting original leaf: blk1->index %d\n", + "%s: picked the wrong leaf? reverting original leaf: blk1->index %d", __func__, blk1->index); } } diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c index 1123d93..40f2985 100644 --- a/fs/xfs/xfs_error.c +++ b/fs/xfs/xfs_error.c @@ -159,7 +159,7 @@ xfs_error_report( { if (level <= xfs_error_level) { xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT, - "Internal error %s at line %d of file %s. Caller 0x%p\n", + "Internal error %s at line %d of file %s. Caller 0x%p", tag, linenum, filename, ra); xfs_stack_trace(); diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 8d4d49b..43e31b0 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -110,7 +110,7 @@ xfs_alert_fsblock_zero( xfs_alert_tag(ip->i_mount, XFS_PTAG_FSBLOCK_ZERO, "Access to block zero in inode %llu " "start_block: %llx start_off: %llx " - "blkcnt: %llx extent-state: %x\n", + "blkcnt: %llx extent-state: %x", (unsigned long long)ip->i_ino, (unsigned long long)imap->br_startblock, (unsigned long long)imap->br_startoff, diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index a2dea10..ea02b48 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -1979,7 +1979,7 @@ xlog_print_tic_res( for (i = 0; i < ticket->t_res_num; i++) { uint r_type = ticket->t_res_arr[i].r_type; - xfs_warn(mp, "region[%u]: %s - %u bytes\n", i, + xfs_warn(mp, "region[%u]: %s - %u bytes", i, ((r_type <= 0 || r_type > XLOG_REG_TYPE_MAX) ? "bad-rtype" : res_type_str[r_type-1]), ticket->t_res_arr[i].r_len); diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 3979749..e7ca48e 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -305,9 +305,9 @@ xlog_header_check_dump( xfs_mount_t *mp, xlog_rec_header_t *head) { - xfs_debug(mp, "%s: SB : uuid = %pU, fmt = %d\n", + xfs_debug(mp, "%s: SB : uuid = %pU, fmt = %d", __func__, &mp->m_sb.sb_uuid, XLOG_FMT); - xfs_debug(mp, " log : uuid = %pU, fmt = %d\n", + xfs_debug(mp, " log : uuid = %pU, fmt = %d", &head->h_fs_uuid, be32_to_cpu(head->h_fmt)); } #else @@ -4077,7 +4077,7 @@ xlog_unpack_data_crc( if (crc != rhead->h_crc) { if (rhead->h_crc || xfs_sb_version_hascrc(&log->l_mp->m_sb)) { xfs_alert(log->l_mp, - "log record CRC mismatch: found 0x%x, expected 0x%x.\n", + "log record CRC mismatch: found 0x%x, expected 0x%x.", le32_to_cpu(rhead->h_crc), le32_to_cpu(crc)); xfs_hex_dump(dp, 32); diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c index 8174aad..fb67091 100644 --- a/fs/xfs/xfs_qm_syscalls.c +++ b/fs/xfs/xfs_qm_syscalls.c @@ -287,7 +287,7 @@ xfs_qm_scall_trunc_qfiles( int error = 0, error2 = 0; if (!xfs_sb_version_hasquota(&mp->m_sb) || flags == 0) { - xfs_debug(mp, "%s: flags=%x m_qflags=%x\n", + xfs_debug(mp, "%s: flags=%x m_qflags=%x", __func__, flags, mp->m_qflags); return XFS_ERROR(EINVAL); } @@ -325,7 +325,7 @@ xfs_qm_scall_quotaon( sbflags = 0; if (flags == 0) { - xfs_debug(mp, "%s: zero flags, m_qflags=%x\n", + xfs_debug(mp, "%s: zero flags, m_qflags=%x", __func__, mp->m_qflags); return XFS_ERROR(EINVAL); } @@ -348,7 +348,7 @@ xfs_qm_scall_quotaon( (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT) == 0 && (flags & XFS_PQUOTA_ENFD))) { xfs_debug(mp, - "%s: Can't enforce without acct, flags=%x sbflags=%x\n", + "%s: Can't enforce without acct, flags=%x sbflags=%x", __func__, flags, mp->m_sb.sb_qflags); return XFS_ERROR(EINVAL); } @@ -648,7 +648,7 @@ xfs_qm_scall_setqlim( q->qi_bsoftlimit = soft; } } else { - xfs_debug(mp, "blkhard %Ld < blksoft %Ld\n", hard, soft); + xfs_debug(mp, "blkhard %Ld < blksoft %Ld", hard, soft); } hard = (newlim->d_fieldmask & FS_DQ_RTBHARD) ? (xfs_qcnt_t) XFS_BB_TO_FSB(mp, newlim->d_rtb_hardlimit) : @@ -664,7 +664,7 @@ xfs_qm_scall_setqlim( q->qi_rtbsoftlimit = soft; } } else { - xfs_debug(mp, "rtbhard %Ld < rtbsoft %Ld\n", hard, soft); + xfs_debug(mp, "rtbhard %Ld < rtbsoft %Ld", hard, soft); } hard = (newlim->d_fieldmask & FS_DQ_IHARD) ? @@ -681,7 +681,7 @@ xfs_qm_scall_setqlim( q->qi_isoftlimit = soft; } } else { - xfs_debug(mp, "ihard %Ld < isoft %Ld\n", hard, soft); + xfs_debug(mp, "ihard %Ld < isoft %Ld", hard, soft); } /* diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c index a5b59d9..a89d0bc 100644 --- a/fs/xfs/xfs_sb.c +++ b/fs/xfs/xfs_sb.c @@ -249,13 +249,13 @@ xfs_mount_validate_sb( if (xfs_sb_version_has_pquotino(sbp)) { if (sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) { xfs_notice(mp, - "Version 5 of Super block has XFS_OQUOTA bits.\n"); + "Version 5 of Super block has XFS_OQUOTA bits."); return XFS_ERROR(EFSCORRUPTED); } } else if (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD | XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) { xfs_notice(mp, -"Superblock earlier than Version 5 has XFS_[PQ]UOTA_{ENFD|CHKD} bits.\n"); +"Superblock earlier than Version 5 has XFS_[PQ]UOTA_{ENFD|CHKD} bits."); return XFS_ERROR(EFSCORRUPTED); } diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index 15188cc..77093de 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -1246,7 +1246,7 @@ xfs_fs_remount( */ #if 0 xfs_info(mp, - "mount option \"%s\" not supported for remount\n", p); + "mount option \"%s\" not supported for remount", p); return -EINVAL; #else break; _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4 V2] xfs: remove newlines from strings passed to __xfs_printk 2013-10-12 1:59 ` [PATCH 1/4 V2] xfs: remove newlines from strings passed to __xfs_printk Eric Sandeen @ 2013-10-12 21:07 ` Mark Tinguely 0 siblings, 0 replies; 19+ messages in thread From: Mark Tinguely @ 2013-10-12 21:07 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On 10/11/13 20:59, Eric Sandeen wrote: > __xfs_printk adds its own "\n". Having it in the original string > leads to unintentional blank lines from these messages. > > Most format strings have no newline, but a few do, leading to > i.e.: > > [ 7347.119911] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 > [ 7347.119911] > [ 7347.119919] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05 > [ 7347.119919] > > Fix them all. > > Signed-off-by: Eric Sandeen<sandeen@redhat.com> > --- Thanks for the updated patch. It looks good. Reviewed-by: Mark Tinguely <tinguely@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/4] xfs: reject completely bogus remount options 2013-10-11 19:07 [PATCH 0/4] xfs: old lost patches Eric Sandeen 2013-10-11 19:09 ` [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings Eric Sandeen @ 2013-10-11 19:11 ` Eric Sandeen 2013-10-11 21:34 ` Mark Tinguely 2013-10-13 21:52 ` Dave Chinner 2013-10-11 19:12 ` [PATCH 3/4] xfs: don't emit corruption noise on fs probes Eric Sandeen ` (2 subsequent siblings) 4 siblings, 2 replies; 19+ messages in thread From: Eric Sandeen @ 2013-10-11 19:11 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs-oss There's a long comment about handling non-remountable options in xfs_fs_remount, but nothing addresses the case of completely bogus mount options at remount time, which can lead to some severe strangeness: # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done # grep sdb4 /etc/mtab /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0 This is a bit of a hack, but we can re-use xfs_parseargs() with a dummy mount struct to just vet all of the remount options which were passed in. With this, we get a saner result: [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value if we try to remount with something ridiculous. In the long run we should probably revamp a lot of the mount option handling... Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- Note, not really sure what KM_* flag would be appropriate here, if it fails, it really is ok, other than missing the verification. But maybe that's too "nice?" commit 72e6ddd901dc8a8ecb835324eb4e11b0d7ad8cf8 Author: Eric Sandeen <sandeen@redhat.com> Date: Fri Oct 11 14:03:59 2013 -0500 xfs: reject completely bogus remount options There's a long comment about handling non-remountable options in xfs_fs_remount, but nothing addresses the case of completely bogus mount options at remount time, which can lead to some severe strangeness: # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done # grep sdb4 /etc/mtab /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0 This is a bit of a hack, but we can re-use xfs_parseargs() with a dummy mount struct to just vet all of the remount options which were passed in. With this, we get a saner result: [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value if we try to remount with something ridiculous. In the long run we should probably revamp a lot of the mount option handling... Signed-off-by: Eric Sandeen <sandeen@redhat.com> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index 15188cc..00a06d6 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -1202,11 +1202,25 @@ xfs_fs_remount( int *flags, char *options) { - struct xfs_mount *mp = XFS_M(sb); + struct xfs_mount *mp = XFS_M(sb), *dummy_mp; substring_t args[MAX_OPT_ARGS]; char *p; int error; + /* + * Check all the mount options presented to be sure + * there's nothing too crazy in there. Non-remountable + * but valid options are a different issue. + */ + dummy_mp = kmem_zalloc(sizeof(*dummy_mp), KM_MAYFAIL); + if (dummy_mp) { + dummy_mp->m_super = sb; + error = xfs_parseargs(dummy_mp, options); + kfree(dummy_mp); + if (error) + return -error; + } + while ((p = strsep(&options, ",")) != NULL) { int token; _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xfs: reject completely bogus remount options 2013-10-11 19:11 ` [PATCH 2/4] xfs: reject completely bogus remount options Eric Sandeen @ 2013-10-11 21:34 ` Mark Tinguely 2013-10-12 1:40 ` Eric Sandeen 2013-10-13 21:52 ` Dave Chinner 1 sibling, 1 reply; 19+ messages in thread From: Mark Tinguely @ 2013-10-11 21:34 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On 10/11/13 14:11, Eric Sandeen wrote: > There's a long comment about handling non-remountable > options in xfs_fs_remount, but nothing addresses the case > of completely bogus mount options at remount time, which > can lead to some severe strangeness: > > # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done > # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done > # grep sdb4 /etc/mtab > /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0 > > This is a bit of a hack, but we can re-use xfs_parseargs() > with a dummy mount struct to just vet all of the remount > options which were passed in. With this, we get a saner > result: > > [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value > > if we try to remount with something ridiculous. > > In the long run we should probably revamp a lot of the mount option > handling... > > Signed-off-by: Eric Sandeen<sandeen@redhat.com> > --- I don't seem to get the duplicate mtab entries on a top of tree kernel. Is this still appropriate? --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xfs: reject completely bogus remount options 2013-10-11 21:34 ` Mark Tinguely @ 2013-10-12 1:40 ` Eric Sandeen 2013-10-12 21:11 ` Mark Tinguely 0 siblings, 1 reply; 19+ messages in thread From: Eric Sandeen @ 2013-10-12 1:40 UTC (permalink / raw) To: Mark Tinguely; +Cc: Eric Sandeen, xfs-oss On 10/11/13 4:34 PM, Mark Tinguely wrote: > On 10/11/13 14:11, Eric Sandeen wrote: >> There's a long comment about handling non-remountable >> options in xfs_fs_remount, but nothing addresses the case >> of completely bogus mount options at remount time, which >> can lead to some severe strangeness: >> >> # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done >> # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done >> # grep sdb4 /etc/mtab >> /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0 >> >> This is a bit of a hack, but we can re-use xfs_parseargs() >> with a dummy mount struct to just vet all of the remount >> options which were passed in. With this, we get a saner >> result: >> >> [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value >> >> if we try to remount with something ridiculous. >> >> In the long run we should probably revamp a lot of the mount option >> handling... >> >> Signed-off-by: Eric Sandeen<sandeen@redhat.com> >> --- > > > I don't seem to get the duplicate mtab entries on a top of tree kernel. > Is this still appropriate? Maybe different mount(8) behavior on your system? (probably symlinked to /proc/mounts) On RHEL6: # mount /dev/sdb1 /mnt/test # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test; done # mount | grep sdb1 /dev/sdb1 on /mnt/test type xfs (rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl) # uname -a Linux hostname 3.12.0-rc4+ #41 SMP Fri Oct 11 19:43:01 CDT 2013 x86_64 x86_64 x86_64 GNU/Linux -Eric > --Mark. > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xfs: reject completely bogus remount options 2013-10-12 1:40 ` Eric Sandeen @ 2013-10-12 21:11 ` Mark Tinguely 0 siblings, 0 replies; 19+ messages in thread From: Mark Tinguely @ 2013-10-12 21:11 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On 10/11/13 20:40, Eric Sandeen wrote: > On 10/11/13 4:34 PM, Mark Tinguely wrote: >> On 10/11/13 14:11, Eric Sandeen wrote: >>> There's a long comment about handling non-remountable >>> options in xfs_fs_remount, but nothing addresses the case >>> of completely bogus mount options at remount time, which >>> can lead to some severe strangeness: >>> >>> # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done >>> # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done >>> # grep sdb4 /etc/mtab >>> /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0 >>> >>> This is a bit of a hack, but we can re-use xfs_parseargs() >>> with a dummy mount struct to just vet all of the remount >>> options which were passed in. With this, we get a saner >>> result: >>> >>> [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value >>> >>> if we try to remount with something ridiculous. >>> >>> In the long run we should probably revamp a lot of the mount option >>> handling... >>> >>> Signed-off-by: Eric Sandeen<sandeen@redhat.com> >>> --- >> >> >> I don't seem to get the duplicate mtab entries on a top of tree kernel. >> Is this still appropriate? > > Maybe different mount(8) behavior on your system? (probably symlinked to /proc/mounts) > > On RHEL6: > > # mount /dev/sdb1 /mnt/test > # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test; done > # mount | grep sdb1 > /dev/sdb1 on /mnt/test type xfs (rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl) > # uname -a > Linux hostname 3.12.0-rc4+ #41 SMP Fri Oct 11 19:43:01 CDT 2013 x86_64 x86_64 x86_64 GNU/Linux > > > -Eric Yep, confirmed the described behavior on a RHEL 6 box without patch. The patch looks good. Reviewed-by: Mark Tinguely <tinguely@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xfs: reject completely bogus remount options 2013-10-11 19:11 ` [PATCH 2/4] xfs: reject completely bogus remount options Eric Sandeen 2013-10-11 21:34 ` Mark Tinguely @ 2013-10-13 21:52 ` Dave Chinner 2013-10-14 2:42 ` Eric Sandeen 1 sibling, 1 reply; 19+ messages in thread From: Dave Chinner @ 2013-10-13 21:52 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On Fri, Oct 11, 2013 at 02:11:18PM -0500, Eric Sandeen wrote: > There's a long comment about handling non-remountable > options in xfs_fs_remount, but nothing addresses the case > of completely bogus mount options at remount time, which > can lead to some severe strangeness: > > # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done > # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done > # grep sdb4 /etc/mtab > /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0 > > This is a bit of a hack, but we can re-use xfs_parseargs() > with a dummy mount struct to just vet all of the remount > options which were passed in. With this, we get a saner > result: > > [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value ext4? Really? :) > +++ b/fs/xfs/xfs_super.c > @@ -1202,11 +1202,25 @@ xfs_fs_remount( > int *flags, > char *options) > { > - struct xfs_mount *mp = XFS_M(sb); > + struct xfs_mount *mp = XFS_M(sb), *dummy_mp; > substring_t args[MAX_OPT_ARGS]; > char *p; > int error; > > + /* > + * Check all the mount options presented to be sure > + * there's nothing too crazy in there. Non-remountable > + * but valid options are a different issue. > + */ > + dummy_mp = kmem_zalloc(sizeof(*dummy_mp), KM_MAYFAIL); > + if (dummy_mp) { > + dummy_mp->m_super = sb; > + error = xfs_parseargs(dummy_mp, options); > + kfree(dummy_mp); > + if (error) > + return -error; This, at minimum, leaks dummy_mp->m_fsname, and it will leak other strings that are also kstrdup()d by xfs_parseargs(). Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xfs: reject completely bogus remount options 2013-10-13 21:52 ` Dave Chinner @ 2013-10-14 2:42 ` Eric Sandeen 2013-10-14 4:45 ` Dave Chinner 0 siblings, 1 reply; 19+ messages in thread From: Eric Sandeen @ 2013-10-14 2:42 UTC (permalink / raw) To: Dave Chinner; +Cc: Eric Sandeen, xfs-oss On 10/13/13 4:52 PM, Dave Chinner wrote: > On Fri, Oct 11, 2013 at 02:11:18PM -0500, Eric Sandeen wrote: >> There's a long comment about handling non-remountable >> options in xfs_fs_remount, but nothing addresses the case >> of completely bogus mount options at remount time, which >> can lead to some severe strangeness: >> >> # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done >> # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done >> # grep sdb4 /etc/mtab >> /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0 >> >> This is a bit of a hack, but we can re-use xfs_parseargs() >> with a dummy mount struct to just vet all of the remount >> options which were passed in. With this, we get a saner >> result: >> >> [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value > > ext4? Really? :) uhhh ;) >> +++ b/fs/xfs/xfs_super.c >> @@ -1202,11 +1202,25 @@ xfs_fs_remount( >> int *flags, >> char *options) >> { >> - struct xfs_mount *mp = XFS_M(sb); >> + struct xfs_mount *mp = XFS_M(sb), *dummy_mp; >> substring_t args[MAX_OPT_ARGS]; >> char *p; >> int error; >> >> + /* >> + * Check all the mount options presented to be sure >> + * there's nothing too crazy in there. Non-remountable >> + * but valid options are a different issue. >> + */ >> + dummy_mp = kmem_zalloc(sizeof(*dummy_mp), KM_MAYFAIL); >> + if (dummy_mp) { >> + dummy_mp->m_super = sb; >> + error = xfs_parseargs(dummy_mp, options); >> + kfree(dummy_mp); >> + if (error) >> + return -error; > > This, at minimum, leaks dummy_mp->m_fsname, and it will leak other > strings that are also kstrdup()d by xfs_parseargs(). nnngh. Forgot about that side effect, sorry. Dammit. Think it's still worth doing this if I handle freeing them all up? -Eric > Cheers, > > Dave. > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xfs: reject completely bogus remount options 2013-10-14 2:42 ` Eric Sandeen @ 2013-10-14 4:45 ` Dave Chinner 2013-10-15 18:13 ` Eric Sandeen 0 siblings, 1 reply; 19+ messages in thread From: Dave Chinner @ 2013-10-14 4:45 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On Sun, Oct 13, 2013 at 09:42:37PM -0500, Eric Sandeen wrote: > On 10/13/13 4:52 PM, Dave Chinner wrote: > > On Fri, Oct 11, 2013 at 02:11:18PM -0500, Eric Sandeen wrote: > >> There's a long comment about handling non-remountable > >> options in xfs_fs_remount, but nothing addresses the case > >> of completely bogus mount options at remount time, which > >> can lead to some severe strangeness: > >> > >> # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done > >> # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done > >> # grep sdb4 /etc/mtab > >> /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0 > >> > >> This is a bit of a hack, but we can re-use xfs_parseargs() > >> with a dummy mount struct to just vet all of the remount > >> options which were passed in. With this, we get a saner > >> result: > >> > >> [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value > > > > ext4? Really? :) > > > uhhh ;) > > >> +++ b/fs/xfs/xfs_super.c > >> @@ -1202,11 +1202,25 @@ xfs_fs_remount( > >> int *flags, > >> char *options) > >> { > >> - struct xfs_mount *mp = XFS_M(sb); > >> + struct xfs_mount *mp = XFS_M(sb), *dummy_mp; > >> substring_t args[MAX_OPT_ARGS]; > >> char *p; > >> int error; > >> > >> + /* > >> + * Check all the mount options presented to be sure > >> + * there's nothing too crazy in there. Non-remountable > >> + * but valid options are a different issue. > >> + */ > >> + dummy_mp = kmem_zalloc(sizeof(*dummy_mp), KM_MAYFAIL); > >> + if (dummy_mp) { > >> + dummy_mp->m_super = sb; > >> + error = xfs_parseargs(dummy_mp, options); > >> + kfree(dummy_mp); > >> + if (error) > >> + return -error; > > > > This, at minimum, leaks dummy_mp->m_fsname, and it will leak other > > strings that are also kstrdup()d by xfs_parseargs(). > > nnngh. Forgot about that side effect, sorry. Dammit. > > Think it's still worth doing this if I handle freeing them all up? If you wrap it all in a helper function (xfs_check-args()?) that does all the temporary structure allocation and freeing, I think it will be fine. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xfs: reject completely bogus remount options 2013-10-14 4:45 ` Dave Chinner @ 2013-10-15 18:13 ` Eric Sandeen 0 siblings, 0 replies; 19+ messages in thread From: Eric Sandeen @ 2013-10-15 18:13 UTC (permalink / raw) To: Dave Chinner; +Cc: Eric Sandeen, xfs-oss On 10/13/13 11:45 PM, Dave Chinner wrote: > On Sun, Oct 13, 2013 at 09:42:37PM -0500, Eric Sandeen wrote: >> On 10/13/13 4:52 PM, Dave Chinner wrote: >>> On Fri, Oct 11, 2013 at 02:11:18PM -0500, Eric Sandeen wrote: >>>> There's a long comment about handling non-remountable >>>> options in xfs_fs_remount, but nothing addresses the case >>>> of completely bogus mount options at remount time, which >>>> can lead to some severe strangeness: >>>> >>>> # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done >>>> # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done I'm going to just drop this patch for now; it seems too hacky. really, mount option handling just needs a big rework. -Eric _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/4] xfs: don't emit corruption noise on fs probes 2013-10-11 19:07 [PATCH 0/4] xfs: old lost patches Eric Sandeen 2013-10-11 19:09 ` [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings Eric Sandeen 2013-10-11 19:11 ` [PATCH 2/4] xfs: reject completely bogus remount options Eric Sandeen @ 2013-10-11 19:12 ` Eric Sandeen 2013-10-11 21:21 ` Mark Tinguely 2013-10-15 19:42 ` Christoph Hellwig 2013-10-11 19:14 ` [PATCH 4/4] xfs: don't break from growfs ag update loop on error Eric Sandeen 2013-10-17 18:51 ` [PATCH 0/4] xfs: old lost patches Ben Myers 4 siblings, 2 replies; 19+ messages in thread From: Eric Sandeen @ 2013-10-11 19:12 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs-oss If we get EWRONGFS due to probing of non-xfs filesystems, there's no need to issue the scary corruption error and backtrace. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c index a5b59d9..d327b2c 100644 --- a/fs/xfs/xfs_sb.c +++ b/fs/xfs/xfs_sb.c @@ -624,8 +624,9 @@ xfs_sb_read_verify( out_error: if (error) { - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, - mp, bp->b_addr); + if (error != EWRONGFS) + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, + mp, bp->b_addr); xfs_buf_ioerror(bp, error); } } _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] xfs: don't emit corruption noise on fs probes 2013-10-11 19:12 ` [PATCH 3/4] xfs: don't emit corruption noise on fs probes Eric Sandeen @ 2013-10-11 21:21 ` Mark Tinguely 2013-10-15 19:42 ` Christoph Hellwig 1 sibling, 0 replies; 19+ messages in thread From: Mark Tinguely @ 2013-10-11 21:21 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On 10/11/13 14:12, Eric Sandeen wrote: > If we get EWRONGFS due to probing of non-xfs filesystems, > there's no need to issue the scary corruption error and backtrace. > > Signed-off-by: Eric Sandeen<sandeen@redhat.com> > --- Looks good. Reviewed-by: Mark Tinguely <tinguely@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] xfs: don't emit corruption noise on fs probes 2013-10-11 19:12 ` [PATCH 3/4] xfs: don't emit corruption noise on fs probes Eric Sandeen 2013-10-11 21:21 ` Mark Tinguely @ 2013-10-15 19:42 ` Christoph Hellwig 1 sibling, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2013-10-15 19:42 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On Fri, Oct 11, 2013 at 02:12:31PM -0500, Eric Sandeen wrote: > If we get EWRONGFS due to probing of non-xfs filesystems, > there's no need to issue the scary corruption error and backtrace. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/4] xfs: don't break from growfs ag update loop on error 2013-10-11 19:07 [PATCH 0/4] xfs: old lost patches Eric Sandeen ` (2 preceding siblings ...) 2013-10-11 19:12 ` [PATCH 3/4] xfs: don't emit corruption noise on fs probes Eric Sandeen @ 2013-10-11 19:14 ` Eric Sandeen 2013-10-17 18:51 ` [PATCH 0/4] xfs: old lost patches Ben Myers 4 siblings, 0 replies; 19+ messages in thread From: Eric Sandeen @ 2013-10-11 19:14 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs-oss When xfs_growfs_data_private() is updating backup superblocks, it bails out on the first error encountered, whether reading or writing: * If we get an error writing out the alternate superblocks, * just issue a warning and continue. The real work is * already done and committed. This can cause a problem later during repair, because repair looks at all superblocks, and picks the most prevalent one as correct. If we bail out early in the backup superblock loop, we can end up with more "bad" matching superblocks than good, and a post-growfs repair may revert the filesystem to the old geometry. With the combination of superblock verifiers and old bugs, we're more likely to encounter read errors due to verification. And perhaps even worse, we don't even properly write any of the newly-added superblocks in the new AGs. Even with this change, growfs will still say: xfs_growfs: XFS_IOC_FSGROWFSDATA xfsctl failed: Structure needs cleaning data blocks changed from 319815680 to 335216640 which might be confusing to the user, but it at least communicates that something has gone wrong, and dmesg will probably highlight the need for an xfs_repair. And this is still best-effort; if verifiers fail on more than half the backup supers, they may still "win" - but that's probably best left to repair to more gracefully handle by doing its own strict verification as part of the backup super "voting." Signed-off-by: Eric Sandeen <sandeen@redhat.com> Acked-by: Dave Chinner <david@fromorbit.com> Reviewed-by: Mark Tinguely <tinguely@sgi.com> --- diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index e64ee52..634000c 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -153,7 +153,7 @@ xfs_growfs_data_private( xfs_buf_t *bp; int bucket; int dpct; - int error; + int error, saved_error = 0; xfs_agnumber_t nagcount; xfs_agnumber_t nagimax = 0; xfs_rfsblock_t nb, nb_mod; @@ -496,29 +496,33 @@ xfs_growfs_data_private( error = ENOMEM; } + /* + * If we get an error reading or writing alternate superblocks, + * continue. xfs_repair chooses the "best" superblock based + * on most matches; if we break early, we'll leave more + * superblocks un-updated than updated, and xfs_repair may + * pick them over the properly-updated primary. + */ if (error) { xfs_warn(mp, "error %d reading secondary superblock for ag %d", error, agno); - break; + saved_error = error; + continue; } xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, XFS_SB_ALL_BITS); - /* - * If we get an error writing out the alternate superblocks, - * just issue a warning and continue. The real work is - * already done and committed. - */ error = xfs_bwrite(bp); xfs_buf_relse(bp); if (error) { xfs_warn(mp, "write error %d updating secondary superblock for ag %d", error, agno); - break; /* no point in continuing */ + saved_error = error; + continue; } } - return error; + return saved_error ? saved_error : error; error0: xfs_trans_cancel(tp, XFS_TRANS_ABORT); _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/4] xfs: old lost patches 2013-10-11 19:07 [PATCH 0/4] xfs: old lost patches Eric Sandeen ` (3 preceding siblings ...) 2013-10-11 19:14 ` [PATCH 4/4] xfs: don't break from growfs ag update loop on error Eric Sandeen @ 2013-10-17 18:51 ` Ben Myers 4 siblings, 0 replies; 19+ messages in thread From: Ben Myers @ 2013-10-17 18:51 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs-oss On Fri, Oct 11, 2013 at 02:07:54PM -0500, Eric Sandeen wrote: > 4 patches I found in patchworks in various state of review, but no merges. > > Resending. Applied patch 1v2, 3, and 4. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2013-10-17 18:51 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-11 19:07 [PATCH 0/4] xfs: old lost patches Eric Sandeen 2013-10-11 19:09 ` [PATCH 1/4] xfs: remove newlines from 3 xfs_alert_tag error strings Eric Sandeen 2013-10-11 21:59 ` Mark Tinguely 2013-10-12 1:45 ` Eric Sandeen 2013-10-12 1:59 ` [PATCH 1/4 V2] xfs: remove newlines from strings passed to __xfs_printk Eric Sandeen 2013-10-12 21:07 ` Mark Tinguely 2013-10-11 19:11 ` [PATCH 2/4] xfs: reject completely bogus remount options Eric Sandeen 2013-10-11 21:34 ` Mark Tinguely 2013-10-12 1:40 ` Eric Sandeen 2013-10-12 21:11 ` Mark Tinguely 2013-10-13 21:52 ` Dave Chinner 2013-10-14 2:42 ` Eric Sandeen 2013-10-14 4:45 ` Dave Chinner 2013-10-15 18:13 ` Eric Sandeen 2013-10-11 19:12 ` [PATCH 3/4] xfs: don't emit corruption noise on fs probes Eric Sandeen 2013-10-11 21:21 ` Mark Tinguely 2013-10-15 19:42 ` Christoph Hellwig 2013-10-11 19:14 ` [PATCH 4/4] xfs: don't break from growfs ag update loop on error Eric Sandeen 2013-10-17 18:51 ` [PATCH 0/4] xfs: old lost patches Ben Myers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox