[Qemu-devel] [PATCH 1/2] qxl: unbreak after memory API conversion

[Qemu-devel] [PATCH 1/2] qxl: unbreak after memory API conversion

[Alon Levy]

Break is only noticable with newer spice-server library (0.8.2 release
or 0.9.0 and newer on master branch).

ioport_write's val was changed from uint32_t to uint64_t, this
broke two printfs. Use PRId64 instead of %d.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index db7ae7a..1d31ccd 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1189,7 +1189,7 @@ async_common:
         }
         d->current_async = orig_io_port;
         qemu_mutex_unlock(&d->async_lock);
-        dprint(d, 2, "start async %d (%d)\n", io_port, val);
+        dprint(d, 2, "start async %d (%"PRId64")\n", io_port, val);
         break;
     default:
         break;
@@ -1305,7 +1305,8 @@ async_common:
         break;
     }
     case QXL_IO_FLUSH_SURFACES_ASYNC:
-        dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC (%d) (%s, s#=%d, res#=%d)\n",
+        dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC"
+                     " (%"PRId64") (%s, s#=%d, res#=%d)\n",
                val, qxl_mode_to_string(d->mode), d->guest_surfaces.count,
                d->num_free_res);
         qxl_spice_flush_surfaces_async(d);
-- 
1.7.6

[Qemu-devel] [PATCH 2/2] ui/spice-core: report version in 'info spice'

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 ui/spice-core.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 3d77c01..342ed6c 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -372,6 +372,10 @@ void do_info_spice_print(Monitor *mon, const QObject *data)
         monitor_printf(mon, "     address: %s:%d [tls]\n", host, port);
     }
     monitor_printf(mon, "        auth: %s\n", qdict_get_str(server, "auth"));
+    monitor_printf(mon, "     version: %d.%d.%d\n",
+                   (SPICE_SERVER_VERSION & 0xff0000) >> 16,
+                   (SPICE_SERVER_VERSION & 0xff00) >> 8,
+                    SPICE_SERVER_VERSION & 0xff);
 
     channels = qdict_get_qlist(server, "channels");
     if (qlist_empty(channels)) {
-- 
1.7.6

Re: [Qemu-devel] [PATCH 2/2] ui/spice-core: report version in 'info spice'

[Gerd Hoffmann]

On 08/09/11 22:53, Alon Levy wrote:
> Signed-off-by: Alon Levy<alevy@redhat.com>
> ---
>   ui/spice-core.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 3d77c01..342ed6c 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -372,6 +372,10 @@ void do_info_spice_print(Monitor *mon, const QObject *data)
>           monitor_printf(mon, "     address: %s:%d [tls]\n", host, port);
>       }
>       monitor_printf(mon, "        auth: %s\n", qdict_get_str(server, "auth"));
> +    monitor_printf(mon, "     version: %d.%d.%d\n",
> +                   (SPICE_SERVER_VERSION&  0xff0000)>>  16,
> +                   (SPICE_SERVER_VERSION&  0xff00)>>  8,
> +                    SPICE_SERVER_VERSION&  0xff);

No.  You should should stick this info into the dict in do_info_spice, 
then just get it from the dict for printing like it is done for auth. 
This way you'll have the information in the qmp monitor too.

I think it would also be nice to somehow hint this is the spice version 
qemu was compiled against.  The shared library which is actually used 
might be newer.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 2/2] ui/spice-core: report version in 'info spice'

[Alon Levy]

On Wed, Aug 10, 2011 at 08:54:36AM +0200, Gerd Hoffmann wrote:
> On 08/09/11 22:53, Alon Levy wrote:
> >Signed-off-by: Alon Levy<alevy@redhat.com>
> >---
> >  ui/spice-core.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> >diff --git a/ui/spice-core.c b/ui/spice-core.c
> >index 3d77c01..342ed6c 100644
> >--- a/ui/spice-core.c
> >+++ b/ui/spice-core.c
> >@@ -372,6 +372,10 @@ void do_info_spice_print(Monitor *mon, const QObject *data)
> >          monitor_printf(mon, "     address: %s:%d [tls]\n", host, port);
> >      }
> >      monitor_printf(mon, "        auth: %s\n", qdict_get_str(server, "auth"));
> >+    monitor_printf(mon, "     version: %d.%d.%d\n",
> >+                   (SPICE_SERVER_VERSION&  0xff0000)>>  16,
> >+                   (SPICE_SERVER_VERSION&  0xff00)>>  8,
> >+                    SPICE_SERVER_VERSION&  0xff);
> 
> No.  You should should stick this info into the dict in
> do_info_spice, then just get it from the dict for printing like it
> is done for auth. This way you'll have the information in the qmp
> monitor too.
> 
> I think it would also be nice to somehow hint this is the spice
sent a second patch (fixing the above), but didn't do any hinting. I'll send a third.

> version qemu was compiled against.  The shared library which is
> actually used might be newer.

I thought of this problem but decided it's better to at least have the compiled
version (getting the linked against version is not hard, but I'd rather
not force to do it at this moment - I guess via dlopen. Which is linux specific - 
will need yet another wrapper for that api to compile on windows).

> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [PATCH 2/2] ui/spice-core: report version in 'info spice'

[Gerd Hoffmann]

   Hi,

>> I think it would also be nice to somehow hint this is the spice
> sent a second patch (fixing the above), but didn't do any hinting. I'll send a third.

Saw v2.  I would just stick a "version" string into the dict instead of 
the three values for major, minor and micro.

>> version qemu was compiled against.  The shared library which is
>> actually used might be newer.
>
> I thought of this problem but decided it's better to at least have the compiled
> version (getting the linked against version is not hard, but I'd rather
> not force to do it at this moment - I guess via dlopen. Which is linux specific -
> will need yet another wrapper for that api to compile on windows).

The compiled version is perfectly fine, I think it is more useful to 
have that one as it decides which features qemu is able to use.

If we want report the runtime version too (any reason to do this?) I'd 
just add a spice_server_get_version() function to the library.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 2/2] ui/spice-core: report version in 'info spice'

[Alon Levy]

On Wed, Aug 10, 2011 at 10:10:59AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >>I think it would also be nice to somehow hint this is the spice
> >sent a second patch (fixing the above), but didn't do any hinting. I'll send a third.
> 
> Saw v2.  I would just stick a "version" string into the dict instead
> of the three values for major, minor and micro.
> 

Argh. Sent v3 with the three ints. Do you prefer a single version?

> >>version qemu was compiled against.  The shared library which is
> >>actually used might be newer.
> >
> >I thought of this problem but decided it's better to at least have the compiled
> >version (getting the linked against version is not hard, but I'd rather
> >not force to do it at this moment - I guess via dlopen. Which is linux specific -
> >will need yet another wrapper for that api to compile on windows).
> 
> The compiled version is perfectly fine, I think it is more useful to
> have that one as it decides which features qemu is able to use.
> 

Just completeness :) I'm not going to do it right now anyway.

> If we want report the runtime version too (any reason to do this?)
> I'd just add a spice_server_get_version() function to the library.
> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [PATCH 2/2] ui/spice-core: report version in 'info spice'

[Gerd Hoffmann]

   Hi,

> Argh. Sent v3 with the three ints. Do you prefer a single version?

Yea, would be great.

thanks,
   Gerd

Re: [Qemu-devel] [PATCH 1/2] qxl: unbreak after memory API conversion

[Gerd Hoffmann]

On 08/09/11 22:53, Alon Levy wrote:
> Break is only noticable with newer spice-server library (0.8.2 release
> or 0.9.0 and newer on master branch).
>
> ioport_write's val was changed from uint32_t to uint64_t, this
> broke two printfs. Use PRId64 instead of %d.

Added to the spice patch queue.

thanks,
   Gerd