Re: [Qemu-devel] [PATCH 2/2] qxl: change rom size to 8192

[Cole Robinson]

On 01/22/2013 05:09 AM, Gerd Hoffmann wrote:
> From: Alon Levy <address@hidden>
> 
> This is a simpler solution to 869981, where migration breaks since qxl's
> rom bar size has changed. Instead of ignoring fields in QXLRom, which is what 
> has
> actually changed, we remove some of the modes, a mechanism already
> accounted for by the guest. The modes left allow for portrait and
> landscape only modes, corresponding to orientations 0 and 1.
> Orientations 2 and 3 are dropped.
> 
> Added assert so that rom size will fit the future QXLRom increases via
> spice-protocol changes.
> 
> This patch has been tested with 6.1.0.10015. With the newer 6.1.0.10016
> there are problems with both "(flipped)" modes prior to the patch, and
> the patch loses the ability to set "Portrait" modes. But this is a
> separate bug to be fixed in the driver, and besides the patch doesn't
> affect the new arbitrary mode setting functionality.
> 
> Signed-off-by: Alon Levy <address@hidden>
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  hw/qxl.c |   13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 0d81816..a125e29 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -80,9 +80,7 @@
>  
>  #define QXL_MODE_EX(x_res, y_res)                 \
>      QXL_MODE_16_32(x_res, y_res, 0),              \
> -    QXL_MODE_16_32(y_res, x_res, 1),              \
> -    QXL_MODE_16_32(x_res, y_res, 2),              \
> -    QXL_MODE_16_32(y_res, x_res, 3)
> +    QXL_MODE_16_32(x_res, y_res, 1)
>  
>  static QXLMode qxl_modes[] = {
>      QXL_MODE_EX(640, 480),
> @@ -306,10 +304,13 @@ static inline uint32_t msb_mask(uint32_t val)
>  
>  static ram_addr_t qxl_rom_size(void)
>  {
> -    uint32_t rom_size = sizeof(QXLRom) + sizeof(QXLModes) + 
> sizeof(qxl_modes);
> +    uint32_t required_rom_size = sizeof(QXLRom) + sizeof(QXLModes) +
> +                                 sizeof(qxl_modes);
> +    uint32_t rom_size = 8192; /* two pages */
>  
> -    rom_size = MAX(rom_size, TARGET_PAGE_SIZE);
> -    rom_size = msb_mask(rom_size * 2 - 1);
> +    required_rom_size = MAX(required_rom_size, TARGET_PAGE_SIZE);
> +    required_rom_size = msb_mask(required_rom_size * 2 - 1);
> +    assert(required_rom_size <= rom_size);
>      return rom_size;
>  }
>  
> 

This breaks migration for me with -M pc-1.0 at least.

Before this commit:

./x86_64-softmmu/qemu-system-x86_64 -M pc-1.0 -vga qxl -spice port=5905
-monitor stdio
> migrate "exec: cat > foo.img"
> quit

After this commit:

./x86_64-softmmu/qemu-system-x86_64 -M pc-1.0 -vga qxl -spice port=5905
-monitor stdio -incoming "exec: cat foo.img"
qemu: warning: error while loading state for instance 0x0 of device 'ram'
cat: write error: Broken pipe
load of migration failed

Thanks,
Cole