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 <jc...@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.
