| Message ID | 20220714182836.89602-6-wuhaotsh@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Misc NPCM7XX patches | expand |
Hao Wu <wuhaotsh@google.com> writes: > This type is used to represent block devs that are not suitable to > be represented by other existing types. > > A sample use is to represent an at24c eeprom device defined in > hw/nvram/eeprom_at24c.c. The block device can be used to contain the > content of the said eeprom device. > > Signed-off-by: Hao Wu <wuhaotsh@google.com> Let me add a bit more history in the hope of helping other reviewers. v3 of this series[1] used IF_NONE for the EEPROM device. Peter Maydell questioned that[2], and we concluded it's wrong. I wrote [A] board can use any traditional interface type. If none of them matches, and common decency prevents use of a non-matching one, invent a new one. We could do IF_OTHER. Thomas Huth cleaned up the existing abuse of IF_NONE to use IF_PFLASH instead, in commit 6b717a8d44: hw/misc/sifive_u_otp: Use IF_PFLASH for the OTP device instead of IF_NONE Configuring a drive with "if=none" is meant for creation of a backend only, it should not get automatically assigned to a device frontend. Use "if=pflash" for the One-Time-Programmable device instead (like it is e.g. also done for the efuse device in hw/arm/xlnx-zcu102.c). Since the old way of configuring the device has already been published with the previous QEMU versions, we cannot remove this immediately, but have to deprecate it and support it for at least two more releases. Signed-off-by: Thomas Huth <thuth@redhat.com> Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20211119102549.217755-1-thuth@redhat.com Signed-off-by: Alistair Francis <alistair.francis@wdc.com> An OTP device isn't really a parallel flash, and neither are eFuses. More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of other interface types, too. This patch introduces IF_OTHER. The patch after next uses it for an EEPROM device. Do we want IF_OTHER? If no, I guess we get to abuse IF_PFLASH some more. If yes, I guess we should use IF_PFLASH only for actual parallel flash memory going forward. Cleaning up existing abuse of IF_PFLASH may not be worth the trouble, though. Thoughts? > --- > blockdev.c | 4 +++- > include/sysemu/blockdev.h | 1 + > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/blockdev.c b/blockdev.c > index 9230888e34..befd69ac5f 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -82,6 +82,7 @@ static const char *const if_name[IF_COUNT] = { > [IF_MTD] = "mtd", > [IF_SD] = "sd", > [IF_VIRTIO] = "virtio", > + [IF_OTHER] = "other", > [IF_XEN] = "xen", > }; > > @@ -726,7 +727,8 @@ QemuOptsList qemu_legacy_drive_opts = { > },{ > .name = "if", > .type = QEMU_OPT_STRING, > - .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", > + .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio," > + " other)", > },{ > .name = "file", > .type = QEMU_OPT_STRING, > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h > index 3211b16513..d9dd5af291 100644 > --- a/include/sysemu/blockdev.h > +++ b/include/sysemu/blockdev.h > @@ -21,6 +21,7 @@ typedef enum { > */ > IF_NONE = 0, > IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, > + IF_OTHER, > IF_COUNT > } BlockInterfaceType; [1] https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg00372.html [2] Subject: does drive_get_next(IF_NONE) make sense? Message-ID: <CAFEAcA9zmPds0+jHm8VY465XEhK6bbVPd+nDob1ruRPaHOua_Q@mail.gmail.com> https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg00710.html
On 18/07/2022 11.49, Markus Armbruster wrote: [...] > An OTP device isn't really a parallel flash, and neither are eFuses. > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > other interface types, too. > > This patch introduces IF_OTHER. The patch after next uses it for an > EEPROM device. > > Do we want IF_OTHER? > > If no, I guess we get to abuse IF_PFLASH some more. > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > be worth the trouble, though. > > Thoughts? Maybe we should simply rename IF_PFLASH to IF_EPROM or IF_FLASH to make it clear that it is not only about parallel flashes anymore? Just my 0.02 €. Thomas
Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: > Hao Wu <wuhaotsh@google.com> writes: > > > This type is used to represent block devs that are not suitable to > > be represented by other existing types. > > > > A sample use is to represent an at24c eeprom device defined in > > hw/nvram/eeprom_at24c.c. The block device can be used to contain the > > content of the said eeprom device. > > > > Signed-off-by: Hao Wu <wuhaotsh@google.com> > > Let me add a bit more history in the hope of helping other reviewers. > > v3 of this series[1] used IF_NONE for the EEPROM device. > > Peter Maydell questioned that[2], and we concluded it's wrong. I wrote > > [A] board can use any traditional interface type. If none of them > matches, and common decency prevents use of a non-matching one, > invent a new one. We could do IF_OTHER. > > Thomas Huth cleaned up the existing abuse of IF_NONE to use IF_PFLASH > instead, in commit 6b717a8d44: > > hw/misc/sifive_u_otp: Use IF_PFLASH for the OTP device instead of IF_NONE > > Configuring a drive with "if=none" is meant for creation of a backend > only, it should not get automatically assigned to a device frontend. > Use "if=pflash" for the One-Time-Programmable device instead (like > it is e.g. also done for the efuse device in hw/arm/xlnx-zcu102.c). > > Since the old way of configuring the device has already been published > with the previous QEMU versions, we cannot remove this immediately, but > have to deprecate it and support it for at least two more releases. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > Message-id: 20211119102549.217755-1-thuth@redhat.com > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > An OTP device isn't really a parallel flash, and neither are eFuses. > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > other interface types, too. > > This patch introduces IF_OTHER. The patch after next uses it for an > EEPROM device. > > Do we want IF_OTHER? What would the semantics even be? Any block device that doesn't pick up a different category may pick up IF_OTHER backends? It certainly feels like a strange interface to ask for "other" disk and then getting as surprise what this other thing might be. It's essentially the same as having an explicit '-device other', and I suppose most people would find that strange. > If no, I guess we get to abuse IF_PFLASH some more. > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > be worth the trouble, though. > > Thoughts? If the existing types aren't good enough (I don't have an opinion on whether IF_PFLASH is a good match), let's add a new one. But a specific new one, not just "other". Kevin
On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: > > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: > > An OTP device isn't really a parallel flash, and neither are eFuses. > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > > other interface types, too. > > > > This patch introduces IF_OTHER. The patch after next uses it for an > > EEPROM device. > > > > Do we want IF_OTHER? > > What would the semantics even be? Any block device that doesn't pick up > a different category may pick up IF_OTHER backends? > > It certainly feels like a strange interface to ask for "other" disk and > then getting as surprise what this other thing might be. It's > essentially the same as having an explicit '-device other', and I > suppose most people would find that strange. > > > If no, I guess we get to abuse IF_PFLASH some more. > > > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > > be worth the trouble, though. > > > > Thoughts? > > If the existing types aren't good enough (I don't have an opinion on > whether IF_PFLASH is a good match), let's add a new one. But a specific > new one, not just "other". I think the common thread is "this isn't what anybody actually thinks of as being a 'disk', but we would like to back it with a block device anyway". That can cover a fair range of possibilities... thanks -- PMM
Am 28.07.2022 um 11:46 hat Peter Maydell geschrieben: > On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: > > > > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: > > > An OTP device isn't really a parallel flash, and neither are eFuses. > > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > > > other interface types, too. > > > > > > This patch introduces IF_OTHER. The patch after next uses it for an > > > EEPROM device. > > > > > > Do we want IF_OTHER? > > > > What would the semantics even be? Any block device that doesn't pick up > > a different category may pick up IF_OTHER backends? > > > > It certainly feels like a strange interface to ask for "other" disk and > > then getting as surprise what this other thing might be. It's > > essentially the same as having an explicit '-device other', and I > > suppose most people would find that strange. > > > > > If no, I guess we get to abuse IF_PFLASH some more. > > > > > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > > > be worth the trouble, though. > > > > > > Thoughts? > > > > If the existing types aren't good enough (I don't have an opinion on > > whether IF_PFLASH is a good match), let's add a new one. But a specific > > new one, not just "other". > > I think the common thread is "this isn't what anybody actually thinks > of as being a 'disk', but we would like to back it with a block device > anyway". That can cover a fair range of possibilities... How confident are we that no board will ever have two devices of this kind? As long as every board has at most one, if=other is a bad user interface in terms of descriptiveness, but still more or less workable as long as you know what it means for the specific board you use. But if you have more than one device, it becomes hard to predict which device gets which backend - it depends on the initialisation order in the code then, and I'm pretty sure that this isn't something that should have significance in external interfaces and therefore become a stable API. Kevin
On Thu, 28 Jul 2022 at 14:30, Kevin Wolf <kwolf@redhat.com> wrote: > > Am 28.07.2022 um 11:46 hat Peter Maydell geschrieben: > > On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: > > > If the existing types aren't good enough (I don't have an opinion on > > > whether IF_PFLASH is a good match), let's add a new one. But a specific > > > new one, not just "other". > > > > I think the common thread is "this isn't what anybody actually thinks > > of as being a 'disk', but we would like to back it with a block device > > anyway". That can cover a fair range of possibilities... > > How confident are we that no board will ever have two devices of this > kind? > > As long as every board has at most one, if=other is a bad user interface > in terms of descriptiveness, but still more or less workable as long as > you know what it means for the specific board you use. Extremely non-confident, but that holds for all these things, unless we add new IF_* for every possible new thing: IF_PFLASH IF_EEPROM IF_EFUSE IF_BBRAM ... ? thanks -- PMM
On 7/28/22 15:29, Kevin Wolf wrote: > Am 28.07.2022 um 11:46 hat Peter Maydell geschrieben: >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: >>> >>> Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: >>>> An OTP device isn't really a parallel flash, and neither are eFuses. >>>> More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of >>>> other interface types, too. >>>> >>>> This patch introduces IF_OTHER. The patch after next uses it for an >>>> EEPROM device. >>>> >>>> Do we want IF_OTHER? >>> >>> What would the semantics even be? Any block device that doesn't pick up >>> a different category may pick up IF_OTHER backends? >>> >>> It certainly feels like a strange interface to ask for "other" disk and >>> then getting as surprise what this other thing might be. It's >>> essentially the same as having an explicit '-device other', and I >>> suppose most people would find that strange. >>> >>>> If no, I guess we get to abuse IF_PFLASH some more. >>>> >>>> If yes, I guess we should use IF_PFLASH only for actual parallel flash >>>> memory going forward. Cleaning up existing abuse of IF_PFLASH may not >>>> be worth the trouble, though. >>>> >>>> Thoughts? >>> >>> If the existing types aren't good enough (I don't have an opinion on >>> whether IF_PFLASH is a good match), let's add a new one. But a specific >>> new one, not just "other". >> >> I think the common thread is "this isn't what anybody actually thinks >> of as being a 'disk', but we would like to back it with a block device >> anyway". That can cover a fair range of possibilities... > > How confident are we that no board will ever have two devices of this > kind? The BMC machines have a lot of eeproms. > > As long as every board has at most one, if=other is a bad user interface > in terms of descriptiveness, but still more or less workable as long as > you know what it means for the specific board you use. > > But if you have more than one device, it becomes hard to predict which > device gets which backend - it depends on the initialisation order in > the code then, and I'm pretty sure that this isn't something that should > have significance in external interfaces and therefore become a stable > API. Can't we use the drive index ? There has been various attempts to solve this problem for the Aspeed machines also. See below. May be we should introduce and IF_EEPROM for the purpose. Thanks, C. [PATCH v2] aspeed: qcom: add block backed FRU devices https://www.mail-archive.com/qemu-devel@nongnu.org/msg900485.html [PATCH] aspeed: Enable backend file for eeprom http://patchwork.ozlabs.org/project/qemu-devel/patch/20220728061228.152704-1-wangzhiqiang02@inspur.com/
Kevin Wolf <kwolf@redhat.com> writes: > Am 28.07.2022 um 11:46 hat Peter Maydell geschrieben: >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: >> > >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: >> > > An OTP device isn't really a parallel flash, and neither are eFuses. >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of >> > > other interface types, too. >> > > >> > > This patch introduces IF_OTHER. The patch after next uses it for an >> > > EEPROM device. >> > > >> > > Do we want IF_OTHER? >> > >> > What would the semantics even be? Any block device that doesn't pick up >> > a different category may pick up IF_OTHER backends? >> > >> > It certainly feels like a strange interface to ask for "other" disk and >> > then getting as surprise what this other thing might be. It's >> > essentially the same as having an explicit '-device other', and I >> > suppose most people would find that strange. >> > >> > > If no, I guess we get to abuse IF_PFLASH some more. >> > > >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not >> > > be worth the trouble, though. >> > > >> > > Thoughts? >> > >> > If the existing types aren't good enough (I don't have an opinion on >> > whether IF_PFLASH is a good match), let's add a new one. But a specific >> > new one, not just "other". >> >> I think the common thread is "this isn't what anybody actually thinks >> of as being a 'disk', but we would like to back it with a block device >> anyway". That can cover a fair range of possibilities... > > How confident are we that no board will ever have two devices of this > kind? > > As long as every board has at most one, if=other is a bad user interface > in terms of descriptiveness, but still more or less workable as long as > you know what it means for the specific board you use. > > But if you have more than one device, it becomes hard to predict which > device gets which backend - it depends on the initialisation order in > the code then, Really? Board code should use IF_OTHER devices just like it uses the other interface types, namely connecting each frontend device to a backend device with a well-known and fixed interface type and index (or bus and unit instead, where appropriate). > and I'm pretty sure that this isn't something that should > have significance in external interfaces and therefore become a stable > API. I agree that "implied by execution order" is a bad idea: commit 95fd260f0a "blockdev: Drop unused drive_get_next()".
On Thu, 28 Jul 2022 at 15:50, Markus Armbruster <armbru@redhat.com> wrote: > Kevin Wolf <kwolf@redhat.com> writes: > > > > But if you have more than one device, it becomes hard to predict which > > device gets which backend - it depends on the initialisation order in > > the code then, > > Really? Board code should use IF_OTHER devices just like it uses the > other interface types, namely connecting each frontend device to a > backend device with a well-known and fixed interface type and index (or > bus and unit instead, where appropriate). I think part of the problem is that unlike the typical disk interface, where there is some idea of bus-and-unit-number or index number that it makes sense to expose to users, these "miscellaneous storage" devices don't have any particular index concept -- in the real hardware there are just a random set of devices that are connected in various places. So you're requiring users to look up the documentation for "index 0 is this eeprom, index 1 is that other eeprom, index 2 is ...". thanks -- PMM
Am 28.07.2022 um 16:50 hat Markus Armbruster geschrieben: > Kevin Wolf <kwolf@redhat.com> writes: > > > Am 28.07.2022 um 11:46 hat Peter Maydell geschrieben: > >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: > >> > > >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: > >> > > An OTP device isn't really a parallel flash, and neither are eFuses. > >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > >> > > other interface types, too. > >> > > > >> > > This patch introduces IF_OTHER. The patch after next uses it for an > >> > > EEPROM device. > >> > > > >> > > Do we want IF_OTHER? > >> > > >> > What would the semantics even be? Any block device that doesn't pick up > >> > a different category may pick up IF_OTHER backends? > >> > > >> > It certainly feels like a strange interface to ask for "other" disk and > >> > then getting as surprise what this other thing might be. It's > >> > essentially the same as having an explicit '-device other', and I > >> > suppose most people would find that strange. > >> > > >> > > If no, I guess we get to abuse IF_PFLASH some more. > >> > > > >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > >> > > be worth the trouble, though. > >> > > > >> > > Thoughts? > >> > > >> > If the existing types aren't good enough (I don't have an opinion on > >> > whether IF_PFLASH is a good match), let's add a new one. But a specific > >> > new one, not just "other". > >> > >> I think the common thread is "this isn't what anybody actually thinks > >> of as being a 'disk', but we would like to back it with a block device > >> anyway". That can cover a fair range of possibilities... > > > > How confident are we that no board will ever have two devices of this > > kind? > > > > As long as every board has at most one, if=other is a bad user interface > > in terms of descriptiveness, but still more or less workable as long as > > you know what it means for the specific board you use. > > > > But if you have more than one device, it becomes hard to predict which > > device gets which backend - it depends on the initialisation order in > > the code then, > > Really? Board code should use IF_OTHER devices just like it uses the > other interface types, namely connecting each frontend device to a > backend device with a well-known and fixed interface type and index (or > bus and unit instead, where appropriate). > > > and I'm pretty sure that this isn't something that should > > have significance in external interfaces and therefore become a stable > > API. > > I agree that "implied by execution order" is a bad idea: commit > 95fd260f0a "blockdev: Drop unused drive_get_next()". Ah good, I was indeed thinking of something drive_get_next()-like. In case the board works with explicit indices, the situation is not as bad as I was afraid. It will certainly be workable (even if not obvious) for any boards that have a fixed number of devices with block backends, which should cover everything we're intending to cover here. I still consider if=other a bad user interface because what it means is completely opaque, but if that's okay for you in your board, who am I to object. (Of course, the real solution would be having a generic way to set qdev properties for on-board devices. I'm not expecting that we're getting this anytime soon, though.) Kevin
Peter Maydell <peter.maydell@linaro.org> writes: > On Thu, 28 Jul 2022 at 15:50, Markus Armbruster <armbru@redhat.com> wrote: >> Kevin Wolf <kwolf@redhat.com> writes: >> > >> > But if you have more than one device, it becomes hard to predict which >> > device gets which backend - it depends on the initialisation order in >> > the code then, >> >> Really? Board code should use IF_OTHER devices just like it uses the >> other interface types, namely connecting each frontend device to a >> backend device with a well-known and fixed interface type and index (or >> bus and unit instead, where appropriate). > > I think part of the problem is that unlike the typical disk > interface, where there is some idea of bus-and-unit-number or > index number that it makes sense to expose to users, these > "miscellaneous storage" devices don't have any particular index > concept -- in the real hardware there are just a random set of > devices that are connected in various places. So you're requiring > users to look up the documentation for "index 0 is this eeprom, > index 1 is that other eeprom, index 2 is ...". "Unit number on a bus" makes perfect sense for SCSI and PATA. For SATA, the only valid unit number is 0, which may or may not make sense to users. Not a problem in practice, though. Bus numbers are arbitrary, though. Harmless enough when you have to deal only with very few of them, e.g. a single SCSI HBA (one bus, number 0), a single PATA HBA (two buses, number 0 and 1), a single SATA HBA (typically six buses, numbers 0..5). For anything else, we use "index" rather than "bus" and "unit", and the indexes are completely arbitrary. Again, harmless enough when you have to deal only with a few of each interface type. *Names* rather than arbitrary index or bus numbers would arguably be a better interface. Nothing of this is new with IF_OTHER.
On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: > On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: > > > > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: > > > An OTP device isn't really a parallel flash, and neither are eFuses. > > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > > > other interface types, too. > > > > > > This patch introduces IF_OTHER. The patch after next uses it for an > > > EEPROM device. > > > > > > Do we want IF_OTHER? > > > > What would the semantics even be? Any block device that doesn't pick up > > a different category may pick up IF_OTHER backends? > > > > It certainly feels like a strange interface to ask for "other" disk and > > then getting as surprise what this other thing might be. It's > > essentially the same as having an explicit '-device other', and I > > suppose most people would find that strange. > > > > > If no, I guess we get to abuse IF_PFLASH some more. > > > > > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > > > be worth the trouble, though. > > > > > > Thoughts? > > > > If the existing types aren't good enough (I don't have an opinion on > > whether IF_PFLASH is a good match), let's add a new one. But a specific > > new one, not just "other". > > I think the common thread is "this isn't what anybody actually thinks > of as being a 'disk', but we would like to back it with a block device > anyway". That can cover a fair range of possibilities... Given that, do we even want/have to use -drive for this ? We can use -blockdev for the backend and reference that from any -device we want to create, and leave -drive out of the picture entirely With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: >> > >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: >> > > An OTP device isn't really a parallel flash, and neither are eFuses. >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of >> > > other interface types, too. >> > > >> > > This patch introduces IF_OTHER. The patch after next uses it for an >> > > EEPROM device. >> > > >> > > Do we want IF_OTHER? >> > >> > What would the semantics even be? Any block device that doesn't pick up >> > a different category may pick up IF_OTHER backends? >> > >> > It certainly feels like a strange interface to ask for "other" disk and >> > then getting as surprise what this other thing might be. It's >> > essentially the same as having an explicit '-device other', and I >> > suppose most people would find that strange. >> > >> > > If no, I guess we get to abuse IF_PFLASH some more. >> > > >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not >> > > be worth the trouble, though. >> > > >> > > Thoughts? >> > >> > If the existing types aren't good enough (I don't have an opinion on >> > whether IF_PFLASH is a good match), let's add a new one. But a specific >> > new one, not just "other". >> >> I think the common thread is "this isn't what anybody actually thinks >> of as being a 'disk', but we would like to back it with a block device >> anyway". That can cover a fair range of possibilities... > > Given that, do we even want/have to use -drive for this ? We can use > -blockdev for the backend and reference that from any -device we want > to create, and leave -drive out of the picture entirely -drive is our only means to configure onboard devices. We've talked about better means a few times, but no conclusions. I can dig up pointers, if you're interested.
On Thu, Aug 04, 2022 at 04:56:15PM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: > >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: > >> > > >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: > >> > > An OTP device isn't really a parallel flash, and neither are eFuses. > >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > >> > > other interface types, too. > >> > > > >> > > This patch introduces IF_OTHER. The patch after next uses it for an > >> > > EEPROM device. > >> > > > >> > > Do we want IF_OTHER? > >> > > >> > What would the semantics even be? Any block device that doesn't pick up > >> > a different category may pick up IF_OTHER backends? > >> > > >> > It certainly feels like a strange interface to ask for "other" disk and > >> > then getting as surprise what this other thing might be. It's > >> > essentially the same as having an explicit '-device other', and I > >> > suppose most people would find that strange. > >> > > >> > > If no, I guess we get to abuse IF_PFLASH some more. > >> > > > >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > >> > > be worth the trouble, though. > >> > > > >> > > Thoughts? > >> > > >> > If the existing types aren't good enough (I don't have an opinion on > >> > whether IF_PFLASH is a good match), let's add a new one. But a specific > >> > new one, not just "other". > >> > >> I think the common thread is "this isn't what anybody actually thinks > >> of as being a 'disk', but we would like to back it with a block device > >> anyway". That can cover a fair range of possibilities... > > > > Given that, do we even want/have to use -drive for this ? We can use > > -blockdev for the backend and reference that from any -device we want > > to create, and leave -drive out of the picture entirely > > -drive is our only means to configure onboard devices. > > We've talked about better means a few times, but no conclusions. I can > dig up pointers, if you're interested. For onboard pflash with x86, we've just got properties against the machine that we can point to a blockdev. With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Thu, Aug 04, 2022 at 04:56:15PM +0200, Markus Armbruster wrote: >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >> > On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: >> >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: >> >> > >> >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: >> >> > > An OTP device isn't really a parallel flash, and neither are eFuses. >> >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of >> >> > > other interface types, too. >> >> > > >> >> > > This patch introduces IF_OTHER. The patch after next uses it for an >> >> > > EEPROM device. >> >> > > >> >> > > Do we want IF_OTHER? >> >> > >> >> > What would the semantics even be? Any block device that doesn't pick up >> >> > a different category may pick up IF_OTHER backends? >> >> > >> >> > It certainly feels like a strange interface to ask for "other" disk and >> >> > then getting as surprise what this other thing might be. It's >> >> > essentially the same as having an explicit '-device other', and I >> >> > suppose most people would find that strange. >> >> > >> >> > > If no, I guess we get to abuse IF_PFLASH some more. >> >> > > >> >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash >> >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not >> >> > > be worth the trouble, though. >> >> > > >> >> > > Thoughts? >> >> > >> >> > If the existing types aren't good enough (I don't have an opinion on >> >> > whether IF_PFLASH is a good match), let's add a new one. But a specific >> >> > new one, not just "other". >> >> >> >> I think the common thread is "this isn't what anybody actually thinks >> >> of as being a 'disk', but we would like to back it with a block device >> >> anyway". That can cover a fair range of possibilities... >> > >> > Given that, do we even want/have to use -drive for this ? We can use >> > -blockdev for the backend and reference that from any -device we want >> > to create, and leave -drive out of the picture entirely >> >> -drive is our only means to configure onboard devices. >> >> We've talked about better means a few times, but no conclusions. I can >> dig up pointers, if you're interested. > > For onboard pflash with x86, we've just got properties against the > machine that we can point to a blockdev. True, but the vast majority of onboard block devices doesn't come with such properties. Please see Subject: On configuring onboard block devices with -blockdev (was: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards) Date: Mon, 15 Nov 2021 16:28:33 +0100 Message-ID: <875ystigke.fsf_-_@dusky.pond.sub.org> https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg03173.html
On Thu, Aug 04, 2022 at 05:30:40PM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > On Thu, Aug 04, 2022 at 04:56:15PM +0200, Markus Armbruster wrote: > >> Daniel P. Berrangé <berrange@redhat.com> writes: > >> > >> > On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: > >> >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: > >> >> > > >> >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: > >> >> > > An OTP device isn't really a parallel flash, and neither are eFuses. > >> >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > >> >> > > other interface types, too. > >> >> > > > >> >> > > This patch introduces IF_OTHER. The patch after next uses it for an > >> >> > > EEPROM device. > >> >> > > > >> >> > > Do we want IF_OTHER? > >> >> > > >> >> > What would the semantics even be? Any block device that doesn't pick up > >> >> > a different category may pick up IF_OTHER backends? > >> >> > > >> >> > It certainly feels like a strange interface to ask for "other" disk and > >> >> > then getting as surprise what this other thing might be. It's > >> >> > essentially the same as having an explicit '-device other', and I > >> >> > suppose most people would find that strange. > >> >> > > >> >> > > If no, I guess we get to abuse IF_PFLASH some more. > >> >> > > > >> >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > >> >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > >> >> > > be worth the trouble, though. > >> >> > > > >> >> > > Thoughts? > >> >> > > >> >> > If the existing types aren't good enough (I don't have an opinion on > >> >> > whether IF_PFLASH is a good match), let's add a new one. But a specific > >> >> > new one, not just "other". > >> >> > >> >> I think the common thread is "this isn't what anybody actually thinks > >> >> of as being a 'disk', but we would like to back it with a block device > >> >> anyway". That can cover a fair range of possibilities... > >> > > >> > Given that, do we even want/have to use -drive for this ? We can use > >> > -blockdev for the backend and reference that from any -device we want > >> > to create, and leave -drive out of the picture entirely > >> > >> -drive is our only means to configure onboard devices. > >> > >> We've talked about better means a few times, but no conclusions. I can > >> dig up pointers, if you're interested. > > > > For onboard pflash with x86, we've just got properties against the > > machine that we can point to a blockdev. > > True, but the vast majority of onboard block devices doesn't come with > such properties. Please see > > Subject: On configuring onboard block devices with -blockdev (was: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards) > Date: Mon, 15 Nov 2021 16:28:33 +0100 > Message-ID: <875ystigke.fsf_-_@dusky.pond.sub.org> > https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg03173.html My take away from your mail there is that in the absence of better ideas we should at least use machine properties for anything new we do so we don't make the problem worse than it already is. It feels more useful than inventing new IF_xxx possibilities for something we think is the wrong approach already. With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Thu, Aug 04, 2022 at 05:30:40PM +0200, Markus Armbruster wrote: >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >> > On Thu, Aug 04, 2022 at 04:56:15PM +0200, Markus Armbruster wrote: >> >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >> >> >> > On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: >> >> >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: >> >> >> > >> >> >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: >> >> >> > > An OTP device isn't really a parallel flash, and neither are eFuses. >> >> >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of >> >> >> > > other interface types, too. >> >> >> > > >> >> >> > > This patch introduces IF_OTHER. The patch after next uses it for an >> >> >> > > EEPROM device. >> >> >> > > >> >> >> > > Do we want IF_OTHER? >> >> >> > >> >> >> > What would the semantics even be? Any block device that doesn't pick up >> >> >> > a different category may pick up IF_OTHER backends? >> >> >> > >> >> >> > It certainly feels like a strange interface to ask for "other" disk and >> >> >> > then getting as surprise what this other thing might be. It's >> >> >> > essentially the same as having an explicit '-device other', and I >> >> >> > suppose most people would find that strange. >> >> >> > >> >> >> > > If no, I guess we get to abuse IF_PFLASH some more. >> >> >> > > >> >> >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash >> >> >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not >> >> >> > > be worth the trouble, though. >> >> >> > > >> >> >> > > Thoughts? >> >> >> > >> >> >> > If the existing types aren't good enough (I don't have an opinion on >> >> >> > whether IF_PFLASH is a good match), let's add a new one. But a specific >> >> >> > new one, not just "other". >> >> >> >> >> >> I think the common thread is "this isn't what anybody actually thinks >> >> >> of as being a 'disk', but we would like to back it with a block device >> >> >> anyway". That can cover a fair range of possibilities... >> >> > >> >> > Given that, do we even want/have to use -drive for this ? We can use >> >> > -blockdev for the backend and reference that from any -device we want >> >> > to create, and leave -drive out of the picture entirely >> >> >> >> -drive is our only means to configure onboard devices. >> >> >> >> We've talked about better means a few times, but no conclusions. I can >> >> dig up pointers, if you're interested. >> > >> > For onboard pflash with x86, we've just got properties against the >> > machine that we can point to a blockdev. >> >> True, but the vast majority of onboard block devices doesn't come with >> such properties. Please see >> >> Subject: On configuring onboard block devices with -blockdev (was: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards) >> Date: Mon, 15 Nov 2021 16:28:33 +0100 >> Message-ID: <875ystigke.fsf_-_@dusky.pond.sub.org> >> https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg03173.html > > My take away from your mail there is that in the absence of better ideas > we should at least use machine properties for anything new we do so we > don't make the problem worse than it already is. It feels more useful > than inventing new IF_xxx possibilities for something we think is the > wrong approach already. The difficulty of providing machine properties for existing devices and the lack of adoption even for new devices make me doubt they are a viable path forward. In the message I referenced above, I wrote: If "replace them all by machine properties" is the way forward, we need to get going. At the current rate of one file a year (measured charitably), we'll be done around 2090, provided we don't add more (we've added quite a few since I did the first replacement). I figure this has slipped to the 22nd century by now. Yes, more uses of -drive are steps backward. But they are trivially easy, and also drops in the bucket. Machine properties are more difficult, and whether they actually take us forward seems doubtful.
Am 08.08.2022 um 08:26 hat Markus Armbruster geschrieben: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > On Thu, Aug 04, 2022 at 05:30:40PM +0200, Markus Armbruster wrote: > >> Daniel P. Berrangé <berrange@redhat.com> writes: > >> > >> > On Thu, Aug 04, 2022 at 04:56:15PM +0200, Markus Armbruster wrote: > >> >> Daniel P. Berrangé <berrange@redhat.com> writes: > >> >> > >> >> > On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: > >> >> >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: > >> >> >> > > >> >> >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: > >> >> >> > > An OTP device isn't really a parallel flash, and neither are eFuses. > >> >> >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of > >> >> >> > > other interface types, too. > >> >> >> > > > >> >> >> > > This patch introduces IF_OTHER. The patch after next uses it for an > >> >> >> > > EEPROM device. > >> >> >> > > > >> >> >> > > Do we want IF_OTHER? > >> >> >> > > >> >> >> > What would the semantics even be? Any block device that doesn't pick up > >> >> >> > a different category may pick up IF_OTHER backends? > >> >> >> > > >> >> >> > It certainly feels like a strange interface to ask for "other" disk and > >> >> >> > then getting as surprise what this other thing might be. It's > >> >> >> > essentially the same as having an explicit '-device other', and I > >> >> >> > suppose most people would find that strange. > >> >> >> > > >> >> >> > > If no, I guess we get to abuse IF_PFLASH some more. > >> >> >> > > > >> >> >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash > >> >> >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not > >> >> >> > > be worth the trouble, though. > >> >> >> > > > >> >> >> > > Thoughts? > >> >> >> > > >> >> >> > If the existing types aren't good enough (I don't have an opinion on > >> >> >> > whether IF_PFLASH is a good match), let's add a new one. But a specific > >> >> >> > new one, not just "other". > >> >> >> > >> >> >> I think the common thread is "this isn't what anybody actually thinks > >> >> >> of as being a 'disk', but we would like to back it with a block device > >> >> >> anyway". That can cover a fair range of possibilities... > >> >> > > >> >> > Given that, do we even want/have to use -drive for this ? We can use > >> >> > -blockdev for the backend and reference that from any -device we want > >> >> > to create, and leave -drive out of the picture entirely > >> >> > >> >> -drive is our only means to configure onboard devices. > >> >> > >> >> We've talked about better means a few times, but no conclusions. I can > >> >> dig up pointers, if you're interested. > >> > > >> > For onboard pflash with x86, we've just got properties against the > >> > machine that we can point to a blockdev. > >> > >> True, but the vast majority of onboard block devices doesn't come with > >> such properties. Please see > >> > >> Subject: On configuring onboard block devices with -blockdev (was: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards) > >> Date: Mon, 15 Nov 2021 16:28:33 +0100 > >> Message-ID: <875ystigke.fsf_-_@dusky.pond.sub.org> > >> https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg03173.html > > > > My take away from your mail there is that in the absence of better ideas > > we should at least use machine properties for anything new we do so we > > don't make the problem worse than it already is. It feels more useful > > than inventing new IF_xxx possibilities for something we think is the > > wrong approach already. > > The difficulty of providing machine properties for existing devices and > the lack of adoption even for new devices make me doubt they are a > viable path forward. In the message I referenced above, I wrote: > > If "replace them all by machine properties" is the way forward, we > need to get going. At the current rate of one file a year (measured > charitably), we'll be done around 2090, provided we don't add more > (we've added quite a few since I did the first replacement). > > I figure this has slipped to the 22nd century by now. > > Yes, more uses of -drive are steps backward. But they are trivially > easy, and also drops in the bucket. Machine properties are more > difficult, and whether they actually take us forward seems doubtful. Machine properties are also not what we really want, but just a workaround. How about implementing the real thing, providing qdev properties for built-in devices? Looking at a few random users of drive_get(), they look like this: DriveInfo *dinfo = drive_get(...); dev = qdev_new("driver-type"); qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo)); qdev_realize_and_unref(...); What if we assigned a name to every built-in device and had a qdev_new_builtin(type, id) that applies qdev properties given on the command line to the device? Could be either with plain old '-device' (we're already doing a similar thing with default devices where adding -device for the existing device removes the default device) or with a new command line option. The qdev_prop_set_drive() would then become conditional and would only be done if the property wasn't already set in qdev_new_builtin(). Or maybe a new function that checks for conflicting -drive and -device. Though that part is just implementation details. Kevin
Adding Paolo and Eduardo since we're wandering into QOM-land. Kevin Wolf <kwolf@redhat.com> writes: > Am 08.08.2022 um 08:26 hat Markus Armbruster geschrieben: >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >> > On Thu, Aug 04, 2022 at 05:30:40PM +0200, Markus Armbruster wrote: >> >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >> >> >> > On Thu, Aug 04, 2022 at 04:56:15PM +0200, Markus Armbruster wrote: >> >> >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >> >> >> >> >> > On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: >> >> >> >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@redhat.com> wrote: >> >> >> >> > >> >> >> >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: >> >> >> >> > > An OTP device isn't really a parallel flash, and neither are eFuses. >> >> >> >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and maybe of >> >> >> >> > > other interface types, too. >> >> >> >> > > >> >> >> >> > > This patch introduces IF_OTHER. The patch after next uses it for an >> >> >> >> > > EEPROM device. >> >> >> >> > > >> >> >> >> > > Do we want IF_OTHER? >> >> >> >> > >> >> >> >> > What would the semantics even be? Any block device that doesn't pick up >> >> >> >> > a different category may pick up IF_OTHER backends? >> >> >> >> > >> >> >> >> > It certainly feels like a strange interface to ask for "other" disk and >> >> >> >> > then getting as surprise what this other thing might be. It's >> >> >> >> > essentially the same as having an explicit '-device other', and I >> >> >> >> > suppose most people would find that strange. >> >> >> >> > >> >> >> >> > > If no, I guess we get to abuse IF_PFLASH some more. >> >> >> >> > > >> >> >> >> > > If yes, I guess we should use IF_PFLASH only for actual parallel flash >> >> >> >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH may not >> >> >> >> > > be worth the trouble, though. >> >> >> >> > > >> >> >> >> > > Thoughts? >> >> >> >> > >> >> >> >> > If the existing types aren't good enough (I don't have an opinion on >> >> >> >> > whether IF_PFLASH is a good match), let's add a new one. But a specific >> >> >> >> > new one, not just "other". >> >> >> >> >> >> >> >> I think the common thread is "this isn't what anybody actually thinks >> >> >> >> of as being a 'disk', but we would like to back it with a block device >> >> >> >> anyway". That can cover a fair range of possibilities... >> >> >> > >> >> >> > Given that, do we even want/have to use -drive for this ? We can use >> >> >> > -blockdev for the backend and reference that from any -device we want >> >> >> > to create, and leave -drive out of the picture entirely >> >> >> >> >> >> -drive is our only means to configure onboard devices. >> >> >> >> >> >> We've talked about better means a few times, but no conclusions. I can >> >> >> dig up pointers, if you're interested. >> >> > >> >> > For onboard pflash with x86, we've just got properties against the >> >> > machine that we can point to a blockdev. >> >> >> >> True, but the vast majority of onboard block devices doesn't come with >> >> such properties. Please see >> >> >> >> Subject: On configuring onboard block devices with -blockdev (was: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards) >> >> Date: Mon, 15 Nov 2021 16:28:33 +0100 >> >> Message-ID: <875ystigke.fsf_-_@dusky.pond.sub.org> >> >> https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg03173.html >> > >> > My take away from your mail there is that in the absence of better ideas >> > we should at least use machine properties for anything new we do so we >> > don't make the problem worse than it already is. It feels more useful >> > than inventing new IF_xxx possibilities for something we think is the >> > wrong approach already. >> >> The difficulty of providing machine properties for existing devices and >> the lack of adoption even for new devices make me doubt they are a >> viable path forward. In the message I referenced above, I wrote: >> >> If "replace them all by machine properties" is the way forward, we >> need to get going. At the current rate of one file a year (measured >> charitably), we'll be done around 2090, provided we don't add more >> (we've added quite a few since I did the first replacement). >> >> I figure this has slipped to the 22nd century by now. >> >> Yes, more uses of -drive are steps backward. But they are trivially >> easy, and also drops in the bucket. Machine properties are more >> difficult, and whether they actually take us forward seems doubtful. > > Machine properties are also not what we really want, but just a > workaround. How about implementing the real thing, providing qdev > properties for built-in devices? > > Looking at a few random users of drive_get(), they look like this: > > DriveInfo *dinfo = drive_get(...); > dev = qdev_new("driver-type"); > qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo)); > qdev_realize_and_unref(...); > > What if we assigned a name to every built-in device Before we continue: I like the idea of giving onboard devices user-friendly, stable names. "serial0" beats "/machine/unattached/device[13]" hands down. Even more so where the "device[13]" part depends on other options (e.g. with "-vga none" it's "device[12]", both for i440FX). The question is what these names could be. They can't be qdev IDs, because we blew our chance to reserve names. Could we solve this in QOM? > and had a > qdev_new_builtin(type, id) that applies qdev properties given on the > command line to the device? Could be either with plain old '-device' > (we're already doing a similar thing with default devices where adding > -device for the existing device removes the default device) or with a > new command line option. > > The qdev_prop_set_drive() would then become conditional and would only > be done if the property wasn't already set in qdev_new_builtin(). Or > maybe a new function that checks for conflicting -drive and -device. > Though that part is just implementation details. The idea sounds vaguely familiar. Whether it's because we discussed similar ones on the list, or because I mulled over similar ones in my head I can't say for sure. Overloading -device so it can also configure existing devices feels iffy. We already have means to set properties for (onboard) devices to pick up: -global. But it's per device *type*, and therefore falls apart as soon as you have more than one of the type. We need something that affects a specific (onboard) device, and for that we need a way to address one. The QOM paths we have don't cut it, as I illustrated above.
diff --git a/blockdev.c b/blockdev.c index 9230888e34..befd69ac5f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -82,6 +82,7 @@ static const char *const if_name[IF_COUNT] = { [IF_MTD] = "mtd", [IF_SD] = "sd", [IF_VIRTIO] = "virtio", + [IF_OTHER] = "other", [IF_XEN] = "xen", }; @@ -726,7 +727,8 @@ QemuOptsList qemu_legacy_drive_opts = { },{ .name = "if", .type = QEMU_OPT_STRING, - .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", + .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio," + " other)", },{ .name = "file", .type = QEMU_OPT_STRING, diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index 3211b16513..d9dd5af291 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -21,6 +21,7 @@ typedef enum { */ IF_NONE = 0, IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, + IF_OTHER, IF_COUNT } BlockInterfaceType;
This type is used to represent block devs that are not suitable to be represented by other existing types. A sample use is to represent an at24c eeprom device defined in hw/nvram/eeprom_at24c.c. The block device can be used to contain the content of the said eeprom device. Signed-off-by: Hao Wu <wuhaotsh@google.com> --- blockdev.c | 4 +++- include/sysemu/blockdev.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-)