natefoo / slurm-drmaa

DRMAA for Slurm: Implementation of the DRMAA C bindings for Slurm
GNU General Public License v3.0
48 stars 22 forks source link

slurm-20.11.1 compatibility. #46

Closed aroudgar closed 3 years ago

aroudgar commented 3 years ago

Hi,

We are using slurm-drmaa for our galaxy instance in our HPC environment. We recently updated our slurm to slurm-20.11.1 however, even the latest version of slurm-drmaa is no longer compatible with our new slurm:

libtool: compile: gcc -DHAVE_CONFIG_H -I. -I.. -I/opt/software/slurm/include/ -I../drmaa_utils/ -Wno-long-long -D_REENTRANT -D_THREAD_SAFE -DNDEBUG -D_GNU_SOURCE -DCONFDIR=/opt/software/slurm-drmaa/etc -Wall -W -Wno-unused-parameter -Wno-format-zero-length -pedantic -std=c99 -g -O2 -pthread -MT libdrmaa_la-drmaa.lo -MD -MP -MF .deps/libdrmaa_la-drmaa.Tpo -c drmaa.c -fPIC -DPIC -o .libs/libdrmaa_la-drmaa.o drmaa.c: In function ‘slurmdrmaa_get_DRM_system’: drmaa.c:65:3: error: unknown type name ‘slurm_ctl_conf_t’; did you mean ‘slurm_conf_t’? 65 slurm_ctl_conf_t * conf_info_msg_ptr = NULL; ^~~~ slurm_conf_t drmaa.c:66:44: warning: passing argument 2 of ‘slurm_load_ctl_conf’ from incompatible pointer type [-Wincompatible-pointer-types] 66 if ( slurm_load_ctl_conf ((time_t) NULL, &conf_info_msg_ptr ) == -1 ) ^~~~~~
int **
In file included from drmaa.c:31: /opt/software/slurm/include/slurm/slurm.h:3715:47: note: expected ‘slurm_conf_t ’ {aka ‘struct ’} but argument is of type ‘int **’ 3715 slurm_conf_t **slurm_ctl_conf_ptr); ~~~^~~~~~ drmaa.c:73:101: error: request for member ‘version’ in something not a structure or union 73 fsd_snprintf(NULL, slurmdrmaa_version, sizeof(slurmdrmaa_version)-1,"SLURM %s", conf_info_msg_ptr->version); ^~ drmaa.c:74:25: warning: passing argument 1 of ‘slurm_free_ctl_conf’ from incompatible pointer type [-Wincompatible-pointer-types] 74 slurm_free_ctl_conf (conf_info_msg_ptr); ^~~~~
int *

In file included from drmaa.c:31: /opt/software/slurm/include/slurm/slurm.h:3722:47: note: expected ‘slurm_conf_t ’ {aka ‘struct ’} but argument is of type ‘int ’ 3722 | extern void slurm_free_ctl_conf(slurm_conf_t slurm_ctl_conf_ptr); | ~~~~^~~~

Is there any plan to update slurm-drmaa?

Thanks, Cheers, Ata

aroudgar commented 3 years ago

ok, I found a solution for that. In file slurm_drmaa/drmaa.c replace: slurm_ctl_conf_t conf_info_msg_ptr = NULL; with slurm_conf_t conf_info_msg_ptr = NULL;

Cheers, Ata

pawsey-kbuckley commented 3 years ago

Just wanted to add a bit more info on this one, in case it is of any use.

The relevant SchedMD commit was

348529e49c Rename slurm_ctl_conf_t to slurm_conf_t.

and there are three places in the slurm-drmaa-1.1.1 tarball source where it is refered to.

A full patch is

--- slurm-drmaa-1.1.1.vanilla/slurm_drmaa/drmaa.c       2019-01-30 01:52:45.000000000 +0800
+++ slurm-drmaa-1.1.1/slurm_drmaa/drmaa.c       2021-01-13 14:31:38.000000000 +0800
@@ -62,7 +62,7 @@
 {
        if(slurmdrmaa_version[0] == '\0') /*no locks as drmaa_get_drm_system is usually called only once */
        {
-               slurm_ctl_conf_t * conf_info_msg_ptr = NULL; 
+               slurm_conf_t * conf_info_msg_ptr = NULL; 
                if ( slurm_load_ctl_conf ((time_t) NULL, &conf_info_msg_ptr ) == -1 ) 
                { 
                        fsd_log_error(("slurm_load_ctl_conf error: %s",slurm_strerror(slurm_get_errno())));
--- slurm-drmaa-1.1.1.vanilla/test/slurm_ping.c 2020-04-01 03:02:18.000000000 +0800
+++ slurm-drmaa-1.1.1/test/slurm_ping.c 2021-01-13 14:32:03.000000000 +0800
@@ -3,13 +3,13 @@
 /* Slurm doesn't provide a public method to just use the local config, which we need in order to set a timeout without
  * having to contact slurmctld first... */
 extern int slurm_conf_destroy(void);
-extern slurm_ctl_conf_t *slurm_conf_lock(void);
+extern slurm_conf_t *slurm_conf_lock(void);
 extern void slurm_conf_unlock(void);

 int main(int argc, char **argv) {
        int status = 1;

-       slurm_ctl_conf_t *slurm_ctl_conf_ptr = slurm_conf_lock();
+       slurm_conf_t *slurm_ctl_conf_ptr = slurm_conf_lock();
        slurm_ctl_conf_ptr->msg_timeout = 3;
        slurm_conf_unlock();

Not familiar enough with slurm-drmaa to suggest a better patch that test's for Slurm versions.

pawsey-kbuckley commented 3 years ago

Am now familiar enough with slurm-drmaa to suggest a better patch !

--- slurm-drmaa-1.1.1.vanilla/slurm_drmaa/drmaa.c       2019-01-30 01:52:45.000000000 +0800
+++ slurm-drmaa-1.1.1/slurm_drmaa/drmaa.c       2021-01-13 15:44:32.000000000 +0800
@@ -62,7 +62,11 @@
 {
        if(slurmdrmaa_version[0] == '\0') /*no locks as drmaa_get_drm_system is usually called only once */
        {
+#if SLURM_VERSION_NUMBER < SLURM_VERSION_NUM(20,11,0)
                slurm_ctl_conf_t * conf_info_msg_ptr = NULL; 
+#else
+               slurm_conf_t * conf_info_msg_ptr = NULL; 
+#endif
                if ( slurm_load_ctl_conf ((time_t) NULL, &conf_info_msg_ptr ) == -1 ) 
                { 
                        fsd_log_error(("slurm_load_ctl_conf error: %s",slurm_strerror(slurm_get_errno())));
--- slurm-drmaa-1.1.1.vanilla/test/slurm_ping.c 2020-04-01 03:02:18.000000000 +0800
+++ slurm-drmaa-1.1.1/test/slurm_ping.c 2021-01-13 15:41:56.000000000 +0800
@@ -3,13 +3,21 @@
 /* Slurm doesn't provide a public method to just use the local config, which we need in order to set a timeout without
  * having to contact slurmctld first... */
 extern int slurm_conf_destroy(void);
+#if SLURM_VERSION_NUMBER < SLURM_VERSION_NUM(20,11,0)
 extern slurm_ctl_conf_t *slurm_conf_lock(void);
+#else
+extern slurm_conf_t *slurm_conf_lock(void);
+#endif
 extern void slurm_conf_unlock(void);

 int main(int argc, char **argv) {
        int status = 1;

+#if SLURM_VERSION_NUMBER < SLURM_VERSION_NUM(20,11,0)
        slurm_ctl_conf_t *slurm_ctl_conf_ptr = slurm_conf_lock();
+#else
+       slurm_conf_t *slurm_ctl_conf_ptr = slurm_conf_lock();
+#endif 
        slurm_ctl_conf_ptr->msg_timeout = 3;
        slurm_conf_unlock();
natefoo commented 3 years ago

Thanks! This should be fixed in #47 and #48 and will be included in the next release (most likely version 1.1.2) shortly.