17.05.2021 19:05, Max Reitz wrote:
On 17.05.21 08:44, Vladimir Sementsov-Ogievskiy wrote:
We are going to convert backup_top to full featured public filter,
which can be used in separate of backup job. Start from renaming from
"how it used" to "what it does".

Is this safe?  The name was externally visible in queries after all. (I’m not 
saying it is unsafe, I just don’t know and would like to know whether you’ve 
considered this already.)

(Regardless, renaming files and so on is fine, of course.)

Hmmm. I don't know.

It was visible yes.. But we've never documented it. And if someone depends on name of the 
format of the filter automatically inserted during backup job, it's a kind of 
"undocumented feature" use..

Another change I is changing child from backing to file in 11, from this point 
of view it's unsafe too. But ше even more reasonable than good name: having all 
public filters behave similar is a very good thing.

So, may be it a bit risky, but I think good interface worth that risk. And we always can 
say "sorry guys, but that was not documented, we didn't promise anything".

But I'm OK to go on with "backup-top" and "backing", is someone has strict 
opinion about this.


While updating comments in 283 iotest, drop and rephrase also things
about ".active", as this field is now dropped, and filter doesn't have
"inactive" mode.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
  block/{backup-top.h => copy-before-write.h} |  28 +++---
  block/backup.c                              |  22 ++---
  block/{backup-top.c => copy-before-write.c} | 100 ++++++++++----------
  MAINTAINERS                                 |   4 +-
  block/meson.build                           |   2 +-
  tests/qemu-iotests/283                      |  35 +++----
  tests/qemu-iotests/283.out                  |   4 +-
  7 files changed, 95 insertions(+), 100 deletions(-)
  rename block/{backup-top.h => copy-before-write.h} (56%)
  rename block/{backup-top.c => copy-before-write.c} (62%)

[...]

diff --git a/block/backup-top.c b/block/copy-before-write.c
similarity index 62%
rename from block/backup-top.c
rename to block/copy-before-write.c
index 425e3778be..40e91832d7 100644
--- a/block/backup-top.c
+++ b/block/copy-before-write.c

[...]

@@ -32,25 +32,25 @@

[...]

-static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
-                                       uint64_t bytes, BdrvRequestFlags flags)
+static coroutine_fn int cbw_cbw(BlockDriverState *bs, uint64_t offset,
+                                uint64_t bytes, BdrvRequestFlags flags)

I’m sure you noticed it, too, but cbw_cbw() is weird.  Perhaps cbw_do_cbw() at 
least?


OK. Maybe even cbw_do_copy_before_write()


--
Best regards,
Vladimir

Reply via email to