Crmv1 #129
Open
Crmv1 #129
+536
−35
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Client command grant/revoke was returning success exit code on some failures (site not configured, arbitrator, no tickets given). Test case is running `booth grant -s $IP; echo $?`, where IP is not configured in the config file. Patch fixes this behavior so error code is returned. Signed-off-by: Jan Friesse <[email protected]>
attr_get and attr_list were allocation GString but they were calling g_string_free with second argument set to FALSE, what means GLib was returning character data to caller (see g_string_free for complete documentation) instead of freeing them. This return value was ignored by attr_* functions so memory leaked. Patch to fix this problem is simple - just set second argument of g_string_free to TRUE so GLib takes care of freeing all data. Signed-off-by: Jan Friesse <[email protected]>
Ticket attribute hash table is created only when some attribute exists. If it doesn't, list and get operations were producing glib assert. Patch adds check in attr_get and attr_list so hash table is used only when it has been initialized. Signed-off-by: Jan Friesse <[email protected]>
Python 3.12 warns about invalid escape sequence '\s'. It is still correctly converted, but I guess it is better to escape '\' character properly to avoid future breakage. Signed-off-by: Jan Friesse <[email protected]>
Instead of hand-written changelog export changelog from git during tarball creation. Signed-off-by: Jan Friesse <[email protected]>
Makefile used to tag and create official tarballs. Signed-off-by: Jan Friesse <[email protected]>
Signed-off-by: Jan Friesse <[email protected]>
make distcheck calls configure script with --prefix to allow
installation to tmp directory. ocfdir is not using ${prefix} and instead
contains absolute path (either taken from resource-agents.pc or
hardcoded one) so make install fails when running as non-root user.
Solution is taken from pacemaker project and it relies on setting
AM_DISTCHECK_CONFIGURE_FLAGS so --with-ocfdir is added with directory
where user has write permissions.
Big thanks to Fabio M. Di Nitto <[email protected]> for finding this
solution.
Signed-off-by: Jan Friesse <[email protected]>
Signed-off-by: Jan Friesse <[email protected]>
|
@dmuhamedagic Hi, I was thinking about this PR whole yesterday and was asking myself if same couldn't be achieved by having generic option for ticket called (for example) "notify-script" which would contain path to executable file (most probably shell script, but for this crmv1 it may make sense to have it written in python) which would be executed on ticket acquire/loss? This would allow us to simplify code a lot, and specially get rid of bigger changes in C, give us generic functionality and solve (or at least partly solve) #99. Also such functionality would be probably really used even with pcmk and crmv1 could be great example how to use such notify functionality. But I don't want to stop you from having fun with the code, it's great you found time to write some booth code again ;) so take my comment just as my opinion/idea which you may like and maybe not. |
|
Hi Jan,
On Tue, Oct 24, 2023 at 12:08:41AM -0700, Jan Friesse wrote:
@dmuhamedagic Hi, I was thinking about this PR whole yesterday
and was asking myself if same couldn't be achieved by having
generic option for ticket called (for example) "notify-script"
which would contain path to executable file (most probably
shell script, but for this crmv1 it may make sense to have it
written in python) which would be executed on ticket
acquire/loss? This would allow us to simplify code a lot, and
specially get rid of bigger changes in C, give us generic
functionality and solve (or at least partly solve) #99. Also
such functionality would be probably really used even with pcmk
and crmv1 could be great example how to use such notify
functionality.
This actually includes some refactoring which improves the C
code.
But, if you find a better solution, I'm all for it! My only need
is the need for the functionality :)
Cheers,
Dejan
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please don't merge, not tested yet.