| Message ID | 20220620110205.1357829-19-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. > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>
* Daniel P. Berrangé (berrange@redhat.com) wrote: > This directly implements the get_buffer logic using QIOChannel APIs. > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Coverity is pointing out a fun deadcode path from this: > diff --git a/migration/qemu-file.c b/migration/qemu-file.c > index 5eb8cf0e28..df438724cd 100644 > --- a/migration/qemu-file.c > +++ b/migration/qemu-file.c > @@ -377,8 +377,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); > + the next code is: if (len > 0) { f->buf_size += len; f->total_transferred += len; } else if (len == 0) { qemu_file_set_error_obj(f, -EIO, local_error); } else if (len != -EAGAIN) { qemu_file_set_error_obj(f, len, local_error); } else { **** error_free(local_error); } because of the while loop, we should never actually see len = QIO_CHANNEL_ERR_BLOCK out of the bottom; so the only error value we should have is -EIO; so that error_free is not hittable. Dave > if (len > 0) { > f->buf_size += len; > f->total_transferred += len; > diff --git a/migration/qemu-file.h b/migration/qemu-file.h > index 4a3beedb5b..f7ed568894 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 >
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 5eb8cf0e28..df438724cd 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -377,8 +377,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 4a3beedb5b..f7ed568894 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;