Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative

[seeteena <s1seetee@linux.vnet.ibm.com>]

On 08/30/2017 10:40 PM, Philippe Mathieu-Daudé wrote:
> Hi Seeteena,
>
>>>> On 08/29/2017 02:45 AM, Seeteena Thoufeek wrote:
>>>>> ---Steps to Reproduce---
>>>>>
>>>>> When passed a negative number to 'maxcpus' parameter, Qemu aborts
>>>>> with a core dump.
>>>>>
>>>>> Run the following command with maxcpus argument as negative number
>>>>>
>>>>> ppc64-softmmu/qemu-system-ppc64 --nographic -vga none -machine
>>>>> pseries,accel=kvm,kvm-type=HV -m size=200g -device virtio-blk-pci,
>>>>> drive=rootdisk -drive file=/home/images/pegas-1.0-ppc64le.qcow2,
>>>>> if=none,cache=none,id=rootdisk,format=qcow2 -monitor telnet
>>>>> :127.0.0.1:1234,server,nowait -net nic,model=virtio -net
>>>>> user -redir tcp:2000::22 -device nec-usb-xhci -smp 8,cores=1,
>>>>> threads=1,maxcpus=-12
>
> Using 'extern unsigned int max_cpus;' I get:
>
> qemu-system-ppc64: Number of SMP CPUs requested (-12) exceeds max CPUs 
> supported by machine 'pseries-2.10' (1024)
>
> Which looks weird but sane :)
:). With ''extern unsigned int max_cpus;" How about changing the message 
to print as follows so it is more meaningful..

qemu-system-ppc64: Invalid SMP CPUs (-12) max CPUs supported by machine 
'pseries-artful' (1024)

Tried this with various combinations - Here is the results -

/qemu_ubuntu1710/qemu-2.10~rc3+dfsg# ppc64-softmmu/qemu-system-ppc64 
--nographic -vga none -machine pseries,accel=kvm,kvm-type=HV -m size=20g 
-device virtio-blk-pci,drive=rootdisk -drive 
file=/var/lib/libvirt/images/avocado-fvt-wrapper/data/avocado-vt/images/ubuntu-17.10-ppc64le.qcow2,if=none,cache=none,id=rootdisk,format=qcow2 
-monitor telnet:127.0.0.1:1234,server,nowait -net nic,model=virtio -net 
user -device nec-usb-xhci -smp 8,cores=1,threads=1,maxcpus=-12

qemu-system-ppc64: Invalid SMP CPUs (-12) max CPUs supported by machine 
'pseries-artful' (1024)

/qemu_ubuntu1710/qemu-2.10~rc3+dfsg# ppc64-softmmu/qemu-system-ppc64 
--nographic -vga none -machine pseries,accel=kvm,kvm-type=HV -m size=20g 
-device virtio-blk-pci,drive=rootdisk -drive 
file=/var/lib/libvirt/images/avocado-fvt-wrapper/data/avocado-vt/images/ubuntu-17.10-ppc64le.qcow2,if=none,cache=none,id=rootdisk,format=qcow2 
-monitor telnet:127.0.0.1:1234,server,nowait -net nic,model=virtio -net 
user -device nec-usb-xhci -smp 8,cores=1,threads=1,maxcpus=1026

qemu-system-ppc64: Invalid SMP CPUs (1026) max CPUs supported by machine 
'pseries-artful' (1024)

/qemu_ubuntu1710/qemu-2.10~rc3+dfsg# ppc64-softmmu/qemu-system-ppc64 
--nographic -vga none -machine pseries,accel=kvm,kvm-type=HV -m size=20g 
-device virtio-blk-pci,drive=rootdisk -drive 
file=/var/lib/libvirt/images/avocado-fvt-wrapper/data/avocado-vt/images/ubuntu-17.10-ppc64le.qcow2,if=none,cache=none,id=rootdisk,format=qcow2 
-monitor telnet:127.0.0.1:1234,server,nowait -net nic,model=virtio -net 
user -device nec-usb-xhci -smp 8,cores=1,threads=1,maxcpus=10


SLOF **********************************************************************
QEMU Starting
  Build Date = Jan  4 2017 05:15:48
  FW Version = buildd@ release 20161019
  Press "s" to enter Open Firmware.

/qemu_ubuntu1710/qemu-2.10~rc3+dfsg# ppc64-softmmu/qemu-system-ppc64 
--nographic -vga none -machine pseries,accel=kvm,kvm-type=HV -m size=20g 
-device virtio-blk-pci,drive=rootdisk -drive 
file=/var/lib/libvirt/images/avocado-fvt-wrapper/data/avocado-vt/images/ubuntu-17.10-ppc64le.qcow2,if=none,cache=none,id=rootdisk,format=qcow2 
-monitor telnet:127.0.0.1:1234,server,nowait -net nic,model=virtio -net 
user -device nec-usb-xhci -smp 8,cores=-1,threads=-1,maxcpus=0
qemu-system-ppc64: maxcpus must be equal to or greater than smp

~/qemu_ubuntu1710/qemu-2.10~rc3+dfsg# ppc64-softmmu/qemu-system-ppc64 
--nographic -vga none -machine pseries,accel=kvm,kvm-type=HV -m size=20g 
-device virtio-blk-pci,drive=rootdisk -drive 
file=/var/lib/libvirt/images/avocado-fvt-wrapper/data/avocado-vt/images/ubuntu-17.10-ppc64le.qcow2,if=none,cache=none,id=rootdisk,format=qcow2 
-monitor telnet:127.0.0.1:1234,server,nowait -net nic,model=virtio -net 
user -device nec-usb-xhci -smp 8,cores=-1,threads=-1,maxcpus=-1256

qemu-system-ppc64: Invalid SMP CPUs (-1256) max CPUs supported by 
machine 'pseries-artful' (1024)


>
>>>>>
>>>>> (process:12149): GLib-ERROR **: gmem.c:130: failed to allocate
>>>>>   18446744073709550568 bytes
>>>>>
>>>>> Trace/breakpoint trap
>>>>>
>>>>> Reported-by: R.Nageswara Sastry <rnsastry@linux.vnet.ibm.com>
>>>>> Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
>>>>> ---
>>>>> v1 -> v2:
>>>>>    - Fix the error check in vl.c to make it generic.
>>>>> v2 -> v3:
>>>>>    - Fix coding style pointed out by patchew.
>>>>>    - Fix check for "<= 0" instead of just "< 0".
>>>>> v3 -> v4:
>>>>>    - Fix subject line.
>>>>>    - Removed space before ":" from vl.c:1248
>>>>>    - Removed Reviewed-by: flag.
>>>>> ---
>>>>>   vl.c | 5 ++++-
>>>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/vl.c b/vl.c
>>>>> index 8e247cc..2d9e73d 100644
>>>>> --- a/vl.c
>>>>> +++ b/vl.c
>>>>> @@ -1244,7 +1244,10 @@ static void smp_parse(QemuOpts *opts)
>>>>>           }
>>>>>             max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
>>>>> -
>>>>> +        if (max_cpus <= 0) {
>>>>> +            error_report("Invalid max_cpus: %d", max_cpus);
>>>>
>>>> I disagree with this patch, I think the correct fix is to declare 
>>>> max_cpus as unsigned.
>>>> Looking at the codebase I can't find any signed use of it.
>>>    if I declare max_cpus as unsigned, the error check is no more 
>>> valid as the value max_cpus fetches is of unsigned type and hence we 
>>> cannot do this below check.
>>>                                                     if (max_cpus <= 
>>> 0) {
>>> error_report("Invalid max_cpus: %d", max_cpus);
>>>
>>> When I remove the error check with max_cpus defined as unsigned, the 
>>> code behaves as follows when negetive value is passed for maxcpus
>>>    ppc64-softmmu/qemu-system-ppc64 --nographic -vga none -machine 
>>> pseries,accel=kvm,kvm-type=HV -m size=20g -device 
>>> virtio-blk-pci,drive=rootdisk -drive 
>>> file=/var/lib/libvirt/images/avocado-fvt-wrapper/data/avocado-vt/images/ubuntu-17.10-ppc64le.qcow2,if=none,cache=none,id=rootdisk,format=qcow2 
>>> -monitor telnet:127.0.0.1:1234,server,nowait -net nic,model=virtio 
>>> -net user -device nec-usb-xhci -smp 8,cores=1,threads=1,maxcpus=-12
>>>
>>>
>>> SLOF 
>>> **********************************************************************
>>> QEMU Starting
>>>  Build Date = Jan  4 2017 05:15:48
>>>  FW Version = buildd@ release 20161019
>>>  Press "s" to enter Open Firmware.
>>>
>>
>> Adding .. between we have max_cpus declared as extern int max_cpus; 
>> in sysemu.h file.
>>>>
>>>>> +            exit(1);
>>>>> +        }
>>>>>           if (max_cpus < cpus) {
>>>>>               error_report("maxcpus must be equal to or greater 
>>>>> than smp");
>>>>>               exit(1);
>>>>>
>>>>
>>>> Regards,
>>>>
>>>> Phil.
>>>>
>>>
>>
>