Re: [Qemu-devel] [PATCH 0/4] target-sparc: Update to use VMStateDescription

[Mark Cave-Ayland]

On 14/08/15 13:15, Artyom Tarasenko wrote:

Hi Artyom,

> Hi Mark,
> 
> On Fri, Aug 14, 2015 at 12:37 AM, Mark Cave-Ayland
> <address@hidden> wrote:
>> On 10/08/15 13:34, Peter Maydell wrote:
>>
>>> This patchset updates target-sparc to use VMStateDescription
>>> rather than hand-written save/load functions. (This and CRIS
>>> are the last two targets still using the old approach.)
>>>
>>> It's based on some patches from back in 2012 by Juan which
>>> I've updated, rebased and made some tweaks to.
>>>
>>> This is a migration compatibility break; we don't care about
>>> cross-version migration on SPARC guests, and not having to
>>> maintain the old wire format allows a cleaner vmstate
>>> description in several ways.
>>>
>>> NB that the 'split cpu_put_psr' patch seems to me to be a
>>> bugfix in and of itself, since currently we might try to
>>> call cpu_check_irqs() and deliver interrupts while we're
>>> halfway through updating a PSR value...
>>>
>>> Juan Quintela (2):
>>>   vmstate: introduce CPU_DoubleU arrays
>>>   target-sparc: Convert to VMStateDescription
>>>
>>> Peter Maydell (2):
>>>   target-sparc: Split cpu_put_psr into side-effect and no-side-effect
>>>     parts
>>>   target-sparc: Don't flush TLB in cpu_load function
>>>
>>>  hw/sparc64/sun4u.c          |  20 ---
>>>  include/migration/vmstate.h |   7 +
>>>  migration/vmstate.c         |  23 +++
>>>  target-sparc/cpu-qom.h      |   4 +
>>>  target-sparc/cpu.c          |   1 +
>>>  target-sparc/cpu.h          |   7 +-
>>>  target-sparc/machine.c      | 360 
>>> ++++++++++++++++++++------------------------
>>>  target-sparc/win_helper.c   |  19 ++-
>>>  8 files changed, 210 insertions(+), 231 deletions(-)
>>
>> Hi Peter,
>>
>> Thanks for looking into this! In general the patches look very
>> reasonable (although I will need to give them a more thorough testing
>> when I get a chance) - my only concern is the break in migration
>> compatibility. Am I right in thinking that with this patch applied a
>> loadvm cannot restore a savevm from an earlier version?
>>
>> Not so much for qemu-system-sparc64 which is still somewhat
>> experimental, however qemu-system-sparc has become very usable since
>> 2012 with the advent of the cg3 and OpenBIOS changes that can now run
>> Solaris/SunOS and I do have a slight concern that people could lose
>> their qcow2 snapshots. Then again if we document this loudly in the
>> release notes then I guess it is possible to convert a snapshot back to
>> a raw, boot that and then savevm it back to the newer qcow2 again...
> 
> I think you and Peter speak about different snapshots. The filesystem
> snapshots are not affected with this series, so no need to convert
> qcow2 back and force.
> What would be broken is the live system snapshot - it won't be
> possible to live migrate from one QEMU version to another one without
> rebooting the guest.
> But I guess a reboot for a QEMU upgrade is not too expensive for our
> current users.

Let me try and clarify myself a bit more here - I do have some test
snapshots with qcow2 images created with "savevm" which also saves the
complete machine state alongside the disk image snapshot to enable me to
jump straight to a specific point in time.

I was wondering if it would be possible to "loadvm" a machine state from
a snapshot running under the old QEMU version so that the image can be
shutdown correctly and then bring up just the qcow2 filesystem snapshot
in the new QEMU version, at which point I can get the VM to a similar
point and then "savevm" again to get back to where I was. If that is the
case then I'm okay with the changes as long as we note this in the
release notes (so I'm also fine with requiring a reboot after the upgrade).


ATB,

Mark.