11:23 a.m.
Hi Everyone,
We had a requirement to skip all CPU feature validations from libvirt and totally rely on
Qemu for following reasons.
* As libvirt always lies behind in term of cpu feature management compare to qemu, so we
may not be able to use all of the latest support provided by qemu till it is updated in
libvirt too. For example like if new cpu features are added in qemu but not yet updated in
libvirt.
* Libvirt does not support cpu model versioning as Qemu.
* Libvirt checks if a features is supported or not based on cpuid, a feature supported
by host may not be supported by qemu or there can be some features which are emulated by
Qemu so libvirt level checking does not make sense in those cases.
* Qemu already provide an option 'enforce' to validate if features with which vm
is started is exactly same as one provided and nothing is silently dropped.
Basically we want another check option like 'qemu-enforce' which will skip all
feature and cpu model verfications in libvirt and pass 'enforce' option to qemu.
It will work similar to check 'none' i mean no check to verify if provide features
are supported by host, also on top of that skip things like virCPUValidateFeatures where
ever required.
I understand this is specific to just Qemu and libvirt has to take care for many other
emulator, so wanted to check if something like that makes sense as it will be very useful
for Qemu atleast. If so we can send a patch.
Thanks
Manish Mishra
3:52 a.m.
Hi.
On Wed, Nov 16, 2022 at 22:53:18 +0530, manish.mishra wrote:
We had a requirement to skip all CPU feature validations from
libvirt
and totally rely on Qemu for following reasons.
You are mixing two different things here. The "enforce" CPU checking for
making sure a guest gets exactly what the user asked QEMU for and
missing CPU models or features support in libvirt.
* As libvirt always lies behind in term of cpu feature management
compare to qemu, so we may not be able to use all of the latest
support provided by qemu till it is updated in libvirt too. For
example like if new cpu features are added in qemu but not yet
updated in libvirt.
So you effectively ask for a "passthrough" configuration, that is, CPU
model and features would be passed directly to QEMU without libvirt even
looking at them, right?
Anyway, the current goal is to make libvirt ask for supported CPU models
including their definition so that we don't have to keep our own
definitions. That is, libvirt would support all models as they are added
to QEMU without any additional work. Not sure if CPU features would
follow, but adding them is fairly easy and what's more important they
don't change in time (unlike CPU models).
* Libvirt checks if a features is supported or not based on cpuid,
a
feature supported by host may not be supported by qemu or there can
be some features which are emulated by Qemu so libvirt level
checking does not make sense in those cases.
This has not been the case for several years already. Libvirt asks QEMU
what CPU features it can provide on the host (either natively or
emulated) and checks against them. So no issue here. Well, expect that
some features are strange and QEMU does not report them as enabled for
-cpu host even though it would enable them for some specific CPU models.
But that's more in a "bug" area which needs to be solved between QEMU
and libvirt rather than being it a principal issue.
* Qemu already provide an option 'enforce' to validate if
features
with which vm is started is exactly same as one provided and nothing
is silently dropped.
Right, but it's not enough. In addition to removed features libvirt also
checks for unexpectedly added features. And you really need to do both.
Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and QEMU
says everything is fine, the guest might see more than what you asked.
For example, if a feature is enabled only if a host supports it you may
or may not get it without any complains from QEMU. But if you get it you
really need to explicitly ask for it during migration, otherwise the
feature can just silently disappear. Of course, this would be a really
bad behavior from QEMU, but that does not mean it can't happen (I think
SVM is a bit problematic in this way) and the whole point of libvirt's
checks is to prevent this kind of issues.
So once QEMU starts we checks all features enabled in the vCPU and note
those that were (unexpectedly) added so that we can explicitly ask for
them during migration and also check that no other features were added
on top. And for this whole process to work, we need to understand, i.e.,
have explicit support, for all CPU features (QEMU report a lot of CPU
properties and we need to know which one needs to be stored as enabled
features) and CPU models to make sure both sides of migration understand
the CPU definition in domain XML in the same way.
Basically we want another check option like 'qemu-enforce'
which will
skip all feature and cpu model verfications in libvirt and pass
'enforce' option to qemu. It will work similar to check 'none' i mean
no check to verify if provide features are supported by host, also on
top of that skip things like virCPUValidateFeatures where ever
required.
In short, I don't think this is a good idea and we should or could
support it and still maintain migration safety. If you don't care about
migration safety, you don't need to care about "enforce" QEMU check
either and can just use host-passthrough and get whatever QEMU supports
on the host.
Jirka
4:18 a.m.
On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
> * Qemu already provide an option 'enforce' to validate
if features
> with which vm is started is exactly same as one provided and nothing
> is silently dropped.
Right, but it's not enough. In addition to removed features libvirt also
checks for unexpectedly added features. And you really need to do both.
Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and QEMU
says everything is fine, the guest might see more than what you asked.
For example, if a feature is enabled only if a host supports it you may
or may not get it without any complains from QEMU. But if you get it you
really need to explicitly ask for it during migration, otherwise the
feature can just silently disappear. Of course, this would be a really
bad behavior from QEMU, but that does not mean it can't happen (I think
SVM is a bit problematic in this way) and the whole point of libvirt's
checks is to prevent this kind of issues.
Hi Jiri, I'm not following this very well. I think you're saying that qemu has
had bugs previously where features get silently enabled, and it's libvirt's
job/role to paper over those issues? Do you have some specific cases of this?
thanks
john
5:30 a.m.
On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
> > * Qemu already provide an option 'enforce' to validate if features
> > with which vm is started is exactly same as one provided and nothing
> > is silently dropped.
>
> Right, but it's not enough. In addition to removed features libvirt also
> checks for unexpectedly added features. And you really need to do both.
> Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and QEMU
> says everything is fine, the guest might see more than what you asked.
> For example, if a feature is enabled only if a host supports it you may
> or may not get it without any complains from QEMU. But if you get it you
> really need to explicitly ask for it during migration, otherwise the
> feature can just silently disappear. Of course, this would be a really
> bad behavior from QEMU, but that does not mean it can't happen (I think
> SVM is a bit problematic in this way) and the whole point of libvirt's
> checks is to prevent this kind of issues.
Hi Jiri, I'm not following this very well. I think you're saying that qemu has
had bugs previously where features get silently enabled,
Personally, I haven't actually witnessed any bug in this area (as far as
I can remember, which is not that far :-)), but got some reports of at
least one, even though without any proof.
Specifically, SVM is quite strange as it is included in all AMD CPU
models in QEMU and yet if you try to start it on a host without nesting
enabled "enforce" does not complain. I saw the feature is enabled with
older machine types, but I was told the magic behind this feature looks
like not only machine type but even host configuration itself is
involved. Anyway, although I saw reports of that the feature could be
enabled without an explicit request even with new machine types I still
haven't seen any proof of this happening. So I hope it just does not
happen and users are only afraid of this possibility.
and it's libvirt's job/role to paper over those issues? Do
you have
some specific cases of this?
This is not about papering. It's actually the opposite, that is about
detecting if something like this happens and reporting it as a failure
rather than papering it and hoping everything goes well. So I think
doing this is a good idea even though I don't think we actually saw any
issue of this kind.
Jirka
1:17 a.m.
On 18/11/22 5:00 pm, Jiri Denemark wrote:
On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
>
>>> * Qemu already provide an option 'enforce' to validate if
features
>>> with which vm is started is exactly same as one provided and nothing
>>> is silently dropped.
>> Right, but it's not enough. In addition to removed features libvirt also
>> checks for unexpectedly added features. And you really need to do both.
>> Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and QEMU
>> says everything is fine, the guest might see more than what you asked.
>> For example, if a feature is enabled only if a host supports it you may
>> or may not get it without any complains from QEMU. But if you get it you
>> really need to explicitly ask for it during migration, otherwise the
>> feature can just silently disappear. Of course, this would be a really
>> bad behavior from QEMU, but that does not mean it can't happen (I think
>> SVM is a bit problematic in this way) and the whole point of libvirt's
>> checks is to prevent this kind of issues.
> Hi Jiri, I'm not following this very well. I think you're saying that qemu
has
> had bugs previously where features get silently enabled,
Personally, I haven't actually witnessed any bug in this area (as far as
I can remember, which is not that far :-)), but got some reports of at
least one, even though without any proof.
Specifically, SVM is quite strange as it is included in all AMD CPU
models in QEMU and yet if you try to start it on a host without nesting
enabled "enforce" does not complain. I saw the feature is enabled with
older machine types, but I was told the magic behind this feature looks
like not only machine type but even host configuration itself is
involved. Anyway, although I saw reports of that the feature could be
enabled without an explicit request even with new machine types I still
haven't seen any proof of this happening. So I hope it just does not
happen and users are only afraid of this possibility.
> and it's libvirt's job/role to paper over those issues? Do you have
> some specific cases of this?
This is not about papering. It's actually the opposite, that is about
detecting if something like this happens and reporting it as a failure
rather than papering it and hoping everything goes well. So I think
doing this is a good idea even though I don't think we actually saw any
issue of this kind.
Hi Jiri, I see now with libvirt master, with check=='full' we verify both silently
dropped as well as added features. But as you already stated Qemu silently adding feature
is a Qemu bug and libvirt just reports that bug, so it should be very unlikely, i agree
that is not a good reasoning :). Our requirement is that we want to use CPU Models and
features which are defined in Qemu but not in libvirt for e.g if we want to use
Icelake-Server-V4 directly or newly added vmx feature, libvirt does not allow. Currently
we take help of qemu to do validations but for cpu feature verfication and model
definations we still use libvirt defined definations which prevent us to use anything
which is not defined in libvirt. I see there are already efforts going on to get all model
and feature defination from qemu itself, but not sure how much time it will take. Till
that happens we thought safest option is to have an option to remove all validations from
libvirt and rely on qemu 'enforce' for
migration safetly. I understand qemu-enforce does not check for silently added features,
but that case we can assume is very unlikely and Qemu should fix otherwise VMs will not
poweron anyway with check=='full'. Basically we want it as an modification of
check='none' but also skipping things like virCPUValidateFeatures and passing
option 'enforce' to Qemu. Or if silently adding features is that big concern we
can have a check in Qemu itself? I understand current qemu-enforce is not as migration
safe as check=='full' but probably suitable for our use case for time being?
Thanks
Manish Mishra
Jirka
6:48 a.m.
On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
On 18/11/22 5:00 pm, Jiri Denemark wrote:
> On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
>> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
>>
>>>> * Qemu already provide an option 'enforce' to validate if
features
>>>> with which vm is started is exactly same as one provided and nothing
>>>> is silently dropped.
>>> Right, but it's not enough. In addition to removed features libvirt
also
>>> checks for unexpectedly added features. And you really need to do both.
>>> Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and QEMU
>>> says everything is fine, the guest might see more than what you asked.
>>> For example, if a feature is enabled only if a host supports it you may
>>> or may not get it without any complains from QEMU. But if you get it you
>>> really need to explicitly ask for it during migration, otherwise the
>>> feature can just silently disappear. Of course, this would be a really
>>> bad behavior from QEMU, but that does not mean it can't happen (I think
>>> SVM is a bit problematic in this way) and the whole point of libvirt's
>>> checks is to prevent this kind of issues.
>> Hi Jiri, I'm not following this very well. I think you're saying that
qemu has
>> had bugs previously where features get silently enabled,
> Personally, I haven't actually witnessed any bug in this area (as far as
> I can remember, which is not that far :-)), but got some reports of at
> least one, even though without any proof.
>
> Specifically, SVM is quite strange as it is included in all AMD CPU
> models in QEMU and yet if you try to start it on a host without nesting
> enabled "enforce" does not complain. I saw the feature is enabled with
> older machine types, but I was told the magic behind this feature looks
> like not only machine type but even host configuration itself is
> involved. Anyway, although I saw reports of that the feature could be
> enabled without an explicit request even with new machine types I still
> haven't seen any proof of this happening. So I hope it just does not
> happen and users are only afraid of this possibility.
>
>> and it's libvirt's job/role to paper over those issues? Do you have
>> some specific cases of this?
> This is not about papering. It's actually the opposite, that is about
> detecting if something like this happens and reporting it as a failure
> rather than papering it and hoping everything goes well. So I think
> doing this is a good idea even though I don't think we actually saw any
> issue of this kind.
Hi Jiri, I see now with libvirt master, with check=='full' we verify
both silently dropped as well as added features. But as you already
stated Qemu silently adding feature is a Qemu bug and libvirt just
reports that bug, so it should be very unlikely, i agree that is not a
good reasoning :). Our requirement is that we want to use CPU Models
and features which are defined in Qemu but not in libvirt for e.g if
we want to use Icelake-Server-V4 directly or newly added vmx feature,
libvirt does not allow. Currently we take help of qemu to do
validations but for cpu feature verfication and model definations we
still use libvirt defined definations which prevent us to use anything
which is not defined in libvirt. I see there are already efforts going
on to get all model and feature defination from qemu itself, but not
sure how much time it will take. Till that happens we thought safest
option is to have an option to remove all validations from libvirt and
rely on qemu 'enforce' for migration safetly. I understand
qemu-enforce does not check for silently added features, but that case
we can assume is very unlikely and Qemu should fix otherwise VMs will
not poweron anyway with check=='full'. Basically we want it as an
modification of check='none' but also skipping things like
virCPUValidateFeatures and passing option 'enforce' to Qemu. Or if
silently adding features is that big concern we can have a check in
Qemu itself? I understand current qemu-enforce is not as migration
safe as check=='full' but probably suitable for our use case for time
being?
I understand why you want this, but on the other hand adding just a
plain pass-through for CPU model and features is quite dangerous as it
can be used to set any -cpu option even if it's not a feature. And
especially the parts that are configured in other areas of domain XML
(such as hypervisor features). So I think instead of adding a new mode
for <cpu> we should go another way. For things that are not yet
supported by libvirt we support various elements in qemu namespace,
e.g., setting QEMU command line options, environment variables, or even
overriding options libvirt would normally set on the command line. So I
guess we could have a special <qemu:cpu> element which would be used
when a user wants full control over the -cpu option without any libvirt
intervention.
Jirka
2:10 a.m.
On 22/11/22 6:18 pm, Jiri Denemark wrote:
On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
> On 18/11/22 5:00 pm, Jiri Denemark wrote:
>> On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
>>> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
>>>
>>>>> * Qemu already provide an option 'enforce' to validate if
features
>>>>> with which vm is started is exactly same as one provided and
nothing
>>>>> is silently dropped.
>>>> Right, but it's not enough. In addition to removed features libvirt
also
>>>> checks for unexpectedly added features. And you really need to do both.
>>>> Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and QEMU
>>>> says everything is fine, the guest might see more than what you asked.
>>>> For example, if a feature is enabled only if a host supports it you may
>>>> or may not get it without any complains from QEMU. But if you get it you
>>>> really need to explicitly ask for it during migration, otherwise the
>>>> feature can just silently disappear. Of course, this would be a really
>>>> bad behavior from QEMU, but that does not mean it can't happen (I
think
>>>> SVM is a bit problematic in this way) and the whole point of
libvirt's
>>>> checks is to prevent this kind of issues.
>>> Hi Jiri, I'm not following this very well. I think you're saying that
qemu has
>>> had bugs previously where features get silently enabled,
>> Personally, I haven't actually witnessed any bug in this area (as far as
>> I can remember, which is not that far :-)), but got some reports of at
>> least one, even though without any proof.
>>
>> Specifically, SVM is quite strange as it is included in all AMD CPU
>> models in QEMU and yet if you try to start it on a host without nesting
>> enabled "enforce" does not complain. I saw the feature is enabled with
>> older machine types, but I was told the magic behind this feature looks
>> like not only machine type but even host configuration itself is
>> involved. Anyway, although I saw reports of that the feature could be
>> enabled without an explicit request even with new machine types I still
>> haven't seen any proof of this happening. So I hope it just does not
>> happen and users are only afraid of this possibility.
>>
>>> and it's libvirt's job/role to paper over those issues? Do you have
>>> some specific cases of this?
>> This is not about papering. It's actually the opposite, that is about
>> detecting if something like this happens and reporting it as a failure
>> rather than papering it and hoping everything goes well. So I think
>> doing this is a good idea even though I don't think we actually saw any
>> issue of this kind.
>
> Hi Jiri, I see now with libvirt master, with check=='full' we verify
> both silently dropped as well as added features. But as you already
> stated Qemu silently adding feature is a Qemu bug and libvirt just
> reports that bug, so it should be very unlikely, i agree that is not a
> good reasoning :). Our requirement is that we want to use CPU Models
> and features which are defined in Qemu but not in libvirt for e.g if
> we want to use Icelake-Server-V4 directly or newly added vmx feature,
> libvirt does not allow. Currently we take help of qemu to do
> validations but for cpu feature verfication and model definations we
> still use libvirt defined definations which prevent us to use anything
> which is not defined in libvirt. I see there are already efforts going
> on to get all model and feature defination from qemu itself, but not
> sure how much time it will take. Till that happens we thought safest
> option is to have an option to remove all validations from libvirt and
> rely on qemu 'enforce' for migration safetly. I understand
> qemu-enforce does not check for silently added features, but that case
> we can assume is very unlikely and Qemu should fix otherwise VMs will
> not poweron anyway with check=='full'. Basically we want it as an
> modification of check='none' but also skipping things like
> virCPUValidateFeatures and passing option 'enforce' to Qemu. Or if
> silently adding features is that big concern we can have a check in
> Qemu itself? I understand current qemu-enforce is not as migration
> safe as check=='full' but probably suitable for our use case for time
> being?
I understand why you want this, but on the other hand adding just a
plain pass-through for CPU model and features is quite dangerous as it
can be used to set any -cpu option even if it's not a feature. And
especially the parts that are configured in other areas of domain XML
(such as hypervisor features). So I think instead of adding a new mode
for <cpu> we should go another way. For things that are not yet
supported by libvirt we support various elements in qemu namespace,
e.g., setting QEMU command line options, environment variables, or even
overriding options libvirt would normally set on the command line. So I
guess we could have a special <qemu:cpu> element which would be used
when a user wants full control over the -cpu option without any libvirt
intervention.
Jirka
Hi Jiri, Thanks, We discussed internally, yes something like that works for us. We wanted
to dicusss how it will look like roughly before sending patch. Does something like below
works.
<qemu:cpu> will be a sub-property within <cpu> and it will contains details
only about cpu-model, cpu-features and check everything else can be as it is. So for CPU
feature and model either user can define those in <cpu> itself as in example 1 or in
<qemu:cpu> as in example 2. Also either example 1 or example 2 are valid, if user
mixes up both, i mean define cpu feature or models in both <qemu:cpu> and
<cpu> it will be a parsing error to avoid conflicts.
In feature policy we as of now need only require/disable for <qemu:cpu> case and no
need of options like match, mode if <qemu:cpu> is defined. Please let us know if
something like that works. It may vary a bit while working on patch, so probaly it will be
more clear when patch is out, I just wanted to put our requirements roughly before
starting working on patch :).
Example 1:
<cpu mode='custom' match='exact' check='partial'>
<model fallback='forbid'>Icelake-Server</model>
<vendor>Intel</vendor>
<topology sockets='240' dies='1' cores='1'
threads='1'/>
<cache level='3' mode='emulate'/>
<feature policy='require' name='smap'/>
<feature policy='require' name='avx'/>
<feature policy='require' name='xsaveopt'/>
<feature policy='disable' name='tsc_adjust'/>
<numa>
<cell id='0' cpus='0-239' memory='8388608'
unit='KiB' memAccess='shared'/>
</numa>
</cpu>
Example 2:
<cpu>
<vendor>Intel</vendor>
<topology sockets='240' dies='1' cores='1'
threads='1'/>
<cache level='3' mode='emulate'/>
<qemu:cpu check='enfoce'>
<model>Icelake-Server-V4</model>
<feature policy='require' name='smap'/>
<feature policy='require' name='avx'/>
<feature policy='require' name='xsaveopt'/>
<feature policy='disable' name='tsc_adjust'/>
<qemu:cpu />
<numa>
<cell id='0' cpus='0-239' memory='8388608'
unit='KiB' memAccess='shared'/>
</numa>
</cpu>
Thanks
Manish Mishra
5:09 a.m.
On Wed, Nov 23, 2022 at 13:40:21 +0530, manish.mishra wrote:
Hi Jiri, Thanks, We discussed internally, yes something like that
works for us. We wanted to dicusss how it will look like roughly
before sending patch. Does something like below works. <qemu:cpu> will
be a sub-property within <cpu> and it will contains details only about
cpu-model, cpu-features and check everything else can be as it is. So
for CPU feature and model either user can define those in <cpu> itself
as in example 1 or in <qemu:cpu> as in example 2. Also either example
1 or example 2 are valid, if user mixes up both, i mean define cpu
feature or models in both <qemu:cpu> and <cpu> it will be a parsing
error to avoid conflicts. In feature policy we as of now need only
require/disable for <qemu:cpu> case and no need of options like match,
mode if <qemu:cpu> is defined. Please let us know if something like
that works. It may vary a bit while working on patch, so probaly it
will be more clear when patch is out, I just wanted to put our
requirements roughly before starting working on patch :).
<cpu>
<vendor>Intel</vendor>
<topology sockets='240' dies='1' cores='1'
threads='1'/>
<cache level='3' mode='emulate'/>
<qemu:cpu check='enfoce'>
<model>Icelake-Server-V4</model>
<feature policy='require' name='smap'/>
<feature policy='require' name='avx'/>
<feature policy='require' name='xsaveopt'/>
<feature policy='disable' name='tsc_adjust'/>
<qemu:cpu />
<numa>
<cell id='0' cpus='0-239' memory='8388608'
unit='KiB' memAccess='shared'/>
</numa>
</cpu>
I think this would be quite hard to notice in the XML as it looks almost
like a normal CPU definition and it would also complicate the <cpu>
parser. We should keep this completely separated similarly to other
elements in qemu namespace (as described in
https://libvirt.org/drvqemu.html#pass-through-of-arbitrary-qemu-commands
So for example,
<domain type='qemu'
xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
...
<cpu>
<topology sockets='240' dies='1' cores='1'
threads='1'/>
<cache level='3' mode='emulate'/>
<numa>...</numa>
...
</cpu>
<qemu:cpu>
<qemu:model name='Icelake-Server-V4'/>
<qemu:option name='enforce' value='on'/>
<qemu:option name='smap' value='on'/>
<qemu:option name='avx' value='on'/>
<qemu:option name='tsc-adjust' value='off'/>
<qemu:option name='strange-option' value='1234'/>
</qemu:cpu>
<devices>
...
</devices>
</domain>
and this would directly translate to
-cpu Icelake-Server-V4,enforce=on,smap=on,avx=on,tsc-adjust=off,strange-option=1234
In other words, make it a complete passthrough of the values to QEMU
without any extra parsing or translating on libvirt side. This way you
can express anything QEMU supports for the -cpu command line option and
libvirt has not implemented yet.
Defining a CPU model both in <cpu> and <qemu:cpu> would be an error
indeed. But eware, even usage of hypervisor features or other XML
configuration outside of <cpu> that are translated into -cpu options
could be unsupported and overridden by the content of <qemu:cpu>. We
have several options for dealing with this:
1. check all of this and report an error
2. document the new <qemu:cpu> element to completely override
anything libvirt would pass to -cpu and it's up to the user to
include anything they need there (such as hypervisor features)
3. use the content of <qemu:cpu> as a base for -cpu and add other
staff there as usual with a proper documentation that mentioning
a specific option both in <qemu:cpu> and elsewhere results in an
undefined behavior
Personally, I would vote for 2 or 3 and perhaps slightly more for 3 as
it is a bit more user friendly in case the goal is to override the CPU
model but keep using normal configuration for hypervisor features.
And of course, using <qemu:cpu> element would result in the domain being
tainted with a new VIR_DOMAIN_TAINT_CUSTOM_CPU.
Jirka
5:18 a.m.
On Tue, Nov 22, 2022 at 01:48:39PM +0100, Jiri Denemark wrote:
On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
>
> On 18/11/22 5:00 pm, Jiri Denemark wrote:
> > On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
> >> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
> >>
> >>>> * Qemu already provide an option 'enforce' to validate
if features
> >>>> with which vm is started is exactly same as one provided and
nothing
> >>>> is silently dropped.
> >>> Right, but it's not enough. In addition to removed features libvirt
also
> >>> checks for unexpectedly added features. And you really need to do
both.
> >>> Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and QEMU
> >>> says everything is fine, the guest might see more than what you asked.
> >>> For example, if a feature is enabled only if a host supports it you
may
> >>> or may not get it without any complains from QEMU. But if you get it
you
> >>> really need to explicitly ask for it during migration, otherwise the
> >>> feature can just silently disappear. Of course, this would be a really
> >>> bad behavior from QEMU, but that does not mean it can't happen (I
think
> >>> SVM is a bit problematic in this way) and the whole point of
libvirt's
> >>> checks is to prevent this kind of issues.
> >> Hi Jiri, I'm not following this very well. I think you're saying
that qemu has
> >> had bugs previously where features get silently enabled,
> > Personally, I haven't actually witnessed any bug in this area (as far as
> > I can remember, which is not that far :-)), but got some reports of at
> > least one, even though without any proof.
> >
> > Specifically, SVM is quite strange as it is included in all AMD CPU
> > models in QEMU and yet if you try to start it on a host without nesting
> > enabled "enforce" does not complain. I saw the feature is enabled
with
> > older machine types, but I was told the magic behind this feature looks
> > like not only machine type but even host configuration itself is
> > involved. Anyway, although I saw reports of that the feature could be
> > enabled without an explicit request even with new machine types I still
> > haven't seen any proof of this happening. So I hope it just does not
> > happen and users are only afraid of this possibility.
> >
> >> and it's libvirt's job/role to paper over those issues? Do you
have
> >> some specific cases of this?
> > This is not about papering. It's actually the opposite, that is about
> > detecting if something like this happens and reporting it as a failure
> > rather than papering it and hoping everything goes well. So I think
> > doing this is a good idea even though I don't think we actually saw any
> > issue of this kind.
>
>
> Hi Jiri, I see now with libvirt master, with check=='full' we verify
> both silently dropped as well as added features. But as you already
> stated Qemu silently adding feature is a Qemu bug and libvirt just
> reports that bug, so it should be very unlikely, i agree that is not a
> good reasoning :). Our requirement is that we want to use CPU Models
> and features which are defined in Qemu but not in libvirt for e.g if
> we want to use Icelake-Server-V4 directly or newly added vmx feature,
> libvirt does not allow. Currently we take help of qemu to do
> validations but for cpu feature verfication and model definations we
> still use libvirt defined definations which prevent us to use anything
> which is not defined in libvirt. I see there are already efforts going
> on to get all model and feature defination from qemu itself, but not
> sure how much time it will take. Till that happens we thought safest
> option is to have an option to remove all validations from libvirt and
> rely on qemu 'enforce' for migration safetly. I understand
> qemu-enforce does not check for silently added features, but that case
> we can assume is very unlikely and Qemu should fix otherwise VMs will
> not poweron anyway with check=='full'. Basically we want it as an
> modification of check='none' but also skipping things like
> virCPUValidateFeatures and passing option 'enforce' to Qemu. Or if
> silently adding features is that big concern we can have a check in
> Qemu itself? I understand current qemu-enforce is not as migration
> safe as check=='full' but probably suitable for our use case for time
> being?
I understand why you want this, but on the other hand adding just a
plain pass-through for CPU model and features is quite dangerous as it
can be used to set any -cpu option even if it's not a feature. And
especially the parts that are configured in other areas of domain XML
(such as hypervisor features). So I think instead of adding a new mode
for <cpu> we should go another way. For things that are not yet
supported by libvirt we support various elements in qemu namespace,
e.g., setting QEMU command line options, environment variables, or even
overriding options libvirt would normally set on the command line. So I
guess we could have a special <qemu:cpu> element which would be used
when a user wants full control over the -cpu option without any libvirt
intervention.
I really don't think we should allow this, especially not for something
which is clearly intended to be used for production deployment. Our
hypervisor CLI passthrough is there to allow for short term workarounds
for bugs, or to experiment with a feature before it is mapped into
libvirt in a supported manner.
If there are aspects related to QEMU configuration thiat are not in
libvirt yet, we need to address those gaps, not provide yet another
way to bypass libvirt mapping.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
5:29 a.m.
On Wed, Nov 23, 2022 at 11:18:01 +0000, Daniel P. Berrangé wrote:
On Tue, Nov 22, 2022 at 01:48:39PM +0100, Jiri Denemark wrote:
> On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
> >
> > On 18/11/22 5:00 pm, Jiri Denemark wrote:
> > > On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
> > >> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
> > >>
> > >>>> * Qemu already provide an option 'enforce' to
validate if features
> > >>>> with which vm is started is exactly same as one provided
and nothing
> > >>>> is silently dropped.
> > >>> Right, but it's not enough. In addition to removed features
libvirt also
> > >>> checks for unexpectedly added features. And you really need to do
both.
> > >>> Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and
QEMU
> > >>> says everything is fine, the guest might see more than what you
asked.
> > >>> For example, if a feature is enabled only if a host supports it
you may
> > >>> or may not get it without any complains from QEMU. But if you get
it you
> > >>> really need to explicitly ask for it during migration, otherwise
the
> > >>> feature can just silently disappear. Of course, this would be a
really
> > >>> bad behavior from QEMU, but that does not mean it can't happen
(I think
> > >>> SVM is a bit problematic in this way) and the whole point of
libvirt's
> > >>> checks is to prevent this kind of issues.
> > >> Hi Jiri, I'm not following this very well. I think you're
saying that qemu has
> > >> had bugs previously where features get silently enabled,
> > > Personally, I haven't actually witnessed any bug in this area (as far
as
> > > I can remember, which is not that far :-)), but got some reports of at
> > > least one, even though without any proof.
> > >
> > > Specifically, SVM is quite strange as it is included in all AMD CPU
> > > models in QEMU and yet if you try to start it on a host without nesting
> > > enabled "enforce" does not complain. I saw the feature is
enabled with
> > > older machine types, but I was told the magic behind this feature looks
> > > like not only machine type but even host configuration itself is
> > > involved. Anyway, although I saw reports of that the feature could be
> > > enabled without an explicit request even with new machine types I still
> > > haven't seen any proof of this happening. So I hope it just does not
> > > happen and users are only afraid of this possibility.
> > >
> > >> and it's libvirt's job/role to paper over those issues? Do you
have
> > >> some specific cases of this?
> > > This is not about papering. It's actually the opposite, that is about
> > > detecting if something like this happens and reporting it as a failure
> > > rather than papering it and hoping everything goes well. So I think
> > > doing this is a good idea even though I don't think we actually saw
any
> > > issue of this kind.
> >
> >
> > Hi Jiri, I see now with libvirt master, with check=='full' we verify
> > both silently dropped as well as added features. But as you already
> > stated Qemu silently adding feature is a Qemu bug and libvirt just
> > reports that bug, so it should be very unlikely, i agree that is not a
> > good reasoning :). Our requirement is that we want to use CPU Models
> > and features which are defined in Qemu but not in libvirt for e.g if
> > we want to use Icelake-Server-V4 directly or newly added vmx feature,
> > libvirt does not allow. Currently we take help of qemu to do
> > validations but for cpu feature verfication and model definations we
> > still use libvirt defined definations which prevent us to use anything
> > which is not defined in libvirt. I see there are already efforts going
> > on to get all model and feature defination from qemu itself, but not
> > sure how much time it will take. Till that happens we thought safest
> > option is to have an option to remove all validations from libvirt and
> > rely on qemu 'enforce' for migration safetly. I understand
> > qemu-enforce does not check for silently added features, but that case
> > we can assume is very unlikely and Qemu should fix otherwise VMs will
> > not poweron anyway with check=='full'. Basically we want it as an
> > modification of check='none' but also skipping things like
> > virCPUValidateFeatures and passing option 'enforce' to Qemu. Or if
> > silently adding features is that big concern we can have a check in
> > Qemu itself? I understand current qemu-enforce is not as migration
> > safe as check=='full' but probably suitable for our use case for time
> > being?
>
> I understand why you want this, but on the other hand adding just a
> plain pass-through for CPU model and features is quite dangerous as it
> can be used to set any -cpu option even if it's not a feature. And
> especially the parts that are configured in other areas of domain XML
> (such as hypervisor features). So I think instead of adding a new mode
> for <cpu> we should go another way. For things that are not yet
> supported by libvirt we support various elements in qemu namespace,
> e.g., setting QEMU command line options, environment variables, or even
> overriding options libvirt would normally set on the command line. So I
> guess we could have a special <qemu:cpu> element which would be used
> when a user wants full control over the -cpu option without any libvirt
> intervention.
I really don't think we should allow this, especially not for something
which is clearly intended to be used for production deployment. Our
hypervisor CLI passthrough is there to allow for short term workarounds
for bugs, or to experiment with a feature before it is mapped into
libvirt in a supported manner.
If there are aspects related to QEMU configuration thiat are not in
libvirt yet, we need to address those gaps, not provide yet another
way to bypass libvirt mapping.
Indeed, this was definitely meant as a short term workaround for stuff
we don't have support for yet, for testing or experimental purposes. The
supported approach is for implement the missing parts in libvirt (and
QEMU) as soon as possible. I don't see why would a properly documented
support for experiments with -cpu would be an issue.
If anyone thinks it's a good idea to use such stuff for anything even
remotely close to production then that's their call. We can't really
prevent that. Just as with any other <qemu:...> stuff we already have.
Jirka
5:34 a.m.
On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
> I really don't think we should allow this, especially not
for something
> which is clearly intended to be used for production deployment. Our
> hypervisor CLI passthrough is there to allow for short term workarounds
> for bugs, or to experiment with a feature before it is mapped into
> libvirt in a supported manner.
>
> If there are aspects related to QEMU configuration thiat are not in
> libvirt yet, we need to address those gaps, not provide yet another
> way to bypass libvirt mapping.
Indeed, this was definitely meant as a short term workaround for stuff
we don't have support for yet, for testing or experimental purposes. The
supported approach is for implement the missing parts in libvirt (and
QEMU) as soon as possible. I don't see why would a properly documented
support for experiments with -cpu would be an issue.
If anyone thinks it's a good idea to use such stuff for anything even
remotely close to production then that's their call. We can't really
prevent that. Just as with any other <qemu:...> stuff we already have.
Having two copies of essentially the same information that must be kept
perfectly in sync has been the direct cause of multiple serious bugs for us
(some self-inflicted, probably, but certainly not all). I understand that "fix
libvirt every time there's a disparity" is appealing but unfortunately in
practice this just hasn't been the case.
We're struggling to see what value the libvirt layer is bringing us in this
area, as opposed to a single-source-of-truth approach.
regards
john
5:48 a.m.
On Wed, Nov 23, 2022 at 11:34:24AM +0000, John Levon wrote:
On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
> > I really don't think we should allow this, especially not for something
> > which is clearly intended to be used for production deployment. Our
> > hypervisor CLI passthrough is there to allow for short term workarounds
> > for bugs, or to experiment with a feature before it is mapped into
> > libvirt in a supported manner.
> >
> > If there are aspects related to QEMU configuration thiat are not in
> > libvirt yet, we need to address those gaps, not provide yet another
> > way to bypass libvirt mapping.
>
> Indeed, this was definitely meant as a short term workaround for stuff
> we don't have support for yet, for testing or experimental purposes. The
> supported approach is for implement the missing parts in libvirt (and
> QEMU) as soon as possible. I don't see why would a properly documented
> support for experiments with -cpu would be an issue.
>
> If anyone thinks it's a good idea to use such stuff for anything even
> remotely close to production then that's their call. We can't really
> prevent that. Just as with any other <qemu:...> stuff we already have.
Having two copies of essentially the same information that must be kept
perfectly in sync has been the direct cause of multiple serious bugs for us
(some self-inflicted, probably, but certainly not all). I understand that "fix
libvirt every time there's a disparity" is appealing but unfortunately in
practice this just hasn't been the case.
We're struggling to see what value the libvirt layer is bringing us in this
area, as opposed to a single-source-of-truth approach.
Well libvirt's job is to insulate applications from the specific hypervisor
implementation details.
We support multiple hypervisors, and as such it is a goal that CPU feature
names at least are able to be made consistent across hypervisor drivers in
libvirt, but ideally CPU model names too that is more challenging to
achieve, depending on what config flexibility the hypervisor offers.
Still, directly exposing the hypervisor's specific names is an explicit
anti-goal.
Another factor is that QEMU configuration syntax changes over time, and
libvirt aims to transparently adapt to such changes, that includes
potential renames of flags/models.
There is always a chance of bugs when mapping from the hypervisor to
libvirt, but that's not a reason to abandon the core goals of libvirt
to isolate apps from the raw hypervisor impl details. Our focus needs
to be on reducing the chance of such bugs in libvirt and reducing any
time lag between features.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
5:57 a.m.
On Wed, Nov 23, 2022 at 11:48:17AM +0000, Daniel P. Berrangé wrote:
> We're struggling to see what value the libvirt layer is
bringing us in this
> area, as opposed to a single-source-of-truth approach.
Well libvirt's job is to insulate applications from the specific hypervisor
implementation details.
Sure, of course, this isn't relevant to other drivers or something that would be
turned on from (say) a virt-manager context. We're not trying to argue this
should be a default or anything, but this would hardly be the first thing that's
only supported by the qemu driver.
regards
john
6:10 a.m.
On Wed, Nov 23, 2022 at 11:57:08AM +0000, John Levon wrote:
On Wed, Nov 23, 2022 at 11:48:17AM +0000, Daniel P. Berrangé wrote:
> > We're struggling to see what value the libvirt layer is bringing us in
this
> > area, as opposed to a single-source-of-truth approach.
>
> Well libvirt's job is to insulate applications from the specific hypervisor
> implementation details.
Sure, of course, this isn't relevant to other drivers or something that would be
turned on from (say) a virt-manager context. We're not trying to argue this
should be a default or anything, but this would hardly be the first thing that's
only supported by the qemu driver.
There's a big difference between a feature only supported by one of
the hypervisor drivers, and a feature where we just blindly do
passthrough of the hypervisor terminology as-is.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:12 a.m.
On Wed, Nov 23, 2022 at 11:34:24 +0000, John Levon wrote:
On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
> > I really don't think we should allow this, especially not for something
> > which is clearly intended to be used for production deployment. Our
> > hypervisor CLI passthrough is there to allow for short term workarounds
> > for bugs, or to experiment with a feature before it is mapped into
> > libvirt in a supported manner.
> >
> > If there are aspects related to QEMU configuration thiat are not in
> > libvirt yet, we need to address those gaps, not provide yet another
> > way to bypass libvirt mapping.
>
> Indeed, this was definitely meant as a short term workaround for stuff
> we don't have support for yet, for testing or experimental purposes. The
> supported approach is for implement the missing parts in libvirt (and
> QEMU) as soon as possible. I don't see why would a properly documented
> support for experiments with -cpu would be an issue.
>
> If anyone thinks it's a good idea to use such stuff for anything even
> remotely close to production then that's their call. We can't really
> prevent that. Just as with any other <qemu:...> stuff we already have.
Having two copies of essentially the same information that must be kept
perfectly in sync has been the direct cause of multiple serious bugs for us
(some self-inflicted, probably, but certainly not all). I understand that "fix
libvirt every time there's a disparity" is appealing but unfortunately in
practice this just hasn't been the case.
Well, but that's not how it is all supposed to work. First, libvirt and
QEMU do not strictly need to be in sync with everything.
The important part is that features are in sync (pretty much trivial and
easy, it's just that someone needs to be motivated to send a patch).
Libvirt needs to understand the features to be able to report host
capabilities. Not to mention that QEMU changed names of several features
and even deprecated the old spellings which is completely transparent to
libvirt users as they can still use the old names no matter what version
of QEMU they use. This is one of the key benefits of libvirt.
CPU models need to be added to libvirt too, but their definition does
not have to be in sync with QEMU. And as far as I can tell the goal is
to dynamically probe the models and their details from QEMU so that we
don't need to keep our own copy of CPU models. This approach should be
possible (and it's actually a requirement due to the increased number of
supported CPU models) with versioned CPUs, but we need some more support
for this probing in QEMU.
Anyway, with the currently suggested use case we should have a single
source of truth and that is QEMU. In domain capabilities XML we report
what CPU models QEMU can run on a given host. You can take any of these
models and use them with <cpu check='none' ...> to start a domain with
that CPU models whatever that means to QEMU. Libvirt then translates the
created CPU to its internal (so far) definition which is stable across
all versions of libvirt to make sure any version of libvirt can
understand it (during migration, save/restore, snapshots, upgrade) and
can ensure the CPU does not change in any way until the domain shuts
down.
In other words, libvirt code should be in a state when differences
between QEMU and libvirt in CPU model definitions should not matter.
Modulo bugs, of course, but such bugs should be reported, analyzed and
fixed similarly to any other bugs in any software.
We're struggling to see what value the libvirt layer is bringing
us in this
area, as opposed to a single-source-of-truth approach.
The ABI stability and compatibility with different versions of QEMU and
libvirt. We definitely want to minimize duplicity and use QEMU as much
as possible to implement our APIs and semantics. For example, the all of
the CPU model related functionality is offloaded to QEMU on s390. But
x86_64 lacks the required parts no to mention that the CPU modelling
there is much more complicated.
Jirka
7:35 a.m.
On Wed, Nov 23, 2022 at 02:12:59PM +0100, Jiri Denemark wrote:
Not to mention that QEMU changed names of several features and even
deprecated
the old spellings which is completely transparent to libvirt users as they can
still use the old names no matter what version of QEMU they use. This is one
of the key benefits of libvirt.
It's certainly very unfortunate that qemu did that (and we are still paying the
price for the arch-facilities/arch-capabilities thing). If I'm reading your
email right, you're saying that libvirt is (trying to?) move in a direction
where instead of having say src/cpu_map/x86_Icelake-Client.xml, instead there is
a very minimal layer that knows that some qemu feature got renamed, and is
effectively just a filter "papering over" such qemu issues.
And in particular it's no longer the case that every single cpu model, cpu model
version, and cpu feature needs commits to both qemu and libvirt, and they must
be in sync.
I'm presuming this will also mean plumbing through qemu model versions too, as
that's an essential part I think.
Do I have this right at all?
regards
john
7:45 a.m.
On Wed, Nov 23, 2022 at 01:35:42PM +0000, John Levon wrote:
On Wed, Nov 23, 2022 at 02:12:59PM +0100, Jiri Denemark wrote:
> Not to mention that QEMU changed names of several features and even deprecated
> the old spellings which is completely transparent to libvirt users as they can
> still use the old names no matter what version of QEMU they use. This is one
> of the key benefits of libvirt.
It's certainly very unfortunate that qemu did that (and we are still paying the
price for the arch-facilities/arch-capabilities thing).
Note, it depends on your POV of 'unfortunate'. Of course any time QEMU
changes something, it has an impact on things using QEMU, but libvirt
is here precisely to give isolation from those changes.
Indeed, the very fact that libvirt exists and is widely used, is what
allows QEMU to deprecated and rename stuff on a pretty aggressive
schedule. If libvirt wasn't providing this isolation, then it is
unlikely QEMU would have adopted its 2 release deprecated cycle,
and would be locked into their mistakes for a much longer period.
IOW, libvirt is allowing QEMU to fix their own technical debt on a
more aggressive timeframe, albeit at some cost to libvirt maintainers.
And in particular it's no longer the case that every single cpu
model, cpu model
version, and cpu feature needs commits to both qemu and libvirt, and they must
be in sync.
I would *always* expect there to be libvirt work to define the CPU
feature names, and possibly CPU model names. IIUC, we don't need
to copy over the entire CPU model expansion though, since we can
query that direct from QEMU given the model name.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:54 a.m.
On Wed, Nov 23, 2022 at 01:45:37PM +0000, Daniel P. Berrangé wrote:
Indeed, the very fact that libvirt exists and is widely used, is
what
allows QEMU to deprecated and rename stuff on a pretty aggressive
schedule. If libvirt wasn't providing this isolation, then it is
unlikely QEMU would have adopted its 2 release deprecated cycle,
and would be locked into their mistakes for a much longer period.
My understanding was that qemu no longer change existing CPU model definitions,
but instead introduce new versions (like IcelakeServer-V6), and that's how they
can make changes as needed. Or are they still arbitrarily changing/breaking
*existing* definitions in incompatible ways?
IOW, libvirt is allowing QEMU to fix their own technical debt on a
more aggressive timeframe, albeit at some cost to libvirt maintainers.
I'd be interested in one or two specific examples so I can understand this
properly.
thanks
john
7:59 a.m.
On Wed, Nov 23, 2022 at 01:54:50PM +0000, John Levon wrote:
On Wed, Nov 23, 2022 at 01:45:37PM +0000, Daniel P. Berrangé wrote:
> Indeed, the very fact that libvirt exists and is widely used, is what
> allows QEMU to deprecated and rename stuff on a pretty aggressive
> schedule. If libvirt wasn't providing this isolation, then it is
> unlikely QEMU would have adopted its 2 release deprecated cycle,
> and would be locked into their mistakes for a much longer period.
My understanding was that qemu no longer change existing CPU model definitions,
but instead introduce new versions (like IcelakeServer-V6), and that's how they
can make changes as needed. Or are they still arbitrarily changing/breaking
*existing* definitions in incompatible ways?
The existing CPU model versions should not be getting changed
by machine types any more. It hsould be purely additive with
new versions being introduced, at least for x86. Other arches
handle CPUs differently in QEMU :-(
> IOW, libvirt is allowing QEMU to fix their own technical debt on
a
> more aggressive timeframe, albeit at some cost to libvirt maintainers.
I'd be interested in one or two specific examples so I can understand this
properly.
The poster child example of QEMU evolving is the block layer
which has many syntax versions
* -hda / -sda / etc
* -drive if=ide,.....
* -drive if=none,... -device virtio-blk
* -blockdev driver=... -device virtio-blk
* -blockdev json:{...} -device virtio-blk
though at the same time its a bad example, as QEMU has failed to
actually get around to deleting the old syntaxes so far.
There is a quite a long list of things where QEMU has actually
remove the old syntax though:
https://www.qemu.org/docs/master/about/removed-features.html
and libvirt has adapted to a great many of those.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:13 a.m.
On Wed, Nov 23, 2022 at 01:59:54PM +0000, Daniel P. Berrangé wrote:
The poster child example of QEMU evolving is the block layer
which has many syntax versions
Sure, those sorts of things have indeed changed an awful lot, and libvirt
obviously brings value in that interface. But that feels IMHO quite different
from the CPU model/feature stuff. For one thing, it's unavoidable if you're
going to support multiple qemu versions.
The wholesale removal of Icelake-Client is the only salient example listed here,
right? Given this is in libvirt too, how did libvirt help with this?
regards
john
8:38 a.m.
On Wed, Nov 23, 2022 at 13:35:42 +0000, John Levon wrote:
On Wed, Nov 23, 2022 at 02:12:59PM +0100, Jiri Denemark wrote:
> Not to mention that QEMU changed names of several features and even deprecated
> the old spellings which is completely transparent to libvirt users as they can
> still use the old names no matter what version of QEMU they use. This is one
> of the key benefits of libvirt.
It's certainly very unfortunate that qemu did that (and we are still paying the
price for the arch-facilities/arch-capabilities thing). If I'm reading your
email right, you're saying that libvirt is (trying to?) move in a direction
where instead of having say src/cpu_map/x86_Icelake-Client.xml
That's mostly where I hope we can get. I'd love to see CPU models being
probed dynamically from QEMU. And especially the details of each CPU
models, i.e., what features it's going to enable. Well, for proper host
CPU detection we will still most likely need to have a mapping between
CPU signatures and models, but this is unrelated to the issue we're
discussing in the thread.
instead there is a very minimal layer that knows that some qemu
feature got renamed, and is effectively just a filter "papering over"
such qemu issues.
This depends on what you mean by a very minimal layer :-) See below...
And in particular it's no longer the case that every single cpu
model,
cpu model version (needs commits to both qemu and libvirt)
Yes, I hope so.
and cpu feature needs commits to both qemu and libvirt
No, libvirt will still need each CPU feature to be added explicitly. But
this is pretty much a non issue. Defining a new feature is fast and
simple (well, unless Intel invents yet another way of advertising
supported features) and the features never change.
and they must be in sync.
Features need to be in sync, but CPU models do not (and of course,
there's even nothing to sync if we probe the definition in runtime).
I'm presuming this will also mean plumbing through qemu model
versions
too, as that's an essential part I think.
Right.
Jirka
5:41 a.m.
On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
On Wed, Nov 23, 2022 at 11:18:01 +0000, Daniel P. Berrangé wrote:
> On Tue, Nov 22, 2022 at 01:48:39PM +0100, Jiri Denemark wrote:
> > On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
> > >
> > > On 18/11/22 5:00 pm, Jiri Denemark wrote:
> > > > On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
> > > >> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
> > > >>
> > > >>>> * Qemu already provide an option 'enforce' to
validate if features
> > > >>>> with which vm is started is exactly same as one
provided and nothing
> > > >>>> is silently dropped.
> > > >>> Right, but it's not enough. In addition to removed
features libvirt also
> > > >>> checks for unexpectedly added features. And you really need
to do both.
> > > >>> Because if you ask for -cpu Model,feat1=on,feat2=on,enforce
and QEMU
> > > >>> says everything is fine, the guest might see more than what
you asked.
> > > >>> For example, if a feature is enabled only if a host supports
it you may
> > > >>> or may not get it without any complains from QEMU. But if you
get it you
> > > >>> really need to explicitly ask for it during migration,
otherwise the
> > > >>> feature can just silently disappear. Of course, this would be
a really
> > > >>> bad behavior from QEMU, but that does not mean it can't
happen (I think
> > > >>> SVM is a bit problematic in this way) and the whole point of
libvirt's
> > > >>> checks is to prevent this kind of issues.
> > > >> Hi Jiri, I'm not following this very well. I think you're
saying that qemu has
> > > >> had bugs previously where features get silently enabled,
> > > > Personally, I haven't actually witnessed any bug in this area (as
far as
> > > > I can remember, which is not that far :-)), but got some reports of
at
> > > > least one, even though without any proof.
> > > >
> > > > Specifically, SVM is quite strange as it is included in all AMD CPU
> > > > models in QEMU and yet if you try to start it on a host without
nesting
> > > > enabled "enforce" does not complain. I saw the feature is
enabled with
> > > > older machine types, but I was told the magic behind this feature
looks
> > > > like not only machine type but even host configuration itself is
> > > > involved. Anyway, although I saw reports of that the feature could
be
> > > > enabled without an explicit request even with new machine types I
still
> > > > haven't seen any proof of this happening. So I hope it just does
not
> > > > happen and users are only afraid of this possibility.
> > > >
> > > >> and it's libvirt's job/role to paper over those issues?
Do you have
> > > >> some specific cases of this?
> > > > This is not about papering. It's actually the opposite, that is
about
> > > > detecting if something like this happens and reporting it as a
failure
> > > > rather than papering it and hoping everything goes well. So I think
> > > > doing this is a good idea even though I don't think we actually
saw any
> > > > issue of this kind.
> > >
> > >
> > > Hi Jiri, I see now with libvirt master, with check=='full' we
verify
> > > both silently dropped as well as added features. But as you already
> > > stated Qemu silently adding feature is a Qemu bug and libvirt just
> > > reports that bug, so it should be very unlikely, i agree that is not a
> > > good reasoning :). Our requirement is that we want to use CPU Models
> > > and features which are defined in Qemu but not in libvirt for e.g if
> > > we want to use Icelake-Server-V4 directly or newly added vmx feature,
> > > libvirt does not allow. Currently we take help of qemu to do
> > > validations but for cpu feature verfication and model definations we
> > > still use libvirt defined definations which prevent us to use anything
> > > which is not defined in libvirt. I see there are already efforts going
> > > on to get all model and feature defination from qemu itself, but not
> > > sure how much time it will take. Till that happens we thought safest
> > > option is to have an option to remove all validations from libvirt and
> > > rely on qemu 'enforce' for migration safetly. I understand
> > > qemu-enforce does not check for silently added features, but that case
> > > we can assume is very unlikely and Qemu should fix otherwise VMs will
> > > not poweron anyway with check=='full'. Basically we want it as an
> > > modification of check='none' but also skipping things like
> > > virCPUValidateFeatures and passing option 'enforce' to Qemu. Or
if
> > > silently adding features is that big concern we can have a check in
> > > Qemu itself? I understand current qemu-enforce is not as migration
> > > safe as check=='full' but probably suitable for our use case for
time
> > > being?
> >
> > I understand why you want this, but on the other hand adding just a
> > plain pass-through for CPU model and features is quite dangerous as it
> > can be used to set any -cpu option even if it's not a feature. And
> > especially the parts that are configured in other areas of domain XML
> > (such as hypervisor features). So I think instead of adding a new mode
> > for <cpu> we should go another way. For things that are not yet
> > supported by libvirt we support various elements in qemu namespace,
> > e.g., setting QEMU command line options, environment variables, or even
> > overriding options libvirt would normally set on the command line. So I
> > guess we could have a special <qemu:cpu> element which would be used
> > when a user wants full control over the -cpu option without any libvirt
> > intervention.
>
> I really don't think we should allow this, especially not for something
> which is clearly intended to be used for production deployment. Our
> hypervisor CLI passthrough is there to allow for short term workarounds
> for bugs, or to experiment with a feature before it is mapped into
> libvirt in a supported manner.
>
> If there are aspects related to QEMU configuration thiat are not in
> libvirt yet, we need to address those gaps, not provide yet another
> way to bypass libvirt mapping.
Indeed, this was definitely meant as a short term workaround for stuff
we don't have support for yet, for testing or experimental purposes. The
supported approach is for implement the missing parts in libvirt (and
QEMU) as soon as possible. I don't see why would a properly documented
support for experiments with -cpu would be an issue.
Why can't it just use the exsting QEMU passthrough syntax we have.
I don't think we should be adding specifial support just for CPUs
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
5:50 a.m.
On 23/11/22 5:11 pm, Daniel P. Berrangé wrote:
On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
> On Wed, Nov 23, 2022 at 11:18:01 +0000, Daniel P. Berrangé wrote:
>> On Tue, Nov 22, 2022 at 01:48:39PM +0100, Jiri Denemark wrote:
>>> On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
>>>> On 18/11/22 5:00 pm, Jiri Denemark wrote:
>>>>> On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
>>>>>> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark wrote:
>>>>>>
>>>>>>>> * Qemu already provide an option 'enforce' to
validate if features
>>>>>>>> with which vm is started is exactly same as one
provided and nothing
>>>>>>>> is silently dropped.
>>>>>>> Right, but it's not enough. In addition to removed
features libvirt also
>>>>>>> checks for unexpectedly added features. And you really need
to do both.
>>>>>>> Because if you ask for -cpu Model,feat1=on,feat2=on,enforce
and QEMU
>>>>>>> says everything is fine, the guest might see more than what
you asked.
>>>>>>> For example, if a feature is enabled only if a host supports
it you may
>>>>>>> or may not get it without any complains from QEMU. But if you
get it you
>>>>>>> really need to explicitly ask for it during migration,
otherwise the
>>>>>>> feature can just silently disappear. Of course, this would be
a really
>>>>>>> bad behavior from QEMU, but that does not mean it can't
happen (I think
>>>>>>> SVM is a bit problematic in this way) and the whole point of
libvirt's
>>>>>>> checks is to prevent this kind of issues.
>>>>>> Hi Jiri, I'm not following this very well. I think you're
saying that qemu has
>>>>>> had bugs previously where features get silently enabled,
>>>>> Personally, I haven't actually witnessed any bug in this area (as
far as
>>>>> I can remember, which is not that far :-)), but got some reports of
at
>>>>> least one, even though without any proof.
>>>>>
>>>>> Specifically, SVM is quite strange as it is included in all AMD CPU
>>>>> models in QEMU and yet if you try to start it on a host without
nesting
>>>>> enabled "enforce" does not complain. I saw the feature is
enabled with
>>>>> older machine types, but I was told the magic behind this feature
looks
>>>>> like not only machine type but even host configuration itself is
>>>>> involved. Anyway, although I saw reports of that the feature could
be
>>>>> enabled without an explicit request even with new machine types I
still
>>>>> haven't seen any proof of this happening. So I hope it just does
not
>>>>> happen and users are only afraid of this possibility.
>>>>>
>>>>>> and it's libvirt's job/role to paper over those issues?
Do you have
>>>>>> some specific cases of this?
>>>>> This is not about papering. It's actually the opposite, that is
about
>>>>> detecting if something like this happens and reporting it as a
failure
>>>>> rather than papering it and hoping everything goes well. So I think
>>>>> doing this is a good idea even though I don't think we actually
saw any
>>>>> issue of this kind.
>>>>
>>>> Hi Jiri, I see now with libvirt master, with check=='full' we
verify
>>>> both silently dropped as well as added features. But as you already
>>>> stated Qemu silently adding feature is a Qemu bug and libvirt just
>>>> reports that bug, so it should be very unlikely, i agree that is not a
>>>> good reasoning :). Our requirement is that we want to use CPU Models
>>>> and features which are defined in Qemu but not in libvirt for e.g if
>>>> we want to use Icelake-Server-V4 directly or newly added vmx feature,
>>>> libvirt does not allow. Currently we take help of qemu to do
>>>> validations but for cpu feature verfication and model definations we
>>>> still use libvirt defined definations which prevent us to use anything
>>>> which is not defined in libvirt. I see there are already efforts going
>>>> on to get all model and feature defination from qemu itself, but not
>>>> sure how much time it will take. Till that happens we thought safest
>>>> option is to have an option to remove all validations from libvirt and
>>>> rely on qemu 'enforce' for migration safetly. I understand
>>>> qemu-enforce does not check for silently added features, but that case
>>>> we can assume is very unlikely and Qemu should fix otherwise VMs will
>>>> not poweron anyway with check=='full'. Basically we want it as
an
>>>> modification of check='none' but also skipping things like
>>>> virCPUValidateFeatures and passing option 'enforce' to Qemu. Or
if
>>>> silently adding features is that big concern we can have a check in
>>>> Qemu itself? I understand current qemu-enforce is not as migration
>>>> safe as check=='full' but probably suitable for our use case for
time
>>>> being?
>>> I understand why you want this, but on the other hand adding just a
>>> plain pass-through for CPU model and features is quite dangerous as it
>>> can be used to set any -cpu option even if it's not a feature. And
>>> especially the parts that are configured in other areas of domain XML
>>> (such as hypervisor features). So I think instead of adding a new mode
>>> for <cpu> we should go another way. For things that are not yet
>>> supported by libvirt we support various elements in qemu namespace,
>>> e.g., setting QEMU command line options, environment variables, or even
>>> overriding options libvirt would normally set on the command line. So I
>>> guess we could have a special <qemu:cpu> element which would be used
>>> when a user wants full control over the -cpu option without any libvirt
>>> intervention.
>> I really don't think we should allow this, especially not for something
>> which is clearly intended to be used for production deployment. Our
>> hypervisor CLI passthrough is there to allow for short term workarounds
>> for bugs, or to experiment with a feature before it is mapped into
>> libvirt in a supported manner.
>>
>> If there are aspects related to QEMU configuration thiat are not in
>> libvirt yet, we need to address those gaps, not provide yet another
>> way to bypass libvirt mapping.
> Indeed, this was definitely meant as a short term workaround for stuff
> we don't have support for yet, for testing or experimental purposes. The
> supported approach is for implement the missing parts in libvirt (and
> QEMU) as soon as possible. I don't see why would a properly documented
> support for experiments with -cpu would be an issue.
Why can't it just use the exsting QEMU passthrough syntax we have.
I don't think we should be adding specifial support just for CPUs
With regards,
Daniel
Daniel, We actually want migratibility of VM too, i mean best set of migratable of
features supported by both source and destination Qemu. Sometimes important cpu
model/version or features take a lot of time to be updated in libvirt, also other class of
issues like what John mentioned multiple source of truths is very common.
Thanks
Manish Mishra
5:51 a.m.
On Wed, Nov 23, 2022 at 11:41:03 +0000, Daniel P. Berrangé wrote:
On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
> On Wed, Nov 23, 2022 at 11:18:01 +0000, Daniel P. Berrangé wrote:
> > On Tue, Nov 22, 2022 at 01:48:39PM +0100, Jiri Denemark wrote:
> > > On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
> > > >
> > > > On 18/11/22 5:00 pm, Jiri Denemark wrote:
> > > > > On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
> > > > >> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark
wrote:
> > > > >>
> > > > >>>> * Qemu already provide an option
'enforce' to validate if features
> > > > >>>> with which vm is started is exactly same as one
provided and nothing
> > > > >>>> is silently dropped.
> > > > >>> Right, but it's not enough. In addition to removed
features libvirt also
> > > > >>> checks for unexpectedly added features. And you really
need to do both.
> > > > >>> Because if you ask for -cpu
Model,feat1=on,feat2=on,enforce and QEMU
> > > > >>> says everything is fine, the guest might see more than
what you asked.
> > > > >>> For example, if a feature is enabled only if a host
supports it you may
> > > > >>> or may not get it without any complains from QEMU. But
if you get it you
> > > > >>> really need to explicitly ask for it during migration,
otherwise the
> > > > >>> feature can just silently disappear. Of course, this
would be a really
> > > > >>> bad behavior from QEMU, but that does not mean it
can't happen (I think
> > > > >>> SVM is a bit problematic in this way) and the whole
point of libvirt's
> > > > >>> checks is to prevent this kind of issues.
> > > > >> Hi Jiri, I'm not following this very well. I think
you're saying that qemu has
> > > > >> had bugs previously where features get silently enabled,
> > > > > Personally, I haven't actually witnessed any bug in this
area (as far as
> > > > > I can remember, which is not that far :-)), but got some reports
of at
> > > > > least one, even though without any proof.
> > > > >
> > > > > Specifically, SVM is quite strange as it is included in all AMD
CPU
> > > > > models in QEMU and yet if you try to start it on a host without
nesting
> > > > > enabled "enforce" does not complain. I saw the feature
is enabled with
> > > > > older machine types, but I was told the magic behind this
feature looks
> > > > > like not only machine type but even host configuration itself
is
> > > > > involved. Anyway, although I saw reports of that the feature
could be
> > > > > enabled without an explicit request even with new machine types
I still
> > > > > haven't seen any proof of this happening. So I hope it just
does not
> > > > > happen and users are only afraid of this possibility.
> > > > >
> > > > >> and it's libvirt's job/role to paper over those
issues? Do you have
> > > > >> some specific cases of this?
> > > > > This is not about papering. It's actually the opposite, that
is about
> > > > > detecting if something like this happens and reporting it as a
failure
> > > > > rather than papering it and hoping everything goes well. So I
think
> > > > > doing this is a good idea even though I don't think we
actually saw any
> > > > > issue of this kind.
> > > >
> > > >
> > > > Hi Jiri, I see now with libvirt master, with check=='full' we
verify
> > > > both silently dropped as well as added features. But as you already
> > > > stated Qemu silently adding feature is a Qemu bug and libvirt just
> > > > reports that bug, so it should be very unlikely, i agree that is not
a
> > > > good reasoning :). Our requirement is that we want to use CPU Models
> > > > and features which are defined in Qemu but not in libvirt for e.g if
> > > > we want to use Icelake-Server-V4 directly or newly added vmx
feature,
> > > > libvirt does not allow. Currently we take help of qemu to do
> > > > validations but for cpu feature verfication and model definations we
> > > > still use libvirt defined definations which prevent us to use
anything
> > > > which is not defined in libvirt. I see there are already efforts
going
> > > > on to get all model and feature defination from qemu itself, but not
> > > > sure how much time it will take. Till that happens we thought safest
> > > > option is to have an option to remove all validations from libvirt
and
> > > > rely on qemu 'enforce' for migration safetly. I understand
> > > > qemu-enforce does not check for silently added features, but that
case
> > > > we can assume is very unlikely and Qemu should fix otherwise VMs
will
> > > > not poweron anyway with check=='full'. Basically we want it
as an
> > > > modification of check='none' but also skipping things like
> > > > virCPUValidateFeatures and passing option 'enforce' to Qemu.
Or if
> > > > silently adding features is that big concern we can have a check in
> > > > Qemu itself? I understand current qemu-enforce is not as migration
> > > > safe as check=='full' but probably suitable for our use case
for time
> > > > being?
> > >
> > > I understand why you want this, but on the other hand adding just a
> > > plain pass-through for CPU model and features is quite dangerous as it
> > > can be used to set any -cpu option even if it's not a feature. And
> > > especially the parts that are configured in other areas of domain XML
> > > (such as hypervisor features). So I think instead of adding a new mode
> > > for <cpu> we should go another way. For things that are not yet
> > > supported by libvirt we support various elements in qemu namespace,
> > > e.g., setting QEMU command line options, environment variables, or even
> > > overriding options libvirt would normally set on the command line. So I
> > > guess we could have a special <qemu:cpu> element which would be
used
> > > when a user wants full control over the -cpu option without any libvirt
> > > intervention.
> >
> > I really don't think we should allow this, especially not for something
> > which is clearly intended to be used for production deployment. Our
> > hypervisor CLI passthrough is there to allow for short term workarounds
> > for bugs, or to experiment with a feature before it is mapped into
> > libvirt in a supported manner.
> >
> > If there are aspects related to QEMU configuration thiat are not in
> > libvirt yet, we need to address those gaps, not provide yet another
> > way to bypass libvirt mapping.
>
> Indeed, this was definitely meant as a short term workaround for stuff
> we don't have support for yet, for testing or experimental purposes. The
> supported approach is for implement the missing parts in libvirt (and
> QEMU) as soon as possible. I don't see why would a properly documented
> support for experiments with -cpu would be an issue.
Why can't it just use the exsting QEMU passthrough syntax we have.
I don't think we should be adding specifial support just for CPUs
That would be nice, but the QEMU passthrough syntax cannot be used for
changing options that libvirt already passes to QEMU. So using it would
likely result in two separate -cpu options on QEMU command line. And it
would not rule out the CPU verification code in libvirt. Remember, we
add a default -cpu model in case there's none configured in the XML.
Jirka
6:14 a.m.
On Wed, Nov 23, 2022 at 12:51:39PM +0100, Jiri Denemark wrote:
On Wed, Nov 23, 2022 at 11:41:03 +0000, Daniel P. Berrangé wrote:
> On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
> > On Wed, Nov 23, 2022 at 11:18:01 +0000, Daniel P. Berrangé wrote:
> > > On Tue, Nov 22, 2022 at 01:48:39PM +0100, Jiri Denemark wrote:
> > > > On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
> > > > >
> > > > > On 18/11/22 5:00 pm, Jiri Denemark wrote:
> > > > > > On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
> > > > > >> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark
wrote:
> > > > > >>
> > > > > >>>> * Qemu already provide an option
'enforce' to validate if features
> > > > > >>>> with which vm is started is exactly same as
one provided and nothing
> > > > > >>>> is silently dropped.
> > > > > >>> Right, but it's not enough. In addition to
removed features libvirt also
> > > > > >>> checks for unexpectedly added features. And you
really need to do both.
> > > > > >>> Because if you ask for -cpu
Model,feat1=on,feat2=on,enforce and QEMU
> > > > > >>> says everything is fine, the guest might see more
than what you asked.
> > > > > >>> For example, if a feature is enabled only if a host
supports it you may
> > > > > >>> or may not get it without any complains from QEMU.
But if you get it you
> > > > > >>> really need to explicitly ask for it during
migration, otherwise the
> > > > > >>> feature can just silently disappear. Of course,
this would be a really
> > > > > >>> bad behavior from QEMU, but that does not mean it
can't happen (I think
> > > > > >>> SVM is a bit problematic in this way) and the whole
point of libvirt's
> > > > > >>> checks is to prevent this kind of issues.
> > > > > >> Hi Jiri, I'm not following this very well. I think
you're saying that qemu has
> > > > > >> had bugs previously where features get silently
enabled,
> > > > > > Personally, I haven't actually witnessed any bug in
this area (as far as
> > > > > > I can remember, which is not that far :-)), but got some
reports of at
> > > > > > least one, even though without any proof.
> > > > > >
> > > > > > Specifically, SVM is quite strange as it is included in all
AMD CPU
> > > > > > models in QEMU and yet if you try to start it on a host
without nesting
> > > > > > enabled "enforce" does not complain. I saw the
feature is enabled with
> > > > > > older machine types, but I was told the magic behind this
feature looks
> > > > > > like not only machine type but even host configuration
itself is
> > > > > > involved. Anyway, although I saw reports of that the
feature could be
> > > > > > enabled without an explicit request even with new machine
types I still
> > > > > > haven't seen any proof of this happening. So I hope it
just does not
> > > > > > happen and users are only afraid of this possibility.
> > > > > >
> > > > > >> and it's libvirt's job/role to paper over those
issues? Do you have
> > > > > >> some specific cases of this?
> > > > > > This is not about papering. It's actually the opposite,
that is about
> > > > > > detecting if something like this happens and reporting it
as a failure
> > > > > > rather than papering it and hoping everything goes well. So
I think
> > > > > > doing this is a good idea even though I don't think we
actually saw any
> > > > > > issue of this kind.
> > > > >
> > > > >
> > > > > Hi Jiri, I see now with libvirt master, with
check=='full' we verify
> > > > > both silently dropped as well as added features. But as you
already
> > > > > stated Qemu silently adding feature is a Qemu bug and libvirt
just
> > > > > reports that bug, so it should be very unlikely, i agree that is
not a
> > > > > good reasoning :). Our requirement is that we want to use CPU
Models
> > > > > and features which are defined in Qemu but not in libvirt for
e.g if
> > > > > we want to use Icelake-Server-V4 directly or newly added vmx
feature,
> > > > > libvirt does not allow. Currently we take help of qemu to do
> > > > > validations but for cpu feature verfication and model
definations we
> > > > > still use libvirt defined definations which prevent us to use
anything
> > > > > which is not defined in libvirt. I see there are already efforts
going
> > > > > on to get all model and feature defination from qemu itself, but
not
> > > > > sure how much time it will take. Till that happens we thought
safest
> > > > > option is to have an option to remove all validations from
libvirt and
> > > > > rely on qemu 'enforce' for migration safetly. I
understand
> > > > > qemu-enforce does not check for silently added features, but
that case
> > > > > we can assume is very unlikely and Qemu should fix otherwise VMs
will
> > > > > not poweron anyway with check=='full'. Basically we want
it as an
> > > > > modification of check='none' but also skipping things
like
> > > > > virCPUValidateFeatures and passing option 'enforce' to
Qemu. Or if
> > > > > silently adding features is that big concern we can have a check
in
> > > > > Qemu itself? I understand current qemu-enforce is not as
migration
> > > > > safe as check=='full' but probably suitable for our use
case for time
> > > > > being?
> > > >
> > > > I understand why you want this, but on the other hand adding just a
> > > > plain pass-through for CPU model and features is quite dangerous as
it
> > > > can be used to set any -cpu option even if it's not a feature.
And
> > > > especially the parts that are configured in other areas of domain
XML
> > > > (such as hypervisor features). So I think instead of adding a new
mode
> > > > for <cpu> we should go another way. For things that are not
yet
> > > > supported by libvirt we support various elements in qemu namespace,
> > > > e.g., setting QEMU command line options, environment variables, or
even
> > > > overriding options libvirt would normally set on the command line. So
I
> > > > guess we could have a special <qemu:cpu> element which would be
used
> > > > when a user wants full control over the -cpu option without any
libvirt
> > > > intervention.
> > >
> > > I really don't think we should allow this, especially not for
something
> > > which is clearly intended to be used for production deployment. Our
> > > hypervisor CLI passthrough is there to allow for short term workarounds
> > > for bugs, or to experiment with a feature before it is mapped into
> > > libvirt in a supported manner.
> > >
> > > If there are aspects related to QEMU configuration thiat are not in
> > > libvirt yet, we need to address those gaps, not provide yet another
> > > way to bypass libvirt mapping.
> >
> > Indeed, this was definitely meant as a short term workaround for stuff
> > we don't have support for yet, for testing or experimental purposes. The
> > supported approach is for implement the missing parts in libvirt (and
> > QEMU) as soon as possible. I don't see why would a properly documented
> > support for experiments with -cpu would be an issue.
>
> Why can't it just use the exsting QEMU passthrough syntax we have.
> I don't think we should be adding specifial support just for CPUs
That would be nice, but the QEMU passthrough syntax cannot be used for
changing options that libvirt already passes to QEMU. So using it would
likely result in two separate -cpu options on QEMU command line. And it
would not rule out the CPU verification code in libvirt. Remember, we
add a default -cpu model in case there's none configured in the XML.
Two -cpu options isn't a problem, the latter will override the former,
which is fine from the level of support intended for QEMU passthrough
usage.
For the libvirt checking, isn't the 'check=none' attr sufficient
to skip checks libvirt does.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
6:28 a.m.
On 23/11/22 5:44 pm, Daniel P. Berrangé wrote:
On Wed, Nov 23, 2022 at 12:51:39PM +0100, Jiri Denemark wrote:
> On Wed, Nov 23, 2022 at 11:41:03 +0000, Daniel P. Berrangé wrote:
>> On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
>>> On Wed, Nov 23, 2022 at 11:18:01 +0000, Daniel P. Berrangé wrote:
>>>> On Tue, Nov 22, 2022 at 01:48:39PM +0100, Jiri Denemark wrote:
>>>>> On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
>>>>>> On 18/11/22 5:00 pm, Jiri Denemark wrote:
>>>>>>> On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon wrote:
>>>>>>>> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri Denemark
wrote:
>>>>>>>>
>>>>>>>>>> * Qemu already provide an option
'enforce' to validate if features
>>>>>>>>>> with which vm is started is exactly same as
one provided and nothing
>>>>>>>>>> is silently dropped.
>>>>>>>>> Right, but it's not enough. In addition to
removed features libvirt also
>>>>>>>>> checks for unexpectedly added features. And you
really need to do both.
>>>>>>>>> Because if you ask for -cpu
Model,feat1=on,feat2=on,enforce and QEMU
>>>>>>>>> says everything is fine, the guest might see more
than what you asked.
>>>>>>>>> For example, if a feature is enabled only if a host
supports it you may
>>>>>>>>> or may not get it without any complains from QEMU.
But if you get it you
>>>>>>>>> really need to explicitly ask for it during
migration, otherwise the
>>>>>>>>> feature can just silently disappear. Of course, this
would be a really
>>>>>>>>> bad behavior from QEMU, but that does not mean it
can't happen (I think
>>>>>>>>> SVM is a bit problematic in this way) and the whole
point of libvirt's
>>>>>>>>> checks is to prevent this kind of issues.
>>>>>>>> Hi Jiri, I'm not following this very well. I think
you're saying that qemu has
>>>>>>>> had bugs previously where features get silently enabled,
>>>>>>> Personally, I haven't actually witnessed any bug in this
area (as far as
>>>>>>> I can remember, which is not that far :-)), but got some
reports of at
>>>>>>> least one, even though without any proof.
>>>>>>>
>>>>>>> Specifically, SVM is quite strange as it is included in all
AMD CPU
>>>>>>> models in QEMU and yet if you try to start it on a host
without nesting
>>>>>>> enabled "enforce" does not complain. I saw the
feature is enabled with
>>>>>>> older machine types, but I was told the magic behind this
feature looks
>>>>>>> like not only machine type but even host configuration itself
is
>>>>>>> involved. Anyway, although I saw reports of that the feature
could be
>>>>>>> enabled without an explicit request even with new machine
types I still
>>>>>>> haven't seen any proof of this happening. So I hope it
just does not
>>>>>>> happen and users are only afraid of this possibility.
>>>>>>>
>>>>>>>> and it's libvirt's job/role to paper over those
issues? Do you have
>>>>>>>> some specific cases of this?
>>>>>>> This is not about papering. It's actually the opposite,
that is about
>>>>>>> detecting if something like this happens and reporting it as
a failure
>>>>>>> rather than papering it and hoping everything goes well. So I
think
>>>>>>> doing this is a good idea even though I don't think we
actually saw any
>>>>>>> issue of this kind.
>>>>>>
>>>>>> Hi Jiri, I see now with libvirt master, with
check=='full' we verify
>>>>>> both silently dropped as well as added features. But as you
already
>>>>>> stated Qemu silently adding feature is a Qemu bug and libvirt
just
>>>>>> reports that bug, so it should be very unlikely, i agree that is
not a
>>>>>> good reasoning :). Our requirement is that we want to use CPU
Models
>>>>>> and features which are defined in Qemu but not in libvirt for e.g
if
>>>>>> we want to use Icelake-Server-V4 directly or newly added vmx
feature,
>>>>>> libvirt does not allow. Currently we take help of qemu to do
>>>>>> validations but for cpu feature verfication and model definations
we
>>>>>> still use libvirt defined definations which prevent us to use
anything
>>>>>> which is not defined in libvirt. I see there are already efforts
going
>>>>>> on to get all model and feature defination from qemu itself, but
not
>>>>>> sure how much time it will take. Till that happens we thought
safest
>>>>>> option is to have an option to remove all validations from
libvirt and
>>>>>> rely on qemu 'enforce' for migration safetly. I
understand
>>>>>> qemu-enforce does not check for silently added features, but that
case
>>>>>> we can assume is very unlikely and Qemu should fix otherwise VMs
will
>>>>>> not poweron anyway with check=='full'. Basically we want
it as an
>>>>>> modification of check='none' but also skipping things
like
>>>>>> virCPUValidateFeatures and passing option 'enforce' to
Qemu. Or if
>>>>>> silently adding features is that big concern we can have a check
in
>>>>>> Qemu itself? I understand current qemu-enforce is not as
migration
>>>>>> safe as check=='full' but probably suitable for our use
case for time
>>>>>> being?
>>>>> I understand why you want this, but on the other hand adding just a
>>>>> plain pass-through for CPU model and features is quite dangerous as
it
>>>>> can be used to set any -cpu option even if it's not a feature.
And
>>>>> especially the parts that are configured in other areas of domain
XML
>>>>> (such as hypervisor features). So I think instead of adding a new
mode
>>>>> for <cpu> we should go another way. For things that are not
yet
>>>>> supported by libvirt we support various elements in qemu namespace,
>>>>> e.g., setting QEMU command line options, environment variables, or
even
>>>>> overriding options libvirt would normally set on the command line. So
I
>>>>> guess we could have a special <qemu:cpu> element which would be
used
>>>>> when a user wants full control over the -cpu option without any
libvirt
>>>>> intervention.
>>>> I really don't think we should allow this, especially not for
something
>>>> which is clearly intended to be used for production deployment. Our
>>>> hypervisor CLI passthrough is there to allow for short term workarounds
>>>> for bugs, or to experiment with a feature before it is mapped into
>>>> libvirt in a supported manner.
>>>>
>>>> If there are aspects related to QEMU configuration thiat are not in
>>>> libvirt yet, we need to address those gaps, not provide yet another
>>>> way to bypass libvirt mapping.
>>> Indeed, this was definitely meant as a short term workaround for stuff
>>> we don't have support for yet, for testing or experimental purposes. The
>>> supported approach is for implement the missing parts in libvirt (and
>>> QEMU) as soon as possible. I don't see why would a properly documented
>>> support for experiments with -cpu would be an issue.
>> Why can't it just use the exsting QEMU passthrough syntax we have.
>> I don't think we should be adding specifial support just for CPUs
> That would be nice, but the QEMU passthrough syntax cannot be used for
> changing options that libvirt already passes to QEMU. So using it would
> likely result in two separate -cpu options on QEMU command line. And it
> would not rule out the CPU verification code in libvirt. Remember, we
> add a default -cpu model in case there's none configured in the XML.
Two -cpu options isn't a problem, the latter will override the former,
which is fine from the level of support intended for QEMU passthrough
usage.
For the libvirt checking, isn't the 'check=none' attr sufficient
to skip checks libvirt does.
With regards,
Daniel
Actually even within check='none' libvirt does few checks e.g.
virCPUValidateFeatures if feature string passed is proper and defined in libvirt. We could
extend check='none' to remove even these kind of things. So Jiri suggested that
making such kind of changes in general path is not good idea so discussion went to using
separate cpu class <qemu:cpu>.
Thanks
Manish Mishra
6:30 a.m.
On Wed, Nov 23, 2022 at 05:58:41PM +0530, manish.mishra wrote:
On 23/11/22 5:44 pm, Daniel P. Berrangé wrote:
> On Wed, Nov 23, 2022 at 12:51:39PM +0100, Jiri Denemark wrote:
> > On Wed, Nov 23, 2022 at 11:41:03 +0000, Daniel P. Berrangé wrote:
> > > Why can't it just use the exsting QEMU passthrough syntax we have.
> > > I don't think we should be adding specifial support just for CPUs
> > That would be nice, but the QEMU passthrough syntax cannot be used for
> > changing options that libvirt already passes to QEMU. So using it would
> > likely result in two separate -cpu options on QEMU command line. And it
> > would not rule out the CPU verification code in libvirt. Remember, we
> > add a default -cpu model in case there's none configured in the XML.
> Two -cpu options isn't a problem, the latter will override the former,
> which is fine from the level of support intended for QEMU passthrough
> usage.
>
> For the libvirt checking, isn't the 'check=none' attr sufficient
> to skip checks libvirt does.
>
>
> With regards,
> Daniel
Actually even within check='none' libvirt does few checks e.g.
virCPUValidateFeatures if feature string passed is proper and
defined in libvirt. We could extend check='none' to remove even
these kind of things. So Jiri suggested that making such kind
of changes in general path is not good idea so discussion went
to using separate cpu class <qemu:cpu>.
There is no need to remove any such check, as you wouldn't
be specifying any <cpu> element with custom features set,
so the virCPUValidateFeuatres check should not be creating
any problems at that point.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:04 a.m.
On 23/11/22 6:00 pm, Daniel P. Berrangé wrote:
On Wed, Nov 23, 2022 at 05:58:41PM +0530, manish.mishra wrote:
> On 23/11/22 5:44 pm, Daniel P. Berrangé wrote:
>> On Wed, Nov 23, 2022 at 12:51:39PM +0100, Jiri Denemark wrote:
>>> On Wed, Nov 23, 2022 at 11:41:03 +0000, Daniel P. Berrangé wrote:
>>>> Why can't it just use the exsting QEMU passthrough syntax we have.
>>>> I don't think we should be adding specifial support just for CPUs
>>> That would be nice, but the QEMU passthrough syntax cannot be used for
>>> changing options that libvirt already passes to QEMU. So using it would
>>> likely result in two separate -cpu options on QEMU command line. And it
>>> would not rule out the CPU verification code in libvirt. Remember, we
>>> add a default -cpu model in case there's none configured in the XML.
>> Two -cpu options isn't a problem, the latter will override the former,
>> which is fine from the level of support intended for QEMU passthrough
>> usage.
>>
>> For the libvirt checking, isn't the 'check=none' attr sufficient
>> to skip checks libvirt does.
>>
>>
>> With regards,
>> Daniel
> Actually even within check='none' libvirt does few checks e.g.
> virCPUValidateFeatures if feature string passed is proper and
> defined in libvirt. We could extend check='none' to remove even
> these kind of things. So Jiri suggested that making such kind
> of changes in general path is not good idea so discussion went
> to using separate cpu class <qemu:cpu>.
There is no need to remove any such check, as you wouldn't
be specifying any <cpu> element with custom features set,
so the virCPUValidateFeuatres check should not be creating
any problems at that point.
oh i see you meant something like this,
<cpu mode='custom' match='exact' check='none'>
<numa>
<cell id='0' cpus='0-239' memory='8388608'
unit='KiB' memAccess='shared'/>
</numa>
</cpu>
<qemu:commandline>
<qemu:arg value='-cpu'/>
<qemu:arg value='enforce,Icelake-v4,x2apic=on,hv-time,kvm-pv-eoi=on'/>
</qemu:commandline>
oh ok gives us sometime to evaluate this will revert soon, I am more worried about if we
have to deal with other cpu related things too other than just cpu feature and model. Our
current usecase was only around cpu features and cpu model :).
Thanks
Manish Mishra
With regards,
Daniel
6:10 a.m.
On Wed, Nov 23, 2022 at 18:34:38 +0530, manish.mishra wrote:
oh i see you meant something like this,
<cpu mode='custom' match='exact' check='none'>
<numa>
<cell id='0' cpus='0-239' memory='8388608'
unit='KiB' memAccess='shared'/>
</numa>
</cpu>
<qemu:commandline>
<qemu:arg value='-cpu'/>
<qemu:arg
value='enforce,Icelake-v4,x2apic=on,hv-time,kvm-pv-eoi=on'/>
</qemu:commandline>
This won't work as libvirt should add a default CPU model there and
replace check='none' with check='full' once the domain starts.
But we actually have a way to do what you need even without any change
in libvirt:
<cpu mode='host-passthrough' check='none'>
<topology .../>
<numa>
...
</numa>
...
</cpu>
<qemu:commandline>
<qemu:arg value='-cpu'/>
<qemu:arg
value='Icelake-Server-v4,enforce,x2apic=on,hv-time,kvm-pv-eoi=on'/>
</qemu:commandline>
This will result in
-cpu host,migratable=on
...
-cpu Icelake-Server-v4,enforce,x2apic=on,hv-time,kvm-pv-eoi=on
and libvirt will not touch the check='none' attribute once the domain
starts and it would not complain about any unknown CPU features or
models as there's nothing to check in <cpu/>.
That is, there's no need to come up with anything special to support
passing arbitrary options to -cpu.
Jirka
7:19 a.m.
On 24/11/22 5:40 pm, Jiri Denemark wrote:
On Wed, Nov 23, 2022 at 18:34:38 +0530, manish.mishra wrote:
> oh i see you meant something like this,
>
> <cpu mode='custom' match='exact' check='none'>
> <numa>
> <cell id='0' cpus='0-239' memory='8388608'
unit='KiB' memAccess='shared'/>
> </numa>
> </cpu>
>
> <qemu:commandline>
> <qemu:arg value='-cpu'/>
> <qemu:arg
value='enforce,Icelake-v4,x2apic=on,hv-time,kvm-pv-eoi=on'/>
> </qemu:commandline>
This won't work as libvirt should add a default CPU model there and
replace check='none' with check='full' once the domain starts.
But we actually have a way to do what you need even without any change
in libvirt:
<cpu mode='host-passthrough' check='none'>
<topology .../>
<numa>
...
</numa>
...
</cpu>
<qemu:commandline>
<qemu:arg value='-cpu'/>
<qemu:arg
value='Icelake-Server-v4,enforce,x2apic=on,hv-time,kvm-pv-eoi=on'/>
</qemu:commandline>
This will result in
-cpu host,migratable=on
...
-cpu Icelake-Server-v4,enforce,x2apic=on,hv-time,kvm-pv-eoi=on
and libvirt will not touch the check='none' attribute once the domain
starts and it would not complain about any unknown CPU features or
models as there's nothing to check in <cpu/>.
That is, there's no need to come up with anything special to support
passing arbitrary options to -cpu.
Jirka
Thank you so much Jirka, let me give this a try with few different cases. I will let you
know how this goes or if we need anything extra.
Thanks
Manish Mishra
7:18 a.m.
On Wed, Nov 23, 2022 at 12:14:45 +0000, Daniel P. Berrangé wrote:
On Wed, Nov 23, 2022 at 12:51:39PM +0100, Jiri Denemark wrote:
> On Wed, Nov 23, 2022 at 11:41:03 +0000, Daniel P. Berrangé wrote:
> > On Wed, Nov 23, 2022 at 12:29:24PM +0100, Jiri Denemark wrote:
> > > On Wed, Nov 23, 2022 at 11:18:01 +0000, Daniel P. Berrangé wrote:
> > > > On Tue, Nov 22, 2022 at 01:48:39PM +0100, Jiri Denemark wrote:
> > > > > On Mon, Nov 21, 2022 at 12:47:58 +0530, manish.mishra wrote:
> > > > > >
> > > > > > On 18/11/22 5:00 pm, Jiri Denemark wrote:
> > > > > > > On Fri, Nov 18, 2022 at 10:18:59 +0000, John Levon
wrote:
> > > > > > >> On Fri, Nov 18, 2022 at 10:52:32AM +0100, Jiri
Denemark wrote:
> > > > > > >>
> > > > > > >>>> * Qemu already provide an option
'enforce' to validate if features
> > > > > > >>>> with which vm is started is exactly
same as one provided and nothing
> > > > > > >>>> is silently dropped.
> > > > > > >>> Right, but it's not enough. In addition to
removed features libvirt also
> > > > > > >>> checks for unexpectedly added features. And
you really need to do both.
> > > > > > >>> Because if you ask for -cpu
Model,feat1=on,feat2=on,enforce and QEMU
> > > > > > >>> says everything is fine, the guest might see
more than what you asked.
> > > > > > >>> For example, if a feature is enabled only if a
host supports it you may
> > > > > > >>> or may not get it without any complains from
QEMU. But if you get it you
> > > > > > >>> really need to explicitly ask for it during
migration, otherwise the
> > > > > > >>> feature can just silently disappear. Of
course, this would be a really
> > > > > > >>> bad behavior from QEMU, but that does not mean
it can't happen (I think
> > > > > > >>> SVM is a bit problematic in this way) and the
whole point of libvirt's
> > > > > > >>> checks is to prevent this kind of issues.
> > > > > > >> Hi Jiri, I'm not following this very well. I
think you're saying that qemu has
> > > > > > >> had bugs previously where features get silently
enabled,
> > > > > > > Personally, I haven't actually witnessed any bug
in this area (as far as
> > > > > > > I can remember, which is not that far :-)), but got
some reports of at
> > > > > > > least one, even though without any proof.
> > > > > > >
> > > > > > > Specifically, SVM is quite strange as it is included
in all AMD CPU
> > > > > > > models in QEMU and yet if you try to start it on a
host without nesting
> > > > > > > enabled "enforce" does not complain. I saw
the feature is enabled with
> > > > > > > older machine types, but I was told the magic behind
this feature looks
> > > > > > > like not only machine type but even host configuration
itself is
> > > > > > > involved. Anyway, although I saw reports of that the
feature could be
> > > > > > > enabled without an explicit request even with new
machine types I still
> > > > > > > haven't seen any proof of this happening. So I
hope it just does not
> > > > > > > happen and users are only afraid of this possibility.
> > > > > > >
> > > > > > >> and it's libvirt's job/role to paper over
those issues? Do you have
> > > > > > >> some specific cases of this?
> > > > > > > This is not about papering. It's actually the
opposite, that is about
> > > > > > > detecting if something like this happens and reporting
it as a failure
> > > > > > > rather than papering it and hoping everything goes
well. So I think
> > > > > > > doing this is a good idea even though I don't
think we actually saw any
> > > > > > > issue of this kind.
> > > > > >
> > > > > >
> > > > > > Hi Jiri, I see now with libvirt master, with
check=='full' we verify
> > > > > > both silently dropped as well as added features. But as you
already
> > > > > > stated Qemu silently adding feature is a Qemu bug and
libvirt just
> > > > > > reports that bug, so it should be very unlikely, i agree
that is not a
> > > > > > good reasoning :). Our requirement is that we want to use
CPU Models
> > > > > > and features which are defined in Qemu but not in libvirt
for e.g if
> > > > > > we want to use Icelake-Server-V4 directly or newly added
vmx feature,
> > > > > > libvirt does not allow. Currently we take help of qemu to
do
> > > > > > validations but for cpu feature verfication and model
definations we
> > > > > > still use libvirt defined definations which prevent us to
use anything
> > > > > > which is not defined in libvirt. I see there are already
efforts going
> > > > > > on to get all model and feature defination from qemu
itself, but not
> > > > > > sure how much time it will take. Till that happens we
thought safest
> > > > > > option is to have an option to remove all validations from
libvirt and
> > > > > > rely on qemu 'enforce' for migration safetly. I
understand
> > > > > > qemu-enforce does not check for silently added features,
but that case
> > > > > > we can assume is very unlikely and Qemu should fix
otherwise VMs will
> > > > > > not poweron anyway with check=='full'. Basically we
want it as an
> > > > > > modification of check='none' but also skipping
things like
> > > > > > virCPUValidateFeatures and passing option 'enforce'
to Qemu. Or if
> > > > > > silently adding features is that big concern we can have a
check in
> > > > > > Qemu itself? I understand current qemu-enforce is not as
migration
> > > > > > safe as check=='full' but probably suitable for our
use case for time
> > > > > > being?
> > > > >
> > > > > I understand why you want this, but on the other hand adding
just a
> > > > > plain pass-through for CPU model and features is quite dangerous
as it
> > > > > can be used to set any -cpu option even if it's not a
feature. And
> > > > > especially the parts that are configured in other areas of
domain XML
> > > > > (such as hypervisor features). So I think instead of adding a
new mode
> > > > > for <cpu> we should go another way. For things that are
not yet
> > > > > supported by libvirt we support various elements in qemu
namespace,
> > > > > e.g., setting QEMU command line options, environment variables,
or even
> > > > > overriding options libvirt would normally set on the command
line. So I
> > > > > guess we could have a special <qemu:cpu> element which
would be used
> > > > > when a user wants full control over the -cpu option without any
libvirt
> > > > > intervention.
> > > >
> > > > I really don't think we should allow this, especially not for
something
> > > > which is clearly intended to be used for production deployment. Our
> > > > hypervisor CLI passthrough is there to allow for short term
workarounds
> > > > for bugs, or to experiment with a feature before it is mapped into
> > > > libvirt in a supported manner.
> > > >
> > > > If there are aspects related to QEMU configuration thiat are not in
> > > > libvirt yet, we need to address those gaps, not provide yet another
> > > > way to bypass libvirt mapping.
> > >
> > > Indeed, this was definitely meant as a short term workaround for stuff
> > > we don't have support for yet, for testing or experimental purposes.
The
> > > supported approach is for implement the missing parts in libvirt (and
> > > QEMU) as soon as possible. I don't see why would a properly
documented
> > > support for experiments with -cpu would be an issue.
> >
> > Why can't it just use the exsting QEMU passthrough syntax we have.
> > I don't think we should be adding specifial support just for CPUs
>
> That would be nice, but the QEMU passthrough syntax cannot be used for
> changing options that libvirt already passes to QEMU. So using it would
> likely result in two separate -cpu options on QEMU command line. And it
> would not rule out the CPU verification code in libvirt. Remember, we
> add a default -cpu model in case there's none configured in the XML.
Two -cpu options isn't a problem, the latter will override the former,
which is fine from the level of support intended for QEMU passthrough
usage.
Oh, OK. In that case this would be usable. But see below.
For the libvirt checking, isn't the 'check=none' attr
sufficient
to skip checks libvirt does.
It does, although once a domain is started, we update the CPU definition
in the XML to use check='full'. So check='none' is only usable for
starting a domain. Migration will always use check='full'. We would need
to come up with a new check value to completely turn off this
functionality.
Jirka
5:20 a.m.
Hi Jiri, Thank you so much for response.
On 18/11/22 3:22 pm, Jiri Denemark wrote:
Hi.
On Wed, Nov 16, 2022 at 22:53:18 +0530, manish.mishra wrote:
> We had a requirement to skip all CPU feature validations from libvirt
> and totally rely on Qemu for following reasons.
You are mixing two different things here. The "enforce" CPU checking for
making sure a guest gets exactly what the user asked QEMU for and
missing CPU models or features support in libvirt.
I actually meant, if we do checks in libvirt then if some cpu features or models are
missing in libvirt we can not use those. 'enforce' was just fallback for checks in
that case as atleast someone need to verify if what we passed is taken so that live
migrations does not have issues.
> * As libvirt always lies behind in term of cpu feature
management
> compare to qemu, so we may not be able to use all of the latest
> support provided by qemu till it is updated in libvirt too. For
> example like if new cpu features are added in qemu but not yet
> updated in libvirt.
So you effectively ask for a "passthrough" configuration, that is, CPU
model and features would be passed directly to QEMU without libvirt even
looking at them, right?
Yes but we want live migrations too.
Anyway, the current goal is to make libvirt ask for supported CPU models
including their definition so that we don't have to keep our own
definitions. That is, libvirt would support all models as they are added
to QEMU without any additional work. Not sure if CPU features would
follow, but adding them is fairly easy and what's more important they
don't change in time (unlike CPU models).
Jiri, you mean libvirt will ask qemu for all supported models and features instead of
maintaining its own list? Is it there in libvirt currently? We also had similar idea in
one of our KVM forum talk but thought it is little long task so may take some time, till
then we thought we can have this workaround.
> * Libvirt checks if a features is supported or not based on
cpuid, a
> feature supported by host may not be supported by qemu or there can
> be some features which are emulated by Qemu so libvirt level
> checking does not make sense in those cases.
This has not been the case for several years already. Libvirt asks QEMU
what CPU features it can provide on the host (either natively or
emulated) and checks against them. So no issue here. Well, expect that
some features are strange and QEMU does not report them as enabled for
-cpu host even though it would enable them for some specific CPU models.
But that's more in a "bug" area which needs to be solved between QEMU
and libvirt rather than being it a principal issue.
You mean with check =='full', i never tried that but wondered how it works for cpu
feature strings which are not defined in libvirt, because if some feature is not defined
in libvirt we fail in basic checks like virCPUValidateFeatures. Currently also i agree
libvirt asks Qemu for CPU features but i thought that is just for domcapability output or
qemuCapability cache file but it was not used in checks.
> * Qemu already provide an option 'enforce' to validate
if features
> with which vm is started is exactly same as one provided and nothing
> is silently dropped.
Right, but it's not enough. In addition to removed features libvirt also
checks for unexpectedly added features. And you really need to do both.
Because if you ask for -cpu Model,feat1=on,feat2=on,enforce and QEMU
says everything is fine, the guest might see more than what you asked.
For example, if a feature is enabled only if a host supports it you may
or may not get it without any complains from QEMU. But if you get it you
really need to explicitly ask for it during migration, otherwise the
feature can just silently disappear. Of course, this would be a really
bad behavior from QEMU, but that does not mean it can't happen (I think
SVM is a bit problematic in this way) and the whole point of libvirt's
checks is to prevent this kind of issues.
So once QEMU starts we checks all features enabled in the vCPU and note
those that were (unexpectedly) added so that we can explicitly ask for
them during migration and also check that no other features were added
on top. And for this whole process to work, we need to understand, i.e.,
have explicit support, for all CPU features (QEMU report a lot of CPU
properties and we need to know which one needs to be stored as enabled
features) and CPU models to make sure both sides of migration understand
the CPU definition in domain XML in the same way.
Our assumption was that qemu-enforce will make sure live migrations are safe as nothing
can be silently dropped with enforce and it will be user's resposnsibilty to always
pass suported features. But i was not aware of this, that Qemu can drop some features
silently even with enforce, Jiri can you please point us to some of the example for this.
How libvirt handles these cases as of now with check == 'full' ? Then again i had
same doubt does it work fine for features which are defined in qemu but not in libvirt?
> Basically we want another check option like
'qemu-enforce' which will
> skip all feature and cpu model verfications in libvirt and pass
> 'enforce' option to qemu. It will work similar to check 'none' i
mean
> no check to verify if provide features are supported by host, also on
> top of that skip things like virCPUValidateFeatures where ever
> required.
In short, I don't think this is a good idea and we should or could
support it and still maintain migration safety. If you don't care about
migration safety, you don't need to care about "enforce" QEMU check
either and can just use host-passthrough and get whatever QEMU supports
on the host.
Jirka
Thanks
Manish Mishra
721
days inactive
729
days old
31 comments
4 participants
participants (4)
-
Daniel P. Berrangé -
Jiri Denemark -
John Levon -
manish.mishra