Dan Kenigsberg has posted comments on this change.

Change subject: vdsm: only specify graphics ports as needed for spice
......................................................................


Patch Set 5: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/libvirtvm.py
Line 827: 
Line 828:         if 'qxl' in self.conf['display']:
Line 829:             if self.conf.get('spiceSecureChannels'):
Line 830:                 
graphics.setAttrs(tlsPort=self.conf['displaySecurePort'])
Line 831:                 for channel in 
self.conf['spiceSecureChannels'].split(','):
with this patch, the <channel> items added here have no meaning - we never 
encrypt only a part of the channels. They are all encrypted (on 
displaySecurePort) or all plaintext (on displayPort).

I'd rather find a way to solve the bug on the spiceclient side. But if this is 
impossible, we should be frank about the change of semantics, state in in the 
commit message, and drop the <channel>s altogether.
Line 832:                     graphics.appendChildWithArgs('channel', 
name=channel[1:],
Line 833:                                                  mode='secure')
Line 834:             else:
Line 835:                 graphics.setAttrs(port=self.conf['displayPort'])


--
To view, visit http://gerrit.ovirt.org/12291
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc866259d4cfa95a3b137dce101ce04f215f9f5a
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com>
Gerrit-Reviewer: Peter V. Saveliev <p...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to