Re: [Qemu-devel] [PATCH v6 6/7] cpu: Add a last_cpu macro

From: "Andreas Färber" <afaerber@suse.de>
To: Matthew Rosato <mjrosato@linux.vnet.ibm.com>, qemu-devel@nongnu.org
Cc: dahi@linux.vnet.ibm.com, agraf@suse.de, borntraeger@de.ibm.com,
	imammedo@redhat.com, bharata@linux.vnet.ibm.com,
	cornelia.huck@de.ibm.com, pbonzini@redhat.com, rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH v6 6/7] cpu: Add a last_cpu macro
Date: Mon, 22 Feb 2016 18:30:03 +0100	[thread overview]
Message-ID: <56CB459B.5050605@suse.de> (raw)
In-Reply-To: <1456160797-832-7-git-send-email-mjrosato@linux.vnet.ibm.com>

Am 22.02.2016 um 18:06 schrieb Matthew Rosato:
> Add last_cpu to grab last CPU in the queue.  Rename one existing
> use of last_cpu as a variable name.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> ---
>  hw/intc/openpic.c | 12 ++++++------
>  include/qom/cpu.h |  1 +
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
> index 903888c..0dd908e 100644
> --- a/hw/intc/openpic.c
> +++ b/hw/intc/openpic.c
> @@ -217,7 +217,7 @@ typedef struct IRQSource {
>      uint32_t ivpr;  /* IRQ vector/priority register */
>      uint32_t idr;   /* IRQ destination register */
>      uint32_t destmask; /* bitmap of CPU destinations */
> -    int last_cpu;
> +    int cpu_last;

If we do need to rename this, what about last_cpu_index? cpu_last reads
really ugly.

>      int output;     /* IRQ level, e.g. OPENPIC_OUTPUT_INT */
>      int pending;    /* TRUE if IRQ is pending */
>      IRQType type;
> @@ -476,9 +476,9 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
>          return;
>      }
>  
> -    if (src->destmask == (1 << src->last_cpu)) {
> +    if (src->destmask == (1 << src->cpu_last)) {
>          /* Only one CPU is allowed to receive this IRQ */
> -        IRQ_local_pipe(opp, src->last_cpu, n_IRQ, active, was_active);
> +        IRQ_local_pipe(opp, src->cpu_last, n_IRQ, active, was_active);
>      } else if (!(src->ivpr & IVPR_MODE_MASK)) {
>          /* Directed delivery mode */
>          for (i = 0; i < opp->nb_cpus; i++) {
> @@ -488,13 +488,13 @@ static void openpic_update_irq(OpenPICState *opp, int n_IRQ)
>          }
>      } else {
>          /* Distributed delivery mode */
> -        for (i = src->last_cpu + 1; i != src->last_cpu; i++) {
> +        for (i = src->cpu_last + 1; i != src->cpu_last; i++) {
>              if (i == opp->nb_cpus) {
>                  i = 0;
>              }
>              if (src->destmask & (1 << i)) {
>                  IRQ_local_pipe(opp, i, n_IRQ, active, was_active);
> -                src->last_cpu = i;
> +                src->cpu_last = i;
>                  break;
>              }
>          }
> @@ -1444,7 +1444,7 @@ static const VMStateDescription vmstate_openpic_irqsource = {
>          VMSTATE_UINT32(ivpr, IRQSource),
>          VMSTATE_UINT32(idr, IRQSource),
>          VMSTATE_UINT32(destmask, IRQSource),
> -        VMSTATE_INT32(last_cpu, IRQSource),
> +        VMSTATE_INT32(cpu_last, IRQSource),

This name change shows up in the VMState description, e.g. in the JSON.
Migration will still work, but we should avoid it for debugging
cross-version migration.

>          VMSTATE_INT32(pending, IRQSource),
>          VMSTATE_END_OF_LIST()
>      }
[actual change snipped]

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)