Re: [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

On Thu, Oct 06, 2016 at 01:14:08PM -0500, Eric Blake wrote:
> On 10/06/2016 11:40 AM, Stefan Hajnoczi wrote:
> > +static int vsock_connect_addr(const struct sockaddr_vm *svm, bool *in_progress,
> > +                              ConnectState *connect_state, Error **errp)
> > +{
> > +    int sock, rc;
> > +
> > +    *in_progress = false;
> > +
> > +    sock = qemu_socket(AF_VSOCK, SOCK_STREAM, 0);
> > +    if (sock < 0) {
> > +        error_setg_errno(errp, errno, "Failed to create socket");
> > +        return -1;
> > +    }
> > +    if (connect_state != NULL) {
> > +        qemu_set_nonblock(sock);
> 
> Isn't the presence of vsock support sufficient to prove that we have
> SOCK_NONBLOCK support as part of our socket() call?  In which case,
> wouldn't it be better to pass that option up front to atomically get a
> non-blocking socket, rather than having to change its state after the fact?

I'm sending a separate patch series to introduce a QemuSockFlags
argument for qemu_socket() and qemu_accept().  It will convert existing
qemu_set_nonblock() callers too.

> > +static VsockSocketAddress *vsock_parse(const char *str, Error **errp)
> > +{
> > +    VsockSocketAddress *addr = NULL;
> > +    char cid[33];
> > +    char port[33];
> > +
> > +    if (sscanf(str, "%32[^:]:%32[^,]", cid, port) != 2) {
> 
> Would it be a wise idea to also use %n to ensure that you aren't
> ignoring trailing garbage?

Okay, will fix in the next version.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]