[libvirt] [PATCH] Fix minor details in apic eoi

[Martin Kletzander]

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