Patchwork define qemukvm-1.2 machine type

login
register
mail settings
Submitter Marcelo Tosatti
Date Dec. 12, 2012, 10:39 p.m.
Message ID <20121212223919.GA3190@amt.cnet>
Download mbox | patch
Permalink /patch/205676/
State New
Headers show

Comments

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

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Paolo Bonzini - Dec. 13, 2012, 8:35 a.m.
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.
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.
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.
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.
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.
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.
> > > > 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.
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.
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

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);