diff mbox series

[v3,26/33] block: Use child_of_bds in remaining places

Message ID 20200218124242.584644-27-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show
Series block: Introduce real BdrvChildRole | expand

Commit Message

Max Reitz Feb. 18, 2020, 12:42 p.m. UTC 
Replace child_file by child_of_bds in all remaining places (excluding
tests).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block.c              |  3 ++-
 block/backup-top.c   |  4 ++--
 block/blklogwrites.c |  4 ++--
 block/blkreplay.c    |  5 +++--
 block/raw-format.c   | 15 +++++++++++++--
 5 files changed, 22 insertions(+), 9 deletions(-)

Comments

Kevin Wolf May 6, 2020, 5:04 p.m. UTC | #1 
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> Replace child_file by child_of_bds in all remaining places (excluding
> tests).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/block/blkreplay.c b/block/blkreplay.c
> index f97493f45a..71628f4d56 100644
> --- a/block/blkreplay.c
> +++ b/block/blkreplay.c
> @@ -27,8 +27,9 @@ static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
>      int ret;
>  
>      /* Open the image file */
> -    bs->file = bdrv_open_child(NULL, options, "image",
> -                               bs, &child_file, 0, false, &local_err);
> +    bs->file = bdrv_open_child(NULL, options, "image", bs, &child_of_bds,
> +                               BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY,
> +                               false, &local_err);

Why isn't blkreplay a filter?

Kevin
Max Reitz May 7, 2020, 9:33 a.m. UTC | #2 
On 06.05.20 19:04, Kevin Wolf wrote:
> Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
>> Replace child_file by child_of_bds in all remaining places (excluding
>> tests).
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
>> diff --git a/block/blkreplay.c b/block/blkreplay.c
>> index f97493f45a..71628f4d56 100644
>> --- a/block/blkreplay.c
>> +++ b/block/blkreplay.c
>> @@ -27,8 +27,9 @@ static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
>>      int ret;
>>  
>>      /* Open the image file */
>> -    bs->file = bdrv_open_child(NULL, options, "image",
>> -                               bs, &child_file, 0, false, &local_err);
>> +    bs->file = bdrv_open_child(NULL, options, "image", bs, &child_of_bds,
>> +                               BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY,
>> +                               false, &local_err);
> 
> Why isn't blkreplay a filter?
I don’t know, but that’s pre-existing.  (It doesn’t set .is_filter.)

I’m afraid I have little insight into what blkreplay actually does.  I
thought maybe in conjunction with the whole replay stuff it might turn
out not to be a filter.

So is it a filter?  If so, no problem.  I suppose we can fix it up in
the same patch that marks mirror-top and commit-top as filters.

Max
Kevin Wolf May 7, 2020, 11:32 a.m. UTC | #3 
Am 07.05.2020 um 11:33 hat Max Reitz geschrieben:
> On 06.05.20 19:04, Kevin Wolf wrote:
> > Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> >> Replace child_file by child_of_bds in all remaining places (excluding
> >> tests).
> >>
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >> Reviewed-by: Eric Blake <eblake@redhat.com>
> > 
> >> diff --git a/block/blkreplay.c b/block/blkreplay.c
> >> index f97493f45a..71628f4d56 100644
> >> --- a/block/blkreplay.c
> >> +++ b/block/blkreplay.c
> >> @@ -27,8 +27,9 @@ static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
> >>      int ret;
> >>  
> >>      /* Open the image file */
> >> -    bs->file = bdrv_open_child(NULL, options, "image",
> >> -                               bs, &child_file, 0, false, &local_err);
> >> +    bs->file = bdrv_open_child(NULL, options, "image", bs, &child_of_bds,
> >> +                               BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY,
> >> +                               false, &local_err);
> > 
> > Why isn't blkreplay a filter?
> I don’t know, but that’s pre-existing.  (It doesn’t set .is_filter.)
> 
> I’m afraid I have little insight into what blkreplay actually does.  I
> thought maybe in conjunction with the whole replay stuff it might turn
> out not to be a filter.
> 
> So is it a filter?  If so, no problem.  I suppose we can fix it up in
> the same patch that marks mirror-top and commit-top as filters.

I think it is one. If I understand correctly, it basically just changes
the timing of requests, but nothing about their data.

Adding .is_filter to it in the same patch with mirror-top/commit-top
makes sense to me.

Kevin
diff mbox series

Patch

diff --git a/block.c b/block.c
index 1f573f3815..d3beed1819 100644
--- a/block.c
+++ b/block.c
@@ -3242,7 +3242,8 @@  static BlockDriverState *bdrv_open_inherit(const char *filename,
         BlockDriverState *file_bs;
 
         file_bs = bdrv_open_child_bs(filename, options, "file", bs,
-                                     &child_file, 0, true, &local_err);
+                                     &child_of_bds, BDRV_CHILD_IMAGE,
+                                     true, &local_err);
         if (local_err) {
             goto fail;
         }
diff --git a/block/backup-top.c b/block/backup-top.c
index c173e61250..3d6f6530a2 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -210,8 +210,8 @@  BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
              source->supported_zero_flags);
 
     bdrv_ref(target);
-    state->target = bdrv_attach_child(top, target, "target", &child_file, 0,
-                                      errp);
+    state->target = bdrv_attach_child(top, target, "target", &child_of_bds,
+                                      BDRV_CHILD_DATA, errp);
     if (!state->target) {
         bdrv_unref(target);
         bdrv_unref(top);
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index 78b0c49460..3a57b273fc 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -167,8 +167,8 @@  static int blk_log_writes_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Open the log file */
-    s->log_file = bdrv_open_child(NULL, options, "log", bs, &child_file, 0,
-                                  false, &local_err);
+    s->log_file = bdrv_open_child(NULL, options, "log", bs, &child_of_bds,
+                                  BDRV_CHILD_METADATA, false, &local_err);
     if (local_err) {
         ret = -EINVAL;
         error_propagate(errp, local_err);
diff --git a/block/blkreplay.c b/block/blkreplay.c
index f97493f45a..71628f4d56 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -27,8 +27,9 @@  static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
     int ret;
 
     /* Open the image file */
-    bs->file = bdrv_open_child(NULL, options, "image",
-                               bs, &child_file, 0, false, &local_err);
+    bs->file = bdrv_open_child(NULL, options, "image", bs, &child_of_bds,
+                               BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY,
+                               false, &local_err);
     if (local_err) {
         ret = -EINVAL;
         error_propagate(errp, local_err);
diff --git a/block/raw-format.c b/block/raw-format.c
index 33f5942474..c6470e4622 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -444,6 +444,7 @@  static int raw_open(BlockDriverState *bs, QDict *options, int flags,
     BDRVRawState *s = bs->opaque;
     bool has_size;
     uint64_t offset, size;
+    BdrvChildRole file_role;
     int ret;
 
     ret = raw_read_options(options, &offset, &has_size, &size, errp);
@@ -451,8 +452,18 @@  static int raw_open(BlockDriverState *bs, QDict *options, int flags,
         return ret;
     }
 
-    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, 0,
-                               false, errp);
+    /*
+     * Without offset and a size limit, this driver behaves very much
+     * like a filter.  With any such limit, it does not.
+     */
+    if (offset || has_size) {
+        file_role = BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY;
+    } else {
+        file_role = BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY;
+    }
+
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
+                               file_role, false, errp);
     if (!bs->file) {
         return -EINVAL;
     }