qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative
Date: Wed, 30 Aug 2017 14:10:18 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

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 <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.






reply via email to

[Prev in Thread] Current Thread [Next in Thread]