1:45 a.m.
On 09/13/2012 04:52 PM, Michal Privoznik wrote:
On 13.09.2012 16:12, Martin Kletzander wrote:
> New options is added to support EOI (End of Interrupt) exposure for
> guests. As it makes sense only when APIC is enabled, I added this into
> the <apic> element in <features> because this should be tri-state
> option (cannot be handled as standalone feature).
> ---
> docs/formatdomain.html.in | 7 +++++++
> docs/schemas/domaincommon.rng | 9 ++++++++-
> src/conf/domain_conf.c | 35 ++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 11 +++++++++++
> src/libvirt_private.syms | 2 ++
> 5 files changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 503685f..66319d0 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1018,6 +1018,13 @@
> <dd>ACPI is useful for power management, for example, with
> KVM guests it is required for graceful shutdown to work.
> </dd>
> + <dt><code>apic</code></dt>
> + <dd>APIC allows the use of programmable IRQ
> + management. <span class="since">Since 0.10.2 (QEMU
only)</span>
> + there is an optional attribute <code>eoi</code> with values
"on"
> + and "off" which toggle the availability of EOI (End of
s/toggle/toggles/
> + Interrupt) for the guest.
> + </dd>
> <dt><code>hap</code></dt>
> <dd>Enable use of Hardware Assisted Paging if available in
> the hardware.
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 292cc9a..89c08da 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -110,6 +110,11 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
> "viridian",
> "privnet")
>
> +VIR_ENUM_IMPL(virDomainApicEoi, VIR_DOMAIN_APIC_EOI_LAST,
> + "default",
> + "on",
> + "off")
> +
> VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST,
> "destroy",
> "restart",
> @@ -8621,6 +8626,28 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
> goto error;
> }
> def->features |= (1 << val);
> + if (val == VIR_DOMAIN_FEATURE_APIC) {
> + char *attrstr = NULL;
> + if (virAsprintf(&attrstr,
> + "string(./features/%s/@eoi)",
> + nodes[i]->name) < 0)
> + goto no_memory;
> +
Why do we want runtime constructed string esp. if it is static one?
> + tmp = virXPathString(attrstr, ctxt);
> + if (tmp) {
> + int eoi;
> + if ((eoi = virDomainApicEoiTypeFromString(tmp)) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown value for attribute eoi:
%s"),
> + nodes[i]->name);
s/nodes[i]->name/tmp/
> + VIR_FREE(tmp);
> + goto error;
> + }
> + def->apic_eoi = eoi;
> + VIR_FREE(tmp);
> + }
> + VIR_FREE(attrstr);
> + }
> }
> VIR_FREE(nodes);
> }
ACK with this squashed in:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d13f671..16afb15 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8846,26 +8846,19 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
}
def->features |= (1 << val);
if (val == VIR_DOMAIN_FEATURE_APIC) {
- char *attrstr = NULL;
- if (virAsprintf(&attrstr,
- "string(./features/%s/@eoi)",
- nodes[i]->name) < 0)
- goto no_memory;
-
- tmp = virXPathString(attrstr, ctxt);
+ tmp = virXPathString("string(./features/apic/@eoi)", ctxt);
if (tmp) {
int eoi;
- if ((eoi = virDomainApicEoiTypeFromString(tmp)) < 0) {
+ if ((eoi = virDomainApicEoiTypeFromString(tmp)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown value for attribute eoi:
%s"),
- nodes[i]->name);
+ tmp);
VIR_FREE(tmp);
goto error;
}
def->apic_eoi = eoi;
VIR_FREE(tmp);
}
- VIR_FREE(attrstr);
}
}
VIR_FREE(nodes);
Michal
Thanks guys, fushed with the fixes,
Martin