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

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

[Ekaterina Tumanova]

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_xxxxxx 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

[Ekaterina Tumanova]

Add driver functions for geometry and blocksize detection

Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 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_RDWR        0x0002
 #define BDRV_O_SNAPSHOT    0x0008 /* 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 2/5] raw-posix: Factor block size detection out of raw_probe_alignment()

[Ekaterina Tumanova]

Put it in new probe_logical_blocksize().

Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 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, &sector_size) >= 0) {
-        bs->request_alignment = sector_size;
+        *sector_size_p = sector_size;
+        success = true;
     }
 #endif
 #ifdef DKIOCGETBLOCKSIZE
     if (ioctl(fd, DKIOCGETBLOCKSIZE, &sector_size) >= 0) {
-        bs->request_alignment = sector_size;
+        *sector_size_p = sector_size;
+        success = true;
     }
 #endif
 #ifdef DIOCGSECTORSIZE
     if (ioctl(fd, DIOCGSECTORSIZE, &sector_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 v7 3/5] block: Add driver methods to probe blocksizes and geometry

[Ekaterina Tumanova]

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 <tumanova@linux.vnet.ibm.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 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 <linux/cdrom.h>
 #include <linux/fd.h>
 #include <linux/fs.h>
+#include <linux/hdreg.h>
+#ifdef __s390__
+#include <asm/dasd.h>
+#endif
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL                     0x00800000 /* 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, bsz);
+}
+
+static int raw_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+    return bdrv_probe_geometry(bs->file, geo);
+}
+
 BlockDriver bdrv_raw = {
     .format_name          = "raw",
     .bdrv_probe           = &raw_probe,
@@ -252,6 +262,8 @@ 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,
-- 
2.1.4

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

[Ekaterina Tumanova]

Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 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 5/5] BlockConf: Call backend functions to detect geometry and blocksizes

[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 <tumanova@linux.vnet.ibm.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 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(IDEDevice *dev, IDEDriveKind kind)
         return -1;
     }
 
+    blkconf_blocksizes(&dev->conf);
     if (dev->conf.logical_block_size != 512) {
         error_report("logical_block_size must be 512 for IDE");
         return -1;
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index f65618d..2921728 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2251,6 +2251,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
     }
 
     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, &err);
         if (err) {
@@ -2290,6 +2291,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+    blkconf_blocksizes(&s->qdev.conf);
     s->qdev.blocksize = s->qdev.conf.logical_block_size;
     s->qdev.type = TYPE_DISK;
     if (!s->product) {
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 4539733..cc02dfd 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -611,6 +611,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
     }
 
     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 0d0ce9a..8d7c4b4 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.blk),                      \
     DEFINE_PROP_BLOCKSIZE("logical_block_size", _state,                 \
-                          _conf.logical_block_size, 512),               \
+                          _conf.logical_block_size),                    \
     DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
-                          _conf.physical_block_size, 512),              \
+                          _conf.physical_block_size),                   \
     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_UINT32("discard_granularity", _state, \
@@ -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 */
 
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 070006c..2fdcbce 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -149,8 +149,8 @@ extern PropertyInfo qdev_prop_arraylen;
                         LostTickPolicy)
 #define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
-#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
+#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t)
 #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
 
-- 
2.1.4

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

[Christian Borntraeger]

Am 16.02.2015 um 12:47 schrieb Ekaterina Tumanova:
> 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.


v7 retested
- make check on x86_64/s390x 
- dasd geo/blocksize/partition detection

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

[Stefan Hajnoczi]

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

On Mon, Feb 16, 2015 at 12:47:53PM +0100, Ekaterina Tumanova wrote:
> 
> 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.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

[Max Reitz]

On 2015-02-16 at 06:47, Ekaterina Tumanova wrote:
> 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 <tumanova@linux.vnet.ibm.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   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(-)

This patch makes qemu segfault if the drive property is not set for a 
scsi-hd device:

$ x86_64-softmmu/qemu-system-x86_64 -device virtio-scsi-pci -device scsi-hd
[1]    13368 segmentation fault (core dumped) 
x86_64-softmmu/qemu-system-x86_64 -device virtio-scsi-pci -device scsi-hd

(gdb) bt
#0  0x00007f0a77620f50 in blk_probe_blocksizes (blk=0x0, 
bsz=0x7fffd989f110) at block/block-backend.c:898
#1  0x00007f0a774eb943 in blkconf_blocksizes 
(conf=conf@entry=0x7f0a7b1e71c0) at hw/block/block.c:34
#2  0x00007f0a7755e5a8 in scsi_hd_realize (dev=0x7f0a7b1e7130, 
errp=0x7fffd989f150) at hw/scsi/scsi-disk.c:2294
#3  0x00007f0a77564671 in scsi_qdev_realize (errp=0x7fffd989f150, 
s=0x7f0a7b1e7130) at hw/scsi/scsi-bus.c:50
#4  0x00007f0a77564671 in scsi_qdev_realize (qdev=<optimized out>, 
errp=0x7fffd989f190) at hw/scsi/scsi-bus.c:197
#5  0x00007f0a77502b71 in device_set_realized (obj=0x7f0a7b1e7130, 
value=<optimized out>, errp=0x7fffd989f2c8) at hw/core/qdev.c:1047
#6  0x00007f0a775b1e0e in property_set_bool (obj=0x7f0a7b1e7130, 
v=<optimized out>, opaque=0x7f0a7b1e73e0, name=<optimized out>, 
errp=0x7fffd989f2c8) at qom/object.c:1514
#7  0x00007f0a775b4707 in object_property_set_qobject 
(obj=0x7f0a7b1e7130, value=<optimized out>, name=0x7f0a7769e3cd 
"realized", errp=0x7fffd989f2c8) at qom/qom-qobject.c:24
#8  0x00007f0a775b32a0 in object_property_set_bool 
(obj=obj@entry=0x7f0a7b1e7130, value=value@entry=true, 
name=name@entry=0x7f0a7769e3cd "realized", errp=errp@entry=0x7fffd989f2c8)
     at qom/object.c:905
#9  0x00007f0a774a8ca5 in qdev_device_add (opts=0x7f0a799fa3f0) at 
qdev-monitor.c:574
#10 0x00007f0a774b1df9 in device_init_func (opts=<optimized out>, 
opaque=<optimized out>) at qemu/vl.c:2127
#11 0x00007f0a7766803b in qemu_opts_foreach (list=<optimized out>, 
func=0x7f0a774b1df0 <device_init_func>, opaque=0x0, 
abort_on_failure=<optimized out>) at util/qemu-option.c:1057
#12 0x00007f0a773b96ec in main (argc=<optimized out>, argv=<optimized 
out>, envp=<optimized out>) at vl.c:4239

Before this patch:

$ x86_64-softmmu/qemu-system-x86_64 -device virtio-scsi-pci -device scsi-hd
qemu-system-x86_64: -device scsi-hd: drive property not set
qemu-system-x86_64: -device scsi-hd: Device 'scsi-hd' could not be 
initialized

Max

[Qemu-devel] [PATCH 0/1]

[Ekaterina Tumanova]

for Max Reitz:

Can you please apply this patch and re-test?

Thanks!
Kate

p.s. This is supposed to be merged with patch 5/5 of
"Geometry and blocksize detection for backing devices"

Ekaterina Tumanova (1):
  scsi-hd: fix property unset case

 hw/scsi/scsi-disk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.1.4

[Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Ekaterina Tumanova]

check conf.blk before calling blkconf_blocksizes

Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
---
 hw/scsi/scsi-disk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 2921728..df5140e 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-    blkconf_blocksizes(&s->qdev.conf);
+    if (s->qdev.conf.blk) {
+        blkconf_blocksizes(&s->qdev.conf);
+    }
     s->qdev.blocksize = s->qdev.conf.logical_block_size;
     s->qdev.type = TYPE_DISK;
     if (!s->product) {
-- 
2.1.4

Re: [Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Max Reitz]

On 2015-02-27 at 13:26, Ekaterina Tumanova wrote:
> check conf.blk before calling blkconf_blocksizes
>
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> ---
>   hw/scsi/scsi-disk.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Stefan Hajnoczi]

On Fri, Feb 27, 2015 at 6:29 PM, Max Reitz <mreitz@redhat.com> wrote:
> On 2015-02-27 at 13:26, Ekaterina Tumanova wrote:
>>
>> check conf.blk before calling blkconf_blocksizes
>>
>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>> ---
>>   hw/scsi/scsi-disk.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Markus Armbruster]

Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes:

> check conf.blk before calling blkconf_blocksizes
>
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> ---
>  hw/scsi/scsi-disk.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 2921728..df5140e 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
>  static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
>  {
>      SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> -    blkconf_blocksizes(&s->qdev.conf);
> +    if (s->qdev.conf.blk) {
> +        blkconf_blocksizes(&s->qdev.conf);
> +    }

Looks suspicious on first glance, because block device model realize()
methods are supposed to fail when the backend is missing.  But...

>      s->qdev.blocksize = s->qdev.conf.logical_block_size;
>      s->qdev.type = TYPE_DISK;
>      if (!s->product) {
           s->product = g_strdup("QEMU HARDDISK");
       }
       scsi_realize(&s->qdev, errp);

... scsi_realize() errors out then.  Worth a comment.  Or maybe call
blkconf_blocksizes() only after scsi_realize().  Your choice.

Re: [Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Ekaterina Tumanova]

On 03/02/2015 11:46 AM, Markus Armbruster wrote:
> Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes:
>
>> check conf.blk before calling blkconf_blocksizes
>>
>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>> ---
>>   hw/scsi/scsi-disk.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>> index 2921728..df5140e 100644
>> --- a/hw/scsi/scsi-disk.c
>> +++ b/hw/scsi/scsi-disk.c
>> @@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
>>   static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
>>   {
>>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
>> -    blkconf_blocksizes(&s->qdev.conf);
>> +    if (s->qdev.conf.blk) {
>> +        blkconf_blocksizes(&s->qdev.conf);
>> +    }
>
> Looks suspicious on first glance, because block device model realize()
> methods are supposed to fail when the backend is missing.  But...
>

it will properly fail in scsi_realize

>>       s->qdev.blocksize = s->qdev.conf.logical_block_size;
>>       s->qdev.type = TYPE_DISK;
>>       if (!s->product) {
>             s->product = g_strdup("QEMU HARDDISK");
>         }
>         scsi_realize(&s->qdev, errp);
>
> ... scsi_realize() errors out then.  Worth a comment.  Or maybe call
> blkconf_blocksizes() only after scsi_realize().  Your choice.

can't call it later. conf.logical_block_size, which blkconf_blocksizes
sets it used earlier.

Re: [Qemu-devel] [PATCH 0/1]

[Kevin Wolf]

Am 27.02.2015 um 19:26 hat Ekaterina Tumanova geschrieben:
> for Max Reitz:
> 
> Can you please apply this patch and re-test?
> 
> Thanks!
> Kate
> 
> p.s. This is supposed to be merged with patch 5/5 of
> "Geometry and blocksize detection for backing devices"
> 
> Ekaterina Tumanova (1):
>   scsi-hd: fix property unset case

Can someone please add a qemu-iotests case somewhere to check the
failing command line?

Kevin

Re: [Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Markus Armbruster]

Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes:

> On 03/02/2015 11:46 AM, Markus Armbruster wrote:
>> Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes:
>>
>>> check conf.blk before calling blkconf_blocksizes
>>>
>>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>> ---
>>>   hw/scsi/scsi-disk.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>>> index 2921728..df5140e 100644
>>> --- a/hw/scsi/scsi-disk.c
>>> +++ b/hw/scsi/scsi-disk.c
>>> @@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
>>>   static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
>>>   {
>>>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
>>> -    blkconf_blocksizes(&s->qdev.conf);
>>> +    if (s->qdev.conf.blk) {
>>> +        blkconf_blocksizes(&s->qdev.conf);
>>> +    }
>>
>> Looks suspicious on first glance, because block device model realize()
>> methods are supposed to fail when the backend is missing.  But...
>>
>
> it will properly fail in scsi_realize
>
>>>       s->qdev.blocksize = s->qdev.conf.logical_block_size;
>>>       s->qdev.type = TYPE_DISK;
>>>       if (!s->product) {
>>             s->product = g_strdup("QEMU HARDDISK");
>>         }
>>         scsi_realize(&s->qdev, errp);
>>
>> ... scsi_realize() errors out then.  Worth a comment.  Or maybe call
>> blkconf_blocksizes() only after scsi_realize().  Your choice.
>
> can't call it later. conf.logical_block_size, which blkconf_blocksizes
> sets it used earlier.

Okay, I recommend a comment then.

[Qemu-devel] [PATCH] Add testcase for scsi-hd devices without drive property

[Christian Borntraeger]

CC: Max Reitz <mreitz@redhat.com> 
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 tests/qemu-iotests/051     |    9 +++++++++
 tests/qemu-iotests/051.out |    8 ++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 11c858f..1eec350 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -95,6 +95,15 @@ run_qemu -drive file="$TEST_IMG",driver=foo
 run_qemu -drive file="$TEST_IMG",driver=raw,format=qcow2
 
 echo
+echo === Device without drive ===
+echo
+
+# virtio-scsi-pci, virtio-scsi-ccw, virtio-scsi-s390...
+VIRTSCSIDEV=`${QEMU} -device help 2>&1 | grep -v virtio-scsi-device | grep -m 1 virtio-scsi | cut -d \" -f 2`
+
+run_qemu -device ${VIRTSCSIDEV} -device scsi-hd 
+
+echo
 echo === Overriding backing file ===
 echo
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index f497c57..e8e3258 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -44,6 +44,14 @@ Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2
 QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: could not open disk image TEST_DIR/t.qcow2: Driver specified twice
 
 
+=== Device without drive ===
+
+Testing: -device virtio-scsi-pci -device scsi-hd
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) QEMU_PROG: -device scsi-hd: drive property not set
+QEMU_PROG: -device scsi-hd: Device 'scsi-hd' could not be initialized
+
+
 === Overriding backing file ===
 
 Testing: -drive file=TEST_DIR/t.qcow2,driver=qcow2,backing.file.filename=TEST_DIR/t.qcow2.orig -nodefaults
-- 
1.7.1

Re: [Qemu-devel] [PATCH] Add testcase for scsi-hd devices without drive property

[Christian Borntraeger]

Am 03.03.2015 um 09:15 schrieb Christian Borntraeger:
> CC: Max Reitz <mreitz@redhat.com> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  tests/qemu-iotests/051     |    9 +++++++++
>  tests/qemu-iotests/051.out |    8 ++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 11c858f..1eec350 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -95,6 +95,15 @@ run_qemu -drive file="$TEST_IMG",driver=foo
>  run_qemu -drive file="$TEST_IMG",driver=raw,format=qcow2
> 
>  echo
> +echo === Device without drive ===
> +echo
> +
> +# virtio-scsi-pci, virtio-scsi-ccw, virtio-scsi-s390...
> +VIRTSCSIDEV=`${QEMU} -device help 2>&1 | grep -v virtio-scsi-device | grep -m 1 virtio-scsi | cut -d \" -f 2`
> +

Hmmm. Doesnt work reliably on s390.


> +run_qemu -device ${VIRTSCSIDEV} -device scsi-hd 

Maybe we can just use virtio-scsi-pci hardcoded and fixup s390 later on as
there are other io tests in here that need some special s390 changes as well.
(We are working on it)

So something like

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 11c858f..6ab40e2 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -95,6 +95,12 @@ run_qemu -drive file="$TEST_IMG",driver=foo
 run_qemu -drive file="$TEST_IMG",driver=raw,format=qcow2
 
 echo
+echo === Device without drive ===
+echo
+
+run_qemu -device virtio-scsi-pci -device scsi-hd
+
+echo
 echo === Overriding backing file ===
 echo


plus the test  output. Makes sense?

Christian

Re: [Qemu-devel] [PATCH] Add testcase for scsi-hd devices without drive property

[Max Reitz]

On 2015-03-03 at 09:17, Christian Borntraeger wrote:
> Am 03.03.2015 um 09:15 schrieb Christian Borntraeger:
>> CC: Max Reitz <mreitz@redhat.com>
>> Suggested-by: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>   tests/qemu-iotests/051     |    9 +++++++++
>>   tests/qemu-iotests/051.out |    8 ++++++++
>>   2 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
>> index 11c858f..1eec350 100755
>> --- a/tests/qemu-iotests/051
>> +++ b/tests/qemu-iotests/051
>> @@ -95,6 +95,15 @@ run_qemu -drive file="$TEST_IMG",driver=foo
>>   run_qemu -drive file="$TEST_IMG",driver=raw,format=qcow2
>>
>>   echo
>> +echo === Device without drive ===
>> +echo
>> +
>> +# virtio-scsi-pci, virtio-scsi-ccw, virtio-scsi-s390...
>> +VIRTSCSIDEV=`${QEMU} -device help 2>&1 | grep -v virtio-scsi-device | grep -m 1 virtio-scsi | cut -d \" -f 2`
>> +
> Hmmm. Doesnt work reliably on s390.
>
>
>> +run_qemu -device ${VIRTSCSIDEV} -device scsi-hd
> Maybe we can just use virtio-scsi-pci hardcoded and fixup s390 later on as
> there are other io tests in here that need some special s390 changes as well.
> (We are working on it)
>
> So something like
>
> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 11c858f..6ab40e2 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -95,6 +95,12 @@ run_qemu -drive file="$TEST_IMG",driver=foo
>   run_qemu -drive file="$TEST_IMG",driver=raw,format=qcow2
>   
>   echo
> +echo === Device without drive ===
> +echo
> +
> +run_qemu -device virtio-scsi-pci -device scsi-hd
> +
> +echo
>   echo === Overriding backing file ===
>   echo
>
>
> plus the test  output. Makes sense?

Fine with me.

Max