Fix: sbd-pacemaker: sync with pacemakerd for robustness #113
Conversation
wenningerk
changed the title
wenningerk
force-pushed
the
sync_with_pacemakerd_generic_ipc_api
branch
from
a10ffa1 to
b1f7e47
Compare
|
retest this please |
wenningerk
changed the title
|
retest this please |
kgaillot
left a comment
kgaillot
left a comment
There was a problem hiding this comment.
Looks good. BTW title/commit message has typo "pacemkaer"
src/sbd-pacemaker.c
Outdated
| #include <crm/common/ipc_pacemakerd.h> | ||
|
|
||
| static pcmk_ipc_api_t *pacemakerd_api = NULL; | ||
| time_t last_ok = (time_t) 0; |
| char timeout_sysrq_char = 'b'; | ||
| bool move_to_root_cgroup = true; | ||
| bool enforce_moving_to_root_cgroup = false; | ||
| bool sync_resource_startup = false; |
There was a problem hiding this comment.
It would be nice if the default were true when USE_PACEMAKERD_API is set, but pacemaker would have no way of knowing that. I can think of a couple of ways around that:
- distros can change the sysconfig setting when they know their versions are compatible
- or sysconfig could be a .in file substituted at configure time
There was a problem hiding this comment.
yes, a pitty that the default has to be false to serve its purpose :-(
it was the intention to change the sysconfig-setting from the distro-spec-file.
there is already a sed-line for s390x. having a configure configure switch
(set by the distro-spec-file + maybe some additional auto-magic derived
from what is seen in pacemaker) would probably be more elegant though.
| #if !USE_PACEMAKERD_API | ||
| if (sync_resource_startup) { | ||
| fprintf(stderr, "Failed to sync resource-startup as " | ||
| "SBD was built against pacemaker not supporting pacemakerd-API.\n"); |
There was a problem hiding this comment.
that is an arguable point. using fprintf is at least consistent as all early-error-exits are using fprintf so far. if changed I guess it should be done consistently as an own commit.
wenningerk
force-pushed
the
sync_with_pacemakerd_generic_ipc_api
branch
from
c80b7aa to
65674d7
Compare
wenningerk
changed the title
|
looks good to me, though travis disagrees :) |
|
Travis has an issue with loopback mounts. CI needs 3 loopback-devices. The 3rd one is now /dev/loop8 which is one too much for the default of 8 available loopback devices. Obviously the stuff travis does on the VMs before actually starting the docker-containers now needs one loopback-device more than before. Haven't figured out yet how to pass something into the cmdline travis uses for the VM kernel. Reason why it just hits the x86_64 tests is that they are the only ones offering system-virtualization where there are loopback-devices available at all. All the other environments are containers where I have to configure the preload library for sbd so that it accepts plain files. As the loopback-device + device-mapper simulation is nearer to real devices I'd like to keep that though if possible somehow. |
State query ping of pacemakerd prevents pacemakerd from starting any sub-daemons (and thus services) if sbd can't reach it via ipc. As a health-check get timestamp from pacemakerd. On shudown fetch info about graceful shutdown from pacemakerd. Use new pacemakerd-api provided by pacemaker.
wenningerk
force-pushed
the
sync_with_pacemakerd_generic_ipc_api
branch
from
65674d7 to
f4d38a0
Compare
2 participants
State query ping of pacemakerd prevents pacemakerd from
starting any sub-daemons (and thus services) if sbd can't
reach it via ipc. As a health-check get timestamp from
pacemakerd. On shudown fetch info about graceful
shutdown from pacemakerd.
Use new pacemakerd-api provided by pacemaker.
This refactors #106 sync with pacemakerd
to use new pacemakerd-api in pacemaker based on
generic client api framework.
Added SBD_SYNC_RESOURCE_STARTUP defaulting to 'no' so
that upgrading pacemaker without sbd doesn't lead to pacemaker
locked waiting to be pinged by sbd.