[RFC] add support for reset command #228
Conversation
src/client/lldpcli.c
Outdated
| @@ -438,7 +474,6 @@ main(int argc, char *argv[]) | |||
| ctlname = lldpctl_get_default_transport(); | |||
|
|
|||
| signal(SIGHUP, SIG_IGN); | |||
|
|
|||
|
Also needed (but I can do it after merging):
|
src/daemon/client.c
Outdated
| config->c_reset == 1) { | ||
| log_debug("rpc", "client asked to reset lldpd's config"); | ||
| cfg->g_config.c_reset = 1; //mark that reset is being processed by daemon | ||
| //so that client won't accept any new reset |
There was a problem hiding this comment.
You don't really need an "in progress" marker. lldpd is totally "evented", so nothing will execute in parallel until the end of this function. So, nobody will be able to observer a case where cfg->g_onfig.c_reset == 1. You can remove that and some other code on the client side about that.
| if (lldpctl_atom_get_int(config, lldpctl_k_config_reset) == 1) { | ||
| log_debug("lldpctl", "lldpd already has a reset config request in it's queue. Please try later."); | ||
| lldpctl_atom_dec_ref(config); | ||
| return 1; |
There was a problem hiding this comment.
This is an impossible case. No client can never get a value of 1 at this point.
| @@ -793,6 +793,7 @@ typedef enum { | |||
| lldpctl_k_custom_tlv_oui_subtype, /**< `(I,WO)` custom TLV subtype. Default is 0 (1 byte) */ | |||
| lldpctl_k_custom_tlv_oui_info_string, /**< `(I,WO)` custom TLV Organizationally Unique Identifier Information String (up to 507 bytes) */ | |||
| lldpctl_k_custom_tlv_op, /**< `(I,WO)` custom TLV operation */ | |||
| lldpctl_k_config_reset, /**< `(I)` lldpd is reset */ | |||
There was a problem hiding this comment.
This is (I,WO). Also, small nitpick, uses some tabs to align it with the other comments.
| lldpctl_atom_dec_ref(config); | ||
| return 1; | ||
| } | ||
|
|
There was a problem hiding this comment.
This whole function should use tabs instead of spaces for indentation.
|
thanks for your feedback.
I will get back with a new approach ASAP (could take some time since I have
other engagements right now).
…On Sat, Apr 29, 2017 at 4:33 PM, Vincent Bernat ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Sorry for the very late answer. We don't get a notification when new
commits are pushed, so I just forgot about this PR!
------------------------------
In src/client/lldpcli.c
<#228 (comment)>:
> + * @return 1 on success, 0 on error
+ */
+static int
+cmd_reset(struct lldpctl_conn_t *conn, struct writer *w,
+ struct cmd_env *env, void *arg)
+{
+ lldpctl_atom_t *config = lldpctl_get_configuration(conn);
+ if (config == NULL) {
+ log_warnx("lldpctl", "unable to get configuration from lldpd. %s",
+ lldpctl_last_strerror(conn));
+ return 0;
+ }
+ if (lldpctl_atom_get_int(config, lldpctl_k_config_reset) == 1) {
+ log_debug("lldpctl", "lldpd already has a reset config request in it's queue. Please try later.");
+ lldpctl_atom_dec_ref(config);
+ return 1;
This is an impossible case. No client can never get a value of 1 at this
point.
------------------------------
In src/lib/lldpctl.h
<#228 (comment)>:
> @@ -793,6 +793,7 @@ typedef enum {
lldpctl_k_custom_tlv_oui_subtype, /**< `(I,WO)` custom TLV subtype. Default is 0 (1 byte) */
lldpctl_k_custom_tlv_oui_info_string, /**< `(I,WO)` custom TLV Organizationally Unique Identifier Information String (up to 507 bytes) */
lldpctl_k_custom_tlv_op, /**< `(I,WO)` custom TLV operation */
+ lldpctl_k_config_reset, /**< `(I)` lldpd is reset */
This is (I,WO). Also, small nitpick, uses some tabs to align it with the
other comments.
------------------------------
In src/client/lldpcli.c
<#228 (comment)>:
> + log_debug("lldpctl", "lldpd already has a reset config request in it's queue. Please try later.");
+ lldpctl_atom_dec_ref(config);
+ return 1;
+ }
+ if (lldpctl_atom_set_int(config,
+ lldpctl_k_config_reset, 1) == NULL) {
+ log_warnx("lldpctl", "unable to ask lldpd to reset operations. %s",
+ lldpctl_last_strerror(conn));
+ lldpctl_atom_dec_ref(config);
+ return 0;
+ }
+ log_info("lldpctl", "lldpd should reset configs");
+ lldpctl_atom_dec_ref(config);
+ return 1;
+}
+
This whole function should use tabs instead of spaces for indentation.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#228 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/APvfsyw5sy2JnxDx8Sh3dcqdxo4OJOV9ks5r0zwbgaJpZM4M7VBg>
.
|
|
So, an update from my side. This would probably need to be picked up by someone else; and probably re-designed. I don't know if there is direct interest [within Riverbed] to continue this. Probably other stuff is more important. This can be left as-is [from my side] as a reference-point. Thanks |
|
We can keep it open. I don't mind having opened PR. This is easier to find than a closed PR. I have also very little time and have some other issues to handle before this. So, let's not make promises here! |
this is in response to #153
not sure if calling lldpd_cleanup(cfg) in daemon/client.c is enough to reset the configuration.