On Fri, Nov 30, 2018 at 04:03:37PM -0600, Eric Blake wrote:
> Add some parameters to make this function reusable in upcoming
> export listing, where we will want to capture the name and
> description rather than compare against a user-supplied name.
> No change in semantics to the existing caller.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> nbd/client.c | 66 +++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index 1dc8f83e19a..27785c55d0a 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -232,18 +232,21 @@ static int nbd_handle_reply_err(QIOChannel *ioc,
> NBDOptionReply *reply,
> return result;
> }
>
> -/* 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. */
> +/* Process another portion of the NBD_OPT_LIST reply. If @want, then
> + * set *@match if the current reply matches @want or if the server
> + * does not support NBD_OPT_LIST, otherwise leave @match alone.
> + * Otherwise, @nameout and @description are malloc'd to contain
> + * NUL-terminated copies of the reply. 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)
> + char **nameout, char **description, Error **errp)
> {
> NBDOptionReply reply;
> uint32_t len;
> uint32_t namelen;
> - char name[NBD_MAX_NAME_SIZE + 1];
> + char array[NBD_MAX_NAME_SIZE + 1];
> + char *name = array;
> int error;
>
> if (nbd_receive_option_reply(ioc, NBD_OPT_LIST, &reply, errp) < 0) {
> @@ -253,7 +256,12 @@ static int nbd_receive_list(QIOChannel *ioc, const char
> *want, bool *match,
> 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;
> + if (want) {
> + *match = true;
> + } else if (!error) {
> + error_setg(errp, "Server does not support export lists");
> + error = -1;
> + }
> return error;
> }
> len = reply.length;
> @@ -290,30 +298,49 @@ static int nbd_receive_list(QIOChannel *ioc, const char
> *want, bool *match,
> nbd_send_opt_abort(ioc);
> return -1;
> }
> - if (namelen != strlen(want)) {
> - if (nbd_drop(ioc, len, errp) < 0) {
> - error_prepend(errp,
> - "failed to skip export name with wrong length: ");
> - nbd_send_opt_abort(ioc);
> - return -1;
> + if (want) {
> + if (namelen != strlen(want)) {
> + if (nbd_drop(ioc, len, errp) < 0) {
> + error_prepend(errp,
> + "failed to skip export name with wrong length:
> ");
> + nbd_send_opt_abort(ioc);
> + return -1;
> + }
> + return 1;
> }
> - return 1;
> + assert(namelen < sizeof(array));
> + } else {
> + assert(nameout);
> + *nameout = name = g_new(char, namelen + 1);
> }
>
> - assert(namelen < sizeof(name));
> if (nbd_read(ioc, name, namelen, errp) < 0) {
> error_prepend(errp, "failed to read export name: ");
> nbd_send_opt_abort(ioc);
> + if (!want) {
> + free(name);
> + }
> return -1;
> }
> name[namelen] = '\0';
> len -= namelen;
> - if (nbd_drop(ioc, len, errp) < 0) {
> + if (!want) {
> + assert(description);
> + *description = g_new(char, len + 1);
> + if (nbd_read(ioc, *description, len, errp) < 0) {
> + error_prepend(errp, "failed to read export description: ");
> + nbd_send_opt_abort(ioc);
> + free(name);
> + free(*description);
> + return -1;
> + }
> + (*description)[len] = '\0';
> + } else if (nbd_drop(ioc, len, errp) < 0) {
> error_prepend(errp, "failed to read export description: ");
> nbd_send_opt_abort(ioc);
> return -1;
> }
> - if (!strcmp(name, want)) {
> + if (want && !strcmp(name, want)) {
> *match = true;
> }
> return 1;
> @@ -498,7 +525,8 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
> }
>
> while (1) {
> - int ret = nbd_receive_list(ioc, wantname, &foundExport, errp);
> + int ret = nbd_receive_list(ioc, wantname, &foundExport,
> + NULL, NULL, errp);
>
> if (ret < 0) {
> /* Server gave unexpected reply */
I found this patch a lot easier to review once I started to use the
‘git show -w’ option. The changes look like a reasonable adaptation
of the function, and the only consumer of the function (at this point)
is unaffected, so:
Reviewed-by: Richard W.M. Jones <rjo...@redhat.com>
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
