[virt-tools-list] [PATCH v4 2/2] gfxdetails: add listen type option
Pavel Hrdina
phrdina at redhat.com
Tue Dec 13 14:29:07 UTC 2016
On Mon, Nov 28, 2016 at 02:47:46PM +0400, marcandre.lureau at redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
>
> Similarly to virt-install --listen=none, add a combobox to select
> the listen type: "address" or "none" for now, as suggested by Pavel
> Hrdina.
Hi, I'm sorry for the delay with review. This looks promising but there are
some implementation details that should be addressed.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> ---
> ui/gfxdetails.ui | 64 ++++++++++++++++++++++++++++++++++------------
> virtManager/addhardware.py | 14 ++++++----
> virtManager/details.py | 7 ++---
> virtManager/domain.py | 11 +++++---
> virtManager/gfxdetails.py | 38 ++++++++++++++++++++++-----
> virtinst/devicegraphics.py | 16 +++++++++++-
> 6 files changed, 115 insertions(+), 35 deletions(-)
[...]
> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
> index 7a3c8f2..3f868aa 100644
> --- a/virtManager/addhardware.py
> +++ b/virtManager/addhardware.py
> @@ -1541,16 +1541,20 @@ class vmmAddHardware(vmmGObjectUI):
>
> def _validate_page_graphics(self):
> try:
> - (gtype, port,
> - tlsport, addr, passwd, keymap, gl) = self._gfxdetails.get_values()
> + (gtype, port, tlsport, listen,
> + addr, passwd, keymap, gl) = self._gfxdetails.get_values()
>
> self._dev = virtinst.VirtualGraphics(self.conn.get_backend())
> self._dev.type = gtype
> - self._dev.port = port
> self._dev.passwd = passwd
> - self._dev.listen = addr
> - self._dev.tlsPort = tlsport
> self._dev.gl = gl
> +
> + if not listen:
> + self._dev.set_listen_none()
> + else:
> + self._dev.listen = addr
> + self._dev.port = port
> + self._dev.tlsPort = tlsport
This only validates the graphics device, but I would prefer to do it properly,
something like this:
if not listen or listen == "none":
self._dev.set_listen_none()
elif listen == "address":
self._dev.listen = addr
self._dev.port = port
self._dev.tlsPort = tlsport
else
raise ValueError(_("invalid listen type"))
> if keymap:
> self._dev.keymap = keymap
> except ValueError, e:
> diff --git a/virtManager/details.py b/virtManager/details.py
> index d803d52..d4e6629 100644
> --- a/virtManager/details.py
> +++ b/virtManager/details.py
> @@ -2135,15 +2135,16 @@ class vmmDetails(vmmGObjectUI):
> devobj=devobj)
>
> def config_graphics_apply(self, devobj):
> - (gtype, port,
> - tlsport, addr, passwd, keymap, gl) = self.gfxdetails.get_values()
> + (gtype, port, tlsport, listen,
> + addr, passwd, keymap, gl) = self.gfxdetails.get_values()
>
> kwargs = {}
>
> if self.edited(EDIT_GFX_PASSWD):
> kwargs["passwd"] = passwd
> if self.edited(EDIT_GFX_ADDRESS):
> - kwargs["listen"] = addr
> + kwargs["listen"] = listen
> + kwargs["addr"] = addr
Now that we support to select listen type it would be probably better to
separate the listen from address. That would mean create all the bits as we
already have for address.
> if self.edited(EDIT_GFX_KEYMAP):
> kwargs["keymap"] = keymap
> if self.edited(EDIT_GFX_PORT):
> diff --git a/virtManager/domain.py b/virtManager/domain.py
> index 6e742b9..f5159d6 100644
> --- a/virtManager/domain.py
> +++ b/virtManager/domain.py
> @@ -824,7 +824,7 @@ class vmmDomain(vmmLibvirtObject):
> self._redefine_xmlobj(xmlobj)
>
> def define_graphics(self, devobj, do_hotplug,
> - listen=_SENTINEL, port=_SENTINEL, tlsport=_SENTINEL,
> + listen=_SENTINEL, addr=_SENTINEL, port=_SENTINEL, tlsport=_SENTINEL,
> passwd=_SENTINEL, keymap=_SENTINEL, gtype=_SENTINEL,
> gl=_SENTINEL):
> xmlobj = self._make_xmlobj_to_define()
> @@ -832,8 +832,8 @@ class vmmDomain(vmmLibvirtObject):
> if not editdev:
> return
>
> - if listen != _SENTINEL:
> - editdev.listen = listen
> + if addr != _SENTINEL:
> + editdev.listen = addr
> if port != _SENTINEL:
> editdev.port = port
> if tlsport != _SENTINEL:
> @@ -846,6 +846,11 @@ class vmmDomain(vmmLibvirtObject):
> editdev.type = gtype
> if gl != _SENTINEL:
> editdev.gl = gl
> + if listen != _SENTINEL:
> + if listen == 'none':
> + editdev.set_listen_none()
> + else:
> + editdev.remove_listen_none()
>
> if do_hotplug:
> self.hotplug(device=editdev)
> diff --git a/virtManager/gfxdetails.py b/virtManager/gfxdetails.py
> index f3cd3a9..d900b0b 100644
> --- a/virtManager/gfxdetails.py
> +++ b/virtManager/gfxdetails.py
> @@ -50,8 +50,9 @@ class vmmGraphicsDetails(vmmGObjectUI):
> "on_graphics_tlsport_auto_toggled": self._change_tlsport_auto,
> "on_graphics_use_password": self._change_password_chk,
>
> + "on_graphics_listen_type_changed": self._change_graphics_address,
> "on_graphics_password_changed": lambda ignore: self.emit("changed-password"),
> - "on_graphics_address_changed": lambda ignore: self.emit("changed-address"),
> + "on_graphics_address_changed": self._change_graphics_address,
> "on_graphics_tlsport_changed": lambda ignore: self.emit("changed-tlsport"),
> "on_graphics_port_changed": lambda ignore: self.emit("changed-port"),
> "on_graphics_keymap_changed": lambda ignore: self.emit("changed-keymap"),
> @@ -78,6 +79,14 @@ class vmmGraphicsDetails(vmmGObjectUI):
> graphics_model.append(["spice", _("Spice server")])
> graphics_model.append(["vnc", _("VNC server")])
>
> + graphics_listen_list = self.widget("graphics-listen-type")
> + graphics_listen_model = Gtk.ListStore(str, str)
> + graphics_listen_list.set_model(graphics_listen_model)
> + uiutil.init_combo_text_column(graphics_listen_list, 1)
> + graphics_listen_model.clear()
> + graphics_listen_model.append(["address", _("Address")])
> + graphics_listen_model.append(["none", _("None")])
> +
> self.widget("graphics-address").set_model(Gtk.ListStore(str, str))
> uiutil.init_combo_text_column(self.widget("graphics-address"), 1)
>
> @@ -123,6 +132,7 @@ class vmmGraphicsDetails(vmmGObjectUI):
> uiutil.set_grid_row_visible(self.widget("graphics-xauth"), False)
>
> self.widget("graphics-type").set_active(0)
> + self.widget("graphics-listen-type").set_active(0)
> self.widget("graphics-address").set_active(0)
> self.widget("graphics-keymap").set_active(0)
>
> @@ -136,6 +146,7 @@ class vmmGraphicsDetails(vmmGObjectUI):
> def get_values(self):
> gtype = uiutil.get_list_selection(self.widget("graphics-type"))
> port, tlsport = self._get_config_graphics_ports()
> + listen = uiutil.get_list_selection(self.widget("graphics-listen-type"))
> addr = uiutil.get_list_selection(self.widget("graphics-address"))
> keymap = uiutil.get_list_selection(self.widget("graphics-keymap"))
> if keymap == "auto":
> @@ -147,7 +158,7 @@ class vmmGraphicsDetails(vmmGObjectUI):
>
> gl = self.widget("graphics-opengl").get_active()
>
> - return gtype, port, tlsport, addr, passwd, keymap, gl
> + return gtype, port, tlsport, listen, addr, passwd, keymap, gl
>
> def set_dev(self, gfx):
> self.reset_state()
> @@ -181,8 +192,12 @@ class vmmGraphicsDetails(vmmGObjectUI):
> use_passwd = gfx.passwd is not None
>
> set_port("graphics-port", gfx.port)
> - uiutil.set_list_selection(
> - self.widget("graphics-address"), gfx.listen)
> + if gfx.has_listen_none():
> + uiutil.set_list_selection(self.widget("graphics-listen-type"), 'none')
> + else:
> + uiutil.set_list_selection(self.widget("graphics-listen-type"), 'address')
> + uiutil.set_list_selection(
> + self.widget("graphics-address"), gfx.listen)
> uiutil.set_list_selection(
> self.widget("graphics-keymap"), gfx.keymap or None)
>
> @@ -216,10 +231,15 @@ class vmmGraphicsDetails(vmmGObjectUI):
> "graphics-tlsport-box", "graphics-opengl"]
>
> gtype = uiutil.get_list_selection(self.widget("graphics-type"))
> + listen = uiutil.get_list_selection(self.widget("graphics-listen-type"))
> +
> sdl_rows = ["graphics-xauth", "graphics-display"]
> - vnc_rows = ["graphics-password-box", "graphics-address",
> - "graphics-port-box", "graphics-keymap"]
> - spice_rows = vnc_rows[:] + ["graphics-tlsport-box"]
> + vnc_rows = ["graphics-password-box", "graphics-keymap"]
> + if listen == 'address':
> + vnc_rows.extend(["graphics-port-box", "graphics-address"])
> + spice_rows = vnc_rows[:]
> + if listen == 'address':
> + spice_rows.extend(["graphics-tlsport-box"])
> if self.conn.check_support(self.conn.SUPPORT_CONN_SPICE_GL):
> spice_rows.extend(["graphics-opengl"])
>
> @@ -238,6 +258,10 @@ class vmmGraphicsDetails(vmmGObjectUI):
> self._show_rows_from_type()
> self.emit("changed-type")
>
> + def _change_graphics_address(self, ignore):
> + self._show_rows_from_type()
> + self.emit("changed-address")
> +
> def _change_port_auto(self, ignore):
> self.widget("graphics-port-auto").set_inconsistent(False)
> self._change_ports()
> diff --git a/virtinst/devicegraphics.py b/virtinst/devicegraphics.py
> index 07b554e..e885418 100644
> --- a/virtinst/devicegraphics.py
> +++ b/virtinst/devicegraphics.py
> @@ -202,6 +202,11 @@ class VirtualGraphics(VirtualDevice):
> self.remove_child(find_listen[0])
> else:
> find_listen[0].address = val
> +
> + if self.port is None and self.tlsPort is None and self.type == "spice":
> + self.port = -1
> + self.tlsPort = -1
> +
> return val
> listen = XMLProperty("./@listen", set_converter=_set_listen)
>
> @@ -219,16 +224,25 @@ class VirtualGraphics(VirtualDevice):
> for listen in self.listens:
> self.remove_child(listen)
>
> + def remove_listen_none(self):
> + for listen in self.listens:
> + if listen.type == "none":
> + self.remove_child(listen)
> +
This method is not strictly required because of the fact that if there is a
listen type=none there should not be any other listen (libvirt will reject such
XML) so it's safe to call remove_all_listens.
> def add_listen(self):
> obj = _GraphicsListen(self.conn)
> self.add_child(obj)
> return obj
>
> + def has_listen_none(self):
> + return len(self.listens) > 0 and self.listens[0].type == "none"
> +
I would make this function more generic to get listen type of first listen
element or None. So far only one listen type is supported in libvirt and
virt-manager also supports only one listen type.
Pavel
> def set_listen_none(self):
> self.remove_all_listens()
> + self.listen = None
> self.port = None
> self.tlsPort = None
> - self.autoport = False
> + self.autoport = None
> self.socket = None
>
> if self.conn.check_support(
> --
> 2.10.0
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20161213/ce65c78f/attachment.sig>
More information about the virt-tools-list
mailing list