[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/8] [PATCH RFC v3] s390 cpu hotplug

From: Andreas Färber
Subject: Re: [Qemu-devel] [PATCH 0/8] [PATCH RFC v3] s390 cpu hotplug
Date: Thu, 05 Sep 2013 16:06:00 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130801 Thunderbird/17.0.8

Am 05.09.2013 15:10, schrieb Alexander Graf:
> On 05.09.2013, at 15:05, Andreas Färber wrote:
>> Am 05.09.2013 14:54, schrieb Alexander Graf:
>>> Very simple and clean patch set. I don't think it deserves the RFC tag.
>> Negative, see my review. If you want to fix up and queue patches 1-2
>> that's fine with me, but the others need a respin. No major blocker
>> though, just some more footwork mostly related to QOM and Jason's
>> shifted focus on cpu-add rather than device_add.
> Yeah, that's what I'm referring to. I've seen a lot worse patch sets at v8 
> than this RFC :).
> I don't think we should apply it as is, and I'm very happy to see your review 
> and comment on the modeling bits :). But I try to never apply or cherry pick 
> RFC patches - and this set looks like he sent it with the intent of getting 
> it merged.

Agreed, we can continue with "PATCH v4". I was more upset about the
"very simple and clean" bit after I commented on a number of unclean
things to improve - mostly about doing things in different places.

If you could find some time to review my two model string patches then I
could supply Jason with a branch or even a pull to base on:


I would also volunteer to provide a base patch for the link<> issue if
there is agreement. Apart from the QOM API question this depends on the
contradictory modelling of whether we allow CPU addresses 0..max_cpus as
seen in this series or 0..somemax with <= max_cpus non-NULL as discussed
on #zkvm.
(child<s390-cpu> properties would allow to model the latter sparse
address space very well, but an object can only have one parent in the
hot-add case. We could of course add cpu[n] link<s390-cpu> properties as
CPUs get added, but that doesn't strike me as very clean. My underlying
thought is to offload the error handling to QOM so that we don't start
hardcoding s/smp_cpus/max_cpus/g (or some max_cpu_address) all around

Btw an unanswered question: ipi_states is just pointers to CPUs
currently, no further state. So what's "ipi" in the name? Will that
array need to carry state beyond S390CPU someday?


SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

reply via email to

[Prev in Thread] Current Thread [Next in Thread]