Re: [Qemu-block] [Qemu-devel] [PATCH 08/10] block: Ignore multiple children in bdrv_check_update_perm()

[Eric Blake]

On 03/06/2017 10:22 AM, Kevin Wolf wrote:
> change_parent_backing_link() will need to update multiple BdrvChild
> objects at once. Checking permissions reference by reference doesn't
> work because permissions need to be consistent only with all parents
> moved to the new child.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block.c                   | 35 ++++++++++++++++++++++-------------
>  include/block/block_int.h |  2 +-
>  2 files changed, 23 insertions(+), 14 deletions(-)
> 


>  static int bdrv_check_update_perm(BlockDriverState *bs, uint64_t 
> new_used_perm,
>                                    uint64_t new_shared_perm,
> -                                  BdrvChild *ignore_child, Error **errp)
> +                                  GSList *ignore_children, Error **errp)
>  {
>      BdrvChild *c;
>      uint64_t cumulative_perms = new_used_perm;
> @@ -1572,7 +1574,7 @@ static int bdrv_check_update_perm(BlockDriverState *bs, 
> uint64_t new_used_perm,
>      assert(new_shared_perm & BLK_PERM_WRITE_UNCHANGED);
>  
>      QLIST_FOREACH(c, &bs->parents, next_parent) {
> -        if (c == ignore_child) {
> +        if (g_slist_find(ignore_children, c)) {

Quadratic complexity (searching one list for each element of another
list). Hopefully the combination of lists isn't too long in practice (it
might be a potential problem if someone creates 200 snapshots then tries
to commit them down to 1 image in a single operation, where
ignore_children would then be a long list of intermediate children being
iterated on multiple times, but I don't know if bs->parents can easily
be made as long).  We could always create some sort of hash table to
reduce complexity, as a followup patch, if it turns out to be a problem
in practice, but for now I'm okay with it.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature