[Asterisk-code-review] sd notify (systemd status notifications) support (asterisk[master])

[Corey Farrell]

Corey Farrell has posted comments on this change.

Change subject: sd_notify (systemd status notifications) support
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

> (1 comment)
 > 
 > A cherry-pick to 13 should probably not be a problem (and I'll need
 > it myself for my package). It may take a while for me to get to it,
 > though.
 > 
 > Also, just to give an idea for future improvements: sd_notify()
 > also has STATUS= (a free-form text status line). I couldn't think
 > of anything useful to do with it. If this is used, there may be a
 > need to wrap sd_notifyf() as well.

I'd say don't worry about STATUS= for now, that's a separate feature.  I feel like STATUS= should be user definable, a dialplan function would allow AMI/ARI to set.

One possible issue: do we need to unset the SD environment ($NOTIFY_SOCKET) before forking in ast_safe_system?

https://gerrit.asterisk.org/#/c/3094/2/main/io.c
File main/io.c:

Line 389: #include <systemd/sd-daemon.h>
> Adding a header that deep inside is normally not recommended. I preferred t
Putting the header in it's own ifdef near the other system headers would be better.


-- 
To view, visit https://gerrit.asterisk.org/3094
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied6a59dafd5ef331c5c7ae8f3ccd2dfc94be7811
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Tzafrir Cohen <tzafrir.cohen at xorcom.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Tzafrir Cohen <tzafrir.cohen at xorcom.com>
Gerrit-HasComments: Yes