Re: [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid

[Laszlo Ersek <lersek@redhat.com>]

On 11/05/13 01:33, Amos Kong wrote:
> We can't assume that qmp_query_uuid() always returns available value.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> v2: free errp if it's set
> ---
>  block/iscsi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index a2a961e..4051bdd 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1059,6 +1059,7 @@ static char *parse_initiator_name(const char *target)
>      const char *name;
>      char *iscsi_name;
>      UuidInfo *uuid_info;
> +    Error *errp = NULL;

I think we usually call this "local_err" or something similar.

>  
>      list = qemu_find_opts("iscsi");
>      if (list) {
> @@ -1074,8 +1075,11 @@ static char *parse_initiator_name(const char *target)
>          }
>      }
>  
> -    uuid_info = qmp_query_uuid(NULL);
> -    if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> +    uuid_info = qmp_query_uuid(&errp);
> +    if (error_is_set(&errp)) {
> +        name = qemu_get_vm_name();

I wonder if such an error should make parse_initiator_name() fail (which
is currently not possible), or maybe succeed with qemu_get_vm_name() but
print the UUID error message as a "warning" (could be pointless though).

> +        error_free(errp);
> +    } else if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
>          name = qemu_get_vm_name();
>      } else {
>          name = uuid_info->UUID;
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>