Re: [PATCH spice-server v2] gstreamer: Check if ORC library can work

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

 



> 
> On Fri, Aug 25, 2017 at 10:33:28AM -0400, Frediano Ziglio wrote:
> > > 
> > > On Fri, Aug 18, 2017 at 12:32:12PM +0100, Frediano Ziglio wrote:
> > > > ORC lirabry is used internally by GStreamer to generate code
> > > > dynamically.
> > > 
> > > "The ORC library"
> > > 
> > > > If ORC cannot allocate executable memory the failure cause
> > > > an abort(3) to be called.
> > > 
> > > "memory, the failure causes"
> > > 
> > > > This happens on some SELinux configuration that disable executable
> > > > memory allocation (execmem boolean).
> > > 
> > > Either "SELinux configurations" or "that disables"
> > > 
> > > > Check that ORC could work before attempting to use GStreamer to
> > > > avoid crashes. The log will report an error.
> > > 

Maybe I should extend the "The log will report an error." with a

"While this check is done ORC library output an error which
will be well visible in Qemu output."

> > > > 
> > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > > ---
> > > >  configure.ac               |  6 ++++++
> > > >  server/Makefile.am         |  2 ++
> > > >  server/gstreamer-encoder.c | 30 ++++++++++++++++++++++++++++++
> > > >  3 files changed, 38 insertions(+)
> > > > 
> > > > Changes since v1:
> > > > - rename orc_cached to orc_dynamic_code_ok.
> > > > 
> > > > diff --git a/configure.ac b/configure.ac
> > > > index e1e74862..4bdf8588 100644
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -116,6 +116,12 @@ AS_IF([test x"$missing_gstreamer_elements" =
> > > > xyes],
> > > >      [SPICE_WARNING([The GStreamer video encoder can be built but may
> > > >      not
> > > >      work.])
> > > >  ])
> > > >  
> > > > +if test "x$have_gstreamer_0_10" = "xyes" -o "x$have_gstreamer_1_0" =
> > > > "xyes"; then
> > > > +    PKG_CHECK_MODULES(ORC, orc-0.4)
> > > > +    AC_SUBST(ORC_CFLAGS)
> > > > +    AC_SUBST(ORC_LIBS)
> > > > +fi
> > > > +
> > > >  AC_ARG_ENABLE([automated_tests],
> > > >                AS_HELP_STRING([--enable-automated-tests], [Enable
> > > >                automated
> > > >                tests using spicy-screenshot (part of spice-gtk)]),,
> > > >                [enable_automated_tests="no"])
> > > > diff --git a/server/Makefile.am b/server/Makefile.am
> > > > index ef8d31fc..5d5590af 100644
> > > > --- a/server/Makefile.am
> > > > +++ b/server/Makefile.am
> > > > @@ -20,6 +20,7 @@ AM_CPPFLAGS =					\
> > > >  	$(SSL_CFLAGS)				\
> > > >  	$(VISIBILITY_HIDDEN_CFLAGS)		\
> > > >  	$(WARN_CFLAGS)				\
> > > > +	$(ORC_CFLAGS)				\
> > > >  	$(NULL)
> > > >  
> > > >  noinst_LTLIBRARIES = libserver.la
> > > > @@ -54,6 +55,7 @@ libserver_la_LIBADD =							\
> > > >  	$(SSL_LIBS)							\
> > > >  	$(Z_LIBS)							\
> > > >  	$(SPICE_NONPKGCONFIG_LIBS)					\
> > > > +	$(ORC_LIBS)							\
> > > >  	$(NULL)
> > > >  
> > > >  libspice_serverincludedir = $(includedir)/spice-server
> > > > diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> > > > index bb4f27ba..2a39154f 100644
> > > > --- a/server/gstreamer-encoder.c
> > > > +++ b/server/gstreamer-encoder.c
> > > > @@ -27,6 +27,7 @@
> > > >  #include <gst/app/gstappsrc.h>
> > > >  #include <gst/app/gstappsink.h>
> > > >  #include <gst/video/video.h>
> > > > +#include <orc/orcprogram.h>
> > > >  
> > > >  #include "red-common.h"
> > > >  #include "video-encoder.h"
> > > > @@ -1702,6 +1703,30 @@ static void
> > > > spice_gst_encoder_get_stats(VideoEncoder
> > > > *video_encoder,
> > > >      }
> > > >  }
> > > >  
> > > > +/* Check if ORC library can work.
> > > > + * ORC library is used quite extensively by GStreamer
> > > > + * to generate code dynamically. If ORC cannot work GStreamer
> > > 
> > > "cannot work, GStreamer"
> > > 
> > > > + * will abort(3) the entire process.
> > > > + */
> > > > +static bool orc_check(void)
> > > > +{
> > > > +    static bool orc_checked = false;
> > > > +    static bool orc_dynamic_code_ok = false;
> > > > +
> > > > +    if (SPICE_UNLIKELY(!orc_checked)) {
> > > > +        OrcCode *code = orc_code_new();
> > > > +        if (code) {
> > > > +            /* allocating 0 byte for code make the function not crash
> > > 
> > > "makes"
> > > 
> > > > +             * but doing all initializations and checks */
> > > 
> > > I would use "but it does all the initializations and checks"
> > > 
> > > Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> > > 
> > 
> > I'll do the changes.
> > 
> > I just noted that if the check fails is quite silence.
> > Maybe the first time we discover the issue we should log
> > something?
> 
> Ah, wanted to ask about that since you mention logging in the commit
> log, and then forgot :( Yeah, a log would be nice.
> 
> Christophe
> 

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




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