Add the VIR_DOMAIN_DEF_COPY_NEWGENID to indicate that the generated
domain definition should adjust the genid value before returning the
@def to the caller.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
src/conf/domain_conf.c | 12 ++++++++++++
src/conf/domain_conf.h | 5 +++++
src/qemu/qemu_driver.c | 3 ++-
src/test/test_driver.c | 3 ++-
4 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1411034e7c..6d4db50998 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -27909,6 +27909,18 @@ virDomainDefCopy(virDomainDefPtr src,
ret = virDomainDefParseString(xml, caps, xmlopt, parseOpaque, parse_flags);
+ /* If we have a genid and we're being called from a path that would
+ * require a different genid value, then regardless of whether it was
+ * generated or not generate a new one. */
+ if (ret && ret->genidRequested && (flags & VIR_DOMAIN_DEF_COPY_NEWGENID)) {
+ if (virUUIDGenerate(ret->genid)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to generate a new genid"));
+ virDomainDefFree(ret);
+ ret = NULL;
+ }
+ }
+
VIR_FREE(xml);
return ret;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1b61fd8c03..910b3ca4d1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2875,6 +2875,11 @@ virDomainDefPtr virDomainObjGetOneDef(virDomainObjPtr vm, unsigned int flags);
typedef enum {
/* Set when creating a copy of a definition for the purpose of migration */
VIR_DOMAIN_DEF_COPY_MIGRATABLE = 1 << 0,
+
+ /* Set when the copy should create a new genid value if supported
+ * the domain def.
+ */
+ VIR_DOMAIN_DEF_COPY_NEWGENID = 1 << 1,
} virDomainDefCopyFlags;
virDomainDefPtr virDomainDefCopy(virDomainDefPtr src,
virCapsPtr caps,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 61e49bea24..bfb7973100 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15858,7 +15858,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (snap->def->dom) {
config = virDomainDefCopy(snap->def->dom, caps,
driver->xmlopt, NULL,
- VIR_DOMAIN_DEF_COPY_MIGRATABLE);
+ VIR_DOMAIN_DEF_COPY_MIGRATABLE |
+ VIR_DOMAIN_DEF_COPY_NEWGENID);
if (!config)
goto endjob;
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 8877a467f6..7bac017e1a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6690,7 +6690,8 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
snap->def->current = true;
config = virDomainDefCopy(snap->def->dom, privconn->caps,
privconn->xmlopt, NULL,
- VIR_DOMAIN_DEF_COPY_MIGRATABLE);
+ VIR_DOMAIN_DEF_COPY_MIGRATABLE |
+ VIR_DOMAIN_DEF_COPY_NEWGENID);
if (!config)
goto cleanup;
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Apr 23, 2018 at 19:59:59 -0400, John Ferlan wrote: > Add the VIR_DOMAIN_DEF_COPY_NEWGENID to indicate that the generated > domain definition should adjust the genid value before returning the > @def to the caller. > > Signed-off-by: John Ferlan <jferlan@redhat.com> > --- > src/conf/domain_conf.c | 12 ++++++++++++ > src/conf/domain_conf.h | 5 +++++ > src/qemu/qemu_driver.c | 3 ++- > src/test/test_driver.c | 3 ++- > 4 files changed, 21 insertions(+), 2 deletions(-) IMO all this code should not be necessary. Code paths which knowingly need to change the GENID should do so explicitly rather than pulling the logic here. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/24/2018 03:24 AM, Peter Krempa wrote: > On Mon, Apr 23, 2018 at 19:59:59 -0400, John Ferlan wrote: >> Add the VIR_DOMAIN_DEF_COPY_NEWGENID to indicate that the generated >> domain definition should adjust the genid value before returning the >> @def to the caller. >> >> Signed-off-by: John Ferlan <jferlan@redhat.com> >> --- >> src/conf/domain_conf.c | 12 ++++++++++++ >> src/conf/domain_conf.h | 5 +++++ >> src/qemu/qemu_driver.c | 3 ++- >> src/test/test_driver.c | 3 ++- >> 4 files changed, 21 insertions(+), 2 deletions(-) > > IMO all this code should not be necessary. Code paths which knowingly > need to change the GENID should do so explicitly rather than pulling the > logic here. > Honestly, I found it easier to have the code in one place rather than cut-copy-paste in two places... But I can move it out. However, I may have forgotten a transition though w/r/t domain copy. Unfortunately clone could be a bugger since it uses parse and format, but if startup always generates one, then that's perhaps covered. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 24, 2018 at 14:13:15 -0400, John Ferlan wrote: > > > On 04/24/2018 03:24 AM, Peter Krempa wrote: > > On Mon, Apr 23, 2018 at 19:59:59 -0400, John Ferlan wrote: > >> Add the VIR_DOMAIN_DEF_COPY_NEWGENID to indicate that the generated > >> domain definition should adjust the genid value before returning the > >> @def to the caller. > >> > >> Signed-off-by: John Ferlan <jferlan@redhat.com> > >> --- > >> src/conf/domain_conf.c | 12 ++++++++++++ > >> src/conf/domain_conf.h | 5 +++++ > >> src/qemu/qemu_driver.c | 3 ++- > >> src/test/test_driver.c | 3 ++- > >> 4 files changed, 21 insertions(+), 2 deletions(-) > > > > IMO all this code should not be necessary. Code paths which knowingly > > need to change the GENID should do so explicitly rather than pulling the > > logic here. > > > > Honestly, I found it easier to have the code in one place rather than > cut-copy-paste in two places... But I can move it out. The problem is that the copying of domain definition should not attempt any smarts. The caller expects a copy, so it should be the same. If the caller wishes to use the definition in a different way it needs to be modified afterwards. > However, I may have forgotten a transition though w/r/t domain copy. > Unfortunately clone could be a bugger since it uses parse and format, > but if startup always generates one, then that's perhaps covered. Copying of the domain definition should always copy the GUID along with the flag whether it was autogenerated or not. You may need a XML attribute for that. Otherwise you will not be able to differentiate the situation when the GUID was provided by the user, or when it was autogenerated but needs to be used as-is. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/25/2018 03:42 AM, Peter Krempa wrote: > On Tue, Apr 24, 2018 at 14:13:15 -0400, John Ferlan wrote: >> >> >> On 04/24/2018 03:24 AM, Peter Krempa wrote: >>> On Mon, Apr 23, 2018 at 19:59:59 -0400, John Ferlan wrote: >>>> Add the VIR_DOMAIN_DEF_COPY_NEWGENID to indicate that the generated >>>> domain definition should adjust the genid value before returning the >>>> @def to the caller. >>>> >>>> Signed-off-by: John Ferlan <jferlan@redhat.com> >>>> --- >>>> src/conf/domain_conf.c | 12 ++++++++++++ >>>> src/conf/domain_conf.h | 5 +++++ >>>> src/qemu/qemu_driver.c | 3 ++- >>>> src/test/test_driver.c | 3 ++- >>>> 4 files changed, 21 insertions(+), 2 deletions(-) >>> >>> IMO all this code should not be necessary. Code paths which knowingly >>> need to change the GENID should do so explicitly rather than pulling the >>> logic here. >>> >> >> Honestly, I found it easier to have the code in one place rather than >> cut-copy-paste in two places... But I can move it out. > > The problem is that the copying of domain definition should not attempt > any smarts. The caller expects a copy, so it should be the same. If the > caller wishes to use the definition in a different way it needs to be > modified afterwards. > That ship already sailed by passing migratable, hence why it seemed fine to use the function. But like I said, I'll move it out. Although from this discussion it appears parseOpaque could be unnecessary - all callers pass NULL in the 4th parameter. Still it could be well enough hidden in some other way... John >> However, I may have forgotten a transition though w/r/t domain copy. >> Unfortunately clone could be a bugger since it uses parse and format, >> but if startup always generates one, then that's perhaps covered. > > Copying of the domain definition should always copy the GUID along with > the flag whether it was autogenerated or not. You may need a XML > attribute for that. Otherwise you will not be able to differentiate the > situation when the GUID was provided by the user, or when it was > autogenerated but needs to be used as-is. > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.