All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] iscsi: add error handling for qmp_query_uuid
@ 2013-11-05  0:08 Amos Kong
  2013-11-05  0:33 ` [Qemu-devel] [PATCH v2] " Amos Kong
  2013-11-05  9:03 ` [Qemu-devel] [PATCH] " Paolo Bonzini
  0 siblings, 2 replies; 7+ messages in thread
From: Amos Kong @ 2013-11-05  0:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, lersek

We can't assume that qmp_query_uuid() always returns available value.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 block/iscsi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index a2a961e..1fc1da4 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;
 
     list = qemu_find_opts("iscsi");
     if (list) {
@@ -1074,8 +1075,10 @@ 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();
+    } else if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
         name = qemu_get_vm_name();
     } else {
         name = uuid_info->UUID;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid
  2013-11-05  0:08 [Qemu-devel] [PATCH] iscsi: add error handling for qmp_query_uuid Amos Kong
@ 2013-11-05  0:33 ` Amos Kong
  2013-11-05  7:52   ` Laszlo Ersek
  2013-11-05 13:17   ` Stefan Hajnoczi
  2013-11-05  9:03 ` [Qemu-devel] [PATCH] " Paolo Bonzini
  1 sibling, 2 replies; 7+ messages in thread
From: Amos Kong @ 2013-11-05  0:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, lersek

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;
 
     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();
+        error_free(errp);
+    } else if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
         name = qemu_get_vm_name();
     } else {
         name = uuid_info->UUID;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid
  2013-11-05  0:33 ` [Qemu-devel] [PATCH v2] " Amos Kong
@ 2013-11-05  7:52   ` Laszlo Ersek
  2013-11-05 13:17   ` Stefan Hajnoczi
  1 sibling, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2013-11-05  7:52 UTC (permalink / raw)
  To: Amos Kong; +Cc: kwolf, pbonzini, qemu-devel

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>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH] iscsi: add error handling for qmp_query_uuid
  2013-11-05  0:08 [Qemu-devel] [PATCH] iscsi: add error handling for qmp_query_uuid Amos Kong
  2013-11-05  0:33 ` [Qemu-devel] [PATCH v2] " Amos Kong
@ 2013-11-05  9:03 ` Paolo Bonzini
  2013-11-05 15:05   ` Amos Kong
  1 sibling, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2013-11-05  9:03 UTC (permalink / raw)
  To: Amos Kong; +Cc: kwolf, lersek, qemu-devel

Il 05/11/2013 01:08, Amos Kong ha scritto:
> We can't assume that qmp_query_uuid() always returns available value.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  block/iscsi.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index a2a961e..1fc1da4 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;
>  
>      list = qemu_find_opts("iscsi");
>      if (list) {
> @@ -1074,8 +1075,10 @@ 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();
> +    } else if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
>          name = qemu_get_vm_name();
>      } else {
>          name = uuid_info->UUID;
> 

Does this fix an actual bug?  qmp_query_uuid will never fail, it just
has an Error* argument to comply with the requirements of the code
generator.

Paolo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid
  2013-11-05  0:33 ` [Qemu-devel] [PATCH v2] " Amos Kong
  2013-11-05  7:52   ` Laszlo Ersek
@ 2013-11-05 13:17   ` Stefan Hajnoczi
  2013-11-05 13:44     ` Paolo Bonzini
  1 sibling, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2013-11-05 13:17 UTC (permalink / raw)
  To: Amos Kong; +Cc: kwolf, pbonzini, lersek, qemu-devel

On Tue, Nov 05, 2013 at 08:33:14AM +0800, 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(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

I guess this will go through Paolo's SCSI tree.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid
  2013-11-05 13:17   ` Stefan Hajnoczi
@ 2013-11-05 13:44     ` Paolo Bonzini
  0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2013-11-05 13:44 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kwolf, Amos Kong, lersek, qemu-devel

Il 05/11/2013 14:17, Stefan Hajnoczi ha scritto:
> On Tue, Nov 05, 2013 at 08:33:14AM +0800, 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(-)
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> I guess this will go through Paolo's SCSI tree.

Actually I'm not sure this is necessary, but if people want it, it will. :)

Paolo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH] iscsi: add error handling for qmp_query_uuid
  2013-11-05  9:03 ` [Qemu-devel] [PATCH] " Paolo Bonzini
@ 2013-11-05 15:05   ` Amos Kong
  0 siblings, 0 replies; 7+ messages in thread
From: Amos Kong @ 2013-11-05 15:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kwolf, lersek, qemu-devel

On Tue, Nov 05, 2013 at 10:03:10AM +0100, Paolo Bonzini wrote:
> Il 05/11/2013 01:08, Amos Kong ha scritto:
> > We can't assume that qmp_query_uuid() always returns available value.
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  block/iscsi.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/iscsi.c b/block/iscsi.c
> > index a2a961e..1fc1da4 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;
> >  
> >      list = qemu_find_opts("iscsi");
> >      if (list) {
> > @@ -1074,8 +1075,10 @@ 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();
> > +    } else if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> >          name = qemu_get_vm_name();
> >      } else {
> >          name = uuid_info->UUID;
> > 
> 
> Does this fix an actual bug?

No a bug fixing.

> qmp_query_uuid will never fail, it just
> has an Error* argument to comply with the requirements of the code
> generator.

Yes, this patch isn't necessary for current qmp_query_uuid()
We can ignore this patch.
 
> Paolo

-- 
			Amos.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-11-05 15:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-05  0:08 [Qemu-devel] [PATCH] iscsi: add error handling for qmp_query_uuid Amos Kong
2013-11-05  0:33 ` [Qemu-devel] [PATCH v2] " Amos Kong
2013-11-05  7:52   ` Laszlo Ersek
2013-11-05 13:17   ` Stefan Hajnoczi
2013-11-05 13:44     ` Paolo Bonzini
2013-11-05  9:03 ` [Qemu-devel] [PATCH] " Paolo Bonzini
2013-11-05 15:05   ` Amos Kong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.