Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 27: removed the 'display' treatment in saveState (it breaked recovery after the last changes). I lost a +1 but it was needed :( Verification in progress, using unpatched engine. SPICE part OK: - creation - recovery - migration destination/source, repeatead - hibernation/dehibernation - verified vdsClient getVmStats output (display* params) - verified engine logs, no worrysome messages spotted Will tick Verified once VNC is tested as above -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 27: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8981/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9122/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8193/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Dan Kenigsberg has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 27: Code-Review-1
(4 comments)
http://gerrit.ovirt.org/#/c/26895/27//COMMIT_MSG
Commit Message:
Line 24:
Line 25: Noteworthy side effects of this patch:
Line 26: * It is now possible to create a VM without any display
Line 27: (aka headless VM), and they are supported.
Line 28: * The input 'display' parameter of the Vm.create API is
I'm all for headless VMs, but what was the motivation of introducing this side
effect in this refactoring patch?
Line 29: no longer mandatory
Line 30: * In the API schema, the display* parameters are now
Line 31: marked as optional, even though Engine is expected to still
Line 32: send them in the near/medium term.
Line 29: no longer mandatory
Line 30: * In the API schema, the display* parameters are now
Line 31: marked as optional, even though Engine is expected to still
Line 32: send them in the near/medium term.
Line 33: * setTicket and the internally used _reviveTicket can now
it's fine for setTicket, as it is called from outside. But _reviveTicket must
never be called if there is no ticket to revive.
Line 34: fail if they are invoked against a VM without graphic devices.
Line 35:
Line 36: Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
http://gerrit.ovirt.org/#/c/26895/27/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4281: def setTicket(self, otp, seconds, connAct, params):
Line 4282: try:
Line 4283: graphics =
_domParseStr(self._dom.XMLDesc(0)).childNodes[0]. \
Line 4284: getElementsByTagName('graphics')[0]
Line 4285: except IndexError:
Why do you add support for optional display in this patch? I thought it
intended to be refactoring-only, with no functional effect.
Line 4286: return {
Line 4287: 'status': {'code':
errCode['ticketErr']['status']['code'],
Line 4288:'message': 'no graphics devices
configured'}}
Line 4289: graphics.setAttribute('passwd', otp)
Line 4309: try:
Line 4310: graphics = _domParseStr(
Line 4311: self._dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)). \
Line 4312: childNodes[0].getElementsByTagName('graphics')[0]
Line 4313: except IndexError:
Why is this needed? It seems that this function is called only if
self._vm.hasSpice.
Swallowing an error like this is a sure way to hide bugs.
Line 4314: self.log.error('no graphics devices configured')
Line 4315: return
Line 4316: validto =
max(time.strptime(graphics.getAttribute('passwdValidTo'),
Line 4317: '%Y-%m-%dT%H:%M:%S'),
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Michal Skrivanek has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 27:
(2 comments)
http://gerrit.ovirt.org/#/c/26895/27/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4281: def setTicket(self, otp, seconds, connAct, params):
Line 4282: try:
Line 4283: graphics =
_domParseStr(self._dom.XMLDesc(0)).childNodes[0]. \
Line 4284: getElementsByTagName('graphics')[0]
Line 4285: except IndexError:
Why do you add support for optional display in this patch? I thought it i
I'd say regardless the new feature, it's a safenet which is good to have
anyway.
(I'm good either way if it's here or next patch)
Line 4286: return {
Line 4287: 'status': {'code':
errCode['ticketErr']['status']['code'],
Line 4288:'message': 'no graphics devices
configured'}}
Line 4289: graphics.setAttribute('passwd', otp)
Line 4309: try:
Line 4310: graphics = _domParseStr(
Line 4311: self._dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)). \
Line 4312: childNodes[0].getElementsByTagName('graphics')[0]
Line 4313: except IndexError:
Why is this needed? It seems that this function is called only if self._vm.
again as a protection it makes sense to me
Line 4314: self.log.error('no graphics devices configured')
Line 4315: return
Line 4316: validto =
max(time.strptime(graphics.getAttribute('passwdValidTo'),
Line 4317: '%Y-%m-%dT%H:%M:%S'),
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 27: Verified+1 -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 27:
(4 comments)
http://gerrit.ovirt.org/#/c/26895/27//COMMIT_MSG
Commit Message:
Line 24:
Line 25: Noteworthy side effects of this patch:
Line 26: * It is now possible to create a VM without any display
Line 27: (aka headless VM), and they are supported.
Line 28: * The input 'display' parameter of the Vm.create API is
I'm all for headless VMs, but what was the motivation of introducing this s
A plain and simple mistake. Patch splitted into:
http://gerrit.ovirt.org/#/c/27846/
Now the current patch is stripped back to the bare refactoring changes and
preparation for the other patches in the serie.
Line 29: no longer mandatory
Line 30: * In the API schema, the display* parameters are now
Line 31: marked as optional, even though Engine is expected to still
Line 32: send them in the near/medium term.
Line 29: no longer mandatory
Line 30: * In the API schema, the display* parameters are now
Line 31: marked as optional, even though Engine is expected to still
Line 32: send them in the near/medium term.
Line 33: * setTicket and the internally used _reviveTicket can now
it's fine for setTicket, as it is called from outside. But _reviveTicket mu
Rechecked, should be fine because _reviveTicket is called only if vm.hasSpice.
Line 34: fail if they are invoked against a VM without graphic devices.
Line 35:
Line 36: Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
http://gerrit.ovirt.org/#/c/26895/27/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4281: def setTicket(self, otp, seconds, connAct, params):
Line 4282: try:
Line 4283: graphics =
_domParseStr(self._dom.XMLDesc(0)).childNodes[0]. \
Line 4284: getElementsByTagName('graphics')[0]
Line 4285: except IndexError:
I'd say regardless the new feature, it's a safenet which is good to have
Kept, in a different patch.
Line 4286: return {
Line 4287: 'status': {'code':
errCode['ticketErr']['status']['code'],
Line 4288:'message': 'no graphics devices
configured'}}
Line 4289: graphics.setAttribute('passwd', otp)
Line 4309: try:
Line 4310: graphics = _domParseStr(
Line 4311: self._dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)). \
Line 4312: childNodes[0].getElementsByTagName('graphics')[0]
Line 4313: except IndexError:
again as a protection it makes sense to me
Kept, in a different patch.
Line 4314: self.log.error('no graphics devices configured')
Line 4315: return
Line 4316: validto =
max(time.strptime(graphics.getAttribute('passwdValidTo'),
Line 4317: '%Y-%m-%dT%H:%M:%S'),
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 28: Verified+1 Verified all the flows - creation (spice, vnc) - migration source/destination with vanilla 4.14.8.1 (spice, vnc) - recovery - hibernation/dehibernation. -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Antoni Segura Puimedon has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 28: Code-Review+1
(1 comment)
http://gerrit.ovirt.org/#/c/26895/28/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4326: def hasSpice(self):
Line 4327: return (self.conf.get('display') == 'qxl' or
Line 4328: any(dev['device'] == 'spice'
Line 4329: for dev in self.conf.get('devices', [])
Line 4330: if dev['type'] == GRAPHICS_DEVICES))
Wouldn't it be more readable:
any(dev for dev in self.conf.get('devices', ()) if
dev['type'] = GRAPHICS_DEVICES and dev['device'] == 'spice')
Line 4331:
Line 4332: def _getPid(self):
Line 4333: pid = '0'
Line 4334: try:
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 28
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Dan Kenigsberg has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 28: Code-Review+2
(1 comment)
http://gerrit.ovirt.org/#/c/26895/28/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4326: def hasSpice(self):
Line 4327: return (self.conf.get('display') == 'qxl' or
Line 4328: any(dev['device'] == 'spice'
Line 4329: for dev in self.conf.get('devices', [])
Line 4330: if dev['type'] == GRAPHICS_DEVICES))
Wouldn't it be more readable:
I'm not sure. And much like you, not going to block on that.
Line 4331:
Line 4332: def _getPid(self):
Line 4333: pid = '0'
Line 4334: try:
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 28
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Dan Kenigsberg has submitted this change and it was merged. Change subject: virt: graphdev: add the GraphicsDevice class .. virt: graphdev: add the GraphicsDevice class this patch introduces the Graphics Device class to be used to define the Vm display properties. No more than one Graphics Device is supported. If VDSM receives a Graphic Device definition, it uses that; otherwise, tries to fallback to display* parameters received as input. VDSM will continue to supply the legacy display* parameters in stats output and in the conf dictionary (VmDefinition) to preserve the backward compatibility. This includes migrations, so a VM created with a single new style graphic device can be migrated to old VDSMs without breaking cluster compatibility. Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Signed-off-by: Francesco Romani from...@redhat.com Reviewed-on: http://gerrit.ovirt.org/26895 Reviewed-by: Antoni Segura Puimedon asegu...@redhat.com Reviewed-by: Dan Kenigsberg dan...@redhat.com --- M tests/functional/virtTests.py M tests/vmTests.py M tests/vmTestsData.py M vdsm/API.py M vdsm/virt/vm.py M vdsm_api/vdsmapi-schema.json 6 files changed, 393 insertions(+), 132 deletions(-) Approvals: Antoni Segura Puimedon: 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/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 29 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 29: Build Successful http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1312/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 29 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 28: Build Unstable http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9001/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9142/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8213/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Antoni Segura Puimedon has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 26: Code-Review+1 Thanks for the changes to the testing. It is much safer now ;-) -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 26: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8918/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9055/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8128/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 25: Addressed Toni's comment. We agreed to support headless VM, so enhance the commit message and the API schema. -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 25: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8869/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9005/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8079/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 24: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8828/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8964/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8038/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 24: greatly simplified after the discussion in the last VDSM sync call; removed the 'display' mandatory parameter, thus much more simpler and straightforward implementation. Restored full Vm.conf backward compatibility. -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Antoni Segura Puimedon has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 24: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/26895/24/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1395: if not utils.tobool(self.specParams.get('disableTicketing', '')):
I think that False would be better than '' considering utils.tobool would raise
and catch an exception with '' before returning false.
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Antoni Segura Puimedon has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 24:
(6 comments)
http://gerrit.ovirt.org/#/c/26895/24/vdsm/API.py
File vdsm/API.py:
Line 199: except:
Line 200: self.log.error(Error restoring VM parameters,
Line 201:exc_info=True)
Line 202:
Line 203: requiredParams = ['vmId', 'memSize']
To maintain the same behavior (error reporting), shouldn't you have a check
that there is either 'display' or at least on graphic device
Line 204: for param in requiredParams:
Line 205: if param not in vmParams:
Line 206: self.log.error('Missing required parameter %s' %
(param))
Line 207: return {'status': {'code': errCode['MissParam']
http://gerrit.ovirt.org/#/c/26895/24/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4848: self.conf['devices'].append(diskDev)
Line 4849:
Line 4850: def _getUnderlyingGraphicsDeviceInfo(self):
Line 4851:
Line 4852: Obtain graphics framebuffer devices info from libvirt.
s/graphics/graphic/
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping
between
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in
libvirt
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics
devices,
Line 4849:
Line 4850: def _getUnderlyingGraphicsDeviceInfo(self):
Line 4851:
Line 4852: Obtain graphics framebuffer devices info from libvirt.
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping
between
s/only allows 0 or 1/allows at most one/
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in
libvirt
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics
devices,
Line 4857: virsh responds: error: unsupported configuration: only 1
graphics
Line 4851:
Line 4852: Obtain graphics framebuffer devices info from libvirt.
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping
between
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in
libvirt
s/as in/as of/
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics
devices,
Line 4857: virsh responds: error: unsupported configuration: only 1
graphics
Line 4858: device of each type (sdl, vnc, spice) is supported
Line 4859:
Line 4867: if dev.get('device') == graphicsType:
Line 4868: port = gxml.getAttribute('port')
Line 4869: if port:
Line 4870: dev['port'] = port
Line 4871: port = gxml.getAttribute('tlsPort')
better not to reuse the local variable port, I'd rather have tlsport as a
variable name.
Line 4872: if port:
Line 4873: dev['tlsPort'] = port
Line 4874: self._updateLegacyConf(dev)
Line 4875:
Line 5070: try:
Line 5071: nets = netinfo.networks()
Line 5072: device = nets[network].get('iface', network)
Line 5073: ip = netinfo.getaddr(device)
Line 5074: except:
no naked excepts allowed ;-)
Line 5075: ip = config.get('addresses', 'guests_gateway_ip')
Line 5076: if ip == '':
Line 5077: ip = '0'
Line 5078: logging.info('network %s: using %s', network, ip)
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 24:
(7 comments)
http://gerrit.ovirt.org/#/c/26895/24/vdsm/API.py
File vdsm/API.py:
Line 199: except:
Line 200: self.log.error(Error restoring VM parameters,
Line 201:exc_info=True)
Line 202:
Line 203: requiredParams = ['vmId', 'memSize']
To maintain the same behavior (error reporting), shouldn't you have a check
Yes, but this requires a fair amount of duplication of what we'll do in
buildConfDevices. I'll save this until we decide what to do with the support
for VM with no graphics devices/display.
Line 204: for param in requiredParams:
Line 205: if param not in vmParams:
Line 206: self.log.error('Missing required parameter %s' %
(param))
Line 207: return {'status': {'code': errCode['MissParam']
http://gerrit.ovirt.org/#/c/26895/24/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1395: if not utils.tobool(self.specParams.get('disableTicketing', '')):
I think that False would be better than '' considering utils.tobool would r
Agreed, will fix.
Line 4848: self.conf['devices'].append(diskDev)
Line 4849:
Line 4850: def _getUnderlyingGraphicsDeviceInfo(self):
Line 4851:
Line 4852: Obtain graphics framebuffer devices info from libvirt.
s/graphics/graphic/
Done
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping
between
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in
libvirt
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics
devices,
Line 4849:
Line 4850: def _getUnderlyingGraphicsDeviceInfo(self):
Line 4851:
Line 4852: Obtain graphics framebuffer devices info from libvirt.
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping
between
s/only allows 0 or 1/allows at most one/
Done
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in
libvirt
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics
devices,
Line 4857: virsh responds: error: unsupported configuration: only 1
graphics
Line 4851:
Line 4852: Obtain graphics framebuffer devices info from libvirt.
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping
between
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in
libvirt
s/as in/as of/
Done
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics
devices,
Line 4857: virsh responds: error: unsupported configuration: only 1
graphics
Line 4858: device of each type (sdl, vnc, spice) is supported
Line 4859:
Line 4867: if dev.get('device') == graphicsType:
Line 4868: port = gxml.getAttribute('port')
Line 4869: if port:
Line 4870: dev['port'] = port
Line 4871: port = gxml.getAttribute('tlsPort')
better not to reuse the local variable port, I'd rather have tlsport as a v
Done
Line 4872: if port:
Line 4873: dev['tlsPort'] = port
Line 4874: self._updateLegacyConf(dev)
Line 4875:
Line 5070: try:
Line 5071: nets = netinfo.networks()
Line 5072: device = nets[network].get('iface', network)
Line 5073: ip = netinfo.getaddr(device)
Line 5074: except:
no naked excepts allowed ;-)
Indeed. Will fix.
Line 5075: ip = config.get('addresses', 'guests_gateway_ip')
Line 5076: if ip == '':
Line 5077: ip = '0'
Line 5078: logging.info('network %s: using %s', network, ip)
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments:
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Dan Kenigsberg has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 23: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/26895/23/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4922: if port:
Line 4923: dev['port'] = port
Line 4924: port = gxml.getAttribute('tlsPort')
Line 4925: if port:
Line 4926: dev['tlsPort'] = port
_getUnderlyingDisplayPort used to update self.conf['displayPort']. This should
be maintained for backward compatibility.
Line 4927:
Line 4928: def _getUnderlyingNetworkInterfaceInfo(self):
Line 4929:
Line 4930: Obtain network interface info from libvirt.
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 23: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8764/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8900/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7974/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 22:
(2 comments)
http://gerrit.ovirt.org/#/c/26895/22/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1358: vmc.appendChildWithArgs('target', type='virtio',
Line 1359: name='com.redhat.spice.0')
Line 1360: return vmc
Line 1361:
Line 1362: def _getSpiceChannels(self):
actually, I would have preferred this to occur in a separate patch. This p
You are right. If you want me to split it up (with no code changes) just tell
me and I'll do with zero problems.
Line 1363: for name in
self.specParams['spiceSecureChannels'].split(','):
Line 1364: if name in GraphicsDevice.SPICE_CHANNEL_NAMES:
Line 1365: yield name
Line 1366: elif (name[0] == 's' and name[1:] in
Line 2650: # we need to handle the case getStats is called before
'devices' is
Line 2651: # updated in Vm._run (buildConfDevices and the following
code).
Line 2652: # easily reproduced with functional tests, but also possible
Line 2653: # in the wild
Line 2654: for dev in self.conf.get('devices', []) +
self.getConfGraphics():
Seeing that we've missed such a serious flaw previously, does not make me f
I don't want to hide anything under the carpet, but this is IMHO less serious
than it looks.
In my tests, engine (3.5.0-master) do pass a 'devices' entry,
and the buildConfDevices had always run before a getStats() call, so the issue
hasn't shown up.
The functional tests does not pass a 'devices' entry for the Vm configuration,
so the old code
self.conf.get('devices', self.getConfGraphics())
Had the intended effect; however, that was incorrect and fixed only the very
specific functional test scenario.
The new testRunningVm passed an empty devices collection (but a dict instead a
list, this is a bug which deservers a different patch), and that triggered the
bug this change fixed.
I'll work to have stricter check for future verification.
Line 2655: if dev['type'] == GRAPHICS_DEVICES:
Line 2656: stats.update(getInfo(dev))
Line 2657: break
Line 2658:
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 22:
(1 comment)
http://gerrit.ovirt.org/#/c/26895/22/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4376: @property
Line 4377: def hasSpice(self):
Line 4378: return any(
Line 4379: dev['device'] == 'spice'
Line 4380: for dev in self.conf.get('devices',
self.getConfGraphics())
Same problem as line 2654. Will fix in a separate followup patch
Line 4381: if dev['type'] == GRAPHICS_DEVICES)
Line 4382:
Line 4383: def _getPid(self):
Line 4384: pid = '0'
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 23: Verified+1
Vetrified with http://gerrit.ovirt.org/#/c/27595/ ,
http://gerrit.ovirt.org/#/c/27596/ and http://gerrit.ovirt.org/#/c/27215/
Verification (all the following using unpatched engine 3.5.0 snapshot:
* creation flow: booted a VM, connected with spice
* recovery: booted a VM, restarted VDSM, connected with spice
* migration: migrated back and forth another host running VDSM 4.14.8.1
* hibernate: booted a VM, restarted and connected with spice.
* verified the output of vdsClient
* cursory look at vdsm logs and to engine logs seems OK.
Found
Received a spice Device without an address when processing VM
025d59e3-f12c-41b8-be3f-de8dc22d23ce devices, skipping device: {port=5900,
specParams={keyMap=en-us, copyPasteEnable=false, displayIp=0,
fileTransferEnable=false}, device=spice, type=graphics}
but that is expected since the graphics device do not have PCI addresses.
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 22:
(1 comment)
http://gerrit.ovirt.org/#/c/26895/22/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4376: @property
Line 4377: def hasSpice(self):
Line 4378: return any(
Line 4379: dev['device'] == 'spice'
Line 4380: for dev in self.conf.get('devices',
self.getConfGraphics())
Same problem as line 2654. Will fix in a separate followup patch
fixed in http://gerrit.ovirt.org/#/c/27596/1
Line 4381: if dev['type'] == GRAPHICS_DEVICES)
Line 4382:
Line 4383: def _getPid(self):
Line 4384: pid = '0'
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 21: Verified+1 verified migrations moving back and forth a VM between patched VDSM and vanilla v4.14.8.1 -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Dan Kenigsberg has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 21: Code-Review-1
(5 comments)
http://gerrit.ovirt.org/#/c/26895/21/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1387: graphicsAttrs['keymap'] = self.specParams['keyMap']
Line 1388:
Line 1389: graphics = XMLElement('graphics', **graphicsAttrs)
Line 1390:
Line 1391: # handle deprecated channel name in a smart way,
add TODO string (I do not see added smartness).
Line 1392: # not just chop 1st char!
Line 1393: if graphicsAttrs['type'] == 'spice':
Line 1394: if self.specParams.get('spiceSecureChannels'):
Line 1395: for chan in
self.specParams['spiceSecureChannels'].split(','):
Line 1862: Normalize graphics device provided by conf.
Line 1863:
Line 1864: return [{
Line 1865: 'type': GRAPHICS_DEVICES,
Line 1866: 'device': 'spice' if self.conf.get('display', 'qxl') ==
'qxl'
if you want to drop support for qxlnc (and I won't mind that, if you verify
that engine = 3.0 does not send it), do it properly, in a separate patch.
Line 1867: else 'vnc',
Line 1868: 'specParams': dict(
Line 1869: (v, self.conf[k])
Line 1870: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems()
Line 2482: params['devices'] = newDevices
Line 2483: params.update(dispParams)
Line 2484: return params
Line 2485:
Line 2486: def getDisplayParams(self, dev):
used once, and does not use self. better be declared as private staticmethod,
or a local function in getRemoteMachineParams()
Line 2487:
Line 2488: rebuild display* parameters as old engine would send them.
Line 2489: symmetric to getConfGraphics.
Line 2490: This is needed because all the configuration VDSM should
accept
Line 2493: way to achieve this goal.
Line 2494:
Line 2495: params = {
Line 2496: 'display': 'qxl' if dev['device'] == 'spice' else
dev['device']}
Line 2497: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems():
renaming (k, v) to (legacyName, newName) would make this function more readable.
Line 2498: try:
Line 2499: params[k] = dev['specParams'][v]
Line 2500: except KeyError:
Line 2501: # just go ahead: either no specParams or no param
specified
Line 2497: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems():
Line 2498: try:
Line 2499: params[k] = dev['specParams'][v]
Line 2500: except KeyError:
Line 2501: # just go ahead: either no specParams or no param
specified
My credo does not allow me to accept this. A missing 'specParams' in a
graphics_device is a bug that we should never encounter, and as such - never
swallow. The following code makes the comment redundant:
if v in dev['specParams']:
params[k] = dev['specParams'][v]
Line 2502: pass
Line 2503: return params
Line 2504:
Line 2505: def getStats(self):
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 21:
(5 comments)
http://gerrit.ovirt.org/#/c/26895/21/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1387: graphicsAttrs['keymap'] = self.specParams['keyMap']
Line 1388:
Line 1389: graphics = XMLElement('graphics', **graphicsAttrs)
Line 1390:
Line 1391: # handle deprecated channel name in a smart way,
add TODO string (I do not see added smartness).
Right, I just forgot, and I'll fix ASAP.
Line 1392: # not just chop 1st char!
Line 1393: if graphicsAttrs['type'] == 'spice':
Line 1394: if self.specParams.get('spiceSecureChannels'):
Line 1395: for chan in
self.specParams['spiceSecureChannels'].split(','):
Line 1862: Normalize graphics device provided by conf.
Line 1863:
Line 1864: return [{
Line 1865: 'type': GRAPHICS_DEVICES,
Line 1866: 'device': 'spice' if self.conf.get('display', 'qxl') ==
'qxl'
if you want to drop support for qxlnc (and I won't mind that, if you verify
Actually here I forgot about qxlnc, which I'll to remove in a future separate
patch.
Line 1867: else 'vnc',
Line 1868: 'specParams': dict(
Line 1869: (v, self.conf[k])
Line 1870: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems()
Line 2482: params['devices'] = newDevices
Line 2483: params.update(dispParams)
Line 2484: return params
Line 2485:
Line 2486: def getDisplayParams(self, dev):
used once, and does not use self. better be declared as private staticmet
Will move inside getRemoteMachneParams.
Line 2487:
Line 2488: rebuild display* parameters as old engine would send them.
Line 2489: symmetric to getConfGraphics.
Line 2490: This is needed because all the configuration VDSM should
accept
Line 2493: way to achieve this goal.
Line 2494:
Line 2495: params = {
Line 2496: 'display': 'qxl' if dev['device'] == 'spice' else
dev['device']}
Line 2497: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems():
renaming (k, v) to (legacyName, newName) would make this function more read
Agreed and done
Line 2498: try:
Line 2499: params[k] = dev['specParams'][v]
Line 2500: except KeyError:
Line 2501: # just go ahead: either no specParams or no param
specified
Line 2497: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems():
Line 2498: try:
Line 2499: params[k] = dev['specParams'][v]
Line 2500: except KeyError:
Line 2501: # just go ahead: either no specParams or no param
specified
My credo does not allow me to accept this. A missing 'specParams' in a grap
I adhere to this credo. Changed as you suggested.
Line 2502: pass
Line 2503: return params
Line 2504:
Line 2505: def getStats(self):
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 22: rebased; addressed reviewer comments handled spice channels in a proper way -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 22: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8704/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7914/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8835/ : FAILURE -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Dan Kenigsberg has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 22: Code-Review+1
(2 comments)
http://gerrit.ovirt.org/#/c/26895/22/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1358: vmc.appendChildWithArgs('target', type='virtio',
Line 1359: name='com.redhat.spice.0')
Line 1360: return vmc
Line 1361:
Line 1362: def _getSpiceChannels(self):
actually, I would have preferred this to occur in a separate patch. This patch
is too complex anyway. Adding more refactoring does not help. Never mind now.
Line 1363: for name in
self.specParams['spiceSecureChannels'].split(','):
Line 1364: if name in GraphicsDevice.SPICE_CHANNEL_NAMES:
Line 1365: yield name
Line 1366: elif (name[0] == 's' and name[1:] in
Line 2650: # we need to handle the case getStats is called before
'devices' is
Line 2651: # updated in Vm._run (buildConfDevices and the following
code).
Line 2652: # easily reproduced with functional tests, but also possible
Line 2653: # in the wild
Line 2654: for dev in self.conf.get('devices', []) +
self.getConfGraphics():
Seeing that we've missed such a serious flaw previously, does not make me feel
safe. How was it missed by prior verifications?
Line 2655: if dev['type'] == GRAPHICS_DEVICES:
Line 2656: stats.update(getInfo(dev))
Line 2657: break
Line 2658:
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Michal Skrivanek has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 19: (1 comment) http://gerrit.ovirt.org/#/c/26895/19/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 191: self._vm.setDownStatus(NORMAL, vmexitreason.SAVE_STATE_SUCCEEDED) Line 192: self.status['status']['message'] = 'SaveState done' Line 193: Line 194: def _patchConfigForGraphics(self): Line 195: if 'display' in self._machineParams: 'display' is dropped for all clusterLevels - as far as I understand this br I'd think so… Only the spice+vnc configs do not need to maintain BC as older vdsms wouldn't understand it in the old way. But if the VM has only one display here we should send it the old way The migration of spice+vnc VMs to older cluster levels/vdsms should be prevented on the engine side, in case of vdsClient or bug in engine the migration would fail (which is correct) Line 196: del self._machineParams['display'] Line 197: if 'displayIp' in self._machineParams: Line 198: del self._machineParams['displayIp'] Line 199: -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Michal Skrivanek has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 19:
(1 comment)
http://gerrit.ovirt.org/#/c/26895/19/vdsm/API.py
File vdsm/API.py:
Line 206: return {'status': {'code': errCode['MissParam']
Line 207:
['status']['code'],
Line 208:'message': 'Missing required '
Line 209:'parameter %s' % (param)}}
Line 210: if 'display' not in vmParams and 'migrationDest' not in
vmParams:
would you remind me why this relaxation of requiredParams is needed, and wh
I see the motivation for migrationDest to handle incoming old configs, but
what about an old engine running on old cluster level using this new
host/vdsm?
Line 211: return {'status': {'code': errCode['MissParam']
Line 212: ['status']['code'],
Line 213:'message': 'Missing required '
Line 214:'parameter display'}}
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 19:
(4 comments)
http://gerrit.ovirt.org/#/c/26895/19//COMMIT_MSG
Commit Message:
Line 14: The GraphicDevice is built internally automatically
Line 15: if no graphic device is passed into the device list,
Line 16: but if the display{,Type,Network,...} parameters are passed.
Line 17:
Line 18: Backward compatibility with old unaware engines is preserved
We must maintain backward compat with old unaware Vdsms, too.
Understood. Then I'll change te code to do migrations using the 'old' way
(display* params, only one graphic device).
Line 19: both in the creation and reporting; however, in all VDSM=VDSM
Line 20: communications, e.g. migrations and state saving for recovery,
Line 21: the new graphic device representation is used.
Line 22:
http://gerrit.ovirt.org/#/c/26895/19/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:
Line 937: if linkDict.get('device') == iface:
Line 938: yield linkDict['name']
Line 939:
Line 940:
Line 941: def getNetworkIp(network):
I wish the movement of this function had a dedicated commit message. This f
Sorry, I didn't know this function was so evil :)
I'll move into vm.py as private.
Line 942: try:
Line 943: nets = networks()
Line 944: device = nets[network].get('iface', network)
Line 945: ip = getaddr(device)
http://gerrit.ovirt.org/#/c/26895/19/vdsm/API.py
File vdsm/API.py:
Line 206: return {'status': {'code': errCode['MissParam']
Line 207:
['status']['code'],
Line 208:'message': 'Missing required '
Line 209:'parameter %s' % (param)}}
Line 210: if 'display' not in vmParams and 'migrationDest' not in
vmParams:
I see the motivation for migrationDest to handle incoming old configs, bu
With this patch VDSM will report two graphic devices in the stats, and thus as
remoteMachineParams for migrations:
- one expressed through display* params, for backward compatibility with old
client (engine being the first)
- one expressed as proper graphic device, which is the official one and from
which the above display* params are reconstructed
however, this break migrations because the destination VM will see two graphic
device with the same type (one being reconstructed for bc, one real).
So here the solution implemented was to NOT require the legacy way of
expressing a graphic device only for internal VDSM=VDSM communications, so
relaxing the requiredParams for the migrationDest create flow.
But as Dan and Michal pointed out commenting the changes to migration.py, this
will break the clusterLevel
so I need to turn this concept the way around and to pass only the display*
params.
Will also check the scenario Michal described here to avoid more subtle
breakage.
Line 211: return {'status': {'code': errCode['MissParam']
Line 212: ['status']['code'],
Line 213:'message': 'Missing required '
Line 214:'parameter display'}}
http://gerrit.ovirt.org/#/c/26895/19/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 191: self._vm.setDownStatus(NORMAL,
vmexitreason.SAVE_STATE_SUCCEEDED)
Line 192: self.status['status']['message'] = 'SaveState done'
Line 193:
Line 194: def _patchConfigForGraphics(self):
Line 195: if 'display' in self._machineParams:
I'd think so… Only the spice+vnc configs do not need to maintain BC as olde
Same rationale as explained for the change in API.py (please see it).
I need to re-check but I think if we sent a Graphic Device (aka the new way) to
an unaware VDSM it will just silently discard it.
Other than that, I'll amend the code to pass the display* configuration the
'old way' during migrations.
Line 196: del self._machineParams['display']
Line 197: if 'displayIp' in self._machineParams:
Line 198: del self._machineParams['displayIp']
Line 199:
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 20: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8605/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7814/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8732/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 20: (1 comment) http://gerrit.ovirt.org/#/c/26895/20/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 196: the first graphic device, the one which overlaps with the display* Line 197: params, must NOT be declared as real device to not break clusterLevel. Line 198: Line 199: self._machineParams['devices'] = self._vm.filterDevicesForMigration( Line 200: self._machineParams['devices'][:], self._machineParams['display']) This is wrong, but I already have the correct version under testing. Line 201: Line 202: def _patchConfigForLegacy(self): Line 203: Line 204: Remove from the VM config drives list cdrom and floppy -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Dan Kenigsberg has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 20: (1 comment) http://gerrit.ovirt.org/#/c/26895/20/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 193: Line 194: def _patchConfigForGraphics(self): Line 195: Line 196: the first graphic device, the one which overlaps with the display* Line 197: params, must NOT be declared as real device to not break clusterLevel. It would be much simpler to limit this logic to the construct_from_legacy code: if any graphics device exists in conf, display* should be ignored. Line 198: Line 199: self._machineParams['devices'] = self._vm.filterDevicesForMigration( Line 200: self._machineParams['devices'][:], self._machineParams['display']) Line 201: -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 20: (1 comment) http://gerrit.ovirt.org/#/c/26895/20/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 193: Line 194: def _patchConfigForGraphics(self): Line 195: Line 196: the first graphic device, the one which overlaps with the display* Line 197: params, must NOT be declared as real device to not break clusterLevel. It would be much simpler to limit this logic to the construct_from_legacy c I like it, and I'll amend my current patchset to implement it. Line 198: Line 199: self._machineParams['devices'] = self._vm.filterDevicesForMigration( Line 200: self._machineParams['devices'][:], self._machineParams['display']) Line 201: -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 20: (1 comment) http://gerrit.ovirt.org/#/c/26895/20/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 193: Line 194: def _patchConfigForGraphics(self): Line 195: Line 196: the first graphic device, the one which overlaps with the display* Line 197: params, must NOT be declared as real device to not break clusterLevel. I like it, and I'll amend my current patchset to implement it. But in the case of a single graphics device we must do the translation anyway to not break clusterLevel. Line 198: Line 199: self._machineParams['devices'] = self._vm.filterDevicesForMigration( Line 200: self._machineParams['devices'][:], self._machineParams['display']) Line 201: -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 21: fixed the machine parameter passed on migrations. Addresser reviewer's remark either with code changes (most of time) or with inline code comments/docstrings. Preliminary verifications seems OK (migrations/recovery), more tests still in progress, thus not ticking verified. -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 21: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8613/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7823/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8741/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 21: re-verified using an unaware old engine (sending legacy graphic display) * creation * migration (between two patched VDSM) * recovert * suspend/resume manually inspected -but with cursory glance- the VDSM logs: domain XML looks OK, no unexpected things found. Still to be verified: migration between patched VDSM and unpatched VDSM e.g. 4.14.6 -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Dan Kenigsberg has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
..
Patch Set 19: Code-Review-1
(4 comments)
http://gerrit.ovirt.org/#/c/26895/19//COMMIT_MSG
Commit Message:
Line 14: The GraphicDevice is built internally automatically
Line 15: if no graphic device is passed into the device list,
Line 16: but if the display{,Type,Network,...} parameters are passed.
Line 17:
Line 18: Backward compatibility with old unaware engines is preserved
We must maintain backward compat with old unaware Vdsms, too.
Line 19: both in the creation and reporting; however, in all VDSM=VDSM
Line 20: communications, e.g. migrations and state saving for recovery,
Line 21: the new graphic device representation is used.
Line 22:
http://gerrit.ovirt.org/#/c/26895/19/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:
Line 937: if linkDict.get('device') == iface:
Line 938: yield linkDict['name']
Line 939:
Line 940:
Line 941: def getNetworkIp(network):
I wish the movement of this function had a dedicated commit message. This
function is loaded with ancient backward compatibility, display network
specifics (the fallback to guests_gateway_ip and to '0'), and an evil logless
bare except clause. As it is, it must not be moved to netinfo.py, where it
begs for more users.
Line 942: try:
Line 943: nets = networks()
Line 944: device = nets[network].get('iface', network)
Line 945: ip = getaddr(device)
http://gerrit.ovirt.org/#/c/26895/19/vdsm/API.py
File vdsm/API.py:
Line 206: return {'status': {'code': errCode['MissParam']
Line 207:
['status']['code'],
Line 208:'message': 'Missing required '
Line 209:'parameter %s' % (param)}}
Line 210: if 'display' not in vmParams and 'migrationDest' not in
vmParams:
would you remind me why this relaxation of requiredParams is needed, and why in
this patch? (best reminder would be a commit message of a separate patch, if
that is possible)
Line 211: return {'status': {'code': errCode['MissParam']
Line 212: ['status']['code'],
Line 213:'message': 'Missing required '
Line 214:'parameter display'}}
http://gerrit.ovirt.org/#/c/26895/19/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 191: self._vm.setDownStatus(NORMAL,
vmexitreason.SAVE_STATE_SUCCEEDED)
Line 192: self.status['status']['message'] = 'SaveState done'
Line 193:
Line 194: def _patchConfigForGraphics(self):
Line 195: if 'display' in self._machineParams:
'display' is dropped for all clusterLevels - as far as I understand this breaks
migration from ovirt-3.5's vdsm to earlier ones?
Line 196: del self._machineParams['display']
Line 197: if 'displayIp' in self._machineParams:
Line 198: del self._machineParams['displayIp']
Line 199:
--
To view, visit http://gerrit.ovirt.org/26895
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com
Gerrit-Reviewer: Martin Polednik mpole...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 17: Build Failed http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/1311/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8520/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7730/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8647/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 18: Build Failed http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/1314/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8543/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7753/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8670/ : FAILURE -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 19: Verified+1 fixed recovery and migration at the cost to ad more mess and special case handling in virt/migration.py and API.py. As stated in the commit message, all the VDSM=VDSM communication uses the new graphic device representation; IMO it makes less sense the opposite way (e.g. to save/pass display* params). Also verified with 27215 and 26987 using an unaware engine 3.5 and vdsClient; all the following were done an handful of times (in the order of 10 times) * start/stop VM, verified runs and connects to console using engine; VDSM logs seems OK as well * migrated VM while console connected; verified console connects after VM migrated * restarted VDSM to try recovery without migration, before migration and after migration on destination host; verified console connects after all those recoveries * tried suspend/resume, verified console connects afterwards. verified engine logs, nothing strange found at cursory glance. -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 19: Build Failed http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/1315/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8548/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7758/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8675/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 13: Patch set 13: - removed redundant fields in getConfGraphics, stripped to the minimum; the removed fields are the one overwritten anyway in GraphicsDevice.__init__ - fixed some timing-related bugs if getStats is called 'too early', e.g. before buildConfDevices is called and Vm.conf['devices'] is updated accordingly. Found using functional tests, which now pass cleanly. -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 13: Build Failed http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/1307/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8493/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7703/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8618/ : FAILURE -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 13: patch set 14: handle the case a device does not have the 'device' field. -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 14: Build Failed http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/1308/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8498/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7708/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8623/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 15: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/1309/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8503/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7713/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8629/ : FAILURE -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 15: * rationalize hasSpice, remove legacy handling and push it into getConfGraphics (mostly) * found and fixed another timing issue in getStats * extracted _updateDevices to help testing -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 12: Patch set 12: - removed half of the statichmetod (the silly one) at the price of moving getNetworkIp into netinfo - moved initialization in the GraphicsDevice __init__ where really belongs - refactored/improved the tests -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class .. Patch Set 12: Build Failed http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/1304/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8452/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7662/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8573/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani from...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Frank Kobzik fkob...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
