diff mbox series

[v2,05/11] qemu-options: finesse the recommendations around -blockdev

Message ID 20230403134920.2132362-6-alex.bennee@linaro.org
State New
Headers show
Series more misc fixes for 8.0 (tests, gdbstub, meta, docs) | expand

Commit Message

Alex Bennée April 3, 2023, 1:49 p.m. UTC
We are a bit premature in recommending -blockdev/-device as the best
way to configure block devices, especially in the common case.
Improve the language to hopefully make things clearer.

Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20230330101141.30199-5-alex.bennee@linaro.org>
---
 qemu-options.hx | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Kevin Wolf April 4, 2023, 1:57 p.m. UTC | #1
Am 03.04.2023 um 15:49 hat Alex Bennée geschrieben:
> We are a bit premature in recommending -blockdev/-device as the best
> way to configure block devices, especially in the common case.
> Improve the language to hopefully make things clearer.
> 
> Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Message-Id: <20230330101141.30199-5-alex.bennee@linaro.org>
> ---
>  qemu-options.hx | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 59bdf67a2c..9a69ed838e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set and complexity
>  of the block layer have grown. Many online guides to QEMU often
>  reference older and deprecated options, which can lead to confusion.
>  
> -The recommended modern way to describe disks is to use a combination of
> +The most explicit way to describe disks is to use a combination of
>  ``-device`` to specify the hardware device and ``-blockdev`` to
>  describe the backend. The device defines what the guest sees and the
> -backend describes how QEMU handles the data.
> +backend describes how QEMU handles the data. The ``-drive`` option
> +combines the device and backend into a single command line options
> +which is useful in the majority of cases. Older options like ``-hda``
> +bake in a lot of assumptions from the days when QEMU was emulating a
> +legacy PC, they are not recommended for modern configurations.

Let's not make the use of -drive look more advisable than it really is.
If you're writing a management tool/script and you're still using -drive
today, you're doing it wrong.

Maybe this is actually the point where we should just clearly define
that -blockdev is the only supported stable API (like QMP), and that
-drive etc. are convenient shortcuts for human users with no
compatibility promise (like HMP).

What stopped us from doing so is that there are certain boards that
don't allow the user to configure the onboard devices, but that look at
-drive. These wouldn't provide any stable API any more after this
change. However, if this hasn't been solved in many years, maybe it's
time to view it as the board's problem, and use this change to motivate
them to implement ways to configure the devices. Or maybe some don't
even want to bother with a stable API, who knows.

Kevin
Alex Bennée April 4, 2023, 2:55 p.m. UTC | #2
Kevin Wolf <kwolf@redhat.com> writes:

> Am 03.04.2023 um 15:49 hat Alex Bennée geschrieben:
>> We are a bit premature in recommending -blockdev/-device as the best
>> way to configure block devices, especially in the common case.
>> Improve the language to hopefully make things clearer.
>> 
>> Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Message-Id: <20230330101141.30199-5-alex.bennee@linaro.org>
>> ---
>>  qemu-options.hx | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 59bdf67a2c..9a69ed838e 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set and complexity
>>  of the block layer have grown. Many online guides to QEMU often
>>  reference older and deprecated options, which can lead to confusion.
>>  
>> -The recommended modern way to describe disks is to use a combination of
>> +The most explicit way to describe disks is to use a combination of
>>  ``-device`` to specify the hardware device and ``-blockdev`` to
>>  describe the backend. The device defines what the guest sees and the
>> -backend describes how QEMU handles the data.
>> +backend describes how QEMU handles the data. The ``-drive`` option
>> +combines the device and backend into a single command line options
>> +which is useful in the majority of cases. Older options like ``-hda``
>> +bake in a lot of assumptions from the days when QEMU was emulating a
>> +legacy PC, they are not recommended for modern configurations.
>
> Let's not make the use of -drive look more advisable than it really is.
> If you're writing a management tool/script and you're still using -drive
> today, you're doing it wrong.
>
> Maybe this is actually the point where we should just clearly define
> that -blockdev is the only supported stable API (like QMP), and that
> -drive etc. are convenient shortcuts for human users with no
> compatibility promise (like HMP).

OK I'll drop this patch from today's PR and await a better description
in due course.

>
> What stopped us from doing so is that there are certain boards that
> don't allow the user to configure the onboard devices, but that look at
> -drive. These wouldn't provide any stable API any more after this
> change. However, if this hasn't been solved in many years, maybe it's
> time to view it as the board's problem, and use this change to motivate
> them to implement ways to configure the devices. Or maybe some don't
> even want to bother with a stable API, who knows.
>
> Kevin
Michael Tokarev April 4, 2023, 3:07 p.m. UTC | #3
04.04.2023 16:57, Kevin Wolf пишет:
> Let's not make the use of -drive look more advisable than it really is.
> If you're writing a management tool/script and you're still using -drive
> today, you're doing it wrong.

Kevin, maybe I'm wrong here, but what to do with the situation which
started it all, -- with -snapshot?

If anything, I think there should be a bold note that -snapshot is broken
by -blockdev.  Users are learning that the *hard* way, after losing their
data..

/mjt
Kevin Wolf April 4, 2023, 4:17 p.m. UTC | #4
Am 04.04.2023 um 17:07 hat Michael Tokarev geschrieben:
> 04.04.2023 16:57, Kevin Wolf пишет:
> > Let's not make the use of -drive look more advisable than it really is.
> > If you're writing a management tool/script and you're still using -drive
> > today, you're doing it wrong.
> 
> Kevin, maybe I'm wrong here, but what to do with the situation which
> started it all, -- with -snapshot?
> 
> If anything, I think there should be a bold note that -snapshot is
> broken by -blockdev.  Users are learning that the *hard* way, after
> losing their data..

Ah, I missed this context.

Maybe -snapshot should error out if -blockdev is in use. You'd generally
expect that either -blockdev is used primarily and snapshots are done
externally (if the command line is generated by some management tool),
or that -drive is used consistently (by a human who likes the
convenience). In both cases, we wouldn't hit the error path.

There may be some exceptional cases where you have both -drive and
-blockdev (maybe because a human users needs more control for one
specific disk). This is the case where you can get a nasty surprise and
that would error out. If you legitimately want the -drive images
snapshotted, but not the -blockdev ones, you can still use individual
'-drive snapshot=on' options instead of the global '-snapshot' (and the
error message should mention this).

Would you see any problems with such an approach?

Kevin
Reinoud Zandijk April 6, 2023, 8:23 p.m. UTC | #5
On Tue, Apr 04, 2023 at 06:17:45PM +0200, Kevin Wolf wrote:
> Am 04.04.2023 um 17:07 hat Michael Tokarev geschrieben:
> > 04.04.2023 16:57, Kevin Wolf пишет:
> Maybe -snapshot should error out if -blockdev is in use. You'd generally
> expect that either -blockdev is used primarily and snapshots are done
> externally (if the command line is generated by some management tool),
> or that -drive is used consistently (by a human who likes the
> convenience). In both cases, we wouldn't hit the error path.
> 
> There may be some exceptional cases where you have both -drive and
> -blockdev (maybe because a human users needs more control for one
> specific disk). This is the case where you can get a nasty surprise and
> that would error out. If you legitimately want the -drive images
> snapshotted, but not the -blockdev ones, you can still use individual
> '-drive snapshot=on' options instead of the global '-snapshot' (and the
> error message should mention this).

I didn't know that! I normally use the -snapshot as global option. Is there a
reason why -blockdev isn't honouring -snapshot?

With regards,
Reinoud
Kevin Wolf April 11, 2023, 12:09 p.m. UTC | #6
Am 06.04.2023 um 22:23 hat Reinoud Zandijk geschrieben:
> On Tue, Apr 04, 2023 at 06:17:45PM +0200, Kevin Wolf wrote:
> > Am 04.04.2023 um 17:07 hat Michael Tokarev geschrieben:
> > > 04.04.2023 16:57, Kevin Wolf пишет:
> > Maybe -snapshot should error out if -blockdev is in use. You'd generally
> > expect that either -blockdev is used primarily and snapshots are done
> > externally (if the command line is generated by some management tool),
> > or that -drive is used consistently (by a human who likes the
> > convenience). In both cases, we wouldn't hit the error path.
> > 
> > There may be some exceptional cases where you have both -drive and
> > -blockdev (maybe because a human users needs more control for one
> > specific disk). This is the case where you can get a nasty surprise and
> > that would error out. If you legitimately want the -drive images
> > snapshotted, but not the -blockdev ones, you can still use individual
> > '-drive snapshot=on' options instead of the global '-snapshot' (and the
> > error message should mention this).
> 
> I didn't know that! I normally use the -snapshot as global option. Is there a
> reason why -blockdev isn't honouring -snapshot?

The philosophy behind -blockdev is that you're explicit about every
image file (and other block node) you want to use and that QEMU doesn't
magically insert or change things behind your back.

For simple use cases that might not seem necessary, but many of the
newer functions added to the block layer, like the block jobs, are
operations that can work on any node in the block graph (i.e. any of the
open images, including backing files etc.). If QEMU changed something
behind your back, you can easily access the wrong image. Especially for
management software like libvirt this kind of magic that -drive involves
was really hard to work with because it always had to second guess what
the world _really_ looked like on the QEMU side.

For example, imagine you open foo.img with -snapshot. Now you want to
create a backup of your current state, so tell QEMU to backup the block
node for foo.img because that's what your VM is currently running on,
right? Except that nobody told you that the active image is actually a
temporary qcow2 image file that -snapshot created internally. You're
backing up the wrong image without the changes of your running VM.

So it's better to always be explicit, and then it's unambiguous which
image file you really mean in operations.

Kevin
Alex Bennée April 11, 2023, 1:03 p.m. UTC | #7
Kevin Wolf <kwolf@redhat.com> writes:

> Am 06.04.2023 um 22:23 hat Reinoud Zandijk geschrieben:
>> On Tue, Apr 04, 2023 at 06:17:45PM +0200, Kevin Wolf wrote:
>> > Am 04.04.2023 um 17:07 hat Michael Tokarev geschrieben:
>> > > 04.04.2023 16:57, Kevin Wolf пишет:
>> > Maybe -snapshot should error out if -blockdev is in use. You'd generally
>> > expect that either -blockdev is used primarily and snapshots are done
>> > externally (if the command line is generated by some management tool),
>> > or that -drive is used consistently (by a human who likes the
>> > convenience). In both cases, we wouldn't hit the error path.
>> > 
>> > There may be some exceptional cases where you have both -drive and
>> > -blockdev (maybe because a human users needs more control for one
>> > specific disk). This is the case where you can get a nasty surprise and
>> > that would error out. If you legitimately want the -drive images
>> > snapshotted, but not the -blockdev ones, you can still use individual
>> > '-drive snapshot=on' options instead of the global '-snapshot' (and the
>> > error message should mention this).
>> 
>> I didn't know that! I normally use the -snapshot as global option. Is there a
>> reason why -blockdev isn't honouring -snapshot?
>
> The philosophy behind -blockdev is that you're explicit about every
> image file (and other block node) you want to use and that QEMU doesn't
> magically insert or change things behind your back.
>
> For simple use cases that might not seem necessary, but many of the
> newer functions added to the block layer, like the block jobs, are
> operations that can work on any node in the block graph (i.e. any of the
> open images, including backing files etc.). If QEMU changed something
> behind your back, you can easily access the wrong image. Especially for
> management software like libvirt this kind of magic that -drive involves
> was really hard to work with because it always had to second guess what
> the world _really_ looked like on the QEMU side.
>
> For example, imagine you open foo.img with -snapshot. Now you want to
> create a backup of your current state, so tell QEMU to backup the block
> node for foo.img because that's what your VM is currently running on,
> right? Except that nobody told you that the active image is actually a
> temporary qcow2 image file that -snapshot created internally. You're
> backing up the wrong image without the changes of your running VM.
>
> So it's better to always be explicit, and then it's unambiguous which
> image file you really mean in operations.

With that in mind please review:

  Subject: [PATCH v3] qemu-options: finesse the recommendations around -blockdev
  Date: Thu,  6 Apr 2023 10:53:17 +0100
  Message-Id: <20230406095317.3321318-1-alex.bennee@linaro.org>
diff mbox series

Patch

diff --git a/qemu-options.hx b/qemu-options.hx
index 59bdf67a2c..9a69ed838e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1143,10 +1143,14 @@  have gone through several iterations as the feature set and complexity
 of the block layer have grown. Many online guides to QEMU often
 reference older and deprecated options, which can lead to confusion.
 
-The recommended modern way to describe disks is to use a combination of
+The most explicit way to describe disks is to use a combination of
 ``-device`` to specify the hardware device and ``-blockdev`` to
 describe the backend. The device defines what the guest sees and the
-backend describes how QEMU handles the data.
+backend describes how QEMU handles the data. The ``-drive`` option
+combines the device and backend into a single command line options
+which is useful in the majority of cases. Older options like ``-hda``
+bake in a lot of assumptions from the days when QEMU was emulating a
+legacy PC, they are not recommended for modern configurations.
 
 ERST