Change in vdsm[master]: vm: Support for non-ascii vm name

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: vm: Support for non-ascii vm name
......................................................................

vm: Support for non-ascii vm name

Libvirt does not support non-ascii vm name, failing creation of such vm.
Use generated vm name based on vm id, used when vm name is not provided.

For easier debugging, we log he non-ascii name when starting a vm:

Non-ASCII vm name 'מכונה-ויראטואלית'

And also the generaed name (same when vm name is not provided):

Using generated vm name 'nb938182d-7f19-435b-9499-4cb8f1578e1b'

Note: this change may break engine side, if it expects the original vm
name in vdsm response. Fixing this on the engine side will avoid this
issue and may be better idea.

Tested with both jsonrpc and xmlrpc:
- Creating vm with non-ascii name
- Staring vm
- Opening spice console
- Stopping vm
- Restarting vdsm while vm is running
- Migrating to another host (broken with jsonrpc see bz1282054)

Change-Id: I2386c0f98aeb8161feaf19c2862be229f73eb0df
Bug-Url: https://bugzilla.redhat.com/1260131
Relates-To: https://bugzilla.redhat.com/1062943
Signed-off-by: Nir Soffer 
---
M vdsm/virt/vm.py
1 file changed, 17 insertions(+), 2 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/48570/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index f9bc25c..a6d2bc8 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -299,8 +299,7 @@
         self._devices = self._emptyDevMap()

self._connection = libvirtconnection.get(cif)
-        if 'vmName' not in self.conf:
-            self.conf['vmName'] = 'n%s' % self.id
+        self.conf['vmName'] = self._normalizedVmName(params)
         self._guestSocketFile = self._makeChannelPath(_VMCHANNEL_DEVICE_NAME)
         self._qemuguestSocketFile = self._makeChannelPath(_QEMU_GA_DEVICE_NAME)
         self.guestAgent = guestagent.GuestAgent(
@@ -380,6 +379,22 @@
                 break
         return str(idx)

+    def _normalizedVmName(self, params):
+        name = params.get('vmName')
+        if isinstance(name, unicode):
+            # Libvirt does not support non-ascii vm name
+            # See https://bugzilla.redhat.com/1260131
+            try:
+                name = name.encode('ascii')
+            except UnicodeEncodeError:
+                self.log.debug("Non-ASCII vm name '%s'", name.encode('utf8'))
+                name = None
+        if name is None:
+            # Missing or non-ascii name
+            name = 'n' + params['vmId']
+            self.log.debug("Using generated vm name %r", name)
+        return name
+
     def _normalizeVdsmImg(self, drv):
         drv['reqsize'] = drv.get('reqsize', '0')  # Backward compatible
         if 'device' not in drv:

gerrit-hooks has posted comments on this change.

Change subject: vm: Support for non-ascii vm name
......................................................................

Patch Set 1:

* #1260131::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1260131::OK, public bug
* Check Product::#1260131::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Support for non-ascii vm name
......................................................................

Patch Set 1: Verified+1

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vm: Support for non-ascii vm name
......................................................................

Patch Set 1: Code-Review-1

I am really surprised that Engine passes non-unicode vmName. The ancient virt/vm.py code assumes that

pid = supervdsm.getProxy().getVmPid(
                self.name.encode('utf-8'))

and was tested in the (distant) past.

+1 for fixing this in Engine.

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Support for non-ascii vm name
......................................................................

Patch Set 1:

Dan, engine sends unicode name, the code you quote proves that
vdsm expect to get unicode name, so what can we fix in the engine?

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: vm: Support for non-ascii vm name
......................................................................

Patch Set 1: Code-Review-1

I briefly checked the engine code and when we send vm.create we fetch the name from DB which probably means that it uses user provided input. This issue should be fixed in the engine before we send it to vdsm.

Here we can only validate the name.

[Jenkins CI RO]

Jenkins CI RO has abandoned this change.

Change subject: vm: Support for non-ascii vm name
......................................................................

Abandoned

Abandoned due to no activity - please restore if still relevant

gerrit-hooks has posted comments on this change.

Change subject: vm: Support for non-ascii vm name
......................................................................

Patch Set 3:

* #1260131::Update tracker: OK