qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Cole Robinson <crobinso@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] define qemukvm-1.2 machine type
Date: Wed, 16 Jan 2013 18:17:03 -0500	[thread overview]
Message-ID: <50F734EF.3030402@redhat.com> (raw)
In-Reply-To: <50F583E6.1040907@redhat.com>

On 01/15/2013 11:29 AM, Paolo Bonzini wrote:
> Il 15/01/2013 04:45, Cole Robinson ha scritto:
>> Libvirt always specifies an explicit machine type and carries it for the life
>> of the VM. What we want is for 'qemu-kvm-1.2 -M pc-1.2' to migrate seamlessly
>> into 'qemu-1.3+ -M pc-1.2' without the user or libvirt having to change
>> machine type.
>>
>> So what we want to carry in Fedora is:
>>
>>
>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>> index aa3e7f4..b8f5f8a 100644
>> --- a/hw/pc_piix.c
>> +++ b/hw/pc_piix.c
>> @@ -315,6 +315,10 @@ static QEMUMachine pc_machine_v1_3 = {
>>              .driver   = "VGA",\
>>              .property = "mmio",\
>>              .value    = "off",\
>> +        },{\
>> +            .driver   = "cirrus-vga",\
>> +            .property = "vgamem_mb",\
>> +            .value    = stringify(8),\
>>          }
>>
>>  static QEMUMachine pc_machine_v1_2 = {
>>
>>
>> That sticks the cirrus compat block on the end of PC_COMPAT_1_2, which should
>> fix cirrus migration from qemu-kvm for all machine types <= pc-1.2. Obviously
>> this isn't suitable for upstream but we can hide it behind something like
>> ./configure --qemu-kvm-migrate-compat
> 
> Let's look at the source to reconstruct the changes.
> 
> In qemu-kvm-0.15.1 (Fedora 16), file hw/vga_int.h, we had
> 
>     #define VGA_RAM_SIZE (16 * 1024 * 1024)
> 
> In qemu-0.15.1, same file, we had
> 
>     #define VGA_RAM_SIZE (8192 * 1024)
> 
> The same holds all the way back to 0.9, which had them in hw/pc.h.
> 
> hw/vga-pci.c in qemu-kvm-1.2 and qemu-1.2 had both this line:
> 
>     DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb, 16),
> 
> but hw/cirrus_vga.c had respectively:
> 
>     #define VGA_RAM_SIZE (16384 * 1024)
>     #define VGA_RAM_SIZE (8192 * 1024)
> 
> So the right patch for downstreams that used qemu-kvm is this:
> 
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 85529b2..c29ea0d 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -410,22 +410,6 @@ static QEMUMachine pc_machine_v1_2 = {
>              .property = "param_change",\
>              .value    = "off",\
>          },{\
> -            .driver   = "VGA",\
> -            .property = "vgamem_mb",\
> -            .value    = stringify(8),\
> -        },{\
> -            .driver   = "vmware-svga",\
> -            .property = "vgamem_mb",\
> -            .value    = stringify(8),\
> -        },{\
> -            .driver   = "qxl-vga",\
> -            .property = "vgamem_mb",\
> -            .value    = stringify(8),\
> -        },{\
> -            .driver   = "qxl",\
> -            .property = "vgamem_mb",\
> -            .value    = stringify(8),\
> -        },{\
>              .driver   = "virtio-blk-pci",\
>              .property = "config-wce",\
>              .value    = "off",\
> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
> index 9bef96e..8c94428 100644
> --- a/hw/cirrus_vga.c
> +++ b/hw/cirrus_vga.c
> @@ -2975,7 +2975,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
> 
>  static Property pci_vga_cirrus_properties[] = {
>      DEFINE_PROP_UINT32("vgamem_mb", struct PCICirrusVGAState,
> -                       cirrus_vga.vga.vram_size_mb, 8),
> +                       cirrus_vga.vga.vram_size_mb, 16),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> 
> i.e. use 16 MB for all machine types and all cards.  The latter should
> probably be pushed into 1.4 with compat properties for 1.3.  At this
> point, you need to remove the compat property as in the pc_piix.c above.
> 

Thanks for the analysis Paolo, here's the final patch I used:

http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0004-Fix-migration-compat-with-qemu-kvm.patch

Thanks,
Cole

  reply	other threads:[~2013-01-16 23:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-12 22:39 [Qemu-devel] [PATCH] define qemukvm-1.2 machine type Marcelo Tosatti
2012-12-13  8:35 ` Paolo Bonzini
2012-12-13 20:29   ` Marcelo Tosatti
2012-12-13 21:25   ` Eduardo Habkost
2012-12-13 21:35     ` Anthony Liguori
2012-12-13 21:41       ` Eduardo Habkost
2012-12-14  7:54         ` Paolo Bonzini
2012-12-14 13:29           ` Anthony Liguori
2012-12-14 13:46             ` Paolo Bonzini
2012-12-13 21:15 ` Anthony Liguori
2013-01-15  3:45 ` Cole Robinson
2013-01-15 16:29   ` Paolo Bonzini
2013-01-16 23:17     ` Cole Robinson [this message]
2013-01-18 12:12       ` Marcelo Tosatti
2013-01-18 17:04         ` Cole Robinson
2013-01-18 17:34           ` Paolo Bonzini
2013-01-18 20:09             ` Marcelo Tosatti
2013-01-19 14:17               ` Paolo Bonzini
2013-01-18 19:50           ` Marcelo Tosatti

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50F734EF.3030402@redhat.com \
    --to=crobinso@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).