Re: [Qemu-devel] [PATCH] more check for replaced node

[Wen Congyang]

On 07/01/2015 04:39 PM, Stefan Hajnoczi wrote:
> On Thu, Jun 25, 2015 at 02:55:10PM +0800, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <address@hidden>
>> ---
>>  block.c               | 5 +++--
>>  block/mirror.c        | 3 ++-
>>  blockdev.c            | 2 +-
>>  include/block/block.h | 3 ++-
>>  4 files changed, 8 insertions(+), 5 deletions(-)
> 
> This patch is missing a commit description.  What is the justification
> for this change?

Sorry, I forgot to add commit messages..

Without this patch, the replace node can be any node, and it can be
top BDS with BB, or another quorum's child. With this patch, the replace node
must be this quorum's child.

Thanks
Wen Congyang

> 
> Leaving this patch to Jeff since I'm CC only but the email is to Jeff.
> 
>> diff --git a/block.c b/block.c
>> index 7168575..70ee0f6 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -4033,7 +4033,8 @@ bool bdrv_is_first_non_filter(BlockDriverState 
>> *candidate)
>>      return false;
>>  }
>>  
>> -BlockDriverState *check_to_replace_node(const char *node_name, Error **errp)
>> +BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
>> +                                        const char *node_name, Error **errp)
>>  {
>>      BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
>>      AioContext *aio_context;
>> @@ -4056,7 +4057,7 @@ BlockDriverState *check_to_replace_node(const char 
>> *node_name, Error **errp)
>>       * Another benefit is that this tests exclude backing files which are
>>       * blocked by the backing blockers.
>>       */
>> -    if (!bdrv_is_first_non_filter(to_replace_bs)) {
>> +    if (!bdrv_recurse_is_first_non_filter(parent_bs, to_replace_bs)) {
>>          error_setg(errp, "Only top most non filter can be replaced");
>>          to_replace_bs = NULL;
>>          goto out;
>> diff --git a/block/mirror.c b/block/mirror.c
>> index 0d06cc2..f132f35 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -612,7 +612,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
>>      if (s->replaces) {
>>          AioContext *replace_aio_context;
>>  
>> -        s->to_replace = check_to_replace_node(s->replaces, &local_err);
>> +        s->to_replace = check_to_replace_node(s->common.bs, s->replaces,
>> +                                              &local_err);
>>          if (!s->to_replace) {
>>              error_propagate(errp, local_err);
>>              return;
>> diff --git a/blockdev.c b/blockdev.c
>> index f3a3097..a0e13b0 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -2756,7 +2756,7 @@ void qmp_drive_mirror(const char *device, const char 
>> *target,
>>              goto out;
>>          }
>>  
>> -        to_replace_bs = check_to_replace_node(replaces, &local_err);
>> +        to_replace_bs = check_to_replace_node(bs, replaces, &local_err);
>>  
>>          if (!to_replace_bs) {
>>              error_propagate(errp, local_err);
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 07bb724..e0dad54 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -311,7 +311,8 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState 
>> *bs,
>>  bool bdrv_is_first_non_filter(BlockDriverState *candidate);
>>  
>>  /* check if a named node can be replaced when doing drive-mirror */
>> -BlockDriverState *check_to_replace_node(const char *node_name, Error 
>> **errp);
>> +BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
>> +                                        const char *node_name, Error 
>> **errp);
>>  
>>  /* async block I/O */
>>  typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,