On Sun, 02/09 10:48, Paolo Bonzini wrote:
> Before:
> $ ./qemu-io-old
> qemu-io-old> open -r -o file.driver=nbd
> one of path and host must be specified.
> qemu-io-old: can't open device (null): Could not open image: Invalid
> argument
> $ ./qemu-io-old
> qemu-io-old> open -r -o file.driver=nbd,file.host=foo,file.path=bar
> path and host may not be used at the same time.
> qemu-io-old: can't open device (null): Could not open image: Invalid
> argument
>
> After:
> $ ./qemu-io
> qemu-io> open -r -o file.driver=nbd
> qemu-io: can't open device (null): one of path and host must be specified.
> $ ./qemu-io
> qemu-io> open -r -o file.driver=nbd,file.host=foo,file.path=bar
> qemu-io: can't open device (null): path and host may not be used at the
> same time.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> block/nbd.c | 34 ++++++++++++++++------------------
> include/block/nbd.h | 1 -
> nbd.c | 12 ------------
> 3 files changed, 16 insertions(+), 31 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index fd89083..69f336b 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -188,19 +188,18 @@ out:
> g_free(file);
> }
>
> -static int nbd_config(BDRVNBDState *s, QDict *options, char **export)
> +static void nbd_config(BDRVNBDState *s, QDict *options, char **export,
> + Error **errp)
> {
> Error *local_err = NULL;
>
> if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
> if (qdict_haskey(options, "path")) {
> - qerror_report(ERROR_CLASS_GENERIC_ERROR, "path and host may not "
> - "be used at the same time.");
> + error_setg(errp, "path and host may not be used at the same
> time.");
> } else {
> - qerror_report(ERROR_CLASS_GENERIC_ERROR, "one of path and host "
> - "must be specified.");
> + error_setg(errp, "one of path and host must be specified.");
> }
> - return -EINVAL;
> + return;
> }
>
> s->client.is_unix = qdict_haskey(options, "path");
> @@ -209,9 +208,8 @@ static int nbd_config(BDRVNBDState *s, QDict *options,
> char **export)
>
> qemu_opts_absorb_qdict(s->socket_opts, options, &local_err);
> if (error_is_set(&local_err)) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> - return -EINVAL;
> + error_propagate(errp, local_err);
> + return;
> }
>
> if (!qemu_opt_get(s->socket_opts, "port")) {
> @@ -222,19 +220,17 @@ static int nbd_config(BDRVNBDState *s, QDict *options,
> char **export)
> if (*export) {
> qdict_del(options, "export");
> }
> -
> - return 0;
> }
>
> -static int nbd_establish_connection(BlockDriverState *bs)
> +static int nbd_establish_connection(BlockDriverState *bs, Error **errp)
> {
> BDRVNBDState *s = bs->opaque;
> int sock;
>
> if (s->client.is_unix) {
> - sock = unix_socket_outgoing(qemu_opt_get(s->socket_opts, "path"));
> + sock = unix_connect(qemu_opt_get(s->socket_opts, "path"), errp);
Why not use unix_connect_opts like below?
> } else {
> - sock = tcp_socket_outgoing_opts(s->socket_opts);
> + sock = inet_connect_opts(s->socket_opts, errp, NULL, NULL);
> if (sock >= 0) {
> socket_set_nodelay(sock);
> }
> @@ -255,17 +251,19 @@ static int nbd_open(BlockDriverState *bs, QDict
> *options, int flags,
> BDRVNBDState *s = bs->opaque;
> char *export = NULL;
> int result, sock;
> + Error *local_err = NULL;
>
> /* Pop the config into our state object. Exit if invalid. */
> - result = nbd_config(s, options, &export);
> - if (result != 0) {
> - return result;
> + nbd_config(s, options, &export, &local_err);
> + if (local_err) {
Isn't error_is_set() better here?
> + error_propagate(errp, local_err);
> + return -EINVAL;
> }
>
> /* establish TCP connection, return error if it fails
> * TODO: Configurable retry-until-timeout behaviour.
> */
> - sock = nbd_establish_connection(bs);
> + sock = nbd_establish_connection(bs, errp);
> if (sock < 0) {
> return sock;
> }
Thanks,
Fam
