From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWnDr-0003Wf-Kt for qemu-devel@nongnu.org; Wed, 24 Sep 2014 10:07:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWnDl-0001MM-FB for qemu-devel@nongnu.org; Wed, 24 Sep 2014 10:07:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2659) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWnDl-0001Lv-8u for qemu-devel@nongnu.org; Wed, 24 Sep 2014 10:07:05 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8OE6xff016878 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 24 Sep 2014 10:06:59 -0400 From: Markus Armbruster References: <1411490885-29782-1-git-send-email-jsnow@redhat.com> <1411490885-29782-2-git-send-email-jsnow@redhat.com> Date: Wed, 24 Sep 2014 16:06:56 +0200 In-Reply-To: <1411490885-29782-2-git-send-email-jsnow@redhat.com> (John Snow's message of "Tue, 23 Sep 2014 12:48:00 -0400") Message-ID: <87zjdpp0xr.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 1/6] blockdev: Orphaned drive search List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mst@redhat.com John Snow writes: > When users use command line options like -hda, -cdrom, > or even -drive if=ide, it is up to the board initialization > routines to pick up these drives and create backing > devices for them. > > Some boards, like Q35, have not been doing this. > However, there is no warning explaining why certain > drive specifications are just silently ignored, > so this function adds a check to print some warnings > to assist users in debugging these sorts of issues > in the future. > > This patch will warn about drives added with if_none, Judging from my testing, I suspect you mean "will not warn" ;) > for which it is not possible to tell in advance if > the omission of a backing device is an issue. > > A warning in these cases is considered appropriate. > > Signed-off-by: John Snow > --- > blockdev.c | 21 +++++++++++++++++++++ > include/sysemu/blockdev.h | 2 ++ > vl.c | 12 +++++++++++- > 3 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/blockdev.c b/blockdev.c > index b361fbb..81398e7 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -166,6 +166,27 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit) > return NULL; > } > > +bool drive_check_orphaned(void) > +{ > + DriveInfo *dinfo; > + bool rs = false; > + > + QTAILQ_FOREACH(dinfo, &drives, next) { > + /* If dinfo->bdrv->dev is NULL, it has no device attached. */ > + /* Unless this is a default drive, this may be an oversight. */ > + if (!dinfo->bdrv->dev && !dinfo->is_default && > + dinfo->type != IF_NONE) { > + fprintf(stderr, "Warning: Orphaned drive without device: " > + "id=%s,file=%s,if=%s,bus=%d,unit=%d\n", > + dinfo->id, dinfo->bdrv->filename, if_name[dinfo->type], > + dinfo->bus, dinfo->unit); > + rs = true; > + } > + } > + > + return rs; > +} > + > DriveInfo *drive_get_by_index(BlockInterfaceType type, int index) > { > return drive_get(type, > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h > index 23a5d10..80f768d 100644 > --- a/include/sysemu/blockdev.h > +++ b/include/sysemu/blockdev.h > @@ -38,6 +38,7 @@ struct DriveInfo { > 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; > QemuOpts *opts; > @@ -46,6 +47,7 @@ struct DriveInfo { > }; > > DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit); > +bool drive_check_orphaned(void); > DriveInfo *drive_get_by_index(BlockInterfaceType type, int index); > int drive_get_max_bus(BlockInterfaceType type); > DriveInfo *drive_get_next(BlockInterfaceType type); > diff --git a/vl.c b/vl.c > index dc792fe..eaef240 100644 > --- a/vl.c > +++ b/vl.c > @@ -1168,6 +1168,7 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type, > int index, const char *optstr) > { > QemuOpts *opts; > + DriveInfo *dinfo; > > if (!enable || drive_get_by_index(type, index)) { > return; > @@ -1177,9 +1178,13 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type, > if (snapshot) { > drive_enable_snapshot(opts, NULL); > } > - if (!drive_new(opts, type)) { > + > + dinfo = drive_new(opts, type); > + if (!dinfo) { > exit(1); > } > + dinfo->is_default = true; > + > } > > void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque) > @@ -4457,6 +4462,11 @@ int main(int argc, char **argv, char **envp) > if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1) != 0) > exit(1); > > + /* anybody left over? */ > + if (drive_check_orphaned()) { > + fprintf(stderr, "Warning: found drives without a backing device.\n"); > + } > + > net_check_clients(); > > ds = init_displaystate(); Terminology: the device model is the front end, the thing created by -drive is the back end. I'd simply drop this message. If you want to keep it, rephrase it to avoid "backing device".