Re: [Qemu-block] [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames

[Eric Blake]

On 01/25/2017 11:22 AM, Jeff Cody wrote:
> In bdrv_find_backing_image(), if we are searching an image for a backing
> file that contains a protocol, we currently only compare unmodified
> paths.
> 
> However, some management software will change the backing filename to be
> a relative filename in a path.  QEMU is able to handle this fine,
> because internally it will use path_combine to put together the full
> protocol URI.
> 
> However, this can lead to an inability to match an image during a QAPI
> command that needs to use bdrv_find_backing_image() to find the image,
> when it is searched by the full URI.
> 
> When searching for a protocol filename, if the straight comparison
> fails, this patch will also compare against the full backing filename to
> see if that is a match.
> 
> Signed-off-by: Jeff Cody <address@hidden>
> ---
>  block.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Does this overlap with Max' work on 'block: Fix some filename generation
issues'?  But in isolation, it looks okay to me,

Reviewed-by: Eric Blake <address@hidden>

> 
> diff --git a/block.c b/block.c
> index 39ddea3..a173afc 100644
> --- a/block.c
> +++ b/block.c
> @@ -3145,6 +3145,7 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>      int is_protocol = 0;
>      BlockDriverState *curr_bs = NULL;
>      BlockDriverState *retval = NULL;
> +    Error *local_error = NULL;

I might have scoped this...

>  
>      if (!bs || !bs->drv || !backing_file) {
>          return NULL;
> @@ -3165,6 +3166,17 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>                  retval = curr_bs->backing->bs;
>                  break;
>              }

...as the first line within the if that uses it. But no big deal.

> +            /* Also check against the full backing filename for the image */
> +            bdrv_get_full_backing_filename(curr_bs, backing_file_full, 
> PATH_MAX,
> +                                           &local_error);
> +            if (local_error == NULL) {
> +                if (strcmp(backing_file, backing_file_full) == 0) {
> +                    retval = curr_bs->backing->bs;
> +                    break;
> +                }
> +            } else {
> +                error_free(local_error);
> +            }
>          } else {
>              /* If not an absolute filename path, make it relative to the 
> current
>               * image's filename path */
> 

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

signature.asc
Description: OpenPGP digital signature