[Qemu-devel] [PATCH 2/2] hd-geometry.c: Integrate HDIO_GETGEO in guessing

[Qemu-devel] [PATCH 2/2] hd-geometry.c: Integrate HDIO_GETGEO in guessing

[Jens Freimann]

From: Einar Lueck <elelueck@linux.vnet.ibm.com>

This patch extends the function guess_disk_lchs. If no geo could
be derived from reading disk content, HDIO_GETGEO ioctl is issued.
If this is not successful (e.g. image files) geometry is derived
from the size of the disk (as before). To achieve this, the MSDOS
partition table reading logic and the translation computation logic
have been moved into a separate function guess_disk_msdosgeo. The
HDIO_GETGEO logic is captured in a new function guess_disk_ioctlgeo.
guess_disk_lchs then encapsulates both (the overall guessing logic).
The new HDIO_GETGEO logic is required for two use cases:
a) Support for geometries of Direct Attached Storage Disks (DASD)
on s390x configured as backing of virtio block devices.
b) Support for FCP attached SCSI disks that do not yet have a
partition table. Without this patch, fdisk -l on the host would
return different results then fdisk -l in the guest.

Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

---
 hw/hd-geometry.c | 118 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 81 insertions(+), 37 deletions(-)

diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 1cdb9fb..fc29075 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -33,6 +33,10 @@
 #include "block.h"
 #include "hw/block-common.h"
 #include "trace.h"
+#ifdef __linux__
+#include <linux/fs.h>
+#include <linux/hdreg.h>
+#endif
 
 struct partition {
         uint8_t boot_ind;           /* 0x80 - active */
@@ -47,13 +51,59 @@ struct partition {
         uint32_t nr_sects;          /* nr of sectors in partition */
 } QEMU_PACKED;
 
+static void guess_chs_for_size(BlockDriverState *bs,
+                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
+{
+    uint64_t nb_sectors;
+    int cylinders;
+
+    bdrv_get_geometry(bs, &nb_sectors);
+
+    cylinders = nb_sectors / (16 * 63);
+    if (cylinders > 16383) {
+        cylinders = 16383;
+    } else if (cylinders < 2) {
+        cylinders = 2;
+    }
+    *pcyls = cylinders;
+    *pheads = 16;
+    *psecs = 63;
+}
+
+/* try to get geometry from disk via HDIO_GETGEO ioctl
+   Return 0 if OK, -1 if ioctl does not work (e.g. image file) */
+static inline int guess_disk_ioctlgeo(BlockDriverState *bs,
+                                     uint32_t *pcylinders, uint32_t *pheads,
+                                     uint32_t *psectors, int *ptranslation)
+{
+#ifdef __linux__
+    struct hd_geometry geo;
+
+    if (bdrv_ioctl(bs, HDIO_GETGEO, &geo)) {
+        return -1;
+    }
+
+    *pheads = geo.heads;
+    *psectors = geo.sectors;
+    *pcylinders = geo.cylinders;
+    *ptranslation = BIOS_ATA_TRANSLATION_NONE;
+    trace_hd_geometry_lchs_guess(bs, *pcylinders, *pheads, *psectors);
+    return 0;
+#else
+    return -1;
+#endif
+}
+
+
 /* try to guess the disk logical geometry from the MSDOS partition table.
    Return 0 if OK, -1 if could not guess */
-static int guess_disk_lchs(BlockDriverState *bs,
-                           int *pcylinders, int *pheads, int *psectors)
+static int guess_disk_msdosgeo(BlockDriverState *bs,
+                               uint32_t *pcylinders, uint32_t *pheads,
+                               uint32_t *psectors, int *ptranslation)
 {
     uint8_t buf[BDRV_SECTOR_SIZE];
-    int i, heads, sectors, cylinders;
+    int i, translation;
+    uint32_t heads, sectors, cylinders;
     struct partition *p;
     uint32_t nr_sects;
     uint64_t nb_sectors;
@@ -87,9 +137,26 @@ static int guess_disk_lchs(BlockDriverState *bs,
             if (cylinders < 1 || cylinders > 16383) {
                 continue;
             }
+
+            if (heads > 16) {
+                /* LCHS guess with heads > 16 means that a BIOS LBA
+                   translation was active, so a standard physical disk
+                   geometry is OK */
+                guess_chs_for_size(bs, &cylinders, &heads, &sectors);
+                translation = cylinders * heads <= 131072
+                    ? BIOS_ATA_TRANSLATION_LARGE
+                    : BIOS_ATA_TRANSLATION_LBA;
+            } else {
+                /* LCHS guess with heads <= 16: use as physical geometry */
+                /* disable any translation to be in sync with
+                   the logical geometry */
+                translation = BIOS_ATA_TRANSLATION_NONE;
+            }
             *pheads = heads;
             *psectors = sectors;
             *pcylinders = cylinders;
+            *ptranslation = translation;
+
             trace_hd_geometry_lchs_guess(bs, cylinders, heads, sectors);
             return 0;
         }
@@ -97,51 +164,28 @@ static int guess_disk_lchs(BlockDriverState *bs,
     return -1;
 }
 
-static void guess_chs_for_size(BlockDriverState *bs,
-                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
-{
-    uint64_t nb_sectors;
-    int cylinders;
-
-    bdrv_get_geometry(bs, &nb_sectors);
-
-    cylinders = nb_sectors / (16 * 63);
-    if (cylinders > 16383) {
-        cylinders = 16383;
-    } else if (cylinders < 2) {
-        cylinders = 2;
+/* try to guess the disk logical geometry
+   Return 0 if OK, -1 if could not guess */
+static int guess_disk_lchs(BlockDriverState *bs,
+                           uint32_t *pcylinders, uint32_t *pheads,
+                           uint32_t *psectors, int *ptranslation) {
+    if (!guess_disk_msdosgeo(bs, pcylinders, pheads, psectors, ptranslation)) {
+        return 0;
     }
-    *pcyls = cylinders;
-    *pheads = 16;
-    *psecs = 63;
+
+    return guess_disk_ioctlgeo(bs, pcylinders, pheads, psectors, ptranslation);
 }
 
 void hd_geometry_guess(BlockDriverState *bs,
                        uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
                        int *ptrans)
 {
-    int cylinders, heads, secs, translation;
+    int translation;
 
-    if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
+    if (guess_disk_lchs(bs, pcyls, pheads, psecs, &translation) < 0) {
         /* no LCHS guess: use a standard physical disk geometry  */
         guess_chs_for_size(bs, pcyls, pheads, psecs);
         translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
-    } else if (heads > 16) {
-        /* LCHS guess with heads > 16 means that a BIOS LBA
-           translation was active, so a standard physical disk
-           geometry is OK */
-        guess_chs_for_size(bs, pcyls, pheads, psecs);
-        translation = *pcyls * *pheads <= 131072
-            ? BIOS_ATA_TRANSLATION_LARGE
-            : BIOS_ATA_TRANSLATION_LBA;
-    } else {
-        /* LCHS guess with heads <= 16: use as physical geometry */
-        *pcyls = cylinders;
-        *pheads = heads;
-        *psecs = secs;
-        /* disable any translation to be in sync with
-           the logical geometry */
-        translation = BIOS_ATA_TRANSLATION_NONE;
     }
     if (ptrans) {
         *ptrans = translation;
-- 
1.7.11.4

[Qemu-devel] [PATCH 0/2] s390: block size auto-sensing and geometry guessing changes in common code

[Jens Freimann]

[resend because the first attempt didn't make it to the mailing list]

Here are two patches for block device characteristics detection. 
The first one implements block size auto-sensing for virtio-block and 
the second patch changes geometry guessing common code. 
More details in the patch descriptions.

Einar Lueck (2):
  virtio-block: support auto-sensing of block sizes
  hd-geometry.c: Integrate HDIO_GETGEO in guessing

 hw/block-common.c    |  23 ++++++++++
 hw/block-common.h    |  12 ++++--
 hw/hd-geometry.c     | 118 +++++++++++++++++++++++++++++++++++----------------
 hw/qdev-properties.c |   4 +-
 hw/s390-virtio-bus.c |   2 +-
 hw/virtio-blk.c      |   1 +
 6 files changed, 118 insertions(+), 42 deletions(-)

-- 
1.7.11.4

[Qemu-devel] [PATCH 1/2] virtio-block: support auto-sensing of block sizes

[Jens Freimann]

From: Einar Lueck <elelueck@linux.vnet.ibm.com>

Virtio-blk does not impose fixed block sizes for access to
backing devices. This patch introduces support for auto
lookup of the block sizes of the backing block device. This
automatic lookup needs to be enabled explicitly. Users may
do this by specifying (physical|logical)_block_size=0.
Machine types may do this for their defaults, too.
To achieve this, a new function blkconf_blocksizes is
implemented. If physical or logical block size are zero
a corresponding ioctl tries to find an appropriate value.
If this does not work, 512 is used. blkconf_blocksizes
is therefore only called w/in the virtio-blk context.
For s390-virtio, this patch configures auto lookup as
default.

Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 hw/block-common.c    | 23 +++++++++++++++++++++++
 hw/block-common.h    | 12 +++++++++---
 hw/qdev-properties.c |  4 +++-
 hw/s390-virtio-bus.c |  2 +-
 hw/virtio-blk.c      |  1 +
 5 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/hw/block-common.c b/hw/block-common.c
index f0196d7..444edd2 100644
--- a/hw/block-common.c
+++ b/hw/block-common.c
@@ -10,6 +10,9 @@
 #include "blockdev.h"
 #include "hw/block-common.h"
 #include "qemu-error.h"
+#ifdef __linux__
+#include <linux/fs.h>
+#endif
 
 void blkconf_serial(BlockConf *conf, char **serial)
 {
@@ -24,6 +27,26 @@ void blkconf_serial(BlockConf *conf, char **serial)
     }
 }
 
+void blkconf_blocksizes(BlockConf *conf)
+{
+    int block_size;
+
+    if (!conf->physical_block_size) {
+        if (bdrv_ioctl(conf->bs, BLKPBSZGET, &block_size) == 0) {
+            conf->physical_block_size = (uint16_t) block_size;
+        } else {
+            conf->physical_block_size = BLOCK_PROPERTY_STD_BLKSIZE;
+        }
+    }
+    if (!conf->logical_block_size) {
+        if (bdrv_ioctl(conf->bs, BLKSSZGET, &block_size) == 0) {
+            conf->logical_block_size = (uint16_t) block_size;
+        } else {
+            conf->logical_block_size = BLOCK_PROPERTY_STD_BLKSIZE;
+        }
+    }
+}
+
 int blkconf_geometry(BlockConf *conf, int *ptrans,
                      unsigned cyls_max, unsigned heads_max, unsigned secs_max)
 {
diff --git a/hw/block-common.h b/hw/block-common.h
index bb808f7..d593128 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -40,18 +40,23 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
     return exp;
 }
 
-#define DEFINE_BLOCK_PROPERTIES(_state, _conf)                          \
+#define BLOCK_PROPERTY_STD_BLKSIZE 512
+#define DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, _blksize)       \
     DEFINE_PROP_DRIVE("drive", _state, _conf.bs),                       \
     DEFINE_PROP_BLOCKSIZE("logical_block_size", _state,                 \
-                          _conf.logical_block_size, 512),               \
+                          _conf.logical_block_size, _blksize),          \
     DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
-                          _conf.physical_block_size, 512),              \
+                          _conf.physical_block_size, _blksize),         \
     DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
     DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
     DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \
     DEFINE_PROP_UINT32("discard_granularity", _state, \
                        _conf.discard_granularity, 0)
 
+#define DEFINE_BLOCK_PROPERTIES(_state, _conf)                          \
+    DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf,                     \
+                                     BLOCK_PROPERTY_STD_BLKSIZE)
+
 #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf)      \
     DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0),  \
     DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
@@ -60,6 +65,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
 /* Configuration helpers */
 
 void blkconf_serial(BlockConf *conf, char **serial);
+void blkconf_blocksizes(BlockConf *conf);
 int blkconf_geometry(BlockConf *conf, int *trans,
                      unsigned cyls_max, unsigned heads_max, unsigned secs_max);
 
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 8aca0d4..e99dee4 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -904,7 +904,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
         error_propagate(errp, local_err);
         return;
     }
-    if (value < min || value > max) {
+
+    /* value == 0 indicates that block size should be sensed later on */
+    if ((value < min || value > max) && value > 0) {
         error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
                   dev->id?:"", name, (int64_t)value, min, max);
         return;
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index a245684..562964e 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -401,7 +401,7 @@ static TypeInfo s390_virtio_net = {
 };
 
 static Property s390_virtio_blk_properties[] = {
-    DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf),
+    DEFINE_BLOCK_PROPERTIES_EXTENDED(VirtIOS390Device, blk.conf, 0),
     DEFINE_BLOCK_CHS_PROPERTIES(VirtIOS390Device, blk.conf),
     DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial),
 #ifdef __linux__
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index f21757e..7686433 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -600,6 +600,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     }
 
     blkconf_serial(&blk->conf, &blk->serial);
+    blkconf_blocksizes(&blk->conf);
     if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
         return NULL;
     }
-- 
1.7.11.4

[Qemu-devel] [PATCH 2/2] hd-geometry.c: Integrate HDIO_GETGEO in guessing

[Jens Freimann]

From: Einar Lueck <elelueck@linux.vnet.ibm.com>

This patch extends the function guess_disk_lchs. If no geo could
be derived from reading disk content, HDIO_GETGEO ioctl is issued.
If this is not successful (e.g. image files) geometry is derived
from the size of the disk (as before). To achieve this, the MSDOS
partition table reading logic and the translation computation logic
have been moved into a separate function guess_disk_msdosgeo. The
HDIO_GETGEO logic is captured in a new function guess_disk_ioctlgeo.
guess_disk_lchs then encapsulates both (the overall guessing logic).
The new HDIO_GETGEO logic is required for two use cases:
a) Support for geometries of Direct Attached Storage Disks (DASD)
on s390x configured as backing of virtio block devices.
b) Support for FCP attached SCSI disks that do not yet have a
partition table. Without this patch, fdisk -l on the host would
return different results then fdisk -l in the guest.

Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

---
 hw/hd-geometry.c | 118 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 81 insertions(+), 37 deletions(-)

diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 1cdb9fb..fc29075 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -33,6 +33,10 @@
 #include "block.h"
 #include "hw/block-common.h"
 #include "trace.h"
+#ifdef __linux__
+#include <linux/fs.h>
+#include <linux/hdreg.h>
+#endif
 
 struct partition {
         uint8_t boot_ind;           /* 0x80 - active */
@@ -47,13 +51,59 @@ struct partition {
         uint32_t nr_sects;          /* nr of sectors in partition */
 } QEMU_PACKED;
 
+static void guess_chs_for_size(BlockDriverState *bs,
+                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
+{
+    uint64_t nb_sectors;
+    int cylinders;
+
+    bdrv_get_geometry(bs, &nb_sectors);
+
+    cylinders = nb_sectors / (16 * 63);
+    if (cylinders > 16383) {
+        cylinders = 16383;
+    } else if (cylinders < 2) {
+        cylinders = 2;
+    }
+    *pcyls = cylinders;
+    *pheads = 16;
+    *psecs = 63;
+}
+
+/* try to get geometry from disk via HDIO_GETGEO ioctl
+   Return 0 if OK, -1 if ioctl does not work (e.g. image file) */
+static inline int guess_disk_ioctlgeo(BlockDriverState *bs,
+                                     uint32_t *pcylinders, uint32_t *pheads,
+                                     uint32_t *psectors, int *ptranslation)
+{
+#ifdef __linux__
+    struct hd_geometry geo;
+
+    if (bdrv_ioctl(bs, HDIO_GETGEO, &geo)) {
+        return -1;
+    }
+
+    *pheads = geo.heads;
+    *psectors = geo.sectors;
+    *pcylinders = geo.cylinders;
+    *ptranslation = BIOS_ATA_TRANSLATION_NONE;
+    trace_hd_geometry_lchs_guess(bs, *pcylinders, *pheads, *psectors);
+    return 0;
+#else
+    return -1;
+#endif
+}
+
+
 /* try to guess the disk logical geometry from the MSDOS partition table.
    Return 0 if OK, -1 if could not guess */
-static int guess_disk_lchs(BlockDriverState *bs,
-                           int *pcylinders, int *pheads, int *psectors)
+static int guess_disk_msdosgeo(BlockDriverState *bs,
+                               uint32_t *pcylinders, uint32_t *pheads,
+                               uint32_t *psectors, int *ptranslation)
 {
     uint8_t buf[BDRV_SECTOR_SIZE];
-    int i, heads, sectors, cylinders;
+    int i, translation;
+    uint32_t heads, sectors, cylinders;
     struct partition *p;
     uint32_t nr_sects;
     uint64_t nb_sectors;
@@ -87,9 +137,26 @@ static int guess_disk_lchs(BlockDriverState *bs,
             if (cylinders < 1 || cylinders > 16383) {
                 continue;
             }
+
+            if (heads > 16) {
+                /* LCHS guess with heads > 16 means that a BIOS LBA
+                   translation was active, so a standard physical disk
+                   geometry is OK */
+                guess_chs_for_size(bs, &cylinders, &heads, &sectors);
+                translation = cylinders * heads <= 131072
+                    ? BIOS_ATA_TRANSLATION_LARGE
+                    : BIOS_ATA_TRANSLATION_LBA;
+            } else {
+                /* LCHS guess with heads <= 16: use as physical geometry */
+                /* disable any translation to be in sync with
+                   the logical geometry */
+                translation = BIOS_ATA_TRANSLATION_NONE;
+            }
             *pheads = heads;
             *psectors = sectors;
             *pcylinders = cylinders;
+            *ptranslation = translation;
+
             trace_hd_geometry_lchs_guess(bs, cylinders, heads, sectors);
             return 0;
         }
@@ -97,51 +164,28 @@ static int guess_disk_lchs(BlockDriverState *bs,
     return -1;
 }
 
-static void guess_chs_for_size(BlockDriverState *bs,
-                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
-{
-    uint64_t nb_sectors;
-    int cylinders;
-
-    bdrv_get_geometry(bs, &nb_sectors);
-
-    cylinders = nb_sectors / (16 * 63);
-    if (cylinders > 16383) {
-        cylinders = 16383;
-    } else if (cylinders < 2) {
-        cylinders = 2;
+/* try to guess the disk logical geometry
+   Return 0 if OK, -1 if could not guess */
+static int guess_disk_lchs(BlockDriverState *bs,
+                           uint32_t *pcylinders, uint32_t *pheads,
+                           uint32_t *psectors, int *ptranslation) {
+    if (!guess_disk_msdosgeo(bs, pcylinders, pheads, psectors, ptranslation)) {
+        return 0;
     }
-    *pcyls = cylinders;
-    *pheads = 16;
-    *psecs = 63;
+
+    return guess_disk_ioctlgeo(bs, pcylinders, pheads, psectors, ptranslation);
 }
 
 void hd_geometry_guess(BlockDriverState *bs,
                        uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
                        int *ptrans)
 {
-    int cylinders, heads, secs, translation;
+    int translation;
 
-    if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
+    if (guess_disk_lchs(bs, pcyls, pheads, psecs, &translation) < 0) {
         /* no LCHS guess: use a standard physical disk geometry  */
         guess_chs_for_size(bs, pcyls, pheads, psecs);
         translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
-    } else if (heads > 16) {
-        /* LCHS guess with heads > 16 means that a BIOS LBA
-           translation was active, so a standard physical disk
-           geometry is OK */
-        guess_chs_for_size(bs, pcyls, pheads, psecs);
-        translation = *pcyls * *pheads <= 131072
-            ? BIOS_ATA_TRANSLATION_LARGE
-            : BIOS_ATA_TRANSLATION_LBA;
-    } else {
-        /* LCHS guess with heads <= 16: use as physical geometry */
-        *pcyls = cylinders;
-        *pheads = heads;
-        *psecs = secs;
-        /* disable any translation to be in sync with
-           the logical geometry */
-        translation = BIOS_ATA_TRANSLATION_NONE;
     }
     if (ptrans) {
         *ptrans = translation;
-- 
1.7.11.4

[Qemu-devel] [PATCH 2/2] hd-geometry.c: Integrate HDIO_GETGEO in guessing

[Jens Freimann]

From: Einar Lueck <elelueck@linux.vnet.ibm.com>

This patch extends the function guess_disk_lchs. If no geo could
be derived from reading disk content, HDIO_GETGEO ioctl is issued.
If this is not successful (e.g. image files) geometry is derived
from the size of the disk (as before). To achieve this, the MSDOS
partition table reading logic and the translation computation logic
have been moved into a separate function guess_disk_msdosgeo. The
HDIO_GETGEO logic is captured in a new function guess_disk_ioctlgeo.
guess_disk_lchs then encapsulates both (the overall guessing logic).
The new HDIO_GETGEO logic is required for two use cases:
a) Support for geometries of Direct Attached Storage Disks (DASD)
on s390x configured as backing of virtio block devices.
b) Support for FCP attached SCSI disks that do not yet have a
partition table. Without this patch, fdisk -l on the host would
return different results then fdisk -l in the guest.

Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 hw/hd-geometry.c | 118 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 81 insertions(+), 37 deletions(-)

diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 1cdb9fb..fc29075 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -33,6 +33,10 @@
 #include "block.h"
 #include "hw/block-common.h"
 #include "trace.h"
+#ifdef __linux__
+#include <linux/fs.h>
+#include <linux/hdreg.h>
+#endif
 
 struct partition {
         uint8_t boot_ind;           /* 0x80 - active */
@@ -47,13 +51,59 @@ struct partition {
         uint32_t nr_sects;          /* nr of sectors in partition */
 } QEMU_PACKED;
 
+static void guess_chs_for_size(BlockDriverState *bs,
+                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
+{
+    uint64_t nb_sectors;
+    int cylinders;
+
+    bdrv_get_geometry(bs, &nb_sectors);
+
+    cylinders = nb_sectors / (16 * 63);
+    if (cylinders > 16383) {
+        cylinders = 16383;
+    } else if (cylinders < 2) {
+        cylinders = 2;
+    }
+    *pcyls = cylinders;
+    *pheads = 16;
+    *psecs = 63;
+}
+
+/* try to get geometry from disk via HDIO_GETGEO ioctl
+   Return 0 if OK, -1 if ioctl does not work (e.g. image file) */
+static inline int guess_disk_ioctlgeo(BlockDriverState *bs,
+                                     uint32_t *pcylinders, uint32_t *pheads,
+                                     uint32_t *psectors, int *ptranslation)
+{
+#ifdef __linux__
+    struct hd_geometry geo;
+
+    if (bdrv_ioctl(bs, HDIO_GETGEO, &geo)) {
+        return -1;
+    }
+
+    *pheads = geo.heads;
+    *psectors = geo.sectors;
+    *pcylinders = geo.cylinders;
+    *ptranslation = BIOS_ATA_TRANSLATION_NONE;
+    trace_hd_geometry_lchs_guess(bs, *pcylinders, *pheads, *psectors);
+    return 0;
+#else
+    return -1;
+#endif
+}
+
+
 /* try to guess the disk logical geometry from the MSDOS partition table.
    Return 0 if OK, -1 if could not guess */
-static int guess_disk_lchs(BlockDriverState *bs,
-                           int *pcylinders, int *pheads, int *psectors)
+static int guess_disk_msdosgeo(BlockDriverState *bs,
+                               uint32_t *pcylinders, uint32_t *pheads,
+                               uint32_t *psectors, int *ptranslation)
 {
     uint8_t buf[BDRV_SECTOR_SIZE];
-    int i, heads, sectors, cylinders;
+    int i, translation;
+    uint32_t heads, sectors, cylinders;
     struct partition *p;
     uint32_t nr_sects;
     uint64_t nb_sectors;
@@ -87,9 +137,26 @@ static int guess_disk_lchs(BlockDriverState *bs,
             if (cylinders < 1 || cylinders > 16383) {
                 continue;
             }
+
+            if (heads > 16) {
+                /* LCHS guess with heads > 16 means that a BIOS LBA
+                   translation was active, so a standard physical disk
+                   geometry is OK */
+                guess_chs_for_size(bs, &cylinders, &heads, &sectors);
+                translation = cylinders * heads <= 131072
+                    ? BIOS_ATA_TRANSLATION_LARGE
+                    : BIOS_ATA_TRANSLATION_LBA;
+            } else {
+                /* LCHS guess with heads <= 16: use as physical geometry */
+                /* disable any translation to be in sync with
+                   the logical geometry */
+                translation = BIOS_ATA_TRANSLATION_NONE;
+            }
             *pheads = heads;
             *psectors = sectors;
             *pcylinders = cylinders;
+            *ptranslation = translation;
+
             trace_hd_geometry_lchs_guess(bs, cylinders, heads, sectors);
             return 0;
         }
@@ -97,51 +164,28 @@ static int guess_disk_lchs(BlockDriverState *bs,
     return -1;
 }
 
-static void guess_chs_for_size(BlockDriverState *bs,
-                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
-{
-    uint64_t nb_sectors;
-    int cylinders;
-
-    bdrv_get_geometry(bs, &nb_sectors);
-
-    cylinders = nb_sectors / (16 * 63);
-    if (cylinders > 16383) {
-        cylinders = 16383;
-    } else if (cylinders < 2) {
-        cylinders = 2;
+/* try to guess the disk logical geometry
+   Return 0 if OK, -1 if could not guess */
+static int guess_disk_lchs(BlockDriverState *bs,
+                           uint32_t *pcylinders, uint32_t *pheads, 
+                           uint32_t *psectors, int *ptranslation) {
+    if (!guess_disk_msdosgeo(bs, pcylinders, pheads, psectors, ptranslation)) {
+        return 0;
     }
-    *pcyls = cylinders;
-    *pheads = 16;
-    *psecs = 63;
+
+    return guess_disk_ioctlgeo(bs, pcylinders, pheads, psectors, ptranslation);
 }
 
 void hd_geometry_guess(BlockDriverState *bs,
                        uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
                        int *ptrans)
 {
-    int cylinders, heads, secs, translation;
+    int translation;
 
-    if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
+    if (guess_disk_lchs(bs, pcyls, pheads, psecs, &translation) < 0) {
         /* no LCHS guess: use a standard physical disk geometry  */
         guess_chs_for_size(bs, pcyls, pheads, psecs);
         translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
-    } else if (heads > 16) {
-        /* LCHS guess with heads > 16 means that a BIOS LBA
-           translation was active, so a standard physical disk
-           geometry is OK */
-        guess_chs_for_size(bs, pcyls, pheads, psecs);
-        translation = *pcyls * *pheads <= 131072
-            ? BIOS_ATA_TRANSLATION_LARGE
-            : BIOS_ATA_TRANSLATION_LBA;
-    } else {
-        /* LCHS guess with heads <= 16: use as physical geometry */
-        *pcyls = cylinders;
-        *pheads = heads;
-        *psecs = secs;
-        /* disable any translation to be in sync with
-           the logical geometry */
-        translation = BIOS_ATA_TRANSLATION_NONE;
     }
     if (ptrans) {
         *ptrans = translation;
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH 2/2] hd-geometry.c: Integrate HDIO_GETGEO in guessing

[Paolo Bonzini]

Il 15/08/2012 09:44, Jens Freimann ha scritto:
> From: Einar Lueck <elelueck@linux.vnet.ibm.com>
> 
> This patch extends the function guess_disk_lchs. If no geo could
> be derived from reading disk content, HDIO_GETGEO ioctl is issued.
> If this is not successful (e.g. image files) geometry is derived
> from the size of the disk (as before). To achieve this, the MSDOS
> partition table reading logic and the translation computation logic
> have been moved into a separate function guess_disk_msdosgeo. The
> HDIO_GETGEO logic is captured in a new function guess_disk_ioctlgeo.
> guess_disk_lchs then encapsulates both (the overall guessing logic).
> The new HDIO_GETGEO logic is required for two use cases:
> a) Support for geometries of Direct Attached Storage Disks (DASD)
> on s390x configured as backing of virtio block devices.
> b) Support for FCP attached SCSI disks that do not yet have a
> partition table. Without this patch, fdisk -l on the host would
> return different results then fdisk -l in the guest.
> 
> Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
>  hw/hd-geometry.c | 118 ++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 81 insertions(+), 37 deletions(-)
> 
> diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
> index 1cdb9fb..fc29075 100644
> --- a/hw/hd-geometry.c
> +++ b/hw/hd-geometry.c
> @@ -33,6 +33,10 @@
>  #include "block.h"
>  #include "hw/block-common.h"
>  #include "trace.h"
> +#ifdef __linux__
> +#include <linux/fs.h>
> +#include <linux/hdreg.h>
> +#endif
>  
>  struct partition {
>          uint8_t boot_ind;           /* 0x80 - active */
> @@ -47,13 +51,59 @@ struct partition {
>          uint32_t nr_sects;          /* nr of sectors in partition */
>  } QEMU_PACKED;
>  
> +static void guess_chs_for_size(BlockDriverState *bs,
> +                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
> +{
> +    uint64_t nb_sectors;
> +    int cylinders;
> +
> +    bdrv_get_geometry(bs, &nb_sectors);
> +
> +    cylinders = nb_sectors / (16 * 63);
> +    if (cylinders > 16383) {
> +        cylinders = 16383;
> +    } else if (cylinders < 2) {
> +        cylinders = 2;
> +    }
> +    *pcyls = cylinders;
> +    *pheads = 16;
> +    *psecs = 63;
> +}
> +
> +/* try to get geometry from disk via HDIO_GETGEO ioctl
> +   Return 0 if OK, -1 if ioctl does not work (e.g. image file) */
> +static inline int guess_disk_ioctlgeo(BlockDriverState *bs,
> +                                     uint32_t *pcylinders, uint32_t *pheads,
> +                                     uint32_t *psectors, int *ptranslation)
> +{
> +#ifdef __linux__
> +    struct hd_geometry geo;
> +
> +    if (bdrv_ioctl(bs, HDIO_GETGEO, &geo)) {
> +        return -1;
> +    }
> +
> +    *pheads = geo.heads;
> +    *psectors = geo.sectors;
> +    *pcylinders = geo.cylinders;
> +    *ptranslation = BIOS_ATA_TRANSLATION_NONE;
> +    trace_hd_geometry_lchs_guess(bs, *pcylinders, *pheads, *psectors);
> +    return 0;
> +#else
> +    return -1;
> +#endif
> +}
> +
> +
>  /* try to guess the disk logical geometry from the MSDOS partition table.
>     Return 0 if OK, -1 if could not guess */
> -static int guess_disk_lchs(BlockDriverState *bs,
> -                           int *pcylinders, int *pheads, int *psectors)
> +static int guess_disk_msdosgeo(BlockDriverState *bs,
> +                               uint32_t *pcylinders, uint32_t *pheads,
> +                               uint32_t *psectors, int *ptranslation)
>  {
>      uint8_t buf[BDRV_SECTOR_SIZE];
> -    int i, heads, sectors, cylinders;
> +    int i, translation;
> +    uint32_t heads, sectors, cylinders;
>      struct partition *p;
>      uint32_t nr_sects;
>      uint64_t nb_sectors;
> @@ -87,9 +137,26 @@ static int guess_disk_lchs(BlockDriverState *bs,
>              if (cylinders < 1 || cylinders > 16383) {
>                  continue;
>              }
> +
> +            if (heads > 16) {
> +                /* LCHS guess with heads > 16 means that a BIOS LBA
> +                   translation was active, so a standard physical disk
> +                   geometry is OK */
> +                guess_chs_for_size(bs, &cylinders, &heads, &sectors);
> +                translation = cylinders * heads <= 131072
> +                    ? BIOS_ATA_TRANSLATION_LARGE
> +                    : BIOS_ATA_TRANSLATION_LBA;
> +            } else {
> +                /* LCHS guess with heads <= 16: use as physical geometry */
> +                /* disable any translation to be in sync with
> +                   the logical geometry */
> +                translation = BIOS_ATA_TRANSLATION_NONE;
> +            }
>              *pheads = heads;
>              *psectors = sectors;
>              *pcylinders = cylinders;
> +            *ptranslation = translation;
> +
>              trace_hd_geometry_lchs_guess(bs, cylinders, heads, sectors);
>              return 0;
>          }
> @@ -97,51 +164,28 @@ static int guess_disk_lchs(BlockDriverState *bs,
>      return -1;
>  }
>  
> -static void guess_chs_for_size(BlockDriverState *bs,
> -                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
> -{
> -    uint64_t nb_sectors;
> -    int cylinders;
> -
> -    bdrv_get_geometry(bs, &nb_sectors);
> -
> -    cylinders = nb_sectors / (16 * 63);
> -    if (cylinders > 16383) {
> -        cylinders = 16383;
> -    } else if (cylinders < 2) {
> -        cylinders = 2;
> +/* try to guess the disk logical geometry
> +   Return 0 if OK, -1 if could not guess */
> +static int guess_disk_lchs(BlockDriverState *bs,
> +                           uint32_t *pcylinders, uint32_t *pheads, 
> +                           uint32_t *psectors, int *ptranslation) {
> +    if (!guess_disk_msdosgeo(bs, pcylinders, pheads, psectors, ptranslation)) {
> +        return 0;
>      }
> -    *pcyls = cylinders;
> -    *pheads = 16;
> -    *psecs = 63;
> +
> +    return guess_disk_ioctlgeo(bs, pcylinders, pheads, psectors, ptranslation);
>  }
>  
>  void hd_geometry_guess(BlockDriverState *bs,
>                         uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
>                         int *ptrans)
>  {
> -    int cylinders, heads, secs, translation;
> +    int translation;
>  
> -    if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
> +    if (guess_disk_lchs(bs, pcyls, pheads, psecs, &translation) < 0) {
>          /* no LCHS guess: use a standard physical disk geometry  */
>          guess_chs_for_size(bs, pcyls, pheads, psecs);
>          translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
> -    } else if (heads > 16) {
> -        /* LCHS guess with heads > 16 means that a BIOS LBA
> -           translation was active, so a standard physical disk
> -           geometry is OK */
> -        guess_chs_for_size(bs, pcyls, pheads, psecs);
> -        translation = *pcyls * *pheads <= 131072
> -            ? BIOS_ATA_TRANSLATION_LARGE
> -            : BIOS_ATA_TRANSLATION_LBA;
> -    } else {
> -        /* LCHS guess with heads <= 16: use as physical geometry */
> -        *pcyls = cylinders;
> -        *pheads = heads;
> -        *psecs = secs;
> -        /* disable any translation to be in sync with
> -           the logical geometry */
> -        translation = BIOS_ATA_TRANSLATION_NONE;
>      }
>      if (ptrans) {
>          *ptrans = translation;
> 

Looks good.

Paolo