On Mon, 7 Jun 2010 15:42:19 +0100 "Daniel P. Berrange" <berra...@redhat.com> wrote:
> This converts the drive options if, trans, media, cache, aio, > rerror and werror to use the QEMU_OPT_ENUM datatype. This > standardizes the string parsing and error reporting > > $ qemu -drive file=foo,werror=stop3 > qemu: -drive file=foo,if=mtd,werror=stop3: Parameter 'werror' expects > report, ignore, enospc, stop > > $ qemu -readconfig bar.cfg > qemu:bar.cfg:6: Parameter 'werror' expects report, ignore, enospc, stop > read config bar.cfg: Invalid argument > > * block.c: Implementations for all enumerations > * block.h, sysemu.h: Declare enumerations > * qemu-config.c: Convert if, trans, media, cache, aio, > rerror and werror to use the QEMU_OPT_ENUM > * vl.c: Remove handcrafted string -> int conversions in drive_init > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > block.c | 32 +++++++----- > block.h | 55 +++++++++++++++++--- > hw/qdev.c | 2 +- > qemu-config.c | 73 +++++++++++++++++++++++--- > sysemu.h | 23 +++++++-- > vl.c | 162 > ++++++++++++++++----------------------------------------- > 6 files changed, 197 insertions(+), 150 deletions(-) > > diff --git a/block.c b/block.c > index 39724c1..7ca55e2 100644 > --- a/block.c > +++ b/block.c > @@ -27,6 +27,7 @@ > #include "block_int.h" > #include "module.h" > #include "qemu-objects.h" > +#include "sysemu.h" > > #ifdef CONFIG_BSD > #include <sys/types.h> > @@ -42,6 +43,22 @@ > #include <windows.h> > #endif > > +QEMU_ENUM_IMPL(bdrv_type, BDRV_TYPE_LAST, > + "hd", "cdrom", "floppy"); > +QEMU_ENUM_IMPL(bdrv_bios_ata_translation, BIOS_ATA_TRANSLATION_LAST, > + "auto", "none", "lba", "large", "rechs"); > +QEMU_ENUM_IMPL(bdrv_cache, BDRV_CACHE_LAST, > + "none", "off", "writeback", "writethrough", "unsafe"); > +QEMU_ENUM_IMPL(bdrv_if_type, IF_LAST, > + "none", "ide", "scsi", "floppy", "pflash", > + "mtd", "sd", "virtio", "xen"); > +QEMU_ENUM_IMPL(bdrv_if_error_action, BLOCK_ERR_LAST, > + "report", "ignore", "enospc", "stop"); > +QEMU_ENUM_IMPL(bdrv_media, BDRV_MEDIA_LAST, > + "disk", "cdrom"); > +QEMU_ENUM_IMPL(bdrv_aio, BDRV_AIO_LAST, > + "native", "threads"); > + > static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs, > int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, > BlockDriverCompletionFunc *cb, void *opaque); > @@ -1454,19 +1471,8 @@ void bdrv_info(Monitor *mon, QObject **ret_data) > > QTAILQ_FOREACH(bs, &bdrv_states, list) { > QObject *bs_obj; > - const char *type = "unknown"; > - > - switch(bs->type) { > - case BDRV_TYPE_HD: > - type = "hd"; > - break; > - case BDRV_TYPE_CDROM: > - type = "cdrom"; > - break; > - case BDRV_TYPE_FLOPPY: > - type = "floppy"; > - break; > - } > + const char *type = bdrv_type_to_string(bs->type); > + assert(type != NULL); 'unknown' has to be dropped from query-block's documentation in qemu-monitor.hx too. > > bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': %s, " > "'removable': %i, 'locked': %i }", > diff --git a/block.h b/block.h > index 756670d..11a029b 100644 > --- a/block.h > +++ b/block.h > @@ -4,6 +4,7 @@ > #include "qemu-aio.h" > #include "qemu-common.h" > #include "qemu-option.h" > +#include "qemu-enum.h" > #include "qobject.h" > > /* block.c */ > @@ -128,14 +129,52 @@ int bdrv_has_zero_init(BlockDriverState *bs); > int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int > nb_sectors, > int *pnum); > > -#define BDRV_TYPE_HD 0 > -#define BDRV_TYPE_CDROM 1 > -#define BDRV_TYPE_FLOPPY 2 > -#define BIOS_ATA_TRANSLATION_AUTO 0 > -#define BIOS_ATA_TRANSLATION_NONE 1 > -#define BIOS_ATA_TRANSLATION_LBA 2 > -#define BIOS_ATA_TRANSLATION_LARGE 3 > -#define BIOS_ATA_TRANSLATION_RECHS 4 > +enum { > + BDRV_TYPE_HD, > + BDRV_TYPE_CDROM, > + BDRV_TYPE_FLOPPY, > + > + BDRV_TYPE_LAST > +}; > +QEMU_ENUM_DECL(bdrv_type); > + > +enum { > + BIOS_ATA_TRANSLATION_AUTO, > + BIOS_ATA_TRANSLATION_NONE, > + BIOS_ATA_TRANSLATION_LBA, > + BIOS_ATA_TRANSLATION_LARGE, > + BIOS_ATA_TRANSLATION_RECHS, > + > + BIOS_ATA_TRANSLATION_LAST > +}; > +QEMU_ENUM_DECL(bdrv_bios_ata_translation); > + > +enum { > + BDRV_CACHE_NONE, > + BDRV_CACHE_OFF, > + BDRV_CACHE_WRITEBACK, > + BDRV_CACHE_WRITETHROUGH, > + BDRV_CACHE_UNSAFE, > + > + BDRV_CACHE_LAST > +}; > +QEMU_ENUM_DECL(bdrv_cache); > + > +enum { > + BDRV_MEDIA_DISK, > + BDRV_MEDIA_CDROM, > + > + BDRV_MEDIA_LAST > +}; > +QEMU_ENUM_DECL(bdrv_media); > + > +enum { > + BDRV_AIO_NATIVE, > + BDRV_AIO_THREADS, > + > + BDRV_AIO_LAST > +}; > +QEMU_ENUM_DECL(bdrv_aio); > > void bdrv_set_geometry_hint(BlockDriverState *bs, > int cyls, int heads, int secs); > diff --git a/hw/qdev.c b/hw/qdev.c > index aa2ce01..1186dfa 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -414,7 +414,7 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo > *nd) > } > } > > -static int next_block_unit[IF_COUNT]; > +static int next_block_unit[IF_LAST]; > > /* 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 > diff --git a/qemu-config.c b/qemu-config.c > index 5a4e61b..f656e6b 100644 > --- a/qemu-config.c > +++ b/qemu-config.c > @@ -4,6 +4,7 @@ > #include "qemu-config.h" > #include "sysemu.h" > #include "hw/qdev.h" > +#include "block.h" > > QemuOptsList qemu_drive_opts = { > .name = "drive", > @@ -19,8 +20,16 @@ QemuOptsList qemu_drive_opts = { > .help = "unit number (i.e. lun for scsi)", > },{ > .name = "if", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_ENUM, > .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", > + .validate = { > + .optEnum = { > + .to_string = bdrv_if_type_to_string, > + .to_string_list = bdrv_if_type_to_string_list, > + .from_string = bdrv_if_type_from_string, > + .last = IF_LAST > + } > + }, > },{ > .name = "index", > .type = QEMU_OPT_NUMBER, > @@ -38,12 +47,28 @@ QemuOptsList qemu_drive_opts = { > .help = "number of sectors (ide disk geometry)", > },{ > .name = "trans", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_ENUM, > .help = "chs translation (auto, lba. none)", > + .validate = { > + .optEnum = { > + .to_string = bdrv_bios_ata_translation_to_string, > + .to_string_list = > bdrv_bios_ata_translation_to_string_list, > + .from_string = bdrv_bios_ata_translation_from_string, > + .last = BIOS_ATA_TRANSLATION_LAST, > + } > + } > },{ > .name = "media", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_ENUM, > .help = "media type (disk, cdrom)", > + .validate = { > + .optEnum = { > + .to_string = bdrv_media_to_string, > + .to_string_list = bdrv_media_to_string_list, > + .from_string = bdrv_media_from_string, > + .last = BDRV_MEDIA_LAST, > + } > + } > },{ > .name = "snapshot", > .type = QEMU_OPT_BOOL, > @@ -53,25 +78,57 @@ QemuOptsList qemu_drive_opts = { > .help = "disk image", > },{ > .name = "cache", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_ENUM, > .help = "host cache usage (none, writeback, writethrough, > unsafe)", > + .validate = { > + .optEnum = { > + .to_string = bdrv_cache_to_string, > + .to_string_list = bdrv_cache_to_string_list, > + .from_string = bdrv_cache_from_string, > + .last = BDRV_CACHE_LAST > + }, > + }, > },{ > .name = "aio", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_ENUM, > .help = "host AIO implementation (threads, native)", > + .validate = { > + .optEnum = { > + .to_string = bdrv_aio_to_string, > + .to_string_list = bdrv_aio_to_string_list, > + .from_string = bdrv_aio_from_string, > + .last = BDRV_CACHE_LAST > + }, > + }, > },{ > .name = "format", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_STRING, /* Be nice as an enum, but the data is > too dynamic */ > .help = "disk format (raw, qcow2, ...)", > },{ > .name = "serial", > .type = QEMU_OPT_STRING, > },{ > .name = "rerror", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_ENUM, > + .validate = { > + .optEnum = { > + .to_string = bdrv_if_error_action_to_string, > + .to_string_list = bdrv_if_error_action_to_string_list, > + .from_string = bdrv_if_error_action_from_string, > + .last = BLOCK_ERR_LAST, > + }, > + }, > },{ > .name = "werror", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_ENUM, > + .validate = { > + .optEnum = { > + .to_string = bdrv_if_error_action_to_string, > + .to_string_list = bdrv_if_error_action_to_string_list, > + .from_string = bdrv_if_error_action_from_string, > + .last = BLOCK_ERR_LAST, > + }, > + }, > },{ > .name = "addr", > .type = QEMU_OPT_STRING, > diff --git a/sysemu.h b/sysemu.h > index 879446a..f0c5eb8 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -6,6 +6,7 @@ > #include "qemu-option.h" > #include "qemu-queue.h" > #include "qemu-timer.h" > +#include "qemu-enum.h" > > #ifdef _WIN32 > #include <windows.h> > @@ -149,14 +150,28 @@ extern unsigned int nb_prom_envs; > > typedef enum { > IF_NONE, > - IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, > - IF_COUNT > + IF_IDE, > + IF_SCSI, > + IF_FLOPPY, > + IF_PFLASH, > + IF_MTD, > + IF_SD, > + IF_VIRTIO, > + IF_XEN, > + > + IF_LAST > } BlockInterfaceType; > +QEMU_ENUM_DECL(bdrv_if_type); > > typedef enum { > - BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC, > - BLOCK_ERR_STOP_ANY > + BLOCK_ERR_REPORT, > + BLOCK_ERR_IGNORE, > + BLOCK_ERR_STOP_ENOSPC, > + BLOCK_ERR_STOP_ANY, > + > + BLOCK_ERR_LAST > } BlockInterfaceErrorAction; > +QEMU_ENUM_DECL(bdrv_if_error_action); > > #define BLOCK_SERIAL_STRLEN 20 > > diff --git a/vl.c b/vl.c > index 3d08a44..16491c4 100644 > --- a/vl.c > +++ b/vl.c > @@ -754,23 +754,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, void *opaque, > int *fatal_error) > { > @@ -780,7 +763,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > const char *serial; > const char *mediastr = ""; > BlockInterfaceType type; > - enum { MEDIA_DISK, MEDIA_CDROM } media; > + int media; > int bus_id, unit_id; > int cyls, heads, secs, translation; > BlockDriver *drv = NULL; > @@ -796,8 +779,6 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > > *fatal_error = 1; > > - translation = BIOS_ATA_TRANSLATION_AUTO; > - > if (machine && machine->use_scsi) { > type = IF_SCSI; > max_devs = MAX_SCSI_DEVS; > @@ -807,7 +788,6 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > max_devs = MAX_IDE_DEVS; > pstrcpy(devname, sizeof(devname), "ide"); > } > - media = MEDIA_DISK; > > /* extract parameters */ > bus_id = qemu_opt_get_number(opts, "bus", 0); > @@ -824,40 +804,15 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > file = qemu_opt_get(opts, "file"); > serial = qemu_opt_get(opts, "serial"); > > - if ((buf = qemu_opt_get(opts, "if")) != NULL) { > + type = qemu_opt_get_enum(opts, "if", IF_IDE); > + if (type == IF_IDE) > + max_devs = MAX_IDE_DEVS; > + else if (type == IF_SCSI) > + max_devs = MAX_SCSI_DEVS; > + else > + max_devs = 0; > + if ((buf = qemu_opt_get(opts, "if"))) > 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 { > - fprintf(stderr, "qemu: unsupported bus type '%s'\n", buf); > - return NULL; > - } > - } > > if (cyls || heads || secs) { > if (cyls < 1 || (type == IF_IDE && cyls > 16383)) { > @@ -874,67 +829,52 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > } > } > > - if ((buf = qemu_opt_get(opts, "trans")) != NULL) { > + translation = qemu_opt_get_enum(opts, "trans", > BIOS_ATA_TRANSLATION_AUTO); > + if (translation != BIOS_ATA_TRANSLATION_AUTO) { > if (!cyls) { > fprintf(stderr, > "qemu: '%s' trans must be used with cyls,heads and > secs\n", > buf); > return NULL; > } > - if (!strcmp(buf, "none")) > - translation = BIOS_ATA_TRANSLATION_NONE; > - else if (!strcmp(buf, "lba")) > - translation = BIOS_ATA_TRANSLATION_LBA; > - else if (!strcmp(buf, "auto")) > - translation = BIOS_ATA_TRANSLATION_AUTO; > - else { > - fprintf(stderr, "qemu: '%s' invalid translation type\n", buf); > - return NULL; > - } > } > > - if ((buf = qemu_opt_get(opts, "media")) != NULL) { > - if (!strcmp(buf, "disk")) { > - media = MEDIA_DISK; > - } else if (!strcmp(buf, "cdrom")) { > - if (cyls || secs || heads) { > - fprintf(stderr, > - "qemu: '%s' invalid physical CHS format\n", buf); > - return NULL; > - } > - media = MEDIA_CDROM; > - } else { > - fprintf(stderr, "qemu: '%s' invalid media\n", buf); > + media = qemu_opt_get_enum(opts, "media", BDRV_MEDIA_DISK); > + > + if (media == BDRV_MEDIA_CDROM) { > + if (cyls || secs || heads) { > + fprintf(stderr, > + "qemu: '%s' invalid physical CHS format\n", buf); > return NULL; > } > } > > 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 { > + int mode = bdrv_cache_from_string(buf); > + int flags[] = { > + BDRV_O_NOCACHE, /* none */ > + BDRV_O_NOCACHE, /* off */ > + BDRV_O_CACHE_WB, /* writeback */ > + 0, /* writethrough */ > + BDRV_O_CACHE_WB|BDRV_O_NO_FLUSH /* unsafe */ > + }; > + verify_true(ARRAY_SIZE(flags) == BDRV_CACHE_LAST); > + if (mode < 0) { > fprintf(stderr, "qemu: invalid cache option\n"); > return NULL; > } > + bdrv_flags |= flags[mode]; > } > > #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 { > + int aio = bdrv_aio_from_string(buf); > + if (aio < 0) { > fprintf(stderr, "qemu: invalid aio option\n"); > return NULL; > } > + if (aio == BDRV_AIO_NATIVE) > + bdrv_flags |= BDRV_O_NATIVE_AIO; > } > #endif > > @@ -952,28 +892,18 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > } > } > > - on_write_error = BLOCK_ERR_STOP_ENOSPC; > - if ((buf = qemu_opt_get(opts, "werror")) != NULL) { > + on_write_error = qemu_opt_get_enum(opts, "werror", > BLOCK_ERR_STOP_ENOSPC); > + if (on_write_error != BLOCK_ERR_STOP_ENOSPC) { > if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type > != IF_NONE) { > - fprintf(stderr, "werror is no supported by this format\n"); > - return NULL; > - } > - > - on_write_error = parse_block_error_action(buf, 0); > - if (on_write_error < 0) { > + fprintf(stderr, "werror is not supported by this format\n"); > return NULL; > } > } > > - on_read_error = BLOCK_ERR_REPORT; > - if ((buf = qemu_opt_get(opts, "rerror")) != NULL) { > + on_read_error = qemu_opt_get_enum(opts, "rerror", BLOCK_ERR_REPORT); > + if (on_read_error != BLOCK_ERR_REPORT) { > if (type != IF_IDE && type != IF_VIRTIO && type != IF_NONE) { > - fprintf(stderr, "rerror is no supported by this format\n"); > - return NULL; > - } > - > - on_read_error = parse_block_error_action(buf, 1); > - if (on_read_error < 0) { > + fprintf(stderr, "rerror is not supported by this format\n"); > return NULL; > } > } > @@ -1044,7 +974,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > /* no id supplied -> create one */ > dinfo->id = qemu_mallocz(32); > if (type == IF_IDE || type == IF_SCSI) > - mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; > + mediastr = (media == BDRV_MEDIA_CDROM) ? "-cd" : "-hd"; > if (max_devs) > snprintf(dinfo->id, 32, "%s%i%s%i", > devname, bus_id, mediastr, unit_id); > @@ -1070,16 +1000,16 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > case IF_XEN: > case IF_NONE: > switch(media) { > - case MEDIA_DISK: > + case BDRV_MEDIA_DISK: > if (cyls != 0) { > bdrv_set_geometry_hint(dinfo->bdrv, cyls, heads, secs); > bdrv_set_translation_hint(dinfo->bdrv, translation); > } > - break; > - case MEDIA_CDROM: > + break; > + case BDRV_MEDIA_CDROM: > bdrv_set_type_hint(dinfo->bdrv, BDRV_TYPE_CDROM); > - break; > - } > + break; > + } > break; > case IF_SD: > /* FIXME: This isn't really a floppy, but it's a reasonable > @@ -1098,7 +1028,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > if (devaddr) > qemu_opt_set(opts, "addr", devaddr); > break; > - case IF_COUNT: > + default: > abort(); > } > if (!file) { > @@ -1111,7 +1041,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > bdrv_flags |= (BDRV_O_SNAPSHOT|BDRV_O_CACHE_WB|BDRV_O_NO_FLUSH); > } > > - if (media == MEDIA_CDROM) { > + if (media == BDRV_MEDIA_CDROM) { > /* CDROM is fine for any interface, don't check. */ > ro = 1; > } else if (ro == 1) {