[Bug 1037309] New: [lvmlockd regression] lvmlockd always fails to behave in latest version 2.02.170
http://bugzilla.suse.com/show_bug.cgi?id=1037309 Bug ID: 1037309 Summary: [lvmlockd regression] lvmlockd always fails to behave in latest version 2.02.170 Classification: openSUSE Product: openSUSE Tumbleweed Version: Current Hardware: Other OS: Other Status: NEW Severity: Major Priority: P5 - None Component: High Availability Assignee: lmb@suse.com Reporter: zren@suse.com QA Contact: qa-bugs@suse.de Found By: --- Blocker: --- tw1:~ # vgcreate --shared vgtest /dev/vdb Using a shared lock type requires lvmlockd. Failed to detect a running lock manager to select lock type. (gdb) bt #0 lockd_running_lock_type (cmd=0x555555b8bfa0, found_multiple=<synthetic pointer>) at ../include/lvmlockd.h:228 #1 vgcreate_params_set_from_args (cmd=cmd@entry=0x555555b8bfa0, vp_new=vp_new@entry=0x7fffffffddf0, vp_def=vp_def@entry=0x7fffffffde40) at toollib.c:950 #2 0x00005555555c42ac in vgcreate (cmd=0x555555b8bfa0, argc=<optimized out>, argv=0x7fffffffe380) at vgcreate.c:55 #3 0x00005555555a604f in lvm_run_command (cmd=cmd@entry=0x555555b8bfa0, argc=<optimized out>, argc@entry=4, argv=<optimized out>, argv@entry=0x7fffffffe368) at lvmcmdline.c:2928 #4 0x00005555555a6d15 in lvm2_main (argc=4, argv=0x7fffffffe368) at lvmcmdline.c:3454 #5 0x00007ffff6b31541 in __libc_start_main () from /lib64/libc.so.6 #6 0x000055555558697a in _start () at ../sysdeps/x86_64/start.S:120 (gdb) list 223 return 1; 224 } 225 226 static inline const char *lockd_running_lock_type(struct cmd_context *cmd, int *found_multiple) 227 { 228 log_error("Using a shared lock type requires lvmlockd."); 229 return NULL; 230 } -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c1
zhen ren
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c2
--- Comment #2 from zhen ren
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c3
--- Comment #3 from Tomáš Chvátal
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c4
--- Comment #4 from zhen ren
Hmm, it seems upstream is a bit funny there.
I will check the commits between the version 168 and 170.
You can compile separate lockd as we do but it won't be loaded.
You mean splitting lockd out from lvm2-clvm? Sorry, but what do you mean by "it won't be loaded"?
If you check the .h file it states for the LVMLOCKD_SUPPORT undefined to inline those functions (see 'static inline') on the declarations.
Yes, macro LVMLOCKD_SUPPORT should be "1" so that the real code can be compiled in.
Unless I am mistaken this means for the compiler to actually put the code in the binary rather than issue follow up call for the function later on.
Yes, the problem is, i think, why LVMLOCKD_SUPPORT is not "1"? I'm not familiar with make-auto tools, but I will dig more. -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c5
--- Comment #5 from Tomáš Chvátal
Hi!
(In reply to Tomáš Chvátal from comment #3)
Hmm, it seems upstream is a bit funny there.
I will check the commits between the version 168 and 170.
You can compile separate lockd as we do but it won't be loaded.
You mean splitting lockd out from lvm2-clvm? Sorry, but what do you mean by "it won't be loaded"?
We compile lockd as a separate package, but the main files are provided in lvm2.spec Those main files like vgcreate are compiled with LVMLOCKD_SUPPORT=0
If you check the .h file it states for the LVMLOCKD_SUPPORT undefined to inline those functions (see 'static inline') on the declarations.
Yes, macro LVMLOCKD_SUPPORT should be "1" so that the real code can be compiled in.
Unless I am mistaken this means for the compiler to actually put the code in the binary rather than issue follow up call for the function later on.
Yes, the problem is, i think, why LVMLOCKD_SUPPORT is not "1"? I'm not familiar with make-auto tools, but I will dig more.
It is correctly 0 for the vgcreate and others as per above. Problem is that this header file is written in such a way that it breaks if you try to overload the function later on to actually allow the loading of the full proper lockd functions. The detection like this should not be decided by the compilation flag but done in runtime like in clvm... Something like https://build.opensuse.org/package/view_file/Base:System/lvm2/pvmove_support... And of course the header must not inline those functions in either case. -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c6
--- Comment #6 from zhen ren
(In reply to zhen ren from comment #4)
Hi!
(In reply to Tomáš Chvátal from comment #3)
Hmm, it seems upstream is a bit funny there.
I will check the commits between the version 168 and 170.
You can compile separate lockd as we do but it won't be loaded.
You mean splitting lockd out from lvm2-clvm? Sorry, but what do you mean by "it won't be loaded"?
We compile lockd as a separate package, but the main files are provided in lvm2.spec
Those main files like vgcreate are compiled with LVMLOCKD_SUPPORT=0
OK! I understand the problem now. So, it became problem after you moved lockd from lvm2.spec to lvm2-clvm.spec?
If you check the .h file it states for the LVMLOCKD_SUPPORT undefined to inline those functions (see 'static inline') on the declarations.
Yes, macro LVMLOCKD_SUPPORT should be "1" so that the real code can be compiled in.
Unless I am mistaken this means for the compiler to actually put the code in the binary rather than issue follow up call for the function later on.
Yes, the problem is, i think, why LVMLOCKD_SUPPORT is not "1"? I'm not familiar with make-auto tools, but I will dig more.
It is correctly 0 for the vgcreate and others as per above.
Yes.
Problem is that this header file is written in such a way that it breaks if
Do you think it's a problem that should be fixed upstream? But looks upstream doesn't break because they don't build lvmlockd the way like us, right?
you try to overload the function later on to actually allow the loading of the full proper lockd functions.
Sorry, I don't understand what you mean by "you try to overload the function later". Could you please be more specific? IMHO, C language has not "fuction overload" support, but C++ does. So, what's "overload the function" here? My understanding is, like you've mentioned above, main files (like vgcreate, etc.) are built with "LVMLOCKD_SUPPORT=0", so the compiler built them with those "empty" functions which are intended for NON-lvmlockd building. That's it.
The detection like this should not be decided by the compilation flag but done in runtime like in clvm...
It's decided during pre-compilation time by "#ifdef...#else..." instructions. I don't think it's a problem, but a usual solution I've seen in other projects like: 1. https://github.com/renzhengeek/ocfs2-tools/blob/master/libo2dlm/o2dlm.c#L112...
Something like https://build.opensuse.org/package/view_file/Base:System/lvm2/ pvmove_support_clustered_vg.diff?expand=1
That's about implementation then, which would need much changes in upstream...
And of course the header must not inline those functions in either case.
Why? I think removing all "inline" in the header couldn't solve this problem, right? Thanks. -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c7
--- Comment #7 from Tomáš Chvátal
Hi Tomas!
(In reply to Tomáš Chvátal from comment #5)
(In reply to zhen ren from comment #4)
Hi!
(In reply to Tomáš Chvátal from comment #3)
Hmm, it seems upstream is a bit funny there.
I will check the commits between the version 168 and 170.
You can compile separate lockd as we do but it won't be loaded.
You mean splitting lockd out from lvm2-clvm? Sorry, but what do you mean by "it won't be loaded"?
We compile lockd as a separate package, but the main files are provided in lvm2.spec
Those main files like vgcreate are compiled with LVMLOCKD_SUPPORT=0
OK! I understand the problem now. So, it became problem after you moved lockd from lvm2.spec to lvm2-clvm.spec?
If you check the .h file it states for the LVMLOCKD_SUPPORT undefined to inline those functions (see 'static inline') on the declarations.
Yes, macro LVMLOCKD_SUPPORT should be "1" so that the real code can be compiled in.
Unless I am mistaken this means for the compiler to actually put the code in the binary rather than issue follow up call for the function later on.
Yes, the problem is, i think, why LVMLOCKD_SUPPORT is not "1"? I'm not familiar with make-auto tools, but I will dig more.
It is correctly 0 for the vgcreate and others as per above.
Yes.
Problem is that this header file is written in such a way that it breaks if
Do you think it's a problem that should be fixed upstream? But looks upstream doesn't break because they don't build lvmlockd the way like us, right?
you try to overload the function later on to actually allow the loading of the full proper lockd functions.
Sorry, I don't understand what you mean by "you try to overload the function later". Could you please be more specific? IMHO, C language has not "fuction overload" support, but C++ does. So, what's "overload the function" here?
My understanding is, like you've mentioned above, main files (like vgcreate, etc.) are built with "LVMLOCKD_SUPPORT=0", so the compiler built them with those "empty" functions which are intended for NON-lvmlockd building. That's it.
The detection like this should not be decided by the compilation flag but done in runtime like in clvm...
It's decided during pre-compilation time by "#ifdef...#else..." instructions. I don't think it's a problem, but a usual solution I've seen in other projects like: 1. https://github.com/renzhengeek/ocfs2-tools/blob/master/libo2dlm/o2dlm.c#L112...
Something like https://build.opensuse.org/package/view_file/Base:System/lvm2/ pvmove_support_clustered_vg.diff?expand=1
That's about implementation then, which would need much changes in upstream...
Yup, we need to tell upstream. Or patch it like we did with cluster part.
And of course the header must not inline those functions in either case.
Why? I think removing all "inline" in the header couldn't solve this problem, right?
In theory, we would rely on the compiler to not inline the code if it is not specified, there are some complexity calculations whether to do it or not, try it and let us see ;) -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c8
--- Comment #8 from zhen ren
That's about implementation then, which would need much changes in upstream...
Yup, we need to tell upstream. Or patch it like we did with cluster part.
Honestly, I'm not going to do that, because: 1. I don't know exactly what the problem is in upstream code, and how much efforts it takes to work out a patch for it. It's useless to just tell upstream that "Hi, lvmlock doesn't work for us if we package it separately", without giving strong reasons and respective patch set. 2. Patch it like what? Sorry, I don't catch up with you here. I don't think it's worthy changing code much so that we can just build lvmlockd separately. Honestly, I neither have time nor interest to do so. 3. As you can see in the mailist, upstream is working a stable release now: """ Version 2.02.171 Another development checkpoint release. We have stopped adding features as we work towards the next stable release, but please still only use this release for testing purposes while we continue to find and fix bugs. """ This is not a bug, but maybe a design defect. I can imagine it's hard to persuade upstream to accept big change for it even if we work out a patch set.
And of course the header must not inline those functions in either case.
Why? I think removing all "inline" in the header couldn't solve this problem, right?
In theory, we would rely on the compiler to not inline the code if it is not specified, there are some complexity calculations whether to do it or not, try it and let us see ;)
Only removing the "inline" modifiers would not make a difference, I realize. In addition, having the call declarations in header file and moving their definitions (both real and empty functions) to source files may work around. Let me have a try;-) Thanks, Eric -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c9
--- Comment #9 from zhen ren
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c10
--- Comment #10 from zhen ren
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c11
zhen ren
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c12
zhen ren
http://bugzilla.suse.com/show_bug.cgi?id=1037309
zhen ren
http://bugzilla.suse.com/show_bug.cgi?id=1037309
Stefan Behlert
http://bugzilla.suse.com/show_bug.cgi?id=1037309
Martin Pluskal
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c16
zhen ren
http://bugzilla.suse.com/show_bug.cgi?id=1037309
http://bugzilla.suse.com/show_bug.cgi?id=1037309#c17
zhen ren
participants (1)
-
bugzilla_noreply＠novell.com