Either would be fine with me. If you left in the verbose message then it
might be a bit confusing to any user that might see it.
On Fri, Jan 3, 2014 at 9:13 AM, Ralph Castain <r...@open-mpi.org> wrote:
> That would work. Alternatively, you could try just returning OPAL_SUCCESS
> in place of OPAL_ERR_NOT_AVAILABLE. This would still avoid opening the
> components for no reason (thus saving some memory) while not causing
> opal_init to abort.
>
>
> On Jan 3, 2014, at 3:19 AM, Adrian Reber <adr...@lisas.de> wrote:
>
> > So removing all output like in this patch would be ok?
> >
> > diff --git a/opal/mca/compress/base/compress_base_open.c
> b/opal/mca/compress/base/compress_base_open.c
> > index a09fe59..f487752 100644
> > --- a/opal/mca/compress/base/compress_base_open.c
> > +++ b/opal/mca/compress/base/compress_base_open.c
> > @@ -14,13 +14,9 @@
> >
> > #include "opal_config.h"
> >
> > -#include <string.h>
> > -#include "opal/mca/mca.h"
> > #include "opal/mca/base/base.h"
> > #include "opal/include/opal/constants.h"
> > -#include "opal/mca/compress/compress.h"
> > #include "opal/mca/compress/base/base.h"
> > -#include "opal/util/output.h"
> >
> > #include "opal/mca/compress/base/static-components.h"
> >
> > @@ -45,13 +41,6 @@ MCA_BASE_FRAMEWORK_DECLARE(opal, compress, NULL,
> opal_compress_base_register, op
> >
> > static int opal_compress_base_register (mca_base_register_flag_t flags)
> > {
> > - /* Compression currently only used with C/R */
> > - if( !opal_cr_is_enabled ) {
> > - opal_output_verbose(10,
> opal_compress_base_framework.framework_output,
> > - "compress:open: FT is not enabled,
> skipping!");
> > - return OPAL_ERR_NOT_AVAILABLE;
> > - }
> > -
> > return OPAL_SUCCESS;
> > }
> >
> > @@ -61,13 +50,6 @@ static int opal_compress_base_register
> (mca_base_register_flag_t flags)
> > */
> > int opal_compress_base_open(mca_base_open_flag_t flags)
> > {
> > - /* Compression currently only used with C/R */
> > - if( !opal_cr_is_enabled ) {
> > - opal_output_verbose(10,
> opal_compress_base_framework.framework_output,
> > - "compress:open: FT is not enabled,
> skipping!");
> > - return OPAL_SUCCESS;
> > - }
> > -
> > /* Open up all available components */
> > return mca_base_framework_components_open
> (&opal_compress_base_framework, flags);
> > }
> >
> >
> >
> > On Thu, Jan 02, 2014 at 12:32:32PM -0500, Josh Hursey wrote:
> >> I think the only reason I protected that framework is to reduce the
> >> overhead of an application using a build of Open MPI with CR support,
> but
> >> no enabling it at runtime. Nothing in the compress framework depends on
> the
> >> CR infrastructure (although the CR infrastructure can use the compress
> >> framework if the user chooses to). So I bet we can remove the protection
> >> altogether and be fine.
> >>
> >> So I think this patch is fine. I might also go as far as removing the
> 'if'
> >> block altogether as the protection should not been needed any longer.
> >>
> >> Thanks,
> >> Josh
> >>
> >>
> >>
> >> On Fri, Dec 27, 2013 at 3:46 PM, Adrian Reber <adr...@lisas.de> wrote:
> >>
> >>> Right now the C/R code fails because of a change introduced in
> >>> opal/mca/compress/base/compress_base_open.c in 2013 with commit
> >>>
> >>> git 734c724ff76d9bf814f3ab0396bcd9ee6fddcd1b
> >>> svn r28239
> >>>
> >>> Update OPAL frameworks to use the MCA framework system.
> >>>
> >>> This commit changed a lot but also the return value of the function
> from
> >>> OPAL_SUCCESS to OPAL_ERR_NOT_AVAILABLE. With following patch I can make
> >>> it work again:
> >>>
> >>> diff --git a/opal/mca/compress/base/compress_base_open.c
> >>> b/opal/mca/compress/base/compress_base_open.c
> >>> index a09fe59..69eabfa 100644
> >>> --- a/opal/mca/compress/base/compress_base_open.c
> >>> +++ b/opal/mca/compress/base/compress_base_open.c
> >>> @@ -45,11 +45,11 @@ MCA_BASE_FRAMEWORK_DECLARE(opal, compress, NULL,
> >>> opal_compress_base_register, op
> >>>
> >>> static int opal_compress_base_register (mca_base_register_flag_t flags)
> >>> {
> >>> /* Compression currently only used with C/R */
> >>> if( !opal_cr_is_enabled ) {
> >>> opal_output_verbose(10,
> >>> opal_compress_base_framework.framework_output,
> >>> "compress:open: FT is not enabled,
> >>> skipping!");
> >>> - return OPAL_ERR_NOT_AVAILABLE;
> >>> + return OPAL_SUCCESS;
> >>> }
> >>>
> >>> return OPAL_SUCCESS;
> >>>
> >>>
> >>> My question is if OPAL_ERR_NOT_AVAILABLE is indeed the correct return
> value
> >>> and the function calling opal_compress_base_register() should actually
> >>> handle OPAL_ERR_NOT_AVAILABLE or if returning OPAL_SUCCESS is still the
> >>> right
> >>> return value?
> >>>
> >>> Adrian
> >>> _______________________________________________
> >>> devel mailing list
> >>> de...@open-mpi.org
> >>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
> >>>
> >>
> >>
> >>
> >> --
> >> Joshua Hursey
> >> Assistant Professor of Computer Science
> >> University of Wisconsin-La Crosse
> >> http://cs.uwlax.edu/~jjhursey
> >
> >> _______________________________________________
> >> devel mailing list
> >> de...@open-mpi.org
> >> http://www.open-mpi.org/mailman/listinfo.cgi/devel
> > _______________________________________________
> > devel mailing list
> > de...@open-mpi.org
> > http://www.open-mpi.org/mailman/listinfo.cgi/devel
>
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>
--
Joshua Hursey
Assistant Professor of Computer Science
University of Wisconsin-La Crosse
http://cs.uwlax.edu/~jjhursey
