Re: [Xen-devel] [PATCH] Play with spice for xen-upstream-qemu on upstream Xen

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

ZhouPeng writes ("Re: [Xen-devel] [PATCH] Play with spice for
xen-upstream-qemu on upstream Xen"):
> [Ian Jackson:]
> > Did you add the ": " after testing ?  If so then perhaps the existing
> > logging functions are wrong.
>
> The output is like this,
> libxl: something : at least one of the spiceport ....

That's strange.

> It is
> void libxl__logv(...)
>     libxl__logv
>         xtl_log(ctx->lg, msglevel, errnoval, "libxl",
>                 "%s%s%s%s" "%s",
>                 fileline, func&&file?":":"", func?func:"",
> func||file?" ":"", // here

Yes, I think this is arguably wrong.

> output the msg.
> It use a space not ': ' between func name and log msg.
> So no bug
> But I feel the format like below is more clear
> void libxl__logv(...)
>     libxl__logv
>         xtl_log(ctx->lg, msglevel, errnoval, "libxl",
>                 "%s%s%s%s" "%s",
>                 fileline, func&&file?":":"", func?func:"",
> func||file?": ":"",   // here
> 
> If you reply to agree to use ': ' instead of ' ',
> I will send a little patch for this.

Yes, I agree.  I will make this change myself as it's just one
character :-).  Thanks for digging.

> Any way, I agree with you to  trim the header ':' like below
> ": at least one of the spiceport or tls_port must be provided" to
> "at least one of the spiceport or tls_port must be provided"

Right, that's what I meant.

> > Secondly, your patch has a lot of rather long lines in new code.  Can
> > you please try to keep your lines down to 75 characters (or 80 if you
> > absolutely must) ?
>
> Fixed in this patch.

Good, thanks.

> There are many lines up to 80 characters in libxl.idl
> and even in libxl_dm.c, that's why I turn a blind eye
> to libxl.idl in my last patch.

Right, yes, that's fine.  Well it would be nicer if libxl.idl were
narrower but I don't propose to argue about that right now :-).

I have applied your patch.  Thanks for your contribution.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel