Re: [PATCH kvmtool] Makefile: 'lvm version' works incorrect. Because CFLAGS can not get sub-make variable $(KVMTOOLS_VERSION)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 10 Dec 2021 10:55:40 +0800
"haibiao.xiao" <haibiao.xiao@xxxxxxxxx> wrote:

Hi,

> On Thu, 9 Dec 2021 15:57:46 +0000, Andre Przywara wrote:
> > On Sat,  4 Dec 2021 14:14:36 +0800
> > "haibiao.xiao" <haibiao.xiao@xxxxxxxxx> wrote:
> > 
> > Hi,
> >   
> >> From: "haibiao.xiao" <xiaohaibiao331@xxxxxxxxxxx>
> >>
> >> Command 'lvm version' works incorrect.
> >> It is expected to print:
> >>
> >>     # ./lvm version
> >>     # kvm tool [KVMTOOLS_VERSION]
> >>
> >> but the KVMTOOLS_VERSION is missed:
> >>
> >>     # ./lvm version
> >>     # kvm tool
> >>
> >> The KVMTOOLS_VERSION is defined in the KVMTOOLS-VERSION-FILE file which
> >> is included at the end of Makefile. Since the CFLAGS is a 'Simply
> >> expanded variables' which means CFLAGS is only scanned once. So the
> >> definetion of KVMTOOLS_VERSION at the end of Makefile would not scanned
> >> by CFLAGS. So the '-DKVMTOOLS_VERSION=' remains empty.
> >>
> >> I fixed the bug by moving the '-include $(OUTPUT)KVMTOOLS-VERSION-FILE'
> >> before the CFLAGS.  
> > 
> > While this is indeed a bug that this patch fixes, I wonder if we should
> > actually get rid of this whole versioning attempt altogether at this
> > point. Originally this was following the containing kernel version, but
> > it is stuck ever since at v3.18, without any change.
> > 
> > So either we introduce proper versioning (not sure it's worth it?), or we
> > just remove all code that pretends to print a version number? Or just
> > hardcode v3.18 into the printf, at least for now? At the very least I
> > think we don't need a KVMTOOLS-VERSION-FILE anymore.
> >   
> 
> Thanks for your reply. 
> 
> The reason I look at this project is tend to learn something about kvm. 
> With the version number I can tell which kernel(kvm) is compatible.

I am afraid this is not what the version number tells you. The Linux
kernel provides a stable interface to userland, that includes KVM.
So you should be able to run any kvmtool version on any host kernel
(ignoring bugs). Granted, you will only get the features implemented in
both parts, but this only applies to new features (like PMU
virtualisation), not the main functionality.

> Although it is stuck at v3.18, there still some commits in recent 
> months, which means the kvmtool still changing according to the kvm 
> features.

Sure, we keep it alive, but adapting to new kernel features is only one
part of that. Recently we mostly improved functionality (like
adding firmware support, emulating later PCIe versions, ...) and were
fixing bugs, independent of the kernel version.

> So I think what's kvmtool need is a version control, but not 
> remove/hardcode.

So whether that's really worth it, is the question. If you need some number
to compare, distributions tend to use the date of the last commit for that.

Cheers,
Andre


> 
> Thanks,
> haibiao.xiao
> > Cheers,
> > Andre
> >   
> >>
> >> Signed-off-by: haibiao.xiao <xiaohaibiao331@xxxxxxxxxxx>
> >> ---
> >>  Makefile | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index bb7ad3e..9afb5e3 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -17,6 +17,7 @@ export E Q
> >>  
> >>  include config/utilities.mak
> >>  include config/feature-tests.mak
> >> +-include $(OUTPUT)KVMTOOLS-VERSION-FILE
> >>  
> >>  CC	:= $(CROSS_COMPILE)gcc
> >>  CFLAGS	:=
> >> @@ -559,5 +560,4 @@ ifneq ($(MAKECMDGOALS),clean)
> >>  
> >>  KVMTOOLS-VERSION-FILE:
> >>  	@$(SHELL_PATH) util/KVMTOOLS-VERSION-GEN $(OUTPUT)
> >> --include $(OUTPUT)KVMTOOLS-VERSION-FILE
> >> -endif
> >> +endif
> >> \ No newline at end of file  




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux