LU-4833 osd-zfs: object leak in __osd_object_create

[fs/lustre-release.git] / lustre / osd-zfs / osd_object.c

diff --git a/lustre/osd-zfs/osd_object.c b/lustre/osd-zfs/osd_object.c
index 0a4646b..b502585 100644 (file)
--- a/lustre/osd-zfs/osd_object.c
+++ b/lustre/osd-zfs/osd_object.c
@@ -632,9 +632,21 @@ static int osd_object_destroy(const struct lu_env *env,
                GOTO(out, rc);
        }
 
-       /* do object accounting */
-       osd_zfs_acct_uid(env, osd, obj->oo_attr.la_uid, -1, oh);
-       osd_zfs_acct_gid(env, osd, obj->oo_attr.la_gid, -1, oh);
+       /* Remove object from inode accounting. It is not fatal for the destroy
+        * operation if something goes wrong while updating accounting, but we
+        * still log an error message to notify the administrator */
+       rc = -zap_increment_int(osd->od_objset.os, osd->od_iusr_oid,
+                       obj->oo_attr.la_uid, -1, oh->ot_tx);
+       if (rc)
+               CERROR("%s: failed to remove "DFID" from accounting ZAP for usr"
+                       " %d: rc = %d\n", osd->od_svname, PFID(fid),
+                       obj->oo_attr.la_uid, rc);
+       rc = -zap_increment_int(osd->od_objset.os, osd->od_igrp_oid,
+                               obj->oo_attr.la_gid, -1, oh->ot_tx);
+       if (rc)
+               CERROR("%s: failed to remove "DFID" from accounting ZAP for grp"
+                       " %d: rc = %d\n", osd->od_svname, PFID(fid),
+                       obj->oo_attr.la_gid, rc);
 
        /* kill object */
        rc = __osd_object_destroy(env, obj, oh->ot_tx, osd_obj_tag);
@@ -944,14 +956,34 @@ static int osd_attr_set(const struct lu_env *env, struct dt_object *dt,
 
        /* do both accounting updates outside oo_attr_lock below */
        if ((la->la_valid & LA_UID) && (la->la_uid != obj->oo_attr.la_uid)) {
-               /* do object accounting */
-               osd_zfs_acct_uid(env, osd, la->la_uid, 1, oh);
-               osd_zfs_acct_uid(env, osd, obj->oo_attr.la_uid, -1, oh);
+               /* Update user accounting. Failure isn't fatal, but we still
+                * log an error message */
+               rc = -zap_increment_int(osd->od_objset.os, osd->od_iusr_oid,
+                                       la->la_uid, 1, oh->ot_tx);
+               if (rc)
+                       CERROR("%s: failed to update accounting ZAP for user "
+                               "%d (%d)\n", osd->od_svname, la->la_uid, rc);
+               rc = -zap_increment_int(osd->od_objset.os, osd->od_iusr_oid,
+                                       obj->oo_attr.la_uid, -1, oh->ot_tx);
+               if (rc)
+                       CERROR("%s: failed to update accounting ZAP for user "
+                               "%d (%d)\n", osd->od_svname,
+                               obj->oo_attr.la_uid, rc);
        }
        if ((la->la_valid & LA_GID) && (la->la_gid != obj->oo_attr.la_gid)) {
-               /* do object accounting */
-               osd_zfs_acct_gid(env, osd, la->la_gid, 1, oh);
-               osd_zfs_acct_gid(env, osd, obj->oo_attr.la_gid, -1, oh);
+               /* Update group accounting. Failure isn't fatal, but we still
+                * log an error message */
+               rc = -zap_increment_int(osd->od_objset.os, osd->od_igrp_oid,
+                                       la->la_gid, 1, oh->ot_tx);
+               if (rc)
+                       CERROR("%s: failed to update accounting ZAP for user "
+                               "%d (%d)\n", osd->od_svname, la->la_gid, rc);
+               rc = -zap_increment_int(osd->od_objset.os, osd->od_igrp_oid,
+                                       obj->oo_attr.la_gid, -1, oh->ot_tx);
+               if (rc)
+                       CERROR("%s: failed to update accounting ZAP for user "
+                               "%d (%d)\n", osd->od_svname,
+                               obj->oo_attr.la_gid, rc);
        }
 
        write_lock(&obj->oo_attr_lock);
@@ -1194,10 +1226,6 @@ int __osd_object_create(const struct lu_env *env, udmu_objset_t *uos,
        int      rc;
 
        LASSERT(tag);
-       spin_lock(&uos->lock);
-       uos->objects++;
-       spin_unlock(&uos->lock);
-
        /* Assert that the transaction has been assigned to a
           transaction group. */
        LASSERT(tx->tx_txg != 0);
@@ -1206,14 +1234,24 @@ int __osd_object_create(const struct lu_env *env, udmu_objset_t *uos,
        oid = dmu_object_alloc(uos->os, DMU_OT_PLAIN_FILE_CONTENTS, 0,
                               DMU_OT_SA, DN_MAX_BONUSLEN, tx);
        rc = -sa_buf_hold(uos->os, oid, tag, dbp);
-       if (rc)
-               return rc;
+       LASSERTF(rc == 0, "sa_buf_hold "LPU64" failed: %d\n", oid, rc);
 
        LASSERT(la->la_valid & LA_MODE);
        la->la_size = 0;
        la->la_nlink = 1;
 
-       return __osd_attr_init(env, uos, oid, tx, la, parent);
+       rc = __osd_attr_init(env, uos, oid, tx, la, parent);
+       if (rc != 0) {
+               sa_buf_rele(*dbp, tag);
+               *dbp = NULL;
+               dmu_object_free(uos->os, oid, tx);
+               return rc;
+       }
+
+       spin_lock(&uos->lock);
+       uos->objects++;
+       spin_unlock(&uos->lock);
+       return 0;
 }
 
 /*
@@ -1473,9 +1511,18 @@ static int osd_object_create(const struct lu_env *env, struct dt_object *dt,
 
        /* Add new object to inode accounting.
         * Errors are not considered as fatal */
-       /* XXX: UID/GID must be defined otherwise we can break accounting */
-       osd_zfs_acct_uid(env, osd, attr->la_uid, 1, oh);
-       osd_zfs_acct_gid(env, osd, attr->la_gid, 1, oh);
+       rc = -zap_increment_int(osd->od_objset.os, osd->od_iusr_oid,
+                               (attr->la_valid & LA_UID) ? attr->la_uid : 0, 1,
+                               oh->ot_tx);
+       if (rc)
+               CERROR("%s: failed to add "DFID" to accounting ZAP for usr %d "
+                       "(%d)\n", osd->od_svname, PFID(fid), attr->la_uid, rc);
+       rc = -zap_increment_int(osd->od_objset.os, osd->od_igrp_oid,
+                               (attr->la_valid & LA_GID) ? attr->la_gid : 0, 1,
+                               oh->ot_tx);
+       if (rc)
+               CERROR("%s: failed to add "DFID" to accounting ZAP for grp %d "
+                       "(%d)\n", osd->od_svname, PFID(fid), attr->la_gid, rc);
 
        /* configure new osd object */
        obj->oo_db = db;
@@ -1710,13 +1757,16 @@ static struct obd_capa *osd_capa_get(const struct lu_env *env,
        RETURN(oc);
 }
 
-static int osd_object_sync(const struct lu_env *env, struct dt_object *dt)
+static int osd_object_sync(const struct lu_env *env, struct dt_object *dt,
+                          __u64 start, __u64 end)
 {
        struct osd_device *osd = osd_obj2dev(osd_dt_obj(dt));
        ENTRY;
 
        /* XXX: no other option than syncing the whole filesystem until we
-        * support ZIL */
+        * support ZIL.  If the object tracked the txg that it was last
+        * modified in, it could pass that txg here instead of "0".  Maybe
+        * the changes are already committed, so no wait is needed at all? */
        txg_wait_synced(dmu_objset_pool(osd->od_objset.os), 0ULL);
 
        RETURN(0);