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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list