[Top][All Lists]

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

Re: [Qemu-devel] [PATCH 7/8] [PATCH RFC v2] s390-qemu: cpu hotplug - Inf

From: Jason J. Herne
Subject: Re: [Qemu-devel] [PATCH 7/8] [PATCH RFC v2] s390-qemu: cpu hotplug - Infrastructure for Cpu Devices
Date: Mon, 10 Jun 2013 12:00:52 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6

On 06/08/2013 06:50 PM, Andreas Färber wrote:

Am 07.06.2013 19:28, schrieb Jason J. Herne:
From: "Jason J. Herne" <address@hidden>

Add infrastructure for treating cpus as devices. This patch allows cpus to be
specified using a combination of '-smp' and '-device cpu'.  This approach
forces a change in the way cpus are counted via smp_cpus.

Signed-off-by: Jason J. Herne <address@hidden>
  include/hw/boards.h |    3 ++
  vl.c                |   95 +++++++++++++++++++++++++++++++++++++++++++--------
  2 files changed, 84 insertions(+), 14 deletions(-)

This feels like an ugly hack. And nack to CPUS_ARE_DEVICES(), since all
CPUs are devices already.

Could you please give a bit more rationale for each change? What's the
intended use case here? This is not just an s390 change but a design
question, so again please CC me as CPU maintainer.

As per upstream request from the last S390 cpu hotplug submission, the end goal here is cpu hotplug capability using the QOM infrastructure. This means no new "cpu_add" command. Instead make hotplug work through device_add. All cpu creation/initialization details should be handled through QOM object construction routines.

There's a number of questions that your series touches on but doesn't
discuss the concept in the cover letter or in the commit messages:

1) Mixing -smp N and -device s390-cpu

I would expect to get N+1 CPUs. Do you agree or disagree?

I agree. This patch has the desired outcome here.

-smp 0 is probably not well tested, if at all, so why specify -device
s390-cpu on the command line at all? Of course if there's bugs I'll be
happy to accept fixes, but I'm seeing device_add as more relevant than
-device honestly.

I agree here as well. but considering the parsing of the command line -device and the device_add qmp/hmp command are common I see no easy way of enabling hotpluging via device_add bit rejecting the -device command line form. I suppose we could explicitly check for "-device s390-cpu" in main() and fail with an error message. But then we need to add a new one for each architecture?

2) QEMUMachine::max_cpus

I believe -device s390-cpu should honor the limit, do you agree?
If so, then there's no need to iterate over -device options, because
that'll miss hot-added devices, but instead move any checks to the CPU's
realize function. If a user creates more CPUs than
qom/cpu.c:cpu_common_realizefn() should be made to fail. Note that my
upcoming CPUState series moves qemu_init_vcpu() there, so we will be
able to bail out before creating vCPU threads etc. for that CPU.

I agree. -device should honor max_cpus. This patch does exactly that. The check during a cpu hotplug happens in qdev_device_add. I was not aware that this could be done in qom/cpu.c:cpu_common_realizefn(). Sounds like a good place to put it if we can effectively use that one check to remove both the vl.c check and the qdev_device_add. I'll investigate that.

3) vCPU initialization hooks

We already have a QEMUMachine::hot_add_cpu hook. If you need additional
hooks, I'd like to first understand why that cannot go into S390CPU's
realize function, and then we can think about a more generic solution
like adding a Notifier that anyone can use.

You are again talking about the new post_cpu_init() hook I added? If so, I've addressed that concern in my reply to your last set of comments. If not, can you please elaborate?

The hot_add_cpu hook is called by qmp_cpu_add. I would need to call it from qmp_device_add. In which case, I still have no indication if we're truly hot adding or if we're still in pre-boot guest initialization. Is there a way to tell?

I guess we should step back and look at the "end goal" here. I was attempting to make the cpu initialization and hot plug as similar as possible. It seems as though that is required if we're going to be using the same basic code paths (through QOM object creation routines) for both cases. Am I way off track here? Should this not be my goal?


-- Jason J. Herne (address@hidden)

reply via email to

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