All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.