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

[seeteena]

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

/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. The max CPUs supported by 
machine 'pseries-artful' is 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=12


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=-1024

qemu-system-ppc64: Invalid SMP CPUs -1024. The max CPUs supported by 
machine 'pseries-artful' is 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. The max CPUs supported by 
machine 'pseries-artful' is 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=0
qemu-system-ppc64: maxcpus must be equal to or greater than smp

> > > > > (process:12149): GLib-ERROR **: gmem.c:130: failed to allocate
> > > > >   18446744073709550568 bytes
> > > > >
> > > > > Trace/breakpoint trap
> > > > >
> > > > > Reported-by: R.Nageswara Sastry <address@hidden>
> > > > > Signed-off-by: Seeteena Thoufeek <address@hidden>
> > > > > ---
> > > > > 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.