Add a bunch of auxiliarry methods and a feature flag to work with
SEEKABLE channels. Currently the only channel considered seekable is
QIOChannelFile. Also add a bunch of helper functions to QEMUFile that
can make use of this channel feature. All of this is in prepration for
supporting 'fixed-ram' migration stream feature.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
include/io/channel.h | 1 +
include/migration/qemu-file-types.h | 2 +
io/channel-file.c | 5 +++
migration/qemu-file.c | 59 +++++++++++++++++++++++++++++
migration/qemu-file.h | 3 ++
5 files changed, 70 insertions(+)
diff --git a/include/io/channel.h b/include/io/channel.h
index c680ee748021..4fc37c78e68c 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -41,6 +41,7 @@ enum QIOChannelFeature {
QIO_CHANNEL_FEATURE_SHUTDOWN,
QIO_CHANNEL_FEATURE_LISTEN,
QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY,
+ QIO_CHANNEL_FEATURE_SEEKABLE,
};
diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
index 2867e3da84ab..eb0325ee8687 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/io/channel-file.c b/io/channel-file.c
index da17d0a11ba7..d84a6737f2f7 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -35,6 +35,7 @@ qio_channel_file_new_fd(int fd)
ioc->fd = fd;
+ qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
trace_qio_channel_file_new_fd(ioc, fd);
return ioc;
@@ -59,6 +60,10 @@ qio_channel_file_new_path(const char *path,
return NULL;
}
+ if (lseek(ioc->fd, 0, SEEK_CUR) != (off_t)-1) {
+ qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
+ }
+
trace_qio_channel_file_new_path(ioc, path, flags, mode, ioc->fd);
return ioc;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 4f400c2e5265..07ba1125e83f 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -30,6 +30,7 @@
#include "qemu-file.h"
#include "trace.h"
#include "qapi/error.h"
+#include "io/channel-file.h"
#define IO_BUF_SIZE 32768
#define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
@@ -260,6 +261,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
@@ -538,6 +543,60 @@ 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;
+ struct iovec iov = { .iov_base = (char *)buf, .iov_len = buflen };
+
+ if (f->last_error) {
+ return;
+ }
+
+ qemu_fflush(f);
+
+ if (qio_channel_file_pwritev(f->ioc, &iov, 1, pos, &err) == (off_t)-1)
+ goto error;
+
+ return;
+
+ error:
+ qemu_file_set_error_obj(f, -EIO, err);
+ return;
+}
+
+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 fa13d04d787c..33cfc07b81d1 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -148,6 +148,9 @@ int qemu_file_shutdown(QEMUFile *f);
QEMUFile *qemu_file_get_return_path(QEMUFile *f);
void qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
+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);
void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
--
2.34.1On Mon, Oct 10, 2022 at 04:34:02PM +0300, Nikolay Borisov wrote:
> Add a bunch of auxiliarry methods and a feature flag to work with
> SEEKABLE channels. Currently the only channel considered seekable is
> QIOChannelFile. Also add a bunch of helper functions to QEMUFile that
> can make use of this channel feature. All of this is in prepration for
> supporting 'fixed-ram' migration stream feature.
QIOChannelBuffer/Null are also seekable.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> include/io/channel.h | 1 +
> include/migration/qemu-file-types.h | 2 +
> io/channel-file.c | 5 +++
> migration/qemu-file.c | 59 +++++++++++++++++++++++++++++
> migration/qemu-file.h | 3 ++
> 5 files changed, 70 insertions(+)
Can you separate the migration/ tree bits into a second patch
that follows the io/ bits.
>
> diff --git a/include/io/channel.h b/include/io/channel.h
> index c680ee748021..4fc37c78e68c 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -41,6 +41,7 @@ enum QIOChannelFeature {
> QIO_CHANNEL_FEATURE_SHUTDOWN,
> QIO_CHANNEL_FEATURE_LISTEN,
> QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY,
> + QIO_CHANNEL_FEATURE_SEEKABLE,
> };
>
>
> diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
> index 2867e3da84ab..eb0325ee8687 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/io/channel-file.c b/io/channel-file.c
> index da17d0a11ba7..d84a6737f2f7 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -35,6 +35,7 @@ qio_channel_file_new_fd(int fd)
>
> ioc->fd = fd;
>
> + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
> trace_qio_channel_file_new_fd(ioc, fd);
>
> return ioc;
> @@ -59,6 +60,10 @@ qio_channel_file_new_path(const char *path,
> return NULL;
> }
>
> + if (lseek(ioc->fd, 0, SEEK_CUR) != (off_t)-1) {
> + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
> + }
> +
> trace_qio_channel_file_new_path(ioc, path, flags, mode, ioc->fd);
Wondering why you do the lseek() sanitytest for only one of the
two constructors ? Shouldn't we do it for both ?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 18.10.22 г. 13:14 ч., Daniel P. Berrangé wrote:
> On Mon, Oct 10, 2022 at 04:34:02PM +0300, Nikolay Borisov wrote:
>> Add a bunch of auxiliarry methods and a feature flag to work with
>> SEEKABLE channels. Currently the only channel considered seekable is
>> QIOChannelFile. Also add a bunch of helper functions to QEMUFile that
>> can make use of this channel feature. All of this is in prepration for
>> supporting 'fixed-ram' migration stream feature.
>
> QIOChannelBuffer/Null are also seekable.
Right, however I think for seek we also want to rely on the feature of
filesystem that when you seek beyond EOF you won't actually allocate/use
up the space from (eof, CUR_SEEK), with ChannelBuffer we'd have to
actually allocate the space or add this support on top.
>
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> include/io/channel.h | 1 +
>> include/migration/qemu-file-types.h | 2 +
>> io/channel-file.c | 5 +++
>> migration/qemu-file.c | 59 +++++++++++++++++++++++++++++
>> migration/qemu-file.h | 3 ++
>> 5 files changed, 70 insertions(+)
>
> Can you separate the migration/ tree bits into a second patch
> that follows the io/ bits.
>
>>
>> diff --git a/include/io/channel.h b/include/io/channel.h
>> index c680ee748021..4fc37c78e68c 100644
>> --- a/include/io/channel.h
>> +++ b/include/io/channel.h
>> @@ -41,6 +41,7 @@ enum QIOChannelFeature {
>> QIO_CHANNEL_FEATURE_SHUTDOWN,
>> QIO_CHANNEL_FEATURE_LISTEN,
>> QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY,
>> + QIO_CHANNEL_FEATURE_SEEKABLE,
>> };
>>
>>
>> diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
>> index 2867e3da84ab..eb0325ee8687 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/io/channel-file.c b/io/channel-file.c
>> index da17d0a11ba7..d84a6737f2f7 100644
>> --- a/io/channel-file.c
>> +++ b/io/channel-file.c
>> @@ -35,6 +35,7 @@ qio_channel_file_new_fd(int fd)
>>
>> ioc->fd = fd;
>>
>> + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
>> trace_qio_channel_file_new_fd(ioc, fd);
>>
>> return ioc;
>> @@ -59,6 +60,10 @@ qio_channel_file_new_path(const char *path,
>> return NULL;
>> }
>>
>> + if (lseek(ioc->fd, 0, SEEK_CUR) != (off_t)-1) {
>> + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
>> + }
>> +
>> trace_qio_channel_file_new_path(ioc, path, flags, mode, ioc->fd);
>
> Wondering why you do the lseek() sanitytest for only one of the
> two constructors ? Shouldn't we do it for both ?
Good point, AFAIR my train of thought was something along the lines of
"what if the path being passed actually lies on a remote fs, we
definitely need to test for this support", however now that I think
about it a bit more - nothing prevents for the passed in fd to also be
lying on a remote fs, so good point. Will fix this in next iteration.
>
>
>
>
> With regards,
> Daniel
On Tue, Oct 18, 2022 at 01:46:45PM +0300, Nikolay Borisov wrote: > > > On 18.10.22 г. 13:14 ч., Daniel P. Berrangé wrote: > > On Mon, Oct 10, 2022 at 04:34:02PM +0300, Nikolay Borisov wrote: > > > Add a bunch of auxiliarry methods and a feature flag to work with > > > SEEKABLE channels. Currently the only channel considered seekable is > > > QIOChannelFile. Also add a bunch of helper functions to QEMUFile that > > > can make use of this channel feature. All of this is in prepration for > > > supporting 'fixed-ram' migration stream feature. > > > > QIOChannelBuffer/Null are also seekable. > > Right, however I think for seek we also want to rely on the feature of > filesystem that when you seek beyond EOF you won't actually allocate/use up > the space from (eof, CUR_SEEK), with ChannelBuffer we'd have to actually > allocate the space or add this support on top. I'm fine with not implementing this for ChannelBuffer. Mostly making the point that the APIs should be in QIOChannel base class, so that someone could implement it in any subclass in future where it makes sense. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2026 Red Hat, Inc.