Commit c4f26c9f authored by Markus Armbruster's avatar Markus Armbruster
Browse files

blockdev: Convert drive_new() to Error



Calling error_report() from within a function that takes an Error **
argument is suspicious.  drive_new() calls error_report() even though
it can run within drive_init_func(), which takes an Error ** argument.
drive_init_func()'s caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway:

* Convert drive_new() to Error

* Update add_init_drive() to report the error received from
  drive_new()

* Make main() pass &error_fatal through qemu_opts_foreach(),
  drive_init_func() to drive_new()

* Make default_drive() pass &error_abort through qemu_opts_foreach(),
  drive_init_func() to drive_new()

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Reviewed-by: default avatarMax Reitz <mreitz@redhat.com>
Reviewed-by: default avatarPhilippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20181017082702.5581-34-armbru@redhat.com>
parent f881ca19
Loading
Loading
Loading
Loading
+14 −13
Original line number Diff line number Diff line
@@ -759,7 +759,8 @@ QemuOptsList qemu_legacy_drive_opts = {
    },
};

DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type,
                     Error **errp)
{
    const char *value;
    BlockBackend *blk;
@@ -808,7 +809,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
        qemu_opt_rename(all_opts, opt_renames[i].from, opt_renames[i].to,
                        &local_err);
        if (local_err) {
            error_report_err(local_err);
            error_propagate(errp, local_err);
            return NULL;
        }
    }
@@ -819,7 +820,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
        bool writethrough;

        if (bdrv_parse_cache_mode(value, &flags, &writethrough) != 0) {
            error_report("invalid cache option");
            error_setg(errp, "invalid cache option");
            return NULL;
        }

@@ -847,7 +848,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
                                   &error_abort);
    qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err);
    if (local_err) {
        error_report_err(local_err);
        error_propagate(errp, local_err);
        goto fail;
    }

@@ -860,7 +861,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
            media = MEDIA_CDROM;
            read_only = true;
        } else {
            error_report("'%s' invalid media", value);
            error_setg(errp, "'%s' invalid media", value);
            goto fail;
        }
    }
@@ -885,7 +886,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
             type++) {
        }
        if (type == IF_COUNT) {
            error_report("unsupported bus type '%s'", value);
            error_setg(errp, "unsupported bus type '%s'", value);
            goto fail;
        }
    } else {
@@ -902,7 +903,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)

    if (index != -1) {
        if (bus_id != 0 || unit_id != -1) {
            error_report("index cannot be used with bus and unit");
            error_setg(errp, "index cannot be used with bus and unit");
            goto fail;
        }
        bus_id = drive_index_to_bus_id(type, index);
@@ -921,12 +922,12 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
    }

    if (max_devs && unit_id >= max_devs) {
        error_report("unit %d too big (max is %d)", unit_id, max_devs - 1);
        error_setg(errp, "unit %d too big (max is %d)", unit_id, max_devs - 1);
        goto fail;
    }

    if (drive_get(type, bus_id, unit_id) != NULL) {
        error_report("drive with bus=%d, unit=%d (index=%d) exists",
        error_setg(errp, "drive with bus=%d, unit=%d (index=%d) exists",
                   bus_id, unit_id, index);
        goto fail;
    }
@@ -970,7 +971,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
    if (werror != NULL) {
        if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO &&
            type != IF_NONE) {
            error_report("werror is not supported by this bus type");
            error_setg(errp, "werror is not supported by this bus type");
            goto fail;
        }
        qdict_put_str(bs_opts, "werror", werror);
@@ -980,7 +981,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
    if (rerror != NULL) {
        if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI &&
            type != IF_NONE) {
            error_report("rerror is not supported by this bus type");
            error_setg(errp, "rerror is not supported by this bus type");
            goto fail;
        }
        qdict_put_str(bs_opts, "rerror", rerror);
@@ -991,7 +992,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
    bs_opts = NULL;
    if (!blk) {
        if (local_err) {
            error_report_err(local_err);
            error_propagate(errp, local_err);
        }
        goto fail;
    } else {
+4 −1
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
#include "qapi/qmp/qdict.h"
#include "qapi/error.h"
#include "qemu/config-file.h"
#include "qemu/option.h"
#include "sysemu/sysemu.h"
@@ -36,6 +37,7 @@

static DriveInfo *add_init_drive(const char *optstr)
{
    Error *err = NULL;
    DriveInfo *dinfo;
    QemuOpts *opts;
    MachineClass *mc;
@@ -45,8 +47,9 @@ static DriveInfo *add_init_drive(const char *optstr)
        return NULL;

    mc = MACHINE_GET_CLASS(current_machine);
    dinfo = drive_new(opts, mc->block_default_type);
    dinfo = drive_new(opts, mc->block_default_type, &err);
    if (!dinfo) {
        error_report_err(err);
        qemu_opts_del(opts);
        return NULL;
    }
+2 −1
Original line number Diff line number Diff line
@@ -54,7 +54,8 @@ DriveInfo *drive_get_next(BlockInterfaceType type);
QemuOpts *drive_def(const char *optstr);
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
                    const char *optstr);
DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type,
                     Error **errp);

/* device-hotplug */

+4 −4
Original line number Diff line number Diff line
@@ -1129,7 +1129,7 @@ static int drive_init_func(void *opaque, QemuOpts *opts, Error **errp)
{
    BlockInterfaceType *block_default_type = opaque;

    return drive_new(opts, *block_default_type) == NULL;
    return drive_new(opts, *block_default_type, errp) == NULL;
}

static int drive_enable_snapshot(void *opaque, QemuOpts *opts, Error **errp)
@@ -1155,8 +1155,7 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type,
        drive_enable_snapshot(NULL, opts, NULL);
    }

    dinfo = drive_new(opts, type);
    assert(dinfo);
    dinfo = drive_new(opts, type, &error_abort);
    dinfo->is_default = true;

}
@@ -4397,7 +4396,8 @@ int main(int argc, char **argv, char **envp)
                          NULL, NULL);
    }
    if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
                          &machine_class->block_default_type, NULL)) {
                          &machine_class->block_default_type, &error_fatal)) {
        /* We printed help */
        exit(1);
    }