qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>,
	qemu-devel@nongnu.org, marcel@redhat.com, eblake@redhat.com,
	armbru@redhat.com, drjones@redhat.com, libvir-list@redhat.com
Subject: Re: [Qemu-devel] [RFC 12/42] pc: initialize legacy hotplug only for 2.6 and older machine types
Date: Thu, 12 May 2016 10:05:54 +0300	[thread overview]
Message-ID: <20160512100046-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <20160511233600.GJ4457@thinpad.lan.raisama.net>

On Wed, May 11, 2016 at 08:36:00PM -0300, Eduardo Habkost wrote:
> On Wed, May 11, 2016 at 03:50:39PM +0200, Igor Mammedov wrote:
> > On Tue, 10 May 2016 17:24:14 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> > > On Mon, May 02, 2016 at 02:33:21PM +0200, Igor Mammedov wrote:
> > > > on old machine types CPU hotplug was uncondtionally
> > > > enabled since it was introduced, consuming IO ports
> > > > and providing AML regardles of whether it was actually
> > > > in use or not. Keep it so for 2.6 and older machines.
> > > > 
> > > > New machine types will have an option to turn CPU
> > > > hotplug on if it's needed while by default it stays
> > > > disabled not consuming extra RAM/IO resources.
> > > > 
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>  
> > > 
> > > What if people are using "-machine pc -smp N,max_cpus=M"?
> > > Shouldn't we at least warning about missing CPU hotplug support
> > > when using just "max_cpus" with no "cpu-hotplug=on" with pc-2.7
> > > and newer?
> > Yep, I'll add it on next respin.
> > Would hard error better than warning?
> 
> Not sure. It would break older libvirt versions, wouldn't it?
> But: isn't the new legacy-cpu-hotplug=false default going to
> break old libvirt versions anyway? Should we?
> 
> (CCing libvirt list)

Even if not, we should not break scripts people use.  Maybe it was a
mistake to enable hotplug by default but we can't just remove it now
after all this time.  Why not have new hotplug on by default as well?
If you see value in ability to remove this feature, add an option for that.
 
> > 
> > > Should max_cpus > smp_cpus automatically set
> > > cpu-hotplug=on?
> > I'd prefer dumb explicit feature enablement,
> > as it doesn't put any assumptions on other options and
> > QEMU + mgmt don't have to maintain logic for implicit
> > rules that might enable it.
> > 
> > and if I didn't manage to push 'device_add x86cpu' in 2.7 time frame,
> > guess work gets a bit confusing with current cpu-add semantic,
> > consider current:
> > 
> >  SRC-QEMU -smp 1,maxcpus=2
> >    cpu-add 1
> >  DST-QEMU -smp 2,maxcpus=2
> > 
> > vs would be device_add:
> > 
> >  SRC-QEMU -smp 1,maxcpus=2
> >    device_add cpu
> >  DST-QEMU -smp 1,maxcpus=2 -device cpu
> > 
> > so instead of qemu/users guessing, I suggest to make it explictly
> > enabled to get feature (which is mostly optional) or
> > cleanly fail qemu start if confusing options are specified
> > with a clear error message.
> 
> Agreed we shouldn't encourage people to use the old option to get
> the new behavior.
> 
> But I am worried about breaking existing configurations on a
> machine-type change. Is it possible to avoid that?
> 
> -- 
> Eduardo

  reply	other threads:[~2016-05-12  7:06 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-02 12:33 [Qemu-devel] [RFC 00/42] ACPI CPU hotplug refactoring to support more than 255 CPUs and PXM/OST methods Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 01/42] acpi: add aml_debug() Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 02/42] acpi: add aml_refof() Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 03/42] pc: acpi: remove AML for empty/not used GPE handlers Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 04/42] pc: acpi: consolidate CPU hotplug AML Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 05/42] pc: acpi: consolidate \GPE._E02 with the rest of " Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 06/42] pc: acpi: cpu-hotplug: make AML CPU_foo defines local to cpu_hotplug_acpi_table.c Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 07/42] pc: acpi: mark current CPU hotplug functions as legacy Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 08/42] pc: acpi: consolidate legacy CPU hotplug in one file Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 09/42] pc: acpi: simplify build_legacy_cpu_hotplug_aml() signature Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 10/42] pc: piix4/ich9: add 'cpu-hotplug-legacy' property Igor Mammedov
2016-05-10 20:19   ` Eduardo Habkost
2016-05-11 13:33     ` Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 11/42] pc: add 2.7 machine Igor Mammedov
2016-05-10 20:20   ` Eduardo Habkost
2016-05-02 12:33 ` [Qemu-devel] [RFC 12/42] pc: initialize legacy hotplug only for 2.6 and older machine types Igor Mammedov
2016-05-10 20:24   ` Eduardo Habkost
2016-05-11 13:50     ` Igor Mammedov
2016-05-11 21:51       ` Michael S. Tsirkin
2016-05-11 23:28         ` Eduardo Habkost
2016-05-12  7:07           ` Michael S. Tsirkin
2016-05-12 10:29             ` Igor Mammedov
2016-05-12 10:52               ` Eduardo Habkost
2016-05-12 11:04                 ` [Qemu-devel] [libvirt] " Peter Krempa
2016-05-12 13:20                   ` Igor Mammedov
2016-05-12 13:27                     ` Peter Krempa
2016-05-12 14:30                       ` Igor Mammedov
2016-05-11 23:36       ` [Qemu-devel] " Eduardo Habkost
2016-05-12  7:05         ` Michael S. Tsirkin [this message]
2016-05-12 10:39           ` Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 13/42] tests: bios-tables-test: update tables with CPHP turned off by default Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 14/42] tests: acpi: check legacy CPU hotplug AML is present for 2.6 machine type Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 15/42] acpi: extend ACPI interface to provide send_event hook Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 16/42] pc: use AcpiDeviceIfClass.send_event to issue GPE events Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 17/42] docs: update ACPI CPU hotplug spec with new protocol Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 18/42] acpi: hardware side of CPU hotplug Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 19/42] pc: add generic CPU unplug callbacks Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 20/42] machine: add cpu-hotplug machine option Igor Mammedov
2016-05-10 20:27   ` Eduardo Habkost
2016-05-11 14:00     ` Igor Mammedov
2016-05-11 15:00       ` Eduardo Habkost
2016-05-02 12:33 ` [Qemu-devel] [RFC 21/42] pc: q35: initialize new CPU hotplug hw Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 22/42] pc: piix4: " Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 23/42] tests: pc-cpu-test: turn on cpu-hotplug explicily Igor Mammedov
2016-05-10 20:28   ` Eduardo Habkost
2016-05-02 12:33 ` [Qemu-devel] [RFC 24/42] pc: acpi: cpuhp-legacy: switch ProcessorID to possible_cpus idx Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 25/42] tests: acpi: update cphp_legacy case with new ProcessorID values Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 26/42] pc: acpi: introduce AcpiDeviceIfClass.madt_cpu hook Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 27/42] acpi: add CPU devices AML to DSDT Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 28/42] acpi: add CPU hotplug methods " Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 29/42] tests: acpi: update expected files with new CPU AML description Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 30/42] qdev: hotplug: Introduce HotplugHandler.pre_plug() callback Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 31/42] pc: numa: replace node_cpu indexing by apic_id with possible_cpus index Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 32/42] pc: set X86CPU.node property if QEMU starts with numa enabled Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 33/42] target-i386: add X86CPU.node property Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 34/42] acpi: cpuhp: add command and data registers Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 35/42] acpi: cpuhp: provide cpu._PXM method if running in numa mode Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 36/42] acpi: cpuhp: add cpu._OST handling Igor Mammedov
2016-05-10 21:07   ` Eric Blake
2016-05-11 14:02     ` Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 37/42] tests: acpi: report names of expected files in verbose mode Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 38/42] tests: acpi: add CPU hotplug testcase Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 39/42] tests: acpi: add expected tables for CPU hotplug case Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 40/42] tests: acpi: extend CPU hotplug test with NUMA support Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 41/42] tests: acpi: cpuhp: add expected SRAT tables and update DSDT with _PXM support Igor Mammedov
2016-05-02 12:33 ` [Qemu-devel] [RFC 42/42] DO NOT APPLY: simulate CPU unplug when executin cpu-add on a present CPU Igor Mammedov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160512100046-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=armbru@redhat.com \
    --cc=drjones@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=marcel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).