[PATCH][STABLE-0.12] qemu-kvm: Disable cpu_set command to avoid segfault

[PATCH][STABLE-0.12] qemu-kvm: Disable cpu_set command to avoid segfault

[Ryota Ozaki]

CPU hotplug feature is lost in this version. Even worse
cpu_set qemu command may cause segfault.

This patch fix the problem by just disabling it. It is
enough for the stable branch.

Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
Cc: Gleb Natapov <gleb@redhat.com>
---
 monitor.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 6ff6e1f..4b566d2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -829,7 +829,9 @@ static void do_cpu_set_nr(Monitor *mon, const QDict *qdict)
         return;
     }
 #if defined(TARGET_I386) || defined(TARGET_X86_64)
-    qemu_system_cpu_hot_add(value, state);
+    //qemu_system_cpu_hot_add(value, state);
+    monitor_printf(mon, "The feature is not avaiable in this version. "
+                        "It will be back in a future version.\n");
 #endif
 }
 
-- 
1.6.5.2

Re: [PATCH][STABLE-0.12] qemu-kvm: Disable cpu_set command to avoid segfault

[Avi Kivity]

On 01/11/2010 07:27 PM, Ryota Ozaki wrote:
> CPU hotplug feature is lost in this version. Even worse
> cpu_set qemu command may cause segfault.
>
> This patch fix the problem by just disabling it. It is
> enough for the stable branch.
>
> Signed-off-by: Ryota Ozaki<ozaki.ryota@gmail.com>
> Cc: Gleb Natapov<gleb@redhat.com>
> ---
>   monitor.c |    4 +++-
>   1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 6ff6e1f..4b566d2 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -829,7 +829,9 @@ static void do_cpu_set_nr(Monitor *mon, const QDict *qdict)
>           return;
>       }
>   #if defined(TARGET_I386) || defined(TARGET_X86_64)
> -    qemu_system_cpu_hot_add(value, state);
> +    //qemu_system_cpu_hot_add(value, state);
> +    monitor_printf(mon, "The feature is not avaiable in this version. "
> +                        "It will be back in a future version.\n");
>   #endif
>   }
>    

Daniel, how is libvirt going to react to this?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH][STABLE-0.12] qemu-kvm: Disable cpu_set command to avoid segfault

[Daniel P. Berrange]

On Mon, Jan 18, 2010 at 01:34:21PM +0200, Avi Kivity wrote:
> On 01/11/2010 07:27 PM, Ryota Ozaki wrote:
> >CPU hotplug feature is lost in this version. Even worse
> >cpu_set qemu command may cause segfault.
> >
> >This patch fix the problem by just disabling it. It is
> >enough for the stable branch.
> >
> >Signed-off-by: Ryota Ozaki<ozaki.ryota@gmail.com>
> >Cc: Gleb Natapov<gleb@redhat.com>
> >---
> >  monitor.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> >
> >diff --git a/monitor.c b/monitor.c
> >index 6ff6e1f..4b566d2 100644
> >--- a/monitor.c
> >+++ b/monitor.c
> >@@ -829,7 +829,9 @@ static void do_cpu_set_nr(Monitor *mon, const QDict 
> >*qdict)
> >          return;
> >      }
> >  #if defined(TARGET_I386) || defined(TARGET_X86_64)
> >-    qemu_system_cpu_hot_add(value, state);
> >+    //qemu_system_cpu_hot_add(value, state);
> >+    monitor_printf(mon, "The feature is not avaiable in this version. "
> >+                        "It will be back in a future version.\n");
> >  #endif
> >  }
> 
> Daniel, how is libvirt going to react to this?

At this precise point in time it won't react since its not using it, but 
in a week's time when we have finished the libvirt code to use 'cpu_set' 
it'll be less happy !

Can't we actually fix CPU hotplug in the stable tree rather than just
commenting it out ?


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [PATCH][STABLE-0.12] qemu-kvm: Disable cpu_set command to avoid segfault

[Avi Kivity]

On 01/18/2010 01:49 PM, Daniel P. Berrange wrote:
>
>> Daniel, how is libvirt going to react to this?
>>      
> At this precise point in time it won't react since its not using it, but
> in a week's time when we have finished the libvirt code to use 'cpu_set'
> it'll be less happy !
>
> Can't we actually fix CPU hotplug in the stable tree rather than just
> commenting it out ?
>    

We should certainly fix it, but I doubt it's a good fit for the stable tree.

Gleb, what would it take?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH][STABLE-0.12] qemu-kvm: Disable cpu_set command to avoid segfault

[Gleb Natapov]

On Mon, Jan 18, 2010 at 01:57:04PM +0200, Avi Kivity wrote:
> On 01/18/2010 01:49 PM, Daniel P. Berrange wrote:
> >
> >>Daniel, how is libvirt going to react to this?
> >At this precise point in time it won't react since its not using it, but
> >in a week's time when we have finished the libvirt code to use 'cpu_set'
> >it'll be less happy !
> >
> >Can't we actually fix CPU hotplug in the stable tree rather than just
> >commenting it out ?
> 
> We should certainly fix it, but I doubt it's a good fit for the stable tree.
> 
> Gleb, what would it take?
I have no idea about what causes segfault, so can't really tell how
invasive fix would be, but fixing segfault is not enough we will have to
backport BIOS code to seabios and IMO this code is too much for stable.

--
			Gleb.

Re: [PATCH][STABLE-0.12] qemu-kvm: Disable cpu_set command to avoid segfault

[Avi Kivity]

On 01/18/2010 02:03 PM, Gleb Natapov wrote:
>
>> We should certainly fix it, but I doubt it's a good fit for the stable tree.
>>
>> Gleb, what would it take?
>>      
> I have no idea about what causes segfault, so can't really tell how
> invasive fix would be, but fixing segfault is not enough we will have to
> backport BIOS code to seabios and IMO this code is too much for stable.
>    

I was asking about the BIOS code.  Can you point me at it?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH][STABLE-0.12] qemu-kvm: Disable cpu_set command to avoid segfault

[Gleb Natapov]

On Mon, Jan 18, 2010 at 02:08:40PM +0200, Avi Kivity wrote:
> On 01/18/2010 02:03 PM, Gleb Natapov wrote:
> >
> >>We should certainly fix it, but I doubt it's a good fit for the stable tree.
> >>
> >>Gleb, what would it take?
> >I have no idea about what causes segfault, so can't really tell how
> >invasive fix would be, but fixing segfault is not enough we will have to
> >backport BIOS code to seabios and IMO this code is too much for stable.
> 
> I was asking about the BIOS code.  Can you point me at it?
> 
This is majority of AML part of it:
http://git.kernel.org/?p=virt/kvm/pcbios.git;a=blob;f=acpi-ssdt.dsl;h=d998867d70b6384c1f40356b9dd8a06b78b59022;hb=master

Seabios should be changed to allocate MADT table from memory that will
be marked as ACPI NVS in e820 and it should pass MADT table address into
AML code somehow (BOCH bios put it at address 0x0514 and AML read it
from there).

Note also that currently seabios created this SSDT dynamically and thus
can support any number of vcpus, by porting those changes we will limit
it to 15 vpus.

--
			Gleb.