[XFS] Propagate errors from xfs_trans_commit().
authorDavid Chinner <dgc@sgi.com>
Thu, 10 Apr 2008 02:21:18 +0000 (12:21 +1000)
committerLachlan McIlroy <lachlan@redback.melbourne.sgi.com>
Fri, 18 Apr 2008 01:58:17 +0000 (11:58 +1000)
xfs_trans_commit() can return errors when there are problems in the
transaction subsystem. They are indicative that the entire transaction may
be incomplete, and hence the error should be propagated as there is a good
possibility that there is something fatally wrong in the filesystem. Catch
and propagate or warn about commit errors in the places where they are
currently ignored.

SGI-PV: 980084
SGI-Modid: xfs-linux-melb:xfs-kern:30795a

Signed-off-by: David Chinner <dgc@sgi.com>
Signed-off-by: Niv Sardi <xaiki@sgi.com>
Signed-off-by: Lachlan McIlroy <lachlan@sgi.com>
fs/xfs/quota/xfs_qm.c
fs/xfs/quota/xfs_qm_syscalls.c
fs/xfs/xfs_inode.c
fs/xfs/xfs_log_recover.c
fs/xfs/xfs_mount.c
fs/xfs/xfs_rtalloc.c
fs/xfs/xfs_vfsops.c
fs/xfs/xfs_vnodeops.c

index 6aa3445cabad68c9c4892df509c50d03c0a7fb06..40ea56409561cd8b98a180f6c1cd639c8f30476e 100644 (file)
@@ -2392,9 +2392,9 @@ xfs_qm_write_sb_changes(
        }
 
        xfs_mod_sb(tp, flags);
-       (void) xfs_trans_commit(tp, 0);
+       error = xfs_trans_commit(tp, 0);
 
-       return 0;
+       return error;
 }
 
 
index 556018d24cad4e891b79d8c2dd5aeab14be079bb..8342823dbdc34913548e9bd285de1bb8aa991f7c 100644 (file)
@@ -734,12 +734,12 @@ xfs_qm_scall_setqlim(
        xfs_trans_log_dquot(tp, dqp);
 
        xfs_dqtrace_entry(dqp, "Q_SETQLIM: COMMIT");
-       xfs_trans_commit(tp, 0);
+       error = xfs_trans_commit(tp, 0);
        xfs_qm_dqprint(dqp);
        xfs_qm_dqrele(dqp);
        mutex_unlock(&(XFS_QI_QOFFLOCK(mp)));
 
-       return (0);
+       return error;
 }
 
 STATIC int
index d7514f8317df424e2cb75dd3ce2ce09a28cac80e..63e66890f0631a7d46bf82b37e7692435932cd14 100644 (file)
@@ -1699,33 +1699,16 @@ xfs_itruncate_finish(
                         * blocks in the file system, but oh well.
                         */
                        xfs_bmap_cancel(&free_list);
-                       if (committed) {
-                               /*
-                                * If the passed in transaction committed
-                                * in xfs_bmap_finish(), then we want to
-                                * add the inode to this one before returning.
-                                * This keeps things simple for the higher
-                                * level code, because it always knows that
-                                * the inode is locked and held in the
-                                * transaction that returns to it whether
-                                * errors occur or not.  We don't mark the
-                                * inode dirty so that this transaction can
-                                * be easily aborted if possible.
-                                */
-                               xfs_trans_ijoin(ntp, ip,
-                                       XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
-                               xfs_trans_ihold(ntp, ip);
-                       }
+                       if (committed)
+                               goto error_join;
                        return error;
                }
 
                if (committed) {
                        /*
-                        * The first xact was committed,
-                        * so add the inode to the new one.
-                        * Mark it dirty so it will be logged
-                        * and moved forward in the log as
-                        * part of every commit.
+                        * The first xact was committed, so add the inode to
+                        * the new one.  Mark it dirty so it will be logged and
+                        * moved forward in the log as part of every commit.
                         */
                        xfs_trans_ijoin(ntp, ip,
                                        XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
@@ -1733,19 +1716,16 @@ xfs_itruncate_finish(
                        xfs_trans_log_inode(ntp, ip, XFS_ILOG_CORE);
                }
                ntp = xfs_trans_dup(ntp);
-               (void) xfs_trans_commit(*tp, 0);
+               error = xfs_trans_commit(*tp, 0);
                *tp = ntp;
+               if (error)
+                       goto error_join;
                error = xfs_trans_reserve(ntp, 0, XFS_ITRUNCATE_LOG_RES(mp), 0,
                                          XFS_TRANS_PERM_LOG_RES,
                                          XFS_ITRUNCATE_LOG_COUNT);
-               /*
-                * Add the inode being truncated to the next chained
-                * transaction.
-                */
-               xfs_trans_ijoin(ntp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
-               xfs_trans_ihold(ntp, ip);
                if (error)
-                       return (error);
+                       goto error_join;
+
        }
        /*
         * Only update the size in the case of the data fork, but
@@ -1777,6 +1757,18 @@ xfs_itruncate_finish(
               (ip->i_d.di_nextents == 0));
        xfs_itrunc_trace(XFS_ITRUNC_FINISH2, ip, 0, new_size, 0, 0);
        return 0;
+
+error_join:
+       /*
+        * Add the inode being truncated to the next chained transaction.  This
+        * keeps things simple for the higher level code, because it always
+        * knows that the inode is locked and held in the transaction that
+        * returns to it whether errors occur or not.  We don't mark the inode
+        * dirty so that this transaction can be easily aborted if possible.
+        */
+       xfs_trans_ijoin(ntp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
+       xfs_trans_ihold(ntp, ip);
+       return error;
 }
 
 
index 957b8caddf1ee56db8720ee6b479d50f2ca66f81..418582b709ebcdfcb99f35f84aeec037854842bb 100644 (file)
@@ -3017,7 +3017,7 @@ xlog_recover_process_efi(
        }
 
        efip->efi_flags |= XFS_EFI_RECOVERED;
-       xfs_trans_commit(tp, 0);
+       error = xfs_trans_commit(tp, 0);
        return error;
 }
 
@@ -3131,16 +3131,13 @@ xlog_recover_clear_agi_bucket(
                error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
                                   XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
                                   XFS_FSS_TO_BB(mp, 1), 0, &agibp);
-       if (error) {
-               xfs_trans_cancel(tp, XFS_TRANS_ABORT);
-               return;
-       }
+       if (error)
+               goto out_abort;
 
+       error = EINVAL;
        agi = XFS_BUF_TO_AGI(agibp);
-       if (be32_to_cpu(agi->agi_magicnum) != XFS_AGI_MAGIC) {
-               xfs_trans_cancel(tp, XFS_TRANS_ABORT);
-               return;
-       }
+       if (be32_to_cpu(agi->agi_magicnum) != XFS_AGI_MAGIC)
+               goto out_abort;
 
        agi->agi_unlinked[bucket] = cpu_to_be32(NULLAGINO);
        offset = offsetof(xfs_agi_t, agi_unlinked) +
@@ -3148,7 +3145,17 @@ xlog_recover_clear_agi_bucket(
        xfs_trans_log_buf(tp, agibp, offset,
                          (offset + sizeof(xfs_agino_t) - 1));
 
-       (void) xfs_trans_commit(tp, 0);
+       error = xfs_trans_commit(tp, 0);
+       if (error)
+               goto out_error;
+       return;
+
+out_abort:
+       xfs_trans_cancel(tp, XFS_TRANS_ABORT);
+out_error:
+       xfs_fs_cmn_err(CE_WARN, mp, "xlog_recover_clear_agi_bucket: "
+                       "failed to clear agi %d. Continuing.", agno);
+       return;
 }
 
 /*
index 244aa1b9f134e942dd3f8f8bd480c34f2d0c3b1d..2d03fe194c2cee9bc0612aaa19f6dcfc2f6bb634 100644 (file)
@@ -45,7 +45,7 @@
 #include "xfs_fsops.h"
 #include "xfs_utils.h"
 
-STATIC void    xfs_mount_log_sb(xfs_mount_t *, __int64_t);
+STATIC int     xfs_mount_log_sb(xfs_mount_t *, __int64_t);
 STATIC int     xfs_uuid_mount(xfs_mount_t *);
 STATIC void    xfs_uuid_unmount(xfs_mount_t *mp);
 STATIC void    xfs_unmountfs_wait(xfs_mount_t *);
@@ -1189,8 +1189,13 @@ xfs_mountfs(
        /*
         * If fs is not mounted readonly, then update the superblock changes.
         */
-       if (update_flags && !(mp->m_flags & XFS_MOUNT_RDONLY))
-               xfs_mount_log_sb(mp, update_flags);
+       if (update_flags && !(mp->m_flags & XFS_MOUNT_RDONLY)) {
+               error = xfs_mount_log_sb(mp, update_flags);
+               if (error) {
+                       cmn_err(CE_WARN, "XFS: failed to write sb changes");
+                       goto error4;
+               }
+       }
 
        /*
         * Initialise the XFS quota management subsystem for this mount
@@ -1320,8 +1325,10 @@ xfs_unmountfs(xfs_mount_t *mp, struct cred *cr)
                cmn_err(CE_WARN, "XFS: Unable to free reserved block pool. "
                                "Freespace may not be correct on next mount.");
 
-
-       xfs_log_sbcount(mp, 1);
+       error = xfs_log_sbcount(mp, 1);
+       if (error)
+               cmn_err(CE_WARN, "XFS: Unable to update superblock counters. "
+                               "Freespace may not be correct on next mount.");
        xfs_unmountfs_writesb(mp);
        xfs_unmountfs_wait(mp);                 /* wait for async bufs */
        xfs_log_unmount(mp);                    /* Done! No more fs ops. */
@@ -1413,9 +1420,8 @@ xfs_log_sbcount(
        xfs_mod_sb(tp, XFS_SB_IFREE | XFS_SB_ICOUNT | XFS_SB_FDBLOCKS);
        if (sync)
                xfs_trans_set_sync(tp);
-       xfs_trans_commit(tp, 0);
-
-       return 0;
+       error = xfs_trans_commit(tp, 0);
+       return error;
 }
 
 STATIC void
@@ -1913,24 +1919,27 @@ xfs_uuid_unmount(
  * be altered by the mount options, as well as any potential sb_features2
  * fixup. Only the first superblock is updated.
  */
-STATIC void
+STATIC int
 xfs_mount_log_sb(
        xfs_mount_t     *mp,
        __int64_t       fields)
 {
        xfs_trans_t     *tp;
+       int             error;
 
        ASSERT(fields & (XFS_SB_UNIT | XFS_SB_WIDTH | XFS_SB_UUID |
                         XFS_SB_FEATURES2 | XFS_SB_BAD_FEATURES2));
 
        tp = xfs_trans_alloc(mp, XFS_TRANS_SB_UNIT);
-       if (xfs_trans_reserve(tp, 0, mp->m_sb.sb_sectsize + 128, 0, 0,
-                               XFS_DEFAULT_LOG_COUNT)) {
+       error = xfs_trans_reserve(tp, 0, mp->m_sb.sb_sectsize + 128, 0, 0,
+                               XFS_DEFAULT_LOG_COUNT);
+       if (error) {
                xfs_trans_cancel(tp, 0);
-               return;
+               return error;
        }
        xfs_mod_sb(tp, fields);
-       xfs_trans_commit(tp, 0);
+       error = xfs_trans_commit(tp, 0);
+       return error;
 }
 
 
index 9cd6471cd60fd6a8c25dd0bea63bff353121614c..a0dc6e5bc5b9fe5a7fa0fd2cc6e4be759c3399d9 100644 (file)
@@ -124,14 +124,14 @@ xfs_growfs_rt_alloc(
                                XFS_GROWRTALLOC_LOG_RES(mp), 0,
                                XFS_TRANS_PERM_LOG_RES,
                                XFS_DEFAULT_PERM_LOG_COUNT)))
-                       goto error_exit;
+                       goto error_cancel;
                cancelflags = XFS_TRANS_RELEASE_LOG_RES;
                /*
                 * Lock the inode.
                 */
                if ((error = xfs_trans_iget(mp, tp, ino, 0,
                                                XFS_ILOCK_EXCL, &ip)))
-                       goto error_exit;
+                       goto error_cancel;
                XFS_BMAP_INIT(&flist, &firstblock);
                /*
                 * Allocate blocks to the bitmap file.
@@ -144,14 +144,16 @@ xfs_growfs_rt_alloc(
                if (!error && nmap < 1)
                        error = XFS_ERROR(ENOSPC);
                if (error)
-                       goto error_exit;
+                       goto error_cancel;
                /*
                 * Free any blocks freed up in the transaction, then commit.
                 */
                error = xfs_bmap_finish(&tp, &flist, &committed);
                if (error)
-                       goto error_exit;
-               xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
+                       goto error_cancel;
+               error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
+               if (error)
+                       goto error;
                /*
                 * Now we need to clear the allocated blocks.
                 * Do this one block per transaction, to keep it simple.
@@ -166,13 +168,13 @@ xfs_growfs_rt_alloc(
                         */
                        if ((error = xfs_trans_reserve(tp, 0,
                                        XFS_GROWRTZERO_LOG_RES(mp), 0, 0, 0)))
-                               goto error_exit;
+                               goto error_cancel;
                        /*
                         * Lock the bitmap inode.
                         */
                        if ((error = xfs_trans_iget(mp, tp, ino, 0,
                                                        XFS_ILOCK_EXCL, &ip)))
-                               goto error_exit;
+                               goto error_cancel;
                        /*
                         * Get a buffer for the block.
                         */
@@ -181,14 +183,16 @@ xfs_growfs_rt_alloc(
                                mp->m_bsize, 0);
                        if (bp == NULL) {
                                error = XFS_ERROR(EIO);
-                               goto error_exit;
+                               goto error_cancel;
                        }
                        memset(XFS_BUF_PTR(bp), 0, mp->m_sb.sb_blocksize);
                        xfs_trans_log_buf(tp, bp, 0, mp->m_sb.sb_blocksize - 1);
                        /*
                         * Commit the transaction.
                         */
-                       xfs_trans_commit(tp, 0);
+                       error = xfs_trans_commit(tp, 0);
+                       if (error)
+                               goto error;
                }
                /*
                 * Go on to the next extent, if any.
@@ -196,8 +200,9 @@ xfs_growfs_rt_alloc(
                oblocks = map.br_startoff + map.br_blockcount;
        }
        return 0;
-error_exit:
+error_cancel:
        xfs_trans_cancel(tp, cancelflags);
+error:
        return error;
 }
 
@@ -1876,6 +1881,7 @@ xfs_growfs_rt(
        xfs_trans_t     *tp;            /* transaction pointer */
 
        sbp = &mp->m_sb;
+       cancelflags = 0;
        /*
         * Initial error checking.
         */
@@ -2042,13 +2048,15 @@ xfs_growfs_rt(
                 */
                mp->m_rsumlevels = nrsumlevels;
                mp->m_rsumsize = nrsumsize;
-               /*
-                * Commit the transaction.
-                */
-               xfs_trans_commit(tp, 0);
+
+               error = xfs_trans_commit(tp, 0);
+               if (error) {
+                       tp = NULL;
+                       break;
+               }
        }
 
-       if (error)
+       if (error && tp)
                xfs_trans_cancel(tp, cancelflags);
 
        /*
index 6351efb569c7350d78ab00de811f16d6a776d40b..09e186d02c114322e8f202ae2b44939a9b67e4c8 100644 (file)
@@ -672,6 +672,8 @@ void
 xfs_attr_quiesce(
        xfs_mount_t     *mp)
 {
+       int     error = 0;
+
        /* wait for all modifications to complete */
        while (atomic_read(&mp->m_active_trans) > 0)
                delay(100);
@@ -682,7 +684,11 @@ xfs_attr_quiesce(
        ASSERT_ALWAYS(atomic_read(&mp->m_active_trans) == 0);
 
        /* Push the superblock and write an unmount record */
-       xfs_log_sbcount(mp, 1);
+       error = xfs_log_sbcount(mp, 1);
+       if (error)
+               xfs_fs_cmn_err(CE_WARN, mp,
+                               "xfs_attr_quiesce: failed to log sb changes. "
+                               "Frozen image may not be consistent.");
        xfs_log_unmount_write(mp);
        xfs_unmountfs_writesb(mp);
 }
@@ -1316,8 +1322,11 @@ xfs_syncsub(
         * of sync if we crash or get a forced shutdown. We don't want to force
         * this to disk, just get a transaction into the iclogs....
         */
-       if (flags & SYNC_SUPER)
-               xfs_log_sbcount(mp, 0);
+       if (flags & SYNC_SUPER) {
+               error = xfs_log_sbcount(mp, 0);
+               if (error)
+                       last_error = error;
+       }
 
        /*
         * Now check to see if the log needs a "dummy" transaction.
index d46f24c684989e663a22aba1b8484a40d953a81b..bc0a4707189a0147212bf2d46cdbfb59cfd6f58f 100644 (file)
@@ -1447,28 +1447,22 @@ xfs_inactive_attrs(
        tp = *tpp;
        mp = ip->i_mount;
        ASSERT(ip->i_d.di_forkoff != 0);
-       xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
+       error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
        xfs_iunlock(ip, XFS_ILOCK_EXCL);
+       if (error)
+               goto error_unlock;
 
        error = xfs_attr_inactive(ip);
-       if (error) {
-               *tpp = NULL;
-               xfs_iunlock(ip, XFS_IOLOCK_EXCL);
-               return error; /* goto out */
-       }
+       if (error)
+               goto error_unlock;
 
        tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
        error = xfs_trans_reserve(tp, 0,
                                  XFS_IFREE_LOG_RES(mp),
                                  0, XFS_TRANS_PERM_LOG_RES,
                                  XFS_INACTIVE_LOG_COUNT);
-       if (error) {
-               ASSERT(XFS_FORCED_SHUTDOWN(mp));
-               xfs_trans_cancel(tp, 0);
-               *tpp = NULL;
-               xfs_iunlock(ip, XFS_IOLOCK_EXCL);
-               return error;
-       }
+       if (error)
+               goto error_cancel;
 
        xfs_ilock(ip, XFS_ILOCK_EXCL);
        xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
@@ -1479,6 +1473,14 @@ xfs_inactive_attrs(
 
        *tpp = tp;
        return 0;
+
+error_cancel:
+       ASSERT(XFS_FORCED_SHUTDOWN(mp));
+       xfs_trans_cancel(tp, 0);
+error_unlock:
+       *tpp = NULL;
+       xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+       return error;
 }
 
 int