From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1bx4-0001pd-Aq for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:30:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1bx3-0006ik-5k for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:30:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35690) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1bx2-0006hm-Tg for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:30:17 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 439C3C0AA287 for ; Tue, 1 Nov 2016 16:30:16 +0000 (UTC) From: Paolo Bonzini Date: Tue, 1 Nov 2016 17:29:31 +0100 Message-Id: <1478017783-7703-19-git-send-email-pbonzini@redhat.com> In-Reply-To: <1478017783-7703-1-git-send-email-pbonzini@redhat.com> References: <1478017783-7703-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PULL 18/30] nbd: Less allocation during NBD_OPT_LIST List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org From: Eric Blake Since we know that the maximum name we are willing to accept is small enough to stack-allocate, rework the iteration over NBD_OPT_LIST responses to reuse a stack buffer rather than allocating every time. Furthermore, we don't even have to allocate if we know the server's length doesn't match what we are searching for. Signed-off-by: Eric Blake Message-Id: <1476469998-28592-12-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini --- nbd/client.c | 139 ++++++++++++++++++++++++++++------------------------------- 1 file changed, 67 insertions(+), 72 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 5d94e34..0afb8be 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -254,19 +254,28 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply, return result; } -static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp) +/* Process another portion of the NBD_OPT_LIST reply. Set *@match if + * the current reply matches @want or if the server does not support + * NBD_OPT_LIST, otherwise leave @match alone. Return 0 if iteration + * is complete, positive if more replies are expected, or negative + * with @errp set if an unrecoverable error occurred. */ +static int nbd_receive_list(QIOChannel *ioc, const char *want, bool *match, + Error **errp) { nbd_opt_reply reply; uint32_t len; uint32_t namelen; + char name[NBD_MAX_NAME_SIZE + 1]; int error; - *name = NULL; if (nbd_receive_option_reply(ioc, NBD_OPT_LIST, &reply, errp) < 0) { return -1; } error = nbd_handle_reply_err(ioc, &reply, errp); if (error <= 0) { + /* The server did not support NBD_OPT_LIST, so set *match on + * the assumption that any name will be accepted. */ + *match = true; return error; } len = reply.length; @@ -277,105 +286,91 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp) nbd_send_opt_abort(ioc); return -1; } - } else if (reply.type == NBD_REP_SERVER) { - if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) { - error_setg(errp, "incorrect option length %" PRIu32, len); - nbd_send_opt_abort(ioc); - return -1; - } - if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) { - error_setg(errp, "failed to read option name length"); - nbd_send_opt_abort(ioc); - return -1; - } - namelen = be32_to_cpu(namelen); - len -= sizeof(namelen); - if (len < namelen) { - error_setg(errp, "incorrect option name length"); - nbd_send_opt_abort(ioc); - return -1; - } - if (namelen > NBD_MAX_NAME_SIZE) { - error_setg(errp, "export name length too long %" PRIu32, namelen); - nbd_send_opt_abort(ioc); - return -1; - } + return 0; + } else if (reply.type != NBD_REP_SERVER) { + error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x", + reply.type, NBD_REP_SERVER); + nbd_send_opt_abort(ioc); + return -1; + } - *name = g_new0(char, namelen + 1); - if (read_sync(ioc, *name, namelen) != namelen) { - error_setg(errp, "failed to read export name"); - g_free(*name); - *name = NULL; - nbd_send_opt_abort(ioc); - return -1; - } - (*name)[namelen] = '\0'; - len -= namelen; + if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) { + error_setg(errp, "incorrect option length %" PRIu32, len); + nbd_send_opt_abort(ioc); + return -1; + } + if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) { + error_setg(errp, "failed to read option name length"); + nbd_send_opt_abort(ioc); + return -1; + } + namelen = be32_to_cpu(namelen); + len -= sizeof(namelen); + if (len < namelen) { + error_setg(errp, "incorrect option name length"); + nbd_send_opt_abort(ioc); + return -1; + } + if (namelen != strlen(want)) { if (drop_sync(ioc, len) != len) { - error_setg(errp, "failed to read export description"); - g_free(*name); - *name = NULL; + error_setg(errp, "failed to skip export name with wrong length"); nbd_send_opt_abort(ioc); return -1; } - } else { - error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x", - reply.type, NBD_REP_SERVER); + return 1; + } + + assert(namelen < sizeof(name)); + if (read_sync(ioc, name, namelen) != namelen) { + error_setg(errp, "failed to read export name"); + nbd_send_opt_abort(ioc); + return -1; + } + name[namelen] = '\0'; + len -= namelen; + if (drop_sync(ioc, len) != len) { + error_setg(errp, "failed to read export description"); nbd_send_opt_abort(ioc); return -1; } + if (!strcmp(name, want)) { + *match = true; + } return 1; } +/* Return -1 on failure, 0 if wantname is an available export. */ static int nbd_receive_query_exports(QIOChannel *ioc, const char *wantname, Error **errp) { bool foundExport = false; - TRACE("Querying export list"); + TRACE("Querying export list for '%s'", wantname); if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) { return -1; } TRACE("Reading available export names"); while (1) { - char *name = NULL; - int ret = nbd_receive_list(ioc, &name, errp); + int ret = nbd_receive_list(ioc, wantname, &foundExport, errp); if (ret < 0) { - g_free(name); - name = NULL; + /* Server gave unexpected reply */ return -1; + } else if (ret == 0) { + /* Done iterating. */ + if (!foundExport) { + error_setg(errp, "No export with name '%s' available", + wantname); + nbd_send_opt_abort(ioc); + return -1; + } + TRACE("Found desired export name '%s'", wantname); + return 0; } - if (ret == 0) { - /* Server doesn't support export listing, so - * we will just assume an export with our - * wanted name exists */ - foundExport = true; - break; - } - if (name == NULL) { - TRACE("End of export name list"); - break; - } - if (g_str_equal(name, wantname)) { - foundExport = true; - TRACE("Found desired export name '%s'", name); - } else { - TRACE("Ignored export name '%s'", name); - } - g_free(name); } - - if (!foundExport) { - error_setg(errp, "No export with name '%s' available", wantname); - nbd_send_opt_abort(ioc); - return -1; - } - - return 0; } static QIOChannel *nbd_receive_starttls(QIOChannel *ioc, -- 2.7.4