Comment # 6 on bug 1037309 from zhen ren

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#L1121

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