[Spread-users] patches for spread 5.0.0rc1
John Lane Schultz
jschultz at spreadconcepts.com
Tue Jan 3 09:34:46 EST 2017
Daniel,
Thank you for testing and your detailed patch notes! I’ll incorporate them into the next release candidate, which should be out in the next week or two.
Cheers!
-----
John Lane Schultz
Spread Concepts LLC
Cell: 443 838 2200
On Jan 1, 2017, at 8:22 PM, Daniel F. Savarese <dfs at savarese.org> wrote:
Before I get to the patches, I'd like to report that libssrcspread
1.0.12 compiles against spread-5.0.0rc1 with no changes and passes all
unit tests. In addition, software using the libssrcspread dynamic
library compiled against spread-4.x does not need to be recompiled.
Our entire stack of software sitting on top of spread via
libssrcspread worked with spread-5.0.0rc1 without a hitch without
recompiling (the key being the use of dynamic libraries). Also, all
of the unit tests for said software passed. It looks like
spread-5.0.0rc1 by and large doesn't suffer from regressions.
There are, however, some minor issues that required correcting which
I'll share via inline patches (I'm uncertain if the mailing list
strips attachments) in this email. These patches are part of a larger
set of patches replacing select with FreeBSD kqueue or Linux epoll for
the daemon and POSIX poll for the client library when available.
Therefore, the line numbers may not always match up with the released
source; but most of the patches should apply based on context. I
don't intend for the patches to be applied verbatim, but rather
present them to explain the problems I've run into with the latest
spread release so that those who are more familiar with the code can
fix the issues in a more appropriate manner if required.
Patch 1:
[Fixed FreeBSD compilation error in daemon/ tree (INT32_MAX undefined).]
-------
The first patch addresses a long-standing problem with compiling
out of the box on FreeBSD. The definition for INT32_MAX doesn't
get picked up by monitor.c and causes a compilation error. I
chose to make the change to arch.h instead of monitor.c to avoid
potential problems with new files in the future. Successful linking
on FreeBSD requires an empty definition for the LIBSPREADUTIL_LIBS
variable, but we build on FreeBSD with "make LIBSPREADUTIL_LIBS=" via
another build/packaging layer, so I don't have a cross-platform patch
for that. FreeBSD doesn't have libdl because the dlfoo functions are
part of libc, so if you don't override LIBSPREADUTIL_LIBS, the linker
will error out not being able to find libdl.
diff --git a/daemon/arch.h b/daemon/arch.h
index b02e3b0..41840f8 100644
--- a/daemon/arch.h
+++ b/daemon/arch.h
@@ -36,6 +36,8 @@
#ifndef INC_ARCH
#define INC_ARCH
+#include <stdint.h>
+
/*
* Each record in this file represents an architecture.
* Each record contains the following fields:
Patch 2:
[Fixed fixpaths to work with Perl 5.22.]
-------
The fixpaths script fails when executed using Perl 5.22 or higher,
causing "make install" to fail. This patch makes fixpaths work with
Perl 5.22 without breaking it for Perl 5.20.
diff --git a/buildtools/fixpaths b/buildtools/fixpaths
index 7e4178e..b868fff 100644
--- a/buildtools/fixpaths
+++ b/buildtools/fixpaths
@@ -5,7 +5,7 @@
$usage = "Usage: $0 [-Dstring=replacement] [[infile] ...]\n";
-if (!defined(@ARGV)) { die ("$usage"); }
+if (!@ARGV) { die ("$usage"); }
# read in the command line and get some definitions
while ($_=$ARGV[0], /^-/) {
@@ -23,7 +23,7 @@ while ($_=$ARGV[0], /^-/) {
}
} # while parsing arguments
-if (!defined(%def)) {
+if (!%def) {
die ("$0: nothing to do - no substitutions listed!\n");
}
Patch 3:
[Fixed daemon session log initialization sequence.]
-------
When running spread with -l, the log is opened in the current
directory before being reopened in a potentially different directory
after a chdir call. This effect is evident when starting spread via
an init system susch as OpenRC, which changes the current directory to
/ before starting the daemon. In such a case, spread creates a log
file in / before changing to the configured runtime directory (e.g.,
/var/run/spread) and reopening the file there.
This patch initializes the log after the chdir to the runtime
directory, ensuring only one file is created. It also comments out
the closing and reopening of the log file that--at least in my case--
was causing data loss at the beginning of log lines. If the log needs
to be closed and reopened, then at least the ftell/fseeks can be
omitted and the file can be reopened in append mode. I don't
understand the purpose of the existing code, which is why I commented
it out instead of removing it.
diff --git a/daemon/log.c b/daemon/log.c
index 00b14d8..36980a0 100644
--- a/daemon/log.c
+++ b/daemon/log.c
@@ -96,8 +96,9 @@ void Log_init()
static void Log_alive(int dummy, void *dummy_p)
{
+ /*
long file_pos;
-
+ */
if( !Is_inited ) return;
#if ( SPREAD_PROTOCOL == 3 )
fprintf( fd, "A %13ld %11d\n",E_get_time().sec, Highest_seq );
@@ -106,16 +107,18 @@ static void Log_alive(int dummy, void *dummy_p)
#endif
if( fseek( fd, -28, SEEK_CUR ) )
Alarm( EXIT, "Log_alive: error (%s) in fseek -28 on %s\n", strerror(errno), My_name);
- file_pos = ftell(fd);
+ /*file_pos = ftell(fd);*/
if( fseek( fd, 28, SEEK_CUR ) )
Alarm( EXIT, "Log_alive: error (%s) in fseek 28 on %s\n", strerror(errno), My_name);
+ fflush( fd );
+ /*
fclose(fd);
fd = fopen( My_name, "r+" );
if( fd == NULL )
Alarm( EXIT, "Log_alive: error (%s) could not open file %s\n",strerror(errno), My_name );
if( fseek( fd, file_pos, SEEK_SET ) )
Alarm( EXIT, "Log_alive: error (%s) in fseek file_pos (%ld) on %s\n", strerror(errno), file_pos, My_name);
-
+ */
E_queue( Log_alive, 0, NULL, alive_time );
}
diff --git a/daemon/spread.c b/daemon/spread.c
index 8bba340..38a6947 100644
--- a/daemon/spread.c
+++ b/daemon/spread.c
@@ -208,7 +208,6 @@ int main(int argc, char *argv[])
Sess_init();
Stat_init();
- if( Log ) Log_init();
#ifndef ARCH_PC_WIN95
@@ -236,6 +235,13 @@ int main(int argc, char *argv[])
#endif /* ARCH_PC_WIN95 */
+ /* 2016-12-28 dfs
+ *
+ * Init log AFTER chdir, NOT before. Otherwise, more than one
+ * file could be opened in more than one directory.
+ */
+ if( Log ) Log_init();
+
E_handle_events();
Sess_fini();
Patch 4:
[Restored functioning of loopback config file and fixed FreeBSD scope id.]
-------
This is the most important patch of the bunch. Compiled out of the
box, the following IPv4 segment definition that used to work with
spread-4.x no longer works with spread-5.0.0rc2 on FreeBSD:
Spread_Segment 127.0.0.255:4803 {
localhost 127.0.0.1 {
D 127.0.0.1
}
}
Also, for IPv6, the equivalent segment definition doesn't work on
FreeBSD (trying to set interface scope id's with "%interface" doesn't
get around the issue):
Spread_Segment [::1]:4803 {
localhost ::1 VirtualID = 1 {
D ::1
}
}
In addition, the first definition causes spread to bind to
127.0.0.255 on Linux. Spread 4.x would skip binding to
127.0.0.255 and bind only to 127.0.0.1.
This portion of my commit message for the patch explains the situation:
Loopback only config files stopped working with Spread 5.0.0rc1.
Spread 4.x would not bind a broadcast address if not needed, so I
ported that feature by avoiding binding the broadcast address
when not needed.
On FreeBSD getifaddrs fills in the scope ids of the returned addresses.
Spread changes those scope ids, causing at least the loopback address
to not bind. I prevented the override when __FreeBSD__ is defined.
There's probably a more correct way to get the loopback config files
working given that bcast_bound[] is supposed to track whether or not
to bind the address, but this is how I got the behavior I expected on
both Linux and FreeBSD.
diff --git a/daemon/ip_enum.c b/daemon/ip_enum.c
index 760210b..0397fe3 100644
--- a/daemon/ip_enum.c
+++ b/daemon/ip_enum.c
@@ -295,8 +295,15 @@ static void ip_enum_getifaddrs(ip_array *array)
if (spu_addr_from_sockaddr_known(&addr, curr->ifa_addr))
Alarmp(SPLOG_FATAL, CONF_SYS, SPLOC ": ip_enum_getifaddrs: BUG! spu_addr_from_sockaddr_known failed?!");
+ /* 2016-12-29 dfs
+ *
+ * FreeBSD fills in the proper scope id, which can be 0 for loopback, so
+ * don't override it.
+ */
+#if !defined(__FreeBSD__)
if (curr->ifa_addr->sa_family == AF_INET6)
addr.ipv6.sin6_scope_id = if_nametoindex(curr->ifa_name);
+#endif
add_ip(array, &addr);
}
diff --git a/daemon/network.c b/daemon/network.c
index c83d2c4..cd79092 100644
--- a/daemon/network.c
+++ b/daemon/network.c
@@ -287,6 +287,7 @@ void Net_init()
*/
#ifndef ARCH_PC_WIN95
+ if(Bcast_needed) {
for (i = 0; i < Bcast_addrs_num; ++i)
if (!bcast_bound[i]) /* didn't already explicity or implicitly bind to Bcast_addrs[i] above */
{
@@ -307,6 +308,7 @@ void Net_init()
++Num_bcast_channels;
}
+ }
#endif
}
Patch 5:
[Corrected priority and fd_type range checking in events.c.]
-------
This patch will not apply cleanly because I made the change on our
custom kqueue/epoll branch and affects lines not present in the
original source. It should, however, be easy enough to do a search
and replace to make the changes assuming I'm correct about the
changes. This portion of my commit message explains the patch:
Checks were being performed for > NUM_PRIORITY instead of >= NUM_PRIORITY.
Corrected return value check in sp.c that treated a zero-byte send as an
error. Only a negative return value should have been treated as an error.
diff --git a/libspread-util/src/events.c b/libspread-util/src/events.c
index 1defbd7..98bcf0a 100644
--- a/libspread-util/src/events.c
+++ b/libspread-util/src/events.c
@@ -812,12 +812,12 @@ int E_attach_fd(int fd, int fd_type,
return -1;
}
- if( priority < 0 || priority > NUM_PRIORITY )
+ if( priority < 0 || priority >= NUM_PRIORITY )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_attach_fd: invalid priority %d for fd %d with fd_type %d\n", priority, fd, fd_type );
return( -1 );
}
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_attach_fd: invalid fd_type %d for fd %d with priority %d\n", fd_type, fd, priority );
return( -1 );
@@ -882,7 +882,7 @@ int E_detach_fd_priority( int fd, int fd_type, int priority )
return found;
}
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_detach_fd: invalid fd_type %d for fd %d\n", fd_type, fd );
return found;
@@ -915,7 +915,7 @@ int E_deactivate_fd( int fd, int fd_type )
return found;
}
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_deactivate_fd: invalid fd_type %d for fd %d\n", fd_type, fd );
return found;
@@ -948,7 +948,7 @@ int E_activate_fd( int fd, int fd_type )
return found;
}
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_activate_fd: invalid fd_type %d for fd %d\n", fd_type, fd );
return found;
@@ -986,7 +986,7 @@ int E_set_active_threshold( int priority )
{
int i;
- if( priority < 0 || priority > NUM_PRIORITY )
+ if( priority < 0 || priority >= NUM_PRIORITY )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_set_active_threshold: invalid priority %d\n", priority );
return( -1 );
@@ -1075,12 +1075,12 @@ int E_attach_fd( int fd, int fd_type,
int num_fds;
int j;
- if( priority < 0 || priority > NUM_PRIORITY )
+ if( priority < 0 || priority >= NUM_PRIORITY )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_attach_fd: invalid priority %d for fd %d with fd_type %d\n", priority, fd, fd_type );
return( -1 );
}
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_attach_fd: invalid fd_type %d for fd %d with priority %d\n", fd_type, fd, priority );
return( -1 );
@@ -1089,7 +1089,7 @@ int E_attach_fd( int fd, int fd_type,
/* Windows bug: Reports FD_SETSIZE of 64 but select works on all
* fd's even ones with numbers greater then 64.
*/
- if( fd < 0 || fd > FD_SETSIZE )
+ if( fd < 0 || fd >= FD_SETSIZE )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_attach_fd: invalid fd %d (max %d) with fd_type %d with priority %d\n", fd, FD_SETSIZE, fd_type, priority );
return( -1 );
@@ -1141,7 +1141,7 @@ int E_detach_fd_priority( int fd, int fd_type, int priority )
int i;
int found;
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_detach_fd: invalid fd_type %d for fd %d\n", fd_type, fd );
return( -1 );
@@ -1173,7 +1173,7 @@ int E_deactivate_fd( int fd, int fd_type )
int i,j;
int found;
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_deactivate_fd: invalid fd_type %d for fd %d\n", fd_type, fd );
return( -1 );
@@ -1205,7 +1205,7 @@ int E_activate_fd( int fd, int fd_type )
int i,j;
int found;
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_activate_fd: invalid fd_type %d for fd %d\n", fd_type, fd );
return( -1 );
@@ -1237,7 +1237,7 @@ int E_set_active_threshold( int priority )
int fd_type;
int i,j;
- if( priority < 0 || priority > NUM_PRIORITY )
+ if( priority < 0 || priority >= NUM_PRIORITY )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_set_active_threshold: invalid priority %d\n", priority );
return( -1 );
@@ -1281,7 +1281,7 @@ int E_detach_fd( int fd, int fd_type )
}
#endif
- if( fd_type < 0 || fd_type > NUM_FDTYPES )
+ if( fd_type < 0 || fd_type >= NUM_FDTYPES )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_detach_fd: invalid fd_type %d for fd %d\n", fd_type, fd );
return found;
@@ -1300,7 +1300,7 @@ int E_detach_fd( int fd, int fd_type )
int E_num_active( int priority )
{
- if( priority < 0 || priority > NUM_PRIORITY )
+ if( priority < 0 || priority >= NUM_PRIORITY )
{
Alarmp( SPLOG_PRINT, EVENTS, "E_num_active: invalid priority %d\n", priority );
return( -1 );
diff --git a/libspread/sp.c b/libspread/sp.c
index 1df4f4e..aa19b76 100644
--- a/libspread/sp.c
+++ b/libspread/sp.c
@@ -1238,7 +1238,7 @@ static int SP_internal_multicast( mailbox mbox, service service_type,
while(((ret=send( mbox, &head_buf[buf_len], sizeof(message_header)+MAX_GROUP_NAME*num_groups - buf_len, 0 )) == -1)
&& ((sock_errno == EINTR) || (sock_errno == EAGAIN) || (sock_errno == EWOULDBLOCK)) )
;
- if( ret <=0 )
+ if( ret < 0 )
{
Alarm( SESSION, "SP_internal_multicast: error %d sending header and groups on mailbox %d: %s \n", ret, mbox, sock_strerror(sock_errno));
Mutex_lock( &Struct_mutex );
_______________________________________________
Spread-users mailing list
Spread-users at lists.spread.org
http://lists.spread.org/mailman/listinfo/spread-users
More information about the Spread-users
mailing list