On Thu, May 08, 2014 at 04:07:54PM +0100, Peter Maydell wrote: > On 8 May 2014 16:00, Eduardo Habkost <ehabk...@redhat.com> wrote: > > On Thu, May 08, 2014 at 09:26:36AM +0100, Peter Maydell wrote: > >> On 7 May 2014 23:20, Andreas Färber <afaer...@suse.de> wrote: > >> > Am 07.05.2014 23:48, schrieb Peter Maydell: > >> >> This is confusing. What is max_cpus > >> > > >> > Specified via -smp, defaults to actual CPUs. > >> > >> I thought that was smp_cpus ... > > > > smp_cpus is the first argument to -smp. max_cpus is the "maxcpus" option > > of -smp. > > > Please don't do this. If the maxcpus value from the command-line is not > > supported, QEMU should abort instead of silently ignoring the value > > provided by the user. > > Ah, I see. This code is confusing because we've split the > handling of user errors in the -smp argument between > the smp_parse() function and this bit of inline code. > Ideally we should put all the error handling in one > place...
The split makes sense to me: smp_parse() is just for basic parsing that doesn't depend on any state other than the provided QemuOpts parameter, and can run at any point in time. The machine->max_cpus check, on the other hand depend on the machine object to be created, and even changes the machine->max_cpus value. It is more dependent on ordering of initialization. To me, it makes sense to have it outside smp_parse(). But while looking at that code, I noticed that we can at least make the MAX_CPUMASK_BITS check and the machine->max_cpus check a single one. I will send a new patch. -- Eduardo
