Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crmv1 #129

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Crmv1 #129

wants to merge 13 commits into from

Conversation

dmuhamedagic
Copy link

Please don't merge, not tested yet.

jfriesse and others added 13 commits October 20, 2023 14:37
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 <jfriesse@redhat.com>
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 <jfriesse@redhat.com>
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 <jfriesse@redhat.com>
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 <jfriesse@redhat.com>
Instead of hand-written changelog export changelog from git during
tarball creation.

Signed-off-by: Jan Friesse <jfriesse@redhat.com>
Makefile used to tag and create official tarballs.

Signed-off-by: Jan Friesse <jfriesse@redhat.com>
Signed-off-by: Jan Friesse <jfriesse@redhat.com>
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 <fdinitto@redhat.com> for finding this
solution.

Signed-off-by: Jan Friesse <jfriesse@redhat.com>
Signed-off-by: Jan Friesse <jfriesse@redhat.com>
@jfriesse
Copy link
Member

jfriesse commented Oct 24, 2023

@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.

@dmuhamedagic
Copy link
Author

dmuhamedagic commented Oct 24, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants