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

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