All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h
@ 2012-07-11 13:08 Markus Armbruster
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 1/4] hw/block-common: Move BlockConf & friends from block.h Markus Armbruster
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Markus Armbruster @ 2012-07-11 13:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Applies on top of "[PATCH v3 00/29] Disk geometry cleanup".

Markus Armbruster (4):
  hw/block-common: Move BlockConf & friends from block.h
  hw/block-common: Factor out fall back to legacy -drive serial=...
  blockdev: Don't limit DriveInfo serial to 20 characters
  hw/block-common: Factor out fall back to legacy -drive cyls=...

 block.h              |   45 -----------------------------------
 blockdev.c           |    4 +--
 blockdev.h           |    4 +--
 hw/Makefile.objs     |    2 +-
 hw/block-common.c    |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/block-common.h    |   50 +++++++++++++++++++++++++++++++++++++++
 hw/ide/core.c        |   30 +++++++++++-----------
 hw/ide/internal.h    |    1 +
 hw/ide/qdev.c        |   31 +++---------------------
 hw/scsi-disk.c       |   37 ++--------------------------
 hw/scsi.h            |    1 +
 hw/usb.h             |    1 -
 hw/usb/dev-storage.c |   10 +------
 hw/virtio-blk.c      |   37 ++--------------------------
 hw/virtio-blk.h      |    2 +-
 hw/virtio.h          |    1 -
 16 files changed, 147 insertions(+), 173 deletions(-)
 create mode 100644 hw/block-common.c

-- 
1.7.6.5

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

* [Qemu-devel] [PATCH 1/4] hw/block-common: Move BlockConf & friends from block.h
  2012-07-11 13:08 [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Markus Armbruster
@ 2012-07-11 13:08 ` Markus Armbruster
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 2/4] hw/block-common: Factor out fall back to legacy -drive serial= Markus Armbruster
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2012-07-11 13:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

This stuff doesn't belong to block layer, and was put there only
because a better home didn't exist then.  Now it does.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.h           |   45 ---------------------------------------------
 hw/block-common.h |   45 +++++++++++++++++++++++++++++++++++++++++++++
 hw/ide/internal.h |    1 +
 hw/scsi.h         |    1 +
 hw/usb.h          |    1 -
 hw/virtio-blk.h   |    2 +-
 hw/virtio.h       |    1 -
 7 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/block.h b/block.h
index 29c5eab..c89590d 100644
--- a/block.h
+++ b/block.h
@@ -403,49 +403,4 @@ typedef enum {
 #define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
 void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
 
-
-/* Convenience for block device models */
-
-typedef struct BlockConf {
-    BlockDriverState *bs;
-    uint16_t physical_block_size;
-    uint16_t logical_block_size;
-    uint16_t min_io_size;
-    uint32_t opt_io_size;
-    int32_t bootindex;
-    uint32_t discard_granularity;
-    /* geometry, not all devices use this */
-    uint32_t cyls, heads, secs;
-} BlockConf;
-
-static inline unsigned int get_physical_block_exp(BlockConf *conf)
-{
-    unsigned int exp = 0, size;
-
-    for (size = conf->physical_block_size;
-        size > conf->logical_block_size;
-        size >>= 1) {
-        exp++;
-    }
-
-    return exp;
-}
-
-#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),               \
-    DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
-                          _conf.physical_block_size, 512),              \
-    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
-    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
-    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \
-    DEFINE_PROP_UINT32("discard_granularity", _state, \
-                       _conf.discard_granularity, 0)
-
-#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf)      \
-    DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0),  \
-    DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
-    DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
-
 #endif
diff --git a/hw/block-common.h b/hw/block-common.h
index 31e12ba..f0d509b 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -13,6 +13,51 @@
 
 #include "qemu-common.h"
 
+/* Configuration */
+
+typedef struct BlockConf {
+    BlockDriverState *bs;
+    uint16_t physical_block_size;
+    uint16_t logical_block_size;
+    uint16_t min_io_size;
+    uint32_t opt_io_size;
+    int32_t bootindex;
+    uint32_t discard_granularity;
+    /* geometry, not all devices use this */
+    uint32_t cyls, heads, secs;
+} BlockConf;
+
+static inline unsigned int get_physical_block_exp(BlockConf *conf)
+{
+    unsigned int exp = 0, size;
+
+    for (size = conf->physical_block_size;
+        size > conf->logical_block_size;
+        size >>= 1) {
+        exp++;
+    }
+
+    return exp;
+}
+
+#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),               \
+    DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
+                          _conf.physical_block_size, 512),              \
+    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
+    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
+    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \
+    DEFINE_PROP_UINT32("discard_granularity", _state, \
+                       _conf.discard_granularity, 0)
+
+#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf)      \
+    DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0),  \
+    DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
+    DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+
+
 /* Hard disk geometry */
 
 #define BIOS_ATA_TRANSLATION_AUTO   0
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c3ecafc..7170bd9 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -11,6 +11,7 @@
 #include "iorange.h"
 #include "dma.h"
 #include "sysemu.h"
+#include "hw/block-common.h"
 #include "hw/scsi-defs.h"
 
 /* debug IDE devices */
diff --git a/hw/scsi.h b/hw/scsi.h
index 76f06d4..d90e970 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -3,6 +3,7 @@
 
 #include "qdev.h"
 #include "block.h"
+#include "hw/block-common.h"
 #include "sysemu.h"
 
 #define MAX_SCSI_DEVS	255
diff --git a/hw/usb.h b/hw/usb.h
index 7ed8fb8..432ccae 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -25,7 +25,6 @@
  * THE SOFTWARE.
  */
 
-#include "block.h"
 #include "qdev.h"
 #include "qemu-queue.h"
 
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index d785001..79ebccc 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -15,7 +15,7 @@
 #define _QEMU_VIRTIO_BLK_H
 
 #include "virtio.h"
-#include "block.h"
+#include "hw/block-common.h"
 
 /* from Linux's linux/virtio_blk.h */
 
diff --git a/hw/virtio.h b/hw/virtio.h
index 85aabe5..42a7762 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -18,7 +18,6 @@
 #include "net.h"
 #include "qdev.h"
 #include "sysemu.h"
-#include "block.h"
 #include "event_notifier.h"
 #ifdef CONFIG_LINUX
 #include "9p.h"
-- 
1.7.6.5

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

* [Qemu-devel] [PATCH 2/4] hw/block-common: Factor out fall back to legacy -drive serial=...
  2012-07-11 13:08 [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Markus Armbruster
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 1/4] hw/block-common: Move BlockConf & friends from block.h Markus Armbruster
@ 2012-07-11 13:08 ` Markus Armbruster
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 3/4] blockdev: Don't limit DriveInfo serial to 20 characters Markus Armbruster
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2012-07-11 13:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/Makefile.objs     |    2 +-
 hw/block-common.c    |   24 ++++++++++++++++++++++++
 hw/block-common.h    |    3 +++
 hw/ide/qdev.c        |   12 ++----------
 hw/scsi-disk.c       |    8 +-------
 hw/usb/dev-storage.c |   10 ++--------
 hw/virtio-blk.c      |    8 +-------
 7 files changed, 34 insertions(+), 33 deletions(-)
 create mode 100644 hw/block-common.c

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index c3bdedc..8327e55 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -138,7 +138,7 @@ common-obj-$(CONFIG_MAX111X) += max111x.o
 common-obj-$(CONFIG_DS1338) += ds1338.o
 common-obj-y += i2c.o smbus.o smbus_eeprom.o
 common-obj-y += eeprom93xx.o
-common-obj-y += scsi-disk.o cdrom.o hd-geometry.o
+common-obj-y += scsi-disk.o cdrom.o hd-geometry.o block-common.o
 common-obj-y += scsi-generic.o scsi-bus.o
 common-obj-y += hid.o
 common-obj-$(CONFIG_SSI) += ssi.o
diff --git a/hw/block-common.c b/hw/block-common.c
new file mode 100644
index 0000000..036334b
--- /dev/null
+++ b/hw/block-common.c
@@ -0,0 +1,24 @@
+/*
+ * Common code for block device models
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "blockdev.h"
+#include "hw/block-common.h"
+
+void blkconf_serial(BlockConf *conf, char **serial)
+{
+    DriveInfo *dinfo;
+
+    if (!*serial) {
+        /* try to fall back to value set with legacy -drive serial=... */
+        dinfo = drive_get_by_blockdev(conf->bs);
+        if (*dinfo->serial) {
+            *serial = g_strdup(dinfo->serial);
+        }
+    }
+}
diff --git a/hw/block-common.h b/hw/block-common.h
index f0d509b..52bddda 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -57,6 +57,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
     DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
     DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
 
+/* Configuration helpers */
+
+void blkconf_serial(BlockConf *conf, char **serial);
 
 /* Hard disk geometry */
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index de9db3b..7fe803c 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -142,7 +142,6 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 {
     IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
     IDEState *s = bus->ifs + dev->unit;
-    const char *serial;
     DriveInfo *dinfo;
 
     if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) {
@@ -150,14 +149,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
         return -1;
     }
 
-    serial = dev->serial;
-    if (!serial) {
-        /* try to fall back to value set with legacy -drive serial=... */
-        dinfo = drive_get_by_blockdev(dev->conf.bs);
-        if (*dinfo->serial) {
-            serial = dinfo->serial;
-        }
-    }
+    blkconf_serial(&dev->conf, &dev->serial);
 
     if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
         /* try to fall back to value set with legacy -drive cyls=... */
@@ -177,7 +169,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     }
 
     if (ide_init_drive(s, dev->conf.bs, kind,
-                       dev->version, serial, dev->model, dev->wwn,
+                       dev->version, dev->serial, dev->model, dev->wwn,
                        dev->conf.cyls, dev->conf.heads, dev->conf.secs,
                        dev->chs_trans) < 0) {
         return -1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 0a182f9..39a07d7 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1777,13 +1777,7 @@ static int scsi_initfn(SCSIDevice *dev)
         }
     }
 
-    if (!s->serial) {
-        /* try to fall back to value set with legacy -drive serial=... */
-        dinfo = drive_get_by_blockdev(s->qdev.conf.bs);
-        if (*dinfo->serial) {
-            s->serial = g_strdup(dinfo->serial);
-        }
-    }
+    blkconf_serial(&s->qdev.conf, &s->serial);
 
     if (!s->version) {
         s->version = g_strdup(qemu_get_version());
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 251e7de..7fa8b83 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -532,13 +532,14 @@ static int usb_msd_initfn(USBDevice *dev)
 {
     MSDState *s = DO_UPCAST(MSDState, dev, dev);
     BlockDriverState *bs = s->conf.bs;
-    DriveInfo *dinfo;
 
     if (!bs) {
         error_report("drive property not set");
         return -1;
     }
 
+    blkconf_serial(&s->conf, &s->serial);
+
     /*
      * Hack alert: this pretends to be a block device, but it's really
      * a SCSI bus that can serve only a single device, which it
@@ -551,13 +552,6 @@ static int usb_msd_initfn(USBDevice *dev)
     bdrv_detach_dev(bs, &s->dev.qdev);
     s->conf.bs = NULL;
 
-    if (!s->serial) {
-        /* try to fall back to value set with legacy -drive serial=... */
-        dinfo = drive_get_by_blockdev(bs);
-        if (*dinfo->serial) {
-            s->serial = strdup(dinfo->serial);
-        }
-    }
     if (s->serial) {
         usb_desc_set_string(dev, STR_SERIALNUMBER, s->serial);
     } else {
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 3885904..ba087bc 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -600,13 +600,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
         return NULL;
     }
 
-    if (!blk->serial) {
-        /* try to fall back to value set with legacy -drive serial=... */
-        dinfo = drive_get_by_blockdev(blk->conf.bs);
-        if (*dinfo->serial) {
-            blk->serial = strdup(dinfo->serial);
-        }
-    }
+    blkconf_serial(&blk->conf, &blk->serial);
 
     s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
                                           sizeof(struct virtio_blk_config),
-- 
1.7.6.5

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

* [Qemu-devel] [PATCH 3/4] blockdev: Don't limit DriveInfo serial to 20 characters
  2012-07-11 13:08 [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Markus Armbruster
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 1/4] hw/block-common: Move BlockConf & friends from block.h Markus Armbruster
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 2/4] hw/block-common: Factor out fall back to legacy -drive serial= Markus Armbruster
@ 2012-07-11 13:08 ` Markus Armbruster
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 4/4] hw/block-common: Factor out fall back to legacy -drive cyls= Markus Armbruster
  2012-07-12  9:02 ` [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Kevin Wolf
  4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2012-07-11 13:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

All current users (IDE, SCSI and virtio-blk) happen to share this 20
characters limit.  Still, it should be left to device models.  They
already enforce their limits.  They have to, as the DriveInfo limit
only affects legacy -drive serial=..., not the qdev properties.

usb-storage, which doesn't limit serial number length, also uses
DriveInfo for -usbdevice.  But that doesn't provide access to
DriveInfo serial.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev.c        |    4 +---
 blockdev.h        |    4 +---
 hw/block-common.c |    2 +-
 hw/ide/core.c     |    6 +++---
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5f8677e..3d75015 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -536,9 +536,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
     dinfo->trans = translation;
     dinfo->opts = opts;
     dinfo->refcount = 1;
-    if (serial) {
-        pstrcpy(dinfo->serial, sizeof(dinfo->serial), serial);
-    }
+    dinfo->serial = serial;
     QTAILQ_INSERT_TAIL(&drives, dinfo, next);
 
     bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error);
diff --git a/blockdev.h b/blockdev.h
index bc8c2dc..33b5772 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -17,8 +17,6 @@
 void blockdev_mark_auto_del(BlockDriverState *bs);
 void blockdev_auto_del(BlockDriverState *bs);
 
-#define BLOCK_SERIAL_STRLEN 20
-
 typedef enum {
     IF_DEFAULT = -1,            /* for use with drive_add() only */
     IF_NONE,
@@ -37,7 +35,7 @@ struct DriveInfo {
     int media_cd;
     int cyls, heads, secs, trans;
     QemuOpts *opts;
-    char serial[BLOCK_SERIAL_STRLEN + 1];
+    const char *serial;
     QTAILQ_ENTRY(DriveInfo) next;
     int refcount;
 };
diff --git a/hw/block-common.c b/hw/block-common.c
index 036334b..0a0542a 100644
--- a/hw/block-common.c
+++ b/hw/block-common.c
@@ -17,7 +17,7 @@ void blkconf_serial(BlockConf *conf, char **serial)
     if (!*serial) {
         /* try to fall back to value set with legacy -drive serial=... */
         dinfo = drive_get_by_blockdev(conf->bs);
-        if (*dinfo->serial) {
+        if (dinfo->serial) {
             *serial = g_strdup(dinfo->serial);
         }
     }
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 58a454f..5378fc3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2095,9 +2095,9 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                 trans = hd_bios_chs_auto_trans(cyls, heads, secs);
             }
             if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
-                               dinfo->media_cd ? IDE_CD : IDE_HD, NULL,
-                               *dinfo->serial ? dinfo->serial : NULL,
-                               NULL, 0, cyls, heads, secs, trans) < 0) {
+                               dinfo->media_cd ? IDE_CD : IDE_HD,
+                               NULL, dinfo->serial, NULL, 0,
+                               cyls, heads, secs, trans) < 0) {
                 error_report("Can't set up IDE drive %s", dinfo->id);
                 exit(1);
             }
-- 
1.7.6.5

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

* [Qemu-devel] [PATCH 4/4] hw/block-common: Factor out fall back to legacy -drive cyls=...
  2012-07-11 13:08 [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Markus Armbruster
                   ` (2 preceding siblings ...)
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 3/4] blockdev: Don't limit DriveInfo serial to 20 characters Markus Armbruster
@ 2012-07-11 13:08 ` Markus Armbruster
  2012-07-12  9:02 ` [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Kevin Wolf
  4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2012-07-11 13:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/block-common.c |   40 ++++++++++++++++++++++++++++++++++++++++
 hw/block-common.h |    2 ++
 hw/ide/core.c     |   24 ++++++++++++------------
 hw/ide/qdev.c     |   19 ++-----------------
 hw/scsi-disk.c    |   31 +++----------------------------
 hw/virtio-blk.c   |   31 +++----------------------------
 6 files changed, 62 insertions(+), 85 deletions(-)

diff --git a/hw/block-common.c b/hw/block-common.c
index 0a0542a..f0196d7 100644
--- a/hw/block-common.c
+++ b/hw/block-common.c
@@ -9,6 +9,7 @@
 
 #include "blockdev.h"
 #include "hw/block-common.h"
+#include "qemu-error.h"
 
 void blkconf_serial(BlockConf *conf, char **serial)
 {
@@ -22,3 +23,42 @@ void blkconf_serial(BlockConf *conf, char **serial)
         }
     }
 }
+
+int blkconf_geometry(BlockConf *conf, int *ptrans,
+                     unsigned cyls_max, unsigned heads_max, unsigned secs_max)
+{
+    DriveInfo *dinfo;
+
+    if (!conf->cyls && !conf->heads && !conf->secs) {
+        /* try to fall back to value set with legacy -drive cyls=... */
+        dinfo = drive_get_by_blockdev(conf->bs);
+        conf->cyls  = dinfo->cyls;
+        conf->heads = dinfo->heads;
+        conf->secs  = dinfo->secs;
+        if (ptrans) {
+            *ptrans = dinfo->trans;
+        }
+    }
+    if (!conf->cyls && !conf->heads && !conf->secs) {
+        hd_geometry_guess(conf->bs,
+                          &conf->cyls, &conf->heads, &conf->secs,
+                          ptrans);
+    } else if (ptrans && *ptrans == BIOS_ATA_TRANSLATION_AUTO) {
+        *ptrans = hd_bios_chs_auto_trans(conf->cyls, conf->heads, conf->secs);
+    }
+    if (conf->cyls || conf->heads || conf->secs) {
+        if (conf->cyls < 1 || conf->cyls > cyls_max) {
+            error_report("cyls must be between 1 and %u", cyls_max);
+            return -1;
+        }
+        if (conf->heads < 1 || conf->heads > heads_max) {
+            error_report("heads must be between 1 and %u", heads_max);
+            return -1;
+        }
+        if (conf->secs < 1 || conf->secs > secs_max) {
+            error_report("secs must be between 1 and %u", secs_max);
+            return -1;
+        }
+    }
+    return 0;
+}
diff --git a/hw/block-common.h b/hw/block-common.h
index 52bddda..bb808f7 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -60,6 +60,8 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
 /* Configuration helpers */
 
 void blkconf_serial(BlockConf *conf, char **serial);
+int blkconf_geometry(BlockConf *conf, int *trans,
+                     unsigned cyls_max, unsigned heads_max, unsigned secs_max);
 
 /* Hard disk geometry */
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5378fc3..d65ef3d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1935,18 +1935,6 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     s->drive_kind = kind;
 
     bdrv_get_geometry(bs, &nb_sectors);
-    if (cylinders < 1 || cylinders > 65535) {
-        error_report("cyls must be between 1 and 65535");
-        return -1;
-    }
-    if (heads < 1 || heads > 16) {
-        error_report("heads must be between 1 and 16");
-        return -1;
-    }
-    if (secs < 1 || secs > 255) {
-        error_report("secs must be between 1 and 255");
-        return -1;
-    }
     s->cylinders = cylinders;
     s->heads = heads;
     s->sectors = secs;
@@ -2094,6 +2082,18 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
             } else if (trans == BIOS_ATA_TRANSLATION_AUTO) {
                 trans = hd_bios_chs_auto_trans(cyls, heads, secs);
             }
+            if (cyls < 1 || cyls > 65535) {
+                error_report("cyls must be between 1 and 65535");
+                exit(1);
+            }
+            if (heads < 1 || heads > 16) {
+                error_report("heads must be between 1 and 16");
+                exit(1);
+            }
+            if (secs < 1 || secs > 255) {
+                error_report("secs must be between 1 and 255");
+                exit(1);
+            }
             if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
                                dinfo->media_cd ? IDE_CD : IDE_HD,
                                NULL, dinfo->serial, NULL, 0,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 7fe803c..22e58df 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -142,7 +142,6 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 {
     IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
     IDEState *s = bus->ifs + dev->unit;
-    DriveInfo *dinfo;
 
     if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) {
         error_report("discard_granularity must be 512 for ide");
@@ -150,22 +149,8 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     }
 
     blkconf_serial(&dev->conf, &dev->serial);
-
-    if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
-        /* try to fall back to value set with legacy -drive cyls=... */
-        dinfo = drive_get_by_blockdev(dev->conf.bs);
-        dev->conf.cyls  = dinfo->cyls;
-        dev->conf.heads = dinfo->heads;
-        dev->conf.secs  = dinfo->secs;
-        dev->chs_trans  = dinfo->trans;
-    }
-    if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
-        hd_geometry_guess(dev->conf.bs,
-                          &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs,
-                          &dev->chs_trans);
-    } else if (dev->chs_trans == BIOS_ATA_TRANSLATION_AUTO) {
-        dev->chs_trans = hd_bios_chs_auto_trans(dev->conf.cyls,
-                                        dev->conf.heads, dev->conf.secs);
+    if (blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255) < 0) {
+        return -1;
     }
 
     if (ide_init_drive(s, dev->conf.bs, kind,
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 39a07d7..525816c 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1737,7 +1737,6 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
 static int scsi_initfn(SCSIDevice *dev)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-    DriveInfo *dinfo;
 
     if (!s->qdev.conf.bs) {
         error_report("drive property not set");
@@ -1750,34 +1749,10 @@ static int scsi_initfn(SCSIDevice *dev)
         return -1;
     }
 
-    if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
-        /* try to fall back to value set with legacy -drive cyls=... */
-        dinfo = drive_get_by_blockdev(s->qdev.conf.bs);
-        dev->conf.cyls = dinfo->cyls;
-        dev->conf.heads = dinfo->heads;
-        dev->conf.secs = dinfo->secs;
-    }
-    if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
-        hd_geometry_guess(s->qdev.conf.bs,
-                          &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs,
-                          NULL);
-    }
-    if (dev->conf.cyls || dev->conf.heads || dev->conf.secs) {
-        if (dev->conf.cyls < 1 || dev->conf.cyls > 65535) {
-            error_report("cyls must be between 1 and 65535");
-            return -1;
-        }
-        if (dev->conf.heads < 1 || dev->conf.heads > 255) {
-            error_report("heads must be between 1 and 255");
-            return -1;
-        }
-        if (dev->conf.secs < 1 || dev->conf.secs > 255) {
-            error_report("secs must be between 1 and 255");
-            return -1;
-        }
-    }
-
     blkconf_serial(&s->qdev.conf, &s->serial);
+    if (blkconf_geometry(&dev->conf, NULL, 65535, 255, 255) < 0) {
+        return -1;
+    }
 
     if (!s->version) {
         s->version = g_strdup(qemu_get_version());
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index ba087bc..f21757e 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -589,7 +589,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
 {
     VirtIOBlock *s;
     static int virtio_blk_id;
-    DriveInfo *dinfo;
 
     if (!blk->conf.bs) {
         error_report("drive property not set");
@@ -601,6 +600,9 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     }
 
     blkconf_serial(&blk->conf, &blk->serial);
+    if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
+        return NULL;
+    }
 
     s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
                                           sizeof(struct virtio_blk_config),
@@ -615,33 +617,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
-    if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) {
-        /* try to fall back to value set with legacy -drive cyls=... */
-        dinfo = drive_get_by_blockdev(blk->conf.bs);
-        blk->conf.cyls = dinfo->cyls;
-        blk->conf.heads = dinfo->heads;
-        blk->conf.secs = dinfo->secs;
-    }
-    if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) {
-        hd_geometry_guess(s->bs,
-                          &blk->conf.cyls, &blk->conf.heads, &blk->conf.secs,
-                          NULL);
-    }
-    if (blk->conf.cyls || blk->conf.heads || blk->conf.secs) {
-        if (blk->conf.cyls < 1 || blk->conf.cyls > 65535) {
-            error_report("cyls must be between 1 and 65535");
-            return NULL;
-        }
-        if (blk->conf.heads < 1 || blk->conf.heads > 255) {
-            error_report("heads must be between 1 and 255");
-            return NULL;
-        }
-        if (blk->conf.secs < 1 || blk->conf.secs > 255) {
-            error_report("secs must be between 1 and 255");
-            return NULL;
-        }
-    }
-
     s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
 
     qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
-- 
1.7.6.5

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

* Re: [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h
  2012-07-11 13:08 [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Markus Armbruster
                   ` (3 preceding siblings ...)
  2012-07-11 13:08 ` [Qemu-devel] [PATCH 4/4] hw/block-common: Factor out fall back to legacy -drive cyls= Markus Armbruster
@ 2012-07-12  9:02 ` Kevin Wolf
  4 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2012-07-12  9:02 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel

Am 11.07.2012 15:08, schrieb Markus Armbruster:
> Applies on top of "[PATCH v3 00/29] Disk geometry cleanup".
> 
> Markus Armbruster (4):
>   hw/block-common: Move BlockConf & friends from block.h
>   hw/block-common: Factor out fall back to legacy -drive serial=...
>   blockdev: Don't limit DriveInfo serial to 20 characters
>   hw/block-common: Factor out fall back to legacy -drive cyls=...
> 
>  block.h              |   45 -----------------------------------
>  blockdev.c           |    4 +--
>  blockdev.h           |    4 +--
>  hw/Makefile.objs     |    2 +-
>  hw/block-common.c    |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/block-common.h    |   50 +++++++++++++++++++++++++++++++++++++++
>  hw/ide/core.c        |   30 +++++++++++-----------
>  hw/ide/internal.h    |    1 +
>  hw/ide/qdev.c        |   31 +++---------------------
>  hw/scsi-disk.c       |   37 ++--------------------------
>  hw/scsi.h            |    1 +
>  hw/usb.h             |    1 -
>  hw/usb/dev-storage.c |   10 +------
>  hw/virtio-blk.c      |   37 ++--------------------------
>  hw/virtio-blk.h      |    2 +-
>  hw/virtio.h          |    1 -
>  16 files changed, 147 insertions(+), 173 deletions(-)
>  create mode 100644 hw/block-common.c
> 

Thanks, applied all to the block branch.

Kevin

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

end of thread, other threads:[~2012-07-12  9:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-11 13:08 [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Markus Armbruster
2012-07-11 13:08 ` [Qemu-devel] [PATCH 1/4] hw/block-common: Move BlockConf & friends from block.h Markus Armbruster
2012-07-11 13:08 ` [Qemu-devel] [PATCH 2/4] hw/block-common: Factor out fall back to legacy -drive serial= Markus Armbruster
2012-07-11 13:08 ` [Qemu-devel] [PATCH 3/4] blockdev: Don't limit DriveInfo serial to 20 characters Markus Armbruster
2012-07-11 13:08 ` [Qemu-devel] [PATCH 4/4] hw/block-common: Factor out fall back to legacy -drive cyls= Markus Armbruster
2012-07-12  9:02 ` [Qemu-devel] [PATCH 0/4] Cleanups around hw/block-common.h Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.