From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48241) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XYa0U-0007dC-Dn for qemu-devel@nongnu.org; Mon, 29 Sep 2014 08:24:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XYa0O-0001SE-1W for qemu-devel@nongnu.org; Mon, 29 Sep 2014 08:24:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:61922) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XYa0N-0001R4-Rh for qemu-devel@nongnu.org; Mon, 29 Sep 2014 08:24:39 -0400 Date: Mon, 29 Sep 2014 14:24:29 +0200 From: Kevin Wolf Message-ID: <20140929122429.GF3910@noname.str.redhat.com> References: <1410891148-28849-1-git-send-email-armbru@redhat.com> <1410891148-28849-19-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1410891148-28849-19-git-send-email-armbru@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 18/23] blockdev: Fix blockdev-add not to create IDE drive (0, 0) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: mreitz@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, benoit.canet@nodalink.com Am 16.09.2014 um 20:12 hat Markus Armbruster geschrieben: > 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 ee13ed1c. > > Board initialization code looking for IDE drive (0,0) can pick up one > of these bogus drives. Not sure whether getting the QMP command > executed early enough is likely in practice, though. > > Fix by creating DriveInfo in drive_new(). Block backends created by > blockdev-add don't get one. > > A few places assume a block backend always has a DriveInfo. Fix them > up. > > Signed-off-by: Markus Armbruster > --- > block/block-backend.c | 4 +--- > blockdev.c | 10 ++-------- > hw/block/block.c | 16 ++++++++++------ > hw/ide/qdev.c | 2 +- > hw/scsi/scsi-disk.c | 2 +- > 5 files changed, 15 insertions(+), 19 deletions(-) > > diff --git a/block/block-backend.c b/block/block-backend.c > index 141a31b..b55f0b4 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -101,9 +101,7 @@ static void drive_info_del(DriveInfo *dinfo) > if (!dinfo) { > return; > } > - if (dinfo->opts) { > - qemu_opts_del(dinfo->opts); > - } > + qemu_opts_del(dinfo->opts); > g_free(dinfo->serial); > g_free(dinfo); > } Doesn't look directly related? > diff --git a/blockdev.c b/blockdev.c > index 2def256..0d99be0 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -285,7 +285,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; > @@ -457,9 +456,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; > @@ -903,21 +899,19 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) > } > > /* Set legacy DriveInfo fields */ > - dinfo = blk_legacy_dinfo(blk); > + dinfo = g_malloc0(sizeof(*dinfo)); > dinfo->enable_auto_del = true; > dinfo->opts = all_opts; > - > dinfo->cyls = cyls; > dinfo->heads = heads; > dinfo->secs = secs; > dinfo->trans = translation; > - > dinfo->type = type; > dinfo->bus = bus_id; > dinfo->unit = unit_id; > dinfo->devaddr = devaddr; > - > dinfo->serial = g_strdup(serial); > + blk_set_legacy_dinfo(blk, dinfo); You don't like the grouping? > switch(type) { > case IF_IDE: Kevin