We are still hoping all of such checks will be moved there and this is one small
step in that direction.
One of the things that this is improving is the fact that instead of error
message *that was wrong) you get when starting a domain with SMM and i440fx we
allow the setting to go through. SMM option exists and makes sense on i440fx as
well (basically whenever that _SMM_OPT capability is set).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
src/qemu/qemu_capabilities.c | 11 -----------
src/qemu/qemu_capabilities.h | 3 ---
src/qemu/qemu_command.c | 12 ++----------
src/qemu/qemu_domain.c | 15 ++++++++++++---
4 files changed, 14 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 52178f0b0d86..5f48042ac752 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4773,17 +4773,6 @@ virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
}
-bool
-virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps,
- const virDomainDef *def)
-{
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT))
- return false;
-
- return qemuDomainIsQ35(def);
-}
-
-
bool
virQEMUCapsIsMachineSupported(virQEMUCapsPtr qemuCaps,
const char *canonical_machine)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index aad8f398caed..7e602049ca71 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -505,9 +505,6 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps,
bool virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
const virDomainDef *def);
-bool virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps,
- const virDomainDef *def);
-
char *virQEMUCapsFlagsString(virQEMUCapsPtr qemuCaps);
const char *virQEMUCapsGetBinary(virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5190a88ad3ff..6bc9bf5ffab8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7136,16 +7136,8 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
virTristateSwitchTypeToString(vmport));
}
- if (smm) {
- if (!virQEMUCapsSupportsSMM(qemuCaps, def)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("smm is not available with this QEMU binary"));
- goto cleanup;
- }
-
- virBufferAsprintf(&buf, ",smm=%s",
- virTristateSwitchTypeToString(smm));
- }
+ if (smm)
+ virBufferAsprintf(&buf, ",smm=%s", virTristateSwitchTypeToString(smm));
if (def->mem.dump_core) {
virBufferAsprintf(&buf, ",dump-guest-core=%s",
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c5237e4d418d..a2c4d3a36090 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3743,7 +3743,8 @@ qemuDomainDefGetVcpuHotplugGranularity(const virDomainDef *def)
static int
-qemuDomainDefValidateFeatures(const virDomainDef *def)
+qemuDomainDefValidateFeatures(const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
{
size_t i;
@@ -3790,6 +3791,15 @@ qemuDomainDefValidateFeatures(const virDomainDef *def)
}
break;
+ case VIR_DOMAIN_FEATURE_SMM:
+ if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("smm is not available with this QEMU binary"));
+ return -1;
+ }
+ break;
+
case VIR_DOMAIN_FEATURE_ACPI:
case VIR_DOMAIN_FEATURE_APIC:
case VIR_DOMAIN_FEATURE_PAE:
@@ -3802,7 +3812,6 @@ qemuDomainDefValidateFeatures(const virDomainDef *def)
case VIR_DOMAIN_FEATURE_CAPABILITIES:
case VIR_DOMAIN_FEATURE_PMU:
case VIR_DOMAIN_FEATURE_VMPORT:
- case VIR_DOMAIN_FEATURE_SMM:
case VIR_DOMAIN_FEATURE_VMCOREINFO:
case VIR_DOMAIN_FEATURE_LAST:
break;
@@ -3925,7 +3934,7 @@ qemuDomainDefValidate(const virDomainDef *def,
}
}
- if (qemuDomainDefValidateFeatures(def) < 0)
+ if (qemuDomainDefValidateFeatures(def, qemuCaps) < 0)
goto cleanup;
ret = 0;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
The commit summary speaks of moving checks, but the commit actually relaxes them. I would not expect functional changes from that message. How about: qemu: relax and move SMM checks to validation phase On Thu, Jun 07, 2018 at 10:37:40AM +0200, Martin Kletzander wrote: >We are still hoping all of such checks will be moved there and this is one small >step in that direction. > >One of the things that this is improving is the fact that instead of error >message *that was wrong) you get when starting a domain with SMM and i440fx we (that was wrong) >allow the setting to go through. SMM option exists and makes sense on i440fx as >well (basically whenever that _SMM_OPT capability is set). > >Signed-off-by: Martin Kletzander <mkletzan@redhat.com> >--- > src/qemu/qemu_capabilities.c | 11 ----------- > src/qemu/qemu_capabilities.h | 3 --- > src/qemu/qemu_command.c | 12 ++---------- > src/qemu/qemu_domain.c | 15 ++++++++++++--- > 4 files changed, 14 insertions(+), 27 deletions(-) > With the commit summary adjusted or changes split in two: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 07, 2018 at 09:31:50PM +0200, Ján Tomko wrote: >The commit summary speaks of moving checks, but the commit actually >relaxes them. I would not expect functional changes from that message. > >How about: >qemu: relax and move SMM checks to validation phase > Here you suggest changing the commit message to describe both changes. >On Thu, Jun 07, 2018 at 10:37:40AM +0200, Martin Kletzander wrote: >>We are still hoping all of such checks will be moved there and this is one small >>step in that direction. >> >>One of the things that this is improving is the fact that instead of error >>message *that was wrong) you get when starting a domain with SMM and i440fx we > >(that was wrong) > >>allow the setting to go through. SMM option exists and makes sense on i440fx as >>well (basically whenever that _SMM_OPT capability is set). >> >>Signed-off-by: Martin Kletzander <mkletzan@redhat.com> >>--- >> src/qemu/qemu_capabilities.c | 11 ----------- >> src/qemu/qemu_capabilities.h | 3 --- >> src/qemu/qemu_command.c | 12 ++---------- >> src/qemu/qemu_domain.c | 15 ++++++++++++--- >> 4 files changed, 14 insertions(+), 27 deletions(-) >> > >With the commit summary adjusted or changes split in two: > Here you are suggesting to split it into two. I'm fine with both, just tell me what you meant so I can fix it up, thanks. >Reviewed-by: Ján Tomko <jtomko@redhat.com> > >Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 07, 2018 at 11:04:48PM +0200, Martin Kletzander wrote: >On Thu, Jun 07, 2018 at 09:31:50PM +0200, Ján Tomko wrote: >>The commit summary speaks of moving checks, but the commit actually >>relaxes them. I would not expect functional changes from that message. >> >>How about: >>qemu: relax and move SMM checks to validation phase >> > >Here you suggest changing the commit message to describe both changes. > >>On Thu, Jun 07, 2018 at 10:37:40AM +0200, Martin Kletzander wrote: >>>We are still hoping all of such checks will be moved there and this is one small >>>step in that direction. >>> >>>One of the things that this is improving is the fact that instead of error >>>message *that was wrong) you get when starting a domain with SMM and i440fx we >> >>(that was wrong) >> >>>allow the setting to go through. SMM option exists and makes sense on i440fx as >>>well (basically whenever that _SMM_OPT capability is set). >>> >>>Signed-off-by: Martin Kletzander <mkletzan@redhat.com> >>>--- >>> src/qemu/qemu_capabilities.c | 11 ----------- >>> src/qemu/qemu_capabilities.h | 3 --- >>> src/qemu/qemu_command.c | 12 ++---------- >>> src/qemu/qemu_domain.c | 15 ++++++++++++--- >>> 4 files changed, 14 insertions(+), 27 deletions(-) >>> >> >>With the commit summary adjusted or changes split in two: >> > >Here you are suggesting to split it into two. I'm fine with both, just tell me >what you meant so I can fix it up, thanks. Summary adjusted OR changes split, IOW: don't leave functional changes in the commit named 'Move' Of course, the split is nicer. Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 07, 2018 at 11:09:41PM +0200, Ján Tomko wrote:
>On Thu, Jun 07, 2018 at 11:04:48PM +0200, Martin Kletzander wrote:
>>On Thu, Jun 07, 2018 at 09:31:50PM +0200, Ján Tomko wrote:
>>>The commit summary speaks of moving checks, but the commit actually
>>>relaxes them. I would not expect functional changes from that message.
>>>
>>>How about:
>>>qemu: relax and move SMM checks to validation phase
>>>
>>
>>Here you suggest changing the commit message to describe both changes.
>>
>>>On Thu, Jun 07, 2018 at 10:37:40AM +0200, Martin Kletzander wrote:
>>>>We are still hoping all of such checks will be moved there and this is one small
>>>>step in that direction.
>>>>
>>>>One of the things that this is improving is the fact that instead of error
>>>>message *that was wrong) you get when starting a domain with SMM and i440fx we
>>>
>>>(that was wrong)
>>>
>>>>allow the setting to go through. SMM option exists and makes sense on i440fx as
>>>>well (basically whenever that _SMM_OPT capability is set).
>>>>
>>>>Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>>>>---
>>>> src/qemu/qemu_capabilities.c | 11 -----------
>>>> src/qemu/qemu_capabilities.h | 3 ---
>>>> src/qemu/qemu_command.c | 12 ++----------
>>>> src/qemu/qemu_domain.c | 15 ++++++++++++---
>>>> 4 files changed, 14 insertions(+), 27 deletions(-)
>>>>
>>>
>>>With the commit summary adjusted or changes split in two:
^^ DUH!!!
>>>
>>
>>Here you are suggesting to split it into two. I'm fine with both, just tell me
>>what you meant so I can fix it up, thanks.
>
>Summary adjusted OR changes split, IOW: don't leave functional changes
>in the commit named 'Move'
>
>Of course, the split is nicer.
>
Of course, will do that.--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.