Re: [Qemu-devel] [PATCH 1/6] blockdev: Orphaned drive search

[Markus Armbruster]

John Snow <address@hidden> 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 <address@hidden>
> ---
>  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".