[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/17] blockdev: Moving parsing of geometry opti
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [PATCH 09/17] blockdev: Moving parsing of geometry options to drive_init |
Date: |
Fri, 20 Sep 2013 17:04:00 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Le Friday 20 Sep 2013 à 13:54:22 (+0200), Kevin Wolf a écrit :
> This moves all of the geometry options (cyls/heads/secs/trans) to
> drive_init so that they can only be accessed using legacy functions, but
> never with anything blockdev-add related.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> blockdev.c | 136
> +++++++++++++++++++++++++++++++------------------------------
> 1 file changed, 69 insertions(+), 67 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index dc3f01a..ba8b6b4 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -314,7 +314,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
> const char *serial;
> const char *mediastr = "";
> int bus_id, unit_id;
> - int cyls, heads, secs, translation;
> int max_devs;
> int index;
> int ro = 0;
> @@ -332,8 +331,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
> bool has_driver_specific_opts;
> BlockDriver *drv = NULL;
>
> - translation = BIOS_ATA_TRANSLATION_AUTO;
> -
> /* Check common options by copying from bs_opts to opts, all other
> options
> * stay in bs_opts for processing by bdrv_open(). */
> id = qdict_get_try_str(bs_opts, "id");
> @@ -362,10 +359,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
> unit_id = qemu_opt_get_number(opts, "unit", -1);
> index = qemu_opt_get_number(opts, "index", -1);
>
> - cyls = qemu_opt_get_number(opts, "cyls", 0);
> - heads = qemu_opt_get_number(opts, "heads", 0);
> - secs = qemu_opt_get_number(opts, "secs", 0);
> -
> snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
> ro = qemu_opt_get_bool(opts, "read-only", 0);
> copy_on_read = qemu_opt_get_bool(opts, "copy-on-read", false);
> @@ -375,46 +368,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
>
> max_devs = if_max_devs[type];
>
> - if (cyls || heads || secs) {
> - if (cyls < 1) {
> - error_report("invalid physical cyls number");
> - return NULL;
> - }
> - if (heads < 1) {
> - error_report("invalid physical heads number");
> - return NULL;
> - }
> - if (secs < 1) {
> - error_report("invalid physical secs number");
> - return NULL;
> - }
> - }
> -
> - if ((buf = qemu_opt_get(opts, "trans")) != NULL) {
> - if (!cyls) {
> - error_report("'%s' trans must be used with cyls, heads and secs",
> - 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 {
> - error_report("'%s' invalid translation type", buf);
> - return NULL;
> - }
> - }
> -
> - if (media == MEDIA_CDROM) {
> - if (cyls || secs || heads) {
> - error_report("CHS can't be set with media=cdrom");
> - return NULL;
> - }
> - }
> -
> if ((buf = qemu_opt_get(opts, "discard")) != NULL) {
> if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) {
> error_report("invalid discard option");
> @@ -609,10 +562,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
> dinfo->type = type;
> dinfo->bus = bus_id;
> dinfo->unit = unit_id;
> - dinfo->cyls = cyls;
> - dinfo->heads = heads;
> - dinfo->secs = secs;
> - dinfo->trans = translation;
> dinfo->refcount = 1;
> if (serial != NULL) {
> dinfo->serial = g_strdup(serial);
> @@ -745,6 +694,22 @@ QemuOptsList qemu_legacy_drive_opts = {
> .name = "if",
> .type = QEMU_OPT_STRING,
> .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
> + },{
> + .name = "cyls",
> + .type = QEMU_OPT_NUMBER,
> + .help = "number of cylinders (ide disk geometry)",
> + },{
> + .name = "heads",
> + .type = QEMU_OPT_NUMBER,
> + .help = "number of heads (ide disk geometry)",
> + },{
> + .name = "secs",
> + .type = QEMU_OPT_NUMBER,
> + .help = "number of sectors (ide disk geometry)",
> + },{
> + .name = "trans",
> + .type = QEMU_OPT_STRING,
> + .help = "chs translation (auto, lba. none)",
> },
> { /* end of list */ }
> },
> @@ -758,6 +723,7 @@ DriveInfo *drive_init(QemuOpts *all_opts,
> BlockInterfaceType block_default_type)
> QemuOpts *legacy_opts;
> DriveMediaType media = MEDIA_DISK;
> BlockInterfaceType type;
> + int cyls, heads, secs, translation;
> Error *local_err = NULL;
>
> /* Change legacy command line options into QMP ones */
> @@ -847,6 +813,53 @@ DriveInfo *drive_init(QemuOpts *all_opts,
> BlockInterfaceType block_default_type)
> type = block_default_type;
> }
>
> + /* Geometry */
> + cyls = qemu_opt_get_number(legacy_opts, "cyls", 0);
> + heads = qemu_opt_get_number(legacy_opts, "heads", 0);
> + secs = qemu_opt_get_number(legacy_opts, "secs", 0);
> +
> + if (cyls || heads || secs) {
> + if (cyls < 1) {
> + error_report("invalid physical cyls number");
> + return NULL;
> + }
> + if (heads < 1) {
> + error_report("invalid physical heads number");
> + return NULL;
> + }
> + if (secs < 1) {
> + error_report("invalid physical secs number");
> + return NULL;
> + }
> + }
> +
> + translation = BIOS_ATA_TRANSLATION_AUTO;
> + value = qemu_opt_get(legacy_opts, "trans");
> + if (value != NULL) {
> + if (!cyls) {
> + error_report("'%s' trans must be used with cyls, heads and secs",
> + value);
> + return NULL;
> + }
> + if (!strcmp(value, "none")) {
> + translation = BIOS_ATA_TRANSLATION_NONE;
> + } else if (!strcmp(value, "lba")) {
> + translation = BIOS_ATA_TRANSLATION_LBA;
> + } else if (!strcmp(value, "auto")) {
> + translation = BIOS_ATA_TRANSLATION_AUTO;
> + } else {
> + error_report("'%s' invalid translation type", value);
> + return NULL;
> + }
> + }
> +
> + if (media == MEDIA_CDROM) {
> + if (cyls || secs || heads) {
> + error_report("CHS can't be set with media=cdrom");
> + return NULL;
> + }
> + }
> +
> /* Actual block device init: Functionality shared with blockdev-add */
> dinfo = blockdev_init(bs_opts, type, media);
> if (dinfo == NULL) {
> @@ -857,6 +870,11 @@ DriveInfo *drive_init(QemuOpts *all_opts,
> BlockInterfaceType block_default_type)
> dinfo->enable_auto_del = true;
> dinfo->opts = all_opts;
>
> + dinfo->cyls = cyls;
> + dinfo->heads = heads;
> + dinfo->secs = secs;
> + dinfo->trans = translation;
> +
> fail:
> qemu_opts_del(legacy_opts);
> return dinfo;
> @@ -2200,22 +2218,6 @@ QemuOptsList qemu_common_drive_opts = {
> .type = QEMU_OPT_NUMBER,
> .help = "index number",
> },{
> - .name = "cyls",
> - .type = QEMU_OPT_NUMBER,
> - .help = "number of cylinders (ide disk geometry)",
> - },{
> - .name = "heads",
> - .type = QEMU_OPT_NUMBER,
> - .help = "number of heads (ide disk geometry)",
> - },{
> - .name = "secs",
> - .type = QEMU_OPT_NUMBER,
> - .help = "number of sectors (ide disk geometry)",
> - },{
> - .name = "trans",
> - .type = QEMU_OPT_STRING,
> - .help = "chs translation (auto, lba. none)",
> - },{
> .name = "snapshot",
> .type = QEMU_OPT_BOOL,
> .help = "enable/disable snapshot mode",
> --
> 1.8.1.4
>
>
Aside from the "goto fail;" impacting multiple patch:
Reviewed-for-C-bugs-by: Benoit Canet <address@hidden>
- Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command, (continued)
- [Qemu-devel] [PATCH 07/17] blockdev: Move parsing of 'media' option to drive_init, Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 08/17] blockdev: Move parsing of 'if' option to drive_init, Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 09/17] blockdev: Moving parsing of geometry options to drive_init, Kevin Wolf, 2013/09/20
- Re: [Qemu-devel] [PATCH 09/17] blockdev: Moving parsing of geometry options to drive_init,
Benoît Canet <=
- [Qemu-devel] [PATCH 10/17] blockdev: Move parsing of 'boot' option to drive_init, Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 11/17] blockdev: Move bus/unit/index processing to drive_init, Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 12/17] blockdev: Move virtio-blk device creation to drive_init, Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 13/17] blockdev: Remove IF_* check for read-only blockdev_init, Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 14/17] qemu-iotests: Check autodel behaviour for device_del, Kevin Wolf, 2013/09/20