[Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Daniel P. Berrange]

Currently both object_del and device_del require that the
client provide the object/device short ID. While user
creatable objects require an ID to be provided at time of
creation, qdev devices may be created without giving an
ID. The only unique identifier they would then have is the
QOM object path.

Allowing device_del to accept an object path ensures all
devices are deletable regardless of whether they have an
ID.

 (qemu) device_add usb-mouse
 (qemu) qom-list /machine/peripheral-anon
 device[0] (child<usb-mouse>)
 type (string)
 (qemu) device_del /machine/peripheral-anon/device[0]

Although objects require an ID to be provided upfront,
there may be cases where the client would prefer to
use QOM paths when deleting.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 hmp-commands.hx |  6 ++++--
 qdev-monitor.c  | 14 +++++++++-----
 qmp-commands.hx | 17 +++++++++++++----
 qmp.c           | 10 +++++++---
 4 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..c0c113d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -675,7 +675,8 @@ ETEXI
 STEXI
 @item device_del @var{id}
 @findex device_del
-Remove device @var{id}.
+Remove device @var{id}. @var{id} may be a short ID
+or a QOM object path.
 ETEXI
 
     {
@@ -1279,7 +1280,8 @@ ETEXI
 STEXI
 @item object_del
 @findex object_del
-Destroy QOM object.
+Destroy QOM object. @var{id} may be a short ID
+or a QOM object path.
 ETEXI
 
 #ifdef CONFIG_SLIRP
diff --git a/qdev-monitor.c b/qdev-monitor.c
index f9e2d62..894b799 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -785,13 +785,17 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
 void qmp_device_del(const char *id, Error **errp)
 {
     Object *obj;
-    char *root_path = object_get_canonical_path(qdev_get_peripheral());
-    char *path = g_strdup_printf("%s/%s", root_path, id);
 
-    g_free(root_path);
-    obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
-    g_free(path);
+    if (id[0] == '/') {
+        obj = object_resolve_path(id, NULL);
+    } else {
+        char *root_path = object_get_canonical_path(qdev_get_peripheral());
+        char *path = g_strdup_printf("%s/%s", root_path, id);
 
+        g_free(root_path);
+        obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
+        g_free(path);
+    }
     if (!obj) {
         error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
                   "Device '%s' not found", id);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ba630b1..d3070cf 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -321,14 +321,19 @@ Remove a device.
 
 Arguments:
 
-- "id": the device's ID (json-string)
-
+- "id": the device's ID or QOM path (json-string)
+EQ
 Example:
 
 -> { "execute": "device_del", "arguments": { "id": "net1" } }
 <- { "return": {} }
 
-EQMP
+Example:
+
+-> { "execute": "device_del", "arguments": { "id": "/machine/peripheral-anon/device[0]" } }
+<- { "return": {} }
+
+MP
 
     {
         .name       = "send-key",
@@ -965,7 +970,7 @@ Remove QOM object.
 
 Arguments:
 
-- "id": the object's ID (json-string)
+- "id": the object's ID or QOM path (json-string)
 
 Example:
 
@@ -973,6 +978,10 @@ Example:
 <- { "return": {} }
 
 
+-> { "execute": "object-del", "arguments": { "id": "/objects/hostmem1" } }
+<- { "return": {} }
+
+
 EQMP
 
 
diff --git a/qmp.c b/qmp.c
index 403805a..11e9f51 100644
--- a/qmp.c
+++ b/qmp.c
@@ -678,11 +678,15 @@ void qmp_object_add(QDict *qdict, QObject **ret, Error **errp)
 
 void qmp_object_del(const char *id, Error **errp)
 {
-    Object *container;
     Object *obj;
 
-    container = object_get_objects_root();
-    obj = object_resolve_path_component(container, id);
+    if (id[0] == '/') {
+        obj = object_resolve_path(id, NULL);
+    } else {
+        Object *container;
+        container = object_get_objects_root();
+        obj = object_resolve_path_component(container, id);
+    }
     if (!obj) {
         error_setg(errp, "object id not found");
         return;
-- 
2.4.3

Re: [Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

On 08/27/2015 09:37 AM, Daniel P. Berrange wrote:
> Currently both object_del and device_del require that the
> client provide the object/device short ID. While user
> creatable objects require an ID to be provided at time of
> creation, qdev devices may be created without giving an
> ID. The only unique identifier they would then have is the
> QOM object path.
> 
> Allowing device_del to accept an object path ensures all
> devices are deletable regardless of whether they have an
> ID.
> 
>  (qemu) device_add usb-mouse
>  (qemu) qom-list /machine/peripheral-anon
>  device[0] (child<usb-mouse>)
>  type (string)
>  (qemu) device_del /machine/peripheral-anon/device[0]
> 
> Although objects require an ID to be provided upfront,
> there may be cases where the client would prefer to
> use QOM paths when deleting.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  hmp-commands.hx |  6 ++++--
>  qdev-monitor.c  | 14 +++++++++-----
>  qmp-commands.hx | 17 +++++++++++++----
>  qmp.c           | 10 +++++++---
>  4 files changed, 33 insertions(+), 14 deletions(-)

Might also want to touch qapi-schema.json for consistent documentation
of the @id parameter (our goal is for qmp-commands.hx to go away some
day; and while we can merge contents at that time, it's nicer to keep
them in sync now)

Reviewed-by: Eric Blake <eblake@redhat.com

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Gonglei]

On 2015/8/27 23:37, Daniel P. Berrange wrote:
> Currently both object_del and device_del require that the
> client provide the object/device short ID. While user
> creatable objects require an ID to be provided at time of
> creation, qdev devices may be created without giving an
> ID. The only unique identifier they would then have is the
> QOM object path.
> 
> Allowing device_del to accept an object path ensures all
> devices are deletable regardless of whether they have an
> ID.
> 
>  (qemu) device_add usb-mouse
>  (qemu) qom-list /machine/peripheral-anon
>  device[0] (child<usb-mouse>)
>  type (string)
>  (qemu) device_del /machine/peripheral-anon/device[0]
> 
> Although objects require an ID to be provided upfront,
> there may be cases where the client would prefer to
> use QOM paths when deleting.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  hmp-commands.hx |  6 ++++--
>  qdev-monitor.c  | 14 +++++++++-----
>  qmp-commands.hx | 17 +++++++++++++----
>  qmp.c           | 10 +++++++---
>  4 files changed, 33 insertions(+), 14 deletions(-)
> 

It's useful.

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..c0c113d 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -675,7 +675,8 @@ ETEXI
>  STEXI
>  @item device_del @var{id}
>  @findex device_del
> -Remove device @var{id}.
> +Remove device @var{id}. @var{id} may be a short ID
> +or a QOM object path.
>  ETEXI
>  
>      {
> @@ -1279,7 +1280,8 @@ ETEXI
>  STEXI
>  @item object_del
>  @findex object_del
> -Destroy QOM object.
> +Destroy QOM object. @var{id} may be a short ID
> +or a QOM object path.
>  ETEXI
>  
>  #ifdef CONFIG_SLIRP
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index f9e2d62..894b799 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -785,13 +785,17 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
>  void qmp_device_del(const char *id, Error **errp)
>  {
>      Object *obj;
> -    char *root_path = object_get_canonical_path(qdev_get_peripheral());
> -    char *path = g_strdup_printf("%s/%s", root_path, id);
>  
> -    g_free(root_path);
> -    obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
> -    g_free(path);
> +    if (id[0] == '/') {
> +        obj = object_resolve_path(id, NULL);
> +    } else {
> +        char *root_path = object_get_canonical_path(qdev_get_peripheral());
> +        char *path = g_strdup_printf("%s/%s", root_path, id);
>  
> +        g_free(root_path);
> +        obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
> +        g_free(path);
> +    }
>      if (!obj) {
>          error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>                    "Device '%s' not found", id);
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ba630b1..d3070cf 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -321,14 +321,19 @@ Remove a device.
>  
>  Arguments:
>  
> -- "id": the device's ID (json-string)
> -
> +- "id": the device's ID or QOM path (json-string)
> +EQ
>  Example:
>  
>  -> { "execute": "device_del", "arguments": { "id": "net1" } }
>  <- { "return": {} }
>  
> -EQMP
> +Example:
> +
> +-> { "execute": "device_del", "arguments": { "id": "/machine/peripheral-anon/device[0]" } }
> +<- { "return": {} }
> +
> +MP
>  
>      {
>          .name       = "send-key",
> @@ -965,7 +970,7 @@ Remove QOM object.
>  
>  Arguments:
>  
> -- "id": the object's ID (json-string)
> +- "id": the object's ID or QOM path (json-string)
>  
>  Example:
>  
> @@ -973,6 +978,10 @@ Example:
>  <- { "return": {} }
>  
>  
> +-> { "execute": "object-del", "arguments": { "id": "/objects/hostmem1" } }
> +<- { "return": {} }
> +
> +
>  EQMP
>  
>  
> diff --git a/qmp.c b/qmp.c
> index 403805a..11e9f51 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -678,11 +678,15 @@ void qmp_object_add(QDict *qdict, QObject **ret, Error **errp)
>  
>  void qmp_object_del(const char *id, Error **errp)
>  {
> -    Object *container;
>      Object *obj;
>  
> -    container = object_get_objects_root();
> -    obj = object_resolve_path_component(container, id);
> +    if (id[0] == '/') {
> +        obj = object_resolve_path(id, NULL);
> +    } else {
> +        Object *container;
> +        container = object_get_objects_root();
> +        obj = object_resolve_path_component(container, id);
> +    }
>      if (!obj) {
>          error_setg(errp, "object id not found");
>          return;
> 

Re: [Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Markus Armbruster]

Copying Andreas and Paolo for QOM expertise.

"Daniel P. Berrange" <berrange@redhat.com> writes:

> Currently both object_del and device_del require that the
> client provide the object/device short ID. While user
> creatable objects require an ID to be provided at time of
> creation, qdev devices may be created without giving an
> ID. The only unique identifier they would then have is the
> QOM object path.
>
> Allowing device_del to accept an object path ensures all
> devices are deletable regardless of whether they have an
> ID.
>
>  (qemu) device_add usb-mouse
>  (qemu) qom-list /machine/peripheral-anon
>  device[0] (child<usb-mouse>)
>  type (string)
>  (qemu) device_del /machine/peripheral-anon/device[0]
>
> Although objects require an ID to be provided upfront,
> there may be cases where the client would prefer to
> use QOM paths when deleting.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

I believe this makes sense no matter what we do about device IDs (see
thread "Should we auto-generate IDs?").

> ---
>  hmp-commands.hx |  6 ++++--
>  qdev-monitor.c  | 14 +++++++++-----
>  qmp-commands.hx | 17 +++++++++++++----
>  qmp.c           | 10 +++++++---
>  4 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..c0c113d 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -675,7 +675,8 @@ ETEXI
>  STEXI
>  @item device_del @var{id}
>  @findex device_del
> -Remove device @var{id}.
> +Remove device @var{id}. @var{id} may be a short ID
> +or a QOM object path.
>  ETEXI
>  
>      {
> @@ -1279,7 +1280,8 @@ ETEXI
>  STEXI
>  @item object_del
>  @findex object_del
> -Destroy QOM object.
> +Destroy QOM object. @var{id} may be a short ID
> +or a QOM object path.
>  ETEXI
>  
>  #ifdef CONFIG_SLIRP
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index f9e2d62..894b799 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -785,13 +785,17 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
>  void qmp_device_del(const char *id, Error **errp)
>  {
>      Object *obj;
> -    char *root_path = object_get_canonical_path(qdev_get_peripheral());
> -    char *path = g_strdup_printf("%s/%s", root_path, id);
>  
> -    g_free(root_path);
> -    obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
> -    g_free(path);
> +    if (id[0] == '/') {
> +        obj = object_resolve_path(id, NULL);
> +    } else {
> +        char *root_path = object_get_canonical_path(qdev_get_peripheral());
> +        char *path = g_strdup_printf("%s/%s", root_path, id);
>  
> +        g_free(root_path);
> +        obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
> +        g_free(path);
> +    }
>      if (!obj) {
>          error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>                    "Device '%s' not found", id);

Easier to review with whitespace change ignored:

    diff --git a/qdev-monitor.c b/qdev-monitor.c
    index f9e2d62..894b799 100644
    --- a/qdev-monitor.c
    +++ b/qdev-monitor.c
    @@ -785,13 +785,17 @@
     void qmp_device_del(const char *id, Error **errp)
     {
         Object *obj;
    +
    +    if (id[0] == '/') {
    +        obj = object_resolve_path(id, NULL);
    +    } else {
         char *root_path = object_get_canonical_path(qdev_get_peripheral());
         char *path = g_strdup_printf("%s/%s", root_path, id);

         g_free(root_path);
         obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
         g_free(path);
    -
    +    }
         if (!obj) {
             error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
                       "Device '%s' not found", id);

I'd keep the blank line between the code finding obj and the if (!obj)
error check.

@id is a suboptimal name now, but changing it isn't worth the churn.

> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ba630b1..d3070cf 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -321,14 +321,19 @@ Remove a device.
>  
>  Arguments:
>  
> -- "id": the device's ID (json-string)
> -
> +- "id": the device's ID or QOM path (json-string)
> +EQ
>  Example:
>  
>  -> { "execute": "device_del", "arguments": { "id": "net1" } }
>  <- { "return": {} }
>  
> -EQMP
> +Example:
> +
> +-> { "execute": "device_del", "arguments": { "id": "/machine/peripheral-anon/device[0]" } }
> +<- { "return": {} }
> +
> +MP
>  
>      {
>          .name       = "send-key",
> @@ -965,7 +970,7 @@ Remove QOM object.
>  
>  Arguments:
>  
> -- "id": the object's ID (json-string)
> +- "id": the object's ID or QOM path (json-string)
>  
>  Example:
>  
> @@ -973,6 +978,10 @@ Example:
>  <- { "return": {} }
>  
>  
> +-> { "execute": "object-del", "arguments": { "id": "/objects/hostmem1" } }
> +<- { "return": {} }
> +
> +
>  EQMP
>  
>  

Thanks for adding examples.

As Eric said, you should also update qapi-schema.json.

The duplication is unfortunate.  There's an RFC PATCH series from
Marc-André pending review, which should get us one step closer to
killing qmp-commands.hx.

> diff --git a/qmp.c b/qmp.c
> index 403805a..11e9f51 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -678,11 +678,15 @@ void qmp_object_add(QDict *qdict, QObject **ret, Error **errp)
>  
>  void qmp_object_del(const char *id, Error **errp)
>  {
> -    Object *container;
>      Object *obj;
>  
> -    container = object_get_objects_root();
> -    obj = object_resolve_path_component(container, id);
> +    if (id[0] == '/') {
> +        obj = object_resolve_path(id, NULL);
> +    } else {
> +        Object *container;
> +        container = object_get_objects_root();
> +        obj = object_resolve_path_component(container, id);
> +    }
>      if (!obj) {
>          error_setg(errp, "object id not found");
>          return;

Ignoring whitespace change:

    diff --git a/qmp.c b/qmp.c
    index 403805a..11e9f51 100644
    --- a/qmp.c
    +++ b/qmp.c
    @@ -678,11 +678,15 @@

     void qmp_object_del(const char *id, Error **errp)
     {
    -    Object *container;
         Object *obj;

    +    if (id[0] == '/') {
    +        obj = object_resolve_path(id, NULL);
    +    } else {
    +        Object *container;
         container = object_get_objects_root();
         obj = object_resolve_path_component(container, id);
    +    }
         if (!obj) {
             error_setg(errp, "object id not found");
             return;

I'd keep container right where it is.  We generally prefer keeping
declarations at the beginning of the function body, at least for small
functions.

Update qapi-schema.json updated the obvious way, and you can have my
R-by.  Also addressing my stylistic nitpicks would be nice.

Thanks for tackling this, it's long overdue!

Re: [Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 02:53:41PM +0200, Markus Armbruster wrote:
> Copying Andreas and Paolo for QOM expertise.
> 
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> 
> > Currently both object_del and device_del require that the
> > client provide the object/device short ID. While user
> > creatable objects require an ID to be provided at time of
> > creation, qdev devices may be created without giving an
> > ID. The only unique identifier they would then have is the
> > QOM object path.
> >
> > Allowing device_del to accept an object path ensures all
> > devices are deletable regardless of whether they have an
> > ID.
> >
> >  (qemu) device_add usb-mouse
> >  (qemu) qom-list /machine/peripheral-anon
> >  device[0] (child<usb-mouse>)
> >  type (string)
> >  (qemu) device_del /machine/peripheral-anon/device[0]
> >
> > Although objects require an ID to be provided upfront,
> > there may be cases where the client would prefer to
> > use QOM paths when deleting.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> 
> I believe this makes sense no matter what we do about device IDs (see
> thread "Should we auto-generate IDs?").

[snip]

> Update qapi-schema.json updated the obvious way, and you can have my
> R-by.  Also addressing my stylistic nitpicks would be nice.

Already posted a v2 with the qapi-schema.json addition after Eric
pointed it out :-)

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Paolo Bonzini]

On 28/08/2015 14:53, Markus Armbruster wrote:
> I believe this makes sense no matter what we do about device IDs (see
> thread "Should we auto-generate IDs?").

I haven't read that huge thread yet, but I think it gives the user too
much power.  There are internal objects that are not supposed to be
freed, and freeing them would likely result in a SEGV or similar.

Paolo

Re: [Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Gonglei]

On 2015/8/29 14:49, Paolo Bonzini wrote:
> 
> 
> On 28/08/2015 14:53, Markus Armbruster wrote:
>> I believe this makes sense no matter what we do about device IDs (see
>> thread "Should we auto-generate IDs?").
> 
> I haven't read that huge thread yet, but I think it gives the user too
> much power.  There are internal objects that are not supposed to be
> freed, and freeing them would likely result in a SEGV or similar.
> 
If one object do not support hot-unplug, the interface will return simply
with error messages. ;)

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Daniel P. Berrange]

On Sat, Aug 29, 2015 at 08:49:58AM +0200, Paolo Bonzini wrote:
> 
> 
> On 28/08/2015 14:53, Markus Armbruster wrote:
> > I believe this makes sense no matter what we do about device IDs (see
> > thread "Should we auto-generate IDs?").
> 
> I haven't read that huge thread yet, but I think it gives the user too
> much power.  There are internal objects that are not supposed to be
> freed, and freeing them would likely result in a SEGV or similar.

I'll double check, but I thought it raised a nice error if the object
did not implement the user-creatable interface.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] monitor: allow object_del & device_del to accept QOM paths

[Daniel P. Berrange]

On Tue, Sep 01, 2015 at 09:58:14AM +0100, Daniel P. Berrange wrote:
> On Sat, Aug 29, 2015 at 08:49:58AM +0200, Paolo Bonzini wrote:
> > 
> > 
> > On 28/08/2015 14:53, Markus Armbruster wrote:
> > > I believe this makes sense no matter what we do about device IDs (see
> > > thread "Should we auto-generate IDs?").
> > 
> > I haven't read that huge thread yet, but I think it gives the user too
> > much power.  There are internal objects that are not supposed to be
> > freed, and freeing them would likely result in a SEGV or similar.
> 
> I'll double check, but I thought it raised a nice error if the object
> did not implement the user-creatable interface.

Ok, I'm wrong - we get an assertion error because it isn't user-creatble
and then we abort. I'll send a v3 which returns a pretty error instead
of aborting.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|