Re: [PATCH v3 32/33] block: Pass BdrvChildRole in remaining cases
Am 07.05.2020 um 11:36 hat Max Reitz geschrieben: > On 06.05.20 19:13, Kevin Wolf wrote: > > Am 18.02.2020 um 13:42 hat Max Reitz geschrieben: > >> These calls have no real use for the child role yet, but it will not > >> harm to give one. > >> > >> Notably, the bdrv_root_attach_child() call in blockjob.c is left > >> unmodified because there is not much the generic BlockJob object wants > >> from its children. > >> > >> Signed-off-by: Max Reitz > >> Reviewed-by: Eric Blake > > > >> diff --git a/block/vvfat.c b/block/vvfat.c > >> index 8f4ff5a97e..d4f4218924 100644 > >> --- a/block/vvfat.c > >> +++ b/block/vvfat.c > >> @@ -3186,7 +3186,7 @@ static int enable_write_target(BlockDriverState *bs, > >> Error **errp) > >> options = qdict_new(); > >> qdict_put_str(options, "write-target.driver", "qcow"); > >> s->qcow = bdrv_open_child(s->qcow_filename, options, "write-target", > >> bs, > >> - &child_vvfat_qcow, 0, false, errp); > >> + &child_vvfat_qcow, BDRV_CHILD_DATA, false, > >> errp); > > > > Doesn't it contain metadata, too? > > Aw, I don’t know... This is vvfat, I don’t want to know. > > Do you mean metadata beyond the filesystem structures? Are those > structures data or metadata in this context? Does it even matter? I can't say I understand what the qcow node is even used for in detail. vvfat checks the allocation status in the qcow node in a few places, does this count as metadata? > I suppose I just don’t want to think about all of that, and the simplest > way to do it is to indeed pass METADATA, too. Yep, that was my thinking. If we can't decide whether it's just DATA or also METADATA and want to err on the safe side, setting both should do. Kevin signature.asc Description: PGP signature
Re: [PATCH v3 32/33] block: Pass BdrvChildRole in remaining cases
On 06.05.20 19:13, Kevin Wolf wrote: > Am 18.02.2020 um 13:42 hat Max Reitz geschrieben: >> These calls have no real use for the child role yet, but it will not >> harm to give one. >> >> Notably, the bdrv_root_attach_child() call in blockjob.c is left >> unmodified because there is not much the generic BlockJob object wants >> from its children. >> >> Signed-off-by: Max Reitz >> Reviewed-by: Eric Blake > >> diff --git a/block/vvfat.c b/block/vvfat.c >> index 8f4ff5a97e..d4f4218924 100644 >> --- a/block/vvfat.c >> +++ b/block/vvfat.c >> @@ -3186,7 +3186,7 @@ static int enable_write_target(BlockDriverState *bs, >> Error **errp) >> options = qdict_new(); >> qdict_put_str(options, "write-target.driver", "qcow"); >> s->qcow = bdrv_open_child(s->qcow_filename, options, "write-target", bs, >> - &child_vvfat_qcow, 0, false, errp); >> + &child_vvfat_qcow, BDRV_CHILD_DATA, false, >> errp); > > Doesn't it contain metadata, too? Aw, I don’t know... This is vvfat, I don’t want to know. Do you mean metadata beyond the filesystem structures? Are those structures data or metadata in this context? Does it even matter? I suppose I just don’t want to think about all of that, and the simplest way to do it is to indeed pass METADATA, too. Max signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 32/33] block: Pass BdrvChildRole in remaining cases
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben: > These calls have no real use for the child role yet, but it will not > harm to give one. > > Notably, the bdrv_root_attach_child() call in blockjob.c is left > unmodified because there is not much the generic BlockJob object wants > from its children. > > Signed-off-by: Max Reitz > Reviewed-by: Eric Blake > diff --git a/block/vvfat.c b/block/vvfat.c > index 8f4ff5a97e..d4f4218924 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -3186,7 +3186,7 @@ static int enable_write_target(BlockDriverState *bs, > Error **errp) > options = qdict_new(); > qdict_put_str(options, "write-target.driver", "qcow"); > s->qcow = bdrv_open_child(s->qcow_filename, options, "write-target", bs, > - &child_vvfat_qcow, 0, false, errp); > + &child_vvfat_qcow, BDRV_CHILD_DATA, false, > errp); Doesn't it contain metadata, too? Kevin
[PATCH v3 32/33] block: Pass BdrvChildRole in remaining cases
These calls have no real use for the child role yet, but it will not
harm to give one.
Notably, the bdrv_root_attach_child() call in blockjob.c is left
unmodified because there is not much the generic BlockJob object wants
from its children.
Signed-off-by: Max Reitz
Reviewed-by: Eric Blake
---
block/block-backend.c | 11 +++
block/vvfat.c | 2 +-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 9e0078bfb5..e655e15675 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -401,8 +401,9 @@ BlockBackend *blk_new_open(const char *filename, const char
*reference,
return NULL;
}
-blk->root = bdrv_root_attach_child(bs, "root", &child_root, 0, blk->ctx,
- perm, BLK_PERM_ALL, blk, errp);
+blk->root = bdrv_root_attach_child(bs, "root", &child_root,
+ BDRV_CHILD_FILTERED |
BDRV_CHILD_PRIMARY,
+ blk->ctx, perm, BLK_PERM_ALL, blk,
errp);
if (!blk->root) {
blk_unref(blk);
return NULL;
@@ -812,8 +813,10 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs,
Error **errp)
{
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
bdrv_ref(bs);
-blk->root = bdrv_root_attach_child(bs, "root", &child_root, 0, blk->ctx,
- blk->perm, blk->shared_perm, blk, errp);
+blk->root = bdrv_root_attach_child(bs, "root", &child_root,
+ BDRV_CHILD_FILTERED |
BDRV_CHILD_PRIMARY,
+ blk->ctx, blk->perm, blk->shared_perm,
+ blk, errp);
if (blk->root == NULL) {
return -EPERM;
}
diff --git a/block/vvfat.c b/block/vvfat.c
index 8f4ff5a97e..d4f4218924 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3186,7 +3186,7 @@ static int enable_write_target(BlockDriverState *bs,
Error **errp)
options = qdict_new();
qdict_put_str(options, "write-target.driver", "qcow");
s->qcow = bdrv_open_child(s->qcow_filename, options, "write-target", bs,
- &child_vvfat_qcow, 0, false, errp);
+ &child_vvfat_qcow, BDRV_CHILD_DATA, false, errp);
qobject_unref(options);
if (!s->qcow) {
ret = -EINVAL;
--
2.24.1
