[libvirt PATCH v3 00/25] introduce external snapshot revert support

Pavel Hrdina posted 25 patches 11 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1692005543.git.phrdina@redhat.com
src/conf/schemas/domainsnapshot.rng |   7 +
src/conf/snapshot_conf.c            |  55 +-
src/conf/snapshot_conf.h            |  11 +-
src/conf/virdomainmomentobjlist.c   |  17 +
src/conf/virdomainmomentobjlist.h   |   4 +
src/libvirt_private.syms            |   6 +
src/qemu/qemu_backup.c              |   2 +-
src/qemu/qemu_blockjob.c            |   2 +-
src/qemu/qemu_domain.c              |   8 +-
src/qemu/qemu_domain.h              |   2 +-
src/qemu/qemu_snapshot.c            | 973 ++++++++++++++++++++++------
src/test/test_driver.c              |   2 +-
12 files changed, 886 insertions(+), 203 deletions(-)
[libvirt PATCH v3 00/25] introduce external snapshot revert support
Posted by Pavel Hrdina 11 months, 1 week ago
This implements virDomainRevertToSnapshot to work with external
snapshots. In addition it modifies virDomainSnapshotDelete to work
correctly when we revert to non-leaf snapshot or when there is
non-linear snapshot tree with multiple branches.

Gitlab repo with the patches:
https://gitlab.com/hrdina/libvirt/-/tree/snapshot-revert-external

changes in v3:

    - `revertdisks` is properly freed in virDomainSnapshotDefDispose()

    - qemuSnapshotCreateQcow2Files() no longer takes `reuse` as argument
      and was changed to take `virDomainDef *` instead of `virDomainObj *`

    - proper commit message for `qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT
      when reverting snapshot`

    - fixed incorrect usage of `ssize_t i`

    - dropped the weird logic from qemuSnapshotRevertExternalInactive() as
      we only need offline VM definition and preserve correct error message
      if creating qcow files fails

    - qemuSnapshotClearRevertdisks() correctly frees `revertdisks`

    - added new patches 'qemuDomainGetImageIds: pass domain definition directly`
      as we need to modify the function to take `virDomainDef *` directly

    - qemuSnapshotDiskHasBackingDisk() now uses qemuDomainGetImageIds() to get
      correct UID and GID for virStorageSourceGetMetadata() and also for
      virCommandRun() as well by storing it in
      `struct _qemuSnapshotDisksWithBackingStoreData`


Pavel Hrdina (25):
  libvirt_private: list virDomainMomentDefPostParse
  snapshot_conf: export virDomainSnapshotDiskDefClear
  snapshot_conf: use alternate domain definition in
    virDomainSnapshotDefAssignExternalNames
  snapshot_conf: introduce <revertDisks> metadata element
  virDomainSnapshotAlignDisks: Allow overriding user-configured snapshot
    default
  qemu_snapshot: introduce qemuSnapshotDomainDefUpdateDisk
  qemu_snapshot: use virDomainDiskByName while updating domain def
  qemu_snapshot: introduce qemuSnapshotCreateQcow2Files
  qemuSnapshotCreateQcow2Files: use domain definition directly
  qemu_snapshot: move external disk prepare to single function
  qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot
  qemu_snapshot: introduce external snapshot revert support
  qemu_snapshot: rename qemuSnapshotDeleteExternalPrepare
  qemu_snapshot: extract external snapshot delete prepare to function
  qemu_snapshot: add merge to external snapshot delete prepare data
  qemu_snapshot: prepare data for non-active leaf external snapshot
    deletion
  qemu_snapshot: add support to delete external snapshot without block
    commit
  qemu_snapshot: delete: properly update parent snapshot with revert
    data
  qemu_snapshot: remove revertdisks when creating new snapshot
  virdomainmomentobjlist: introduce virDomainMomentIsAncestor
  qemuDomainGetImageIds: pass domain definition directly
  qemu_snapshot: update backing store after deleting external snapshot
  qemu_snapshot: check only once if snapshot is external
  qemu_snapshot: add checks for external snapshot deletion
  qemu_snapshot: allow snapshot revert for external snapshots

 src/conf/schemas/domainsnapshot.rng |   7 +
 src/conf/snapshot_conf.c            |  55 +-
 src/conf/snapshot_conf.h            |  11 +-
 src/conf/virdomainmomentobjlist.c   |  17 +
 src/conf/virdomainmomentobjlist.h   |   4 +
 src/libvirt_private.syms            |   6 +
 src/qemu/qemu_backup.c              |   2 +-
 src/qemu/qemu_blockjob.c            |   2 +-
 src/qemu/qemu_domain.c              |   8 +-
 src/qemu/qemu_domain.h              |   2 +-
 src/qemu/qemu_snapshot.c            | 973 ++++++++++++++++++++++------
 src/test/test_driver.c              |   2 +-
 12 files changed, 886 insertions(+), 203 deletions(-)

-- 
2.41.0
Re: [libvirt PATCH v3 00/25] introduce external snapshot revert support
Posted by Peter Krempa 11 months ago
On Mon, Aug 14, 2023 at 11:35:52 +0200, Pavel Hrdina wrote:
> This implements virDomainRevertToSnapshot to work with external
> snapshots. In addition it modifies virDomainSnapshotDelete to work
> correctly when we revert to non-leaf snapshot or when there is
> non-linear snapshot tree with multiple branches.
> 
> Gitlab repo with the patches:
> https://gitlab.com/hrdina/libvirt/-/tree/snapshot-revert-external
> 
> changes in v3:
> 
>     - `revertdisks` is properly freed in virDomainSnapshotDefDispose()
> 
>     - qemuSnapshotCreateQcow2Files() no longer takes `reuse` as argument
>       and was changed to take `virDomainDef *` instead of `virDomainObj *`
> 
>     - proper commit message for `qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT
>       when reverting snapshot`
> 
>     - fixed incorrect usage of `ssize_t i`
> 
>     - dropped the weird logic from qemuSnapshotRevertExternalInactive() as
>       we only need offline VM definition and preserve correct error message
>       if creating qcow files fails
> 
>     - qemuSnapshotClearRevertdisks() correctly frees `revertdisks`
> 
>     - added new patches 'qemuDomainGetImageIds: pass domain definition directly`
>       as we need to modify the function to take `virDomainDef *` directly
> 
>     - qemuSnapshotDiskHasBackingDisk() now uses qemuDomainGetImageIds() to get
>       correct UID and GID for virStorageSourceGetMetadata() and also for
>       virCommandRun() as well by storing it in
>       `struct _qemuSnapshotDisksWithBackingStoreData`

Reviewed-by: Peter Krempa <pkrempa@redhat.com>