qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] mirror: prevent 'top' mode mirroring when no ba


From: sochin.jiang
Subject: Re: [Qemu-devel] [PATCH] mirror: prevent 'top' mode mirroring when no backing file specified on the destination
Date: Tue, 20 Dec 2016 08:44:32 +0800
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0

 I got the idea, thanks, Max.


 Sochin.Jiang

On 2016/12/19 23:31, Max Reitz wrote:
> On 19.12.2016 23:38, sochin jiang wrote:
>>  Mirroring using 'top' mode without backing file specified on the target can 
>> be success,
>>  but end with a disaster.
>>
>>  For example:
>>    Migration can be success in this situation while the virtual machine on 
>> the destination
>>  is no longer usable because of backing lost.
>>
>>  Remind the user earlier and return error in case of misoperation.
>>
>> Signed-off-by: sochin jiang <address@hidden>
>> ---
>>  block/mirror.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/block/mirror.c b/block/mirror.c
>> index 301ba92..3476696 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -1038,6 +1038,12 @@ void mirror_start(const char *job_id, 
>> BlockDriverState *bs,
>>          error_setg(errp, "Sync mode 'incremental' not supported");
>>          return;
>>      }
>> +    if (mode == MIRROR_SYNC_MODE_TOP && !backing_bs(target))
>> +    {
> 
> Syntactic issue: The opening brace should be on the same line as the "if".
> 
>> +        error_setg(errp, "Target Backing required using Sync mode 'top'");
>> +        return;
>> +    }
>> +
>>      is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
>>      base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
>>      mirror_start_job(job_id, bs, BLOCK_JOB_DEFAULT, target, replaces,
> 
> General issue: For blockdev-mirror, I think this is a legitimate use
> case. As far as I'm aware, libvirt uses the mirror block job for all
> backups -- they do so by cancelling the block job after the
> BLOCK_JOB_READY event instead of letting it complete. So a user might
> want to mirror some drive somewhere else (in sync=top mode, with the new
> location not yet having assigned a backing file to it), then cancel the
> block job after BLOCK_JOB_READY and only assign the backing file at some
> later point.
> 
> An even greater issue is that qmp_drive_mirror() opens the target BDS
> with BDRV_O_NO_BACKING. Therefore, this will always error out with
> drive-mirror and sync=top (unless the source image does not have a
> backing file, in which case the sync=top will silently be converted to
> sync=full).
> 
> For drive-mirror, the target's backing chain will not be set up until
> mirror_complete().
> 
> Max
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]