[Qemu-devel] [PATCH v1 0/1] vmstate: fix failed iotests case 68 and 91

[Qemu-devel] [PATCH v1 0/1] vmstate: fix failed iotests case 68 and 91

[QingFeng Hao]

Hi All,
This patch is to fix the failed iotests case 68 and 91 and has been
tested. It's based on commit dd4d2578215 "Merge remote-tracking branch
'remotes/kraxel/tags/pull-fixes-20170309-1' into staging" and according
to Halil and Dave's comments. Also thanks for Fam and Kevin.

QingFeng Hao (1):
  vmstate: fix failed iotests case 68 and 91

 migration/vmstate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v1 1/1] vmstate: fix failed iotests case 68 and 91

[QingFeng Hao]

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(-)

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

Re: [Qemu-devel] [PATCH v1 1/1] vmstate: fix failed iotests case 68 and 91

[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>

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

Re: [Qemu-devel] [PATCH v1 1/1] vmstate: fix failed iotests case 68 and 91

[QingFeng Hao]

在 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
>

-- 
Regards
QingFeng Hao

Re: [Qemu-devel] [PATCH v1 1/1] vmstate: fix failed iotests case 68 and 91

[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

Re: [Qemu-devel] [PATCH v1 1/1] vmstate: fix failed iotests case 68 and 91

[QingFeng Hao]

在 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!

-- 
Regards
QingFeng Hao