* [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 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 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] 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).