subject:"\[libvirt\] \[PATCH v2 05\/12\] graphics\: move port definition to listen element"

Re: [libvirt] [PATCH v2 05/12] graphics: move port definition to listen element

2016-05-12 Thread Pavel Hrdina

On Thu, May 12, 2016 at 01:23:18PM +0200, Christophe Fergeau wrote:
> On Wed, May 11, 2016 at 05:08:24PM +0200, Pavel Hrdina wrote:
> > So far we have only two listen types that supports only address:port
> > method, but in the future we may want to add a new different listen
> > type, for example socket.
> > 
> > This patch moves the ports values out of graphics unions into listen
> > element.  The domain XML will now duplicate the ports from first listen
> > element into the graphics element as we do also for address.
> > 
> > This allows us to make part of the graphics code as listen-driven and
> > prepare the code for new listen types.
> > 
> > To support migration back to older versions the new attributes from
> > listen elements are not written into migratable XML.
> 
> This patch keeps the port=-1 legacy syntax in the  element,
> shouln't we use this as an opportunity to drop it? Or is it making the
> code much more complicated to handle 'port'/'tlsPort' differently in
>  nodes?

It's possible for SPICE to have something like this:


  


or:


  


So we cannot remove it from listen elements.  The first example is the case
where you specify port, but tlsPort is automatically generated.  The second
example is the only way, how to get websocket automatically generated because
for security reasons autoport='yes' will generate only port.

> 
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  docs/formatdomain.html.in  |  87 +++--
> >  docs/schemas/domaincommon.rng  |  40 +++
> >  src/conf/domain_conf.c | 349 
> > -
> >  src/conf/domain_conf.h |  23 +-
> >  src/libxl/libxl_conf.c |  53 ++--
> >  src/libxl/libxl_domain.c   |  17 +-
> >  src/qemu/qemu_command.c| 167 +-
> >  src/qemu/qemu_hotplug.c|  31 +-
> >  src/qemu/qemu_migration.c  |  14 +-
> >  src/qemu/qemu_parse_command.c  |  39 ++-
> >  src/qemu/qemu_process.c| 257 ---
> >  src/vbox/vbox_common.c |  26 +-
> >  src/vbox/vbox_tmpl.c   |  34 +-
> >  src/vbox/vbox_uniformed_api.h  |   4 +-
> >  src/vmx/vmx.c  |  52 +--
> >  src/vz/vz_sdk.c|  30 +-
> >  src/xenconfig/xen_common.c |  67 ++--
> >  src/xenconfig/xen_sxpr.c   |  69 ++--
> >  src/xenconfig/xen_xl.c |  44 +--
> >  .../generic-graphics-listen-back-compat-ports.xml  |  30 ++
> >  ...generic-graphics-vnc-listen-element-minimal.xml |   2 +-
> >  ...aphics-vnc-listen-element-with-address-port.xml |  30 ++
> >  .../generic-graphics-listen-back-compat-ports.xml  |  30 ++
> >  .../generic-graphics-listen-back-compat.xml|   2 +-
> >  .../generic-graphics-vnc-listen-attr-only.xml  |   2 +-
> >  ...generic-graphics-vnc-listen-element-minimal.xml |   4 +-
> >  ...aphics-vnc-listen-element-with-address-port.xml |  30 ++
> >  ...ic-graphics-vnc-listen-element-with-address.xml |   2 +-
> >  .../generic-graphics-vnc-manual-port.xml   |   2 +-
> >  .../generic-graphics-vnc-minimal.xml   |   2 +-
> >  tests/genericxml2xmltest.c |   3 +
> >  .../qemuargv2xml-graphics-vnc-policy.xml   |   2 +-
> >  .../qemuargv2xml-graphics-vnc-sasl.xml |   2 +-
> >  .../qemuargv2xml-graphics-vnc-tls.xml  |   2 +-
> >  .../qemuargv2xml-graphics-vnc-websocket.xml|   2 +-
> >  .../qemuargv2xmldata/qemuargv2xml-graphics-vnc.xml |   2 +-
> >  ...qemuhotplug-console-compat-2+console-virtio.xml |   2 +-
> >  .../qemuxml2argv-console-compat-2.xml  |   2 +-
> >  .../qemuxml2xmlout-graphics-listen-network.xml |   2 +-
> >  .../qemuxml2xmlout-graphics-listen-network2.xml|   4 +-
> >  .../qemuxml2xmlout-graphics-spice-compression.xml  |   2 +-
> >  .../qemuxml2xmlout-graphics-spice-qxl-vga.xml  |   2 +-
> >  .../qemuxml2xmlout-graphics-spice-timeout.xml  |   2 +-
> >  .../qemuxml2xmlout-graphics-spice.xml  |   2 +-
> >  .../qemuxml2xmlout-graphics-vnc-autosocket.xml |   2 +-
> >  .../qemuxml2xmlout-graphics-vnc-no-listen-attr.xml |   2 +-
> >  .../qemuxml2xmlout-graphics-vnc-sasl.xml   |   2 +-
> >  .../qemuxml2xmlout-graphics-vnc-tls.xml|   2 +-
> >  .../qemuxml2xmlout-graphics-vnc-websocket.xml  |   2 +-
> >  .../qemuxml2xmlout-graphics-vnc.xml|   2 +-
> >  .../qemuxml2xmlout-interface-server.xml|   2 +-
> >  .../qemuxml2xmlout-net-bandwidth.xml   |   2 +-
> >  .../qemuxml2xmlout-net-bandwidth2.xml  |   2 +-
> >  .../qemuxml2xmlout-pci-bridge.xml

Re: [libvirt] [PATCH v2 05/12] graphics: move port definition to listen element

2016-05-12 Thread Christophe Fergeau

On Wed, May 11, 2016 at 05:08:24PM +0200, Pavel Hrdina wrote:
> So far we have only two listen types that supports only address:port
> method, but in the future we may want to add a new different listen
> type, for example socket.
> 
> This patch moves the ports values out of graphics unions into listen
> element.  The domain XML will now duplicate the ports from first listen
> element into the graphics element as we do also for address.
> 
> This allows us to make part of the graphics code as listen-driven and
> prepare the code for new listen types.
> 
> To support migration back to older versions the new attributes from
> listen elements are not written into migratable XML.

This patch keeps the port=-1 legacy syntax in the  element,
shouln't we use this as an opportunity to drop it? Or is it making the
code much more complicated to handle 'port'/'tlsPort' differently in
 nodes?

> 
> Signed-off-by: Pavel Hrdina 
> ---
>  docs/formatdomain.html.in  |  87 +++--
>  docs/schemas/domaincommon.rng  |  40 +++
>  src/conf/domain_conf.c | 349 
> -
>  src/conf/domain_conf.h |  23 +-
>  src/libxl/libxl_conf.c |  53 ++--
>  src/libxl/libxl_domain.c   |  17 +-
>  src/qemu/qemu_command.c| 167 +-
>  src/qemu/qemu_hotplug.c|  31 +-
>  src/qemu/qemu_migration.c  |  14 +-
>  src/qemu/qemu_parse_command.c  |  39 ++-
>  src/qemu/qemu_process.c| 257 ---
>  src/vbox/vbox_common.c |  26 +-
>  src/vbox/vbox_tmpl.c   |  34 +-
>  src/vbox/vbox_uniformed_api.h  |   4 +-
>  src/vmx/vmx.c  |  52 +--
>  src/vz/vz_sdk.c|  30 +-
>  src/xenconfig/xen_common.c |  67 ++--
>  src/xenconfig/xen_sxpr.c   |  69 ++--
>  src/xenconfig/xen_xl.c |  44 +--
>  .../generic-graphics-listen-back-compat-ports.xml  |  30 ++
>  ...generic-graphics-vnc-listen-element-minimal.xml |   2 +-
>  ...aphics-vnc-listen-element-with-address-port.xml |  30 ++
>  .../generic-graphics-listen-back-compat-ports.xml  |  30 ++
>  .../generic-graphics-listen-back-compat.xml|   2 +-
>  .../generic-graphics-vnc-listen-attr-only.xml  |   2 +-
>  ...generic-graphics-vnc-listen-element-minimal.xml |   4 +-
>  ...aphics-vnc-listen-element-with-address-port.xml |  30 ++
>  ...ic-graphics-vnc-listen-element-with-address.xml |   2 +-
>  .../generic-graphics-vnc-manual-port.xml   |   2 +-
>  .../generic-graphics-vnc-minimal.xml   |   2 +-
>  tests/genericxml2xmltest.c |   3 +
>  .../qemuargv2xml-graphics-vnc-policy.xml   |   2 +-
>  .../qemuargv2xml-graphics-vnc-sasl.xml |   2 +-
>  .../qemuargv2xml-graphics-vnc-tls.xml  |   2 +-
>  .../qemuargv2xml-graphics-vnc-websocket.xml|   2 +-
>  .../qemuargv2xmldata/qemuargv2xml-graphics-vnc.xml |   2 +-
>  ...qemuhotplug-console-compat-2+console-virtio.xml |   2 +-
>  .../qemuxml2argv-console-compat-2.xml  |   2 +-
>  .../qemuxml2xmlout-graphics-listen-network.xml |   2 +-
>  .../qemuxml2xmlout-graphics-listen-network2.xml|   4 +-
>  .../qemuxml2xmlout-graphics-spice-compression.xml  |   2 +-
>  .../qemuxml2xmlout-graphics-spice-qxl-vga.xml  |   2 +-
>  .../qemuxml2xmlout-graphics-spice-timeout.xml  |   2 +-
>  .../qemuxml2xmlout-graphics-spice.xml  |   2 +-
>  .../qemuxml2xmlout-graphics-vnc-autosocket.xml |   2 +-
>  .../qemuxml2xmlout-graphics-vnc-no-listen-attr.xml |   2 +-
>  .../qemuxml2xmlout-graphics-vnc-sasl.xml   |   2 +-
>  .../qemuxml2xmlout-graphics-vnc-tls.xml|   2 +-
>  .../qemuxml2xmlout-graphics-vnc-websocket.xml  |   2 +-
>  .../qemuxml2xmlout-graphics-vnc.xml|   2 +-
>  .../qemuxml2xmlout-interface-server.xml|   2 +-
>  .../qemuxml2xmlout-net-bandwidth.xml   |   2 +-
>  .../qemuxml2xmlout-net-bandwidth2.xml  |   2 +-
>  .../qemuxml2xmlout-pci-bridge.xml  |   2 +-
>  ...emuxml2xmlout-seclabel-dynamic-none-relabel.xml |   2 +-
>  .../qemuxml2xmlout-serial-spiceport.xml|   2 +-
>  .../qemuxml2xmlout-video-virtio-gpu-spice-gl.xml   |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-curmem.xml   |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml  |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml  |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml|   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml  |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml |   2 +-
>

Re: [libvirt] [PATCH v2 05/12] graphics: move port definition to listen element

2016-05-11 Thread Marc-André Lureau

Hi

On Wed, May 11, 2016 at 5:08 PM, Pavel Hrdina  wrote:
> So far we have only two listen types that supports only address:port
> method, but in the future we may want to add a new different listen
> type, for example socket.
>
> This patch moves the ports values out of graphics unions into listen
> element.  The domain XML will now duplicate the ports from first listen
> element into the graphics element as we do also for address.
>
> This allows us to make part of the graphics code as listen-driven and
> prepare the code for new listen types.
>
> To support migration back to older versions the new attributes from
> listen elements are not written into migratable XML.
>
> Signed-off-by: Pavel Hrdina 

This patch is pretty large, so I have been only skimming through, most
changes are straightforward. Overall, it looks good to me. Someone
more familiar with libvirt & network code, and migration support,
should do a more thorough review.

Tested-by: Marc-André Lureau 


-- 
Marc-André Lureau

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 05/12] graphics: move port definition to listen element

Re: [libvirt] [PATCH v2 05/12] graphics: move port definition to listen element

Re: [libvirt] [PATCH v2 05/12] graphics: move port definition to listen element

3 matches

Site Navigation

Mail list logo

Footer information