Re: [Qemu-devel] [PATCH 09/10] block: Handle permission errors in change_parent_backing_link()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1656 bytes --]

On 03/06/2017 10:22 AM, Kevin Wolf wrote:
> Instead of just trying to change parents by parent over to reference @to
> instead of @from, and abort()ing whenever the permissions don't allow
> this, do proper permission checking beforehand and pass any error to the
> callers.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 44 insertions(+), 6 deletions(-)
> 

> +    /* Now actually perform the change. We performed the permission check for
> +     * all elements of @list at once, so set the permissions all at once at the
> +     * very end. */
> +    for (p = list; p != NULL; p = p->next) {
> +        c = p->data;
>  
>          bdrv_ref(to);
> -        /* FIXME Are we sure that bdrv_replace_child() can't run into
> -         * &error_abort because of permissions? */
> -        bdrv_replace_child(c, to, true);
> +        bdrv_replace_child_noperm(c, to);
>          bdrv_unref(from);

Looks awesome to get rid of a FIXME...

> @@ -2995,7 +3032,8 @@ void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState *new)
>  
>      bdrv_ref(old);
>  
> -    change_parent_backing_link(old, new);
> +    /* FIXME Proper error handling */
> +    change_parent_backing_link(old, new, &error_abort);

...until this shows we are merely trimming it down to one less instance,
but are still fighting the overall battle.  But we'll get there in the
end :)

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]