Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-11-20 Thread Code Review 
From Dan Kenigsberg :

Dan Kenigsberg has submitted this change and it was merged.

Change subject: net: Ignore missing graphics from VM conf on target host
..


net: Ignore missing graphics from VM conf on target host

The new VM migration libvirt hook handling, recreates the interface and
graphics devices of the domxml to fit the target host. It does so using
the VM conf data.

Unfortunate, there was a bug in Engine that did not pushed the graphics
configuration to VDSM, leaving VM conf without the display information.
VDSM on the other hand, used a default, listen to all, when this data
has not been received from Engine.

On migration, the domxml includes the graphics section while the target
VM conf does not, causing an exception that fails the action.

This patch silently ignores such scenarios, leaving the source domxml as
is in the graphics section.

Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Signed-off-by: Edward Haas 
Reviewed-on: https://gerrit.ovirt.org/64300
Continuous-Integration: Jenkins CI
Reviewed-by: Petr Horáček 
Reviewed-by: Dan Kenigsberg 
---
M vdsm/virt/vm_migrate_hook.py
1 file changed, 14 insertions(+), 2 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Petr Horáček: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved
  Edward Haas: Verified



-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-11-20 Thread Code Review 
From Dan Kenigsberg :

Dan Kenigsberg has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 5: Code-Review+2

as agreed, the hook is disabled by default, fixes to it would be taken 
temporarily, until we push a nicer support through libvirt.

-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-10-03 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/64300/1/vdsm/virt/vm_migrate_hook.py
File vdsm/virt/vm_migrate_hook.py:

Line 82: raise VmMigrationHookError('No devices entity in VM conf')
Line 83: 
Line 84: try:
Line 85: _set_graphics(devices, target_vm_conf)
Line 86: except VmMigrationMissingDisplayConf:
> it is definitely eventually going to be dropped, so yes, it should be docum
So long as a VM started by a buggy Engine exists, so long lives the problem, an 
this gives life to thy patch.
Line 87: # Due to a bug in Engine, there can be a scenario where the 
domxml
Line 88: # includes a graphics section, however, the VM config on 
target does
Line 89: # not. In such cases, ignore and do not touch this section.
Line 90: pass


PS1, Line 87: a bug
add a ref to https://bugzilla.redhat.com/show_bug.cgi?id=1379122


-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-10-03 Thread michal . skrivanek 
Michal Skrivanek has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/64300/1/vdsm/virt/vm_migrate_hook.py
File vdsm/virt/vm_migrate_hook.py:

Line 82: raise VmMigrationHookError('No devices entity in VM conf')
Line 83: 
Line 84: try:
Line 85: _set_graphics(devices, target_vm_conf)
Line 86: except VmMigrationMissingDisplayConf:
> I am not sure if that will be even correct.
it is definitely eventually going to be dropped, so yes, it should be 
documented when it's no longer relevant
Line 87: # Due to a bug in Engine, there can be a scenario where the 
domxml
Line 88: # includes a graphics section, however, the VM config on 
target does
Line 89: # not. In such cases, ignore and do not touch this section.
Line 90: pass


-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-10-01 Thread edwardh 
Edward Haas has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/64300/1//COMMIT_MSG
Commit Message:

PS1, Line 10: fir
> ?
Done


-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-10-01 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 2:

* 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.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-10-01 Thread edwardh 
Edward Haas has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/64300/1/vdsm/virt/vm_migrate_hook.py
File vdsm/virt/vm_migrate_hook.py:

Line 82: raise VmMigrationHookError('No devices entity in VM conf')
Line 83: 
Line 84: try:
Line 85: _set_graphics(devices, target_vm_conf)
Line 86: except VmMigrationMissingDisplayConf:
> Add TODO to drop the code when we stop supporting version ?
I am not sure if that will be even correct.
Has the fix in Engine took care of upgrades? (Upon upgrade, fix the VM conf)
Or has it just made sure a new VM is correctly configured?
Line 87: # Due to a bug in Engine, there can be a scenario where the 
domxml
Line 88: # includes a graphics section, however, the VM config on 
target does
Line 89: # not. In such cases, ignore and do not touch this section.
Line 90: pass


-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-09-30 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 1: -Code-Review

-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-09-29 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/64300/1/vdsm/virt/vm_migrate_hook.py
File vdsm/virt/vm_migrate_hook.py:

Line 82: raise VmMigrationHookError('No devices entity in VM conf')
Line 83: 
Line 84: try:
Line 85: _set_graphics(devices, target_vm_conf)
Line 86: except VmMigrationMissingDisplayConf:
> Add TODO to drop the code when we stop supporting version ?
+1
Line 87: # Due to a bug in Engine, there can be a scenario where the 
domxml
Line 88: # includes a graphics section, however, the VM config on 
target does
Line 89: # not. In such cases, ignore and do not touch this section.
Line 90: pass


-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-09-27 Thread phoracek 
Petr Horáček has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/64300/1//COMMIT_MSG
Commit Message:

PS1, Line 10: fir
?


https://gerrit.ovirt.org/#/c/64300/1/vdsm/virt/vm_migrate_hook.py
File vdsm/virt/vm_migrate_hook.py:

Line 82: raise VmMigrationHookError('No devices entity in VM conf')
Line 83: 
Line 84: try:
Line 85: _set_graphics(devices, target_vm_conf)
Line 86: except VmMigrationMissingDisplayConf:
Add TODO to drop the code when we stop supporting version ?
Line 87: # Due to a bug in Engine, there can be a scenario where the 
domxml
Line 88: # includes a graphics section, however, the VM config on 
target does
Line 89: # not. In such cases, ignore and do not touch this section.
Line 90: pass


-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-09-25 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-09-22 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: net: Ignore missing graphics from VM conf on target host
..


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.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Ignore missing graphics from VM conf on target host

2016-09-22 Thread edwardh 
Edward Haas has uploaded a new change for review.

Change subject: net: Ignore missing graphics from VM conf on target host
..

net: Ignore missing graphics from VM conf on target host

The new VM migration libvirt hook handling, recreate the interface and
graphics devices of the domxml to fir the target host. It does so using
the VM conf data.

Unfortunate, there was a bug in Engine that did not pushed the graphics
configuration to VDSM, leaving VM conf without the display information.
VDSM on the other hand, used a default, listen to all, when this data
has not been received from Engine.

On migration, the domxml includes the graphics section while the target
VM conf does not, causing an exception that fails the action.

This patch silently ignores such scenarios, leaving the source domxml as
is in the graphics section.

Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Signed-off-by: Edward Haas 
---
M vdsm/virt/vm_migrate_hook.py
1 file changed, 13 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/64300/1

diff --git a/vdsm/virt/vm_migrate_hook.py b/vdsm/virt/vm_migrate_hook.py
index 633e4f0..0ae3995 100755
--- a/vdsm/virt/vm_migrate_hook.py
+++ b/vdsm/virt/vm_migrate_hook.py
@@ -37,6 +37,10 @@
 pass
 
 
+class VmMigrationMissingDisplayConf(Exception):
+pass
+
+
 def main(domain, event, phase, stdin=sys.stdin, stdout=sys.stdout, *args):
 if event not in ('migrate', 'restore'):
 sys.exit(0)
@@ -77,7 +81,14 @@
 if 'devices' not in target_vm_conf:
 raise VmMigrationHookError('No devices entity in VM conf')
 
-_set_graphics(devices, target_vm_conf)
+try:
+_set_graphics(devices, target_vm_conf)
+except VmMigrationMissingDisplayConf:
+# Due to a bug in Engine, there can be a scenario where the domxml
+# includes a graphics section, however, the VM config on target does
+# not. In such cases, ignore and do not touch this section.
+pass
+
 _set_bridge_interfaces(devices, target_vm_conf)
 
 
@@ -174,7 +185,7 @@
 if params and 'displayNetwork' in params and 'displayIp' in params:
 return params['displayNetwork'], params['displayIp']
 
-raise VmMigrationHookError('VM conf graphics not detected')
+raise VmMigrationMissingDisplayConf('VM conf graphics not detected')
 
 
 def _vm_item(vdscli, vm_uuid):


-- 
To view, visit https://gerrit.ovirt.org/64300
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8cc730c6448b1f70500c86b3ab39af21fa23bd5a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org