Re: [PATCH] Fix minor details in apic eoi

[Peter Krempa <pkrempa@xxxxxxxxxx>]

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list