Francesco Romani has uploaded a new change for review. Change subject: vm: move device configuration in a module ......................................................................
vm: move device configuration in a module The configuration of the device of a Vm is a fairly independent step which just needs to access (in RO) some Vm fields. It is an operation done once during the lifecycle of the Vm, so it make sense to move it into a module of the vmdevices package, to improve the modularity and also to trim the still humongous size of vm.py This patch does not logic changes. Change-Id: If9b133a75cd4cb5d11b1f09aff711846eca33ff8 Signed-off-by: Francesco Romani <[email protected]> --- M debian/vdsm.install M tests/vmTests.py M vdsm.spec.in M vdsm/virt/vm.py M vdsm/virt/vmdevices/Makefile.am M vdsm/virt/vmdevices/__init__.py A vdsm/virt/vmdevices/configuration.py 7 files changed, 273 insertions(+), 209 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/36266/1 diff --git a/debian/vdsm.install b/debian/vdsm.install index 34dd776..ca2acb5 100644 --- a/debian/vdsm.install +++ b/debian/vdsm.install @@ -159,6 +159,7 @@ ./usr/share/vdsm/virt/vmxml.py ./usr/share/vdsm/virt/utils.py ./usr/share/vdsm/virt/vmdevices/__init__.py +./usr/share/vdsm/virt/vmdevices/configuration.py ./usr/share/vdsm/virt/vmdevices/core.py ./usr/share/vdsm/virt/vmdevices/graphics.py ./usr/share/vdsm/virt/vmdevices/hwclass.py diff --git a/tests/vmTests.py b/tests/vmTests.py index 28494d5..123cbf8 100644 --- a/tests/vmTests.py +++ b/tests/vmTests.py @@ -491,8 +491,8 @@ vmConf.update(self.conf) with fake.VM(vmConf) as testvm: - dev = (testvm.getConfGraphics() if isLegacy - else vmConf['devices'])[0] + dev = (vmdevices.configuration.graphics_from_conf(vmConf) + if isLegacy else vmConf['devices'])[0] graph = vmdevices.graphics.Graphics(vmConf, self.log, **dev) self.assertXML(graph.getXML(), xml) diff --git a/vdsm.spec.in b/vdsm.spec.in index a32d1dd..1466525 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -1040,6 +1040,7 @@ %{_datadir}/%{vdsm_name}/virt/vmxml.py* %{_datadir}/%{vdsm_name}/virt/utils.py* %{_datadir}/%{vdsm_name}/virt/vmdevices/__init__.py* +%{_datadir}/%{vdsm_name}/virt/vmdevices/configuration.py* %{_datadir}/%{vdsm_name}/virt/vmdevices/core.py* %{_datadir}/%{vdsm_name}/virt/vmdevices/graphics.py* %{_datadir}/%{vdsm_name}/virt/vmdevices/hwclass.py* diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index e1a765d..05237d7 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -77,8 +77,6 @@ _QEMU_GA_DEVICE_NAME = 'org.qemu.guest_agent.0' _AGENT_CHANNEL_DEVICES = (_VMCHANNEL_DEVICE_NAME, _QEMU_GA_DEVICE_NAME) -DEFAULT_BRIDGE = config.get("vars", "default_bridge") - METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0' # A libvirt constant for undefined cpu quota @@ -768,31 +766,9 @@ log = logging.getLogger("vm.Vm") # limit threads number until the libvirt lock will be fixed _ongoingCreations = threading.BoundedSemaphore(4) - DeviceMapping = ((hwclass.DISK, vmdevices.storage.Drive), - (hwclass.NIC, vmdevices.network.Interface), - (hwclass.SOUND, vmdevices.core.Sound), - (hwclass.VIDEO, vmdevices.core.Video), - (hwclass.GRAPHICS, vmdevices.graphics.Graphics), - (hwclass.CONTROLLER, vmdevices.core.Controller), - (hwclass.GENERAL, vmdevices.core.Generic), - (hwclass.BALLOON, vmdevices.core.Balloon), - (hwclass.WATCHDOG, vmdevices.core.Watchdog), - (hwclass.CONSOLE, vmdevices.core.Console), - (hwclass.REDIR, vmdevices.core.Redir), - (hwclass.RNG, vmdevices.core.Rng), - (hwclass.SMARTCARD, vmdevices.core.Smartcard), - (hwclass.TPM, vmdevices.core.Tpm)) - - def _makeDeviceDict(self): - return dict((dev, []) for dev, _ in self.DeviceMapping) def _makeChannelPath(self, deviceName): return constants.P_LIBVIRT_VMCHANNELS + self.id + '.' + deviceName - - def _getDefaultDiskInterface(self): - DEFAULT_DISK_INTERFACES = {caps.Architecture.X86_64: 'ide', - caps.Architecture.PPC64: 'scsi'} - return DEFAULT_DISK_INTERFACES[self.arch] def __init__(self, cif, params, recover=False): """ @@ -853,7 +829,7 @@ self.stopDisksStatsCollection() self._vmCreationEvent = threading.Event() self._pathsPreparedEvent = threading.Event() - self._devices = self._makeDeviceDict() + self._devices = vmdevices.configuration.make() self._connection = libvirtconnection.get(cif) if 'vmName' not in self.conf: @@ -932,47 +908,11 @@ drv['truesize'] = 0 drv['apparentsize'] = 0 - def __legacyDrives(self): - """ - Backward compatibility for qa scripts that specify direct paths. - """ - legacies = [] - DEVICE_SPEC = ((0, 'hda'), (1, 'hdb'), (2, 'hdc'), (3, 'hdd')) - for index, linuxName in DEVICE_SPEC: - path = self.conf.get(linuxName) - if path: - legacies.append({'type': hwclass.DISK, - 'device': 'disk', 'path': path, - 'iface': 'ide', 'index': index, - 'truesize': 0}) - return legacies - - def __removableDrives(self): - removables = [{ - 'type': hwclass.DISK, - 'device': 'cdrom', - 'iface': self._getDefaultDiskInterface(), - 'path': self.conf.get('cdrom', ''), - 'index': 2, - 'truesize': 0}] - floppyPath = self.conf.get('floppy') - if floppyPath: - removables.append({ - 'type': hwclass.DISK, - 'device': 'floppy', - 'path': floppyPath, - 'iface': 'fdc', - 'index': 0, - 'truesize': 0}) - return removables - def buildConfDevices(self): """ Return the "devices" section of this Vm's conf. If missing, create it according to old API. """ - devices = self._makeDeviceDict() - # For BC we need to save previous behaviour for old type parameters. # The new/old type parameter will be distinguished # by existence/absence of the 'devices' key @@ -984,25 +924,12 @@ devConf = utils.picklecopy(self.conf['devices']) except KeyError: # (very) old Engines do not send device configuration - devices[hwclass.DISK] = self.getConfDrives() - devices[hwclass.NIC] = self.getConfNetworkInterfaces() - devices[hwclass.SOUND] = self.getConfSound() - devices[hwclass.VIDEO] = self.getConfVideo() - devices[hwclass.GRAPHICS] = self.getConfGraphics() - devices[hwclass.CONTROLLER] = self.getConfController() + devices = vmdevices.configuration.from_legacy_conf(self) else: - for dev in devConf: - try: - devices[dev['type']].append(dev) - except KeyError: - self.log.warn("Unknown type found, device: '%s' " - "found", dev) - devices[hwclass.GENERAL].append(dev) + devices = vmdevices.configuration.from_vm_devices( + devConf, self.conf, self.log) - if not devices[hwclass.GRAPHICS]: - devices[hwclass.GRAPHICS] = self.getConfGraphics() - - self._checkDeviceLimits(devices) + vmdevices.configuration.check_limits(devices) # Normalize vdsm images for drv in devices[hwclass.DISK]: @@ -1037,133 +964,6 @@ if not balloonDevices: balloonDevices.append(EMPTY_BALLOON) - - def _checkDeviceLimits(self, devices): - # libvirt only support one watchdog and one console device - for device in (hwclass.WATCHDOG, hwclass.CONSOLE): - if len(devices[device]) > 1: - raise ValueError("only a single %s device is " - "supported" % device) - graphDevTypes = set() - for dev in devices[hwclass.GRAPHICS]: - if dev.get('device') not in graphDevTypes: - graphDevTypes.add(dev.get('device')) - else: - raise ValueError("only a single graphic device " - "per type is supported") - - def getConfController(self): - """ - Normalize controller device. - """ - controllers = [] - # For now we create by default only 'virtio-serial' controller - controllers.append({'type': hwclass.CONTROLLER, - 'device': 'virtio-serial'}) - return controllers - - def getConfVideo(self): - """ - Normalize video device provided by conf. - """ - - DEFAULT_VIDEOS = {caps.Architecture.X86_64: 'cirrus', - caps.Architecture.PPC64: 'vga'} - - vcards = [] - if self.conf.get('display') == 'vnc': - devType = DEFAULT_VIDEOS[self.arch] - elif self.hasSpice: - devType = 'qxl' - - monitors = int(self.conf.get('spiceMonitors', '1')) - vram = '65536' if (monitors <= 2) else '32768' - for idx in range(monitors): - vcards.append({'type': hwclass.VIDEO, - 'specParams': {'vram': vram}, - 'device': devType}) - - return vcards - - def getConfGraphics(self): - """ - Normalize graphics device provided by conf. - """ - return [{ - 'type': hwclass.GRAPHICS, - 'device': ( - 'spice' - if self.conf['display'] in ('qxl', 'qxlnc') - else 'vnc'), - 'specParams': vmdevices.graphics.makeSpecParams(self.conf)}] - - def getConfSound(self): - """ - Normalize sound device provided by conf. - """ - scards = [] - if self.conf.get('soundDevice'): - scards.append({'type': hwclass.SOUND, - 'device': self.conf.get('soundDevice')}) - - return scards - - def getConfNetworkInterfaces(self): - """ - Normalize networks interfaces provided by conf. - """ - nics = [] - macs = self.conf.get('macAddr', '').split(',') - models = self.conf.get('nicModel', '').split(',') - bridges = self.conf.get('bridge', DEFAULT_BRIDGE).split(',') - if macs == ['']: - macs = [] - if models == ['']: - models = [] - if bridges == ['']: - bridges = [] - if len(models) < len(macs) or len(models) < len(bridges): - raise ValueError('Bad nic specification') - if models and not (macs or bridges): - raise ValueError('Bad nic specification') - if not macs or not models or not bridges: - return '' - macs = macs + [macs[-1]] * (len(models) - len(macs)) - bridges = bridges + [bridges[-1]] * (len(models) - len(bridges)) - - for mac, model, bridge in zip(macs, models, bridges): - if model == 'pv': - model = 'virtio' - nics.append({'type': hwclass.NIC, - 'macAddr': mac, - 'nicModel': model, 'network': bridge, - 'device': 'bridge'}) - return nics - - def getConfDrives(self): - """ - Normalize drives provided by conf. - """ - # FIXME - # Will be better to change the self.conf but this implies an API change - # Remove this when the API parameters will be consistent. - confDrives = self.conf.get('drives', []) - if not confDrives: - confDrives.extend(self.__legacyDrives()) - confDrives.extend(self.__removableDrives()) - - for drv in confDrives: - drv['type'] = hwclass.DISK - drv['format'] = drv.get('format') or 'raw' - drv['propagateErrors'] = drv.get('propagateErrors') or 'off' - drv['readonly'] = False - drv['shared'] = False - # FIXME: For BC we have now two identical keys: iface = if - # Till the day that conf will not returned as a status anymore. - drv['iface'] = drv.get('iface') or \ - drv.get('if', self._getDefaultDiskInterface()) - - return confDrives def updateDriveIndex(self, drv): if not drv['iface'] in self._usedIndices: @@ -2184,7 +1984,7 @@ # rebooting it. Evident on, but not limited to, the HE case. self._fixLegacyConf() - for devType, devClass in self.DeviceMapping: + for devType, devClass in vmdevices.configuration.DEVICE_MAPPING: for dev in devices[devType]: self._devices[devType].append(devClass(self.conf, self.log, **dev)) @@ -4312,7 +4112,8 @@ knownDev = True # Add unknown disk device to vm's conf if not knownDev: - archIface = self._getDefaultDiskInterface() + archIface = vmdevices.configuration.default_disk_interface( + self.arch) iface = archIface if address['type'] == 'drive' else 'pci' diskDev = {'type': hwclass.DISK, 'device': devType, 'iface': iface, 'path': devPath, 'name': name, diff --git a/vdsm/virt/vmdevices/Makefile.am b/vdsm/virt/vmdevices/Makefile.am index 71b0573..870183a 100644 --- a/vdsm/virt/vmdevices/Makefile.am +++ b/vdsm/virt/vmdevices/Makefile.am @@ -21,6 +21,7 @@ vmdevicesdir = $(vdsmdir)/virt/vmdevices dist_vmdevices_PYTHON = \ __init__.py \ + configuration.py \ core.py \ graphics.py \ hwclass.py \ diff --git a/vdsm/virt/vmdevices/__init__.py b/vdsm/virt/vmdevices/__init__.py index 2835caf..4338d76 100644 --- a/vdsm/virt/vmdevices/__init__.py +++ b/vdsm/virt/vmdevices/__init__.py @@ -18,6 +18,7 @@ # Refer to the README and COPYING files for full details of the license # +from . import configuration from . import core from . import hwclass from . import graphics diff --git a/vdsm/virt/vmdevices/configuration.py b/vdsm/virt/vmdevices/configuration.py new file mode 100644 index 0000000..e28b7cc --- /dev/null +++ b/vdsm/virt/vmdevices/configuration.py @@ -0,0 +1,259 @@ +# +# Copyright 2008-2014 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +import logging + +from vdsm.config import config + +import caps + +from . import core +from . import hwclass +from . import graphics +from . import network +from . import storage + + +DEFAULT_BRIDGE = config.get("vars", "default_bridge") + + +DEVICE_MAPPING = ( + (hwclass.DISK, storage.Drive), + (hwclass.NIC, network.Interface), + (hwclass.SOUND, core.Sound), + (hwclass.VIDEO, core.Video), + (hwclass.GRAPHICS, graphics.Graphics), + (hwclass.CONTROLLER, core.Controller), + (hwclass.GENERAL, core.Generic), + (hwclass.BALLOON, core.Balloon), + (hwclass.WATCHDOG, core.Watchdog), + (hwclass.CONSOLE, core.Console), + (hwclass.REDIR, core.Redir), + (hwclass.RNG, core.Rng), + (hwclass.SMARTCARD, core.Smartcard), + (hwclass.TPM, core.Tpm)) + + +def make(): + return dict((dev, []) for dev, _ in DEVICE_MAPPING) + + +def from_legacy(vm): + devices = make() + # (very) old Engines do not send device configuration + devices[hwclass.DISK] = drives_from_conf(vm.conf, vm.arch) + devices[hwclass.NIC] = network_interfaces_from_conf(vm.conf) + devices[hwclass.SOUND] = sound_from_conf(vm.conf) + devices[hwclass.VIDEO] = video_from_conf(vm) + devices[hwclass.GRAPHICS] = graphics_from_conf(vm.conf) + devices[hwclass.CONTROLLER] = controller_from_conf() + return devices + + +def from_vm_devices(dev_conf, vm_conf=None, log=None): + log = log or logging.getLogger('vmdevices.configuration') + devices = make() + for dev in dev_conf: + try: + devices[dev['type']].append(dev) + except KeyError: + log.warn("Unknown type found, device: '%s' " + "found", dev) + devices[hwclass.GENERAL].append(dev) + + if not devices[hwclass.GRAPHICS] and vm_conf: + devices[hwclass.GRAPHICS] = graphics_from_conf(vm_conf) + return devices + + +def check_limits(devices): + # libvirt only support one watchdog and one console device + for device in (hwclass.WATCHDOG, hwclass.CONSOLE): + if len(devices[device]) > 1: + raise ValueError("only a single %s device is " + "supported" % device) + graphDevTypes = set() + for dev in devices[hwclass.GRAPHICS]: + if dev.get('device') not in graphDevTypes: + graphDevTypes.add(dev.get('device')) + else: + raise ValueError("only a single graphic device " + "per type is supported") + + +def controller_from_conf(): + """ + Normalize controller device. + """ + controllers = [] + # For now we create by default only 'virtio-serial' controller + controllers.append({'type': hwclass.CONTROLLER, + 'device': 'virtio-serial'}) + return controllers + + +def video_from_conf(vm): + """ + Normalize video device provided by conf. + """ + + DEFAULT_VIDEOS = {caps.Architecture.X86_64: 'cirrus', + caps.Architecture.PPC64: 'vga'} + + vcards = [] + if conf.get('display') == 'vnc': + devType = DEFAULT_VIDEOS[vm.arch] + elif vm.hasSpice: + devType = 'qxl' + + monitors = int(vm.conf.get('spiceMonitors', '1')) + vram = '65536' if (monitors <= 2) else '32768' + for idx in range(monitors): + vcards.append({'type': hwclass.VIDEO, + 'specParams': {'vram': vram}, + 'device': devType}) + + return vcards + + +def graphics_from_conf(conf): + """ + Normalize graphics device provided by conf. + """ + return [{ + 'type': hwclass.GRAPHICS, + 'device': ( + 'spice' + if conf['display'] in ('qxl', 'qxlnc') + else 'vnc'), + 'specParams': graphics.makeSpecParams(conf)}] + + +def sound_from_conf(conf): + """ + Normalize sound device provided by conf. + """ + scards = [] + if conf.get('soundDevice'): + scards.append({'type': hwclass.SOUND, + 'device': conf.get('soundDevice')}) + + return scards + + +def network_interfaces_from_conf(conf): + """ + Normalize networks interfaces provided by conf. + """ + nics = [] + macs = conf.get('macAddr', '').split(',') + models = conf.get('nicModel', '').split(',') + bridges = conf.get('bridge', DEFAULT_BRIDGE).split(',') + if macs == ['']: + macs = [] + if models == ['']: + models = [] + if bridges == ['']: + bridges = [] + if len(models) < len(macs) or len(models) < len(bridges): + raise ValueError('Bad nic specification') + if models and not (macs or bridges): + raise ValueError('Bad nic specification') + if not macs or not models or not bridges: + return '' + macs = macs + [macs[-1]] * (len(models) - len(macs)) + bridges = bridges + [bridges[-1]] * (len(models) - len(bridges)) + + for mac, model, bridge in zip(macs, models, bridges): + if model == 'pv': + model = 'virtio' + nics.append({'type': hwclass.NIC, + 'macAddr': mac, + 'nicModel': model, 'network': bridge, + 'device': 'bridge'}) + return nics + + +def drives_from_conf(conf, arch): + """ + Normalize drives provided by conf. + """ + # FIXME + # Will be better to change the self.conf but this implies an API change + # Remove this when the API parameters will be consistent. + confDrives = self.conf.get('drives', []) + if not confDrives: + confDrives.extend(_legacy_drives(conf)) + confDrives.extend(_removable_drives(conf, arch)) + + for drv in confDrives: + drv['type'] = hwclass.DISK + drv['format'] = drv.get('format') or 'raw' + drv['propagateErrors'] = drv.get('propagateErrors') or 'off' + drv['readonly'] = False + drv['shared'] = False + # FIXME: For BC we have now two identical keys: iface = if + # Till the day that conf will not returned as a status anymore. + drv['iface'] = drv.get('iface') or \ + drv.get('if', default_disk_interface(arch)) + + return confDrives + + +def default_disk_interface(arch): + DEFAULT_DISK_INTERFACES = {caps.Architecture.X86_64: 'ide', + caps.Architecture.PPC64: 'scsi'} + return DEFAULT_DISK_INTERFACES[arch] + + +def _legacy_drives(conf): + """ + Backward compatibility for qa scripts that specify direct paths. + """ + legacies = [] + DEVICE_SPEC = ((0, 'hda'), (1, 'hdb'), (2, 'hdc'), (3, 'hdd')) + for index, linuxName in DEVICE_SPEC: + path = conf.get(linuxName) + if path: + legacies.append({'type': hwclass.DISK, + 'device': 'disk', 'path': path, + 'iface': 'ide', 'index': index, + 'truesize': 0}) + return legacies + + +def _removable_drives(conf, arch): + removables = [{ + 'type': hwclass.DISK, + 'device': 'cdrom', + 'iface': default_disk_interface(arch), + 'path': conf.get('cdrom', ''), + 'index': 2, + 'truesize': 0}] + floppyPath = conf.get('floppy') + if floppyPath: + removables.append({ + 'type': hwclass.DISK, + 'device': 'floppy', + 'path': floppyPath, + 'iface': 'fdc', + 'index': 0, + 'truesize': 0}) + return removables -- To view, visit http://gerrit.ovirt.org/36266 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If9b133a75cd4cb5d11b1f09aff711846eca33ff8 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
