Re: [PATCH v2 15/21] migration: remove the QEMUFileOps 'shut_down' callback

[Juan Quintela <quintela@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> wrote:
> This directly implements the shutdown 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>

> -{
> -    QIOChannel *ioc = QIO_CHANNEL(opaque);
> -
> -    if (qio_channel_has_feature(ioc,
> -                                QIO_CHANNEL_FEATURE_SHUTDOWN)) {
> -        QIOChannelShutdown mode;
> -        if (rd && wr) {
> -            mode = QIO_CHANNEL_SHUTDOWN_BOTH;
> -        } else if (rd) {
> -            mode = QIO_CHANNEL_SHUTDOWN_READ;
> -        } else {
> -            mode = QIO_CHANNEL_SHUTDOWN_WRITE;
> -        }
> -        if (qio_channel_shutdown(ioc, mode, errp) < 0) {
> -            return -EIO;
> -        }
> -    }
> -    return 0;
> -}

Here we don't return ENOSYS, we return 0 when the channel don't have the feature.

>      f->shutdown = true;
> -    if (!f->ops->shut_down) {
> +    if (!qio_channel_has_feature(f->ioc,
> +                                 QIO_CHANNEL_FEATURE_SHUTDOWN)) {
>          return -ENOSYS;
>      }

Here we return -ENOSYS.

It could only matter here:

./migration.c\02209:        ret = qemu_file_shutdown(ms->to_dst_file);
./migration.c\02218:        ret = qemu_file_shutdown(mis->from_src_file);

And both are for migrate_pause (postcopy recovery), that requires an
implementation that has a qiochannel, so my understanding is that it
don't matter at all.

Code is much better than what we had, anyways.

Later, Juan.