[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid |
Date: |
Tue, 05 Nov 2013 08:52:36 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130912 Thunderbird/17.0.9 |
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 <address@hidden>
> ---
> 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 <address@hidden>