Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On Mon, Apr 23, 2018 at 20:00:00 -0400, John Ferlan wrote: > The VM Generation ID is a mechanism to provide a unique 128-bit, > cryptographically random, and integer value identifier known as > the GUID (Globally Unique Identifier) to the guest OS. The value > is used to help notify the guest operating system when the virtual > machine is executed with a different configuration. > > This patch adds support for a new "genid" XML element similar to > the "uuid" element. The "genid" element can have two forms "" > or "$GUID". If the $GUID is not provided, libvirt > will generate one. > > For the ABI checks add avoidance for the genid comparison if the > appropriate flag is set. > > Since adding support for a generated GUID (or UUID like) value to > be displayed only for an active guest, modifying the xml2xml test > to include virrandommock.so is necessary since it will generate a > "known" UUID value that can be compared against for the active test. > > Signed-off-by: John Ferlan > --- > docs/formatdomain.html.in| 29 > docs/schemas/domaincommon.rng| 8 > src/conf/domain_conf.c | 59 > > src/conf/domain_conf.h | 3 ++ > tests/qemuxml2argvdata/genid-auto.xml| 32 + > tests/qemuxml2argvdata/genid.xml | 32 + > tests/qemuxml2xmloutdata/genid-active.xml| 32 + > tests/qemuxml2xmloutdata/genid-auto-active.xml | 32 + > tests/qemuxml2xmloutdata/genid-auto-inactive.xml | 32 + > tests/qemuxml2xmloutdata/genid-inactive.xml | 32 + > tests/qemuxml2xmltest.c | 5 +- > 11 files changed, 295 insertions(+), 1 deletion(-) > create mode 100644 tests/qemuxml2argvdata/genid-auto.xml > create mode 100644 tests/qemuxml2argvdata/genid.xml > create mode 100644 tests/qemuxml2xmloutdata/genid-active.xml > create mode 100644 tests/qemuxml2xmloutdata/genid-auto-active.xml > create mode 100644 tests/qemuxml2xmloutdata/genid-auto-inactive.xml > create mode 100644 tests/qemuxml2xmloutdata/genid-inactive.xml > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index ada0df227f..6a140f3e40 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -34,6 +34,7 @@ >> MyGuest >4dea22b3-1d52-d8f3-2516-782e98ab3fa0 > +43dc0cf8-809b-4adb-9bea-a9abb5f3d90e Since we encourage to use the variant with this being empty I'd show that here. >A short description - title - of the domain >Some human readable description >> @@ -61,6 +62,34 @@ > specification. Since 0.0.1, sysinfo > since 0.8.7 > > + genid > + Since 4.3.0, the genid > +element can be used to add a Virtual Machine Generation ID which > +exposes a 128-bit, cryptographically random, integer value > identifier, > +referred to as a Globally Unique Identifier (GUID) using the same > +format as the uuid. The value is used to help notify > +the guest operating system when the virtual machine is executed > +with a different configuration, such as: > + > + > + snapshot execution > + backup recovery > + failover in a disaster recovery environment > + creation from template (import, copy, clone) > + > + > +The guest operating system notices the change and is then able to > +react as appropriate by marking its copies of distributed databases > +as dirty, re-initializing its random number generator, etc. > + > + > +When a GUID value is not provided, e.g. using the XML syntax > + , then libvirt will automatically generate a GUID. > +This is the recommended configuration since the hypervisor then > +can handle changing the GUID value for specific state transitions. > +Using a static GUID value may result in failures for starting from > +snapshot, restoring from backup, starting a cloned domain, > etc. > + >title >The optional element title provides space for a > short description of the domain. The title should not contain [...] > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 6d4db50998..8c30adf850 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -18793,6 +18793,34 @@ virDomainDefParseXML(xmlDocPtr xml, > VIR_FREE(tmp); > } > > +/* Extract domain genid - a genid can either be provided or generated */ > +if ((n = virXPathNodeSet("./genid", ctxt, &nodes)) < 0) > +goto error; > + > +if (n > 0) { > +if (n != 1) { > +virReportError(VIR_ERR_XML_ERROR, "%s", > + _("eleme
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On 04/24/2018 03:21 AM, Peter Krempa wrote: > On Mon, Apr 23, 2018 at 20:00:00 -0400, John Ferlan wrote: >> The VM Generation ID is a mechanism to provide a unique 128-bit, >> cryptographically random, and integer value identifier known as >> the GUID (Globally Unique Identifier) to the guest OS. The value >> is used to help notify the guest operating system when the virtual >> machine is executed with a different configuration. >> >> This patch adds support for a new "genid" XML element similar to >> the "uuid" element. The "genid" element can have two forms "" >> or "$GUID". If the $GUID is not provided, libvirt >> will generate one. >> >> For the ABI checks add avoidance for the genid comparison if the >> appropriate flag is set. >> >> Since adding support for a generated GUID (or UUID like) value to >> be displayed only for an active guest, modifying the xml2xml test >> to include virrandommock.so is necessary since it will generate a >> "known" UUID value that can be compared against for the active test. >> >> Signed-off-by: John Ferlan >> --- >> docs/formatdomain.html.in| 29 >> docs/schemas/domaincommon.rng| 8 >> src/conf/domain_conf.c | 59 >> >> src/conf/domain_conf.h | 3 ++ >> tests/qemuxml2argvdata/genid-auto.xml| 32 + >> tests/qemuxml2argvdata/genid.xml | 32 + >> tests/qemuxml2xmloutdata/genid-active.xml| 32 + >> tests/qemuxml2xmloutdata/genid-auto-active.xml | 32 + >> tests/qemuxml2xmloutdata/genid-auto-inactive.xml | 32 + >> tests/qemuxml2xmloutdata/genid-inactive.xml | 32 + >> tests/qemuxml2xmltest.c | 5 +- >> 11 files changed, 295 insertions(+), 1 deletion(-) >> create mode 100644 tests/qemuxml2argvdata/genid-auto.xml >> create mode 100644 tests/qemuxml2argvdata/genid.xml >> create mode 100644 tests/qemuxml2xmloutdata/genid-active.xml >> create mode 100644 tests/qemuxml2xmloutdata/genid-auto-active.xml >> create mode 100644 tests/qemuxml2xmloutdata/genid-auto-inactive.xml >> create mode 100644 tests/qemuxml2xmloutdata/genid-inactive.xml >> >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in >> index ada0df227f..6a140f3e40 100644 >> --- a/docs/formatdomain.html.in >> +++ b/docs/formatdomain.html.in >> @@ -34,6 +34,7 @@ >>>> MyGuest >>4dea22b3-1d52-d8f3-2516-782e98ab3fa0 >> +43dc0cf8-809b-4adb-9bea-a9abb5f3d90e > > Since we encourage to use the variant with this being empty I'd show > that here. > I'd agree except for the fact this is a "live" XML example since "id='1'", so it should stay as is unless of course it's desired to never show the GUID for the running VM. >>A short description - title - of the domain >>Some human readable description >>>> @@ -61,6 +62,34 @@ >> specification. Since 0.0.1, sysinfo >> since 0.8.7 >> >> + genid >> + Since 4.3.0, the genid >> +element can be used to add a Virtual Machine Generation ID which >> +exposes a 128-bit, cryptographically random, integer value >> identifier, >> +referred to as a Globally Unique Identifier (GUID) using the same >> +format as the uuid. The value is used to help notify >> +the guest operating system when the virtual machine is executed >> +with a different configuration, such as: >> + >> + >> + snapshot execution >> + backup recovery >> + failover in a disaster recovery environment >> + creation from template (import, copy, clone) >> + >> + >> +The guest operating system notices the change and is then able to >> +react as appropriate by marking its copies of distributed databases >> +as dirty, re-initializing its random number generator, etc. >> + >> + >> +When a GUID value is not provided, e.g. using the XML syntax >> + , then libvirt will automatically generate a GUID. >> +This is the recommended configuration since the hypervisor then >> +can handle changing the GUID value for specific state transitions. >> +Using a static GUID value may result in failures for starting from >> +snapshot, restoring from backup, starting a cloned domain, >> etc. >> + >>title >>The optional element title provides space for a >> short description of the domain. The title should not contain > > [...] > >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 6d4db50998..8c30adf850 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -18793,6 +18793,34 @@ virDomainDefParseXML(xmlDocPtr xml, >> VIR_FREE
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On Tue, Apr 24, 2018 at 15:38:40 -0400, John Ferlan wrote: > > > On 04/24/2018 03:21 AM, Peter Krempa wrote: > > On Mon, Apr 23, 2018 at 20:00:00 -0400, John Ferlan wrote: > >> The VM Generation ID is a mechanism to provide a unique 128-bit, > >> cryptographically random, and integer value identifier known as > >> the GUID (Globally Unique Identifier) to the guest OS. The value > >> is used to help notify the guest operating system when the virtual > >> machine is executed with a different configuration. > >> > >> This patch adds support for a new "genid" XML element similar to > >> the "uuid" element. The "genid" element can have two forms "" > >> or "$GUID". If the $GUID is not provided, libvirt > >> will generate one. > >> > >> For the ABI checks add avoidance for the genid comparison if the > >> appropriate flag is set. > >> > >> Since adding support for a generated GUID (or UUID like) value to > >> be displayed only for an active guest, modifying the xml2xml test > >> to include virrandommock.so is necessary since it will generate a > >> "known" UUID value that can be compared against for the active test. > >> > >> Signed-off-by: John Ferlan > >> --- > >> docs/formatdomain.html.in| 29 > >> docs/schemas/domaincommon.rng| 8 > >> src/conf/domain_conf.c | 59 > >> > >> src/conf/domain_conf.h | 3 ++ > >> tests/qemuxml2argvdata/genid-auto.xml| 32 + > >> tests/qemuxml2argvdata/genid.xml | 32 + > >> tests/qemuxml2xmloutdata/genid-active.xml| 32 + > >> tests/qemuxml2xmloutdata/genid-auto-active.xml | 32 + > >> tests/qemuxml2xmloutdata/genid-auto-inactive.xml | 32 + > >> tests/qemuxml2xmloutdata/genid-inactive.xml | 32 + > >> tests/qemuxml2xmltest.c | 5 +- > >> 11 files changed, 295 insertions(+), 1 deletion(-) > >> create mode 100644 tests/qemuxml2argvdata/genid-auto.xml > >> create mode 100644 tests/qemuxml2argvdata/genid.xml > >> create mode 100644 tests/qemuxml2xmloutdata/genid-active.xml > >> create mode 100644 tests/qemuxml2xmloutdata/genid-auto-active.xml > >> create mode 100644 tests/qemuxml2xmloutdata/genid-auto-inactive.xml > >> create mode 100644 tests/qemuxml2xmloutdata/genid-inactive.xml > >> > >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > >> index ada0df227f..6a140f3e40 100644 > >> --- a/docs/formatdomain.html.in > >> +++ b/docs/formatdomain.html.in > >> @@ -34,6 +34,7 @@ > >>> >> MyGuest > >>4dea22b3-1d52-d8f3-2516-782e98ab3fa0 > >> +43dc0cf8-809b-4adb-9bea-a9abb5f3d90e > > > > Since we encourage to use the variant with this being empty I'd show > > that here. > > > > I'd agree except for the fact this is a "live" XML example since > "id='1'", so it should stay as is unless of course it's desired to never > show the GUID for the running VM. Hmm, right. It feels slightly wrong though that we are describing configuration on the example of a live XML. > > >>A short description - title - of the domain > >>Some human readable description > >>[...] > >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > >> index 6d4db50998..8c30adf850 100644 > >> --- a/src/conf/domain_conf.c > >> +++ b/src/conf/domain_conf.c > >> @@ -18793,6 +18793,34 @@ virDomainDefParseXML(xmlDocPtr xml, > >> VIR_FREE(tmp); > >> } > >> > >> +/* Extract domain genid - a genid can either be provided or generated > >> */ > >> +if ((n = virXPathNodeSet("./genid", ctxt, &nodes)) < 0) > >> +goto error; > >> + > >> +if (n > 0) { > >> +if (n != 1) { > >> +virReportError(VIR_ERR_XML_ERROR, "%s", > >> + _("element 'genid' can only appear once")); > >> +goto error; > >> +} > >> +def->genidRequested = true; > >> +if (!(tmp = virXPathString("string(./genid[1])", ctxt))) { > >> +if (virUUIDGenerate(def->genid) < 0) { > >> +virReportError(VIR_ERR_INTERNAL_ERROR, > >> + "%s", _("Failed to generate genid")); > > > > Generating this here is pointless, the code using it throws this away > > and generates a new one. While we don't show this value to the user and > > thus don't create any false impressions I think that the logic should be > > shuffled around so that it's generated only when it's required. > > > > > > How so? Not generating one here would cause active xml-2-xml to fail (as Basically because a GUID for an inactive definition is nonsense. It will necessarily change upon startup, so having it generated in the parser is pointless. The parser only needs to restore a user-provided
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: > > Well, that depends. I did not read the docs for this thoroughly enough. > If it is okay for us to generate a new GUID upon every boot of a VM then > this will be for a rather simple implementation, since we have a very > limited set of situations when we are starting a new qemu process which > should NOT change the GUID and we will change it in all other scenarios. AFAIK, we *must* change GUID on every cold boot > A second consideration then is whether to allow user-specified GUID at > all, but I guess mgmt applications may have a different idea when it's > necessary to change and thus it makes sense to allow that. For that > situation the provided GUID should be always honoured. The microsoft spec describes exactly why GUID must change, and mgmt applications must not deviate from those rules to make up their own. So the question is not whether the mgmt app has a different idea of when to change GUID. Rather, we should consider whether there is any situation in which it is impossible for libvirt todo the right thing according to the microsoft spec. If libvirt has enough knowledge to always do the right thing, then we could refuse to make it user configurable - just report what is set. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On Wed, Apr 25, 2018 at 09:39:49 +0100, Daniel Berrange wrote: > On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: > > > > Well, that depends. I did not read the docs for this thoroughly enough. > > If it is okay for us to generate a new GUID upon every boot of a VM then > > this will be for a rather simple implementation, since we have a very > > limited set of situations when we are starting a new qemu process which > > should NOT change the GUID and we will change it in all other scenarios. > > AFAIK, we *must* change GUID on every cold boot Good, that makes things rather simple. > > A second consideration then is whether to allow user-specified GUID at > > all, but I guess mgmt applications may have a different idea when it's > > necessary to change and thus it makes sense to allow that. For that > > situation the provided GUID should be always honoured. > > The microsoft spec describes exactly why GUID must change, and mgmt > applications must not deviate from those rules to make up their own. > > So the question is not whether the mgmt app has a different idea of > when to change GUID. Rather, we should consider whether there is any > situation in which it is impossible for libvirt todo the right thing > according to the microsoft spec. > > If libvirt has enough knowledge to always do the right thing, then > we could refuse to make it user configurable - just report what is > set. The only thing that comes into my mind is the non-managed save/restore case. In that scenario both options make somewhat sense and libvirt can't really tell which is the case. That can be handled with an API flag though, since we can use the GUID stored in the save image. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On 04/25/2018 04:46 AM, Peter Krempa wrote: > On Wed, Apr 25, 2018 at 09:39:49 +0100, Daniel Berrange wrote: >> On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: >>> >>> Well, that depends. I did not read the docs for this thoroughly enough. >>> If it is okay for us to generate a new GUID upon every boot of a VM then >>> this will be for a rather simple implementation, since we have a very >>> limited set of situations when we are starting a new qemu process which >>> should NOT change the GUID and we will change it in all other scenarios. >> >> AFAIK, we *must* change GUID on every cold boot > > Good, that makes things rather simple. > This one is not really 100% clear to me. The "spec" is like 6 pages - it's a pretty quick read... http://go.microsoft.com/fwlink/?LinkId=260709 The last 2 pages describe "when" the GUID should change and specifically calling out cold boot is not one of those. What leads me to believe otherwise is the two boot scenarios and the unspecified VM config changes have this "undertone" that using the same GUID for those scenarios is fine/expected. I really don't want to follow the ~20 attempts to get genid into QEMU - I'll give up way before then ;-)! >>> A second consideration then is whether to allow user-specified GUID at >>> all, but I guess mgmt applications may have a different idea when it's >>> necessary to change and thus it makes sense to allow that. For that >>> situation the provided GUID should be always honoured. >> >> The microsoft spec describes exactly why GUID must change, and mgmt >> applications must not deviate from those rules to make up their own. >> >> So the question is not whether the mgmt app has a different idea of >> when to change GUID. Rather, we should consider whether there is any >> situation in which it is impossible for libvirt todo the right thing >> according to the microsoft spec. >> >> If libvirt has enough knowledge to always do the right thing, then >> we could refuse to make it user configurable - just report what is >> set. Not sure we can "decide" to not make it user configurable, but then again it's also not specifically stated from how I read things. > > The only thing that comes into my mind is the non-managed save/restore > case. In that scenario both options make somewhat sense and libvirt > can't really tell which is the case. > > That can be handled with an API flag though, since we can use the GUID > stored in the save image. > The spec says : "Restoring from backup – Restoring a backup image will cause a previous workload time interval to be re-executed. Upon restore, the components of the backup are enumerated and replaced on the restore target by the VSS system. The affected configuration files are simply copied and not re-realized on the host. The restore sequence will be modified to post process the restore target and apply new generation identifiers to the restored configuration files." I read that as change the GUID and did so during startup processing from the qemuDomainSaveImageStartVM. Although I suppose it could also be read as altering the GUID before the virDomainObjListAdd in qemuDomainRestoreFlags and before the virDomainObjAssignDef in qemuDomainObjRestore. Since both called the latter and the running config for which the GUID is formatted is saved in the latter it "felt reasonable" to not have duplicated code. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On Wed, Apr 25, 2018 at 08:02:35AM -0400, John Ferlan wrote: > > > On 04/25/2018 04:46 AM, Peter Krempa wrote: > > On Wed, Apr 25, 2018 at 09:39:49 +0100, Daniel Berrange wrote: > >> On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: > >>> > >>> Well, that depends. I did not read the docs for this thoroughly enough. > >>> If it is okay for us to generate a new GUID upon every boot of a VM then > >>> this will be for a rather simple implementation, since we have a very > >>> limited set of situations when we are starting a new qemu process which > >>> should NOT change the GUID and we will change it in all other scenarios. > >> > >> AFAIK, we *must* change GUID on every cold boot > > > > Good, that makes things rather simple. > > > > This one is not really 100% clear to me. The "spec" is like 6 pages - > it's a pretty quick read... > > http://go.microsoft.com/fwlink/?LinkId=260709 > > The last 2 pages describe "when" the GUID should change and specifically > calling out cold boot is not one of those. What leads me to believe > otherwise is the two boot scenarios and the unspecified VM config > changes have this "undertone" that using the same GUID for those > scenarios is fine/expected. Yeah the table at the very end is the key bit and it seems I was actually wrong Scenario GenID changed --- Virtual machine is paused or resumed No Virtual machine reboots No Virtual machine host reboots No Virtual machine starts executing a snapshot Yes Virtual machine is recovered from backup Yes Virtual machine is failed over in a disaster recovery env Yes Virtual machine is live migrated No Virtual machine is imported, copied, or clonedYes Virtual machine is failed over in a clustered environment No Virtual machine's configuration changes Unspecified My reading of "Virtual machine reboots" and "Virtual machine host reboots" rows in particular is that we can *NOT* change GUID on every boot up operation. The spec literally only wants it to be changed when there is the possibility that the VM is potentially re-executing something that has already been executed before. The transient VM feature is the real killer for libvirt - we have no way of knowing when virDomainCreateXML launches a transient VM whether that is starting up after a revert to backup/snapshot, or whether it is a normal boot. Only the mgmt app like oVirt / OpenStack has enough info to decide that. So we must allow the mgmt app to provide a GUID in the XML document themselves, and then change it in places where we know it is needed to change. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On 04/25/2018 04:32 AM, Peter Krempa wrote: > On Tue, Apr 24, 2018 at 15:38:40 -0400, John Ferlan wrote: >> >> >> On 04/24/2018 03:21 AM, Peter Krempa wrote: >>> On Mon, Apr 23, 2018 at 20:00:00 -0400, John Ferlan wrote: The VM Generation ID is a mechanism to provide a unique 128-bit, cryptographically random, and integer value identifier known as the GUID (Globally Unique Identifier) to the guest OS. The value is used to help notify the guest operating system when the virtual machine is executed with a different configuration. This patch adds support for a new "genid" XML element similar to the "uuid" element. The "genid" element can have two forms "" or "$GUID". If the $GUID is not provided, libvirt will generate one. For the ABI checks add avoidance for the genid comparison if the appropriate flag is set. Since adding support for a generated GUID (or UUID like) value to be displayed only for an active guest, modifying the xml2xml test to include virrandommock.so is necessary since it will generate a "known" UUID value that can be compared against for the active test. Signed-off-by: John Ferlan --- docs/formatdomain.html.in| 29 docs/schemas/domaincommon.rng| 8 src/conf/domain_conf.c | 59 src/conf/domain_conf.h | 3 ++ tests/qemuxml2argvdata/genid-auto.xml| 32 + tests/qemuxml2argvdata/genid.xml | 32 + tests/qemuxml2xmloutdata/genid-active.xml| 32 + tests/qemuxml2xmloutdata/genid-auto-active.xml | 32 + tests/qemuxml2xmloutdata/genid-auto-inactive.xml | 32 + tests/qemuxml2xmloutdata/genid-inactive.xml | 32 + tests/qemuxml2xmltest.c | 5 +- 11 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/genid-auto.xml create mode 100644 tests/qemuxml2argvdata/genid.xml create mode 100644 tests/qemuxml2xmloutdata/genid-active.xml create mode 100644 tests/qemuxml2xmloutdata/genid-auto-active.xml create mode 100644 tests/qemuxml2xmloutdata/genid-auto-inactive.xml create mode 100644 tests/qemuxml2xmloutdata/genid-inactive.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ada0df227f..6a140f3e40 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -34,6 +34,7 @@MyGuest 4dea22b3-1d52-d8f3-2516-782e98ab3fa0 +43dc0cf8-809b-4adb-9bea-a9abb5f3d90e >>> >>> Since we encourage to use the variant with this being empty I'd show >>> that here. >>> >> >> I'd agree except for the fact this is a "live" XML example since >> "id='1'", so it should stay as is unless of course it's desired to never >> show the GUID for the running VM. > > Hmm, right. It feels slightly wrong though that we are describing > configuration on the example of a live XML. > I can remove the id='1' and then use ... It's not that important to print the GUID to me... >>A short description - title - of the domain Some human readable description > > [...] > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d4db50998..8c30adf850 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18793,6 +18793,34 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(tmp); } +/* Extract domain genid - a genid can either be provided or generated */ +if ((n = virXPathNodeSet("./genid", ctxt, &nodes)) < 0) +goto error; + +if (n > 0) { +if (n != 1) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("element 'genid' can only appear once")); +goto error; +} +def->genidRequested = true; +if (!(tmp = virXPathString("string(./genid[1])", ctxt))) { +if (virUUIDGenerate(def->genid) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to generate genid")); >>> >>> Generating this here is pointless, the code using it throws this away >>> and generates a new one. While we don't show this value to the user and >>> thus don't create any false impressions I think that the logic should be >>> shuffled around so that it's generated only when it's required. >>> >>> >> >> How so? Not generating one here would cause active xml-2-xml to fail (as > > Basically because a GUID for an inactive def
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On Wed, Apr 25, 2018 at 13:25:57 +0100, Daniel Berrange wrote: > On Wed, Apr 25, 2018 at 08:02:35AM -0400, John Ferlan wrote: > > > > > > On 04/25/2018 04:46 AM, Peter Krempa wrote: > > > On Wed, Apr 25, 2018 at 09:39:49 +0100, Daniel Berrange wrote: > > >> On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: > > >>> > > >>> Well, that depends. I did not read the docs for this thoroughly enough. > > >>> If it is okay for us to generate a new GUID upon every boot of a VM then > > >>> this will be for a rather simple implementation, since we have a very > > >>> limited set of situations when we are starting a new qemu process which > > >>> should NOT change the GUID and we will change it in all other scenarios. > > >> > > >> AFAIK, we *must* change GUID on every cold boot > > > > > > Good, that makes things rather simple. > > > > > > > This one is not really 100% clear to me. The "spec" is like 6 pages - > > it's a pretty quick read... > > > > http://go.microsoft.com/fwlink/?LinkId=260709 > > > > The last 2 pages describe "when" the GUID should change and specifically > > calling out cold boot is not one of those. What leads me to believe > > otherwise is the two boot scenarios and the unspecified VM config > > changes have this "undertone" that using the same GUID for those > > scenarios is fine/expected. > > Yeah the table at the very end is the key bit and it seems I was actually > wrong > > Scenario GenID changed > --- > Virtual machine is paused or resumed No > Virtual machine reboots No > Virtual machine host reboots No > Virtual machine starts executing a snapshot Yes > Virtual machine is recovered from backup Yes > Virtual machine is failed over in a disaster recovery env Yes > Virtual machine is live migrated No > Virtual machine is imported, copied, or clonedYes > Virtual machine is failed over in a clustered environment No > Virtual machine's configuration changes Unspecified > > > My reading of "Virtual machine reboots" and "Virtual machine host reboots" > rows in particular is that we can *NOT* change GUID on every boot up > operation. The spec literally only wants it to be changed when there is > the possibility that the VM is potentially re-executing something that > has already been executed before. > > The transient VM feature is the real killer for libvirt - we have no > way of knowing when virDomainCreateXML launches a transient VM whether > that is starting up after a revert to backup/snapshot, or whether it > is a normal boot. Only the mgmt app like oVirt / OpenStack has enough > info to decide that. So we must allow the mgmt app to provide a GUID > in the XML document themselves, and then change it in places where we > know it is needed to change. Okay. So that means that we actually need to generate it in the parser, but we should also always report it back even for offline configurations. The only problem then will be what to do with the save/restore functionality, because that is really unknown to us since that API both includes the "Virtual machine is paused or resumed" and "Virtual machine starts executing a snapshot" scenario. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On Wed, Apr 25, 2018 at 02:36:52PM +0200, Peter Krempa wrote: > On Wed, Apr 25, 2018 at 13:25:57 +0100, Daniel Berrange wrote: > > On Wed, Apr 25, 2018 at 08:02:35AM -0400, John Ferlan wrote: > > > > > > > > > On 04/25/2018 04:46 AM, Peter Krempa wrote: > > > > On Wed, Apr 25, 2018 at 09:39:49 +0100, Daniel Berrange wrote: > > > >> On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: > > > >>> > > > >>> Well, that depends. I did not read the docs for this thoroughly > > > >>> enough. > > > >>> If it is okay for us to generate a new GUID upon every boot of a VM > > > >>> then > > > >>> this will be for a rather simple implementation, since we have a very > > > >>> limited set of situations when we are starting a new qemu process > > > >>> which > > > >>> should NOT change the GUID and we will change it in all other > > > >>> scenarios. > > > >> > > > >> AFAIK, we *must* change GUID on every cold boot > > > > > > > > Good, that makes things rather simple. > > > > > > > > > > This one is not really 100% clear to me. The "spec" is like 6 pages - > > > it's a pretty quick read... > > > > > > http://go.microsoft.com/fwlink/?LinkId=260709 > > > > > > The last 2 pages describe "when" the GUID should change and specifically > > > calling out cold boot is not one of those. What leads me to believe > > > otherwise is the two boot scenarios and the unspecified VM config > > > changes have this "undertone" that using the same GUID for those > > > scenarios is fine/expected. > > > > Yeah the table at the very end is the key bit and it seems I was actually > > wrong > > > > Scenario GenID changed > > --- > > Virtual machine is paused or resumed No > > Virtual machine reboots No > > Virtual machine host reboots No > > Virtual machine starts executing a snapshot Yes > > Virtual machine is recovered from backup Yes > > Virtual machine is failed over in a disaster recovery env Yes > > Virtual machine is live migrated No > > Virtual machine is imported, copied, or clonedYes > > Virtual machine is failed over in a clustered environment No > > Virtual machine's configuration changes Unspecified > > > > > > My reading of "Virtual machine reboots" and "Virtual machine host reboots" > > rows in particular is that we can *NOT* change GUID on every boot up > > operation. The spec literally only wants it to be changed when there is > > the possibility that the VM is potentially re-executing something that > > has already been executed before. > > > > The transient VM feature is the real killer for libvirt - we have no > > way of knowing when virDomainCreateXML launches a transient VM whether > > that is starting up after a revert to backup/snapshot, or whether it > > is a normal boot. Only the mgmt app like oVirt / OpenStack has enough > > info to decide that. So we must allow the mgmt app to provide a GUID > > in the XML document themselves, and then change it in places where we > > know it is needed to change. > > Okay. So that means that we actually need to generate it in the parser, > but we should also always report it back even for offline > configurations. > > The only problem then will be what to do with the save/restore > functionality, because that is really unknown to us since that API both > includes the "Virtual machine is paused or resumed" and "Virtual machine > starts executing a snapshot" scenario. I think it would fall under the "starts executing a snapshot" scenario no matter what, because the spec doesn't say anything about whether the original VM carried on running after the snapshot was taken. Just that a snapshot was taken and this snapshot is then run. So the fact that our save/restore can be view as a way to "pause" execution doesn't matter - we've implemented "pause" by creating a snapshot and then "resume" by running the snapshot. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On 04/25/2018 08:41 AM, Daniel P. Berrangé wrote: > On Wed, Apr 25, 2018 at 02:36:52PM +0200, Peter Krempa wrote: >> On Wed, Apr 25, 2018 at 13:25:57 +0100, Daniel Berrange wrote: >>> On Wed, Apr 25, 2018 at 08:02:35AM -0400, John Ferlan wrote: On 04/25/2018 04:46 AM, Peter Krempa wrote: > On Wed, Apr 25, 2018 at 09:39:49 +0100, Daniel Berrange wrote: >> On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: >>> >>> Well, that depends. I did not read the docs for this thoroughly enough. >>> If it is okay for us to generate a new GUID upon every boot of a VM then >>> this will be for a rather simple implementation, since we have a very >>> limited set of situations when we are starting a new qemu process which >>> should NOT change the GUID and we will change it in all other scenarios. >> >> AFAIK, we *must* change GUID on every cold boot > > Good, that makes things rather simple. > This one is not really 100% clear to me. The "spec" is like 6 pages - it's a pretty quick read... http://go.microsoft.com/fwlink/?LinkId=260709 The last 2 pages describe "when" the GUID should change and specifically calling out cold boot is not one of those. What leads me to believe otherwise is the two boot scenarios and the unspecified VM config changes have this "undertone" that using the same GUID for those scenarios is fine/expected. >>> >>> Yeah the table at the very end is the key bit and it seems I was actually >>> wrong >>> >>> Scenario GenID changed >>> --- >>> Virtual machine is paused or resumed No >>> Virtual machine reboots No >>> Virtual machine host reboots No >>> Virtual machine starts executing a snapshot Yes >>> Virtual machine is recovered from backup Yes >>> Virtual machine is failed over in a disaster recovery env Yes >>> Virtual machine is live migrated No >>> Virtual machine is imported, copied, or clonedYes >>> Virtual machine is failed over in a clustered environment No >>> Virtual machine's configuration changes Unspecified >>> >>> >>> My reading of "Virtual machine reboots" and "Virtual machine host reboots" >>> rows in particular is that we can *NOT* change GUID on every boot up >>> operation. The spec literally only wants it to be changed when there is >>> the possibility that the VM is potentially re-executing something that >>> has already been executed before. >>> >>> The transient VM feature is the real killer for libvirt - we have no >>> way of knowing when virDomainCreateXML launches a transient VM whether >>> that is starting up after a revert to backup/snapshot, or whether it >>> is a normal boot. Only the mgmt app like oVirt / OpenStack has enough >>> info to decide that. So we must allow the mgmt app to provide a GUID >>> in the XML document themselves, and then change it in places where we >>> know it is needed to change. Also allows virt-clone to adjust it too... >> >> Okay. So that means that we actually need to generate it in the parser, >> but we should also always report it back even for offline >> configurations. >> >> The only problem then will be what to do with the save/restore >> functionality, because that is really unknown to us since that API both >> includes the "Virtual machine is paused or resumed" and "Virtual machine >> starts executing a snapshot" scenario. > > I think it would fall under the "starts executing a snapshot" scenario > no matter what, because the spec doesn't say anything about whether the > original VM carried on running after the snapshot was taken. Just that > a snapshot was taken and this snapshot is then run. So the fact that > our save/restore can be view as a way to "pause" execution doesn't > matter - we've implemented "pause" by creating a snapshot and then > "resume" by running the snapshot. > So given all this - beyond not altering virDomainDefCopy to add a new flag and removing the ABI flag since it was only put there because of RevertToSnapshot @config copy difference, does just altering the genid via the START_GEN_VMID flag then cover things? Is there some other transition that needs that? IOW: Drop patches 2, 3 & 5... Merge patch 4 & 6 and alter 8/9 to not worry about abiflags. Do we print the GUID on inactive domains where we generate? I would think it wouldn't matter and the answer should be no. The print would only happen when active, but it's not that important. Tks - John and yes, 4.3.0 is out of the question here - it'd be in 4.4.0 at the earliest. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/11] conf: Add VM Generation ID parse/format support
On Wed, Apr 25, 2018 at 09:23:59AM -0400, John Ferlan wrote: > > > On 04/25/2018 08:41 AM, Daniel P. Berrangé wrote: > > On Wed, Apr 25, 2018 at 02:36:52PM +0200, Peter Krempa wrote: > >> On Wed, Apr 25, 2018 at 13:25:57 +0100, Daniel Berrange wrote: > >>> On Wed, Apr 25, 2018 at 08:02:35AM -0400, John Ferlan wrote: > > > On 04/25/2018 04:46 AM, Peter Krempa wrote: > > On Wed, Apr 25, 2018 at 09:39:49 +0100, Daniel Berrange wrote: > >> On Wed, Apr 25, 2018 at 10:32:05AM +0200, Peter Krempa wrote: > >>> > >>> Well, that depends. I did not read the docs for this thoroughly > >>> enough. > >>> If it is okay for us to generate a new GUID upon every boot of a VM > >>> then > >>> this will be for a rather simple implementation, since we have a very > >>> limited set of situations when we are starting a new qemu process > >>> which > >>> should NOT change the GUID and we will change it in all other > >>> scenarios. > >> > >> AFAIK, we *must* change GUID on every cold boot > > > > Good, that makes things rather simple. > > > > This one is not really 100% clear to me. The "spec" is like 6 pages - > it's a pretty quick read... > > http://go.microsoft.com/fwlink/?LinkId=260709 > > The last 2 pages describe "when" the GUID should change and specifically > calling out cold boot is not one of those. What leads me to believe > otherwise is the two boot scenarios and the unspecified VM config > changes have this "undertone" that using the same GUID for those > scenarios is fine/expected. > >>> > >>> Yeah the table at the very end is the key bit and it seems I was actually > >>> wrong > >>> > >>> Scenario GenID changed > >>> --- > >>> Virtual machine is paused or resumed No > >>> Virtual machine reboots No > >>> Virtual machine host reboots No > >>> Virtual machine starts executing a snapshot Yes > >>> Virtual machine is recovered from backup Yes > >>> Virtual machine is failed over in a disaster recovery env Yes > >>> Virtual machine is live migrated No > >>> Virtual machine is imported, copied, or clonedYes > >>> Virtual machine is failed over in a clustered environment No > >>> Virtual machine's configuration changes Unspecified > >>> > >>> > >>> My reading of "Virtual machine reboots" and "Virtual machine host reboots" > >>> rows in particular is that we can *NOT* change GUID on every boot up > >>> operation. The spec literally only wants it to be changed when there is > >>> the possibility that the VM is potentially re-executing something that > >>> has already been executed before. > >>> > >>> The transient VM feature is the real killer for libvirt - we have no > >>> way of knowing when virDomainCreateXML launches a transient VM whether > >>> that is starting up after a revert to backup/snapshot, or whether it > >>> is a normal boot. Only the mgmt app like oVirt / OpenStack has enough > >>> info to decide that. So we must allow the mgmt app to provide a GUID > >>> in the XML document themselves, and then change it in places where we > >>> know it is needed to change. > > Also allows virt-clone to adjust it too... > > >> > >> Okay. So that means that we actually need to generate it in the parser, > >> but we should also always report it back even for offline > >> configurations. > >> > >> The only problem then will be what to do with the save/restore > >> functionality, because that is really unknown to us since that API both > >> includes the "Virtual machine is paused or resumed" and "Virtual machine > >> starts executing a snapshot" scenario. > > > > I think it would fall under the "starts executing a snapshot" scenario > > no matter what, because the spec doesn't say anything about whether the > > original VM carried on running after the snapshot was taken. Just that > > a snapshot was taken and this snapshot is then run. So the fact that > > our save/restore can be view as a way to "pause" execution doesn't > > matter - we've implemented "pause" by creating a snapshot and then > > "resume" by running the snapshot. > > > > So given all this - beyond not altering virDomainDefCopy to add a new > flag and removing the ABI flag since it was only put there because of > RevertToSnapshot @config copy difference, does just altering the genid > via the START_GEN_VMID flag then cover things? Is there some other > transition that needs that? > > IOW: Drop patches 2, 3 & 5... Merge patch 4 & 6 and alter 8/9 to not > worry about abiflags. > > Do we print the GUID on inactive domains where we generate? I would > think it wouldn't matter and the answer should be no. The print would > only happen
