Re: [PATCH RFC 1/1] qemu: capabilities: disable csske for host cpu

[Christian Borntraeger]

Am 11.03.22 um 16:24 schrieb Christian Borntraeger:



Am 11.03.22 um 15:56 schrieb Daniel P. Berrangé:

On Fri, Mar 11, 2022 at 03:52:57PM +0100, Christian Borntraeger wrote:



Am 11.03.22 um 14:08 schrieb Daniel P. Berrangé:

On Fri, Mar 11, 2022 at 12:37:46PM +, Daniel P. Berrangé wrote:

On Fri, Mar 11, 2022 at 01:12:35PM +0100, Christian Borntraeger wrote:



Am 11.03.22 um 10:23 schrieb David Hildenbrand:

On 11.03.22 10:17, Daniel P. Berrangé wrote:

On Thu, Mar 10, 2022 at 11:17:38PM -0500, Collin Walling wrote:

CPU models past gen16a will no longer support the csske feature. In
order to secure migration of guests running on machines that still
support this feature to machines that do not, let's disable csske
in the host-model.


Sorry to say, removing CPU features is a no-go when wanting to guarantee
forward migration without taking care about CPU model details manually
and simply using the host model. Self-made HW vendor problem.


And this simply does not reflect reality. Intel and Power have removed TX
for example. We can now sit back and please ourselves how we live in our
world of dreams. Or we can try to define an interface that deals with
reality and actually solves problems.


This proposal wouldn't have helped in the case of Intel removing
TSX, because it was removed without prior warning in the middle
of the product lifecycle. At that time there were already millions
of VMs in existance using the removed feature.


The problem scenario you describe is the intended semantics of
host-model though. It enables all features available in the host
that you launched on. It lets you live migrate to a target host
with the same, or a greater number of features. If the target has
a greater number of features, it should restrict the VM to the
subset of features that were present on the original source CPU.
If the target has fewer features, then you simply can't live
migrate a VM using host-model.

To get live migration in both directions across CPUs with differing
featuresets, then the VM needs to be configured with a named CPU
model that is a subset of both, rather than host-model.


Right, and cpu-model-baseline does that job for you if you're lazy to
lookup the proper model.


Yes baseline will work, but this requires tooling like openstack. The normal
user will just use the default and this is host-model.

Let me explain the usecase for this feature. Migration between different versins
baseline: always works
host-passthrough: you get what you deserve
default model: works
We have disabled CSSKE from our default models (-cpu gen15a will not present 
csske).
So that works as well.
host-model: Also works for all machines that have csske.
Now: Lets say gen17 will no longer support this. That means that we can not 
migrate
host-model from gen16 or gen15 because those will have csske.
What options do we have? If we disable csske in the host capabilities that 
would mean
that a host compare against an xml from an older QEMU would fail (even if you 
move
from gen14 to gen14). So this is not a good option.

By disabling deprecated features ONLY for the _initial_ expansion of 
model-model, but
keeping it in the host capabilities you can migrate existing guests (with the
feature) as we only disable in the expansion, but manually asking for it still 
works.
AND it will allow to move this instantiation of the guest to future machines 
without
the feature. Basically everything works.


The change you proposal works functionally, but none the less it is
changing the semantics of host-model. It is defined to expose all the
features in the host, and the proposal changes yet. If an app actually
/wants/ to use the deprecated feature and it exists in the host, then
host-model should be allowing that as it does today.

The problem scenario you describe is ultimately that OpenStack does
not have a future proof default CPU choice. Libvirt and QEMU provide
a mechanism for them to pick other CPU models that would address the
problem, but they're not using that. The challenge is that OpenStack
defaults currently are a zero-interaction thing.

They could retain their zero-interaction defaults, if at install time
they queried the libvirt capabilities to learn which named CPU models
are available, whereupon they could decide to use gen15a.  The main
challenge here is that the list of named CPU models is an unordered
set, so it is hard to programatically figure out which of the available
named CPU models is the newest/best/recommended.

IOW, what's missing is a way for apps to easily identify that 'gen15a'
is the best CPU to use on the host, without needing human interaction.


I think this could be solved with a change to query-cpu-definitions
in QEMU, to add an extra 'recommended: bool' attribute to the
CpuDefinitionInfo struct.  This would be defined to be only set for
1 CPU model in the list, and would reflect the recommended CPU model
given the current version of QEMU, kernel and hardware. Or we could
allow

Re: [PATCH RFC 1/1] qemu: capabilities: disable csske for host cpu

[Christian Borntraeger]

Am 11.03.22 um 15:56 schrieb Daniel P. Berrangé:

On Fri, Mar 11, 2022 at 03:52:57PM +0100, Christian Borntraeger wrote:



Am 11.03.22 um 14:08 schrieb Daniel P. Berrangé:

On Fri, Mar 11, 2022 at 12:37:46PM +, Daniel P. Berrangé wrote:

On Fri, Mar 11, 2022 at 01:12:35PM +0100, Christian Borntraeger wrote:



Am 11.03.22 um 10:23 schrieb David Hildenbrand:

On 11.03.22 10:17, Daniel P. Berrangé wrote:

On Thu, Mar 10, 2022 at 11:17:38PM -0500, Collin Walling wrote:

CPU models past gen16a will no longer support the csske feature. In
order to secure migration of guests running on machines that still
support this feature to machines that do not, let's disable csske
in the host-model.


Sorry to say, removing CPU features is a no-go when wanting to guarantee
forward migration without taking care about CPU model details manually
and simply using the host model. Self-made HW vendor problem.


And this simply does not reflect reality. Intel and Power have removed TX
for example. We can now sit back and please ourselves how we live in our
world of dreams. Or we can try to define an interface that deals with
reality and actually solves problems.


This proposal wouldn't have helped in the case of Intel removing
TSX, because it was removed without prior warning in the middle
of the product lifecycle. At that time there were already millions
of VMs in existance using the removed feature.


The problem scenario you describe is the intended semantics of
host-model though. It enables all features available in the host
that you launched on. It lets you live migrate to a target host
with the same, or a greater number of features. If the target has
a greater number of features, it should restrict the VM to the
subset of features that were present on the original source CPU.
If the target has fewer features, then you simply can't live
migrate a VM using host-model.

To get live migration in both directions across CPUs with differing
featuresets, then the VM needs to be configured with a named CPU
model that is a subset of both, rather than host-model.


Right, and cpu-model-baseline does that job for you if you're lazy to
lookup the proper model.


Yes baseline will work, but this requires tooling like openstack. The normal
user will just use the default and this is host-model.

Let me explain the usecase for this feature. Migration between different versins
baseline: always works
host-passthrough: you get what you deserve
default model: works
We have disabled CSSKE from our default models (-cpu gen15a will not present 
csske).
So that works as well.
host-model: Also works for all machines that have csske.
Now: Lets say gen17 will no longer support this. That means that we can not 
migrate
host-model from gen16 or gen15 because those will have csske.
What options do we have? If we disable csske in the host capabilities that 
would mean
that a host compare against an xml from an older QEMU would fail (even if you 
move
from gen14 to gen14). So this is not a good option.

By disabling deprecated features ONLY for the _initial_ expansion of 
model-model, but
keeping it in the host capabilities you can migrate existing guests (with the
feature) as we only disable in the expansion, but manually asking for it still 
works.
AND it will allow to move this instantiation of the guest to future machines 
without
the feature. Basically everything works.


The change you proposal works functionally, but none the less it is
changing the semantics of host-model. It is defined to expose all the
features in the host, and the proposal changes yet. If an app actually
/wants/ to use the deprecated feature and it exists in the host, then
host-model should be allowing that as it does today.

The problem scenario you describe is ultimately that OpenStack does
not have a future proof default CPU choice. Libvirt and QEMU provide
a mechanism for them to pick other CPU models that would address the
problem, but they're not using that. The challenge is that OpenStack
defaults currently are a zero-interaction thing.

They could retain their zero-interaction defaults, if at install time
they queried the libvirt capabilities to learn which named CPU models
are available, whereupon they could decide to use gen15a.  The main
challenge here is that the list of named CPU models is an unordered
set, so it is hard to programatically figure out which of the available
named CPU models is the newest/best/recommended.

IOW, what's missing is a way for apps to easily identify that 'gen15a'
is the best CPU to use on the host, without needing human interaction.


I think this could be solved with a change to query-cpu-definitions
in QEMU, to add an extra 'recommended: bool' attribute to the
CpuDefinitionInfo struct.  This would be defined to be only set for
1 CPU model in the list, and would reflect the recommended CPU model
given the current version of QEMU, kernel and hardware. Or we could
allow 'recommended' to be set for more than 1 CPU, provided we

Re: [PATCH RFC 1/1] qemu: capabilities: disable csske for host cpu

[Christian Borntraeger]

Am 11.03.22 um 14:08 schrieb Daniel P. Berrangé:

On Fri, Mar 11, 2022 at 12:37:46PM +, Daniel P. Berrangé wrote:

On Fri, Mar 11, 2022 at 01:12:35PM +0100, Christian Borntraeger wrote:



Am 11.03.22 um 10:23 schrieb David Hildenbrand:

On 11.03.22 10:17, Daniel P. Berrangé wrote:

On Thu, Mar 10, 2022 at 11:17:38PM -0500, Collin Walling wrote:

CPU models past gen16a will no longer support the csske feature. In
order to secure migration of guests running on machines that still
support this feature to machines that do not, let's disable csske
in the host-model.


Sorry to say, removing CPU features is a no-go when wanting to guarantee
forward migration without taking care about CPU model details manually
and simply using the host model. Self-made HW vendor problem.


And this simply does not reflect reality. Intel and Power have removed TX
for example. We can now sit back and please ourselves how we live in our
world of dreams. Or we can try to define an interface that deals with
reality and actually solves problems.


This proposal wouldn't have helped in the case of Intel removing
TSX, because it was removed without prior warning in the middle
of the product lifecycle. At that time there were already millions
of VMs in existance using the removed feature.


The problem scenario you describe is the intended semantics of
host-model though. It enables all features available in the host
that you launched on. It lets you live migrate to a target host
with the same, or a greater number of features. If the target has
a greater number of features, it should restrict the VM to the
subset of features that were present on the original source CPU.
If the target has fewer features, then you simply can't live
migrate a VM using host-model.

To get live migration in both directions across CPUs with differing
featuresets, then the VM needs to be configured with a named CPU
model that is a subset of both, rather than host-model.


Right, and cpu-model-baseline does that job for you if you're lazy to
lookup the proper model.


Yes baseline will work, but this requires tooling like openstack. The normal
user will just use the default and this is host-model.

Let me explain the usecase for this feature. Migration between different versins
baseline: always works
host-passthrough: you get what you deserve
default model: works
We have disabled CSSKE from our default models (-cpu gen15a will not present 
csske).
So that works as well.
host-model: Also works for all machines that have csske.
Now: Lets say gen17 will no longer support this. That means that we can not 
migrate
host-model from gen16 or gen15 because those will have csske.
What options do we have? If we disable csske in the host capabilities that 
would mean
that a host compare against an xml from an older QEMU would fail (even if you 
move
from gen14 to gen14). So this is not a good option.

By disabling deprecated features ONLY for the _initial_ expansion of 
model-model, but
keeping it in the host capabilities you can migrate existing guests (with the
feature) as we only disable in the expansion, but manually asking for it still 
works.
AND it will allow to move this instantiation of the guest to future machines 
without
the feature. Basically everything works.


The change you proposal works functionally, but none the less it is
changing the semantics of host-model. It is defined to expose all the
features in the host, and the proposal changes yet. If an app actually
/wants/ to use the deprecated feature and it exists in the host, then
host-model should be allowing that as it does today.

The problem scenario you describe is ultimately that OpenStack does
not have a future proof default CPU choice. Libvirt and QEMU provide
a mechanism for them to pick other CPU models that would address the
problem, but they're not using that. The challenge is that OpenStack
defaults currently are a zero-interaction thing.

They could retain their zero-interaction defaults, if at install time
they queried the libvirt capabilities to learn which named CPU models
are available, whereupon they could decide to use gen15a.  The main
challenge here is that the list of named CPU models is an unordered
set, so it is hard to programatically figure out which of the available
named CPU models is the newest/best/recommended.

IOW, what's missing is a way for apps to easily identify that 'gen15a'
is the best CPU to use on the host, without needing human interaction.


I think this could be solved with a change to query-cpu-definitions
in QEMU, to add an extra 'recommended: bool' attribute to the
CpuDefinitionInfo struct.  This would be defined to be only set for
1 CPU model in the list, and would reflect the recommended CPU model
given the current version of QEMU, kernel and hardware. Or we could
allow 'recommended' to be set for more than 1 CPU, provided we define
an explicit ordering of returned CPU models.


I like the recommended: bool attribute. It should provide what we need

Re: [PATCH RFC 1/1] qemu: capabilities: disable csske for host cpu

[Christian Borntraeger]

Am 11.03.22 um 13:27 schrieb David Hildenbrand:

On 11.03.22 13:12, Christian Borntraeger wrote:



Am 11.03.22 um 10:23 schrieb David Hildenbrand:

On 11.03.22 10:17, Daniel P. Berrangé wrote:

On Thu, Mar 10, 2022 at 11:17:38PM -0500, Collin Walling wrote:

CPU models past gen16a will no longer support the csske feature. In
order to secure migration of guests running on machines that still
support this feature to machines that do not, let's disable csske
in the host-model.


Sorry to say, removing CPU features is a no-go when wanting to guarantee
forward migration without taking care about CPU model details manually
and simply using the host model. Self-made HW vendor problem.


And this simply does not reflect reality. Intel and Power have removed TX
for example. We can now sit back and please ourselves how we live in our
world of dreams. Or we can try to define an interface that deals with
reality and actually solves problems.



Ehm, so, I spell out the obvious and get such a reaction? Okay, thank you.


Sorry, reading my writing again shows that I clearly miscommunicated in a
very bad style. My point was rather trying to solve a problem instead
I wrote something up in a hurry which resulted in something offensive.

Please accept my apologies.

Re: [PATCH RFC 0/1] s390x CPU Model Feature Deprecation

[Christian Borntraeger]

Am 11.03.22 um 10:30 schrieb David Hildenbrand:

On 11.03.22 05:17, Collin Walling wrote:

The s390x architecture has a growing list of features that will no longer
be supported on future hardware releases. This introduces an issue with
migration such that guests, running on models with these features enabled,
will be rejected outright by machines that do not support these features.

A current example is the CSSKE feature that has been deprecated for some time.
It has been publicly announced that gen15 will be the last release to
support this feature, however we have postponed this to gen16a. A possible
solution to remedy this would be to create a new QEMU QMP Response that allows
users to query for deprecated/unsupported features.

This presents two parts of the puzzle: how to report deprecated features to
a user (libvirt) and how should libvirt handle this information.

First, let's discuss the latter. The patch presented alongside this cover letter
attempts to solve the migration issue by hard-coding the CSSKE feature to be
disabled for all s390x CPU models. This is done by simply appending the CSSKE
feature with the disabled policy to the host-model.

libvirt pseudo:

if arch is s390x
 set CSSKE to disabled for host-model


That violates host-model semantics and possibly the user intend. There
would have to be some toggle to manually specify this, for example, a
new model type or a some magical flag.


What we actually want to do is to disable csske completely from QEMU and
thus from the host-model. Then it would not violate the spec.
But this has all kind of issues (you cannot migrate from older versions
of software and machines) although the hardware still can provide the feature.

The hardware guys promised me to deprecate things two generations earlier
and we usually deprecate things that are not used or where software has a
runtime switch.

From what I hear from you is that you do not want to modify the host-model
semantics to something more useful but rather define a new thing (e.g. 
"host-sane") ?



Gluing this to the "host-model" feels wrong.

The other concern I have is that deprecated features are a moving
target, and with a new QEMU version you could suddenly have more
deprecated features. Hm.


Maybe you'd want some kind of a host-based-model from QEMU that does
this automatically? I need more coffee to get creative on a name.

Re: [PATCH RFC 1/1] qemu: capabilities: disable csske for host cpu

[Christian Borntraeger]

Am 11.03.22 um 10:23 schrieb David Hildenbrand:

On 11.03.22 10:17, Daniel P. Berrangé wrote:

On Thu, Mar 10, 2022 at 11:17:38PM -0500, Collin Walling wrote:

CPU models past gen16a will no longer support the csske feature. In
order to secure migration of guests running on machines that still
support this feature to machines that do not, let's disable csske
in the host-model.


Sorry to say, removing CPU features is a no-go when wanting to guarantee
forward migration without taking care about CPU model details manually
and simply using the host model. Self-made HW vendor problem.


And this simply does not reflect reality. Intel and Power have removed TX
for example. We can now sit back and please ourselves how we live in our
world of dreams. Or we can try to define an interface that deals with
reality and actually solves problems.





The problem scenario you describe is the intended semantics of
host-model though. It enables all features available in the host
that you launched on. It lets you live migrate to a target host
with the same, or a greater number of features. If the target has
a greater number of features, it should restrict the VM to the
subset of features that were present on the original source CPU.
If the target has fewer features, then you simply can't live
migrate a VM using host-model.

To get live migration in both directions across CPUs with differing
featuresets, then the VM needs to be configured with a named CPU
model that is a subset of both, rather than host-model.


Right, and cpu-model-baseline does that job for you if you're lazy to
lookup the proper model.


Yes baseline will work, but this requires tooling like openstack. The normal
user will just use the default and this is host-model.

Let me explain the usecase for this feature. Migration between different versins
baseline: always works
host-passthrough: you get what you deserve
default model: works
We have disabled CSSKE from our default models (-cpu gen15a will not present 
csske).
So that works as well.
host-model: Also works for all machines that have csske.
Now: Lets say gen17 will no longer support this. That means that we can not 
migrate
host-model from gen16 or gen15 because those will have csske.
What options do we have? If we disable csske in the host capabilities that 
would mean
that a host compare against an xml from an older QEMU would fail (even if you 
move
from gen14 to gen14). So this is not a good option.

By disabling deprecated features ONLY for the _initial_ expansion of 
model-model, but
keeping it in the host capabilities you can migrate existing guests (with the
feature) as we only disable in the expansion, but manually asking for it still 
works.
AND it will allow to move this instantiation of the guest to future machines 
without
the feature. Basically everything works.

The alternative of removing csske would result in too many failure scenarios.

Re: [PATCH] qemu: validate: Allow 'preserve' action for on_crash lifecycle action

[Christian Borntraeger]

On 15.09.21 13:14, Peter Krempa wrote:

In fact keeping the VM around for debugging is a desirable configuration
and actually the implementation has no code as we keep the VM around.

Remove the validation and add a note that it's actually used.

Fixes: b1b85a475fb251b9068b75f629479f5c452f1b43
Reported-by: Christian Borntraeger 

Acked-by: Christian Borntraeger 


Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_driver.c   | 1 +
  src/qemu/qemu_validate.c | 5 ++---
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dfc27572c4..6ae678b165 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3647,6 +3647,7 @@ processGuestPanicEvent(virQEMUDriver *driver,
  break;

  case VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE:
+/* the VM is kept around for debugging */
  break;

  default:
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 9d93f373ab..6b685881a8 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1083,10 +1083,9 @@ qemuValidateLifecycleAction(virDomainLifecycleAction 
onPoweroff,
  /* The qemu driver doesn't yet implement any meaningful handling for
   * VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE */
  if (onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
-onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
-onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
+onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("qemu driver doesn't support the 'preserve' action for 
'on_reboot'/'on_poweroff'/'on_crash'"));
+   _("qemu driver doesn't support the 'preserve' action for 
'on_reboot'/'on_poweroff'"));
  return -1;
  }

Re: [PATCH 07/22] qemu: Reject 'preserve' action for 'on_reboot'/'on_poweroff'/'on_crash'

[Christian Borntraeger]

On 15.09.21 11:45, Christian Borntraeger wrote:

On 24.08.21 16:44, Peter Krempa wrote:

The qemu driver didn't ever implement any meaningful handling for the
'preserve' action.

Forbid the flag in the qemu def validator and update the documentation
to be factual.

Signed-off-by: Peter Krempa 


NACK. It is a perfectly sane usecase to have "preserve" on a crash. The qemu
will simply sit in that state and the user can then do virsh dump or similar.



To make this statement stronger. I use this A LOT in real life to force libvirt
to leave the QEMU in that state to get dump and debug info.

Re: [PATCH 07/22] qemu: Reject 'preserve' action for 'on_reboot'/'on_poweroff'/'on_crash'

[Christian Borntraeger]

On 24.08.21 16:44, Peter Krempa wrote:

The qemu driver didn't ever implement any meaningful handling for the
'preserve' action.

Forbid the flag in the qemu def validator and update the documentation
to be factual.

Signed-off-by: Peter Krempa 


NACK. It is a perfectly sane usecase to have "preserve" on a crash. The qemu
will simply sit in that state and the user can then do virsh dump or similar.



---
  docs/formatdomain.rst|  3 +--
  src/qemu/qemu_validate.c | 10 ++
  2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index fba4765e35..a894a51c00 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1747,8 +1747,7 @@ Each of these states allow for the same four possible 
actions.
 supported by the libxl hypervisor driver.)

  QEMU/KVM supports the ``on_poweroff`` and ``on_reboot`` events handling the
-``destroy`` and ``restart`` actions. The ``preserve`` action for an
-``on_reboot`` event is treated as a ``destroy``.
+``destroy`` and ``restart`` actions.

  The ``on_crash`` event supports these additional actions :since:`since 0.8.4` 
.

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 7bd8d8d9e7..d27c3b6c6c 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1079,6 +1079,16 @@ qemuValidateLifecycleAction(virDomainLifecycleAction 
onPoweroff,
  return -1;
  }

+/* The qemu driver doesn't yet implement any meaningful handling for
+ * VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE */
+if (onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
+onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
+onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("qemu driver doesn't support the 'preserve' action for 
'on_reboot'/'on_poweroff'/'on_crash'"));
+return -1;
+}
+
  return 0;
  }

Re: [libvirt PATCH 5/6] hostcpu: Implement virHostCPUGetSignature for s390

[Christian Borntraeger]

On 20.05.20 14:20, Boris Fiuczynski wrote:
> On 5/18/20 2:56 PM, Jiri Denemark wrote:
>> Signed-off-by: Jiri Denemark 
>> ---
>>   src/util/virhostcpu.c    | 16 +++-
>>   .../linux-s390x-with-frequency.signature |  1 +
>>   2 files changed, 16 insertions(+), 1 deletion(-)
>>   create mode 100644 
>> tests/virhostcpudata/linux-s390x-with-frequency.signature
>>
>> diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
>> index 0ce895cb39..0caf7959ef 100644
>> --- a/src/util/virhostcpu.c
>> +++ b/src/util/virhostcpu.c
>> @@ -1430,8 +1430,9 @@ virHostCPUReadSignature(virArch arch,
>>   g_autofree char *model = NULL;
>>   g_autofree char *stepping = NULL;
>>   g_autofree char *revision = NULL;
>> +    g_autofree char *proc = NULL;
>>   -    if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch))
>> +    if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch) && !ARCH_IS_S390(arch))
>>   return 0;
>>     while (fgets(line, lineLen, cpuinfo)) {
>> @@ -1479,6 +1480,19 @@ virHostCPUReadSignature(virArch arch,
>>   *signature = g_strdup_printf("%s, rev %s", name, revision);
>>   return 0;
>>   }
>> +    } else if (ARCH_IS_S390(arch)) {
>> +    if (STREQ(parts[0], "vendor_id")) {
>> +    if (!vendor)
>> +    vendor = g_steal_pointer([1]);
>> +    } else if (STREQ(parts[0], "processor 0")) {
>> +    if (!proc)
>> +    proc = g_steal_pointer([1]);
>> +    }
> 
> To catch scenarios of moving from LPAR to nested kvm or the other way around 
> facilties must be added here as well.
> 

Yes. This would also cover the case when a firmware update adds features.

Re: [PATCH] docs: Describe protected virtualization guest setup

[Christian Borntraeger]

On 29.04.20 16:09, Christian Borntraeger wrote:
> 
> 
> On 29.04.20 15:25, Daniel P. Berrangé wrote:
>> On Wed, Apr 29, 2020 at 10:19:20AM -0300, Daniel Henrique Barboza wrote:
>>>
>>>
>>> On 4/28/20 12:58 PM, Boris Fiuczynski wrote:
>>>> From: Viktor Mihajlovski 
>>>>
>>>
>>> [...]
>>>> +
>>>> +If the check fails despite the host system actually supporting
>>>> +protected virtualization guests, this can be caused by a stale
>>>> +libvirt capabilities cache. To recover, run the following
>>>> +commands
>>>> +
>>>> +::
>>>> +
>>>> +   $ systemctl stop libvirtd
>>>> +   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
>>>> +   $ systemctl start libvirtd
>>>> +
>>>> +
>>>
>>>
>>> Why isn't Libvirt re-fetching the capabilities after host changes that 
>>> affects
>>> KVM capabilities? I see that we're following up QEMU timestamps to detect
>>> if the binary changes, which is sensible, but what about /dev/kvm? Shouldn't
>>> we refresh domain capabilities every time following a host reboot?
>>
>> Caching of capabilities was done precisely  to avoid refreshing on every boot
>> because it resulted in slow startup for apps using libvirt after boot.
> 
> Caching beyond the life time of /dev/kvm seems broken from a kernel 
> perspective.
> It is totally valid to load a new version of the kvm module with new 
> capabilities.
> 
> I am curious, Why did it take so long? we should - on normal system - only 
> refresh _one_ binary as most binaries are just TCG.
> 
> As Boris said, we are going to provide yet another check (besides the nested
> thing.
> 
> But in general I think invalidating the cache for that one and only binary
> that provides KVM after a change of /dev/kvm seems like the proper solution.
> 



Looking into that, I found a handler for the "nested" module parameter. Now: We 
also
have a hpage parameter that decides if you can use large pages or not in the 
host.
Do we need to check that as well or is that something that libvirt does not 
care 
about?
This parameter will go away at a later point in time though as soon as we have 
added
the code to have hpage and nested at the same time. 

So given the shakiness of these things, the time stamp of /dev/kvm really seems 
to be the best way to go forward long term. Would be really interesting to 
understand
the conditions where you saw the long startup times due to rescanning.

Re: [PATCH] docs: Describe protected virtualization guest setup

[Christian Borntraeger]

On 29.04.20 15:25, Daniel P. Berrangé wrote:
> On Wed, Apr 29, 2020 at 10:19:20AM -0300, Daniel Henrique Barboza wrote:
>>
>>
>> On 4/28/20 12:58 PM, Boris Fiuczynski wrote:
>>> From: Viktor Mihajlovski 
>>>
>>
>> [...]
>>> +
>>> +If the check fails despite the host system actually supporting
>>> +protected virtualization guests, this can be caused by a stale
>>> +libvirt capabilities cache. To recover, run the following
>>> +commands
>>> +
>>> +::
>>> +
>>> +   $ systemctl stop libvirtd
>>> +   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
>>> +   $ systemctl start libvirtd
>>> +
>>> +
>>
>>
>> Why isn't Libvirt re-fetching the capabilities after host changes that 
>> affects
>> KVM capabilities? I see that we're following up QEMU timestamps to detect
>> if the binary changes, which is sensible, but what about /dev/kvm? Shouldn't
>> we refresh domain capabilities every time following a host reboot?
> 
> Caching of capabilities was done precisely  to avoid refreshing on every boot
> because it resulted in slow startup for apps using libvirt after boot.

Caching beyond the life time of /dev/kvm seems broken from a kernel perspective.
It is totally valid to load a new version of the kvm module with new 
capabilities.

I am curious, Why did it take so long? we should - on normal system - only 
refresh _one_ binary as most binaries are just TCG.

As Boris said, we are going to provide yet another check (besides the nested
thing.

But in general I think invalidating the cache for that one and only binary
that provides KVM after a change of /dev/kvm seems like the proper solution.


> 
> We look for specific features that change as a way to indicate a refresh
> is needed.  If there's a need to delete the capabilities manually that
> indicates we're missing some feature when deciding whether the cache is
> stale.

Re: [PATCH] qemu: capabilities: update qemu-4.2 capabilities for s390x

[Christian Borntraeger]

On 25.03.20 08:06, Bjoern Walk wrote:
> Update s390x capabilities for QEMU 4.2 with the actual GA version for
> QEMU and on the latest z15 machine.

Maybe add:

As these files were generated on a z15 and the previous data was taken on a z13
this now indicates, z15 (gen15a) AND z14 to be available.

to the patch description?

The cpu features changes look sane to me.

Acked-by: Christian Borntraeger 

 


> This picks up the new blockdev capability, so we need to refresh a bunch
> of test cases as well.
> 
> Reviewed-by: Boris Fiuczynski 
> Signed-off-by: Bjoern Walk 
> ---
> I have stripped the replies file to save some bandwidth on the mailing
> list. Full patch can be found here:
> 
> https://gitlab.com/bwalk/libvirt/-/commit/5fc05853f2cc6d191a07f897cdd4e12fa9881562
> 
>  tests/domaincapsdata/qemu_4.2.0.s390x.xml |   47 +-
>  .../caps_4.2.0.s390x.replies  | 3230 +
>  .../qemucapabilitiesdata/caps_4.2.0.s390x.xml |  216 +-
>  ...default-video-type-s390x.s390x-latest.args |8 +-
>  .../fs9p-ccw.s390x-latest.args|7 +-
>  ...othreads-virtio-scsi-ccw.s390x-latest.args |   14 +-
>  ...t-cpu-kvm-ccw-virtio-4.2.s390x-latest.args |8 +-
>  .../s390x-ccw-graphics.s390x-latest.args  |8 +-
>  .../s390x-ccw-headless.s390x-latest.args  |8 +-
>  .../vhost-vsock-ccw-auto.s390x-latest.args|7 +-
>  .../vhost-vsock-ccw.s390x-latest.args |7 +-
>  11 files changed, 1871 insertions(+), 1689 deletions(-)
> 
> diff --git a/tests/domaincapsdata/qemu_4.2.0.s390x.xml 
> b/tests/domaincapsdata/qemu_4.2.0.s390x.xml
> index fbb3905f..6b87e450 100644
> --- a/tests/domaincapsdata/qemu_4.2.0.s390x.xml
> +++ b/tests/domaincapsdata/qemu_4.2.0.s390x.xml
> @@ -27,9 +27,17 @@
>
>  
>  
> -  z13.2-base
> +  gen15a-base
>
> +  
> +  
>
> +  
> +  
> +  
> +  
> +  
> +  
>
>
>
> @@ -38,15 +46,26 @@
>
>
>
> +  
>
> +  
>
>
> +  
>
> +  
> +  
>
> +  
> +  
> +  
> +  
>
> +  
>
>
>
> +  
>
>
>
> @@ -58,8 +77,8 @@
>z890.2-base
>z9EC.2
>z13.2
> -  z990.5-base
>z9BC-base
> +  z990.5-base
>z890.2
>z890
>z9BC
> @@ -69,9 +88,9 @@
>z990.3
>z13s-base
>z9EC
> -  gen15a
> -  z14ZR1-base
> -  z14.2-base
> +  gen15a
> +  z14ZR1-base
> +  z14.2-base
>z900.3-base
>z13.2-base
>z196.2-base
> @@ -86,18 +105,18 @@
>z10EC.2
>z10EC-base
>z900.3
> -  z14ZR1
> +  z14ZR1
>z10BC
>z10BC.2-base
> -  z9BC.2
>z990.2
> +  z9BC.2
>z990
> -  z14
> -  gen15b-base
> +  z14
> +  gen15b-base
>z990.4
>max
>z10EC.2-base
> -  gen15a-base
> +  gen15a-base
>z800
>zEC12.2
>z10EC
> @@ -111,12 +130,12 @@
>z196-base
>z9EC.2-base
>z196.2
> -  z14.2
> +  z14.2
>z990-base
>z900.2
>z890-base
>z10EC.3
> -  z14-base
> +  z14-base
>z990.4-base
>z10EC.3-base
>z10BC-base
> @@ -126,7 +145,7 @@
>zBC12
>z890.3-base
>z990.5
> -  gen15b
> +  gen15b
>qemu
>  
>
> @@ -198,7 +217,7 @@
>  
>  
>  
> -
> +
>  
>  
>
> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.replies 
> b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.replies
> index be709b3c..82b60bf0 100644
> --- a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.replies
> +++ b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.replies
> @@ -17,11 +17,11 @@
> [...]
> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml 
> b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
> index 37776e1b..e46259e6 100644
> --- a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
> +++ b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
> @@ -8,12 +8,14 @@
>
>
>
> +  
>
>
>
>
>
>
> +  
>
>
>
> @@ -49,6 +51,7 @@
>
>
>
> +  
>
>
>
> @@ -115,6 +118,7 @@
>
>
>
> +  
>
>
>

Re: [libvirt] [PATCH 0/4] qemu: Use host-model CPU on s390 by default

[Christian Borntraeger]

On 15.11.19 15:47, Jiri Denemark wrote:
> On Fri, Nov 15, 2019 at 15:12:18 +0100, Boris Fiuczynski wrote:
>> Just a heads up.
>> After installing libvirt rpms of this branch all my existing kvm s390 
>> domains ended up with
>>
>>
>>  qemu
>>
>>
>> Newly defined domains without specified cpu do so as well.
> 
> Unless the domains are all TCG, it seems your QEMU is too old. You need
> a fairly recent one which contains commit v4.1.0-1683-gde60a92ea7
> (s390x/kvm: Set default cpu model for all machine classes)
> 
> I the domains all use KVM and you have new enough QEMU, there might be a
> bug somewhere. Which should not happen :=)

So shouldnt libvirt fence this rework (add default model) to qemu 4.2 and newer?


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] s390: change default cpu model to host-model?

[Christian Borntraeger]

On 08.11.19 14:10, Daniel P. Berrangé wrote:
> On Fri, Nov 08, 2019 at 01:56:47PM +0100, Christian Borntraeger wrote:
>>
>>
>> On 08.11.19 12:52, Daniel P. Berrangé wrote:
>>> On Fri, Nov 08, 2019 at 12:49:23PM +0100, Christian Borntraeger wrote:
>>>>
>>>>
>>>> On 08.11.19 12:43, Daniel P. Berrangé wrote:
>>>>> On Mon, Nov 04, 2019 at 11:49:01AM +0100, David Hildenbrand wrote:
>>>>>> On 02.11.19 11:32, Daniel P. Berrangé wrote:
>>>>>>> On Fri, Nov 01, 2019 at 06:43:16PM +0100, Christian Borntraeger wrote:
>>>>>>>> On the KVM forum I have discussed the default cpu model mode on s390.
>>>>>>>> Right now if the xml does not specify anything, libvirt defaults to
>>>>>>>> not specifying anything on the qemu command line (no -cpu statement)
>>>>>>>> which is the equivalent of -cpu host for s390 which is equivalent to
>>>>>>>> host-passthrough. While this enables all features it does not provide
>>>>>>>> any migration safety by default.
>>>>>>>>
>>>>>>>> So in fact we are kind of "broken" right now when it comes to safery.
>>>>>>>>
>>>>>>>> So we discussed that it would make sense that an empty xml should 
>>>>>>>> actually
>>>>>>>> be defaulted to host-model, which results in - as of today - the same 
>>>>>>>> guest
>>>>>>>> features but in a migration safe way.
>>>>>>>>
>>>>>>>> There is another change planned right now to actually make the cpu 
>>>>>>>> model
>>>>>>>> present in an xml if none was specified. So we could actually do this 
>>>>>>>> change
>>>>>>>> before, together  or after te other. Jiri and I think it probably 
>>>>>>>> makes most
>>>>>>>> sense to have both changes at the same time (in terms of libvirt 
>>>>>>>> version).
>>>>>>>>
>>>>>>>> Does anyone see an issue with changing the default model mode to 
>>>>>>>> "host-model"
>>>>>>>> if the xml does not specify anything else?
>>>>>>>
>>>>>>> Changing from "host-passthrough" to "host-model" is not a huge 
>>>>>>> difference,
>>>>>>> but it is none the less a guest ABI change. "host-passthrough" doesn't
>>>>>>> provide migration safety in the face of differing hardware, it should 
>>>>>>> still
>>>>>>> be valid for people with homogeneous hardware. So changing the model 
>>>>>>> will
>>>>>>> potentially break some existing usage.
>>>>>>
>>>>>> I guess on s390x this is not the case ("-cpu host", no "-cpu", and 
>>>>>> passing
>>>>>> the expanded "host" model will result in the same guest ABI, in contrast 
>>>>>> to
>>>>>> x86 AFAIK). There is this special case, though, where we have old QEMUs
>>>>>> without CPU model support. Not sure how to deal with that, then.
>>>>>
>>>>> I'm still not sure I understand the s390 CPU ABI rules.
>>>>>
>>>>> Current libvirt, no , and thus no -cpu.
>>>>>
>>>>> IIUC this is functionally identical to using "-cpu host" and/or
>>>>> 
>>>>>
>>>>> If you are using "-cpu host" /  can you
>>>>> live migrate to another host with identical physical CPUs + firmware ?
>>>>>
>>>>>
>>>>> Assuming this is possible, then, can you live migrate a QEMU guest
>>>>> booted with , to a QEMU guest booted
>>>>> with   ?
>>>>
>>>> Not sure I understand your question. With "can", do  you mean "the guest
>>>> has the same guest visible CPU features and types"?
>>>
>>> Yes, I mean the migration should succeed from QEMU's POV and additionally
>>> the guest OS should not see any change in CPU ABI exposed from the host.
>>
>> The guest ABI is the same and migration also seems to work. 
>> I think your concern boils down to the case that source and target
>> have a different libvirt version (but qemu and kernel and firmware
>> and hardware are identical). So for that case this change would break
>> things if host-model and host-passthrough are not identical.
>> So as of today we have no concern. 
>>
>> Now: Would it be a concern if a future QEMU decides to change that
>> equivalence somehow?
> 
> If they're the same guest ABI, then what's the benefit in changing the
> default to "host-model" instead of just continuing with "host-passthrough".
> It seems like we're fine to just carry on with "host-passthrough" as
> the default and insulate ourselves from any future risk of change.

The benefit is that that todays default is not migration safe and users will
find that out by random guest crashes if any of the parameters (CPU, kernel,
qemu, firmware) is different. So really, todays default is just completely
broken on s390 and thats why I want to change it.

host-model instead is expanded by libvirt and the migration will be rejected
if the target is incompatible (qemu will reject to run).


migh

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] s390: change default cpu model to host-model?

[Christian Borntraeger]

On 08.11.19 12:52, Daniel P. Berrangé wrote:
> On Fri, Nov 08, 2019 at 12:49:23PM +0100, Christian Borntraeger wrote:
>>
>>
>> On 08.11.19 12:43, Daniel P. Berrangé wrote:
>>> On Mon, Nov 04, 2019 at 11:49:01AM +0100, David Hildenbrand wrote:
>>>> On 02.11.19 11:32, Daniel P. Berrangé wrote:
>>>>> On Fri, Nov 01, 2019 at 06:43:16PM +0100, Christian Borntraeger wrote:
>>>>>> On the KVM forum I have discussed the default cpu model mode on s390.
>>>>>> Right now if the xml does not specify anything, libvirt defaults to
>>>>>> not specifying anything on the qemu command line (no -cpu statement)
>>>>>> which is the equivalent of -cpu host for s390 which is equivalent to
>>>>>> host-passthrough. While this enables all features it does not provide
>>>>>> any migration safety by default.
>>>>>>
>>>>>> So in fact we are kind of "broken" right now when it comes to safery.
>>>>>>
>>>>>> So we discussed that it would make sense that an empty xml should 
>>>>>> actually
>>>>>> be defaulted to host-model, which results in - as of today - the same 
>>>>>> guest
>>>>>> features but in a migration safe way.
>>>>>>
>>>>>> There is another change planned right now to actually make the cpu model
>>>>>> present in an xml if none was specified. So we could actually do this 
>>>>>> change
>>>>>> before, together  or after te other. Jiri and I think it probably makes 
>>>>>> most
>>>>>> sense to have both changes at the same time (in terms of libvirt 
>>>>>> version).
>>>>>>
>>>>>> Does anyone see an issue with changing the default model mode to 
>>>>>> "host-model"
>>>>>> if the xml does not specify anything else?
>>>>>
>>>>> Changing from "host-passthrough" to "host-model" is not a huge difference,
>>>>> but it is none the less a guest ABI change. "host-passthrough" doesn't
>>>>> provide migration safety in the face of differing hardware, it should 
>>>>> still
>>>>> be valid for people with homogeneous hardware. So changing the model will
>>>>> potentially break some existing usage.
>>>>
>>>> I guess on s390x this is not the case ("-cpu host", no "-cpu", and passing
>>>> the expanded "host" model will result in the same guest ABI, in contrast to
>>>> x86 AFAIK). There is this special case, though, where we have old QEMUs
>>>> without CPU model support. Not sure how to deal with that, then.
>>>
>>> I'm still not sure I understand the s390 CPU ABI rules.
>>>
>>> Current libvirt, no , and thus no -cpu.
>>>
>>> IIUC this is functionally identical to using "-cpu host" and/or
>>> 
>>>
>>> If you are using "-cpu host" /  can you
>>> live migrate to another host with identical physical CPUs + firmware ?
>>>
>>>
>>> Assuming this is possible, then, can you live migrate a QEMU guest
>>> booted with , to a QEMU guest booted
>>> with   ?
>>
>> Not sure I understand your question. With "can", do  you mean "the guest
>> has the same guest visible CPU features and types"?
> 
> Yes, I mean the migration should succeed from QEMU's POV and additionally
> the guest OS should not see any change in CPU ABI exposed from the host.

The guest ABI is the same and migration also seems to work. 
I think your concern boils down to the case that source and target
have a different libvirt version (but qemu and kernel and firmware
and hardware are identical). So for that case this change would break
things if host-model and host-passthrough are not identical. 

So as of today we have no concern. 

Now: Would it be a concern if a future QEMU decides to change that
equivalence somehow?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] s390: change default cpu model to host-model?

[Christian Borntraeger]

On 08.11.19 12:43, Daniel P. Berrangé wrote:
> On Mon, Nov 04, 2019 at 11:49:01AM +0100, David Hildenbrand wrote:
>> On 02.11.19 11:32, Daniel P. Berrangé wrote:
>>> On Fri, Nov 01, 2019 at 06:43:16PM +0100, Christian Borntraeger wrote:
>>>> On the KVM forum I have discussed the default cpu model mode on s390.
>>>> Right now if the xml does not specify anything, libvirt defaults to
>>>> not specifying anything on the qemu command line (no -cpu statement)
>>>> which is the equivalent of -cpu host for s390 which is equivalent to
>>>> host-passthrough. While this enables all features it does not provide
>>>> any migration safety by default.
>>>>
>>>> So in fact we are kind of "broken" right now when it comes to safery.
>>>>
>>>> So we discussed that it would make sense that an empty xml should actually
>>>> be defaulted to host-model, which results in - as of today - the same guest
>>>> features but in a migration safe way.
>>>>
>>>> There is another change planned right now to actually make the cpu model
>>>> present in an xml if none was specified. So we could actually do this 
>>>> change
>>>> before, together  or after te other. Jiri and I think it probably makes 
>>>> most
>>>> sense to have both changes at the same time (in terms of libvirt version).
>>>>
>>>> Does anyone see an issue with changing the default model mode to 
>>>> "host-model"
>>>> if the xml does not specify anything else?
>>>
>>> Changing from "host-passthrough" to "host-model" is not a huge difference,
>>> but it is none the less a guest ABI change. "host-passthrough" doesn't
>>> provide migration safety in the face of differing hardware, it should still
>>> be valid for people with homogeneous hardware. So changing the model will
>>> potentially break some existing usage.
>>
>> I guess on s390x this is not the case ("-cpu host", no "-cpu", and passing
>> the expanded "host" model will result in the same guest ABI, in contrast to
>> x86 AFAIK). There is this special case, though, where we have old QEMUs
>> without CPU model support. Not sure how to deal with that, then.
> 
> I'm still not sure I understand the s390 CPU ABI rules.
> 
> Current libvirt, no , and thus no -cpu.
> 
> IIUC this is functionally identical to using "-cpu host" and/or
> 
> 
> If you are using "-cpu host" /  can you
> live migrate to another host with identical physical CPUs + firmware ?
> 
> 
> Assuming this is possible, then, can you live migrate a QEMU guest
> booted with , to a QEMU guest booted
> with   ?

Not sure I understand your question. With "can", do  you mean "the guest
has the same guest visible CPU features and types"?  

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 00/52] qemu: Store default CPU in domain XML

[Christian Borntraeger]

As discussed, does it make sense to add the default change to host-model for 
s390 
in this series or should that be a separate patch?


On 05.11.19 14:26, Jiri Denemark wrote:
> When starting a domain without a CPU model specified in the domain XML,
> QEMU will choose a default one. Which is fine unless the domain gets
> migrated to another host because libvirt doesn't perform any CPU ABI
> checks and the virtual CPU provided by QEMU on the destination host can
> differ from the one on the source host.
> 
> With QEMU 4.2.0 we can probe for the default CPU model used by QEMU for
> a particular machine type and store it in the domain XML. This way the
> chosen CPU model is more visible to users and libvirt will make sure
> the guest will see the exact same CPU after migration.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1598151
> https://bugzilla.redhat.com/show_bug.cgi?id=1598162
> 
> Version 2:
> - more tests
> - TCG-only support for non x86_64 architectures
> 
> Version 3:
> - as a result of talking with QEMU developers dealing with s390 and
>   ppc64 I have to enhance the series so that libvirt is able to fetch
>   different default CPU models on TCG vs. KVM
> 
> ---
> 
> Some patches were too large so I decided to shorten them before sending
> to the list. You can check the full version of this series with
> 
> git fetch https://gitlab.com/jirkade/libvirt cpu-default-type
> 
> Jiri Denemark (52):
>   tests: Add capabilities for QEMU 4.2.0 on s390x
>   tests: Update 4.2.0 capabilities data on ppc64
>   conf: Use VIR_AUTO* in virDomainCapsCPUModelsAdd
>   conf: Drop nameLen parameter from virDomainCapsCPUModelsAdd
>   qemu: Copy CPU models in virQEMUCapsGetCPUDefinitions
>   qemu: Filter models in virQEMUCapsGetCPUDefinitions
>   qemu: Use virQEMUCapsGetCPUDefinitions more
>   qemu: Use g_autoptr in qemuMonitorJSONGetCPUDefinitions
>   qemu: Change return type of virQEMUCapsFetchCPUDefinitions
>   qemu: Introduce qemuMonitorCPUDefs struct
>   qemu: Flatten qemuMonitorCPUDefs.cpus
>   qemu: Add qemuMonitorCPUDefsCopy
>   qemu: Use g_autofree in virQEMUCapsLoadCPUModels
>   qemu: Use virDomainCapsCPUUsable in qemuMonitorCPUDefInfo
>   qemu: Introduce virQEMUCapsCPUDefsToModels
>   qemu: Rename virQEMUCaps{Get,Fetch}CPUDefinitions
>   qemu: Split virQEMUCapsFetchCPUModels
>   qemu: Switch qemuCaps to use qemuMonitorCPUDefs
>   conf: Drop unused virDomainCapsCPUModelsFilter
>   conf: Drop virDomainCapsCPUModelsAddSteal
>   qemu: Store typename from query-cpu-definitions in qemuCaps
>   qemu: Drop unused virQEMUCapsGetDefaultMachine
>   qemu: Add virQEMUCaps{Load,Format}Accel
>   qemu: Introduce virQEMUCapsAccel structure
>   qemu: Introduce virQEMUCapsAccelCopy
>   qemu: Introduce virQEMUCapsAccelClear
>   qemu: Introduce and use virQEMUCapsGetAccel
>   qemu: Drop virQEMUCapsGetHostCPUData
>   qemu: Refactor virQEMUCapsLoadAccel
>   qemu: Refactor virQEMUCapsFormatAccel
>   qemu: Introduce virQEMUCapsProbeCPUDefinitionsTest
>   qemu: Refactor probing of accelerator dependent data
>   qemu: Make virQEMUCapsGetMachineTypesCaps static
>   qemu: Make virQEMUCapsIsMachineSupported static
>   qemu: Refactor virQEMUCapsLoadCache a bit
>   qemu: Refactor virQEMUCapsFormatCache a bit
>   qemu: Pass virDomainVirtType to APIs dealing with machine types
>   qemu: Move machine type data in capabilities cache
>   qemu: Use typedef for virQEMUCapsMachineType
>   qemu: Introduce virQEMUCapsCopyMachineTypes
>   qemu: Make probed machine types depend on accelerator
>   qemu: Probe machine types for both KVM and TCG
>   qemu: Probe for default CPU types
>   qemu: Introduce virQEMUCapsGetMachineDefaultCPU
>   qemu: Use g_autoptr in qemuDomainDefPostParse
>   conf: Define g_autoptr cleanup function for virCPUDef
>   qemuxml2argvtest: Update host arch for DO_TEST*ARCH* tests
>   qemuxml2*test: Add test cases for default CPU models on aarch64
>   qemuxml2*test: Add test cases for default CPU models on ppc64
>   qemuxml2*test: Add test cases for default CPU models on s390x
>   qemuxml2*test: Add test cases for default CPU models on x86_64
>   qemu: Store default CPU in domain XML
> 
>  src/conf/cpu_conf.h   | 1 +
>  src/conf/domain_capabilities.c|86 +-
>  src/conf/domain_capabilities.h|10 +-
>  src/libvirt_private.syms  | 2 -
>  src/qemu/qemu_capabilities.c  |  1064 +-
>  src/qemu/qemu_capabilities.h  |29 +-
>  src/qemu/qemu_capspriv.h  | 5 +-
>  src/qemu/qemu_domain.c|97 +-
>  src/qemu/qemu_driver.c| 4 +-
>  src/qemu/qemu_monitor.c   |61 +-
>  src/qemu/qemu_monitor.h   |19 +-
>  src/qemu/qemu_monitor_json.c  |82 +-
>  src/qemu/qemu_monitor_json.h  | 2 +-
>  src/qemu/qemu_process.c   |24 +-
>  

Re: [libvirt] s390: change default cpu model to host-model?

[Christian Borntraeger]

On 04.11.19 11:49, David Hildenbrand wrote:
> On 02.11.19 11:32, Daniel P. Berrangé wrote:
>> On Fri, Nov 01, 2019 at 06:43:16PM +0100, Christian Borntraeger wrote:
>>> On the KVM forum I have discussed the default cpu model mode on s390.
>>> Right now if the xml does not specify anything, libvirt defaults to
>>> not specifying anything on the qemu command line (no -cpu statement)
>>> which is the equivalent of -cpu host for s390 which is equivalent to
>>> host-passthrough. While this enables all features it does not provide
>>> any migration safety by default.
>>>
>>> So in fact we are kind of "broken" right now when it comes to safery.
>>>
>>> So we discussed that it would make sense that an empty xml should actually
>>> be defaulted to host-model, which results in - as of today - the same guest
>>> features but in a migration safe way.
>>>
>>> There is another change planned right now to actually make the cpu model
>>> present in an xml if none was specified. So we could actually do this change
>>> before, together  or after te other. Jiri and I think it probably makes most
>>> sense to have both changes at the same time (in terms of libvirt version).
>>>
>>> Does anyone see an issue with changing the default model mode to 
>>> "host-model"
>>> if the xml does not specify anything else?
>>
>> Changing from "host-passthrough" to "host-model" is not a huge difference,
>> but it is none the less a guest ABI change. "host-passthrough" doesn't
>> provide migration safety in the face of differing hardware, it should still
>> be valid for people with homogeneous hardware. So changing the model will
>> potentially break some existing usage.
> 
> I guess on s390x this is not the case ("-cpu host", no "-cpu", and passing 
> the expanded "host" model will result in the same guest ABI, in contrast to 
> x86 AFAIK). 
Yes.


There is this special case, though, where we have old QEMUs without CPU model 
support. Not sure how to deal with that, then.

Those QEMUs will not provide the same guest APIs anyway so there is no chance 
to 
build something with libvirt that is equivalent. 

In fact, I think changing to cpu-model is actually a kind of bugfix for s390
given that the only user visible change would be to have now migration safety
while right now there is none.
> 
>>
>> As the top priority we should definitely make sure that the guest XML
>> gets updated to list "host-passthrough" when no CPU is specified, so
>> that it reflects the current reality. If libvirt gets this information
> 
> +1 to that

Yes, that is what Jiri said. We should do this chance together (at least
within the same version) as his change to provide an expanded default 
cpu model if the initial XML does not have one. 

For s390 this would then be cpu-model. 

> 
>> from QEMU, then at some point down the line we can potentially change
>> the default by tieing a new default to a versioned machine type. We
>> have to wait a little while for old libvirt's to become irrelevant
>> wrt new QEMU though.   This is the same issue with changing the default
>> CPU on x86 which, though that's possibly harder on x86 as the scope of
>> any change is more significant.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] s390: change default cpu model to host-model?

[Christian Borntraeger]

On the KVM forum I have discussed the default cpu model mode on s390.
Right now if the xml does not specify anything, libvirt defaults to 
not specifying anything on the qemu command line (no -cpu statement)
which is the equivalent of -cpu host for s390 which is equivalent to
host-passthrough. While this enables all features it does not provide
any migration safety by default.

So in fact we are kind of "broken" right now when it comes to safery.

So we discussed that it would make sense that an empty xml should actually
be defaulted to host-model, which results in - as of today - the same guest
features but in a migration safe way.

There is another change planned right now to actually make the cpu model
present in an xml if none was specified. So we could actually do this change
before, together  or after te other. Jiri and I think it probably makes most
sense to have both changes at the same time (in terms of libvirt version).

Does anyone see an issue with changing the default model mode to "host-model"
if the xml does not specify anything else?

Christian

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] libvirt? qemu change that mmaps ELF files breaks libvirt svirt handling for os.kernel

[Christian Borntraeger]

On 04.10.19 14:36, Daniel P. Berrangé wrote:
> On Fri, Oct 04, 2019 at 02:18:49PM +0200, Christian Borntraeger wrote:
>>
>>
>> On 04.10.19 14:13, Paolo Bonzini wrote:
>>> On 04/10/19 14:03, Christian Borntraeger wrote:
>>>> Stefano, Paolo,
>>>>
>>>> I have an interesting fail in QEMU 
>>>>
>>>> 2019-10-04T12:00:32.675188Z qemu-system-s390x: GLib: g_mapped_file_unref: 
>>>> assertion 'file != NULL' failed
>>>> that bisected to 
>>>> commit 816b9fe450220e19acb91a0ce4a8ade7000648d1 (refs/bisect/bad)
>>>> elf-ops.h: Map into memory the ELF to load
>>>>
>>>> strace tells that I can read the ELF file, but not mmap
>>>> strace:
>>>> 214365 openat(AT_FDCWD, "/var/lib/libvirt/images/test_cpu_timer.elf", 
>>>> O_RDONLY) = 36
>>>> 214365 read(46, "\177ELF\2\2\1\0\0\0\0\0\0\0\0\0", 16) = 16
>>>> 214365 lseek(46, 0, SEEK_SET)   = 0
>>>> [...]
>>>> 214365 fstat(46, {st_mode=S_IFREG|0755, st_size=168176, ...}) = 0
>>>> 214365 mmap(NULL, 168176, PROT_READ|PROT_WRITE, MAP_PRIVATE, 46, 0) = -1 
>>>> EACCES (Permission denied)
>>>>
>>>> So reading from /var/lib/libvirt/images/test_cpu_timer.elf does work, 
>>>> mmaping does not.
>>>> setenforce 0 makes the problem go away. 
>>>>
>>>> This might be more of an issue in libvirt, setting the svirt context too
>>>> restrictive, but I am not too deep into the svirt part of libvirt.
>>>> Reverting the qemu commit makes the problem go away.
>>>
>>> Yes, the policy is too restrictive in my opinion.
>>>
>>> Can you include the output of "audit2allow" and/or "audit2allow -R"?
>>>
>>> Thanks,
>>>
>>> Paolo
>>>
>>
>> require {
>>  type unconfined_t;
>>  type virt_content_t;
>>  type svirt_t;
>>  type systemd_tmpfiles_t;
>>  type user_home_t;
>>  type NetworkManager_t;
>>  class file { entrypoint execute ioctl lock map open read write };
>>  class bpf prog_run;
>> }
>>
>> #= svirt_t ==
>> allow svirt_t user_home_t:file { entrypoint execute ioctl lock open read 
>> write };
>>
>> # This avc can be allowed using the boolean 'domain_can_mmap_files'
> 
> This is an unrelated boolean and we don't want to use that so ignore
> this suggestion !
> 
>> allow svirt_t virt_content_t:file map;
> 
> This matches your strace.  virt_content_t is the label we use on
> files that are exposed to QEMU read-only.
> 
> The svirt policy only allows mmap on files that re exposed read-write
> currently.
> 
> I believe we can safely allow this mmap on read-only files too
> because  the read-only restriction is enforced at time of open,
> and any writes to the mapped file  will result in a private
> copy on write.
> 
> Please file a bz entry against the selinux-policy component for
> whatever distro you're using, and/or Fedora rawhide, and CC me
> on it too.

Done. This was on Fedora 30.
https://bugzilla.redhat.com/show_bug.cgi?id=1758525

 Now sure about others like RHEL. RHV.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] libvirt? qemu change that mmaps ELF files breaks libvirt svirt handling for os.kernel

[Christian Borntraeger]

On 04.10.19 14:13, Paolo Bonzini wrote:
> On 04/10/19 14:03, Christian Borntraeger wrote:
>> Stefano, Paolo,
>>
>> I have an interesting fail in QEMU 
>>
>> 2019-10-04T12:00:32.675188Z qemu-system-s390x: GLib: g_mapped_file_unref: 
>> assertion 'file != NULL' failed
>> that bisected to 
>> commit 816b9fe450220e19acb91a0ce4a8ade7000648d1 (refs/bisect/bad)
>> elf-ops.h: Map into memory the ELF to load
>>
>> strace tells that I can read the ELF file, but not mmap
>> strace:
>> 214365 openat(AT_FDCWD, "/var/lib/libvirt/images/test_cpu_timer.elf", 
>> O_RDONLY) = 36
>> 214365 read(46, "\177ELF\2\2\1\0\0\0\0\0\0\0\0\0", 16) = 16
>> 214365 lseek(46, 0, SEEK_SET)   = 0
>> [...]
>> 214365 fstat(46, {st_mode=S_IFREG|0755, st_size=168176, ...}) = 0
>> 214365 mmap(NULL, 168176, PROT_READ|PROT_WRITE, MAP_PRIVATE, 46, 0) = -1 
>> EACCES (Permission denied)
>>
>> So reading from /var/lib/libvirt/images/test_cpu_timer.elf does work, 
>> mmaping does not.
>> setenforce 0 makes the problem go away. 
>>
>> This might be more of an issue in libvirt, setting the svirt context too
>> restrictive, but I am not too deep into the svirt part of libvirt.
>> Reverting the qemu commit makes the problem go away.
> 
> Yes, the policy is too restrictive in my opinion.
> 
> Can you include the output of "audit2allow" and/or "audit2allow -R"?
> 
> Thanks,
> 
> Paolo
> 

require {
type unconfined_t;
type virt_content_t;
type svirt_t;
type systemd_tmpfiles_t;
type user_home_t;
type NetworkManager_t;
class file { entrypoint execute ioctl lock map open read write };
class bpf prog_run;
}

#= svirt_t ==
allow svirt_t user_home_t:file { entrypoint execute ioctl lock open read write 
};

# This avc can be allowed using the boolean 'domain_can_mmap_files'
allow svirt_t virt_content_t:file map;
corecmd_bin_entry_type(svirt_t)
userdom_manage_user_home_content_dirs(svirt_t)
userdom_map_user_home_files(svirt_t)
virt_rw_svirt_image(svirt_t)

#= systemd_tmpfiles_t ==
kernel_read_usermodehelper_state(systemd_tmpfiles_t)

#= unconfined_t ==
allow unconfined_t NetworkManager_t:bpf prog_run;

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] libvirt? qemu change that mmaps ELF files breaks libvirt svirt handling for os.kernel

[Christian Borntraeger]

Stefano, Paolo,

I have an interesting fail in QEMU 

2019-10-04T12:00:32.675188Z qemu-system-s390x: GLib: g_mapped_file_unref: 
assertion 'file != NULL' failed
that bisected to 
commit 816b9fe450220e19acb91a0ce4a8ade7000648d1 (refs/bisect/bad)
elf-ops.h: Map into memory the ELF to load

strace tells that I can read the ELF file, but not mmap
strace:
214365 openat(AT_FDCWD, "/var/lib/libvirt/images/test_cpu_timer.elf", O_RDONLY) 
= 36
214365 read(46, "\177ELF\2\2\1\0\0\0\0\0\0\0\0\0", 16) = 16
214365 lseek(46, 0, SEEK_SET)   = 0
[...]
214365 fstat(46, {st_mode=S_IFREG|0755, st_size=168176, ...}) = 0
214365 mmap(NULL, 168176, PROT_READ|PROT_WRITE, MAP_PRIVATE, 46, 0) = -1 EACCES 
(Permission denied)

So reading from /var/lib/libvirt/images/test_cpu_timer.elf does work, mmaping 
does not.
setenforce 0 makes the problem go away. 

This might be more of an issue in libvirt, setting the svirt context too
restrictive, but I am not too deep into the svirt part of libvirt.
Reverting the qemu commit makes the problem go away.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [REPOST PATCH v2 00/12] Allow modification of IOThread polling values (redux)

[Christian Borntraeger]

On 11/05/2018 01:58 PM, John Ferlan wrote:
> v2: https://www.redhat.com/archives/libvir-list/2018-October/msg01065.html
> 
> NB: Minor mods for this are change using 4.10.0 instead of 4.9.0, merge of
> qemu_capabilities conflict, and updated news.xml
> 
> .. v2 Cover Letter:
> 
> v1: https://www.redhat.com/archives/libvir-list/2018-October/msg00456.html
> 
> Changes since v1 (from code review)
> 
> Patch 2: Move the job back into qemuDomainGetIOThreadsLive
> 
> Patch 3: Add check for ActiveJob before allowing, use true for
>  *StatsWorker, and print 'iothread' in output not 'block'
> 
> Patch 5: Use virCheckPositiveArgGoto(nparams, error) instead of using
>  virCheckNonNegativeArgGoto(nparams, error).  And then remove
>  the if (nparams) before the virCheckNonNullArgGoto(params, error);
> 
> Patch 6: Add ability to determine which parameter was set via bool
>  set_poll_{max_ns|grow|shrink} values.  Then use those in
>  the macro that sets the value to determine whether or not
>  the value will be set based on whether it was changed.
> 
> Patch 10: Use bool's to set_ when the value is found in the incoming
>   params list.  Remove the check that says poll_max_ns needs
>   to be set. Testing proves that if it's set to 0, then the
>   grow and shrink values can be changed (although they do
>   nothing)
> 
> Patch 12: (NEW) - News article
> 
> 
> .. v1 Cover Letter:
> 
> This series attempts to resurrect the concept of being able to modify
> the IOThread polling parameters; however, in a slightly different
> manner than the previous series posted by posted by Pavel Hrdina
> :
> 
>   https://www.redhat.com/archives/libvir-list/2017-February/msg01047.html
> 
> The work is prompted by continued pleas found in the bz:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1545732
> 
> to provide some way to modify the paremters without needing to supply
> QEMU command line pass through values.
> 
> It's accepted that the values being changed are fairly or extremely
> low level type knobs; however, it's also shown that by being able to
> turn the knob it is possible for certain, specific appliances to be
> able to gain a performance benefit for the thread at the expense of
> other competing threads.
> 
> Unlike the previous series, this series does not attempt to save the
> polling values in the guest XML. Rather, it only modifies the live
> guest's IOThread with new values. It also doesn't provide the polling
> values in a virsh iothread* command, rather it uses the domstats
> in order to fetch and display the values. The theory being this
> leaves the onus on the higher level appliance/application to provide
> the "proper guidance" and "concerns" related to changing the values
> to the consumer. Not saving the values means whatever values that
> are chosen do not "live" in perpetuity. Once the guest is shut down
> or the IOThread removed from guest, the hypervisor default values
> take over again. Perhaps not a perfect situation in terms of what
> the bz requests; however, storage of default values that could
> cause performance issues is not an optimal situation. So this I
> figured is a "comprimise" of sorts.
> 
> If it's still felt that no we don't want to do this, then fine,
> but please in doing so own the bz, state your case, and close it.
> I'm 50/50 on it, but figured at least I'd present this option and
> see what the concensus was.
> 
> 
> John Ferlan (12):
>   qemu: Check for and return IOThread polling values if available
>   qemu: Split qemuDomainGetIOThreadsLive
>   qemu: Implement the ability to return IOThread stats
>   virsh: Add ability to display IOThread stats
>   lib: Introduce virDomainSetIOThreadParams
>   qemu: Add monitor functions to set IOThread params
>   qemu: Alter qemuDomainChgIOThread to take enum instead of bool
>   qemu: Alter qemuDomainChgIOThread to take qemuMonitorIOThreadInfo
>   qemu: Detect whether iothread polling is supported
>   qemu: Introduce qemuDomainSetIOThreadParams
>   tools: Add virsh iothreadset command
>   docs: Add news article for IOThread polling

For the feature itself consider the patch series
Acked-by: Christian Borntraeger 

I always wanted this kind of control.


> 
>  docs/news.xml |  13 +
>  include/libvirt/libvirt-domain.h  |  45 ++
>  src/driver-hypervisor.h   |   8 +
>  src/libvirt-domain.c  | 108 +
>  src/libvirt_public.syms   |   5 +
>  src/qemu/qemu_capabilities.c  |   2 +
>  src/qemu/qemu_capabil

Re: [libvirt] [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated

[Christian Borntraeger]

On 10/26/2018 11:42 AM, Daniel P. Berrangé wrote:
> On Fri, Oct 26, 2018 at 12:33:55PM +0530, P J P wrote:
>>   Hello Dan, all
>>
>> +-- On Thu, 25 Oct 2018, Daniel P. Berrangé wrote --+
>> | On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote:
>> | > While being at it deprecate cirrus too.
>> | > 
>> | > Reason (short version): use stdvga instead.
>> | > Verbose version:
>> | > 
>> https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful
>> | 
>> | 
>> | I don't debate the points in the blog post above that stdvga is a
>> | better choice, but I don't think that's enough to justify deprecating
>> | cirrus at this point in time, because when it then gets deleted it
>> | will break way too many existing deployments.
>> | 
>> | We need to socialize info in that blog post above more widely and
>> | especially ensure that apps are not using that by default. I don't
>> | see it being viable to formally deprecate it in QEMU any time soon
>> | though given existing usage.
>>
>> To note, IMO there are other devices/sources in QEMU which are potential 
>> candidates for deprecation, similar to adlib etc. It'll help if we could 
>> device a process to deprecate/remove such code base. Other than maintenance 
>> it 
>> invariably also becomes source of security issues.
>>
>> Ex.(similar to Fedora) we could announce such candidate on qemu-devel list 
>> and 
>> after review over a period of say a month, candidate will be
>> deprecated/expunged. (thinking aloud)
> 
> QEMU has a deprecation process:
> 
>   https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features
> 
> Most of the stuff deprecated is CLI args / monitor commands, etc where
> mgmt apps just adjust the way they are calling QEMU, so end user's VMs
> are largely not impacted.
> 
> Deprecating a device type that is widely used is not desirable because
> that will cause breakage of existing guests.  Distros are free to disable
> devices in their builds if they want to reduce the scope for CVEs in
> packages they maintain, but again they should think carefully about how
> many users they are going to break by doing so.

I agree with what Daniel said. Deprecating something that is in heavy use 
by users just because we have trouble maintaining it not going to help the
QEMU project - quite the opposite. 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v6 09/13] conf: Allocate/release 'uid' and 'fid' in PCI address

[Christian Borntraeger]

On 10/18/2018 05:44 PM, Andrea Bolognani wrote:
> On Thu, 2018-10-18 at 15:13 +0800, Yi Min Zhao wrote:
>> 在 2018/10/17 下午10:30, Andrea Bolognani 写道:
>>> On Tue, 2018-10-16 at 11:28 +0800, Yi Min Zhao wrote:
 I think this would make things complex. If either PCI address or
 zPCI address exists, we have to do more checks for calling
 virDomainPCIAddressReserveAddr(). And there are amounts of
 code calling ***IsWanted() to call ***ReserveNext***(). I think
 keeping them separately is better.
>>>
>>> Again, I might be missing something because I haven't actually tried
>>> implementing any of this, but at least from the theoretical point of
>>> view I don't see how keeping them separate would make things simpler:
>>> if anything, it seems to me like it would make them more complicated
>>> for the calling code because now you have to worry about the PCI
>>> address extensions *in addition* to the PCI address itself.
>>
>> For example, during collection stage, checking both PCI address and 
>> extension address
>> is requried, and still need to separately do some additional checks for 
>> PCI address if it
>> is present, at last in reserving addr function we still check if PCI 
>> normal address or
>> extension address exists to separately reserve present one. So that we 
>> have to do the
>> check on the same condition repetively. If you don't have strong 
>> opposition, I want to
>> send the new version ASAP.
> 
> So I gave an half-hearted stab at implementing my own suggestion
> today, and quite unsurprisingly I have gained more sympathy for your
> argument in the process :)
> 
> The main problem I see is that, as you noticed, we have a lot of
> calls to IsWanted(), IsPresent(), ReserveAddr() and ReserveNextAddr()
> where really we should be using EnsureAddr() pretty much all of the
> time and hide most of the details from the drivers, which in turn
> would make it easier to change them in a transparent manner.
> 
> Another big problem, which I highlighted in the past, is that the
> current API was not designed with the idea that PCI addresses could
> have "parts" in mind, and so it's not nuanced enough: if I call
> IsEmpty() on and address where the PCI part itself has been filled
> in but the zPCI part hasn't, or vice versa, what should I get back?
> The answer is probably that, after we've made sure those functions
> are used as little as possible thanks to the changes outlined above,
> we should replace them with better named alternatives.
> 
> Of course it would be entirely unfair to ask you to perform such
> a massive refactoring before your series can be considered for
> inclusion, so at this point I'm okay with merging it and cleaning
> up the pre-existing mess afterwards.
> 
> There's still the question of whether Dan is now okay with the XML
> structure as currently implemented; assuming that's the case, it
> would be great if Laine could also take a quick look at the series
> before it's pushed.


As I said before, I think the current XML is the right variant. This is
exactly how QEMU implements it (have a real classic pci bus naming scheme
augmented with some additional data named uid/fid).
So having an zcpi name space (instead of a pci one) would be wrong.

Daniel, having said this, are you ok with the current variant?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/11] Allow modification of IOThread polling values (redux)

[Christian Borntraeger]

On 10/15/2018 04:28 PM, John Ferlan wrote:
> 
> ping?
> 
> Any takers or thoughts?

No review, but I think it makes a lot of sense to expose these tuneables.

> 
> Tks,
> 
> John
> 
> 
> On 10/7/18 9:00 AM, John Ferlan wrote:
>> This series attempts to resurrect the concept of being able to modify
>> the IOThread polling parameters; however, in a slightly different
>> manner than the previous series posted by posted by Pavel Hrdina
>> :
>>
>>   https://www.redhat.com/archives/libvir-list/2017-February/msg01047.html
>>
>> The work is prompted by continued pleas found in the bz:
>>
>>   https://bugzilla.redhat.com/show_bug.cgi?id=1545732
>>
>> to provide some way to modify the paremters without needing to supply
>> QEMU command line pass through values.
>>
>> It's accepted that the values being changed are fairly or extremely
>> low level type knobs; however, it's also shown that by being able to
>> turn the knob it is possible for certain, specific appliances to be
>> able to gain a performance benefit for the thread at the expense of
>> other competing threads.
>>
>> Unlike the previous series, this series does not attempt to save the
>> polling values in the guest XML. Rather, it only modifies the live
>> guest's IOThread with new values. It also doesn't provide the polling
>> values in a virsh iothread* command, rather it uses the domstats
>> in order to fetch and display the values. The theory being this
>> leaves the onus on the higher level appliance/application to provide
>> the "proper guidance" and "concerns" related to changing the values
>> to the consumer. Not saving the values means whatever values that
>> are chosen do not "live" in perpetuity. Once the guest is shut down
>> or the IOThread removed from guest, the hypervisor default values
>> take over again. Perhaps not a perfect situation in terms of what
>> the bz requests; however, storage of default values that could
>> cause performance issues is not an optimal situation. So this I
>> figured is a "comprimise" of sorts.
>>
>> If it's still felt that no we don't want to do this, then fine,
>> but please in doing so own the bz, state your case, and close it.
>> I'm 50/50 on it, but figured at least I'd present this option and
>> see what the concensus was.
>>
>> John Ferlan (11):
>>   qemu: Check for and return IOThread polling values if available
>>   qemu: Split qemuDomainGetIOThreadsLive
>>   qemu: Implement the ability to return IOThread stats
>>   virsh: Add ability to display IOThread stats
>>   lib: Introduce virDomainSetIOThreadParams
>>   qemu: Add monitor functions to set IOThread params
>>   qemu: Alter qemuDomainChgIOThread to take enum instead of bool
>>   qemu: Alter qemuDomainChgIOThread to take qemuMonitorIOThreadInfo
>>   qemu: Detect whether iothread polling is supported
>>   qemu: Introduce qemuDomainSetIOThreadParams
>>   tools: Add virsh iothreadset command
>>
>>  include/libvirt/libvirt-domain.h  |  45 ++
>>  src/driver-hypervisor.h   |   8 +
>>  src/libvirt-domain.c  | 109 +
>>  src/libvirt_public.syms   |   5 +
>>  src/qemu/qemu_capabilities.c  |   2 +
>>  src/qemu/qemu_capabilities.h  |   1 +
>>  src/qemu/qemu_driver.c| 393 --
>>  src/qemu/qemu_monitor.c   |  19 +
>>  src/qemu/qemu_monitor.h   |   6 +
>>  src/qemu/qemu_monitor_json.c  |  48 +++
>>  src/qemu/qemu_monitor_json.h  |   4 +
>>  src/remote/remote_driver.c|   1 +
>>  src/remote/remote_protocol.x  |  21 +-
>>  src/remote_protocol-structs   |  10 +
>>  .../caps_2.10.0.aarch64.xml   |   1 +
>>  .../caps_2.10.0.ppc64.xml |   1 +
>>  .../caps_2.10.0.s390x.xml |   1 +
>>  .../caps_2.10.0.x86_64.xml|   1 +
>>  .../caps_2.11.0.s390x.xml |   1 +
>>  .../caps_2.11.0.x86_64.xml|   1 +
>>  .../caps_2.12.0.aarch64.xml   |   1 +
>>  .../caps_2.12.0.ppc64.xml |   1 +
>>  .../caps_2.12.0.s390x.xml |   1 +
>>  .../caps_2.12.0.x86_64.xml|   1 +
>>  .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |   1 +
>>  .../qemucapabilitiesdata/caps_2.9.0.s390x.xml |   1 +
>>  .../caps_2.9.0.x86_64.xml |   1 +
>>  .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |   1 +
>>  .../caps_3.0.0.riscv32.xml|   1 +
>>  .../caps_3.0.0.riscv64.xml|   1 +
>>  .../qemucapabilitiesdata/caps_3.0.0.s390x.xml |   1 +
>>  .../caps_3.0.0.x86_64.xml |   1 +
>>  tools/virsh-domain-monitor.c  |   7 +
>>  tools/virsh-domain.c  | 110 +
>>  tools/virsh.pod   |  47 ++-
>>  35 files changed, 810 

Re: [libvirt] [PATCH v6 00/13] PCI passthrough support on s390

[Christian Borntraeger]

On 10/15/2018 09:30 AM, Boris Fiuczynski wrote:
> On 10/14/18 2:47 PM, Andrea Bolognani wrote:
>> On Fri, 2018-10-12 at 16:04 +0100, Daniel P. Berrangé wrote:
>>> On Fri, Sep 28, 2018 at 04:46:13PM +0800, Yi Min Zhao wrote:
>> [...]
    
  
  
    
  
  >>> function='0x0'>
    
  
    
>>>
>>> I'm not sure if this was discussed in earlier versions, but to me
>>> this use of a child element looks wrong.
>>>
>>> What we're effectively saying is that s390 has a different addressing
>>> scheme. It happens to share some fields with the current PCI addressing
>>> scheme, but it is none the less a distinct scheme.
>>>
>>> IOW, I think it should be
>>>
>>>     >>  function='0x0' uid='0x0003' fid='0x0027'/>
>>>
>>> Of course internally we can still share much logic for assigning the
>>> addreses between "pci" and "zpci".
>>
>> So what happens with PCI devices on s390 is that *two* devices will
>> be added to the guest: one is the usual virtio-net-pci or what have
>> you, which has its own PCI address allocated using the same algorithm
>> as other architectures; the other one is a '-device zpci', which IIUC
>> works basically like an adapter between the PCI device itself and the
>> guest OS, and which is identified using uid and fid.
>>
>> Calling it a completely different address type seems like a bit of a
>> stretch: there is definitely a PCI address involved, which is why the
>> zPCI part was implemented through a potentially reusable "PCI address
>> extension" mechanism.
>>
> I thought that we discussed this in v1 or v2 already when uid anf fid were 
> still embedded in the address element itself. In v5 Andrea suggested to model 
> the two zpci extension parameters outside as a child element of address which 
> corresponds kind of to what is happening in qemu (see Andreas paragraph 
> above).
> The original idea was for users on s390 to make pci no different than on 
> other platforms. Creating a zpci address type would introduce the opposite.
> Currently uid and fid are optional attributes for the user on s390. He can 
> simply enter any kind of pci address as for other platforms. If he does so on 
> s390 the uid and fid would be automatically generated for him. Only if he 
> chooses to specifically set these attributes himself he has to specify uid 
> and/or fid.
Agreed.
In QEMU this is really modelled as PCI (with the classic bus addresses).
The fact that the guest uses the fid/uid instead does not make the PCI bus
addresses in QEMU go away and it should not change the modelling of
the devices.
So I think that the current proposal from Andrea implemented
by Yi Min and endorsed by Boris:

   

is really the best solution.
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC PATCH 0/3] qemu: guest dedicated crypto adapters

[Christian Borntraeger]

FWIW,

the qemu patches are now in master.
https://git.qemu.org/?p=qemu.git;a=commit;h=69ac8c4cb93f2685839ff7b857cef306b388ff3c




On 10/08/2018 06:25 PM, Boris Fiuczynski wrote:
> This RFC patch series introduces initial libvirt support for guest
> dedicated crypto adapters on S390.
> It essentially allows to specify a vfio-ap mediated device in a domain.
> Extensive documentation about AP is available in patch 6 of
> the QEMU patch series.
> 
> KVM/kernel: guest dedicated crypto adapters
> https://lkml.org/lkml/2018/9/26/25
> 
> QEMU: s390x: vfio-ap: guest dedicated crypto adapters
> https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg03538.html
> 
> Boris Fiuczynski (3):
>   qemu: add vfio-ap capability
>   qemu: vfio-ap device support
>   news: Update news for vfio-ap support
> 
>  docs/formatdomain.html.in  | 3 ++-
>  docs/news.xml  | 9 +
>  docs/schemas/domaincommon.rng  | 1 +
>  src/qemu/qemu_capabilities.c   | 2 ++
>  src/qemu/qemu_capabilities.h   | 1 +
>  src/qemu/qemu_command.c| 8 
>  src/qemu/qemu_domain_address.c | 4 
>  src/util/virmdev.c | 3 ++-
>  src/util/virmdev.h | 1 +
>  9 files changed, 30 insertions(+), 2 deletions(-)
> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH for-3.1] s390x: remove 's390-squash-mcss' option

[Christian Borntraeger]

Acked-by: Christian Borntraeger 


On 07/24/2018 11:24 AM, Cornelia Huck wrote:
> This option has been deprecated for two releases; remove it.
> 
> Signed-off-by: Cornelia Huck 
> ---
>  hw/s390x/3270-ccw.c|  2 +-
>  hw/s390x/css-bridge.c  |  1 -
>  hw/s390x/css.c |  6 ++
>  hw/s390x/s390-ccw.c|  2 +-
>  hw/s390x/s390-virtio-ccw.c | 37 ++---
>  hw/s390x/virtio-ccw.c  |  2 +-
>  include/hw/s390x/css-bridge.h  |  1 -
>  include/hw/s390x/css.h |  9 +++--
>  include/hw/s390x/s390-virtio-ccw.h |  1 -
>  qemu-deprecated.texi   |  8 
>  qemu-options.hx| 10 --
>  target/s390x/cpu.c | 10 --
>  target/s390x/cpu.h |  1 -
>  13 files changed, 10 insertions(+), 80 deletions(-)
> 
> diff --git a/hw/s390x/3270-ccw.c b/hw/s390x/3270-ccw.c
> index 3af13ea027..cf58b81fc0 100644
> --- a/hw/s390x/3270-ccw.c
> +++ b/hw/s390x/3270-ccw.c
> @@ -104,7 +104,7 @@ static void emulated_ccw_3270_realize(DeviceState *ds, 
> Error **errp)
>  SubchDev *sch;
>  Error *err = NULL;
>  
> -sch = css_create_sch(cdev->devno, cbus->squash_mcss, errp);
> +sch = css_create_sch(cdev->devno, errp);
>  if (!sch) {
>  return;
>  }
> diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
> index a02d708239..1bd6c8b458 100644
> --- a/hw/s390x/css-bridge.c
> +++ b/hw/s390x/css-bridge.c
> @@ -106,7 +106,6 @@ VirtualCssBus *virtual_css_bus_init(void)
>  /* Create bus on bridge device */
>  bus = qbus_create(TYPE_VIRTUAL_CSS_BUS, dev, "virtual-css");
>  cbus = VIRTUAL_CSS_BUS(bus);
> -cbus->squash_mcss = s390_get_squash_mcss();
>  
>  /* Enable hotplugging */
>  qbus_set_hotplug_handler(bus, dev, _abort);
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 5424ea4562..5a9fe45ce8 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -2359,15 +2359,13 @@ const PropertyInfo css_devid_ro_propinfo = {
>  .get = get_css_devid,
>  };
>  
> -SubchDev *css_create_sch(CssDevId bus_id, bool squash_mcss, Error **errp)
> +SubchDev *css_create_sch(CssDevId bus_id, Error **errp)
>  {
>  uint16_t schid = 0;
>  SubchDev *sch;
>  
>  if (bus_id.valid) {
> -if (squash_mcss) {
> -bus_id.cssid = channel_subsys.default_cssid;
> -} else if (!channel_subsys.css[bus_id.cssid]) {
> +if (!channel_subsys.css[bus_id.cssid]) {
>  css_create_css_image(bus_id.cssid, false);
>  }
>  
> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
> index 214c940593..d1280bf631 100644
> --- a/hw/s390x/s390-ccw.c
> +++ b/hw/s390x/s390-ccw.c
> @@ -78,7 +78,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
> *sysfsdev, Error **errp)
>  goto out_err_propagate;
>  }
>  
> -sch = css_create_sch(ccw_dev->devno, cbus->squash_mcss, );
> +sch = css_create_sch(ccw_dev->devno, );
>  if (!sch) {
>  goto out_mdevid_free;
>  }
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 7983185d04..380a41d806 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -282,19 +282,8 @@ static void ccw_init(MachineState *machine)
>  virtio_ccw_register_hcalls();
>  
>  s390_enable_css_support(s390_cpu_addr2state(0));
> -/*
> - * Non mcss-e enabled guests only see the devices from the default
> - * css, which is determined by the value of the squash_mcss property.
> - */
> -if (css_bus->squash_mcss) {
> -ret = css_create_css_image(0, true);
> -} else {
> -ret = css_create_css_image(VIRTUAL_CSSID, true);
> -}
> -if (qemu_opt_get(qemu_get_machine_opts(), "s390-squash-mcss")) {
> -warn_report("The machine property 's390-squash-mcss' is deprecated"
> -" (obsoleted by lifting the cssid restrictions).");
> -}
> +
> +ret = css_create_css_image(VIRTUAL_CSSID, true);
>  
>  assert(ret == 0);
>  if (css_migration_enabled()) {
> @@ -575,21 +564,6 @@ static void machine_set_loadparm(Object *obj, const char 
> *val, Error **errp)
>  ms->loadparm[i] = ' '; /* pad right with spaces */
>  }
>  }
> -static inline bool machine_get_squash_mcss(Object *obj, Error **errp)
> -{
> -S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> -
> -return ms->s390_squash_mcss;
> -}
> -
> -static inline void machine_set_squash_mcss(Object *obj, bool value,

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Christian Borntraeger]

On 07/04/2018 03:34 PM, Kevin Wolf wrote:
> Am 04.07.2018 um 15:02 hat Cornelia Huck geschrieben:
>> On Tue, 3 Jul 2018 13:32:29 +0200
>> Kevin Wolf  wrote:
>>
>> Has serial/gemoetry been fixed meanwhile and will it make it into the
>> next release?  
>
> I cannot find an archive that has it, but it is on the libvirt mailing
> list as "[libvirt] [PATCH v3] qemu: format serial and geometry on 
> frontend disk device".
> Review seems done, but it has missed libvirt 4.5 which was released 
> today.  

 Just posted latest version here:

   https://www.redhat.com/archives/libvir-list/2018-July/msg00130.html

 It will be in the next release on ~ Aug 1st  
>>>
>>> It would have been a lot nicer to have it the July release because this
>>> means that we'll have the released libvirt broken during almost the
>>> whole rc phase of QEMU 3.0, but the release is planned for Aug 8th the
>>> earliest, so I guess we're still okay. People using QEMU from git will
>>> just need libvirt from git as well.
>>
>> Speaking as an innocent* bystander:
>>
>> I would usually presume that I can use any recent libvirt to test
>> current QEMU, even bleeding edge. In this case, not even the latest
>> released libvirt version will be fine, I would also need to build
>> libvirt from git (which is probably not something a non-libvirt
>> developer will usually do). If everything goes according to plan, I can
>> only test QEMU with a released libvirt version at the very tail end of
>> hardfreeze, where only release blockers are appropriate.
> 
> I understand where you're coming from, but let's be honest: It's not as
> if disk geometry or serial numbers were features that absolutely need
> to be there to give QEMU any testing.

For s390 it really has values when you use image files with a dasd geometry.
I also use the serial number a lot for mount by disk-id. So it certainly breaks
parts of my tests.

> 
> Also, my understanding has always been that we expect users to have a
> libvirt version that isn't older than QEMU. It would be useful to set a
> clear policy for this and document it.
> 
>> I think it would be really beneficial to general QEMU test coverage to
>> push deleting this option back a release or two. We should make testing
>> QEMU in conjunction with libvirt as uncomplicated as possible.
> 
> Essentially, what is important to me isn't getting these options dropped
> exactly in 3.0, but not setting a bad precedence that deprecation isn't
> actually worth anything. We may easily end up with this deprecation
> process:
> 
> depreate a feature
> release QEMU version n + 1
> release QEMU version n + 2
> remove the feature
> while libvirt hasn't removed use of the feature:
> # ...and why should it when everything is still working?
> reinstate the feature
> release QEMU version n + x
> remove the feature
> 
> When management tools know that this is the process, the motivation to
> remove the use of the feature gets even lower (not out of malice, but
> because there will be always more important things), so this will
> "optimise" itself into an endless loop and we're back to never actually
> removing old cruft that impedes development.

I think this is really a theoretical issue that assumes that libvirt people
are evil and try to undermine our deprecation. And this is simply not true.
Everybody tries to do his best, but we are all busy. So we had the issue
because we were sloppy and have not dealt with the deprecation properly
(by pushing the libvirt people at time of deprecation) And the reason is
that we really have no process for qemu->libvirt requirements. I mean:
look at virtio-vsock and other things.  How long it took from qemu to 
libvirt show that the real issue is actually somewhere else and we might 
want to talk about "how to improve qemu<->libvirt interaction" at the qemu
summit.


> 
> The libvirt patch has just been merged (and I'm almost sure that this
> wouldn't have happened so quickly if I had just reverted the patch right
> away), so at least we know now that this specific instance of the loop
> is going to terminate.
> 
> What's left is first and foremost that we need to sort out our broken
> deprecation mechanism, and if that gets done, I don't mind if someone
> wants to revert the patch for 3.0 as long as they also take care that it
> gets back into 3.1.

So what about the following:
- revert these patches now
- add these patches to the block-next-for-3.1 branch immediately (or whatever 
branch
name you have for that)


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Christian Borntraeger]

On 07/03/2018 01:35 PM, Peter Maydell wrote:
> On 3 July 2018 at 12:32, Kevin Wolf  wrote:
>> Am 03.07.2018 um 13:22 hat Daniel P. Berrangé geschrieben:
>>> Just posted latest version here:
>>>
>>>   https://www.redhat.com/archives/libvir-list/2018-July/msg00130.html
>>>
>>> It will be in the next release on ~ Aug 1st
>>
>> It would have been a lot nicer to have it the July release because this
>> means that we'll have the released libvirt broken during almost the
>> whole rc phase of QEMU 3.0, but the release is planned for Aug 8th the
>> earliest, so I guess we're still okay. People using QEMU from git will
>> just need libvirt from git as well.
> 
> I'm still not clear what we gain from having a QEMU that's dropped
> a feature that is still used by everything except leading-edge
> not-yet-released versions of libvirt. Is there a strong reason we
> can't just revert the deletion of the deprecated feature for a QEMU
> release or two?

+1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Christian Borntraeger]

On 07/02/2018 10:04 AM, Kevin Wolf wrote:
> Am 25.06.2018 um 13:45 hat Peter Krempa geschrieben:
>> On Mon, Jun 25, 2018 at 13:41:06 +0200, Kevin Wolf wrote:
>>> Am 25.06.2018 um 11:53 hat Daniel P. Berrangé geschrieben:
>>>> On Fri, Jun 22, 2018 at 03:31:46PM +0100, Daniel P. Berrangé wrote:
>>>>> On Fri, Jun 22, 2018 at 04:25:13PM +0200, Kevin Wolf wrote:
>>>>>> Am 22.06.2018 um 15:36 hat Christian Borntraeger geschrieben:
>>
>> [...]
>>
>>>
>>> Thanks!
>>>
>>> I'll look into werror/rerror support for usb-storage. It shouldn't be
>>> too hard, though it's strictly speaking a separate problem related to
>>> using -blockdev rather than option deprecation.
>>>
>>> If Peter wants to wait for QEMU support before converting werror/rerror
>>
>> Definitely. I don't want to keep around yet another hack that will
>> satisfy one specific case and then add another capability for it. We
>> should then gate the moving of the feature based on the presence of
>> werror for usb-storage.
>>
>>> to -device, maybe it would make sense to split your patch for v2 so that
>>> geometry and serial can get fixed right away?
>>
>> Yes this can be done right away.
> 
> Has serial/gemoetry been fixed meanwhile and will it make it into the
> next release?

I cannot find an archive that has it, but it is on the libvirt mailing
list as "[libvirt] [PATCH v3] qemu: format serial and geometry on frontend disk 
device".
Review seems done, but it has missed libvirt 4.5 which was released today. 

Christian

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Christian Borntraeger]

On 06/22/2018 05:01 PM, Kevin Wolf wrote:
> Am 22.06.2018 um 16:38 hat Christian Borntraeger geschrieben:
>>
>>
>> On 06/22/2018 04:25 PM, Kevin Wolf wrote:
>>> Am 22.06.2018 um 15:36 hat Christian Borntraeger geschrieben:
>>>>
>>>>
>>>> On 06/22/2018 02:55 PM, Kevin Wolf wrote:
>>>>> Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben:
>>>>>>
>>>>>> On 06/15/2018 04:21 PM, Kevin Wolf wrote:
>>>>>>> The -drive option serial was deprecated in QEMU 2.10. It's time to
>>>>>>> remove it.
>>>>>>>
>>>>>>> Tests need to be updated to set the serial number with -global instead
>>>>>>> of using the -drive option.
>>>>>>
>>>>>> libvirt 4.5 still creates those (at least on s390x)
>>>>>>
>>>>>> 
>>>>>>   >>>>> iothread='1'/>
>>>>>>   
>>>>>>   
>>>>>>   skel
>>>>>>   
>>>>>>   
>>>>>> 
>>>>>>
>>>>>>
>>>>>> -> 
>>>>>> [...]
>>>>>> -drive 
>>>>>> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native
>>>>>>  -device 
>>>>>> virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on
>>>>>>  
>>>>>> [...]
>>>>>>
>>>>>> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive 
>>>>>> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native:
>>>>>>  Block format 'qcow2' does not support the option 'serial'
>>>>>> 2018-06-22 11:25:21.098+: shutting down, reason=failed
>>>>>>
>>>>>> So it seems that this breaks s390x.
>>>>
>>>> To me it seems that this is also broken on x86.
>>>>>
>>>>> Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is
>>>>> released.
>>>>
>>>> I think this is definitely too short notice. We should not break existing
>>>> setups just by insisting that users have to update libvirt when they update
>>>> QEMU. Yes, this might be our policy, but doing so "just because we can"
>>>> is certainly a very bad attitude. I see no fundamental technical reason why
>>>> we should not revert this change.
>>>
>>> This was in fact one release longer than our deprecation policy says.
>>> Are we serious about the deprecation policy or aren't we?
>>
>> I think it makes more sense to have 2 releases after everything was fixed
>> instead of 2 releases after it was announced.
> 
> This means effectively banning feature removal. The only time that
> people actually starting fixing things is when it breaks. So if you
> never remove it before everything is fixed, you just never remove it at
> all.

With the proposal that is floating around (like the --no-deprecation option
to be used for regression test suites) this could be solved. 

> 
> It's unfortunate, but breaking things at some point is necessary. I hope
> the breakage will only last a few days because libvirt will fix this.
> 
> Maybe one thing we could look into for the future is a special
> deprecation warning function rather than just error_report(), and we
> would make that one fatal in non-release builds so that things break
> early, but you can still override it with a ./configure option.
> 
>> So if everyone has adopted we can certainly follow our deprecation policy.
>> Now if deprecation breaks some real world cases it makes no sense to
>> "insist" on that deprecation policy. Really: If latest greatest libvirt
>> does not work 2 weeks before soft freeze I consider this too late.
>>
>> Why: This breaks MY regression test setup before softfreeze. So I will stop
>> testing qemu in the most critical point in time.
>>
>> If you would come up with your statement (taking deprecation policy more
>> serious than users) in the Linux kernel I can pretty much guarantee that
>> Linus would call you names.
> 
> Users shouldn't use random git snapshots. Developers can revert the
> change locally until libvirt is fixed.g
> 
> If contrary to all expectations, libvirt doesn't manage to get this
> fixed until 3.0-rc2, I will consider reverting the patch. But not
> significantly earlier than that.


I think I made it clear that I consider this wrong on so many levels.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Christian Borntraeger]

On 06/22/2018 04:25 PM, Kevin Wolf wrote:
> Am 22.06.2018 um 15:36 hat Christian Borntraeger geschrieben:
>>
>>
>> On 06/22/2018 02:55 PM, Kevin Wolf wrote:
>>> Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben:
>>>>
>>>> On 06/15/2018 04:21 PM, Kevin Wolf wrote:
>>>>> The -drive option serial was deprecated in QEMU 2.10. It's time to
>>>>> remove it.
>>>>>
>>>>> Tests need to be updated to set the serial number with -global instead
>>>>> of using the -drive option.
>>>>
>>>> libvirt 4.5 still creates those (at least on s390x)
>>>>
>>>> 
>>>>   >>> iothread='1'/>
>>>>   
>>>>   
>>>>   skel
>>>>   
>>>>   
>>>> 
>>>>
>>>>
>>>> -> 
>>>> [...]
>>>> -drive 
>>>> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native
>>>>  -device 
>>>> virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on
>>>>  
>>>> [...]
>>>>
>>>> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive 
>>>> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native:
>>>>  Block format 'qcow2' does not support the option 'serial'
>>>> 2018-06-22 11:25:21.098+: shutting down, reason=failed
>>>>
>>>> So it seems that this breaks s390x.
>>
>> To me it seems that this is also broken on x86.
>>>
>>> Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is
>>> released.
>>
>> I think this is definitely too short notice. We should not break existing
>> setups just by insisting that users have to update libvirt when they update
>> QEMU. Yes, this might be our policy, but doing so "just because we can"
>> is certainly a very bad attitude. I see no fundamental technical reason why
>> we should not revert this change.
> 
> This was in fact one release longer than our deprecation policy says.
> Are we serious about the deprecation policy or aren't we?

I think it makes more sense to have 2 releases after everything was fixed
instead of 2 releases after it was announced.

So if everyone has adopted we can certainly follow our deprecation policy.
Now if deprecation breaks some real world cases it makes no sense to
"insist" on that deprecation policy. Really: If latest greatest libvirt
does not work 2 weeks before soft freeze I consider this too late.

Why: This breaks MY regression test setup before softfreeze. So I will stop
testing qemu in the most critical point in time.

If you would come up with your statement (taking deprecation policy more
serious than users) in the Linux kernel I can pretty much guarantee that
Linus would call you names.


> 
> I might consider reverting a change if it turned out that this requires
> some massive work in libvirt. But I think this one should be rather easy
> to fix in libvirt until 3.0 is released.

I have not heard any reason what we gain by removing these features (and no
I dont believe that this increases your maintenance burden a lot). But it
clearly breaks things.

I suggest: revert the removal patches (all of them cyls,secs,serial etc)
and redo them for 3.1.



> 
>>> Sadly, it also shows that deprecation warnings in log files go
>>> unnoticed.
>>
>> In fact whoever added the deprication notice should have followed up
>> with the libvirt team to implement that change. no?
> 
> I expect the libvirt developers to read the QEMU Changelog at least for
> incompatible changes and deprecations. We can't reasonably go and hunt
> for developers for every management tool for QEMU that exists.
> 
> And anyway, if you come across a deprecation warning, that's the time
> you should act, not only when it finally breaks.
> 
> Kevin
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Christian Borntraeger]

On 06/22/2018 03:36 PM, Christian Borntraeger wrote:
> 
> 
> On 06/22/2018 02:55 PM, Kevin Wolf wrote:
>> Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben:
>>>
>>> On 06/15/2018 04:21 PM, Kevin Wolf wrote:
>>>> The -drive option serial was deprecated in QEMU 2.10. It's time to
>>>> remove it.
>>>>
>>>> Tests need to be updated to set the serial number with -global instead
>>>> of using the -drive option.
>>>
>>> libvirt 4.5 still creates those (at least on s390x)
>>>
>>> 
>>>   >> iothread='1'/>
>>>   
>>>   
>>>   skel
>>>   
>>>   
>>> 
>>>
>>>
>>> -> 
>>> [...]
>>> -drive 
>>> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native
>>>  -device 
>>> virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on
>>>  
>>> [...]
>>>
>>> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive 
>>> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native:
>>>  Block format 'qcow2' does not support the option 'serial'
>>> 2018-06-22 11:25:21.098+: shutting down, reason=failed
>>>
>>> So it seems that this breaks s390x.
> 
> To me it seems that this is also broken on x86.
>>
>> Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is
>> released.
> 
> I think this is definitely too short notice. We should not break existing
> setups just by insisting that users have to update libvirt when they update
> QEMU. Yes, this might be our policy, but doing so "just because we can"
> is certainly a very bad attitude. I see no fundamental technical reason why
> we should not revert this change.
> 
> 
>>
>> Sadly, it also shows that deprecation warnings in log files go
>> unnoticed.
> 
> In fact whoever added the deprication notice should have followed up
> with the libvirt team to implement that change. no?

FWIW cyls, heads, secs and trans also seem to be affected by this.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Christian Borntraeger]

On 06/22/2018 02:55 PM, Kevin Wolf wrote:
> Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben:
>>
>> On 06/15/2018 04:21 PM, Kevin Wolf wrote:
>>> The -drive option serial was deprecated in QEMU 2.10. It's time to
>>> remove it.
>>>
>>> Tests need to be updated to set the serial number with -global instead
>>> of using the -drive option.
>>
>> libvirt 4.5 still creates those (at least on s390x)
>>
>> 
>>   > iothread='1'/>
>>   
>>   
>>   skel
>>   
>>   
>> 
>>
>>
>> -> 
>> [...]
>> -drive 
>> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native
>>  -device 
>> virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on
>>  
>> [...]
>>
>> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive 
>> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native:
>>  Block format 'qcow2' does not support the option 'serial'
>> 2018-06-22 11:25:21.098+: shutting down, reason=failed
>>
>> So it seems that this breaks s390x.

To me it seems that this is also broken on x86.
> 
> Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is
> released.

I think this is definitely too short notice. We should not break existing
setups just by insisting that users have to update libvirt when they update
QEMU. Yes, this might be our policy, but doing so "just because we can"
is certainly a very bad attitude. I see no fundamental technical reason why
we should not revert this change.


> 
> Sadly, it also shows that deprecation warnings in log files go
> unnoticed.

In fact whoever added the deprication notice should have followed up
with the libvirt team to implement that change. no?


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] request a revert for "block: Remove deprecated -drive option serial" (was block: Remove deprecated -drive option serial)

[Christian Borntraeger]

adding more CC.

On 06/22/2018 01:38 PM, Christian Borntraeger wrote:
> 
> On 06/15/2018 04:21 PM, Kevin Wolf wrote:
>> The -drive option serial was deprecated in QEMU 2.10. It's time to
>> remove it.
>>
>> Tests need to be updated to set the serial number with -global instead
>> of using the -drive option.
> 
> libvirt 4.5 still creates those (at least on s390x)
> 
> 
>   
>   
>   
>   skel
>   
>   
> 
> 
> 
> -> 
> [...]
> -drive 
> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native
>  -device 
> virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on
>  
> [...]
> 
> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive 
> file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native:
>  Block format 'qcow2' does not support the option 'serial'
> 2018-06-22 11:25:21.098+: shutting down, reason=failed
> 
> So it seems that this breaks s390x.

So what about reverting commit b0083267444a5e0f28391f6c2831a539f878d424
"block: Remove deprecated -drive option serial" and redo the removal in
qemu 3.1 (or 3.2) ?
Even if we fix libvirt today, this is certainly a too short period of 
time to get things fixed in the field.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Christian Borntraeger]

On 06/15/2018 04:21 PM, Kevin Wolf wrote:
> The -drive option serial was deprecated in QEMU 2.10. It's time to
> remove it.
> 
> Tests need to be updated to set the serial number with -global instead
> of using the -drive option.

libvirt 4.5 still creates those (at least on s390x)


  
  
  
  skel
  
  



-> 
[...]
-drive 
file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native
 -device 
virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on
 
[...]

2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive 
file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native:
 Block format 'qcow2' does not support the option 'serial'
2018-06-22 11:25:21.098+: shutting down, reason=failed

So it seems that this breaks s390x.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 02/12] Introduce new domain create API virDomainCreateWithParams

[Christian Borntraeger]

On 05/16/2018 05:35 PM, Daniel P. Berrangé wrote:
> On Wed, May 16, 2018 at 05:30:33PM +0200, Marc Hartmayer wrote:
>> On Wed, May 09, 2018 at 05:40 PM +0200, "Daniel P. Berrangé" 
>>  wrote:
>>> On Wed, May 09, 2018 at 04:56:12PM +0200, Marc Hartmayer wrote:
 Introduce new libvirt API virDomainCreateWithParams that allows to
 temporarily boot from another boot device, to use another kernel,
 initrd, and cmdline than defined in the persistent domain
 definition. All typed parameters are optional.

 The design of the API was chosen to ease future extensions.
>>>
>>> I don't really see the point in doing this. We already have the ability
>>> to temporary boot with a different configuration than is stored in
>>> the persistent XML. Just call virDomainCreateXML() passing in the
>>> alternative XML doc. This allows changing *any* aspect of the guest
>>> configuration, so we're not restricted to just bot device, kernel
>>> initrd and cmdline, and thus won't need to write more code anytime
>>> someone asks to be able to override something else too.
>>
>> I know there is the API virDomainCreateXML for creating a transient
>> domain and that it’s possible to temporarily replace parts of the
>> persistent XML with it. But my idea is _not_ to add a functionality to
>> override parts of the persistent XML. My idea is to provide support
>> allowing an easy one-time switch of the boot device in a persistently
>> defined domain. For s390 it’s essential to have an easy way to change
>> the boot configuration since it “knows” only one boot device at a time
>> and it has no support for interactively changing the boot device during
>> the boot/IPL process.
> 
> virDomainCreateXML does not change the persistent XML.  If you have
> an existing persistent guest, and use virDomainCreateXML it lets you
> start that guest with that customized XML just once, without affecting
> the persistent XML at all.
> 
>> I started out with a fixed API (as it was done for example with
>> virDomanCreateWithFiles) but than I liked the approach of providing an
>> more extensible and flexible API better. This is also the reason why I
>> used typed parameters for passing the arguments. This type of API is
>> also not restricted to boot order changes since it could be freely
>> expanded (e.g. passing file descriptors). I can certainly revert back to
>> the static API.
> 
> Using virDomainCreateXML is the most flexible API because it lets you
> changed anything in the XML for just that one boot up attempt.

To clarify this:
would it be ok for you to implement this feature (the command line parameter
to "virsh start") in virsh by doing the xml mangling in the virsh command line
tool itself?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/1] Bug: Sandbox: libvirt breakdowns qemu guest

[Christian Borntraeger]

On 05/07/2018 02:02 PM, Ján Tomko wrote:
> On Mon, May 07, 2018 at 12:33:20PM +0200, Eduardo Otubo wrote:
>> On 07/05/2018 - 11:29:57, Christian Borntraeger wrote:
>>> On 05/07/2018 05:32 AM, Yi Min Zhao wrote:
>>> > 1. Problem Description
>>> > ==
>>> > If QEMU is built without seccomp support, 'elevatorprivileges' remains 
>>> > compiled.
>>> > This option of sandbox is treated as an indication for seccomp blacklist 
>>> > support
>>> > in libvirt. This behavior is introduced by the libvirt commits 31ca6a5 and
>>> > 3527f9d. It would make libvirt build wrong QEMU cmdline, and then the 
>>> > guest
>>> > startup would fail.
>>>
>>> Adding libvirt list.
>>>
>>> This would still fail with older QEMUs, so the question is if we should 
>>> also OR instead
>>> change something in libvirt.
>>
>> Perhaps I'm missing something here, but libvirt can differentiate between
>> different versions of QEMU, therefore not calling it with wrong or outdated
>> arguments.
>>
> 
> The code introduced in libvirt commit 31ca6a5 specifically looks for
> 'elevateprivileges' in 'parameters' of the 'sandbox' option through
> query-command-line-options.
> 
> Outdated QEMUs should not have this option there.
> 
> However, libvirtd does add the option by default not knowing whether it
> can fail for other reasons, e.g. SECCOMP not being enabled in the
> running kernel. I wonder if that is worth addressing.

So you prefer the qemu patch (with cc stable) as the best solution?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/1] Bug: Sandbox: libvirt breakdowns qemu guest

[Christian Borntraeger]

On 05/07/2018 05:32 AM, Yi Min Zhao wrote:
> 1. Problem Description
> ==
> If QEMU is built without seccomp support, 'elevatorprivileges' remains 
> compiled.
> This option of sandbox is treated as an indication for seccomp blacklist 
> support
> in libvirt. This behavior is introduced by the libvirt commits 31ca6a5 and
> 3527f9d. It would make libvirt build wrong QEMU cmdline, and then the guest
> startup would fail.

Adding libvirt list.

This would still fail with older QEMUs, so the question is if we should also OR 
instead
change something in libvirt.

> 
> 2. Libvirt Log
> ==
> qemu-system-s390x: -sandbox 
> on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
> resourcecontrol=deny: seccomp support is disabled
> 
> 3. Fixup
> 
> Wrap the options except 'enable' for qemu_sandbox_opts by CONFIG_SECCOMP.
> 
> Yi Min Zhao (1):
>   sandbox: avoid to compile options if CONFIG_SECCOMP undefined
> 
>  vl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

[Christian Borntraeger]

On 10/27/2017 03:40 PM, Halil Pasic wrote:
> 
> 
> On 10/27/2017 02:57 PM, Christian Borntraeger wrote:
>>
>>
>> On 10/27/2017 02:45 PM, Christian Borntraeger wrote:
>>>
>>>
>>> On 10/27/2017 02:31 PM, Halil Pasic wrote:
>>> gs is explicitly disabled.
>>>>
>>>> Now that I think about it, maybe the 2.9 binary is going to reject
>>>> the explicit gs flag altogether, because it's unknown.
>>>>
>>>> Isn't this a problem? 
>>>
>>> No. This is exactly the _solution_ and not the problem. The target will 
>>> reject
>>> unknown cpu features and migration will be aborted. This is exactly what 
>>> the CPU
>>> model is for.
> 
> I'm not sure we talk abut the same thing. I'm talking about the following. I
> want to disable a cpu-model feature for the sake of migration (because I know
> that binary version X does not support the feature, because it does not know
> about it). Now if I do it via let's say -cpu z13,gs=off on let's say 2.11,
> and start with the exact same command line (-cpu z13,gs=off) on lets say 2.9
> my migration will explode because of the unknown feature I'm specifying
> not to be used.

The migration will be rejected because the target qemu will not startup.
You can easily simulate that, e.g. by doing

qemu-system-s390x -cpu z13,notyetknown=off
qemu-system-s390x: can't apply global z13-s390x-cpu.notyetknown=off: Property 
'.notyetknown' not found

But libvirt will not use a full model (and the disable things) instead it will
use the base model and add things. (So libvirt should never use xxx=off)


I think this is really not an issue. If you specify a feature that is not known 
then
QEMU will not start on the target and migration is rejected. The guest 
continues to run
on the source. So if you specify a "too new" facility yourself its really a 
user error.
Everything that uses an explicit model (e.g. -cpu z13 or -cpu,sief2=on) will 
work, but only
as long as the conditions are met. If you specify -cpu z14, it does not matter 
if it fails
if the kernel or QEMU is too old, or if you just happen to run on a z13.

The  only question is/was:  what is about "host-model".
With my patch (+ the gs fixup) the following things will work:
- host-model will work on z13
- host-model will work on z14 (any machine version)
- host model on z13 and then migrating to z14 will work (any machine version)
- host model on z13 and then migrating to z14 and then migrating back will work 
(any version)
- qemu with fixup + host model on z14 with machine version 2.10 can be migrated

The only thing that does not work is
- qemu with fixup + host model on z14 with machine 2.9 can not be migrated to 
qemu 2.9 on z14.

Now: this would have not worked anyway, because qemu 2.9 does not know z14. So 
in theory 
QEMU must forbit z14 for compat machines (which we do not know).

I talked to several people and it seems that on x86 the host model will also 
enable new features
that are not known by older QEMUs and its considered works as designed. (see 
also Jiris mail)


> 
> Well I'm not sure what I describe is relevant. My thinking is along the lines
> some features are added incrementally. How do use those of the features not 
> included
> in -base model which both of my environments support and disable those that
> are unsupported by one of the environments.
> 
> I will think about it some more. I've asked Boris about this situation,
> and he did not put my mind at ease (to be more precise he seemed to
> see this as a potential problem too), so I've decided to mention it.
> Sorry if I've generated some unnecessary noise.
> 
> I think the root of the problem is that I don't understand the difference 
> between
> z13-base and z13, and the associated rules and expected/intended usages. 

z13-base contains only those features that a guaranteed to be there (there is
the list of non-hypervisor managed features). z13 is z13-base + all features 
that
will be available in a reasonably recent kernel+qemu combination and make sense
to be there a default. So it might happen that you cannot start -cpu z14, e.g. 
if you run on a kernel < 4.12.

 
>> FWIW, I think in your particular case the QEMU will reject the z14 cpu and 
>> not even come
>> to checking the gs. 
>>
> 
> I had a z13 cpu model in mind. I don't mention a z14 cpu-model (QEMU, not hw) 
> in
> my whole email.
> 
> Regards,
> Halil
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

[Christian Borntraeger]

On 10/27/2017 02:45 PM, Christian Borntraeger wrote:
> 
> 
> On 10/27/2017 02:31 PM, Halil Pasic wrote:
> gs is explicitly disabled.
>>
>> Now that I think about it, maybe the 2.9 binary is going to reject
>> the explicit gs flag altogether, because it's unknown.
>>
>> Isn't this a problem? 
> 
> No. This is exactly the _solution_ and not the problem. The target will reject
> unknown cpu features and migration will be aborted. This is exactly what the 
> CPU
> model is for.
FWIW, I think in your particular case the QEMU will reject the z14 cpu and not 
even come
to checking the gs. 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

[Christian Borntraeger]

On 10/27/2017 02:31 PM, Halil Pasic wrote:
gs is explicitly disabled.
> 
> Now that I think about it, maybe the 2.9 binary is going to reject
> the explicit gs flag altogether, because it's unknown.
> 
> Isn't this a problem? 

No. This is exactly the _solution_ and not the problem. The target will reject
unknown cpu features and migration will be aborted. This is exactly what the CPU
model is for.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

[Christian Borntraeger]

On 10/27/2017 02:31 PM, Halil Pasic wrote:
> 
> 
> On 10/25/2017 08:13 PM, Jason J. Herne wrote:
>> On 10/20/2017 10:54 AM, Christian Borntraeger wrote:
>>> Starting a guest with
>>>     
>>>  hvm
>>>    
>>>    
>>>
>>> on an IBM z14 results in
>>>
>>> "qemu-system-s390x: Some features requested in the CPU model are not
>>> available in the configuration: gs"
>>>
>>> This is because guarded storage is fenced for compat machines that did not 
>>> have
>>> guarded storage support, but libvirt expands the cpu model according to the
>>> latest available machine.
>>>
>>> While this prevents future migration abort (by not starting the guest at 
>>> all),
>>> not being able to start a "host-model" guest is very much unexpected.  As it
>>> turns out, even if we would modify libvirt to not expand the cpu model to
>>> contain "gs" for compat machines, it cannot guarantee that a migration will
>>> succeed. For example if the kernel changes its features (or the user has
>>> nested=1 on one host but not on the other) the migration will fail
>>> nevertheless.  So instead of fencing "gs" for machines <= 2.9 lets allow it 
>>> for
>>> all machine types that support the CPU model. This will make "host-model"
>>> runnable all the time, while relying on the CPU model to reject invalid
>>> migration attempts.
>> ...
>>> -    if (gs_allowed()) {
>>> +    if (cpu_model_allowed()) {
>>>   if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
>>>   cap_gs = 1;
> 
> 
> @Jason
> 
> Hi Jason,
> 
> I don't have access to a z14 at the moment, and since you do, I would
> like to try out something.
> 
> I will first describe my concern, and then the test scenario.
> 
> The last line above, cap_gs = 1, has the side effect of returning
> true ever after.
> 
> int kvm_s390_get_gs(void) 
>   
> { 
>   
> return cap_gs;
>   
> }  
> 
> Now considering
> static bool gscb_needed(void *opaque)
> {
> return kvm_s390_get_gs();
> }

Yes, we should also replace that with

 return s390_has_feat(S390_FEAT_GUARDED_STORAGE)

I can fixup my patch or provide a 2nd one.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

[Christian Borntraeger]

On 10/26/2017 01:35 AM, Halil Pasic wrote:
 try the most interesting scenarios out.
> 
> The idea of the patch is very clear, but I don't understand the bigger gs
> feature context fully.
> 
> From what I read in the code, the attempt to enable the gs capability in
> the kernel is made regardless of the cpu model. If the attempt was
> successful kvm_s390_get_gs will keep returning true. That would in turn
> affect migration, as far as I see (usages of kvm_s390_get_gs). I could
> not figure out how does gs being turned off via cpu-model (-cpu
> z14,gs=off) does turn of gs support -- at least not the details. I wanted
> to give a timely review, so I've limited myself there. 

When the CPU model turns off gs, facility bit 133 will be turned off.
Then the kernel does the right thing, see priv.c handle_gs.

guarded storage is enabled lazily. Whenever the guest issues a Gs instruction
we will get an exit and only enable GS if facility 133 is set.

> 
> From what I see, this patch does what it advertises, and since I think
> it's the right thing to do in the current situation I gonna give it an:
> Acked-by: Halil Pasic 
> 
> At the same time, I would prefer the commit message being reworded. IMHO
> this patch is a good stop-gap measure, but essentially it trades an
> annoying and obvious bug for a subtle and hopefully painless one.
> 
> Let me explain this last statement. For starters, I  do share some of the
> concerns Boris has voiced.  I won't repeat those. Same goes for the
> example Christian paraphrased previously, and the the fear of an implicit
> requirement for having to support a Cartesian product of the advertised
> machine-types and cpu-models (for each qemu binary).

I will try to come up with a patch description that explains the open issues
and it will tell that additional might or might not be necessary depending
on followup discussions.
I will schedule this patch for 2.11 then.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

[Christian Borntraeger]

Ping, I plan to submit belows patch for 2.11. We can then still look into
a libvirt<->qemu interface for limiting host-model depending on machine versions
(or not).

On 10/20/2017 04:54 PM, Christian Borntraeger wrote:
> Starting a guest with
>
> hvm
>   
>   
> 
> on an IBM z14 results in
> 
> "qemu-system-s390x: Some features requested in the CPU model are not
> available in the configuration: gs"
> 
> This is because guarded storage is fenced for compat machines that did not 
> have
> guarded storage support, but libvirt expands the cpu model according to the
> latest available machine.
> 
> While this prevents future migration abort (by not starting the guest at all),
> not being able to start a "host-model" guest is very much unexpected.  As it
> turns out, even if we would modify libvirt to not expand the cpu model to
> contain "gs" for compat machines, it cannot guarantee that a migration will
> succeed. For example if the kernel changes its features (or the user has
> nested=1 on one host but not on the other) the migration will fail
> nevertheless.  So instead of fencing "gs" for machines <= 2.9 lets allow it 
> for
> all machine types that support the CPU model. This will make "host-model"
> runnable all the time, while relying on the CPU model to reject invalid
> migration attempts.
> 
> Suggested-by: David Hildenbrand <da...@redhat.com>
> Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 8 
>  include/hw/s390x/s390-virtio-ccw.h | 3 ---
>  target/s390x/kvm.c | 2 +-
>  3 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index fabe4a6..ae5b01a 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -414,7 +414,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
> *data)
>  s390mc->ri_allowed = true;
>  s390mc->cpu_model_allowed = true;
>  s390mc->css_migration_enabled = true;
> -s390mc->gs_allowed = true;
>  mc->init = ccw_init;
>  mc->reset = s390_machine_reset;
>  mc->hot_add_cpu = s390_hot_add_cpu;
> @@ -495,12 +494,6 @@ bool cpu_model_allowed(void)
>  return get_machine_class()->cpu_model_allowed;
>  }
> 
> -bool gs_allowed(void)
> -{
> -/* for "none" machine this results in true */
> -return get_machine_class()->gs_allowed;
> -}
> -
>  static char *machine_get_loadparm(Object *obj, Error **errp)
>  {
>  S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> @@ -740,7 +733,6 @@ static void ccw_machine_2_9_class_options(MachineClass 
> *mc)
>  {
>  S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
> 
> -s390mc->gs_allowed = false;
>  ccw_machine_2_10_class_options(mc);
>  SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
>  s390mc->css_migration_enabled = false;
> diff --git a/include/hw/s390x/s390-virtio-ccw.h 
> b/include/hw/s390x/s390-virtio-ccw.h
> index a9a90c2..ac896e3 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -40,15 +40,12 @@ typedef struct S390CcwMachineClass {
>  bool ri_allowed;
>  bool cpu_model_allowed;
>  bool css_migration_enabled;
> -bool gs_allowed;
>  } S390CcwMachineClass;
> 
>  /* runtime-instrumentation allowed by the machine */
>  bool ri_allowed(void);
>  /* cpu model allowed by the machine */
>  bool cpu_model_allowed(void);
> -/* guarded-storage allowed by the machine */
> -bool gs_allowed(void);
> 
>  /**
>   * Returns true if (vmstate based) migration of the channel subsystem
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 4c85ed8..020a7ea 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -363,7 +363,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>  cap_ri = 1;
>  }
>  }
> -if (gs_allowed()) {
> +if (cpu_model_allowed()) {
>  if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
>  cap_gs = 1;
>  }
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

[Christian Borntraeger]

FWIW, this patch alone probably makes sense to fix this particular issue.

The question is do we want to have some additional interfaces between 
libvirt/qemu
that tell libvirt what CPUs/features are allowed for a specific machine version?

Halil just brough up an example. Imagine to have a QEMU binary that can emulate
an x86 and an s390x depending on the machine type. Should libvirt be able to 
detect
that -cpu pentium only makes sense for the q35 machines, but not for the 
s390-ccw-virtio
ones?




On 10/20/2017 04:54 PM, Christian Borntraeger wrote:
> Starting a guest with
>
> hvm
>   
>   
> 
> on an IBM z14 results in
> 
> "qemu-system-s390x: Some features requested in the CPU model are not
> available in the configuration: gs"
> 
> This is because guarded storage is fenced for compat machines that did not 
> have
> guarded storage support, but libvirt expands the cpu model according to the
> latest available machine.
> 
> While this prevents future migration abort (by not starting the guest at all),
> not being able to start a "host-model" guest is very much unexpected.  As it
> turns out, even if we would modify libvirt to not expand the cpu model to
> contain "gs" for compat machines, it cannot guarantee that a migration will
> succeed. For example if the kernel changes its features (or the user has
> nested=1 on one host but not on the other) the migration will fail
> nevertheless.  So instead of fencing "gs" for machines <= 2.9 lets allow it 
> for
> all machine types that support the CPU model. This will make "host-model"
> runnable all the time, while relying on the CPU model to reject invalid
> migration attempts.
> 
> Suggested-by: David Hildenbrand <da...@redhat.com>
> Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 8 
>  include/hw/s390x/s390-virtio-ccw.h | 3 ---
>  target/s390x/kvm.c | 2 +-
>  3 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index fabe4a6..ae5b01a 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -414,7 +414,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
> *data)
>  s390mc->ri_allowed = true;
>  s390mc->cpu_model_allowed = true;
>  s390mc->css_migration_enabled = true;
> -s390mc->gs_allowed = true;
>  mc->init = ccw_init;
>  mc->reset = s390_machine_reset;
>  mc->hot_add_cpu = s390_hot_add_cpu;
> @@ -495,12 +494,6 @@ bool cpu_model_allowed(void)
>  return get_machine_class()->cpu_model_allowed;
>  }
> 
> -bool gs_allowed(void)
> -{
> -/* for "none" machine this results in true */
> -return get_machine_class()->gs_allowed;
> -}
> -
>  static char *machine_get_loadparm(Object *obj, Error **errp)
>  {
>  S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> @@ -740,7 +733,6 @@ static void ccw_machine_2_9_class_options(MachineClass 
> *mc)
>  {
>  S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
> 
> -s390mc->gs_allowed = false;
>  ccw_machine_2_10_class_options(mc);
>  SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
>  s390mc->css_migration_enabled = false;
> diff --git a/include/hw/s390x/s390-virtio-ccw.h 
> b/include/hw/s390x/s390-virtio-ccw.h
> index a9a90c2..ac896e3 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -40,15 +40,12 @@ typedef struct S390CcwMachineClass {
>  bool ri_allowed;
>  bool cpu_model_allowed;
>  bool css_migration_enabled;
> -bool gs_allowed;
>  } S390CcwMachineClass;
> 
>  /* runtime-instrumentation allowed by the machine */
>  bool ri_allowed(void);
>  /* cpu model allowed by the machine */
>  bool cpu_model_allowed(void);
> -/* guarded-storage allowed by the machine */
> -bool gs_allowed(void);
> 
>  /**
>   * Returns true if (vmstate based) migration of the channel subsystem
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 4c85ed8..020a7ea 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -363,7 +363,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>  cap_ri = 1;
>  }
>  }
> -if (gs_allowed()) {
> +if (cpu_model_allowed()) {
>  if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
>  cap_gs = 1;
>  }
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

[Christian Borntraeger]

Starting a guest with
   
hvm
  
  

on an IBM z14 results in

"qemu-system-s390x: Some features requested in the CPU model are not
available in the configuration: gs"

This is because guarded storage is fenced for compat machines that did not have
guarded storage support, but libvirt expands the cpu model according to the
latest available machine.

While this prevents future migration abort (by not starting the guest at all),
not being able to start a "host-model" guest is very much unexpected.  As it
turns out, even if we would modify libvirt to not expand the cpu model to
contain "gs" for compat machines, it cannot guarantee that a migration will
succeed. For example if the kernel changes its features (or the user has
nested=1 on one host but not on the other) the migration will fail
nevertheless.  So instead of fencing "gs" for machines <= 2.9 lets allow it for
all machine types that support the CPU model. This will make "host-model"
runnable all the time, while relying on the CPU model to reject invalid
migration attempts.

Suggested-by: David Hildenbrand <da...@redhat.com>
Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 8 
 include/hw/s390x/s390-virtio-ccw.h | 3 ---
 target/s390x/kvm.c | 2 +-
 3 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index fabe4a6..ae5b01a 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -414,7 +414,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
*data)
 s390mc->ri_allowed = true;
 s390mc->cpu_model_allowed = true;
 s390mc->css_migration_enabled = true;
-s390mc->gs_allowed = true;
 mc->init = ccw_init;
 mc->reset = s390_machine_reset;
 mc->hot_add_cpu = s390_hot_add_cpu;
@@ -495,12 +494,6 @@ bool cpu_model_allowed(void)
 return get_machine_class()->cpu_model_allowed;
 }
 
-bool gs_allowed(void)
-{
-/* for "none" machine this results in true */
-return get_machine_class()->gs_allowed;
-}
-
 static char *machine_get_loadparm(Object *obj, Error **errp)
 {
 S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -740,7 +733,6 @@ static void ccw_machine_2_9_class_options(MachineClass *mc)
 {
 S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
 
-s390mc->gs_allowed = false;
 ccw_machine_2_10_class_options(mc);
 SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
 s390mc->css_migration_enabled = false;
diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index a9a90c2..ac896e3 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -40,15 +40,12 @@ typedef struct S390CcwMachineClass {
 bool ri_allowed;
 bool cpu_model_allowed;
 bool css_migration_enabled;
-bool gs_allowed;
 } S390CcwMachineClass;
 
 /* runtime-instrumentation allowed by the machine */
 bool ri_allowed(void);
 /* cpu model allowed by the machine */
 bool cpu_model_allowed(void);
-/* guarded-storage allowed by the machine */
-bool gs_allowed(void);
 
 /**
  * Returns true if (vmstate based) migration of the channel subsystem
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 4c85ed8..020a7ea 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -363,7 +363,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 cap_ri = 1;
 }
 }
-if (gs_allowed()) {
+if (cpu_model_allowed()) {
 if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
 cap_gs = 1;
 }
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Question about the host-model CPU mode

[Christian Borntraeger]

On 10/20/2017 04:06 PM, David Hildenbrand wrote:
> On 20.10.2017 16:02, Christian Borntraeger wrote:
>>
>>
>> On 10/20/2017 03:51 PM, David Hildenbrand wrote:
>> [...]
>>>> The problem goes much further.
>>>> A fresh guest with
>>>>
>>>> 
>>>>  hvm
>>>>
>>>>
>>>>
>>>> does not start. No migration from an older system is necessary.
>>>>
>>>
>>> Yes, as stated in the documentation "copying host CPU definition from
>>> capabilities XML" this can not work. And it works just as documented.
>>> Not saying this is a nice thing :)
>>>
>>> I think we should try to fix gs_allowed (if possible) and avoid
>>> something like that in the future. This would avoid other complexity
>>> involved when suddenly having X host models.
>>
>> Maybe this one is really a proper fix. It will allow the guest to start
>> and on migration the cpu model will complain if the target cannot provide gs.
>> Similar things can happen if - for example - the host kernel lacks some 
>> features.
> 
> Right, just what I had in mind.
> 
>>
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 77169bb..97a08fa 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -430,7 +430,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
>> *data)
>>  s390mc->ri_allowed = true;
>>  s390mc->cpu_model_allowed = true;
>>  s390mc->css_migration_enabled = true;
>> -s390mc->gs_allowed = true;
>>  mc->init = ccw_init;
>>  mc->reset = s390_machine_reset;
>>  mc->hot_add_cpu = s390_hot_add_cpu;
>> @@ -509,12 +508,6 @@ bool cpu_model_allowed(void)
>>  return get_machine_class()->cpu_model_allowed;
>>  }
>>  
>> -bool gs_allowed(void)
>> -{
>> -/* for "none" machine this results in true */
>> -return get_machine_class()->gs_allowed;
>> -}
>> -
>>  static char *machine_get_loadparm(Object *obj, Error **errp)
>>  {
>>  S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>> @@ -757,7 +750,6 @@ static void ccw_machine_2_9_class_options(MachineClass 
>> *mc)
>>  {
>>  S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
>>  
>> -s390mc->gs_allowed = false;
>>  ccw_machine_2_10_class_options(mc);
>>  SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
>>  s390mc->css_migration_enabled = false;
>> diff --git a/include/hw/s390x/s390-virtio-ccw.h 
>> b/include/hw/s390x/s390-virtio-ccw.h
>> index a9a90c2..1de53b0 100644
>> --- a/include/hw/s390x/s390-virtio-ccw.h
>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>> @@ -40,7 +40,6 @@ typedef struct S390CcwMachineClass {
>>  bool ri_allowed;
>>  bool cpu_model_allowed;
>>  bool css_migration_enabled;
>> -bool gs_allowed;
>>  } S390CcwMachineClass;
>>  
>>  /* runtime-instrumentation allowed by the machine */
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index a0d5052..3f13fc2 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -362,7 +362,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>  cap_ri = 1;
>>  }
>>  }
>> -if (gs_allowed()) {
>> +if (cpu_model_allowed()) {
>>  if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
>>  cap_gs = 1;
>>  }
>>
> 
> And the last hunk makes sure we keep same handling for machines without
> CPU model support and therefore no way to mask support. For all recent
> machines, we expect CPU model checks to be in place.
> 
> Will have to think about this a bit more. Will you send this as a proper
> patch?

After thinking about it :-)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Question about the host-model CPU mode

[Christian Borntraeger]

On 10/20/2017 03:51 PM, David Hildenbrand wrote:
[...]
>> The problem goes much further.
>> A fresh guest with
>>
>> 
>>  hvm
>>
>>
>>
>> does not start. No migration from an older system is necessary.
>>
> 
> Yes, as stated in the documentation "copying host CPU definition from
> capabilities XML" this can not work. And it works just as documented.
> Not saying this is a nice thing :)
> 
> I think we should try to fix gs_allowed (if possible) and avoid
> something like that in the future. This would avoid other complexity
> involved when suddenly having X host models.

Maybe this one is really a proper fix. It will allow the guest to start
and on migration the cpu model will complain if the target cannot provide gs.
Similar things can happen if - for example - the host kernel lacks some 
features.


diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 77169bb..97a08fa 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -430,7 +430,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
*data)
 s390mc->ri_allowed = true;
 s390mc->cpu_model_allowed = true;
 s390mc->css_migration_enabled = true;
-s390mc->gs_allowed = true;
 mc->init = ccw_init;
 mc->reset = s390_machine_reset;
 mc->hot_add_cpu = s390_hot_add_cpu;
@@ -509,12 +508,6 @@ bool cpu_model_allowed(void)
 return get_machine_class()->cpu_model_allowed;
 }
 
-bool gs_allowed(void)
-{
-/* for "none" machine this results in true */
-return get_machine_class()->gs_allowed;
-}
-
 static char *machine_get_loadparm(Object *obj, Error **errp)
 {
 S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -757,7 +750,6 @@ static void ccw_machine_2_9_class_options(MachineClass *mc)
 {
 S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
 
-s390mc->gs_allowed = false;
 ccw_machine_2_10_class_options(mc);
 SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
 s390mc->css_migration_enabled = false;
diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index a9a90c2..1de53b0 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -40,7 +40,6 @@ typedef struct S390CcwMachineClass {
 bool ri_allowed;
 bool cpu_model_allowed;
 bool css_migration_enabled;
-bool gs_allowed;
 } S390CcwMachineClass;
 
 /* runtime-instrumentation allowed by the machine */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a0d5052..3f13fc2 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -362,7 +362,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 cap_ri = 1;
 }
 }
-if (gs_allowed()) {
+if (cpu_model_allowed()) {
 if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
 cap_gs = 1;
 }

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Question about the host-model CPU mode

[Christian Borntraeger]

On 10/20/2017 03:43 PM, David Hildenbrand wrote:
> On 20.10.2017 15:36, Christian Borntraeger wrote:
>>
>>
>> On 10/20/2017 03:16 PM, David Hildenbrand wrote:
>>>
>>>> Hi all,
>>>>
>>>> we recently encountered the problem that the 'host-model' [1] has to be
>>>> related to the machine type of a domain. We have following problem:
>>>>
>>>>Let's assume we've a z13 system with a QEMU 2.9 and we define a
>>>>domain using the default s390-virtio-ccw machine together with the
>>>>host-model CPU mode [1]. The definition will have the machine
>>>>expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode
>>>>in the domain definition. In a next step we upgrade to QEMU 2.10
>>>>(first version to recognize z14). Everything is still fine, even
>>>>though the machine runs in 2.9 compatibility mode. Finally we
>>>>upgrade to a z14. As a consequence it is not possible to start the
>>>>domain anymore as the machine type doesn't support our CPU host
>>>>model (which is expanded at start time of the domain).
>>>
>>> Actually, what is the cause of that problem? I assume it is the gs
>>> feature (gs_allowed)?
>>>
>>> We should really avoid such things (..._allowed) for CPU model features
>>> in the future and clue all new such stuff to cpumodel_allowed.
>>
>> Yes, starting a guest with
>>
>> hvm
>>   
>>   
>>
>> results in
>>
>> qemu-system-s390x: Some features requested in the CPU model are not 
>> available in the configuration: gs 
>>
>> Tying it to cpumodel_allowed would not help, migration-wise though.
>> libvirt would still transform
>>
>>
>> hvm
>>   
>>   
> 
> My point was, that the host model would have to be copied and _remain_
> there when s390-ccw-virtio was expanded to s390-ccw-virtio-2.9.
> 
> So really replacing  by the model z13-base
> This would at least fix this issue. Just like s390-ccw-virtio get's
> replaced and remains thats way.
> 
> But this might for sure have other problems.

The problem goes much further.
A fresh guest with


 hvm
   
   

does not start. No migration from an older system is necessary.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Question about the host-model CPU mode

[Christian Borntraeger]

On 10/20/2017 03:16 PM, David Hildenbrand wrote:
> 
>> Hi all,
>>
>> we recently encountered the problem that the 'host-model' [1] has to be
>> related to the machine type of a domain. We have following problem:
>>
>>Let's assume we've a z13 system with a QEMU 2.9 and we define a
>>domain using the default s390-virtio-ccw machine together with the
>>host-model CPU mode [1]. The definition will have the machine
>>expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode
>>in the domain definition. In a next step we upgrade to QEMU 2.10
>>(first version to recognize z14). Everything is still fine, even
>>though the machine runs in 2.9 compatibility mode. Finally we
>>upgrade to a z14. As a consequence it is not possible to start the
>>domain anymore as the machine type doesn't support our CPU host
>>model (which is expanded at start time of the domain).
> 
> Actually, what is the cause of that problem? I assume it is the gs
> feature (gs_allowed)?
> 
> We should really avoid such things (..._allowed) for CPU model features
> in the future and clue all new such stuff to cpumodel_allowed.

Yes, starting a guest with
   
hvm
  
  

results in

qemu-system-s390x: Some features requested in the CPU model are not available 
in the configuration: gs 

Tying it to cpumodel_allowed would not help, migration-wise though.
libvirt would still transform

   
hvm
  
  


into 
-cpu 
z14-base,aen=on,cmmnt=on,aefsi=on,mepoch=on,msa8=on,msa7=on,msa6=on,msa5=on,msa4=on,
 \
msa3=on,msa2=on,msa1=on,sthyi=on,edat=on,ri=on,edat2=on,vx=on,ipter=on,vxeh=on,vxpd=on,
 \
esop=on,iep=on,cte=on,ais=on,gs=on,zpci=on,sea_esop2=on,te=on,cmm=on
 ^
because cpu model is certainly there. Now the guest would start but migration 
would
later fail because what we create now would never have been possible with 2.9.

If libvirt could get information from QEMU depending on the machine version, 
this would
make it work. But of course this has other issues.

Christian

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 1/3] conf : Add loadparm boot option for a boot device

[Christian Borntraeger]

On 05/19/2017 06:56 PM, Farhan Ali wrote:
> Update the per device boot schema to add an optional loadparm parameter.
> Extend the virDomainDeviceInfo to support loadparm option.
> Modify the appropriate functions to parse loadparm from boot device xml.
> 
> Signed-off-by: Farhan Ali 
> Reviewed-by: Bjoern Walk 
> Reviewed-by: Boris Fiuczynski 
> Reviewed-by: Marc Hartmayer 
> ---
>  docs/formatdomain.html.in |  8 +++--
>  docs/news.xml |  9 ++
>  docs/schemas/domaincommon.rng |  7 +
>  src/conf/device_conf.h|  1 +
>  src/conf/domain_conf.c| 69 
> +--
>  5 files changed, 90 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 3135db4..016febe 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3022,8 +3022,12 @@
>boot
>Specifies that the disk is bootable. The order
>  attribute determines the order in which devices will be tried during
> -boot sequence. The per-device boot elements cannot be
> -used together with general boot elements in
> +boot sequence. On S390 architecture only the first boot device is 
> used.
> +The optional loadparm attribute can be used to select a
> +boot entry for S390 architecture.
> +Since 3.4.0
> +The per-device boot elements cannot be used together
> +with general boot elements in
>  BIOS bootloader section.
>  Since 0.8.8
>
> diff --git a/docs/news.xml b/docs/news.xml
> index 52915ee..bc6b070 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -37,6 +37,15 @@
>  
>
>  
> +  qemu: Add support for loadparm for a boot device
> +
> +
> +  Add an optional boot parameter 'loadparm' for a boot device.
> +  On S390 architecture the loadparm is used to select a boot entry.

I would change that description and refer more to the architectural meaning
(8 byte parameter that can be queried by the guest via sclp and diag 308).
The linux use case of the selecting a boot entry withing that disk can be 
mentioned
but it certainly is not the only use case.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v1 0/3] Loadparm support

[Christian Borntraeger]

On 05/19/2017 07:34 PM, Daniel P. Berrange wrote:
> On Fri, May 19, 2017 at 12:56:44PM -0400, Farhan Ali wrote:
>> This patch series introduces the support for new s390x 'loadparm'
>> feature. The 'loadparm' can be used to select the boot entry to
>> boot from, for a boot device.
> 
> Why do we need / want any of this when we already have bootindex=NN
> support. Inventing a S390-only way to select the boot device order
> is pretty horrible IMHO

loadparm is an 8 byte character parameter for s390 systems and available
on all other hypervisors (LPAR/z/VM). It is really a part of the architecture.
Linux does use it to select a boot entry within a disk, but for other guest
operating systems it is used to pass along other kind of parameters for the 
ipl process, 

e.g. check  
https://www.ibm.com/support/knowledgecenter/zosbasics/com.ibm.zos.zsysprog/zsysprogc_systemIPL.htm
to see what z/OS uses.

Not having it was really a missing part of the s390 support.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Making panic great again

[Christian Borntraeger]

On 04/28/2017 02:34 AM, Ed Swierk wrote:
> The panic device is currently documented as a way for "libvirt to receive 
> panic notification from a QEMU guest".
> 
> This is true, but not the whole story. When a guest triggers the panic 
> device, QEMU pauses the guest, and libvirt takes the action specified by 
> on_crash. This can interfere with the guest's own crash handling actions 
> (e.g. writing a dump file and rebooting itself) if the guest triggers the 
> panic device first (as Windows does).
> 
> None of this is an obvious side effect of a notification mechanism, so the 
> panic device documentation should mention it. (I'll send a documentation 
> patch shortly.)
> 
> Nor is this a desirable side effect, for guests that are configured to deal 
> with crashes themselves. Sure, you can avoid using the panic device with such 
> guests, but then virsh list or another application using the libvirt API to 
> monitor domain state won't notice guest crashes. And if you still want 
> libvirt to take action on guests that don't do it themselves, then you have 
> to be careful to include the panic device only for those domains.
> 
> Ideally libvirt would offer (1) a state indicating "this guest crashed and 
> needs help" independently of triggering an action, and (2) a way to trigger 
> an action only when needed to recover from the crash, excluding guests that 
> deal with their own crashes.
> 
> Sadly pvpanic and the HyperV crash MSR convey only that the guest crashed, 
> not whether the guest is configured to take some action on its own. So 
> there's no way to know precisely that a crashed (and not paused) guest is in 
> need of assistance.
> 
> But a state indicating "this guest crashed N minutes ago and hasn't rebooted 
> itself" would be a useful approximation. And triggering an action N minutes 
> after a guest crash if it hasn't rebooted itself in the meantime would make 
> it easy to cap the downtime of crashed domains. Both could be implemented 
> without changing either QEMU or panic device semantics.
> 
> Does this seem useful to anyone else?


On s390 we only have a "pseudo" panic device.
Our guests load a disabled wait PSW to indicate a crash. This is wired up in 
QEMU as
panic state and thus notifies libvirt that the guest is in crashed state. If 
the guest
does kdump or similar it will never load a disabled wait PSW. So from my 
perspective
this works exactly as I like to it to behave, but I find it interesting that
others seem to trigger the panic device even if the guest handles that.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Release of libvirt-3.0.0

[Christian Borntraeger]

On 01/17/2017 07:40 PM, Daniel Veillard wrote:
>   So I got mixed reports in the last day about the state of the head
> but one of the big issues seems solved, and I'm not sure keeping the
> freeze much longer will help, so libvirt-3.0.0 is out. It's tagged in
> git, signed tarball and rpms are available at thet usual place:

Very unfortunate.
Can we please spin a 3.0.1 with namespaces disabled soon?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Availability of libvirt-3.0.0 release candidate 2

[Christian Borntraeger]

On 01/17/2017 05:39 PM, Daniel P. Berrange wrote:

> I'm thinking we've hit the limit of what we should try to force into the
> 3.0.0 release.
> 
> My vote at this poiint is to change the code so that namespaces are
> disabled out of the box, and do a 3.0.0 release. Look at fixing the

Ack.

> bugs to turn it back on by default in 3.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V3 2/2] qemu: Allow use of hot plugged host CPUs if no affinity set

[Christian Borntraeger]

On 11/25/2016 02:57 PM, Viktor Mihajlovski wrote:
> If the cpuset cgroup controller is disabled in /etc/libvirt/qemu.conf
> QEMU virtual machines can in principle use all host CPUs, even if they
> are hot plugged, if they have no explicit CPU affinity defined.
> 
> However, there's libvirt code supposed to handle the situation where
> the libvirt daemon itself is not using all host CPUs. The code in
> qemuProcessInitCpuAffinity attempts to set an affinity mask including
> all defined host CPUs. Unfortunately, the resulting affinity mask for
> the process will not contain the offline CPUs. See also the
> sched_setaffinity(2) man page.
> 
> That means that even if the host CPUs come online again, they won't be
> used by the QEMU process anymore. The same is true for newly hot
> plugged CPUs. So we are effectively preventing that QEMU uses all
> processors instead of enabling it to use them.
> 
> It only makes sense to set the QEMU process affinity if we're able
> to actually grow the set of usable CPUs, i.e. if the process affinity
> is a subset of the online host CPUs.
> 
> There's still the chance that for some reason the deliberately chosen
> libvirtd affinity matches the online host CPU mask by accident. In this
> case the behavior remains as it was before (CPUs offline while setting
> the affinity will not be used if they show up later on).


FWIW, newly started QEMU do indeed use new CPUs, only the ones that are 
currently running have this bug. So this fix does not only allow hotplug
it also fixes an imbalance for running/new QEMUs.

So I think it makes a lot of sense to also allow running QEMUs to use new
CPUs.

Acked-by: Christian Borntraeger <borntrae...@de.ibm.com>

> 
> Signed-off-by: Viktor Mihajlovski <mihaj...@linux.vnet.ibm.com>
> Tested-by: Matthew Rosato <mjros...@linux.vnet.ibm.com>




> ---
>  src/qemu/qemu_process.c | 33 -
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4758c49..9d1bfa4 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2202,6 +2202,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
>  int ret = -1;
>  virBitmapPtr cpumap = NULL;
>  virBitmapPtr cpumapToSet = NULL;
> +virBitmapPtr hostcpumap = NULL;
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> 
>  if (!vm->pid) {
> @@ -2223,21 +2224,34 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
>   * the spawned QEMU instance to all pCPUs if no map is given in
>   * its config file */
>  int hostcpus;
> +cpumap = virProcessGetAffinity(vm->pid);
> 
> -/* setaffinity fails if you set bits for CPUs which
> - * aren't present, so we have to limit ourselves */
> -if ((hostcpus = virHostCPUGetCount()) < 0)
> +if (virHostCPUHasBitmap())
> +hostcpumap = virHostCPUGetOnlineBitmap();
> +
> +if (hostcpumap && cpumap && virBitmapEqual(hostcpumap, cpumap)) {
> +/* we're using all available CPUs, no reason to set
> + * mask. If libvirtd is running without explicit
> + * affinity, we can use hotplugged CPUs for this VM */
> +ret = 0;
>  goto cleanup;
> +} else {
> +/* setaffinity fails if you set bits for CPUs which
> + * aren't present, so we have to limit ourselves */
> +if ((hostcpus = virHostCPUGetCount()) < 0)
> +goto cleanup;
> 
> -if (hostcpus > QEMUD_CPUMASK_LEN)
> -hostcpus = QEMUD_CPUMASK_LEN;
> +if (hostcpus > QEMUD_CPUMASK_LEN)
> +hostcpus = QEMUD_CPUMASK_LEN;
> 
> -if (!(cpumap = virBitmapNew(hostcpus)))
> -goto cleanup;
> +virBitmapFree(cpumap);
> +if (!(cpumap = virBitmapNew(hostcpus)))
> +goto cleanup;
> 
> -virBitmapSetAll(cpumap);
> +virBitmapSetAll(cpumap);
> 
> -cpumapToSet = cpumap;
> +cpumapToSet = cpumap;
> +}
>  }
>  }
> 
> @@ -2248,6 +2262,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
> 
>   cleanup:
>  virBitmapFree(cpumap);
> +virBitmapFree(hostcpumap);
>  return ret;
>  }
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC 0/5] Qemu: s390: Cpu Model Support

[Christian Borntraeger]

On 11/03/2016 02:37 PM, Jiri Denemark wrote:
> On Wed, Nov 02, 2016 at 16:34:30 -0400, Jason J. Herne wrote:
> ...
>> Patch #5 updates qemuBuildCpuModelArgStr's VIR_CPU_MODE_HOST_MODEL case. It
>> seems like all archs except PPC64 do not support host model mode.
> 
> x86 supports host-model too, but ppc64 (mis)used the host-model mode in
> a very specific way.
> 
>> Here is a list of outstanding things to do and some questions.
>>
>> 1. virsh domcapabilities --emulatorbin flag
>>
>> This works today if the command is passed --emulatorbin /usr/bin/qemu-kvm.
>> If no emulatorbin is given then Libvirt seems to choose
>> /usr/bin/qemu-system-s390x my system which does not have kvm enabled.
> 
> Does s390x still use the abandoned qemu-kvm name or is it just the
> stupid shell wrapper which calls the real binary with -machine
> accel=kvm? You say /usr/bin/qemu-system-s390x does not have kvm enabled,
> does it mean you cannot use kvm with this binary at all or you need to
> explicitly enable it with the -machine accel=kvm option?

Its the latter. qemu-system-s390x has tcg and kvm and requires accel=kvm to 
return
useful data

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC 0/5] Qemu: s390: Cpu Model Support

[Christian Borntraeger]

On 11/03/2016 02:37 PM, Jiri Denemark wrote:
> Well, this should be correct, or is QEMU on s390 able to use -cpu host
> in TCG mode?



> 
> Does "virsh domcapabilities --virttype kvm" report host-passthrough as
> supported?

yes, this is what I get 


  /usr/bin/qemu-system-s390x
  kvm
  s390-ccw-virtio-2.8
  s390x
  
  

  
rom
pflash
  
  
yes
no
  

  
  


  host


  z10EC
  z13s
  z13
  zEC12-base
  z890.3
  z890.2
  z196.2-base
  z890
  z990-base
  z900.2-base
  z114
  z13s-base
  z196-base
  z900
  z9BC
  z990.5-base
  zEC12.2
  zEC12.2-base
  zBC12-base
  z990
  z10BC-base
  z196.2
  z890.3-base
  z13.2
  z10EC.3
  z10EC.2
  z10BC.2
  host
  z9EC.3-base
  zEC12
  zBC12
  z10EC.3-base
  qemu
  z900.3-base
  z9BC-base
  z800-base
  z890-base
  z800
  z990.5
  z990.4
  z9EC.2-base
  z990.3
  z990.2
  z900.3
  z900.2
  z10EC-base
  z10EC.2-base
  z990.3-base
  z900-base
  z9BC.2-base
  z114-base
  z10BC.2-base
  z13.2-base
  z990.2-base
  z9EC-base
  z10BC
  z9EC
  z9EC.3
  z9EC.2
  z9BC.2
  z13-base
  z990.4-base
  z890.2-base
  z196

  
  

  
disk
cdrom
floppy
lun
  
  
ide
fdc
scsi
virtio
  


  
sdl
vnc
  


  
qxl
virtio
  


  
subsystem
  
  
default
mandatory
requisite
optional
  
  
usb
pci
scsi
  
  
  

  
  

  



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

[Christian Borntraeger]

On 10/07/2016 12:43 PM, Jiri Denemark wrote:
> GCC on s390 complains
> 
> util/virconf.c: In function 'virConfGetValueSizeT':
> util/virconf.c:1220:9: error: format '%zu' expects argument of type
> 'size_t', but argument 9 has type 'unsigned int' [-Werror=format=]

Interesting, we have never seen this. is this 31bit (s390) or 64bit(s390x)?
What gcc? Can you maybe provide the virconf.i file to see how the SIZE_MAX 
define
was resolved?

Christian

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] pvpanic plans?

[Christian Borntraeger]

On 22/08/13 20:33, Anthony Liguori wrote:
 Laszlo Ersek ler...@redhat.com writes:
 
 On 08/22/13 18:10, Paolo Bonzini wrote:
 The thread from yesterday has died off (perhaps also because of
 my inappropriate answer to Michael, for which I apologize to him
 and everyone).  I took some time to discuss the libvirt requirements
 further with Daniel Berrange and Eric Blake on IRC.  If anyone is
 interested, I can give logs.  This is a suggestion for how to
 proceed in both QEMU and libvirt.

 The analysis is pretty overwhelming :)

 I have read Anthony's response and I'm not trying to argue -- I'm just
 spending a few thoughts on this and I'm willing to let them go to waste.

 In general I think we should minimize the quirks the user (who edits the
 libvirt XML) has to know about. Interactions between some XML elements,
 without explicit inter-references (formulated as attributes, like
 controller/index) are Bad (TM). So,

 == Builtin pvpanic ==

 QEMU will remove pvpanic from pc-1.5 in 1.6.1 and 1.5.4.  This does not
 break migration.


 == Support in libvirt for current functionality ==

 libvirt will add a panic-notifier/ element, and possibly a capability
 for it accessible via virsh capabilities.  There are two possibilities:

 1) On QEMU 1.5.4/1.6.1 and newer (and on QEMU 1.6.0 with a machine type
other than pc-1.5), on_crash will only work if the element is there.
On QEMU 1.5.0-1.5.3, and on QEMU 1.6.0 with the pc-1.5 machine type,
on_crash will be obeyed always, and may override e.g. reboot-on-panic
if a guest driver exist.

 I don't like this because there's some interplay between on_crash and
 panic_notifier, which even depends on the qemu version.



 2) On all versions, on_crash will only work if the element is there.

 I like this, because, if on_crash doesn't work without panic_notifier
 *at all*, then we can just drop panic_notifier, and make on_crash mean
 (on_crash  panic_notifier) in the original sense.

 IOW, drop panic_notifier, and make on_crash work *always*.



 In turn, there are two ways to implement (2):

 2a) libvirt will always add -global pvpanic.iobase=0 to neutralize
 the builtin pvpanic device if present.  panic-notifier/
 will create the device with -device pvpanic,iobase=0x505

 Advantage: no changes to QEMU

 Disadvantage 1: writes to port 0 with QEMU 1.{5.0,5.1,5.2,5.3,6.0}
   and pc-1.5 machine type will write to a pvpanic device instead of
   the DMA controller.  Probably harmless, and limited to some QEMU
   versions.

 Disadvantage 2: libvirt has knowledge of the pvpanic port number

 Updating this paragraph with my above suggestion:

 - (s/pvpanic.iobase/pvpanic.ioport/g)

 - if on_crash is absent:
   - for 1.{5.0,5.1,5.2,5.3,6.0}, add -global pvpanic.ioport=0
   - for other versions, do nothing

 - if on_crash is present:
   - for 1.{5.0,5.1,5.2,5.3,6.0}, do nothing,
   - for other versions, pass -device pvpanic
 (knowledge of 0x505 is unneeded)
 
 Just to further complicate things...
 
 pvpanic is not going to be present on S390, PPC, ARM, or MIPS because of
 the fact that it's x86 specific.

What about crashed state? I have implemented this state after the guest entered
disabled wait (by convention used by all s390 OSes for crashes)

See commit 08eb8c85e3967b97865d46acadf26dc908fbb094
Author: Christian Borntraeger borntrae...@de.ibm.com
Date:   Fri Apr 26 11:24:47 2013 +0800

Wire up disabled wait a panicked event on s390



Should we remove that as well? From s390 point of view, after allowing
the crashed-running transition the feature would probably work as expected.


Christian

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [qemu-devel] Default machine type setting for ppc64

[Christian Borntraeger]

On 21/05/13 14:04, Anthony Liguori wrote:
 Daniel P. Berrange berra...@redhat.com writes:
 
 On Tue, May 21, 2013 at 07:55:27PM +1000, Paul Mackerras wrote:
 On Tue, May 21, 2013 at 09:39:53AM +0100, Daniel P. Berrange wrote:
 QEMU has the notion of a default machine for each target, and that is
 what libvirt uses if the user hasn't specified a machine.  It is not
 libvirt's job to override QEMU's notion of the default machine here,
 so if the 'mac99' machine type isn't suitable as the default either
 QEMU needs to change that for the ppc target, or the user needs to
 explicitly specify their desired machine type.

 We are getting the default changed to 'pseries', at least for cases
 where pseries support is compiled in, which isn't necessarily
 always.  That will of course not satisfy the Freescale guys.

 I think libvirt needs some more sensible way to ask qemu what its
 capabilities are.  Currently it has no way to ask qemu what machines
 can you emulate with kvm acceleration?  If the user has asked for a
 KVM domain then the default machine should be one that can be provided
 by KVM.  At present it isn't, on PowerPC.

 If QEMU can provide more intelligent info in this respect, then
 libvirt can use it. We're doing the best we can with picking
 defaults given the info QEMU currently provides us.
 
 We've talked in the past about having an accelerator specific machine
 default.  I think this is a perfectly reasonable thing to do and would
 solve the problem for ARM and for PPC.

If we get such thing, then virtio-ccw might also be the right default for kvm 
on s390.

Christian

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list