[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when sea
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames |
Date: |
Wed, 25 Jan 2017 13:48:45 -0500 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, Jan 25, 2017 at 07:27:07PM +0100, Max Reitz wrote:
> On 25.01.2017 18:22, 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(+)
> >
> > 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;
> >
> > 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;
> > }
> > + /* 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);
>
> Oops, I was a bit too quick there. You should either follow Eric's
> remark about the scope, or you have to reset local_error to NULL here.
>
OK. I'll submit a v2, and add an iotest to that as well.
>
> > + }
> > } else {
> > /* If not an absolute filename path, make it relative to the
> > current
> > * image's filename path */
> >
>
>