On Mon, 2020-08-17 at 23:33 +0800, Zhiqiang Liu wrote:
>
> On 2020/8/17 16:35, Martin Wilck wrote:
> > On Sun, 2020-08-16 at 09:44 +0800, Zhiqiang Liu wrote:
> > > We adopt static char* array (sd_notify_status_msg) in
> > > sd_notify_status func, so it looks more simpler and easier
> > > to expand.
> > >
> > > Signed-off-by: Zhiqiang Liu <liuzhiqiang26@xxxxxxxxxx>
> > > Signed-off-by: lixiaokeng <lixiaokeng@xxxxxxxxxx>
> > > ---
> > > multipathd/main.c | 26 ++++++++++++--------------
> > > 1 file changed, 12 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/multipathd/main.c b/multipathd/main.c
> > > index cab1d0d..a09ccd1 100644
> > > --- a/multipathd/main.c
> > > +++ b/multipathd/main.c
> > > @@ -177,23 +177,21 @@ daemon_status(void)
> > > * I love you too, systemd ...
> > > */
> > > #ifdef USE_SYSTEMD
> > > +static const char *sd_notify_status_msg[DAEMON_STATUS_SIZE] = {
> > > + [DAEMON_INIT] = "STATUS=init",
> > > + [DAEMON_START] = "STATUS=startup",
> > > + [DAEMON_CONFIGURE] = "STATUS=configure",
> > > + [DAEMON_IDLE] = "STATUS=up",
> > > + [DAEMON_RUNNING] = "STATUS=up",
> > > + [DAEMON_SHUTDOWN] = "STATUS=shutdown",
> > > +};
> > > +
> >
> > This repetition of "STATUS=" looks clumsy. It's not your fault,
> > because
> > the current code does the same thing. But if you want to clean this
> > up,
> > please create the notification string in a dynamic buffer, and use
> > daemon_status() for those cases where it applies.
> >
> > Regards
> > Martin
> >
> Thanks for your reply.
> Besides the prefixes "STATUS=", there are also some differences
> between
> sd_notify_status_msg[DAEMON_IDLE|DAEMON_RUNNING] and
> demon_status_msg[DAEMON_IDLE|DAEMON_RUNNING].
>
> For example,
> sd_notify_status_msg[DAEMON_RUNNING] = "STATUS=up",
> demon_status_msg[DAEMON_RUNNING] = "running"
>
> So if we create the notification string in a dynamic buffer with
> using daemon_status, we have to make some special judgement
> on DAEMON_RUNNING and DAEMON_IDLE status. This may be why
> the sd_notify_status func was created.
>
> We can implement both solutions. Martin, which one do you prefer?
That's what I meant with "where it applies". You treat the IDLE and
RUNNING cases first (probably in a switch statement), and use
daemon_status() for the "default" case.
The reason we don't differentiate between "running" and "idle" in the
sd_notify() code path is that the daemon switches between these states
very often (at least once per tick) and these notifications causes lots
of dbus traffic without much value.
Regards
Martin
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
