Re: SPICE logging facilities

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

 



Hi,

On Wed, Jul 04, 2018 at 11:21:56AM +0200, Christophe Fergeau wrote:
> On Tue, Jul 03, 2018 at 05:10:23PM +0200, Christophe de Dinechin wrote:
> > There is a lot of debate, distributed across a large number of patches, regarding similar SPICE and glib facilities. For a number of things, there are two sets of parallel APIs with slightly different behaviour. This is undesirable, as it introduces confusion.
> > 
> > Pros: this lets us customize the behavior for SPICE
> > 
> > Cons: the SPICE macros are less documented, and not obvious to tell how they differ from the glib counterpart.
> > 
> > 
> > As I wrote in a response to a patch comment, I personally value
> > consistency relatively high, so if a file currently uses
> > spice_return_if_fail, that’s what I will use in that file. While I may
> > agree that using g_return_if_fail in general could be preferable to
> > spice_return_if_fail, I would like
> > 
> > a) to avoid having repeated and unproductive comments about “why not
> > use the glib version” for every patch that touches one of these
> > macros.
> > 
> > b) to have a clearer understanding of what the benefits of the SPICE
> > and glib variants are (I somewhat understand the difference, my
> > question is more about whether there is real value add to the SPICE
> > variant or not).
> > 
> > 
> > I had started explaining my preferences here, but I realize it’s
> > probably better to gather everyone’s opinion first. Please share your
> > preference, e.g. should we switch to glib wholesale, piecemeal, not at
> > all, on a per-macro basis, etc, and the rationale behind your
> > preference. Meanwhile, I’ll do some additional experiments to solidify
> > or change my own preference.
> 
> Initially, spice-server/spice-common had no glib dependency, so
> spice_warning & friends made sense. Then they started to depend
> on glib, so some of the stuff they contained became redundant
> with what glib provides. Then spice_log was patched to go
> through g_log, while retaining its slight behaviourial
> differences, mostly for backward compatibility. At this point,
> the plan was to slowly get rid of spice_* and to move to glib
> logging. Which is why I push for using glib logging when a
> patch tries to use the deprecated spice logging API.  If the
> inconsistency within a file is a problem, then the file can be
> switched to the new API first, and the new code can use the
> glib API and everything is consistent. Keeping using spice
> logging API for the sake of consistency is not going to make
> the situation better.
> 
> Christophe

I do like the approach of sanitizing the file(s) that the
following up patch will touch. Should be easier to test and
review but would take several patches to sanitize the code, etc.

Now might be a good time to start with this as we just released
spice-gtk and a release of spice is not too far either.

And, yes, I'm in favor of removing spice related functions/macros
that there is a glib equivalent.

Cheers,
        toso

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]