* [Qemu-devel] [PATCH 01/10] scsi hotplug: Set DriveInfo member bus correctly
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 02/10] blockdev: New drive_get_next(), replacing qdev_init_bdrv() Markus Armbruster
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
drive_init() picks the first free bus and unit number, unless the user
specifies them.
This isn't a good fit for the drive_add monitor command, because there
we specify the controller by PCI address instead of using bus number
set by drive_init().
scsi_hot_add() takes care to replace the unit number set by
drive_init() by the real one, but it neglects to replace the bus
number. Thus, bus/unit in DriveInfo may be bogus. Affects
drive_get() and drive_get_max_bus(). I'm not aware of anything bad
happening because of that; looks like by the time we're hot-plugging,
the two functions aren't used anymore. Fix it anyway.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci-hotplug.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 270a982..b6dcbda 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -90,6 +90,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
* specified).
*/
dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
+ dinfo->bus = scsibus->busnr;
scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit, false);
if (!scsidev) {
return -1;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 02/10] blockdev: New drive_get_next(), replacing qdev_init_bdrv()
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 01/10] scsi hotplug: Set DriveInfo member bus correctly Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 03/10] blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h Markus Armbruster
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
qdev_init_bdrv() doesn't belong into qdev.c; it's about drives, not
qdevs. Rename to drive_get_next, move to blockdev.c, drop the bogus
DeviceState argument, and return DriveInfo instead of
BlockDriverState.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 10 ++++++++++
blockdev.h | 1 +
hw/pl181.c | 7 ++++---
hw/qdev.c | 14 --------------
hw/qdev.h | 2 --
hw/ssi-sd.c | 7 ++++---
6 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index f7f591f..883117a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -93,6 +93,16 @@ int drive_get_max_bus(BlockInterfaceType type)
return max_bus;
}
+/* Get a block device. This should only be used for single-drive devices
+ (e.g. SD/Floppy/MTD). Multi-disk devices (scsi/ide) should use the
+ appropriate bus. */
+DriveInfo *drive_get_next(BlockInterfaceType type)
+{
+ static int next_block_unit[IF_COUNT];
+
+ return drive_get(type, 0, next_block_unit[type]++);
+}
+
DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
{
DriveInfo *dinfo;
diff --git a/blockdev.h b/blockdev.h
index 4536b5c..5c2144c 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -36,6 +36,7 @@ struct DriveInfo {
DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
int drive_get_max_bus(BlockInterfaceType type);
+DriveInfo *drive_get_next(BlockInterfaceType type);
void drive_uninit(DriveInfo *dinfo);
DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
diff --git a/hw/pl181.c b/hw/pl181.c
index 3e5f92f..36d9d02 100644
--- a/hw/pl181.c
+++ b/hw/pl181.c
@@ -7,6 +7,7 @@
* This code is licenced under the GPL.
*/
+#include "blockdev.h"
#include "sysbus.h"
#include "sd.h"
@@ -449,15 +450,15 @@ static int pl181_init(SysBusDevice *dev)
{
int iomemtype;
pl181_state *s = FROM_SYSBUS(pl181_state, dev);
- BlockDriverState *bd;
+ DriveInfo *dinfo;
iomemtype = cpu_register_io_memory(pl181_readfn, pl181_writefn, s,
DEVICE_NATIVE_ENDIAN);
sysbus_init_mmio(dev, 0x1000, iomemtype);
sysbus_init_irq(dev, &s->irq[0]);
sysbus_init_irq(dev, &s->irq[1]);
- bd = qdev_init_bdrv(&dev->qdev, IF_SD);
- s->card = sd_init(bd, 0);
+ dinfo = drive_get_next(IF_SD);
+ s->card = sd_init(dinfo ? dinfo->bdrv : NULL, 0);
qemu_register_reset(pl181_reset, s);
pl181_reset(s);
/* ??? Save/restore. */
diff --git a/hw/qdev.c b/hw/qdev.c
index 5b8d374..0c94fb2 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -458,20 +458,6 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
}
}
-static int next_block_unit[IF_COUNT];
-
-/* Get a block device. This should only be used for single-drive devices
- (e.g. SD/Floppy/MTD). Multi-disk devices (scsi/ide) should use the
- appropriate bus. */
-BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type)
-{
- int unit = next_block_unit[type]++;
- DriveInfo *dinfo;
-
- dinfo = drive_get(type, 0, unit);
- return dinfo ? dinfo->bdrv : NULL;
-}
-
BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
{
BusState *bus;
diff --git a/hw/qdev.h b/hw/qdev.h
index e520aaa..9808f85 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -137,8 +137,6 @@ bool qdev_machine_modified(void);
qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
-BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type);
-
BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
/*** Device API. ***/
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index a1a63b2..fb4b649 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -7,6 +7,7 @@
* This code is licenced under the GNU GPL v2.
*/
+#include "blockdev.h"
#include "ssi.h"
#include "sd.h"
@@ -231,11 +232,11 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
static int ssi_sd_init(SSISlave *dev)
{
ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev);
- BlockDriverState *bs;
+ DriveInfo *dinfo;
s->mode = SSI_SD_CMD;
- bs = qdev_init_bdrv(&dev->qdev, IF_SD);
- s->sd = sd_init(bs, 1);
+ dinfo = drive_get_next(IF_SD);
+ s->sd = sd_init(dinfo ? dinfo->bdrv : NULL, 1);
register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
return 0;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 03/10] blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 01/10] scsi hotplug: Set DriveInfo member bus correctly Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 02/10] blockdev: New drive_get_next(), replacing qdev_init_bdrv() Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 04/10] blockdev: Put BlockInterfaceType names and max_devs in tables Markus Armbruster
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.h | 6 ++++++
qemu-common.h | 6 ------
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/blockdev.h b/blockdev.h
index 5c2144c..f616855 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -18,6 +18,12 @@ void blockdev_auto_del(BlockDriverState *bs);
#define BLOCK_SERIAL_STRLEN 20
+typedef enum {
+ IF_NONE,
+ IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+ IF_COUNT
+} BlockInterfaceType;
+
struct DriveInfo {
BlockDriverState *bdrv;
char *id;
diff --git a/qemu-common.h b/qemu-common.h
index c351131..9cde519 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -267,12 +267,6 @@ typedef struct VirtIODevice VirtIODevice;
typedef uint64_t pcibus_t;
-typedef enum {
- IF_NONE,
- IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
- IF_COUNT
-} BlockInterfaceType;
-
void cpu_exec_init_all(unsigned long tb_size);
/* CPU save/load. */
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 04/10] blockdev: Put BlockInterfaceType names and max_devs in tables
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
` (2 preceding siblings ...)
2011-01-27 12:29 ` [Qemu-devel] [PATCH 03/10] blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 05/10] blockdev: Make drive_add() take explicit type, index parameters Markus Armbruster
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Turns drive_init()'s lengthy conditional into a concise loop, and
makes the data available elsewhere.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 49 +++++++++++++++++++++----------------------------
1 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 883117a..e8dd353 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -19,6 +19,23 @@
static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
+static const char *const if_name[IF_COUNT] = {
+ [IF_NONE] = "none",
+ [IF_IDE] = "ide",
+ [IF_SCSI] = "scsi",
+ [IF_FLOPPY] = "floppy",
+ [IF_PFLASH] = "pflash",
+ [IF_MTD] = "mtd",
+ [IF_SD] = "sd",
+ [IF_VIRTIO] = "virtio",
+ [IF_XEN] = "xen",
+};
+
+static const int if_max_devs[IF_COUNT] = {
+ [IF_IDE] = MAX_IDE_DEVS,
+ [IF_SCSI] = MAX_SCSI_DEVS,
+};
+
/*
* We automatically delete the drive when a device using it gets
* unplugged. Questionable feature, but we can't just drop it.
@@ -199,37 +216,13 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
if ((buf = qemu_opt_get(opts, "if")) != NULL) {
pstrcpy(devname, sizeof(devname), buf);
- if (!strcmp(buf, "ide")) {
- type = IF_IDE;
- max_devs = MAX_IDE_DEVS;
- } else if (!strcmp(buf, "scsi")) {
- type = IF_SCSI;
- max_devs = MAX_SCSI_DEVS;
- } else if (!strcmp(buf, "floppy")) {
- type = IF_FLOPPY;
- max_devs = 0;
- } else if (!strcmp(buf, "pflash")) {
- type = IF_PFLASH;
- max_devs = 0;
- } else if (!strcmp(buf, "mtd")) {
- type = IF_MTD;
- max_devs = 0;
- } else if (!strcmp(buf, "sd")) {
- type = IF_SD;
- max_devs = 0;
- } else if (!strcmp(buf, "virtio")) {
- type = IF_VIRTIO;
- max_devs = 0;
- } else if (!strcmp(buf, "xen")) {
- type = IF_XEN;
- max_devs = 0;
- } else if (!strcmp(buf, "none")) {
- type = IF_NONE;
- max_devs = 0;
- } else {
+ for (type = 0; type < IF_COUNT && strcmp(buf, if_name[type]); type++)
+ ;
+ if (type == IF_COUNT) {
error_report("unsupported bus type '%s'", buf);
return NULL;
}
+ max_devs = if_max_devs[type];
}
if (cyls || heads || secs) {
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 05/10] blockdev: Make drive_add() take explicit type, index parameters
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
` (3 preceding siblings ...)
2011-01-27 12:29 ` [Qemu-devel] [PATCH 04/10] blockdev: Put BlockInterfaceType names and max_devs in tables Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 06/10] blockdev: Replace drive_add()'s fmt, ... by optstr parameter Markus Armbruster
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Before, type & index were hidden in printf-like fmt, ... parameters,
which get expanded into an option string. Rather inconvenient for
uses later in this series.
New IF_DEFAULT to ask for the machine's default interface. Before,
that was done by having no option "if" in the option string.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 20 +++++++++++++++++---
blockdev.h | 8 +++++++-
hw/device-hotplug.c | 2 +-
vl.c | 43 +++++++++++++++++++++++--------------------
4 files changed, 48 insertions(+), 25 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index e8dd353..2c4246f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -61,20 +61,34 @@ void blockdev_auto_del(BlockDriverState *bs)
}
}
-QemuOpts *drive_add(const char *file, const char *fmt, ...)
+QemuOpts *drive_def(const char *optstr)
+{
+ return qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
+}
+
+QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
+ const char *fmt, ...)
{
va_list ap;
char optstr[1024];
QemuOpts *opts;
+ char buf[32];
va_start(ap, fmt);
vsnprintf(optstr, sizeof(optstr), fmt, ap);
va_end(ap);
- opts = qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
+ opts = drive_def(optstr);
if (!opts) {
return NULL;
}
+ if (type != IF_DEFAULT) {
+ qemu_opt_set(opts, "if", if_name[type]);
+ }
+ if (index >= 0) {
+ snprintf(buf, sizeof(buf), "%d", index);
+ qemu_opt_set(opts, "index", buf);
+ }
if (file)
qemu_opt_set(opts, "file", file);
return opts;
@@ -461,7 +475,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
if (devaddr)
qemu_opt_set(opts, "addr", devaddr);
break;
- case IF_COUNT:
+ default:
abort();
}
if (!file || !*file) {
diff --git a/blockdev.h b/blockdev.h
index f616855..cd64a70 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -19,6 +19,7 @@ void blockdev_auto_del(BlockDriverState *bs);
#define BLOCK_SERIAL_STRLEN 20
typedef enum {
+ IF_DEFAULT = -1, /* for use with drive_add() only */
IF_NONE,
IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
IF_COUNT
@@ -46,7 +47,12 @@ DriveInfo *drive_get_next(BlockInterfaceType type);
void drive_uninit(DriveInfo *dinfo);
DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
-QemuOpts *drive_add(const char *file, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+QemuOpts *drive_def(const char *optstr);
+QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
+ const char *fmt, ...) /*GCC_FMT_ATTR(4, 5)*/;
+ /* GCC_FMT_ATTR() commented out to avoid the (pretty useless)
+ * "zero-length gnu_printf format string" warning we insist to
+ * enable */
DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
/* device-hotplug */
diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c
index 9704e2f..95a6372 100644
--- a/hw/device-hotplug.c
+++ b/hw/device-hotplug.c
@@ -33,7 +33,7 @@ DriveInfo *add_init_drive(const char *optstr)
DriveInfo *dinfo;
QemuOpts *opts;
- opts = drive_add(NULL, "%s", optstr);
+ opts = drive_def(optstr);
if (!opts)
return NULL;
diff --git a/vl.c b/vl.c
index 14255c4..5399e33 100644
--- a/vl.c
+++ b/vl.c
@@ -621,12 +621,13 @@ static int bt_parse(const char *opt)
/***********************************************************/
/* QEMU Block devices */
-#define HD_ALIAS "index=%d,media=disk"
-#define CDROM_ALIAS "index=2,media=cdrom"
-#define FD_ALIAS "index=%d,if=floppy"
-#define PFLASH_ALIAS "if=pflash"
-#define MTD_ALIAS "if=mtd"
-#define SD_ALIAS "index=0,if=sd"
+/* Any % in the following strings must be escaped as %% */
+#define HD_OPTS "media=disk"
+#define CDROM_OPTS "media=cdrom"
+#define FD_OPTS ""
+#define PFLASH_OPTS ""
+#define MTD_OPTS ""
+#define SD_OPTS ""
static int drive_init_func(QemuOpts *opts, void *opaque)
{
@@ -1987,7 +1988,7 @@ int main(int argc, char **argv, char **envp)
if (optind >= argc)
break;
if (argv[optind][0] != '-') {
- hda_opts = drive_add(argv[optind++], HD_ALIAS, 0);
+ hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
} else {
const QEMUOption *popt;
@@ -2027,11 +2028,11 @@ int main(int argc, char **argv, char **envp)
break;
case QEMU_OPTION_hda:
if (cyls == 0)
- hda_opts = drive_add(optarg, HD_ALIAS, 0);
+ hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS);
else
- hda_opts = drive_add(optarg, HD_ALIAS
+ hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS
",cyls=%d,heads=%d,secs=%d%s",
- 0, cyls, heads, secs,
+ cyls, heads, secs,
translation == BIOS_ATA_TRANSLATION_LBA ?
",trans=lba" :
translation == BIOS_ATA_TRANSLATION_NONE ?
@@ -2040,10 +2041,11 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_hdb:
case QEMU_OPTION_hdc:
case QEMU_OPTION_hdd:
- drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda);
+ drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
+ HD_OPTS);
break;
case QEMU_OPTION_drive:
- drive_add(NULL, "%s", optarg);
+ drive_def(optarg);
break;
case QEMU_OPTION_set:
if (qemu_set_option(optarg) != 0)
@@ -2054,13 +2056,13 @@ int main(int argc, char **argv, char **envp)
exit(1);
break;
case QEMU_OPTION_mtdblock:
- drive_add(optarg, MTD_ALIAS);
+ drive_add(IF_MTD, -1, optarg, MTD_OPTS);
break;
case QEMU_OPTION_sd:
- drive_add(optarg, SD_ALIAS);
+ drive_add(IF_SD, 0, optarg, SD_OPTS);
break;
case QEMU_OPTION_pflash:
- drive_add(optarg, PFLASH_ALIAS);
+ drive_add(IF_PFLASH, -1, optarg, PFLASH_OPTS);
break;
case QEMU_OPTION_snapshot:
snapshot = 1;
@@ -2139,7 +2141,7 @@ int main(int argc, char **argv, char **envp)
kernel_cmdline = optarg;
break;
case QEMU_OPTION_cdrom:
- drive_add(optarg, CDROM_ALIAS);
+ drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
break;
case QEMU_OPTION_boot:
{
@@ -2192,7 +2194,8 @@ int main(int argc, char **argv, char **envp)
break;
case QEMU_OPTION_fda:
case QEMU_OPTION_fdb:
- drive_add(optarg, FD_ALIAS, popt->index - QEMU_OPTION_fda);
+ drive_add(IF_FLOPPY, popt->index - QEMU_OPTION_fda,
+ optarg, FD_OPTS);
break;
case QEMU_OPTION_no_fd_bootchk:
fd_bootchk = 0;
@@ -2892,17 +2895,17 @@ int main(int argc, char **argv, char **envp)
if (default_cdrom) {
/* we always create the cdrom drive, even if no disk is there */
- drive_add(NULL, CDROM_ALIAS);
+ drive_add(IF_DEFAULT, 2, NULL, CDROM_OPTS);
}
if (default_floppy) {
/* we always create at least one floppy */
- drive_add(NULL, FD_ALIAS, 0);
+ drive_add(IF_FLOPPY, 0, NULL, FD_OPTS);
}
if (default_sdcard) {
/* we always create one sd slot, even if no card is in it */
- drive_add(NULL, SD_ALIAS);
+ drive_add(IF_SD, 0, NULL, SD_OPTS);
}
/* open the virtual block devices */
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 06/10] blockdev: Replace drive_add()'s fmt, ... by optstr parameter
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
` (4 preceding siblings ...)
2011-01-27 12:29 ` [Qemu-devel] [PATCH 05/10] blockdev: Make drive_add() take explicit type, index parameters Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 07/10] blockdev: Factor drive_index_to_{bus, unit}_id out of drive_init() Markus Armbruster
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Let the callers build the optstr. Only one wants to. All the others
become simpler, because they don't have to worry about escaping '%'.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 8 +-------
blockdev.h | 5 +----
vl.c | 13 +++++++------
3 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 2c4246f..e67f93c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -67,17 +67,11 @@ QemuOpts *drive_def(const char *optstr)
}
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
- const char *fmt, ...)
+ const char *optstr)
{
- va_list ap;
- char optstr[1024];
QemuOpts *opts;
char buf[32];
- va_start(ap, fmt);
- vsnprintf(optstr, sizeof(optstr), fmt, ap);
- va_end(ap);
-
opts = drive_def(optstr);
if (!opts) {
return NULL;
diff --git a/blockdev.h b/blockdev.h
index cd64a70..e29fa5e 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -49,10 +49,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
QemuOpts *drive_def(const char *optstr);
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
- const char *fmt, ...) /*GCC_FMT_ATTR(4, 5)*/;
- /* GCC_FMT_ATTR() commented out to avoid the (pretty useless)
- * "zero-length gnu_printf format string" warning we insist to
- * enable */
+ const char *optstr);
DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
/* device-hotplug */
diff --git a/vl.c b/vl.c
index 5399e33..2fa1ec0 100644
--- a/vl.c
+++ b/vl.c
@@ -621,7 +621,6 @@ static int bt_parse(const char *opt)
/***********************************************************/
/* QEMU Block devices */
-/* Any % in the following strings must be escaped as %% */
#define HD_OPTS "media=disk"
#define CDROM_OPTS "media=cdrom"
#define FD_OPTS ""
@@ -1912,6 +1911,7 @@ int main(int argc, char **argv, char **envp)
const char *incoming = NULL;
int show_vnc_port = 0;
int defconfig = 1;
+ char buf[256];
#ifdef CONFIG_SIMPLE_TRACE
const char *trace_file = NULL;
@@ -2028,16 +2028,17 @@ int main(int argc, char **argv, char **envp)
break;
case QEMU_OPTION_hda:
if (cyls == 0)
- hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS);
+ snprintf(buf, sizeof(buf), "%s", HD_OPTS);
else
- hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS
- ",cyls=%d,heads=%d,secs=%d%s",
- cyls, heads, secs,
+ snprintf(buf, sizeof(buf),
+ "%s,cyls=%d,heads=%d,secs=%d%s",
+ HD_OPTS , cyls, heads, secs,
translation == BIOS_ATA_TRANSLATION_LBA ?
",trans=lba" :
translation == BIOS_ATA_TRANSLATION_NONE ?
",trans=none" : "");
- break;
+ drive_add(IF_DEFAULT, 0, optarg, buf);
+ break;
case QEMU_OPTION_hdb:
case QEMU_OPTION_hdc:
case QEMU_OPTION_hdd:
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 07/10] blockdev: Factor drive_index_to_{bus, unit}_id out of drive_init()
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
` (5 preceding siblings ...)
2011-01-27 12:29 ` [Qemu-devel] [PATCH 06/10] blockdev: Replace drive_add()'s fmt, ... by optstr parameter Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 08/10] blockdev: New drive_get_by_index() Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 09/10] blockdev: Reject multiple definitions for the same drive Markus Armbruster
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index e67f93c..4d8bb3d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -61,6 +61,18 @@ void blockdev_auto_del(BlockDriverState *bs)
}
}
+static int drive_index_to_bus_id(BlockInterfaceType type, int index)
+{
+ int max_devs = if_max_devs[type];
+ return max_devs ? index / max_devs : 0;
+}
+
+static int drive_index_to_unit_id(BlockInterfaceType type, int index)
+{
+ int max_devs = if_max_devs[type];
+ return max_devs ? index % max_devs : index;
+}
+
QemuOpts *drive_def(const char *optstr)
{
return qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
@@ -364,14 +376,8 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
error_report("index cannot be used with bus and unit");
return NULL;
}
- if (max_devs == 0)
- {
- unit_id = index;
- bus_id = 0;
- } else {
- unit_id = index % max_devs;
- bus_id = index / max_devs;
- }
+ bus_id = drive_index_to_bus_id(type, index);
+ unit_id = drive_index_to_unit_id(type, index);
}
/* if user doesn't specify a unit_id,
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 08/10] blockdev: New drive_get_by_index()
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
` (6 preceding siblings ...)
2011-01-27 12:29 ` [Qemu-devel] [PATCH 07/10] blockdev: Factor drive_index_to_{bus, unit}_id out of drive_init() Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
2011-01-27 12:29 ` [Qemu-devel] [PATCH 09/10] blockdev: Reject multiple definitions for the same drive Markus Armbruster
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 7 +++++++
blockdev.h | 1 +
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 4d8bb3d..08c9d7e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -116,6 +116,13 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit)
return NULL;
}
+DriveInfo *drive_get_by_index(BlockInterfaceType type, int index)
+{
+ return drive_get(type,
+ drive_index_to_bus_id(type, index),
+ drive_index_to_unit_id(type, index));
+}
+
int drive_get_max_bus(BlockInterfaceType type)
{
int max_bus;
diff --git a/blockdev.h b/blockdev.h
index e29fa5e..2d518c5 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -42,6 +42,7 @@ struct DriveInfo {
#define MAX_SCSI_DEVS 255
DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
+DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
int drive_get_max_bus(BlockInterfaceType type);
DriveInfo *drive_get_next(BlockInterfaceType type);
void drive_uninit(DriveInfo *dinfo);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 09/10] blockdev: Reject multiple definitions for the same drive
2011-01-27 12:29 [Qemu-devel] [PATCH 00/10] -drive/drive_add fixes and cleanups Markus Armbruster
` (7 preceding siblings ...)
2011-01-27 12:29 ` [Qemu-devel] [PATCH 08/10] blockdev: New drive_get_by_index() Markus Armbruster
@ 2011-01-27 12:29 ` Markus Armbruster
8 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2011-01-27 12:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
We silently ignore multiple definitions for the same drive:
$ qemu-system-x86_64 -nodefaults -vnc :1 -S -monitor stdio -drive if=ide,index=1,file=tmp.qcow2 -drive if=ide,index=1,file=nonexistant
QEMU 0.13.50 monitor - type 'help' for more information
(qemu) info block
ide0-hd1: type=hd removable=0 file=tmp.qcow2 backing_file=tmp.img ro=0 drv=qcow2 encrypted=0
With if=none, this can become quite confusing:
$ qemu-system-x86_64 -nodefaults -vnc :1 -S -monitor stdio -drive if=none,index=1,file=tmp.qcow2,id=eins -drive if=none,index=1,file=nonexistant,id=zwei -device ide-drive,drive=eins -device ide-drive,drive=zwei
qemu-system-x86_64: -device ide-drive,drive=zwei: Property 'ide-drive.drive' can't find value 'zwei'
The second -device fails, because it refers to drive zwei, which got
silently ignored.
Make multiple drive definitions fail cleanly.
Unfortunately, there's code that relies on multiple drive definitions
being silently ignored: main() merrily adds default drives even when
the user already defined these drives. Fix that up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 5 +++--
vl.c | 45 ++++++++++++++++++++++++++++++---------------
2 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 08c9d7e..a2fd4d5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -411,11 +411,12 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
}
/*
- * ignore multiple definitions
+ * catch multiple definitions
*/
if (drive_get(type, bus_id, unit_id) != NULL) {
- *fatal_error = 0;
+ error_report("drive with bus=%d, unit=%d (index=%d) exists",
+ bus_id, unit_id, index);
return NULL;
}
diff --git a/vl.c b/vl.c
index 2fa1ec0..3637095 100644
--- a/vl.c
+++ b/vl.c
@@ -648,6 +648,29 @@ static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
return 0;
}
+static void default_drive(int enable, int snapshot, int use_scsi,
+ BlockInterfaceType type, int index,
+ const char *optstr)
+{
+ QemuOpts *opts;
+
+ if (type == IF_DEFAULT) {
+ type = use_scsi ? IF_SCSI : IF_IDE;
+ }
+
+ if (!enable || drive_get_by_index(type, index)) {
+ return;
+ }
+
+ opts = drive_add(type, index, NULL, optstr);
+ if (snapshot) {
+ drive_enable_snapshot(opts, NULL);
+ }
+ if (drive_init_func(opts, &use_scsi)) {
+ exit(1);
+ }
+}
+
void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
{
boot_set_handler = func;
@@ -2894,27 +2917,19 @@ int main(int argc, char **argv, char **envp)
blk_mig_init();
- if (default_cdrom) {
- /* we always create the cdrom drive, even if no disk is there */
- drive_add(IF_DEFAULT, 2, NULL, CDROM_OPTS);
- }
-
- if (default_floppy) {
- /* we always create at least one floppy */
- drive_add(IF_FLOPPY, 0, NULL, FD_OPTS);
- }
-
- if (default_sdcard) {
- /* we always create one sd slot, even if no card is in it */
- drive_add(IF_SD, 0, NULL, SD_OPTS);
- }
-
/* open the virtual block devices */
if (snapshot)
qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);
if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, &machine->use_scsi, 1) != 0)
exit(1);
+ default_drive(default_cdrom, snapshot, machine->use_scsi,
+ IF_DEFAULT, 2, CDROM_OPTS);
+ default_drive(default_floppy, snapshot, machine->use_scsi,
+ IF_FLOPPY, 0, FD_OPTS);
+ default_drive(default_sdcard, snapshot, machine->use_scsi,
+ IF_SD, 0, SD_OPTS);
+
register_savevm_live(NULL, "ram", 0, 4, NULL, ram_save_live, NULL,
ram_load, NULL);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 10+ messages in thread