| Message ID | 5283A88D.9020608@dev.mellanox.co.il (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Hal Rosenstock |
| Headers | show |
On 11/13/13 17:27, Hal Rosenstock wrote: > > From: Vladimir Koushnir <vladimirk@mellanox.com> > > Signed-off-by: Vladimir Koushnir <vladimirk@mellanox.com> > --- > opensm/osm_sa.c | 20 ++++++++++++++++---- > 1 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/opensm/osm_sa.c b/opensm/osm_sa.c > index 8c5ef5d..d5c1275 100644 > --- a/opensm/osm_sa.c > +++ b/opensm/osm_sa.c > @@ -525,25 +525,37 @@ opensm_dump_to_file(osm_opensm_t * p_osm, const char *file_name, > void (*dump_func) (osm_opensm_t * p_osm, FILE * file)) > { > char path[1024]; > + char path_tmp[1032]; > FILE *file; > + int status = 0; > > snprintf(path, sizeof(path), "%s/%s", > p_osm->subn.opt.dump_files_dir, file_name); > > - file = fopen(path, "w"); > + snprintf(path_tmp, sizeof(path_tmp), "%s.tmp", path); > + > + file = fopen(path_tmp, "w"); > if (!file) { > OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 4C01: " > "cannot open file \'%s\': %s\n", > - file_name, strerror(errno)); > + path_tmp, strerror(errno)); > return -1; > } > > - chmod(path, S_IRUSR | S_IWUSR); > + chmod(path_tmp, S_IRUSR | S_IWUSR); > > dump_func(p_osm, file); > > fclose(file); > - return 0; > + > + status = rename(path_tmp, path); > + if (status) { > + OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 4C0B: " > + "Failed to rename file:%s (err:%s)\n", > + path_tmp, strerror(errno)); > + } > + > + return status; > } > > static void mcast_mgr_dump_one_port(cl_map_item_t * p_map_item, void *cxt) Isn't an fdatasync() call missing after dump_func() and before fclose() ? According to Theodore Ts'o calling fdatasync() or fsync() before fclose() is essential during an atomic update. See also http://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync/ for more information. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bart, On 11/13/2013 1:00 PM, Bart Van Assche wrote: > Isn't an fdatasync() call missing after dump_func() and before fclose() > ? According to Theodore Ts'o calling fdatasync() or fsync() before > fclose() is essential during an atomic update. See also > http://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync/ for more > information. Thanks for the pointer. Yes, an fsync is appropriate here. Do you want to send a patch for that or should I just cruft one up ? -- Hal > Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/22/13 15:57, Hal Rosenstock wrote: > Hi Bart, > > On 11/13/2013 1:00 PM, Bart Van Assche wrote: >> Isn't an fdatasync() call missing after dump_func() and before fclose() >> ? According to Theodore Ts'o calling fdatasync() or fsync() before >> fclose() is essential during an atomic update. See also >> http://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync/ for more >> information. > > Thanks for the pointer. Yes, an fsync is appropriate here. Do you want > to send a patch for that or should I just cruft one up ? If you can look into this that's fine for me. I'm already involved in too many different open source projects :-) Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/opensm/osm_sa.c b/opensm/osm_sa.c index 8c5ef5d..d5c1275 100644 --- a/opensm/osm_sa.c +++ b/opensm/osm_sa.c @@ -525,25 +525,37 @@ opensm_dump_to_file(osm_opensm_t * p_osm, const char *file_name, void (*dump_func) (osm_opensm_t * p_osm, FILE * file)) { char path[1024]; + char path_tmp[1032]; FILE *file; + int status = 0; snprintf(path, sizeof(path), "%s/%s", p_osm->subn.opt.dump_files_dir, file_name); - file = fopen(path, "w"); + snprintf(path_tmp, sizeof(path_tmp), "%s.tmp", path); + + file = fopen(path_tmp, "w"); if (!file) { OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 4C01: " "cannot open file \'%s\': %s\n", - file_name, strerror(errno)); + path_tmp, strerror(errno)); return -1; } - chmod(path, S_IRUSR | S_IWUSR); + chmod(path_tmp, S_IRUSR | S_IWUSR); dump_func(p_osm, file); fclose(file); - return 0; + + status = rename(path_tmp, path); + if (status) { + OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 4C0B: " + "Failed to rename file:%s (err:%s)\n", + path_tmp, strerror(errno)); + } + + return status; } static void mcast_mgr_dump_one_port(cl_map_item_t * p_map_item, void *cxt)