On 09/18/2012 04:33 PM, Peter Krempa wrote:
> On 09/18/12 16:01, Martin Kletzander wrote:
>> The introduction of APIC EOI patches had a few little details that
>> could look better, so this patch fixes that.
>>
>> The fixes:
>> - "on" and "off" as values are changed to <code>on</code> and
>> <code>off</code> respectively, because the code around uses the
>> same tags for such values.
>> - VIR_FREE is unnecessary as it is done in the error handling as well
>> - one empty line stayed in my local changes not included in the sent
>> patch, the code around is separated in similar fashion
>> - For const strings, virBufferAddLit should be used instead of
>> virBufferAsprintf.
>> ---
>> docs/formatdomain.html.in | 6 +++---
>> src/conf/domain_conf.c | 4 ++--
>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 8bdfbf1..51f897c 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -1020,9 +1020,9 @@
>> </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 toggles the availability of EOI (End of
>> + management. <span class="since">Since 0.10.2 (QEMU only)</span>
>> there is
>> + an optional attribute <code>eoi</code> with values <code>on</code>
>> + and <code>off</code> which toggles the availability of EOI (End of
>> Interrupt) for the guest.
>> </dd>
>> <dt><code>hap</code></dt>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index b8ba0e2..880ac17 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -8851,7 +8851,6 @@ static virDomainDefPtr
>> virDomainDefParseXML(virCapsPtr caps,
>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> _("unknown value for
>> attribute eoi: %s"),
>> tmp);
>> - VIR_FREE(tmp);
>
> I another piece of code with this problem:
>
> Squash this in before pushing:
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 880ac17..15b360a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8617,7 +8617,6 @@ static virDomainDefPtr
> virDomainDefParseXML(virCapsPtr caps,
> virReportError(VIR_ERR_XML_ERROR,
> _("Unsupported CPU placement mode '%s'"),
> tmp);
> - VIR_FREE(tmp);
> goto error;
> }
> VIR_FREE(tmp);
>
>
> On the other hand ... this is out of scope of this patch and not that
> important ... I don't know if it's even worth a separate patch.
>
>
>> goto error;
>> }
>> def->apic_eoi = eoi;
>> @@ -13433,6 +13432,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>> }
>>
>> virBufferAddLit(buf, " <os>\n");
>> +
>> virBufferAddLit(buf, " <type");
>> if (def->os.arch)
>> virBufferAsprintf(buf, " arch='%s'", def->os.arch);
>> @@ -13523,7 +13523,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>> " eoi='%s'",
>>
>> virDomainApicEoiTypeToString(def->apic_eoi));
>> }
>> - virBufferAsprintf(buf, "/>\n");
>> + virBufferAddLit(buf, "/>\n");
>> }
>> }
>> virBufferAddLit(buf, " </features>\n");
>> --
>
> ACK.
>
> Peter
>
I squashed that in and reworded the commit message in order to cover the
squashed part as well. Thanks.
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list