Skip to contentSkip to conversation

block/raw-posix: get right partition size

PATCH New Christoph Egger 2 participants 4 messages May 23, 2011 Block layer

lore
Share & tools
lore
author
#1 lore raw

PATCH 1/1 block/raw-posix: get right size of partition size

In reply to Christoph Egger

This does 2 things:
- use the correct way to get the size of a disk device or partition (from [email protected])
- if given a block device, use the character device instead. (from [email protected])

From: Adam Hamsik <[email protected]>
From: Manuel Bouyer <[email protected]>

Signed-off-by Christoph Egger <[email protected]>
block/raw-posix.c +48 -6
index 6b72470..7cf6461 100644
@@ -64,6 +64,13 @@
 #include <sys/dkio.h>
 #endif
 
+#ifdef __NetBSD__
+#include <sys/ioctl.h>
+#include <sys/disklabel.h>
+#include <sys/dkio.h>
+#include <sys/disk.h>
+#endif
+
 #ifdef __DragonFly__
 #include <sys/ioctl.h>
 #include <sys/diskslice.h>

@@ -141,7 +148,32 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
 {
     BDRVRawState *s = bs->opaque;
     int fd, ret;
+    struct stat sb;
 
+    if (lstat(filename, &sb) < 0) {
+        fprintf(stderr, "%s: stat failed: %s\n", filename, strerror(errno));
+        return -errno;
+    }
+    if (S_ISLNK(sb.st_mode)) {
+        fprintf(stderr, "%s: symbolic link not supported\n", filename);
+        return -EINVAL;
+    }
+#if defined(__NetBSD__)
+    if (S_ISBLK(sb.st_mode)) {
+        static char namebuf[PATH_MAX];
+        const char *dp = strrchr(filename, '/');
+
+        if (dp == NULL) {
+            snprintf(namebuf, PATH_MAX, "r%s", filename);
+        } else {
+            snprintf(namebuf, PATH_MAX, "%.*s/r%s",
+                (int)(dp - filename), filename, dp + 1);
+        }
+        fprintf(stderr, "%s is a block device", filename);
+        filename = namebuf;
+        fprintf(stderr, ", using %s\n", filename);
+    }
+#endif
     s->open_flags = open_flags | O_BINARY;
     s->open_flags &= ~O_ACCMODE;
     if (bdrv_flags & BDRV_O_RDWR) {

@@ -603,7 +635,7 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset)
     return 0;
 }
 
-#ifdef __OpenBSD__
+#if defined(__OpenBSD__) || defined(__NetBSD__)
 static int64_t raw_getlength(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;

@@ -613,12 +645,22 @@ static int64_t raw_getlength(BlockDriverState *bs)
     if (fstat(fd, &st))
         return -1;
     if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-        struct disklabel dl;
+#if defined(__NetBSD__)
+        struct dkwedge_info dkw;
 
-        if (ioctl(fd, DIOCGDINFO, &dl))
-            return -1;
-        return (uint64_t)dl.d_secsize *
-            dl.d_partitions[DISKPART(st.st_rdev)].p_size;
+        if (ioctl(fd, DIOCGWEDGEINFO, &dkw) != -1) {
+            return dkw.dkw_size * 512;
+        } else {
+#endif
+            struct disklabel dl;
+
+            if (ioctl(fd, DIOCGDINFO, &dl))
+                return -1;
+            return (uint64_t)dl.d_secsize *
+                dl.d_partitions[DISKPART(st.st_rdev)].p_size;
+#if defined(__NetBSD__)
+        }
+#endif
     } else
         return st.st_size;
 }
Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Christoph Egger

author
#0 lore raw

PATCH 1/1 block/raw-posix: get right partition size

This does 2 things:
- use the correct way to get the size of a disk device or partition (from [email protected])
- if given a block device, use the character device instead. (from [email protected])

From: Adam Hamsik <[email protected]>
From: Manuel Bouyer <[email protected]>

Signed-off-by Christoph Egger <[email protected]>
Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
block/raw-posix.c +48 -6
index 6b72470..7cf6461 100644
@@ -64,6 +64,13 @@
 #include <sys/dkio.h>
 #endif
 
+#ifdef __NetBSD__
+#include <sys/ioctl.h>
+#include <sys/disklabel.h>
+#include <sys/dkio.h>
+#include <sys/disk.h>
+#endif
+
 #ifdef __DragonFly__
 #include <sys/ioctl.h>
 #include <sys/diskslice.h>

@@ -141,7 +148,32 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
 {
     BDRVRawState *s = bs->opaque;
     int fd, ret;
+    struct stat sb;
 
+    if (lstat(filename, &sb) < 0) {
+        fprintf(stderr, "%s: stat failed: %s\n", filename, strerror(errno));
+        return -errno;
+    }
+    if (S_ISLNK(sb.st_mode)) {
+        fprintf(stderr, "%s: symbolic link not supported\n", filename);
+        return -EINVAL;
+    }
+#if defined(__NetBSD__)
+    if (S_ISBLK(sb.st_mode)) {
+        static char namebuf[PATH_MAX];
+        const char *dp = strrchr(filename, '/');
+
+        if (dp == NULL) {
+            snprintf(namebuf, PATH_MAX, "r%s", filename);
+        } else {
+            snprintf(namebuf, PATH_MAX, "%.*s/r%s",
+                (int)(dp - filename), filename, dp + 1);
+        }
+        fprintf(stderr, "%s is a block device", filename);
+        filename = namebuf;
+        fprintf(stderr, ", using %s\n", filename);
+    }
+#endif
     s->open_flags = open_flags | O_BINARY;
     s->open_flags &= ~O_ACCMODE;
     if (bdrv_flags & BDRV_O_RDWR) {

@@ -603,7 +635,7 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset)
     return 0;
 }
 
-#ifdef __OpenBSD__
+#if defined(__OpenBSD__) || defined(__NetBSD__)
 static int64_t raw_getlength(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;

@@ -613,12 +645,22 @@ static int64_t raw_getlength(BlockDriverState *bs)
     if (fstat(fd, &st))
         return -1;
     if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-        struct disklabel dl;
+#if defined(__NetBSD__)
+        struct dkwedge_info dkw;
 
-        if (ioctl(fd, DIOCGDINFO, &dl))
-            return -1;
-        return (uint64_t)dl.d_secsize *
-            dl.d_partitions[DISKPART(st.st_rdev)].p_size;
+        if (ioctl(fd, DIOCGWEDGEINFO, &dkw) != -1) {
+            return dkw.dkw_size * 512;
+        } else {
+#endif
+            struct disklabel dl;
+
+            if (ioctl(fd, DIOCGDINFO, &dl))
+                return -1;
+            return (uint64_t)dl.d_secsize *
+                dl.d_partitions[DISKPART(st.st_rdev)].p_size;
+#if defined(__NetBSD__)
+        }
+#endif
     } else
         return st.st_size;
 }
#2 lore raw

Re: [Qemu-devel] [PATCH] block/raw-posix: get right size of partition size

In reply to Christoph Egger
Quoted 6 lines · Christoph Egger

This does 2 things:
- use the correct way to get the size of a disk device or partition (from [email protected])
- if given a block device, use the character device instead. (from [email protected])

Please split that into two independent patches.

block/raw-posix.c static int raw_open_common(BlockDriverState *bs, const char *filename, · line 160

Quoted 4 lines
block/raw-posix.c +4 -0
+    if (S_ISLNK(sb.st_mode)) {
+        fprintf(stderr, "%s: symbolic link not supported\n", filename);
+        return -EINVAL;
+    }

Why not, it's a pretty clear regression from current code, and will break various Linux setups where there are lots of symlinks under /dev, as well as users using symlinks for their image files.

Quoted 6 lines
block/raw-posix.c +6 -0
+#if defined(__NetBSD__)
+    if (S_ISBLK(sb.st_mode)) {
+        static char namebuf[PATH_MAX];
+        const char *dp = strrchr(filename, '/');
+
+        if (dp == NULL) {

block/raw-posix.c static int raw_open_common(BlockDriverState *bs, const char *filename, · line 176

Quoted 10 lines
block/raw-posix.c +10 -0
+            snprintf(namebuf, PATH_MAX, "r%s", filename);
+        } else {
+            snprintf(namebuf, PATH_MAX, "%.*s/r%s",
+                (int)(dp - filename), filename, dp + 1);
+        }
+        fprintf(stderr, "%s is a block device", filename);
+        filename = namebuf;
+        fprintf(stderr, ", using %s\n", filename);
+    }
+#endif

Please split this out into a helper, which has a no-op version for other operating systems. It probably should also be enabled for other operating systems having char and block nodes for disk devices. That's at least OpenBSD and Solaris, not sure about Darwin.

Quoted 11 lines
block/raw-posix.c +2 -2
-#ifdef __OpenBSD__
+#if defined(__OpenBSD__) || defined(__NetBSD__)
 static int64_t raw_getlength(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;

@@ -613,12 +645,22 @@ static int64_t raw_getlength(BlockDriverState *bs)
     if (fstat(fd, &st))
         return -1;
     if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-        struct disklabel dl;
+#if defined(__NetBSD__)

block/raw-posix.c static int64_t raw_getlength(BlockDriverState *bs) · line 654

Quoted 10 lines
block/raw-posix.c +5 -4
+        struct dkwedge_info dkw;
 
-        if (ioctl(fd, DIOCGDINFO, &dl))
-            return -1;
-        return (uint64_t)dl.d_secsize *
-            dl.d_partitions[DISKPART(st.st_rdev)].p_size;
+        if (ioctl(fd, DIOCGWEDGEINFO, &dkw) != -1) {
+            return dkw.dkw_size * 512;
+        } else {
+#endif

Please provide a completely separate raw_getlength for NetBSD instead of creating this ifdef mess for almost no shared code.

author
#3 lore raw

Re: [Qemu-devel] [PATCH] block/raw-posix: get right size of partition size

Quoted 9 lines · Christoph Hellwig
Quoted 6 lines · Christoph Egger

This does 2 things:
- use the correct way to get the size of a disk device or partition (from [email protected])
- if given a block device, use the character device instead. (from [email protected])

Please split that into two independent patches.

ok.

Quoted 9 lines

block/raw-posix.c static int raw_open_common(BlockDriverState *bs, const char *filename, · line 160

Quoted 5 lines
block/raw-posix.c +4 -0
+    if (S_ISLNK(sb.st_mode)) {
+        fprintf(stderr, "%s: symbolic link not supported\n", filename);
+        return -EINVAL;
+    }

Why not, it's a pretty clear regression from current code, and will break various Linux setups where there are lots of symlinks under /dev, as well as users using symlinks for their image files.

there's no easy way to find the real target of a symlink, to find the corresponding character device (this could be refinded, because this restriction only applies if the target is a block device).

Quoted 15 lines
Quoted 15 lines
block/raw-posix.c +14 -0
+#if defined(__NetBSD__)
+    if (S_ISBLK(sb.st_mode)) {
+        static char namebuf[PATH_MAX];
+        const char *dp = strrchr(filename, '/');
+
+        if (dp == NULL) {
+            snprintf(namebuf, PATH_MAX, "r%s", filename);
+        } else {
+            snprintf(namebuf, PATH_MAX, "%.*s/r%s",
+                (int)(dp - filename), filename, dp + 1);
+        }
+        fprintf(stderr, "%s is a block device", filename);
+        filename = namebuf;
+        fprintf(stderr, ", using %s\n", filename);
Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+    }
+#endif

Please split this out into a helper, which has a no-op version for other operating systems. It probably should also be enabled for other operating systems having char and block nodes for disk devices. That's at least OpenBSD and Solaris, not sure about Darwin.

Yes, but don't have a test machine right now.

Quoted 13 lines
Quoted 13 lines
block/raw-posix.c +3 -2
-#ifdef __OpenBSD__
+#if defined(__OpenBSD__) || defined(__NetBSD__)
  static int64_t raw_getlength(BlockDriverState *bs)
  {
      BDRVRawState *s = bs->opaque;

@@ -613,12 +645,22 @@ static int64_t raw_getlength(BlockDriverState *bs)
      if (fstat(fd,&st))
          return -1;
      if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-        struct disklabel dl;
+#if defined(__NetBSD__)
+        struct dkwedge_info dkw;
Quoted 11 lines

block/raw-posix.c static int64_t raw_getlength(BlockDriverState *bs) · line 654

Quoted 8 lines
block/raw-posix.c +4 -4
-        if (ioctl(fd, DIOCGDINFO,&dl))
-            return -1;
-        return (uint64_t)dl.d_secsize *
-            dl.d_partitions[DISKPART(st.st_rdev)].p_size;
+        if (ioctl(fd, DIOCGWEDGEINFO,&dkw) != -1) {
+            return dkw.dkw_size * 512;
+        } else {
+#endif

Please provide a completely separate raw_getlength for NetBSD instead of creating this ifdef mess for almost no shared code.

Ok.

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632