[v2] Rework qemu internal active snapshots to use QMP

[PATCH V2 0/4] Rework qemu internal active snapshots to use QMP

Posted by Nikolai Barybin via Devel 3 months, 2 weeks ago

Den, Peter, Daniel thank you for your comments!

I'm sending v2 of this patchset.

Changes since last revision:

- dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text

- added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE

- preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0

- enhanced requirements for allowing snapshotting. All writable disks
  should be qcow2, non-shared. If such disks exist and we have qcow2
  NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate
  to NVRAM

- make char** wrdevs list allocation inside
  qemuSnapshotActiveInternalGetWrdevListHelper()

Nikolai Barybin (4):
  qemu monitor: add snaphot-save/delete QMP commands
  qemu blockjob: add snapshot-save/delete job types
  qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
  qemu snapshot: use QMP snapshot-save/delete for internal snapshots

 src/qemu/qemu_block.c                         |   2 +
 src/qemu/qemu_blockjob.c                      |   6 +-
 src/qemu/qemu_blockjob.h                      |   2 +
 src/qemu/qemu_capabilities.c                  |   4 +
 src/qemu/qemu_capabilities.h                  |   2 +
 src/qemu/qemu_domain.c                        |   4 +
 src/qemu/qemu_monitor.c                       |  30 +++
 src/qemu/qemu_monitor.h                       |  13 ++
 src/qemu/qemu_monitor_json.c                  |  66 ++++++
 src/qemu/qemu_monitor_json.h                  |  13 ++
 src/qemu/qemu_snapshot.c                      | 207 ++++++++++++++++--
 .../caps_6.0.0_aarch64.xml                    |   2 +
 .../qemucapabilitiesdata/caps_6.0.0_s390x.xml |   2 +
 .../caps_6.0.0_x86_64.xml                     |   2 +
 .../caps_6.1.0_x86_64.xml                     |   2 +
 .../caps_6.2.0_aarch64.xml                    |   2 +
 .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml |   2 +
 .../caps_6.2.0_x86_64.xml                     |   2 +
 .../caps_7.0.0_aarch64+hvf.xml                |   2 +
 .../caps_7.0.0_aarch64.xml                    |   2 +
 .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml |   2 +
 .../caps_7.0.0_x86_64.xml                     |   2 +
 .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml |   2 +
 .../caps_7.1.0_x86_64.xml                     |   2 +
 tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml |   2 +
 .../caps_7.2.0_x86_64+hvf.xml                 |   2 +
 .../caps_7.2.0_x86_64.xml                     |   2 +
 .../caps_8.0.0_riscv64.xml                    |   2 +
 .../caps_8.0.0_x86_64.xml                     |   2 +
 .../qemucapabilitiesdata/caps_8.1.0_s390x.xml |   2 +
 .../caps_8.1.0_x86_64.xml                     |   2 +
 .../caps_8.2.0_aarch64.xml                    |   2 +
 .../caps_8.2.0_armv7l.xml                     |   2 +
 .../caps_8.2.0_loongarch64.xml                |   2 +
 .../qemucapabilitiesdata/caps_8.2.0_s390x.xml |   2 +
 .../caps_8.2.0_x86_64.xml                     |   2 +
 .../qemucapabilitiesdata/caps_9.0.0_sparc.xml |   2 +
 .../caps_9.0.0_x86_64.xml                     |   2 +
 .../caps_9.1.0_x86_64.xml                     |   2 +
 39 files changed, 391 insertions(+), 14 deletions(-)

-- 
2.43.5

Re: [PATCH V2 0/4] Rework qemu internal active snapshots to use QMP

Posted by Nikolai Barybin via Devel 2 months, 3 weeks ago

On 7/17/24 20:21, Nikolai Barybin wrote:
> Den, Peter, Daniel thank you for your comments!
>
> I'm sending v2 of this patchset.
>
> Changes since last revision:
>
> - dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
>
> - added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
>
> - preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
>
> - enhanced requirements for allowing snapshotting. All writable disks
>    should be qcow2, non-shared. If such disks exist and we have qcow2
>    NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate
>    to NVRAM
>
> - make char** wrdevs list allocation inside
>    qemuSnapshotActiveInternalGetWrdevListHelper()
>
> Nikolai Barybin (4):
>    qemu monitor: add snaphot-save/delete QMP commands
>    qemu blockjob: add snapshot-save/delete job types
>    qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
>    qemu snapshot: use QMP snapshot-save/delete for internal snapshots
>
>   src/qemu/qemu_block.c                         |   2 +
>   src/qemu/qemu_blockjob.c                      |   6 +-
>   src/qemu/qemu_blockjob.h                      |   2 +
>   src/qemu/qemu_capabilities.c                  |   4 +
>   src/qemu/qemu_capabilities.h                  |   2 +
>   src/qemu/qemu_domain.c                        |   4 +
>   src/qemu/qemu_monitor.c                       |  30 +++
>   src/qemu/qemu_monitor.h                       |  13 ++
>   src/qemu/qemu_monitor_json.c                  |  66 ++++++
>   src/qemu/qemu_monitor_json.h                  |  13 ++
>   src/qemu/qemu_snapshot.c                      | 207 ++++++++++++++++--
>   .../caps_6.0.0_aarch64.xml                    |   2 +
>   .../qemucapabilitiesdata/caps_6.0.0_s390x.xml |   2 +
>   .../caps_6.0.0_x86_64.xml                     |   2 +
>   .../caps_6.1.0_x86_64.xml                     |   2 +
>   .../caps_6.2.0_aarch64.xml                    |   2 +
>   .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml |   2 +
>   .../caps_6.2.0_x86_64.xml                     |   2 +
>   .../caps_7.0.0_aarch64+hvf.xml                |   2 +
>   .../caps_7.0.0_aarch64.xml                    |   2 +
>   .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml |   2 +
>   .../caps_7.0.0_x86_64.xml                     |   2 +
>   .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml |   2 +
>   .../caps_7.1.0_x86_64.xml                     |   2 +
>   tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml |   2 +
>   .../caps_7.2.0_x86_64+hvf.xml                 |   2 +
>   .../caps_7.2.0_x86_64.xml                     |   2 +
>   .../caps_8.0.0_riscv64.xml                    |   2 +
>   .../caps_8.0.0_x86_64.xml                     |   2 +
>   .../qemucapabilitiesdata/caps_8.1.0_s390x.xml |   2 +
>   .../caps_8.1.0_x86_64.xml                     |   2 +
>   .../caps_8.2.0_aarch64.xml                    |   2 +
>   .../caps_8.2.0_armv7l.xml                     |   2 +
>   .../caps_8.2.0_loongarch64.xml                |   2 +
>   .../qemucapabilitiesdata/caps_8.2.0_s390x.xml |   2 +
>   .../caps_8.2.0_x86_64.xml                     |   2 +
>   .../qemucapabilitiesdata/caps_9.0.0_sparc.xml |   2 +
>   .../caps_9.0.0_x86_64.xml                     |   2 +
>   .../caps_9.1.0_x86_64.xml                     |   2 +
>   39 files changed, 391 insertions(+), 14 deletions(-)
>
Hello everyone! This is a ping email.

Re: [PATCH V2 0/4] Rework qemu internal active snapshots to use QMP

Posted by Nikolai Barybin via Devel 3 months ago

Hello everyone! This is a ping email

On 7/17/24 20:21, Nikolai Barybin wrote:
> Den, Peter, Daniel thank you for your comments!
>
> I'm sending v2 of this patchset.
>
> Changes since last revision:
>
> - dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
>
> - added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
>
> - preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
>
> - enhanced requirements for allowing snapshotting. All writable disks
>    should be qcow2, non-shared. If such disks exist and we have qcow2
>    NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate
>    to NVRAM
>
> - make char** wrdevs list allocation inside
>    qemuSnapshotActiveInternalGetWrdevListHelper()
>
> Nikolai Barybin (4):
>    qemu monitor: add snaphot-save/delete QMP commands
>    qemu blockjob: add snapshot-save/delete job types
>    qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
>    qemu snapshot: use QMP snapshot-save/delete for internal snapshots
>
>   src/qemu/qemu_block.c                         |   2 +
>   src/qemu/qemu_blockjob.c                      |   6 +-
>   src/qemu/qemu_blockjob.h                      |   2 +
>   src/qemu/qemu_capabilities.c                  |   4 +
>   src/qemu/qemu_capabilities.h                  |   2 +
>   src/qemu/qemu_domain.c                        |   4 +
>   src/qemu/qemu_monitor.c                       |  30 +++
>   src/qemu/qemu_monitor.h                       |  13 ++
>   src/qemu/qemu_monitor_json.c                  |  66 ++++++
>   src/qemu/qemu_monitor_json.h                  |  13 ++
>   src/qemu/qemu_snapshot.c                      | 207 ++++++++++++++++--
>   .../caps_6.0.0_aarch64.xml                    |   2 +
>   .../qemucapabilitiesdata/caps_6.0.0_s390x.xml |   2 +
>   .../caps_6.0.0_x86_64.xml                     |   2 +
>   .../caps_6.1.0_x86_64.xml                     |   2 +
>   .../caps_6.2.0_aarch64.xml                    |   2 +
>   .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml |   2 +
>   .../caps_6.2.0_x86_64.xml                     |   2 +
>   .../caps_7.0.0_aarch64+hvf.xml                |   2 +
>   .../caps_7.0.0_aarch64.xml                    |   2 +
>   .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml |   2 +
>   .../caps_7.0.0_x86_64.xml                     |   2 +
>   .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml |   2 +
>   .../caps_7.1.0_x86_64.xml                     |   2 +
>   tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml |   2 +
>   .../caps_7.2.0_x86_64+hvf.xml                 |   2 +
>   .../caps_7.2.0_x86_64.xml                     |   2 +
>   .../caps_8.0.0_riscv64.xml                    |   2 +
>   .../caps_8.0.0_x86_64.xml                     |   2 +
>   .../qemucapabilitiesdata/caps_8.1.0_s390x.xml |   2 +
>   .../caps_8.1.0_x86_64.xml                     |   2 +
>   .../caps_8.2.0_aarch64.xml                    |   2 +
>   .../caps_8.2.0_armv7l.xml                     |   2 +
>   .../caps_8.2.0_loongarch64.xml                |   2 +
>   .../qemucapabilitiesdata/caps_8.2.0_s390x.xml |   2 +
>   .../caps_8.2.0_x86_64.xml                     |   2 +
>   .../qemucapabilitiesdata/caps_9.0.0_sparc.xml |   2 +
>   .../caps_9.0.0_x86_64.xml                     |   2 +
>   .../caps_9.1.0_x86_64.xml                     |   2 +
>   39 files changed, 391 insertions(+), 14 deletions(-)
>

Re: [PATCH V2 0/4] Rework qemu internal active snapshots to use QMP

Posted by Peter Krempa 2 months, 2 weeks ago

On Wed, Jul 17, 2024 at 21:21:33 +0300, Nikolai Barybin via Devel wrote:
> Den, Peter, Daniel thank you for your comments!
> 
> I'm sending v2 of this patchset.
> 
> Changes since last revision:
> 
> - dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
> 
> - added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
> 
> - preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
> 
> - enhanced requirements for allowing snapshotting. All writable disks
>   should be qcow2, non-shared. If such disks exist and we have qcow2
>   NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate
>   to NVRAM
> 
> - make char** wrdevs list allocation inside
>   qemuSnapshotActiveInternalGetWrdevListHelper()

Note that I'll clean up the patches, so there's no need for you to send
another version. My review will follow regardless as documentation of
what I'll be changing.

Re: [PATCH V2 0/4] Rework qemu internal active snapshots to use QMP

Posted by Nikolai Barybin via Devel 2 months ago

On 8/14/24 14:03, Peter Krempa wrote:

> On Wed, Jul 17, 2024 at 21:21:33 +0300, Nikolai Barybin via Devel wrote:
>> Den, Peter, Daniel thank you for your comments!
>>
>> I'm sending v2 of this patchset.
>>
>> Changes since last revision:
>>
>> - dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
>>
>> - added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
>>
>> - preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
>>
>> - enhanced requirements for allowing snapshotting. All writable disks
>>    should be qcow2, non-shared. If such disks exist and we have qcow2
>>    NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate
>>    to NVRAM
>>
>> - make char** wrdevs list allocation inside
>>    qemuSnapshotActiveInternalGetWrdevListHelper()
> Note that I'll clean up the patches, so there's no need for you to send
> another version. My review will follow regardless as documentation of
> what I'll be changing.
>

Peter, thank you for your review! Do you have ETA for these changes to 
appear in master branch? We need them in our product.

Re: [PATCH V2 0/4] Rework qemu internal active snapshots to use QMP

Posted by Peter Krempa 2 months ago

On Thu, Aug 29, 2024 at 11:09:23 +0200, Nikolai Barybin wrote:
> On 8/14/24 14:03, Peter Krempa wrote:
> 
> > On Wed, Jul 17, 2024 at 21:21:33 +0300, Nikolai Barybin via Devel wrote:
> > > Den, Peter, Daniel thank you for your comments!
> > > 
> > > I'm sending v2 of this patchset.
> > > 
> > > Changes since last revision:
> > > 
> > > - dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
> > > 
> > > - added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
> > > 
> > > - preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
> > > 
> > > - enhanced requirements for allowing snapshotting. All writable disks
> > >    should be qcow2, non-shared. If such disks exist and we have qcow2
> > >    NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate
> > >    to NVRAM
> > > 
> > > - make char** wrdevs list allocation inside
> > >    qemuSnapshotActiveInternalGetWrdevListHelper()
> > Note that I'll clean up the patches, so there's no need for you to send
> > another version. My review will follow regardless as documentation of
> > what I'll be changing.
> > 
> 
> Peter, thank you for your review! Do you have ETA for these changes to
> appear in master branch? We need them in our product.

I'm halfway through fixing the last patch as I had other things to do as
well.

I'll be sending them out hopefully next week.

Re: [PATCH V2 0/4] Rework qemu internal active snapshots to use QMP

Posted by Nikolai Barybin via Devel 3 weeks, 6 days ago

On 8/29/24 11:12, Peter Krempa wrote:
> On Thu, Aug 29, 2024 at 11:09:23 +0200, Nikolai Barybin wrote:
>> On 8/14/24 14:03, Peter Krempa wrote:
>>
>>> On Wed, Jul 17, 2024 at 21:21:33 +0300, Nikolai Barybin via Devel wrote:
>>>> Den, Peter, Daniel thank you for your comments!
>>>>
>>>> I'm sending v2 of this patchset.
>>>>
>>>> Changes since last revision:
>>>>
>>>> - dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
>>>>
>>>> - added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
>>>>
>>>> - preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
>>>>
>>>> - enhanced requirements for allowing snapshotting. All writable disks
>>>>     should be qcow2, non-shared. If such disks exist and we have qcow2
>>>>     NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate
>>>>     to NVRAM
>>>>
>>>> - make char** wrdevs list allocation inside
>>>>     qemuSnapshotActiveInternalGetWrdevListHelper()
>>> Note that I'll clean up the patches, so there's no need for you to send
>>> another version. My review will follow regardless as documentation of
>>> what I'll be changing.
>>>
>> Peter, thank you for your review! Do you have ETA for these changes to
>> appear in master branch? We need them in our product.
> I'm halfway through fixing the last patch as I had other things to do as
> well.
>
> I'll be sending them out hopefully next week.


Hello Peter! Do you have any updates regarding merging this series?

P. S. Sorry, I forgot to forward previous message to devel@list and sent 
in personally to you