Q: status of kvm/ subdir in qemu-kvm tarball?

Q: status of kvm/ subdir in qemu-kvm tarball?

[Michael Tokarev]

What's the current status of files in kvm/ subdirectory
in the qemu-kvm distribution (0.14 or git) ?

As far as I can see, all that is old and unused stuff,
which only confuses users.  It is not used during build
the Makefiles are obsolete and refers to non-existed
files/directories, some remaining bits (like vgabios)
are too old to be useful for current qemu-kvm, etc.

Maybe it's better to just drop the whole thing to stop
confusion?

If it may be needed in the future, it can be taken from
older source tarballs.  Just letting that stuff to bitrot
isn't a nice thing to do. ;)

I mentioned this yesterday to Avi (my question was
especially about vgabios), and he told me to send a
patch with justification.  But it's way too simpler
to just do `git rm -r kvm' than apply a patch to
remove the whole thing.

Thanks!

/mjt

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Jan Kiszka]

On 2011-02-23 12:04, Michael Tokarev wrote:
> What's the current status of files in kvm/ subdirectory
> in the qemu-kvm distribution (0.14 or git) ?
> 
> As far as I can see, all that is old and unused stuff,
> which only confuses users.  It is not used during build
> the Makefiles are obsolete and refers to non-existed
> files/directories, some remaining bits (like vgabios)
> are too old to be useful for current qemu-kvm, etc.
> 
> Maybe it's better to just drop the whole thing to stop
> confusion?
> 
> If it may be needed in the future, it can be taken from
> older source tarballs.  Just letting that stuff to bitrot
> isn't a nice thing to do. ;)
> 
> I mentioned this yesterday to Avi (my question was
> especially about vgabios), and he told me to send a
> patch with justification.  But it's way too simpler
> to just do `git rm -r kvm' than apply a patch to
> remove the whole thing.

I think there are a few bits still in use:

- kvm/kvm_stat
- kvm/include (latest kernel headers) *)

Otherwise, removing this cruft would be good.

Jan

*) This should really be discussed upstream again: Carry private copy of
kvm headers or rely on distro / kvm-kmod to provide required defines,
types etc.? I'm more and more in favor of option 1.

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Michael Tokarev]

23.02.2011 14:31, Jan Kiszka wrote:
> On 2011-02-23 12:04, Michael Tokarev wrote:
>> What's the current status of files in kvm/ subdirectory
>> in the qemu-kvm distribution (0.14 or git) ?
[]
> I think there are a few bits still in use:
> 
> - kvm/kvm_stat

this is a tiny useful tool, yes, and we package
it.

> - kvm/include (latest kernel headers) *)

This directory is always used (in configure)
unless -kerneldir is specified - who uses that
nowadays?  I dunno.

> Otherwise, removing this cruft would be good.
> 
> Jan
> 
> *) This should really be discussed upstream again: Carry private copy of
> kvm headers or rely on distro / kvm-kmod to provide required defines,
> types etc.? I'm more and more in favor of option 1.

Yes indeed, having current defines of all the features
we actually support is a good idea.

/mjt

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Jan Kiszka]

On 2011-02-23 12:42, Michael Tokarev wrote:
> 23.02.2011 14:31, Jan Kiszka wrote:
>> On 2011-02-23 12:04, Michael Tokarev wrote:
>>> What's the current status of files in kvm/ subdirectory
>>> in the qemu-kvm distribution (0.14 or git) ?
> []
>> I think there are a few bits still in use:
>>
>> - kvm/kvm_stat
> 
> this is a tiny useful tool, yes, and we package
> it.
> 
>> - kvm/include (latest kernel headers) *)
> 
> This directory is always used (in configure)
> unless -kerneldir is specified - who uses that
> nowadays?  I dunno.

It may not be relevant to distros, provided they manage to keep their
/usr/include/linux stuff in sync with the kernel they ship. But if you
build qemu-kvm for a kernel (or kvm-kmod) that is more recent than your
user space headers, this matters.

> 
>> Otherwise, removing this cruft would be good.
>>
>> Jan
>>
>> *) This should really be discussed upstream again: Carry private copy of
>> kvm headers or rely on distro / kvm-kmod to provide required defines,
>> types etc.? I'm more and more in favor of option 1.
> 
> Yes indeed, having current defines of all the features
> we actually support is a good idea.

Would you like to propose this upstream in form of a patch series (that
demonstrates its usefulness by dropping tones of #ifdefs)?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Michael Tokarev]

23.02.2011 15:58, Jan Kiszka wrote:
> On 2011-02-23 12:42, Michael Tokarev wrote:
[]
>>> - kvm/include (latest kernel headers) *)
>>
>> This directory is always used (in configure)
>> unless -kerneldir is specified - who uses that
>> nowadays?  I dunno.
> 
> It may not be relevant to distros, provided they manage to keep their
> /usr/include/linux stuff in sync with the kernel they ship. But if you
> build qemu-kvm for a kernel (or kvm-kmod) that is more recent than your
> user space headers, this matters.

It only matters if you want to build qemu-kvm for OLDER
kernel without support of some features which are used by
the userspace you're building.  Provided 3 things:

 1) qemu-kvm userspace actually checks a feature before
    using it, and omits some code if the feature isn't
    available.  This means tons of #ifdefs, for a very
    little gain,

 2) kernel interface stays backward-compatible, and

 3) the headers included in qemu-kvm tarball actually
    matches the feature set in the userspace part.

Using more RECENT kernel headers to build userspace which
don't know about the more recent features anyway should
result in exactly the same binary, there's no reason to
use more recent headers.

>>> *) This should really be discussed upstream again: Carry private copy of
>>> kvm headers or rely on distro / kvm-kmod to provide required defines,
>>> types etc.? I'm more and more in favor of option 1.
>>
>> Yes indeed, having current defines of all the features
>> we actually support is a good idea.

That's the key: current features we actually support.
If kernel headers (be them embedded into the userspace
tarball or already installed on the system or headers
from just built kernel.org git tree) define less than
that, we either have tons of #ifdefs or just fail at
build/configure time telling "use more recent headers".
If kernel headers define more than we can understand,
we just ignore the rest...

> Would you like to propose this upstream in form of a patch series (that
> demonstrates its usefulness by dropping tones of #ifdefs)?

..unless I completely missed the point.  Which upstream
you're talking about -- qemu or kernel?  Where the #ifdefs
are, what they're for currently are they really needed?

Does qemu[-kvm] want to stay compatible with older kernels,
or kernel headers?  If yes, how old is old?

/mjt

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Jan Kiszka]

On 2011-02-23 14:15, Michael Tokarev wrote:
> 23.02.2011 15:58, Jan Kiszka wrote:
>> On 2011-02-23 12:42, Michael Tokarev wrote:
> []
>>>> - kvm/include (latest kernel headers) *)
>>>
>>> This directory is always used (in configure)
>>> unless -kerneldir is specified - who uses that
>>> nowadays?  I dunno.
>>
>> It may not be relevant to distros, provided they manage to keep their
>> /usr/include/linux stuff in sync with the kernel they ship. But if you
>> build qemu-kvm for a kernel (or kvm-kmod) that is more recent than your
>> user space headers, this matters.
> 
> It only matters if you want to build qemu-kvm for OLDER
> kernel without support of some features which are used by
> the userspace you're building.  Provided 3 things:

It also matters if you are building for a kernel more recent than the
headers you have (wherever they came from). Think of the scenario that
you build a recent customized kernel for you host. Then you also need to
remember running "make header_install" and picking up those headers for
the qemu build.

> 
>  1) qemu-kvm userspace actually checks a feature before
>     using it, and omits some code if the feature isn't
>     available.  This means tons of #ifdefs, for a very
>     little gain,
> 
>  2) kernel interface stays backward-compatible, and
> 
>  3) the headers included in qemu-kvm tarball actually
>     matches the feature set in the userspace part.
> 
> Using more RECENT kernel headers to build userspace which
> don't know about the more recent features anyway should
> result in exactly the same binary, there's no reason to
> use more recent headers.
> 
>>>> *) This should really be discussed upstream again: Carry private copy of
>>>> kvm headers or rely on distro / kvm-kmod to provide required defines,
>>>> types etc.? I'm more and more in favor of option 1.
>>>
>>> Yes indeed, having current defines of all the features
>>> we actually support is a good idea.
> 
> That's the key: current features we actually support.
> If kernel headers (be them embedded into the userspace
> tarball or already installed on the system or headers
> from just built kernel.org git tree) define less than
> that, we either have tons of #ifdefs or just fail at
> build/configure time telling "use more recent headers".
> If kernel headers define more than we can understand,
> we just ignore the rest...

Right. Kernel headers tend to change in a way that is build-wise
backward compatible.

> 
>> Would you like to propose this upstream in form of a patch series (that
>> demonstrates its usefulness by dropping tones of #ifdefs)?
> 
> ..unless I completely missed the point.  Which upstream
> you're talking about -- qemu or kernel?

This is a user space topic, and qemu is upstream in this case.

>  Where the #ifdefs
> are, what they're for currently are they really needed?

In qemu, all KVM related #ifdefs should be needed, I've recently walk
through them again and consolidated the checks. I'm not that sure about
qemu-kvm, even in my current consolidation tree. But that can be sorted
out separately (e.g. while porting more feature upstream, cleaning them
up at this chance).

> 
> Does qemu[-kvm] want to stay compatible with older kernels,
> or kernel headers?  If yes, how old is old?

Our current minimum requirement is the KVM ABI provided by 2.6.29. The
runtime check is detecting this via the existence of certain key
capabilities. The related build-time check takes place during configure,
probing a list of minimally required KVM_CAPs.

This last check would become obsolete if we always pick the headers from
a qemu-local copy. Furthermore, we could drop "#ifdef KVM_CAP" around
features that are more recent than our kernel baseline. Runtime checking
will still be required, of course.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Michael Tokarev]

23.02.2011 16:45, Jan Kiszka wrote:
> On 2011-02-23 14:15, Michael Tokarev wrote:
[]
>> It only matters if you want to build qemu-kvm for OLDER
>> kernel without support of some features which are used by
>> the userspace you're building.  Provided 3 things:
> 
> It also matters if you are building for a kernel more recent than the
> headers you have (wherever they came from). Think of the scenario that
> you build a recent customized kernel for you host. Then you also need to
> remember running "make header_install" and picking up those headers for
> the qemu build.

Um. We're going in circles :)

I mean, qemu userspace does not need kernels more recent
than it can understand.  So there's no need to use
external headers instead of using a copy embedded in
the tarball - which may be not up-to-date wrt currently
running kernel or currently installed headers, but the
extra features available there wont be used anyway since
that particular qemu userspace does not understand them
anyway.

The only situation where using kernel headers not provided
by qemu itself is when you want to build qemu for older
kernel and omit some features and runtime tests.  But this
is hardly a good goal to support.

For the above scenario (installing a custom very recent
kernel but forgetting to run `make header_install') to
be of any use, you also need qemu userspace that is
able to use the new feature in this shiny new kernel.

So to sum it up, we've two options (actually 3, but
3rd isn't very interesting):

 1) embed "recent enough" (for this version of qemu)
  kernel headers and always use this copy, or

 2) use system headers and always assume all features
  we actually understand are present in there, and
  fail if they're not.  This can be a configure-time
  check (it already is, sort of), so configure can
  tell the user the minimum required version to
  install.

I've no preference for either of the two, first is
more convinient and second has less duplicates.

(By "qemu" I mean either qemu or qemu-kvm, since the
 two tries to become the same thing)

[]
>> Does qemu[-kvm] want to stay compatible with older kernels,
>> or kernel headers?  If yes, how old is old?
> 
> Our current minimum requirement is the KVM ABI provided by 2.6.29. The
> runtime check is detecting this via the existence of certain key
> capabilities. The related build-time check takes place during configure,
> probing a list of minimally required KVM_CAPs.
> 
> This last check would become obsolete if we always pick the headers from
> a qemu-local copy. Furthermore, we could drop "#ifdef KVM_CAP" around
> features that are more recent than our kernel baseline. Runtime checking
> will still be required, of course.

That check is useful to error out and tell user if the minimum
requiriment is not satisfied.  Keeping lots of ifdefs isn't useful
IMHO.

So I'd say keep --with-kernelheaders, keep the checks in configure
(maybe activate them if --with-kernelheaders is specified), keep
local kernel headers which are recent enough, and remove irrelevant
#ifdefs around new API.

I'm not sure again I understand your statement about removing
#ifdefs in the previous email -- now you say you verified that
all #ifdefs are needed.  They're either needed or can be removed... ;)

But that all went quite far away from dropping old cruft from
kvm/ subdir - as we found out, there are 2 useful pieces in
there, kvm_stat helper and kernel headers, so let's keep them
for now and remove the rest.

Anyway, that's quite long "discussion" about nearly nothing... :)

Thanks!

/mjt

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Jan Kiszka]

On 2011-02-23 15:30, Michael Tokarev wrote:
> 23.02.2011 16:45, Jan Kiszka wrote:
>> On 2011-02-23 14:15, Michael Tokarev wrote:
> []
>>> It only matters if you want to build qemu-kvm for OLDER
>>> kernel without support of some features which are used by
>>> the userspace you're building.  Provided 3 things:
>>
>> It also matters if you are building for a kernel more recent than the
>> headers you have (wherever they came from). Think of the scenario that
>> you build a recent customized kernel for you host. Then you also need to
>> remember running "make header_install" and picking up those headers for
>> the qemu build.
> 
> Um. We're going in circles :)
> 
> I mean, qemu userspace does not need kernels more recent
> than it can understand.  So there's no need to use
> external headers instead of using a copy embedded in
> the tarball - which may be not up-to-date wrt currently
> running kernel or currently installed headers, but the
> extra features available there wont be used anyway since
> that particular qemu userspace does not understand them
> anyway.

Right.

> 
> The only situation where using kernel headers not provided
> by qemu itself is when you want to build qemu for older
> kernel and omit some features and runtime tests.  But this
> is hardly a good goal to support.

I don't think the goal of the #ifdefs was ever some kind of size
optimization but just for the sake of avoiding build breakages.

> 
> For the above scenario (installing a custom very recent
> kernel but forgetting to run `make header_install') to
> be of any use, you also need qemu userspace that is
> able to use the new feature in this shiny new kernel.
> 
> So to sum it up, we've two options (actually 3, but
> 3rd isn't very interesting):
> 
>  1) embed "recent enough" (for this version of qemu)
>   kernel headers and always use this copy, or
> 
>  2) use system headers and always assume all features
>   we actually understand are present in there, and
>   fail if they're not.  This can be a configure-time
>   check (it already is, sort of), so configure can
>   tell the user the minimum required version to
>   install.

2) is no option as the delta between a qemu version and the available
headers can easily become too large (e.g. in Debian...).

> 
> I've no preference for either of the two, first is
> more convinient and second has less duplicates.
> 
> (By "qemu" I mean either qemu or qemu-kvm, since the
>  two tries to become the same thing)
> 
> []
>>> Does qemu[-kvm] want to stay compatible with older kernels,
>>> or kernel headers?  If yes, how old is old?
>>
>> Our current minimum requirement is the KVM ABI provided by 2.6.29. The
>> runtime check is detecting this via the existence of certain key
>> capabilities. The related build-time check takes place during configure,
>> probing a list of minimally required KVM_CAPs.
>>
>> This last check would become obsolete if we always pick the headers from
>> a qemu-local copy. Furthermore, we could drop "#ifdef KVM_CAP" around
>> features that are more recent than our kernel baseline. Runtime checking
>> will still be required, of course.
> 
> That check is useful to error out and tell user if the minimum
> requiriment is not satisfied.  Keeping lots of ifdefs isn't useful
> IMHO.

That configure check already reduced the size of that forest. Or course,
we could also move all inlined #ifdefs to the configure check - if we
continue to allow building against different headers than the included
ones. But I would prefer to avoid this in order to reduce the variations
(of which one combination always break).

> 
> So I'd say keep --with-kernelheaders, keep the checks in configure
> (maybe activate them if --with-kernelheaders is specified), keep
> local kernel headers which are recent enough, and remove irrelevant
> #ifdefs around new API.
> 
> I'm not sure again I understand your statement about removing
> #ifdefs in the previous email -- now you say you verified that
> all #ifdefs are needed.  They're either needed or can be removed... ;)

They are needed ATM as we allow building against any headers of >=
2.6.29. They would become obsolete if we provided the right ones in-tree.

> 
> But that all went quite far away from dropping old cruft from
> kvm/ subdir - as we found out, there are 2 useful pieces in
> there, kvm_stat helper and kernel headers, so let's keep them
> for now and remove the rest.
> 
> Anyway, that's quite long "discussion" about nearly nothing... :)

Well, "rm -r kvm" is a rather minor thing. But the header discussion is
important IMO as we continue to see breakages in KVM builds (mostly qemu
upstream) due to missing #ifdefs. That means:
 - we do not test all variants (because it's impractical)
 - people use older headers
 - too much effort is wasted on fixing distributed problems that can be
   solved centrally

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Avi Kivity]

On 02/23/2011 04:57 PM, Jan Kiszka wrote:
> >
> >  The only situation where using kernel headers not provided
> >  by qemu itself is when you want to build qemu for older
> >  kernel and omit some features and runtime tests.  But this
> >  is hardly a good goal to support.
>
> I don't think the goal of the #ifdefs was ever some kind of size
> optimization but just for the sake of avoiding build breakages.
>

Yes.

> Well, "rm -r kvm" is a rather minor thing. But the header discussion is
> important IMO as we continue to see breakages in KVM builds (mostly qemu
> upstream) due to missing #ifdefs. That means:
>   - we do not test all variants (because it's impractical)

We could teach buildbot about it.

>   - people use older headers
>   - too much effort is wasted on fixing distributed problems that can be
>     solved centrally

Yes.  But on the other hand carrying headers is the Wrong Thing, isn't 
it?  If everyone did that we'd be in a mess of duplication.  I'd like 
not to contribute to that.

-- 
error compiling committee.c: too many arguments to function

Re: Q: status of kvm/ subdir in qemu-kvm tarball?

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

On 2011-02-27 17:28, Avi Kivity wrote:
> On 02/23/2011 04:57 PM, Jan Kiszka wrote:
>> >
>> >  The only situation where using kernel headers not provided
>> >  by qemu itself is when you want to build qemu for older
>> >  kernel and omit some features and runtime tests.  But this
>> >  is hardly a good goal to support.
>>
>> I don't think the goal of the #ifdefs was ever some kind of size
>> optimization but just for the sake of avoiding build breakages.
>>
> 
> Yes.
> 
>> Well, "rm -r kvm" is a rather minor thing. But the header discussion is
>> important IMO as we continue to see breakages in KVM builds (mostly qemu
>> upstream) due to missing #ifdefs. That means:
>>   - we do not test all variants (because it's impractical)
> 
> We could teach buildbot about it.

We could, but the test matrix wouldn't be simple. Think of CAPs that
depend on other CAPs. I don't think it would be worth the effort. And if
we forget to update a test after adding a CAP, the situation won't be
different from today.

> 
>>   - people use older headers
>>   - too much effort is wasted on fixing distributed problems that can be
>>     solved centrally
> 
> Yes.  But on the other hand carrying headers is the Wrong Thing, isn't
> it?

Well, there are also different views on this, see e.g.
http://kernelnewbies.org/KernelHeaders

>  If everyone did that we'd be in a mess of duplication.  I'd like
> not to contribute to that.

Not all distros keep their kernel headers sufficiently recent and/or
users forget to update them when updating the kernel (I just noticed
that vhost remained off in some builds here due to pre-historic distro
headers). As long as we continue adding or changing the kernel ABI, qemu
will face these problems.

We can't avoid a certain degree of mess, either in form of in-tree
header copies or via a continuously increasing number of #ifdefs in our
code. I'm really convinced now the former is both more user-friendly and
cleaner than the latter.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]