[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/4] block: in commit, determine base image f
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/4] block: in commit, determine base image from the top image |
Date: |
Tue, 16 Oct 2012 11:31:42 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
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 <address@hidden>
>> ---
>> 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.