[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 <listen> 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
<listen> nodes?

> 
> Signed-off-by: Pavel Hrdina <phrdina redhat com>
> ---
>  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 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml  |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml  |   2 +-
>  .../sexpr2xml-fv-serial-dev-2-ports.xml            |   2 +-
>  .../sexpr2xml-fv-serial-dev-2nd-port.xml           |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml   |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml   |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml   |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml    |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml  |   2 +-
>  .../sexpr2xml-fv-serial-tcp-telnet.xml             |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml    |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml    |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml   |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml     |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-sound.xml         |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml      |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml     |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-utc.xml           |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv-v2.xml            |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-fv.xml               |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml  |   2 +-
>  .../sexpr2xml-pv-vfb-new-vncdisplay.xml            |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml       |   2 +-
>  .../sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml  |   2 +-
>  tests/sexpr2xmldata/sexpr2xml-vif-rate.xml         |   2 +-
>  tests/vmx2xmldata/vmx2xml-graphics-vnc.xml         |   2 +-
>  .../test-disk-positional-parms-full.xml            |   2 +-
>  .../test-disk-positional-parms-partial.xml         |   2 +-
>  ...est-fullvirt-direct-kernel-boot-bogus-extra.xml |   2 +-
>  .../test-fullvirt-direct-kernel-boot-extra.xml     |   2 +-
>  .../test-fullvirt-direct-kernel-boot.xml           |   2 +-
>  tests/xlconfigdata/test-fullvirt-multiusb.xml      |   2 +-
>  tests/xlconfigdata/test-fullvirt-nohap.xml         |   2 +-
>  tests/xlconfigdata/test-new-disk.xml               |   2 +-
>  tests/xlconfigdata/test-rbd-multihost-noauth.xml   |   2 +-
>  tests/xlconfigdata/test-spice-features.xml         |   2 +-
>  tests/xlconfigdata/test-spice.xml                  |   2 +-
>  tests/xlconfigdata/test-vif-rate.xml               |   2 +-
>  tests/xmconfigdata/test-escape-paths.xml           |   2 +-
>  .../xmconfigdata/test-fullvirt-default-feature.xml |   2 +-
>  tests/xmconfigdata/test-fullvirt-force-hpet.xml    |   2 +-
>  tests/xmconfigdata/test-fullvirt-force-nohpet.xml  |   2 +-
>  tests/xmconfigdata/test-fullvirt-localtime.xml     |   2 +-
>  tests/xmconfigdata/test-fullvirt-net-netfront.xml  |   2 +-
>  tests/xmconfigdata/test-fullvirt-new-cdrom.xml     |   2 +-
>  tests/xmconfigdata/test-fullvirt-nohap.xml         |   2 +-
>  tests/xmconfigdata/test-fullvirt-parallel-tcp.xml  |   2 +-
>  .../test-fullvirt-serial-dev-2-ports.xml           |   2 +-
>  .../test-fullvirt-serial-dev-2nd-port.xml          |   2 +-
>  tests/xmconfigdata/test-fullvirt-serial-file.xml   |   2 +-
>  tests/xmconfigdata/test-fullvirt-serial-null.xml   |   2 +-
>  tests/xmconfigdata/test-fullvirt-serial-pipe.xml   |   2 +-
>  tests/xmconfigdata/test-fullvirt-serial-pty.xml    |   2 +-
>  tests/xmconfigdata/test-fullvirt-serial-stdio.xml  |   2 +-
>  .../test-fullvirt-serial-tcp-telnet.xml            |   2 +-
>  tests/xmconfigdata/test-fullvirt-serial-tcp.xml    |   2 +-
>  tests/xmconfigdata/test-fullvirt-serial-udp.xml    |   2 +-
>  tests/xmconfigdata/test-fullvirt-serial-unix.xml   |   2 +-
>  tests/xmconfigdata/test-fullvirt-sound.xml         |   2 +-
>  tests/xmconfigdata/test-fullvirt-usbmouse.xml      |   2 +-
>  tests/xmconfigdata/test-fullvirt-usbtablet.xml     |   2 +-
>  tests/xmconfigdata/test-fullvirt-utc.xml           |   2 +-
>  tests/xmconfigdata/test-no-source-cdrom.xml        |   2 +-
>  tests/xmconfigdata/test-paravirt-net-e1000.xml     |   2 +-
>  tests/xmconfigdata/test-paravirt-net-vifname.xml   |   2 +-
>  .../test-paravirt-new-pvfb-vncdisplay.xml          |   2 +-
>  tests/xmconfigdata/test-paravirt-new-pvfb.xml      |   2 +-
>  tests/xmconfigdata/test-pci-devs.xml               |   2 +-
>  131 files changed, 1029 insertions(+), 715 deletions(-)
>  create mode 100644 tests/genericxml2xmlindata/generic-graphics-listen-back-compat-ports.xml
>  create mode 100644 tests/genericxml2xmlindata/generic-graphics-vnc-listen-element-with-address-port.xml
>  create mode 100644 tests/genericxml2xmloutdata/generic-graphics-listen-back-compat-ports.xml
>  create mode 100644 tests/genericxml2xmloutdata/generic-graphics-vnc-listen-element-with-address-port.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index fd2dd33..b0847b7 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5090,7 +5090,7 @@ qemu-kvm -net nic,model=? /dev/null
>    &lt;devices&gt;
>      &lt;graphics type='sdl' display=':0.0'/&gt;
>      &lt;graphics type='vnc' port='5904' sharePolicy='allow-exclusive'&gt;
> -      &lt;listen type='address' address='1.2.3.4'/&gt;
> +      &lt;listen type='address' address='1.2.3.4' port='5904'/&gt;
>      &lt;/graphics&gt;
>      &lt;graphics type='rdp' autoport='yes' multiUser='yes' /&gt;
>      &lt;graphics type='desktop' fullscreen='yes'/&gt;
> @@ -5122,16 +5122,13 @@ qemu-kvm -net nic,model=? /dev/null
>            <dt><code>vnc</code></dt>
>            <dd>
>              <p>
> -              Starts a VNC server. The <code>port</code> attribute specifies
> -              the TCP port number (with -1 as legacy syntax indicating that it
> -              should be auto-allocated). The <code>autoport</code> attribute is
> -              the new preferred syntax for indicating auto-allocation of the TCP
> -              port to use. The <code>passwd</code> attribute provides a VNC
> -              password in clear text. The <code>keymap</code> attribute specifies
> -              the keymap to use. It is possible to set a limit on the validity of
> +              Starts a VNC server. To set port or address use <code>listen</code>
> +              element. The <code>passwd</code> attribute provides a VNC password
> +              in clear text. The <code>keymap</code> attribute specifies the
> +              keymap to use. It is possible to set a limit on the validity of
>                the password by giving an timestamp
> -              <code>passwdValidTo='2010-04-09T15:51:00'</code> assumed to be
> -              in UTC. The <code>connected</code> attribute allows control of
> +              <code>passwdValidTo='2010-04-09T15:51:00'</code> assumed to be in
> +              UTC. The <code>connected</code> attribute allows control of
>                connected client during password changes. VNC accepts
>                <code>keep</code> value only <span class="since">since 0.9.3</span>.
>                NB, this may not be supported by all hypervisors.
> @@ -5152,22 +5149,12 @@ qemu-kvm -net nic,model=? /dev/null
>                Rather than using listen/port, QEMU supports a socket
>                attribute for listening on a unix domain socket path

I'd amend the sentence above too if you remove references to
listen/port.
Shouldn't we keep a mention of the old listen/port/... attributes
somewhere (for people looking at old XML, for people who want to write
backward compatible XML,. ..)

Christophe

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]