From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWvAl-0002BZ-8M for qemu-devel@nongnu.org; Wed, 24 Sep 2014 18:36:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWvAf-0001cE-2X for qemu-devel@nongnu.org; Wed, 24 Sep 2014 18:36:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:28480) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWvAe-0001Y7-S5 for qemu-devel@nongnu.org; Wed, 24 Sep 2014 18:36:25 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8OMaI8m009139 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 24 Sep 2014 18:36:19 -0400 Message-ID: <5423475D.2030701@redhat.com> Date: Wed, 24 Sep 2014 18:36:13 -0400 From: John Snow MIME-Version: 1.0 References: <1411490885-29782-1-git-send-email-jsnow@redhat.com> <1411490885-29782-2-git-send-email-jsnow@redhat.com> <87zjdpp0xr.fsf@blackfin.pond.sub.org> In-Reply-To: <87zjdpp0xr.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/6] blockdev: Orphaned drive search List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mst@redhat.com On 09/24/2014 10:06 AM, Markus Armbruster wrote: > John Snow writes: > >> When users use command line options like -hda, -cdrom, >> or even -drive if=3Dide, 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" ;) A fun story: I did actually mean what I wrote, but in my own testing,=20 found that this produced warnings for the test suite, so I decided to=20 change the behavior to not warn for IF_NONE. I figured it would make sense if it gave a little warning that amounted=20 to "Don't forget to add a device for this drive!" but I didn't want to=20 see any more output on the test suite, so I nixed it. I amended my commit message for V2. >> 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 =3D 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 !=3D IF_NONE) { >> + fprintf(stderr, "Warning: Orphaned drive without device: = " >> + "id=3D%s,file=3D%s,if=3D%s,bus=3D%d,unit=3D%d\n", >> + dinfo->id, dinfo->bdrv->filename, if_name[dinfo->= type], >> + dinfo->bus, dinfo->unit); >> + rs =3D 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 snapsh= ot, 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 snaps= hot, BlockInterfaceType type, >> if (snapshot) { >> drive_enable_snapshot(opts, NULL); >> } >> - if (!drive_new(opts, type)) { >> + >> + dinfo =3D drive_new(opts, type); >> + if (!dinfo) { >> exit(1); >> } >> + dinfo->is_default =3D 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) !=3D 0) >> exit(1); >> >> + /* anybody left over? */ >> + if (drive_check_orphaned()) { >> + fprintf(stderr, "Warning: found drives without a backing devi= ce.\n"); >> + } >> + >> net_check_clients(); >> >> ds =3D 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". > --=20 =97js