| Message ID | 20220524110235.145079-18-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: remove QEMUFileOps concept and assume use of QIOChannel | expand |
* Daniel P. Berrangé (berrange@redhat.com) wrote: > This directly implements the get_buffer logic using QIOChannel APIs. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > migration/qemu-file-channel.c | 29 ----------------------------- > migration/qemu-file.c | 18 ++++++++++++++++-- > migration/qemu-file.h | 9 --------- > 3 files changed, 16 insertions(+), 40 deletions(-) > > diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c > index 8ff58e81f9..7b32831752 100644 > --- a/migration/qemu-file-channel.c > +++ b/migration/qemu-file-channel.c > @@ -74,34 +74,6 @@ static ssize_t channel_writev_buffer(void *opaque, > } > > > -static ssize_t channel_get_buffer(void *opaque, > - uint8_t *buf, > - int64_t pos, > - size_t size, > - Error **errp) > -{ > - QIOChannel *ioc = QIO_CHANNEL(opaque); > - ssize_t ret; > - > - do { > - ret = qio_channel_read(ioc, (char *)buf, size, errp); > - if (ret < 0) { > - if (ret == QIO_CHANNEL_ERR_BLOCK) { > - if (qemu_in_coroutine()) { > - qio_channel_yield(ioc, G_IO_IN); > - } else { > - qio_channel_wait(ioc, G_IO_IN); > - } > - } else { > - return -EIO; > - } > - } > - } while (ret == QIO_CHANNEL_ERR_BLOCK); > - > - return ret; > -} > - > - > static QEMUFile *channel_get_input_return_path(void *opaque) > { > QIOChannel *ioc = QIO_CHANNEL(opaque); > @@ -117,7 +89,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque) > } > > static const QEMUFileOps channel_input_ops = { > - .get_buffer = channel_get_buffer, > .get_return_path = channel_get_input_return_path, > }; > > diff --git a/migration/qemu-file.c b/migration/qemu-file.c > index a855ce33dc..e024b43851 100644 > --- a/migration/qemu-file.c > +++ b/migration/qemu-file.c > @@ -374,8 +374,22 @@ static ssize_t qemu_fill_buffer(QEMUFile *f) > return 0; > } > > - len = f->ops->get_buffer(f->ioc, f->buf + pending, f->total_transferred, > - IO_BUF_SIZE - pending, &local_error); > + do { > + len = qio_channel_read(f->ioc, Yes, I think that's OK - not that 'len' is an int where 'ret' was a ssize_t; but I think our buffers are guranteed to be 'small'. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > + (char *)f->buf + pending, > + IO_BUF_SIZE - pending, > + &local_error); > + if (len == QIO_CHANNEL_ERR_BLOCK) { > + if (qemu_in_coroutine()) { > + qio_channel_yield(f->ioc, G_IO_IN); > + } else { > + qio_channel_wait(f->ioc, G_IO_IN); > + } > + } else if (len < 0) { > + len = EIO; > + } > + } while (len == QIO_CHANNEL_ERR_BLOCK); > + > if (len > 0) { > f->buf_size += len; > f->total_transferred += len; > diff --git a/migration/qemu-file.h b/migration/qemu-file.h > index 7ec105bf96..cd49184c00 100644 > --- a/migration/qemu-file.h > +++ b/migration/qemu-file.h > @@ -29,14 +29,6 @@ > #include "exec/cpu-common.h" > #include "io/channel.h" > > -/* Read a chunk of data from a file at the given position. The pos argument > - * can be ignored if the file is only be used for streaming. The number of > - * bytes actually read should be returned. > - */ > -typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf, > - int64_t pos, size_t size, > - Error **errp); > - > /* > * This function writes an iovec to file. The handler must write all > * of the data or return a negative errno value. > @@ -77,7 +69,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f, > typedef QEMUFile *(QEMURetPathFunc)(void *opaque); > > typedef struct QEMUFileOps { > - QEMUFileGetBufferFunc *get_buffer; > QEMUFileWritevBufferFunc *writev_buffer; > QEMURetPathFunc *get_return_path; > } QEMUFileOps; > -- > 2.36.1 >
On Thu, Jun 09, 2022 at 05:46:29PM +0100, Dr. David Alan Gilbert wrote: > * Daniel P. Berrangé (berrange@redhat.com) wrote: > > This directly implements the get_buffer logic using QIOChannel APIs. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > migration/qemu-file-channel.c | 29 ----------------------------- > > migration/qemu-file.c | 18 ++++++++++++++++-- > > migration/qemu-file.h | 9 --------- > > 3 files changed, 16 insertions(+), 40 deletions(-) > > > > diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c > > index 8ff58e81f9..7b32831752 100644 > > --- a/migration/qemu-file-channel.c > > +++ b/migration/qemu-file-channel.c > > @@ -74,34 +74,6 @@ static ssize_t channel_writev_buffer(void *opaque, > > } > > > > > > -static ssize_t channel_get_buffer(void *opaque, > > - uint8_t *buf, > > - int64_t pos, > > - size_t size, > > - Error **errp) > > -{ > > - QIOChannel *ioc = QIO_CHANNEL(opaque); > > - ssize_t ret; > > - > > - do { > > - ret = qio_channel_read(ioc, (char *)buf, size, errp); > > - if (ret < 0) { > > - if (ret == QIO_CHANNEL_ERR_BLOCK) { > > - if (qemu_in_coroutine()) { > > - qio_channel_yield(ioc, G_IO_IN); > > - } else { > > - qio_channel_wait(ioc, G_IO_IN); > > - } > > - } else { > > - return -EIO; > > - } > > - } > > - } while (ret == QIO_CHANNEL_ERR_BLOCK); > > - > > - return ret; > > -} > > - > > - > > static QEMUFile *channel_get_input_return_path(void *opaque) > > { > > QIOChannel *ioc = QIO_CHANNEL(opaque); > > @@ -117,7 +89,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque) > > } > > > > static const QEMUFileOps channel_input_ops = { > > - .get_buffer = channel_get_buffer, > > .get_return_path = channel_get_input_return_path, > > }; > > > > diff --git a/migration/qemu-file.c b/migration/qemu-file.c > > index a855ce33dc..e024b43851 100644 > > --- a/migration/qemu-file.c > > +++ b/migration/qemu-file.c > > @@ -374,8 +374,22 @@ static ssize_t qemu_fill_buffer(QEMUFile *f) > > return 0; > > } > > > > - len = f->ops->get_buffer(f->ioc, f->buf + pending, f->total_transferred, > > - IO_BUF_SIZE - pending, &local_error); > > + do { > > + len = qio_channel_read(f->ioc, > > Yes, I think that's OK - not that 'len' is an int where 'ret' > was a ssize_t; but I think our buffers are guranteed to be 'small'. There are a few places in qemu-file.c where we're fast & loose with int rather than size_t, that are probably worth cleaning. > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> With regards, Daniel
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c index 8ff58e81f9..7b32831752 100644 --- a/migration/qemu-file-channel.c +++ b/migration/qemu-file-channel.c @@ -74,34 +74,6 @@ static ssize_t channel_writev_buffer(void *opaque, } -static ssize_t channel_get_buffer(void *opaque, - uint8_t *buf, - int64_t pos, - size_t size, - Error **errp) -{ - QIOChannel *ioc = QIO_CHANNEL(opaque); - ssize_t ret; - - do { - ret = qio_channel_read(ioc, (char *)buf, size, errp); - if (ret < 0) { - if (ret == QIO_CHANNEL_ERR_BLOCK) { - if (qemu_in_coroutine()) { - qio_channel_yield(ioc, G_IO_IN); - } else { - qio_channel_wait(ioc, G_IO_IN); - } - } else { - return -EIO; - } - } - } while (ret == QIO_CHANNEL_ERR_BLOCK); - - return ret; -} - - static QEMUFile *channel_get_input_return_path(void *opaque) { QIOChannel *ioc = QIO_CHANNEL(opaque); @@ -117,7 +89,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque) } static const QEMUFileOps channel_input_ops = { - .get_buffer = channel_get_buffer, .get_return_path = channel_get_input_return_path, }; diff --git a/migration/qemu-file.c b/migration/qemu-file.c index a855ce33dc..e024b43851 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -374,8 +374,22 @@ static ssize_t qemu_fill_buffer(QEMUFile *f) return 0; } - len = f->ops->get_buffer(f->ioc, f->buf + pending, f->total_transferred, - IO_BUF_SIZE - pending, &local_error); + do { + len = qio_channel_read(f->ioc, + (char *)f->buf + pending, + IO_BUF_SIZE - pending, + &local_error); + if (len == QIO_CHANNEL_ERR_BLOCK) { + if (qemu_in_coroutine()) { + qio_channel_yield(f->ioc, G_IO_IN); + } else { + qio_channel_wait(f->ioc, G_IO_IN); + } + } else if (len < 0) { + len = EIO; + } + } while (len == QIO_CHANNEL_ERR_BLOCK); + if (len > 0) { f->buf_size += len; f->total_transferred += len; diff --git a/migration/qemu-file.h b/migration/qemu-file.h index 7ec105bf96..cd49184c00 100644 --- a/migration/qemu-file.h +++ b/migration/qemu-file.h @@ -29,14 +29,6 @@ #include "exec/cpu-common.h" #include "io/channel.h" -/* Read a chunk of data from a file at the given position. The pos argument - * can be ignored if the file is only be used for streaming. The number of - * bytes actually read should be returned. - */ -typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf, - int64_t pos, size_t size, - Error **errp); - /* * This function writes an iovec to file. The handler must write all * of the data or return a negative errno value. @@ -77,7 +69,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f, typedef QEMUFile *(QEMURetPathFunc)(void *opaque); typedef struct QEMUFileOps { - QEMUFileGetBufferFunc *get_buffer; QEMUFileWritevBufferFunc *writev_buffer; QEMURetPathFunc *get_return_path; } QEMUFileOps;
This directly implements the get_buffer logic using QIOChannel APIs. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- migration/qemu-file-channel.c | 29 ----------------------------- migration/qemu-file.c | 18 ++++++++++++++++-- migration/qemu-file.h | 9 --------- 3 files changed, 16 insertions(+), 40 deletions(-)