7:43 a.m.
On 10/29/2014 06:26 PM, Paulo Ricardo Paz Vital wrote:
On Tue, 2014-10-28 at 10:43 -0200, Aline Manera wrote:
> When QEMU is not able to solve hostname for a remote CDROM path, we need
> to convert the hostname to IP to avoid problems.
> To make xmlutils/ independent of model instances, that verification must
> be done prior to the XML generation.
>
> Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
> ---
> src/kimchi/model/vmstorages.py | 16 ++++++++++++++++
> tests/test_model.py | 11 ++++++++++-
> 2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py
> index 70240b0..576d66d 100644
> --- a/src/kimchi/model/vmstorages.py
> +++ b/src/kimchi/model/vmstorages.py
> @@ -17,12 +17,15 @@
> # License along with this library; if not, write to the Free Software
> # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>
> +import socket
> import string
> +import urlparse
>
> from lxml import etree
>
> from kimchi.exception import InvalidOperation, InvalidParameter, NotFoundError
> from kimchi.exception import OperationFailed
> +from kimchi.model.config import CapabilitiesModel
> from kimchi.model.vms import DOM_STATE_MAP, VMModel
> from kimchi.model.storagevolumes import StorageVolumeModel
> from kimchi.model.utils import get_vm_config_flag
> @@ -122,6 +125,13 @@ class VMStoragesModel(object):
> params['path'] = vol_info['path']
>
> params.update(self._get_available_bus_address(params['bus'],
vm_name))
> +
> + # Check QEMU can resolve hostname
> + hostname = urlparse.urlparse(params['path']).hostname
> + if hostname is not None and not CapabilitiesModel().qemu_stream_dns:
> + ip = socket.gethostbyname(hostname)
> + params['path'] = params['path'].replace(hostname, ip)
> +
> # Add device to VM
> dev, xml = get_disk_xml(params)
> try:
> @@ -178,6 +188,12 @@ class VMStorageModel(object):
> params['path'] = params.get('path', '')
> dev_info.update(params)
>
> + # Check QEMU can resolve hostname
> + hostname = urlparse.urlparse(params['path']).hostname
> + if hostname is not None and not CapabilitiesModel().qemu_stream_dns:
> + ip = socket.gethostbyname(hostname)
> + params['path'] = params['path'].replace(hostname, ip)
> +
The same block of code in two different parts? Why not create a new
function on utils a function to do this check?
Sure. I will do that on V3.
One more question? Shouldn't the "dev_info.update(params)" line moved to
be after the "Check QEMU can resolve hostname" block? Looks like the
params['path'] will contain the wrong value if the "if" statement is
true.
Yeap. I will fix it on V3 too.
Thanks.
> dev, xml = get_disk_xml(dev_info)
> try:
> dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all'))
> diff --git a/tests/test_model.py b/tests/test_model.py
> index e407fe5..f094ed7 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -25,9 +25,11 @@ import psutil
> import pwd
> import re
> import shutil
> +import socket
> import tempfile
> import threading
> import time
> +import urlparse
> import unittest
> import uuid
>
> @@ -433,7 +435,14 @@ class ModelTests(unittest.TestCase):
> {'path': valid_remote_iso_path})
> cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
> cur_cdrom_path = re.sub(":80/", '/',
cdrom_info['path'])
> - self.assertEquals(valid_remote_iso_path, cur_cdrom_path)
> +
> + # As Kimchi server is not running during this test case
> + # CapabilitiesModel.qemu_stream_dns will be always False
> + # so we need to convert the hostname to IP
> + output = urlparse.urlparse(valid_remote_iso_path)
> + hostname = socket.gethostbyname(output.hostname)
> + url = valid_remote_iso_path.replace(output.hostname, hostname)
> + self.assertEquals(url, cur_cdrom_path)
>
> @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
> def test_vm_storage_provisioning(self):