All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Alberto Garcia <berto@igalia.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen
Date: Thu, 25 Feb 2021 20:02:33 +0300	[thread overview]
Message-ID: <05d57ba7-684b-a897-5e7e-1ebb6350ca31@virtuozzo.com> (raw)
In-Reply-To: <20210224123305.GA11025@merkur.fritz.box>

24.02.2021 15:33, Kevin Wolf wrote:
> Am 09.02.2021 um 09:03 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 08.02.2021 21:44, Alberto Garcia wrote:
>>> Signed-off-by: Alberto Garcia <berto@igalia.com>
>>> ---
>>>    qapi/block-core.json       |  2 +-
>>>    include/block/block.h      |  1 +
>>>    block.c                    | 16 +++++--
>>>    blockdev.c                 | 85 +++++++++++++++++++++-----------------
>>>    tests/qemu-iotests/155     |  9 ++--
>>>    tests/qemu-iotests/165     |  4 +-
>>>    tests/qemu-iotests/245     | 27 +++++++-----
>>>    tests/qemu-iotests/248     |  2 +-
>>>    tests/qemu-iotests/248.out |  2 +-
>>>    tests/qemu-iotests/298     |  4 +-
>>>    10 files changed, 89 insertions(+), 63 deletions(-)
>>>
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index c0e7c23331..b9fcf20a81 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -4177,7 +4177,7 @@
>>>    # Since: 4.0
>>>    ##
>>>    { 'command': 'x-blockdev-reopen',
>>> -  'data': 'BlockdevOptions', 'boxed': true }
>>> +  'data': { 'options': ['BlockdevOptions'] } }
>>
>> Do we also want to drop x- prefix?
> 
> libvirt really wants to have a stable blockdev-reopen interface in 6.0
> because enabling the incremental backup code depends on this (they just
> toggle the readonly flag if I understand correctly, so most of the work
> we're currently doing isn't even relevant at this moment for libvirt).

Do you know what is the case exactly? If they do it to remove dirty bitmap
from backing image after snapshot operation, probably we'd better improve
block-dirty-bitmap-remove command to be able to reopen r-o image?

(I just recently faced such a task)

> 
> Given that the soft freeze is coming closer (March 16), I wonder if we
> should just make this API change and declare the interface stable. We
> can then make Vladimir's fixes and the file reopening on top of it - if
> it's in time for 6.0, that would be good, but if not we could move it to
> 6.1 without impacting libvirt.
> 
> I think we're reasonable confident that the QAPI interfaces are right,
> even if maybe not that all aspects of the implementation are right yet.
> 
> What do you think?
> 

I think it's OK.. We have it since 4.0. What will we win keeping -x for years? Even latest change from updating one device to several could be easily done with help of 'alternate' if the command was already stable.


-- 
Best regards,
Vladimir


  reply	other threads:[~2021-02-25 17:07 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-08 18:44 [RFC PATCH v2 0/4] Allow changing bs->file on reopen Alberto Garcia
2021-02-08 18:44 ` [RFC PATCH v2 1/4] block: " Alberto Garcia
2021-02-09  7:37   ` Vladimir Sementsov-Ogievskiy
2021-02-10 16:52   ` Kevin Wolf
2021-02-16 12:06     ` Alberto Garcia
2021-02-08 18:44 ` [RFC PATCH v2 2/4] iotests: Update 245 to support replacing files with x-blockdev-reopen Alberto Garcia
2021-02-10 17:17   ` Kevin Wolf
2021-02-08 18:44 ` [RFC PATCH v2 3/4] block: Support multiple reopening " Alberto Garcia
2021-02-09  8:03   ` Vladimir Sementsov-Ogievskiy
2021-02-16 16:33     ` Alberto Garcia
2021-02-24 12:33     ` Kevin Wolf
2021-02-25 17:02       ` Vladimir Sementsov-Ogievskiy [this message]
2021-03-01 11:07         ` Kevin Wolf
2021-03-01 11:57           ` Vladimir Sementsov-Ogievskiy
2021-03-01 12:21           ` Peter Krempa
2021-02-26 11:51       ` Alberto Garcia
2021-02-08 18:44 ` [RFC PATCH v2 4/4] iotests: Test reopening multiple devices at the same time Alberto Garcia
2021-02-09  7:15 ` [RFC PATCH v2 0/4] Allow changing bs->file on reopen Vladimir Sementsov-Ogievskiy
2021-02-10 17:26 ` Kevin Wolf
2021-02-12 14:41   ` Peter Krempa
2021-02-16 16:36   ` Alberto Garcia
2021-02-16 16:48     ` Kevin Wolf
2021-02-16 17:25       ` Alberto Garcia
2021-02-16 17:51         ` Kevin Wolf

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=05d57ba7-684b-a897-5e7e-1ebb6350ca31@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=berto@igalia.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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.