9:30 a.m.
Francesco Romani has uploaded a new change for review.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
vdsm: prepareVolumePath payload misdetection fix
The changeset http://gerrit.ovirt.org/#/c/22324/3
used a condition too broad for its check, causing
a device path misdetection when cloud-init is used.
This in turn makes vdsm to produce an invalid xml
with duplicate cdrom IDs, culminating in a qemu
failure at start.
See the buzilla linked below for a full description
of a misbehaviour.
This patch provides a band-aid fix to the issue
by tightening the prepareVolumePath check.
Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Bug-Url: https://bugzilla.redhat.com/1047356
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/clientIF.py
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/25/22925/1
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 7e6fcf8..8e7f5f4 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -278,7 +278,8 @@
# next line can be removed in future, when < 3.3 engine
# is not supported
drive['specParams'].get('path', '') ==
'' and
- drive.get('path', '') == ''):
+ drive.get('path', '') == '' and
+ 'vmPayload' not in drive['specParams']):
volPath = ''
# ... or load the drive from vmPayload:
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
6:22 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
vdsm: prepareVolumePath payload misdetection fix
The changeset http://gerrit.ovirt.org/#/c/22324/3
used a condition too broad for its check, causing
a device path misdetection when cloud-init is used.
It is triggered in the following case:
specParams vmPayload attribute has
"file"(with content), path" non existent.
Device's (drive's) "path" is set to ''.
The code then consider the cdrom with vmPayload
a case of empty CDROM and creating it as such.
It did work before because the condition results
false when there is no 'path' in drive['specParams']
(which is the case here) and goes to the
next condition section (the right one with payload handling)
This patch provides a band-aid fix to the issue
by tightening the prepareVolumePath check.
Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Bug-Url: https://bugzilla.redhat.com/1047356
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/22925
Reviewed-by: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm/clientIF.py
1 file changed, 2 insertions(+), 1 deletion(-)
Approvals:
Vinzenz Feenstra: Looks good to me, but someone else must approve
Dan Kenigsberg: Looks good to me, approved
Francesco Romani: Verified
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
11:57 a.m.
Francesco Romani has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 1:
(2 comments)
....................................................
Commit Message
Line 9: The changeset http://gerrit.ovirt.org/#/c/22324/3
Line 10: used a condition too broad for its check, causing
Line 11: a device path misdetection when cloud-init is used.
Line 12:
Line 13: This in turn makes vdsm to produce an invalid xml
I agree it could be put down in a better way. Maybe it is better to quote
https://bugzilla.redhat.com/show_bug.cgi?id=1047356#c9 as you suggest below.
Line 14: with duplicate cdrom IDs, culminating in a qemu
Line 15: failure at start.
Line 16:
Line 17: See the buzilla linked below for a full description
Line 13: This in turn makes vdsm to produce an invalid xml
Line 14: with duplicate cdrom IDs, culminating in a qemu
Line 15: failure at start.
Line 16:
Line 17: See the buzilla linked below for a full description
OK, Will update. On this topic: you think it is better fixed the way it is now or the way
it is in the comment linked above?
Line 18: of a misbehaviour.
Line 19:
Line 20: This patch provides a band-aid fix to the issue
Line 21: by tightening the prepareVolumePath check.
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
11:20 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
+1 for the code, but I think that the commit message is a bit confusing as it stands.
....................................................
Commit Message
Line 9: The changeset http://gerrit.ovirt.org/#/c/22324/3
Line 10: used a condition too broad for its check, causing
Line 11: a device path misdetection when cloud-init is used.
Line 12:
Line 13: This in turn makes vdsm to produce an invalid xml
Is this really the case? On BZ 1047360 you can see that two cdrom entries were passed from
Engine. The problem here is that we produce an empty cdrom entry.
Line 14: with duplicate cdrom IDs, culminating in a qemu
Line 15: failure at start.
Line 16:
Line 17: See the buzilla linked below for a full description
Line 13: This in turn makes vdsm to produce an invalid xml
Line 14: with duplicate cdrom IDs, culminating in a qemu
Line 15: failure at start.
Line 16:
Line 17: See the buzilla linked below for a full description
the bugzilla has long logs and a many comments - this commit message should be enough for
a succinct explanation like that of
https://bugzilla.redhat.com/show_bug.cgi?id=1047356#c9
Line 18: of a misbehaviour.
Line 19:
Line 20: This patch provides a band-aid fix to the issue
Line 21: by tightening the prepareVolumePath check.
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9:30 a.m.
Francesco Romani has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 1:
this is the counterpart for master of
http://gerrit.ovirt.org/#/c/22923/
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-HasComments: No
9:33 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6519/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5626/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6432/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:09 p.m.
Francesco Romani has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 1: Verified+1
...actually marked as verified on my side.
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:06 a.m.
Francesco Romani has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 1:
All tests were made using 'Run Once'.
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
5:42 a.m.
Francesco Romani has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 2: Verified+1
verified ok on my side
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
5:34 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:09 p.m.
Francesco Romani has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 1:
Verified the following:
* new VM from scratch, linux, just disk (no cdrom), cloud init enabled: started
* new VM from scratch, linux, just disk (no cdrom), NO cloud init: started
* new VM from scratch, linux, disk, cdrom (boot from cdrom), cloud init enabled: started
* existing VM, linux, already installed on disk, no cdrom, cloud init enabled: started
* existing VM, linux, already installed on disk, no cdrom, NO cloud initL started
* existing VM, linux, already installed on disk, cdrom (boot from disk), cloud init
enabled: started
thus marking as verified on my side
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3:17 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6528/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5635/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6441/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3:14 a.m.
Francesco Romani has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 2:
Improved the commit message.
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:19 a.m.
Vinzenz Feenstra has posted comments on this change.
Change subject: vdsm: prepareVolumePath payload misdetection fix
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/22925
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie847eb92f28d24f4d8189e03a47cd64cc6597363
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3807
days inactive
3808
days old
vdsm-patches@lists.fedorahosted.org
13 comments
4 participants
participants (4)
-
Dan Kenigsberg -
fromani＠redhat.com -
Jenkins CI RO -
Vinzenz Feenstra