Re: review: Simple patch to remove the dmapi support from xfsdump

Bill Kendall
2006-08-04T14:25:44Z

I'm concerned about distributions inadvertently turning off dmapi
support in their xfsdump builds. An explicit ./configure switch
to disable dmapi support would be better, IMHO. But either way
it would be a build-time decision rather than a run-time decision.

xfsdump only uses one DMAPI call (dm_make_handle), so this could
easily be done with dlopen/dlsym, and this would eliminate the
dmapi requirement for those not using -a. dmapi would still be
a build requirement for xfsdump.

Ideally, hsmapi.c wouldn't even exist in xfsdump -- the current
implementation belongs in DMF. There should be a plugin mechanism
added to xfsdump to allow the loading libraries that implement
the API in hsmapi.c (though that interface needs some work first).

Bill

Russell Cattelan wrote:
> Since very few people run xfs with dmapi enabled it seem kind of silly
> to force the install of the dmapi utils/libraries just for xfs
> dump/restore.
>
> This patch stubs out the Hsm functions and adjusts the autoconf
> to not fail in the case of missing dmapi headers/libs.
>
> If somebody does trie to do a dump with the -a option a ERROR message
> is issued, the dump does continue? this might not be the desired behavior.
>
>
> ------------------------------------------------------------------------
>
> Index: xfs-cmds/xfsdump/aclocal.m4
> ===================================================================
> --- xfs-cmds.orig/xfsdump/aclocal.m4 2006-08-02 14:51:34.000000000 -0500
> +++ xfs-cmds/xfsdump/aclocal.m4 2006-08-02 14:58:11.250605250 -0500
> @@ -161,27 +161,31 @@ AC_DEFUN([AC_PACKAGE_NEED_XFS_DMAPI_H],
> [ AC_CHECK_HEADERS([xfs/dmapi.h])
> if test "$ac_cv_header_xfs_dmapi_h" != yes; then
> echo
> - echo 'FATAL ERROR: could not find a valid DMAPI library header.'
> - echo 'Install the data migration API (dmapi) development package.'
> + echo 'WARNING: could not find a valid DMAPI library header.'
> + echo 'xfsdump / xfsrestore will not support dmapi file systems.'
> echo 'Alternatively, run "make install-dev" from the dmapi source.'
> - exit 1
> fi
> ])
>
> AC_DEFUN([AC_PACKAGE_NEED_MAKEHANDLE_LIBDM],
> - [ AC_CHECK_LIB(dm, dm_make_handle,, [
> + [ AC_CHECK_LIB(dm, dm_make_handle,
> + [have_dmapi=true
> + libdm="-ldm"
> + test -f `pwd`/../dmapi/libdm/libdm.la && \
> + libdm="`pwd`/../dmapi/libdm/libdm.la"
> + test -f ${libexecdir}${libdirsuffix}/libdm.la && \
> + libdm="${libexecdir}${libdirsuffix}/libdm.la"
> + ],
> + [
> echo
> - echo 'FATAL ERROR: could not find a valid DMAPI base library.'
> - echo 'Install the data migration API (dmapi) library package.'
> + echo 'WARNING: could not find a valid DMAPI base library.'
> + echo 'xfsdump / xfsrestore will not support dmapi file systems.'
> echo 'Alternatively, run "make install" from the dmapi source.'
> - exit 1
> + have_dmapi=false
> + libdm = ""
> ])
> - libdm="-ldm"
> - test -f `pwd`/../dmapi/libdm/libdm.la && \
> - libdm="`pwd`/../dmapi/libdm/libdm.la"
> - test -f ${libexecdir}${libdirsuffix}/libdm.la && \
> - libdm="${libexecdir}${libdirsuffix}/libdm.la"
> AC_SUBST(libdm)
> + AC_SUBST(have_dmapi)
> ])
>
> #
> Index: xfs-cmds/xfsdump/common/hsmapi_noop.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ xfs-cmds/xfsdump/common/hsmapi_noop.c 2006-08-02 15:38:39.046333000 -0500
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright (c) 2000-2004 Silicon Graphics, Inc.
> + * All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation,
> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <xfs/xfs.h>
> +#include <attr/attributes.h>
> +#include <xfs/handle.h>
> +
> +#include "hsmapi.h"
> +#include "mlog.h"
> +#include "exit.h"
> +
> +typedef struct {
> +} dmf_fs_ctxt_t;
> +
> +extern hsm_fs_ctxt_t *
> +HsmInitFsysContext(
> +const char *mountpoint,
> + int dumpversion)
> +{
> + mlog( MLOG_NORMAL | MLOG_ERROR, _(
> + "-%c Dumping of DMF dualstate file not supported in this version\n"));
> + return NULL;
> +}
> +
> +extern void
> +HsmDeleteFsysContext(
> + hsm_fs_ctxt_t *contextp)
> +{
> +}
> +
> +extern int
> +HsmEstimateFileSpace(
> + hsm_fs_ctxt_t *contextp,
> +const xfs_bstat_t *statp,
> + off64_t *bytes)
> +{
> + return 0;
> +}
> +
> +extern int
> +HsmEstimateFileOffset(
> + hsm_fs_ctxt_t *contextp,
> +const xfs_bstat_t *statp,
> + off64_t bytecount,
> + off64_t *byteoffset)
> +{
> + return 0;
> +}
> +
> +extern hsm_f_ctxt_t *
> +HsmAllocateFileContext(
> + hsm_fs_ctxt_t *contextp)
> +{
> + return NULL;
> +}
> +
> +extern void
> +HsmDeleteFileContext(
> + hsm_f_ctxt_t *contextp)
> +{
> +}
> +
> +extern int
> +HsmInitFileContext(
> + hsm_f_ctxt_t *contextp,
> +const xfs_bstat_t *statp)
> +{
> + return 0;
> +}
> +
> +extern int
> +HsmModifyInode(
> + hsm_f_ctxt_t *contextp,
> + xfs_bstat_t *statp)
> +{
> + return 0;
> +}
> +
> +extern int
> +HsmModifyExtentMap(
> + hsm_f_ctxt_t *contextp,
> + struct getbmapx *bmap)
> +{
> + return 0;
> +}
> +
> +extern int
> +HsmFilterExistingAttribute(
> + hsm_f_ctxt_t *hsm_f_ctxtp,
> +const char *namep, /* attribute name */
> + u_int32_t valuesz, /* value size */
> + int flag,
> + int *skip_entry)
> +{
> + return 0;
> +}
> +
> +extern int
> +HsmAddNewAttribute(
> + hsm_f_ctxt_t *hsm_f_ctxtp,
> + int cursor,
> + int flag,
> + char **namepp, /* pointer to new attribute name */
> + char **valuepp, /* pointer to its value */
> + u_int32_t *valueszp) /* pointer to the value size */
> +{
> + return 0;
> +}
> +
> +extern void
> +HsmBeginRestoreFile(
> + bstat_t *bstatp,
> + int fd,
> + int *hsm_flagp)
> +{
> +}
> +
> +extern void
> +HsmRestoreAttribute(
> + int flag, /* ext attr flags */
> + char *namep, /* pointer to new attribute name */
> + int *hsm_flagp)
> +{
> +}
> +
> +extern void
> +HsmEndRestoreFile(
> + char *path,
> + int fd,
> + int *hsm_flagp)
> +{
> +}
> Index: xfs-cmds/xfsdump/dump/Makefile
> ===================================================================
> --- xfs-cmds.orig/xfsdump/dump/Makefile 2006-08-02 14:51:34.000000000 -0500
> +++ xfs-cmds/xfsdump/dump/Makefile 2006-08-02 14:52:15.076345750 -0500
> @@ -57,7 +57,6 @@ COMMON = \
> fs.c \
> getdents.c \
> global.c \
> - hsmapi.c \
> lock.c \
> main.c \
> mlog.c \
> @@ -69,6 +68,12 @@ COMMON = \
> util.c \
> sproc.c
>
> +ifeq ($(HAVE_DMAPI), true)
> + COMMON += hsmapi.c
> +else
> + COMMON += hsmapi_noop.c
> +endif
> +
> LOCALS = \
> content.c \
> inomap.c \
> Index: xfs-cmds/xfsdump/include/builddefs.in
> ===================================================================
> --- xfs-cmds.orig/xfsdump/include/builddefs.in 2006-08-02 14:51:34.000000000 -0500
> +++ xfs-cmds/xfsdump/include/builddefs.in 2006-08-02 14:52:15.080346000 -0500
> @@ -66,6 +66,8 @@ ENABLE_GETTEXT = @enable_gettext@
>
> HAVE_ZIPPED_MANPAGES = @have_zipped_manpages@
>
> +HAVE_DMAPI = @have_dmapi@
> +
> LCFLAGS += -DXFS_BIG_FILES=1 -DXFS_BIG_FILESYSTEMS=1
>
> ifeq ($(PKG_PLATFORM),linux)
> Index: xfs-cmds/xfsdump/restore/Makefile
> ===================================================================
> --- xfs-cmds.orig/xfsdump/restore/Makefile 2006-08-02 14:51:34.000000000 -0500
> +++ xfs-cmds/xfsdump/restore/Makefile 2006-08-02 14:52:15.080346000 -0500
> @@ -55,7 +55,6 @@ COMMON = \
> fs.c \
> getdents.c \
> global.c \
> - hsmapi.c \
> lock.c \
> main.c \
> mlog.c \
> @@ -67,6 +66,12 @@ COMMON = \
> stream.c \
> util.c
>
> +ifeq ($(HAVE_DMAPI), true)
> + COMMON += hsmapi.c
> +else
> + COMMON += hsmapi_noop.c
> +endif
> +
> LOCALS = \
> bag.c \
> content.c \