[libvirt PATCH 03/20] snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames

Pavel Hrdina posted 20 patches 1 year, 4 months ago
There is a newer version of this series
[libvirt PATCH 03/20] snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames
Posted by Pavel Hrdina 1 year, 4 months ago
Commit <ef3f3884a2432958bdd4ea0ce45509d47a91a453> introduced new
argument for virDomainSnapshotAlignDisks() that allows passing alternate
domain definition in case the snapshot parent.dom is NULL.

In case of redefining snapshot it will not hit the part of code that
unconditionally uses parent.dom as there will not be need to generate
default external file names.

It should be still fixed to make it safe. Future external snapshot
revert code will use this to generate default file names and in this
case it would crash.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/snapshot_conf.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 1329292293..da1c694cb9 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -489,12 +489,14 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDef *def,
 /**
  * virDomainSnapshotDefAssignExternalNames:
  * @def: snapshot def object
+ * @domdef: domain def object
  *
  * Generate default external file names for snapshot targets. Returns 0 on
  * success, -1 on error.
  */
 static int
-virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
+virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def,
+                                        virDomainDef *domdef)
 {
     const char *origpath;
     char *tmppath;
@@ -518,7 +520,7 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
             return -1;
         }
 
-        if (!(origpath = virDomainDiskGetSource(def->parent.dom->disks[i]))) {
+        if (!(origpath = virDomainDiskGetSource(domdef->disks[i]))) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("cannot generate external snapshot name "
                              "for disk '%s' without source"),
@@ -711,7 +713,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
     }
 
     /* Generate default external file names for external snapshot locations */
-    if (virDomainSnapshotDefAssignExternalNames(snapdef) < 0)
+    if (virDomainSnapshotDefAssignExternalNames(snapdef, domdef) < 0)
         return -1;
 
     return 0;
-- 
2.39.2
Re: [libvirt PATCH 03/20] snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames
Posted by Peter Krempa 1 year, 3 months ago
On Mon, Mar 13, 2023 at 16:42:04 +0100, Pavel Hrdina wrote:
> Commit <ef3f3884a2432958bdd4ea0ce45509d47a91a453> introduced new
> argument for virDomainSnapshotAlignDisks() that allows passing alternate
> domain definition in case the snapshot parent.dom is NULL.
> 
> In case of redefining snapshot it will not hit the part of code that
> unconditionally uses parent.dom as there will not be need to generate
> default external file names.
> 
> It should be still fixed to make it safe. Future external snapshot
> revert code will use this to generate default file names and in this
> case it would crash.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/conf/snapshot_conf.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

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