From 8bad2c91692e570bb3c8bfa36e053dc4bc72a292 Mon Sep 17 00:00:00 2001 From: Michael MacDonald Date: Fri, 7 Mar 2014 11:18:10 -0500 Subject: [PATCH 1/1] LU-4724 hsm: Safe copytool event logging Protect against concurrent event log writes by multiple threads within a copytool process. Fixes sanity-hsm test_71 failures. Signed-off-by: Michael MacDonald Change-Id: Ie18f7368f386acf0949a038e6bdb2ba7dca5c3a3 Reviewed-on: http://review.whamcloud.com/9553 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: John L. Hammond Reviewed-by: Jinshan Xiong Reviewed-by: Oleg Drokin --- lustre/utils/liblustreapi_hsm.c | 61 ++++++++++++++++++---------------------- lustre/utils/liblustreapi_json.c | 32 +++++++-------------- 2 files changed, 38 insertions(+), 55 deletions(-) diff --git a/lustre/utils/liblustreapi_hsm.c b/lustre/utils/liblustreapi_hsm.c index 3f1b8b9..4831ad4 100644 --- a/lustre/utils/liblustreapi_hsm.c +++ b/lustre/utils/liblustreapi_hsm.c @@ -113,7 +113,7 @@ enum ct_event { }; /* initialized in llapi_hsm_register_event_fifo() */ -FILE *llapi_hsm_event_fp; +int llapi_hsm_event_fd = -1; static inline const char *llapi_hsm_ct_ev2str(int type) { @@ -173,12 +173,14 @@ int llapi_hsm_write_json_event(struct llapi_json_item_list **event) { int rc; char time_string[40]; + char json_buf[PIPE_BUF]; + FILE *buf_file; time_t event_time = time(0); struct tm time_components; struct llapi_json_item_list *json_items; - /* Noop unless the event fp was initialized */ - if (llapi_hsm_event_fp == NULL) + /* Noop unless the event fd was initialized */ + if (llapi_hsm_event_fd < 0) return 0; if (event == NULL || *event == NULL) @@ -203,21 +205,24 @@ int llapi_hsm_write_json_event(struct llapi_json_item_list **event) return rc; } - rc = llapi_json_write_list(event, llapi_hsm_event_fp); - if (rc < 0) { - /* Ignore write failures due to missing reader. */ - if (rc == -EPIPE) - return 0; + buf_file = fmemopen(json_buf, sizeof(json_buf), "w"); + if (buf_file == NULL) + return -errno; - /* Skip llapi_error() here because there's no point - * in creating a JSON-formatted error message about - * failing to write a JSON-formatted message. - */ - fprintf(stderr, - "\nFATAL ERROR IN llapi_hsm_write_list(): rc %d", rc); + rc = llapi_json_write_list(event, buf_file); + if (rc < 0) { + fclose(buf_file); return rc; } + fclose(buf_file); + + if (write(llapi_hsm_event_fd, json_buf, strlen(json_buf)) < 0) { + /* Ignore write failures due to missing reader. */ + if (errno != EPIPE) + return -errno; + } + return 0; } @@ -443,7 +448,7 @@ out_free: */ int llapi_hsm_register_event_fifo(char *path) { - int read_fd, write_fd; + int read_fd; struct stat statbuf; /* Create the FIFO if necessary. */ @@ -476,8 +481,8 @@ int llapi_hsm_register_event_fifo(char *path) /* Open the FIFO for writes, but don't block on waiting * for a reader. */ - write_fd = open(path, O_WRONLY | O_NONBLOCK); - if (write_fd < 0) { + llapi_hsm_event_fd = open(path, O_WRONLY | O_NONBLOCK); + if (llapi_hsm_event_fd < 0) { llapi_error(LLAPI_MSG_ERROR, errno, "cannot open(%s) for write", path); return -errno; @@ -488,19 +493,9 @@ int llapi_hsm_register_event_fifo(char *path) * events are lost. NOTE: Only one reader at a time! */ close(read_fd); - llapi_hsm_event_fp = fdopen(write_fd, "w"); - if (llapi_hsm_event_fp == NULL) { - llapi_error(LLAPI_MSG_ERROR, errno, - "cannot fdopen(%s) for write", path); - return -errno; - } - /* Ignore SIGPIPEs -- can occur if the reader goes away. */ signal(SIGPIPE, SIG_IGN); - /* Don't buffer the event stream. */ - setbuf(llapi_hsm_event_fp, NULL); - return 0; } @@ -514,16 +509,16 @@ int llapi_hsm_register_event_fifo(char *path) */ int llapi_hsm_unregister_event_fifo(char *path) { - /* Noop unless the event fp was initialized */ - if (llapi_hsm_event_fp == NULL) + /* Noop unless the event fd was initialized */ + if (llapi_hsm_event_fd < 0) return 0; - if (fclose(llapi_hsm_event_fp) != 0) + if (close(llapi_hsm_event_fd) < 0) return -errno; unlink(path); - llapi_hsm_event_fp = NULL; + llapi_hsm_event_fd = -1; return 0; } @@ -550,8 +545,8 @@ void llapi_hsm_log_error(enum llapi_message_level level, int _rc, va_list args2; struct llapi_json_item_list *json_items; - /* Noop unless the event fp was initialized */ - if (llapi_hsm_event_fp == NULL) + /* Noop unless the event fd was initialized */ + if (llapi_hsm_event_fd < 0) return; rc = llapi_json_init_list(&json_items); diff --git a/lustre/utils/liblustreapi_json.c b/lustre/utils/liblustreapi_json.c index be9a1e8..a09a1e0 100644 --- a/lustre/utils/liblustreapi_json.c +++ b/lustre/utils/liblustreapi_json.c @@ -124,8 +124,7 @@ int llapi_json_write_list(struct llapi_json_item_list **json_items, FILE *fp) list = *json_items; item = list->ljil_items; - if (fprintf(fp, "{") < 0) - return -errno; + fprintf(fp, "{"); for (i = 0; i < list->ljil_item_count; i++) { if (item == NULL) { llapi_err_noerrno(LLAPI_MSG_ERROR, @@ -136,34 +135,26 @@ int llapi_json_write_list(struct llapi_json_item_list **json_items, FILE *fp) break; } - if (fprintf(fp, "\"%s\": ", item->lji_key) < 0) - return -errno; + fprintf(fp, "\"%s\": ", item->lji_key); switch (item->lji_type) { case LLAPI_JSON_INTEGER: - if (fprintf(fp, "%d", item->lji_integer) < 0) - return -errno; + fprintf(fp, "%d", item->lji_integer); break; case LLAPI_JSON_BIGNUM: - if (fprintf(fp, LPU64, item->lji_u64) < 0) - return -errno; + fprintf(fp, LPU64, item->lji_u64); break; case LLAPI_JSON_REAL: - if (fprintf(fp, "%f", item->lji_real) < 0) - return -errno; + fprintf(fp, "%f", item->lji_real); break; case LLAPI_JSON_STRING: if (llapi_json_escape_string(&escaped_string, - item->lji_string) < 0) { + item->lji_string) < 0) { if (escaped_string != NULL) free(escaped_string); return -errno; } - if (fprintf(fp, "\"%s\"", escaped_string) < 0) { - if (escaped_string != NULL) - free(escaped_string); - return -errno; - } + fprintf(fp, "\"%s\"", escaped_string); if (escaped_string != NULL) free(escaped_string); @@ -172,19 +163,16 @@ int llapi_json_write_list(struct llapi_json_item_list **json_items, FILE *fp) llapi_err_noerrno(LLAPI_MSG_ERROR, "Invalid item type: %d", item->lji_type); /* Ensure valid JSON */ - if (fprintf(fp, "\"\"") < 0) - return -errno; + fprintf(fp, "\"\""); break; } if (i < list->ljil_item_count - 1) - if (fprintf(fp, ", ") < 0) - return -errno; + fprintf(fp, ", "); item = item->lji_next; } - if (fprintf(fp, "}\n") < 0) - return -errno; + fprintf(fp, "}\n"); return 0; } -- 1.8.3.1