"Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
>> * Juan Quintela (quint...@redhat.com) wrote:
>> > Be sure that we are not doing neither read/write after shutdown of the
>> > QEMUFile.
>> >
>> > Signed-off-by: Juan Quintela <quint...@redhat.com>
>> > ---
>> > migration/qemu-file.c | 13 +++++++++++++
>> > 1 file changed, 13 insertions(+)
>> >
>> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>> > index 26fb25ddc1..1e5543a279 100644
>> > --- a/migration/qemu-file.c
>> > +++ b/migration/qemu-file.c
>> > @@ -53,6 +53,8 @@ struct QEMUFile {
>> >
>> > int last_error;
>> > Error *last_error_obj;
>> > + /* has the file has been shutdown */
>> > + bool shutdown;
>> > };
>> >
>> > /*
>> > @@ -61,6 +63,7 @@ struct QEMUFile {
>> > */
>> > int qemu_file_shutdown(QEMUFile *f)
>> > {
>> > + f->shutdown = true;
>> > if (!f->ops->shut_down) {
>> > return -ENOSYS;
>> > }
>> > @@ -214,6 +217,9 @@ void qemu_fflush(QEMUFile *f)
>> > return;
>> > }
>> >
>> > + if (f->shutdown) {
>> > + return;
>> > + }
>>
>> OK, I did wonder if you need to free the iovec.
>>
>> > if (f->iovcnt > 0) {
>> > expect = iov_size(f->iov, f->iovcnt);
>> > ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
>> > @@ -328,6 +334,10 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>> > f->buf_index = 0;
>> > f->buf_size = pending;
>> >
>> > + if (f->shutdown) {
>> > + return 0;
>> > + }
>>
>> I also wondered if perhaps an error would be reasonable here; but I'm
>> not sure what a read(2) does after a shutdown(2).
>>
>> Still,
>>
>>
>> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>
> Actually, it turns out this breaks an assumption - 'shutdown' must cause
> reads/writes/etc to fail and for the qemu_file to go into error state.
> There's a few places we loop doing IO until we either change migration
> state or the file goes into error.
>
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 1e5543a279..bbb2b63927 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -63,11 +63,18 @@ struct QEMUFile {
> */
> int qemu_file_shutdown(QEMUFile *f)
> {
> + int ret;
> +
> f->shutdown = true;
> if (!f->ops->shut_down) {
> return -ENOSYS;
> }
> - return f->ops->shut_down(f->opaque, true, true, NULL);
> + ret = f->ops->shut_down(f->opaque, true, true, NULL);
> +
> + if (!f->last_error) {
> + qemu_file_set_error(f, -EIO);
> + }
> + return ret;
> }
>
> /*
>
>
> seems to fix it for me.
will gve it a try later.
Thanks.
