Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tool Connection to PMIx Server #68

Merged
merged 1 commit into from
Jul 7, 2016
Merged

Conversation

rhc54
Copy link
Contributor

@rhc54 rhc54 commented Mar 31, 2016

tl -> address.sun_family = AF_UNIX;
tl->protocol = PMIX_PROTOCOL_TOOL;
/* Get up to 10 chars of hostname.*/
gethostname(myhostname, myhostnamelen);
Copy link
Contributor

Choose a reason for hiding this comment

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

@rhc54 you need this inline patch

diff --git a/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server.c b/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server.c
index 0533b38..18ac503 100644
--- a/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server.c
+++ b/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server.c
@@ -227,7 +227,7 @@ PMIX_EXPORT pmix_status_t PMIx_server_init(pmix_server_module_t *module,
     size_t n, m;
     pmix_kval_t kv;
     pmix_listener_t *lt;
-    int myhostnamelen = 10;
+    int myhostnamelen = 11;
     char myhostname[myhostnamelen];
     char *pmix_pid, *tdir;
     char **protected = NULL;
@@ -293,6 +293,10 @@ PMIX_EXPORT pmix_status_t PMIx_server_init(pmix_server_module_t *module,
                 tl->protocol = PMIX_PROTOCOL_TOOL;
                 /* Get up to 10 chars of hostname.*/
                 gethostname(myhostname, myhostnamelen);
+                /* gethostname might not null terminate myhostname
+                 * if hostname size is greater than myhostnamelen,
+                 * so do it ourself */
+                myhostname[10] = '\0';
                 /* need to put this in the global tmpdir as opposed to
                  * where the server tmpdir might be */
                 if (NULL == (tdir = getenv("TMPDIR"))) {

otherwise i get some random hangs on my centos 7 box with a long hostname

Copy link
Contributor

Choose a reason for hiding this comment

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

@rhc54 it turned out the race condition was something different.
i wrote a fix in #112

@ggouaillardet
Copy link
Contributor

this pr should be rebased because of the conflict caused by #112

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 5, 2016

working on it...

* creating a separate Unix domain socket for tool/PMIx server rendezvous
  in a location both agree upon. The system will automatically look for
  the rendezvous file in a standard tmp directory using the format of
  "TMPDIR/pmix.hostname.pid". If only one is found, then that is used.
  If multiple are found and no directive was given as to which pid to
  use, then an error is returned. If a directive is given, then only
  that pid is searched for and an error returned if not found.

* defining a new "pmix_tool.h" header that solely contains two APIs:
    * PMIx_Tool_init - initializes the connection to the local PMIx server
    * PMIx_Tool_finalize - closes the connection

* adds new PMIx_Query_info[_nb] APIs by which the tool can request
  info from the RM via the local PMIx server

The commit includes a unit test simple/simptool that exercises the
new functionality, and an example program

Modify the configure code so we have the option of installing the pmix library without the tests and examples

Remove stale header reference

Resolve merge conflicts with master

Cleanups from Coverity

Fix Coverity warning

Allow longer hostname lengths, ensure they are NULL terminated

Provide the host RM with the uid and gid of the connecting tool
@rhc54 rhc54 merged commit 9fc9c9e into openpmix:master Jul 7, 2016
@rhc54 rhc54 deleted the topic/tools branch July 7, 2016 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants