Change in vdsm[master]: vm: Require format attribute for drives

nsoffer at redhat.com nsoffer at redhat.com
Sun Feb 9 12:22:04 UTC 2014


Nir Soffer has uploaded a new change for review.

Change subject: vm: Require format attribute for drives
......................................................................

vm: Require format attribute for drives

We have seen sampling errors where a drive has no "format" attribute.
These errors spam the system logs, and does not help to debug the real
issue - why the required format attribute is not set?

This patch ensure that a drive cannot be created without a format
attribute, avoding log spam by sampling errors, and hopefully revealing
the real reason for this bug.

One broken test creating a drive without a format was fixed and new test
ensure that creating drive without a format raises.

Change-Id: I01ab1e071ecb76f383cc6dc7d99782e10cc90136
Relates-To: http://gerrit.ovirt.org/22551
Relates-To: https://bugzilla.redhat.com/994534
Relates-To: http://lists.ovirt.org/pipermail/users/2014-February/021007.html
Bug-Url: https://bugzilla.redhat.com/1055437
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M tests/vmTests.py
M vdsm/vm.py
2 files changed, 28 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/34/24234/1

diff --git a/tests/vmTests.py b/tests/vmTests.py
index 1e0a3f6..724d458 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -449,6 +449,18 @@
         redir = vm.RedirDevice(self.conf, self.log, **dev)
         self.assertXML(redir.getXML(), redirXML)
 
+    def testDriveRequiredParameters(self):
+        # TODO: It is not clear what are the other required parameters, and it
+        # the parameters depend on the type of drive. We probbaly need bigger
+        # test here that test many combinations.
+        # Currently this test only missing "format" attribute.
+        conf = {'index': '2', 'propagateErrors': 'off', 'iface': 'ide',
+                'name': 'hdc', 'device': 'cdrom', 'path': '/tmp/fedora.iso',
+                'type': 'disk', 'readonly': 'True', 'shared': 'none',
+                'serial': '54-a672-23e5b495a9ea'}
+        self.assertRaises(vm.InvalidDeviceParameters, vm.Drive, {}, self.log,
+                          **conf)
+
     def testDriveSharedStatus(self):
         sharedConfigs = [
             # Backward compatibility
@@ -470,7 +482,8 @@
             'exclusive', 'shared', 'none', 'transient',
         ]
 
-        driveConfig = {'index': '0', 'iface': 'virtio', 'device': 'disk'}
+        driveConfig = {'index': '0', 'iface': 'virtio', 'device': 'disk',
+                       'format': 'raw'}
 
         for driveInput, driveOutput in zip(sharedConfigs, expectedStates):
             driveInput.update(driveConfig)
diff --git a/vdsm/vm.py b/vdsm/vm.py
index aae8bd6..b7151b1 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -80,6 +80,10 @@
 SMARTCARD_DEVICES = 'smartcard'
 
 
+class InvalidDeviceParameters(Exception):
+    """ Raised when creating device with invalid parameters """
+
+
 def isVdsmImage(drive):
     """
     Tell if drive looks like a vdsm image
@@ -1438,6 +1442,9 @@
     VOLWM_CHUNK_REPLICATE_MULT = 2  # Chunk multiplier during replication
 
     def __init__(self, conf, log, **kwargs):
+        if 'format' not in kwargs:
+            raise InvalidDeviceParameters('"format" attribute is required:'
+                                          ' %r' % kwargs)
         if not kwargs.get('serial'):
             self.serial = kwargs.get('imageID'[-20:]) or ''
         VmDevice.__init__(self, conf, log, **kwargs)
@@ -3108,8 +3115,13 @@
 
         for devType, devClass in self.DeviceMapping:
             for dev in devices[devType]:
-                self._devices[devType].append(devClass(self.conf, self.log,
-                                                       **dev))
+                try:
+                    drive = devClass(self.conf, self.log, **dev)
+                except InvalidDeviceParameters as e:
+                    self.log.error('Ignoring device with invalid parameters:'
+                                   ' %s', e, exc_info=True)
+                else:
+                    self._devices[devType].append(drive)
 
         # We should set this event as a last part of drives initialization
         self._pathsPreparedEvent.set()


-- 
To view, visit http://gerrit.ovirt.org/24234
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I01ab1e071ecb76f383cc6dc7d99782e10cc90136
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list