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 "<genid/>"
or "<genid>$GUID</genid>". 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 <jferlan@redhat.com>
---
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 @@
<domain type='kvm' id='1'>
<name>MyGuest</name>
<uuid>4dea22b3-1d52-d8f3-2516-782e98ab3fa0</uuid>
+ <genid>43dc0cf8-809b-4adb-9bea-a9abb5f3d90e</genid>
<title>A short description - title - of the domain</title>
<description>Some human readable description</description>
<metadata>
@@ -61,6 +62,34 @@
specification. <span class="since">Since 0.0.1, sysinfo
since 0.8.7</span></dd>
+ <dt><code>genid</code></dt>
+ <dd><span class="since">Since 4.3.0</span>, the <code>genid</code>
+ 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 <code>uuid</code>. The value is used to help notify
+ the guest operating system when the virtual machine is executed
+ with a different configuration, such as:
+
+ <ul>
+ <li>snapshot execution</li>
+ <li>backup recovery</li>
+ <li>failover in a disaster recovery environment</li>
+ <li>creation from template (import, copy, clone)</li>
+ </ul>
+
+ 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.
+
+ <p>
+ When a GUID value is not provided, e.g. using the XML syntax
+ <genid/>, 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.</p></dd>
+
<dt><code>title</code></dt>
<dd>The optional element <code>title</code> provides space for a
short description of the domain. The title should not contain
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4cab55f05d..1892a7c63c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -502,6 +502,14 @@
<ref name="UUID"/>
</element>
</optional>
+ <optional>
+ <element name="genid">
+ <choice>
+ <ref name="UUID"/>
+ <empty/>
+ </choice>
+ </element>
+ </optional>
</interleave>
</define>
<define name="idmap">
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"));
+ goto error;
+ }
+ def->genidGenerated = true;
+ } else {
+ if (virUUIDParse(tmp, def->genid) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("malformed genid element"));
+ goto error;
+ }
+ }
+ }
+ VIR_FREE(nodes);
+
/* Extract short description of domain (title) */
def->title = virXPathString("string(./title[1])", ctxt);
if (def->title && strchr(def->title, '\n')) {
@@ -21904,6 +21932,26 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr src,
goto error;
}
+ if (src->genidRequested != dst->genidRequested) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Target domain requested genid does not match source"));
+ goto error;
+ }
+
+ if (src->genidRequested &&
+ !(flags & VIR_DOMAIN_DEF_ABI_CHECK_SKIP_GENID) &&
+ memcmp(src->genid, dst->genid, VIR_UUID_BUFLEN) != 0) {
+ char guidsrc[VIR_UUID_STRING_BUFLEN];
+ char guiddst[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(src->genid, guidsrc);
+ virUUIDFormat(dst->genid, guiddst);
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target domain genid %s does not match source %s"),
+ guiddst, guidsrc);
+ goto error;
+ }
+
/* Not strictly ABI related, but we want to make sure domains
* don't get silently re-named through the backdoor when passing
* custom XML into various APIs, since this would create havoc
@@ -26541,6 +26589,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virUUIDFormat(uuid, uuidstr);
virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr);
+ if (def->genidRequested) {
+ char genidstr[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(def->genid, genidstr);
+ if (!def->genidGenerated ||
+ !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
+ virBufferAsprintf(buf, "<genid>%s</genid>\n", genidstr);
+ else
+ virBufferAddLit(buf, "<genid/>\n");
+ }
+
virBufferEscapeString(buf, "<title>%s</title>\n", def->title);
virBufferEscapeString(buf, "<description>%s</description>\n",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 910b3ca4d1..e9248a34c2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2952,6 +2952,9 @@ typedef enum {
/* Set when domain lock must be released and there exists the possibility
* that some external action could alter the value, such as cur_balloon. */
VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE = 1 << 0,
+
+ /* Set when the ABI check should skip the genid comparison */
+ VIR_DOMAIN_DEF_ABI_CHECK_SKIP_GENID = 1 << 1,
} virDomainDefABICheckFlags;
virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr,
diff --git a/tests/qemuxml2argvdata/genid-auto.xml b/tests/qemuxml2argvdata/genid-auto.xml
new file mode 100644
index 0000000000..96ad9ddda8
--- /dev/null
+++ b/tests/qemuxml2argvdata/genid-auto.xml
@@ -0,0 +1,32 @@
+<domain type='qemu' id='1'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <genid/>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/genid.xml b/tests/qemuxml2argvdata/genid.xml
new file mode 100644
index 0000000000..fc41f2dd28
--- /dev/null
+++ b/tests/qemuxml2argvdata/genid.xml
@@ -0,0 +1,32 @@
+<domain type='qemu' id='1'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <genid>e9392370-2917-565e-692b-d057f46512d6</genid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/genid-active.xml b/tests/qemuxml2xmloutdata/genid-active.xml
new file mode 100644
index 0000000000..fc41f2dd28
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/genid-active.xml
@@ -0,0 +1,32 @@
+<domain type='qemu' id='1'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <genid>e9392370-2917-565e-692b-d057f46512d6</genid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/genid-auto-active.xml b/tests/qemuxml2xmloutdata/genid-auto-active.xml
new file mode 100644
index 0000000000..aeca0d7fc0
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/genid-auto-active.xml
@@ -0,0 +1,32 @@
+<domain type='qemu' id='1'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <genid>00010203-0405-4607-8809-0a0b0c0d0e0f</genid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/genid-auto-inactive.xml b/tests/qemuxml2xmloutdata/genid-auto-inactive.xml
new file mode 100644
index 0000000000..83c924395b
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/genid-auto-inactive.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <genid/>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/genid-inactive.xml b/tests/qemuxml2xmloutdata/genid-inactive.xml
new file mode 100644
index 0000000000..8bd526a7a9
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/genid-inactive.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <genid>e9392370-2917-565e-692b-d057f46512d6</genid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 9e77b9fb13..e999810e12 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -276,6 +276,8 @@ mymain(void)
setenv("PATH", "/bin", 1);
DO_TEST("minimal", NONE);
+ DO_TEST("genid", NONE);
+ DO_TEST("genid-auto", NONE);
DO_TEST("machine-core-on", NONE);
DO_TEST("machine-core-off", NONE);
DO_TEST("machine-loadparm-multiple-disks-nets-s390", NONE);
@@ -1209,7 +1211,8 @@ mymain(void)
}
VIR_TEST_MAIN_PRELOAD(mymain,
- abs_builddir "/.libs/virpcimock.so")
+ abs_builddir "/.libs/virpcimock.so",
+ abs_builddir "/.libs/virrandommock.so")
#else
--
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 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 "<genid/>"
> or "<genid>$GUID</genid>". 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 <jferlan@redhat.com>
> ---
> 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 @@
> <domain type='kvm' id='1'>
> <name>MyGuest</name>
> <uuid>4dea22b3-1d52-d8f3-2516-782e98ab3fa0</uuid>
> + <genid>43dc0cf8-809b-4adb-9bea-a9abb5f3d90e</genid>
Since we encourage to use the variant with this being empty I'd show
that here.
> <title>A short description - title - of the domain</title>
> <description>Some human readable description</description>
> <metadata>
> @@ -61,6 +62,34 @@
> specification. <span class="since">Since 0.0.1, sysinfo
> since 0.8.7</span></dd>
>
> + <dt><code>genid</code></dt>
> + <dd><span class="since">Since 4.3.0</span>, the <code>genid</code>
> + 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 <code>uuid</code>. The value is used to help notify
> + the guest operating system when the virtual machine is executed
> + with a different configuration, such as:
> +
> + <ul>
> + <li>snapshot execution</li>
> + <li>backup recovery</li>
> + <li>failover in a disaster recovery environment</li>
> + <li>creation from template (import, copy, clone)</li>
> + </ul>
> +
> + 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.
> +
> + <p>
> + When a GUID value is not provided, e.g. using the XML syntax
> + <genid/>, 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.</p></dd>
> +
> <dt><code>title</code></dt>
> <dd>The optional element <code>title</code> 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",
> + _("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.
> + }
> + def->genidGenerated = true;
> + } else {
> + if (virUUIDParse(tmp, def->genid) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("malformed genid element"));
> + goto error;
> + }
> + }
> + }
> + VIR_FREE(nodes);
> +
> /* Extract short description of domain (title) */
> def->title = virXPathString("string(./title[1])", ctxt);
> if (def->title && strchr(def->title, '\n')) {
> @@ -21904,6 +21932,26 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr src,
> goto error;
> }
>
> + if (src->genidRequested != dst->genidRequested) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Target domain requested genid does not match source"));
> + goto error;
> + }
> +
> + if (src->genidRequested &&
> + !(flags & VIR_DOMAIN_DEF_ABI_CHECK_SKIP_GENID) &&
The above change will also possibly remove the need for this flag. In
cases when it needs to change (snapshots) the code is skipping the check
anyways. In cases when it needs to stay the same we will know that since
it will be filled in.
> + memcmp(src->genid, dst->genid, VIR_UUID_BUFLEN) != 0) {
> + char guidsrc[VIR_UUID_STRING_BUFLEN];
> + char guiddst[VIR_UUID_STRING_BUFLEN];
> +
> + virUUIDFormat(src->genid, guidsrc);
> + virUUIDFormat(dst->genid, guiddst);
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target domain genid %s does not match source %s"),
> + guiddst, guidsrc);
> + goto error;
> + }
> +
> /* Not strictly ABI related, but we want to make sure domains
> * don't get silently re-named through the backdoor when passing
> * custom XML into various APIs, since this would create havoc
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 "<genid/>"
>> or "<genid>$GUID</genid>". 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 <jferlan@redhat.com>
>> ---
>> 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 @@
>> <domain type='kvm' id='1'>
>> <name>MyGuest</name>
>> <uuid>4dea22b3-1d52-d8f3-2516-782e98ab3fa0</uuid>
>> + <genid>43dc0cf8-809b-4adb-9bea-a9abb5f3d90e</genid>
>
> 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.
>> <title>A short description - title - of the domain</title>
>> <description>Some human readable description</description>
>> <metadata>
>> @@ -61,6 +62,34 @@
>> specification. <span class="since">Since 0.0.1, sysinfo
>> since 0.8.7</span></dd>
>>
>> + <dt><code>genid</code></dt>
>> + <dd><span class="since">Since 4.3.0</span>, the <code>genid</code>
>> + 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 <code>uuid</code>. The value is used to help notify
>> + the guest operating system when the virtual machine is executed
>> + with a different configuration, such as:
>> +
>> + <ul>
>> + <li>snapshot execution</li>
>> + <li>backup recovery</li>
>> + <li>failover in a disaster recovery environment</li>
>> + <li>creation from template (import, copy, clone)</li>
>> + </ul>
>> +
>> + 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.
>> +
>> + <p>
>> + When a GUID value is not provided, e.g. using the XML syntax
>> + <genid/>, 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.</p></dd>
>> +
>> <dt><code>title</code></dt>
>> <dd>The optional element <code>title</code> 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",
>> + _("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
I expect) because the GUID would be 00000-0000-0000-0000-000000000000.
At the end of the series the usage of virUUIDGenerate for ->genid is
done for:
1. This code, virDomainDefParseXML
2. virDomainDefCopy when genidRequested and flags &
VIR_DOMAIN_DEF_COPY_NEWGENID which is currently only used by the
snapshot code, but could be used elsewhere - something I may have been
thinking about at least w/r/t one of the qemu_driver paths.
3. qemuProcessGenID *when* flags & VIR_QEMU_PROCESS_START_GEN_VMID
If anything, perhaps what you may be referring to is qemuProcessGenID
processing using the VIR_QEMU_PROCESS_START_GEN_VMID flag when called
from qemuDomainRevertToSnapshot. The transitions there caused me to be
"super cautious", but the Copy, then Start I think upon reflection does
overwrite. The same flag is used in qemuDomainSaveImageStartVM, which
yes does overwrite, but that's by rule/design.
Perhaps it'll help to summarize the transitions...
Change is not required for the following transitions:
-> Pause/Resume
-> VM Reboot
-> Host reboot
-> Live migrate
-> Failover in a clustered environment
Change is required for the following transitions:
-> Application of offline/online snapshot
-> Restoring from backup
-> Failover to replication target
-> Importing a VM (or copy or clone)
Change is "Unspecified" when a VM's configuration changes.
So the 'design decisions' were:
- For snapshot, the choice was use the virDomainDefCopy and ABI flags.
- For the restore from backup, the choice was regenerate and store in
the config during startup processing. The docs indicate "The restore
sequence will be modified to post process the restore target and apply
new generation identifiers to the restored configuration files." - so
where it was done would
- It's not 100% clear if we need something special for failover from
replication target. Seems like Snapshot or Save/Restore is in the same
category, but perhaps there's some hypervisor specific transition.
- For import and copy, changing the genid of the copy "seems right".
The tricky part is the clone code which would require a separate change
to virt-clone since it uses parses and formats XML in separate passes.
So given all that - are you still of the opinion that the design needs
to change even more? If so, then please also characterize your view of
how this should work.
Tks -
John
>> + }
>> + def->genidGenerated = true;
>> + } else {
>> + if (virUUIDParse(tmp, def->genid) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + "%s", _("malformed genid element"));
>> + goto error;
>> + }
>> + }
>> + }
>> + VIR_FREE(nodes);
>> +
>> /* Extract short description of domain (title) */
>> def->title = virXPathString("string(./title[1])", ctxt);
>> if (def->title && strchr(def->title, '\n')) {
>> @@ -21904,6 +21932,26 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr src,
>> goto error;
>> }
>>
>> + if (src->genidRequested != dst->genidRequested) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("Target domain requested genid does not match source"));
>> + goto error;
>> + }
>> +
>> + if (src->genidRequested &&
>> + !(flags & VIR_DOMAIN_DEF_ABI_CHECK_SKIP_GENID) &&
>
> The above change will also possibly remove the need for this flag. In
> cases when it needs to change (snapshots) the code is skipping the check
> anyways. In cases when it needs to stay the same we will know that since
> it will be filled in.
>
>> + memcmp(src->genid, dst->genid, VIR_UUID_BUFLEN) != 0) {
>> + char guidsrc[VIR_UUID_STRING_BUFLEN];
>> + char guiddst[VIR_UUID_STRING_BUFLEN];
>> +
>> + virUUIDFormat(src->genid, guidsrc);
>> + virUUIDFormat(dst->genid, guiddst);
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("Target domain genid %s does not match source %s"),
>> + guiddst, guidsrc);
>> + goto error;
>> + }
>> +
>> /* Not strictly ABI related, but we want to make sure domains
>> * don't get silently re-named through the backdoor when passing
>> * custom XML into various APIs, since this would create havoc
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 "<genid/>"
> >> or "<genid>$GUID</genid>". 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 <jferlan@redhat.com>
> >> ---
> >> 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 @@
> >> <domain type='kvm' id='1'>
> >> <name>MyGuest</name>
> >> <uuid>4dea22b3-1d52-d8f3-2516-782e98ab3fa0</uuid>
> >> + <genid>43dc0cf8-809b-4adb-9bea-a9abb5f3d90e</genid>
> >
> > 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.
>
> >> <title>A short description - title - of the domain</title>
> >> <description>Some human readable description</description>
> >> <metadata>
[...]
> >> 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 GUID or the one which
is store
> I expect) because the GUID would be 00000-0000-0000-0000-000000000000.
> At the end of the series the usage of virUUIDGenerate for ->genid is
> done for:
>
> 1. This code, virDomainDefParseXML
> 2. virDomainDefCopy when genidRequested and flags &
> VIR_DOMAIN_DEF_COPY_NEWGENID which is currently only used by the
> snapshot code, but could be used elsewhere - something I may have been
> thinking about at least w/r/t one of the qemu_driver paths.
> 3. qemuProcessGenID *when* flags & VIR_QEMU_PROCESS_START_GEN_VMID
>
> If anything, perhaps what you may be referring to is qemuProcessGenID
> processing using the VIR_QEMU_PROCESS_START_GEN_VMID flag when called
> from qemuDomainRevertToSnapshot. The transitions there caused me to be
> "super cautious", but the Copy, then Start I think upon reflection does
> overwrite. The same flag is used in qemuDomainSaveImageStartVM, which
> yes does overwrite, but that's by rule/design.
>
> Perhaps it'll help to summarize the transitions...
>
> Change is not required for the following transitions:
>
> -> Pause/Resume
> -> VM Reboot
Both above are the same emulator instance.
> -> Host reboot
This is wrong. Host reboot causes the VM to be killed. So when booting
the GUID will change since we start a new process. While it should not
result in any problems if the ID would not change (since the guest OS
rebooted anyways) we should treat this as start of the VM.
> -> Live migrate
This is technically the same emulator instance. No rollback of execution
was possible.
> -> Failover in a clustered environment
Thankfully it does not apply at this point since we don't support any of
this implicitly. It also probably depends on the way the failover
scenario is executed. In the qemu world I read only about coarse-grained
lockstepping as a case of failover and that has situations where some
rollback could happen, so in that case the VM id should probably change.
But as said, thankfully we don't have to deal with it currenly and also
it would not be possible since qemu can't change the id during runtime.
> Change is required for the following transitions:
>
> -> Application of offline/online snapshot
> -> Restoring from backup
This is too vague. If you mean a disk backup, the VM will boot, thus the
ID should change (unless the user specified an explicit one).
> -> Failover to replication target
See above.
> -> Importing a VM (or copy or clone)
>
> Change is "Unspecified" when a VM's configuration changes.
So in our case it will change.
>
> So the 'design decisions' were:
>
> - For snapshot, the choice was use the virDomainDefCopy and ABI flags.
Actually, the GUID when creating the snapshot is irrelevant. When
reverting a snapshot the ID should always change thus it will always
require an emulator restart.
> - For the restore from backup, the choice was regenerate and store in
> the config during startup processing. The docs indicate "The restore
> sequence will be modified to post process the restore target and apply
> new generation identifiers to the restored configuration files." - so
> where it was done would
As pointed out above, this is vague. If it's a restore of the disk state
only, the guid will necessarily change in our design when we start the
new qemu process.
> - It's not 100% clear if we need something special for failover from
> replication target. Seems like Snapshot or Save/Restore is in the same
> category, but perhaps there's some hypervisor specific transition.
What is a failover here? You mean if a management starts a new VM after
a different host with VM has failed? In such case it's a new start of VM
for us and thus will get a new GUID.
> - For import and copy, changing the genid of the copy "seems right".
> The tricky part is the clone code which would require a separate change
> to virt-clone since it uses parses and formats XML in separate passes.
That depends on the implementation. Currently yes.
> So given all that - are you still of the opinion that the design needs
> to change even more? If so, then please also characterize your view of
> how this should work.
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.
If the documentation does not allow for the above it will be more
complex and we'll need to discuss that further.
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.
This means that we'll probably need a new attribute which will specify
that the GUID is custom (or the opposite, that it was generated). If
that property is in the default state the startup procedure should
generate a new GUID exept for the migration case.
We also might want to consider the managed-save case to bear the same
GUID after startup, since we know that we start the new qemu process
from the same state as we've saved it.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
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. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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
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 cloned Yes 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
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 cloned Yes > 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. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 cloned Yes > > 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
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 cloned Yes >>> 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
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 cloned Yes > >>> 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. Once we've generated a GUID we are preserving it across normalk VM restarts. So I think we should always list it in XML regardless of whether VM is running, because it is telling mgmt app what the VM will be started with next time. 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
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 "<genid/>"
>>>> or "<genid>$GUID</genid>". 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 <jferlan@redhat.com>
>>>> ---
>>>> 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 @@
>>>> <domain type='kvm' id='1'>
>>>> <name>MyGuest</name>
>>>> <uuid>4dea22b3-1d52-d8f3-2516-782e98ab3fa0</uuid>
>>>> + <genid>43dc0cf8-809b-4adb-9bea-a9abb5f3d90e</genid>
>>>
>>> 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 <genid/>... It's not that
important to print the GUID to me...
>>
>>>> <title>A short description - title - of the domain</title>
>>>> <description>Some human readable description</description>
>>>> <metadata>
>
> [...]
>
>
>>>> 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.
>
As noted in the other part of this reply thread - it changes for
specific transactions. It's not clear "how" it's initially generated -
whether user specified or self generated - only that for certain
transitions, it must change and doing so is done by generating a new one.
> The parser only needs to restore a user-provided GUID or the one which
> is store
>
>> I expect) because the GUID would be 00000-0000-0000-0000-000000000000.
>> At the end of the series the usage of virUUIDGenerate for ->genid is
>> done for:
>>
>> 1. This code, virDomainDefParseXML
>> 2. virDomainDefCopy when genidRequested and flags &
>> VIR_DOMAIN_DEF_COPY_NEWGENID which is currently only used by the
>> snapshot code, but could be used elsewhere - something I may have been
>> thinking about at least w/r/t one of the qemu_driver paths.
>> 3. qemuProcessGenID *when* flags & VIR_QEMU_PROCESS_START_GEN_VMID
>>
>> If anything, perhaps what you may be referring to is qemuProcessGenID
>> processing using the VIR_QEMU_PROCESS_START_GEN_VMID flag when called
>> from qemuDomainRevertToSnapshot. The transitions there caused me to be
>> "super cautious", but the Copy, then Start I think upon reflection does
>> overwrite. The same flag is used in qemuDomainSaveImageStartVM, which
>> yes does overwrite, but that's by rule/design.
>>
>> Perhaps it'll help to summarize the transitions...
>>
>> Change is not required for the following transitions:
>>
>> -> Pause/Resume
>> -> VM Reboot
>
> Both above are the same emulator instance.
>
>> -> Host reboot
>
> This is wrong. Host reboot causes the VM to be killed. So when booting
> the GUID will change since we start a new process. While it should not
> result in any problems if the ID would not change (since the guest OS
> rebooted anyways) we should treat this as start of the VM.
>
Well you may call it wrong, but that's what is in the spec.
>> -> Live migrate
>
> This is technically the same emulator instance. No rollback of execution
> was possible.
>
>> -> Failover in a clustered environment
>
> Thankfully it does not apply at this point since we don't support any of
> this implicitly. It also probably depends on the way the failover
> scenario is executed. In the qemu world I read only about coarse-grained
> lockstepping as a case of failover and that has situations where some
> rollback could happen, so in that case the VM id should probably change.
>
> But as said, thankfully we don't have to deal with it currenly and also
> it would not be possible since qemu can't change the id during runtime.
>
>
>> Change is required for the following transitions:
>>
>> -> Application of offline/online snapshot
>> -> Restoring from backup
>
> This is too vague. If you mean a disk backup, the VM will boot, thus the
> ID should change (unless the user specified an explicit one).
>
Hopefully covered in the other half of this thread.
>> -> Failover to replication target
>
> See above.
>
>> -> Importing a VM (or copy or clone)
>>
>> Change is "Unspecified" when a VM's configuration changes.
>
> So in our case it will change.
>
It can change based on transitions that require it or it can be used as
found in the config file. This is the real bugger in the description.
>>
>> So the 'design decisions' were:
>>
>> - For snapshot, the choice was use the virDomainDefCopy and ABI flags.
>
> Actually, the GUID when creating the snapshot is irrelevant. When
> reverting a snapshot the ID should always change thus it will always
> require an emulator restart.
>
Wouldn't the GUID for the creation of the snapshot be whatever was set?
The whole snapshot config and domain config saved within is not
something I depend on others to understand in greater detail than I do.
Still, my choice was alter the GUID when starting and causing an error
for the transition to use qemuMonitorLoadSnapshot since we cannot modify
the genid. Again, another area that I hoped review would provide details
if this was the "wrong choice".
>> - For the restore from backup, the choice was regenerate and store in
>> the config during startup processing. The docs indicate "The restore
>> sequence will be modified to post process the restore target and apply
>> new generation identifiers to the restored configuration files." - so
>> where it was done would
>
> As pointed out above, this is vague. If it's a restore of the disk state
> only, the guid will necessarily change in our design when we start the
> new qemu process.
>
Cannot disagree - the choice was to change only when/if we restart the
VM since that's really the only time it matters.
>> - It's not 100% clear if we need something special for failover from
>> replication target. Seems like Snapshot or Save/Restore is in the same
>> category, but perhaps there's some hypervisor specific transition.
>
> What is a failover here? You mean if a management starts a new VM after
> a different host with VM has failed? In such case it's a new start of VM
> for us and thus will get a new GUID.
>
It was me commenting and trying not to read too much into things.
Perhaps the shorter version of your failover comments from above.
>> - For import and copy, changing the genid of the copy "seems right".
>> The tricky part is the clone code which would require a separate change
>> to virt-clone since it uses parses and formats XML in separate passes.
>
> That depends on the implementation. Currently yes.
>
>> So given all that - are you still of the opinion that the design needs
>> to change even more? If so, then please also characterize your view of
>> how this should work.
>
> 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.
It's not really clear that a new GUID for every new boot is "required".
>
> If the documentation does not allow for the above it will be more
> complex and we'll need to discuss that further.
We shouldn't over complicate things either.
John
[the rest is covered in the other thread - this is the hazards of
cutting threads up like this]
>
> 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.
>
> This means that we'll probably need a new attribute which will specify
> that the GUID is custom (or the opposite, that it was generated). If
> that property is in the default state the startup procedure should
> generate a new GUID exept for the migration case.
>
> We also might want to consider the managed-save case to bear the same
> GUID after startup, since we know that we start the new qemu process
> from the same state as we've saved it.
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.