from:"Ekaterina Tumanova"

geometry: hd_geometry_guess function autodetects the drive geometry.
This patch adds a block backend call, that probes the backing device
geometry. If the inner driver method is implemented and succeeds
(currently only for DASDs), the blkconf_geometry will pass-through
the backing device geometry. Otherwise will fallback to old logic.

blocksize: This patch initializes blocksize properties to 0.
In order to set the property a blkconf_blocksizes was introduced.
If user didn't set physical or logical blocksize, it will
retrieve its value from a driver (only succeeds for DASD), otherwise
it will set default 512 value.

The blkconf_blocksizes call was added to all users of BlkConf.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
---
 hw/block/block.c | 24 
 hw/block/hd-geometry.c   | 10 +-
 hw/block/nvme.c  |  1 +
 hw/block/virtio-blk.c|  1 +
 hw/core/qdev-properties.c|  3 ++-
 hw/ide/qdev.c|  1 +
 hw/scsi/scsi-disk.c  |  2 ++
 hw/usb/dev-storage.c |  1 +
 include/hw/block/block.h |  5 +++--
 include/hw/qdev-properties.h |  4 ++--
 10 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..f7243e5 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,30 @@ void blkconf_serial(BlockConf *conf, char **serial)
 }
 }
 
+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+BlockSizes blocksizes;
+int backend_ret;
+
+backend_ret = blk_probe_blocksizes(blk, &blocksizes);
+/* fill in detected values if they are not defined via qemu command line */
+if (!conf->physical_block_size) {
+if (!backend_ret) {
+   conf->physical_block_size = blocksizes.phys;
+} else {
+conf->physical_block_size = BDRV_SECTOR_SIZE;
+}
+}
+if (!conf->logical_block_size) {
+if (!backend_ret) {
+conf->logical_block_size = blocksizes.log;
+} else {
+conf->logical_block_size = BDRV_SECTOR_SIZE;
+}
+}
+}
+
 void blkconf_geometry(BlockConf *conf, int *ptrans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..b187878 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,8 +121,16 @@ void hd_geometry_guess(BlockBackend *blk,
int *ptrans)
 {
 int cylinders, heads, secs, translation;
+HDGeometry geo;
 
-if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
+/* Try to probe the backing device geometry, otherwise fallback
+   to the old logic. (as of 12/2014 probing only succeeds on DASDs) */
+if (blk_probe_geometry(blk, &geo) == 0) {
+*pcyls = geo.cylinders;
+*psecs = geo.sectors;
+*pheads = geo.heads;
+translation = BIOS_ATA_TRANSLATION_NONE;
+} else if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
 guess_chs_for_size(blk, pcyls, pheads, psecs);
 translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index ce079ae..0f3dfb9 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -765,6 +765,7 @@ static int nvme_init(PCIDevice *pci_dev)
 if (!n->serial) {
 return -1;
 }
+blkconf_blocksizes(&n->conf);
 
 pci_conf = pci_dev->config;
 pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 1a8a176..a414ecb 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -888,6 +888,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 error_propagate(errp, err);
 return;
 }
+blkconf_blocksizes(&conf->conf);
 
 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2e47f70..ba81709 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -580,7 +580,8 @@ static void set_blocksize(Object *obj, Visitor *v, void 
*opaque,
 error_propagate(errp, local_err);
 return;
 }
-if (value < min || value > max) {
+/* value of 0 means "unset" */
+if (value && (value < min || value > max)) {
 error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
   dev->id?:"", name, (int64_t)value, min, max);
 return;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1ebb58d..4d41cbd 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -163,6 +163,7 @@ static int ide_dev_initfn(I

[Qemu-devel] [PATCH v7 3/5] block: Add driver methods to probe blocksizes and geometry

Introduce driver methods of defining disk blocksizes (physical and
logical) and hard drive geometry.
Methods are only implemented for "host_device". For "raw" devices
driver calls child's method.

For now geometry detection will only work for DASD devices. To check
that a local check_for_dasd function was introduced. It calls BIODASDINFO2
ioctl and returns its rc.

Blocksizes detection function will probe sizes for DASD devices.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
---
 block/raw-posix.c | 103 ++
 block/raw_bsd.c   |  12 +++
 2 files changed, 115 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c306897..eba4532 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,10 @@
 #include 
 #include 
 #include 
+#include 
+#ifdef __s390__
+#include 
+#endif
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
@@ -251,6 +255,23 @@ static int probe_logical_blocksize(int fd, unsigned int 
*sector_size_p)
 return success ? 0 : -errno;
 }
 
+/**
+ * Get physical block size of @fd.
+ * On success, store it in @blk_size and return 0.
+ * On failure, return -errno.
+ */
+static int probe_physical_blocksize(int fd, unsigned int *blk_size)
+{
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
+return -errno;
+}
+return 0;
+#else
+return -ENOTSUP;
+#endif
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
@@ -674,6 +695,86 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 bs->bl.opt_mem_alignment = s->buf_align;
 }
 
+static int check_for_dasd(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#else
+return -1;
+#endif
+}
+
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BDRVRawState *s = bs->opaque;
+int ret;
+
+/* If DASD, get blocksizes */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+ret = probe_logical_blocksize(s->fd, &bsz->log);
+if (ret < 0) {
+return ret;
+}
+return probe_physical_blocksize(s->fd, &bsz->phys);
+}
+
+/**
+ * Try to get @bs's geometry: cyls, heads, sectors.
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * (Allows block driver to assign default geometry values that guest sees)
+ */
+#ifdef __linux__
+static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BDRVRawState *s = bs->opaque;
+struct hd_geometry ioctl_geo = {0};
+uint32_t blksize;
+
+/* If DASD, get its geometry */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
+return -errno;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+return -ENOTSUP;
+}
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+return -ENOTSUP;
+}
+geo->heads = ioctl_geo.heads;
+geo->sectors = ioctl_geo.sectors;
+if (!probe_physical_blocksize(s->fd, &blksize)) {
+/* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */
+geo->cylinders = bdrv_nb_sectors(bs) / (blksize / BDRV_SECTOR_SIZE)
+ / (geo->heads * geo->sectors);
+return 0;
+}
+geo->cylinders = ioctl_geo.cylinders;
+
+return 0;
+}
+#else /* __linux__ */
+static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+return -ENOTSUP;
+}
+#endif
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2213,6 +2314,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,
 
 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 05b02c7..e3d2d04 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -235,6 +235,16 @@ static int raw_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 1;
 }
 
+static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+return bdrv_probe_blocksizes(bs->file,

[Qemu-devel] [PATCH v7 0/5] Geometry and blocksize detection for backing devices.


Updates v6 -> v7:

Fix  
1. Set default blocksizes via blkconf_blocksizes call in ide_dev_initfn
and scsi_hd_realise before using these values.
2. Remove asserts in bdrv functions, check drv for being NULL instead.
3. Adjust logic in blkconf_blocksizes to use command-line blocksize values
(if they are in place) without any dependency on block backend probing.

p.s. Make check now suceeds.

p.p.s. Stefan, please notify if you need to re-review.

Thanks,
Kate.

---
   Patchset Description (didn't change):

Proper geometry and blocksize information is vital for support of
DASD/ECKD drives in Linux guests. Otherwise things will fail in
certain cases.

The existing geometry and blocksize qemu defaults have no sense
for DASD drives (hd_geometry_guess detection and 512 for sizes).
Setting this information manually in XML file is far from user-friendly,
as the admin has to manually look up the properties of the
host disks and then modify the guest definition accordingly.

Since Linux uses DASDs as normal block devices, we actually
want to use virtio-blk to pass those to KVM guests.

In order to avoid any change in behavior of other drives, the DASD
special casing was advised. We call ioctl BIODASDINFO2 on the block
device, which will only succeed if the device is really a DASD.

In order to retrieve the underlying device geometry and blocksizes
a new block-backend functions and underlying driver functions were
introduced (blk_probe_blocksizes anf blk_probe_geometry wrappers
and corresponding bdrv_xx functions).

As for now only "host_device" driver received new detection methods.
For "raw" we call childs method as usual. In future one may update
other drivers to add some other detection heuristics.

If the host_device appears to be a DASD, the driver functions
(hdev_probe_blocksizes and hdev_probe_geometry) will call certain
ioctls in order to detect geometry and blocksizes of the underlying device.
if probing failed bdrv_probe_blocksizes caller will set defaults,
and bdrv_probe_geometry will fail to allow fallback to old detection logic.

The front-end (BlockConf API) was updated:
1. a new blkconf_blocksizes function was added. It doesn't
change user-defined blocksize values. If properties are unset, it will
set values, returned by blk_probe_backend. In order to allow this logic,
blocksize properties were initialized with 0. (driver will return 512 if
backing device probing didn't succeed or if driver method is not defined).
2. hd_geometry guess was changed to firstly try to retrieve values via
blk_probe_geometry and if it fails, fallback to the old logic.

Ekaterina Tumanova (5):
  block: add bdrv functions for geometry and blocksize
  raw-posix: Factor block size detection out of raw_probe_alignment()
  block: Add driver methods to probe blocksizes and geometry
  block-backend: Add wrappers for blocksizes and geometry probing
  BlockConf: Call backend functions to detect geometry and blocksizes

 block.c|  34 +
 block/block-backend.c  |  10 +++
 block/raw-posix.c  | 154 -
 block/raw_bsd.c|  12 
 hw/block/block.c   |  24 +++
 hw/block/hd-geometry.c |  10 ++-
 hw/block/nvme.c|   1 +
 hw/block/virtio-blk.c  |   1 +
 hw/core/qdev-properties.c  |   3 +-
 hw/ide/qdev.c  |   1 +
 hw/scsi/scsi-disk.c|   2 +
 hw/usb/dev-storage.c   |   1 +
 include/block/block.h  |  13 
 include/block/block_int.h  |  15 
 include/hw/block/block.h   |   5 +-
 include/hw/qdev-properties.h   |   4 +-
 include/sysemu/block-backend.h |   2 +
 17 files changed, 270 insertions(+), 22 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v7 1/5] block: add bdrv functions for geometry and blocksize

Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Thomas Huth 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   | 34 ++
 include/block/block.h | 13 +
 include/block/block_int.h | 15 +++
 3 files changed, 62 insertions(+)

diff --git a/block.c b/block.c
index 210fd5f..f690b5e 100644
--- a/block.c
+++ b/block.c
@@ -569,6 +569,40 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz struct and return 0.
+ * On failure return -errno.
+ * @bs must not be empty.
+ */
+int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BlockDriver *drv = bs->drv;
+
+if (drv && drv->bdrv_probe_blocksizes) {
+return drv->bdrv_probe_blocksizes(bs, bsz);
+}
+
+return -ENOTSUP;
+}
+
+/**
+ * Try to get @bs's geometry (cyls, heads, sectors).
+ * On success, store them in @geo struct and return 0.
+ * On failure return -errno.
+ * @bs must not be empty.
+ */
+int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BlockDriver *drv = bs->drv;
+
+if (drv && drv->bdrv_probe_geometry) {
+return drv->bdrv_probe_geometry(bs, geo);
+}
+
+return -ENOTSUP;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
diff --git a/include/block/block.h b/include/block/block.h
index 321295e..75f0b90 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -60,6 +60,17 @@ typedef enum {
 BDRV_REQ_MAY_UNMAP= 0x4,
 } BdrvRequestFlags;
 
+typedef struct BlockSizes {
+uint32_t phys;
+uint32_t log;
+} BlockSizes;
+
+typedef struct HDGeometry {
+uint32_t heads;
+uint32_t sectors;
+uint32_t cylinders;
+} HDGeometry;
+
 #define BDRV_O_RDWR0x0002
 #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
in a snapshot */
 #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
@@ -550,6 +561,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
  * the old #AioContext is not executing.
  */
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
+int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
 
 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7ad1950..5321b02 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -273,6 +273,21 @@ struct BlockDriver {
 void (*bdrv_io_unplug)(BlockDriverState *bs);
 void (*bdrv_flush_io_queue)(BlockDriverState *bs);
 
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz);
+/**
+ * Try to get @bs's geometry (cyls, heads, sectors)
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * Only drivers that want to override guest geometry implement this
+ * callback; see hd_geometry_guess().
+ */
+int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
+
 QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
2.1.4

[Qemu-devel] [PATCH v7 4/5] block-backend: Add wrappers for blocksizes and geometry probing

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
---
 block/block-backend.c  | 10 ++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index c28e240..9ffd469 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -668,3 +668,13 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, 
BlockBackend *blk,
 {
 return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
 }
+
+int blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz)
+{
+return bdrv_probe_blocksizes(blk->bs, bsz);
+}
+
+int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo)
+{
+return bdrv_probe_geometry(blk->bs, geo);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index aab12b9..909bb7c 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -151,5 +151,7 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk);
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque);
+int blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz);
+int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo);
 
 #endif
-- 
2.1.4

[Qemu-devel] [PATCH v7 2/5] raw-posix: Factor block size detection out of raw_probe_alignment()

Put it in new probe_logical_blocksize().

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
---
 block/raw-posix.c | 51 +++
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e474c17..c306897 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -218,39 +218,58 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
-static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+/*
+ * Get logical block size via ioctl. On success store it in @sector_size_p.
+ */
+static int probe_logical_blocksize(int fd, unsigned int *sector_size_p)
 {
-BDRVRawState *s = bs->opaque;
-char *buf;
 unsigned int sector_size;
+bool success = false;
 
-/* For /dev/sg devices the alignment is not really used.
-   With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !s->needs_alignment) {
-bs->request_alignment = 1;
-s->buf_align = 1;
-return;
-}
+errno = ENOTSUP;
 
 /* Try a few ioctls to get the right size */
-bs->request_alignment = 0;
-s->buf_align = 0;
-
 #ifdef BLKSSZGET
 if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
 #ifdef DKIOCGETBLOCKSIZE
 if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
 #ifdef DIOCGSECTORSIZE
 if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
+
+return success ? 0 : -errno;
+}
+
+static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !s->needs_alignment) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+bs->request_alignment = 0;
+s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) {
+bs->request_alignment = 0;
+}
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
-- 
2.1.4

Re: [Qemu-devel] [PATCH v6 5/5] BlockConf: Call backend functions to detect geometry and blocksizes

2015-02-13 Thread Ekaterina Tumanova


On 02/13/2015 03:23 PM, Christian Borntraeger wrote:

Am 19.01.2015 um 15:35 schrieb Ekaterina Tumanova:

geometry: hd_geometry_guess function autodetects the drive geometry.
This patch adds a block backend call, that probes the backing device
geometry. If the inner driver method is implemented and succeeds
(currently only for DASDs), the blkconf_geometry will pass-through
the backing device geometry. Otherwise will fallback to old logic.

blocksize: This patch initializes blocksize properties to 0.
In order to set the properly a blkconf_blocksizes was introduced.
If user didn't set physical or logical blocksize, it will
retrieve its value from a driver (which will return a default 512 for
any backend but DASD).

The blkconf_blocksizes call was added to all users of BlkConf.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
---
  hw/block/block.c | 20 
  hw/block/hd-geometry.c   | 10 +-
  hw/block/nvme.c  |  1 +
  hw/block/virtio-blk.c|  1 +
  hw/core/qdev-properties.c|  3 ++-
  hw/ide/qdev.c|  1 +
  hw/scsi/scsi-disk.c  |  1 +
  hw/usb/dev-storage.c |  1 +
  include/hw/block/block.h |  5 +++--
  include/hw/qdev-properties.h |  4 ++--
  10 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..09dd5f1 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,26 @@ void blkconf_serial(BlockConf *conf, char **serial)
  }
  }

+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+BlockSizes blocksizes;
+int backend_ret;
+
+backend_ret = blk_probe_blocksizes(blk, &blocksizes);
+/* fill in detected values if they are not defined via qemu command line */
+if (!conf->physical_block_size && !backend_ret) {
+conf->physical_block_size = blocksizes.phys;
+} else {
+conf->physical_block_size = BDRV_SECTOR_SIZE;
+}
+if (!conf->logical_block_size && !backend_ret) {
+conf->logical_block_size = blocksizes.log;
+} else {
+conf->logical_block_size = BDRV_SECTOR_SIZE;
+}


When we are going to fix this, I found another bug:

This code will fail when logical_block_size and physical_block_size are given 
at the command line AND detection (backend_ret != 0) did not work. It will use 
BDRV_SECTOR_SIZE instead of the command line value.
With something like

--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -33,15 +33,19 @@ void blkconf_blocksizes(BlockConf *conf)

  backend_ret = blk_probe_blocksizes(blk, &blocksizes);
  /* fill in detected values if they are not defined via qemu command line 
*/
-if (!conf->physical_block_size && !backend_ret) {
-conf->physical_block_size = blocksizes.phys;
-} else {
-conf->physical_block_size = BDRV_SECTOR_SIZE;
+if (!conf->physical_block_size) {
+if (!backend_ret) {
+conf->physical_block_size = blocksizes.phys;
+} else {
+conf->physical_block_size = BDRV_SECTOR_SIZE;
+}
  }
-if (!conf->logical_block_size && !backend_ret) {
-conf->logical_block_size = blocksizes.log;
-} else {
-conf->logical_block_size = BDRV_SECTOR_SIZE;
+if (!conf->logical_block_size) {
+   if (!backend_ret) {
+   conf->logical_block_size = blocksizes.log;
+   } else {
+   conf->logical_block_size = BDRV_SECTOR_SIZE;
+   }
  }
  }


No?



yes.
will be fix in v7.

Re: [Qemu-devel] [PATCH v6 0/5] Geometry and blocksize detection for backing devices.

2015-02-04 Thread Ekaterina Tumanova


On 01/19/2015 05:34 PM, Ekaterina Tumanova wrote:

Updates v5 -> v6:

Minor Updates according the last review from Stefan Hajnoczi:
1. Do not change the flow of code, factored out of raw_probe_alignment.
2. added #ifdef __linux__ in 2 places of raw-posix.c, mentioned by reviewer.
3. adjusted the comment hdev_probe_geometry according suggestment.
4. use bdrv_nb_sectors(bs) instead of bs->total_sectors.
5. do not discard error blk_probe_blocksizes(). now has rc.
6. put the 512-byte default blocksize value in blkconf_blocksizes.
7. drop the default parameter from the DEFINE_PROP_BLOCKSIZE() macro.

Thanks,
Kate.

---
Patchset Description (didn't change):

Proper geometry and blocksize information is vital for support of
DASD/ECKD drives in Linux guests. Otherwise things will fail in
certain cases.

The existing geometry and blocksize qemu defaults have no sense
for DASD drives (hd_geometry_guess detection and 512 for sizes).
Setting this information manually in XML file is far from user-friendly,
as the admin has to manually look up the properties of the
host disks and then modify the guest definition accordingly.

Since Linux uses DASDs as normal block devices, we actually
want to use virtio-blk to pass those to KVM guests.

In order to avoid any change in behavior of other drives, the DASD
special casing was advised. We call ioctl BIODASDINFO2 on the block
device, which will only succeed if the device is really a DASD.

In order to retrieve the underlying device geometry and blocksizes
a new block-backend functions and underlying driver functions were
introduced (blk_probe_blocksizes anf blk_probe_geometry wrappers
and corresponding bdrv_xx functions).

As for now only "host_device" driver received new detection methods.
For "raw" we call childs method as usual. In future one may update
other drivers to add some other detection heuristics.

If the host_device appears to be a DASD, the driver functions
(hdev_probe_blocksizes and hdev_probe_geometry) will call certain
ioctls in order to detect geometry and blocksizes of the underlying device.
if probing failed bdrv_probe_blocksizes caller will set defaults,
and bdrv_probe_geometry will fail to allow fallback to old detection logic.

The front-end (BlockConf API) was updated:
1. a new blkconf_blocksizes function was added. It doesn't
change user-defined blocksize values. If properties are unset, it will
set values, returned by blk_probe_backend. In order to allow this logic,
blocksize properties were initialized with 0. (driver will return 512 if
backing device probing didn't succeed or if driver method is not defined).
2. hd_geometry guess was changed to firstly try to retrieve values via
blk_probe_geometry and if it fails, fallback to the old logic.

Ekaterina Tumanova (5):
   block: add bdrv functions for geometry and blocksize
   raw-posix: Factor block size detection out of raw_probe_alignment()
   block: Add driver methods to probe blocksizes and geometry
   block-backend: Add wrappers for blocksizes and geometry probing
   BlockConf: Call backend functions to detect geometry and blocksizes

  block.c|  36 ++
  block/block-backend.c  |  10 +++
  block/raw-posix.c  | 154 -
  block/raw_bsd.c|  12 
  hw/block/block.c   |  20 ++
  hw/block/hd-geometry.c |  10 ++-
  hw/block/nvme.c|   1 +
  hw/block/virtio-blk.c  |   1 +
  hw/core/qdev-properties.c  |   3 +-
  hw/ide/qdev.c  |   1 +
  hw/scsi/scsi-disk.c|   1 +
  hw/usb/dev-storage.c   |   1 +
  include/block/block.h  |  13 
  include/block/block_int.h  |  15 
  include/hw/block/block.h   |   5 +-
  include/hw/qdev-properties.h   |   4 +-
  include/sysemu/block-backend.h |   2 +
  17 files changed, 267 insertions(+), 22 deletions(-)



ping

[Qemu-devel] [PATCH v6 5/5] BlockConf: Call backend functions to detect geometry and blocksizes

geometry: hd_geometry_guess function autodetects the drive geometry.
This patch adds a block backend call, that probes the backing device
geometry. If the inner driver method is implemented and succeeds
(currently only for DASDs), the blkconf_geometry will pass-through
the backing device geometry. Otherwise will fallback to old logic.

blocksize: This patch initializes blocksize properties to 0.
In order to set the properly a blkconf_blocksizes was introduced.
If user didn't set physical or logical blocksize, it will
retrieve its value from a driver (which will return a default 512 for
any backend but DASD).

The blkconf_blocksizes call was added to all users of BlkConf.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
---
 hw/block/block.c | 20 
 hw/block/hd-geometry.c   | 10 +-
 hw/block/nvme.c  |  1 +
 hw/block/virtio-blk.c|  1 +
 hw/core/qdev-properties.c|  3 ++-
 hw/ide/qdev.c|  1 +
 hw/scsi/scsi-disk.c  |  1 +
 hw/usb/dev-storage.c |  1 +
 include/hw/block/block.h |  5 +++--
 include/hw/qdev-properties.h |  4 ++--
 10 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..09dd5f1 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,26 @@ void blkconf_serial(BlockConf *conf, char **serial)
 }
 }
 
+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+BlockSizes blocksizes;
+int backend_ret;
+
+backend_ret = blk_probe_blocksizes(blk, &blocksizes);
+/* fill in detected values if they are not defined via qemu command line */
+if (!conf->physical_block_size && !backend_ret) {
+conf->physical_block_size = blocksizes.phys;
+} else {
+conf->physical_block_size = BDRV_SECTOR_SIZE;
+}
+if (!conf->logical_block_size && !backend_ret) {
+conf->logical_block_size = blocksizes.log;
+} else {
+conf->logical_block_size = BDRV_SECTOR_SIZE;
+}
+}
+
 void blkconf_geometry(BlockConf *conf, int *ptrans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..b187878 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,8 +121,16 @@ void hd_geometry_guess(BlockBackend *blk,
int *ptrans)
 {
 int cylinders, heads, secs, translation;
+HDGeometry geo;
 
-if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
+/* Try to probe the backing device geometry, otherwise fallback
+   to the old logic. (as of 12/2014 probing only succeeds on DASDs) */
+if (blk_probe_geometry(blk, &geo) == 0) {
+*pcyls = geo.cylinders;
+*psecs = geo.sectors;
+*pheads = geo.heads;
+translation = BIOS_ATA_TRANSLATION_NONE;
+} else if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
 guess_chs_for_size(blk, pcyls, pheads, psecs);
 translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index ce079ae..0f3dfb9 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -765,6 +765,7 @@ static int nvme_init(PCIDevice *pci_dev)
 if (!n->serial) {
 return -1;
 }
+blkconf_blocksizes(&n->conf);
 
 pci_conf = pci_dev->config;
 pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..6f01565 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -745,6 +745,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 error_propagate(errp, err);
 return;
 }
+blkconf_blocksizes(&conf->conf);
 
 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2e47f70..ba81709 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -580,7 +580,8 @@ static void set_blocksize(Object *obj, Visitor *v, void 
*opaque,
 error_propagate(errp, local_err);
 return;
 }
-if (value < min || value > max) {
+/* value of 0 means "unset" */
+if (value && (value < min || value > max)) {
 error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
   dev->id?:"", name, (int64_t)value, min, max);
 return;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1ebb58d..353854c 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -169,6 +169,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 }
 
 blkconf_serial(&dev->conf, &dev->

[Qemu-devel] [PATCH v6 1/5] block: add bdrv functions for geometry and blocksize

Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Thomas Huth 
Reviewed-by: Markus Armbruster 
---
 block.c   | 36 
 include/block/block.h | 13 +
 include/block/block_int.h | 15 +++
 3 files changed, 64 insertions(+)

diff --git a/block.c b/block.c
index cbe4a32..7e60892 100644
--- a/block.c
+++ b/block.c
@@ -569,6 +569,42 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz struct and return 0.
+ * On failure return -errno.
+ * @bs must not be empty.
+ */
+int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_blocksizes) {
+return drv->bdrv_probe_blocksizes(bs, bsz);
+}
+
+return -ENOTSUP;
+}
+
+/**
+ * Try to get @bs's geometry (cyls, heads, sectors).
+ * On success, store them in @geo struct and return 0.
+ * On failure return -errno.
+ * @bs must not be empty.
+ */
+int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_geometry) {
+return drv->bdrv_probe_geometry(bs, geo);
+}
+
+return -ENOTSUP;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
diff --git a/include/block/block.h b/include/block/block.h
index 3082d2b..f445a10 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -60,6 +60,17 @@ typedef enum {
 BDRV_REQ_MAY_UNMAP= 0x4,
 } BdrvRequestFlags;
 
+typedef struct BlockSizes {
+uint32_t phys;
+uint32_t log;
+} BlockSizes;
+
+typedef struct HDGeometry {
+uint32_t heads;
+uint32_t sectors;
+uint32_t cylinders;
+} HDGeometry;
+
 #define BDRV_O_RDWR0x0002
 #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
in a snapshot */
 #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
@@ -546,6 +557,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
  * the old #AioContext is not executing.
  */
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
+int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
 
 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 06a21dd..b216156 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -273,6 +273,21 @@ struct BlockDriver {
 void (*bdrv_io_unplug)(BlockDriverState *bs);
 void (*bdrv_flush_io_queue)(BlockDriverState *bs);
 
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz);
+/**
+ * Try to get @bs's geometry (cyls, heads, sectors)
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * Only drivers that want to override guest geometry implement this
+ * callback; see hd_geometry_guess().
+ */
+int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
+
 QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
2.1.4

[Qemu-devel] [PATCH v6 2/5] raw-posix: Factor block size detection out of raw_probe_alignment()

Put it in new probe_logical_blocksize().

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
---
 block/raw-posix.c | 51 +++
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e51293a..48525f8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -217,39 +217,58 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
-static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+/*
+ * Get logical block size via ioctl. On success store it in @sector_size_p.
+ */
+static int probe_logical_blocksize(int fd, unsigned int *sector_size_p)
 {
-BDRVRawState *s = bs->opaque;
-char *buf;
 unsigned int sector_size;
+bool success = false;
 
-/* For /dev/sg devices the alignment is not really used.
-   With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !s->needs_alignment) {
-bs->request_alignment = 1;
-s->buf_align = 1;
-return;
-}
+errno = ENOTSUP;
 
 /* Try a few ioctls to get the right size */
-bs->request_alignment = 0;
-s->buf_align = 0;
-
 #ifdef BLKSSZGET
 if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
 #ifdef DKIOCGETBLOCKSIZE
 if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
 #ifdef DIOCGSECTORSIZE
 if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
+
+return success ? 0 : -errno;
+}
+
+static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !s->needs_alignment) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+bs->request_alignment = 0;
+s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) {
+bs->request_alignment = 0;
+}
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
-- 
2.1.4

[Qemu-devel] [PATCH v6 0/5] Geometry and blocksize detection for backing devices.

Updates v5 -> v6:

Minor Updates according the last review from Stefan Hajnoczi:
1. Do not change the flow of code, factored out of raw_probe_alignment.
2. added #ifdef __linux__ in 2 places of raw-posix.c, mentioned by reviewer.
3. adjusted the comment hdev_probe_geometry according suggestment.
4. use bdrv_nb_sectors(bs) instead of bs->total_sectors.
5. do not discard error blk_probe_blocksizes(). now has rc.
6. put the 512-byte default blocksize value in blkconf_blocksizes.
7. drop the default parameter from the DEFINE_PROP_BLOCKSIZE() macro.

Thanks,
Kate.

---
   Patchset Description (didn't change):

Proper geometry and blocksize information is vital for support of
DASD/ECKD drives in Linux guests. Otherwise things will fail in
certain cases.

The existing geometry and blocksize qemu defaults have no sense
for DASD drives (hd_geometry_guess detection and 512 for sizes).
Setting this information manually in XML file is far from user-friendly,
as the admin has to manually look up the properties of the
host disks and then modify the guest definition accordingly.

Since Linux uses DASDs as normal block devices, we actually
want to use virtio-blk to pass those to KVM guests.

In order to avoid any change in behavior of other drives, the DASD
special casing was advised. We call ioctl BIODASDINFO2 on the block
device, which will only succeed if the device is really a DASD.

In order to retrieve the underlying device geometry and blocksizes
a new block-backend functions and underlying driver functions were
introduced (blk_probe_blocksizes anf blk_probe_geometry wrappers
and corresponding bdrv_xx functions).

As for now only "host_device" driver received new detection methods.
For "raw" we call childs method as usual. In future one may update
other drivers to add some other detection heuristics.

If the host_device appears to be a DASD, the driver functions
(hdev_probe_blocksizes and hdev_probe_geometry) will call certain
ioctls in order to detect geometry and blocksizes of the underlying device.
if probing failed bdrv_probe_blocksizes caller will set defaults,
and bdrv_probe_geometry will fail to allow fallback to old detection logic.

The front-end (BlockConf API) was updated:
1. a new blkconf_blocksizes function was added. It doesn't
change user-defined blocksize values. If properties are unset, it will
set values, returned by blk_probe_backend. In order to allow this logic,
blocksize properties were initialized with 0. (driver will return 512 if
backing device probing didn't succeed or if driver method is not defined).
2. hd_geometry guess was changed to firstly try to retrieve values via
blk_probe_geometry and if it fails, fallback to the old logic.

Ekaterina Tumanova (5):
  block: add bdrv functions for geometry and blocksize
  raw-posix: Factor block size detection out of raw_probe_alignment()
  block: Add driver methods to probe blocksizes and geometry
  block-backend: Add wrappers for blocksizes and geometry probing
  BlockConf: Call backend functions to detect geometry and blocksizes

 block.c|  36 ++
 block/block-backend.c  |  10 +++
 block/raw-posix.c  | 154 -
 block/raw_bsd.c|  12 
 hw/block/block.c   |  20 ++
 hw/block/hd-geometry.c |  10 ++-
 hw/block/nvme.c|   1 +
 hw/block/virtio-blk.c  |   1 +
 hw/core/qdev-properties.c  |   3 +-
 hw/ide/qdev.c  |   1 +
 hw/scsi/scsi-disk.c|   1 +
 hw/usb/dev-storage.c   |   1 +
 include/block/block.h  |  13 
 include/block/block_int.h  |  15 
 include/hw/block/block.h   |   5 +-
 include/hw/qdev-properties.h   |   4 +-
 include/sysemu/block-backend.h |   2 +
 17 files changed, 267 insertions(+), 22 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v6 3/5] block: Add driver methods to probe blocksizes and geometry

Introduce driver methods of defining disk blocksizes (physical and
logical) and hard drive geometry.
Methods are only implemented for "host_device". For "raw" devices
driver calls child's method.

For now geometry detection will only work for DASD devices. To check
that a local check_for_dasd function was introduced. It calls BIODASDINFO2
ioctl and returns its rc.

Blocksizes detection function will probe sizes for DASD devices and
set default for other devices.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
---
 block/raw-posix.c | 103 ++
 block/raw_bsd.c   |  12 +++
 2 files changed, 115 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 48525f8..86ba479 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,10 @@
 #include 
 #include 
 #include 
+#include 
+#ifdef __s390__
+#include 
+#endif
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
@@ -250,6 +254,23 @@ static int probe_logical_blocksize(int fd, unsigned int 
*sector_size_p)
 return success ? 0 : -errno;
 }
 
+/**
+ * Get physical block size of @fd.
+ * On success, store it in @blk_size and return 0.
+ * On failure, return -errno.
+ */
+static int probe_physical_blocksize(int fd, unsigned int *blk_size)
+{
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
+return -errno;
+}
+return 0;
+#else
+return -ENOTSUP;
+#endif
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
@@ -672,6 +693,86 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 bs->bl.opt_mem_alignment = s->buf_align;
 }
 
+static int check_for_dasd(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#else
+return -1;
+#endif
+}
+
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BDRVRawState *s = bs->opaque;
+int ret;
+
+/* If DASD, get blocksizes */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+ret = probe_logical_blocksize(s->fd, &bsz->log);
+if (ret < 0) {
+return ret;
+}
+return probe_physical_blocksize(s->fd, &bsz->phys);
+}
+
+/**
+ * Try to get @bs's geometry: cyls, heads, sectors.
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * (Allows block driver to assign default geometry values that guest sees)
+ */
+#ifdef __linux__
+static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BDRVRawState *s = bs->opaque;
+struct hd_geometry ioctl_geo = {0};
+uint32_t blksize;
+
+/* If DASD, get its geometry */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
+return -errno;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+return -ENOTSUP;
+}
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+return -ENOTSUP;
+}
+geo->heads = ioctl_geo.heads;
+geo->sectors = ioctl_geo.sectors;
+if (!probe_physical_blocksize(s->fd, &blksize)) {
+/* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */
+geo->cylinders = bdrv_nb_sectors(bs) / (blksize / BDRV_SECTOR_SIZE)
+ / (geo->heads * geo->sectors);
+return 0;
+}
+geo->cylinders = ioctl_geo.cylinders;
+
+return 0;
+}
+#else /* __linux__ */
+static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+return -ENOTSUP;
+}
+#endif
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2137,6 +2238,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,
 
 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 05b02c7..e3d2d04 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -235,6 +235,16 @@ static int raw_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 1;
 }
 
+static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+return bdrv_probe_blocksizes(bs->file,

[Qemu-devel] [PATCH v6 4/5] block-backend: Add wrappers for blocksizes and geometry probing

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
---
 block/block-backend.c  | 10 ++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index d00c129..0f7baea 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -663,3 +663,13 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, 
BlockBackend *blk,
 {
 return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
 }
+
+int blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz)
+{
+return bdrv_probe_blocksizes(blk->bs, bsz);
+}
+
+int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo)
+{
+return bdrv_probe_geometry(blk->bs, geo);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8871a02..2c97726 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -150,5 +150,7 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk);
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque);
+int blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz);
+int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo);
 
 #endif
-- 
2.1.4

Re: [Qemu-devel] [PATCH v5 2/5] raw-posix: Refactor logical block size detection.

2015-01-13 Thread Ekaterina Tumanova


On 01/02/2015 02:52 PM, Stefan Hajnoczi wrote:

On Thu, Dec 18, 2014 at 12:18:01PM +0100, Ekaterina Tumanova wrote:

+#if defined(BLKSSZGET)
+#  define SECTOR_SIZE BLKSSZGET
+#elif defined(DKIOCGETBLOCKSIZE)
+#  define SECTOR_SIZE DKIOCGETBLOCKSIZE
+#elif defined(DIOCGSECTORSIZE)
+#  define SECTOR_SIZE DIOCGSECTORSIZE
+#else
+return -ENOTSUP
+#endif
+if (ioctl(fd, SECTOR_SIZE, sector_size) < 0) {
+return -errno;
+}
+return 0;
+#undef SECTOR_SIZE


Not a reason to respin, but I would have preferred simply moving the old
code.

I think the new code works because BLKSSZGET is Linux, DKIOCGETBLOCKSIZE
is Mac OS, and DIOCGSECTORSIZE is FreeBSD.

If there is a host OS where more than one ioctl is available and the
first one fails then the new code is broken.  The old code didn't use
#elif so each ioctl had a chance to run.



In this case, why should it have a chance to run, if we only use one
result at a time? (Old code overwrites first result with the second)

Plus as far as I understand, in this hypothetical case of 2 ioctls
defined, one will most probably will be a redefinition of another.

p.s. ok to all further comments to patches.

Thanks!
Kate.



Also, the name SECTOR_SIZE is misleading.  It's not a sector size value
but the "get sector size" ioctl request code.

Stefan

Re: [Qemu-devel] [PATCH v5 0/5] Geometry and blocksize detection for backing devices.

2015-01-13 Thread Ekaterina Tumanova


On 01/02/2015 02:30 PM, Stefan Hajnoczi wrote:

On Thu, Dec 18, 2014 at 12:17:59PM +0100, Ekaterina Tumanova wrote:

Updates v4 -> v5:

Minor Updates according the last review from Markus:
1. update commit message for patch 2
2. fix comment typos
3. fix check_for_dasd to return -1 instead of -ENOTSUP

Thanks,
Kate.

---
Patchset Description (didn't change):

Proper geometry and blocksize information is vital for support of
DASD/ECKD drives in Linux guests. Otherwise things will fail in
certain cases.

The existing geometry and blocksize qemu defaults have no sense
for DASD drives (hd_geometry_guess detection and 512 for sizes).
Setting this information manually in XML file is far from user-friendly,
as the admin has to manually look up the properties of the
host disks and then modify the guest definition accordingly.


What is the expected behavior when you dd the disk image from the DASD
to a raw image file on an NFS file system on the host?

It seems you'd lose the geometry detection and the disk image could be
unusable from inside the guest.



This patch set doesn't change the handling of the images, dd'ed from
DASDs. Please refer to Christian's reply for details.

Thanks a lot for you review!

Kate.

[Qemu-devel] [PATCH v5 4/5] block-backend: Add wrappers for blocksizes and geometry probing

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
---
 block/block-backend.c  | 10 ++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index ef16d73..4b9ed85 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -667,3 +667,13 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, 
BlockBackend *blk,
 {
 return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
 }
+
+void blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz)
+{
+bdrv_probe_blocksizes(blk->bs, bsz);
+}
+
+int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo)
+{
+return bdrv_probe_geometry(blk->bs, geo);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8871a02..aecdc53 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -150,5 +150,7 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk);
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque);
+void blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz);
+int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo);
 
 #endif
-- 
1.8.5.5

[Qemu-devel] [PATCH v5 2/5] raw-posix: Refactor logical block size detection.

1. Move block size detection into dedicated function.
2. Select exactly one IOCTL that detects blocksize, specific to the host OS.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
---
 block/raw-posix.c | 41 -
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e51293a..38172ca 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -217,11 +217,31 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
+/*
+ * Get logical block size via ioctl. On success return 0. Otherwise -errno.
+ */
+static int probe_logical_blocksize(int fd, unsigned int *sector_size)
+{
+#if defined(BLKSSZGET)
+#  define SECTOR_SIZE BLKSSZGET
+#elif defined(DKIOCGETBLOCKSIZE)
+#  define SECTOR_SIZE DKIOCGETBLOCKSIZE
+#elif defined(DIOCGSECTORSIZE)
+#  define SECTOR_SIZE DIOCGSECTORSIZE
+#else
+return -ENOTSUP
+#endif
+if (ioctl(fd, SECTOR_SIZE, sector_size) < 0) {
+return -errno;
+}
+return 0;
+#undef SECTOR_SIZE
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
 char *buf;
-unsigned int sector_size;
 
 /* For /dev/sg devices the alignment is not really used.
With buffered I/O, we don't have any restrictions. */
@@ -231,25 +251,12 @@ static void raw_probe_alignment(BlockDriverState *bs, int 
fd, Error **errp)
 return;
 }
 
-/* Try a few ioctls to get the right size */
 bs->request_alignment = 0;
 s->buf_align = 0;
-
-#ifdef BLKSSZGET
-if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
-}
-#endif
-#ifdef DKIOCGETBLOCKSIZE
-if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+/* Let's try to use the logical blocksize for the alignment. */
+if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) {
+bs->request_alignment = 0;
 }
-#endif
-#ifdef DIOCGSECTORSIZE
-if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
-}
-#endif
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
-- 
1.8.5.5

[Qemu-devel] [PATCH v5 5/5] BlockConf: Call backend functions to detect geometry and blocksizes

geometry: hd_geometry_guess function autodetects the drive geometry.
This patch adds a block backend call, that probes the backing device
geometry. If the inner driver method is implemented and succeeds
(currently only for DASDs), the blkconf_geometry will pass-through
the backing device geometry. Otherwise will fallback to old logic.

blocksize: This patch initializes blocksize properties to 0.
In order to set the properly a blkconf_blocksizes was introduced.
If user didn't set physical or logical blocksize, it will
retrieve it's value from a driver (which will return a default 512 for
any backend but DASD).

The blkconf_blocksizes call was added to all users of BlkConf.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
---
 hw/block/block.c  | 15 +++
 hw/block/hd-geometry.c| 10 +-
 hw/block/nvme.c   |  1 +
 hw/block/virtio-blk.c |  1 +
 hw/core/qdev-properties.c |  3 ++-
 hw/ide/qdev.c |  1 +
 hw/scsi/scsi-disk.c   |  1 +
 hw/usb/dev-storage.c  |  1 +
 include/hw/block/block.h  |  5 +++--
 9 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..a4f4f06 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,21 @@ void blkconf_serial(BlockConf *conf, char **serial)
 }
 }
 
+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+BlockSizes blocksizes;
+
+blk_probe_blocksizes(blk, &blocksizes);
+/* fill in detected values if they are not defined via qemu command line */
+if (!conf->physical_block_size) {
+conf->physical_block_size = blocksizes.phys;
+}
+if (!conf->logical_block_size) {
+conf->logical_block_size = blocksizes.log;
+}
+}
+
 void blkconf_geometry(BlockConf *conf, int *ptrans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..b187878 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,8 +121,16 @@ void hd_geometry_guess(BlockBackend *blk,
int *ptrans)
 {
 int cylinders, heads, secs, translation;
+HDGeometry geo;
 
-if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
+/* Try to probe the backing device geometry, otherwise fallback
+   to the old logic. (as of 12/2014 probing only succeeds on DASDs) */
+if (blk_probe_geometry(blk, &geo) == 0) {
+*pcyls = geo.cylinders;
+*psecs = geo.sectors;
+*pheads = geo.heads;
+translation = BIOS_ATA_TRANSLATION_NONE;
+} else if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
 guess_chs_for_size(blk, pcyls, pheads, psecs);
 translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index aa1ed98..2c630bc 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -763,6 +763,7 @@ static int nvme_init(PCIDevice *pci_dev)
 if (!n->serial) {
 return -1;
 }
+blkconf_blocksizes(&n->conf);
 
 pci_conf = pci_dev->config;
 pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..6f01565 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -745,6 +745,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 error_propagate(errp, err);
 return;
 }
+blkconf_blocksizes(&conf->conf);
 
 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2e47f70..ba81709 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -580,7 +580,8 @@ static void set_blocksize(Object *obj, Visitor *v, void 
*opaque,
 error_propagate(errp, local_err);
 return;
 }
-if (value < min || value > max) {
+/* value of 0 means "unset" */
+if (value && (value < min || value > max)) {
 error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
   dev->id?:"", name, (int64_t)value, min, max);
 return;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1ebb58d..353854c 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -169,6 +169,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 }
 
 blkconf_serial(&dev->conf, &dev->serial);
+blkconf_blocksizes(&dev->conf);
 if (kind != IDE_CD) {
 blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
 if (err) {
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index f65618d..e7244e6 100644
--- a/hw/

[Qemu-devel] [PATCH v5 1/5] block: add bdrv functions for geometry and blocksize

Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Thomas Huth 
Reviewed-by: Markus Armbruster 
---
 block.c   | 35 +++
 include/block/block.h | 13 +
 include/block/block_int.h | 15 +++
 3 files changed, 63 insertions(+)

diff --git a/block.c b/block.c
index 4165d42..0586785 100644
--- a/block.c
+++ b/block.c
@@ -548,6 +548,41 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+/**
+ * Get @bs's logical and physical block size, store them in @bsz.
+ * @bs must not be empty.
+ */
+void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_blocksizes &&
+!drv->bdrv_probe_blocksizes(bs, bsz)) {
+return;
+}
+bsz->log = BDRV_SECTOR_SIZE;
+bsz->phys = BDRV_SECTOR_SIZE;
+}
+
+/**
+ * Try to get @bs's geometry (cyls, heads, sectors)
+ * On success, store them in @geo struct and return 0.
+ * On failure return -errno.
+ * @bs must not be empty.
+ */
+int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_geometry) {
+return drv->bdrv_probe_geometry(bs, geo);
+}
+
+return -ENOTSUP;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
diff --git a/include/block/block.h b/include/block/block.h
index 6e7275d..14ac3b1 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -60,6 +60,17 @@ typedef enum {
 BDRV_REQ_MAY_UNMAP= 0x4,
 } BdrvRequestFlags;
 
+typedef struct BlockSizes {
+uint32_t phys;
+uint32_t log;
+} BlockSizes;
+
+typedef struct HDGeometry {
+uint32_t heads;
+uint32_t sectors;
+uint32_t cylinders;
+} HDGeometry;
+
 #define BDRV_O_RDWR0x0002
 #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
in a snapshot */
 #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
@@ -539,6 +550,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
  * the old #AioContext is not executing.
  */
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
+int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
 
 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 06a21dd..b216156 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -273,6 +273,21 @@ struct BlockDriver {
 void (*bdrv_io_unplug)(BlockDriverState *bs);
 void (*bdrv_flush_io_queue)(BlockDriverState *bs);
 
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz);
+/**
+ * Try to get @bs's geometry (cyls, heads, sectors)
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * Only drivers that want to override guest geometry implement this
+ * callback; see hd_geometry_guess().
+ */
+int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
+
 QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
1.8.5.5

[Qemu-devel] [PATCH v5 3/5] block: Add driver methods to probe blocksizes and geometry

Introduce driver methods of defining disk blocksizes (physical and
logical) and hard drive geometry.
Methods are only implemented for "host_device". For "raw" devices
driver calls child's method.

For now geometry detection will only work for DASD devices. To check
that a local check_for_dasd function was introduced. It calls BIODASDINFO2
ioctl and returns its rc.

Blocksizes detection function will probe sizes for DASD devices and
set default for other devices.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
---
 block/raw-posix.c | 97 +++
 block/raw_bsd.c   | 14 
 2 files changed, 111 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 38172ca..1783acf 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,7 @@
 #include 
 #include 
 #include 
+#include 
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
@@ -90,6 +91,10 @@
 #include 
 #endif
 
+#ifdef __s390__
+#include 
+#endif
+
 //#define DEBUG_FLOPPY
 
 //#define DEBUG_BLOCK
@@ -238,6 +243,23 @@ static int probe_logical_blocksize(int fd, unsigned int 
*sector_size)
 #undef SECTOR_SIZE
 }
 
+/**
+ * Get physical block size of @fd.
+ * On success, store it in @blk_size and return 0.
+ * On failure, return -errno.
+ */
+static int probe_physical_blocksize(int fd, unsigned int *blk_size)
+{
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
+return -errno;
+}
+return 0;
+#else
+return -ENOTSUP;
+#endif
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
@@ -660,6 +682,79 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 bs->bl.opt_mem_alignment = s->buf_align;
 }
 
+static int check_for_dasd(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#else
+return -1;
+#endif
+}
+
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BDRVRawState *s = bs->opaque;
+int ret;
+
+/* If DASD, get blocksizes */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+ret = probe_logical_blocksize(s->fd, &bsz->log);
+if (ret < 0) {
+return ret;
+}
+return probe_physical_blocksize(s->fd, &bsz->phys);
+}
+
+/**
+ * Try to get @bs's geometry (cyls, heads, sectos)
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * (as for 12/2014 only DASDs are supported)
+ */
+static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BDRVRawState *s = bs->opaque;
+struct hd_geometry ioctl_geo = {0};
+uint32_t blksize;
+
+/* If DASD, get it's geometry */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
+return -errno;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+return -ENOTSUP;
+}
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+return -ENOTSUP;
+}
+geo->heads = ioctl_geo.heads;
+geo->sectors = ioctl_geo.sectors;
+if (!probe_physical_blocksize(s->fd, &blksize)) {
+/* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */
+geo->cylinders = bs->total_sectors / (blksize / BDRV_SECTOR_SIZE)
+   / (geo->heads * geo->sectors);
+return 0;
+}
+geo->cylinders = ioctl_geo.cylinders;
+
+return 0;
+}
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2125,6 +2220,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,
 
 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 05b02c7..f3d532b 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -235,6 +235,18 @@ static int raw_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 1;
 }
 
+static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+bdrv_probe_blocksizes(bs->file, bsz);
+
+return 0;
+}
+
+static int raw_probe_geometry(BlockDriverState *bs, HDGeometry *g

[Qemu-devel] [PATCH v5 0/5] Geometry and blocksize detection for backing devices.

Updates v4 -> v5:

Minor Updates according the last review from Markus:
1. update commit message for patch 2
2. fix comment typos
3. fix check_for_dasd to return -1 instead of -ENOTSUP

Thanks,
Kate.

---
   Patchset Description (didn't change):

Proper geometry and blocksize information is vital for support of
DASD/ECKD drives in Linux guests. Otherwise things will fail in
certain cases.

The existing geometry and blocksize qemu defaults have no sense
for DASD drives (hd_geometry_guess detection and 512 for sizes).
Setting this information manually in XML file is far from user-friendly,
as the admin has to manually look up the properties of the
host disks and then modify the guest definition accordingly.

Since Linux uses DASDs as normal block devices, we actually
want to use virtio-blk to pass those to KVM guests.

In order to avoid any change in behavior of other drives, the DASD
special casing was advised. We call ioctl BIODASDINFO2 on the block
device, which will only succeed if the device is really a DASD.

In order to retrieve the underlying device geometry and blocksizes
a new block-backend functions and underlying driver functions were
introduced (blk_probe_blocksizes anf blk_probe_geometry wrappers
and corresponding bdrv_xx functions).

As for now only "host_device" driver received new detection methods.
For "raw" we call childs method as usual. In future one may update
other drivers to add some other detection heuristics.

If the host_device appears to be a DASD, the driver functions
(hdev_probe_blocksizes and hdev_probe_geometry) will call certain
ioctls in order to detect geometry and blocksizes of the underlying device.
if probing failed bdrv_probe_blocksizes caller will set defaults,
and bdrv_probe_geometry will fail to allow fallback to old detection logic.

The front-end (BlockConf API) was updated:
1. a new blkconf_blocksizes function was added. It doesn't
change user-defined blocksize values. If properties are unset, it will
set values, returned by blk_probe_backend. In order to allow this logic,
blocksize properties were initialized with 0. (driver will return 512 if
backing device probing didn't succeed or if driver method is not defined).
2. hd_geometry guess was changed to firstly try to retrieve values via
blk_probe_geometry and if it fails, fallback to the old logic.

Ekaterina Tumanova (5):
  block: add bdrv functions for geometry and blocksize
  raw-posix: Refactor logical block size detection.
  block: Add driver methods to probe blocksizes and geometry
  block-backend: Add wrappers for blocksizes and geometry probing
  BlockConf: Call backend functions to detect geometry and blocksizes

 block.c|  35 +++
 block/block-backend.c  |  10 +++
 block/raw-posix.c  | 138 -
 block/raw_bsd.c|  14 +
 hw/block/block.c   |  15 +
 hw/block/hd-geometry.c |  10 ++-
 hw/block/nvme.c|   1 +
 hw/block/virtio-blk.c  |   1 +
 hw/core/qdev-properties.c  |   3 +-
 hw/ide/qdev.c  |   1 +
 hw/scsi/scsi-disk.c|   1 +
 hw/usb/dev-storage.c   |   1 +
 include/block/block.h  |  13 
 include/block/block_int.h  |  15 +
 include/hw/block/block.h   |   5 +-
 include/sysemu/block-backend.h |   2 +
 16 files changed, 244 insertions(+), 21 deletions(-)

-- 
1.8.5.5

[Qemu-devel] [PATCH v4 5/5] BlockConf: Call backend functions to detect geometry and blocksizes

geometry: hd_geometry_guess function autodetects the drive geometry.
This patch adds a block backend call, that probes the backing device
geometry. If the inner driver method is implemented and succeeds
(currently only for DASDs), the blkconf_geometry will pass-through
the backing device geometry. Otherwise will fallback to old logic.

blocksize: This patch initializes blocksize properties to 0.
In order to set the properly a blkconf_blocksizes was introduced.
If user didn't set physical or logical blocksize, it will
retrieve it's value from a driver (which will return a default 512 for
any backend but DASD).

The blkconf_blocksizes call was added to all users of BlkConf.

Signed-off-by: Ekaterina Tumanova 
---
 hw/block/block.c  | 15 +++
 hw/block/hd-geometry.c| 10 +-
 hw/block/nvme.c   |  1 +
 hw/block/virtio-blk.c |  1 +
 hw/core/qdev-properties.c |  3 ++-
 hw/ide/qdev.c |  1 +
 hw/scsi/scsi-disk.c   |  1 +
 hw/usb/dev-storage.c  |  1 +
 include/hw/block/block.h  |  5 +++--
 9 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..a4f4f06 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,21 @@ void blkconf_serial(BlockConf *conf, char **serial)
 }
 }
 
+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+BlockSizes blocksizes;
+
+blk_probe_blocksizes(blk, &blocksizes);
+/* fill in detected values if they are not defined via qemu command line */
+if (!conf->physical_block_size) {
+conf->physical_block_size = blocksizes.phys;
+}
+if (!conf->logical_block_size) {
+conf->logical_block_size = blocksizes.log;
+}
+}
+
 void blkconf_geometry(BlockConf *conf, int *ptrans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..b187878 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,8 +121,16 @@ void hd_geometry_guess(BlockBackend *blk,
int *ptrans)
 {
 int cylinders, heads, secs, translation;
+HDGeometry geo;
 
-if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
+/* Try to probe the backing device geometry, otherwise fallback
+   to the old logic. (as of 12/2014 probing only succeeds on DASDs) */
+if (blk_probe_geometry(blk, &geo) == 0) {
+*pcyls = geo.cylinders;
+*psecs = geo.sectors;
+*pheads = geo.heads;
+translation = BIOS_ATA_TRANSLATION_NONE;
+} else if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
 guess_chs_for_size(blk, pcyls, pheads, psecs);
 translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index aa1ed98..2c630bc 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -763,6 +763,7 @@ static int nvme_init(PCIDevice *pci_dev)
 if (!n->serial) {
 return -1;
 }
+blkconf_blocksizes(&n->conf);
 
 pci_conf = pci_dev->config;
 pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..6f01565 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -745,6 +745,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 error_propagate(errp, err);
 return;
 }
+blkconf_blocksizes(&conf->conf);
 
 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2e47f70..ba81709 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -580,7 +580,8 @@ static void set_blocksize(Object *obj, Visitor *v, void 
*opaque,
 error_propagate(errp, local_err);
 return;
 }
-if (value < min || value > max) {
+/* value of 0 means "unset" */
+if (value && (value < min || value > max)) {
 error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
   dev->id?:"", name, (int64_t)value, min, max);
 return;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1ebb58d..353854c 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -169,6 +169,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 }
 
 blkconf_serial(&dev->conf, &dev->serial);
+blkconf_blocksizes(&dev->conf);
 if (kind != IDE_CD) {
 blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
 if (err) {
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 2f75d7d..5288129 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scs

[Qemu-devel] [PATCH v4 3/5] block: Add driver methods to probe blocksizes and geometry

Introduce driver methods of defining disk blocksizes (physical and
logical) and hard drive geometry.
Methods are only implemented for "host_device". For "raw" devices
driver calls child's method.

For now geometry detection will only work for DASD devices. To check
that a local check_for_dasd function was introduced. It calls BIODASDINFO2
ioctl and returns its rc.

Blocksizes detection function will probe sizes for DASD devices and
set default for other devices.

Signed-off-by: Ekaterina Tumanova 
---
 block/raw-posix.c | 97 +++
 block/raw_bsd.c   | 14 
 2 files changed, 111 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 38172ca..e1e7b29 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,7 @@
 #include 
 #include 
 #include 
+#include 
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
@@ -90,6 +91,10 @@
 #include 
 #endif
 
+#ifdef __s390__
+#include 
+#endif
+
 //#define DEBUG_FLOPPY
 
 //#define DEBUG_BLOCK
@@ -238,6 +243,23 @@ static int probe_logical_blocksize(int fd, unsigned int 
*sector_size)
 #undef SECTOR_SIZE
 }
 
+/**
+ * Get physical block size of @fd.
+ * On success, store it in @blk_size and return 0.
+ * On failure, return -errno.
+ */
+static int probe_physical_blocksize(int fd, unsigned int *blk_size)
+{
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
+return -errno;
+}
+return 0;
+#else
+return -ENOTSUP;
+#endif
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
@@ -660,6 +682,79 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 bs->bl.opt_mem_alignment = s->buf_align;
 }
 
+static int check_for_dasd(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#else
+return -ENOTSUP;
+#endif
+}
+
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BDRVRawState *s = bs->opaque;
+int ret;
+
+/* If DASD, get blocksizes */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+ret = probe_logical_blocksize(s->fd, &bsz->log);
+if (ret < 0) {
+return ret;
+}
+return probe_physical_blocksize(s->fd, &bsz->phys);
+}
+
+/**
+ * Try to get @bs's geometry (cyls, heads, sectos)
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * (as for 12/2014 only DASDs are supported)
+ */
+static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BDRVRawState *s = bs->opaque;
+struct hd_geometry ioctl_geo = {0};
+uint32_t blksize;
+
+/* If DASD, get it's geometry */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
+return -errno;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+return -ENOTSUP;
+}
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+return -ENOTSUP;
+}
+geo->heads = ioctl_geo.heads;
+geo->sectors = ioctl_geo.sectors;
+if (!probe_physical_blocksize(s->fd, &blksize)) {
+/* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */
+geo->cylinders = bs->total_sectors / (blksize / BDRV_SECTOR_SIZE)
+   / (geo->heads * geo->sectors);
+return 0;
+}
+geo->cylinders = ioctl_geo.cylinders;
+
+return 0;
+}
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2125,6 +2220,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,
 
 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 05b02c7..f3d532b 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -235,6 +235,18 @@ static int raw_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 1;
 }
 
+static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+bdrv_probe_blocksizes(bs->file, bsz);
+
+return 0;
+}
+
+static int raw_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+return bdrv_probe_geometry(bs

[Qemu-devel] [PATCH v4 4/5] block-backend: Add wrappers for blocksizes and geometry probing

Signed-off-by: Ekaterina Tumanova 
---
 block/block-backend.c  | 10 ++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index ef16d73..4b9ed85 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -667,3 +667,13 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, 
BlockBackend *blk,
 {
 return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
 }
+
+void blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz)
+{
+bdrv_probe_blocksizes(blk->bs, bsz);
+}
+
+int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo)
+{
+return bdrv_probe_geometry(blk->bs, geo);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8871a02..aecdc53 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -150,5 +150,7 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk);
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque);
+void blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz);
+int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo);
 
 #endif
-- 
1.8.5.5

[Qemu-devel] [PATCH v4 1/5] block: add bdrv functions for geometry and blocksize

Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Thomas Huth 
---
 block.c   | 34 ++
 include/block/block.h | 13 +
 include/block/block_int.h | 15 +++
 3 files changed, 62 insertions(+)

diff --git a/block.c b/block.c
index 4165d42..93409f5 100644
--- a/block.c
+++ b/block.c
@@ -548,6 +548,40 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+/**
+ * Get @bs's logical and physical block size, store them in @bsz.
+ * @bs must not be empty.
+ */
+void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_blocksizes &&
+!drv->bdrv_probe_blocksizes(bs, bsz)) {
+return;
+}
+bsz->log = BDRV_SECTOR_SIZE;
+bsz->phys = BDRV_SECTOR_SIZE;
+}
+
+/**
+ * Try to get @bs's geometry (cyls, heads, sectos)
+ * On success, store them in @geo struct and return 0.
+ * On failure return -errno. @bs must not be empty.
+ */
+int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_geometry) {
+return drv->bdrv_probe_geometry(bs, geo);
+}
+
+return -ENOTSUP;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
diff --git a/include/block/block.h b/include/block/block.h
index 6e7275d..14ac3b1 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -60,6 +60,17 @@ typedef enum {
 BDRV_REQ_MAY_UNMAP= 0x4,
 } BdrvRequestFlags;
 
+typedef struct BlockSizes {
+uint32_t phys;
+uint32_t log;
+} BlockSizes;
+
+typedef struct HDGeometry {
+uint32_t heads;
+uint32_t sectors;
+uint32_t cylinders;
+} HDGeometry;
+
 #define BDRV_O_RDWR0x0002
 #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
in a snapshot */
 #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
@@ -539,6 +550,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
  * the old #AioContext is not executing.
  */
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
+int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
 
 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 06a21dd..c2c5f0e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -273,6 +273,21 @@ struct BlockDriver {
 void (*bdrv_io_unplug)(BlockDriverState *bs);
 void (*bdrv_flush_io_queue)(BlockDriverState *bs);
 
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz);
+/**
+ * Try to get @bs's geometry (cyls, heads, sectos)
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * Only drivers that want to override guest geometry implement this
+ * callback; see hd_geometry_guess().
+ */
+int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
+
 QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
1.8.5.5

[Qemu-devel] [PATCH v4 0/5] Geometry and blocksize detection for backing devices.

Updates:
v2 -> v3:
1. Fix comments
2. Fix error codes to -ENOTSUP.
3. Reduce LOC in probe_logical_blocksize.
4. Adjust #ifdef - #else logic in couple of places.
5. Rebased.

I hope that I addressed all the comments from the last round of review.
If you think it's ok now, can you please give me your explicit Review-by.
(I would really appreciate if you could give me some feedback this week,
because I'm leaving for vacation next week)

Looking forward to your reply,
Thanks a lot!
Kate.
---
   Patchset Description (didn't change):

Proper geometry and blocksize information is vital for support of
DASD/ECKD drives in Linux guests. Otherwise things will fail in
certain cases.

The existing geometry and blocksize qemu defaults have no sense
for DASD drives (hd_geometry_guess detection and 512 for sizes).
Setting this information manually in XML file is far from user-friendly,
as the admin has to manually look up the properties of the
host disks and then modify the guest definition accordingly.

Since Linux uses DASDs as normal block devices, we actually
want to use virtio-blk to pass those to KVM guests.

In order to avoid any change in behavior of other drives, the DASD
special casing was advised. We call ioctl BIODASDINFO2 on the block
device, which will only succeed if the device is really a DASD.

In order to retrieve the underlying device geometry and blocksizes
a new block-backend functions and underlying driver functions were
introduced (blk_probe_blocksizes anf blk_probe_geometry wrappers
and corresponding bdrv_xx functions).

As for now only "host_device" driver received new detection methods.
For "raw" we call childs method as usual. In future one may update
other drivers to add some other detection heuristics.

If the host_device appears to be a DASD, the driver functions
(hdev_probe_blocksizes and hdev_probe_geometry) will call certain
ioctls in order to detect geometry and blocksizes of the underlying device.
if probing failed bdrv_probe_blocksizes caller will set defaults,
and bdrv_probe_geometry will fail to allow fallback to old detection logic.

The front-end (BlockConf API) was updated:
1. a new blkconf_blocksizes function was added. It doesn't
change user-defined blocksize values. If properties are unset, it will
set values, returned by blk_probe_backend. In order to allow this logic,
blocksize properties were initialized with 0. (driver will return 512 if
backing device probing didn't succeed or if driver method is not defined).
2. hd_geometry guess was changed to firstly try to retrieve values via
blk_probe_geometry and if it fails, fallback to the old logic.


Ekaterina Tumanova (5):
  block: add bdrv functions for geometry and blocksize
  raw-posix: Factor block size detection out of raw_probe_alignment()
  block: Add driver methods to probe blocksizes and geometry
  block-backend: Add wrappers for blocksizes and geometry probing
  BlockConf: Call backend functions to detect geometry and blocksizes

 block.c|  34 ++
 block/block-backend.c  |  10 +++
 block/raw-posix.c  | 138 -
 block/raw_bsd.c|  14 +
 hw/block/block.c   |  15 +
 hw/block/hd-geometry.c |  10 ++-
 hw/block/nvme.c|   1 +
 hw/block/virtio-blk.c  |   1 +
 hw/core/qdev-properties.c  |   3 +-
 hw/ide/qdev.c  |   1 +
 hw/scsi/scsi-disk.c|   1 +
 hw/usb/dev-storage.c   |   1 +
 include/block/block.h  |  13 
 include/block/block_int.h  |  15 +
 include/hw/block/block.h   |   5 +-
 include/sysemu/block-backend.h |   2 +
 16 files changed, 243 insertions(+), 21 deletions(-)

-- 
1.8.5.5

[Qemu-devel] [PATCH v4 2/5] raw-posix: Factor block size detection out of raw_probe_alignment()

Put it in new probe_logical_blocksize().

Signed-off-by: Ekaterina Tumanova 
---
 block/raw-posix.c | 41 -
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e51293a..38172ca 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -217,11 +217,31 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
+/*
+ * Get logical block size via ioctl. On success return 0. Otherwise -errno.
+ */
+static int probe_logical_blocksize(int fd, unsigned int *sector_size)
+{
+#if defined(BLKSSZGET)
+#  define SECTOR_SIZE BLKSSZGET
+#elif defined(DKIOCGETBLOCKSIZE)
+#  define SECTOR_SIZE DKIOCGETBLOCKSIZE
+#elif defined(DIOCGSECTORSIZE)
+#  define SECTOR_SIZE DIOCGSECTORSIZE
+#else
+return -ENOTSUP
+#endif
+if (ioctl(fd, SECTOR_SIZE, sector_size) < 0) {
+return -errno;
+}
+return 0;
+#undef SECTOR_SIZE
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
 char *buf;
-unsigned int sector_size;
 
 /* For /dev/sg devices the alignment is not really used.
With buffered I/O, we don't have any restrictions. */
@@ -231,25 +251,12 @@ static void raw_probe_alignment(BlockDriverState *bs, int 
fd, Error **errp)
 return;
 }
 
-/* Try a few ioctls to get the right size */
 bs->request_alignment = 0;
 s->buf_align = 0;
-
-#ifdef BLKSSZGET
-if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
-}
-#endif
-#ifdef DKIOCGETBLOCKSIZE
-if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+/* Let's try to use the logical blocksize for the alignment. */
+if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) {
+bs->request_alignment = 0;
 }
-#endif
-#ifdef DIOCGSECTORSIZE
-if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
-}
-#endif
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
-- 
1.8.5.5

Re: [Qemu-devel] [PATCH v3 5/5] BlockConf: Call backend functions to detect geometry and blocksizes

2014-12-15 Thread Ekaterina Tumanova


On 12/15/2014 04:50 PM, Markus Armbruster wrote:

Ekaterina Tumanova  writes:


geometry: hd_geometry_guess function autodetects the drive geometry.
This patch adds a block backend call, that probes the backing device
geometry. If the inner driver method is implemented and succeeds
(currently only for DASDs), the blkconf_geometry will pass-through
the backing device geometry. Otherwise will fallback to old logic.

blocksize: This patch initializes blocksize properties to 0.
In order to set the properly a blkconf_blocksizes was introduced.
If user didn't set physical or logical blocksize, it will
retrieve it's value from a driver (which will return a default 512 for
any backend but DASD).

The blkconf_blocksizes call was added to all users of BlkConf.

Signed-off-by: Ekaterina Tumanova 
---
  hw/block/block.c  | 18 ++
  hw/block/hd-geometry.c| 12 
  hw/block/nvme.c   |  1 +
  hw/block/virtio-blk.c |  1 +
  hw/core/qdev-properties.c |  3 ++-
  hw/ide/qdev.c |  1 +
  hw/scsi/scsi-disk.c   |  1 +
  hw/usb/dev-storage.c  |  1 +
  include/hw/block/block.h  |  5 +++--
  9 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..9c07516 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,24 @@ void blkconf_serial(BlockConf *conf, char **serial)
  }
  }

+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+BlockSizes blocksizes;
+
+if (conf->physical_block_size && conf->logical_block_size) {
+return;
+}


This conditional isn't necessary for correctness.  Feel free to drop it.



But this allows to avoid the ioctl call when user has specified both
values. Remove anyway?


+blk_probe_blocksizes(blk, &blocksizes);
+
+if (!conf->physical_block_size) {
+conf->physical_block_size = blocksizes.phys;
+}
+if (!conf->logical_block_size) {
+conf->logical_block_size = blocksizes.log;
+}




I'll add a comment to make it apparent.


This works because you change the default block size to 0 (second to
last hunk).


+}
+
  void blkconf_geometry(BlockConf *conf, int *ptrans,
unsigned cyls_max, unsigned heads_max, unsigned 
secs_max,
Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..fbd602d 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,7 +121,17 @@ void hd_geometry_guess(BlockBackend *blk,
 int *ptrans)
  {
  int cylinders, heads, secs, translation;
+hdGeometry geo;

+/* Try to probe the backing device geometry, otherwise fallback
+   to the old logic. (as of 12/2014 probing only succeeds on DASDs) */
+if (blk_probe_geometry(blk, &geo) == 0) {
+*pcyls = geo.cylinders;
+*psecs = geo.sectors;
+*pheads = geo.heads;
+translation = BIOS_ATA_TRANSLATION_NONE;
+goto done;
+}


"else if" instead of goto, please.


  if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
  /* no LCHS guess: use a standard physical disk geometry  */
  guess_chs_for_size(blk, pcyls, pheads, psecs);
@@ -142,7 +152,9 @@ void hd_geometry_guess(BlockBackend *blk,
  /* disable any translation to be in sync with
 the logical geometry */
  translation = BIOS_ATA_TRANSLATION_NONE;
+
  }


Humor me: put the empty line behind the brace instead of before.


+done:
  if (ptrans) {
  *ptrans = translation;
  }


Much easier to gauge than v2.  Geometry change is a compatibility
problem.  You change it only when blk_probe_geometry() succeeds.  It
succeeds only for DASD.  Mission accomplished.

Recommend to add a hint to the function contract of the
bdrv_probe_geometry() callback in block_int.h:

 /**
  * Try to get @bs's geometry (cyls, heads, sectos)
  * On success, store them in @geo and return 0.
  * On failure return -errno.
  * Only drivers that want to override guest geometry implement this
  * callback; see hd_geometry_guess().
  */
 int (*bdrv_probe_geometry)(BlockDriverState *bs, hdGeometry *geo);


diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 1327658..244e382 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -763,6 +763,7 @@ static int nvme_init(PCIDevice *pci_dev)
  if (!n->serial) {
  return -1;
  }
+blkconf_blocksizes(&n->conf);

  pci_conf = pci_dev->config;
  pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..6f01565 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -745,6 +745,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
  error_propagate(errp, err);
  return;

Re: [Qemu-devel] [PATCH v3 3/5] block: Add driver methods to probe blocksizes and geometry

2014-12-15 Thread Ekaterina Tumanova


On 12/15/2014 04:29 PM, Markus Armbruster wrote:

Ekaterina Tumanova  writes:


This patch introduces driver methods of defining disk blocksizes
(physical and logical) and hard drive geometry.
The method is only implemented for "host_device". For "raw" devices
driver calls child's method.

For the time being geometry detection will only work for DASD devices.
In order to check that a local check_for_dasd function was introduced,
which calls BIODASDINFO2 ioctl and returns its rc.

Blocksizes detection fuction will probe sizes for DASD devices and
set default for other devices.

Signed-off-by: Ekaterina Tumanova 
---
  block/raw-posix.c | 91 +++
  block/raw_bsd.c   | 14 +
  2 files changed, 105 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 633d5bc..33f9983 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,7 @@
  #include 
  #include 
  #include 
+#include 
  #ifndef FS_NOCOW_FL
  #define FS_NOCOW_FL 0x0080 /* Do not cow file */
  #endif
@@ -90,6 +91,10 @@
  #include 
  #endif

+#ifdef __s390__
+#include 
+#endif
+
  //#define DEBUG_FLOPPY

  //#define DEBUG_BLOCK
@@ -242,6 +247,20 @@ static int probe_logical_blocksize(int fd, unsigned int 
*sector_size)
  return 0;
  }

+/*
+ * Set physical block size via ioctl. On success return 0. Otherwise -errno.
+ */


Set?

Ah, now I get your logic, you mean "set *blk_size"!

Suggest to say it like this:

/**
  * Get physical block size of @fd.
  * On success, store it in @blk_size and return 0.
  * On failure, return -errno.
  */


+static int probe_physical_blocksize(int fd, unsigned int *blk_size)
+{
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
+return -errno;
+}
+#endif
+
+return 0;


If !defined(BLKPBSZGET), you return 0 without setting *blk_size.  I
think you need to fail then.  -ENOTSUP should do.


+}
+
  static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
  {
  BDRVRawState *s = bs->opaque;
@@ -662,6 +681,76 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
  bs->bl.opt_mem_alignment = s->buf_align;
  }

+static int check_for_dasd(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#endif
+return -1;
+}
+
+/*
+ * Try to get the device blocksize. On success 0. On failure return -errno.
+ * Currently only implemented for DASD drives.
+ */


Compare to the function contract I wrote for the callback in my review
of PATCH 1.  If you like that one better, feel free to reuse it here.


+static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BDRVRawState *s = bs->opaque;
+int ret;
+
+/* If DASD, get blocksizes */
+if (check_for_dasd(s->fd) < 0) {
+return -1;


This is not a negative error code!  -ENOTSUP.


+}
+ret = probe_logical_blocksize(s->fd, &bsz->log);
+if (ret < 0) {
+return ret;
+}
+return probe_physical_blocksize(s->fd, &bsz->phys);
+}
+
+/*
+ * Try to get the device geometry. On success 0. On failure return -errno.
+ * Currently only implemented for DASD drives.
+ */


Compare to the function contract I wrote for the callback in my review
of PATCH 1.  If you like that one better, feel free to reuse it here.


+static int hdev_probe_geometry(BlockDriverState *bs, hdGeometry *geo)
+{
+BDRVRawState *s = bs->opaque;
+struct hd_geometry ioctl_geo = {0};
+uint32_t blksize;
+
+/* If DASD, get it's geometry */
+if (check_for_dasd(s->fd) < 0) {
+return -1;


This is not a negative error code!  -ENOTSUP.


+}
+if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
+return -errno;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+return -1;


Need an error code.


+}
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+return -1;


Need an error code.


+}
+geo->heads = ioctl_geo.heads;
+geo->sectors = ioctl_geo.sectors;
+if (bs->total_sectors) {
+if (!probe_physical_blocksize(s->fd, &blksize)) {
+geo->cylinders = bs->total_sectors / (blksize / BDRV_SECTOR_SIZE)
+   / (geo->heads * geo->sectors);
+return 0;
+}
+}


How could !bs->total_sectors happen?


+geo->cylinders = ioctl_geo.cylinders;
+
+return 0;
+}
+
  static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
  {
  int ret;
@@ -2127,6 +2216,8 @@ static BlockDriver bdrv_host_device = {
  .bdrv_get_info = raw_get_info,
  .bdrv_get_allocated_file

Re: [Qemu-devel] [PATCH v3 0/5] Geometry and blocksize detection for backing devices.

2014-12-12 Thread Ekaterina Tumanova


On 12/05/2014 08:56 PM, Ekaterina Tumanova wrote:

This 3rd revision of the patch set.

Proper geometry and blocksize information is vital for support of
DASD/ECKD drives in Linux guests. Otherwise things will fail in
certain cases.

The existing geometry and blocksize qemu defaults have no sense
for DASD drives (hd_geometry_guess detection and 512 for sizes).
Setting this information manually in XML file is far from user-friendly,
as the admin has to manually look up the properties of the
host disks and then modify the guest definition accordingly.

Since Linux uses DASDs as normal block devices, we actually
want to use virtio-blk to pass those to KVM guests.

In order to avoid any change in behavior of other drives, the DASD
special casing was advised. We call ioctl BIODASDINFO2 on the block
device, which will only succeed if the device is really a DASD.

In order to retrieve the underlying device geometry and blocksizes
a new block-backend functions and underlying driver functions were
introduced (blk_probe_blocksizes anf blk_probe_geometry wrappers
and corresponding bdrv_xx functions).

As for now only "host_device" driver received new detection methods.
For "raw" we call childs method as usual. In future one may update
other drivers to add some other detection heuristics.

If the host_device appears to be a DASD, the driver functions
(hdev_probe_blocksizes and hdev_probe_geometry) will call certain
ioctls in order to detect geometry and blocksizes of the underlying device.
if probing failed bdrv_probe_blocksizes caller will set defaults,
and bdrv_probe_geometry will fail to allow fallback to old detection logic.

The front-end (BlockConf API) was updated:
1. a new blkconf_blocksizes function was added. It doesn't
change user-defined blocksize values. If properties are unset, it will
set values, returned by blk_probe_backend. In order to allow this logic,
blocksize properties were initialized with 0. (driver will return 512 if
backing device probing didn't succeed or if driver method is not defined).
2. hd_geometry guess was changed to firstly try to retrieve values via
blk_probe_geometry and if it fails, fallback to the old logic.

updates from v2:
- avoid passing structs as a function result in backend part
- fix patch desctiptions
- add the blkconf_blocksizes call to all the users of BlockConf
- do not change geometry and blocksize values set via command-line, to do that:
remove hardcoding 512 in DEFINE_BLOCK_PROPERTIES. set defaults in
blk_probe_blocksizes (now always succeeds) and call it only if property is 0.
- fix cylinders calculation.
- document blk_probe_geometry call as "currently only for DASD".
- anything I forgot :)

Ekaterina Tumanova (5):
   block: add bdrv functions for geometry and blocksize
   raw-posix: Factor block size detection out of raw_probe_alignment()
   block: Add driver methods to probe blocksizes and geometry
   block-backend: Add wrappers for blocksizes and geometry probing
   BlockConf: Call backend functions to detect geometry and blocksizes

  block.c|  35 +++
  block/block-backend.c  |  10 +++
  block/raw-posix.c  | 135 +++--
  block/raw_bsd.c|  14 +
  hw/block/block.c   |  18 ++
  hw/block/hd-geometry.c |  12 
  hw/block/nvme.c|   1 +
  hw/block/virtio-blk.c  |   1 +
  hw/core/qdev-properties.c  |   3 +-
  hw/ide/qdev.c  |   1 +
  hw/scsi/scsi-disk.c|   1 +
  hw/usb/dev-storage.c   |   1 +
  include/block/block.h  |  13 
  include/block/block_int.h  |   5 ++
  include/hw/block/block.h   |   5 +-
  include/sysemu/block-backend.h |   2 +
  16 files changed, 237 insertions(+), 20 deletions(-)



friendly *ping* to maintainers of block layer :) I would rather not send 
next version without getting your feedback first.

Re: [Qemu-devel] [PATCH v3 3/5] block: Add driver methods to probe blocksizes and geometry

2014-12-11 Thread Ekaterina Tumanova


On 12/10/2014 04:14 PM, Thomas Huth wrote:

On Fri,  5 Dec 2014 18:56:19 +0100
Ekaterina Tumanova  wrote:


This patch introduces driver methods of defining disk blocksizes
(physical and logical) and hard drive geometry.
The method is only implemented for "host_device". For "raw" devices
driver calls child's method.

For the time being geometry detection will only work for DASD devices.
In order to check that a local check_for_dasd function was introduced,
which calls BIODASDINFO2 ioctl and returns its rc.

Blocksizes detection fuction will probe sizes for DASD devices and
set default for other devices.

Signed-off-by: Ekaterina Tumanova 
---
  block/raw-posix.c | 91 +++
  block/raw_bsd.c   | 14 +
  2 files changed, 105 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 633d5bc..33f9983 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,7 @@
  #include 
  #include 
  #include 
+#include 
  #ifndef FS_NOCOW_FL
  #define FS_NOCOW_FL 0x0080 /* Do not cow file */
  #endif
@@ -90,6 +91,10 @@
  #include 
  #endif

+#ifdef __s390__
+#include 
+#endif
+
  //#define DEBUG_FLOPPY

  //#define DEBUG_BLOCK
@@ -242,6 +247,20 @@ static int probe_logical_blocksize(int fd, unsigned int 
*sector_size)
  return 0;
  }

+/*
+ * Set physical block size via ioctl. On success return 0. Otherwise -errno.
+ */
+static int probe_physical_blocksize(int fd, unsigned int *blk_size)
+{
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
+return -errno;
+}
+#endif
+
+return 0;
+}
+
  static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
  {
  BDRVRawState *s = bs->opaque;
@@ -662,6 +681,76 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
  bs->bl.opt_mem_alignment = s->buf_align;
  }

+static int check_for_dasd(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#endif
+return -1;


I'd put the "return -1" line into an #else branch of the #ifdef, so
that you do not end up with two consecutive return statements in case
BIODASDINFO2 is defined.


+}
+
+/*
+ * Try to get the device blocksize. On success 0. On failure return -errno.
+ * Currently only implemented for DASD drives.
+ */
+static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BDRVRawState *s = bs->opaque;
+int ret;
+
+/* If DASD, get blocksizes */
+if (check_for_dasd(s->fd) < 0) {
+return -1;
+}
+ret = probe_logical_blocksize(s->fd, &bsz->log);
+if (ret < 0) {
+return ret;
+}
+return probe_physical_blocksize(s->fd, &bsz->phys);
+}
+
+/*
+ * Try to get the device geometry. On success 0. On failure return -errno.


"On success return 0"


+ * Currently only implemented for DASD drives.
+ */
+static int hdev_probe_geometry(BlockDriverState *bs, hdGeometry *geo)
+{
+BDRVRawState *s = bs->opaque;
+struct hd_geometry ioctl_geo = {0};
+uint32_t blksize;
+
+/* If DASD, get it's geometry */
+if (check_for_dasd(s->fd) < 0) {
+return -1;
+}
+if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
+return -errno;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+return -1;
+}
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+return -1;
+}
+geo->heads = ioctl_geo.heads;
+geo->sectors = ioctl_geo.sectors;
+if (bs->total_sectors) {


Maybe add a comment here why you've got to calculate the cylinders here
instead of using ioctl_geo.cylinders ?


+if (!probe_physical_blocksize(s->fd, &blksize)) {
+geo->cylinders = bs->total_sectors / (blksize / BDRV_SECTOR_SIZE)
+   / (geo->heads * geo->sectors);
+return 0;
+}
+}
+geo->cylinders = ioctl_geo.cylinders;
+
+return 0;
+}
+
  static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
  {
  int ret;
@@ -2127,6 +2216,8 @@ static BlockDriver bdrv_host_device = {
  .bdrv_get_info = raw_get_info,
  .bdrv_get_allocated_file_size
  = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,

  .bdrv_detach_aio_context = raw_detach_aio_context,
  .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 401b967..cfd5249 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -173,6 +173,18 @@ static int raw_probe(const uint8_t *buf, int buf_size,

[Qemu-devel] [PATCH v3 2/5] raw-posix: Factor block size detection out of raw_probe_alignment()

Put it in new probe_logical_blocksize().

Signed-off-by: Ekaterina Tumanova 
---
 block/raw-posix.c | 44 +++-
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index b1af77e..633d5bc 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -217,11 +217,35 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
+/*
+ * Set logical block size via ioctl. On success return 0. Otherwise -errno.
+ */
+static int probe_logical_blocksize(int fd, unsigned int *sector_size)
+{
+/* Try a few ioctls to get the right size */
+#ifdef BLKSSZGET
+if (ioctl(fd, BLKSSZGET, sector_size) < 0) {
+return -errno;
+}
+#endif
+#ifdef DKIOCGETBLOCKSIZE
+if (ioctl(fd, DKIOCGETBLOCKSIZE, sector_size) < 0) {
+return -errno;
+}
+#endif
+#ifdef DIOCGSECTORSIZE
+if (ioctl(fd, DIOCGSECTORSIZE, sector_size) < 0) {
+return -errno;
+}
+#endif
+
+return 0;
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
 char *buf;
-unsigned int sector_size;
 
 /* For /dev/sg devices the alignment is not really used.
With buffered I/O, we don't have any restrictions. */
@@ -231,25 +255,11 @@ static void raw_probe_alignment(BlockDriverState *bs, int 
fd, Error **errp)
 return;
 }
 
-/* Try a few ioctls to get the right size */
 bs->request_alignment = 0;
 s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+probe_logical_blocksize(fd, &bs->request_alignment);
 
-#ifdef BLKSSZGET
-if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
-}
-#endif
-#ifdef DKIOCGETBLOCKSIZE
-if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
-}
-#endif
-#ifdef DIOCGSECTORSIZE
-if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
-}
-#endif
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
-- 
1.8.5.5

[Qemu-devel] [PATCH v3 4/5] block-backend: Add wrappers for blocksizes and geometry probing

Signed-off-by: Ekaterina Tumanova 
---
 block/block-backend.c  | 10 ++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index d0692b1..9cd97c6 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -629,3 +629,13 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, 
BlockBackend *blk,
 {
 return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
 }
+
+void blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz)
+{
+bdrv_probe_blocksizes(blk->bs, bsz);
+}
+
+int blk_probe_geometry(BlockBackend *blk, hdGeometry *geo)
+{
+return bdrv_probe_geometry(blk->bs, geo);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 52d13c1..3f934c5 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -138,5 +138,7 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk);
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque);
+void blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz);
+int blk_probe_geometry(BlockBackend *blk, hdGeometry *geo);
 
 #endif
-- 
1.8.5.5

[Qemu-devel] [PATCH v3 1/5] block: add bdrv functions for geometry and blocksize

Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova 
---
 block.c   | 35 +++
 include/block/block.h | 13 +
 include/block/block_int.h |  5 +
 3 files changed, 53 insertions(+)

diff --git a/block.c b/block.c
index a612594..7b0b804 100644
--- a/block.c
+++ b/block.c
@@ -548,6 +548,41 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in bsz struct.
+ * On failure, set default blocksize.
+ */
+void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_blocksizes &&
+!drv->bdrv_probe_blocksizes(bs, bsz)) {
+return;
+}
+bsz->log = BDRV_SECTOR_SIZE;
+bsz->phys = BDRV_SECTOR_SIZE;
+}
+
+/**
+ * Try to get @bs's geometry (cyls, heads, sectos)
+ * On success, store them in geo struct and return 0.
+ * On failure return -errno.
+ */
+int bdrv_probe_geometry(BlockDriverState *bs, hdGeometry *geo)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_geometry) {
+return drv->bdrv_probe_geometry(bs, geo);
+}
+
+return -1;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
diff --git a/include/block/block.h b/include/block/block.h
index 5450610..17184b6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -60,6 +60,17 @@ typedef enum {
 BDRV_REQ_MAY_UNMAP= 0x4,
 } BdrvRequestFlags;
 
+typedef struct BlockSizes {
+uint32_t phys;
+uint32_t log;
+} BlockSizes;
+
+typedef struct hdGeometry {
+uint32_t heads;
+uint32_t sectors;
+uint32_t cylinders;
+} hdGeometry;
+
 #define BDRV_O_RDWR0x0002
 #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
in a snapshot */
 #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
@@ -538,6 +549,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
  * the old #AioContext is not executing.
  */
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
+int bdrv_probe_geometry(BlockDriverState *bs, hdGeometry *geo);
 
 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a1c17b9..16e53e9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -271,6 +271,11 @@ struct BlockDriver {
 void (*bdrv_io_unplug)(BlockDriverState *bs);
 void (*bdrv_flush_io_queue)(BlockDriverState *bs);
 
+/* try to get physical and logical blocksizes */
+int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz);
+/* tey to get hard drive geometry (cyls, head, sectors) */
+int (*bdrv_probe_geometry)(BlockDriverState *bs, hdGeometry *geo);
+
 QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
1.8.5.5

[Qemu-devel] [PATCH v3 3/5] block: Add driver methods to probe blocksizes and geometry

This patch introduces driver methods of defining disk blocksizes
(physical and logical) and hard drive geometry.
The method is only implemented for "host_device". For "raw" devices
driver calls child's method.

For the time being geometry detection will only work for DASD devices.
In order to check that a local check_for_dasd function was introduced,
which calls BIODASDINFO2 ioctl and returns its rc.

Blocksizes detection fuction will probe sizes for DASD devices and
set default for other devices.

Signed-off-by: Ekaterina Tumanova 
---
 block/raw-posix.c | 91 +++
 block/raw_bsd.c   | 14 +
 2 files changed, 105 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 633d5bc..33f9983 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,7 @@
 #include 
 #include 
 #include 
+#include 
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
@@ -90,6 +91,10 @@
 #include 
 #endif
 
+#ifdef __s390__
+#include 
+#endif
+
 //#define DEBUG_FLOPPY
 
 //#define DEBUG_BLOCK
@@ -242,6 +247,20 @@ static int probe_logical_blocksize(int fd, unsigned int 
*sector_size)
 return 0;
 }
 
+/*
+ * Set physical block size via ioctl. On success return 0. Otherwise -errno.
+ */
+static int probe_physical_blocksize(int fd, unsigned int *blk_size)
+{
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
+return -errno;
+}
+#endif
+
+return 0;
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
@@ -662,6 +681,76 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 bs->bl.opt_mem_alignment = s->buf_align;
 }
 
+static int check_for_dasd(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#endif
+return -1;
+}
+
+/*
+ * Try to get the device blocksize. On success 0. On failure return -errno.
+ * Currently only implemented for DASD drives.
+ */
+static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BDRVRawState *s = bs->opaque;
+int ret;
+
+/* If DASD, get blocksizes */
+if (check_for_dasd(s->fd) < 0) {
+return -1;
+}
+ret = probe_logical_blocksize(s->fd, &bsz->log);
+if (ret < 0) {
+return ret;
+}
+return probe_physical_blocksize(s->fd, &bsz->phys);
+}
+
+/*
+ * Try to get the device geometry. On success 0. On failure return -errno.
+ * Currently only implemented for DASD drives.
+ */
+static int hdev_probe_geometry(BlockDriverState *bs, hdGeometry *geo)
+{
+BDRVRawState *s = bs->opaque;
+struct hd_geometry ioctl_geo = {0};
+uint32_t blksize;
+
+/* If DASD, get it's geometry */
+if (check_for_dasd(s->fd) < 0) {
+return -1;
+}
+if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
+return -errno;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+return -1;
+}
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+return -1;
+}
+geo->heads = ioctl_geo.heads;
+geo->sectors = ioctl_geo.sectors;
+if (bs->total_sectors) {
+if (!probe_physical_blocksize(s->fd, &blksize)) {
+geo->cylinders = bs->total_sectors / (blksize / BDRV_SECTOR_SIZE)
+   / (geo->heads * geo->sectors);
+return 0;
+}
+}
+geo->cylinders = ioctl_geo.cylinders;
+
+return 0;
+}
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2127,6 +2216,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,
 
 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 401b967..cfd5249 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -173,6 +173,18 @@ static int raw_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 1;
 }
 
+static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+bdrv_probe_blocksizes(bs->file, bsz);
+
+return 0;
+}
+
+static int raw_probe_geometry(BlockDriverState *bs, hdGeometry *geo)
+{
+return bdrv_probe_geometry(bs->file, geo);
+}
+
 static BlockDriver bdrv_raw = {
 .format_name  = "raw",
 .bdrv_probe   = &raw_probe,
@@ -190,6 +202,8 @@ static Bloc

[Qemu-devel] [PATCH v3 0/5] Geometry and blocksize detection for backing devices.

This 3rd revision of the patch set.

Proper geometry and blocksize information is vital for support of
DASD/ECKD drives in Linux guests. Otherwise things will fail in
certain cases.

The existing geometry and blocksize qemu defaults have no sense
for DASD drives (hd_geometry_guess detection and 512 for sizes).
Setting this information manually in XML file is far from user-friendly,
as the admin has to manually look up the properties of the
host disks and then modify the guest definition accordingly.

Since Linux uses DASDs as normal block devices, we actually
want to use virtio-blk to pass those to KVM guests.

In order to avoid any change in behavior of other drives, the DASD
special casing was advised. We call ioctl BIODASDINFO2 on the block
device, which will only succeed if the device is really a DASD.

In order to retrieve the underlying device geometry and blocksizes
a new block-backend functions and underlying driver functions were
introduced (blk_probe_blocksizes anf blk_probe_geometry wrappers
and corresponding bdrv_xx functions).

As for now only "host_device" driver received new detection methods.
For "raw" we call childs method as usual. In future one may update
other drivers to add some other detection heuristics.

If the host_device appears to be a DASD, the driver functions
(hdev_probe_blocksizes and hdev_probe_geometry) will call certain
ioctls in order to detect geometry and blocksizes of the underlying device.
if probing failed bdrv_probe_blocksizes caller will set defaults,
and bdrv_probe_geometry will fail to allow fallback to old detection logic.

The front-end (BlockConf API) was updated:
1. a new blkconf_blocksizes function was added. It doesn't
change user-defined blocksize values. If properties are unset, it will
set values, returned by blk_probe_backend. In order to allow this logic,
blocksize properties were initialized with 0. (driver will return 512 if
backing device probing didn't succeed or if driver method is not defined).
2. hd_geometry guess was changed to firstly try to retrieve values via
blk_probe_geometry and if it fails, fallback to the old logic.

updates from v2:
- avoid passing structs as a function result in backend part
- fix patch desctiptions
- add the blkconf_blocksizes call to all the users of BlockConf
- do not change geometry and blocksize values set via command-line, to do that:
remove hardcoding 512 in DEFINE_BLOCK_PROPERTIES. set defaults in
blk_probe_blocksizes (now always succeeds) and call it only if property is 0.
- fix cylinders calculation.
- document blk_probe_geometry call as "currently only for DASD".
- anything I forgot :)

Ekaterina Tumanova (5):
  block: add bdrv functions for geometry and blocksize
  raw-posix: Factor block size detection out of raw_probe_alignment()
  block: Add driver methods to probe blocksizes and geometry
  block-backend: Add wrappers for blocksizes and geometry probing
  BlockConf: Call backend functions to detect geometry and blocksizes

 block.c|  35 +++
 block/block-backend.c  |  10 +++
 block/raw-posix.c  | 135 +++--
 block/raw_bsd.c|  14 +
 hw/block/block.c   |  18 ++
 hw/block/hd-geometry.c |  12 
 hw/block/nvme.c|   1 +
 hw/block/virtio-blk.c  |   1 +
 hw/core/qdev-properties.c  |   3 +-
 hw/ide/qdev.c  |   1 +
 hw/scsi/scsi-disk.c|   1 +
 hw/usb/dev-storage.c   |   1 +
 include/block/block.h  |  13 
 include/block/block_int.h  |   5 ++
 include/hw/block/block.h   |   5 +-
 include/sysemu/block-backend.h |   2 +
 16 files changed, 237 insertions(+), 20 deletions(-)

-- 
1.8.5.5

[Qemu-devel] [PATCH v3 5/5] BlockConf: Call backend functions to detect geometry and blocksizes

geometry: hd_geometry_guess function autodetects the drive geometry.
This patch adds a block backend call, that probes the backing device
geometry. If the inner driver method is implemented and succeeds
(currently only for DASDs), the blkconf_geometry will pass-through
the backing device geometry. Otherwise will fallback to old logic.

blocksize: This patch initializes blocksize properties to 0.
In order to set the properly a blkconf_blocksizes was introduced.
If user didn't set physical or logical blocksize, it will
retrieve it's value from a driver (which will return a default 512 for
any backend but DASD).

The blkconf_blocksizes call was added to all users of BlkConf.

Signed-off-by: Ekaterina Tumanova 
---
 hw/block/block.c  | 18 ++
 hw/block/hd-geometry.c| 12 
 hw/block/nvme.c   |  1 +
 hw/block/virtio-blk.c |  1 +
 hw/core/qdev-properties.c |  3 ++-
 hw/ide/qdev.c |  1 +
 hw/scsi/scsi-disk.c   |  1 +
 hw/usb/dev-storage.c  |  1 +
 include/hw/block/block.h  |  5 +++--
 9 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..9c07516 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,24 @@ void blkconf_serial(BlockConf *conf, char **serial)
 }
 }
 
+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+BlockSizes blocksizes;
+
+if (conf->physical_block_size && conf->logical_block_size) {
+return;
+}
+blk_probe_blocksizes(blk, &blocksizes);
+
+if (!conf->physical_block_size) {
+conf->physical_block_size = blocksizes.phys;
+}
+if (!conf->logical_block_size) {
+conf->logical_block_size = blocksizes.log;
+}
+}
+
 void blkconf_geometry(BlockConf *conf, int *ptrans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..fbd602d 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,7 +121,17 @@ void hd_geometry_guess(BlockBackend *blk,
int *ptrans)
 {
 int cylinders, heads, secs, translation;
+hdGeometry geo;
 
+/* Try to probe the backing device geometry, otherwise fallback
+   to the old logic. (as of 12/2014 probing only succeeds on DASDs) */
+if (blk_probe_geometry(blk, &geo) == 0) {
+*pcyls = geo.cylinders;
+*psecs = geo.sectors;
+*pheads = geo.heads;
+translation = BIOS_ATA_TRANSLATION_NONE;
+goto done;
+}
 if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
 guess_chs_for_size(blk, pcyls, pheads, psecs);
@@ -142,7 +152,9 @@ void hd_geometry_guess(BlockBackend *blk,
 /* disable any translation to be in sync with
the logical geometry */
 translation = BIOS_ATA_TRANSLATION_NONE;
+
 }
+done:
 if (ptrans) {
 *ptrans = translation;
 }
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 1327658..244e382 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -763,6 +763,7 @@ static int nvme_init(PCIDevice *pci_dev)
 if (!n->serial) {
 return -1;
 }
+blkconf_blocksizes(&n->conf);
 
 pci_conf = pci_dev->config;
 pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..6f01565 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -745,6 +745,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 error_propagate(errp, err);
 return;
 }
+blkconf_blocksizes(&conf->conf);
 
 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2e47f70..ba81709 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -580,7 +580,8 @@ static void set_blocksize(Object *obj, Visitor *v, void 
*opaque,
 error_propagate(errp, local_err);
 return;
 }
-if (value < min || value > max) {
+/* value of 0 means "unset" */
+if (value && (value < min || value > max)) {
 error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
   dev->id?:"", name, (int64_t)value, min, max);
 return;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index b4f096e..e71ff7f 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -164,6 +164,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 }
 
 blkconf_serial(&dev->conf, &dev->serial);
+blkconf_blocksizes(&dev->conf);
 if (kind != IDE_CD) {
 blkconf_geometry(&dev->conf, &dev->chs_trans, 65

Re: [Qemu-devel] [PATCH v2 2/6] geometry: Detect blocksize via ioctls in separate static functions


On 11/28/2014 03:52 PM, Markus Armbruster wrote:

Ekaterina Tumanova  writes:



Suggest function comment

  /**
   * Return logical block size, or zero if we can't figure it out
   */


   {
-BDRVRawState *s = bs->opaque;
-char *buf;
-unsigned int sector_size;
-
-/* For /dev/sg devices the alignment is not really used.
-   With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !s->needs_alignment) {
-bs->request_alignment = 1;
-s->buf_align = 1;
-return;
-}
+unsigned int sector_size = 0;


Pointless initialization.


If I do not initialize the sector_size, and the ioctl fails,
I will return garbage as a blocksize to the caller.


Where?  As far as I can see, we return it only after ioctl() succeeded.



Sorry,
you're absolutely right. I kept seeing and thinking that I always
returned sector_size variable. ::facepalm::



   /* Try a few ioctls to get the right size */
-bs->request_alignment = 0;
-s->buf_align = 0;
-
   #ifdef BLKSSZGET
   if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
   }
   #endif
   #ifdef DKIOCGETBLOCKSIZE
   if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
   }
   #endif
   #ifdef DIOCGSECTORSIZE
   if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
   }
   #endif
   #ifdef CONFIG_XFS
   if (s->is_xfs) {
   struct dioattr da;
   if (xfsctl(NULL, fd, XFS_IOC_DIOINFO, &da) >= 0) {
-bs->request_alignment = da.d_miniosz;
+sector_size = da.d_miniosz;
   /* The kernel returns wrong information for d_mem */
   /* s->buf_align = da.d_mem; */


Since you keep the enabled assignments to s->buf_align out of this
function, you should keep out this disabled one, too.


+return sector_size;
   }
   }
   #endif

+return 0;
+}
+
+static unsigned int probe_physical_blocksize(BlockDriverState *bs, int fd)


Parameter bs is unused, let's drop it.


+{
+unsigned int blk_size = 0;


Pointless initialization.


Same here.


Again, we return it only after ioctl() succeeded.


+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, &blk_size) >= 0) {
+return blk_size;
+}
+#endif
+
+return 0;
+}
+
+static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !s->needs_alignment) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+bs->request_alignment = probe_logical_blocksize(bs, fd);
+
   /* If we could not get the sizes so far, we can only guess them */
   if (!s->buf_align) {
   size_t align;

Re: [Qemu-devel] [PATCH v2 0/6] Geometry and blocksize support for backing devices


On 11/28/2014 01:35 PM, Stefan Hajnoczi wrote:

On Wed, Nov 26, 2014 at 01:16:48PM +0300, Ekaterina Tumanova wrote:

On 11/25/2014 04:01 PM, Stefan Hajnoczi wrote:

On Wed, Nov 19, 2014 at 11:17:50AM +0100, Ekaterina Tumanova wrote:
3. Why does s390 need to customize hd_geometry_guess()?


Since hd_geometry_guess contains semantics of x86-specific LBA translation,
we have to modify it not to get in the way of z
architecture


If the behavior is x86-specific, it should be made x86-specific.  z,
ppc, sparc, arm, etc should share the non-x86-specific code path.  It's
weird to single out z here, this seems like the wrong way around.

Is the x86-specific behavior a problem in practice?  No one seemed to
mind for the other targets.



on s390 arch this adds 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.

If you guys don't like the way patch goes, I can exclude it from patch
series and we can discuss it later.

But I thought that since it's related to the hard drive geometry,
and since we change hd_geometry_guess in this patchset anyway, why not
get rid of this problem as well.

Kate.

Re: [Qemu-devel] [PATCH v2 2/6] geometry: Detect blocksize via ioctls in separate static functions





Suggest function comment

 /**
  * Return logical block size, or zero if we can't figure it out
  */


  {
-BDRVRawState *s = bs->opaque;
-char *buf;
-unsigned int sector_size;
-
-/* For /dev/sg devices the alignment is not really used.
-   With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !s->needs_alignment) {
-bs->request_alignment = 1;
-s->buf_align = 1;
-return;
-}
+unsigned int sector_size = 0;


Pointless initialization.


If I do not initialize the sector_size, and the ioctl fails,
I will return garbage as a blocksize to the caller.





  /* Try a few ioctls to get the right size */
-bs->request_alignment = 0;
-s->buf_align = 0;
-
  #ifdef BLKSSZGET
  if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
  }
  #endif
  #ifdef DKIOCGETBLOCKSIZE
  if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
  }
  #endif
  #ifdef DIOCGSECTORSIZE
  if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
  }
  #endif
  #ifdef CONFIG_XFS
  if (s->is_xfs) {
  struct dioattr da;
  if (xfsctl(NULL, fd, XFS_IOC_DIOINFO, &da) >= 0) {
-bs->request_alignment = da.d_miniosz;
+sector_size = da.d_miniosz;
  /* The kernel returns wrong information for d_mem */
  /* s->buf_align = da.d_mem; */


Since you keep the enabled assignments to s->buf_align out of this
function, you should keep out this disabled one, too.


+return sector_size;
  }
  }
  #endif

+return 0;
+}
+
+static unsigned int probe_physical_blocksize(BlockDriverState *bs, int fd)


Parameter bs is unused, let's drop it.


+{
+unsigned int blk_size = 0;


Pointless initialization.


Same here.




+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, &blk_size) >= 0) {
+return blk_size;
+}
+#endif
+
+return 0;
+}
+
+static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !s->needs_alignment) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+bs->request_alignment = probe_logical_blocksize(bs, fd);
+
  /* If we could not get the sizes so far, we can only guess them */
  if (!s->buf_align) {
  size_t align;

Re: [Qemu-devel] [PATCH v2 5/6] geometry: Call backend function to detect geometry and blocksize


On 11/28/2014 01:10 PM, Markus Armbruster wrote:

Ekaterina Tumanova  writes:


hd_geometry_guess function autodetects the drive geometry. This patch
adds a block backend call, that probes the backing device geometry.
If the inner driver method is implemented and succeeds (currently only DASDs),
the blkconf_geometry will pass-through the backing device geometry.

Signed-off-by: Ekaterina Tumanova 
---
  hw/block/block.c | 11 +++
  hw/block/hd-geometry.c   |  9 +
  hw/block/virtio-blk.c|  1 +
  include/hw/block/block.h |  1 +
  4 files changed, 22 insertions(+)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..f1d29bc 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,17 @@ void blkconf_serial(BlockConf *conf, char **serial)
  }
  }

+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+struct ProbeBlockSize blocksize = blk_probe_blocksizes(blk);
+
+if (blocksize.rc == 0) {
+conf->physical_block_size = blocksize.size.phys;
+conf->logical_block_size = blocksize.size.log;
+}
+}
+


Uh, doesn't this overwrite the user's geometry?

The existing blkconf_ functions do nothing when the user supplied the
configuration.  In other words, they only provide defaults.  Why should
this one be different?



this will be fixed in the next version


  void blkconf_geometry(BlockConf *conf, int *ptrans,
unsigned cyls_max, unsigned heads_max, unsigned 
secs_max,
Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..4972114 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,6 +121,14 @@ void hd_geometry_guess(BlockBackend *blk,
 int *ptrans)
  {
  int cylinders, heads, secs, translation;
+struct ProbeGeometry geometry = blk_probe_geometry(blk);
+
+if (geometry.rc == 0) {
+*pcyls = geometry.geo.cylinders;
+*psecs = geometry.geo.sectors;
+*pheads = geometry.geo.heads;
+goto done;
+}



hd_geometry_guess() is called by blkconf_geometry() to conjure up a
default geometry when the user didn't pick one.  Fairly elaborate
guesswork.  blkconf_geometry() runs for IDE, SCSI and virtio-blk only.

Your patch makes it pick the backend's geometry as reported by
blk_probe_geometry() before anything else.

This is an incompatible change for backends where blk_probe_geometry()
succeeds.  Currently, it succeeds only for DASD, and there you *want*
the incompatible change.

My point is: if we rely on blk_probe_geometry() failing except for DASD,
then we should call it something like blk_dasd_geometry() to make that
obvious.



This function itself is not DASD specific. It calls the inner method for
"host_device" driver, which currently only succeeds for DASDs.
But in future someone can define methods for other drivers or
even modify the host_device driver.


The commit message needs to spell out the incompatible change.


  if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
  /* no LCHS guess: use a standard physical disk geometry  */
@@ -143,6 +151,7 @@ void hd_geometry_guess(BlockBackend *blk,
 the logical geometry */
  translation = BIOS_ATA_TRANSLATION_NONE;
  }
+done:
  if (ptrans) {
  *ptrans = translation;
  }
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..6f01565 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -745,6 +745,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
  error_propagate(errp, err);
  return;
  }
+blkconf_blocksizes(&conf->conf);

  virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
  sizeof(struct virtio_blk_config));


Why only virtio-blk, and not the other device models sporting
configurable block sizes (nvme, IDE, SCSI, usb-storage)?

This is an incompatible change for backends where blk_probe_blocksizes()
succeeds and returns something other than (512, 512).  Currently, it
succeeds only for DASD.  Is that what we want?

The commit message needs to spell out the incompatible change.

Perhaps this patch should be split up into one for geometry and one for
block sizes.


diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 0d0ce9a..856bf75 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -63,6 +63,7 @@ void blkconf_serial(BlockConf *conf, char **serial);
  void blkconf_geometry(BlockConf *conf, int *trans,
unsigned cyls_max, unsigned heads_max, unsigned 
secs_max,
Error **errp);
+void blkconf_blocksizes(BlockConf *conf);

  /* Hard disk geometry */

Re: [Qemu-devel] [PATCH v2 1/6] geometry: add bdrv functions for geometry and blocksize

2014-11-27 Thread Ekaterina Tumanova


On 11/27/2014 05:55 PM, Markus Armbruster wrote:

I'm sorry for the delay.  I got the flu and have difficulties thinking
straight for longer than a few minutes.

Ekaterina Tumanova  writes:


Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova 
---
  block.c   | 26 ++
  include/block/block.h | 20 
  include/block/block_int.h |  3 +++
  3 files changed, 49 insertions(+)

diff --git a/block.c b/block.c
index a612594..5df35cf 100644
--- a/block.c
+++ b/block.c
@@ -548,6 +548,32 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
  }
  }

+struct ProbeBlockSize bdrv_probe_blocksizes(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+struct ProbeBlockSize err_geo = { .rc = -1 };
+
+assert(drv != NULL);
+if (drv->bdrv_probe_blocksizes) {
+return drv->bdrv_probe_blocksizes(bs);
+}
+
+return err_geo;
+}


I'm not sure about "probe".  Naming is hard.  "get"?

There was already "bdrv_get_geometry", and I wanted the _geometry and 
_blocksize functions to use the same naming convention.

I thought probe might be more suitable, since we do not "get" the value
for sure. maybe "detect"?


Squashing status and actual payload into a single struct to use as
return type isn't wrong, but unusual.  When the payload can't represent
failure conveniently, we usually return status, and write the payload to
a buffer provided by the caller, like this:

 int bdrv_get_blocksizes(BlockDriverState *bs,
 uint16_t *physical_blk_sz,
 uint16_t *logical_blk_sz)

Or, with a struct to hold both sizes:

 int bdrv_get_blocksizes(BlockDriverState *bs, BlockSizes *bsz)


Do you insist on changing that? Returning a struct via stack seemed
useful to me, since there was less probability of caller allocating
a buffer of incorrect size or smth like that.


Such a struct should ideally be used in other places where we store both
sizes.

A brief function contract comment wouldn't hurt.  Something like

 /*
  * Try to get @bs's logical and physical block size.
  * Block sizes are always a multiple of BDRV_SECTOR_SIZE.
  * On success, store them in ... and return 0.
  * On failure, return -errno.
  */


will do


+
+struct ProbeGeometry bdrv_probe_geometry(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+struct ProbeGeometry err_geo = { .rc = -1 };
+
+assert(drv != NULL);
+if (drv->bdrv_probe_geometry) {
+return drv->bdrv_probe_geometry(bs);
+}
+
+return err_geo;
+}
+
  /*
   * Create a uniquely-named empty temporary file.
   * Return 0 upon success, otherwise a negative errno value.
diff --git a/include/block/block.h b/include/block/block.h
index 5450610..3287dbc 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -60,6 +60,24 @@ typedef enum {
  BDRV_REQ_MAY_UNMAP= 0x4,
  } BdrvRequestFlags;

+struct ProbeBlockSize {
+int rc;
+struct BlockSize {
+uint16_t phys;
+uint16_t log;
+} size;
+};


Use of uint16_t for block sizes is silly, but not your fault, you just
follow existing usage.


+
+struct ProbeGeometry {
+int rc;
+struct HDGeometry {
+uint32_t heads;
+uint32_t sectors;
+uint32_t cylinders;
+uint32_t start;
+} geo;
+};
+
  #define BDRV_O_RDWR0x0002
  #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
in a snapshot */
  #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
@@ -538,6 +556,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
   * the old #AioContext is not executing.
   */
  void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+struct ProbeBlockSize bdrv_probe_blocksizes(BlockDriverState *bs);
+struct ProbeGeometry bdrv_probe_geometry(BlockDriverState *bs);

  void bdrv_io_plug(BlockDriverState *bs);
  void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a1c17b9..830e564 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -271,6 +271,9 @@ struct BlockDriver {
  void (*bdrv_io_unplug)(BlockDriverState *bs);
  void (*bdrv_flush_io_queue)(BlockDriverState *bs);

+struct ProbeBlockSize (*bdrv_probe_blocksizes)(BlockDriverState *bs);
+struct ProbeGeometry (*bdrv_probe_geometry)(BlockDriverState *bs);
+
  QLIST_ENTRY(BlockDriver) list;
  };


Callbacks need contracts even more than functions do.  I know this file
is full of bad examples.  Let's not add more :)



Thanks!
Kate.

Re: [Qemu-devel] [PATCH v2 0/6] Geometry and blocksize support for backing devices

2014-11-26 Thread Ekaterina Tumanova


On 11/25/2014 04:01 PM, Stefan Hajnoczi wrote:

On Wed, Nov 19, 2014 at 11:17:50AM +0100, Ekaterina Tumanova wrote:

Hi folks,

I'm sorry for the recent spam. I messed up during code submission last time.
So please ignore any previous notes you received from me and answer only to
this thread.

This is the rework of the geometry+blocksize patch, which was
recently discussed here:
http://lists.gnu.org/archive/html/qemu-devel/2014-11/msg01148.html

Markus suggested that we only detect blocksize and geometry for DASDs.

According to this agreement new version contains DASD special casing.
The driver methods are implemented only for "host_device" and inner hdev_xxx
functions check if the backing storage is a DASD by means of
BIODASDINFO2 ioctl.

Original patchset can be found here:
http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg03791.html


This is description is mainly a changelog.  Links to previous email
threads are useful for additional info but please include a
self-contained description of the series and the rationale behind it.


will include into the next version


Comments:

1. This series overrides the logical_block_size and
physical_block_size options for raw images on DASD devices.  Users
expect their command-line options to be honored, so the options
should not be overriden if they have been given on the command-line.


will fix that


2. Only virtio_blk is modified, this is inconsistent.  All emulated
storage controllers using BlockConf have the same block size
probing behavior.


I will add blkconf_blocksizes call to other BlockConf users.



3. Why does s390 need to customize hd_geometry_guess()?

Since hd_geometry_guess contains semantics of x86-specific LBA 
translation, we have to modify it not to get in the way of z

architecture


4. Please use scripts/checkpatch.pl to check coding style.


I did :)

Thanks a lot,
Kate.

Re: [Qemu-devel] [PATCH] geometry: fix i386 compilation

2014-11-21 Thread Ekaterina Tumanova


On 11/20/2014 07:18 PM, Kevin Wolf wrote:

Am 19.11.2014 um 16:04 hat Cornelia Huck geschrieben:

On Wed, 19 Nov 2014 14:40:07 +
Peter Maydell  wrote:


On 19 November 2014 14:01, Ekaterina Tumanova
 wrote:

Signed-off-by: Ekaterina Tumanova 


Could you give the compiler error/warning message, please
(and the compiler version)?

These changes look sensible but it's not clear to me why
the current code would cause a compilation failure or why
that would be specific to i386...


This patch is referring to Kate's patch set "[PATCH v2 0/6] Geometry
and blocksize support for backing devices", which did not compile on
i386 (see <546c8161.10...@de.ibm.com>).


Where does it need to be squashed in? We want to keep the tree
bisectable, so breaking the build and then fixing it at the end of the
series isn't an option.

Kevin



This is just a small -in-reply-to patch to Christian's note, fixing the
patch set compilation on i386.
It should be squashed into the last patch in a series:
"[PATCH v2 6/6] geometry: Target specific hook for s390x in geometry
guessing".
I just don't want to send a new version before receiving any substantial
comments.

Kate.

[Qemu-devel] [PATCH] geometry: fix i386 compilation

Signed-off-by: Ekaterina Tumanova 
---
 hw/block/hd-geometry.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index b462225..905d2c6 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -147,7 +147,8 @@ void hd_geometry_guess(BlockBackend *blk,
uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans)
 {
-int cylinders, heads, secs, translation;
+uint32_t cylinders, heads, secs;
+int translation = BIOS_ATA_TRANSLATION_NONE;
 struct ProbeGeometry geometry = blk_probe_geometry(blk);
 
 if (geometry.rc == 0) {
@@ -173,9 +174,6 @@ void hd_geometry_guess(BlockBackend *blk,
 *pcyls = cylinders;
 *pheads = heads;
 *psecs = secs;
-/* disable any translation to be in sync with
-   the logical geometry */
-translation = BIOS_ATA_TRANSLATION_NONE;
 }
 done:
 if (ptrans) {
-- 
1.8.5.5

[Qemu-devel] [PATCH v2 6/6] geometry: Target specific hook for s390x in geometry guessing

For target-s390x, the behaviour is chosen as follows:
If no geo could be guessed from backing device, guess_disk_lchs (partition
table guessing) is called.
If this is not successful (e.g. image files) geometry is derived from the
size of the disk (as before).
We have no translation on s390, so we simply set in to NONE.

Signed-off-by: Ekaterina Tumanova 
---
 hw/block/Makefile.objs |  6 +-
 hw/block/hd-geometry.c | 36 +++-
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index d4c3ab7..1f7da7a 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += block.o cdrom.o hd-geometry.o
+common-obj-y += block.o cdrom.o
 common-obj-$(CONFIG_FDC) += fdc.o
 common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
 common-obj-$(CONFIG_NAND) += nand.o
@@ -13,3 +13,7 @@ obj-$(CONFIG_SH4) += tc58128.o
 
 obj-$(CONFIG_VIRTIO) += virtio-blk.o
 obj-$(CONFIG_VIRTIO) += dataplane/
+
+# geometry is target/architecture dependent and therefore needs to be built
+# for every target platform
+obj-y += hd-geometry.o
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 4972114..b462225 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -49,11 +49,12 @@ struct partition {
 
 /* 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(BlockBackend *blk,
-   int *pcylinders, int *pheads, int *psectors)
+static int guess_disk_lchs(BlockBackend *blk, uint32_t *pcylinders,
+   uint32_t *pheads, uint32_t *psectors)
 {
 uint8_t buf[BDRV_SECTOR_SIZE];
-int i, heads, sectors, cylinders;
+int i;
+uint32_t heads, sectors, cylinders;
 struct partition *p;
 uint32_t nr_sects;
 uint64_t nb_sectors;
@@ -116,11 +117,11 @@ static void guess_chs_for_size(BlockBackend *blk,
 *psecs = 63;
 }
 
+#ifdef TARGET_S390X
 void hd_geometry_guess(BlockBackend *blk,
uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans)
 {
-int cylinders, heads, secs, translation;
 struct ProbeGeometry geometry = blk_probe_geometry(blk);
 
 if (geometry.rc == 0) {
@@ -129,7 +130,32 @@ void hd_geometry_guess(BlockBackend *blk,
 *pheads = geometry.geo.heads;
 goto done;
 }
+if (guess_disk_lchs(blk, pcyls, pheads, psecs) == 0) {
+goto done;
+}
+guess_chs_for_size(blk, pcyls, pheads, psecs);
+done:
+if (ptrans) {
+*ptrans = BIOS_ATA_TRANSLATION_NONE;
+}
 
+trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs,
+BIOS_ATA_TRANSLATION_NONE);
+}
+#else
+void hd_geometry_guess(BlockBackend *blk,
+   uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
+   int *ptrans)
+{
+int cylinders, heads, secs, translation;
+struct ProbeGeometry geometry = blk_probe_geometry(blk);
+
+if (geometry.rc == 0) {
+*pcyls = geometry.geo.cylinders;
+*psecs = geometry.geo.sectors;
+*pheads = geometry.geo.heads;
+goto done;
+}
 if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
 guess_chs_for_size(blk, pcyls, pheads, psecs);
@@ -157,7 +183,7 @@ done:
 }
 trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs, translation);
 }
-
+#endif
 int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs)
 {
 return cyls <= 1024 && heads <= 16 && secs <= 63
-- 
1.8.5.5

[Qemu-devel] [PATCH v2 3/6] geometry: Add driver methods to probe blocksizes and geometry

This patch introduces driver methods of defining disk blocksizes
(physical and logical) and hard drive geometry.
The method is only implemented for "host_device". For "raw" devices
driver calls child's method.
The detection will only work for DASD devices. In order to check that
a local CheckForDASD function was introduced, which calls BIODASDINFO2 ioctl
and returns 0 only if it succeeds.

Signed-off-by: Ekaterina Tumanova 
---
 block/raw-posix.c | 65 +++
 block/raw_bsd.c   | 12 ++
 2 files changed, 77 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 45f1d79..274ceda 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,7 @@
 #include 
 #include 
 #include 
+#include 
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
@@ -93,6 +94,10 @@
 #include 
 #endif
 
+#ifdef __s390__
+#include 
+#endif
+
 //#define DEBUG_FLOPPY
 
 //#define DEBUG_BLOCK
@@ -678,6 +683,64 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 bs->bl.opt_mem_alignment = s->buf_align;
 }
 
+static int CheckForDASD(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#endif
+return -1;
+}
+
+static struct ProbeBlockSize hdev_probe_blocksizes(BlockDriverState *bs)
+{
+BDRVRawState *s = bs->opaque;
+struct ProbeBlockSize block_sizes = {0};
+
+block_sizes.rc = CheckForDASD(s->fd);
+/* If DASD, get blocksizes */
+if (block_sizes.rc == 0) {
+block_sizes.size.log = probe_logical_blocksize(bs, s->fd);
+block_sizes.size.phys = probe_physical_blocksize(bs, s->fd);
+}
+
+return block_sizes;
+}
+
+static struct ProbeGeometry hdev_probe_geometry(BlockDriverState *bs)
+{
+BDRVRawState *s = bs->opaque;
+struct ProbeGeometry geometry = {0};
+struct hd_geometry ioctl_geo = {0};
+
+geometry.rc = CheckForDASD(s->fd);
+if (geometry.rc != 0) {
+goto done;
+}
+/* If DASD, get it's geometry */
+geometry.rc = ioctl(s->fd, HDIO_GETGEO, &ioctl_geo);
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+geometry.rc = -1;
+goto done;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+geometry.rc = -1;
+goto done;
+}
+if (geometry.rc == 0) {
+geometry.geo.heads = (uint32_t)ioctl_geo.heads;
+geometry.geo.sectors = (uint32_t)ioctl_geo.sectors;
+geometry.geo.cylinders = (uint32_t)ioctl_geo.cylinders;
+geometry.geo.start = (uint32_t)ioctl_geo.start;
+}
+done:
+   return geometry;
+}
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2128,6 +2191,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,
 
 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 401b967..d164eba 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -173,6 +173,16 @@ static int raw_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 1;
 }
 
+static struct ProbeBlockSize raw_probe_blocksizes(BlockDriverState *bs)
+{
+return bdrv_probe_blocksizes(bs->file);
+}
+
+static struct ProbeGeometry raw_probe_geometry(BlockDriverState *bs)
+{
+return bdrv_probe_geometry(bs->file);
+}
+
 static BlockDriver bdrv_raw = {
 .format_name  = "raw",
 .bdrv_probe   = &raw_probe,
@@ -190,6 +200,8 @@ static BlockDriver bdrv_raw = {
 .has_variable_length  = true,
 .bdrv_get_info= &raw_get_info,
 .bdrv_refresh_limits  = &raw_refresh_limits,
+.bdrv_probe_blocksizes = &raw_probe_blocksizes,
+.bdrv_probe_geometry  = &raw_probe_geometry,
 .bdrv_is_inserted = &raw_is_inserted,
 .bdrv_media_changed   = &raw_media_changed,
 .bdrv_eject   = &raw_eject,
-- 
1.8.5.5

[Qemu-devel] [PATCH v2 1/6] geometry: add bdrv functions for geometry and blocksize

Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova 
---
 block.c   | 26 ++
 include/block/block.h | 20 
 include/block/block_int.h |  3 +++
 3 files changed, 49 insertions(+)

diff --git a/block.c b/block.c
index a612594..5df35cf 100644
--- a/block.c
+++ b/block.c
@@ -548,6 +548,32 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+struct ProbeBlockSize bdrv_probe_blocksizes(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+struct ProbeBlockSize err_geo = { .rc = -1 };
+
+assert(drv != NULL);
+if (drv->bdrv_probe_blocksizes) {
+return drv->bdrv_probe_blocksizes(bs);
+}
+
+return err_geo;
+}
+
+struct ProbeGeometry bdrv_probe_geometry(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+struct ProbeGeometry err_geo = { .rc = -1 };
+
+assert(drv != NULL);
+if (drv->bdrv_probe_geometry) {
+return drv->bdrv_probe_geometry(bs);
+}
+
+return err_geo;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
diff --git a/include/block/block.h b/include/block/block.h
index 5450610..3287dbc 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -60,6 +60,24 @@ typedef enum {
 BDRV_REQ_MAY_UNMAP= 0x4,
 } BdrvRequestFlags;
 
+struct ProbeBlockSize {
+int rc;
+struct BlockSize {
+uint16_t phys;
+uint16_t log;
+} size;
+};
+
+struct ProbeGeometry {
+int rc;
+struct HDGeometry {
+uint32_t heads;
+uint32_t sectors;
+uint32_t cylinders;
+uint32_t start;
+} geo;
+};
+
 #define BDRV_O_RDWR0x0002
 #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
in a snapshot */
 #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
@@ -538,6 +556,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
  * the old #AioContext is not executing.
  */
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+struct ProbeBlockSize bdrv_probe_blocksizes(BlockDriverState *bs);
+struct ProbeGeometry bdrv_probe_geometry(BlockDriverState *bs);
 
 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a1c17b9..830e564 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -271,6 +271,9 @@ struct BlockDriver {
 void (*bdrv_io_unplug)(BlockDriverState *bs);
 void (*bdrv_flush_io_queue)(BlockDriverState *bs);
 
+struct ProbeBlockSize (*bdrv_probe_blocksizes)(BlockDriverState *bs);
+struct ProbeGeometry (*bdrv_probe_geometry)(BlockDriverState *bs);
+
 QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
1.8.5.5

[Qemu-devel] [PATCH v2 5/6] geometry: Call backend function to detect geometry and blocksize

hd_geometry_guess function autodetects the drive geometry. This patch
adds a block backend call, that probes the backing device geometry.
If the inner driver method is implemented and succeeds (currently only DASDs),
the blkconf_geometry will pass-through the backing device geometry.

Signed-off-by: Ekaterina Tumanova 
---
 hw/block/block.c | 11 +++
 hw/block/hd-geometry.c   |  9 +
 hw/block/virtio-blk.c|  1 +
 include/hw/block/block.h |  1 +
 4 files changed, 22 insertions(+)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..f1d29bc 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,17 @@ void blkconf_serial(BlockConf *conf, char **serial)
 }
 }
 
+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+struct ProbeBlockSize blocksize = blk_probe_blocksizes(blk);
+
+if (blocksize.rc == 0) {
+conf->physical_block_size = blocksize.size.phys;
+conf->logical_block_size = blocksize.size.log;
+}
+}
+
 void blkconf_geometry(BlockConf *conf, int *ptrans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..4972114 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,6 +121,14 @@ void hd_geometry_guess(BlockBackend *blk,
int *ptrans)
 {
 int cylinders, heads, secs, translation;
+struct ProbeGeometry geometry = blk_probe_geometry(blk);
+
+if (geometry.rc == 0) {
+*pcyls = geometry.geo.cylinders;
+*psecs = geometry.geo.sectors;
+*pheads = geometry.geo.heads;
+goto done;
+}
 
 if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
@@ -143,6 +151,7 @@ void hd_geometry_guess(BlockBackend *blk,
the logical geometry */
 translation = BIOS_ATA_TRANSLATION_NONE;
 }
+done:
 if (ptrans) {
 *ptrans = translation;
 }
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..6f01565 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -745,6 +745,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 error_propagate(errp, err);
 return;
 }
+blkconf_blocksizes(&conf->conf);
 
 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 0d0ce9a..856bf75 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -63,6 +63,7 @@ void blkconf_serial(BlockConf *conf, char **serial);
 void blkconf_geometry(BlockConf *conf, int *trans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp);
+void blkconf_blocksizes(BlockConf *conf);
 
 /* Hard disk geometry */
 
-- 
1.8.5.5

[Qemu-devel] [PATCH v2 2/6] geometry: Detect blocksize via ioctls in separate static functions

Move the IOCTL calls that detect logical blocksize from raw_probe_alignment
into separate function (probe_logical_blocksize).
Introduce function which detect physical blocksize via IOCTL
(probe_physical_blocksize).
Both functions will be used in the next patch.

Signed-off-by: Ekaterina Tumanova 
---
 block/raw-posix.c | 58 +--
 1 file changed, 39 insertions(+), 19 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e100ae2..45f1d79 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -223,50 +223,70 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
-static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+static unsigned int probe_logical_blocksize(BlockDriverState *bs, int fd)
 {
-BDRVRawState *s = bs->opaque;
-char *buf;
-unsigned int sector_size;
-
-/* For /dev/sg devices the alignment is not really used.
-   With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !s->needs_alignment) {
-bs->request_alignment = 1;
-s->buf_align = 1;
-return;
-}
+unsigned int sector_size = 0;
 
 /* Try a few ioctls to get the right size */
-bs->request_alignment = 0;
-s->buf_align = 0;
-
 #ifdef BLKSSZGET
 if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef DKIOCGETBLOCKSIZE
 if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef DIOCGSECTORSIZE
 if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
 if (xfsctl(NULL, fd, XFS_IOC_DIOINFO, &da) >= 0) {
-bs->request_alignment = da.d_miniosz;
+sector_size = da.d_miniosz;
 /* The kernel returns wrong information for d_mem */
 /* s->buf_align = da.d_mem; */
+return sector_size;
 }
 }
 #endif
 
+return 0;
+}
+
+static unsigned int probe_physical_blocksize(BlockDriverState *bs, int fd)
+{
+unsigned int blk_size = 0;
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, &blk_size) >= 0) {
+return blk_size;
+}
+#endif
+
+return 0;
+}
+
+static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !s->needs_alignment) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+bs->request_alignment = probe_logical_blocksize(bs, fd);
+
 /* If we could not get the sizes so far, we can only guess them */
 if (!s->buf_align) {
 size_t align;
-- 
1.8.5.5

[Qemu-devel] [PATCH v2 4/6] geometry: Add block-backend wrappers for geometry probing

Signed-off-by: Ekaterina Tumanova 
---
 block/block-backend.c  | 10 ++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index d0692b1..6717301 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -629,3 +629,13 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, 
BlockBackend *blk,
 {
 return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
 }
+
+struct ProbeBlockSize blk_probe_blocksizes(BlockBackend *blk)
+{
+return bdrv_probe_blocksizes(blk->bs);
+}
+
+struct ProbeGeometry blk_probe_geometry(BlockBackend *blk)
+{
+return bdrv_probe_geometry(blk->bs);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 52d13c1..e8b497a 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -138,5 +138,7 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk);
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque);
+struct ProbeBlockSize blk_probe_blocksizes(BlockBackend *blk);
+struct ProbeGeometry blk_probe_geometry(BlockBackend *blk);
 
 #endif
-- 
1.8.5.5

[Qemu-devel] [PATCH v2 0/6] Geometry and blocksize support for backing devices

Hi folks,

I'm sorry for the recent spam. I messed up during code submission last time.
So please ignore any previous notes you received from me and answer only to
this thread.

This is the rework of the geometry+blocksize patch, which was
recently discussed here:
http://lists.gnu.org/archive/html/qemu-devel/2014-11/msg01148.html

Markus suggested that we only detect blocksize and geometry for DASDs.

According to this agreement new version contains DASD special casing.
The driver methods are implemented only for "host_device" and inner hdev_xxx
functions check if the backing storage is a DASD by means of
BIODASDINFO2 ioctl.

Original patchset can be found here:
http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg03791.html

Ekaterina Tumanova (6):
  geometry: add bdrv functions for geometry and blocksize
  geometry: Detect blocksize via ioctls in separate static functions
  geometry: Add driver methods to probe blocksizes and geometry
  geometry: Add block-backend wrappers for geometry probing
  geometry: Call backend function to detect geometry and blocksize
  geometry: Target specific hook for s390x in geometry guessing

 block.c|  26 +
 block/block-backend.c  |  10 
 block/raw-posix.c  | 123 ++---
 block/raw_bsd.c|  12 
 hw/block/Makefile.objs |   6 +-
 hw/block/block.c   |  11 
 hw/block/hd-geometry.c |  43 --
 hw/block/virtio-blk.c  |   1 +
 include/block/block.h  |  20 +++
 include/block/block_int.h  |   3 +
 include/hw/block/block.h   |   1 +
 include/sysemu/block-backend.h |   2 +
 12 files changed, 234 insertions(+), 24 deletions(-)

-- 
1.8.5.5

[Qemu-devel] Fwd: [PATCH v2 0/6] Geometry and blocksize support for backing devices



copying to mail list

 Forwarded Message 
Subject: [PATCH v2 0/6] Geometry and blocksize support for backing devices
Date: Tue, 18 Nov 2014 17:09:55 +0100
From: Ekaterina Tumanova 
To: arm...@redhat.com, pbonz...@redhat.com, stefa...@redhat.com, 
kw...@redhat.com
CC: borntrae...@de.ibm.com, cornelia.h...@de.ibm.com, 
d...@linux.vnet.ibm.com, mihaj...@linux.vnet.ibm.com, Ekaterina Tumanova 



This is the rework of the geometry+blocksize patch, which was
recently discussed here:
http://lists.gnu.org/archive/html/qemu-devel/2014-11/msg01148.html

Markus suggested that we only detect blocksize and geometry for DASDs.

According to this agreement new version contains DASD special casing.
The driver methods are implemented only for "host_device" and inner hdev_xxx
functions check if the backing storage is a DASD by means of
BIODASDINFO2 ioctl.

Original patchset can be found here:
http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg03791.html

Ekaterina Tumanova (6):
  geometry: add bdrv functions for geometry and blocksize
  geometry: Detect blocksize via ioctls in separate static functions
  geometry: Add driver methods to probe blocksizes and geometry
  geometry: Add block-backend wrappers for geometry probing
  geometry: Call backend function to detect geometry and blocksize
  geometry: Target specific hook for s390x in geometry guessing

 block.c|  26 +
 block/block-backend.c  |  10 
 block/raw-posix.c  | 123 
++---

 block/raw_bsd.c|  12 
 hw/block/Makefile.objs |   6 +-
 hw/block/block.c   |  11 
 hw/block/hd-geometry.c |  43 --
 hw/block/virtio-blk.c  |   1 +
 include/block/block.h  |  20 +++
 include/block/block_int.h  |   3 +
 include/hw/block/block.h   |   1 +
 include/sysemu/block-backend.h |   2 +
 12 files changed, 234 insertions(+), 24 deletions(-)

--
1.8.5.5

[Qemu-devel] Fwd: [PATCH v2 4/6] geometry: Add block-backend wrappers for geometry probing


copying to mail list


 Forwarded Message 
Subject: [PATCH v2 4/6] geometry: Add block-backend wrappers for 
geometry probing

Date: Tue, 18 Nov 2014 17:09:59 +0100
From: Ekaterina Tumanova 
To: arm...@redhat.com, pbonz...@redhat.com, stefa...@redhat.com, 
kw...@redhat.com
CC: borntrae...@de.ibm.com, cornelia.h...@de.ibm.com, 
d...@linux.vnet.ibm.com, mihaj...@linux.vnet.ibm.com, Ekaterina Tumanova 



Signed-off-by: Ekaterina Tumanova 
---
 block/block-backend.c  | 10 ++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index d0692b1..6717301 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -629,3 +629,13 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, 
BlockBackend *blk,

 {
 return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
 }
+
+struct ProbeBlockSize blk_probe_blocksizes(BlockBackend *blk)
+{
+return bdrv_probe_blocksizes(blk->bs);
+}
+
+struct ProbeGeometry blk_probe_geometry(BlockBackend *blk)
+{
+return bdrv_probe_geometry(blk->bs);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 52d13c1..e8b497a 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -138,5 +138,7 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk);

 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque);
+struct ProbeBlockSize blk_probe_blocksizes(BlockBackend *blk);
+struct ProbeGeometry blk_probe_geometry(BlockBackend *blk);

 #endif
--
1.8.5.5

[Qemu-devel] Fwd: [PATCH v2 6/6] geometry: Target specific hook for s390x in geometry guessing



copying to mail list

 Forwarded Message 
Subject: [PATCH v2 6/6] geometry: Target specific hook for s390x in 
geometry guessing

Date: Tue, 18 Nov 2014 17:10:01 +0100
From: Ekaterina Tumanova 
To: arm...@redhat.com, pbonz...@redhat.com, stefa...@redhat.com, 
kw...@redhat.com
CC: borntrae...@de.ibm.com, cornelia.h...@de.ibm.com, 
d...@linux.vnet.ibm.com, mihaj...@linux.vnet.ibm.com, Ekaterina Tumanova 



For target-s390x, the behaviour is chosen as follows:
If no geo could be guessed from backing device, guess_disk_lchs (partition
table guessing) is called.
If this is not successful (e.g. image files) geometry is derived from the
size of the disk (as before).
We have no translation on s390, so we simply set in to NONE.

Signed-off-by: Ekaterina Tumanova 
---
 hw/block/Makefile.objs |  6 +-
 hw/block/hd-geometry.c | 36 +++-
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index d4c3ab7..1f7da7a 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += block.o cdrom.o hd-geometry.o
+common-obj-y += block.o cdrom.o
 common-obj-$(CONFIG_FDC) += fdc.o
 common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
 common-obj-$(CONFIG_NAND) += nand.o
@@ -13,3 +13,7 @@ obj-$(CONFIG_SH4) += tc58128.o

 obj-$(CONFIG_VIRTIO) += virtio-blk.o
 obj-$(CONFIG_VIRTIO) += dataplane/
+
+# geometry is target/architecture dependent and therefore needs to be built
+# for every target platform
+obj-y += hd-geometry.o
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 4972114..b462225 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -49,11 +49,12 @@ struct partition {

 /* 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(BlockBackend *blk,
-   int *pcylinders, int *pheads, int *psectors)
+static int guess_disk_lchs(BlockBackend *blk, uint32_t *pcylinders,
+   uint32_t *pheads, uint32_t *psectors)
 {
 uint8_t buf[BDRV_SECTOR_SIZE];
-int i, heads, sectors, cylinders;
+int i;
+uint32_t heads, sectors, cylinders;
 struct partition *p;
 uint32_t nr_sects;
 uint64_t nb_sectors;
@@ -116,11 +117,11 @@ static void guess_chs_for_size(BlockBackend *blk,
 *psecs = 63;
 }

+#ifdef TARGET_S390X
 void hd_geometry_guess(BlockBackend *blk,
uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans)
 {
-int cylinders, heads, secs, translation;
 struct ProbeGeometry geometry = blk_probe_geometry(blk);

 if (geometry.rc == 0) {
@@ -129,7 +130,32 @@ void hd_geometry_guess(BlockBackend *blk,
 *pheads = geometry.geo.heads;
 goto done;
 }
+if (guess_disk_lchs(blk, pcyls, pheads, psecs) == 0) {
+goto done;
+}
+guess_chs_for_size(blk, pcyls, pheads, psecs);
+done:
+if (ptrans) {
+*ptrans = BIOS_ATA_TRANSLATION_NONE;
+}

+trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs,
+BIOS_ATA_TRANSLATION_NONE);
+}
+#else
+void hd_geometry_guess(BlockBackend *blk,
+   uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
+   int *ptrans)
+{
+int cylinders, heads, secs, translation;
+struct ProbeGeometry geometry = blk_probe_geometry(blk);
+
+if (geometry.rc == 0) {
+*pcyls = geometry.geo.cylinders;
+*psecs = geometry.geo.sectors;
+*pheads = geometry.geo.heads;
+goto done;
+}
 if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
 guess_chs_for_size(blk, pcyls, pheads, psecs);
@@ -157,7 +183,7 @@ done:
 }
 trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs, translation);
 }
-
+#endif
 int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs)
 {
 return cyls <= 1024 && heads <= 16 && secs <= 63
--
1.8.5.5

[Qemu-devel] Fwd: [PATCH v2 5/6] geometry: Call backend function to detect geometry and blocksize


copying to mail list


 Forwarded Message 
Subject: [PATCH v2 5/6] geometry: Call backend function to detect 
geometry and blocksize

Date: Tue, 18 Nov 2014 17:10:00 +0100
From: Ekaterina Tumanova 
To: arm...@redhat.com, pbonz...@redhat.com, stefa...@redhat.com, 
kw...@redhat.com
CC: borntrae...@de.ibm.com, cornelia.h...@de.ibm.com, 
d...@linux.vnet.ibm.com, mihaj...@linux.vnet.ibm.com, Ekaterina Tumanova 



hd_geometry_guess function autodetects the drive geometry. This patch
adds a block backend call, that probes the backing device geometry.
If the inner driver method is implemented and succeeds (currently only 
DASDs),

the blkconf_geometry will pass-through the backing device geometry.

Signed-off-by: Ekaterina Tumanova 
---
 hw/block/block.c | 11 +++
 hw/block/hd-geometry.c   |  9 +
 hw/block/virtio-blk.c|  1 +
 include/hw/block/block.h |  1 +
 4 files changed, 22 insertions(+)

diff --git a/hw/block/block.c b/hw/block/block.c
index a625773..f1d29bc 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -25,6 +25,17 @@ void blkconf_serial(BlockConf *conf, char **serial)
 }
 }

+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+struct ProbeBlockSize blocksize = blk_probe_blocksizes(blk);
+
+if (blocksize.rc == 0) {
+conf->physical_block_size = blocksize.size.phys;
+conf->logical_block_size = blocksize.size.log;
+}
+}
+
 void blkconf_geometry(BlockConf *conf, int *ptrans,
   unsigned cyls_max, unsigned heads_max, unsigned 
secs_max,

   Error **errp)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6fcf74d..4972114 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -121,6 +121,14 @@ void hd_geometry_guess(BlockBackend *blk,
int *ptrans)
 {
 int cylinders, heads, secs, translation;
+struct ProbeGeometry geometry = blk_probe_geometry(blk);
+
+if (geometry.rc == 0) {
+*pcyls = geometry.geo.cylinders;
+*psecs = geometry.geo.sectors;
+*pheads = geometry.geo.heads;
+goto done;
+}

 if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {
 /* no LCHS guess: use a standard physical disk geometry  */
@@ -143,6 +151,7 @@ void hd_geometry_guess(BlockBackend *blk,
the logical geometry */
 translation = BIOS_ATA_TRANSLATION_NONE;
 }
+done:
 if (ptrans) {
 *ptrans = translation;
 }
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..6f01565 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -745,6 +745,7 @@ static void virtio_blk_device_realize(DeviceState 
*dev, Error **errp)

 error_propagate(errp, err);
 return;
 }
+blkconf_blocksizes(&conf->conf);

 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 0d0ce9a..856bf75 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -63,6 +63,7 @@ void blkconf_serial(BlockConf *conf, char **serial);
 void blkconf_geometry(BlockConf *conf, int *trans,
   unsigned cyls_max, unsigned heads_max, unsigned 
secs_max,

   Error **errp);
+void blkconf_blocksizes(BlockConf *conf);

 /* Hard disk geometry */

--
1.8.5.5

[Qemu-devel] Fwd: [PATCH v2 3/6] geometry: Add driver methods to probe blocksizes and geometry



copying to mail list

 Forwarded Message 
Subject: [PATCH v2 3/6] geometry: Add driver methods to probe blocksizes 
and geometry

Date: Tue, 18 Nov 2014 17:09:58 +0100
From: Ekaterina Tumanova 
To: arm...@redhat.com, pbonz...@redhat.com, stefa...@redhat.com, 
kw...@redhat.com
CC: borntrae...@de.ibm.com, cornelia.h...@de.ibm.com, 
d...@linux.vnet.ibm.com, mihaj...@linux.vnet.ibm.com, Ekaterina Tumanova 



This patch introduces driver methods of defining disk blocksizes
(physical and logical) and hard drive geometry.
The method is only implemented for "host_device". For "raw" devices
driver calls child's method.
The detection will only work for DASD devices. In order to check that
a local CheckForDASD function was introduced, which calls BIODASDINFO2 ioctl
and returns 0 only if it succeeds.

Signed-off-by: Ekaterina Tumanova 
---
 block/raw-posix.c | 65 
+++

 block/raw_bsd.c   | 12 ++
 2 files changed, 77 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 45f1d79..274ceda 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,7 @@
 #include 
 #include 
 #include 
+#include 
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
@@ -93,6 +94,10 @@
 #include 
 #endif

+#ifdef __s390__
+#include 
+#endif
+
 //#define DEBUG_FLOPPY

 //#define DEBUG_BLOCK
@@ -678,6 +683,64 @@ static void raw_refresh_limits(BlockDriverState 
*bs, Error **errp)

 bs->bl.opt_mem_alignment = s->buf_align;
 }

+static int CheckForDASD(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#endif
+return -1;
+}
+
+static struct ProbeBlockSize hdev_probe_blocksizes(BlockDriverState *bs)
+{
+BDRVRawState *s = bs->opaque;
+struct ProbeBlockSize block_sizes = {0};
+
+block_sizes.rc = CheckForDASD(s->fd);
+/* If DASD, get blocksizes */
+if (block_sizes.rc == 0) {
+block_sizes.size.log = probe_logical_blocksize(bs, s->fd);
+block_sizes.size.phys = probe_physical_blocksize(bs, s->fd);
+}
+
+return block_sizes;
+}
+
+static struct ProbeGeometry hdev_probe_geometry(BlockDriverState *bs)
+{
+BDRVRawState *s = bs->opaque;
+struct ProbeGeometry geometry = {0};
+struct hd_geometry ioctl_geo = {0};
+
+geometry.rc = CheckForDASD(s->fd);
+if (geometry.rc != 0) {
+goto done;
+}
+/* If DASD, get it's geometry */
+geometry.rc = ioctl(s->fd, HDIO_GETGEO, &ioctl_geo);
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+geometry.rc = -1;
+goto done;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+geometry.rc = -1;
+goto done;
+}
+if (geometry.rc == 0) {
+geometry.geo.heads = (uint32_t)ioctl_geo.heads;
+geometry.geo.sectors = (uint32_t)ioctl_geo.sectors;
+geometry.geo.cylinders = (uint32_t)ioctl_geo.cylinders;
+geometry.geo.start = (uint32_t)ioctl_geo.start;
+}
+done:
+   return geometry;
+}
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2128,6 +2191,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,

 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 401b967..d164eba 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -173,6 +173,16 @@ static int raw_probe(const uint8_t *buf, int 
buf_size, const char *filename)

 return 1;
 }

+static struct ProbeBlockSize raw_probe_blocksizes(BlockDriverState *bs)
+{
+return bdrv_probe_blocksizes(bs->file);
+}
+
+static struct ProbeGeometry raw_probe_geometry(BlockDriverState *bs)
+{
+return bdrv_probe_geometry(bs->file);
+}
+
 static BlockDriver bdrv_raw = {
 .format_name  = "raw",
 .bdrv_probe   = &raw_probe,
@@ -190,6 +200,8 @@ static BlockDriver bdrv_raw = {
 .has_variable_length  = true,
 .bdrv_get_info= &raw_get_info,
 .bdrv_refresh_limits  = &raw_refresh_limits,
+.bdrv_probe_blocksizes = &raw_probe_blocksizes,
+.bdrv_probe_geometry  = &raw_probe_geometry,
 .bdrv_is_inserted = &raw_is_inserted,
 .bdrv_media_changed   = &raw_media_changed,
 .bdrv_eject   = &raw_eject,
--
1.8.5.5

[Qemu-devel] Fwd: [PATCH v2 2/6] geometry: Detect blocksize via ioctls in separate static functions


copying to mail list


 Forwarded Message 
Subject: [PATCH v2 2/6] geometry: Detect blocksize via ioctls in 
separate static functions

Date: Tue, 18 Nov 2014 17:09:57 +0100
From: Ekaterina Tumanova 
To: arm...@redhat.com, pbonz...@redhat.com, stefa...@redhat.com, 
kw...@redhat.com
CC: borntrae...@de.ibm.com, cornelia.h...@de.ibm.com, 
d...@linux.vnet.ibm.com, mihaj...@linux.vnet.ibm.com, Ekaterina Tumanova 



Move the IOCTL calls that detect logical blocksize from raw_probe_alignment
into separate function (probe_logical_blocksize).
Introduce function which detect physical blocksize via IOCTL
(probe_physical_blocksize).
Both functions will be used in the next patch.

Signed-off-by: Ekaterina Tumanova 
---
 block/raw-posix.c | 58 
+--

 1 file changed, 39 insertions(+), 19 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e100ae2..45f1d79 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -223,50 +223,70 @@ static int raw_normalize_devicepath(const char 
**filename)

 }
 #endif

-static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+static unsigned int probe_logical_blocksize(BlockDriverState *bs, int fd)
 {
-BDRVRawState *s = bs->opaque;
-char *buf;
-unsigned int sector_size;
-
-/* For /dev/sg devices the alignment is not really used.
-   With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !s->needs_alignment) {
-bs->request_alignment = 1;
-s->buf_align = 1;
-return;
-}
+unsigned int sector_size = 0;

 /* Try a few ioctls to get the right size */
-bs->request_alignment = 0;
-s->buf_align = 0;
-
 #ifdef BLKSSZGET
 if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef DKIOCGETBLOCKSIZE
 if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef DIOCGSECTORSIZE
 if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
 if (xfsctl(NULL, fd, XFS_IOC_DIOINFO, &da) >= 0) {
-bs->request_alignment = da.d_miniosz;
+sector_size = da.d_miniosz;
 /* The kernel returns wrong information for d_mem */
 /* s->buf_align = da.d_mem; */
+return sector_size;
 }
 }
 #endif

+return 0;
+}
+
+static unsigned int probe_physical_blocksize(BlockDriverState *bs, int fd)
+{
+unsigned int blk_size = 0;
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, &blk_size) >= 0) {
+return blk_size;
+}
+#endif
+
+return 0;
+}
+
+static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !s->needs_alignment) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+bs->request_alignment = probe_logical_blocksize(bs, fd);
+
 /* If we could not get the sizes so far, we can only guess them */
 if (!s->buf_align) {
 size_t align;
--
1.8.5.5

[Qemu-devel] Fwd: [PATCH v2 1/6] geometry: add bdrv functions for geometry and blocksize



copying to mail list

 Forwarded Message 
Subject: [PATCH v2 1/6] geometry: add bdrv functions for geometry and 
blocksize

Date: Tue, 18 Nov 2014 17:09:56 +0100
From: Ekaterina Tumanova 
To: arm...@redhat.com, pbonz...@redhat.com, stefa...@redhat.com, 
kw...@redhat.com
CC: borntrae...@de.ibm.com, cornelia.h...@de.ibm.com, 
d...@linux.vnet.ibm.com, mihaj...@linux.vnet.ibm.com, Ekaterina Tumanova 



Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova 
---
 block.c   | 26 ++
 include/block/block.h | 20 
 include/block/block_int.h |  3 +++
 3 files changed, 49 insertions(+)

diff --git a/block.c b/block.c
index a612594..5df35cf 100644
--- a/block.c
+++ b/block.c
@@ -548,6 +548,32 @@ void bdrv_refresh_limits(BlockDriverState *bs, 
Error **errp)

 }
 }

+struct ProbeBlockSize bdrv_probe_blocksizes(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+struct ProbeBlockSize err_geo = { .rc = -1 };
+
+assert(drv != NULL);
+if (drv->bdrv_probe_blocksizes) {
+return drv->bdrv_probe_blocksizes(bs);
+}
+
+return err_geo;
+}
+
+struct ProbeGeometry bdrv_probe_geometry(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+struct ProbeGeometry err_geo = { .rc = -1 };
+
+assert(drv != NULL);
+if (drv->bdrv_probe_geometry) {
+return drv->bdrv_probe_geometry(bs);
+}
+
+return err_geo;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
diff --git a/include/block/block.h b/include/block/block.h
index 5450610..3287dbc 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -60,6 +60,24 @@ typedef enum {
 BDRV_REQ_MAY_UNMAP= 0x4,
 } BdrvRequestFlags;

+struct ProbeBlockSize {
+int rc;
+struct BlockSize {
+uint16_t phys;
+uint16_t log;
+} size;
+};
+
+struct ProbeGeometry {
+int rc;
+struct HDGeometry {
+uint32_t heads;
+uint32_t sectors;
+uint32_t cylinders;
+uint32_t start;
+} geo;
+};
+
 #define BDRV_O_RDWR0x0002
 #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save 
writes in a snapshot */

 #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
@@ -538,6 +556,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
  * the old #AioContext is not executing.
  */
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+struct ProbeBlockSize bdrv_probe_blocksizes(BlockDriverState *bs);
+struct ProbeGeometry bdrv_probe_geometry(BlockDriverState *bs);

 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a1c17b9..830e564 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -271,6 +271,9 @@ struct BlockDriver {
 void (*bdrv_io_unplug)(BlockDriverState *bs);
 void (*bdrv_flush_io_queue)(BlockDriverState *bs);

+struct ProbeBlockSize (*bdrv_probe_blocksizes)(BlockDriverState *bs);
+struct ProbeGeometry (*bdrv_probe_geometry)(BlockDriverState *bs);
+
 QLIST_ENTRY(BlockDriver) list;
 };

--
1.8.5.5

Re: [Qemu-devel] Geometry and blocksize support for backing devices

2014-11-07 Thread Ekaterina Tumanova


On 11/07/2014 12:17 PM, Markus Armbruster wrote:

Christian Borntraeger  writes:


Markus, Kevin, Stefan,

here is a (somewhat late) followup of some KVM forum discussions regarding
block size and geometry of pass-through block devices. Let's just do a quick
wrap-up (as of my understanding) and a proposal at the end of the mail





- DASD/ECKD disk devices have several special properties, e.g. the geometry
   still has an influence on the partitions and each track can have a different
   format and the format of the disk actually follows what z/OS has as basic
   data structures. Linux does a low-level format of the disk to look like a
   block device (most common case is formatted with 4K blocks). There are
   still some smalls warts for z/OS compatibility (block0 has 28 bytes of data,
   block1 has 148 bytes and block2 has 84 bytes of data. everything else has
   4k. the dasd device driver will fake 4k blocks for all blocks by ignoring
   writes beyond the data for blocks 0-2 and filling the gaps with 0xe5).

- Since Linux uses DASDs as normal block device, we actually want to use
   virtio-blk to pass those to KVM guests. Due to the warts mentioned above,
   we have to have the proper block size and geometry in the guest. Otherwise
   things will fail in certain cases (linux partition detection for example)

- we have libvirt support to provide this information in the XML. This is far
   from user friendly, though, as the admin has to manually look up the
   properties of the host disks and then modify the guest definition 
accordingly.

- Kate came up with patches (based on initial patches from Einar Lueck) for
   auto-detection of geometry and block size for host block devices

- Stefan and Paolo had some concerns:
1. if the geometry etc is important, then make it part of the guest definition
2. what about migration and the target disk differs
3. is that issue system z specific or generic?

Regarding 1: this does work as of today, but it is pretty complicated for an
admin to do so

Regarding 2: System z system do not have built-in disks, they are always
accessed via fibre channel (either FICON protocol for DASDs or FCP protocol
for scsi). So its quite common to have shared access to the same disk from
different System z boxes. system z admins should be able to setup this
properly. Question is, is it ok to assume that and fail if not?

Regarding 3: No idea.

At KVM forum I talked to several people regarding a solution:

a) Stefan suggested to make the auto detection explicit, e.g.: provide a
"autodetect" tag for the secs, cylc, heads and logical_sector_size properties
This would require changes in qemu, but also in libvirt and its domain
configuration
b) Markus suggested that there are already some cases in QEMU, where we rely
on the admin to provide a proper setup on the target, e.g. an .iso
file as image.
If the target has a different content in the .iso file things will break.

Markus said, there are two classes of this.
a) problems that can be detected by QEMU. Here QEMU will abort migration


Example: device missing on target, target rejects migration when it
receives the device's state.


b) problems that cannot be detected by QEMU (e.g. different iso content). this
will trigger failures later on

a is preferred


Note that the geometry is currently in class (b).  Configuration
generally is.

A perpetual long-term goal of migration is embedding configuration in
the migration stream, to move it from (b) to (a).  Just frontend
configuration, because backend configuration is generally host-specific.

Geometry is a property of the device, thus frontend configuration.

For historical reasons, device geometry properties default to backend
values set with -drive cyls=... or -hdachs.  This is deprecated.

If the user doesn't specify geometry, QEMU makes one up, usually based
on device size.  In certain circumstances, it bases on a DOS partition
table instead.  Misfeature, in my opinion.  Partitioning a disk can give
you a different geometry on the next restart, including migration,
unless you specify the geometry explicitly.  Fortunately, most guests
don't care for geometry at all.  This is entirely undocumented, as far
as I can tell.


Now here comes my proposal:
Markus statement brought up an idea of special casing DASDs support. We can
call an ioctl BIODASDINFO on the block device that will only succeed if the host
disk is really a dasd. We could enable the auto detection for that case.


If BIODASDINFO succeeds, QEMU uses that instead of making up device
geometry as described above.  Correct?

Let's spell out when exactly BIODASDINFO succeeds, to avoid
misunderstandings.  It does when the backend is a DASD (/dev/dasd*).
What about a partition on a DASD?  A file in a filesystem on a DASD?

Auto-detecting geometry on DASDs adds an irregularity to the user
interface.  I guess DASDs are special enough to tolerate that.


In addition, QEMU will check if geometry and block size match during
migration, if

Re: [Qemu-devel] [PATCH 4/4] blocksize: add blkconf_blocksize call to all block devices

2014-09-04 Thread Ekaterina Tumanova

On 09/03/2014 07:46 PM, Stefan Hajnoczi wrote:

On Tue, Jul 29, 2014 at 02:27:19PM +0200, Ekaterina Tumanova wrote:

This patch add the blkconf_blocksize call to all
devices, which use DEFINE_BLOCK_PROPERTIES.
If the underlying driver function fails, blkconf_blocksizes
will set blocksizes to default (512) value.

Wasn't this NACKed before on the grounds that it is likely to upset the
guest after live migration? QEMU doesn't automatically query the
storage because these parameters must be preserved across migration.

Sorry, haven't found this discussion in the mail list. Do you have a link?

As far as I understand, the xx_init functions of the qemu block
devices, which contain blkconf_blocksize calls, will
be called again on the destination host before the guest is resumed. And
since migration requests qemu to be brought on the same disk, the
configuration will receive the same block size from the ioctl, as

before. What do I miss?

The knowledge of these fields belongs in the management tool that
orchestrates migration, not QEMU.

For case of DASDs we need QEMU to know the underlying blocksize.
And you mentioned in your review comment to the Einar's initial patch
that you request this to be implemented for all architectures:

"Detecting the underlying block size is a generally useful configuration
option. This should not be s390-specific, so no need to rename
DEFINE_BLOCK_PROPERTIES()."

http://qemu.11.n7.nabble.com/Qemu-devel-PATCH-V3-0-2-hd-geometry-c-Integrate-HDIO-GETGEO-in-guessing-tt185124.html#none

If you want specific parameters, please put them in your guest
configuration. QEMU and libvirt support that.

I'm concerned that this patch serious is likely to break things and
autodetection doesn't add much value since the management tool needs to
be aware of this information anyway.

Can you please explain what do you mean by "AUTOdetection"?
Do you simply mean "detection by ioctl" or "detection performed without
guest request"?

Stefan

Thanks!

[Qemu-devel] [PATCH 4/4] blocksize: add blkconf_blocksize call to all block devices

This patch add the blkconf_blocksize call to all
devices, which use DEFINE_BLOCK_PROPERTIES.
If the underlying driver function fails, blkconf_blocksizes
will set blocksizes to default (512) value.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: David Hildenbrand 
Acked-by: Cornelia Huck 
---
 hw/block/nvme.c  | 1 +
 hw/block/virtio-blk.c| 1 +
 hw/ide/qdev.c| 1 +
 hw/scsi/scsi-disk.c  | 1 +
 hw/usb/dev-storage.c | 1 +
 include/hw/block/block.h | 4 ++--
 6 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5fd8f89..50fe769 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -761,6 +761,7 @@ static int nvme_init(PCIDevice *pci_dev)
 if (!n->serial) {
 return -1;
 }
+blkconf_blocksizes(&n->conf);
 
 pci_conf = pci_dev->config;
 pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c241c50..b5027b1 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -743,6 +743,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 }
 
 blkconf_serial(&blk->conf, &blk->serial);
+blkconf_blocksizes(&blk->conf);
 s->original_wce = bdrv_enable_write_cache(blk->conf.bs);
 if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
 error_setg(errp, "Error setting geometry");
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6e475e6..6d94d8f 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -161,6 +161,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 }
 
 blkconf_serial(&dev->conf, &dev->serial);
+blkconf_blocksizes(&dev->conf);
 if (kind != IDE_CD
 && blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255) < 0) {
 return -1;
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d47ecd6..bfae48b 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2250,6 +2250,7 @@ static int scsi_initfn(SCSIDevice *dev)
 }
 
 blkconf_serial(&s->qdev.conf, &s->serial);
+blkconf_blocksizes(&s->qdev.conf);
 if (dev->type == TYPE_DISK
 && blkconf_geometry(&dev->conf, NULL, 65535, 255, 255) < 0) {
 return -1;
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ae4efcb..cf50ac1 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -603,6 +603,7 @@ static int usb_msd_initfn_storage(USBDevice *dev)
 }
 
 blkconf_serial(&s->conf, &dev->serial);
+blkconf_blocksizes(&s->conf);
 
 /*
  * Hack alert: this pretends to be a block device, but it's really
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 7a0092e..8560bb2 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -44,9 +44,9 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
 #define DEFINE_BLOCK_PROPERTIES(_state, _conf)  \
 DEFINE_PROP_DRIVE("drive", _state, _conf.bs),   \
 DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \
-  _conf.logical_block_size, 512),   \
+  _conf.logical_block_size, 0), \
 DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,\
-  _conf.physical_block_size, 512),  \
+  _conf.physical_block_size, 0),\
 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),\
-- 
1.8.5.5

[Qemu-devel] [PATCH 1/4] hd-geometry.c: Integrate HDIO_GETGEO in guessing for target-s390x

This patch extends the function hd_geometry_guess. It introduces a
target specific hook. The default implementation for this target
specific hook is empty, has therefore no effect and the existing logic
works as before.

For target-s390x, the behaviour is chosen as follows:
If no geo could be guessed via guess_disk_lchs, a new function called
guess_disk_pchs is called. The latter utilizes HDIO_GET_GEO ioctl to ask
the underlying disk for geometry.
If this is not successful (e.g. image files) geometry is derived from the
size of the disk (as before).

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.

Based on 2013 patch from Einar Lueck 

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: David Hildenbrand 
Acked-by: Cornelia Huck 
---
 hw/block/Makefile.objs |  6 +-
 hw/block/hd-geometry.c | 56 ++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index bf46f03..e4f6205 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += block.o cdrom.o hd-geometry.o
+common-obj-y += block.o cdrom.o
 common-obj-$(CONFIG_FDC) += fdc.o
 common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
 common-obj-$(CONFIG_NAND) += nand.o
@@ -13,3 +13,7 @@ obj-$(CONFIG_SH4) += tc58128.o
 
 obj-$(CONFIG_VIRTIO) += virtio-blk.o
 obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += dataplane/
+
+# geometry is target/architecture dependent and therefore needs to be built
+# for every target platform
+obj-y += hd-geometry.o
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6feb4f8..7988264 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -33,6 +33,10 @@
 #include "block/block.h"
 #include "hw/block/block.h"
 #include "trace.h"
+#ifdef __linux__
+#include 
+#include 
+#endif
 
 struct partition {
 uint8_t boot_ind;   /* 0x80 - active */
@@ -47,6 +51,37 @@ 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);
+
+/* 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_pchs(BlockDriverState *bs, uint32_t *pcylinders,
+  uint32_t *pheads, uint32_t *psectors)
+{
+#ifdef __linux__
+struct hd_geometry geo;
+
+if (bdrv_ioctl(bs, HDIO_GETGEO, &geo)) {
+return -1;
+}
+
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!geo.heads || !geo.sectors || !geo.cylinders) {
+return -1;
+}
+
+*pheads = geo.heads;
+*psectors = geo.sectors;
+*pcylinders = geo.cylinders;
+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,
@@ -116,6 +151,26 @@ static void guess_chs_for_size(BlockDriverState *bs,
 *psecs = 63;
 }
 
+#ifdef TARGET_S390X
+void hd_geometry_guess(BlockDriverState *bs, uint32_t *pcyls, uint32_t *pheads,
+   uint32_t *psecs, int *ptrans)
+{
+if (guess_disk_lchs(bs, (int *)pcyls, (int *)pheads, (int *)psecs) == 0) {
+goto done;
+}
+if (guess_disk_pchs(bs, pcyls, pheads, psecs) == 0) {
+goto done;
+}
+/* still no geometry - try to guess from disk size */
+guess_chs_for_size(bs, pcyls, pheads, psecs);
+done:
+if (ptrans) {
+*ptrans = BIOS_ATA_TRANSLATION_NONE;
+}
+trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs,
+BIOS_ATA_TRANSLATION_NONE);
+}
+#else
 void hd_geometry_guess(BlockDriverState *bs,
uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans)
@@ -148,6 +203,7 @@ void hd_geometry_guess(BlockDriverState *bs,
 }
 trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
 }
+#endif
 
 int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs)
 {
-- 
1.8.5.5

[Qemu-devel] [PATCH 2/4] blocksize: support auto-sensing of blocksizes

The block device model 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.

To achieve this, a new function blkconf_blocksizes is
implemented. This function tries to get values
from the block driver. If this does not work 512 is used,
so the default excecution logic is not changed.

Based on 2013 patch from Einar Lueck 

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: David Hildenbrand 
Acked-by: Cornelia Huck 
---
 block.c   | 12 
 hw/block/block.c  | 25 +
 hw/core/qdev-properties.c |  4 +++-
 include/block/block.h |  1 +
 include/block/block_int.h |  5 +
 include/hw/block/block.h  |  2 ++
 6 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 8cf519b..67de6e9 100644
--- a/block.c
+++ b/block.c
@@ -552,6 +552,18 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+int bdrv_probe_blocksizes(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+
+assert(drv != NULL);
+if (drv->bdrv_probe_blocksizes) {
+return drv->bdrv_probe_blocksizes(bs);
+}
+
+return -1;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
diff --git a/hw/block/block.c b/hw/block/block.c
index 33dd3f3..29a0227 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -10,6 +10,10 @@
 #include "sysemu/blockdev.h"
 #include "hw/block/block.h"
 #include "qemu/error-report.h"
+#include "block/block_int.h"
+#ifdef __linux__
+#include 
+#endif
 
 void blkconf_serial(BlockConf *conf, char **serial)
 {
@@ -22,6 +26,27 @@ void blkconf_serial(BlockConf *conf, char **serial)
 }
 }
 
+void blkconf_blocksizes(BlockConf *conf)
+{
+BlockDriverState *bs = conf->bs;
+
+/* default values as a basis - if probing fails */
+conf->physical_block_size = BLOCK_PROPERTY_STD_BLKSIZE;
+conf->logical_block_size = BLOCK_PROPERTY_STD_BLKSIZE;
+if (bdrv_probe_blocksizes(conf->bs) < 0) {
+return;
+}
+if (bs->logical_block_size) {
+conf->logical_block_size = (uint16_t)(bs->logical_block_size);
+}
+if (bs->physical_block_size) {
+conf->physical_block_size = (uint16_t)(bs->physical_block_size);
+} else if (conf->logical_block_size) {
+/* if driver sets no physical size, try to use logical size */
+conf->physical_block_size = conf->logical_block_size;
+}
+}
+
 int blkconf_geometry(BlockConf *conf, int *ptrans,
  unsigned cyls_max, unsigned heads_max, unsigned secs_max)
 {
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3d12560..49fb1e3 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -579,7 +579,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/include/block/block.h b/include/block/block.h
index f08471d..43af424 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -583,6 +583,7 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
  * the old #AioContext is not executing.
  */
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+int bdrv_probe_blocksizes(BlockDriverState *bs);
 
 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7b541a0..572e954 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -266,6 +266,8 @@ struct BlockDriver {
 void (*bdrv_io_unplug)(BlockDriverState *bs);
 void (*bdrv_flush_io_queue)(BlockDriverState *bs);
 
+int (*bdrv_probe_blocksizes)(BlockDriverState *bs);
+
 QLIST_ENTRY(BlockDriver) list;
 };
 
@@ -360,6 +362,9 @@ struct BlockDriverState {
 /* the block size for which the guest device expects atomicity */
 int guest_block_size;
 
+unsigned int physical_block_size;
+unsigned int logical_block_size;
+
 /* do we need to tell the quest if we have a volatile write cache? */
 int enable_write_cache;
 
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 7c3d6c8..7a0092e 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -40,6 +40,7 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
 return exp;
 }
 
+#define BLOCK_PROPERTY_STD_BLKSIZE

[Qemu-devel] [PATCH 0/4] Geometry and blocksize support for backing devices

This patch set is based on a patch suggested by Einar Lueck
on Feb 08, 2013.

This patch set introduces:
1. s390x specific geometry detection:
Add s390 specific version of hd_geometry_guess function,
which uses HDIO_GETGEO ioctl.

2. A set of blocksize patches for autodetection of logical and
physical blocksizes. Change history:
2.1
Original blocksize patch only configured autolookup for
virtio-blk devices. There was a request from Stefan Hajnoczi
to make this architecture-independent. Now autolookup is
configured by default for all block devices.
2.2
Add driver method to probe blocksizes for "raw" and
"host_device" drivers.
(also requested by the reviewers of the original patch)

Ekaterina Tumanova (4):
  hd-geometry.c: Integrate HDIO_GETGEO in guessing for target-s390x
  blocksize: support auto-sensing of blocksizes
  blocksize: Add driver method to get the blocksizes
  blocksize: add blkconf_blocksize call to all block devices

 block.c   | 12 +
 block/raw-posix.c | 69 ++-
 block/raw_bsd.c   | 14 ++
 hw/block/Makefile.objs|  6 -
 hw/block/block.c  | 25 +
 hw/block/hd-geometry.c| 56 ++
 hw/block/nvme.c   |  1 +
 hw/block/virtio-blk.c |  1 +
 hw/core/qdev-properties.c |  4 ++-
 hw/ide/qdev.c |  1 +
 hw/scsi/scsi-disk.c   |  1 +
 hw/usb/dev-storage.c  |  1 +
 include/block/block.h |  1 +
 include/block/block_int.h |  5 
 include/hw/block/block.h  |  6 +++--
 15 files changed, 180 insertions(+), 23 deletions(-)

-- 
1.8.5.5

[Qemu-devel] [PATCH 3/4] blocksize: Add driver method to get the blocksizes

This patch introduces a new method of defining the physical
and logical blocksizes for "raw" and "host_device" drivers.
It uses various ioctls to determine the logical and physical
blocksizes.

For detecting the logical size it uses ioctl calls, that
were previously coded in raw_probe_alignment (now moved into
separate function probe_logical_blocksize). For detecting the
physical blocksize it uses BLKPBSZGET ioctl.

For "raw" devices driver calls the child's method.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: David Hildenbrand 
Acked-by: Cornelia Huck 
---
 block/raw-posix.c | 69 ---
 block/raw_bsd.c   | 14 +++
 2 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 8e9758e..6e0d80c 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -221,50 +221,70 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
-static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+static unsigned int probe_logical_blocksize(BlockDriverState *bs, int fd)
 {
-BDRVRawState *s = bs->opaque;
-char *buf;
-unsigned int sector_size;
-
-/* For /dev/sg devices the alignment is not really used.
-   With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !(s->open_flags & O_DIRECT)) {
-bs->request_alignment = 1;
-s->buf_align = 1;
-return;
-}
+unsigned int sector_size = 0;
 
 /* Try a few ioctls to get the right size */
-bs->request_alignment = 0;
-s->buf_align = 0;
-
 #ifdef BLKSSZGET
 if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef DKIOCGETBLOCKSIZE
 if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef DIOCGSECTORSIZE
 if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+return sector_size;
 }
 #endif
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
 if (xfsctl(NULL, fd, XFS_IOC_DIOINFO, &da) >= 0) {
-bs->request_alignment = da.d_miniosz;
+sector_size = da.d_miniosz;
 /* The kernel returns wrong information for d_mem */
 /* s->buf_align = da.d_mem; */
+return sector_size;
 }
 }
 #endif
 
+return 0;
+}
+
+static unsigned int probe_physical_blocksize(BlockDriverState *bs, int fd)
+{
+unsigned int blk_size = 0;
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, &blk_size) >= 0) {
+return blk_size;
+}
+#endif
+
+return 0;
+}
+
+static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !(s->open_flags & O_DIRECT)) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+bs->request_alignment = probe_logical_blocksize(bs, fd);
+
 /* If we could not get the sizes so far, we can only guess them */
 if (!s->buf_align) {
 size_t align;
@@ -642,6 +662,16 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 bs->bl.opt_mem_alignment = s->buf_align;
 }
 
+static int hdev_probe_blocksizes(BlockDriverState *bs)
+{
+BDRVRawState *s = bs->opaque;
+
+bs->logical_block_size = probe_logical_blocksize(bs, s->fd);
+bs->physical_block_size = probe_physical_blocksize(bs, s->fd);
+
+return 0;
+}
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2009,6 +2039,7 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
 
 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index f82f4c2..ae400a6 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -173,6 +173,19 @@ static int raw_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 1;
 }
 
+static int raw_probe_blocksizes(BlockDriverState *bs)
+{
+int ret;
+
+ret = bdrv_probe_blocksizes(bs->file);
+if (ret == 0) {
+bs->logical_block_size = bs->file->logical_block_size;
+bs->physical_block_size = bs->file->physical_block_size;
+}
+
+return ret;
+}
+
 static B

[Qemu-devel] [PATCH v3 1/1] Define the architecture for compressed dump format.

Signed-off-by: Ekaterina Tumanova 
---
 dump.c | 7 +--
 target-i386/cpu.h  | 2 ++
 target-s390x/cpu.h | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/dump.c b/dump.c
index 8f64aab..8d85255 100644
--- a/dump.c
+++ b/dump.c
@@ -32,6 +32,9 @@
 #ifdef CONFIG_SNAPPY
 #include 
 #endif
+#ifndef ELF_MACHINE_UNAME
+#define ELF_MACHINE_UNAME "Unknown"
+#endif
 
 static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
 {
@@ -817,7 +820,7 @@ static int create_header32(DumpState *s)
 dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
 bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, block_size) * 2;
 dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
-memcpy(&(dh->utsname.machine), "i686", 4);
+strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, 
sizeof(dh->utsname.machine));
 
 if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
 status |= DUMP_DH_COMPRESSED_ZLIB;
@@ -924,7 +927,7 @@ static int create_header64(DumpState *s)
 dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
 bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, block_size) * 2;
 dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
-memcpy(&(dh->utsname.machine), "x86_64", 6);
+strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, 
sizeof(dh->utsname.machine));
 
 if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
 status |= DUMP_DH_COMPRESSED_ZLIB;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1fcbc82..198743c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -38,8 +38,10 @@
 
 #ifdef TARGET_X86_64
 #define ELF_MACHINE EM_X86_64
+#define ELF_MACHINE_UNAME "x86_64"
 #else
 #define ELF_MACHINE EM_386
+#define ELF_MACHINE_UNAME "i686"
 #endif
 
 #define CPUArchState struct CPUX86State
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 68b5ab7..bf7ae4c 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -28,6 +28,7 @@
 #define TARGET_LONG_BITS 64
 
 #define ELF_MACHINEEM_S390
+#define ELF_MACHINE_UNAME "S390X"
 
 #define CPUArchState struct CPUS390XState
 
-- 
1.8.4.5

[Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.

True! Fixed. Tested.

Can you please put it into push-queue?

Thanks,
Kate.

Ekaterina Tumanova (1):
  Define the architecture for compressed dump format.

 dump.c | 7 +--
 target-i386/cpu.h  | 2 ++
 target-s390x/cpu.h | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

-- 
1.8.4.5

[Qemu-devel] [PATCH v2] Define guest architecture for the compressed dump header.

You're right about the warning. I removed it. I also added the
cast for the destination argument, which caused a warning as well.

I tested on s390 and i386 laptop.

If you ok with the patch, can you push it on top of this series?

Thank you,

Regards,
Kate.

Ekaterina Tumanova (1):
  Define the architecture for compressed dump format.

 dump.c | 7 +--
 target-i386/cpu.h  | 2 ++
 target-s390x/cpu.h | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

-- 
1.8.4.5

[Qemu-devel] [PATCH v2] Define the architecture for compressed dump format.