qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, vsementsov@virtuozzo.com,
	jsnow@redhat.com, mreitz@redhat.com, kwolf@redhat.com,
	den@openvz.org
Subject: [PATCH RFC 0/3] block: drop inherits_from
Date: Thu, 11 Mar 2021 18:15:02 +0300	[thread overview]
Message-ID: <20210311151505.206534-1-vsementsov@virtuozzo.com> (raw)

Hi all!

I now work on v3 for "block: update graph permissions update", and I'm at "[PATCH v2 28/36] block: add bdrv_set_backing_noperm() transaction action".

So, the problem is we should handle inherits_from carefully, and most probably it should be updated in bdrv_replace_child_noperm().. And then, bdrv_replace_child_noperm will become a transaction action, which should store old inherits_from to the transaction state for possible rollback.. Or something like this, I didn't try yet. I just thought, may be we can just drop inherits_from?

I decided to learn the thing a bit, and found that the only usage of inherits_from is to limit reopen process. When adding bs to reopen_queue we do add its children recursively, but only those which inherits from the bs.

That works so starting from

commit 67251a311371c4d22e803f151f47fe817175b6c3
Author: Kevin Wolf <kwolf@redhat.com>
Date:   Thu Apr 9 18:54:04 2015 +0200

    block: Fix reopen flag inheritance


The commit made two things:

1. reopen recursively all* children, not only .file. That's OK.

2. * : not all, but only that inherits_from bs.

[2] Means that we don't reopen some implicitely created children.. And, I want to ask, why?

For me it seems that if we have reopen process.. And bs involved. And it has a child.. And child role defines how that child should inherit options.. Why not to just inherit them?


I decided to check iotests with dropped inherits_from feature.

For ./check -qcow2 on tmpfs only three failed: 30, 245, 258, not bad!

30 and 258 are easily fixed by assuming that if filter driver don't have .bdrv_reopen_prepare handler, it default to noop.

For 245 behavior changes in some places but it seems correct to me. And we have to fix commit job to keep reference to its filter node, otherwise we crash when remove the backing link from node to commit-top-filter of underlying commit job, which is allowed now.


I started this text as a letter, but finally I've fixed problems in 245 and decided to send and RFC series. Probably I miss some core sense of inherits_from, so that's an RFC.. So, what do you think?


Vladimir Sementsov-Ogievskiy (3):
  block/commit: keep reference on commit_top_bs
  block: allow filters to be reopened without .bdrv_reopen_prepare
  block: drop inherits_from logic

 include/block/block_int.h  |  4 --
 block.c                    | 95 ++++++--------------------------------
 block/commit.c             |  8 ++--
 tests/qemu-iotests/245     | 36 +++++++++------
 tests/qemu-iotests/245.out |  8 +++-
 5 files changed, 47 insertions(+), 104 deletions(-)

-- 
2.29.2



             reply	other threads:[~2021-03-11 15:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-11 15:15 Vladimir Sementsov-Ogievskiy [this message]
2021-03-11 15:15 ` [PATCH 1/3] block/commit: keep reference on commit_top_bs Vladimir Sementsov-Ogievskiy
2021-03-11 15:15 ` [PATCH 2/3] block: allow filters to be reopened without .bdrv_reopen_prepare Vladimir Sementsov-Ogievskiy
2021-03-11 15:15 ` [PATCH 3/3] block: drop inherits_from logic Vladimir Sementsov-Ogievskiy
2021-03-11 17:09 ` [PATCH RFC 0/3] block: drop inherits_from Kevin Wolf
2021-03-11 17:28   ` Vladimir Sementsov-Ogievskiy

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=20210311151505.206534-1-vsementsov@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=den@openvz.org \
    --cc=jsnow@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).