Whamcloud - gitweb
b=23298 backport some llapi_changelog/copytool fixes from HSM branch
authorNathan Rutman <nathan.rutman@oracle.com>
Fri, 13 Aug 2010 17:06:24 +0000 (21:06 +0400)
committerMikhail Pershin <tappro@sun.com>
Thu, 26 Aug 2010 11:43:58 +0000 (15:43 +0400)
455f63ad2b92cea37793a3bce5d412b30ea144c5 zero kuc private memory
d1949858dabc7de7738e2179cfc8e4b3d6fa799f make llapi_copytool_recv thread safe
9582d218eb6abef52814b364e16e5b3b24c29658 make llapi_changelog_recv thread safe

libcfs/include/libcfs/libcfs_kernelcomm.h
libcfs/libcfs/kernel_user_comm.c
lustre/utils/lfs.c
lustre/utils/liblustreapi.c

index ff919b1..b2e5406 100644 (file)
@@ -108,7 +108,7 @@ typedef struct lustre_kernelcomm {
 extern int libcfs_ukuc_start(lustre_kernelcomm *l, int groups);
 extern int libcfs_ukuc_stop(lustre_kernelcomm *l);
 extern int libcfs_ukuc_msg_get(lustre_kernelcomm *l, char *buf, int maxsize,
-                        int transport);
+                               int transport);
 
 #endif /* __LIBCFS_KERNELCOMM_H__ */
 
index a36a3bc..357bcf1 100644 (file)
@@ -59,6 +59,7 @@ int libcfs_ukuc_start(lustre_kernelcomm *link, int group)
         if (pipe(pfd) < 0)
                 return -errno;
 
+        memset(link, 0, sizeof(*link));
         link->lk_rfd = pfd[0];
         link->lk_wfd = pfd[1];
         link->lk_group = group;
@@ -79,7 +80,7 @@ int libcfs_ukuc_stop(lustre_kernelcomm *link)
  * Allocates memory, returns handle
  *
  * @param link Private descriptor for pipe/socket.
- * @param buf Buffer to read into
+ * @param buf Buffer to read into, must include size for kuc_hdr
  * @param maxsize Maximum message size allowed
  * @param transport Only listen to messages on this transport
  *      (and the generic transport)
@@ -167,7 +168,7 @@ int libcfs_kkuc_msg_put(cfs_file_t *filp, void *payload)
 #ifdef __KERNEL__
         {
                 loff_t offset = 0;
-                rc = cfs_user_write(filp, (char *)payload, kuch->kuc_msglen, 
+                rc = cfs_user_write(filp, (char *)payload, kuch->kuc_msglen,
                                     &offset);
         }
 #endif
index 7754ccd..399fa13 100644 (file)
@@ -2387,10 +2387,14 @@ static int lfs_changelog(int argc, char **argv)
                 time_t secs;
                 struct tm ts;
 
-                if (endrec && rec->cr_index > endrec)
+                if (endrec && rec->cr_index > endrec) {
+                        llapi_changelog_free(&rec);
                         break;
-                if (rec->cr_index < startrec)
+                }
+                if (rec->cr_index < startrec) {
+                        llapi_changelog_free(&rec);
                         continue;
+                }
 
                 secs = rec->cr_time >> 30;
                 gmtime_r(&secs, &ts);
index a4c8cf6..d5292f7 100644 (file)
@@ -2830,7 +2830,6 @@ struct changelog_private {
         int magic;
         int flags;
         lustre_kernelcomm kuc;
-        char *buf;
 };
 
 /** Start reading from a changelog
@@ -2847,16 +2846,10 @@ int llapi_changelog_start(void **priv, int flags, const char *device,
         int rc;
 
         /* Set up the receiver control struct */
-        cp = malloc(sizeof(*cp));
+        cp = calloc(1, sizeof(*cp));
         if (cp == NULL)
                 return -ENOMEM;
 
-        cp->buf = malloc(CR_MAXSIZE);
-        if (cp->buf == NULL) {
-                rc = -ENOMEM;
-                goto out_free;
-        }
-
         cp->magic = CHANGELOG_PRIV_MAGIC;
         cp->flags = flags;
 
@@ -2882,8 +2875,6 @@ int llapi_changelog_start(void **priv, int flags, const char *device,
         return 0;
 
 out_free:
-        if (cp->buf)
-                free(cp->buf);
         free(cp);
         return rc;
 }
@@ -2897,7 +2888,6 @@ int llapi_changelog_fini(void **priv)
                 return -EINVAL;
 
         libcfs_ukuc_stop(&cp->kuc);
-        free(cp->buf);
         free(cp);
         *priv = NULL;
         return 0;
@@ -2920,14 +2910,17 @@ int llapi_changelog_recv(void *priv, struct changelog_rec **rech)
                 return -EINVAL;
         if (rech == NULL)
                 return -EINVAL;
+        kuch = malloc(CR_MAXSIZE + sizeof(*kuch));
+        if (kuch == NULL)
+                return -ENOMEM;
 
 repeat:
-        rc = libcfs_ukuc_msg_get(&cp->kuc, cp->buf, CR_MAXSIZE,
+        rc = libcfs_ukuc_msg_get(&cp->kuc, (char *)kuch,
+                                 CR_MAXSIZE + sizeof(*kuch),
                                  KUC_TRANSPORT_CHANGELOG);
         if (rc < 0)
-                return rc;
+                goto out_free;
 
-        kuch = (struct kuc_hdr *)cp->buf;
         if ((kuch->kuc_transport != KUC_TRANSPORT_CHANGELOG) ||
             ((kuch->kuc_msgtype != CL_RECORD) &&
              (kuch->kuc_msgtype != CL_EOF))) {
@@ -2948,19 +2941,30 @@ repeat:
                 }
         }
 
-        /* Our message is a changelog_rec */
+        /* Our message is a changelog_rec.  Use pointer math to skip
+         * kuch_hdr and point directly to the message payload.
+         */
         *rech = (struct changelog_rec *)(kuch + 1);
 
         return 0;
 
 out_free:
         *rech = NULL;
+        free(kuch);
         return rc;
 }
 
 /** Release the changelog record when done with it. */
 int llapi_changelog_free(struct changelog_rec **rech)
 {
+        if (*rech) {
+                /* We allocated memory starting at the kuc_hdr, but passed
+                 * the consumer a pointer to the payload.
+                 * Use pointer math to get back to the header.
+                 */
+                struct kuc_hdr *kuch = (struct kuc_hdr *)*rech - 1;
+                free(kuch);
+        }
         *rech = NULL;
         return 0;
 }
@@ -3078,7 +3082,6 @@ int llapi_path2fid(const char *path, lustre_fid *fid)
 #define CT_PRIV_MAGIC 0xC0BE2001
 struct copytool_private {
         int magic;
-        char *buf;
         char *fsname;
         lustre_kernelcomm kuc;
         __u32 archives;
@@ -3105,13 +3108,12 @@ int llapi_copytool_start(void **priv, char *fsname, int flags,
                 return -EINVAL;
         }
 
-        ct = malloc(sizeof(*ct));
+        ct = calloc(1, sizeof(*ct));
         if (ct == NULL)
                 return -ENOMEM;
 
-        ct->buf = malloc(HAL_MAXSIZE);
         ct->fsname = malloc(strlen(fsname) + 1);
-        if (ct->buf == NULL || ct->fsname == NULL) {
+        if (ct->fsname == NULL) {
                 rc = -ENOMEM;
                 goto out_err;
         }
@@ -3149,8 +3151,6 @@ int llapi_copytool_start(void **priv, char *fsname, int flags,
         return 0;
 
 out_err:
-        if (ct->buf)
-                free(ct->buf);
         if (ct->fsname)
                 free(ct->fsname);
         free(ct);
@@ -3172,7 +3172,6 @@ int llapi_copytool_fini(void **priv)
         /* Shut down the kernelcomms */
         libcfs_ukuc_stop(&ct->kuc);
 
-        free(ct->buf);
         free(ct->fsname);
         free(ct);
         *priv = NULL;
@@ -3198,13 +3197,17 @@ int llapi_copytool_recv(void *priv, struct hsm_action_list **halh, int *msgsize)
         if (halh == NULL || msgsize == NULL)
                 return -EINVAL;
 
-        rc = libcfs_ukuc_msg_get(&ct->kuc, ct->buf, HAL_MAXSIZE,
+        kuch = malloc(HAL_MAXSIZE + sizeof(*kuch));
+        if (kuch == NULL)
+                return -ENOMEM;
+
+        rc = libcfs_ukuc_msg_get(&ct->kuc, (char *)kuch,
+                                 HAL_MAXSIZE + sizeof(*kuch),
                                  KUC_TRANSPORT_HSM);
         if (rc < 0)
-                return rc;
+                goto out_free;
 
         /* Handle generic messages */
-        kuch = (struct kuc_hdr *)ct->buf;
         if (kuch->kuc_transport == KUC_TRANSPORT_GENERIC &&
             kuch->kuc_msgtype == KUC_MSG_SHUTDOWN) {
                 rc = -ESHUTDOWN;
@@ -3220,8 +3223,9 @@ int llapi_copytool_recv(void *priv, struct hsm_action_list **halh, int *msgsize)
                 goto out_free;
         }
 
-        /* Our message is an hsm_action_list */
-
+        /* Our message is a hsm_action_list.  Use pointer math to skip
+         * kuch_hdr and point directly to the message payload.
+         */
         hal = (struct hsm_action_list *)(kuch + 1);
 
         /* Check that we have registered for this archive # */
@@ -3240,14 +3244,15 @@ int llapi_copytool_recv(void *priv, struct hsm_action_list **halh, int *msgsize)
 out_free:
         *halh = NULL;
         *msgsize = 0;
+        free(kuch);
         return rc;
 }
 
 /** Release the action list when done with it. */
 int llapi_copytool_free(struct hsm_action_list **hal)
 {
-        *hal = NULL;
-        return 0;
+        /* Reuse the llapi_changelog_free function */
+        return llapi_changelog_free((struct changelog_rec **)hal);
 }
 
 int llapi_get_connect_flags(const char *mnt, __u64 *flags)