Alon,
I am reading the source code of the vdservice for Windows guest OS.
"vdservice" if fact is a proxy software between Windows application and VirIO
serial driver.
I was suppressed to find that " vdservice" open two Windows kernel
driver. In Windows, I only find one VirtIO P
Hi,
> It is not easy when you have 2 components, and it is much less easy when
> you have 3 or 4 components. So why make it more complicated if you can
> avoid it. Especially since there is no functional reason for making the
> qemu/client capabilities/versions dependent on the server internal d
On Tue, Mar 13, 2012 at 03:06:17PM +0800, Charles.Tsai-蔡清海-研究發展部 wrote:
> Alon,
>
> I am reading the source code of the vdservice for Windows guest OS.
> "vdservice" if fact is a proxy software between Windows application and VirIO
> serial driver.
> I was suppressed to find that "
Alon,
Thank you for your clarification. I did not see VDI_PORT_DEV_NAME is being used
in the service.
-Original Message-
From: Alon Levy [mailto:al...@redhat.com]
Sent: Tuesday, March 13, 2012 4:06 PM
To: Charles.Tsai-蔡清海-研究發展部
Cc: spice-devel@lists.freedesktop.org
Subject: Re: [Spice
On Tue, Mar 13, 2012 at 04:09:56PM +0800, Charles.Tsai-蔡清海-研究發展部 wrote:
> Alon,
>
> Thank you for your clarification. I did not see VDI_PORT_DEV_NAME is being
> used in the service.
>
win32-vdagent alon (master)$ git grep VDI_PORT_DEV_NAME
vdservice/pci_vdi_port.cpp:#define VDI_PORT_DEV_NAME
Alon,
Yes, I did see these. But I think this device is no longer used by the service.
That is why I want to clarify my puzzle.
-Original Message-
From: Alon Levy [mailto:al...@redhat.com]
Sent: Tuesday, March 13, 2012 5:27 PM
To: Charles.Tsai-蔡清海-研究發展部
Cc: spice-devel@lists.freedesktop.
On Tue, Mar 13, 2012 at 05:32:13PM +0800, Charles.Tsai-蔡清海-研究發展部 wrote:
> Alon,
>
> Yes, I did see these. But I think this device is no longer used by the
> service.
> That is why I want to clarify my puzzle.
>
Charles, the puzzle is simple: it isn't being used anymore. You don't
create the dev
Thanks. I got it now. How come the code is still there.
I believe that CreateFile API fails definitely because there is no such a
device in the system.
-Original Message-
From: Alon Levy [mailto:al...@redhat.com]
Sent: Tuesday, March 13, 2012 6:17 PM
To: Charles.Tsai-蔡清海-研究發展部
Cc: spice
On Tue, Mar 13, 2012 at 06:32:55PM +0800, Charles.Tsai-蔡清海-研究發展部 wrote:
> Thanks. I got it now. How come the code is still there.
Because the same code base is for guests using the new virtserialport
and the old vdi_port. That device is still in RHEL 5.
> I believe that CreateFile API fails defin
Hi,
as Marc-Andre has decided to drop log4cpp, I would like to at least
revert the part of the message format. The reason is simple, Spice-QA
guys are developing a test framework, which uses the log4cpp format we had.
Opinions?
--
Peter Hatina
EMEA ENG-Desktop Development
Red Hat Czech, Brno
>F
Thanks. Otherwise, I cannot get it when I read Windows service code.
-Original Message-
From: Alon Levy [mailto:al...@redhat.com]
Sent: Tuesday, March 13, 2012 7:09 PM
To: Charles.Tsai-蔡清海-研究發展部
Cc: spice-devel@lists.freedesktop.org
Subject: Re: [Spice-devel] Access local network printer
Hey,
On Tue, Mar 13, 2012 at 12:29:01PM +0100, Peter Hatina wrote:
> Hi,
>
> as Marc-Andre has decided to drop log4cpp, I would like to at least
> revert the part of the message format. The reason is simple, Spice-QA
> guys are developing a test framework, which uses the log4cpp format we had.
>
- Mensaje original -
> Hi,
>
> as Marc-Andre has decided to drop log4cpp, I would like to at least
It was publicly acked by several people in the team.
> revert the part of the message format. The reason is simple, Spice-QA
> guys are developing a test framework, which uses the log4cpp
On 03/13/2012 01:45 PM, Marc-André Lureau wrote:
>
>
> - Mensaje original -
>> Hi,
>>
>> as Marc-Andre has decided to drop log4cpp, I would like to at least
>
> It was publicly acked by several people in the team.
>
>> revert the part of the message format. The reason is simple, Spice-Q
- Mensaje original -
> Hi,
>
> as Marc-Andre has decided to drop log4cpp, I would like to at least
> revert the part of the message format. The reason is simple, Spice-QA
> guys are developing a test framework, which uses the log4cpp format
> we had.
>
> Opinions?
IOW, why not just imp
- Mensaje original -
> glog is missing the "modified" method name, which is added by the
> patch.
> QA guys rely especially on the method name.
That's the problem: we can't refactor, rename function, add arguments or change
order of things, or even perhaps add more debugging. This smell
On Tue, Mar 13, 2012 at 08:58:26AM -0400, Marc-André Lureau wrote:
>
>
> - Mensaje original -
> > glog is missing the "modified" method name, which is added by the
> > patch.
> > QA guys rely especially on the method name.
>
> That's the problem: we can't refactor, rename function, add
>
On 03/13/2012 01:26 PM, Christophe Fergeau wrote:
> Hey,
>
> On Tue, Mar 13, 2012 at 12:29:01PM +0100, Peter Hatina wrote:
>> Hi,
>>
>> as Marc-Andre has decided to drop log4cpp, I would like to at
>> least revert the part of the message format. The reason is
>> simple, Spice-QA guys are developi
On Tue, Mar 13, 2012 at 08:58:26AM -0400, Marc-André Lureau wrote:
>
>
> - Mensaje original -
> > glog is missing the "modified" method name, which is added by the
> > patch.
> > QA guys rely especially on the method name.
>
> That's the problem: we can't refactor, rename function, add a
On Tue, Mar 13, 2012 at 2:02 PM, Christophe Fergeau wrote:
> I think they are exploring using systemtap too. In the mean time, it
> doesn't really hurt to have more expressive logs messages, and logs in
> places where it might be useful...
I agree. So why removing existing debug? Why making the c
Hello Marc-Andre,
I agree as well that parsing logs is not the best way. I had easy way to probe
old spice-client trough systemtap. But since the new client uses gobjects I'm
no more able to trace what I need. It could be possible if we could inject some
stap probes inside code.
The whole thin
Anyway I'm still able to probe the spice-xpi which works well.
I don't agree with argument related to maintenance as everything needs to be
maintained since all layers of the system changes time by time. No matter if
it's libc some function in shared library or format.
Lubos
- Original Mess
On Tue, Mar 13, 2012 at 02:10:50PM +0100, Marc-André Lureau wrote:
> I agree. So why removing existing debug? Why making the code more
> complicated (execv, title handling?)
95% of the patch is either wrapping g_* calls in macros or adding logging,
there isn't a lot of "making the code more compli
On 03/13/2012 01:55 PM, Marc-André Lureau wrote:
>
>
> - Mensaje original -
>> Hi,
>>
>> as Marc-Andre has decided to drop log4cpp, I would like to at least
>> revert the part of the message format. The reason is simple, Spice-QA
>> guys are developing a test framework, which uses the log
- Original Message -
> From: "Christophe Fergeau"
> To: "Marc-André Lureau"
> Cc: spice-devel@lists.freedesktop.org
> Sent: Tuesday, March 13, 2012 2:02:33 PM
> Subject: Re: [Spice-devel] spice-xpi
>
> On Tue, Mar 13, 2012 at 08:58:26AM -0400, Marc-André Lureau wrote:
> >
> >
> > ---
The current spice-gtk configure.ac script only enables
-Wall -Wno-sign-compare -Werror -Wno-deprecated-declarations
IMHO, this is not enough, and applications should strive to enable
as many GCC warning options as is practical for their codebase.
Rather than taking a whitelist approach to compil
From: "Daniel P. Berrange"
gtk-doc.make is created by autogen.sh, therefore it should
not be deleted in distclean, only maintainerclean
---
Makefile.am |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 73b5531..8ab020a 100644
--- a/Makefile
From: "Daniel P. Berrange"
---
gtk/Makefile.am |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/gtk/Makefile.am b/gtk/Makefile.am
index ed40ce8..2057b5a 100644
--- a/gtk/Makefile.am
+++ b/gtk/Makefile.am
@@ -63,8 +63,6 @@ SPICE_COMMON_CPPFLAGS = \
-DUSB_IDS=\""$(
From: "Daniel P. Berrange"
Some functions should be declared to take const char * instead
of plain char *. In other places we intentionally cast away
const-ness, so we should add explicit casts to stop compiler
warnings
---
gtk/snappy.c |2 +-
gtk/spice-channel.c |2 +-
gtk/spic
From: "Daniel P. Berrange"
Fix a case of 'static int inline' to be 'static inline int'
---
common/lines.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/lines.c b/common/lines.c
index 1a14c18..ee52a46 100644
--- a/common/lines.c
+++ b/common/lines.c
@@ -82,7 +82
From: "Daniel P. Berrange"
Various methods are deprecated by using the G_GNUC_DEPRECATED_FOR
macro. Unfortunately this macro was placed in the .c file impl,
instead of the .h file decl. Thus applications building against
SPICE-GTK would never see the deprecation warnings. At the same
time, buildi
From: "Daniel P. Berrange"
Add extra {} braces around if/else statements which only
call SPICE_DEBUG to avoid:
../common/ssl_verify.c: In function 'verify_pubkey':
../common/ssl_verify.c:87:50: warning: suggest braces around empty body in an
'else' statement [-Wempty-body]
../common/ssl_verify.
From: "Daniel P. Berrange"
There are some integer range checks which always evaluate false
due to use of unsigned integer types. One of these would prevent
detection of encoding errors from celt. The others are simply
no-ops.
* gtk/channel-record.c: Make 'frame_size' signed to allow detection
From: "Daniel P. Berrange"
Arithmetic on void * pointers is undefined by the C standard.
Convert the one case of this to use guint8 instead.
* gtk/channel-main.c: s/void */guint8 */
---
gtk/channel-main.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gtk/channel-main
From: "Daniel P. Berrange"
The usb-helper-acl.h header file duplicated some declarations
already present in usb-device-manager.h
The channel-display.c file declared the object init function
which is already done by the GObject helper macros
* gtk/channel-display.c: Remove duplicate decl of init
From: "Daniel P. Berrange"
To allow the compile to detect incorrect printf formats, any
var-args function should have a format annotation
* common/macros.h: Helper to define ATTR_PRINTF for code
which can't depend on glib
* common/canvas_base.c, common/lz.h, common/macros.h: Annotate
some va
From: "Daniel P. Berrange"
* common/quic.c, common/rop3.c, common/sw_canvas.c,
gtk/spice-client-glib-usb-acl-helper.c: s/()/(void)/
---
common/quic.c |6 +++---
common/rop3.c |8
common/sw_canvas.c |2 +-
From: "Daniel P. Berrange"
A number of functions were used without prior declaration. In
some cases this was due to missing include files. In other cases
the functions should have just been static.
Ideally this would allow -Wmissing-declarations to be enabled, but
the files generated by spice_co
From: "Daniel P. Berrange"
* common/quic.h, common/rop3.h, common/sw_canvas.h: s/()/(void)/
---
common/quic.h |2 +-
common/rop3.h |2 +-
common/sw_canvas.h |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/common/quic.h b/common/quic.h
index 74068f3..35
From: "Daniel P. Berrange"
A few functions have very large arrays declared on the stack.
Replace these with heap allocations, to reduce risk of stack
overflows in deep callpaths
---
gtk/channel-playback.c |6 --
gtk/spice-channel.c| 16
2 files changed, 16 insertio
From: "Daniel P. Berrange"
GNULIB has a helpful module 'manywarnings' which makes it easy
to turn on every single GCC warning. The general goal is that
every possible GCC warning should be enabled, except for certain
blacklisted warnings.
This imports the GNULIB m4 macros, and updates configure.
On Tue, Mar 13, 2012 at 01:40:12PM +, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> A few functions have very large arrays declared on the stack.
> Replace these with heap allocations, to reduce risk of stack
> overflows in deep callpaths
> ---
> gtk/channel-playback.c |6 ++
Hi,
On 03/13/2012 02:40 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"
A few functions have very large arrays declared on the stack.
Replace these with heap allocations, to reduce risk of stack
overflows in deep callpaths
---
gtk/channel-playback.c |6 --
gtk/spice-channel.
Hi,
Please see my comments on the original version.
Regards,
Hans
On 03/13/2012 02:51 PM, Daniel P. Berrange wrote:
On Tue, Mar 13, 2012 at 01:40:12PM +, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"
A few functions have very large arrays declared on the stack.
Replace these wit
- Mensaje original -
> The first 14 patches in this series fix misc issues in the code
> which would otherwise trigger warnings. The last patch actually
> enables the warnings. It also enables some GCC runtime protection
> features, commonly used by distros in their packages
A lot of cha
Hi,
Series looks good. I've a few remarks on patch 14/15 though. Also
if you're enable ssp why not also add -DFORTIFY_SOURCE=2 ?
Regards,
Hans
On 03/13/2012 02:39 PM, Daniel P. Berrange wrote:
The current spice-gtk configure.ac script only enables
-Wall -Wno-sign-compare -Werror -Wno-depr
On Tue, Mar 13, 2012 at 02:56:07PM +0100, Hans de Goede wrote:
> Hi,
>
> On 03/13/2012 02:40 PM, Daniel P. Berrange wrote:
> >From: "Daniel P. Berrange"
> >
> >A few functions have very large arrays declared on the stack.
> >Replace these with heap allocations, to reduce risk of stack
> >overflows
On Tue, Mar 13, 2012 at 09:57:25AM -0400, Marc-André Lureau wrote:
>
>
> - Mensaje original -
> > The first 14 patches in this series fix misc issues in the code
> > which would otherwise trigger warnings. The last patch actually
> > enables the warnings. It also enables some GCC runtime
- Mensaje original -
> On Tue, Mar 13, 2012 at 09:57:25AM -0400, Marc-André Lureau wrote:
> >
> >
> > - Mensaje original -
> > > The first 14 patches in this series fix misc issues in the code
> > > which would otherwise trigger warnings. The last patch actually
> > > enables th
On Tue, Mar 13, 2012 at 03:00:04PM +0100, Hans de Goede wrote:
> Hi,
>
> Series looks good. I've a few remarks on patch 14/15 though. Also
> if you're enable ssp why not also add -DFORTIFY_SOURCE=2 ?
We do actually, but not via a compile flag. Instead it uses
AC_DEFINE(_FORTIFY_SOURCE, 2), which
Hi,
On 03/13/2012 03:23 PM, Daniel P. Berrange wrote:
On Tue, Mar 13, 2012 at 03:00:04PM +0100, Hans de Goede wrote:
Hi,
Series looks good. I've a few remarks on patch 14/15 though. Also
if you're enable ssp why not also add -DFORTIFY_SOURCE=2 ?
We do actually, but not via a compile flag. In
- Mensaje original -
> >
> >https://gitorious.org/~berrange/virt-tools/berrange-spice-gtk
> >
>
> Looks good, ack series.
+1
> Don't you have commit rights on spice-gtk yet? If not I think
> it is time you get them :) Marc-André, do you agree? If so
Yes, I do!
> one of us needs
Hi,
On 03/13/2012 03:40 PM, Marc-André Lureau wrote:
- Mensaje original -
https://gitorious.org/~berrange/virt-tools/berrange-spice-gtk
Looks good, ack series.
+1
Don't you have commit rights on spice-gtk yet? If not I think
it is time you get them :) Marc-André, do you a
On Tue, Mar 13, 2012 at 03:33:46PM +0100, Hans de Goede wrote:
> Hi,
>
> On 03/13/2012 03:23 PM, Daniel P. Berrange wrote:
> >On Tue, Mar 13, 2012 at 03:00:04PM +0100, Hans de Goede wrote:
> >>Hi,
> >>
> >>Series looks good. I've a few remarks on patch 14/15 though. Also
> >>if you're enable ssp w
On Tue, Mar 13, 2012 at 01:40:09PM +, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
These fixes apply to spice too, since common was copied out of it
(grumble).
> * common/quic.c, common/rop3.c, common/sw_canvas.c,
> gtk/spice-client-glib-usb-acl-helper.c: s/()/(void)/
> ---
>
On Tue, Mar 13, 2012 at 01:40:10PM +, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> A number of functions were used without prior declaration. In
> some cases this was due to missing include files. In other cases
> the functions should have just been static.
>
same comment as 1
On Tue, Mar 13, 2012 at 01:40:11PM +, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
This one too.
> * common/quic.h, common/rop3.h, common/sw_canvas.h: s/()/(void)/
> ---
> common/quic.h |2 +-
> common/rop3.h |2 +-
> common/sw_canvas.h |2 +-
> 3 files ch
If we run out of watches slots, we return NULL from watch_add, which
means that the other watch_foo functions may get called with a NULL
parameter, protect them against this.
Signed-off-by: Hans de Goede
---
server/red_worker.c | 15 +--
1 file changed, 13 insertions(+), 2 deletion
ping
On Fri, Mar 9, 2012 at 7:20 PM, Marc-André Lureau
wrote:
> Don't attempt to create an unused directory
> See also: https://bugzilla.redhat.com/show_bug.cgi?id=801871
> ---
> SpiceXPI/src/plugin/plugin.cpp | 5 -
> SpiceXPI/src/plugin/plugin.h | 1 -
> 2 files changed, 0 insertio
On Tue, Mar 13, 2012 at 05:39:51PM +0100, Marc-André Lureau wrote:
> ping
>
I'm just not sure about this - maybe someone who actually uses spice-xpi
can say something about it?
> On Fri, Mar 9, 2012 at 7:20 PM, Marc-André Lureau
> wrote:
> > Don't attempt to create an unused directory
> > See a
Ack.
On 03/13/2012 05:39 PM, Marc-André Lureau wrote:
ping
On Fri, Mar 9, 2012 at 7:20 PM, Marc-André Lureau
wrote:
Don't attempt to create an unused directory
See also: https://bugzilla.redhat.com/show_bug.cgi?id=801871
---
SpiceXPI/src/plugin/plugin.cpp |5 -
SpiceXPI/src/plugin
On Tue, Mar 13, 2012 at 04:42:06PM +0100, Hans de Goede wrote:
> If we run out of watches slots, we return NULL from watch_add, which
> means that the other watch_foo functions may get called with a NULL
> parameter, protect them against this.
ACK. Also, sounds horrible (the existing behavior, not
Daniel,
Here is a couple of patches that fixes deprecation warning/errors to
be applied on top of your series after review. They fix compilation
with mingw.
On Tue, Mar 13, 2012 at 3:46 PM, Daniel P. Berrange wrote:
> On Tue, Mar 13, 2012 at 03:33:46PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On
63 matches
Mail list logo
