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

[Alon Levy]

> On 02/06/2013 05:52 PM, Cole Robinson wrote:
> > 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
> > 
> 
> Sorry, I realize that may seem a bit artificial, so just to clarify a
> bit. If
> I start on v1.3.1, then cherry-pick this commit, it breaks migration
> using the
> commands above. I also tried v1.3.1 to git head, reverting the
> seabios rebase
> that caused other migration issues, and got the same error (is there
> any way
> to get better error reporting out of migration failures?).

Migration error reporting sucks. Did you create foo.img with pre-patched qemu 
and then try to read it with post patched, or both are using the same version?

> 
> Thanks,
> Cole
>