Re: [Qemu-devel] [PATCH for-2.9 25/30] block: add a model option for MTD devices

[Kevin Wolf]

Am 29.11.2016 um 18:30 hat Cédric Le Goater geschrieben:
> On 11/29/2016 04:44 PM, Cédric Le Goater wrote:
> > This could be used to define the flash model to use on some boards
> > definitions.
> 
> As this patch was part of a larger set, I did not send the whole 
> set to qemu-block@ list. Could you please take a look at the proposal ? 

This is a device level option rather than a block backend one. We messed
up -drive in its early days by including some device options, but we
don't generally want to add to this.

The correct way would be to add a qdev property for this and specify it
with -device or -global.

Kevin

> > Signed-off-by: Cédric Le Goater <address@hidden>
> > Reviewed-by: Joel Stanley <address@hidden>
> > ---
> >  blockdev.c                | 12 ++++++++++++
> >  include/sysemu/blockdev.h |  1 +
> >  qemu-options.hx           |  4 +++-
> >  3 files changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/blockdev.c b/blockdev.c
> > index 245e1e1d177a..bcc99365b86c 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -742,6 +742,10 @@ QemuOptsList qemu_legacy_drive_opts = {
> >              .type = QEMU_OPT_STRING,
> >              .help = "pci address (virtio only)",
> >          },{
> > +            .name = "model",
> > +            .type = QEMU_OPT_STRING,
> > +            .help = "flash model (mtd only)",
> > +        },{
> >              .name = "serial",
> >              .type = QEMU_OPT_STRING,
> >              .help = "disk serial number",
> > @@ -790,6 +794,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, 
> > BlockInterfaceType block_default_type)
> >      bool read_only = false;
> >      bool copy_on_read;
> >      const char *serial;
> > +    const char *model;
> >      const char *filename;
> >      Error *local_err = NULL;
> >      int i;
> > @@ -1076,6 +1081,12 @@ DriveInfo *drive_new(QemuOpts *all_opts, 
> > BlockInterfaceType block_default_type)
> >          qdict_put(bs_opts, "rerror", qstring_from_str(rerror));
> >      }
> >  
> > +    model = qemu_opt_get(legacy_opts, "model");
> > +    if (model && type != IF_MTD) {
> > +        error_report("model is not supported by this bus type");
> > +        goto fail;
> > +    }
> > +
> >      /* Actual block device init: Functionality shared with blockdev-add */
> >      blk = blockdev_init(filename, bs_opts, &local_err);
> >      bs_opts = NULL;
> > @@ -1102,6 +1113,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, 
> > BlockInterfaceType block_default_type)
> >      dinfo->unit = unit_id;
> >      dinfo->devaddr = devaddr;
> >      dinfo->serial = g_strdup(serial);
> > +    dinfo->model = g_strdup(model);
> >  
> >      blk_set_legacy_dinfo(blk, dinfo);
> >  
> > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > index 16432f350805..10c66e5e86a4 100644
> > --- a/include/sysemu/blockdev.h
> > +++ b/include/sysemu/blockdev.h
> > @@ -39,6 +39,7 @@ struct DriveInfo {
> >      int cyls, heads, secs, trans;
> >      QemuOpts *opts;
> >      char *serial;
> > +    char *model;
> >      QTAILQ_ENTRY(DriveInfo) next;
> >  };
> >  
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 4a5b29f349f7..16add85bd0f5 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -517,7 +517,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
> >      "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
> >      "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
> >      "       
> > [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
> > -    "       [,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
> > +    "       [,serial=s][,model=m][,addr=A][,rerror=ignore|stop|report]\n"
> >      "       
> > [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
> >      "       [,readonly=on|off][,copy-on-read=on|off]\n"
> >      "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
> > @@ -570,6 +570,8 @@ the format.  Can be used to specify format=raw to avoid 
> > interpreting
> >  an untrusted format header.
> >  @item address@hidden
> >  This option specifies the serial number to assign to the device.
> > address@hidden address@hidden
> > +This option specifies the model to use to create a flash device (if=mtd 
> > only).
> >  @item address@hidden
> >  Specify the controller's PCI address (if=virtio only).
> >  @item address@hidden,address@hidden
> > 
>