diff mbox

[v1,1/1] vmstate: fix failed iotests case 68 and 91

Message ID 20170310044402.38880-2-haoqf@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hao QingFeng March 10, 2017, 4:44 a.m. UTC
This problem affects s390x only if we are running without KVM.
Basically, S390CPU.irqstate is unused if we do not use KVM,
and thus no buffer is allocated.
This causes size=0, first_elem=NULL and n_elems=1 in
vmstate_load_state and vmstate_save_state. And the assert fails.
With this fix we can go back to the old behavior and support
VMS_VBUFFER with size 0 and nullptr.

Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 migration/vmstate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Dr. David Alan Gilbert March 14, 2017, 2:13 p.m. UTC | #1
* QingFeng Hao (haoqf@linux.vnet.ibm.com) wrote:
> This problem affects s390x only if we are running without KVM.
> Basically, S390CPU.irqstate is unused if we do not use KVM,
> and thus no buffer is allocated.
> This causes size=0, first_elem=NULL and n_elems=1 in
> vmstate_load_state and vmstate_save_state. And the assert fails.
> With this fix we can go back to the old behavior and support
> VMS_VBUFFER with size 0 and nullptr.
> 
> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>

Thanks, and fixes problem with vmxnet3 migration.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave

> ---
>  migration/vmstate.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 78b3cd4..7b4a607 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -109,7 +109,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
>              vmstate_handle_alloc(first_elem, field, opaque);
>              if (field->flags & VMS_POINTER) {
>                  first_elem = *(void **)first_elem;
> -                assert(first_elem  || !n_elems);
> +                assert(first_elem || !n_elems || !size);
>              }
>              for (i = 0; i < n_elems; i++) {
>                  void *curr_elem = first_elem + size * i;
> @@ -117,7 +117,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
>                  if (field->flags & VMS_ARRAY_OF_POINTER) {
>                      curr_elem = *(void **)curr_elem;
>                  }
> -                if (!curr_elem) {
> +                if (!curr_elem && size) {
>                      /* if null pointer check placeholder and do not follow */
>                      assert(field->flags & VMS_ARRAY_OF_POINTER);
>                      ret = vmstate_info_nullptr.get(f, curr_elem, size, NULL);
> @@ -325,7 +325,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>              trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
>              if (field->flags & VMS_POINTER) {
>                  first_elem = *(void **)first_elem;
> -                assert(first_elem  || !n_elems);
> +                assert(first_elem || !n_elems || !size);
>              }
>              for (i = 0; i < n_elems; i++) {
>                  void *curr_elem = first_elem + size * i;
> @@ -336,7 +336,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>                      assert(curr_elem);
>                      curr_elem = *(void **)curr_elem;
>                  }
> -                if (!curr_elem) {
> +                if (!curr_elem && size) {
>                      /* if null pointer write placeholder and do not follow */
>                      assert(field->flags & VMS_ARRAY_OF_POINTER);
>                      vmstate_info_nullptr.put(f, curr_elem, size, NULL, NULL);
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Hao QingFeng March 15, 2017, 1:21 a.m. UTC | #2
在 2017/3/14 22:13, Dr. David Alan Gilbert 写道:
> * QingFeng Hao (haoqf@linux.vnet.ibm.com) wrote:
>> This problem affects s390x only if we are running without KVM.
>> Basically, S390CPU.irqstate is unused if we do not use KVM,
>> and thus no buffer is allocated.
>> This causes size=0, first_elem=NULL and n_elems=1 in
>> vmstate_load_state and vmstate_save_state. And the assert fails.
>> With this fix we can go back to the old behavior and support
>> VMS_VBUFFER with size 0 and nullptr.
>>
>> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Thanks, and fixes problem with vmxnet3 migration.
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Thank you, Dave!
>
> Dave
>
>> ---
>>   migration/vmstate.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/migration/vmstate.c b/migration/vmstate.c
>> index 78b3cd4..7b4a607 100644
>> --- a/migration/vmstate.c
>> +++ b/migration/vmstate.c
>> @@ -109,7 +109,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
>>               vmstate_handle_alloc(first_elem, field, opaque);
>>               if (field->flags & VMS_POINTER) {
>>                   first_elem = *(void **)first_elem;
>> -                assert(first_elem  || !n_elems);
>> +                assert(first_elem || !n_elems || !size);
>>               }
>>               for (i = 0; i < n_elems; i++) {
>>                   void *curr_elem = first_elem + size * i;
>> @@ -117,7 +117,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
>>                   if (field->flags & VMS_ARRAY_OF_POINTER) {
>>                       curr_elem = *(void **)curr_elem;
>>                   }
>> -                if (!curr_elem) {
>> +                if (!curr_elem && size) {
>>                       /* if null pointer check placeholder and do not follow */
>>                       assert(field->flags & VMS_ARRAY_OF_POINTER);
>>                       ret = vmstate_info_nullptr.get(f, curr_elem, size, NULL);
>> @@ -325,7 +325,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>>               trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
>>               if (field->flags & VMS_POINTER) {
>>                   first_elem = *(void **)first_elem;
>> -                assert(first_elem  || !n_elems);
>> +                assert(first_elem || !n_elems || !size);
>>               }
>>               for (i = 0; i < n_elems; i++) {
>>                   void *curr_elem = first_elem + size * i;
>> @@ -336,7 +336,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>>                       assert(curr_elem);
>>                       curr_elem = *(void **)curr_elem;
>>                   }
>> -                if (!curr_elem) {
>> +                if (!curr_elem && size) {
>>                       /* if null pointer write placeholder and do not follow */
>>                       assert(field->flags & VMS_ARRAY_OF_POINTER);
>>                       vmstate_info_nullptr.put(f, curr_elem, size, NULL, NULL);
>> -- 
>> 1.8.3.1
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
Juan Quintela March 16, 2017, 8:01 a.m. UTC | #3
QingFeng Hao <haoqf@linux.vnet.ibm.com> wrote:
> This problem affects s390x only if we are running without KVM.
> Basically, S390CPU.irqstate is unused if we do not use KVM,
> and thus no buffer is allocated.
> This causes size=0, first_elem=NULL and n_elems=1 in
> vmstate_load_state and vmstate_save_state. And the assert fails.
> With this fix we can go back to the old behavior and support
> VMS_VBUFFER with size 0 and nullptr.
>
> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>

queued
Hao QingFeng March 17, 2017, 2:32 a.m. UTC | #4
在 2017/3/16 16:01, Juan Quintela 写道:
> QingFeng Hao <haoqf@linux.vnet.ibm.com> wrote:
>> This problem affects s390x only if we are running without KVM.
>> Basically, S390CPU.irqstate is unused if we do not use KVM,
>> and thus no buffer is allocated.
>> This causes size=0, first_elem=NULL and n_elems=1 in
>> vmstate_load_state and vmstate_save_state. And the assert fails.
>> With this fix we can go back to the old behavior and support
>> VMS_VBUFFER with size 0 and nullptr.
>>
>> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> queued
Thanks!
diff mbox

Patch

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 78b3cd4..7b4a607 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -109,7 +109,7 @@  int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
             vmstate_handle_alloc(first_elem, field, opaque);
             if (field->flags & VMS_POINTER) {
                 first_elem = *(void **)first_elem;
-                assert(first_elem  || !n_elems);
+                assert(first_elem || !n_elems || !size);
             }
             for (i = 0; i < n_elems; i++) {
                 void *curr_elem = first_elem + size * i;
@@ -117,7 +117,7 @@  int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                 if (field->flags & VMS_ARRAY_OF_POINTER) {
                     curr_elem = *(void **)curr_elem;
                 }
-                if (!curr_elem) {
+                if (!curr_elem && size) {
                     /* if null pointer check placeholder and do not follow */
                     assert(field->flags & VMS_ARRAY_OF_POINTER);
                     ret = vmstate_info_nullptr.get(f, curr_elem, size, NULL);
@@ -325,7 +325,7 @@  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
             trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
             if (field->flags & VMS_POINTER) {
                 first_elem = *(void **)first_elem;
-                assert(first_elem  || !n_elems);
+                assert(first_elem || !n_elems || !size);
             }
             for (i = 0; i < n_elems; i++) {
                 void *curr_elem = first_elem + size * i;
@@ -336,7 +336,7 @@  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                     assert(curr_elem);
                     curr_elem = *(void **)curr_elem;
                 }
-                if (!curr_elem) {
+                if (!curr_elem && size) {
                     /* if null pointer write placeholder and do not follow */
                     assert(field->flags & VMS_ARRAY_OF_POINTER);
                     vmstate_info_nullptr.put(f, curr_elem, size, NULL, NULL);