From be7d7d95850c30a154aaa56e95d6a7f36793409d Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 7 Aug 2019 17:11:51 +0200 Subject: [SERVER] Better lock debugging: Always check lock order Lock order is predefined in locks.h. Immediately bail out if a lock with lower priority is obtained while the same thread already holds one with higher priority. --- src/server/rpc.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'src/server/rpc.c') diff --git a/src/server/rpc.c b/src/server/rpc.c index 5dbcafe..261c6c0 100644 --- a/src/server/rpc.c +++ b/src/server/rpc.c @@ -75,10 +75,9 @@ static json_int_t randomRunId; static pthread_mutex_t aclLock; #define MAX_CLIENTS 50 #define CUTOFF_START 40 -static pthread_mutex_t statusLock; static struct { - int count; - bool overloaded; + atomic_int count; + atomic_bool overloaded; } status; static bool handleStatus(int sock, int permissions, struct field *fields, size_t fields_num, int keepAlive); @@ -91,8 +90,7 @@ static void loadAcl(); void rpc_init() { - mutex_init( &aclLock ); - mutex_init( &statusLock ); + mutex_init( &aclLock, LOCK_RPC_ACL ); randomRunId = (((json_int_t)getpid()) << 16) | (json_int_t)time(NULL); // if ( sizeof(randomRunId) > 4 ) { @@ -123,10 +121,8 @@ void rpc_sendStatsJson(int sock, dnbd3_host_t* host, const void* data, const int return; } do { - mutex_lock( &statusLock ); const int curCount = ++status.count; UPDATE_LOADSTATE( curCount ); - mutex_unlock( &statusLock ); if ( curCount > MAX_CLIENTS ) { sendReply( sock, "503 Service Temporarily Unavailable", "text/plain", "Too many HTTP clients", -1, HTTP_CLOSE ); goto func_return; @@ -198,9 +194,7 @@ void rpc_sendStatsJson(int sock, dnbd3_host_t* host, const void* data, const int if ( minorVersion == 0 || hasHeaderValue( headers, numHeaders, &STR_CONNECTION, &STR_CLOSE ) ) { keepAlive = HTTP_CLOSE; } else { // And if there aren't too many active HTTP sessions - mutex_lock( &statusLock ); if ( status.overloaded ) keepAlive = HTTP_CLOSE; - mutex_unlock( &statusLock ); } } if ( method.s != NULL && path.s != NULL ) { @@ -234,10 +228,8 @@ void rpc_sendStatsJson(int sock, dnbd3_host_t* host, const void* data, const int } while (true); func_return:; do { - mutex_lock( &statusLock ); const int curCount = --status.count; UPDATE_LOADSTATE( curCount ); - mutex_unlock( &statusLock ); } while (0); } -- cgit v1.2.3-55-g7522 From ac1bf45ebdd630fbc9ad2c1fa3c0ea99f5206799 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 28 Aug 2019 13:07:13 +0200 Subject: [SERVER] Make signal handling more POSIX According to POSIX, a signal sent to a PID can be delivered to an arbitrary thread of that process that hasn't the signal blocked. This seens to never happen on Linux, but would mess things up since the code expected the main signal handler to only be executed by the main thread. This should now be fixed by examining the destination PID of the signal as well as the ID of the thread currently running the signal handler. If we notice the signal wasn't sent by our own PID and the handler is not currently run by the main thread, we re-send the signal to the main thread. Otherwise, if the signal was sent by our own PID but the handler is not run in the main thread, do nothing. This way we can use pthread_kill() to wake up threads that might be stuck in a blocking syscall when it's time to shut down. --- src/server/globals.h | 1 + src/server/image.c | 10 ++-------- src/server/integrity.c | 17 +++++++++++++---- src/server/net.c | 11 ++++++----- src/server/rpc.c | 13 ++++++++----- src/server/server.c | 22 +++++++++++++++++----- src/server/threadpool.c | 28 ++++++++++++++++++++++------ src/server/threadpool.h | 5 +++++ 8 files changed, 74 insertions(+), 33 deletions(-) (limited to 'src/server/rpc.c') diff --git a/src/server/globals.h b/src/server/globals.h index 5dd205a..f940666 100644 --- a/src/server/globals.h +++ b/src/server/globals.h @@ -138,6 +138,7 @@ struct _dnbd3_client char hostName[HOSTNAMELEN]; // inet_ntop version of host pthread_mutex_t sendMutex; // Held while writing to sock if image is incomplete (since uplink uses socket too) pthread_mutex_t lock; + pthread_t thread; }; // ####################################################### diff --git a/src/server/image.c b/src/server/image.c index de93cd4..248c12c 100644 --- a/src/server/image.c +++ b/src/server/image.c @@ -562,9 +562,7 @@ bool image_tryFreeAll() if ( _images[i] != NULL && _images[i]->users == 0 ) { dnbd3_image_t *image = _images[i]; _images[i] = NULL; - mutex_unlock( &imageListLock ); image = image_free( image ); - mutex_lock( &imageListLock ); } if ( i + 1 == _num_images && _images[i] == NULL ) _num_images--; } @@ -574,15 +572,13 @@ bool image_tryFreeAll() /** * Free image. DOES NOT check if it's in use. - * Indirectly locks on imageListLock, image.lock, uplink.queueLock + * (Indirectly) locks on image.lock, uplink.queueLock */ static dnbd3_image_t* image_free(dnbd3_image_t *image) { assert( image != NULL ); assert( image->users == 0 ); - if ( !_shutdown ) { - logadd( LOG_INFO, "Freeing image %s:%d", image->name, (int)image->rid ); - } + logadd( ( _shutdown ? LOG_DEBUG1 : LOG_INFO ), "Freeing image %s:%d", image->name, (int)image->rid ); // uplink_shutdown might return false to tell us // that the shutdown is in progress. Bail out since // this will get called again when the uplink is done. @@ -600,8 +596,6 @@ static dnbd3_image_t* image_free(dnbd3_image_t *image) mutex_unlock( &image->lock ); if ( image->readFd != -1 ) close( image->readFd ); mutex_destroy( &image->lock ); - // - memset( image, 0, sizeof(*image) ); free( image ); return NULL ; } diff --git a/src/server/integrity.c b/src/server/integrity.c index f358c46..e7ebeb2 100644 --- a/src/server/integrity.c +++ b/src/server/integrity.c @@ -184,13 +184,20 @@ static void* integrity_main(void * data UNUSED) mutex_unlock( &image->lock ); } #if defined(linux) || defined(__linux) - if ( sync_file_range( fd, start, end - start, SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE | SYNC_FILE_RANGE_WAIT_AFTER ) == -1 ) { + while ( sync_file_range( fd, start, end - start, SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE | SYNC_FILE_RANGE_WAIT_AFTER ) == -1 ) #else - if ( fsync( fd ) == -1 ) { + while ( fsync( fd ) == -1 ) #endif - logadd( LOG_ERROR, "Cannot flush %s for integrity check", image->path ); + { + if ( _shutdown ) + break; + if ( errno == EINTR ) + continue; + logadd( LOG_ERROR, "Cannot flush %s for integrity check (errno=%d)", image->path, errno ); exit( 1 ); } + if ( _shutdown ) + break; // Use direct I/O only if read length is multiple of 4096 to be on the safe side int tfd; if ( direct && ( end % DNBD3_BLOCK_SIZE ) == 0 ) { @@ -266,7 +273,9 @@ static void* integrity_main(void * data UNUSED) } } mutex_unlock( &integrityQueueLock ); - if ( buffer != NULL ) free( buffer ); + if ( buffer != NULL ) { + free( buffer ); + } bRunning = false; return NULL; } diff --git a/src/server/net.c b/src/server/net.c index e0b516e..9c855e4 100644 --- a/src/server/net.c +++ b/src/server/net.c @@ -44,6 +44,7 @@ #include #include #include +#include static dnbd3_client_t *_clients[SERVER_MAX_CLIENTS]; static int _num_clients = 0; @@ -153,6 +154,7 @@ void* net_handleNewConnection(void *clientPtr) { dnbd3_client_t * const client = (dnbd3_client_t *)clientPtr; dnbd3_request_t request; + client->thread = pthread_self(); // Await data from client. Since this is a fresh connection, we expect data right away sock_setTimeout( client->sock, _clientTimeout ); @@ -631,11 +633,10 @@ void net_disconnectAll() int i; mutex_lock( &_clients_lock ); for (i = 0; i < _num_clients; ++i) { - if ( _clients[i] == NULL ) continue; - dnbd3_client_t * const client = _clients[i]; - mutex_lock( &client->lock ); - if ( client->sock >= 0 ) shutdown( client->sock, SHUT_RDWR ); - mutex_unlock( &client->lock ); + if ( _clients[i] == NULL ) + continue; + shutdown( _clients[i]->sock, SHUT_RDWR ); + pthread_kill( _clients[i]->thread, SIGINT ); } mutex_unlock( &_clients_lock ); } diff --git a/src/server/rpc.c b/src/server/rpc.c index 261c6c0..662263e 100644 --- a/src/server/rpc.c +++ b/src/server/rpc.c @@ -137,13 +137,13 @@ void rpc_sendStatsJson(int sock, dnbd3_host_t* host, const void* data, const int bool hasName = false; bool ok; int keepAlive = HTTP_KEEPALIVE; - do { + while ( !_shutdown ) { // Read request from client struct phr_header headers[100]; size_t numHeaders, prevLen = 0, consumed; struct string method, path; int minorVersion; - do { + while ( !_shutdown ) { // Parse before calling recv, there might be a complete pipelined request in the buffer already // If the request is incomplete, we allow exactly one additional recv() to complete it. // This should suffice for real world scenarios as I don't know of any @@ -188,7 +188,9 @@ void rpc_sendStatsJson(int sock, dnbd3_host_t* host, const void* data, const int sendReply( sock, "400 Bad Request", "text/plain", "Server cannot understand what you're trying to say", -1, HTTP_CLOSE ); goto func_return; } - } while ( true ); + } // Loop while request header incomplete + if ( _shutdown ) + break; if ( keepAlive == HTTP_KEEPALIVE ) { // Only keep the connection alive (and indicate so) if the client seems to support this if ( minorVersion == 0 || hasHeaderValue( headers, numHeaders, &STR_CONNECTION, &STR_CLOSE ) ) { @@ -213,7 +215,8 @@ void rpc_sendStatsJson(int sock, dnbd3_host_t* host, const void* data, const int } else { ok = sendReply( sock, "404 Not found", "text/plain", "Nothing", -1, keepAlive ); } - if ( !ok ) break; + if ( !ok ) + break; } // hoff might be beyond end if the client sent another request (burst) const ssize_t extra = hoff - consumed; @@ -225,7 +228,7 @@ void rpc_sendStatsJson(int sock, dnbd3_host_t* host, const void* data, const int hasName = true; setThreadName( "HTTP" ); } - } while (true); + } // Loop while more requests func_return:; do { const int curCount = --status.count; diff --git a/src/server/server.c b/src/server/server.c index 1cdd2ab..0dddea7 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -37,6 +37,8 @@ #include #include #include +#include +#include #define LONGOPT_CRC4 1000 #define LONGOPT_ASSERT 1001 @@ -60,6 +62,7 @@ static _Atomic(job_t *) newJob; static bool hasTimerThread = false; static pthread_t timerThread; +static pid_t mainPid; static pthread_t mainThread; #define DEFAULT_TIMER_TIMEOUT (60) @@ -138,7 +141,7 @@ _Noreturn static void dnbd3_cleanup() logadd( LOG_INFO, "Cleanup..." ); if ( hasTimerThread ) { - pthread_kill( timerThread, SIGHUP ); + pthread_kill( timerThread, SIGINT ); thread_join( timerThread, NULL ); } @@ -162,6 +165,8 @@ _Noreturn static void dnbd3_cleanup() // Wait for clients to disconnect net_waitForAllDisconnected(); + threadpool_waitEmpty(); + // Clean up images retries = 5; while ( !image_tryFreeAll() && --retries > 0 ) { @@ -204,6 +209,7 @@ int main(int argc, char *argv[]) { 0, 0, 0, 0 } }; + mainPid = getpid(); mainThread = pthread_self(); opt = getopt_long( argc, argv, optString, longOpts, &longIndex ); @@ -509,10 +515,16 @@ static void dnbd3_handleSignal(int signum) static void dnbd3_handleSignal2(int signum, siginfo_t *info, void *data UNUSED) { - if ( !pthread_equal( pthread_self(), mainThread ) ) - return; - memcpy( &lastSignal, info, sizeof(siginfo_t) ); - dnbd3_handleSignal( signum ); + if ( info->si_pid != mainPid ) { // Source is not this process + memcpy( &lastSignal, info, sizeof(siginfo_t) ); // Copy signal info + if ( info->si_pid != 0 && !pthread_equal( pthread_self(), mainThread ) ) { + pthread_kill( mainThread, info->si_signo ); // And relay signal if we're not the main thread + } + } + if ( pthread_equal( pthread_self(), mainThread ) ) { + // Signal received by main thread -- handle + dnbd3_handleSignal( signum ); + } } uint32_t dnbd3_serverUptime() diff --git a/src/server/threadpool.c b/src/server/threadpool.c index 3947677..0b46fd6 100644 --- a/src/server/threadpool.c +++ b/src/server/threadpool.c @@ -15,6 +15,7 @@ static void *threadpool_worker(void *entryPtr); static pthread_attr_t threadAttrs; static atomic_int maxIdleThreads = -1; static _Atomic(entry_t *) *pool = NULL; +static atomic_int activeThreads = 0; bool threadpool_init(int maxIdle) { @@ -34,10 +35,9 @@ bool threadpool_init(int maxIdle) void threadpool_close() { - _shutdown = true; - int max = maxIdleThreads; - maxIdleThreads = -1; - if ( max <= 0 ) return; + int max = atomic_exchange( &maxIdleThreads, -1 ); + if ( max <= 0 ) + return; for ( int i = 0; i < max; ++i ) { entry_t *cur = pool[i]; if ( cur != NULL && atomic_compare_exchange_strong( &pool[i], &cur, NULL ) ) { @@ -46,9 +46,23 @@ void threadpool_close() } } +void threadpool_waitEmpty() +{ + if ( activeThreads == 0 ) + return; + do { + sleep( 1 ); + logadd( LOG_INFO, "Threadpool: %d threads still active", (int)activeThreads ); + } while ( activeThreads != 0 ); +} + bool threadpool_run(void *(*startRoutine)(void *), void *arg) { - if ( startRoutine == NULL ) { + if ( unlikely( _shutdown ) ) { + logadd( LOG_MINOR, "Cannot submit work to threadpool while shutting down!" ); + return false; + } + if ( unlikely( startRoutine == NULL ) ) { logadd( LOG_ERROR, "Trying to queue work for thread pool with NULL startRoutine" ); return false; // Or bail out!? } @@ -60,7 +74,7 @@ bool threadpool_run(void *(*startRoutine)(void *), void *arg) break; } } - if ( entry == NULL ) { + if ( unlikely( entry == NULL ) ) { entry = malloc( sizeof(entry_t) ); if ( entry == NULL ) { logadd( LOG_WARNING, "Could not alloc entry_t for new thread\n" ); @@ -78,6 +92,7 @@ bool threadpool_run(void *(*startRoutine)(void *), void *arg) free( entry ); return false; } + activeThreads++; } entry->startRoutine = startRoutine; entry->arg = arg; @@ -130,6 +145,7 @@ keep_going:; } signal_close( entry->signal ); free( entry ); + activeThreads--; return NULL; } diff --git a/src/server/threadpool.h b/src/server/threadpool.h index 15dd151..ee0b3aa 100644 --- a/src/server/threadpool.h +++ b/src/server/threadpool.h @@ -17,6 +17,11 @@ bool threadpool_init(int maxIdleThreadCount); */ void threadpool_close(); +/** + * Block until all threads spawned have exited + */ +void threadpool_waitEmpty(); + /** * Run a thread using the thread pool. * @param startRoutine function to run in new thread -- cgit v1.2.3-55-g7522 From 701e5a967fd6bc97644f39e6fea3714f49a90291 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Fri, 6 Sep 2019 17:32:58 +0200 Subject: [SERVER] rpc: Add cachemap feature --- src/server/globals.h | 2 +- src/server/image.c | 16 ++++++++++++++++ src/server/image.h | 2 ++ src/server/rpc.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 1 deletion(-) (limited to 'src/server/rpc.c') diff --git a/src/server/globals.h b/src/server/globals.h index 58b2c9d..df8c595 100644 --- a/src/server/globals.h +++ b/src/server/globals.h @@ -110,7 +110,7 @@ typedef struct typedef struct { ref reference; - atomic_uint_least8_t map[]; + _Atomic uint8_t map[]; } dnbd3_cache_map_t; /** diff --git a/src/server/image.c b/src/server/image.c index 9fcb866..5fa06d8 100644 --- a/src/server/image.c +++ b/src/server/image.c @@ -274,6 +274,22 @@ bool image_ensureOpen(dnbd3_image_t *image) return image->readFd != -1; } +dnbd3_image_t* image_byId(int imgId) +{ + int i; + mutex_lock( &imageListLock ); + for (i = 0; i < _num_images; ++i) { + dnbd3_image_t * const image = _images[i]; + if ( image != NULL && image->id == imgId ) { + image->users++; + mutex_unlock( &imageListLock ); + return image; + } + } + mutex_unlock( &imageListLock ); + return NULL; +} + /** * Get an image by name+rid. This function increases a reference counter, * so you HAVE TO CALL image_release for every image_get() call at some diff --git a/src/server/image.h b/src/server/image.h index cd87f03..449e31f 100644 --- a/src/server/image.h +++ b/src/server/image.h @@ -17,6 +17,8 @@ void image_markComplete(dnbd3_image_t *image); bool image_ensureOpen(dnbd3_image_t *image); +dnbd3_image_t* image_byId(int imgId); + dnbd3_image_t* image_get(char *name, uint16_t revision, bool checkIfWorking); bool image_reopenCacheFd(dnbd3_image_t *image, const bool force); diff --git a/src/server/rpc.c b/src/server/rpc.c index 662263e..548c80f 100644 --- a/src/server/rpc.c +++ b/src/server/rpc.c @@ -9,6 +9,7 @@ #include "fileutil.h" #include "picohttpparser/picohttpparser.h" #include "urldecode.h" +#include "reference.h" #include #include @@ -43,7 +44,9 @@ _Static_assert( sizeof("test") == 5 && sizeof("test2") == 6, "Stringsize messup DEFSTR(STR_CONNECTION, "connection") DEFSTR(STR_CLOSE, "close") DEFSTR(STR_QUERY, "/query") +DEFSTR(STR_CACHEMAP, "/cachemap") DEFSTR(STR_Q, "q") +DEFSTR(STR_ID, "id") static inline bool equals(struct string *s1,struct string *s2) { @@ -81,6 +84,7 @@ static struct { } status; static bool handleStatus(int sock, int permissions, struct field *fields, size_t fields_num, int keepAlive); +static bool handleCacheMap(int sock, int permissions, struct field *fields, size_t fields_num, int keepAlive); static bool sendReply(int sock, const char *status, const char *ctype, const char *payload, ssize_t plen, int keepAlive); static void parsePath(struct string *path, struct string *file, struct field *getv, size_t *getc); static bool hasHeaderValue(struct phr_header *headers, size_t numHeaders, struct string *name, struct string *value); @@ -212,6 +216,8 @@ void rpc_sendStatsJson(int sock, dnbd3_host_t* host, const void* data, const int // Don't care if GET or POST if ( equals( &file, &STR_QUERY ) ) { ok = handleStatus( sock, permissions, getv, getc, keepAlive ); + } else if ( equals( &file, &STR_CACHEMAP ) ) { + ok = handleCacheMap( sock, permissions, getv, getc, keepAlive ); } else { ok = sendReply( sock, "404 Not found", "text/plain", "Nothing", -1, keepAlive ); } @@ -342,6 +348,44 @@ static bool handleStatus(int sock, int permissions, struct field *fields, size_t return ok; } +static bool handleCacheMap(int sock, int permissions, struct field *fields, size_t fields_num, int keepAlive) +{ + if ( !(permissions & ACL_IMAGE_LIST) ) { + return sendReply( sock, "403 Forbidden", "text/plain", "No permission to access image list", -1, keepAlive ); + } + int imgId = -1; + static const char one = 0xff; + for (size_t i = 0; i < fields_num; ++i) { + if ( equals( &fields[i].name, &STR_ID ) ) { + char *broken; + imgId = strtol( fields[i].value.s, &broken, 10 ); + if ( broken != fields[i].value.s ) + break; + imgId = -1; + } + } + if ( imgId == -1 ) + return sendReply( sock, "400 Bad Request", "text/plain", "Missing parameter 'id'", -1, keepAlive ); + dnbd3_image_t *image = image_byId( imgId ); + if ( image == NULL ) + return sendReply( sock, "404 Not found", "text/plain", "Image not found", -1, keepAlive ); + dnbd3_cache_map_t *cache = ref_get_cachemap( image ); + image_release( image ); + int len; + const char *map; + if ( cache == NULL ) { + map = &one; + len = 1; + } else { + _Static_assert( sizeof(const char) == sizeof(_Atomic uint8_t), "Atomic assumption exploded" ); + map = (const char*)cache->map; + len = IMGSIZE_TO_MAPBYTES( image->virtualFilesize ); + } + bool ok = sendReply( sock, "200 OK", "application/octet-stream", map, len, keepAlive ); + ref_put( &cache->reference ); + return ok; +} + static bool sendReply(int sock, const char *status, const char *ctype, const char *payload, ssize_t plen, int keepAlive) { if ( plen == -1 ) plen = strlen( payload ); -- cgit v1.2.3-55-g7522 From f6d5dad8cd50390bd25b22d70871a89b6d7af268 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 11 Sep 2019 22:15:22 +0200 Subject: [SERVER] rpc: Fix warnings --- src/server/rpc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/server/rpc.c') diff --git a/src/server/rpc.c b/src/server/rpc.c index 548c80f..5daf20c 100644 --- a/src/server/rpc.c +++ b/src/server/rpc.c @@ -354,11 +354,11 @@ static bool handleCacheMap(int sock, int permissions, struct field *fields, size return sendReply( sock, "403 Forbidden", "text/plain", "No permission to access image list", -1, keepAlive ); } int imgId = -1; - static const char one = 0xff; + static const char one = (char)0xff; for (size_t i = 0; i < fields_num; ++i) { if ( equals( &fields[i].name, &STR_ID ) ) { char *broken; - imgId = strtol( fields[i].value.s, &broken, 10 ); + imgId = (int)strtol( fields[i].value.s, &broken, 10 ); if ( broken != fields[i].value.s ) break; imgId = -1; -- cgit v1.2.3-55-g7522 From f700b99a36c8094af0e311b23a2f725120f180ac Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 30 Oct 2019 12:30:48 +0100 Subject: [SERVER] Fix another nullpointer access --- src/server/rpc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/server/rpc.c') diff --git a/src/server/rpc.c b/src/server/rpc.c index 5daf20c..a454d6d 100644 --- a/src/server/rpc.c +++ b/src/server/rpc.c @@ -382,7 +382,9 @@ static bool handleCacheMap(int sock, int permissions, struct field *fields, size len = IMGSIZE_TO_MAPBYTES( image->virtualFilesize ); } bool ok = sendReply( sock, "200 OK", "application/octet-stream", map, len, keepAlive ); - ref_put( &cache->reference ); + if ( cache != NULL ) { + ref_put( &cache->reference ); + } return ok; } -- cgit v1.2.3-55-g7522 From ff4e770e645c05da48baddb30a77b9dc15ca76fd Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Fri, 6 Mar 2020 15:00:46 +0100 Subject: [SERVER] Handle "warn unused result" cases --- src/server/fileutil.c | 2 +- src/server/globals.c | 5 ++++- src/server/image.c | 8 ++++++-- src/server/rpc.c | 2 +- src/server/server.c | 5 ++++- src/server/uplink.c | 14 ++++++++++---- 6 files changed, 26 insertions(+), 10 deletions(-) (limited to 'src/server/rpc.c') diff --git a/src/server/fileutil.c b/src/server/fileutil.c index 336ab68..9a9f066 100644 --- a/src/server/fileutil.c +++ b/src/server/fileutil.c @@ -68,7 +68,7 @@ bool file_setSize(int fd, uint64_t size) // Try really hard... image loading logic relies on the file // having the proper apparent size uint8_t byte = 0; - pread( fd, &byte, 1, size - 1 ); + (void)!pread( fd, &byte, 1, size - 1 ); if ( pwrite( fd, &byte, 1, size - 1 ) == 1 ) return true; return false; } diff --git a/src/server/globals.c b/src/server/globals.c index 2e87400..ac079b1 100644 --- a/src/server/globals.c +++ b/src/server/globals.c @@ -113,7 +113,10 @@ static int ini_handler(void *custom UNUSED, const char* section, const char* key void globals_loadConfig() { char *name = NULL; - asprintf( &name, "%s/%s", _configDir, CONFIG_FILENAME ); + if ( asprintf( &name, "%s/%s", _configDir, CONFIG_FILENAME ) == -1 ) { + logadd( LOG_ERROR, "Memory allocation error for config filename" ); + exit( 1 ); + } if ( name == NULL ) return; if ( initialLoad ) { mutex_init( &loadLock, LOCK_LOAD_CONFIG ); diff --git a/src/server/image.c b/src/server/image.c index 7ffe041..32c9efe 100644 --- a/src/server/image.c +++ b/src/server/image.c @@ -1446,9 +1446,13 @@ static bool image_clone(int sock, char *name, uint16_t revision, uint64_t imageS logadd( LOG_WARNING, "OTF-Clone: Corrupted CRC-32 list. ignored. (%s)", name ); } else { int fd = open( crcFile, O_WRONLY | O_CREAT, 0644 ); - write( fd, &masterCrc, sizeof(uint32_t) ); - write( fd, crc32list, crc32len ); + ssize_t ret = write( fd, &masterCrc, sizeof(masterCrc) ); + ret += write( fd, crc32list, crc32len ); close( fd ); + if ( (size_t)ret != crc32len + sizeof(masterCrc) ) { + logadd( LOG_WARNING, "Could not save freshly received crc32 list for %s:%d", name, (int)revision ); + unlink( crcFile ); + } } } free( crc32list ); diff --git a/src/server/rpc.c b/src/server/rpc.c index a454d6d..b66b8fe 100644 --- a/src/server/rpc.c +++ b/src/server/rpc.c @@ -101,7 +101,7 @@ void rpc_init() int fd = open( "/dev/urandom", O_RDONLY ); if ( fd != -1 ) { uint32_t bla = 1; - read( fd, &bla, 4 ); + (void)!read( fd, &bla, 4 ); randomRunId = (randomRunId << 32) | bla; } close( fd ); diff --git a/src/server/server.c b/src/server/server.c index 0dddea7..c9edc05 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -315,7 +315,10 @@ int main(int argc, char *argv[]) // No one-shot detected, normal server operation or errormsg serving if ( demonize ) { logadd( LOG_INFO, "Forking into background, see log file for further information" ); - daemon( 1, 0 ); + if ( daemon( 0, 0 ) == -1 ) { + logadd( LOG_ERROR, "Could not daemon(): errno=%d", errno ); + exit( 1 ); + } } if ( errorMsg != NULL ) { setupNetwork( bindAddress ); diff --git a/src/server/uplink.c b/src/server/uplink.c index e644e56..71d9f94 100644 --- a/src/server/uplink.c +++ b/src/server/uplink.c @@ -1098,7 +1098,8 @@ static void uplink_addCrc32(dnbd3_uplink_t *uplink) lists_crc = crc32( lists_crc, (uint8_t*)buffer, bytes ); lists_crc = net_order_32( lists_crc ); if ( lists_crc != masterCrc ) { - logadd( LOG_WARNING, "Received corrupted crc32 list from uplink server (%s)!", uplink->image->name ); + logadd( LOG_WARNING, "Received corrupted crc32 list from uplink server (%s:%d)!", + uplink->image->name, (int)uplink->image->rid ); free( buffer ); return; } @@ -1108,10 +1109,15 @@ static void uplink_addCrc32(dnbd3_uplink_t *uplink) char path[len]; snprintf( path, len, "%s.crc", uplink->image->path ); const int fd = open( path, O_WRONLY | O_CREAT, 0644 ); - if ( fd >= 0 ) { - write( fd, &masterCrc, sizeof(uint32_t) ); - write( fd, buffer, bytes ); + if ( fd != -1 ) { + ssize_t ret = write( fd, &masterCrc, sizeof(masterCrc) ); + ret += write( fd, buffer, bytes ); close( fd ); + if ( (size_t)ret != sizeof(masterCrc) + bytes ) { + unlink( path ); + logadd( LOG_WARNING, "Could not write crc32 file for %s:%d", + uplink->image->name, (int)uplink->image->rid ); + } } } -- cgit v1.2.3-55-g7522 From 2e70a0836173c9502ff5cddd849165d432a883cb Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Tue, 17 Mar 2020 13:01:37 +0100 Subject: [SERVER] Include build type and version in RPC Added new query type q=version, which uses the STATS access rights. --- CMakeLists.txt | 7 +++---- get-version.sh | 2 +- src/server/rpc.c | 10 ++++++++-- src/server/server.c | 7 +++++-- src/types.h | 3 +++ src/version.h | 4 ---- 6 files changed, 20 insertions(+), 13 deletions(-) (limited to 'src/server/rpc.c') diff --git a/CMakeLists.txt b/CMakeLists.txt index cc8bfb7..b263f77 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -71,6 +71,7 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/") ADD_DEFINITIONS(-D_FILE_OFFSET_BITS=64) ADD_DEFINITIONS(-DWITH_IPV6) +ADD_DEFINITIONS(-DBUILD_TYPE=${CMAKE_BUILD_TYPE}) FIND_PACKAGE(Threads) @@ -133,14 +134,12 @@ ADD_CUSTOM_TARGET( -P ${CMAKE_BINARY_DIR}/version.cmake ) -INCLUDE_DIRECTORIES( ${CMAKE_BINARY_DIR}/generated ) - ################################################################################ # CLIENT # ################################################################################ if(BUILD_KERNEL_MODULE) - FILE(GLOB_RECURSE CLIENT_SRCS src/client/*.c) + FILE(GLOB_RECURSE CLIENT_SRCS ${CMAKE_BINARY_DIR}/generated/version.c src/client/*.c) ADD_EXECUTABLE(dnbd3-client ${CLIENT_SRCS}) TARGET_LINK_LIBRARIES(dnbd3-client) ADD_DEPENDENCIES(dnbd3-client version) @@ -157,7 +156,7 @@ if(BUILD_SERVER) message(" ######################## Building server for AFL mode - will be useless otherwise!") ADD_DEFINITIONS(-DAFL_MODE) ENDIF() - FILE(GLOB SERVER_SRCS src/server/*.c src/shared/*.c src/server/picohttpparser/*.c) + FILE(GLOB SERVER_SRCS ${CMAKE_BINARY_DIR}/generated/version.c src/server/*.c src/shared/*.c src/server/picohttpparser/*.c) ADD_EXECUTABLE(dnbd3-server ${SERVER_SRCS}) TARGET_INCLUDE_DIRECTORIES(dnbd3-server PRIVATE ${JANSSON_INCLUDE_DIR}) TARGET_LINK_LIBRARIES(dnbd3-server ${CMAKE_THREAD_LIBS_INIT} ${JANSSON_LIBRARIES}) diff --git a/get-version.sh b/get-version.sh index 1d4a8cb..5e5b3e1 100755 --- a/get-version.sh +++ b/get-version.sh @@ -8,7 +8,7 @@ ROOT_DIR="$(dirname "${SELF}")" cd "$ROOT_DIR" if [ -d .git ]; then - [ -n "$(git diff)" ] && MODDED='+MOD' + [ -n "$(git diff HEAD)" ] && MODDED='+MOD' echo $(git describe)$MODDED, branch $(git rev-parse --abbrev-ref HEAD), built "$(date +%Y-%m-%d)" exit 0 fi diff --git a/src/server/rpc.c b/src/server/rpc.c index b66b8fe..12ad0dd 100644 --- a/src/server/rpc.c +++ b/src/server/rpc.c @@ -6,6 +6,7 @@ #include "image.h" #include "altservers.h" #include "../shared/sockhelper.h" +#include "../version.h" #include "fileutil.h" #include "picohttpparser/picohttpparser.h" #include "urldecode.h" @@ -259,7 +260,7 @@ static bool handleStatus(int sock, int permissions, struct field *fields, size_t { bool ok; bool stats = false, images = false, clients = false, space = false; - bool logfile = false, config = false, altservers = false; + bool logfile = false, config = false, altservers = false, version = false; #define SETVAR(var) if ( !var && STRCMP(fields[i].value, #var) ) var = true for (size_t i = 0; i < fields_num; ++i) { if ( !equals( &fields[i].name, &STR_Q ) ) continue; @@ -270,9 +271,10 @@ static bool handleStatus(int sock, int permissions, struct field *fields, size_t else SETVAR(logfile); else SETVAR(config); else SETVAR(altservers); + else SETVAR(version); } #undef SETVAR - if ( ( stats || space ) && !(permissions & ACL_STATS) ) { + if ( ( stats || space || version ) && !(permissions & ACL_STATS) ) { return sendReply( sock, "403 Forbidden", "text/plain", "No permission to access statistics", -1, keepAlive ); } if ( images && !(permissions & ACL_IMAGE_LIST) ) { @@ -308,6 +310,10 @@ static bool handleStatus(int sock, int permissions, struct field *fields, size_t statisticsJson = json_pack( "{sI}", "runId", randomRunId ); } + if ( version ) { + json_object_set_new( statisticsJson, "version", json_string( VERSION_STRING ) ); + json_object_set_new( statisticsJson, "build", json_string( TOSTRING( BUILD_TYPE ) ) ); + } if ( space ) { uint64_t spaceTotal = 0, spaceAvail = 0; file_freeDiskSpace( _basePath, &spaceTotal, &spaceAvail ); diff --git a/src/server/server.c b/src/server/server.c index c9edc05..71a49b9 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -342,7 +342,10 @@ int main(int argc, char *argv[]) net_init(); uplink_globalsInit(); rpc_init(); - logadd( LOG_INFO, "DNBD3 server starting.... Machine type: " ENDIAN_MODE ); + logadd( LOG_INFO, "DNBD3 server starting...." ); + logadd( LOG_INFO, "Machine type: " ENDIAN_MODE ); + logadd( LOG_INFO, "Build Type: " TOSTRING( BUILD_TYPE ) ); + logadd( LOG_INFO, "Version: %s", VERSION_STRING ); if ( altservers_load() < 0 ) { logadd( LOG_WARNING, "Could not load alt-servers. Does the file exist in %s?", _configDir ); @@ -385,7 +388,7 @@ int main(int argc, char *argv[]) exit( EXIT_FAILURE ); } - logadd( LOG_INFO, "Server is ready. (%s)", VERSION_STRING ); + logadd( LOG_INFO, "Server is ready." ); if ( thread_create( &timerThread, NULL, &timerMainloop, NULL ) == 0 ) { hasTimerThread = true; diff --git a/src/types.h b/src/types.h index cb0ccfd..dc8e501 100644 --- a/src/types.h +++ b/src/types.h @@ -34,6 +34,9 @@ #define MAX(a,b) ((a) > (b) ? (a) : (b)) #endif +#define STRINGIFY(x) #x +#define TOSTRING(x) STRINGIFY(x) + #ifdef __GNUC__ #define UNUSED __attribute__ ((unused)) #else diff --git a/src/version.h b/src/version.h index 0c4a66b..1c17442 100644 --- a/src/version.h +++ b/src/version.h @@ -23,8 +23,4 @@ extern const char *VERSION_STRING; -// This is done in a little weird way but otherwise eclipse complains about -// unresolvable symbols etc... -#include "version.c" - #endif /* VERSION_H_ */ -- cgit v1.2.3-55-g7522 From 878a2414b4ed93461bb5b2be7dca026cdc56b43b Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Thu, 19 Mar 2020 13:36:23 +0100 Subject: [SERVER] Shorter wait when closing socket after reply --- src/server/rpc.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/server/rpc.c') diff --git a/src/server/rpc.c b/src/server/rpc.c index 12ad0dd..0002661 100644 --- a/src/server/rpc.c +++ b/src/server/rpc.c @@ -414,6 +414,8 @@ static bool sendReply(int sock, const char *status, const char *ctype, const cha #ifdef AFL_MODE sock = 0; #endif + // Don't wait too long in case other side ignores the shutdown + sock_setTimeout( sock, 600 ); while ( read( sock, buffer, sizeof buffer ) > 0 ); return false; } -- cgit v1.2.3-55-g7522