[Libvir] [PATCH]Report the error from xend for Domain-0
Hi, When I test virsh subcommands, dump save, for Domain-0, It says following normal messages, but the processing is not done. #virsh dump 0 a Domain 0 dumpd to a #echo $? 0 #virsh save 0 a Domain 0 saved to a #echo $? 0 I made a patch which reports the error from xend like this. #virsh dump 0 a libvir: Xen Daemon error : POST operation failed: (xend.err 'Cannot save privileged domain 0') libvir: error : library call virDomainSave failed, possibly not supported error: Failed to save domain 0 to a # echo $? 1 #virsh dump 0 a libvir: Xen Daemon error : POST operation failed: (xend.err 'Cannot dump core for privileged domain 0') libvir: error : library call virDomainCoreDump failed, possibly not supported error: Failed to core dump domain 0 to a # echo $? 1 Signed-off-by: Kazuki Mizushima <mizushima.kazuk@jp.fujitsu.com> Thanks, Kazuki Mizushima Index: xend_internal.c (libvirt-0.2.0) ---------------------------------------------------------------------- --- ../../libvirt-0.2.0/src/xend_internal.c 2007-02-15 01:16:16.000000000 +0900 +++ xend_internal.c 2007-02-27 22:45:05.000000000 +0900 @@ -507,7 +507,10 @@ xend_post(virConnectPtr xend, const char if ((ret < 0) || (ret >= 300)) { virXendError(xend, VIR_ERR_POST_FAILED, content); - } else if ((ret = 202) && (strstr(content, "failed") != NULL)) { + } else if ((ret == 202) && (strstr(content, "failed") != NULL)) { + virXendError(xend, VIR_ERR_POST_FAILED, content); + ret = -1; + } else if ((ret == 202) && (strcasestr(content, "cannot") != NULL)) { virXendError(xend, VIR_ERR_POST_FAILED, content); ret = -1; } ----------------------------------------------------------------------
On Tue, Feb 27, 2007 at 07:34:01PM +0900, Kazuki?$B!!Mizushima wrote:
When I test virsh subcommands, dump save, for Domain-0, It says following normal messages, but the processing is not done.
- } else if ((ret = 202) && (strstr(content, "failed") != NULL)) { + } else if ((ret == 202) && (strstr(content, "failed") != NULL)) {
Yikes - that's a good catch - compiler wasn't even complaining because it was surrounded in brackets. I'll apply this fix shortly. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Tue, Feb 27, 2007 at 01:47:29PM +0000, Daniel P. Berrange wrote:
On Tue, Feb 27, 2007 at 07:34:01PM +0900, Kazuki?$B!!Mizushima wrote:
When I test virsh subcommands, dump save, for Domain-0, It says following normal messages, but the processing is not done.
- } else if ((ret = 202) && (strstr(content, "failed") != NULL)) { + } else if ((ret == 202) && (strstr(content, "failed") != NULL)) {
Yikes - that's a good catch - compiler wasn't even complaining because it was surrounded in brackets. I'll apply this fix shortly.
I made one small change to your patch when committing it. It had been using 'strcasestr(content, "cannot"), but the strcasestr function is a GNU-ism so I replaced it with 'strstr(content, "Cannot")'. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Hi, Dan After I studying this issue, I change my mind about fixing method. How do you think about this? 1)Change the scanning code from "Cannot" to "xend.err" By the survey on xend_internal.c, I found Xend reports errors which surely has "xend.err" term, For this reason, I want to change term from "Cannot" to "xend.err". N.B. When I try to grep xen/xend by "XendError", there are many error strings not catched above term. With this fixes, virsh command can report error from Xend about the information of not only Dom0 but also DomX. (except going on next Driver & guarding at xm) 2)String("xend.err") checking in HTTP return code (200) case Also when http returns normal Status Code(OK, Created, Accepted), I think that now it is better to check the content, because like below it is. # virsh restore a (a is not existing) Domain restored from a # echo $? 0 But if the processing is not done, the cause is that http returns 200 status code though content has error strings. Here is remaking patch. Signed-off-by: Kazuki Mizushima <mizushima.kazuk@jp.fujitsu.com> Thanks, Kazuki Mizushima Index: src/xend_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xend_internal.c,v retrieving revision 1.95 diff -u -p -r1.95 xend_internal.c --- src/xend_internal.c 27 Feb 2007 15:50:03 -0000 1.95 +++ src/xend_internal.c 1 Mar 2007 11:04:46 -0000 @@ -511,7 +511,7 @@ xend_post(virConnectPtr xend, const char } else if ((ret == 202) && (strstr(content, "failed") != NULL)) { virXendError(xend, VIR_ERR_POST_FAILED, content); ret = -1; - } else if ((ret == 202) && (strstr(content, "Cannot") != NULL)) { + } else if ((( ret >= 200) && (ret <= 202)) && (strstr(content, "xend.err") != NULL)) { /* This is to catch case of 'virsh dump Domain-0 foo' * which returns a success code, but the word 'Cannot' * in body to indicate error =================================================================== ----- Original Message ----- From: "Daniel P. Berrange" <berrange@redhat.com> To: "Kazuki?$B!!Mizushima" <mizushima.kazuk@jp.fujitsu.com> Cc: <libvir-list@redhat.com> Sent: Wednesday, February 28, 2007 12:50 AM Subject: Re: [Libvir] [PATCH]Report the error from xend for Domain-0
On Tue, Feb 27, 2007 at 01:47:29PM +0000, Daniel P. Berrange wrote:
On Tue, Feb 27, 2007 at 07:34:01PM +0900, Kazuki?$B!!Mizushima wrote:
When I test virsh subcommands, dump save, for Domain-0, It says following normal messages, but the processing is not done.
- } else if ((ret = 202) && (strstr(content, "failed") != NULL)) { + } else if ((ret == 202) && (strstr(content, "failed") != NULL)) {
Yikes - that's a good catch - compiler wasn't even complaining because it was surrounded in brackets. I'll apply this fix shortly.
I made one small change to your patch when committing it. It had been using 'strcasestr(content, "cannot"), but the strcasestr function is a GNU-ism so I replaced it with 'strstr(content, "Cannot")'.
Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Thu, Mar 01, 2007 at 08:13:40PM +0900, Kazuki Mizushima wrote:
Hi, Dan
After I studying this issue, I change my mind about fixing method. How do you think about this?
1)Change the scanning code from "Cannot" to "xend.err"
By the survey on xend_internal.c, I found Xend reports errors which surely has "xend.err" term, For this reason, I want to change term from "Cannot" to "xend.err".
N.B. When I try to grep xen/xend by "XendError", there are many error strings not catched above term.
With this fixes, virsh command can report error from Xend about the information of not only Dom0 but also DomX. (except going on next Driver & guarding at xm)
2)String("xend.err") checking in HTTP return code (200) case
Also when http returns normal Status Code(OK, Created, Accepted), I think that now it is better to check the content, because like below it is.
Yes, this works much bettter that the current impl, so I've committed it. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Daniel P. Berrange wrote:
On Tue, Feb 27, 2007 at 07:34:01PM +0900, Kazuki?$B!!Mizushima wrote:
When I test virsh subcommands, dump save, for Domain-0, It says following normal messages, but the processing is not done.
- } else if ((ret = 202) && (strstr(content, "failed") != NULL)) { + } else if ((ret == 202) && (strstr(content, "failed") != NULL)) {
Yikes - that's a good catch - compiler wasn't even complaining because it was surrounded in brackets. I'll apply this fix shortly.
Yes, that's a good reason NOT to bracket tests. Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
participants (4)
-
Daniel P. Berrange -
Kazuki Mizushima -
Kazuki Mizushima
-
Richard W.M. Jones