Re: [Qemu-devel] [patch] Introduce per machine based max_cpu variable

[Jes Sorensen]

>>>>> "Anthony" == Anthony Liguori <address@hidden> writes:

>> Index: qemu/hw/an5206.c
>> ===================================================================
>> --- qemu.orig/hw/an5206.c +++ qemu/hw/an5206.c @@ -88,8 +88,9 @@
>> static void an5206_init(ram_addr_t ram_s } QEMUMachine
>> an5206_machine = { - "an5206", - "Arnewsh 5206", - an5206_init, -
>> 512, + .name = "an5206", + .desc = "Arnewsh 5206", + .init =
>> an5206_init, + .ram_require = 512, + .max_cpus = 1, };

Anthony> Your change would be greatly simplified if you defined
Anthony> max_cpus to be such that it was the maximum number of vcpus -
Anthony> 1.

Anthony> Then the default value of 0 would be sufficient for most
Anthony> machine types.

Hi Anthony,

I thought about this, but I think it's a really bad idea. It will
cause confusion over time and I think it is quite reasonable to
require all machines to define the maximum number of CPUs they
support. Thats why I defined the error message so it was quite easy
for anyone to fixup in case I got one of them wrong.

I really think it's better to do this right than hack it.

One option would be to spit out an error asking people to fix zero
into '1' and then let zero mean 1 for a while, but I don't think it's
worth the overhead.

>> Index: qemu/hw/boards.h
>> ===================================================================
>> --- qemu.orig/hw/boards.h +++ qemu/hw/boards.h @@ -16,7 +16,8 @@
>> typedef struct QEMUMachine { QEMUMachineInitFunc *init; #define
>> RAMSIZE_FIXED (1 << 0) ram_addr_t ram_require; - int nodisk_ok; +
>> short max_cpus; + char nodisk_ok; struct QEMUMachine *next; }
>> QEMUMachine;

Anthony> Just use int and please don't change nodisk_ok to char.

I did this to avoid making the structure grow. nodisk_ok is just a
true/false flag and I think even SGI has no plans to grow beyond 16
bit CPU count numbers.

I don't mind keeping both as int, I just thought it was silly to waste
the space for zero gain. Anyway, I'll make the change to the patch.

Anthony> exit() should be passed something between 0 and 255.
Anthony> WEXITSTATUS always returns the least significant 8 bits so
Anthony> the result will always be 0..255.  Moreover, by convention in
Anthony> QEMU we usually exit(1), not exit(-1).

I'll fix this - I am 99.999% sure I copied the exit(-1) from somewhere
else in the code though :-)

cheers,
Jes