new device-mapper required to build latest lvm from cvs

new device-mapper required to build latest lvm from cvs

[Jim Meyering]

FYI, with the change I've just committed,

  http://sources.redhat.com/ml/lvm2-cvs/2007-07/msg00035.html

you now need a device-mapper library and headers built from
today's cvs sources.  Otherwise, you'll get compile warnings
and eventually a link error complaining about missing dm_fclose.

Re: [lvm-devel] new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]

Hi,

Jim Meyering wrote:
> FYI, with the change I've just committed,
> 
>   http://sources.redhat.com/ml/lvm2-cvs/2007-07/msg00035.html
> 
> you now need a device-mapper library and headers built from
> today's cvs sources.  Otherwise, you'll get compile warnings
> and eventually a link error complaining about missing dm_fclose.

This change introduces an error for static build.

/usr/local/lib/libdevmapper.a(libdm-file.o): In function `create_dir':
/work/device-mapper/lib/libdm-file.c:59: multiple definition of `create_dir'
../lib/liblvm.a(lvm-file.o):/work/LVM2/lib/misc/lvm-file.c:177: first defined here
/usr/bin/ld: Warning: size of symbol `create_dir' changed from 176 in ../lib/liblvm.a(lvm-file.o) to 187 in /usr/local/lib/libdevmapper.a(libdm-file.o)

lvm_fclose() calls dm_fclose(), which cause the linker to
bring libdm-file.o in liblvm.a. Then there happens a conflict
of create_dir().

Attached is a patch for device-mapper to rename the function
and export it. It alone fixes the build error.

A patch for LVM2 to use the exported function will follow.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America

[-- Attachment #2: dm-dm_create_dir.patch --]
[-- Type: text/x-patch, Size: 2195 bytes --]

lvm_fclose() calls dm_fclose(), which cause the static linker to
bring libdm-file.o in liblvm.a. Then there happens a conflict
of create_dir().

Fix the following build error:

/usr/local/lib/libdevmapper.a(libdm-file.o): In function `create_dir':
/work/device-mapper/lib/libdm-file.c:59: multiple definition of `create_dir'
../lib/liblvm.a(lvm-file.o):/work/LVM2/lib/misc/lvm-file.c:177: first defined here
/usr/bin/ld: Warning: size of symbol `create_dir' changed from 176 in ../lib/liblvm.a(lvm-file.o) to 187 in /usr/local/lib/libdevmapper.a(libdm-file.o)

Index: device-mapper.work/lib/.exported_symbols
===================================================================
--- device-mapper.work.orig/lib/.exported_symbols
+++ device-mapper.work/lib/.exported_symbols
@@ -1,6 +1,7 @@
 dm_lib_release
 dm_lib_exit
 dm_driver_version
+dm_create_dir
 dm_fclose
 dm_get_library_version
 dm_log
Index: device-mapper.work/lib/libdevmapper.h
===================================================================
--- device-mapper.work.orig/lib/libdevmapper.h
+++ device-mapper.work/lib/libdevmapper.h
@@ -629,6 +629,12 @@ char *dm_basename(const char *path);
  **************************/
 
 /*
+ * Create a directory (with parent directories if necessary).
+ * Returns 1 on success, 0 on failure.
+ */
+int dm_create_dir(const char *dir);
+
+/*
  * Close a stream, with nicer error checking than fclose's.
  * Derived from gnulib's close-stream.c.
  *
Index: device-mapper.work/lib/libdm-file.c
===================================================================
--- device-mapper.work.orig/lib/libdm-file.c
+++ device-mapper.work/lib/libdm-file.c
@@ -55,7 +55,7 @@ out:
 	return r;
 }
 
-int create_dir(const char *dir)
+int dm_create_dir(const char *dir)
 {
 	struct stat info;
 
Index: device-mapper.work/lib/ioctl/libdm-iface.c
===================================================================
--- device-mapper.work.orig/lib/ioctl/libdm-iface.c
+++ device-mapper.work/lib/ioctl/libdm-iface.c
@@ -228,7 +228,7 @@ static int _create_control(const char *c
 		return 0;
 
 	old_umask = umask(0022);
-	ret = create_dir(dm_dir());
+	ret = dm_create_dir(dm_dir());
 	umask(old_umask);
 
 	if (!ret)

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

Hi,

Jim Meyering wrote:
> FYI, with the change I've just committed,
> 
>   http://sources.redhat.com/ml/lvm2-cvs/2007-07/msg00035.html
> 
> you now need a device-mapper library and headers built from
> today's cvs sources.  Otherwise, you'll get compile warnings
> and eventually a link error complaining about missing dm_fclose.

This change introduces an error for static build.

/usr/local/lib/libdevmapper.a(libdm-file.o): In function `create_dir':
/work/device-mapper/lib/libdm-file.c:59: multiple definition of `create_dir'
../lib/liblvm.a(lvm-file.o):/work/LVM2/lib/misc/lvm-file.c:177: first defined here
/usr/bin/ld: Warning: size of symbol `create_dir' changed from 176 in ../lib/liblvm.a(lvm-file.o) to 187 in /usr/local/lib/libdevmapper.a(libdm-file.o)

lvm_fclose() calls dm_fclose(), which cause the linker to
bring libdm-file.o in liblvm.a. Then there happens a conflict
of create_dir().

Attached is a patch for device-mapper to rename the function
and export it. It alone fixes the build error.

A patch for LVM2 to use the exported function will follow.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dm-dm_create_dir.patch
Type: text/x-patch
Size: 2195 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20070725/eaaba228/attachment.bin>

Re: [lvm-devel] new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

> lvm_fclose() calls dm_fclose(), which cause the linker to
> bring libdm-file.o in liblvm.a. Then there happens a conflict
> of create_dir().
> 
> Attached is a patch for device-mapper to rename the function
> and export it. It alone fixes the build error.
> 
> A patch for LVM2 to use the exported function will follow.

This is the patch for LVM2.
LVM2's create_dir() is left there for the case LVM2 specific
something could be put in future.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America

[-- Attachment #2: lvm2-use-dm_create_dir.patch --]
[-- Type: text/x-patch, Size: 1275 bytes --]

Use dm_create_dir() instead of its own.

Index: LVM2.work/lib/misc/lvm-file.c
===================================================================
--- LVM2.work.orig/lib/misc/lvm-file.c
+++ LVM2.work/lib/misc/lvm-file.c
@@ -140,54 +140,9 @@ int dir_exists(const char *path)
 	return 1;
 }
 
-static int _create_dir_recursive(const char *dir)
-{
-	char *orig, *s;
-	int rc;
-
-	log_verbose("Creating directory \"%s\"", dir);
-	/* Create parent directories */
-	orig = s = dm_strdup(dir);
-	while ((s = strchr(s, '/')) != NULL) {
-		*s = '\0';
-		if (*orig) {
-			rc = mkdir(orig, 0777);
-			if (rc < 0 && errno != EEXIST) {
-				if (errno != EROFS)
-					log_sys_error("mkdir", orig);
-				dm_free(orig);
-				return 0;
-			}
-		}
-		*s++ = '/';
-	}
-	dm_free(orig);
-
-	/* Create final directory */
-	rc = mkdir(dir, 0777);
-	if (rc < 0 && errno != EEXIST) {
-		if (errno != EROFS)
-			log_sys_error("mkdir", dir);
-		return 0;
-	}
-	return 1;
-}
-
 int create_dir(const char *dir)
 {
-	struct stat info;
-
-	if (!*dir)
-		return 1;
-
-	if (stat(dir, &info) < 0)
-		return _create_dir_recursive(dir);
-
-	if (S_ISDIR(info.st_mode))
-		return 1;
-
-	log_error("Directory \"%s\" not found", dir);
-	return 0;
+	return dm_create_dir(dir);
 }
 
 int is_empty_dir(const char *dir)

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

> lvm_fclose() calls dm_fclose(), which cause the linker to
> bring libdm-file.o in liblvm.a. Then there happens a conflict
> of create_dir().
> 
> Attached is a patch for device-mapper to rename the function
> and export it. It alone fixes the build error.
> 
> A patch for LVM2 to use the exported function will follow.

This is the patch for LVM2.
LVM2's create_dir() is left there for the case LVM2 specific
something could be put in future.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvm2-use-dm_create_dir.patch
Type: text/x-patch
Size: 1275 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20070725/7c07d538/attachment.bin>

new device-mapper required to build latest lvm from cvs

[Alasdair G Kergon]

On Wed, Jul 25, 2007 at 04:01:13PM -0400, Jun'ichi Nomura wrote:
> This is the patch for LVM2.
> LVM2's create_dir() is left there for the case LVM2 specific
> something could be put in future.

Don't bother: if the functions are identical today, remove the lvm2
wrapper and have everything call the dm version directly.
 
Alasdair
-- 
agk at redhat.com

new device-mapper required to build latest lvm from cvs

[Alasdair G Kergon]

On Wed, Jul 25, 2007 at 09:10:05PM +0100, Alasdair G Kergon wrote:
> Don't bother: if the functions are identical today, remove the lvm2
> wrapper and have everything call the dm version directly.

The functions are not quite identical:-)
  1 - a log_sys_error() cleanup missing
  2 - error suppression for read-only filesystems
  
Alasdair
-- 
agk at redhat.com

new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

Alasdair G Kergon wrote:
> On Wed, Jul 25, 2007 at 09:10:05PM +0100, Alasdair G Kergon wrote:
>> Don't bother: if the functions are identical today, remove the lvm2
>> wrapper and have everything call the dm version directly.
> 
> The functions are not quite identical:-)
>   1 - a log_sys_error() cleanup missing

I thought they are identical after the macro is expanded.
Should I use log_sys_error() in libdevmapper, too?

>   2 - error suppression for read-only filesystems

It can be brought into dm_create_dir() or we can move logging
to the outside of dm_create_dir() and let callers do it.
Which way do you think is better?

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America

new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

Jun'ichi Nomura wrote:
> Alasdair G Kergon wrote:
>> On Wed, Jul 25, 2007 at 09:10:05PM +0100, Alasdair G Kergon wrote:
>>> Don't bother: if the functions are identical today, remove the lvm2
>>> wrapper and have everything call the dm version directly.
>> The functions are not quite identical:-)
>>   1 - a log_sys_error() cleanup missing
> 
> I thought they are identical after the macro is expanded.
> Should I use log_sys_error() in libdevmapper, too?

OK, this patch copies log_sys_error() macro from LVM2
and let libdevmapper use it.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dm-import-lvm2-log_sys-macros.patch
Type: text/x-patch
Size: 8082 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20070726/ac8de832/attachment.bin>

new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

Jun'ichi Nomura wrote:
> Jun'ichi Nomura wrote:
>> Alasdair G Kergon wrote:
>>> On Wed, Jul 25, 2007 at 09:10:05PM +0100, Alasdair G Kergon wrote:
>>>> Don't bother: if the functions are identical today, remove the lvm2
>>>> wrapper and have everything call the dm version directly.
>>> The functions are not quite identical:-)
>>>   1 - a log_sys_error() cleanup missing
>> I thought they are identical after the macro is expanded.
>> Should I use log_sys_error() in libdevmapper, too?
> 
> OK, this patch copies log_sys_error() macro from LVM2
> and let libdevmapper use it.

Sorry there was a typo in the patch. ("fscanf" -> "scanf")
Attached is a revised patch.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dm-import-lvm2-log_sys-macros.patch
Type: text/x-patch
Size: 8084 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20070726/4f5bfd38/attachment.bin>

new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

Jun'ichi Nomura wrote:
> Alasdair G Kergon wrote:
>> The functions are not quite identical:-)
..
>>   2 - error suppression for read-only filesystems
> 
> It can be brought into dm_create_dir() or we can move logging
> to the outside of dm_create_dir() and let callers do it.
> Which way do you think is better?

And this patch adds EROFS check to libdevmapper version of
_create_dir_recursive() so that they are equivalent.
(The patch is applicable after the log_sys_* macro patch.)

For really making them identical, I think LVM2 should call
dm_init_log() with its own log function.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dm-dm_create_dir-EROFS-check.patch
Type: text/x-patch
Size: 860 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20070726/7a2a4544/attachment.bin>

new device-mapper required to build latest lvm from cvs

[Jun'ichi Nomura]

Alasdair G Kergon wrote:
> On Wed, Jul 25, 2007 at 04:01:13PM -0400, Jun'ichi Nomura wrote:
>> This is the patch for LVM2.
>> LVM2's create_dir() is left there for the case LVM2 specific
>> something could be put in future.
> 
> Don't bother: if the functions are identical today, remove the lvm2
> wrapper and have everything call the dm version directly.

OK, then this is a revised version of the patch.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvm2-use-dm_create_dir.patch
Type: text/x-patch
Size: 4183 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20070725/3ae5cd4a/attachment.bin>

Re: [lvm-devel] new device-mapper required to build latest lvm from cvs

[Jim Meyering]

"Jun'ichi Nomura" <j-nomura@ce.jp.nec.com> wrote:
> Jim Meyering wrote:
>> FYI, with the change I've just committed,
>>
>>   http://sources.redhat.com/ml/lvm2-cvs/2007-07/msg00035.html
>>
>> you now need a device-mapper library and headers built from
>> today's cvs sources.  Otherwise, you'll get compile warnings
>> and eventually a link error complaining about missing dm_fclose.
>
> This change introduces an error for static build.

Thank you for the report and the patches.
I've applied four of your patches, three for device-mapper
and one for lvm2:

dm
  Export dm_create_dir (was create_dir) to help fix LVM2 link error
    http://git.et.redhat.com/?p=device-mapper.git;a=commitdiff;h=8748390a79f
  Don't log mkdir fail-with-EROFS, to make create_dir equiv to that in LVM2
    http://git.et.redhat.com/?p=device-mapper.git;a=commitdiff;h=81d93dd4717
  Introduce and use log_sys_* macros from LVM2
    http://git.et.redhat.com/?p=device-mapper.git;a=commitdiff;h=5edca96def8

lvm2
  Remove create_dir function; use now-equivalent dm_create_dir instead
    http://git.et.redhat.com/?p=lvm2.git;a=commitdiff;h=7d85f48b35b7

new device-mapper required to build latest lvm from cvs

[Jim Meyering]

"Jun'ichi Nomura" <j-nomura@ce.jp.nec.com> wrote:
> Jim Meyering wrote:
>> FYI, with the change I've just committed,
>>
>>   http://sources.redhat.com/ml/lvm2-cvs/2007-07/msg00035.html
>>
>> you now need a device-mapper library and headers built from
>> today's cvs sources.  Otherwise, you'll get compile warnings
>> and eventually a link error complaining about missing dm_fclose.
>
> This change introduces an error for static build.

Thank you for the report and the patches.
I've applied four of your patches, three for device-mapper
and one for lvm2:

dm
  Export dm_create_dir (was create_dir) to help fix LVM2 link error
    http://git.et.redhat.com/?p=device-mapper.git;a=commitdiff;h=8748390a79f
  Don't log mkdir fail-with-EROFS, to make create_dir equiv to that in LVM2
    http://git.et.redhat.com/?p=device-mapper.git;a=commitdiff;h=81d93dd4717
  Introduce and use log_sys_* macros from LVM2
    http://git.et.redhat.com/?p=device-mapper.git;a=commitdiff;h=5edca96def8

lvm2
  Remove create_dir function; use now-equivalent dm_create_dir instead
    http://git.et.redhat.com/?p=lvm2.git;a=commitdiff;h=7d85f48b35b7