Re: [Qemu-devel] [PATCH v2 02/18] drive-backup: create do_backup_common

[John Snow]

On 7/4/19 11:06 AM, Max Reitz wrote:
> On 03.07.19 23:55, John Snow wrote:
>> Create a common core that comprises the actual meat of what the backup API
>> boundary needs to do, and then switch drive-backup to use it.
>>
>> Questions:
>>  - do_drive_backup now acquires and releases the aio_context in addition
>>    to do_backup_common doing the same. Can I drop this from drive_backup?
> 
> I wonder why you don’t just make it a requirement that
> do_backup_common() is called with the context acquired?
> 

Ehm, it just honestly didn't occur to me that I could acquire the
context before doing the input sanitizing.

In this case, it is OK to do it, so I will hoist it back up into
do_blockdev_backup.

--js

>> Signed-off-by: John Snow <address@hidden>
>> ---
>>  blockdev.c | 138 ++++++++++++++++++++++++++++++++---------------------
>>  1 file changed, 83 insertions(+), 55 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 4d141e9a1f..5fd663a7e5 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -3425,6 +3425,86 @@ out:
>>      aio_context_release(aio_context);
>>  }
>>  
>> +static BlockJob *do_backup_common(BackupCommon *backup,
>> +                                  BlockDriverState *target_bs,
>> +                                  JobTxn *txn, Error **errp)
>> +{
> 
> [...]
> 
>> +    job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
>> +                            backup->sync, bmap, backup->compress,
>> +                            backup->on_source_error, 
>> backup->on_target_error,
>> +                            job_flags, NULL, NULL, txn, &local_err);
>> +    if (local_err != NULL) {
>> +        error_propagate(errp, local_err);
>> +        goto out;
>> +    }
> 
> Below, you change do_drive_backup() to just pass errp instead of
> local_err and not do error handling.  Why not do the same here?
> 

Inertia.

> Other than that:
> 
> Reviewed-by: Max Reitz <address@hidden>
> 

Suggestions applied, thank you.