Change in vdsm[master]: gluster: fix AttributeError and TypeError in exception.py
Timothy Asir has posted comments on this change. Change subject: gluster: fix AttributeError and TypeError in exception.py .. Patch Set 2: (5 comments) https://gerrit.ovirt.org/#/c/41530/2//COMMIT_MSG Commit Message: Line 13: Line 14: This patch fixes AttributeError and TypeError when the Line 15: exception raised with an extra arguments by calling the Line 16: ancestor's __init__ method explicitly. Line 17: > Add BUG URL to refer the bz#1231722 Done Line 18: Change-Id: Ic4c72daf30aee8f0024b1ef9ee064a205a500620 https://gerrit.ovirt.org/#/c/41530/2/vdsm/gluster/exception.py File vdsm/gluster/exception.py: Line 368: Line 369: class GlusterHostStorageDeviceNotFoundException(GlusterHostException): Line 370: code = 4409 Line 371: Line 372: def __init__(self, deviceList): > Do we need to call GlusterHostException.__init__(self) here? Yes, Added Line 373: self.message = "Device(s) %s not found" % deviceList Line 374: Line 375: Line 376: class GlusterHostStorageDeviceInUseException(GlusterHostException): Line 375: Line 376: class GlusterHostStorageDeviceInUseException(GlusterHostException): Line 377: code = 4410 Line 378: Line 379: def __init__(self, deviceList): > Do we need to call GlusterHostException.__init__(self) here? Yes, Added Line 380: self.message = "Device(s) %s already in use" % deviceList Line 381: Line 382: Line 383: class GlusterHostStorageDeviceMountFailedException(GlusterHostException): Line 436: poolName) Line 437: Line 438: Line 439: class GlusterHostStorageDeviceMakeDirsFailedException(GlusterHostException): Line 440: code = 4516 > Is it 4416 instead if 4516?. Done Line 441: message = "Make directories failed" Line 442: Line 443: Line 444: class GlusterHostStorageMountPointInUseException(GlusterHostException): Line 441: message = "Make directories failed" Line 442: Line 443: Line 444: class GlusterHostStorageMountPointInUseException(GlusterHostException): Line 445: code = 4517 > Is it 4417 instead of 4517? Done Line 446: Line 447: def __init__(self, mountPoint=None): Line 448: GlusterHostException.__init__(self) Line 449: self.message = "Mount point %s is in use" % mountPoint -- To view, visit https://gerrit.ovirt.org/41530 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic4c72daf30aee8f0024b1ef9ee064a205a500620 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Timothy Asir Gerrit-Reviewer: Bala.FA Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ramesh N Gerrit-Reviewer: Shubhendu Tripathi Gerrit-Reviewer: Timothy Asir Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: fix AttributeError and TypeError in exception.py
Timothy Asir has posted comments on this change. Change subject: gluster: fix AttributeError and TypeError in exception.py .. Patch Set 3: Verified+1 -- To view, visit https://gerrit.ovirt.org/41530 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic4c72daf30aee8f0024b1ef9ee064a205a500620 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Timothy Asir Gerrit-Reviewer: Bala.FA Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ramesh N Gerrit-Reviewer: Shubhendu Tripathi Gerrit-Reviewer: Timothy Asir Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: fix AttributeError and TypeError in exception.py
automat...@ovirt.org has posted comments on this change. Change subject: gluster: fix AttributeError and TypeError in exception.py .. Patch Set 3: * update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1231722::ERROR, private bug * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/41530 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic4c72daf30aee8f0024b1ef9ee064a205a500620 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Timothy Asir Gerrit-Reviewer: Bala.FA Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ramesh N Gerrit-Reviewer: Shubhendu Tripathi Gerrit-Reviewer: Timothy Asir Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fc-connect-server: Add FcpConnection class
Nir Soffer has posted comments on this change.
Change subject: fc-connect-server: Add FcpConnection class
..
Patch Set 1:
(5 comments)
https://gerrit.ovirt.org/#/c/44010/1/vdsm/storage/storageServer.py
File vdsm/storage/storageServer.py:
Line 68: NfsConnectionParameters = namedtuple("NfsConnectionParameters",
Line 69: "export, retrans, timeout,
version, "
Line 70: "extraOptions")
Line 71:
Line 72: FcpConnectionParameters = namedtuple("FcpConnectionParameters", "type")
What is type?
Line 73:
Line 74: ConnectionInfo = namedtuple("ConnectionInfo", "type, params")
Line 75:
Line 76:
Line 550: return hsh
Line 551:
Line 552:
Line 553: class FcpConnection(object):
Line 554: def __init__(self, type):
I think you don't need type, so __init__ is not needed. There is no need to
implement __init__ if it does nothing.
Line 555: pass
Line 556:
Line 557: def connect(self):
Line 558: pass
Line 560: def disconnect(self):
Line 561: pass
Line 562:
Line 563: def isConnected(self):
Line 564: pass
Should return always True
Line 565:
Line 566: def __eq__(self, other):
Line 567: return self.__class__ == other.__class
Line 568:
Line 563: def isConnected(self):
Line 564: pass
Line 565:
Line 566: def __eq__(self, other):
Line 567: return self.__class__ == other.__class
Implement also __ne__
Line 568:
Line 569: def __hash__(self):
Line 570: return hash(type(self))
Line 571:
Line 566: def __eq__(self, other):
Line 567: return self.__class__ == other.__class
Line 568:
Line 569: def __hash__(self):
Line 570: return hash(type(self))
Use self.__class__ for consistency with __eq__
Line 571:
Line 572:
Line 573: class LocalDirectoryConnection(object):
Line 574: @property
--
To view, visit https://gerrit.ovirt.org/44010
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a2c937cb997244df7910fc7cfcae11b088d3cdb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Fred Rolland
Gerrit-Reviewer: Freddy Rolland
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fc-connect-server: Support FCP on connect server
Nir Soffer has posted comments on this change.
Change subject: fc-connect-server: Support FCP on connect server
..
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/44011/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 239: cred = iscsi.ChapCredentials(username, password)
Line 240:
Line 241: params = storageServer.IscsiConnectionParameters(target,
iface, cred)
Line 242: elif typeName == 'fcp':
Line 243: params = storageServer.FcpConnectionParameters('fcp')
Please remove type argument.
Line 244: else:
Line 245: raise se.StorageServerActionError()
Line 246:
Line 247: return storageServer.ConnectionInfo(typeName, params)
--
To view, visit https://gerrit.ovirt.org/44011
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I408d8364278a1a502fc94a2e6537cb160c716ff1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Fred Rolland
Gerrit-Reviewer: Freddy Rolland
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Live Merge: Fix pre-extension calculation for chunked drives
Nir Soffer has posted comments on this change. Change subject: Live Merge: Fix pre-extension calculation for chunked drives .. Patch Set 1: Code-Review-1 (3 comments) Looks good, need to fix a typo and can use more meaningful variable name. https://gerrit.ovirt.org/#/c/44331/1//COMMIT_MSG Commit Message: Line 13: qcow2 volume as a result of the writes. The amount of growth can be Line 14: anywhere between 0 (if all writes are replacing old data) and the Line 15: allocated size of the top volume (if all writes target unallocated Line 16: blocks in the base volume). If the top volume is the active layer the Line 17: gamount of rowth required could be more due to writes from the VM that gamount of rowth -> amount of growth Line 18: come after this calculation is performed. Line 19: Line 20: To prevent live merge failures when the base volume's LV is too small we Line 21: request an extension to accomodate the worst case scenario and increase Line 20: To prevent live merge failures when the base volume's LV is too small we Line 21: request an extension to accomodate the worst case scenario and increase Line 22: the base volume's LV by the size of the top volume's LV plus one Line 23: additional chunk. The LV will never be increased beyond the actual Line 24: capacity of the VM disk. For active layer merge, we can extend the base volume when we extend the active layer - just like extending during live storage migration. Line 25: Line 26: The old code extended the base volume's LV to the size of the top Line 27: volume's LV which does not cover all the possible scenarios. Line 28: https://gerrit.ovirt.org/#/c/44331/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4726: # the allocated size of 'top' plus one additional chunk to accomodate Line 4727: # additional writes to 'top' during the live merge operation. Line 4728: if drive.chunked: Line 4729: capacity, alloc, physical = self._getExtendInfo(drive) Line 4730: targetSize = baseSize + topSize Lets rename this to maxAlloc, which explain this calculation better. Line 4731: self.extendDriveVolume(drive, baseVolUUID, targetSize, capacity) Line 4732: Line 4733: # Trigger the collection of stats before returning so that callers Line 4734: # of getVmStats after this returns will see the new job -- To view, visit https://gerrit.ovirt.org/44331 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I959608dc2f5a71afca605610267478e8a16e2c4b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Allow "Replica" 1 domains
Nir Soffer has posted comments on this change. Change subject: gluster: Allow "Replica" 1 domains .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/44332 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia571d333e27357c3de825aaf29202cd4d1c4106a Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Allon Mureinik Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Allow "Replica" 1 domains
Ala Hino has posted comments on this change. Change subject: gluster: Allow "Replica" 1 domains .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/44332 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia571d333e27357c3de825aaf29202cd4d1c4106a Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Allon Mureinik Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Allow "Replica" 1 domains
automat...@ovirt.org has posted comments on this change. Change subject: gluster: Allow "Replica" 1 domains .. Patch Set 2: * Update tracker::#1238093::OK * Check Bug-Url::OK * Check Public Bug::#1238093::OK, public bug * Check Product::#1238093::OK, Correct product oVirt * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/44332 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia571d333e27357c3de825aaf29202cd4d1c4106a Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Allow "Replica" 1 domains
Allon Mureinik has uploaded a new change for review.
Change subject: gluster: Allow "Replica" 1 domains
..
gluster: Allow "Replica" 1 domains
Patch 322ce98d17e585b156891ed711063d5e2c0ce5de introduced the ability to
verify the replica count when connecting to gluster storage, and set the
default allowed replica count to 3.
Contrary to the statement in that patch, "replica" 1 seems to be common
in the field, and we do not want to break (too many) existing
deployments.
This patch includes:
- Changing the default configuration to support replica counts of 1 or
3.
- Update the description of this configuration value accordingly.
- Update the test to match the default configuration and validate that
having a value of '1,3' will indeed support replica counts of both 1
and 3.
Bug-Url: https://bugzilla.redhat.com/1238093
Change-Id: Ia571d333e27357c3de825aaf29202cd4d1c4106a
Signed-off-by: Allon Mureinik
---
M lib/vdsm/config.py.in
M tests/storageServerTests.py
2 files changed, 5 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/44332/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 3221aab..b37176e 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -375,10 +375,9 @@
# Section: [gluster]
('gluster', [
-('allowed_replica_counts', '3',
-'Only replica 3 is supported, this configuration is for '
-'development. Value is comma separated. For example, '
-'to allow replica 1 and replica 3, use 1,3.'),
+('allowed_replica_counts', '1,3',
+'Only replica 1 and 3 are supported. This configuration is for '
+'development only. Value is comma delimeted.'),
]),
]
diff --git a/tests/storageServerTests.py b/tests/storageServerTests.py
index d4119f3..51e1af3 100644
--- a/tests/storageServerTests.py
+++ b/tests/storageServerTests.py
@@ -193,8 +193,8 @@
self.assertEquals(gluster.options, user_options)
@MonkeyPatch(storageServer, 'supervdsm', FakeSupervdsm())
-@MonkeyPatch(GlusterFSConnection, 'ALLOWED_REPLICA_COUNTS', ('3',))
-@permutations([['1', False], ['2', False], ['3', True], ['4', False]])
+@MonkeyPatch(GlusterFSConnection, 'ALLOWED_REPLICA_COUNTS', ('1','3'))
+@permutations([['1', True], ['2', False], ['3', True], ['4', False]])
def test_allowed_gluster_replica_count(self, replica_count, supported):
def glusterVolumeInfo(volumeName=None, remoteServer=None):
--
To view, visit https://gerrit.ovirt.org/44332
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia571d333e27357c3de825aaf29202cd4d1c4106a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Allow "Replica" 1 domains
automat...@ovirt.org has posted comments on this change. Change subject: gluster: Allow "Replica" 1 domains .. Patch Set 1: * Update tracker::#1238093::OK * Check Bug-Url::OK * Check Public Bug::#1238093::OK, public bug * Check Product::#1238093::OK, Correct product oVirt * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/44332 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia571d333e27357c3de825aaf29202cd4d1c4106a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Allon Mureinik Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Live Merge: Fix pre-extension calculation for chunked drives
Adam Litke has posted comments on this change. Change subject: Live Merge: Fix pre-extension calculation for chunked drives .. Patch Set 1: Verified+1 -- To view, visit https://gerrit.ovirt.org/44331 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I959608dc2f5a71afca605610267478e8a16e2c4b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Live Merge: Fix pre-extension calculation for chunked drives
automat...@ovirt.org has posted comments on this change. Change subject: Live Merge: Fix pre-extension calculation for chunked drives .. Patch Set 1: * Update tracker::#1240360::OK * Check Bug-Url::OK * Check Public Bug::#1240360::OK, public bug * Check Product::#1240360::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/44331 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I959608dc2f5a71afca605610267478e8a16e2c4b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Live Merge: Fix pre-extension calculation for chunked drives
Adam Litke has uploaded a new change for review.
Change subject: Live Merge: Fix pre-extension calculation for chunked drives
..
Live Merge: Fix pre-extension calculation for chunked drives
Live merge causes data to be written from the volume that will be
removed (top volume) into its parent (base volume). If the base volume
is on a thinly-provisioned block device (ie. COW_FORMAT) then we need to
make sure the parent's LV is large enough to accomodate growth of the
qcow2 volume as a result of the writes. The amount of growth can be
anywhere between 0 (if all writes are replacing old data) and the
allocated size of the top volume (if all writes target unallocated
blocks in the base volume). If the top volume is the active layer the
gamount of rowth required could be more due to writes from the VM that
come after this calculation is performed.
To prevent live merge failures when the base volume's LV is too small we
request an extension to accomodate the worst case scenario and increase
the base volume's LV by the size of the top volume's LV plus one
additional chunk. The LV will never be increased beyond the actual
capacity of the VM disk.
The old code extended the base volume's LV to the size of the top
volume's LV which does not cover all the possible scenarios.
After live merge completes, the base volume's LV may be larger than is
necessary. This will not impact the VM but a manual cleanup step could
be used to reclaim some storage space.
Change-Id: I959608dc2f5a71afca605610267478e8a16e2c4b
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1240360
Signed-off-by: Adam Litke
---
M vdsm/virt/vm.py
1 file changed, 7 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/44331/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 84393f1..19dbb7c 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -4668,6 +4668,8 @@
self.log.error("merge: Refusing to merge into a shared volume")
return errCode['mergeErr']
+baseSize = int(res['info']['apparentsize'])
+
# Indicate that we expect libvirt to maintain the relative paths of
# backing files. This is necessary to ensure that a volume chain is
# visible from any host even if the mountpoint is different.
@@ -4720,12 +4722,13 @@
# copy all the required data. Normally we'd use monitoring to extend
# the volume on-demand but internal watermark information is not being
# reported by libvirt so we must do the full extension up front. In
-# the worst case, we'll need to extend 'base' to the same size as 'top'
-# plus a bit more to accommodate additional writes to 'top' during the
-# live merge operation.
+# the worst case, the allocated size of 'base' should be increased by
+# the allocated size of 'top' plus one additional chunk to accomodate
+# additional writes to 'top' during the live merge operation.
if drive.chunked:
capacity, alloc, physical = self._getExtendInfo(drive)
-self.extendDriveVolume(drive, baseVolUUID, topSize, capacity)
+targetSize = baseSize + topSize
+self.extendDriveVolume(drive, baseVolUUID, targetSize, capacity)
# Trigger the collection of stats before returning so that callers
# of getVmStats after this returns will see the new job
--
To view, visit https://gerrit.ovirt.org/44331
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I959608dc2f5a71afca605610267478e8a16e2c4b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Live Merge: Prevent merge when base volume is too small
automat...@ovirt.org has posted comments on this change. Change subject: Live Merge: Prevent merge when base volume is too small .. Patch Set 8: * Update tracker::#1232481::OK * Check Bug-Url::OK * Check Public Bug::#1232481::OK, public bug * Check Product::#1232481::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42836 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf77a7c5108b500c6ec34653ef7570a841def1b4 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Live merge: Update base size after live merge
automat...@ovirt.org has posted comments on this change. Change subject: Live merge: Update base size after live merge .. Patch Set 8: * Update tracker::#1232481::OK * Check Bug-Url::OK * Check Public Bug::#1232481::OK, public bug * Check Product::#1232481::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42921 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iae354de36db63ae3bf4b4fc7f72df5e306035784 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Live Merge: Allow extension of non-leaf raw volumes
automat...@ovirt.org has posted comments on this change. Change subject: Live Merge: Allow extension of non-leaf raw volumes .. Patch Set 5: * Update tracker::#1232481::OK * Check Bug-Url::OK * Check Public Bug::#1232481::OK, public bug * Check Product::#1232481::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43407 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82a73356a863abe39d367cdc70a9b1e914f75aee Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Live Merge: Prevent merge when base volume is too small
Dan Kenigsberg has posted comments on this change. Change subject: Live Merge: Prevent merge when base volume is too small .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/42836 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf77a7c5108b500c6ec34653ef7570a841def1b4 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: make _getInterfaceStats a function
Francesco Romani has posted comments on this change. Change subject: sampling: make _getInterfaceStats a function .. Patch Set 19: Verified+1 verified like 40429 manually comparing the output of patched and unpatched VDSM (vdsClient getVdsStats) checking for disappearing o spurious keys. Looks good. -- To view, visit https://gerrit.ovirt.org/40430 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b5796f3463585794d2696ce73f8357cb0fd1f78 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: make _getInterfaceStats a function
Francesco Romani has posted comments on this change. Change subject: sampling: make _getInterfaceStats a function .. Patch Set 19: Jenjins failure unrelated 00:10:22.063 == 00:10:22.063 ERROR: setting another mirror action should not obstract the first one 00:10:22.063 -- 00:10:22.063 Traceback (most recent call last): 00:10:22.063 File "/tmp/run/vdsm/tests/testValidation.py", line 105, in wrapper 00:10:22.063 return f(*args, **kwargs) 00:10:22.063 File "/tmp/run/vdsm/tests/tcTests.py", line 446, in setUp 00:10:22.063 _checkDependencies() 00:10:22.063 File "/tmp/run/vdsm/tests/tcTests.py", line 185, in _checkDependencies 00:10:22.063 dev.delDevice() 00:10:22.063 File "/tmp/run/vdsm/tests/tcTests.py", line 105, in delDevice 00:10:22.063 check_call([EXT_BRCTL, 'delbr', self.devName]) 00:10:22.063 File "/tmp/run/vdsm/tests/tcTests.py", line 62, in check_call 00:10:22.063 out, err) 00:10:22.063 ExecError: Command ['/usr/sbin/brctl', 'delbr', 'vdsm-9exlnKNr3X'] returned non-zero exit status 1. 00:10:22.063 >> begin captured logging << 00:10:22.063 root: DEBUG: /usr/sbin/brctl addbr vdsm-9exlnKNr3X (cwd None) 00:10:22.063 root: DEBUG: SUCCESS: = ''; = 0 00:10:22.063 root: DEBUG: /sbin/ip link set vdsm-9exlnKNr3X up (cwd None) 00:10:22.063 root: DEBUG: SUCCESS: = ''; = 0 00:10:22.063 root: DEBUG: /usr/sbin/tc qdisc add dev vdsm-9exlnKNr3X ingress (cwd None) 00:10:22.063 root: DEBUG: SUCCESS: = ''; = 0 00:10:22.063 root: DEBUG: /sbin/ip link set vdsm-9exlnKNr3X down (cwd None) 00:10:22.063 root: DEBUG: SUCCESS: = ''; = 0 00:10:22.063 root: DEBUG: /usr/sbin/brctl delbr vdsm-9exlnKNr3X (cwd None) 00:10:22.063 root: DEBUG: FAILED: = "bridge vdsm-9exlnKNr3X is still up; can't delete it\n"; = 1 00:10:22.063 - >> end captured logging << - (again?! I thought we solved this some time ago) -- To view, visit https://gerrit.ovirt.org/40430 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b5796f3463585794d2696ce73f8357cb0fd1f78 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: make _getCpuCoresStats a function
Francesco Romani has posted comments on this change. Change subject: sampling: make _getCpuCoresStats a function .. Patch Set 19: Verified+1 verified with the bundled test(s) and comparing the output of patched and unpatched VDSM (vdsClient getVdsStats). No disappeared keys, no spurious keys. V+1. -- To view, visit https://gerrit.ovirt.org/40429 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If602a84096db7aa7d1a6672b84d62287e08b0ac2 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_removeVM
Allon Mureinik has posted comments on this change. Change subject: jsonrpc: StoragePool_removeVM .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/44321 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16acb11982e70ff7f8a05ac816d36b2aa34caea0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hooks: Open vSwitch configurator
Petr Horáček has posted comments on this change.
Change subject: hooks: Open vSwitch configurator
..
Patch Set 51:
(26 comments)
https://gerrit.ovirt.org/#/c/40312/51/tests/functional/networkTestsOVS.py
File tests/functional/networkTestsOVS.py:
Line 16: #
Line 17: # Refer to the README and COPYING files for full details of the license
Line 18: #
Line 19: """ TODO
Line 20: ingerit some functions from NetworkTests
> inherit
Done
Line 21: """
Line 22: from functools import wraps
Line 23:
Line 24: from vdsm.utils import RollbackContext
Line 32: NETWORK_NAME = 'test-network'
Line 33: VLAN_ID = '27'
Line 34: BONDING_NAME = 'bond11'
Line 35:
Line 36: setupModule
> what does this serve?
it just keeps pyflakes quiet now
Line 37: tearDownModule
Line 38:
Line 39:
Line 40: @expandPermutations
Line 61: func(*args, **kwargs)
Line 62: return wrapper
Line 63:
Line 64: @staticmethod
Line 65: def _add_del_network_permutation():
> why do you need it?
i wanted something like permutation of network params (bridged, vlaned,
broken,...) and run tests on all of them in a row (like 512 ests...). This
function should create such permutation.
But i'll just add here a few skipped tests from networkTests for the start.
Line 66: pass
Line 67:
Line 68: @permutations(_add_del_network_permutation())
Line 69: def test_add_del_network_permutation(self, v, n):
https://gerrit.ovirt.org/#/c/40312/51/tests/functional/utils.py
File tests/functional/utils.py:
Line 169: if not attrs.get('bridged', True):
Line 170: raise SkipTest('OVS does not support bridgeless
networks')
Line 171: elif (not attrs.get('nic') and not
attrs.get('bonding') and
Line 172: not attrs.get('remove')):
Line 173: raise SkipTest('OVS requires at least a
nic/bonding ')
> why is that? is there any problem disconnecting a ovs bridge from all nics?
you are right, that should be nic/bonding/vlan
Line 174:
Line 175: # setup every network as OVS network
Line 176: for network in networks:
Line 177: networks[network].update({'custom': {'ovs': True}})
https://gerrit.ovirt.org/#/c/40312/51/vdsm_hooks/ovs/ovs_after_get_caps.py
File vdsm_hooks/ovs/ovs_after_get_caps.py:
> No need for object oriented programming here. OVS_Caps has no real state no
Done
Line 1: #!/usr/bin/env python
Line 2: # Copyright 2015 Red Hat, Inc.
Line 3: #
Line 4: # This program is free software; you can redistribute it and/or modify
Line 30: from ovs_utils import (get_bond_options, is_ovs_bond, rget,
EXT_OVS_APPCTL,
Line 31:EXT_OVS_VSCTL, BRIDGE_NAME)
Line 32:
Line 33:
Line 34: class OVS_Caps:
> new style class
Class removed
Line 35: def __init__(self):
Line 36: self.running_config = RunningConfig()
Line 37: self.dhcpv4ifaces, self.dhcpv6ifaces =
netinfo._get_dhclient_ifaces()
Line 38: self.routes = netinfo._get_routes()
Line 132: def bondings_caps(self):
Line 133: ovs_bonding_caps = {}
Line 134: for bonding, attrs in self.running_config.bonds.items():
Line 135: if is_ovs_bond(attrs):
Line 136: options = get_bond_options(attrs.get('options', ''),
> there are many calls to get_bond_options with attrs.get('options', ''). you
Done
Line 137:keep_custom=True)
Line 138: net_info = self.get_net_info(attrs, bonding)
Line 139: net_info['slaves'] = attrs.get('nics')
Line 140: net_info['active_slave'] =
OVS_Caps.get_active_slave(bonding)
https://gerrit.ovirt.org/#/c/40312/51/vdsm_hooks/ovs/ovs_after_get_stats.py
File vdsm_hooks/ovs/ovs_after_get_stats.py:
Line 39: vlan = attrs.get('vlan')
Line 40: iface = attrs.get('nic') or attrs.get('bonding')
Line 41: if vlan is None:
Line 42: # vlanless networks use OVS bridge as their bridge,
but VDSM
Line 43: # is (?) expecting a bridge with 'network-name' name.
> Engine expects
Done
Line 44: # create a copy of top underlying device stats and
save it
Line 45: # as bridge's stats. NOTE: copy stats of ovsbr0?
Line 46: ovs_networks_stats[network] = stats[iface]
Line 47: else:
Line 44: # create a copy of top underlying device stats and
save it
Line 45: # as bridge's stats. NOTE: copy stats of ovsbr0?
Line 46: ovs_networks_stats[network] = stats[iface]
Line 47: else:
Line 48: # VDSM is expecting stats entries for vlans named
'iface.id'
> actually, it is Engine that expect these
Done
Line 49: vlan_name = '%s.%s' % (iface, vlan)
Line 50: ovs_networks_stats[vlan_name] = stats[network
Change in vdsm[master]: hooks: Open vSwitch configurator
automat...@ovirt.org has posted comments on this change. Change subject: hooks: Open vSwitch configurator .. Patch Set 52: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40312 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id602b6cc87a663424d06c77d1847d2c2d60d289f Gerrit-PatchSet: 52 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_removeVM
Omer Frenkel has posted comments on this change. Change subject: jsonrpc: StoragePool_removeVM .. Patch Set 2: Verified+1 -- To view, visit https://gerrit.ovirt.org/44321 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16acb11982e70ff7f8a05ac816d36b2aa34caea0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_updateVMs
Omer Frenkel has posted comments on this change. Change subject: jsonrpc: StoragePool_updateVMs .. Patch Set 2: Verified+1 verified updateVM is working -- To view, visit https://gerrit.ovirt.org/44271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I26f7011b633c289f0d36bfd987f9392d2edf10f8 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_updateVMs
Maor Lipchuk has posted comments on this change. Change subject: jsonrpc: StoragePool_updateVMs .. Patch Set 2: Added the other patch that fixes also removeVM -- To view, visit https://gerrit.ovirt.org/44271 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I26f7011b633c289f0d36bfd987f9392d2edf10f8 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_removeVM
Maor Lipchuk has posted comments on this change. Change subject: jsonrpc: StoragePool_removeVM .. Patch Set 2: Verified+1 -- To view, visit https://gerrit.ovirt.org/44321 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16acb11982e70ff7f8a05ac816d36b2aa34caea0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_removeVM
automat...@ovirt.org has posted comments on this change. Change subject: jsonrpc: StoragePool_removeVM .. Patch Set 2: -Verified * Update tracker::#1237061::OK * Check Bug-Url::OK * Check Public Bug::#1237061::OK, public bug * Check Product::#1237061::OK, Correct product oVirt * Check TR::#1237061::OK, correct target release 3.5.5 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/44321 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16acb11982e70ff7f8a05ac816d36b2aa34caea0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_removeVM
automat...@ovirt.org has posted comments on this change. Change subject: jsonrpc: StoragePool_removeVM .. Patch Set 1: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/44321 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16acb11982e70ff7f8a05ac816d36b2aa34caea0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_removeVM
Hello Piotr Kliczewski, Adam Litke, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/44321 to review the following change. Change subject: jsonrpc: StoragePool_removeVM .. jsonrpc: StoragePool_removeVM Align naming between API.py and schema. Change-Id: I16acb11982e70ff7f8a05ac816d36b2aa34caea0 Signed-off-by: pkliczewski Signed-off-by: Maor Lipchuk Reviewed-on: http://gerrit.ovirt.org/35548 Reviewed-by: Oved Ourfali Reviewed-by: Adam Litke --- M vdsm/API.py 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/44321/1 diff --git a/vdsm/API.py b/vdsm/API.py index 66149c2..6e4704f 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -1134,8 +1134,8 @@ def updateVMs(self, vmList, storagedomainID=None): return self._irs.updateVM(self._UUID, vmList, storagedomainID) -def removeVM(self, vmUUID, sdUUID): -return self._irs.removeVM(self._UUID, vmUUID, sdUUID) +def removeVM(self, vmUUID, storagedomainID=None): +return self._irs.removeVM(self._UUID, vmUUID, storagedomainID) class Global(APIBase): -- To view, visit https://gerrit.ovirt.org/44321 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I16acb11982e70ff7f8a05ac816d36b2aa34caea0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Piotr Kliczewski ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Shahar Havivi has posted comments on this change.
Change subject: v2v: Import VM from OVA file
..
Patch Set 4:
(4 comments)
https://gerrit.ovirt.org/#/c/43367/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3967: 'data': {'ova_path': 'str'},
Line 3968: 'returns': ['ExternalVmInfo']}
Line 3969:
Line 3970: ##
Line 3971: # @Host.convertOva:
> same comment as https://gerrit.ovirt.org/#/c/43271/4/vdsm/rpc/vdsmapi-schem
Done
Line 3972: #
Line 3973: # Convert VM from external file (OVA) to data domain
Line 3974: #
Line 3975: # @ova_path: actual path to the ova file
https://gerrit.ovirt.org/#/c/43367/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 171: def convert_ova(ova_path, vminfo, job_id, irs):
Line 172: job = ImportVm.from_ova(ova_path, vminfo, job_id, irs)
Line 173: job.start()
Line 174: _add_job(job_id, job)
Line 175: return {'status': doneCode}
> what about using
Done
Line 176:
Line 177:
Line 178: def get_ova_info(ova_path):
Line 179: ns = {'ovf': _OVF_NS, 'rasd': _RASD_NS}
Line 293: @contextmanager
Line 294: def password_file(job_id, file_name, password):
Line 295: if file_name is None:
Line 296: yield
Line 297: return
> return is unneeded here.
I will do something like that
Line 298: fd = os.open(file_name, os.O_WRONLY | os.O_CREAT, 0o600)
Line 299: try:
Line 300: os.write(fd, password.value)
Line 301: finally:
Line 479:
get_storage_domain_path(self._prepared_volumes[0]['path']),
Line 480: self._vminfo['vmName']])
Line 481: return cmd
Line 482:
Line 483: def _from_ova_command(self):
> maybe?
I want to reserve the _command and the name looks good to me
Line 484: cmd = [_VIRT_V2V.cmd,
Line 485:'-i', 'ova', self._ova_path,
Line 486:'-o', 'vdsm',
Line 487:'-of', self._get_disk_format(),
--
To view, visit https://gerrit.ovirt.org/43367
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: jsonrpc: StoragePool_updateVMs
Omer Frenkel has posted comments on this change.
Change subject: jsonrpc: StoragePool_updateVMs
..
Patch Set 2:
i verified this fix the issue on my host (rhel 6.6 with latest 3.5) for
updateVMs but there is still (the same) issue with removeVM :
Thread-161::DEBUG::2015-08-03
15:50:12,438::__init__::481::jsonrpc.JsonRpcServer::(_serveRequest) Calling
'StoragePool.removeVM' in bridge with {'vmUUID':
'9de42bc7-e81a-4efd-bf81-ebb1f74fd6e1', 'storagepoolID':
'0002-0002-0002-0002-033d'}
Thread-161::ERROR::2015-08-03
15:50:12,440::__init__::506::jsonrpc.JsonRpcServer::(_serveRequest) Internal
server error
Traceback (most recent call last):
File "/usr/lib/python2.6/site-packages/yajsonrpc/__init__.py", line 501, in
_serveRequest
res = method(**params)
File "/usr/share/vdsm/rpc/Bridge.py", line 273, in _dynamicMethod
raise InvalidCall(fn, methodArgs, e)
InvalidCall: Attempt to call function: > with arguments:
('9de42bc7-e81a-4efd-bf81-ebb1f74fd6e1',) error: removeVM() takes exactly
3 arguments (2 given)
so please update this patch or send another for remove
--
To view, visit https://gerrit.ovirt.org/44271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I26f7011b633c289f0d36bfd987f9392d2edf10f8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Maor Lipchuk
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk
Gerrit-Reviewer: Omer Frenkel
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: tests: add requiresUnifiedPersistence decorator
automat...@ovirt.org has posted comments on this change. Change subject: net: tests: add requiresUnifiedPersistence decorator .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/44307 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I618a3782d2393d8382ea66e50ea47845cc74132a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: try restore networks harder.
automat...@ovirt.org has posted comments on this change. Change subject: net: try restore networks harder. .. Patch Set 2: * Update tracker::#1242532::OK * Check Bug-Url::OK * Check Public Bug::#1242532::OK, public bug * Check Product::#1242532::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/44274 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b9d314ed1df7fd0781113f6e77c647aaea7a6c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: tests: add requiresUnifiedPersistence decorator
Ido Barkan has posted comments on this change. Change subject: net: tests: add requiresUnifiedPersistence decorator .. Patch Set 1: Verified+1 -- To view, visit https://gerrit.ovirt.org/44307 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I618a3782d2393d8382ea66e50ea47845cc74132a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: tests: add requiresUnifiedPersistence decorator
Ido Barkan has uploaded a new change for review.
Change subject: net: tests: add requiresUnifiedPersistence decorator
..
net: tests: add requiresUnifiedPersistence decorator
So we can stop repeating
if vdsm.config.config.get('vars', 'net_persistence') == 'ifcfg':
...
boiler plate.
Change-Id: I618a3782d2393d8382ea66e50ea47845cc74132a
Signed-off-by: Ido Barkan
---
M tests/functional/networkTests.py
1 file changed, 64 insertions(+), 59 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/44307/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 4b4dbf8..99a4d3f 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -189,6 +189,18 @@
device_name, None, ipv4_address, IP_MASK, IP_GATEWAY)
+def requiresUnifiedPersistence(reason):
+def wrapper(test_method):
+@wraps(test_method)
+def wrapped_test_method(*args, **kwargs):
+if vdsm.config.config.get('vars', 'net_persistence') == 'ifcfg':
+raise SkipTest(reason)
+test_method(*args, **kwargs)
+return wrapped_test_method
+return wrapper
+
+
+
@expandPermutations
class NetworkTest(TestCaseBase):
@@ -287,11 +299,11 @@
'%s found unexpectedly' % bondName)
else:
self.assertTrue(bondName not in netinfo.bondings or (set(nics) !=
-set(netinfo.bondings[bondName]['slaves'])),
+
set(netinfo.bondings[bondName]['slaves'])),
'%s found unexpectedly' % bondName)
if config is not None:
self.assertTrue(bondName not in config.bonds or (set(nics) !=
-set(config.bonds[bondName].get('nics'))),
+
set(config.bonds[bondName].get('nics'))),
'%s found unexpectedly in running config' %
bondName)
@@ -473,7 +485,7 @@
with dummyIf(2) as nics:
status, msg = self.setupNetworks(
{NETWORK_NAME:
-{'bonding': BONDING_NAME, 'bridged': bridged}},
+ {'bonding': BONDING_NAME, 'bridged': bridged}},
{BONDING_NAME: {'nics': nics, 'options': 'mode=2'}}, NOCHK)
self.assertEqual(status, SUCCESS, msg)
self.assertNetworkExists(NETWORK_NAME, bridged)
@@ -501,8 +513,8 @@
with nonChangingOperstate(BONDING_NAME):
status, msg = self.setupNetworks(
{NETWORK_NAME:
-{'bonding': BONDING_NAME, 'bridged': bridged,
- 'vlan': VLAN_ID}},
+ {'bonding': BONDING_NAME, 'bridged': bridged,
+ 'vlan': VLAN_ID}},
{}, NOCHK)
self.assertEqual(status, SUCCESS, msg)
self.assertNetworkExists(NETWORK_NAME, bridged)
@@ -618,7 +630,7 @@
bond=bond_name,
nics=nics,
opts={'bridged':
- bridged})
+ bridged})
self.assertEqual(status, errors.ERR_BAD_BONDING, msg)
@cleanupNet
@@ -1103,11 +1115,11 @@
with dummyIf(3) as nics:
networks = {NETWORK_NAME + '1':
-dict(bonding=BONDING_NAME, bridged=bridged,
- vlan='100'),
+dict(bonding=BONDING_NAME, bridged=bridged,
+ vlan='100'),
NETWORK_NAME + '2':
-dict(bonding=BONDING_NAME, bridged=bridged,
- vlan='200', mtu=MIDI)
+dict(bonding=BONDING_NAME, bridged=bridged,
+ vlan='200', mtu=MIDI)
}
bondings = {BONDING_NAME: dict(nics=nics[:2])}
status, msg = self.setupNetworks(networks, bondings, NOCHK)
@@ -1118,8 +1130,8 @@
nics[1])
network = {NETWORK_NAME + '3':
- dict(bonding=BONDING_NAME, vlan='300', mtu=JUMBO,
-bridged=bridged)}
+ dict(bonding=BONDING_NAME, vlan='300', mtu=JUMBO,
+bridged=bridged)}
status, msg = self.setupNetworks(network, {}, NOCHK)
self.assertEquals(status, SUCCESS, msg)
@@ -1235,7 +1247,7 @@
with self.vdsm_net.pinger():
# Add initial vlanned net over bond
Change in vdsm[master]: net: try restore networks harder.
Ido Barkan has posted comments on this change. Change subject: net: try restore networks harder. .. Patch Set 2: Verified+1 -- To view, visit https://gerrit.ovirt.org/44274 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b9d314ed1df7fd0781113f6e77c647aaea7a6c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: remove HostStatsThread.get()
automat...@ovirt.org has posted comments on this change. Change subject: sampling: remove HostStatsThread.get() .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42036 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I80d919b0adb48a40d41f387543f14be265610405 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: move translation code into hoststats.py
automat...@ovirt.org has posted comments on this change. Change subject: sampling: move translation code into hoststats.py .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42034 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icce6acbd596d087491678d039282d5d37d761904 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: HostStatsThread as periodic operation
automat...@ovirt.org has posted comments on this change. Change subject: sampling: HostStatsThread as periodic operation .. Patch Set 21: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40431 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I39c2e6e4bca286a513992b7231f1356e8dd871a1 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: add 'ncpus' property to HostSample
automat...@ovirt.org has posted comments on this change. Change subject: sampling: add 'ncpus' property to HostSample .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42035 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e7d549b7772e3f38eec2dd9b91bbc6416b549bf Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: get VM information from OVA file
Shahar Havivi has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 4:
(9 comments)
https://gerrit.ovirt.org/#/c/43271/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3953:
Line 3954: ##
Line 3955: # @Host.getOvaInfo:
Line 3956: #
Line 3957: # Get VM information (disks, interfaces, memory etc) from OVA file
> We should spend a bit of time in choosing good names for public API, since
getExternalVMsFromOva sounds good
Line 3958: #
Line 3959: # @ova_path: path to the ova file
Line 3960: #
Line 3961: # Returns:
https://gerrit.ovirt.org/#/c/43271/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 180: _add_general_ovf_info(vm, root, ns)
Line 181: _add_disks_ovf_info(vm, root, ns)
Line 182: _add_networks_ovf_info(vm, root, ns)
Line 183:
Line 184: return {'status': doneCode, 'vmList': [vm]}
> please consider using the response module:
sure, you right!
Line 185:
Line 186:
Line 187: def get_converted_vm(job_id):
Line 188: try:
Line 680: params['networks'].append(i)
Line 681:
Line 682:
Line 683: def _read_ovf_from_ova(ova_path):
Line 684: cmd = ['/usr/bin/tar', 'xf', ova_path, '*.ovf', '--to-stdout']
> question: can we make use of the tarfile package?
There is no way to use the --to-stdout in tarfile package,
I will add a comment in the code
Line 685: rc, output, error = execCmd(cmd)
Line 686: if rc:
Line 687: raise V2VError(error)
Line 688:
Line 688:
Line 689: return ''.join(output)
Line 690:
Line 691:
Line 692: def _add_general_ovf_info(vm, node, ns):
> this function seems to be easily unit-testable. Do we have tests? if not, c
Done
Line 693: try:
Line 694: vm['status'] = 'Down'
Line 695: vm['vmName'] = node.find('./ovf:VirtualSystem/ovf:Name',
ns).text
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 692: def _add_general_ovf_info(vm, node, ns):
Line 693: try:
Line 694: vm['status'] = 'Down'
Line 695: vm['vmName'] = node.find('./ovf:VirtualSystem/ovf:Name',
ns).text
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
> a link to the doc - here or up when you define the _RASD_NS constant would
Done
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
> can we catch more specific exception?
yes, I will fix that
Line 701: raise V2VError('Error parsing ovf information: %s' %
e.message)
Line 702:
Line 703:
Line 704: def _add_disks_ovf_info(vm, node, ns):
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
Line 701: raise V2VError('Error parsing ovf information: %s' %
e.message)
> what's the benefit of having this Exception translation? I see no recovery
The reason is to know where the exception cam from (ie V2VError from v2v)
what do you suggest, not handling the error (ie it will be raised to the log?
Line 702:
Line 703:
Line 704: def _add_disks_ovf_info(vm, node, ns):
Line 705: try:
Line 713: alias =
node.find('.//ovf:References/ovf:File[@ovf:id="%s"]' %
Line 714: fileref, ns)
Line 715: disk['alias'] = alias.attrib.get('{%s}href' % _OVF_NS)
Line 716: vm['disks'].append(disk)
Line 717: except Exception as e:
> same as line 700
Done
Line 718: raise V2VError('Error parsing ovf disk information: %s' %
e.message)
Line 719:
Line 720:
Line 721: def _add_networks_ovf_info(vm, node, ns):
Line 731: net['type'] = 'bridge'
Line 732: else:
Line 733: net['type'] = 'interface'
Line 734: vm['networks'].append(net)
Line 735: except Exception as e:
> same as line 700
Done
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer:
Change in vdsm[master]: Removing prefix definition for qemu-kvm-ev
Michal Skrivanek has posted comments on this change. Change subject: Removing prefix definition for qemu-kvm-ev .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/43993 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3335a58799eed2e4881a05f200aad96fa5d2af92 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing prefix definition for qemu-kvm-ev
Sandro Bonazzola has posted comments on this change. Change subject: Removing prefix definition for qemu-kvm-ev .. Patch Set 2: Not sure what you tested so I can't say :-) but if you installed it on el7 centos and rhel and it worked, it should be fine. -- To view, visit https://gerrit.ovirt.org/43993 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3335a58799eed2e4881a05f200aad96fa5d2af92 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing prefix definition for qemu-kvm-ev
Yaniv Bronhaim has posted comments on this change. Change subject: Removing prefix definition for qemu-kvm-ev .. Patch Set 2: Verified+1 any additional verification is required? -- To view, visit https://gerrit.ovirt.org/43993 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3335a58799eed2e4881a05f200aad96fa5d2af92 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: get VM information from OVA file
Arik Hadas has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/43271/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 175: root = ET.fromstring(_read_ovf_from_ova(ova_path))
Line 176: except ET.ParseError as e:
Line 177: raise V2VError('Error reading ovf from ova, position: %r' %
e.position)
Line 178:
Line 179: vm = {}
> Should this contain exactly one VM?
right, we do it like that so we could reuse the response type of full-list
instead of presenting yet another struct in the engine
Line 180: _add_general_ovf_info(vm, root, ns)
Line 181: _add_disks_ovf_info(vm, root, ns)
Line 182: _add_networks_ovf_info(vm, root, ns)
Line 183:
--
To view, visit https://gerrit.ovirt.org/43271
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c55c846f837bb5bf363717e05daabf5ee6631ca
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: ssl: m2crypto removal
Piotr Kliczewski has posted comments on this change. Change subject: ssl: m2crypto removal .. Patch Set 17: Code-Review-1 Code rebased. Copying review flag. -- To view, visit https://gerrit.ovirt.org/39990 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f2688b6c00eadd3f15be0ced926a397b55c1f33 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Eldad Marciano Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Simone Tiraboschi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientif: hoststats: move elapsedTime in clientIF
Francesco Romani has posted comments on this change. Change subject: clientif: hoststats: move elapsedTime in clientIF .. Patch Set 6: Code-Review-1 it's a bad idea to add more state to clientIF -- To view, visit https://gerrit.ovirt.org/42033 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I92de260ef3988c9961ccff5a633037849a129a06 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: remove HostStatsThread.get()
automat...@ovirt.org has posted comments on this change. Change subject: sampling: remove HostStatsThread.get() .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42036 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I80d919b0adb48a40d41f387543f14be265610405 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: add 'ncpus' property to HostSample
automat...@ovirt.org has posted comments on this change. Change subject: sampling: add 'ncpus' property to HostSample .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42035 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e7d549b7772e3f38eec2dd9b91bbc6416b549bf Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientif: hoststats: move elapsedTime in clientIF
automat...@ovirt.org has posted comments on this change. Change subject: clientif: hoststats: move elapsedTime in clientIF .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42033 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I92de260ef3988c9961ccff5a633037849a129a06 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: make _getInterfaceStats a function
automat...@ovirt.org has posted comments on this change. Change subject: sampling: make _getInterfaceStats a function .. Patch Set 19: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40430 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b5796f3463585794d2696ce73f8357cb0fd1f78 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: make _getCpuCoresStats a function
automat...@ovirt.org has posted comments on this change. Change subject: sampling: make _getCpuCoresStats a function .. Patch Set 19: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40429 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If602a84096db7aa7d1a6672b84d62287e08b0ac2 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: HostStatsThread as periodic operation
automat...@ovirt.org has posted comments on this change. Change subject: sampling: HostStatsThread as periodic operation .. Patch Set 20: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40431 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I39c2e6e4bca286a513992b7231f1356e8dd871a1 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sampling: move translation code into hoststats.py
automat...@ovirt.org has posted comments on this change. Change subject: sampling: move translation code into hoststats.py .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42034 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icce6acbd596d087491678d039282d5d37d761904 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: ssl: m2crypto removal
automat...@ovirt.org has posted comments on this change. Change subject: ssl: m2crypto removal .. Patch Set 17: * Update tracker::#1147148::OK * Check Bug-Url::OK * Check Public Bug::#1147148::OK, public bug * Check Product::#1147148::OK, Correct product oVirt * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/39990 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f2688b6c00eadd3f15be0ced926a397b55c1f33 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Eldad Marciano Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Simone Tiraboschi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storageServer: Move unrelated method
Freddy Rolland has posted comments on this change. Change subject: storageServer: Move unrelated method .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/44295 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib88a491564d3c33d9e914391f0723c34b6fb9b8f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: handle exception on probing external VMs
automat...@ovirt.org has posted comments on this change. Change subject: v2v: handle exception on probing external VMs .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found -- To view, visit https://gerrit.ovirt.org/43259 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3203dde4878a80c65fe3185cadaef2fe00e6a02 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: handle exception on probing external VMs
Shahar Havivi has abandoned this change. Change subject: v2v: handle exception on probing external VMs .. Abandoned -- To view, visit https://gerrit.ovirt.org/43259 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ie3203dde4878a80c65fe3185cadaef2fe00e6a02 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storageServer: Fix MountConnection.__hash__
Freddy Rolland has posted comments on this change. Change subject: storageServer: Fix MountConnection.__hash__ .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/44294 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If895c6ea535c7f293a5c695623c11a8cbc941a9b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storageServer: Add MountConnection.__ne__
Freddy Rolland has posted comments on this change. Change subject: storageServer: Add MountConnection.__ne__ .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/44293 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I209bf318ac840f0d5fb8c4c3e1819ead90015ca3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storageServer: Fix MountConnection.__eq__
Freddy Rolland has posted comments on this change. Change subject: storageServer: Fix MountConnection.__eq__ .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/44292 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d1ace2cc2692f5da6c52ae9e88820d756043b06 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Francesco Romani has posted comments on this change. Change subject: v2v: Import VM from OVA file .. Patch Set 4: Code-Review-1 furthermore: can you please point me where you addressed comments from Arik in V2? -- To view, visit https://gerrit.ovirt.org/43367 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Import VM from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: Import VM from OVA file
..
Patch Set 4:
(4 comments)
big question(s) inside, -1 for visibility.
https://gerrit.ovirt.org/#/c/43367/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3967: 'data': {'ova_path': 'str'},
Line 3968: 'returns': ['ExternalVmInfo']}
Line 3969:
Line 3970: ##
Line 3971: # @Host.convertOva:
same comment as
https://gerrit.ovirt.org/#/c/43271/4/vdsm/rpc/vdsmapi-schema.json
Maybe
convertExternalVmFromOva
?
Line 3972: #
Line 3973: # Convert VM from external file (OVA) to data domain
Line 3974: #
Line 3975: # @ova_path: actual path to the ova file
https://gerrit.ovirt.org/#/c/43367/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 171: def convert_ova(ova_path, vminfo, job_id, irs):
Line 172: job = ImportVm.from_ova(ova_path, vminfo, job_id, irs)
Line 173: job.start()
Line 174: _add_job(job_id, job)
Line 175: return {'status': doneCode}
what about using
response.success()
?
Line 176:
Line 177:
Line 178: def get_ova_info(ova_path):
Line 179: ns = {'ovf': _OVF_NS, 'rasd': _RASD_NS}
Line 293: @contextmanager
Line 294: def password_file(job_id, file_name, password):
Line 295: if file_name is None:
Line 296: yield
Line 297: return
return is unneeded here.
I see why you did this, but it is a little ugly.
I think it is a little nicer if
- we extract what we run in _run() into another helper method:
try:
self._import()
except Exception as ex:
if self._aborted:
logging.debug("Job %r was aborted", self._id)
else:
logging.exception("Job %r failed", self._id)
self._status = STATUS.FAILED
self._description = ex.message
try:
self._abort()
except Exception as e:
logging.exception('Job %r, error trying to abort: %r',
self._id, e)
finally:
self._teardown_volumes()
Let's call this
_run_import_vm
or whatever name is more fitting.
We can also rename existing _run like
_run_import_vm_with_passwd
which should now read like
@traceback(msg="Error importing vm")
def _run(self):
with password_file(self._id, self._passwd_file, self._password):
self._run_import_vm()
or, again, any other better fitting name.
Then, we can rebind _run like you did for _create_command
Indeed this is a little clumsier that I'd like, but seems also clearer and
conforming to the other refactoring you did.
What do you think?
Line 298: fd = os.open(file_name, os.O_WRONLY | os.O_CREAT, 0o600)
Line 299: try:
Line 300: os.write(fd, password.value)
Line 301: finally:
Line 479:
get_storage_domain_path(self._prepared_volumes[0]['path']),
Line 480: self._vminfo['vmName']])
Line 481: return cmd
Line 482:
Line 483: def _from_ova_command(self):
maybe?
_create_command_from_ova?
anyway, should be similar to whatever you choose for the libvirt counterpart
Line 484: cmd = [_VIRT_V2V.cmd,
Line 485:'-i', 'ova', self._ova_path,
Line 486:'-o', 'vdsm',
Line 487:'-of', self._get_disk_format(),
--
To view, visit https://gerrit.ovirt.org/43367
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e440748ecc503f4d61e8f4f61bb0c7387589354
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: get VM information from OVA file
Francesco Romani has posted comments on this change.
Change subject: v2v: get VM information from OVA file
..
Patch Set 4: Code-Review-1
(10 comments)
mostly questions, but quite some of them. -1 for visibility.
https://gerrit.ovirt.org/#/c/43271/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3953:
Line 3954: ##
Line 3955: # @Host.getOvaInfo:
Line 3956: #
Line 3957: # Get VM information (disks, interfaces, memory etc) from OVA file
We should spend a bit of time in choosing good names for public API, since we
need to support them for a long time.
Here I don't really like the 'getOvaInfo' name.
We should pick a more intention-revealing name, and it should also remind us
that this verb is related to getExternalVMs.
Maybe
getExternalVMsFromOva
?
Just a suggestion to kickstart more thought process
Line 3958: #
Line 3959: # @ova_path: path to the ova file
Line 3960: #
Line 3961: # Returns:
https://gerrit.ovirt.org/#/c/43271/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 175: root = ET.fromstring(_read_ovf_from_ova(ova_path))
Line 176: except ET.ParseError as e:
Line 177: raise V2VError('Error reading ovf from ova, position: %r' %
e.position)
Line 178:
Line 179: vm = {}
Should this contain exactly one VM?
Line 180: _add_general_ovf_info(vm, root, ns)
Line 181: _add_disks_ovf_info(vm, root, ns)
Line 182: _add_networks_ovf_info(vm, root, ns)
Line 183:
Line 180: _add_general_ovf_info(vm, root, ns)
Line 181: _add_disks_ovf_info(vm, root, ns)
Line 182: _add_networks_ovf_info(vm, root, ns)
Line 183:
Line 184: return {'status': doneCode, 'vmList': [vm]}
please consider using the response module:
return response.success(vmList=[vm])
Line 185:
Line 186:
Line 187: def get_converted_vm(job_id):
Line 188: try:
Line 680: params['networks'].append(i)
Line 681:
Line 682:
Line 683: def _read_ovf_from_ova(ova_path):
Line 684: cmd = ['/usr/bin/tar', 'xf', ova_path, '*.ovf', '--to-stdout']
question: can we make use of the tarfile package?
https://docs.python.org/2/library/tarfile.html
If not, can you please add a quick comment reminding the future us why we did
like that?
Line 685: rc, output, error = execCmd(cmd)
Line 686: if rc:
Line 687: raise V2VError(error)
Line 688:
Line 688:
Line 689: return ''.join(output)
Line 690:
Line 691:
Line 692: def _add_general_ovf_info(vm, node, ns):
this function seems to be easily unit-testable. Do we have tests? if not, can
you please add them? (same for other _add*). It's OK to add them in a followup
patch.
Line 693: try:
Line 694: vm['status'] = 'Down'
Line 695: vm['vmName'] = node.find('./ovf:VirtualSystem/ovf:Name',
ns).text
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 692: def _add_general_ovf_info(vm, node, ns):
Line 693: try:
Line 694: vm['status'] = 'Down'
Line 695: vm['vmName'] = node.find('./ovf:VirtualSystem/ovf:Name',
ns).text
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
a link to the doc - here or up when you define the _RASD_NS constant would be
nice
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
Line 696: vm['memSize'] =
int(node.find('.//ovf:Item[rasd:ResourceType="4"]/'
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
can we catch more specific exception?
Line 701: raise V2VError('Error parsing ovf information: %s' %
e.message)
Line 702:
Line 703:
Line 704: def _add_disks_ovf_info(vm, node, ns):
Line 697: 'rasd:VirtualQuantity', ns).text)
Line 698: vm['smp'] =
int(node.find('.//ovf:Item[rasd:ResourceType="3"]/'
Line 699: 'rasd:VirtualQuantity', ns).text)
Line 700: except Exception as e:
Line 701: raise V2VError('Error parsing ovf information: %s' %
e.message)
what's the benefit of having this Exception translation? I see no recovery code
in place, so it is really matters which group (general, disks, networks)
failed, granted there is enough context in the generic exception?
Line 702:
Line 703:
Line 704: def _add_disks_ovf_info(vm, node, ns):
Line 705: try:
Line 713: alias =
node.find('.//ovf:References/ovf:File[@ovf:id="%s"]' %
Line 714: fileref, ns)
Line 715: disk['alias'] = alias.attrib.get('{%s}href' % _OVF_NS)
Line 716: vm['disks'].append(disk
Change in vdsm[master]: logging: remove log messages which give little or no value
Dima Kuznetsov has posted comments on this change. Change subject: logging: remove log messages which give little or no value .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/43720 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If5c6648f39b24adb1a5950c80a3883de0182b3b4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: refactor create command
Francesco Romani has posted comments on this change.
Change subject: v2v: refactor create command
..
Patch Set 6: Code-Review+1
(1 comment)
maybe less nice than it could but still looks good enough
https://gerrit.ovirt.org/#/c/42066/6/vdsm/v2v.py
File vdsm/v2v.py:
Line 420: else:
Line 421: raise RuntimeError("Job %r got unexpected parser
event: %s" %
Line 422:(self._id, event))
Line 423:
Line 424: def _from_libvirt_command(self):
maybe clearer as _create_from_libvirt
Line 425: cmd = [_VIRT_V2V.cmd,
Line 426:'-ic', self._uri,
Line 427:'-o', 'vdsm',
Line 428:'-of', self._get_disk_format(),
--
To view, visit https://gerrit.ovirt.org/42066
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4b9edabb2ec324693432ff71d2a8e682afbe3a35
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
