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 nsoffer@redhat.com --- 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 has posted comments on this change.
Change subject: vm: Support for non-ascii vm name ......................................................................
Patch Set 1: Verified+1
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 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 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 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
vdsm-patches@lists.fedorahosted.org