Skip to content

Commit

Permalink
blockdev: Fix blockdev-add not to create DriveInfo
Browse files Browse the repository at this point in the history
blockdev_init() always creates a DriveInfo, but only drive_new() fills
it in.  qmp_blockdev_add() leaves it blank.  This results in a drive
with type = IF_IDE, bus = 0, unit = 0.  Screwed up in commit ee13ed1.

Board initialization code looking for IDE drive (0,0) can pick up one
of these bogus drives.  The QMP command has to execute really early to
be visible.  Not sure how likely that is in practice.

Fix by creating DriveInfo in drive_new().  Block backends created by
blockdev-add don't get one.

Breaks the test for "has been created by qmp_blockdev_add()" in
blockdev_mark_auto_del() and do_drive_del(), because it changes the
value of dinfo && !dinfo->enable_auto_del from true to false.  Simply
test !dinfo instead.

Leaves DriveInfo member enable_auto_del unused.  Drop it.

A few places assume a block backend always has a DriveInfo.  Fix them
up.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
Markus Armbruster authored and kevmw committed Oct 20, 2014
1 parent d3aeb1b commit 26f8b3a
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 25 deletions.
2 changes: 1 addition & 1 deletion block/block-backend.c
Expand Up @@ -18,7 +18,7 @@ struct BlockBackend {
char *name;
int refcnt;
BlockDriverState *bs;
DriveInfo *legacy_dinfo;
DriveInfo *legacy_dinfo; /* null unless created by drive_new() */
QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */
};

Expand Down
22 changes: 7 additions & 15 deletions blockdev.c
Expand Up @@ -116,16 +116,14 @@ void blockdev_mark_auto_del(BlockBackend *blk)
DriveInfo *dinfo = blk_legacy_dinfo(blk);
BlockDriverState *bs = blk_bs(blk);

if (dinfo && !dinfo->enable_auto_del) {
if (!dinfo) {
return;
}

if (bs->job) {
block_job_cancel(bs->job);
}
if (dinfo) {
dinfo->auto_del = 1;
}
dinfo->auto_del = 1;
}

void blockdev_auto_del(BlockBackend *blk)
Expand Down Expand Up @@ -346,7 +344,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
int on_read_error, on_write_error;
BlockBackend *blk;
BlockDriverState *bs;
DriveInfo *dinfo;
ThrottleConfig cfg;
int snapshot = 0;
bool copy_on_read;
Expand Down Expand Up @@ -518,9 +515,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
bdrv_set_io_limits(bs, &cfg);
}

dinfo = g_malloc0(sizeof(*dinfo));
blk_set_legacy_dinfo(blk, dinfo);

if (!file || !*file) {
if (has_driver_specific_opts) {
file = NULL;
Expand Down Expand Up @@ -990,9 +984,8 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
assert(!local_err);
}

/* Set legacy DriveInfo fields */
dinfo = blk_legacy_dinfo(blk);
dinfo->enable_auto_del = true;
/* Create legacy DriveInfo */
dinfo = g_malloc0(sizeof(*dinfo));
dinfo->opts = all_opts;

dinfo->cyls = cyls;
Expand All @@ -1004,9 +997,10 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
dinfo->bus = bus_id;
dinfo->unit = unit_id;
dinfo->devaddr = devaddr;

dinfo->serial = g_strdup(serial);

blk_set_legacy_dinfo(blk, dinfo);

switch(type) {
case IF_IDE:
case IF_SCSI:
Expand Down Expand Up @@ -1810,7 +1804,6 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
const char *id = qdict_get_str(qdict, "id");
BlockBackend *blk;
BlockDriverState *bs;
DriveInfo *dinfo;
AioContext *aio_context;
Error *local_err = NULL;

Expand All @@ -1821,8 +1814,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
}
bs = blk_bs(blk);

dinfo = blk_legacy_dinfo(blk);
if (dinfo && !dinfo->enable_auto_del) {
if (!blk_legacy_dinfo(blk)) {
error_report("Deleting device added with blockdev-add"
" is not supported");
return -1;
Expand Down
16 changes: 10 additions & 6 deletions hw/block/block.c
Expand Up @@ -19,7 +19,9 @@ void blkconf_serial(BlockConf *conf, char **serial)
if (!*serial) {
/* try to fall back to value set with legacy -drive serial=... */
dinfo = blk_legacy_dinfo(conf->blk);
*serial = g_strdup(dinfo->serial);
if (dinfo) {
*serial = g_strdup(dinfo->serial);
}
}
}

Expand All @@ -32,11 +34,13 @@ void blkconf_geometry(BlockConf *conf, int *ptrans,
if (!conf->cyls && !conf->heads && !conf->secs) {
/* try to fall back to value set with legacy -drive cyls=... */
dinfo = blk_legacy_dinfo(conf->blk);
conf->cyls = dinfo->cyls;
conf->heads = dinfo->heads;
conf->secs = dinfo->secs;
if (ptrans) {
*ptrans = dinfo->trans;
if (dinfo) {
conf->cyls = dinfo->cyls;
conf->heads = dinfo->heads;
conf->secs = dinfo->secs;
if (ptrans) {
*ptrans = dinfo->trans;
}
}
}
if (!conf->cyls && !conf->heads && !conf->secs) {
Expand Down
2 changes: 1 addition & 1 deletion hw/ide/qdev.c
Expand Up @@ -252,7 +252,7 @@ static int ide_drive_initfn(IDEDevice *dev)
{
DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);

return ide_dev_initfn(dev, dinfo->media_cd ? IDE_CD : IDE_HD);
return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
}

#define DEFINE_IDE_DEV_PROPERTIES() \
Expand Down
2 changes: 1 addition & 1 deletion hw/scsi/scsi-disk.c
Expand Up @@ -2310,7 +2310,7 @@ static void scsi_disk_realize(SCSIDevice *dev, Error **errp)
}

dinfo = blk_legacy_dinfo(dev->conf.blk);
if (dinfo->media_cd) {
if (dinfo && dinfo->media_cd) {
scsi_cd_realize(dev, errp);
} else {
scsi_hd_realize(dev, errp);
Expand Down
1 change: 0 additions & 1 deletion include/sysemu/blockdev.h
Expand Up @@ -35,7 +35,6 @@ struct DriveInfo {
int bus;
int unit;
int auto_del; /* see blockdev_mark_auto_del() */
bool enable_auto_del; /* Only for legacy drive_new() */
bool is_default; /* Added by default_drive() ? */
int media_cd;
int cyls, heads, secs, trans;
Expand Down

0 comments on commit 26f8b3a

Please sign in to comment.