| Message ID | 1480434248-27138-26-git-send-email-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
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 ? Thanks, C. > Signed-off-by: Cédric Le Goater <clg@kaod.org> > Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > 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 serial=@var{serial} > This option specifies the serial number to assign to the device. > +@item model=@var{model} > +This option specifies the model to use to create a flash device (if=mtd only). > @item addr=@var{addr} > Specify the controller's PCI address (if=virtio only). > @item werror=@var{action},rerror=@var{action} >
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 ? Thanks, C. > Signed-off-by: Cédric Le Goater <clg@kaod.org> > Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > 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 serial=@var{serial} > This option specifies the serial number to assign to the device. > +@item model=@var{model} > +This option specifies the model to use to create a flash device (if=mtd only). > @item addr=@var{addr} > Specify the controller's PCI address (if=virtio only). > @item werror=@var{action},rerror=@var{action} >
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 <clg@kaod.org> > > Reviewed-by: Joel Stanley <joel@jms.id.au> > > --- > > 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 serial=@var{serial} > > This option specifies the serial number to assign to the device. > > +@item model=@var{model} > > +This option specifies the model to use to create a flash device (if=mtd only). > > @item addr=@var{addr} > > Specify the controller's PCI address (if=virtio only). > > @item werror=@var{action},rerror=@var{action} > > >
On 11/29/2016 07:08 PM, Kevin Wolf wrote: > 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. OK. I see. I should use something like this on the command line : -drive file=flash-romulus-test,format=raw,if=mtd,id=bmc \ -device mx25l25635e,drive=bmc \ -drive file=flash-romulus-test2,format=raw,if=mtd,id=bmc2 \ -device mx25l25635e,drive=bmc2 \ -drive file=romulus.pnor,format=raw,if=mtd,id=pnor \ -device mx66l1g45g,drive=pnor and retrieve the attached m25p80 device from the drive with the routine blk_get_attached_dev(). That is changing a bit the way the platform is initialized but it is cleaner as no default devices are automatically added. Thanks, C. > Kevin > >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> Reviewed-by: Joel Stanley <joel@jms.id.au> >>> --- >>> 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 serial=@var{serial} >>> This option specifies the serial number to assign to the device. >>> +@item model=@var{model} >>> +This option specifies the model to use to create a flash device (if=mtd only). >>> @item addr=@var{addr} >>> Specify the controller's PCI address (if=virtio only). >>> @item werror=@var{action},rerror=@var{action} >>> >>
Am 30.11.2016 um 16:09 hat Cédric Le Goater geschrieben: > On 11/29/2016 07:08 PM, Kevin Wolf wrote: > > 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. > > OK. I see. I should use something like this on the command line : > > -drive file=flash-romulus-test,format=raw,if=mtd,id=bmc \ > -device mx25l25635e,drive=bmc \ > -drive file=flash-romulus-test2,format=raw,if=mtd,id=bmc2 \ > -device mx25l25635e,drive=bmc2 \ > -drive file=romulus.pnor,format=raw,if=mtd,id=pnor \ > -device mx66l1g45g,drive=pnor I think you mean if=none instead of if=mtd everywhere, now that you're using an explicit -device. > and retrieve the attached m25p80 device from the drive with the > routine blk_get_attached_dev(). That is changing a bit the way > the platform is initialized but it is cleaner as no default > devices are automatically added. Right. For compatibility (and convenience) you can and probably should still support if=mtd to create the device automatically, but then the user just gets the defaults for the options that -drive doesn't allow to specify. Kevin
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 serial=@var{serial} This option specifies the serial number to assign to the device. +@item model=@var{model} +This option specifies the model to use to create a flash device (if=mtd only). @item addr=@var{addr} Specify the controller's PCI address (if=virtio only). @item werror=@var{action},rerror=@var{action}