[RFC 00/22] migration: convert vmstate_load/save_state

Vladimir Sementsov-Ogievskiy posted 22 patches 2 weeks, 2 days ago
Failed in applying to current master (apply log)
hw/display/virtio-gpu.c        |  66 ++--
hw/net/virtio-net.c            | 118 ++++---
hw/nvram/eeprom93xx.c          |  25 +-
hw/nvram/fw_cfg.c              |  30 +-
hw/pci/msix.c                  |  17 +-
hw/pci/pci.c                   |  66 ++--
hw/pci/shpc.c                  |  22 +-
hw/s390x/virtio-ccw.c          |  25 +-
hw/scsi/scsi-bus.c             |  17 +-
hw/scsi/spapr_vscsi.c          |   6 +-
hw/vfio/pci.c                  |  14 +-
hw/virtio/vhost-user-fs.c      |  65 ++--
hw/virtio/virtio-mmio.c        |  10 +-
hw/virtio/virtio-pci.c         |  10 +-
hw/virtio/virtio.c             |  78 +++--
include/hw/virtio/virtio-bus.h |   4 +-
include/hw/virtio/virtio.h     |   2 -
include/migration/cpr.h        |   4 +-
include/migration/vmstate.h    |  19 +-
migration/cpr.c                |  32 +-
migration/migration.c          |   2 +-
migration/savevm.c             | 108 +++---
migration/vmstate-types.c      | 584 ++++++++++++++++-----------------
migration/vmstate.c            | 187 +++++------
tests/unit/test-vmstate.c      | 116 ++-----
ui/vdagent.c                   |  38 +--
26 files changed, 804 insertions(+), 861 deletions(-)
[RFC 00/22] migration: convert vmstate_load/save_state
Posted by Vladimir Sementsov-Ogievskiy 2 weeks, 2 days ago
Hi all!

That's a proof-of-concept for converting vmstate_load/save_state
to have boolean error value, to fixup this analysis
  https://lore.kernel.org/qemu-devel/aQDdRn8t0B8oE3gf@x1.local/
in code.

As many of .get / .set handlers call vmstate_load/save_state,
let's convert them too, it not too much.

And finally, while touching each file, let's also use
new pre/post _errp() APIs.

So, this series propagate a lot of errors through errp, which
were simply printed out before.

Why it is an RFC:
1. I didn't yet double check the accuracy of all patches
2. Maybe, commit messages need to be more detailed, maybe they
need more arguments about correctness of the change
3. Maybe, it's better first merge new generic interfaces, and
than send per-maintainer small series, to avoid this huge
series, depending on many maintainers.

So, I don't include in CC many maintainers now, to get a first
look from Markus and Peter.

What do you think?

Based on [PATCH v4 0/2] migration: vmsd errp handlers: return bool
Based-on: <20251028130738.29037-1-vsementsov@yandex-team.ru>

Vladimir Sementsov-Ogievskiy (22):
  migration: introduce vmstate_load_vmsd() and vmstate_save_vmsd()
  migration: VMStateInfo: introduce new handlers with errp
  tests/unit/test-vmstate: move to new migration APIs
  ui/vdagent: move to new migration APIs
  hw/s390x/virtio-ccw.c: move to new migration APIs
  hw/scsi/spapr_vscsi: move to new migration APIs
  hw/scsi/scsi-bus.c: use new migration APIs
  hw/vfio/pci: move to new migration APIs
  hw/pci/pci: move to new migration APIs
  hw/pci/msix.c: use new migration APIs
  hw/pci/shpc.c: use new migration APIs
  hw/virtio: move to new migration APIs
  hw/display/virtio-gpu: move to new migration APIs
  hw/net/virtio-net.c: use new migration APIs
  hw/nvram/eeprom93xx.c: use new migration APIs
  hw/nvram/fw_cfg.c: use new migration APIs
  migration/cpr: move to new migration APIs
  migration/savevm: move to new migration APIs
  migration/vmstate-types: move to new migration APIs
  migration: VMStateInfo: remove old .get / .set handlers
  migration: convert vmstate_subsection_save/load functions to bool
  migration: finally convert vmstate_save/load_state() functions

 hw/display/virtio-gpu.c        |  66 ++--
 hw/net/virtio-net.c            | 118 ++++---
 hw/nvram/eeprom93xx.c          |  25 +-
 hw/nvram/fw_cfg.c              |  30 +-
 hw/pci/msix.c                  |  17 +-
 hw/pci/pci.c                   |  66 ++--
 hw/pci/shpc.c                  |  22 +-
 hw/s390x/virtio-ccw.c          |  25 +-
 hw/scsi/scsi-bus.c             |  17 +-
 hw/scsi/spapr_vscsi.c          |   6 +-
 hw/vfio/pci.c                  |  14 +-
 hw/virtio/vhost-user-fs.c      |  65 ++--
 hw/virtio/virtio-mmio.c        |  10 +-
 hw/virtio/virtio-pci.c         |  10 +-
 hw/virtio/virtio.c             |  78 +++--
 include/hw/virtio/virtio-bus.h |   4 +-
 include/hw/virtio/virtio.h     |   2 -
 include/migration/cpr.h        |   4 +-
 include/migration/vmstate.h    |  19 +-
 migration/cpr.c                |  32 +-
 migration/migration.c          |   2 +-
 migration/savevm.c             | 108 +++---
 migration/vmstate-types.c      | 584 ++++++++++++++++-----------------
 migration/vmstate.c            | 187 +++++------
 tests/unit/test-vmstate.c      | 116 ++-----
 ui/vdagent.c                   |  38 +--
 26 files changed, 804 insertions(+), 861 deletions(-)

-- 
2.48.1
Re: [RFC 00/22] migration: convert vmstate_load/save_state
Posted by Peter Xu 2 weeks ago
On Wed, Oct 29, 2025 at 02:13:24AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> That's a proof-of-concept for converting vmstate_load/save_state
> to have boolean error value, to fixup this analysis
>   https://lore.kernel.org/qemu-devel/aQDdRn8t0B8oE3gf@x1.local/
> in code.
> 
> As many of .get / .set handlers call vmstate_load/save_state,
> let's convert them too, it not too much.
> 
> And finally, while touching each file, let's also use
> new pre/post _errp() APIs.
> 
> So, this series propagate a lot of errors through errp, which
> were simply printed out before.
> 
> Why it is an RFC:
> 1. I didn't yet double check the accuracy of all patches
> 2. Maybe, commit messages need to be more detailed, maybe they
> need more arguments about correctness of the change
> 3. Maybe, it's better first merge new generic interfaces, and
> than send per-maintainer small series, to avoid this huge
> series, depending on many maintainers.
> 
> So, I don't include in CC many maintainers now, to get a first
> look from Markus and Peter.

It's still good to collect more opinions on especially rfc series, even if
the list doesn't need to include maintainers for each device. I added
Fabiano and Dan too.

> 
> What do you think?

In general.. I liked it. :) Thanks for trying it.

-- 
Peter Xu
Re: [RFC 00/22] migration: convert vmstate_load/save_state
Posted by Peter Xu 2 weeks ago
On Thu, Oct 30, 2025 at 01:14:22PM -0400, Peter Xu wrote:
> On Wed, Oct 29, 2025 at 02:13:24AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > Hi all!
> > 
> > That's a proof-of-concept for converting vmstate_load/save_state
> > to have boolean error value, to fixup this analysis
> >   https://lore.kernel.org/qemu-devel/aQDdRn8t0B8oE3gf@x1.local/
> > in code.
> > 
> > As many of .get / .set handlers call vmstate_load/save_state,
> > let's convert them too, it not too much.
> > 
> > And finally, while touching each file, let's also use
> > new pre/post _errp() APIs.
> > 
> > So, this series propagate a lot of errors through errp, which
> > were simply printed out before.
> > 
> > Why it is an RFC:
> > 1. I didn't yet double check the accuracy of all patches
> > 2. Maybe, commit messages need to be more detailed, maybe they
> > need more arguments about correctness of the change
> > 3. Maybe, it's better first merge new generic interfaces, and
> > than send per-maintainer small series, to avoid this huge
> > series, depending on many maintainers.
> > 
> > So, I don't include in CC many maintainers now, to get a first
> > look from Markus and Peter.
> 
> It's still good to collect more opinions on especially rfc series, even if
> the list doesn't need to include maintainers for each device. I added
> Fabiano and Dan too.
> 
> > 
> > What do you think?
> 
> In general.. I liked it. :) Thanks for trying it.

Oh wait, so this is still not the full list of what needs to be done?
After I applied your patchset, I still see:

$ git grep -E "\.get | \.set " | wc -l
214

Some of them are outliers but most of them look not.. :(

-- 
Peter Xu
Re: [RFC 00/22] migration: convert vmstate_load/save_state
Posted by Vladimir Sementsov-Ogievskiy 2 weeks ago
On 30.10.25 20:19, Peter Xu wrote:
> On Thu, Oct 30, 2025 at 01:14:22PM -0400, Peter Xu wrote:
>> On Wed, Oct 29, 2025 at 02:13:24AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi all!
>>>
>>> That's a proof-of-concept for converting vmstate_load/save_state
>>> to have boolean error value, to fixup this analysis
>>>    https://lore.kernel.org/qemu-devel/aQDdRn8t0B8oE3gf@x1.local/
>>> in code.
>>>
>>> As many of .get / .set handlers call vmstate_load/save_state,
>>> let's convert them too, it not too much.
>>>
>>> And finally, while touching each file, let's also use
>>> new pre/post _errp() APIs.
>>>
>>> So, this series propagate a lot of errors through errp, which
>>> were simply printed out before.
>>>
>>> Why it is an RFC:
>>> 1. I didn't yet double check the accuracy of all patches
>>> 2. Maybe, commit messages need to be more detailed, maybe they
>>> need more arguments about correctness of the change
>>> 3. Maybe, it's better first merge new generic interfaces, and
>>> than send per-maintainer small series, to avoid this huge
>>> series, depending on many maintainers.
>>>
>>> So, I don't include in CC many maintainers now, to get a first
>>> look from Markus and Peter.
>>
>> It's still good to collect more opinions on especially rfc series, even if
>> the list doesn't need to include maintainers for each device. I added
>> Fabiano and Dan too.
>>
>>>
>>> What do you think?
>>
>> In general.. I liked it. :) Thanks for trying it.
> 
> Oh wait, so this is still not the full list of what needs to be done?
> After I applied your patchset, I still see:
> 
> $ git grep -E "\.get | \.set " | wc -l
> 214
> 
> Some of them are outliers but most of them look not.. :(
> 

I think, they are qdev properties, not VMStateInfo. In this series I do
remove old .get / .set interfaces, and compilation still works for me..

Maybe some modules are not included into my build config? Unfortunately yes:

> git grep -A 5 'VMStateInfo .* = {' | grep '\.get' | awk '{print $1}' | sort | uniq
hw/usb/redirect.c-
target/alpha/machine.c-
target/arm/machine.c-
target/avr/machine.c-
target/hppa/machine.c-
target/microblaze/machine.c-
target/mips/system/machine.c-
target/openrisc/machine.c-
target/ppc/machine.c-
target/sparc/machine.c-


That's 10 more commits. And the whole series would be 32, involving many people..

I think, I'll resend only migration/* commits as "part I", except for 20/22 (to save
old interfaces too), and then other parts in separate smaller series.


-- 
Best regards,
Vladimir