Distributed Replicated Block Device (DRBD) development
 help / color / mirror / Atom feed
* [Drbd-dev] [PATCH] drbdmeta: Fix drbdmeta to support old kernels
@ 2017-08-17  1:57 Artem Pogartsev
  2017-08-17  9:59 ` Lars Ellenberg
  0 siblings, 1 reply; 2+ messages in thread
From: Artem Pogartsev @ 2017-08-17  1:57 UTC (permalink / raw)
  To: drbd-dev

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

Hello,

RedHat 6.9
kernel-2.6.32-431.11.2
drbd-utils-9.0.0
drbd-8.4.10

Problem:

drbdmeta 0 v08 /dev/sda internal create-md
initializing activity log
initializing bitmap (30524 KB) to all zero
ioctl(/dev/sda) failed: Invalid argument

Reason:

Wrong check of the return error code into zeroout_bitmap function 
(user/shared/drbdmeta.c):

err = ioctl(cfg->md_fd, BLKZEROOUT, &range);
if (!err)
         return;
if (errno == ENOTTY) {
         do pwrite stuff;
}

We must check if errno == EINVAL (request code or argp is not valid). We 
will get ENOTTY if the block device doesnt has ioctl into fops structure 
(look at the __blkdev_driver_ioctl into block/ioctl.c) - not out case. 
And we will get EINVAL if the request code is wrong for this type of the 
device (old kernels doesnt have BLKZEROOUT) - our case.

Possible patch is attached.

But maybe better change logic and make ioctl with BLKZEROOUT only if 
BLKZEROOUT was defined in kernel headers (if we dont have BLKZEROOUT 
macro in kernel tree then 100% ioctl with _IO(0x12,127) will fail).


[-- Attachment #2: drbd-utils-9.0.0-blkzeroout.patch --]
[-- Type: text/plain, Size: 462 bytes --]

diff -Nru a/user/shared/drbdmeta.c b/user/shared/drbdmeta.c
--- a/user/shared/drbdmeta.c	2017-04-20 10:35:02.000000000 +0300
+++ b/user/shared/drbdmeta.c	2017-08-17 04:26:35.509000784 +0300
@@ -1716,7 +1716,7 @@
 	if (!err)
 		return;
 
-	if (errno == ENOTTY) {
+	if (errno == EINVAL) {
 		/* need to sector-align this for O_DIRECT.
 		 * "sector" here means hard-sect size, which may be != 512.
 		 * Note that even though ALIGN does round up, for sector sizes

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Drbd-dev] [PATCH] drbdmeta: Fix drbdmeta to support old kernels
  2017-08-17  1:57 [Drbd-dev] [PATCH] drbdmeta: Fix drbdmeta to support old kernels Artem Pogartsev
@ 2017-08-17  9:59 ` Lars Ellenberg
  0 siblings, 0 replies; 2+ messages in thread
From: Lars Ellenberg @ 2017-08-17  9:59 UTC (permalink / raw)
  To: drbd-dev

On Thu, Aug 17, 2017 at 04:57:12AM +0300, Artem Pogartsev wrote:
> Hello,
> 
> RedHat 6.9
> kernel-2.6.32-431.11.2
> drbd-utils-9.0.0
> drbd-8.4.10
> 
> Problem:
> 
> drbdmeta 0 v08 /dev/sda internal create-md
> initializing activity log
> initializing bitmap (30524 KB) to all zero
> ioctl(/dev/sda) failed: Invalid argument

Thanks for pointing this out.

I'm about to commit this instead:

    drbdmeta compat: fixup fallback path for BLKZEROOUT for older kernels
    
    Based on a suggestion on drbd-dev by
     From: Artem Pogartsev <artem.pogartsev@activecloud.com>
     Date: Thu, 17 Aug 2017 04:57:12 +0300
     To: drbd-dev@lists.linbit.com
     Subject: [PATCH] drbdmeta: Fix drbdmeta to support old kernels
     Message-ID: <a1c1b596-8259-a2cf-3b1b-6461761bbca4@activecloud.com>
    
    Though I chose to always execute the fallback code
    if the ioctl BLKZEROOUT fails, regardless of errno.
    
    As a side note,
    just because the define BLKZEROOUT is not in the (userland) headers
    (yet) does not necessarily mean the kernel does not know it.

diff --git a/user/shared/drbdmeta.c b/user/shared/drbdmeta.c
index 6ff877a..86f32ed 100644
--- a/user/shared/drbdmeta.c
+++ b/user/shared/drbdmeta.c
@@ -60,7 +60,12 @@
 
 #include "config.h"
 
-/* BLKZEROOUT, available on linux-3.6 and later. */
+/* BLKZEROOUT, available on linux-3.6 and later,
+ * and maybe backported to distribution kernels,
+ * even if they pretend to be older.
+ * Yes, we encountered a number of systems that already had it in their
+ * kernels, but not yet in the headers used to build userland stuff like this.
+ */
 #ifndef BLKZEROOUT
 # define BLKZEROOUT	_IO(0x12,127)
 #endif
@@ -1716,7 +1721,11 @@ static void zeroout_bitmap(struct format *cfg)
 	if (!err)
 		return;
 
-	if (errno == ENOTTY) {
+	PERROR("ioctl(%s, BLKZEROOUT, [%llu, %llu]) failed", cfg->md_device_name,
+			(unsigned long long)range[0], (unsigned long long)range[1]);
+	fprintf(stderr, "Using slow(er) fallback.\n");
+
+	{
 		/* need to sector-align this for O_DIRECT.
 		 * "sector" here means hard-sect size, which may be != 512.
 		 * Note that even though ALIGN does round up, for sector sizes
@@ -1744,9 +1753,6 @@ static void zeroout_bitmap(struct format *cfg)
 			}
 		}
 		fprintf(stderr,"\r100%%\n");
-	} else {
-		PERROR("ioctl(%s) failed", cfg->md_device_name);
-		exit(10);
 	}
 }
 

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-08-17  9:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-17  1:57 [Drbd-dev] [PATCH] drbdmeta: Fix drbdmeta to support old kernels Artem Pogartsev
2017-08-17  9:59 ` Lars Ellenberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox