Re: [Qemu-devel] [PATCH 12/18] savevm: add error parameter to qemu_loadvm_state()

[Pavel Hrdina <phrdina@redhat.com>]

On 08/30/2012 07:09 PM, Luiz Capitulino wrote:
> On Wed, 15 Aug 2012 09:41:53 +0200
> Pavel Hrdina <phrdina@redhat.com> wrote:
>
>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>> ---
>>   migration.c |  2 +-
>>   savevm.c    | 44 ++++++++++++++++++++++++++++----------------
>>   sysemu.h    |  3 ++-
>>   3 files changed, 31 insertions(+), 18 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index ec2f267..f048faf 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -88,7 +88,7 @@ int qemu_start_incoming_migration(const char *uri, Error **errp)
>>   
>>   void process_incoming_migration(QEMUFile *f)
>>   {
>> -    if (qemu_loadvm_state(f) < 0) {
>> +    if (qemu_loadvm_state(f, NULL) < 0) {
>>           fprintf(stderr, "load of migration failed\n");
>>           exit(0);
>>       }
>> diff --git a/savevm.c b/savevm.c
>> index 0d54115..500eb72 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -1916,7 +1916,8 @@ typedef struct LoadStateEntry {
>>       int version_id;
>>   } LoadStateEntry;
>>   
>> -int qemu_loadvm_state(QEMUFile *f)
>> +int qemu_loadvm_state(QEMUFile *f,
>> +                      Error **errp)
>>   {
>>       QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
>>           QLIST_HEAD_INITIALIZER(loadvm_handlers);
>> @@ -1925,21 +1926,26 @@ int qemu_loadvm_state(QEMUFile *f)
>>       unsigned int v;
>>       int ret;
>>   
>> -    if (qemu_savevm_state_blocked(NULL)) {
>> -        return -EINVAL;
>> +    if (qemu_savevm_state_blocked(errp)) {
>> +        return -ENOTSUP;
>>       }
>>   
>>       v = qemu_get_be32(f);
>> -    if (v != QEMU_VM_FILE_MAGIC)
>> +    if (v != QEMU_VM_FILE_MAGIC) {
>> +        error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> +                  "Unknown vm-state file magic");
>>           return -EINVAL;
>> +    }
>>   
>>       v = qemu_get_be32(f);
>>       if (v == QEMU_VM_FILE_VERSION_COMPAT) {
>> -        fprintf(stderr, "SaveVM v2 format is obsolete and don't work anymore\n");
>> +        error_set(errp, QERR_NOT_SUPPORTED);
>>           return -ENOTSUP;
>>       }
>> -    if (v != QEMU_VM_FILE_VERSION)
>> +    if (v != QEMU_VM_FILE_VERSION) {
>> +        error_set(errp, QERR_NOT_SUPPORTED);
>>           return -ENOTSUP;
>> +    }
>>   
>>       while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
>>           uint32_t instance_id, version_id, section_id;
>> @@ -1961,15 +1967,18 @@ int qemu_loadvm_state(QEMUFile *f)
>>               /* Find savevm section */
>>               se = find_se(idstr, instance_id);
>>               if (se == NULL) {
>> -                fprintf(stderr, "Unknown savevm section or instance '%s' %d\n", idstr, instance_id);
>> +                error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> +                          "Unknown savevm section or instance '%s' %d",
>> +                          idstr, instance_id);
>>                   ret = -EINVAL;
>>                   goto out;
>>               }
>>   
>>               /* Validate version */
>>               if (version_id > se->version_id) {
>> -                fprintf(stderr, "savevm: unsupported version %d for '%s' v%d\n",
>> -                        version_id, idstr, se->version_id);
>> +                error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> +                          "savevm: unsupported version %d for '%s' v%d",
>> +                          version_id, idstr, se->version_id);
>>                   ret = -EINVAL;
>>                   goto out;
>>               }
>> @@ -1984,8 +1993,7 @@ int qemu_loadvm_state(QEMUFile *f)
>>   
>>               ret = vmstate_load(f, le->se, le->version_id);
>>               if (ret < 0) {
>> -                fprintf(stderr, "qemu: warning: error while loading state for instance 0x%x of device '%s'\n",
>> -                        instance_id, idstr);
>> +                error_set(errp, QERR_GENERIC_ERROR, ret);
>>                   goto out;
>>               }
>>               break;
>> @@ -1999,20 +2007,21 @@ int qemu_loadvm_state(QEMUFile *f)
>>                   }
>>               }
>>               if (le == NULL) {
>> -                fprintf(stderr, "Unknown savevm section %d\n", section_id);
>> +                error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> +                          "Unknown savevm section %d", section_id);
> You sure that this error message will be printed to the terminal? This has
> to be done by the caller.

While loading snapshot yes, but while migrating no. I think, that we 
should rewrite migration error propagation too.
>
>>                   ret = -EINVAL;
>>                   goto out;
>>               }
>>   
>>               ret = vmstate_load(f, le->se, le->version_id);
>>               if (ret < 0) {
>> -                fprintf(stderr, "qemu: warning: error while loading state section id %d\n",
>> -                        section_id);
>> +                error_set(errp, QERR_GENERIC_ERROR, ret);
>>                   goto out;
>>               }
>>               break;
>>           default:
>> -            fprintf(stderr, "Unknown savevm section type %d\n", section_type);
>> +            error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> +                      "Unknown savevm section type %d", section_type);
>>               ret = -EINVAL;
>>               goto out;
>>           }
>> @@ -2030,6 +2039,9 @@ out:
>>   
>>       if (ret == 0) {
>>           ret = qemu_file_get_error(f);
>> +        if (ret < 0) {
>> +            error_set(errp, QERR_GENERIC_ERROR, ret);
>> +        }
>>       }
>>   
>>       return ret;
>> @@ -2297,7 +2309,7 @@ int load_vmstate(const char *name)
>>       }
>>   
>>       qemu_system_reset(VMRESET_SILENT);
>> -    ret = qemu_loadvm_state(f);
>> +    ret = qemu_loadvm_state(f, NULL);
>>   
>>       qemu_fclose(f);
>>       if (ret < 0) {
>> diff --git a/sysemu.h b/sysemu.h
>> index 35d6d67..651d96e 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -85,7 +85,8 @@ int qemu_savevm_state_iterate(QEMUFile *f,
>>   int qemu_savevm_state_complete(QEMUFile *f,
>>                                  Error **errp);
>>   void qemu_savevm_state_cancel(QEMUFile *f);
>> -int qemu_loadvm_state(QEMUFile *f);
>> +int qemu_loadvm_state(QEMUFile *f,
>> +                      Error **errp);
>>   
>>   /* SLIRP */
>>   void do_info_slirp(Monitor *mon);