Re: [Qemu-devel] [RFC PATCH 11/13] hmp: add NBD server commands

[Paolo Bonzini <pbonzini@redhat.com>]

Il 18/09/2012 22:22, Luiz Capitulino ha scritto:
>> > +    if (addr.host == NULL || addr.port == NULL) {
>> > +        error_set(&errp, QERR_SOCKET_CREATE_FAILED);
> You can and should use monitor_printf() directly.

Hmm, why? Then I would have two error paths, one with monitor_printf and
one with hmp_handle_error.  But I will change this to add an Error* to
inet_parse.

>> > +        goto exit;
>> > +    }
>> > +
>> > +    qmp_nbd_server_start(&addr, &errp);
>> > +    if (errp != NULL) {
> Please, use error_is_set() instead.

I'll just rename errp to local_err, which is the usual convention.

>> > +        goto exit;
>> > +    }
>> > +
>> > +    /* Then try adding all block devices.  If one fails, close all and
>> > +     * exit.
>> > +     */
>> > +    bs = NULL;
>> > +    while ((bs = bdrv_next(bs))) {
> To keep hmp.c an honest qmp client, you should use qmp_query_block() instead
> of looping through BSs directly. You'll find other examples if you find for
> qmp_query_block() in this file.

Right, thanks!

Paolo