Comment # 7 on bug 1037309 from Tom���� Chv��tal

(In reply to zhen ren from comment #6)
> 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...

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 ;)