All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Eric Blake <eblake@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: Kevin Wolf <kwolf@redhat.com>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 00/13] Add a 'x-blockdev-reopen' QMP command
Date: Wed, 23 Jan 2019 16:56:13 +0100	[thread overview]
Message-ID: <w515zufmkpu.fsf@maestria.local.igalia.com> (raw)
In-Reply-To: <b2915d9a-90f1-a114-5d18-f2d69cc491b8@virtuozzo.com>

On Tue 22 Jan 2019 09:15:25 AM CET, Vladimir Sementsov-Ogievskiy wrote:

>>>    a) Some options cannot be changed (most drivers don't allow that, in
>>>       fact).
>>>    b) If an option is missing, it should be reset to its default value
>>>       (rather than keeping its previous value).
>> 
>> Could we use QAPI alternate types where we could state that:
>> 
>> "option":"new" - set the option
>> "option":null - reset the option to its default
>> omit option - leave the option unchanged
>> 
>> basically, making each of the options an Alternate with JSON null, so
>> that a request to reset to the default becomes an explicit action?
>
> +1
>
> Sorry, I missed the previous discussion. What is the reason to reset
> option to default if it missed? It looks more common to not touch
> things which are not asked to.

The idea is to have a command that works like blockdev-add. If you pass
a set of options to x-blockdev-reopen then the end result should be
similar to what you would get if you had used blockdev-add.

> Also, should we consider an option which may be changed during
> life-cycle of a device not by user request to change it? If yes, it
> should be safer to not reset it.

Do you have any example in mind?

> And here: if we are going to reset to default for not listed options,
> than just not listing "backing" should remove it (as default is null,
> obviously), and we don't need this special "null" parameter.

This is a bit trickier: the default for 'backing' is not to omit the
backing file, but to open the one that is specified in the image
metadata.

$ qemu-img create -f qcow2 base.qcow2 1M
$ qemu-img create -f qcow2 -b base.qcow2 active.qcow2

If you open active.qcow2 with blockdev-add then base.qcow2 is opened as
its backing file, and the only way to prevent that is to specify a
different backing file or null if you don't want to open any.

For x-blockdev-reopen we want to keep the same behavior as much as
possible, but instead of opening base.qcow2 if 'backing' is missing we
force the user to specify that option.

> Moreover, backing is an example of an option I mentioned, it
> definitely may change after block-stream for example, so resetting to
> default is unsafe, and user will have to carefully set backing on
> reopen (not tha backing, that was used with first blockdev-add, but
> backing which we have after block-stream)

Exactly. Since opening the default backing file is not something that we
want to do during a reopen operation we don't allow leaving that option
out.

Berto

  reply	other threads:[~2019-01-23 15:56 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-17 15:33 [Qemu-devel] [PATCH 00/13] Add a 'x-blockdev-reopen' QMP command Alberto Garcia
2019-01-17 15:33 ` [Qemu-devel] [PATCH 01/13] block: Allow freezing BdrvChild links Alberto Garcia
2019-02-12 14:47   ` Kevin Wolf
2019-02-14 14:13     ` Alberto Garcia
2019-02-14 15:52       ` Kevin Wolf
2019-02-18 13:35         ` Alberto Garcia
2019-01-17 15:33 ` [Qemu-devel] [PATCH 02/13] block: Freeze the backing chain for the duration of the commit job Alberto Garcia
2019-02-12 14:54   ` Kevin Wolf
2019-02-14 15:21     ` Alberto Garcia
2019-02-14 15:45       ` Kevin Wolf
2019-01-17 15:33 ` [Qemu-devel] [PATCH 03/13] block: Freeze the backing chain for the duration of the mirror job Alberto Garcia
2019-01-17 15:33 ` [Qemu-devel] [PATCH 04/13] block: Freeze the backing chain for the duration of the stream job Alberto Garcia
2019-02-12 15:15   ` Kevin Wolf
2019-02-12 16:06     ` Alberto Garcia
2019-01-17 15:33 ` [Qemu-devel] [PATCH 05/13] block: Add 'keep_old_opts' parameter to bdrv_reopen_queue() Alberto Garcia
2019-01-17 15:33 ` [Qemu-devel] [PATCH 06/13] block: Handle child references in bdrv_reopen_queue() Alberto Garcia
2019-02-12 16:28   ` Kevin Wolf
2019-02-12 16:55     ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2019-02-19 16:00     ` [Qemu-devel] " Alberto Garcia
2019-01-17 15:33 ` [Qemu-devel] [PATCH 07/13] block: Allow omitting the 'backing' option in certain cases Alberto Garcia
2019-02-12 16:38   ` Kevin Wolf
2019-01-17 15:33 ` [Qemu-devel] [PATCH 08/13] block: Allow changing the backing file on reopen Alberto Garcia
2019-02-12 17:27   ` Kevin Wolf
2019-02-21 14:46     ` Alberto Garcia
2019-02-21 14:53     ` Alberto Garcia
2019-01-17 15:34 ` [Qemu-devel] [PATCH 09/13] block: Add 'runtime_opts' and 'mutable_opts' fields to BlockDriver Alberto Garcia
2019-02-12 18:02   ` Kevin Wolf
2019-03-01 12:12     ` Alberto Garcia
2019-03-01 12:36       ` Kevin Wolf
2019-03-01 12:42         ` Alberto Garcia
2019-03-01 12:56           ` Kevin Wolf
2019-03-01 13:05             ` Alberto Garcia
2019-03-01 14:18               ` Kevin Wolf
2019-03-01 14:37                 ` Alberto Garcia
2019-01-17 15:34 ` [Qemu-devel] [PATCH 10/13] block: Add bdrv_reset_options_allowed() Alberto Garcia
2019-01-17 15:34 ` [Qemu-devel] [PATCH 11/13] block: Remove the AioContext parameter from bdrv_reopen_multiple() Alberto Garcia
2019-01-17 15:34 ` [Qemu-devel] [PATCH 12/13] block: Add an 'x-blockdev-reopen' QMP command Alberto Garcia
2019-01-17 15:34 ` [Qemu-devel] [PATCH 13/13] qemu-iotests: Test the x-blockdev-reopen " Alberto Garcia
2019-01-17 15:50 ` [Qemu-devel] [PATCH 00/13] Add a 'x-blockdev-reopen' " Eric Blake
2019-01-17 16:05   ` Alberto Garcia
2019-01-22  8:15   ` Vladimir Sementsov-Ogievskiy
2019-01-23 15:56     ` Alberto Garcia [this message]
2019-01-31 15:11 ` Alberto Garcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=w515zufmkpu.fsf@maestria.local.igalia.com \
    --to=berto@igalia.com \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.