diff mbox series

[v2,12/12] docs/interop/firmware.json: Prefer -machine to if=pflash

Message ID 20190307172401.29451-13-armbru@redhat.com
State New
Headers show
Series pc: Support firmware configuration with -blockdev | expand

Commit Message

Markus Armbruster March 7, 2019, 5:24 p.m. UTC
The previous commit added a way to configure firmware with -blockdev
rather than -drive if=pflash.  Document it as the preferred way.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/interop/firmware.json | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Comments

Laszlo Ersek March 8, 2019, 8:44 a.m. UTC | #1
On 03/07/19 18:24, Markus Armbruster wrote:
> The previous commit added a way to configure firmware with -blockdev
> rather than -drive if=pflash.  Document it as the preferred way.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/interop/firmware.json | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index 28f9bc1591..db3cb38b6a 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -212,9 +212,13 @@
>  #
>  # @executable: Identifies the firmware executable. The firmware
>  #              executable may be shared by multiple virtual machine
> -#              definitions. The corresponding QEMU command line option
> -#              is "-drive
> -#              if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format".
> +#              definitions. The preferred corresponding QEMU command
> +#              line option is
> +#                  -drive if=none,id=pflash0,readonly=on,file=@executable.@filename,format=@executable.@format
> +#                  -machine pflash0=pflash0
> +#              or equivalent -blockdev.

If we used plural here ("options"), would that be an improvement?

  The preferred corresponding QEMU command line options are
    -drive ...
    -machine ...
  (or -blockdev equivalent to -drive).

> +#              With QEMU versions older than 4.0, you have to use

To make this easier to understand on first read, should we say

  ... you have to use the single option ...

?

(Maybe there is a better term than "single option" for the "-drive if=<not-NONE>" options, i.e. for those that configure both back-end and front-end.)

If the above over-explained things, I'd be fine with the current patch as well. I just got these ideas and wanted to run them by you.

Thanks!
Laszlo


> +#                  -drive if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format
>  #
>  # @nvram-template: Identifies the NVRAM template compatible with
>  #                  @executable. Management software instantiates an
> @@ -225,9 +229,14 @@
>  #                  individual copies of it are. An NVRAM file is
>  #                  typically used for persistently storing the
>  #                  non-volatile UEFI variables of a virtual machine
> -#                  definition. The corresponding QEMU command line
> -#                  option is "-drive
> -#                  if=pflash,unit=1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format".
> +#                  definition. The preferred corresponding QEMU
> +#                  command line option is
> +#                      -drive if=none,id=pflash1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format
> +#                      -machine pflash1=pflash1
> +#                  or equivalent -blockdev.
> +#                  With QEMU versions older than 4.0, you have to use
> +#                      -drive if=pflash,unit=1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format
> +#                  
>  #
>  # Since: 3.0
>  ##
>
Markus Armbruster March 8, 2019, 12:01 p.m. UTC | #2
Laszlo Ersek <lersek@redhat.com> writes:

> On 03/07/19 18:24, Markus Armbruster wrote:
>> The previous commit added a way to configure firmware with -blockdev
>> rather than -drive if=pflash.  Document it as the preferred way.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  docs/interop/firmware.json | 21 +++++++++++++++------
>>  1 file changed, 15 insertions(+), 6 deletions(-)
>> 
>> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
>> index 28f9bc1591..db3cb38b6a 100644
>> --- a/docs/interop/firmware.json
>> +++ b/docs/interop/firmware.json
>> @@ -212,9 +212,13 @@
>>  #
>>  # @executable: Identifies the firmware executable. The firmware
>>  #              executable may be shared by multiple virtual machine
>> -#              definitions. The corresponding QEMU command line option
>> -#              is "-drive
>> -#              if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format".
>> +#              definitions. The preferred corresponding QEMU command
>> +#              line option is
>> +#                  -drive if=none,id=pflash0,readonly=on,file=@executable.@filename,format=@executable.@format
>> +#                  -machine pflash0=pflash0
>> +#              or equivalent -blockdev.
>
> If we used plural here ("options"), would that be an improvement?
>
>   The preferred corresponding QEMU command line options are
>     -drive ...
>     -machine ...
>   (or -blockdev equivalent to -drive).

Definitely an improvement.

>> +#              With QEMU versions older than 4.0, you have to use
>
> To make this easier to understand on first read, should we say
>
>   ... you have to use the single option ...
>
> ?
>
> (Maybe there is a better term than "single option" for the "-drive if=<not-NONE>" options, i.e. for those that configure both back-end and front-end.)

I think we can leave this one to context.

> If the above over-explained things, I'd be fine with the current patch as well. I just got these ideas and wanted to run them by you.

Appreciated!
diff mbox series

Patch

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index 28f9bc1591..db3cb38b6a 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -212,9 +212,13 @@ 
 #
 # @executable: Identifies the firmware executable. The firmware
 #              executable may be shared by multiple virtual machine
-#              definitions. The corresponding QEMU command line option
-#              is "-drive
-#              if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format".
+#              definitions. The preferred corresponding QEMU command
+#              line option is
+#                  -drive if=none,id=pflash0,readonly=on,file=@executable.@filename,format=@executable.@format
+#                  -machine pflash0=pflash0
+#              or equivalent -blockdev.
+#              With QEMU versions older than 4.0, you have to use
+#                  -drive if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format
 #
 # @nvram-template: Identifies the NVRAM template compatible with
 #                  @executable. Management software instantiates an
@@ -225,9 +229,14 @@ 
 #                  individual copies of it are. An NVRAM file is
 #                  typically used for persistently storing the
 #                  non-volatile UEFI variables of a virtual machine
-#                  definition. The corresponding QEMU command line
-#                  option is "-drive
-#                  if=pflash,unit=1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format".
+#                  definition. The preferred corresponding QEMU
+#                  command line option is
+#                      -drive if=none,id=pflash1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format
+#                      -machine pflash1=pflash1
+#                  or equivalent -blockdev.
+#                  With QEMU versions older than 4.0, you have to use
+#                      -drive if=pflash,unit=1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format
+#                  
 #
 # Since: 3.0
 ##