Re: [Qemu-devel] [PATCH v6 04/42] block: Add child access functions

[Kevin Wolf]

Am 10.09.2019 um 14:59 hat Max Reitz geschrieben:
> On 10.09.19 14:48, Kevin Wolf wrote:
> > Am 10.09.2019 um 13:36 hat Max Reitz geschrieben:
> >> On 10.09.19 12:47, Kevin Wolf wrote:
> >>> Am 10.09.2019 um 11:14 hat Max Reitz geschrieben:
> >>>> Maybe we should stop declaring Quorum a filter and then rename the
> >>>> bdrv_recurse_is_first_non_filter() to, I don’t know,
> >>>> bdrv_recurse_can_be_replaced_by_mirror()?
> >>>
> >>> Why not.
> >>
> >> It feels difficult to do in this series because this is a whole new can
> >> of worms.
> >>
> >> In patch 35, I actually replace the mirror use case by
> >> is_filtered_child().  So it looks to me as if that should not be done,
> >> because I should instead fix bdrv_recurse_is_first_non_filter() (and
> >> rename it), because quorum does allow replacing its children by mirror,
> >> even if it does not act as a filter for them.
> >>
> >> OTOH, there are other users of bdrv_is_first_non_filter().  Those are
> >> qmp_block_resize() and external_snapshot_prepare(), who throw an error
> >> if that returns false.
> >>
> >> I think that’s just wrong.  First of all, I don’t even know why we have
> >> that restriction anymore (I can imagine why it used to make sense before
> >> the permission system).  qmp_block_resize() should always work as long
> >> as it can get BLK_PERM_RESIZE; and I don’t know why the parents of some
> >> node would care if you take a snapshot of their child.
> > 
> > Hm, doesn't it make sense in a way for qmp_block_resize() at least? It
> > means that you can't resize just a filter, but you need to resize the
> > image that actually provides the data for the filter.
> 
> Filters generally implement .bdrv_truncate() by passing it through, so
> it should be fine.

Good point.

Then checking bdrv_is_first_non_filter() probably just forbids the only
command that would actually work correctly (resizing the top-level
filter).

Kevin

signature.asc
Description: PGP signature