Re: [RFC v3 2/2] migration: Update error description whenever migration fails

[Tejus GK]

On 18/05/23 5:22 pm, Juan Quintela wrote:
> Tejus GK <tejus.gk@nutanix.com> wrote:
>> There are places outside of migration.c which eventually leads to a
>> migration failure, but the failure reason is never updated. Hence
>> libvirt doesn't know why the migration failed when it queries for it.
>>
>> Signed-off-by: Tejus GK <tejus.gk@nutanix.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>

Thank you for the reviews Juan, but I believe that this particular patch 
shouldn't be approved yet. I have mentioned it in the RFC cover letter that the 
changes in this patch, in the file vmstate.c, end up breaking the build for a 
unit-test, eventually breaking the entire build. 

I was not sure how to implement the error reporting properly in such cases, and 
the aim of this patch was to receive advice on the same.  
> 
> 
> If you have to respin:
> 
>> @@ -1456,6 +1460,7 @@ int 
>> qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>>      int vmdesc_len;
>>      SaveStateEntry *se;
>>      int ret;
>> +    Error *local_err = NULL;
> 
> You can declare this:
> 
>>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>>          if (se->vmsd && se->vmsd->early_setup) {
>> @@ -1475,8 +1480,10 @@ int 
>> qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>>           * bdrv_activate_all() on the other end won't fail. */
>>          ret = bdrv_inactivate_all();
>>          if (ret) {
> 
> here
> 
>> -            error_report("%s: bdrv_inactivate_all() failed (%d)",
>> -                         __func__, ret);
>