Re: [PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback
On Thu, Jun 09, 2022 at 05:46:29PM +0100, Dr. David Alan Gilbert wrote: > * Daniel P. Berrangé (berra...@redhat.com) wrote: > > This directly implements the get_buffer logic using QIOChannel APIs. > > > > Signed-off-by: Daniel P. Berrangé > > --- > > 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, _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 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 :|
Re: [PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback
* Daniel P. Berrangé (berra...@redhat.com) wrote: > This directly implements the get_buffer logic using QIOChannel APIs. > > Signed-off-by: Daniel P. Berrangé > --- > 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, _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 > + (char *)f->buf + pending, > + IO_BUF_SIZE - pending, > + _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 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback
This directly implements the get_buffer logic using QIOChannel APIs. Signed-off-by: Daniel P. Berrangé --- 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, _error); +do { +len = qio_channel_read(f->ioc, + (char *)f->buf + pending, + IO_BUF_SIZE - pending, + _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