For the reason outlined in previous commit qemu doesn't do this
automatically. Handle it manually after the snapshot.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_snapshot.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 5b3aadcbf0..f880d1eeec 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1434,12 +1434,14 @@ qemuSnapshotGetTransientDiskDef(virDomainDiskDef *domdisk,
  * qemuSnapshotDiskUpdateSource:
  * @vm: domain object
  * @dd: snapshot disk data object
+ * @asyncJob: async job type
  *
  * Updates disk definition after a successful snapshot.
  */
 static void
 qemuSnapshotDiskUpdateSource(virDomainObj *vm,
-                             qemuSnapshotDiskData *dd)
+                             qemuSnapshotDiskData *dd,
+                             virDomainAsyncJob asyncJob)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     virQEMUDriver *driver = priv->driver;
@@ -1451,6 +1453,11 @@ qemuSnapshotDiskUpdateSource(virDomainObj *vm,
     if (qemuSecurityMoveImageMetadata(driver, vm, dd->disk->src, dd->src) < 0)
         VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name);

+    /* if the original image has a data-file turn it read-only */
+    if (dd->disk->src->dataFileStore) {
+        ignore_value(qemuBlockReopenReadOnly(vm, dd->disk->src->dataFileStore, asyncJob));
+    }
+
     /* unlock the write lock on the original image as qemu will no longer write to it */
     virDomainLockImageDetach(driver->lockManager, vm, dd->disk->src);

@@ -1470,6 +1477,7 @@ qemuSnapshotDiskUpdateSource(virDomainObj *vm,
         dd->persistsrc->backingStore = g_steal_pointer(&dd->persistdisk->src);
         dd->persistdisk->src = g_steal_pointer(&dd->persistsrc);
     }
+
 }


@@ -1498,7 +1506,7 @@ qemuSnapshotDiskCreate(qemuSnapshotDiskContext *snapctxt)
         virDomainAuditDisk(snapctxt->vm, dd->disk->src, dd->src, "snapshot", rc >= 0);

         if (rc == 0)
-            qemuSnapshotDiskUpdateSource(snapctxt->vm, dd);
+            qemuSnapshotDiskUpdateSource(snapctxt->vm, dd, snapctxt->asyncJob);
     }

     if (rc < 0)
-- 
2.47.0

On Tue, Nov 26, 2024 at 16:16:16 +0100, Peter Krempa wrote:
> For the reason outlined in previous commit qemu doesn't do this
> automatically. Handle it manually after the snapshot.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index 5b3aadcbf0..f880d1eeec 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
...
> @@ -1470,6 +1477,7 @@ qemuSnapshotDiskUpdateSource(virDomainObj *vm,
>          dd->persistsrc->backingStore = g_steal_pointer(&dd->persistdisk->src);
>          dd->persistdisk->src = g_steal_pointer(&dd->persistsrc);
>      }
> +
>  }

Looks like an accidental change, which was not supposed to be included in
the commit.

Jirka

On Thu, Nov 28, 2024 at 10:16:10 +0100, Jiri Denemark wrote:
> On Tue, Nov 26, 2024 at 16:16:16 +0100, Peter Krempa wrote:
> > For the reason outlined in previous commit qemu doesn't do this
> > automatically. Handle it manually after the snapshot.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/qemu/qemu_snapshot.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> > index 5b3aadcbf0..f880d1eeec 100644
> > --- a/src/qemu/qemu_snapshot.c
> > +++ b/src/qemu/qemu_snapshot.c
> ...
> > @@ -1470,6 +1477,7 @@ qemuSnapshotDiskUpdateSource(virDomainObj *vm,
> >          dd->persistsrc->backingStore = g_steal_pointer(&dd->persistdisk->src);
> >          dd->persistdisk->src = g_steal_pointer(&dd->persistsrc);
> >      }
> > +
> >  }
> 
> Looks like an accidental change, which was not supposed to be included in
> the commit.

Indeed. It was the place where I put the code originally, later
realizing that it doesn't make sense :)