Merge pull request open-mpi#9945 from edgargabriel/topic/lockedfile-fixes

Topic/lockedfile fixes

Author: edgargabriel

ompi/mca/common/ompio/common_ompio.h

@@ -158,6 +158,7 @@ struct ompio_file_t {
     int                    f_perm;
     ompi_communicator_t   *f_comm;
     const char            *f_filename;
+    char                  *f_fullfilename;
     char                  *f_datarep;
     opal_convertor_t      *f_mem_convertor;
     opal_convertor_t      *f_file_convertor;

ompi/mca/common/ompio/common_ompio_file_open.c

@@ -41,6 +41,9 @@
 #include <math.h>
 #include "common_ompio.h"
 #include "ompi/mca/topo/topo.h"
+#include "opal/util/opal_getcwd.h"
+#include "opal/util/path.h"
+#include "opal/util/os_path.h"
 
 static mca_common_ompio_generate_current_file_view_fn_t generate_current_file_view_fn;
 static mca_common_ompio_get_mca_parameter_value_fn_t get_mca_parameter_value_fn;
@@ -100,6 +103,22 @@ int mca_common_ompio_file_open (ompi_communicator_t *comm,
     ompio_fh->f_get_mca_parameter_value=get_mca_parameter_value_fn;
 
     ompio_fh->f_filename = filename;
+    if (opal_path_is_absolute(filename) ) {
+        ompio_fh->f_fullfilename = strdup(filename);
+    }
+    else {
+        char path[OPAL_PATH_MAX];
+        ret = opal_getcwd(path, OPAL_PATH_MAX);
+        if (OPAL_SUCCESS != ret) {
+            goto fn_fail;
+        }
+        ompio_fh->f_fullfilename = opal_os_path(0, path, filename, NULL);
+        if (NULL == ompio_fh->f_fullfilename){
+            ret = OMPI_ERROR;
+            goto fn_fail;
+        }
+    }
+    
     mca_common_ompio_set_file_defaults (ompio_fh);
 
     ompio_fh->f_split_coll_req    = NULL;
@@ -285,7 +304,7 @@ int mca_common_ompio_file_close (ompio_file_t *ompio_fh)
 	ret = ompio_fh->f_fs->fs_file_close (ompio_fh);
     }
     if ( delete_flag ) {
-        ret = mca_common_ompio_file_delete ( ompio_fh->f_filename, &(MPI_INFO_NULL->super) );
+        ret = mca_common_ompio_file_delete ( ompio_fh->f_fullfilename, &(MPI_INFO_NULL->super) );
     }
 
     if ( NULL != ompio_fh->f_fs ) {
@@ -350,7 +369,8 @@ int mca_common_ompio_file_close (ompio_file_t *ompio_fh)
         free ( ompio_fh->f_coll_write_time );
         ompio_fh->f_coll_write_time = NULL;
     }
-
+    free (ompio_fh->f_fullfilename);
+    
     if ( NULL != ompio_fh->f_coll_read_time ) {
         free ( ompio_fh->f_coll_read_time );
         ompio_fh->f_coll_read_time = NULL;
@@ -371,8 +391,7 @@ int mca_common_ompio_file_close (ompio_file_t *ompio_fh)
     if ( MPI_DATATYPE_NULL != ompio_fh->f_orig_filetype ){
 	ompi_datatype_destroy (&ompio_fh->f_orig_filetype);
     }
-
-
+    
     if (MPI_COMM_NULL != ompio_fh->f_comm && !(ompio_fh->f_flags & OMPIO_SHAREDFP_IS_SET) )  {
         ompi_comm_free (&ompio_fh->f_comm);
     }

ompi/mca/sharedfp/lockedfile/sharedfp_lockedfile_file_open.c

@@ -37,6 +37,11 @@
 #include <fcntl.h>
 #include <unistd.h>
 
+#include "opal/util/output.h"
+#include "opal/util/opal_getcwd.h"
+#include "opal/util/path.h"
+#include "opal/util/os_path.h"
+
 int mca_sharedfp_lockedfile_file_open (struct ompi_communicator_t *comm,
 				       const char* filename,
 				       int amode,
@@ -110,8 +115,26 @@ int mca_sharedfp_lockedfile_file_open (struct ompi_communicator_t *comm,
         return OMPI_ERR_OUT_OF_RESOURCE;
     }
     snprintf(lockedfilename, filenamelen, "%s-%u-%d%s",filename,masterjobid,int_pid,".lock");
-    module_data->filename = lockedfilename;
-    
+    if (opal_path_is_absolute(lockedfilename) ) {
+        module_data->filename = lockedfilename;
+    } else {
+        char path[OPAL_PATH_MAX];
+        err = opal_getcwd(path, OPAL_PATH_MAX);
+        if (OPAL_SUCCESS != err) {
+            free (sh);
+            free (module_data);
+            free (lockedfilename);
+            return err;
+        }
+        module_data->filename = opal_os_path(0, path, lockedfilename, NULL);
+        if (NULL == module_data->filename){
+            free (sh);
+            free (module_data);
+            free (lockedfilename);
+            return OMPI_ERROR;
+        }
+    }
+
     /*-------------------------------------------------*/
     /*Open the lockedfile without shared file pointer  */
     /*-------------------------------------------------*/
@@ -125,12 +148,19 @@ int mca_sharedfp_lockedfile_file_open (struct ompi_communicator_t *comm,
             opal_output(0, "[%d]mca_sharedfp_lockedfile_file_open: Error during file open\n", 
                         fh->f_rank);
             free (sh);
-            free(module_data);
+            free (module_data);
             free (lockedfilename);
             return OMPI_ERROR;
         }
-	write ( handle, &position, sizeof(OMPI_MPI_OFFSET_TYPE) );
-	close ( handle );
+	err = opal_best_effort_write (handle, &position, sizeof(OMPI_MPI_OFFSET_TYPE));
+        if (OPAL_SUCCESS != err)  {
+            free (sh);
+            free (module_data);
+            free (lockedfilename);
+            close (handle);
+            return err;
+        }
+	close (handle);
     }
     err = comm->c_coll->coll_barrier ( comm, comm->c_coll->coll_barrier_module );
     if ( OMPI_SUCCESS != err ) {

ompi/mca/sharedfp/lockedfile/sharedfp_lockedfile_request_position.c

@@ -25,6 +25,9 @@
 #include "ompi/constants.h"
 #include "ompi/mca/sharedfp/sharedfp.h"
 #include "ompi/mca/sharedfp/base/base.h"
+#include "opal/util/output.h"
+#include "opal/util/fd.h"
+
 
 /*Use fcntl to lock the hidden file which stores the current position*/
 #include <fcntl.h>
@@ -76,7 +79,10 @@ int mca_sharedfp_lockedfile_request_position(struct mca_sharedfp_base_data_t * s
 
     /* read from the file */
     lseek ( fd, 0, SEEK_SET );
-    read ( fd, &buf, sizeof(OMPI_MPI_OFFSET_TYPE));
+    ret = opal_fd_read ( fd,  sizeof(OMPI_MPI_OFFSET_TYPE), &buf);
+    if (OPAL_SUCCESS != ret ) {
+        goto exit;
+    }
     if ( mca_sharedfp_lockedfile_verbose ) {
         opal_output(ompi_sharedfp_base_framework.framework_output,
                     "sharedfp_lockedfile_request_position: Read last_offset=%lld! ret=%d\n",buf, ret);
@@ -92,8 +98,11 @@ int mca_sharedfp_lockedfile_request_position(struct mca_sharedfp_base_data_t * s
 
     /* write to the file  */
     lseek ( fd, 0, SEEK_SET );
-    write ( fd, &position, sizeof(OMPI_MPI_OFFSET_TYPE));
-
+    ret = opal_best_effort_write ( fd, &position, sizeof(OMPI_MPI_OFFSET_TYPE));
+    /* No need to handle error case here, the subsequent steps are identical
+       in case of ret != OPAL_SUCCESS, namely release lock and return ret */
+    
+exit:
     /* unlock the file */
     if ( mca_sharedfp_lockedfile_verbose ) {
         opal_output(ompi_sharedfp_base_framework.framework_output,
@@ -115,7 +124,10 @@ int mca_sharedfp_lockedfile_request_position(struct mca_sharedfp_base_data_t * s
     if (fcntl(fd, F_SETLK, &fl) == -1) {
         opal_output(0,"sharedfp_lockedfile_request_position:failed to release lock for fd: %d\n",fd);
         opal_output(0,"error(%i): %s", errno, strerror(errno));
-        return OMPI_ERROR;
+        /* Only overwrite error code if it was OPAL_SUCCESS previously */
+        if (OPAL_SUCCESS == ret ) {
+            ret = OMPI_ERROR;
+        }
     }
     else {
 	if ( mca_sharedfp_lockedfile_verbose ) {