[RFC PATCH v3 05/30] migration/qemu-file: add utility methods for working with seekable channels

Fabiano Rosas posted 30 patches 1 year ago
Maintainers: Juan Quintela <quintela@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Leonardo Bras <leobras@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[RFC PATCH v3 05/30] migration/qemu-file: add utility methods for working with seekable channels
Posted by Fabiano Rosas 1 year ago
From: Nikolay Borisov <nborisov@suse.com>

Add utility methods that will be needed when implementing 'fixed-ram'
migration capability.

qemu_file_is_seekable
qemu_put_buffer_at
qemu_get_buffer_at
qemu_set_offset
qemu_get_offset

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/migration/qemu-file-types.h |  2 +
 migration/qemu-file.c               | 82 +++++++++++++++++++++++++++++
 migration/qemu-file.h               |  6 +++
 3 files changed, 90 insertions(+)

diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
index 9ba163f333..adec5abc07 100644
--- a/include/migration/qemu-file-types.h
+++ b/include/migration/qemu-file-types.h
@@ -50,6 +50,8 @@ unsigned int qemu_get_be16(QEMUFile *f);
 unsigned int qemu_get_be32(QEMUFile *f);
 uint64_t qemu_get_be64(QEMUFile *f);
 
+bool qemu_file_is_seekable(QEMUFile *f);
+
 static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
 {
     qemu_put_be64(f, *pv);
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 94231ff295..faf6427b91 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -33,6 +33,7 @@
 #include "options.h"
 #include "qapi/error.h"
 #include "rdma.h"
+#include "io/channel-file.h"
 
 #define IO_BUF_SIZE 32768
 #define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
@@ -255,6 +256,10 @@ static void qemu_iovec_release_ram(QEMUFile *f)
     memset(f->may_free, 0, sizeof(f->may_free));
 }
 
+bool qemu_file_is_seekable(QEMUFile *f)
+{
+    return qio_channel_has_feature(f->ioc, QIO_CHANNEL_FEATURE_SEEKABLE);
+}
 
 /**
  * Flushes QEMUFile buffer
@@ -447,6 +452,83 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
     }
 }
 
+void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
+                        off_t pos)
+{
+    Error *err = NULL;
+
+    if (f->last_error) {
+        return;
+    }
+
+    qemu_fflush(f);
+    qio_channel_pwrite(f->ioc, (char *)buf, buflen, pos, &err);
+
+    if (err) {
+        qemu_file_set_error_obj(f, -EIO, err);
+    } else {
+        stat64_add(&mig_stats.qemu_file_transferred, buflen);
+    }
+
+    return;
+}
+
+
+size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
+                          off_t pos)
+{
+    Error *err = NULL;
+    ssize_t ret;
+
+    if (f->last_error) {
+        return 0;
+    }
+
+    ret = qio_channel_pread(f->ioc, (char *)buf, buflen, pos, &err);
+    if (ret == -1 || err) {
+        goto error;
+    }
+
+    return (size_t)ret;
+
+ error:
+    qemu_file_set_error_obj(f, -EIO, err);
+    return 0;
+}
+
+void qemu_set_offset(QEMUFile *f, off_t off, int whence)
+{
+    Error *err = NULL;
+    off_t ret;
+
+    qemu_fflush(f);
+
+    if (!qemu_file_is_writable(f)) {
+        f->buf_index = 0;
+        f->buf_size = 0;
+    }
+
+    ret = qio_channel_io_seek(f->ioc, off, whence, &err);
+    if (ret == (off_t)-1) {
+        qemu_file_set_error_obj(f, -EIO, err);
+    }
+}
+
+off_t qemu_get_offset(QEMUFile *f)
+{
+    Error *err = NULL;
+    off_t ret;
+
+    qemu_fflush(f);
+
+    ret = qio_channel_io_seek(f->ioc, 0, SEEK_CUR, &err);
+    if (ret == (off_t)-1) {
+        qemu_file_set_error_obj(f, -EIO, err);
+    }
+    return ret;
+}
+
+
 void qemu_put_byte(QEMUFile *f, int v)
 {
     if (f->last_error) {
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 8aec9fabf7..32fd4a34fd 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -75,6 +75,12 @@ QEMUFile *qemu_file_get_return_path(QEMUFile *f);
 int qemu_fflush(QEMUFile *f);
 void qemu_file_set_blocking(QEMUFile *f, bool block);
 int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
+void qemu_set_offset(QEMUFile *f, off_t off, int whence);
+off_t qemu_get_offset(QEMUFile *f);
+void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
+                        off_t pos);
+size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
+                          off_t pos);
 
 QIOChannel *qemu_file_get_ioc(QEMUFile *file);
 
-- 
2.35.3


Re: [RFC PATCH v3 05/30] migration/qemu-file: add utility methods for working with seekable channels
Posted by Peter Xu 10 months, 2 weeks ago
On Mon, Nov 27, 2023 at 05:25:47PM -0300, Fabiano Rosas wrote:
> From: Nikolay Borisov <nborisov@suse.com>
> 
> Add utility methods that will be needed when implementing 'fixed-ram'
> migration capability.
> 
> qemu_file_is_seekable
> qemu_put_buffer_at
> qemu_get_buffer_at
> qemu_set_offset
> qemu_get_offset
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  include/migration/qemu-file-types.h |  2 +
>  migration/qemu-file.c               | 82 +++++++++++++++++++++++++++++
>  migration/qemu-file.h               |  6 +++
>  3 files changed, 90 insertions(+)
> 
> diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
> index 9ba163f333..adec5abc07 100644
> --- a/include/migration/qemu-file-types.h
> +++ b/include/migration/qemu-file-types.h
> @@ -50,6 +50,8 @@ unsigned int qemu_get_be16(QEMUFile *f);
>  unsigned int qemu_get_be32(QEMUFile *f);
>  uint64_t qemu_get_be64(QEMUFile *f);
>  
> +bool qemu_file_is_seekable(QEMUFile *f);
> +
>  static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
>  {
>      qemu_put_be64(f, *pv);
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 94231ff295..faf6427b91 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -33,6 +33,7 @@
>  #include "options.h"
>  #include "qapi/error.h"
>  #include "rdma.h"
> +#include "io/channel-file.h"
>  
>  #define IO_BUF_SIZE 32768
>  #define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
> @@ -255,6 +256,10 @@ static void qemu_iovec_release_ram(QEMUFile *f)
>      memset(f->may_free, 0, sizeof(f->may_free));
>  }
>  
> +bool qemu_file_is_seekable(QEMUFile *f)
> +{
> +    return qio_channel_has_feature(f->ioc, QIO_CHANNEL_FEATURE_SEEKABLE);
> +}
>  
>  /**
>   * Flushes QEMUFile buffer
> @@ -447,6 +452,83 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
>      }
>  }
>  
> +void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
> +                        off_t pos)
> +{
> +    Error *err = NULL;
> +
> +    if (f->last_error) {
> +        return;
> +    }
> +
> +    qemu_fflush(f);
> +    qio_channel_pwrite(f->ioc, (char *)buf, buflen, pos, &err);

Partial writes won't set err.  Do we want to check the retval here too and
fail properly if detected partial writes?

> +
> +    if (err) {
> +        qemu_file_set_error_obj(f, -EIO, err);
> +    } else {
> +        stat64_add(&mig_stats.qemu_file_transferred, buflen);

buflen is only accurate if with above, iiuc.

> +    }
> +
> +    return;
> +}
> +
> +
> +size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
> +                          off_t pos)
> +{
> +    Error *err = NULL;
> +    ssize_t ret;
> +
> +    if (f->last_error) {
> +        return 0;
> +    }
> +
> +    ret = qio_channel_pread(f->ioc, (char *)buf, buflen, pos, &err);

Same question here.

> +    if (ret == -1 || err) {
> +        goto error;
> +    }
> +
> +    return (size_t)ret;
> +
> + error:
> +    qemu_file_set_error_obj(f, -EIO, err);
> +    return 0;
> +}
> +
> +void qemu_set_offset(QEMUFile *f, off_t off, int whence)
> +{
> +    Error *err = NULL;
> +    off_t ret;
> +
> +    qemu_fflush(f);
> +
> +    if (!qemu_file_is_writable(f)) {
> +        f->buf_index = 0;
> +        f->buf_size = 0;
> +    }

There's the qemu_file_is_writable() check after all, then put qemu_fflush()
into condition too?

  if (qemu_file_is_writable(f)) {
      qemu_fflush(f);
  } else {
     /* Drop all the cached buffers if existed; will trigger a re-fill later */
     f->buf_index = 0;
     f->buf_size = 0;
  }

> +
> +    ret = qio_channel_io_seek(f->ioc, off, whence, &err);
> +    if (ret == (off_t)-1) {
> +        qemu_file_set_error_obj(f, -EIO, err);
> +    }
> +}
> +
> +off_t qemu_get_offset(QEMUFile *f)
> +{
> +    Error *err = NULL;
> +    off_t ret;
> +
> +    qemu_fflush(f);
> +
> +    ret = qio_channel_io_seek(f->ioc, 0, SEEK_CUR, &err);
> +    if (ret == (off_t)-1) {
> +        qemu_file_set_error_obj(f, -EIO, err);
> +    }
> +    return ret;
> +}
> +
> +
>  void qemu_put_byte(QEMUFile *f, int v)
>  {
>      if (f->last_error) {
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 8aec9fabf7..32fd4a34fd 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -75,6 +75,12 @@ QEMUFile *qemu_file_get_return_path(QEMUFile *f);
>  int qemu_fflush(QEMUFile *f);
>  void qemu_file_set_blocking(QEMUFile *f, bool block);
>  int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
> +void qemu_set_offset(QEMUFile *f, off_t off, int whence);
> +off_t qemu_get_offset(QEMUFile *f);
> +void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
> +                        off_t pos);
> +size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
> +                          off_t pos);
>  
>  QIOChannel *qemu_file_get_ioc(QEMUFile *file);
>  
> -- 
> 2.35.3
> 

-- 
Peter Xu


Re: [RFC PATCH v3 05/30] migration/qemu-file: add utility methods for working with seekable channels
Posted by Fabiano Rosas 10 months, 2 weeks ago
Peter Xu <peterx@redhat.com> writes:

> On Mon, Nov 27, 2023 at 05:25:47PM -0300, Fabiano Rosas wrote:
>> From: Nikolay Borisov <nborisov@suse.com>
>> 
>> Add utility methods that will be needed when implementing 'fixed-ram'
>> migration capability.
>> 
>> qemu_file_is_seekable
>> qemu_put_buffer_at
>> qemu_get_buffer_at
>> qemu_set_offset
>> qemu_get_offset
>> 
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>  include/migration/qemu-file-types.h |  2 +
>>  migration/qemu-file.c               | 82 +++++++++++++++++++++++++++++
>>  migration/qemu-file.h               |  6 +++
>>  3 files changed, 90 insertions(+)
>> 
>> diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
>> index 9ba163f333..adec5abc07 100644
>> --- a/include/migration/qemu-file-types.h
>> +++ b/include/migration/qemu-file-types.h
>> @@ -50,6 +50,8 @@ unsigned int qemu_get_be16(QEMUFile *f);
>>  unsigned int qemu_get_be32(QEMUFile *f);
>>  uint64_t qemu_get_be64(QEMUFile *f);
>>  
>> +bool qemu_file_is_seekable(QEMUFile *f);
>> +
>>  static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
>>  {
>>      qemu_put_be64(f, *pv);
>> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>> index 94231ff295..faf6427b91 100644
>> --- a/migration/qemu-file.c
>> +++ b/migration/qemu-file.c
>> @@ -33,6 +33,7 @@
>>  #include "options.h"
>>  #include "qapi/error.h"
>>  #include "rdma.h"
>> +#include "io/channel-file.h"
>>  
>>  #define IO_BUF_SIZE 32768
>>  #define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
>> @@ -255,6 +256,10 @@ static void qemu_iovec_release_ram(QEMUFile *f)
>>      memset(f->may_free, 0, sizeof(f->may_free));
>>  }
>>  
>> +bool qemu_file_is_seekable(QEMUFile *f)
>> +{
>> +    return qio_channel_has_feature(f->ioc, QIO_CHANNEL_FEATURE_SEEKABLE);
>> +}
>>  
>>  /**
>>   * Flushes QEMUFile buffer
>> @@ -447,6 +452,83 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
>>      }
>>  }
>>  
>> +void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
>> +                        off_t pos)
>> +{
>> +    Error *err = NULL;
>> +
>> +    if (f->last_error) {
>> +        return;
>> +    }
>> +
>> +    qemu_fflush(f);
>> +    qio_channel_pwrite(f->ioc, (char *)buf, buflen, pos, &err);
>
> Partial writes won't set err.  Do we want to check the retval here too and
> fail properly if detected partial writes?
>

Yep.

>> +
>> +    if (err) {
>> +        qemu_file_set_error_obj(f, -EIO, err);
>> +    } else {
>> +        stat64_add(&mig_stats.qemu_file_transferred, buflen);
>
> buflen is only accurate if with above, iiuc.
>
>> +    }
>> +
>> +    return;
>> +}
>> +
>> +
>> +size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
>> +                          off_t pos)
>> +{
>> +    Error *err = NULL;
>> +    ssize_t ret;
>> +
>> +    if (f->last_error) {
>> +        return 0;
>> +    }
>> +
>> +    ret = qio_channel_pread(f->ioc, (char *)buf, buflen, pos, &err);
>
> Same question here.
>
>> +    if (ret == -1 || err) {
>> +        goto error;
>> +    }
>> +
>> +    return (size_t)ret;
>> +
>> + error:
>> +    qemu_file_set_error_obj(f, -EIO, err);
>> +    return 0;
>> +}
>> +
>> +void qemu_set_offset(QEMUFile *f, off_t off, int whence)
>> +{
>> +    Error *err = NULL;
>> +    off_t ret;
>> +
>> +    qemu_fflush(f);
>> +
>> +    if (!qemu_file_is_writable(f)) {
>> +        f->buf_index = 0;
>> +        f->buf_size = 0;
>> +    }
>
> There's the qemu_file_is_writable() check after all, then put qemu_fflush()
> into condition too?
>
>   if (qemu_file_is_writable(f)) {
>       qemu_fflush(f);
>   } else {
>      /* Drop all the cached buffers if existed; will trigger a re-fill later */
>      f->buf_index = 0;
>      f->buf_size = 0;
>   }
>

Could be. I'll change it.

>> +
>> +    ret = qio_channel_io_seek(f->ioc, off, whence, &err);
>> +    if (ret == (off_t)-1) {
>> +        qemu_file_set_error_obj(f, -EIO, err);
>> +    }
>> +}
>> +
>> +off_t qemu_get_offset(QEMUFile *f)
>> +{
>> +    Error *err = NULL;
>> +    off_t ret;
>> +
>> +    qemu_fflush(f);
>> +
>> +    ret = qio_channel_io_seek(f->ioc, 0, SEEK_CUR, &err);
>> +    if (ret == (off_t)-1) {
>> +        qemu_file_set_error_obj(f, -EIO, err);
>> +    }
>> +    return ret;
>> +}
>> +
>> +
>>  void qemu_put_byte(QEMUFile *f, int v)
>>  {
>>      if (f->last_error) {
>> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
>> index 8aec9fabf7..32fd4a34fd 100644
>> --- a/migration/qemu-file.h
>> +++ b/migration/qemu-file.h
>> @@ -75,6 +75,12 @@ QEMUFile *qemu_file_get_return_path(QEMUFile *f);
>>  int qemu_fflush(QEMUFile *f);
>>  void qemu_file_set_blocking(QEMUFile *f, bool block);
>>  int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
>> +void qemu_set_offset(QEMUFile *f, off_t off, int whence);
>> +off_t qemu_get_offset(QEMUFile *f);
>> +void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
>> +                        off_t pos);
>> +size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
>> +                          off_t pos);
>>  
>>  QIOChannel *qemu_file_get_ioc(QEMUFile *file);
>>  
>> -- 
>> 2.35.3
>>