* [Qemu-devel] [PATCH 01/13] block: Move error actions from DriveInfo to BlockDriverState
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 02/13] block: Decouple block device "commit all" from DriveInfo Markus Armbruster
` (11 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
That's where they belong semantically (block device host part), even
though the actions are actually executed by guest device code.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block.c | 12 ++++++++++++
block.h | 8 ++++++++
block_int.h | 1 +
blockdev.c | 17 ++---------------
blockdev.h | 10 ----------
hw/ide/core.c | 2 +-
hw/scsi-disk.c | 2 +-
hw/virtio-blk.c | 3 +--
8 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/block.c b/block.c
index cacf11b..5c2f3d9 100644
--- a/block.c
+++ b/block.c
@@ -1206,6 +1206,18 @@ int bdrv_get_translation_hint(BlockDriverState *bs)
return bs->translation;
}
+void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
+ BlockErrorAction on_write_error)
+{
+ bs->on_read_error = on_read_error;
+ bs->on_write_error = on_write_error;
+}
+
+BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read)
+{
+ return is_read ? bs->on_read_error : bs->on_write_error;
+}
+
int bdrv_is_removable(BlockDriverState *bs)
{
return bs->removable;
diff --git a/block.h b/block.h
index 25744b1..9ceb64d 100644
--- a/block.h
+++ b/block.h
@@ -42,6 +42,11 @@ typedef struct QEMUSnapshotInfo {
#define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1)
typedef enum {
+ BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
+ BLOCK_ERR_STOP_ANY
+} BlockErrorAction;
+
+typedef enum {
BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
} BlockMonEventAction;
@@ -146,6 +151,9 @@ void bdrv_get_geometry_hint(BlockDriverState *bs,
int *pcyls, int *pheads, int *psecs);
int bdrv_get_type_hint(BlockDriverState *bs);
int bdrv_get_translation_hint(BlockDriverState *bs);
+void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
+ BlockErrorAction on_write_error);
+BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
int bdrv_is_removable(BlockDriverState *bs);
int bdrv_is_read_only(BlockDriverState *bs);
int bdrv_is_sg(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index 1a7240c..e3bfd19 100644
--- a/block_int.h
+++ b/block_int.h
@@ -182,6 +182,7 @@ struct BlockDriverState {
drivers. They are not used by the block driver */
int cyls, heads, secs, translation;
int type;
+ BlockErrorAction on_read_error, on_write_error;
char device_name[32];
unsigned long *dirty_bitmap;
int64_t dirty_count;
diff --git a/blockdev.c b/blockdev.c
index bd9783a..0df16c7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -90,19 +90,6 @@ const char *drive_get_serial(BlockDriverState *bdrv)
return "\0";
}
-BlockInterfaceErrorAction drive_get_on_error(
- BlockDriverState *bdrv, int is_read)
-{
- DriveInfo *dinfo;
-
- QTAILQ_FOREACH(dinfo, &drives, next) {
- if (dinfo->bdrv == bdrv)
- return is_read ? dinfo->on_read_error : dinfo->on_write_error;
- }
-
- return is_read ? BLOCK_ERR_REPORT : BLOCK_ERR_STOP_ENOSPC;
-}
-
static void bdrv_format_print(void *opaque, const char *name)
{
fprintf(stderr, " %s", name);
@@ -418,13 +405,13 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
dinfo->type = type;
dinfo->bus = bus_id;
dinfo->unit = unit_id;
- dinfo->on_read_error = on_read_error;
- dinfo->on_write_error = on_write_error;
dinfo->opts = opts;
if (serial)
strncpy(dinfo->serial, serial, sizeof(serial));
QTAILQ_INSERT_TAIL(&drives, dinfo, next);
+ bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error);
+
switch(type) {
case IF_IDE:
case IF_SCSI:
diff --git a/blockdev.h b/blockdev.h
index dfc9de1..9e8a7fc 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -19,11 +19,6 @@ typedef enum {
IF_COUNT
} BlockInterfaceType;
-typedef enum {
- BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
- BLOCK_ERR_STOP_ANY
-} BlockInterfaceErrorAction;
-
#define BLOCK_SERIAL_STRLEN 20
typedef struct DriveInfo {
@@ -34,8 +29,6 @@ typedef struct DriveInfo {
int bus;
int unit;
QemuOpts *opts;
- BlockInterfaceErrorAction on_read_error;
- BlockInterfaceErrorAction on_write_error;
char serial[BLOCK_SERIAL_STRLEN + 1];
QTAILQ_ENTRY(DriveInfo) next;
} DriveInfo;
@@ -51,9 +44,6 @@ extern int drive_get_max_bus(BlockInterfaceType type);
extern void drive_uninit(DriveInfo *dinfo);
extern const char *drive_get_serial(BlockDriverState *bdrv);
-extern BlockInterfaceErrorAction drive_get_on_error(
- BlockDriverState *bdrv, int is_read);
-
extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
int *fatal_error);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 045d18d..0b3b7c2 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -481,7 +481,7 @@ void ide_dma_error(IDEState *s)
static int ide_handle_rw_error(IDEState *s, int error, int op)
{
int is_read = (op & BM_STATUS_RETRY_READ);
- BlockInterfaceErrorAction action = drive_get_on_error(s->bs, is_read);
+ BlockErrorAction action = bdrv_get_on_error(s->bs, is_read);
if (action == BLOCK_ERR_IGNORE) {
bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, is_read);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a9bf7d2..2b38984 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -182,7 +182,7 @@ static void scsi_read_data(SCSIDevice *d, uint32_t tag)
static int scsi_handle_write_error(SCSIDiskReq *r, int error)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
- BlockInterfaceErrorAction action = drive_get_on_error(s->bs, 0);
+ BlockErrorAction action = bdrv_get_on_error(s->bs, 0);
if (action == BLOCK_ERR_IGNORE) {
bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, 0);
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index cdcb492..cb17afa 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -58,8 +58,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
int is_read)
{
- BlockInterfaceErrorAction action =
- drive_get_on_error(req->dev->bs, is_read);
+ BlockErrorAction action = bdrv_get_on_error(req->dev->bs, is_read);
VirtIOBlock *s = req->dev;
if (action == BLOCK_ERR_IGNORE) {
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 02/13] block: Decouple block device "commit all" from DriveInfo
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 01/13] block: Move error actions from DriveInfo to BlockDriverState Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 03/13] monitor: Make "commit FOO" complain when FOO doesn't exist Markus Armbruster
` (10 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
do_commit() and mux_proc_byte() iterate over the list of drives
defined with drive_init(). This misses host block devices defined by
other means. Such means don't exist now, but will be introduced later
in this series.
Change them to use new bdrv_commit_all(), which iterates over all host
block devices.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block.c | 9 +++++++++
block.h | 1 +
blockdev.c | 16 ++++++++--------
qemu-char.c | 7 +------
4 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/block.c b/block.c
index 5c2f3d9..1bef649 100644
--- a/block.c
+++ b/block.c
@@ -788,6 +788,15 @@ ro_cleanup:
return ret;
}
+void bdrv_commit_all(void)
+{
+ BlockDriverState *bs;
+
+ QTAILQ_FOREACH(bs, &bdrv_states, list) {
+ bdrv_commit(bs);
+ }
+}
+
/*
* Return values:
* 0 - success
diff --git a/block.h b/block.h
index 9ceb64d..49e3dcf 100644
--- a/block.h
+++ b/block.h
@@ -85,6 +85,7 @@ int64_t bdrv_getlength(BlockDriverState *bs);
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs);
int bdrv_commit(BlockDriverState *bs);
+void bdrv_commit_all(void);
int bdrv_change_backing_file(BlockDriverState *bs,
const char *backing_file, const char *backing_fmt);
void bdrv_register(BlockDriver *bdrv);
diff --git a/blockdev.c b/blockdev.c
index 0df16c7..60c9211 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -486,16 +486,16 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
void do_commit(Monitor *mon, const QDict *qdict)
{
- int all_devices;
- DriveInfo *dinfo;
const char *device = qdict_get_str(qdict, "device");
+ BlockDriverState *bs;
- all_devices = !strcmp(device, "all");
- QTAILQ_FOREACH(dinfo, &drives, next) {
- if (!all_devices)
- if (strcmp(bdrv_get_device_name(dinfo->bdrv), device))
- continue;
- bdrv_commit(dinfo->bdrv);
+ if (!strcmp(device, "all")) {
+ bdrv_commit_all();
+ } else {
+ bs = bdrv_find(device);
+ if (bs) {
+ bdrv_commit(bs);
+ }
}
}
diff --git a/qemu-char.c b/qemu-char.c
index 87628ea..9b69d92 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -351,12 +351,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
break;
}
case 's':
- {
- DriveInfo *dinfo;
- QTAILQ_FOREACH(dinfo, &drives, next) {
- bdrv_commit(dinfo->bdrv);
- }
- }
+ bdrv_commit_all();
break;
case 'b':
qemu_chr_event(chr, CHR_EVENT_BREAK);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 03/13] monitor: Make "commit FOO" complain when FOO doesn't exist
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 01/13] block: Move error actions from DriveInfo to BlockDriverState Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 02/13] block: Decouple block device "commit all" from DriveInfo Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 04/13] block: New bdrv_next() Markus Armbruster
` (9 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 60c9211..53a0e9c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -493,9 +493,11 @@ void do_commit(Monitor *mon, const QDict *qdict)
bdrv_commit_all();
} else {
bs = bdrv_find(device);
- if (bs) {
- bdrv_commit(bs);
+ if (!bs) {
+ qerror_report(QERR_DEVICE_NOT_FOUND, device);
+ return;
}
+ bdrv_commit(bs);
}
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 04/13] block: New bdrv_next()
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (2 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 03/13] monitor: Make "commit FOO" complain when FOO doesn't exist Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 05/13] block: Decouple savevm from DriveInfo Markus Armbruster
` (8 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
This is a more flexible alternative to bdrv_iterate().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block.c | 8 ++++++++
block.h | 1 +
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/block.c b/block.c
index 1bef649..f42960c 100644
--- a/block.c
+++ b/block.c
@@ -1330,6 +1330,14 @@ BlockDriverState *bdrv_find(const char *name)
return NULL;
}
+BlockDriverState *bdrv_next(BlockDriverState *bs)
+{
+ if (!bs) {
+ return QTAILQ_FIRST(&bdrv_states);
+ }
+ return QTAILQ_NEXT(bs, list);
+}
+
void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque)
{
BlockDriverState *bs;
diff --git a/block.h b/block.h
index 49e3dcf..29993d1 100644
--- a/block.h
+++ b/block.h
@@ -168,6 +168,7 @@ void bdrv_set_change_cb(BlockDriverState *bs,
void (*change_cb)(void *opaque), void *opaque);
void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
BlockDriverState *bdrv_find(const char *name);
+BlockDriverState *bdrv_next(BlockDriverState *bs);
void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs),
void *opaque);
int bdrv_is_encrypted(BlockDriverState *bs);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 05/13] block: Decouple savevm from DriveInfo
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (3 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 04/13] block: New bdrv_next() Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 06/13] blockdev: Give drives internal linkage Markus Armbruster
` (7 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
We find snapshots by iterating over the list of drives defined with
drive_init(). This misses host block devices defined by other means.
Such means don't exist now, but will be introduced later in this
series.
Iterate over all host block devices instead, with bdrv_next().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
savevm.c | 32 ++++++++++++++------------------
1 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/savevm.c b/savevm.c
index 18852cd..12a8ba5 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1593,12 +1593,13 @@ static int bdrv_has_snapshot(BlockDriverState *bs)
static BlockDriverState *get_bs_snapshots(void)
{
BlockDriverState *bs;
- DriveInfo *dinfo;
if (bs_snapshots)
return bs_snapshots;
- QTAILQ_FOREACH(dinfo, &drives, next) {
- bs = dinfo->bdrv;
+ /* FIXME what if bs_snapshots gets hot-unplugged? */
+
+ bs = NULL;
+ while ((bs = bdrv_next(bs))) {
if (bdrv_can_snapshot(bs))
goto ok;
}
@@ -1636,12 +1637,11 @@ static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
static int del_existing_snapshots(Monitor *mon, const char *name)
{
BlockDriverState *bs;
- DriveInfo *dinfo;
QEMUSnapshotInfo sn1, *snapshot = &sn1;
int ret;
- QTAILQ_FOREACH(dinfo, &drives, next) {
- bs = dinfo->bdrv;
+ bs = NULL;
+ while ((bs = bdrv_next(bs))) {
if (bdrv_can_snapshot(bs) &&
bdrv_snapshot_find(bs, snapshot, name) >= 0)
{
@@ -1660,7 +1660,6 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
void do_savevm(Monitor *mon, const QDict *qdict)
{
- DriveInfo *dinfo;
BlockDriverState *bs, *bs1;
QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
int ret;
@@ -1730,8 +1729,8 @@ void do_savevm(Monitor *mon, const QDict *qdict)
/* create the snapshots */
- QTAILQ_FOREACH(dinfo, &drives, next) {
- bs1 = dinfo->bdrv;
+ bs1 = NULL;
+ while ((bs1 = bdrv_next(bs1))) {
if (bdrv_has_snapshot(bs1)) {
/* Write VM state size only to the image that contains the state */
sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
@@ -1750,7 +1749,6 @@ void do_savevm(Monitor *mon, const QDict *qdict)
int load_vmstate(const char *name)
{
- DriveInfo *dinfo;
BlockDriverState *bs, *bs1;
QEMUSnapshotInfo sn;
QEMUFile *f;
@@ -1765,8 +1763,8 @@ int load_vmstate(const char *name)
/* Flush all IO requests so they don't interfere with the new state. */
qemu_aio_flush();
- QTAILQ_FOREACH(dinfo, &drives, next) {
- bs1 = dinfo->bdrv;
+ bs1 = NULL;
+ while ((bs1 = bdrv_next(bs1))) {
if (bdrv_has_snapshot(bs1)) {
ret = bdrv_snapshot_goto(bs1, name);
if (ret < 0) {
@@ -1816,7 +1814,6 @@ int load_vmstate(const char *name)
void do_delvm(Monitor *mon, const QDict *qdict)
{
- DriveInfo *dinfo;
BlockDriverState *bs, *bs1;
int ret;
const char *name = qdict_get_str(qdict, "name");
@@ -1827,8 +1824,8 @@ void do_delvm(Monitor *mon, const QDict *qdict)
return;
}
- QTAILQ_FOREACH(dinfo, &drives, next) {
- bs1 = dinfo->bdrv;
+ bs1 = NULL;
+ while ((bs1 = bdrv_next(bs1))) {
if (bdrv_has_snapshot(bs1)) {
ret = bdrv_snapshot_delete(bs1, name);
if (ret < 0) {
@@ -1846,7 +1843,6 @@ void do_delvm(Monitor *mon, const QDict *qdict)
void do_info_snapshots(Monitor *mon)
{
- DriveInfo *dinfo;
BlockDriverState *bs, *bs1;
QEMUSnapshotInfo *sn_tab, *sn;
int nb_sns, i;
@@ -1858,8 +1854,8 @@ void do_info_snapshots(Monitor *mon)
return;
}
monitor_printf(mon, "Snapshot devices:");
- QTAILQ_FOREACH(dinfo, &drives, next) {
- bs1 = dinfo->bdrv;
+ bs1 = NULL;
+ while ((bs1 = bdrv_next(bs1))) {
if (bdrv_has_snapshot(bs1)) {
if (bs == bs1)
monitor_printf(mon, " %s", bdrv_get_device_name(bs1));
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 06/13] blockdev: Give drives internal linkage
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (4 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 05/13] block: Decouple savevm from DriveInfo Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 07/13] blockdev: Means to destroy blockdev only if made with drive_init() Markus Armbruster
` (6 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
This is the list of drives defined with drive_init(). Hide it, so it
doesn't get abused.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 2 +-
blockdev.h | 2 --
2 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 53a0e9c..ace74e4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -15,7 +15,7 @@
#include "qemu-config.h"
#include "sysemu.h"
-struct drivelist drives = QTAILQ_HEAD_INITIALIZER(drives);
+static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
QemuOpts *drive_add(const char *file, const char *fmt, ...)
{
diff --git a/blockdev.h b/blockdev.h
index 9e8a7fc..23ea576 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -36,8 +36,6 @@ typedef struct DriveInfo {
#define MAX_IDE_DEVS 2
#define MAX_SCSI_DEVS 7
-extern QTAILQ_HEAD(drivelist, DriveInfo) drives;
-
extern DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
extern DriveInfo *drive_get_by_id(const char *id);
extern int drive_get_max_bus(BlockInterfaceType type);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 07/13] blockdev: Means to destroy blockdev only if made with drive_init()
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (5 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 06/13] blockdev: Give drives internal linkage Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-10 14:19 ` [Qemu-devel] " Kevin Wolf
2010-06-02 16:55 ` [Qemu-devel] [PATCH 08/13] qdev: Decouple qdev_prop_drive from DriveInfo Markus Armbruster
` (5 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
All drives are still made that way. They get destroyed along with
their device. That's inappropriate for the alternative way to make
blockdevs that will appear later in this series. These won't have a
DriveInfo.
blockdev_detach() destroys the blockdev only if it has a DriveInfo.
blockdev_attach() does nothing for now. It'll be fleshed out later.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 35 +++++++++++++++++++++++++++++++++++
blockdev.h | 7 +++++++
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index ace74e4..f90d4fc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1,8 +1,12 @@
/*
* QEMU host block devices
*
+ * Copyright (C) 2010 Red Hat Inc.
* Copyright (c) 2003-2008 Fabrice Bellard
*
+ * Authors:
+ * Markus Armbruster <armbru@redhat.com>,
+ *
* This work is licensed under the terms of the GNU GPL, version 2 or
* later. See the COPYING file in the top-level directory.
*/
@@ -17,6 +21,37 @@
static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
+static int blockdev_del_dinfo(BlockDriverState *bs)
+{
+ DriveInfo *dinfo, *next_dinfo;
+ int res = 0;
+
+ QTAILQ_FOREACH_SAFE(dinfo, &drives, next, next_dinfo) {
+ if (dinfo->bdrv == bs) {
+ qemu_opts_del(dinfo->opts);
+ QTAILQ_REMOVE(&drives, dinfo, next);
+ qemu_free(dinfo);
+ res = 1;
+ }
+ }
+
+ return res;
+}
+
+int blockdev_attach(BlockDriverState *bs, DeviceState *qdev)
+{
+ return 0;
+}
+
+void blockdev_detach(BlockDriverState *bs, DeviceState *qdev)
+{
+ /* Backward compatibility: auto-destroy block device made with
+ * drive_init() when its guest device detaches */
+ if (blockdev_del_dinfo(bs)) {
+ bdrv_delete(bs);
+ }
+}
+
QemuOpts *drive_add(const char *file, const char *fmt, ...)
{
va_list ap;
diff --git a/blockdev.h b/blockdev.h
index 23ea576..8607086 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -1,8 +1,12 @@
/*
* QEMU host block devices
*
+ * Copyright (C) 2010 Red Hat Inc.
* Copyright (c) 2003-2008 Fabrice Bellard
*
+ * Authors:
+ * Markus Armbruster <armbru@redhat.com>,
+ *
* This work is licensed under the terms of the GNU GPL, version 2 or
* later. See the COPYING file in the top-level directory.
*/
@@ -13,6 +17,9 @@
#include "block.h"
#include "qemu-queue.h"
+int blockdev_attach(BlockDriverState *, DeviceState *);
+void blockdev_detach(BlockDriverState *, DeviceState *);
+
typedef enum {
IF_NONE,
IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] Re: [PATCH 07/13] blockdev: Means to destroy blockdev only if made with drive_init()
2010-06-02 16:55 ` [Qemu-devel] [PATCH 07/13] blockdev: Means to destroy blockdev only if made with drive_init() Markus Armbruster
@ 2010-06-10 14:19 ` Kevin Wolf
2010-06-10 16:00 ` Markus Armbruster
0 siblings, 1 reply; 24+ messages in thread
From: Kevin Wolf @ 2010-06-10 14:19 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel
Am 02.06.2010 18:55, schrieb Markus Armbruster:
> All drives are still made that way. They get destroyed along with
> their device. That's inappropriate for the alternative way to make
> blockdevs that will appear later in this series. These won't have a
> DriveInfo.
>
> blockdev_detach() destroys the blockdev only if it has a DriveInfo.
>
> blockdev_attach() does nothing for now. It'll be fleshed out later.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> blockdev.c | 35 +++++++++++++++++++++++++++++++++++
> blockdev.h | 7 +++++++
> 2 files changed, 42 insertions(+), 0 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index ace74e4..f90d4fc 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1,8 +1,12 @@
> /*
> * QEMU host block devices
> *
> + * Copyright (C) 2010 Red Hat Inc.
> * Copyright (c) 2003-2008 Fabrice Bellard
> *
> + * Authors:
> + * Markus Armbruster <armbru@redhat.com>,
> + *
> * This work is licensed under the terms of the GNU GPL, version 2 or
> * later. See the COPYING file in the top-level directory.
> */
> @@ -17,6 +21,37 @@
>
> static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
>
> +static int blockdev_del_dinfo(BlockDriverState *bs)
> +{
> + DriveInfo *dinfo, *next_dinfo;
> + int res = 0;
> +
> + QTAILQ_FOREACH_SAFE(dinfo, &drives, next, next_dinfo) {
> + if (dinfo->bdrv == bs) {
> + qemu_opts_del(dinfo->opts);
> + QTAILQ_REMOVE(&drives, dinfo, next);
> + qemu_free(dinfo);
> + res = 1;
> + }
> + }
> +
> + return res;
Can it happen that a BlockDriverState belongs to multiple DriveInfos? If
no, why not returning in the loop? Wouldn't need a FOREACH_SAFE then, too.
It's not worth respinning because of this one, but there were more
comments and I think you'll send a v2 for the actual -blockdev option
anyway once we have decided how to do it.
I have applied patches 1 to 6 now, and I think I could safely go on
until patch 9 if the minor improvements that were mentioned in comments
are made. I'd ignore patch 10 to 13 for now.
Is this what you would have expected or should I handle anything in a
different way?
Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread* [Qemu-devel] Re: [PATCH 07/13] blockdev: Means to destroy blockdev only if made with drive_init()
2010-06-10 14:19 ` [Qemu-devel] " Kevin Wolf
@ 2010-06-10 16:00 ` Markus Armbruster
0 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-10 16:00 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, kraxel
Kevin Wolf <kwolf@redhat.com> writes:
> Am 02.06.2010 18:55, schrieb Markus Armbruster:
>> All drives are still made that way. They get destroyed along with
>> their device. That's inappropriate for the alternative way to make
>> blockdevs that will appear later in this series. These won't have a
>> DriveInfo.
>>
>> blockdev_detach() destroys the blockdev only if it has a DriveInfo.
>>
>> blockdev_attach() does nothing for now. It'll be fleshed out later.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> blockdev.c | 35 +++++++++++++++++++++++++++++++++++
>> blockdev.h | 7 +++++++
>> 2 files changed, 42 insertions(+), 0 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index ace74e4..f90d4fc 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -1,8 +1,12 @@
>> /*
>> * QEMU host block devices
>> *
>> + * Copyright (C) 2010 Red Hat Inc.
>> * Copyright (c) 2003-2008 Fabrice Bellard
>> *
>> + * Authors:
>> + * Markus Armbruster <armbru@redhat.com>,
>> + *
>> * This work is licensed under the terms of the GNU GPL, version 2 or
>> * later. See the COPYING file in the top-level directory.
>> */
>> @@ -17,6 +21,37 @@
>>
>> static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
>>
>> +static int blockdev_del_dinfo(BlockDriverState *bs)
>> +{
>> + DriveInfo *dinfo, *next_dinfo;
>> + int res = 0;
>> +
>> + QTAILQ_FOREACH_SAFE(dinfo, &drives, next, next_dinfo) {
>> + if (dinfo->bdrv == bs) {
>> + qemu_opts_del(dinfo->opts);
>> + QTAILQ_REMOVE(&drives, dinfo, next);
>> + qemu_free(dinfo);
>> + res = 1;
>> + }
>> + }
>> +
>> + return res;
>
> Can it happen that a BlockDriverState belongs to multiple DriveInfos? If
> no, why not returning in the loop? Wouldn't need a FOREACH_SAFE then, too.
No, that shouldn't happen. Defensive coding, I don't want to leave
dinfos with dangling dinfo->bdrv around. Maybe I should put an
assert(!res) before the qemu_opts_del(). Or just forget about it, and
simplify like you suggest.
> It's not worth respinning because of this one, but there were more
> comments and I think you'll send a v2 for the actual -blockdev option
> anyway once we have decided how to do it.
>
> I have applied patches 1 to 6 now, and I think I could safely go on
> until patch 9 if the minor improvements that were mentioned in comments
> are made. I'd ignore patch 10 to 13 for now.
>
> Is this what you would have expected or should I handle anything in a
> different way?
No, that suits me fine. I definitely need to respin from part 8 on
(commit message too terse).
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 08/13] qdev: Decouple qdev_prop_drive from DriveInfo
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (6 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 07/13] blockdev: Means to destroy blockdev only if made with drive_init() Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 19:28 ` [Qemu-devel] " Gerd Hoffmann
2010-06-02 16:55 ` [Qemu-devel] [PATCH 09/13] blockdev: drive_get_by_id() is no longer used, remove Markus Armbruster
` (4 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
Make the property point to BlockDriverState, cutting out the DriveInfo
middleman.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block_int.h | 6 ++----
hw/fdc.c | 22 ++++++++++------------
hw/ide/core.c | 14 +++++++-------
hw/ide/internal.h | 2 +-
hw/ide/qdev.c | 8 ++++----
hw/pci-hotplug.c | 4 ++--
hw/qdev-properties.c | 39 ++++++++++++++++++++++++++++++++-------
hw/qdev.h | 6 +++---
hw/s390-virtio.c | 2 +-
hw/scsi-bus.c | 8 ++++----
hw/scsi-disk.c | 16 ++++++++--------
hw/scsi-generic.c | 5 ++---
hw/scsi.h | 2 +-
hw/usb-msd.c | 10 +++++-----
hw/virtio-blk.c | 2 +-
hw/virtio-pci.c | 12 ++----------
16 files changed, 85 insertions(+), 73 deletions(-)
diff --git a/block_int.h b/block_int.h
index e3bfd19..545b494 100644
--- a/block_int.h
+++ b/block_int.h
@@ -210,10 +210,8 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size);
int is_windows_drive(const char *filename);
#endif
-struct DriveInfo;
-
typedef struct BlockConf {
- struct DriveInfo *dinfo;
+ BlockDriverState *bs;
uint16_t physical_block_size;
uint16_t logical_block_size;
uint16_t min_io_size;
@@ -232,7 +230,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
}
#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
- DEFINE_PROP_DRIVE("drive", _state, _conf.dinfo), \
+ DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \
DEFINE_PROP_UINT16("logical_block_size", _state, \
_conf.logical_block_size, 512), \
DEFINE_PROP_UINT16("physical_block_size", _state, \
diff --git a/hw/fdc.c b/hw/fdc.c
index b978957..2b3cbb9 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -80,7 +80,6 @@ typedef enum FDiskFlags {
} FDiskFlags;
typedef struct FDrive {
- DriveInfo *dinfo;
BlockDriverState *bs;
/* Drive status */
FDriveType drive;
@@ -100,7 +99,6 @@ typedef struct FDrive {
static void fd_init(FDrive *drv)
{
/* Drive */
- drv->bs = drv->dinfo ? drv->dinfo->bdrv : NULL;
drv->drive = FDRIVE_DRV_NONE;
drv->perpendicular = 0;
/* Disk */
@@ -1862,10 +1860,10 @@ FDCtrl *fdctrl_init_isa(DriveInfo **fds)
dev = isa_create("isa-fdc");
if (fds[0]) {
- qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
+ qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]->bdrv);
}
if (fds[1]) {
- qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
+ qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]->bdrv);
}
if (qdev_init(&dev->qdev) < 0)
return NULL;
@@ -1884,10 +1882,10 @@ FDCtrl *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
fdctrl = &sys->state;
fdctrl->dma_chann = dma_chann; /* FIXME */
if (fds[0]) {
- qdev_prop_set_drive(dev, "driveA", fds[0]);
+ qdev_prop_set_drive(dev, "driveA", fds[0]->bdrv);
}
if (fds[1]) {
- qdev_prop_set_drive(dev, "driveB", fds[1]);
+ qdev_prop_set_drive(dev, "driveB", fds[1]->bdrv);
}
qdev_init_nofail(dev);
sysbus_connect_irq(&sys->busdev, 0, irq);
@@ -1905,7 +1903,7 @@ FDCtrl *sun4m_fdctrl_init(qemu_irq irq, target_phys_addr_t io_base,
dev = qdev_create(NULL, "SUNW,fdtwo");
if (fds[0]) {
- qdev_prop_set_drive(dev, "drive", fds[0]);
+ qdev_prop_set_drive(dev, "drive", fds[0]->bdrv);
}
qdev_init_nofail(dev);
sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
@@ -2030,8 +2028,8 @@ static ISADeviceInfo isa_fdc_info = {
.qdev.vmsd = &vmstate_isa_fdc,
.qdev.reset = fdctrl_external_reset_isa,
.qdev.props = (Property[]) {
- DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].dinfo),
- DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].dinfo),
+ DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs),
+ DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs),
DEFINE_PROP_END_OF_LIST(),
},
};
@@ -2053,8 +2051,8 @@ static SysBusDeviceInfo sysbus_fdc_info = {
.qdev.vmsd = &vmstate_sysbus_fdc,
.qdev.reset = fdctrl_external_reset_sysbus,
.qdev.props = (Property[]) {
- DEFINE_PROP_DRIVE("driveA", FDCtrlSysBus, state.drives[0].dinfo),
- DEFINE_PROP_DRIVE("driveB", FDCtrlSysBus, state.drives[1].dinfo),
+ DEFINE_PROP_DRIVE("driveA", FDCtrlSysBus, state.drives[0].bs),
+ DEFINE_PROP_DRIVE("driveB", FDCtrlSysBus, state.drives[1].bs),
DEFINE_PROP_END_OF_LIST(),
},
};
@@ -2066,7 +2064,7 @@ static SysBusDeviceInfo sun4m_fdc_info = {
.qdev.vmsd = &vmstate_sysbus_fdc,
.qdev.reset = fdctrl_external_reset_sysbus,
.qdev.props = (Property[]) {
- DEFINE_PROP_DRIVE("drive", FDCtrlSysBus, state.drives[0].dinfo),
+ DEFINE_PROP_DRIVE("drive", FDCtrlSysBus, state.drives[0].bs),
DEFINE_PROP_END_OF_LIST(),
},
};
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0b3b7c2..a6ea70c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2594,15 +2594,15 @@ void ide_bus_reset(IDEBus *bus)
ide_clear_hob(bus);
}
-void ide_init_drive(IDEState *s, DriveInfo *dinfo,
+void ide_init_drive(IDEState *s, BlockDriverState *bs,
const char *version, const char *serial)
{
int cylinders, heads, secs;
uint64_t nb_sectors;
- s->bs = dinfo->bdrv;
- bdrv_get_geometry(s->bs, &nb_sectors);
- bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
+ s->bs = bs;
+ bdrv_get_geometry(bs, &nb_sectors);
+ bdrv_guess_geometry(bs, &cylinders, &heads, &secs);
s->cylinders = cylinders;
s->heads = heads;
s->sectors = secs;
@@ -2613,9 +2613,9 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo,
s->smart_autosave = 1;
s->smart_errors = 0;
s->smart_selftest_count = 0;
- if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
+ if (bdrv_get_type_hint(bs) == BDRV_TYPE_CDROM) {
s->is_cdrom = 1;
- bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
+ bdrv_set_change_cb(bs, cdrom_change_cb, s);
}
if (serial && *serial) {
strncpy(s->drive_serial_str, serial, sizeof(s->drive_serial_str));
@@ -2668,7 +2668,7 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
dinfo = i == 0 ? hd0 : hd1;
ide_init1(bus, i);
if (dinfo) {
- ide_init_drive(&bus->ifs[i], dinfo, NULL, dinfo->serial);
+ ide_init_drive(&bus->ifs[i], dinfo->bdrv, NULL, dinfo->serial);
} else {
ide_reset(&bus->ifs[i]);
}
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index eef1ee1..0125a9f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -555,7 +555,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
uint32_t ide_data_readl(void *opaque, uint32_t addr);
-void ide_init_drive(IDEState *s, DriveInfo *dinfo,
+void ide_init_drive(IDEState *s, BlockDriverState *bs,
const char *version, const char *serial);
void ide_init2(IDEBus *bus, qemu_irq irq);
void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 0f9f22e..aff9392 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -39,7 +39,7 @@ static int ide_qdev_init(DeviceState *qdev, DeviceInfo *base)
IDEDeviceInfo *info = DO_UPCAST(IDEDeviceInfo, qdev, base);
IDEBus *bus = DO_UPCAST(IDEBus, qbus, qdev->parent_bus);
- if (!dev->conf.dinfo) {
+ if (!dev->conf.bs) {
fprintf(stderr, "%s: no drive specified\n", qdev->info->name);
goto err;
}
@@ -83,7 +83,7 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
dev = qdev_create(&bus->qbus, "ide-drive");
qdev_prop_set_uint32(dev, "unit", unit);
- qdev_prop_set_drive(dev, "drive", drive);
+ qdev_prop_set_drive(dev, "drive", drive->bdrv);
if (qdev_init(dev) < 0)
return NULL;
return DO_UPCAST(IDEDevice, qdev, dev);
@@ -104,10 +104,10 @@ static int ide_drive_initfn(IDEDevice *dev)
serial = dev->serial;
if (!serial) {
/* try to fall back to value set with legacy -drive serial=... */
- serial = dev->conf.dinfo->serial;
+ serial = drive_get_serial(dev->conf.bs);
}
- ide_init_drive(s, dev->conf.dinfo, dev->version, serial);
+ ide_init_drive(s, dev->conf.bs, dev->version, serial);
if (!dev->version) {
dev->version = qemu_strdup(s->version);
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index c39e640..a27a67b 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -89,7 +89,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
* specified).
*/
dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
- scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo, dinfo->unit);
+ scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit);
dinfo->unit = scsidev->id;
if (printinfo)
@@ -211,7 +211,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
return NULL;
}
dev = pci_create(bus, devfn, "virtio-blk-pci");
- qdev_prop_set_drive(&dev->qdev, "drive", dinfo);
+ qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv);
if (qdev_init(&dev->qdev) < 0)
dev = NULL;
break;
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 5a8739d..45ce5c4 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -285,26 +285,41 @@ PropertyInfo qdev_prop_string = {
static int parse_drive(DeviceState *dev, Property *prop, const char *str)
{
- DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
+ BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
+ BlockDriverState *bs;
- *ptr = drive_get_by_id(str);
- if (*ptr == NULL)
+ bs = bdrv_find(str);
+ if (bs == NULL)
return -ENOENT;
+ if (blockdev_attach(bs, dev) < 0)
+ return -EEXIST;
+ *ptr = bs;
return 0;
}
+static void free_drive(DeviceState *dev, Property *prop)
+{
+ BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
+
+ if (*ptr) {
+ blockdev_detach(*ptr, dev);
+ }
+}
+
static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t len)
{
- DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
- return snprintf(dest, len, "%s", (*ptr) ? (*ptr)->id : "<null>");
+ BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
+ return snprintf(dest, len, "%s",
+ *ptr ? bdrv_get_device_name(*ptr) : "<null>");
}
PropertyInfo qdev_prop_drive = {
.name = "drive",
.type = PROP_TYPE_DRIVE,
- .size = sizeof(DriveInfo*),
+ .size = sizeof(BlockDriverState *),
.parse = parse_drive,
.print = print_drive,
+ .free = free_drive,
};
/* --- character device --- */
@@ -627,9 +642,19 @@ void qdev_prop_set_string(DeviceState *dev, const char *name, char *value)
qdev_prop_set(dev, name, &value, PROP_TYPE_STRING);
}
-void qdev_prop_set_drive(DeviceState *dev, const char *name, DriveInfo *value)
+void qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value)
{
+ int res;
+
qdev_prop_set(dev, name, &value, PROP_TYPE_DRIVE);
+ res = blockdev_attach(value, dev);
+ if (res < 0) {
+ error_report("Can't attach drive %s to %s.%s: %s",
+ bdrv_get_device_name(value),
+ dev->id ? dev->id : dev->info->name,
+ name, strerror(res));
+ exit(1);
+ }
}
void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value)
diff --git a/hw/qdev.h b/hw/qdev.h
index be5ad67..7a01a81 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -253,8 +253,8 @@ extern PropertyInfo qdev_prop_pci_devfn;
DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, VLANClientState*)
#define DEFINE_PROP_VLAN(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, VLANState*)
-#define DEFINE_PROP_DRIVE(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_drive, DriveInfo*)
+#define DEFINE_PROP_DRIVE(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
#define DEFINE_PROP_MACADDR(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
@@ -275,7 +275,7 @@ void qdev_prop_set_string(DeviceState *dev, const char *name, char *value);
void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value);
void qdev_prop_set_netdev(DeviceState *dev, const char *name, VLANClientState *value);
void qdev_prop_set_vlan(DeviceState *dev, const char *name, VLANState *value);
-void qdev_prop_set_drive(DeviceState *dev, const char *name, DriveInfo *value);
+void qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value);
void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
/* FIXME: Remove opaque pointer properties. */
void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 898f442..c7c3fc9 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -262,7 +262,7 @@ static void s390_init(ram_addr_t ram_size,
}
dev = qdev_create((BusState *)s390_bus, "virtio-blk-s390");
- qdev_prop_set_drive(dev, "drive", dinfo);
+ qdev_prop_set_drive(dev, "drive", dinfo->bdrv);
qdev_init_nofail(dev);
}
}
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 055a94d..782ec18 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -84,15 +84,15 @@ void scsi_qdev_register(SCSIDeviceInfo *info)
/* handle legacy '-drive if=scsi,...' cmd line args */
/* FIXME callers should check for failure, but don't */
-SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, DriveInfo *dinfo, int unit)
+SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, int unit)
{
const char *driver;
DeviceState *dev;
- driver = bdrv_is_sg(dinfo->bdrv) ? "scsi-generic" : "scsi-disk";
+ driver = bdrv_is_sg(bdrv) ? "scsi-generic" : "scsi-disk";
dev = qdev_create(&bus->qbus, driver);
qdev_prop_set_uint32(dev, "scsi-id", unit);
- qdev_prop_set_drive(dev, "drive", dinfo);
+ qdev_prop_set_drive(dev, "drive", bdrv);
if (qdev_init(dev) < 0)
return NULL;
return DO_UPCAST(SCSIDevice, qdev, dev);
@@ -108,7 +108,7 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
if (dinfo == NULL) {
continue;
}
- scsi_bus_legacy_add_drive(bus, dinfo, unit);
+ scsi_bus_legacy_add_drive(bus, dinfo->bdrv, unit);
}
}
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2b38984..46b526f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1043,26 +1043,26 @@ static void scsi_destroy(SCSIDevice *dev)
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
scsi_disk_purge_requests(s);
- drive_uninit(s->qdev.conf.dinfo);
}
static int scsi_disk_initfn(SCSIDevice *dev)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+ const char *serial;
- if (!s->qdev.conf.dinfo || !s->qdev.conf.dinfo->bdrv) {
+ if (!s->qdev.conf.bs) {
error_report("scsi-disk: drive property not set");
return -1;
}
- s->bs = s->qdev.conf.dinfo->bdrv;
+ s->bs = s->qdev.conf.bs;
if (!s->serial) {
- if (*dev->conf.dinfo->serial) {
- /* try to fall back to value set with legacy -drive serial=... */
- s->serial = qemu_strdup(dev->conf.dinfo->serial);
- } else {
- s->serial = qemu_strdup("0");
+ /* try to fall back to value set with legacy -drive serial=... */
+ serial = drive_get_serial(s->bs);
+ if (!*serial) {
+ serial = "0";
}
+ s->serial = qemu_strdup(serial);
}
if (!s->version) {
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e31060e..f283f6c 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -453,7 +453,6 @@ static void scsi_destroy(SCSIDevice *d)
r = DO_UPCAST(SCSIGenericReq, req, QTAILQ_FIRST(&s->qdev.requests));
scsi_remove_request(r);
}
- drive_uninit(s->qdev.conf.dinfo);
}
static int scsi_generic_initfn(SCSIDevice *dev)
@@ -462,11 +461,11 @@ static int scsi_generic_initfn(SCSIDevice *dev)
int sg_version;
struct sg_scsi_id scsiid;
- if (!s->qdev.conf.dinfo || !s->qdev.conf.dinfo->bdrv) {
+ if (!s->qdev.conf.bs) {
error_report("scsi-generic: drive property not set");
return -1;
}
- s->bs = s->qdev.conf.dinfo->bdrv;
+ s->bs = s->qdev.conf.bs;
/* check we are really using a /dev/sg* file */
if (!bdrv_is_sg(s->bs)) {
diff --git a/hw/scsi.h b/hw/scsi.h
index b668e27..8ca3c2d 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -97,7 +97,7 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
return DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
}
-SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, DriveInfo *dinfo, int unit);
+SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, int unit);
void scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
void scsi_dev_clear_sense(SCSIDevice *dev);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 003bd8a..6ed06ff 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -523,20 +523,20 @@ static int usb_msd_initfn(USBDevice *dev)
{
MSDState *s = DO_UPCAST(MSDState, dev, dev);
- if (!s->conf.dinfo || !s->conf.dinfo->bdrv) {
+ if (!s->conf.bs) {
error_report("usb-msd: drive property not set");
return -1;
}
s->dev.speed = USB_SPEED_FULL;
scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
- s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, s->conf.dinfo, 0);
+ s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, s->conf.bs, 0);
s->bus.qbus.allow_hotplug = 0;
usb_msd_handle_reset(dev);
- if (bdrv_key_required(s->conf.dinfo->bdrv)) {
+ if (bdrv_key_required(s->conf.bs)) {
if (cur_mon) {
- monitor_read_bdrv_key_start(cur_mon, s->conf.dinfo->bdrv,
+ monitor_read_bdrv_key_start(cur_mon, s->conf.bs,
usb_msd_password_cb, s);
s->dev.auto_attach = 0;
} else {
@@ -595,7 +595,7 @@ static USBDevice *usb_msd_init(const char *filename)
if (!dev) {
return NULL;
}
- qdev_prop_set_drive(&dev->qdev, "drive", dinfo);
+ qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv);
if (qdev_init(&dev->qdev) < 0)
return NULL;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index cb17afa..a680512 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -500,7 +500,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
s->vdev.get_config = virtio_blk_update_config;
s->vdev.get_features = virtio_blk_get_features;
s->vdev.reset = virtio_blk_reset;
- s->bs = conf->dinfo->bdrv;
+ s->bs = conf->bs;
s->conf = conf;
s->rq = NULL;
s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index e101fa0..63d55b8 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -547,7 +547,7 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev)
proxy->class_code != PCI_CLASS_STORAGE_OTHER)
proxy->class_code = PCI_CLASS_STORAGE_SCSI;
- if (!proxy->block.dinfo) {
+ if (!proxy->block.bs) {
error_report("virtio-blk-pci: drive property not set");
return -1;
}
@@ -567,14 +567,6 @@ static int virtio_exit_pci(PCIDevice *pci_dev)
return msix_uninit(pci_dev);
}
-static int virtio_blk_exit_pci(PCIDevice *pci_dev)
-{
- VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-
- drive_uninit(proxy->block.dinfo);
- return virtio_exit_pci(pci_dev);
-}
-
static int virtio_serial_init_pci(PCIDevice *pci_dev)
{
VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
@@ -663,7 +655,7 @@ static PCIDeviceInfo virtio_info[] = {
.qdev.name = "virtio-blk-pci",
.qdev.size = sizeof(VirtIOPCIProxy),
.init = virtio_blk_init_pci,
- .exit = virtio_blk_exit_pci,
+ .exit = virtio_exit_pci,
.qdev.props = (Property[]) {
DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, block),
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] Re: [PATCH 08/13] qdev: Decouple qdev_prop_drive from DriveInfo
2010-06-02 16:55 ` [Qemu-devel] [PATCH 08/13] qdev: Decouple qdev_prop_drive from DriveInfo Markus Armbruster
@ 2010-06-02 19:28 ` Gerd Hoffmann
2010-06-04 8:22 ` Markus Armbruster
0 siblings, 1 reply; 24+ messages in thread
From: Gerd Hoffmann @ 2010-06-02 19:28 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel
Hi,
> +static void free_drive(DeviceState *dev, Property *prop)
> +{
> + BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
> +
> + if (*ptr) {
> + blockdev_detach(*ptr, dev);
> + }
> +}
> @@ -1043,26 +1043,26 @@ static void scsi_destroy(SCSIDevice *dev)
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
>
> scsi_disk_purge_requests(s);
> - drive_uninit(s->qdev.conf.dinfo);
> }
Neat. Commit message should better explain that though.
cheers,
Gerd
^ permalink raw reply [flat|nested] 24+ messages in thread* [Qemu-devel] Re: [PATCH 08/13] qdev: Decouple qdev_prop_drive from DriveInfo
2010-06-02 19:28 ` [Qemu-devel] " Gerd Hoffmann
@ 2010-06-04 8:22 ` Markus Armbruster
0 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-04 8:22 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: kwolf, qemu-devel
Gerd Hoffmann <kraxel@redhat.com> writes:
> Hi,
>
>> +static void free_drive(DeviceState *dev, Property *prop)
>> +{
>> + BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
>> +
>> + if (*ptr) {
>> + blockdev_detach(*ptr, dev);
>> + }
>> +}
>
>> @@ -1043,26 +1043,26 @@ static void scsi_destroy(SCSIDevice *dev)
>> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
>>
>> scsi_disk_purge_requests(s);
>> - drive_uninit(s->qdev.conf.dinfo);
>> }
>
> Neat. Commit message should better explain that though.
Point. I'll try to improve it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 09/13] blockdev: drive_get_by_id() is no longer used, remove
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (7 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 08/13] qdev: Decouple qdev_prop_drive from DriveInfo Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 10/13] qemu-option: New qemu_opts_reset() Markus Armbruster
` (3 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 12 ------------
blockdev.h | 1 -
2 files changed, 0 insertions(+), 13 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index f90d4fc..8c51e6e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -87,18 +87,6 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit)
return NULL;
}
-DriveInfo *drive_get_by_id(const char *id)
-{
- DriveInfo *dinfo;
-
- QTAILQ_FOREACH(dinfo, &drives, next) {
- if (strcmp(id, dinfo->id))
- continue;
- return dinfo;
- }
- return NULL;
-}
-
int drive_get_max_bus(BlockInterfaceType type)
{
int max_bus;
diff --git a/blockdev.h b/blockdev.h
index 8607086..bb89bfa 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -44,7 +44,6 @@ typedef struct DriveInfo {
#define MAX_SCSI_DEVS 7
extern DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
-extern DriveInfo *drive_get_by_id(const char *id);
extern int drive_get_max_bus(BlockInterfaceType type);
extern void drive_uninit(DriveInfo *dinfo);
extern const char *drive_get_serial(BlockDriverState *bdrv);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 10/13] qemu-option: New qemu_opts_reset()
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (8 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 09/13] blockdev: drive_get_by_id() is no longer used, remove Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 11/13] qemu-option: New qemu_opt_next(), qemu_opt_name() Markus Armbruster
` (2 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-option.c | 9 +++++++++
qemu-option.h | 1 +
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index acd74f9..87e324c 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -698,6 +698,15 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exist
return opts;
}
+void qemu_opts_reset(QemuOptsList *list)
+{
+ QemuOpts *opts, *next_opts;
+
+ QTAILQ_FOREACH_SAFE(opts, &list->head, next, next_opts) {
+ qemu_opts_del(opts);
+ }
+}
+
int qemu_opts_set(QemuOptsList *list, const char *id,
const char *name, const char *value)
{
diff --git a/qemu-option.h b/qemu-option.h
index 4823219..9e2406c 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -115,6 +115,7 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists);
+void qemu_opts_reset(QemuOptsList *list);
int qemu_opts_set(QemuOptsList *list, const char *id,
const char *name, const char *value);
const char *qemu_opts_id(QemuOpts *opts);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 11/13] qemu-option: New qemu_opt_next(), qemu_opt_name()
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (9 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 10/13] qemu-option: New qemu_opts_reset() Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 12/13] blockdev: Factor option value parsers out of drive_init() Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device Markus Armbruster
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
This is a more flexible alternative to qemu_opt_foreach().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-option.c | 13 +++++++++++++
qemu-option.h | 4 ++++
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 87e324c..c9f8be4 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -642,6 +642,19 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
return 0;
}
+QemuOpt *qemu_opt_next(QemuOpts *opts, QemuOpt *opt)
+{
+ if (!opt) {
+ return QTAILQ_FIRST(&opts->head);
+ }
+ return QTAILQ_NEXT(opt, next);
+}
+
+const char *qemu_opt_name(QemuOpt *opt)
+{
+ return opt->name;
+}
+
int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
int abort_on_failure)
{
diff --git a/qemu-option.h b/qemu-option.h
index 9e2406c..13c6a9d 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -109,6 +109,10 @@ int qemu_opt_get_bool(QemuOpts *opts, const char *name, int defval);
uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
+
+QemuOpt *qemu_opt_next(QemuOpts *opts, QemuOpt *opt);
+const char *qemu_opt_name(QemuOpt *opt);
+
typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque);
int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
int abort_on_failure);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 12/13] blockdev: Factor option value parsers out of drive_init()
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (10 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 11/13] qemu-option: New qemu_opt_next(), qemu_opt_name() Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-02 16:55 ` [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device Markus Armbruster
12 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
The new functions use qerror_report() to make them usable from
QMP-enabled monitor commands. They'll appear in a future patch
series.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 130 +++++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 86 insertions(+), 44 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 8c51e6e..a1e6394 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -21,6 +21,83 @@
static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
+static BlockDriver *parse_block_format(const char *val)
+{
+ BlockDriver *drv = bdrv_find_whitelisted_format(val);
+ if (!drv) {
+ qerror_report(QERR_INVALID_PARAMETER_VALUE,
+ "format", "a block driver name");
+ return NULL;
+ }
+ return drv;
+}
+
+static int parse_block_error_action(const char *val, int is_read)
+{
+ if (!val) {
+ return is_read ? BLOCK_ERR_REPORT : BLOCK_ERR_STOP_ENOSPC;
+ } else if (!strcmp(val, "ignore")) {
+ return BLOCK_ERR_IGNORE;
+ } else if (!is_read && !strcmp(val, "enospc")) {
+ return BLOCK_ERR_STOP_ENOSPC;
+ } else if (!strcmp(val, "stop")) {
+ return BLOCK_ERR_STOP_ANY;
+ } else if (!strcmp(val, "report")) {
+ return BLOCK_ERR_REPORT;
+ } else {
+ if (is_read) {
+ qerror_report(QERR_INVALID_PARAMETER_VALUE, "rerror",
+ "ignore, report, stop");
+ } else {
+ qerror_report(QERR_INVALID_PARAMETER_VALUE, "werror",
+ "enospc, ignore, stop or report");
+ }
+ return -1;
+ }
+}
+
+static int parse_block_cache(const char *val)
+{
+ if (!val) {
+ return 0;
+ } else if (!strcmp(val, "off") || !strcmp(val, "none")) {
+ return BDRV_O_NOCACHE;
+ } else if (!strcmp(val, "writeback")) {
+ return BDRV_O_CACHE_WB;
+ } else if (!strcmp(val, "unsafe")) {
+ return BDRV_O_CACHE_WB;
+ return BDRV_O_NO_FLUSH;
+ } else if (!strcmp(val, "writethrough")) {
+ return 0;
+ } else {
+ qerror_report(QERR_INVALID_PARAMETER_VALUE, "cache",
+ "none, unsafe, writeback or writethrough");
+ return -1;
+ }
+}
+
+static int parse_block_aio(const char *val)
+{
+ if (!val) {
+ return 0;
+#ifdef CONFIG_LINUX_AIO
+ } else if (!strcmp(val, "native")) {
+ return BDRV_O_NATIVE_AIO;
+#endif
+ } else if (!strcmp(val, "threads")) {
+ return 0;
+ } else {
+ qerror_report(QERR_INVALID_PARAMETER_VALUE, "aio",
+#ifdef CONFIG_LINUX_AIO
+ "native or threads"
+#else
+ "threads"
+#endif
+ );
+ return -1;
+ }
+}
+
static int blockdev_del_dinfo(BlockDriverState *bs)
{
DriveInfo *dinfo, *next_dinfo;
@@ -126,23 +203,6 @@ void drive_uninit(DriveInfo *dinfo)
qemu_free(dinfo);
}
-static int parse_block_error_action(const char *buf, int is_read)
-{
- if (!strcmp(buf, "ignore")) {
- return BLOCK_ERR_IGNORE;
- } else if (!is_read && !strcmp(buf, "enospc")) {
- return BLOCK_ERR_STOP_ENOSPC;
- } else if (!strcmp(buf, "stop")) {
- return BLOCK_ERR_STOP_ANY;
- } else if (!strcmp(buf, "report")) {
- return BLOCK_ERR_REPORT;
- } else {
- fprintf(stderr, "qemu: '%s' invalid %s error action\n",
- buf, is_read ? "read" : "write");
- return -1;
- }
-}
-
DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
{
const char *buf;
@@ -280,34 +340,17 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
}
}
- if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
- if (!strcmp(buf, "off") || !strcmp(buf, "none")) {
- bdrv_flags |= BDRV_O_NOCACHE;
- } else if (!strcmp(buf, "writeback")) {
- bdrv_flags |= BDRV_O_CACHE_WB;
- } else if (!strcmp(buf, "unsafe")) {
- bdrv_flags |= BDRV_O_CACHE_WB;
- bdrv_flags |= BDRV_O_NO_FLUSH;
- } else if (!strcmp(buf, "writethrough")) {
- /* this is the default */
- } else {
- fprintf(stderr, "qemu: invalid cache option\n");
- return NULL;
- }
+ ret = parse_block_cache(qemu_opt_get(opts, "cache"));
+ if (ret < 0) {
+ return NULL;
}
+ bdrv_flags |= ret;
-#ifdef CONFIG_LINUX_AIO
- if ((buf = qemu_opt_get(opts, "aio")) != NULL) {
- if (!strcmp(buf, "native")) {
- bdrv_flags |= BDRV_O_NATIVE_AIO;
- } else if (!strcmp(buf, "threads")) {
- /* this is the default */
- } else {
- fprintf(stderr, "qemu: invalid aio option\n");
- return NULL;
- }
+ ret = parse_block_aio(qemu_opt_get(opts, "aio"));
+ if (ret < 0) {
+ return NULL;
}
-#endif
+ bdrv_flags |= ret;
if ((buf = qemu_opt_get(opts, "format")) != NULL) {
if (strcmp(buf, "?") == 0) {
@@ -316,9 +359,8 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
fprintf(stderr, "\n");
return NULL;
}
- drv = bdrv_find_whitelisted_format(buf);
+ drv = parse_block_format(buf);
if (!drv) {
- fprintf(stderr, "qemu: '%s' invalid format\n", buf);
return NULL;
}
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device
2010-06-02 16:55 [Qemu-devel] [PATCH 00/13] New -blockdev to define a host block device Markus Armbruster
` (11 preceding siblings ...)
2010-06-02 16:55 ` [Qemu-devel] [PATCH 12/13] blockdev: Factor option value parsers out of drive_init() Markus Armbruster
@ 2010-06-02 16:55 ` Markus Armbruster
2010-06-03 8:00 ` Christoph Hellwig
` (2 more replies)
12 siblings, 3 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-02 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel
Existing -drive defines both host and guest part. To make it work
with -device, we created if=none. But all this does is peel off guest
device selection. The other guest properties such as geometry,
removable vs. fixed media, and serial number are still in the wrong
place.
Instead of overloading -drive even further, create a new, clean option
to define a host block device. -drive stays around unchanged for
command line convenience and backwards compatibility.
This is just a first step. Future work includes:
* A set of monitor commands to go with it.
* Let device model declare media removable. -drive has that in the
host part, as media=(cdrom|floppy), but it really belongs to the
guest part.
* Turn geometry into device properties. This will also ensure proper
range checking. The existing range checking for -drive can't work
with if=none.
* Make device models reject error actions they don't support. The
existing check for -drive can't work with if=none.
* Like -drive, -blockdev ignores cache= silently when snapshot=on. Do
we really want that?
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++----
blockdev.h | 2 +
qemu-config.c | 38 +++++++++++++++
qemu-config.h | 1 +
qemu-options.hx | 49 +++++++++++++++++++
vl.c | 29 ++++++++++-
6 files changed, 246 insertions(+), 14 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a1e6394..e03ecfc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -98,6 +98,132 @@ static int parse_block_aio(const char *val)
}
}
+static int blockdev_insert(BlockDriverState *bs, QemuOpts *opts)
+{
+ int snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
+ const char *file = qemu_opt_get(opts, "file");
+ const char *cache = qemu_opt_get(opts, "cache");
+ const char *aio = qemu_opt_get(opts, "aio");
+ const char *format = qemu_opt_get(opts, "format");
+ const char *rerror = qemu_opt_get(opts, "rerror");
+ const char *werror = qemu_opt_get(opts, "werror");
+ int readonly = qemu_opt_get_bool(opts, "readonly", 0);
+ BlockDriver *drv;
+ int res, flags;
+ BlockErrorAction on_read_error, on_write_error;
+
+ if (!file) {
+ qerror_report(QERR_MISSING_PARAMETER, "file");
+ return -1;
+ }
+
+ drv = NULL;
+ if (format) {
+ drv = parse_block_format(format);
+ if (!drv) {
+ return -1;
+ }
+ }
+
+ res = parse_block_error_action(rerror, 1);
+ if (on_read_error < 0) {
+ return -1;
+ }
+ on_read_error = res;
+ res = parse_block_error_action(werror, 0);
+ if (res < 0) {
+ return -1;
+ }
+ on_write_error = res;
+
+ flags = 0;
+ res = parse_block_cache(cache);
+ if (res < 0) {
+ return -1;
+ }
+ flags |= res;
+ if (snapshot) {
+ /* always use write-back with snapshot */
+ /* FIXME ignores explicit cache= *silently*; really want that? */
+ flags &= ~BDRV_O_CACHE_MASK;
+ flags |= (BDRV_O_SNAPSHOT | BDRV_O_CACHE_WB);
+ flags |= BDRV_O_SNAPSHOT;
+ }
+ res = parse_block_aio(aio);
+ if (res < 0) {
+ return -1;
+ }
+ flags |= res;
+ flags |= readonly ? 0 : BDRV_O_RDWR;
+
+ bdrv_set_on_error(bs, on_read_error, on_write_error);
+ res = bdrv_open(bs, file, flags, drv);
+ if (res < 0) {
+ qerror_report(QERR_OPEN_FILE_FAILED, file);
+ bdrv_close(bs);
+ return -1;
+ }
+ return 0;
+}
+
+BlockDriverState *blockdev_open(QemuOpts *opts)
+{
+ const char *id = qemu_opts_id(opts);
+ const char *file = qemu_opt_get(opts, "file");
+ BlockDriverState *bs;
+ QemuOpt *opt;
+ const char *name;
+
+ if (!id) {
+ qerror_report(QERR_MISSING_PARAMETER, "id");
+ return NULL;
+ }
+
+ bs = bdrv_find(id);
+ if (bs) {
+ qerror_report(QERR_DUPLICATE_ID, id, "blockdev");
+ return NULL;
+ }
+ bs = bdrv_new(id);
+
+ if (!file) {
+ /* file is optional only if no other options are present; check */
+ opt = NULL;
+ while ((opt = qemu_opt_next(opts, opt))) {
+ name = qemu_opt_name(opt);
+ if (strcmp(name, "file")) {
+ qerror_report(QERR_MISSING_PARAMETER, "file");
+ return NULL;
+ }
+ }
+ /* block device without media wanted */
+ return bs;
+ }
+
+ if (blockdev_insert(bs, opts) < 0) {
+ return NULL;
+ }
+ return bs;
+}
+
+static void blockdev_format_help_iter(void *opaque, const char *name)
+{
+ error_printf(" %s", name);
+}
+
+int blockdev_format_help(QemuOpts *opts)
+{
+ const char *format = qemu_opt_get(opts, "format");
+
+ if (format && !strcmp(format, "?")) {
+ error_printf("Supported block device formats:");
+ bdrv_iterate_format(blockdev_format_help_iter, NULL);
+ error_printf("\n");
+ return 1;
+ }
+ return 0;
+}
+
static int blockdev_del_dinfo(BlockDriverState *bs)
{
DriveInfo *dinfo, *next_dinfo;
@@ -190,11 +316,6 @@ const char *drive_get_serial(BlockDriverState *bdrv)
return "\0";
}
-static void bdrv_format_print(void *opaque, const char *name)
-{
- fprintf(stderr, " %s", name);
-}
-
void drive_uninit(DriveInfo *dinfo)
{
qemu_opts_del(dinfo->opts);
@@ -227,6 +348,10 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
*fatal_error = 1;
+ if (blockdev_format_help(opts)) {
+ return NULL;
+ }
+
translation = BIOS_ATA_TRANSLATION_AUTO;
if (default_to_scsi) {
@@ -353,12 +478,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
bdrv_flags |= ret;
if ((buf = qemu_opt_get(opts, "format")) != NULL) {
- if (strcmp(buf, "?") == 0) {
- fprintf(stderr, "qemu: Supported formats:");
- bdrv_iterate_format(bdrv_format_print, NULL);
- fprintf(stderr, "\n");
- return NULL;
- }
drv = parse_block_format(buf);
if (!drv) {
return NULL;
diff --git a/blockdev.h b/blockdev.h
index bb89bfa..564c64d 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -17,6 +17,8 @@
#include "block.h"
#include "qemu-queue.h"
+int blockdev_format_help(QemuOpts *);
+BlockDriverState *blockdev_open(QemuOpts *);
int blockdev_attach(BlockDriverState *, DeviceState *);
void blockdev_detach(BlockDriverState *, DeviceState *);
diff --git a/qemu-config.c b/qemu-config.c
index 5a4e61b..2c3814b 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -84,6 +84,43 @@ QemuOptsList qemu_drive_opts = {
},
};
+QemuOptsList qemu_blockdev_opts = {
+ .name = "blockdev",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_blockdev_opts.head),
+ .desc = {
+ {
+ .name = "snapshot",
+ .type = QEMU_OPT_BOOL,
+ },{
+ .name = "file",
+ .type = QEMU_OPT_STRING,
+ .help = "disk image",
+ },{
+ .name = "cache",
+ .type = QEMU_OPT_STRING,
+ .help = "host cache usage (none, writeback, writethrough, unsafe)",
+ },{
+ .name = "aio",
+ .type = QEMU_OPT_STRING,
+ .help = "host AIO implementation (threads, native)",
+ },{
+ .name = "format",
+ .type = QEMU_OPT_STRING,
+ .help = "disk format (raw, qcow2, ...)",
+ },{
+ .name = "rerror",
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = "werror",
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = "readonly",
+ .type = QEMU_OPT_BOOL,
+ },
+ { /* end if list */ }
+ },
+};
+
QemuOptsList qemu_chardev_opts = {
.name = "chardev",
.implied_opt_name = "backend",
@@ -338,6 +375,7 @@ QemuOptsList qemu_cpudef_opts = {
static QemuOptsList *vm_config_groups[] = {
&qemu_drive_opts,
+ &qemu_blockdev_opts,
&qemu_chardev_opts,
&qemu_device_opts,
&qemu_netdev_opts,
diff --git a/qemu-config.h b/qemu-config.h
index dca69d4..e6214d6 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -2,6 +2,7 @@
#define QEMU_CONFIG_H
extern QemuOptsList qemu_drive_opts;
+extern QemuOptsList qemu_blockdev_opts;
extern QemuOptsList qemu_chardev_opts;
#ifdef CONFIG_LINUX
extern QemuOptsList qemu_fsdev_opts;
diff --git a/qemu-options.hx b/qemu-options.hx
index a6928b7..38d0573 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1477,6 +1477,55 @@ ETEXI
DEFHEADING()
+DEFHEADING(Block device options:)
+
+DEF("blockdev", HAS_ARG, QEMU_OPTION_blockdev,
+ "-blockdev id=id[,file=file][,format=format][,snapshot=on|off]\n"
+ " [,cache=writethrough|writeback|unsafe|none][,aio=threads|native]\n"
+ " [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]\n"
+ " [,readonly=on|off]\n"
+ " define a host block device\n", QEMU_ARCH_ALL)
+STEXI
+
+The general form of a block device option is:
+
+@table @option
+
+@item -blockdev id=@var{id}[,@var{options}]
+@findex -blockdev
+
+All block devices must have an id. It is used to uniquely identify
+this device in other command line directives.
+
+Available options are:
+
+@table @option
+@item format=@var{format}
+This specifies the disk format to use. If left out, QEMU will attempt
+to determine it automatically. Interpreting an untrusted format
+header is insecure. Use @option{-blockdev format=?} to list available
+formats.
+@item file=@var{file}
+This option defines which disk image (@pxref{disk_images}) to use as
+media for this block device. You can define a block device without
+media.
+@item snapshot=on|off
+Whether to snapshot this block device (see @option{-snapshot}).
+@item cache=none|writeback|unsafe|writethrough
+Control use of host page cache.
+@item aio=threads|native
+Specify whether to use pthread based disk I/O or native Linux AIO.
+@item rerror=ignore|report|stop
+What to do on read error.
+@item werror=enospc|ignore|report|stop
+What to do on write error.
+@item readonly=on|off
+@end table
+@end table
+ETEXI
+
+DEFHEADING()
+
DEFHEADING(Bluetooth(R) options:)
DEF("bt", HAS_ARG, QEMU_OPTION_bt, \
diff --git a/vl.c b/vl.c
index 0a9862f..5ee6024 100644
--- a/vl.c
+++ b/vl.c
@@ -662,7 +662,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque)
return 0;
}
-static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
+static int opts_enable_snapshot(QemuOpts *opts, void *opaque)
{
if (NULL == qemu_opt_get(opts, "snapshot")) {
qemu_opt_set(opts, "snapshot", "on");
@@ -1806,6 +1806,17 @@ static int device_init_func(QemuOpts *opts, void *opaque)
return 0;
}
+static int blockdev_init_func(QemuOpts *opts, void *opaque)
+{
+ if (blockdev_format_help(opts)) {
+ exit(0);
+ }
+ if (!blockdev_open(opts)) {
+ return -1;
+ }
+ return 0;
+}
+
static int chardev_init_func(QemuOpts *opts, void *opaque)
{
CharDriverState *chr;
@@ -2257,6 +2268,12 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_drive:
drive_add(NULL, "%s", optarg);
break;
+ case QEMU_OPTION_blockdev:
+ opts = qemu_opts_parse(&qemu_blockdev_opts, optarg, 0);
+ if (!opts) {
+ exit(1);
+ }
+ break;
case QEMU_OPTION_set:
if (qemu_set_option(optarg) != 0)
exit(1);
@@ -3156,8 +3173,14 @@ int main(int argc, char **argv, char **envp)
}
/* open the virtual block devices */
- if (snapshot)
- qemu_opts_foreach(&qemu_drive_opts, drive_enable_snapshot, NULL, 0);
+ if (snapshot) {
+ qemu_opts_foreach(&qemu_blockdev_opts, opts_enable_snapshot, NULL, 0);
+ qemu_opts_foreach(&qemu_drive_opts, opts_enable_snapshot, NULL, 0);
+ }
+ if (qemu_opts_foreach(&qemu_blockdev_opts, blockdev_init_func, NULL, 1)) {
+ exit(1);
+ }
+ qemu_opts_reset(&qemu_blockdev_opts);
if (qemu_opts_foreach(&qemu_drive_opts, drive_init_func, &machine->use_scsi, 1) != 0)
exit(1);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device
2010-06-02 16:55 ` [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device Markus Armbruster
@ 2010-06-03 8:00 ` Christoph Hellwig
2010-06-04 8:23 ` Markus Armbruster
2010-06-10 15:32 ` [Qemu-devel] " Paolo Bonzini
2010-06-14 14:46 ` [Qemu-devel] " Anthony Liguori
2 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2010-06-03 8:00 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, kraxel
On Wed, Jun 02, 2010 at 06:55:29PM +0200, Markus Armbruster wrote:
> Existing -drive defines both host and guest part. To make it work
> with -device, we created if=none. But all this does is peel off guest
> device selection. The other guest properties such as geometry,
> removable vs. fixed media, and serial number are still in the wrong
> place.
>
> Instead of overloading -drive even further, create a new, clean option
> to define a host block device. -drive stays around unchanged for
> command line convenience and backwards compatibility.
>
> This is just a first step. Future work includes:
One thing we really needs is a protocol option. The current colon
syntax means we can't support filenames with colons in them which
users keep requesting. By making the protocol a separate option
we can sort this out.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device
2010-06-03 8:00 ` Christoph Hellwig
@ 2010-06-04 8:23 ` Markus Armbruster
0 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2010-06-04 8:23 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: kwolf, qemu-devel, kraxel
Christoph Hellwig <hch@lst.de> writes:
> On Wed, Jun 02, 2010 at 06:55:29PM +0200, Markus Armbruster wrote:
>> Existing -drive defines both host and guest part. To make it work
>> with -device, we created if=none. But all this does is peel off guest
>> device selection. The other guest properties such as geometry,
>> removable vs. fixed media, and serial number are still in the wrong
>> place.
>>
>> Instead of overloading -drive even further, create a new, clean option
>> to define a host block device. -drive stays around unchanged for
>> command line convenience and backwards compatibility.
>>
>> This is just a first step. Future work includes:
>
> One thing we really needs is a protocol option. The current colon
> syntax means we can't support filenames with colons in them which
> users keep requesting. By making the protocol a separate option
> we can sort this out.
You're absolutely right. I'll look into it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] Re: [PATCH 13/13] blockdev: New -blockdev to define a host block device
2010-06-02 16:55 ` [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device Markus Armbruster
2010-06-03 8:00 ` Christoph Hellwig
@ 2010-06-10 15:32 ` Paolo Bonzini
2010-06-10 16:32 ` Markus Armbruster
2010-06-14 14:46 ` [Qemu-devel] " Anthony Liguori
2 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2010-06-10 15:32 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, kraxel
On 06/02/2010 06:55 PM, Markus Armbruster wrote:
> * Like -drive, -blockdev ignores cache= silently when snapshot=on. Do
> we really want that?
Yes, the changes are throw-away by definition. Might as well use
cache=unsafe.
> + if (snapshot) {
> + /* always use write-back with snapshot */
> + /* FIXME ignores explicit cache= *silently*; really want that? */
> + flags &= ~BDRV_O_CACHE_MASK;
> + flags |= (BDRV_O_SNAPSHOT | BDRV_O_CACHE_WB);
> + flags |= BDRV_O_SNAPSHOT;
Cut and paste?
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread* [Qemu-devel] Re: [PATCH 13/13] blockdev: New -blockdev to define a host block device
2010-06-10 15:32 ` [Qemu-devel] " Paolo Bonzini
@ 2010-06-10 16:32 ` Markus Armbruster
2010-06-10 17:03 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2010-06-10 16:32 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, qemu-devel, kraxel
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 06/02/2010 06:55 PM, Markus Armbruster wrote:
>> * Like -drive, -blockdev ignores cache= silently when snapshot=on. Do
>> we really want that?
>
> Yes, the changes are throw-away by definition. Might as well use
> cache=unsafe.
I understand why that's the most sensible cache setting. But if the
user explicitly asks for something else, I think we better give it to
him, or tell him no. Ignoring him silently isn't nice.
>> + if (snapshot) {
>> + /* always use write-back with snapshot */
>> + /* FIXME ignores explicit cache= *silently*; really want that? */
>> + flags &= ~BDRV_O_CACHE_MASK;
>> + flags |= (BDRV_O_SNAPSHOT | BDRV_O_CACHE_WB);
>> + flags |= BDRV_O_SNAPSHOT;
>
> Cut and paste?
Pasto indeed, will fix. Thanks!
^ permalink raw reply [flat|nested] 24+ messages in thread* [Qemu-devel] Re: [PATCH 13/13] blockdev: New -blockdev to define a host block device
2010-06-10 16:32 ` Markus Armbruster
@ 2010-06-10 17:03 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2010-06-10 17:03 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, kraxel
On 06/10/2010 06:32 PM, Markus Armbruster wrote:
> I understand why that's the most sensible cache setting. But if the
> user explicitly asks for something else, I think we better give it to
> him, or tell him no. Ignoring him silently isn't nice.
Ah, it's clearer now...
I guess one could use cache=something together with snapshot to do
benchmarking. Actually the same changes in behavior (unsafe as default,
but observe a non-default value) can be done to -drive ...,snapshot too.
Maybe I'll give it a shot.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device
2010-06-02 16:55 ` [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device Markus Armbruster
2010-06-03 8:00 ` Christoph Hellwig
2010-06-10 15:32 ` [Qemu-devel] " Paolo Bonzini
@ 2010-06-14 14:46 ` Anthony Liguori
2 siblings, 0 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-06-14 14:46 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, qemu-devel, kraxel
On 06/02/2010 11:55 AM, Markus Armbruster wrote:
> Existing -drive defines both host and guest part. To make it work
> with -device, we created if=none. But all this does is peel off guest
> device selection. The other guest properties such as geometry,
> removable vs. fixed media, and serial number are still in the wrong
> place.
>
> Instead of overloading -drive even further, create a new, clean option
> to define a host block device. -drive stays around unchanged for
> command line convenience and backwards compatibility.
>
> This is just a first step. Future work includes:
>
> * A set of monitor commands to go with it.
>
> * Let device model declare media removable. -drive has that in the
> host part, as media=(cdrom|floppy), but it really belongs to the
> guest part.
>
> * Turn geometry into device properties. This will also ensure proper
> range checking. The existing range checking for -drive can't work
> with if=none.
>
> * Make device models reject error actions they don't support. The
> existing check for -drive can't work with if=none.
>
> * Like -drive, -blockdev ignores cache= silently when snapshot=on. Do
> we really want that?
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
> blockdev.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> blockdev.h | 2 +
> qemu-config.c | 38 +++++++++++++++
> qemu-config.h | 1 +
> qemu-options.hx | 49 +++++++++++++++++++
> vl.c | 29 ++++++++++-
> 6 files changed, 246 insertions(+), 14 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index a1e6394..e03ecfc 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -98,6 +98,132 @@ static int parse_block_aio(const char *val)
> }
> }
>
> +static int blockdev_insert(BlockDriverState *bs, QemuOpts *opts)
> +{
> + int snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
> + const char *file = qemu_opt_get(opts, "file");
> + const char *cache = qemu_opt_get(opts, "cache");
> + const char *aio = qemu_opt_get(opts, "aio");
> + const char *format = qemu_opt_get(opts, "format");
> + const char *rerror = qemu_opt_get(opts, "rerror");
> + const char *werror = qemu_opt_get(opts, "werror");
> + int readonly = qemu_opt_get_bool(opts, "readonly", 0);
> + BlockDriver *drv;
> + int res, flags;
> + BlockErrorAction on_read_error, on_write_error;
> +
> + if (!file) {
> + qerror_report(QERR_MISSING_PARAMETER, "file");
> + return -1;
> + }
> +
> + drv = NULL;
> + if (format) {
> + drv = parse_block_format(format);
> + if (!drv) {
> + return -1;
> + }
> + }
> +
> + res = parse_block_error_action(rerror, 1);
> + if (on_read_error< 0) {
> + return -1;
> + }
> + on_read_error = res;
> + res = parse_block_error_action(werror, 0);
> + if (res< 0) {
> + return -1;
> + }
> + on_write_error = res;
> +
> + flags = 0;
> + res = parse_block_cache(cache);
> + if (res< 0) {
> + return -1;
> + }
> + flags |= res;
> + if (snapshot) {
> + /* always use write-back with snapshot */
> + /* FIXME ignores explicit cache= *silently*; really want that? */
> + flags&= ~BDRV_O_CACHE_MASK;
> + flags |= (BDRV_O_SNAPSHOT | BDRV_O_CACHE_WB);
> + flags |= BDRV_O_SNAPSHOT;
> + }
> + res = parse_block_aio(aio);
> + if (res< 0) {
> + return -1;
> + }
> + flags |= res;
> + flags |= readonly ? 0 : BDRV_O_RDWR;
> +
> + bdrv_set_on_error(bs, on_read_error, on_write_error);
> + res = bdrv_open(bs, file, flags, drv);
> + if (res< 0) {
> + qerror_report(QERR_OPEN_FILE_FAILED, file);
> + bdrv_close(bs);
> + return -1;
> + }
> + return 0;
> +}
> +
> +BlockDriverState *blockdev_open(QemuOpts *opts)
> +{
> + const char *id = qemu_opts_id(opts);
> + const char *file = qemu_opt_get(opts, "file");
> + BlockDriverState *bs;
> + QemuOpt *opt;
> + const char *name;
> +
> + if (!id) {
> + qerror_report(QERR_MISSING_PARAMETER, "id");
> + return NULL;
> + }
> +
> + bs = bdrv_find(id);
> + if (bs) {
> + qerror_report(QERR_DUPLICATE_ID, id, "blockdev");
> + return NULL;
> + }
> + bs = bdrv_new(id);
> +
> + if (!file) {
> + /* file is optional only if no other options are present; check */
> + opt = NULL;
> + while ((opt = qemu_opt_next(opts, opt))) {
> + name = qemu_opt_name(opt);
> + if (strcmp(name, "file")) {
> + qerror_report(QERR_MISSING_PARAMETER, "file");
> + return NULL;
> + }
> + }
> + /* block device without media wanted */
> + return bs;
> + }
> +
> + if (blockdev_insert(bs, opts)< 0) {
> + return NULL;
> + }
> + return bs;
> +}
> +
> +static void blockdev_format_help_iter(void *opaque, const char *name)
> +{
> + error_printf(" %s", name);
> +}
> +
> +int blockdev_format_help(QemuOpts *opts)
> +{
> + const char *format = qemu_opt_get(opts, "format");
> +
> + if (format&& !strcmp(format, "?")) {
> + error_printf("Supported block device formats:");
> + bdrv_iterate_format(blockdev_format_help_iter, NULL);
> + error_printf("\n");
> + return 1;
> + }
> + return 0;
> +}
> +
> static int blockdev_del_dinfo(BlockDriverState *bs)
> {
> DriveInfo *dinfo, *next_dinfo;
> @@ -190,11 +316,6 @@ const char *drive_get_serial(BlockDriverState *bdrv)
> return "\0";
> }
>
> -static void bdrv_format_print(void *opaque, const char *name)
> -{
> - fprintf(stderr, " %s", name);
> -}
> -
> void drive_uninit(DriveInfo *dinfo)
> {
> qemu_opts_del(dinfo->opts);
> @@ -227,6 +348,10 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
>
> *fatal_error = 1;
>
> + if (blockdev_format_help(opts)) {
> + return NULL;
> + }
> +
> translation = BIOS_ATA_TRANSLATION_AUTO;
>
> if (default_to_scsi) {
> @@ -353,12 +478,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
> bdrv_flags |= ret;
>
> if ((buf = qemu_opt_get(opts, "format")) != NULL) {
> - if (strcmp(buf, "?") == 0) {
> - fprintf(stderr, "qemu: Supported formats:");
> - bdrv_iterate_format(bdrv_format_print, NULL);
> - fprintf(stderr, "\n");
> - return NULL;
> - }
> drv = parse_block_format(buf);
> if (!drv) {
> return NULL;
> diff --git a/blockdev.h b/blockdev.h
> index bb89bfa..564c64d 100644
> --- a/blockdev.h
> +++ b/blockdev.h
> @@ -17,6 +17,8 @@
> #include "block.h"
> #include "qemu-queue.h"
>
> +int blockdev_format_help(QemuOpts *);
> +BlockDriverState *blockdev_open(QemuOpts *);
> int blockdev_attach(BlockDriverState *, DeviceState *);
> void blockdev_detach(BlockDriverState *, DeviceState *);
>
> diff --git a/qemu-config.c b/qemu-config.c
> index 5a4e61b..2c3814b 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -84,6 +84,43 @@ QemuOptsList qemu_drive_opts = {
> },
> };
>
> +QemuOptsList qemu_blockdev_opts = {
> + .name = "blockdev",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_blockdev_opts.head),
> + .desc = {
> + {
> + .name = "snapshot",
> + .type = QEMU_OPT_BOOL,
> + },{
> + .name = "file",
> + .type = QEMU_OPT_STRING,
> + .help = "disk image",
> + },{
> + .name = "cache",
> + .type = QEMU_OPT_STRING,
> + .help = "host cache usage (none, writeback, writethrough, unsafe)",
> + },{
> + .name = "aio",
> + .type = QEMU_OPT_STRING,
> + .help = "host AIO implementation (threads, native)",
> + },{
> + .name = "format",
> + .type = QEMU_OPT_STRING,
> + .help = "disk format (raw, qcow2, ...)",
> + },{
> + .name = "rerror",
> + .type = QEMU_OPT_STRING,
> + },{
> + .name = "werror",
> + .type = QEMU_OPT_STRING,
> + },{
> + .name = "readonly",
> + .type = QEMU_OPT_BOOL,
> + },
> + { /* end if list */ }
> + },
> +};
> +
> QemuOptsList qemu_chardev_opts = {
> .name = "chardev",
> .implied_opt_name = "backend",
> @@ -338,6 +375,7 @@ QemuOptsList qemu_cpudef_opts = {
>
> static QemuOptsList *vm_config_groups[] = {
> &qemu_drive_opts,
> +&qemu_blockdev_opts,
> &qemu_chardev_opts,
> &qemu_device_opts,
> &qemu_netdev_opts,
> diff --git a/qemu-config.h b/qemu-config.h
> index dca69d4..e6214d6 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -2,6 +2,7 @@
> #define QEMU_CONFIG_H
>
> extern QemuOptsList qemu_drive_opts;
> +extern QemuOptsList qemu_blockdev_opts;
> extern QemuOptsList qemu_chardev_opts;
> #ifdef CONFIG_LINUX
> extern QemuOptsList qemu_fsdev_opts;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index a6928b7..38d0573 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1477,6 +1477,55 @@ ETEXI
>
> DEFHEADING()
>
> +DEFHEADING(Block device options:)
> +
> +DEF("blockdev", HAS_ARG, QEMU_OPTION_blockdev,
> + "-blockdev id=id[,file=file][,format=format][,snapshot=on|off]\n"
> + " [,cache=writethrough|writeback|unsafe|none][,aio=threads|native]\n"
> + " [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]\n"
> + " [,readonly=on|off]\n"
> + " define a host block device\n", QEMU_ARCH_ALL)
> +STEXI
> +
> +The general form of a block device option is:
> +
> +@table @option
> +
> +@item -blockdev id=@var{id}[,@var{options}]
> +@findex -blockdev
> +
> +All block devices must have an id. It is used to uniquely identify
> +this device in other command line directives.
> +
> +Available options are:
> +
> +@table @option
> +@item format=@var{format}
> +This specifies the disk format to use. If left out, QEMU will attempt
> +to determine it automatically. Interpreting an untrusted format
> +header is insecure. Use @option{-blockdev format=?} to list available
> +formats.
> +@item file=@var{file}
> +This option defines which disk image (@pxref{disk_images}) to use as
> +media for this block device. You can define a block device without
> +media.
> +@item snapshot=on|off
> +Whether to snapshot this block device (see @option{-snapshot}).
> +@item cache=none|writeback|unsafe|writethrough
> +Control use of host page cache.
> +@item aio=threads|native
> +Specify whether to use pthread based disk I/O or native Linux AIO.
> +@item rerror=ignore|report|stop
> +What to do on read error.
> +@item werror=enospc|ignore|report|stop
> +What to do on write error.
> +@item readonly=on|off
> +@end table
> +@end table
> +ETEXI
> +
> +DEFHEADING()
> +
> DEFHEADING(Bluetooth(R) options:)
>
> DEF("bt", HAS_ARG, QEMU_OPTION_bt, \
> diff --git a/vl.c b/vl.c
> index 0a9862f..5ee6024 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -662,7 +662,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque)
> return 0;
> }
>
> -static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
> +static int opts_enable_snapshot(QemuOpts *opts, void *opaque)
> {
> if (NULL == qemu_opt_get(opts, "snapshot")) {
> qemu_opt_set(opts, "snapshot", "on");
> @@ -1806,6 +1806,17 @@ static int device_init_func(QemuOpts *opts, void *opaque)
> return 0;
> }
>
> +static int blockdev_init_func(QemuOpts *opts, void *opaque)
> +{
> + if (blockdev_format_help(opts)) {
> + exit(0);
> + }
> + if (!blockdev_open(opts)) {
> + return -1;
> + }
> + return 0;
> +}
> +
> static int chardev_init_func(QemuOpts *opts, void *opaque)
> {
> CharDriverState *chr;
> @@ -2257,6 +2268,12 @@ int main(int argc, char **argv, char **envp)
> case QEMU_OPTION_drive:
> drive_add(NULL, "%s", optarg);
> break;
> + case QEMU_OPTION_blockdev:
> + opts = qemu_opts_parse(&qemu_blockdev_opts, optarg, 0);
> + if (!opts) {
> + exit(1);
> + }
> + break;
>
A good test of whether blockdev is an adequate replacement for -drive is
to implement -drive in terms of -blockdev.
Of course, it's not clear how we translate from ,if=virtio to
virtio-blk-pci in a machine neutral way.
Regards,
Anthony Liguori
> case QEMU_OPTION_set:
> if (qemu_set_option(optarg) != 0)
> exit(1);
> @@ -3156,8 +3173,14 @@ int main(int argc, char **argv, char **envp)
> }
>
> /* open the virtual block devices */
> - if (snapshot)
> - qemu_opts_foreach(&qemu_drive_opts, drive_enable_snapshot, NULL, 0);
> + if (snapshot) {
> + qemu_opts_foreach(&qemu_blockdev_opts, opts_enable_snapshot, NULL, 0);
> + qemu_opts_foreach(&qemu_drive_opts, opts_enable_snapshot, NULL, 0);
> + }
> + if (qemu_opts_foreach(&qemu_blockdev_opts, blockdev_init_func, NULL, 1)) {
> + exit(1);
> + }
> + qemu_opts_reset(&qemu_blockdev_opts);
> if (qemu_opts_foreach(&qemu_drive_opts, drive_init_func,&machine->use_scsi, 1) != 0)
> exit(1);
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread