[libvirt] [PATCH 01/35] storage: Properly track that backing chain members are readonly

Peter Krempa posted 35 patches 7 years ago
[libvirt] [PATCH 01/35] storage: Properly track that backing chain members are readonly
Posted by Peter Krempa 7 years ago
Everything besides the top of the chain is readonly. Track this when
parsing the XML and detecting the chain from the disk. Also fix the
state when taking snapshots.

All other cases where the top image is changed already preserve the
readonly state from the original image.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c    | 3 +++
 src/qemu/qemu_driver.c    | 3 +++
 src/util/virstoragefile.c | 1 +
 3 files changed, 7 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b0257068da..21a6c4785a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8734,6 +8734,9 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
     if (VIR_ALLOC(backingStore) < 0)
         goto cleanup;

+    /* backing store is always read-only */
+    backingStore->readonly = true;
+
     /* terminator does not have a type */
     if (!(type = virXMLPropString(ctxt->node, "type"))) {
         VIR_STEAL_PTR(src->backingStore, backingStore);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7484b00e23..542c625962 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14670,6 +14670,9 @@ qemuDomainSnapshotUpdateDiskSources(qemuDomainSnapshotDiskDataPtr dd,
     if (dd->initialized)
         virStorageFileDeinit(dd->src);

+    /* the old disk image is now readonly */
+    dd->disk->src->readonly = true;
+
     VIR_STEAL_PTR(dd->disk->src->relPath, dd->relPath);
     VIR_STEAL_PTR(dd->src->backingStore, dd->disk->src);
     VIR_STEAL_PTR(dd->disk->src, dd->src);
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 531540ac91..d6ad6e1d0f 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3425,6 +3425,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent)
         if (virStorageSourceInitChainElement(ret, parent, true) < 0)
             goto error;

+        ret->readonly = true;
         ret->detected = true;
     }

-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/35] storage: Properly track that backing chain members are readonly
Posted by John Ferlan 7 years ago

On 04/25/2018 11:15 AM, Peter Krempa wrote:
> Everything besides the top of the chain is readonly. Track this when
> parsing the XML and detecting the chain from the disk. Also fix the
> state when taking snapshots.
> 
> All other cases where the top image is changed already preserve the
> readonly state from the original image.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/conf/domain_conf.c    | 3 +++
>  src/qemu/qemu_driver.c    | 3 +++
>  src/util/virstoragefile.c | 1 +
>  3 files changed, 7 insertions(+)
> 

Any concerns with virDomainDiskDefCheckABIStability?  Since true would
now be the default - is it possible a save file comparison causes issues?

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/35] storage: Properly track that backing chain members are readonly
Posted by Peter Krempa 7 years ago
On Tue, May 01, 2018 at 20:06:40 -0400, John Ferlan wrote:
> 
> 
> On 04/25/2018 11:15 AM, Peter Krempa wrote:
> > Everything besides the top of the chain is readonly. Track this when
> > parsing the XML and detecting the chain from the disk. Also fix the
> > state when taking snapshots.
> > 
> > All other cases where the top image is changed already preserve the
> > readonly state from the original image.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/conf/domain_conf.c    | 3 +++
> >  src/qemu/qemu_driver.c    | 3 +++
> >  src/util/virstoragefile.c | 1 +
> >  3 files changed, 7 insertions(+)
> > 
> 
> Any concerns with virDomainDiskDefCheckABIStability?  Since true would
> now be the default - is it possible a save file comparison causes issues?

The ABI stability is checked only for the 'readonly' attribute of the
top layer, since that may have implications visible from the guest. The
rest of the backing chain definitely is not ABI.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list