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 <ak...@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 <ler...@redhat.com>