| Message ID | 1431105726-3682-23-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08.05.2015 19:21, Kevin Wolf wrote: > Some drivers have nested options (e.g. blkdebug rule arrays), which > don't belong to a child node and shouldn't be removed. Don't remove all > options with "." in their name, but check for the complete prefixes of > actually existing child nodes. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 30 +++++++++++++++++++----------- > include/block/block_int.h | 1 + > 2 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/block.c b/block.c > index e329a47..e9a1d76 100644 > --- a/block.c > +++ b/block.c > @@ -81,7 +81,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers = > > static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, > const char *reference, QDict *options, int flags, > - BlockDriverState* parent, > + BlockDriverState* parent, const char *child_name, > const BdrvChildRole *child_role, > BlockDriver *drv, Error **errp); > > @@ -1174,8 +1174,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, > backing_hd = NULL; > ret = bdrv_open_inherit(&backing_hd, > *backing_filename ? backing_filename : NULL, > - reference, options, 0, bs, &child_backing, > - NULL, &local_err); > + reference, options, 0, bs, "backing", > + &child_backing, NULL, &local_err); > if (ret < 0) { > bs->open_flags |= BDRV_O_NO_BACKING; > error_setg(errp, "Could not open backing file: %s", > @@ -1238,7 +1238,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, > } > > ret = bdrv_open_inherit(pbs, filename, reference, image_options, 0, > - parent, child_role, NULL, errp); > + parent, bdref_key, child_role, NULL, errp); > > done: > qdict_del(options, bdref_key); > @@ -1312,11 +1312,13 @@ out: > > static void bdrv_attach_child(BlockDriverState *parent_bs, > BlockDriverState *child_bs, > + const char *child_name, > const BdrvChildRole *child_role) > { > BdrvChild *child = g_new(BdrvChild, 1); > *child = (BdrvChild) { > .bs = child_bs, > + .name = child_name, > .role = child_role, > }; > > @@ -1340,7 +1342,7 @@ static void bdrv_attach_child(BlockDriverState *parent_bs, > */ > static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, > const char *reference, QDict *options, int flags, > - BlockDriverState* parent, > + BlockDriverState* parent, const char *child_name, > const BdrvChildRole *child_role, > BlockDriver *drv, Error **errp) > { > @@ -1376,7 +1378,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, > } > bdrv_ref(bs); > if (child_role) { > - bdrv_attach_child(parent, bs, child_role); > + bdrv_attach_child(parent, bs, child_name, child_role); > } > *pbs = bs; > return 0; > @@ -1519,7 +1521,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, > } > > if (child_role) { > - bdrv_attach_child(parent, bs, child_role); > + bdrv_attach_child(parent, bs, child_name, child_role); > } > > QDECREF(options); > @@ -1563,7 +1565,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, > BlockDriver *drv, Error **errp) > { > return bdrv_open_inherit(pbs, filename, reference, options, flags, NULL, > - NULL, drv, errp); > + NULL, NULL, drv, errp); > } > > typedef struct BlockReopenQueueEntry { > @@ -2093,7 +2095,7 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) > /* The contents of 'tmp' will become bs_top, as we are > * swapping bs_new and bs_top contents. */ > bdrv_set_backing_hd(bs_top, bs_new); > - bdrv_attach_child(bs_top, bs_new, &child_backing); > + bdrv_attach_child(bs_top, bs_new, "backing", &child_backing); > } > > static void bdrv_delete(BlockDriverState *bs) > @@ -4037,13 +4039,19 @@ static bool append_open_options(QDict *d, BlockDriverState *bs) > { > const QDictEntry *entry; > QemuOptDesc *desc; > + BdrvChild *child; > bool found_any = false; > > for (entry = qdict_first(bs->options); entry; > entry = qdict_next(bs->options, entry)) > { > - /* Only take options for this level */ > - if (strchr(qdict_entry_key(entry), '.')) { > + /* Exclude options for children */ > + QLIST_FOREACH(child, &bs->children, next) { > + if (strstart(qdict_entry_key(entry), child->name, NULL)) { I think the prefix should be "#{child->name}.", tested like "if (strstart(..., &ptr) && *ptr == '.')". It doesn't matter now, so I'm not sure whether I can give an R-b anyway... I guess when in doubt, back out. So I won't. :-) Max > + break; > + } > + } > + if (child) { > continue; > } > > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 2fad5f8..90da3f7 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -336,6 +336,7 @@ extern const BdrvChildRole child_format; > > typedef struct BdrvChild { > BlockDriverState *bs; > + const char *name; > const BdrvChildRole *role; > QLIST_ENTRY(BdrvChild) next; > } BdrvChild;
On 13.05.2015 14:49, Max Reitz wrote: > On 08.05.2015 19:21, Kevin Wolf wrote: >> Some drivers have nested options (e.g. blkdebug rule arrays), which >> don't belong to a child node and shouldn't be removed. Don't remove all >> options with "." in their name, but check for the complete prefixes of >> actually existing child nodes. >> >> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >> --- >> block.c | 30 +++++++++++++++++++----------- >> include/block/block_int.h | 1 + >> 2 files changed, 20 insertions(+), 11 deletions(-) >> >> diff --git a/block.c b/block.c >> index e329a47..e9a1d76 100644 >> --- a/block.c >> +++ b/block.c >> @@ -81,7 +81,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers = >> static int bdrv_open_inherit(BlockDriverState **pbs, const char >> *filename, >> const char *reference, QDict *options, >> int flags, >> - BlockDriverState* parent, >> + BlockDriverState* parent, const char >> *child_name, >> const BdrvChildRole *child_role, >> BlockDriver *drv, Error **errp); >> @@ -1174,8 +1174,8 @@ int bdrv_open_backing_file(BlockDriverState >> *bs, QDict *parent_options, >> backing_hd = NULL; >> ret = bdrv_open_inherit(&backing_hd, >> *backing_filename ? backing_filename : >> NULL, >> - reference, options, 0, bs, &child_backing, >> - NULL, &local_err); >> + reference, options, 0, bs, "backing", >> + &child_backing, NULL, &local_err); >> if (ret < 0) { >> bs->open_flags |= BDRV_O_NO_BACKING; >> error_setg(errp, "Could not open backing file: %s", >> @@ -1238,7 +1238,7 @@ int bdrv_open_image(BlockDriverState **pbs, >> const char *filename, >> } >> ret = bdrv_open_inherit(pbs, filename, reference, >> image_options, 0, >> - parent, child_role, NULL, errp); >> + parent, bdref_key, child_role, NULL, errp); >> done: >> qdict_del(options, bdref_key); >> @@ -1312,11 +1312,13 @@ out: >> static void bdrv_attach_child(BlockDriverState *parent_bs, >> BlockDriverState *child_bs, >> + const char *child_name, >> const BdrvChildRole *child_role) >> { >> BdrvChild *child = g_new(BdrvChild, 1); >> *child = (BdrvChild) { >> .bs = child_bs, >> + .name = child_name, >> .role = child_role, >> }; >> @@ -1340,7 +1342,7 @@ static void >> bdrv_attach_child(BlockDriverState *parent_bs, >> */ >> static int bdrv_open_inherit(BlockDriverState **pbs, const char >> *filename, >> const char *reference, QDict *options, >> int flags, >> - BlockDriverState* parent, >> + BlockDriverState* parent, const char >> *child_name, >> const BdrvChildRole *child_role, >> BlockDriver *drv, Error **errp) >> { >> @@ -1376,7 +1378,7 @@ static int bdrv_open_inherit(BlockDriverState >> **pbs, const char *filename, >> } >> bdrv_ref(bs); >> if (child_role) { >> - bdrv_attach_child(parent, bs, child_role); >> + bdrv_attach_child(parent, bs, child_name, child_role); >> } >> *pbs = bs; >> return 0; >> @@ -1519,7 +1521,7 @@ static int bdrv_open_inherit(BlockDriverState >> **pbs, const char *filename, >> } >> if (child_role) { >> - bdrv_attach_child(parent, bs, child_role); >> + bdrv_attach_child(parent, bs, child_name, child_role); >> } >> QDECREF(options); >> @@ -1563,7 +1565,7 @@ int bdrv_open(BlockDriverState **pbs, const >> char *filename, >> BlockDriver *drv, Error **errp) >> { >> return bdrv_open_inherit(pbs, filename, reference, options, >> flags, NULL, >> - NULL, drv, errp); >> + NULL, NULL, drv, errp); >> } >> typedef struct BlockReopenQueueEntry { >> @@ -2093,7 +2095,7 @@ void bdrv_append(BlockDriverState *bs_new, >> BlockDriverState *bs_top) >> /* The contents of 'tmp' will become bs_top, as we are >> * swapping bs_new and bs_top contents. */ >> bdrv_set_backing_hd(bs_top, bs_new); >> - bdrv_attach_child(bs_top, bs_new, &child_backing); >> + bdrv_attach_child(bs_top, bs_new, "backing", &child_backing); >> } >> static void bdrv_delete(BlockDriverState *bs) >> @@ -4037,13 +4039,19 @@ static bool append_open_options(QDict *d, >> BlockDriverState *bs) >> { >> const QDictEntry *entry; >> QemuOptDesc *desc; >> + BdrvChild *child; >> bool found_any = false; >> for (entry = qdict_first(bs->options); entry; >> entry = qdict_next(bs->options, entry)) >> { >> - /* Only take options for this level */ >> - if (strchr(qdict_entry_key(entry), '.')) { >> + /* Exclude options for children */ >> + QLIST_FOREACH(child, &bs->children, next) { >> + if (strstart(qdict_entry_key(entry), child->name, NULL)) { > > I think the prefix should be "#{child->name}.", tested like "if > (strstart(..., &ptr) && *ptr == '.')". Make that "(*ptr == '.' || !*ptr)", I think. > It doesn't matter now, so I'm not sure whether I can give an R-b > anyway... I guess when in doubt, back out. So I won't. :-) > > Max > >> + break; >> + } >> + } >> + if (child) { >> continue; >> } >> diff --git a/include/block/block_int.h b/include/block/block_int.h >> index 2fad5f8..90da3f7 100644 >> --- a/include/block/block_int.h >> +++ b/include/block/block_int.h >> @@ -336,6 +336,7 @@ extern const BdrvChildRole child_format; >> typedef struct BdrvChild { >> BlockDriverState *bs; >> + const char *name; >> const BdrvChildRole *role; >> QLIST_ENTRY(BdrvChild) next; >> } BdrvChild; >
diff --git a/block.c b/block.c index e329a47..e9a1d76 100644 --- a/block.c +++ b/block.c @@ -81,7 +81,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers = static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, const char *reference, QDict *options, int flags, - BlockDriverState* parent, + BlockDriverState* parent, const char *child_name, const BdrvChildRole *child_role, BlockDriver *drv, Error **errp); @@ -1174,8 +1174,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, backing_hd = NULL; ret = bdrv_open_inherit(&backing_hd, *backing_filename ? backing_filename : NULL, - reference, options, 0, bs, &child_backing, - NULL, &local_err); + reference, options, 0, bs, "backing", + &child_backing, NULL, &local_err); if (ret < 0) { bs->open_flags |= BDRV_O_NO_BACKING; error_setg(errp, "Could not open backing file: %s", @@ -1238,7 +1238,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, } ret = bdrv_open_inherit(pbs, filename, reference, image_options, 0, - parent, child_role, NULL, errp); + parent, bdref_key, child_role, NULL, errp); done: qdict_del(options, bdref_key); @@ -1312,11 +1312,13 @@ out: static void bdrv_attach_child(BlockDriverState *parent_bs, BlockDriverState *child_bs, + const char *child_name, const BdrvChildRole *child_role) { BdrvChild *child = g_new(BdrvChild, 1); *child = (BdrvChild) { .bs = child_bs, + .name = child_name, .role = child_role, }; @@ -1340,7 +1342,7 @@ static void bdrv_attach_child(BlockDriverState *parent_bs, */ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, const char *reference, QDict *options, int flags, - BlockDriverState* parent, + BlockDriverState* parent, const char *child_name, const BdrvChildRole *child_role, BlockDriver *drv, Error **errp) { @@ -1376,7 +1378,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, } bdrv_ref(bs); if (child_role) { - bdrv_attach_child(parent, bs, child_role); + bdrv_attach_child(parent, bs, child_name, child_role); } *pbs = bs; return 0; @@ -1519,7 +1521,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, } if (child_role) { - bdrv_attach_child(parent, bs, child_role); + bdrv_attach_child(parent, bs, child_name, child_role); } QDECREF(options); @@ -1563,7 +1565,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, BlockDriver *drv, Error **errp) { return bdrv_open_inherit(pbs, filename, reference, options, flags, NULL, - NULL, drv, errp); + NULL, NULL, drv, errp); } typedef struct BlockReopenQueueEntry { @@ -2093,7 +2095,7 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) /* The contents of 'tmp' will become bs_top, as we are * swapping bs_new and bs_top contents. */ bdrv_set_backing_hd(bs_top, bs_new); - bdrv_attach_child(bs_top, bs_new, &child_backing); + bdrv_attach_child(bs_top, bs_new, "backing", &child_backing); } static void bdrv_delete(BlockDriverState *bs) @@ -4037,13 +4039,19 @@ static bool append_open_options(QDict *d, BlockDriverState *bs) { const QDictEntry *entry; QemuOptDesc *desc; + BdrvChild *child; bool found_any = false; for (entry = qdict_first(bs->options); entry; entry = qdict_next(bs->options, entry)) { - /* Only take options for this level */ - if (strchr(qdict_entry_key(entry), '.')) { + /* Exclude options for children */ + QLIST_FOREACH(child, &bs->children, next) { + if (strstart(qdict_entry_key(entry), child->name, NULL)) { + break; + } + } + if (child) { continue; } diff --git a/include/block/block_int.h b/include/block/block_int.h index 2fad5f8..90da3f7 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -336,6 +336,7 @@ extern const BdrvChildRole child_format; typedef struct BdrvChild { BlockDriverState *bs; + const char *name; const BdrvChildRole *role; QLIST_ENTRY(BdrvChild) next; } BdrvChild;
Some drivers have nested options (e.g. blkdebug rule arrays), which don't belong to a child node and shouldn't be removed. Don't remove all options with "." in their name, but check for the complete prefixes of actually existing child nodes. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 30 +++++++++++++++++++----------- include/block/block_int.h | 1 + 2 files changed, 20 insertions(+), 11 deletions(-)