* [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative @ 2017-08-29 5:45 Seeteena Thoufeek 2017-08-29 10:47 ` Fam Zheng 2017-08-29 13:45 ` Philippe Mathieu-Daudé 0 siblings, 2 replies; 8+ messages in thread From: Seeteena Thoufeek @ 2017-08-29 5:45 UTC (permalink / raw) To: qemu-devel, qemu-ppc, famz, pbonzini; +Cc: s1seetee ---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 (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); + exit(1); + } if (max_cpus < cpus) { error_report("maxcpus must be equal to or greater than smp"); exit(1); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative 2017-08-29 5:45 [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative Seeteena Thoufeek @ 2017-08-29 10:47 ` Fam Zheng 2017-08-29 13:45 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 8+ messages in thread From: Fam Zheng @ 2017-08-29 10:47 UTC (permalink / raw) To: Seeteena Thoufeek; +Cc: qemu-devel, qemu-ppc, pbonzini On Tue, 08/29 11:15, 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 > > (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); > + exit(1); > + } > if (max_cpus < cpus) { > error_report("maxcpus must be equal to or greater than smp"); > exit(1); > -- > 1.8.3.1 > > Reviewed-by: Fam Zheng <famz@redhat.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative 2017-08-29 5:45 [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative Seeteena Thoufeek 2017-08-29 10:47 ` Fam Zheng @ 2017-08-29 13:45 ` Philippe Mathieu-Daudé 2017-08-30 8:07 ` seeteena 1 sibling, 1 reply; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2017-08-29 13:45 UTC (permalink / raw) To: Seeteena Thoufeek, qemu-devel, qemu-ppc, famz, pbonzini, Eric Blake Cc: Peter Maydell 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 > > (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. > + exit(1); > + } > if (max_cpus < cpus) { > error_report("maxcpus must be equal to or greater than smp"); > exit(1); > Regards, Phil. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative 2017-08-29 13:45 ` Philippe Mathieu-Daudé @ 2017-08-30 8:07 ` seeteena 2017-08-30 13:27 ` seeteena 0 siblings, 1 reply; 8+ messages in thread From: seeteena @ 2017-08-30 8:07 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel, qemu-ppc, famz, pbonzini, Eric Blake Cc: Peter Maydell On 08/29/2017 07:15 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 >> >> (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. > >> + exit(1); >> + } >> if (max_cpus < cpus) { >> error_report("maxcpus must be equal to or greater than >> smp"); >> exit(1); >> > > Regards, > > Phil. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative 2017-08-30 8:07 ` seeteena @ 2017-08-30 13:27 ` seeteena 2017-08-30 17:10 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 8+ messages in thread From: seeteena @ 2017-08-30 13:27 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel, qemu-ppc, famz, pbonzini, Eric Blake Cc: Peter Maydell On 08/30/2017 01:37 PM, seeteena wrote: > > > On 08/29/2017 07:15 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 >>> >>> (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. >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative 2017-08-30 13:27 ` seeteena @ 2017-08-30 17:10 ` Philippe Mathieu-Daudé 2017-09-01 10:16 ` seeteena 2017-09-04 7:51 ` seeteena 0 siblings, 2 replies; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2017-08-30 17:10 UTC (permalink / raw) To: seeteena, qemu-devel, qemu-ppc, famz, pbonzini, Eric Blake; +Cc: Peter Maydell 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 :) >>>> >>>> (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. >>> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative 2017-08-30 17:10 ` Philippe Mathieu-Daudé @ 2017-09-01 10:16 ` seeteena 2017-09-04 7:51 ` seeteena 1 sibling, 0 replies; 8+ messages in thread From: seeteena @ 2017-09-01 10:16 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel, qemu-ppc, famz, pbonzini, Eric Blake Cc: Peter Maydell 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. >>>> >>> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative 2017-08-30 17:10 ` Philippe Mathieu-Daudé 2017-09-01 10:16 ` seeteena @ 2017-09-04 7:51 ` seeteena 1 sibling, 0 replies; 8+ messages in thread From: seeteena @ 2017-09-04 7:51 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel, qemu-ppc, famz, pbonzini, Eric Blake Cc: Peter Maydell 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 <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. >>>> >>> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-09-04 7:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-29 5:45 [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative Seeteena Thoufeek 2017-08-29 10:47 ` Fam Zheng 2017-08-29 13:45 ` Philippe Mathieu-Daudé 2017-08-30 8:07 ` seeteena 2017-08-30 13:27 ` seeteena 2017-08-30 17:10 ` Philippe Mathieu-Daudé 2017-09-01 10:16 ` seeteena 2017-09-04 7:51 ` seeteena
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).