You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2017/04/25 22:24:59 UTC

[1/6] qpid-dispatch git commit: DISPATCH-390: refactor - amqp.h constants standard conditions

Repository: qpid-dispatch
Updated Branches:
  refs/heads/master 5e61dd9a9 -> a41a5c8fc


DISPATCH-390: refactor - amqp.h constants standard conditions


Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/a41a5c8f
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/a41a5c8f
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/a41a5c8f

Branch: refs/heads/master
Commit: a41a5c8fcef66e5be5b49235a0a439f3b9001f5e
Parents: 390ebe7
Author: Alan Conway <ac...@redhat.com>
Authored: Thu Mar 30 19:04:21 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Tue Apr 25 18:13:59 2017 -0400

----------------------------------------------------------------------
 src/container.c | 9 +++++----
 src/policy.c    | 9 +++------
 2 files changed, 8 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a41a5c8f/src/container.c
----------------------------------------------------------------------
diff --git a/src/container.c b/src/container.c
index b86bbef..1fb83a6 100644
--- a/src/container.c
+++ b/src/container.c
@@ -28,6 +28,7 @@
 #include <proton/message.h>
 #include <proton/connection.h>
 #include <proton/event.h>
+#include <qpid/dispatch/amqp.h>
 #include <qpid/dispatch/ctools.h>
 #include <qpid/dispatch/hash.h>
 #include <qpid/dispatch/threading.h>
@@ -91,7 +92,7 @@ static void setup_outgoing_link(qd_container_t *container, pn_link_t *pn_link)
 
     if (node == 0) {
         pn_condition_t *cond = pn_link_condition(pn_link);
-        pn_condition_set_name(cond, "amqp:not-found");
+        pn_condition_set_name(cond, QD_AMQP_COND_NOT_FOUND);
         pn_condition_set_description(cond, "Source node does not exist");
         pn_link_close(pn_link);
         return;
@@ -100,7 +101,7 @@ static void setup_outgoing_link(qd_container_t *container, pn_link_t *pn_link)
     qd_link_t *link = new_qd_link_t();
     if (!link) {
         pn_condition_t *cond = pn_link_condition(pn_link);
-        pn_condition_set_name(cond, "amqp:internal-error");
+        pn_condition_set_name(cond, QD_AMQP_COND_INTERNAL_ERROR);
         pn_condition_set_description(cond, "Insufficient memory");
         pn_link_close(pn_link);
         return;
@@ -127,7 +128,7 @@ static void setup_incoming_link(qd_container_t *container, pn_link_t *pn_link)
 
     if (node == 0) {
         pn_condition_t *cond = pn_link_condition(pn_link);
-        pn_condition_set_name(cond, "amqp:not-found");
+        pn_condition_set_name(cond, QD_AMQP_COND_NOT_FOUND);
         pn_condition_set_description(cond, "Target node does not exist");
         pn_link_close(pn_link);
         return;
@@ -136,7 +137,7 @@ static void setup_incoming_link(qd_container_t *container, pn_link_t *pn_link)
     qd_link_t *link = new_qd_link_t();
     if (!link) {
         pn_condition_t *cond = pn_link_condition(pn_link);
-        pn_condition_set_name(cond, "amqp:internal-error");
+        pn_condition_set_name(cond, QD_AMQP_COND_INTERNAL_ERROR);
         pn_condition_set_description(cond, "Insufficient memory");
         pn_link_close(pn_link);
         return;

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/a41a5c8f/src/policy.c
----------------------------------------------------------------------
diff --git a/src/policy.c b/src/policy.c
index 182fc61..39ead3c 100644
--- a/src/policy.c
+++ b/src/policy.c
@@ -45,9 +45,6 @@ static int n_processed = 0;
 //
 // error conditions signaled to effect denial
 //
-static char* RESOURCE_LIMIT_EXCEEDED     = "amqp:resource-limit-exceeded";
-//static char* UNAUTHORIZED_ACCESS         = "amqp:unauthorized-access";
-//static char* CONNECTION_FORCED           = "amqp:connection:forced";
 
 //
 // error descriptions signaled to effect denial
@@ -379,7 +376,7 @@ void qd_policy_private_deny_amqp_connection(pn_connection_t *conn, const char *c
 void qd_policy_deny_amqp_session(pn_session_t *ssn, qd_connection_t *qd_conn)
 {
     pn_condition_t * cond = pn_session_condition(ssn);
-    (void) pn_condition_set_name(       cond, RESOURCE_LIMIT_EXCEEDED);
+    (void) pn_condition_set_name(       cond, QD_AMQP_COND_RESOURCE_LIMIT_EXCEEDED);
     (void) pn_condition_set_description(cond, SESSION_DISALLOWED);
     pn_session_close(ssn);
     qd_conn->policy_settings->denialCounts->sessionDenied++;
@@ -438,7 +435,7 @@ void qd_policy_apply_session_settings(pn_session_t *ssn, qd_connection_t *qd_con
 void _qd_policy_deny_amqp_link(pn_link_t *link, qd_connection_t *qd_conn)
 {
     pn_condition_t * cond = pn_link_condition(link);
-    (void) pn_condition_set_name(       cond, RESOURCE_LIMIT_EXCEEDED);
+    (void) pn_condition_set_name(       cond, QD_AMQP_COND_RESOURCE_LIMIT_EXCEEDED);
     (void) pn_condition_set_description(cond, LINK_DISALLOWED);
     pn_link_close(link);
 }
@@ -728,7 +725,7 @@ void qd_policy_amqp_open(void *context, bool discard)
                 pn_connection_open(conn);
             policy_notify_opened(qd_conn->open_container, qd_conn, qd_conn->context);
         } else {
-            qd_policy_private_deny_amqp_connection(conn, RESOURCE_LIMIT_EXCEEDED, CONNECTION_DISALLOWED);
+            qd_policy_private_deny_amqp_connection(conn, QD_AMQP_COND_RESOURCE_LIMIT_EXCEEDED, CONNECTION_DISALLOWED);
         }
     }
     qd_connection_set_event_stall(qd_conn, false);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[6/6] qpid-dispatch git commit: DISPATCH-390: refactor - log->ref_count use atomic counter

Posted by ac...@apache.org.
DISPATCH-390: refactor - log->ref_count use atomic counter


Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/56e2845a
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/56e2845a
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/56e2845a

Branch: refs/heads/master
Commit: 56e2845a75b08d5574238e15526381fe4cbaa37c
Parents: 336c536
Author: Alan Conway <ac...@redhat.com>
Authored: Mon Mar 27 10:08:44 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Tue Apr 25 18:13:59 2017 -0400

----------------------------------------------------------------------
 include/qpid/dispatch/atomic.h |  7 +++++--
 src/log.c                      | 11 ++++++-----
 2 files changed, 11 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/56e2845a/include/qpid/dispatch/atomic.h
----------------------------------------------------------------------
diff --git a/include/qpid/dispatch/atomic.h b/include/qpid/dispatch/atomic.h
index d72bad2..08d63f1 100644
--- a/include/qpid/dispatch/atomic.h
+++ b/include/qpid/dispatch/atomic.h
@@ -175,7 +175,10 @@ static inline void sys_atomic_destroy(sys_atomic_t *ref)
 
 #endif
 
-#define sys_atomic_inc(ref) sys_atomic_add((ref), 1)
-#define sys_atomic_dec(ref) sys_atomic_sub((ref), 1)
+/** Atomic increase: NOTE returns value *before* increase, like i++ */
+static inline uint32_t sys_atomic_inc(sys_atomic_t *ref) { return sys_atomic_add((ref), 1); }
+
+/** Atomic decrease: NOTE returns value *before* decrease, like i-- */
+static inline uint32_t sys_atomic_dec(sys_atomic_t *ref) { return sys_atomic_sub((ref), 1); }
 
 #endif

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/56e2845a/src/log.c
----------------------------------------------------------------------
diff --git a/src/log.c b/src/log.c
index 896d314..b11fd50 100644
--- a/src/log.c
+++ b/src/log.c
@@ -23,6 +23,7 @@
 #include "entity.h"
 #include "entity_cache.h"
 #include "aprintf.h"
+#include <qpid/dispatch/atomic.h>
 #include <qpid/dispatch/ctools.h>
 #include <qpid/dispatch/dispatch.h>
 #include "alloc.h"
@@ -70,7 +71,7 @@ static void qd_log_entry_free_lh(qd_log_entry_t* entry) {
 
 // Ref-counted log sink, may be shared by several sources.
 typedef struct log_sink_t {
-    int refcount;
+    sys_atomic_t ref_count;
     char *name;
     bool syslog;
     FILE *file;
@@ -97,9 +98,9 @@ static log_sink_t* find_log_sink_lh(const char* name) {
 // Must hold the log_source_lock
 static void log_sink_free_lh(log_sink_t* sink) {
     if (!sink) return;
-    assert(sink->refcount);
+    assert(sink->ref_count);
 
-    if (--sink->refcount == 0) {
+    if (sys_atomic_dec(&sink->ref_count) == 1) {
         DEQ_REMOVE(sink_list, sink);
         free(sink->name);
         if (sink->file && sink->file != stderr)
@@ -113,7 +114,7 @@ static void log_sink_free_lh(log_sink_t* sink) {
 static log_sink_t* log_sink_lh(const char* name) {
     log_sink_t* sink = find_log_sink_lh(name);
     if (sink)
-        sink->refcount++;
+        sys_atomic_inc(&sink->ref_count);
     else {
 
         bool syslog = false;
@@ -373,7 +374,7 @@ qd_log_source_t *qd_log_source_reset(const char *module)
 }
 
 static void qd_log_source_free_lh(qd_log_source_t* src) {
-    DEQ_REMOVE_HEAD(source_list);
+    DEQ_REMOVE(source_list, src);
     log_sink_free_lh(src->sink);
     free(src->module);
     free(src);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[3/6] qpid-dispatch git commit: DISPATCH-390: refactor - remove dead code

Posted by ac...@apache.org.
DISPATCH-390: refactor - remove dead code

- qd_bind_state_t set but never used
- threads_active set but never used
- is_connector set but never used
- pyflakes errors in system_tests_*manage*.py
- mark private connection_manager.c functions as static


Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/c2a1a32d
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/c2a1a32d
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/c2a1a32d

Branch: refs/heads/master
Commit: c2a1a32d1313eb7669aa67a5c216de32c63e70cc
Parents: 5e61dd9
Author: Alan Conway <ac...@redhat.com>
Authored: Fri Mar 3 00:33:29 2017 -0500
Committer: Alan Conway <ac...@redhat.com>
Committed: Tue Apr 25 18:13:59 2017 -0400

----------------------------------------------------------------------
 include/qpid/dispatch/connection_manager.h |   8 --
 include/qpid/dispatch/server.h             |  20 ++++
 src/connection_manager.c                   | 148 +++++++++++-------------
 src/server.c                               |   5 -
 src/server_private.h                       |   6 -
 tests/system_tests_management.py           |  24 +---
 tests/system_tests_qdmanage.py             |   3 +-
 7 files changed, 93 insertions(+), 121 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/include/qpid/dispatch/connection_manager.h
----------------------------------------------------------------------
diff --git a/include/qpid/dispatch/connection_manager.h b/include/qpid/dispatch/connection_manager.h
index 28a944c..12ac35e 100644
--- a/include/qpid/dispatch/connection_manager.h
+++ b/include/qpid/dispatch/connection_manager.h
@@ -54,18 +54,10 @@ void qd_connection_manager_free(qd_connection_manager_t *cm);
 void qd_config_listener_free(qd_connection_manager_t *cm, qd_config_listener_t *cl);
 
 /**
- * Free the SSL Profile. Only SSL Profiles that are not referenced from other connectors/listeners can be freed.
- * @return true if the ssl_profile has been freed, false if it is being referenced and cannot be freed
- */
-bool qd_config_ssl_profile_free(qd_connection_manager_t *cm, qd_config_ssl_profile_t *ssl_profile);
-
-
-/**
  * Free all the resources associated with a config connector
  */
 void qd_config_connector_free(qd_connection_manager_t *cm, qd_config_connector_t *cl);
 
-
 /**
  * Start the configured Listeners and Connectors
  *

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/include/qpid/dispatch/server.h
----------------------------------------------------------------------
diff --git a/include/qpid/dispatch/server.h b/include/qpid/dispatch/server.h
index 529c566..a466ec0 100644
--- a/include/qpid/dispatch/server.h
+++ b/include/qpid/dispatch/server.h
@@ -447,6 +447,26 @@ typedef struct qd_server_config_t {
      * Configured failover list
      */
     qd_failover_list_t *failover_list;
+
+    /**
+     * @name These fields are not primary configuration, they are computed.
+     * @{
+     */
+
+    /**
+     * Concatenated connect/listen address "host:port"
+     */
+    char *host_port;
+
+    /**
+     * Set for listeners that are part of the initial router configuration.
+     * An error in setting up initial listeners must shut down the router.
+     */
+    bool exit_on_error;
+
+    /**
+     * @}
+     */
 } qd_server_config_t;
 
 

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/src/connection_manager.c
----------------------------------------------------------------------
diff --git a/src/connection_manager.c b/src/connection_manager.c
index d8a5ecc..10e6fe4 100644
--- a/src/connection_manager.c
+++ b/src/connection_manager.c
@@ -49,8 +49,6 @@ struct qd_config_ssl_profile_t {
 };
 
 struct qd_config_listener_t {
-    bool                     is_connector;
-    qd_bind_state_t          state;
     qd_listener_t           *listener;
     qd_config_ssl_profile_t *ssl_profile;
     qd_server_config_t       configuration;
@@ -62,7 +60,6 @@ DEQ_DECLARE(qd_config_ssl_profile_t, qd_config_ssl_profile_list_t);
 
 
 struct qd_config_connector_t {
-    bool is_connector;
     DEQ_LINKS(qd_config_connector_t);
     qd_connector_t          *connector;
     qd_server_config_t       configuration;
@@ -415,6 +412,28 @@ bool is_log_component_enabled(qd_log_bits log_message, char *component_name) {
 }
 
 
+static bool config_ssl_profile_free(qd_connection_manager_t *cm, qd_config_ssl_profile_t *ssl_profile)
+{
+    if (sys_atomic_get(&ssl_profile->ref_count) != 0) {
+        return false;
+    }
+
+    DEQ_REMOVE(cm->config_ssl_profiles, ssl_profile);
+
+    free(ssl_profile->name);
+    free(ssl_profile->ssl_password);
+    free(ssl_profile->ssl_trusted_certificate_db);
+    free(ssl_profile->ssl_trusted_certificates);
+    free(ssl_profile->ssl_uid_format);
+    free(ssl_profile->ssl_display_name_file);
+    free(ssl_profile->ssl_certificate_file);
+    free(ssl_profile->ssl_private_key_file);
+    free(ssl_profile);
+    return true;
+
+}
+
+
 qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd_entity_t *entity)
 {
     qd_error_clear();
@@ -475,24 +494,37 @@ qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd
 
     error:
         qd_log(cm->log_source, QD_LOG_ERROR, "Unable to create ssl profile: %s", qd_error_message());
-        qd_config_ssl_profile_free(cm, ssl_profile);
+        config_ssl_profile_free(cm, ssl_profile);
         return 0;
 }
 
 
+
+static void config_listener_free(qd_connection_manager_t *cm, qd_config_listener_t *cl)
+{
+    if (cl->listener) {
+        qd_server_listener_close(cl->listener);
+        qd_server_listener_free(cl->listener);
+        cl->listener = 0;
+    }
+    if (cl->ssl_profile) {
+        sys_atomic_dec(&cl->ssl_profile->ref_count);
+    }
+    free(cl);
+}
+
+
 qd_config_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entity_t *entity)
 {
     qd_error_clear();
     qd_connection_manager_t *cm = qd->connection_manager;
     qd_config_listener_t *cl = NEW(qd_config_listener_t);
-    cl->is_connector = false;
-    cl->state = QD_BIND_NONE;
     cl->listener = 0;
     cl->ssl_profile = 0;
     qd_config_ssl_profile_t *ssl_profile = 0;
     if (load_server_config(qd, &cl->configuration, entity, &ssl_profile) != QD_ERROR_NONE) {
         qd_log(cm->log_source, QD_LOG_ERROR, "Unable to create config listener: %s", qd_error_message());
-        qd_config_listener_free(qd->connection_manager, cl);
+        config_listener_free(qd->connection_manager, cl);
         return 0;
     }
     cl->ssl_profile = ssl_profile;
@@ -503,7 +535,7 @@ qd_config_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entit
         free(fol);
         if (cl->configuration.failover_list == 0) {
             qd_log(cm->log_source, QD_LOG_ERROR, "Error parsing failover list: %s", fol_error);
-            qd_config_listener_free(qd->connection_manager, cl);
+            config_listener_free(qd->connection_manager, cl);
             return 0;
         }
     } else
@@ -535,6 +567,17 @@ qd_error_t qd_entity_refresh_connector(qd_entity_t* entity, void *impl)
 }
 
 
+static void config_connector_free(qd_connection_manager_t *cm, qd_config_connector_t *cc)
+{
+    if (cc->connector)
+        qd_server_connector_free(cc->connector);
+    if (cc->ssl_profile) {
+        sys_atomic_dec(&cc->ssl_profile->ref_count);
+    }
+    free(cc);
+}
+
+
 qd_config_connector_t *qd_dispatch_configure_connector(qd_dispatch_t *qd, qd_entity_t *entity)
 {
     qd_error_clear();
@@ -542,11 +585,10 @@ qd_config_connector_t *qd_dispatch_configure_connector(qd_dispatch_t *qd, qd_ent
     qd_config_connector_t *cc = NEW(qd_config_connector_t);
     ZERO(cc);
 
-    cc->is_connector = true;
     qd_config_ssl_profile_t *ssl_profile = 0;
     if (load_server_config(qd, &cc->configuration, entity, &ssl_profile) != QD_ERROR_NONE) {
         qd_log(cm->log_source, QD_LOG_ERROR, "Unable to create config connector: %s", qd_error_message());
-        qd_config_connector_free(qd->connection_manager, cc);
+        config_connector_free(qd->connection_manager, cc);
         return 0;
     }
     cc->ssl_profile = ssl_profile;
@@ -587,7 +629,7 @@ void qd_connection_manager_free(qd_connection_manager_t *cm)
     while (cl) {
         DEQ_REMOVE_HEAD(cm->config_listeners);
         qd_server_config_free(&cl->configuration);
-        qd_config_listener_free(cm, cl);
+        config_listener_free(cm, cl);
         cl = DEQ_HEAD(cm->config_listeners);
     }
 
@@ -595,13 +637,13 @@ void qd_connection_manager_free(qd_connection_manager_t *cm)
     while (cc) {
         DEQ_REMOVE_HEAD(cm->config_connectors);
         qd_server_config_free(&cc->configuration);
-        qd_config_connector_free(cm, cc);
+        config_connector_free(cm, cc);
         cc = DEQ_HEAD(cm->config_connectors);
     }
 
     qd_config_ssl_profile_t *sslp = DEQ_HEAD(cm->config_ssl_profiles);
     while (sslp) {
-        qd_config_ssl_profile_free(cm, sslp);
+        config_ssl_profile_free(cm, sslp);
         sslp = DEQ_HEAD(cm->config_ssl_profiles);
     }
 
@@ -609,6 +651,9 @@ void qd_connection_manager_free(qd_connection_manager_t *cm)
 }
 
 
+/** NOTE: non-static qd_connection_manager_* functions are called from the python agent */
+
+
 void qd_connection_manager_start(qd_dispatch_t *qd)
 {
     static bool first_start = true;
@@ -616,20 +661,14 @@ void qd_connection_manager_start(qd_dispatch_t *qd)
     qd_config_connector_t *cc = DEQ_HEAD(qd->connection_manager->config_connectors);
 
     while (cl) {
-        if (cl->listener == 0 )
-            if (cl->state == QD_BIND_NONE) { //Try to start listening only if we have never tried to listen on that port before
-                cl->listener = qd_server_listen(qd, &cl->configuration, cl);
-                if (cl->listener)
-                    cl->state = QD_BIND_SUCCESSFUL;
-                else {
-                    cl->state = QD_BIND_FAILED;
-                    if (first_start) {
-                        qd_log(qd->connection_manager->log_source, QD_LOG_CRITICAL,
-                               "Socket bind failed during initial configuration - process exiting");
-                        exit(1);
-                    }
-                }
+        if (cl->listener == 0 ) {
+            cl->listener = qd_server_listen(qd, &cl->configuration, cl);
+            if (!cl->listener &&  first_start) {
+                qd_log(qd->connection_manager->log_source, QD_LOG_CRITICAL,
+                       "Socket bind failed during initial configuration");
+                exit(1);
             }
+        }
         cl = DEQ_NEXT(cl);
     }
 
@@ -643,57 +682,6 @@ void qd_connection_manager_start(qd_dispatch_t *qd)
 }
 
 
-void qd_config_connector_free(qd_connection_manager_t *cm, qd_config_connector_t *cc)
-{
-    if (cc->connector)
-        qd_server_connector_free(cc->connector);
-
-    if (cc->ssl_profile) {
-        sys_atomic_dec(&cc->ssl_profile->ref_count);
-    }
-
-    free(cc);
-}
-
-
-void qd_config_listener_free(qd_connection_manager_t *cm, qd_config_listener_t *cl)
-{
-    if (cl->listener) {
-        qd_server_listener_close(cl->listener);
-        qd_server_listener_free(cl->listener);
-        cl->listener = 0;
-    }
-
-    if (cl->ssl_profile) {
-        sys_atomic_dec(&cl->ssl_profile->ref_count);
-    }
-
-    free(cl);
-}
-
-
-bool qd_config_ssl_profile_free(qd_connection_manager_t *cm, qd_config_ssl_profile_t *ssl_profile)
-{
-    if (sys_atomic_get(&ssl_profile->ref_count) != 0) {
-        return false;
-    }
-
-    DEQ_REMOVE(cm->config_ssl_profiles, ssl_profile);
-
-    free(ssl_profile->name);
-    free(ssl_profile->ssl_password);
-    free(ssl_profile->ssl_trusted_certificate_db);
-    free(ssl_profile->ssl_trusted_certificates);
-    free(ssl_profile->ssl_uid_format);
-    free(ssl_profile->ssl_display_name_file);
-    free(ssl_profile->ssl_certificate_file);
-    free(ssl_profile->ssl_private_key_file);
-    free(ssl_profile);
-    return true;
-
-}
-
-
 void qd_connection_manager_delete_listener(qd_dispatch_t *qd, void *impl)
 {
     qd_config_listener_t *cl = (qd_config_listener_t*) impl;
@@ -701,7 +689,7 @@ void qd_connection_manager_delete_listener(qd_dispatch_t *qd, void *impl)
     if (cl) {
         qd_server_listener_close(cl->listener);
         DEQ_REMOVE(qd->connection_manager->config_listeners, cl);
-        qd_config_listener_free(qd->connection_manager, cl);
+        config_listener_free(qd->connection_manager, cl);
     }
 }
 
@@ -714,7 +702,7 @@ bool qd_connection_manager_delete_ssl_profile(qd_dispatch_t *qd, void *impl)
 {
     qd_config_ssl_profile_t *ssl_profile = (qd_config_ssl_profile_t*) impl;
     if (ssl_profile)
-        return qd_config_ssl_profile_free(qd->connection_manager, ssl_profile);
+        return config_ssl_profile_free(qd->connection_manager, ssl_profile);
     return false;
 }
 
@@ -725,7 +713,7 @@ void qd_connection_manager_delete_connector(qd_dispatch_t *qd, void *impl)
 
     if (cc) {
         DEQ_REMOVE(qd->connection_manager->config_connectors, cc);
-        qd_config_connector_free(qd->connection_manager, cc);
+        config_connector_free(qd->connection_manager, cc);
     }
 }
 

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/src/server.c
----------------------------------------------------------------------
diff --git a/src/server.c b/src/server.c
index 31322ea..d5ad5c4 100644
--- a/src/server.c
+++ b/src/server.c
@@ -76,7 +76,6 @@ struct qd_server_t {
     qd_work_list_t            work_queue;
     qd_timer_list_t           pending_timers;
     bool                      a_thread_is_waiting;
-    int                       threads_active;
     int                       pause_requests;
     int                       threads_paused;
     int                       pause_next_sequence;
@@ -1021,7 +1020,6 @@ static void *thread_run(void *arg)
             if (ctx->owner_thread == CONTEXT_NO_OWNER) {
                 ctx->owner_thread = thread->thread_id;
                 ctx->enqueued = 0;
-                qd_server->threads_active++;
                 cxtr = work->cxtr;
                 free_qd_work_item_t(work);
             } else {
@@ -1073,7 +1071,6 @@ static void *thread_run(void *arg)
                 sys_mutex_free(ctx->deferred_call_lock);
                 qdpn_connector_free(cxtr);
                 free_qd_connection(ctx);
-                qd_server->threads_active--;
                 sys_mutex_unlock(qd_server->lock);
             } else {
                 //
@@ -1081,7 +1078,6 @@ static void *thread_run(void *arg)
                 //
                 sys_mutex_lock(qd_server->lock);
                 ctx->owner_thread = CONTEXT_NO_OWNER;
-                qd_server->threads_active--;
                 sys_mutex_unlock(qd_server->lock);
             }
 
@@ -1344,7 +1340,6 @@ qd_server_t *qd_server(qd_dispatch_t *qd, int thread_count, const char *containe
     DEQ_INIT(qd_server->work_queue);
     DEQ_INIT(qd_server->pending_timers);
     qd_server->a_thread_is_waiting    = false;
-    qd_server->threads_active         = 0;
     qd_server->pause_requests         = 0;
     qd_server->threads_paused         = 0;
     qd_server->pause_next_sequence    = 0;

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/src/server_private.h
----------------------------------------------------------------------
diff --git a/src/server_private.h b/src/server_private.h
index 49e70b2..9581196 100644
--- a/src/server_private.h
+++ b/src/server_private.h
@@ -49,12 +49,6 @@ qd_http_listener_t *qd_listener_http(qd_listener_t *l);
 #define CONTEXT_UNSPECIFIED_OWNER -2
 
 typedef enum {
-    QD_BIND_SUCCESSFUL, // Bind to socket was attempted and the bind succeeded
-    QD_BIND_FAILED,     // Bind to socket was attempted and bind failed
-    QD_BIND_NONE,    // Bind to socket not attempted yet
-} qd_bind_state_t;
-
-typedef enum {
     CXTR_STATE_CONNECTING = 0,
     CXTR_STATE_OPEN,
     CXTR_STATE_FAILED

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/tests/system_tests_management.py
----------------------------------------------------------------------
diff --git a/tests/system_tests_management.py b/tests/system_tests_management.py
index 19f69fc..b9535a8 100644
--- a/tests/system_tests_management.py
+++ b/tests/system_tests_management.py
@@ -20,13 +20,11 @@
 """System tests for management of qdrouter"""
 
 import unittest, system_test, re, os, json, sys
-from qpid_dispatch.management.client import Node, ManagementError, Url, BadRequestStatus, NotImplementedStatus, NotFoundStatus, ForbiddenStatus
+from qpid_dispatch.management.client import Node, ManagementError, Url, BadRequestStatus, NotImplementedStatus, NotFoundStatus
 from qpid_dispatch_internal.management.qdrouter import QdSchema
-from qpid_dispatch_internal.compat import OrderedDict, dictify
-from system_test import Qdrouterd, message, retry, retry_exception, wait_ports, Process
-from proton import ConnectionException
+from qpid_dispatch_internal.compat import dictify
+from system_test import Qdrouterd, message, Process
 from itertools import chain
-from time import sleep
 
 PREFIX = u'org.apache.qpid.dispatch.'
 MANAGEMENT = PREFIX + 'management'
@@ -305,7 +303,7 @@ class ManagementTest(system_test.TestCase):
                 self.assertEqual(
                     {u'operation': u'callme', u'type': DUMMY, u'identity': identity, u'data': data},
                     dummy.call('callme', data=data))
-            except TypeError, e:
+            except TypeError:
                 extype, value, trace = sys.exc_info()
                 raise extype, "data=%r: %s" % (data, value), trace
 
@@ -373,16 +371,6 @@ class ManagementTest(system_test.TestCase):
         """Test that we can access management info of remote nodes using get_mgmt_nodes addresses"""
         nodes = [self.cleanup(Node.connect(Url(r.addresses[0]))) for r in self.routers]
         remotes = sum([n.get_mgmt_nodes() for n in nodes], [])
-        self.assertEqual([u'amqp:/_topo/0/router2/$management', u'amqp:/_topo/0/router1/$management'], remotes)
-        # Query router2 indirectly via router1
-        remote_url = Url(self.routers[0].addresses[0], path=Url(remotes[0]).path)
-        remote = self.cleanup(Node.connect(remote_url))
-        self.assertEqual(["router2"], [r.id for r in remote.query(type=ROUTER).get_entities()])
-
-    def test_remote_node(self):
-        """Test that we can access management info of remote nodes using get_mgmt_nodes addresses"""
-        nodes = [self.cleanup(Node.connect(Url(r.addresses[0]))) for r in self.routers]
-        remotes = sum([n.get_mgmt_nodes() for n in nodes], [])
         self.assertEqual(set([u'amqp:/_topo/0/router%s/$management' % i for i in [0, 1, 2]]),
                          set(remotes))
         self.assertEqual(9, len(remotes))
@@ -398,10 +386,6 @@ class ManagementTest(system_test.TestCase):
         self.assertIn(CONFIGURATION, types[LISTENER])
         self.assertIn(OPERATIONAL, types[LINK])
 
-    def test_get_attributes(self):
-        types = self.node.get_attributes()
-        self.assertIn(SSL_PROFILE, types[CONNECTOR])
-
     def test_get_operations(self):
         result = self.node.get_operations(type=DUMMY)
         self.assertEqual({DUMMY: ["CREATE", "READ", "UPDATE", "DELETE", "CALLME"]}, result)

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/c2a1a32d/tests/system_tests_qdmanage.py
----------------------------------------------------------------------
diff --git a/tests/system_tests_qdmanage.py b/tests/system_tests_qdmanage.py
index 585fc4b..bdbd823 100644
--- a/tests/system_tests_qdmanage.py
+++ b/tests/system_tests_qdmanage.py
@@ -21,7 +21,7 @@ import json, unittest, os
 
 from system_test import TestCase, Process, Qdrouterd, main_module, TIMEOUT, DIR, wait_port
 from subprocess import PIPE, STDOUT
-from qpid_dispatch_internal.compat import OrderedDict, dictify
+from qpid_dispatch_internal.compat import dictify
 from qpid_dispatch_internal.management.qdrouter import QdSchema
 
 DUMMY = "org.apache.qpid.dispatch.dummy"
@@ -215,7 +215,6 @@ class QdmanageTest(TestCase):
 
         self.assertEquals ( good_logs, len(logs) )
 
-
     def test_update(self):
         exception = False
         try:


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[4/6] qpid-dispatch git commit: DISPATCH-390: refactor - drop ssl_profile reference counts

Posted by ac...@apache.org.
DISPATCH-390: refactor - drop ssl_profile reference counts

Copy SSL profile date at the time the connector or listener is configured, so
that it is safe to delete an SSL profile even if there are connectors/listeners
that used it.

Simplifying the lifecycle, getting rid of reference counts and removing the user
restrictions on removing SSL profiles seems to justify the small memory
duplication for the SSL profile data.

This means you cannot change an SSL profile and have the changes applied on the
next retry of a connector, but that seems like it would be an insane and
thread-unsafe thing to do anyway.


Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/336c5367
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/336c5367
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/336c5367

Branch: refs/heads/master
Commit: 336c5367146ebef1d723c01387a89cc0d74368fc
Parents: c2a1a32
Author: Alan Conway <ac...@redhat.com>
Authored: Thu Mar 23 13:05:03 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Tue Apr 25 18:13:59 2017 -0400

----------------------------------------------------------------------
 .../qpid_dispatch_internal/management/agent.py  |   7 +-
 src/connection_manager.c                        |  99 +++++--------
 tests/system_tests_qdmanage.py                  |   4 -
 tests/system_tests_sasl_plain.py                | 138 +++++--------------
 4 files changed, 74 insertions(+), 174 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/336c5367/python/qpid_dispatch_internal/management/agent.py
----------------------------------------------------------------------
diff --git a/python/qpid_dispatch_internal/management/agent.py b/python/qpid_dispatch_internal/management/agent.py
index 71cc836..a52d934 100644
--- a/python/qpid_dispatch_internal/management/agent.py
+++ b/python/qpid_dispatch_internal/management/agent.py
@@ -352,12 +352,7 @@ class SslProfileEntity(EntityAdapter):
         return self._qd.qd_dispatch_configure_ssl_profile(self._dispatch, self)
 
     def _delete(self):
-        deleted = self._qd.qd_connection_manager_delete_ssl_profile(self._dispatch, self._implementations[0].key)
-        # SSL Profiles cannot be deleted if they are referenced by a connector/listener.
-        if not deleted:
-            raise ForbiddenStatus("SSL Profile is referenced by other listeners/connectors. Delete the associated "
-                                  "listeners/connectors before deleting the SSL Profile")
-
+        self._qd.qd_connection_manager_delete_ssl_profile(self._dispatch, self._implementations[0].key)
     def _identifier(self):
         return self.name
 

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/336c5367/src/connection_manager.c
----------------------------------------------------------------------
diff --git a/src/connection_manager.c b/src/connection_manager.c
index 10e6fe4..a68d6e0 100644
--- a/src/connection_manager.c
+++ b/src/connection_manager.c
@@ -45,12 +45,10 @@ struct qd_config_ssl_profile_t {
     char        *ssl_display_name_file;
     char        *ssl_certificate_file;
     char        *ssl_private_key_file;
-    sys_atomic_t ref_count;
 };
 
 struct qd_config_listener_t {
     qd_listener_t           *listener;
-    qd_config_ssl_profile_t *ssl_profile;
     qd_server_config_t       configuration;
     DEQ_LINKS(qd_config_listener_t);
 };
@@ -63,7 +61,6 @@ struct qd_config_connector_t {
     DEQ_LINKS(qd_config_connector_t);
     qd_connector_t          *connector;
     qd_server_config_t       configuration;
-    qd_config_ssl_profile_t *ssl_profile;
 };
 
 DEQ_DECLARE(qd_config_connector_t, qd_config_connector_list_t);
@@ -74,7 +71,6 @@ struct qd_connection_manager_t {
     qd_config_listener_list_t     config_listeners;
     qd_config_connector_list_t    config_connectors;
     qd_config_ssl_profile_list_t  config_ssl_profiles;
-    sys_mutex_t                  *ssl_profile_lock;
 };
 
 const char *qd_log_message_components[] =
@@ -128,6 +124,13 @@ static void qd_server_config_free(qd_server_config_t *cf)
     if (cf->failover_list)   qd_failover_list_free(cf->failover_list);
     if (cf->log_message)     free(cf->log_message);
 
+    if (cf->ssl_certificate_file) free(cf->ssl_certificate_file);
+    if (cf->ssl_private_key_file) free(cf->ssl_private_key_file);
+    if (cf->ssl_password) free(cf->ssl_password);
+    if (cf->ssl_trusted_certificate_db) free(cf->ssl_trusted_certificate_db);
+    if (cf->ssl_trusted_certificates) free(cf->ssl_trusted_certificates);
+    if (cf->ssl_uid_format) free(cf->ssl_uid_format);
+    if (cf->ssl_display_name_file) free(cf->ssl_display_name_file);
     memset(cf, 0, sizeof(*cf));
 }
 
@@ -284,7 +287,7 @@ static qd_log_bits populate_log_message(const qd_server_config_t *config)
 }
 
 
-static qd_error_t load_server_config(qd_dispatch_t *qd, qd_server_config_t *config, qd_entity_t* entity, qd_config_ssl_profile_t **ssl_profile)
+static qd_error_t load_server_config(qd_dispatch_t *qd, qd_server_config_t *config, qd_entity_t* entity)
 {
     qd_error_clear();
 
@@ -378,17 +381,18 @@ static qd_error_t load_server_config(qd_dispatch_t *qd, qd_server_config_t *conf
         config->ssl_require_peer_authentication = config->sasl_mechanisms &&
             strstr(config->sasl_mechanisms, "EXTERNAL") != 0;
 
-        *ssl_profile = qd_find_ssl_profile(qd->connection_manager, config->ssl_profile);
-        if (*ssl_profile) {
-            config->ssl_certificate_file = (*ssl_profile)->ssl_certificate_file;
-            config->ssl_private_key_file = (*ssl_profile)->ssl_private_key_file;
-            config->ssl_password = (*ssl_profile)->ssl_password;
-            config->ssl_trusted_certificate_db = (*ssl_profile)->ssl_trusted_certificate_db;
-            config->ssl_trusted_certificates = (*ssl_profile)->ssl_trusted_certificates;
-            config->ssl_uid_format = (*ssl_profile)->ssl_uid_format;
-            config->ssl_display_name_file = (*ssl_profile)->ssl_display_name_file;
+        qd_config_ssl_profile_t *ssl_profile =
+            qd_find_ssl_profile(qd->connection_manager, config->ssl_profile);
+        if (ssl_profile) {
+#define SSTRDUP(S) ((S) ? strdup(S) : NULL)
+            config->ssl_certificate_file = SSTRDUP(ssl_profile->ssl_certificate_file);
+            config->ssl_private_key_file = SSTRDUP(ssl_profile->ssl_private_key_file);
+            config->ssl_password = SSTRDUP(ssl_profile->ssl_password);
+            config->ssl_trusted_certificate_db = SSTRDUP(ssl_profile->ssl_trusted_certificate_db);
+            config->ssl_trusted_certificates = SSTRDUP(ssl_profile->ssl_trusted_certificates);
+            config->ssl_uid_format = SSTRDUP(ssl_profile->ssl_uid_format);
+            config->ssl_display_name_file = SSTRDUP(ssl_profile->ssl_display_name_file);
         }
-        sys_atomic_inc(&(*ssl_profile)->ref_count);
     }
 
     return QD_ERROR_NONE;
@@ -414,10 +418,6 @@ bool is_log_component_enabled(qd_log_bits log_message, char *component_name) {
 
 static bool config_ssl_profile_free(qd_connection_manager_t *cm, qd_config_ssl_profile_t *ssl_profile)
 {
-    if (sys_atomic_get(&ssl_profile->ref_count) != 0) {
-        return false;
-    }
-
     DEQ_REMOVE(cm->config_ssl_profiles, ssl_profile);
 
     free(ssl_profile->name);
@@ -488,7 +488,6 @@ qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd
     //
     qd_config_ssl_profile_process_password(ssl_profile); CHECK();
 
-    sys_atomic_init(&ssl_profile->ref_count, 0);
     qd_log(cm->log_source, QD_LOG_INFO, "Created SSL Profile with name %s ", ssl_profile->name);
     return ssl_profile;
 
@@ -498,6 +497,14 @@ qd_config_ssl_profile_t *qd_dispatch_configure_ssl_profile(qd_dispatch_t *qd, qd
         return 0;
 }
 
+static void log_config(qd_log_source_t *log, qd_server_config_t *c, const char *what) {
+    qd_log(log, QD_LOG_INFO, "Configured %s: %s proto=%s, role=%s%s%s%s",
+           what, c->host_port, c->protocol_family ? c->protocol_family : "any",
+           c->role,
+           c->http ? ", http" : "",
+           c->ssl_profile ? ", sslProfile=":"",
+           c->ssl_profile ? c->ssl_profile:"");
+}
 
 
 static void config_listener_free(qd_connection_manager_t *cm, qd_config_listener_t *cl)
@@ -507,9 +514,7 @@ static void config_listener_free(qd_connection_manager_t *cm, qd_config_listener
         qd_server_listener_free(cl->listener);
         cl->listener = 0;
     }
-    if (cl->ssl_profile) {
-        sys_atomic_dec(&cl->ssl_profile->ref_count);
-    }
+    qd_server_config_free(&cl->configuration);
     free(cl);
 }
 
@@ -520,14 +525,12 @@ qd_config_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entit
     qd_connection_manager_t *cm = qd->connection_manager;
     qd_config_listener_t *cl = NEW(qd_config_listener_t);
     cl->listener = 0;
-    cl->ssl_profile = 0;
-    qd_config_ssl_profile_t *ssl_profile = 0;
-    if (load_server_config(qd, &cl->configuration, entity, &ssl_profile) != QD_ERROR_NONE) {
+
+    if (load_server_config(qd, &cl->configuration, entity) != QD_ERROR_NONE) {
         qd_log(cm->log_source, QD_LOG_ERROR, "Unable to create config listener: %s", qd_error_message());
         config_listener_free(qd->connection_manager, cl);
         return 0;
     }
-    cl->ssl_profile = ssl_profile;
     char *fol = qd_entity_opt_string(entity, "failoverList", 0);
     if (fol) {
         const char *fol_error = 0;
@@ -542,15 +545,7 @@ qd_config_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entit
         cl->configuration.failover_list = 0;
     DEQ_ITEM_INIT(cl);
     DEQ_INSERT_TAIL(cm->config_listeners, cl);
-
-    qd_log(cm->log_source, QD_LOG_INFO, "Configured Listener: %s:%s proto=%s, role=%s%s%s%s",
-           cl->configuration.host, cl->configuration.port,
-           cl->configuration.protocol_family ? cl->configuration.protocol_family : "any",
-           cl->configuration.role,
-           cl->configuration.http ? ", http" : "",
-           cl->ssl_profile ? ", sslProfile=":"",
-           cl->ssl_profile ? cl->ssl_profile->name:"");
-
+    log_config(cm->log_source, &cl->configuration, "Listener");
     return cl;
 }
 
@@ -569,10 +564,9 @@ qd_error_t qd_entity_refresh_connector(qd_entity_t* entity, void *impl)
 
 static void config_connector_free(qd_connection_manager_t *cm, qd_config_connector_t *cc)
 {
-    if (cc->connector)
+    if (cc->connector) {
         qd_server_connector_free(cc->connector);
-    if (cc->ssl_profile) {
-        sys_atomic_dec(&cc->ssl_profile->ref_count);
+        qd_server_config_free(&cc->configuration);
     }
     free(cc);
 }
@@ -585,22 +579,14 @@ qd_config_connector_t *qd_dispatch_configure_connector(qd_dispatch_t *qd, qd_ent
     qd_config_connector_t *cc = NEW(qd_config_connector_t);
     ZERO(cc);
 
-    qd_config_ssl_profile_t *ssl_profile = 0;
-    if (load_server_config(qd, &cc->configuration, entity, &ssl_profile) != QD_ERROR_NONE) {
+    if (load_server_config(qd, &cc->configuration, entity) != QD_ERROR_NONE) {
         qd_log(cm->log_source, QD_LOG_ERROR, "Unable to create config connector: %s", qd_error_message());
         config_connector_free(qd->connection_manager, cc);
         return 0;
     }
-    cc->ssl_profile = ssl_profile;
     DEQ_ITEM_INIT(cc);
     DEQ_INSERT_TAIL(cm->config_connectors, cc);
-    qd_log(cm->log_source, QD_LOG_INFO, "Configured Connector: %s:%s proto=%s, role=%s %s%s",
-            cc->configuration.host, cc->configuration.port,
-            cc->configuration.protocol_family ? cc->configuration.protocol_family : "any",
-            cc->configuration.role,
-            cc->ssl_profile ? ", sslProfile=":"",
-            cc->ssl_profile ? cc->ssl_profile->name:"");
-
+    log_config(cm->log_source, &cc->configuration, "Connector");
     return cc;
 }
 
@@ -612,7 +598,6 @@ qd_connection_manager_t *qd_connection_manager(qd_dispatch_t *qd)
         return 0;
 
     cm->log_source = qd_log_source("CONN_MGR");
-    cm->ssl_profile_lock = sys_mutex();
     cm->server     = qd->server;
     DEQ_INIT(cm->config_listeners);
     DEQ_INIT(cm->config_connectors);
@@ -628,7 +613,6 @@ void qd_connection_manager_free(qd_connection_manager_t *cm)
     qd_config_listener_t *cl = DEQ_HEAD(cm->config_listeners);
     while (cl) {
         DEQ_REMOVE_HEAD(cm->config_listeners);
-        qd_server_config_free(&cl->configuration);
         config_listener_free(cm, cl);
         cl = DEQ_HEAD(cm->config_listeners);
     }
@@ -636,7 +620,6 @@ void qd_connection_manager_free(qd_connection_manager_t *cm)
     qd_config_connector_t *cc = DEQ_HEAD(cm->config_connectors);
     while (cc) {
         DEQ_REMOVE_HEAD(cm->config_connectors);
-        qd_server_config_free(&cc->configuration);
         config_connector_free(cm, cc);
         cc = DEQ_HEAD(cm->config_connectors);
     }
@@ -646,8 +629,6 @@ void qd_connection_manager_free(qd_connection_manager_t *cm)
         config_ssl_profile_free(cm, sslp);
         sslp = DEQ_HEAD(cm->config_ssl_profiles);
     }
-
-    sys_mutex_free(cm->ssl_profile_lock);
 }
 
 
@@ -694,16 +675,10 @@ void qd_connection_manager_delete_listener(qd_dispatch_t *qd, void *impl)
 }
 
 
-/**
- * Only those SSL Profiles that are not being referenced from other
- * listeners/connectors can be deleted
- */
-bool qd_connection_manager_delete_ssl_profile(qd_dispatch_t *qd, void *impl)
+void qd_connection_manager_delete_ssl_profile(qd_dispatch_t *qd, void *impl)
 {
     qd_config_ssl_profile_t *ssl_profile = (qd_config_ssl_profile_t*) impl;
-    if (ssl_profile)
-        return config_ssl_profile_free(qd->connection_manager, ssl_profile);
-    return false;
+    config_ssl_profile_free(qd->connection_manager, ssl_profile);
 }
 
 

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/336c5367/tests/system_tests_qdmanage.py
----------------------------------------------------------------------
diff --git a/tests/system_tests_qdmanage.py b/tests/system_tests_qdmanage.py
index bdbd823..5218f4c 100644
--- a/tests/system_tests_qdmanage.py
+++ b/tests/system_tests_qdmanage.py
@@ -346,10 +346,6 @@ class QdmanageTest(TestCase):
         output = json.loads(self.run_qdmanage(ssl_create_command))
         self.assertEqual(output['name'], ssl_profile_name)
         self.run_qdmanage('DELETE --type=sslProfile --name=' + ssl_profile_name)
-        # Try to delete the server-ssl profile which is in use.
-        output = self.run_qdmanage('DELETE --type=sslProfile --name=server-ssl',
-                                   expect=Process.EXIT_FAIL)
-        self.assertIn("ForbiddenStatus", output)
 
 if __name__ == '__main__':
     unittest.main(main_module())

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/336c5367/tests/system_tests_sasl_plain.py
----------------------------------------------------------------------
diff --git a/tests/system_tests_sasl_plain.py b/tests/system_tests_sasl_plain.py
index a180d80..491c8cd 100644
--- a/tests/system_tests_sasl_plain.py
+++ b/tests/system_tests_sasl_plain.py
@@ -305,18 +305,19 @@ class RouterTestPlainSaslOverSsl(RouterTestPlainSaslCommon):
             self.skipTest("Cyrus library not available. skipping test")
 
         local_node = Node.connect(self.routers[0].addresses[1], timeout=TIMEOUT)
+        results = local_node.query(type='org.apache.qpid.dispatch.connection').results
 
         # sslProto should be TLSv1/SSLv3
-        self.assertEqual(u'TLSv1/SSLv3', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][10])
+        self.assertEqual(u'TLSv1/SSLv3', results[0][10])
 
         # role should be inter-router
-        self.assertEqual(u'inter-router', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][3])
+        self.assertEqual(u'inter-router', results[0][3])
 
         # sasl must be plain
-        self.assertEqual(u'PLAIN', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][6])
+        self.assertEqual(u'PLAIN', results[0][6])
 
         # user must be test@domain.com
-        self.assertEqual(u'test@domain.com', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][8])
+        self.assertEqual(u'test@domain.com', results[0][8])
 
 
 class RouterTestVerifyHostNameYes(RouterTestPlainSaslCommon):
@@ -399,15 +400,15 @@ class RouterTestVerifyHostNameYes(RouterTestPlainSaslCommon):
             self.skipTest("Cyrus library not available. skipping test")
 
         local_node = Node.connect(self.routers[1].addresses[0], timeout=TIMEOUT)
-
+        results = local_node.query(type='org.apache.qpid.dispatch.connection').results
         # There should be only two connections.
         # There will be no inter-router connection
-        self.assertEqual(2, len(local_node.query(type='org.apache.qpid.dispatch.connection').results))
-        self.assertEqual('in', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][4])
-        self.assertEqual('normal', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][3])
-        self.assertEqual('anonymous', local_node.query(type='org.apache.qpid.dispatch.connection').results[0][8])
-        self.assertEqual('normal', local_node.query(type='org.apache.qpid.dispatch.connection').results[1][3])
-        self.assertEqual('anonymous', local_node.query(type='org.apache.qpid.dispatch.connection').results[1][8])
+        self.assertEqual(2, len(results))
+        self.assertEqual('in', results[0][4])
+        self.assertEqual('normal', results[0][3])
+        self.assertEqual('anonymous', results[0][8])
+        self.assertEqual('normal', results[1][3])
+        self.assertEqual('anonymous', results[1][8])
 
 class RouterTestVerifyHostNameNo(RouterTestPlainSaslCommon):
 
@@ -487,17 +488,6 @@ class RouterTestVerifyHostNameNo(RouterTestPlainSaslCommon):
     def ssl_file(name):
         return os.path.join(DIR, 'ssl_certs', name)
 
-    def run_qdmanage(self, cmd, input=None, expect=Process.EXIT_OK, address=None):
-        p = self.popen(
-            ['qdmanage'] + cmd.split(' ') + ['--bus', address or self.address(), '--indent=-1', '--timeout',
-                                             str(TIMEOUT)], stdin=PIPE, stdout=PIPE, stderr=STDOUT, expect=expect)
-        out = p.communicate(input)[0]
-        try:
-            p.teardown()
-        except Exception, e:
-            raise Exception("%s\n%s" % (e, out))
-        return out
-
     def common_asserts(self, results):
         search = "QDR.X"
         found = False
@@ -534,48 +524,6 @@ class RouterTestVerifyHostNameNo(RouterTestPlainSaslCommon):
 
         self.common_asserts(results)
 
-    def test_zzz_delete_create_connector(self):
-        """
-        Delete an ssl profile before deleting the connector and make sure it fails.
-        Delete an ssl profile after deleting the connector and make sure it succeeds.
-        Re-add the deleted connector and associate it with an ssl profile and make sure
-        that the two routers are able to communicate over the connection.
-        """
-        if not SASL.extended():
-            self.skipTest("Cyrus library not available. skipping test")
-
-        ssl_profile_name = 'client-ssl-profile'
-
-        delete_command = 'DELETE --type=sslProfile --name=' + ssl_profile_name
-
-        cannot_delete = False
-        try:
-            json.loads(self.run_qdmanage(delete_command, address=self.routers[1].addresses[0]))
-        except Exception as e:
-            cannot_delete = True
-            self.assertTrue('ForbiddenStatus: SSL Profile is referenced by other listeners/connectors' in e.message)
-
-        self.assertTrue(cannot_delete)
-
-        # Deleting the connector
-        delete_command = 'DELETE --type=connector --name=connectorToX'
-        self.run_qdmanage(delete_command, address=self.routers[1].addresses[0])
-
-        #Assert here that the connection to QDR.X is gone
-
-        # Re-add connector
-        connector_create_command = 'CREATE --type=connector name=connectorToX host=127.0.0.1 port=' + \
-                                   str(RouterTestVerifyHostNameNo.x_listener_port) + \
-                                   ' saslMechanisms=PLAIN sslProfile=' + ssl_profile_name + \
-                                   ' role=inter-router verifyHostName=no saslUsername=test@domain.com' \
-                                   ' saslPassword=password'
-
-        json.loads(self.run_qdmanage(connector_create_command, address=self.routers[1].addresses[0]))
-        self.routers[1].wait_connectors()
-        local_node = Node.connect(self.routers[1].addresses[0], timeout=TIMEOUT)
-        results = local_node.query(type='org.apache.qpid.dispatch.connection').results
-        self.common_asserts(results)
-
     def test_zzz_delete_create_ssl_profile(self):
         """
         Deletes a connector and its corresponding ssl profile and recreates both
@@ -583,48 +531,34 @@ class RouterTestVerifyHostNameNo(RouterTestPlainSaslCommon):
         if not SASL.extended():
             self.skipTest("Cyrus library not available. skipping test")
 
-        ssl_profile_name = 'client-ssl-profile'
-
-        # Deleting the connector first and then its SSL profile must work.
-        delete_command = 'DELETE --type=connector --name=connectorToX'
-        self.run_qdmanage(delete_command, address=self.routers[1].addresses[0])
+        local_node = self.routers[1].management
 
-        # Delete the connector's associated ssl profile
-        delete_command = 'DELETE --type=sslProfile --name=' + ssl_profile_name
-        self.run_qdmanage(delete_command, address=self.routers[1].addresses[0])
-
-        local_node = Node.connect(self.routers[1].addresses[0], timeout=TIMEOUT)
-        results = local_node.query(type='org.apache.qpid.dispatch.connection').results
-        search = "QDR.X"
-        found = False
-
-        for N in range(0, 3):
-            if results[N][0] == search:
-                found = True
-                break
-
-        self.assertFalse(found)
+        connections = local_node.query(type='org.apache.qpid.dispatch.connection').get_entities()
+        self.assertIn("QDR.X", [c.container for c in connections]) # We can find the connection before
+        local_node.delete(type='connector', name='connectorToX')
+        local_node.delete(type='sslProfile', name='client-ssl-profile')
+        connections = local_node.query(type='org.apache.qpid.dispatch.connection').get_entities()
+        self.assertNotIn("QDR.X", [c.container for c in connections]) # Should not be present now
 
         # re-create the ssl profile
-        long_type = 'org.apache.qpid.dispatch.sslProfile'
-        ssl_create_command = 'CREATE --type=' + long_type + ' certFile=' + self.ssl_file('client-certificate.pem') + \
-                             ' keyFile=' + self.ssl_file('client-private-key.pem') + ' password=client-password' + \
-                             ' name=' + ssl_profile_name + ' certDb=' + self.ssl_file('ca-certificate.pem')
-
-        output = json.loads(self.run_qdmanage(ssl_create_command, address=self.routers[1].addresses[0]))
-        name = output['name']
-        self.assertEqual(name, ssl_profile_name)
-
-        # Re-add connector
-        connector_create_command = 'CREATE --type=connector name=connectorToX host=127.0.0.1 port=' + \
-                                   str(RouterTestVerifyHostNameNo.x_listener_port) + \
-                                   ' saslMechanisms=PLAIN sslProfile=' + ssl_profile_name + \
-                                   ' role=inter-router verifyHostName=no saslUsername=test@domain.com' \
-                                   ' saslPassword=password'
-
-        json.loads(self.run_qdmanage(connector_create_command, address=self.routers[1].addresses[0]))
+        local_node.create({'type': 'sslProfile',
+                     'name': 'client-ssl-profile',
+                     'certFile': self.ssl_file('client-certificate.pem'),
+                     'keyFile': self.ssl_file('client-private-key.pem'),
+                     'password': 'client-password',
+                     'certDb': self.ssl_file('ca-certificate.pem')})
+        # re-create connector
+        local_node.create({'type': 'connector',
+                     'name': 'connectorToX',
+                     'host': '127.0.0.1',
+                     'port': self.x_listener_port,
+                     'saslMechanisms': 'PLAIN',
+                     'sslProfile': 'client-ssl-profile',
+                     'role': 'inter-router',
+                     'verifyHostName': False,
+                     'saslUsername': 'test@domain.com',
+                     'saslPassword': 'password'})
         self.routers[1].wait_connectors()
-        local_node = Node.connect(self.routers[1].addresses[0], timeout=TIMEOUT)
         results = local_node.query(type='org.apache.qpid.dispatch.connection').results
 
         self.common_asserts(results)


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[2/6] qpid-dispatch git commit: DISPATCH-390: refactor - failover_list() use qd_error() for errors

Posted by ac...@apache.org.
DISPATCH-390: refactor - failover_list() use qd_error() for errors


Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/99ef0a77
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/99ef0a77
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/99ef0a77

Branch: refs/heads/master
Commit: 99ef0a77abae1d75f4f11cb9c0dfa7ef48d04606
Parents: 56e2845
Author: Alan Conway <ac...@redhat.com>
Authored: Thu Mar 23 16:32:23 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Tue Apr 25 18:13:59 2017 -0400

----------------------------------------------------------------------
 include/qpid/dispatch/failoverlist.h |  4 +++-
 src/connection_manager.c             |  9 ++++-----
 src/failoverlist.c                   | 13 ++++++++-----
 tests/failoverlist_test.c            | 18 +++++++-----------
 4 files changed, 22 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/99ef0a77/include/qpid/dispatch/failoverlist.h
----------------------------------------------------------------------
diff --git a/include/qpid/dispatch/failoverlist.h b/include/qpid/dispatch/failoverlist.h
index 1fb0c97..c9e6fb5 100644
--- a/include/qpid/dispatch/failoverlist.h
+++ b/include/qpid/dispatch/failoverlist.h
@@ -38,8 +38,10 @@ typedef struct qd_failover_list_t qd_failover_list_t;
  *
  * If scheme is not supplied, it defaults to _not present_.
  * If port is not specified, it defaults to "5672".
+ *
+ * Sets qd_error() if text cannot be parsed.
  */
-qd_failover_list_t *qd_failover_list(const char *text, const char **error);
+qd_failover_list_t *qd_failover_list(const char *text);
 
 /**
  * qd_failover_list_free

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/99ef0a77/src/connection_manager.c
----------------------------------------------------------------------
diff --git a/src/connection_manager.c b/src/connection_manager.c
index a68d6e0..8957185 100644
--- a/src/connection_manager.c
+++ b/src/connection_manager.c
@@ -521,7 +521,6 @@ static void config_listener_free(qd_connection_manager_t *cm, qd_config_listener
 
 qd_config_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entity_t *entity)
 {
-    qd_error_clear();
     qd_connection_manager_t *cm = qd->connection_manager;
     qd_config_listener_t *cl = NEW(qd_config_listener_t);
     cl->listener = 0;
@@ -533,16 +532,16 @@ qd_config_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_entit
     }
     char *fol = qd_entity_opt_string(entity, "failoverList", 0);
     if (fol) {
-        const char *fol_error = 0;
-        cl->configuration.failover_list = qd_failover_list(fol, &fol_error);
+        cl->configuration.failover_list = qd_failover_list(fol);
         free(fol);
         if (cl->configuration.failover_list == 0) {
-            qd_log(cm->log_source, QD_LOG_ERROR, "Error parsing failover list: %s", fol_error);
+            qd_log(cm->log_source, QD_LOG_ERROR, "Error parsing failover list: %s", qd_error_message());
             config_listener_free(qd->connection_manager, cl);
             return 0;
         }
-    } else
+    } else {
         cl->configuration.failover_list = 0;
+    }
     DEQ_ITEM_INIT(cl);
     DEQ_INSERT_TAIL(cm->config_listeners, cl);
     log_config(cm->log_source, &cl->configuration, "Listener");

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/99ef0a77/src/failoverlist.c
----------------------------------------------------------------------
diff --git a/src/failoverlist.c b/src/failoverlist.c
index e83f029..f350c1a 100644
--- a/src/failoverlist.c
+++ b/src/failoverlist.c
@@ -19,6 +19,7 @@
 
 #include <qpid/dispatch/failoverlist.h>
 #include <qpid/dispatch/ctools.h>
+#include <qpid/dispatch/error.h>
 #include <ctype.h>
 #include <string.h>
 
@@ -62,15 +63,17 @@ static char *qd_fol_next(char *text, const char *separator)
 }
 
 
-static qd_failover_item_t *qd_fol_item(char *text, const char **error)
+/* Sets qd_error if there is an error */
+static qd_failover_item_t *qd_fol_item(char *text)
 {
+    qd_error_clear();
     char *after_scheme = qd_fol_next(text, "://");
     char *scheme       = after_scheme ? text : 0;
     char *host         = after_scheme ? after_scheme : text;
     char *port         = qd_fol_next(host, ":");
 
     if (strlen(host) == 0) {
-        *error = "No network host in failover item";
+        qd_error(QD_ERROR_VALUE, "No network host in failover item");
         return 0;
     }
 
@@ -84,12 +87,12 @@ static qd_failover_item_t *qd_fol_item(char *text, const char **error)
 }
 
 
-qd_failover_list_t *qd_failover_list(const char *text, const char **error)
+qd_failover_list_t *qd_failover_list(const char *text)
 {
     qd_failover_list_t *list = NEW(qd_failover_list_t);
     ZERO(list);
 
-    *error = 0;
+    qd_error_clear();
     list->text = (char*) malloc(strlen(text) + 1);
     strcpy(list->text, text);
 
@@ -98,7 +101,7 @@ qd_failover_list_t *qd_failover_list(const char *text, const char **error)
     char *next;
     do {
         next = qd_fol_next(cursor, ",");
-        qd_failover_item_t *item = qd_fol_item(cursor, error);
+        qd_failover_item_t *item = qd_fol_item(cursor);
         if (item == 0) {
             qd_failover_list_free(list);
             return 0;

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/99ef0a77/tests/failoverlist_test.c
----------------------------------------------------------------------
diff --git a/tests/failoverlist_test.c b/tests/failoverlist_test.c
index 62544c1..0a645d0 100644
--- a/tests/failoverlist_test.c
+++ b/tests/failoverlist_test.c
@@ -29,8 +29,7 @@
 
 static char *test_failover_list_empty(void *unused)
 {
-    static char *error;
-    qd_failover_list_t *list = qd_failover_list("", (const char**) &error);
+    qd_failover_list_t *list = qd_failover_list("");
     if (list != 0) {
         qd_failover_list_free(list);
         return "Expected parse failure";
@@ -41,10 +40,9 @@ static char *test_failover_list_empty(void *unused)
 
 static char *test_failover_list_single(void *unused)
 {
-    char *error;
     char *fail = 0;
-    
-    qd_failover_list_t *list = qd_failover_list("host1", (const char**) &error);
+
+    qd_failover_list_t *list = qd_failover_list("host1");
     if (qd_failover_list_size(list) != 1) {
         fail = "1:Expected list size of 1";
     } else if (qd_failover_list_scheme(list, 0) != 0) {
@@ -60,7 +58,7 @@ static char *test_failover_list_single(void *unused)
     if (fail)
         return fail;
 
-    list = qd_failover_list("amqps://host2", (const char**) &error);
+    list = qd_failover_list("amqps://host2");
     if (qd_failover_list_size(list) != 1) {
         fail = "2:Expected list size of 1";
     } else if (strcmp(qd_failover_list_scheme(list, 0), "amqps")) {
@@ -76,7 +74,7 @@ static char *test_failover_list_single(void *unused)
     if (fail)
         return fail;
 
-    list = qd_failover_list("host3:10000", (const char**) &error);
+    list = qd_failover_list("host3:10000");
     if (qd_failover_list_size(list) != 1) {
         fail = "3:Expected list size of 1";
     } else if (qd_failover_list_scheme(list, 0) != 0) {
@@ -92,7 +90,7 @@ static char *test_failover_list_single(void *unused)
     if (fail)
         return fail;
 
-    list = qd_failover_list("amqp://host4:15000", (const char**) &error);
+    list = qd_failover_list("amqp://host4:15000");
     if (qd_failover_list_size(list) != 1) {
         fail = "4:Expected list size of 1";
     } else if (strcmp(qd_failover_list_scheme(list, 0), "amqp")) {
@@ -111,10 +109,8 @@ static char *test_failover_list_single(void *unused)
 
 static char *test_failover_list_multiple(void *unused)
 {
-    char *error;
     char *fail = 0;
-    
-    qd_failover_list_t *list = qd_failover_list("host1,amqps://host2 , host3:10000, amqp://host4:15000", (const char**) &error);
+    qd_failover_list_t *list = qd_failover_list("host1,amqps://host2 , host3:10000, amqp://host4:15000");
     if (qd_failover_list_size(list) != 4) {
         fail = "1:Expected list size of 4";
     } else if (qd_failover_list_scheme(list, 0) != 0) {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[5/6] qpid-dispatch git commit: DISPATCH-390: refactor - more reliable qdmanage test

Posted by ac...@apache.org.
DISPATCH-390: refactor - more reliable qdmanage test

Wait for router to be connected, otherwise query test can get different
results because connection happens during test.


Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/390ebe7f
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/390ebe7f
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/390ebe7f

Branch: refs/heads/master
Commit: 390ebe7fbf296712513aec491b33641d03998e0f
Parents: 99ef0a7
Author: Alan Conway <ac...@redhat.com>
Authored: Tue Mar 28 04:13:29 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Tue Apr 25 18:13:59 2017 -0400

----------------------------------------------------------------------
 tests/system_tests_qdmanage.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/390ebe7f/tests/system_tests_qdmanage.py
----------------------------------------------------------------------
diff --git a/tests/system_tests_qdmanage.py b/tests/system_tests_qdmanage.py
index 5218f4c..17304f1 100644
--- a/tests/system_tests_qdmanage.py
+++ b/tests/system_tests_qdmanage.py
@@ -58,6 +58,7 @@ class QdmanageTest(TestCase):
         ])
         cls.router_2 = cls.tester.qdrouterd('test_router_2', config_2, wait=True)
         cls.router_1 = cls.tester.qdrouterd('test_router_1', config_1, wait=True)
+        cls.router_1.wait_router_connected('R2')
 
     def address(self):
         return self.router_1.addresses[0]
@@ -149,9 +150,7 @@ class QdmanageTest(TestCase):
         self.assertEqual([long_type('listener')]*2, [e['type'] for e in qlistener])
         self.assertEqual(self.router_1.ports[0], int(qlistener[0]['port']))
 
-        qattr = json.loads(
-            self.run_qdmanage('query type name'))
-
+        qattr = json.loads(self.run_qdmanage('query type name'))
         for e in qattr:
             self.assertEqual(2, len(e))
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org