diff mbox

define qemukvm-1.2 machine type

Message ID 20121212223919.GA3190@amt.cnet
State New
Headers show

Commit Message

Marcelo Tosatti Dec. 12, 2012, 10:39 p.m. UTC
To allow migration from qemu-kvm-1.2, where cirrus device has 16 megabytes
of RAM.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Comments

Paolo Bonzini Dec. 13, 2012, 8:35 a.m. UTC | #1
Il 12/12/2012 23:39, Marcelo Tosatti ha scritto:
> 
> To allow migration from qemu-kvm-1.2, where cirrus device has 16 megabytes
> of RAM.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

A similar patch would be needed for all machine types though, no?

I think distros that used to ship qemu-kvm should just change the
default just like for the acpi_piix4.c change.

Paolo

> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 19e342a..ead4b6b 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -347,6 +347,26 @@ static QEMUMachine pc_machine_v1_2 = {
>      },
>  };
>  
> +#define QEMUKVMPC_COMPAT_1_2 \
> +        {\
> +            .driver   = "cirrus-vga",\
> +            .property = "vgamem_mb",\
> +            .value    = "16",\
> +        }
> +
> +static QEMUMachine qemukvmpc_machine_v1_2 = {
> +    .name = "qemukvm-pc-1.2",
> +    .alias = "pc",
> +    .desc = "Standard PC",
> +    .init = pc_init_pci,
> +    .max_cpus = 255,
> +    .compat_props = (GlobalProperty[]) {
> +        QEMUKVMPC_COMPAT_1_2,
> +        PC_COMPAT_1_2,
> +        { /* end of list */ }
> +    },
> +};
> +
>  #define PC_COMPAT_1_1 \
>          PC_COMPAT_1_2,\
>          {\
> @@ -645,6 +665,7 @@ static QEMUMachine xenfv_machine = {
>  
>  static void pc_machine_init(void)
>  {
> +    qemu_register_machine(&qemukvmpc_machine_v1_2);
>      qemu_register_machine(&pc_machine_v1_4);
>      qemu_register_machine(&pc_machine_v1_3);
>      qemu_register_machine(&pc_machine_v1_2);
>
Marcelo Tosatti Dec. 13, 2012, 8:29 p.m. UTC | #2
On Thu, Dec 13, 2012 at 09:35:13AM +0100, Paolo Bonzini wrote:
> Il 12/12/2012 23:39, Marcelo Tosatti ha scritto:
> > 
> > To allow migration from qemu-kvm-1.2, where cirrus device has 16 megabytes
> > of RAM.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> A similar patch would be needed for all machine types though, no?

Why ? That would break incoming migration from QEMU.

> I think distros that used to ship qemu-kvm should just change the
> default just like for the acpi_piix4.c change.
> 
> Paolo

Works for me.
Anthony Liguori Dec. 13, 2012, 9:15 p.m. UTC | #3
Marcelo Tosatti <mtosatti@redhat.com> writes:

> To allow migration from qemu-kvm-1.2, where cirrus device has 16 megabytes
> of RAM.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 19e342a..ead4b6b 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -347,6 +347,26 @@ static QEMUMachine pc_machine_v1_2 = {
>      },
>  };
>  
> +#define QEMUKVMPC_COMPAT_1_2 \
> +        {\
> +            .driver   = "cirrus-vga",\
> +            .property = "vgamem_mb",\
> +            .value    = "16",\
> +        }
> +
> +static QEMUMachine qemukvmpc_machine_v1_2 = {
> +    .name = "qemukvm-pc-1.2",
> +    .alias = "pc",

This shouldn't alias pc...  I think what you need mean to do is alias
pc-1.2 but in order to do that, we need some sort of way to indicate
that 'pc-1.2' should behave like qemu-kvm vs. qemu-system-x86_64.

Regards,

Anthony Liguori

> +    .desc = "Standard PC",
> +    .init = pc_init_pci,
> +    .max_cpus = 255,
> +    .compat_props = (GlobalProperty[]) {
> +        QEMUKVMPC_COMPAT_1_2,
> +        PC_COMPAT_1_2,
> +        { /* end of list */ }
> +    },
> +};
> +
>  #define PC_COMPAT_1_1 \
>          PC_COMPAT_1_2,\
>          {\
> @@ -645,6 +665,7 @@ static QEMUMachine xenfv_machine = {
>  
>  static void pc_machine_init(void)
>  {
> +    qemu_register_machine(&qemukvmpc_machine_v1_2);
>      qemu_register_machine(&pc_machine_v1_4);
>      qemu_register_machine(&pc_machine_v1_3);
>      qemu_register_machine(&pc_machine_v1_2);
Eduardo Habkost Dec. 13, 2012, 9:25 p.m. UTC | #4
On Thu, Dec 13, 2012 at 09:35:13AM +0100, Paolo Bonzini wrote:
> Il 12/12/2012 23:39, Marcelo Tosatti ha scritto:
> > 
> > To allow migration from qemu-kvm-1.2, where cirrus device has 16 megabytes
> > of RAM.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> A similar patch would be needed for all machine types though, no?
> 
> I think distros that used to ship qemu-kvm should just change the
> default just like for the acpi_piix4.c change.

Maybe we could provide a --with-qemu-kvm-compat configure flag to them?
Anthony Liguori Dec. 13, 2012, 9:35 p.m. UTC | #5
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, Dec 13, 2012 at 09:35:13AM +0100, Paolo Bonzini wrote:
>> Il 12/12/2012 23:39, Marcelo Tosatti ha scritto:
>> > 
>> > To allow migration from qemu-kvm-1.2, where cirrus device has 16 megabytes
>> > of RAM.
>> > 
>> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>> 
>> A similar patch would be needed for all machine types though, no?
>> 
>> I think distros that used to ship qemu-kvm should just change the
>> default just like for the acpi_piix4.c change.
>
> Maybe we could provide a --with-qemu-kvm-compat configure flag to
> them?

I think that defeats the purpose of a single binary.

I think it would be better for the distros to have a qemu-kvm script
that was:

/usr/libexec/qemu-kvm:

#!/bin/sh

qemu-system-x86_64 -enable-qemu-kvm-compat "$@"

Regards,

Anthony Liguori

>
> -- 
> Eduardo
Eduardo Habkost Dec. 13, 2012, 9:41 p.m. UTC | #6
On Thu, Dec 13, 2012 at 03:35:03PM -0600, Anthony Liguori wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Thu, Dec 13, 2012 at 09:35:13AM +0100, Paolo Bonzini wrote:
> >> Il 12/12/2012 23:39, Marcelo Tosatti ha scritto:
> >> > 
> >> > To allow migration from qemu-kvm-1.2, where cirrus device has 16 megabytes
> >> > of RAM.
> >> > 
> >> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >> 
> >> A similar patch would be needed for all machine types though, no?
> >> 
> >> I think distros that used to ship qemu-kvm should just change the
> >> default just like for the acpi_piix4.c change.
> >
> > Maybe we could provide a --with-qemu-kvm-compat configure flag to
> > them?
> 
> I think that defeats the purpose of a single binary.
> 
> I think it would be better for the distros to have a qemu-kvm script
> that was:
> 
> /usr/libexec/qemu-kvm:
> 
> #!/bin/sh
> 
> qemu-system-x86_64 -enable-qemu-kvm-compat "$@"

That would be even better. I proposed a configure flag because I
understood (maybe incorrectly) that Paolo proposed a build-time default
change.
Paolo Bonzini Dec. 14, 2012, 7:54 a.m. UTC | #7
> > > > I think distros that used to ship qemu-kvm should just change
> > > > the default just like for the acpi_piix4.c change.
> > >
> > > Maybe we could provide a --with-qemu-kvm-compat configure flag to
> > > them?

I like this.

> > I think that defeats the purpose of a single binary.
> > 
> > I think it would be better for the distros to have a qemu-kvm
> > script that was:
> > 
> > /usr/libexec/qemu-kvm:
> > 
> > #!/bin/sh
> > 
> > qemu-system-x86_64 -enable-qemu-kvm-compat "$@"
> 
> That would be even better. I proposed a configure flag because I
> understood (maybe incorrectly) that Paolo proposed a build-time
> default change.

Yes, that's what I was thinking.  The problem is that Fedora did ship a
qemu-system-x86_64 binary that disabled the qemu-kvm options (including
using TCG by default), but it still had a qemu-kvm-compatible migration
format.

Paolo
Anthony Liguori Dec. 14, 2012, 1:29 p.m. UTC | #8
Paolo Bonzini <pbonzini@redhat.com> writes:

>> > > > I think distros that used to ship qemu-kvm should just change
>> > > > the default just like for the acpi_piix4.c change.
>> > >
>> > > Maybe we could provide a --with-qemu-kvm-compat configure flag to
>> > > them?
>
> I like this.
>
>> > I think that defeats the purpose of a single binary.
>> > 
>> > I think it would be better for the distros to have a qemu-kvm
>> > script that was:
>> > 
>> > /usr/libexec/qemu-kvm:
>> > 
>> > #!/bin/sh
>> > 
>> > qemu-system-x86_64 -enable-qemu-kvm-compat "$@"
>> 
>> That would be even better. I proposed a configure flag because I
>> understood (maybe incorrectly) that Paolo proposed a build-time
>> default change.
>
> Yes, that's what I was thinking.  The problem is that Fedora did ship a
> qemu-system-x86_64 binary that disabled the qemu-kvm options (including
> using TCG by default), but it still had a qemu-kvm-compatible migration
> format.

Can you be more specific?  What's different in the migration format?

Regards,

Anthony Liguori

>
> Paolo
Paolo Bonzini Dec. 14, 2012, 1:46 p.m. UTC | #9
Il 14/12/2012 14:29, Anthony Liguori ha scritto:
>> >
>> > Yes, that's what I was thinking.  The problem is that Fedora did ship a
>> > qemu-system-x86_64 binary that disabled the qemu-kvm options (including
>> > using TCG by default), but it still had a qemu-kvm-compatible migration
>> > format.
> Can you be more specific?  What's different in the migration format?

The amount of VRAM for Cirrus is 16 MB, and the ACPI migration is
incompatible with qemu 1.2 (what Marcelo changed for 1.3).

Paolo
diff mbox

Patch

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 19e342a..ead4b6b 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -347,6 +347,26 @@  static QEMUMachine pc_machine_v1_2 = {
     },
 };
 
+#define QEMUKVMPC_COMPAT_1_2 \
+        {\
+            .driver   = "cirrus-vga",\
+            .property = "vgamem_mb",\
+            .value    = "16",\
+        }
+
+static QEMUMachine qemukvmpc_machine_v1_2 = {
+    .name = "qemukvm-pc-1.2",
+    .alias = "pc",
+    .desc = "Standard PC",
+    .init = pc_init_pci,
+    .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        QEMUKVMPC_COMPAT_1_2,
+        PC_COMPAT_1_2,
+        { /* end of list */ }
+    },
+};
+
 #define PC_COMPAT_1_1 \
         PC_COMPAT_1_2,\
         {\
@@ -645,6 +665,7 @@  static QEMUMachine xenfv_machine = {
 
 static void pc_machine_init(void)
 {
+    qemu_register_machine(&qemukvmpc_machine_v1_2);
     qemu_register_machine(&pc_machine_v1_4);
     qemu_register_machine(&pc_machine_v1_3);
     qemu_register_machine(&pc_machine_v1_2);