[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 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 <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?

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

    <graphics type='spice' port='5900' tlsPort='-1' autoport='no'/>
      <listen type='address' port='5900' tlsPort='-1' autoport='no'/>
    </graphics>

or:

    <graphics type='vnc' port='-1' websocket='-1' autoport='yes'/>
      <listen type='address' websocket='-1' autoport='yes'/>
    </graphics>

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 <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

This is mentioned in the listen part of the documentation that for backward
compatibility they are duplicated into graphics element.

Pavel


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