Re: [Qemu-devel] [PATCH v1 16/21] char: remove qemu_chr_open_socket_fd method

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Mar 09, 2016 at 06:53:29PM +0100, Paolo Bonzini wrote:
> 
> 
> On 09/03/2016 18:28, Daniel P. Berrange wrote:
> > The qemu_chr_open_socket_fd method takes care of either doing a
> > synchronous socket connect, or creating a listener socket. Part
> > of the work when creating the listener socket is to register a
> > watch for incoming clients. The caller of qemu_chr_open_socket_fd
> > may not want this watch created, as it might be doing a synchronous
> > wait for the first client. Rather than passing yet more parameters
> > into qemu_chr_open_socket_fd to let it handle this, just remove
> > the qemu_chr_open_socket_fd method an inline its functionality
> > into the caller. This allows for a clearer control flow and shorter
> > code.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  qemu-char.c | 61 +++++++++++++++++++++++++------------------------------------
> >  1 file changed, 25 insertions(+), 36 deletions(-)
> > 
> > diff --git a/qemu-char.c b/qemu-char.c
> > index 579c620..fee2540 100644
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -3109,32 +3109,6 @@ static void qemu_chr_socket_connected(Object *src, Error *err, void *opaque)
> >      object_unref(OBJECT(sioc));
> >  }
> >  
> > -static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
> > -{
> > -    TCPCharDriver *s = chr->opaque;
> > -    QIOChannelSocket *sioc = qio_channel_socket_new();
> > -
> > -    if (s->is_listen) {
> > -        if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) {
> > -            goto fail;
> > -        }
> > -        s->listen_ioc = sioc;
> > -        s->listen_tag = qio_channel_add_watch(
> > -            QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
> > -    } else {
> > -        if (qio_channel_socket_connect_sync(sioc, s->addr, errp) < 0) {
> > -            goto fail;
> > -        }
> > -        tcp_chr_new_client(chr, sioc);
> > -        object_unref(OBJECT(sioc));
> > -    }
> > -
> > -    return true;
> > -
> > - fail:
> > -    object_unref(OBJECT(sioc));
> > -    return false;
> > -}
> >  
> >  /*********************************************************/
> >  /* Ring buffer chardev */
> > @@ -4349,26 +4323,41 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
> >          s->reconnect_time = reconnect;
> >      }
> >  
> > +    sioc = qio_channel_socket_new();
> > +    qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL);
> 
> This might not be needed after all.

Indeed, it isn't needed anymore.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|