Skip to content

Conversation

@edgargabriel
Copy link
Member

@edgargabriel edgargabriel commented Jan 23, 2017

Fixes a bug reported on the mailing list. ompio did only reposition the individual
file pointer when the file was opened in append mode. Set the shared file
pointer also to point to the end of the file, similarly to the individual
file pointer.

This is the equivalent to commit d3a8d38
on master, cannot be cherry-picked because of differences in the organization
of the ompio files ( usage of mca/common/ompio on master).

Revert the logic of io_ompio_sharedfp_lazy_open. The user now has to explicitely
disable shared fp in order for the structures not to be allocated.
Otherwise, resetting the shared fp e.g. in case the file was opened
in append mode will not work correctly, the code could deadlock.

Signed-off-by: Edgar Gabriel <egabriel@central.uh.edu>
@jsquyres jsquyres added this to the v2.1.0 milestone Jan 23, 2017
@jsquyres jsquyres added the bug label Jan 23, 2017
@jsquyres
Copy link
Member

Just like #2787:

@edgargabriel Got a compile fail:

io_ompio_file_open.c: In function 'ompio_io_ompio_file_open':
io_ompio_file_open.c:257:17: error: 'shared_fp_base_module' undeclared (first use in this function)
                 shared_fp_base_module = ompio_fh->f_sharedfp;
                 ^
io_ompio_file_open.c:257:17: note: each undeclared identifier is reported only once for each function it appears in

Can you have a look?

@edgargabriel
Copy link
Member Author

argh, copy-and-paste error, a line is missing, Will fix.

Fixes a bug reported on the mailing list. ompio did only reposition the individual
file pointer when the file was opened in append mode. Set the shared file
pointer also to point to the end of the file, similarly to the individual
file pointer.

This is the equivalent to commit d3a8d38
on master, cannot be cherry-picked because of differences in the organization
of the ompio files ( usage of mca/common/ompio on master).

Signed-off-by: Edgar Gabriel <egabriel@central.uh.edu>
@edgargabriel edgargabriel force-pushed the pr/sharedfp-append-fix-v2.x branch from f584119 to cfda4b8 Compare January 24, 2017 14:32
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #2787 (review) -- would be nice to see another PR to replace opal_output's with opal_show_helps.

@jsquyres
Copy link
Member

@hppritcha Good to go.

@hppritcha hppritcha merged commit f56ea5d into open-mpi:v2.x Jan 24, 2017
@jsquyres
Copy link
Member

Shoot -- @edgargabriel just mentioned on the v2.0.x version (#2787 (comment)) that we should have waited merging. My fault for not seeing that fast enough... ☹️

@edgargabriel
Copy link
Member Author

I will provide a separate pr for 2.x for the outstanding item. its probably more important to get it in correctly for 2.0.x right now

@jsquyres
Copy link
Member

Thanks @edgargabriel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants