[PATCH] Eliminate name collision for feature tests' VMs
If Kimchi's startup is interrupted for some reason, a VM created during the feature tests may be left defined. When this happens, the next time Kimchi is started it will fail with an error that the domain already exists (for example, "libvirtError: operation failed: domain 'A_SIMPLE_VM' already exists with uuid e6fccea8-f7ed-4320-a4cb-b85217d23985"). This patch just uses the same uuid-based name scheme as was already in the user tests (kvmusertests.py). That gives a miniscule chance for name collision. Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/model/featuretests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 5a45990..aa8d709 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -24,6 +24,7 @@ import socket import subprocess import threading +import uuid from lxml.builder import E @@ -35,7 +36,7 @@ ISO_STREAM_XML = """ <domain type='%(domain)s'> - <name>ISO_STREAMING</name> + <name>'%(name)s'</name> <memory unit='KiB'>1048576</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -57,7 +58,7 @@ SIMPLE_VM_XML = """ <domain type='%(domain)s'> - <name>A_SIMPLE_VM</name> + <name>'%(name)s'</name> <memory unit='KiB'>10240</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -105,8 +106,10 @@ def libvirt_supports_iso_stream(conn, protocol): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "ISO_STREAM_%s" % vm_uuid xml = ISO_STREAM_XML % {'domain': domain_type, 'protocol': protocol, - 'arch': arch} + 'arch': arch, 'name': vm_name} try: FeatureTests.disable_libvirt_error_logging() dom = conn.defineXML(xml) @@ -196,8 +199,11 @@ def has_metadata_support(conn): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "A_SIMPLE_VM_%s" % vm_uuid dom = conn.defineXML(SIMPLE_VM_XML % {'domain': domain_type, - 'arch': arch}) + 'arch': arch, + 'name': vm_name}) rollback.prependDefer(dom.undefine) try: dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, -- 1.9.3
I apologize in advance if any of the unit tests fail. There's something wrong with the tests for F20 that I can't figure out (not for lack of trying). It looked like there weren't any *new* failures created as a result of my patch. - Christy On 02/05/2015 05:15 PM, Christy Perez wrote:
If Kimchi's startup is interrupted for some reason, a VM created during the feature tests may be left defined. When this happens, the next time Kimchi is started it will fail with an error that the domain already exists (for example, "libvirtError: operation failed: domain 'A_SIMPLE_VM' already exists with uuid e6fccea8-f7ed-4320-a4cb-b85217d23985").
This patch just uses the same uuid-based name scheme as was already in the user tests (kvmusertests.py). That gives a miniscule chance for name collision.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/model/featuretests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 5a45990..aa8d709 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -24,6 +24,7 @@ import socket import subprocess import threading +import uuid
from lxml.builder import E @@ -35,7 +36,7 @@
ISO_STREAM_XML = """ <domain type='%(domain)s'> - <name>ISO_STREAMING</name> + <name>'%(name)s'</name> <memory unit='KiB'>1048576</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -57,7 +58,7 @@
SIMPLE_VM_XML = """ <domain type='%(domain)s'> - <name>A_SIMPLE_VM</name> + <name>'%(name)s'</name> <memory unit='KiB'>10240</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -105,8 +106,10 @@ def libvirt_supports_iso_stream(conn, protocol): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "ISO_STREAM_%s" % vm_uuid xml = ISO_STREAM_XML % {'domain': domain_type, 'protocol': protocol, - 'arch': arch} + 'arch': arch, 'name': vm_name} try: FeatureTests.disable_libvirt_error_logging() dom = conn.defineXML(xml) @@ -196,8 +199,11 @@ def has_metadata_support(conn): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "A_SIMPLE_VM_%s" % vm_uuid dom = conn.defineXML(SIMPLE_VM_XML % {'domain': domain_type, - 'arch': arch}) + 'arch': arch, + 'name': vm_name}) rollback.prependDefer(dom.undefine) try: dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT,
It does not fix the problem, only workaround it. Kimchi must not leave leftovers in the user system. On 05/02/2015 21:15, Christy Perez wrote:
If Kimchi's startup is interrupted for some reason, a VM created during the feature tests may be left defined. When this happens, the next time Kimchi is started it will fail with an error that the domain already exists (for example, "libvirtError: operation failed: domain 'A_SIMPLE_VM' already exists with uuid e6fccea8-f7ed-4320-a4cb-b85217d23985").
This patch just uses the same uuid-based name scheme as was already in the user tests (kvmusertests.py). That gives a miniscule chance for name collision.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/model/featuretests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 5a45990..aa8d709 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -24,6 +24,7 @@ import socket import subprocess import threading +import uuid
from lxml.builder import E @@ -35,7 +36,7 @@
ISO_STREAM_XML = """ <domain type='%(domain)s'> - <name>ISO_STREAMING</name> + <name>'%(name)s'</name> <memory unit='KiB'>1048576</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -57,7 +58,7 @@
SIMPLE_VM_XML = """ <domain type='%(domain)s'> - <name>A_SIMPLE_VM</name> + <name>'%(name)s'</name> <memory unit='KiB'>10240</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -105,8 +106,10 @@ def libvirt_supports_iso_stream(conn, protocol): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "ISO_STREAM_%s" % vm_uuid xml = ISO_STREAM_XML % {'domain': domain_type, 'protocol': protocol, - 'arch': arch} + 'arch': arch, 'name': vm_name} try: FeatureTests.disable_libvirt_error_logging() dom = conn.defineXML(xml) @@ -196,8 +199,11 @@ def has_metadata_support(conn): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "A_SIMPLE_VM_%s" % vm_uuid dom = conn.defineXML(SIMPLE_VM_XML % {'domain': domain_type, - 'arch': arch}) + 'arch': arch, + 'name': vm_name}) rollback.prependDefer(dom.undefine) try: dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT,
On 02/06/2015 07:03 AM, Aline Manera wrote:
It does not fix the problem, only workaround it. Kimchi must not leave leftovers in the user system.
Agreed. Can we put it in place a temporary solution? I liked the idea you proposed about the CherryPy listener to clean things up, but I have no idea how to do something like that, and I feel like other people are a bit overwhelmed at the moment. We can add that as a to-do.
On 05/02/2015 21:15, Christy Perez wrote:
If Kimchi's startup is interrupted for some reason, a VM created during the feature tests may be left defined. When this happens, the next time Kimchi is started it will fail with an error that the domain already exists (for example, "libvirtError: operation failed: domain 'A_SIMPLE_VM' already exists with uuid e6fccea8-f7ed-4320-a4cb-b85217d23985").
This patch just uses the same uuid-based name scheme as was already in the user tests (kvmusertests.py). That gives a miniscule chance for name collision.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/model/featuretests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 5a45990..aa8d709 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -24,6 +24,7 @@ import socket import subprocess import threading +import uuid
from lxml.builder import E @@ -35,7 +36,7 @@
ISO_STREAM_XML = """ <domain type='%(domain)s'> - <name>ISO_STREAMING</name> + <name>'%(name)s'</name> <memory unit='KiB'>1048576</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -57,7 +58,7 @@
SIMPLE_VM_XML = """ <domain type='%(domain)s'> - <name>A_SIMPLE_VM</name> + <name>'%(name)s'</name> <memory unit='KiB'>10240</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -105,8 +106,10 @@ def libvirt_supports_iso_stream(conn, protocol): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "ISO_STREAM_%s" % vm_uuid xml = ISO_STREAM_XML % {'domain': domain_type, 'protocol': protocol, - 'arch': arch} + 'arch': arch, 'name': vm_name} try: FeatureTests.disable_libvirt_error_logging() dom = conn.defineXML(xml) @@ -196,8 +199,11 @@ def has_metadata_support(conn): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "A_SIMPLE_VM_%s" % vm_uuid dom = conn.defineXML(SIMPLE_VM_XML % {'domain': domain_type, - 'arch': arch}) + 'arch': arch, + 'name': vm_name}) rollback.prependDefer(dom.undefine) try: dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT,
On 06/02/2015 13:18, Christy Perez wrote:
On 02/06/2015 07:03 AM, Aline Manera wrote:
It does not fix the problem, only workaround it. Kimchi must not leave leftovers in the user system. Agreed. Can we put it in place a temporary solution?
No.
I liked the idea you proposed about the CherryPy listener to clean things up, but I have no idea how to do something like that, and I feel like other people are a bit overwhelmed at the moment. We can add that as a to-do.
There are multiple places in the current upstream code that add listener to cherrypy. Check for "cherrypy.engine.subscribe" alinefm@alinefm:~/kimchi$ git grep cherrypy.engine.subscribe src/kimchi/model/config.py: cherrypy.engine.subscribe('start', self._set_capabilities) src/kimchi/server.py: cherrypy.engine.subscribe('exit', vnc_ws_proxy.terminate) src/kimchi/server.py: cherrypy.engine.subscribe('exit', terminate_proxy) There is a listener, for example, to kill the nginx proxy when cherrypy is stopped. Let me know if you need more help on it.
On 05/02/2015 21:15, Christy Perez wrote:
If Kimchi's startup is interrupted for some reason, a VM created during the feature tests may be left defined. When this happens, the next time Kimchi is started it will fail with an error that the domain already exists (for example, "libvirtError: operation failed: domain 'A_SIMPLE_VM' already exists with uuid e6fccea8-f7ed-4320-a4cb-b85217d23985").
This patch just uses the same uuid-based name scheme as was already in the user tests (kvmusertests.py). That gives a miniscule chance for name collision.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/model/featuretests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 5a45990..aa8d709 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -24,6 +24,7 @@ import socket import subprocess import threading +import uuid
from lxml.builder import E @@ -35,7 +36,7 @@
ISO_STREAM_XML = """ <domain type='%(domain)s'> - <name>ISO_STREAMING</name> + <name>'%(name)s'</name> <memory unit='KiB'>1048576</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -57,7 +58,7 @@
SIMPLE_VM_XML = """ <domain type='%(domain)s'> - <name>A_SIMPLE_VM</name> + <name>'%(name)s'</name> <memory unit='KiB'>10240</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -105,8 +106,10 @@ def libvirt_supports_iso_stream(conn, protocol): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "ISO_STREAM_%s" % vm_uuid xml = ISO_STREAM_XML % {'domain': domain_type, 'protocol': protocol, - 'arch': arch} + 'arch': arch, 'name': vm_name} try: FeatureTests.disable_libvirt_error_logging() dom = conn.defineXML(xml) @@ -196,8 +199,11 @@ def has_metadata_support(conn): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "A_SIMPLE_VM_%s" % vm_uuid dom = conn.defineXML(SIMPLE_VM_XML % {'domain': domain_type, - 'arch': arch}) + 'arch': arch, + 'name': vm_name}) rollback.prependDefer(dom.undefine) try: dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT,
On 02/06/2015 09:51 AM, Aline Manera wrote:
On 06/02/2015 13:18, Christy Perez wrote:
On 02/06/2015 07:03 AM, Aline Manera wrote:
It does not fix the problem, only workaround it. Kimchi must not leave leftovers in the user system. Agreed. Can we put it in place a temporary solution?
No.
Issue Created: https://github.com/kimchi-project/kimchi/issues/589
I liked the idea you proposed about the CherryPy listener to clean things up, but I have no idea how to do something like that, and I feel like other people are a bit overwhelmed at the moment. We can add that as a to-do.
There are multiple places in the current upstream code that add listener to cherrypy.
Check for "cherrypy.engine.subscribe"
alinefm@alinefm:~/kimchi$ git grep cherrypy.engine.subscribe src/kimchi/model/config.py: cherrypy.engine.subscribe('start', self._set_capabilities) src/kimchi/server.py: cherrypy.engine.subscribe('exit', vnc_ws_proxy.terminate) src/kimchi/server.py: cherrypy.engine.subscribe('exit', terminate_proxy)
There is a listener, for example, to kill the nginx proxy when cherrypy is stopped.
Let me know if you need more help on it.
On 05/02/2015 21:15, Christy Perez wrote:
If Kimchi's startup is interrupted for some reason, a VM created during the feature tests may be left defined. When this happens, the next time Kimchi is started it will fail with an error that the domain already exists (for example, "libvirtError: operation failed: domain 'A_SIMPLE_VM' already exists with uuid e6fccea8-f7ed-4320-a4cb-b85217d23985").
This patch just uses the same uuid-based name scheme as was already in the user tests (kvmusertests.py). That gives a miniscule chance for name collision.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/model/featuretests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 5a45990..aa8d709 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -24,6 +24,7 @@ import socket import subprocess import threading +import uuid
from lxml.builder import E @@ -35,7 +36,7 @@
ISO_STREAM_XML = """ <domain type='%(domain)s'> - <name>ISO_STREAMING</name> + <name>'%(name)s'</name> <memory unit='KiB'>1048576</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -57,7 +58,7 @@
SIMPLE_VM_XML = """ <domain type='%(domain)s'> - <name>A_SIMPLE_VM</name> + <name>'%(name)s'</name> <memory unit='KiB'>10240</memory> <os> <type arch='%(arch)s'>hvm</type> @@ -105,8 +106,10 @@ def libvirt_supports_iso_stream(conn, protocol): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "ISO_STREAM_%s" % vm_uuid xml = ISO_STREAM_XML % {'domain': domain_type, 'protocol': protocol, - 'arch': arch} + 'arch': arch, 'name': vm_name} try: FeatureTests.disable_libvirt_error_logging() dom = conn.defineXML(xml) @@ -196,8 +199,11 @@ def has_metadata_support(conn): domain_type = 'test' if conn.getType().lower() == 'test' else 'kvm' arch = 'ppc64' if platform.machine() == 'ppc64le' \ else platform.machine() + vm_uuid = uuid.uuid1() + vm_name = "A_SIMPLE_VM_%s" % vm_uuid dom = conn.defineXML(SIMPLE_VM_XML % {'domain': domain_type, - 'arch': arch}) + 'arch': arch, + 'name': vm_name}) rollback.prependDefer(dom.undefine) try: dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT,
participants (2)
-
Aline Manera -
Christy Perez