Federico Simoncelli has posted comments on this change.
Change subject: vm: Require format attribute for drives
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/24234/1/vdsm/vm.py
File vdsm/vm.py:
Line 1441: VOLWM_FREE_PCT = 100 - config.getint('irs',
'volume_utilization_percent')
Line 1442: VOLWM_CHUNK_REPLICATE_MULT = 2 # Chunk multiplier during replication
Line 1443:
Line 1444: def __init__(self, conf, log, **kwargs):
Line 1445: if 'format' not in kwargs:
We can, but this will hide the faulty code calling us without a
required pa
Setting a default is wrong and failing is harsh.
There's also another case that we want to fix: what if the parameter is wrong? (e.g.
format passed by engine == 'raw' but volume is cow? or the other way around).
On creation:
* if the parameter is missing it should be filled in with a value taken from the metadata
(e.g. getVolumeInfo)
* if the parameter is present and wrong we want to fail
Later on (e.g. migration or vdsm restart):
* if the parameter is missing it should be filled in with a value taken from getVolumeInfo
or using what libvirt reports in its xml (but we probably want to log this as there's
a bug somewhere)
Basically the format passed by engine instead of being mandatory should be optional and
validated for correctness if present.
Line 1446: raise InvalidDeviceParameters('"format" attribute is
required:'
Line 1447: ' %r' % kwargs)
Line 1448: if not kwargs.get('serial'):
Line 1449: self.serial = kwargs.get('imageID'[-20:]) or ''
--
To view, visit
http://gerrit.ovirt.org/24234
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I01ab1e071ecb76f383cc6dc7d99782e10cc90136
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Meital Bourvine <mbourvin(a)redhat.com>
Gerrit-Reviewer: Meital bourvine <meitalbourvine(a)gmail.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes