[PATCH v2] dbus-vmstate: Increase the size of input stream buffer used during load

Priyankar Jain posted 1 patch 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/cdaad4718e62bf22fd5e93ef3e252de20da5c17c.1612273156.git.priyankar.jain@nutanix.com
backends/dbus-vmstate.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)

[PATCH v2] dbus-vmstate: Increase the size of input stream buffer used during load

Posted by Priyankar Jain 3 months ago
This commit fixes an issue where migration is failing in the load phase
because of a false alarm about data unavailability.

Following is the error received when the amount of data to be transferred
exceeds the default buffer size setup by G_BUFFERED_INPUT_STREAM(4KiB),
even when the maximum data size supported by this backend is 1MiB
(DBUS_VMSTATE_SIZE_LIMIT):

  dbus_vmstate_post_load: Invalid vmstate size: 4364
  qemu-kvm: error while loading state for instance 0x0 of device 'dbus-vmstate/dbus-vmstate'

This commit sets the size of the input stream buffer used during load to
DBUS_VMSTATE_SIZE_LIMIT which is the maximum amount of data a helper can
send during save phase.
Secondly, this commit makes sure that the input stream buffer is loaded before
checking the size of the data available in it, rectifying the false alarm about
data unavailability.

Fixes: 5010cec2bc87 ("Add dbus-vmstate object")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 backends/dbus-vmstate.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
index bd050e8..3b8a116 100644
--- a/backends/dbus-vmstate.c
+++ b/backends/dbus-vmstate.c
@@ -204,6 +204,8 @@ static int dbus_vmstate_post_load(void *opaque, int version_id)
     m = g_memory_input_stream_new_from_data(self->data, self->data_size, NULL);
     s = g_data_input_stream_new(m);
     g_data_input_stream_set_byte_order(s, G_DATA_STREAM_BYTE_ORDER_BIG_ENDIAN);
+    g_buffered_input_stream_set_buffer_size(G_BUFFERED_INPUT_STREAM(s),
+                                            DBUS_VMSTATE_SIZE_LIMIT);
 
     nelem = g_data_input_stream_read_uint32(s, NULL, &err);
     if (err) {
@@ -241,11 +243,23 @@ static int dbus_vmstate_post_load(void *opaque, int version_id)
         }
 
         len = g_data_input_stream_read_uint32(s, NULL, &err);
+        if (len > DBUS_VMSTATE_SIZE_LIMIT) {
+            error_report("%s: Invalid vmstate size: %u", __func__, len);
+            return -1;
+        }
+
+        g_buffered_input_stream_fill(G_BUFFERED_INPUT_STREAM(s), len, NULL,
+                                     &err);
+        if (err) {
+            goto error;
+        }
+
         avail = g_buffered_input_stream_get_available(
             G_BUFFERED_INPUT_STREAM(s));
-
-        if (len > DBUS_VMSTATE_SIZE_LIMIT || len > avail) {
-            error_report("%s: Invalid vmstate size: %u", __func__, len);
+        if (len > avail) {
+            error_report("%s: Not enough data available to load for Id: '%s'. "
+                "Available data size: %lu, Actual vmstate size: %u",
+                __func__, id, avail, len);
             return -1;
         }
 
-- 
1.8.3.1


Re: [PATCH v2] dbus-vmstate: Increase the size of input stream buffer used during load

Posted by priyankar jain 2 months, 3 weeks ago
Hi Marc,

Requesting you to please pull these changes. Without this fix, migration 
is failing in case any helper has more than 4KiB of data.

Thanks and Regards,
Priyankar Jain

On 02/02/21 7:24 pm, Priyankar Jain wrote:
> This commit fixes an issue where migration is failing in the load phase
> because of a false alarm about data unavailability.
> 
> Following is the error received when the amount of data to be transferred
> exceeds the default buffer size setup by G_BUFFERED_INPUT_STREAM(4KiB),
> even when the maximum data size supported by this backend is 1MiB
> (DBUS_VMSTATE_SIZE_LIMIT):
> 
>    dbus_vmstate_post_load: Invalid vmstate size: 4364
>    qemu-kvm: error while loading state for instance 0x0 of device 'dbus-vmstate/dbus-vmstate'
> 
> This commit sets the size of the input stream buffer used during load to
> DBUS_VMSTATE_SIZE_LIMIT which is the maximum amount of data a helper can
> send during save phase.
> Secondly, this commit makes sure that the input stream buffer is loaded before
> checking the size of the data available in it, rectifying the false alarm about
> data unavailability.
> 
> Fixes: 5010cec2bc87 ("Add dbus-vmstate object")
> Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   backends/dbus-vmstate.c | 20 +++++++++++++++++---
>   1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
> index bd050e8..3b8a116 100644
> --- a/backends/dbus-vmstate.c
> +++ b/backends/dbus-vmstate.c
> @@ -204,6 +204,8 @@ static int dbus_vmstate_post_load(void *opaque, int version_id)
>       m = g_memory_input_stream_new_from_data(self->data, self->data_size, NULL);
>       s = g_data_input_stream_new(m);
>       g_data_input_stream_set_byte_order(s, G_DATA_STREAM_BYTE_ORDER_BIG_ENDIAN);
> +    g_buffered_input_stream_set_buffer_size(G_BUFFERED_INPUT_STREAM(s),
> +                                            DBUS_VMSTATE_SIZE_LIMIT);
>   
>       nelem = g_data_input_stream_read_uint32(s, NULL, &err);
>       if (err) {
> @@ -241,11 +243,23 @@ static int dbus_vmstate_post_load(void *opaque, int version_id)
>           }
>   
>           len = g_data_input_stream_read_uint32(s, NULL, &err);
> +        if (len > DBUS_VMSTATE_SIZE_LIMIT) {
> +            error_report("%s: Invalid vmstate size: %u", __func__, len);
> +            return -1;
> +        }
> +
> +        g_buffered_input_stream_fill(G_BUFFERED_INPUT_STREAM(s), len, NULL,
> +                                     &err);
> +        if (err) {
> +            goto error;
> +        }
> +
>           avail = g_buffered_input_stream_get_available(
>               G_BUFFERED_INPUT_STREAM(s));
> -
> -        if (len > DBUS_VMSTATE_SIZE_LIMIT || len > avail) {
> -            error_report("%s: Invalid vmstate size: %u", __func__, len);
> +        if (len > avail) {
> +            error_report("%s: Not enough data available to load for Id: '%s'. "
> +                "Available data size: %lu, Actual vmstate size: %u",
> +                __func__, id, avail, len);
>               return -1;
>           }
>   
>