Re: [Qemu-devel] [PATCH v2 2/4] block: in commit, determine base image from the top image

[Jeff Cody <jcody@redhat.com>]

On 10/16/2012 11:22 AM, Eric Blake wrote:
> On 10/16/2012 08:44 AM, Jeff Cody wrote:
>> This simplifies some code and error checking, and also fixes a bug.
>>
>> bdrv_find_backing_image() should only be passed absolute filenames,
>> or filenames relative to the chain.  In the QMP message handler for
>> block commit, when looking up the base do so from the determined top
>> image, so we know it is reachable from top.
>>
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>> ---
>>  block/commit.c |  9 ---------
>>  blockdev.c     | 21 +++++++++++----------
>>  2 files changed, 11 insertions(+), 19 deletions(-)
> 
> As long as you are touching this code:
> 
>> @@ -1182,6 +1172,17 @@ void qmp_block_commit(const char *device,
>>          return;
>>      }
>>  
>> +    if (base && has_base) {
> 
> please swap this to 'has_base && base' to avoid any potential of
> valgrind warnings about conditional jump based on uninitialized memory.
>

OK.

> Also, I raised another bug[1] about a bad error message regarding
> top_bs, if the user passes a different spelling than the canonical name
> of the active image.  Is that worth fixing in this series, or is it okay
> to leave it until you actually add support for committing the top image?
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg02067.html
> 

Since this doesn't pose an actual issue now, I was planning on just
addressing that with the stage-2 patches, since that will likely touch
other related areas of the code anyway.  If you have major heartburn
over this, I'll change it now.