Yang Hongyang
2010-May-13 09:36 UTC
[Xen-devel] [PATCH 2/2]xl: Check invalid domid in find_domain()
If domain id is invalid, find_domain should return error because
the specified domain does not exist.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
diff -r a49dfaa986b0 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Fri May 14 01:27:26 2010 +0800
+++ b/tools/libxl/xl_cmdimpl.c Fri May 14 01:29:36 2010 +0800
@@ -137,7 +137,11 @@
fprintf(stderr, "%s is an invalid domain identifier
(rc=%d)\n", p, rc);
exit(2);
}
- common_domname = was_name ? p : 0;
+ common_domname = was_name ? p : libxl_domid_to_name(&ctx, domid);
+ if (!common_domname) {
+ fprintf(stderr, "%s is an invalid domain identifier.\n", p);
+ exit(2);
+ }
}
#define LOG(_f, _a...) dolog(__FILE__, __LINE__, __func__, _f "\n",
##_a)
--
Regards
Yang Hongyang
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-May-17 23:27 UTC
Re: [Xen-devel] [PATCH 2/2]xl: Check invalid domid in find_domain()
On 05/13/2010 02:36 AM, Yang Hongyang wrote:> If domain id is invalid, find_domain should return error because > the specified domain does not exist. >I think this patch is causing some regressions. For example, I can no longer rename dom0: sh-4.0# xl list 0 Name ID Mem VCPUs State Time(s) (null) 0 3321 4 r-- 741.7 sh-4.0# xl rename 0 dom0 0 is an invalid domain identifier. I haven''t specifically worked out what''s failing here... J> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> > > diff -r a49dfaa986b0 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Fri May 14 01:27:26 2010 +0800 > +++ b/tools/libxl/xl_cmdimpl.c Fri May 14 01:29:36 2010 +0800 > @@ -137,7 +137,11 @@ > fprintf(stderr, "%s is an invalid domain identifier (rc=%d)\n", p, rc); > exit(2); > } > - common_domname = was_name ? p : 0; > + common_domname = was_name ? p : libxl_domid_to_name(&ctx, domid); > + if (!common_domname) { > + fprintf(stderr, "%s is an invalid domain identifier.\n", p); > + exit(2); > + } > } > > #define LOG(_f, _a...) dolog(__FILE__, __LINE__, __func__, _f "\n", ##_a) > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Yang Hongyang
2010-May-18 01:18 UTC
Re: [Xen-devel] [PATCH 2/2]xl: Check invalid domid in find_domain()
Hi jeremy, On 05/18/2010 07:27 AM, Jeremy Fitzhardinge wrote:> On 05/13/2010 02:36 AM, Yang Hongyang wrote: >> If domain id is invalid, find_domain should return error because >> the specified domain does not exist. >> > > I think this patch is causing some regressions. For example, I can no > longer rename dom0: > > sh-4.0# xl list 0 > Name ID Mem VCPUs State Time(s) > (null) 0 3321 4 r-- 741.7 > sh-4.0# xl rename 0 dom0 > 0 is an invalid domain identifier. > > I haven''t specifically worked out what''s failing here...I think there''s some other problems in the kernel or xl. First of all, the (null) name should not appera in the list, if there is, then this VM is not fullly destoried I think, but when I''m trying to destory the vm, libxl reported errors and the VM still exists. Second of all, I notice that your DOM0''s name is (null), and State is running, how could you do that? I manully tryed out ''xl rename "(null)"'' to rename DOM0''s name to (null), but after that I can still rename the name back with my patch. We often met the "(null)" problem''s either, but never met that problem on DOM0... And the "(null)" problem often happened after "xl destory". We can not figure out a method to reproduce this problem. Below is the output(without this patch): [root@centos libxl]# xl list Name ID Mem VCPUs State Time(s) Domain-0 0 1024 2 r-- 250.2 (null) 2 0 1 --d 52657.4 Mini-OS 4 32 1 r-- 0.0 [root@centos libxl]# xl destroy 2 [0] libxl.c:764:libxl_domain_destroy: xs_rm failed for /xapi/2: No such file or directory [root@centos libxl]# xl rename 2 a [root@centos libxl]# xl list Name ID Mem VCPUs State Time(s) Domain-0 0 1024 2 r-- 251.9 a 2 0 1 --d 52657.4 Mini-OS 4 32 1 r-- 0.0 [root@centos libxl]# xl destroy 2 [0] libxl.c:764:libxl_domain_destroy: xs_rm failed for /xapi/2: No such file or directory [root@centos libxl]# xl destroy a a is an invalid domain identifier (rc=-1) [root@centos libxl]# ==================================================================================with the patch: [root@centos libxl]# xl list Name ID Mem VCPUs State Time(s) Domain-0 0 1024 2 r-- 21.9 [root@centos libxl]# xl rename 0 "(null)" [root@centos libxl]# xl list Name ID Mem VCPUs State Time(s) (null) 0 1024 2 r-- 22.0 [root@centos libxl]# xl rename 0 Domain-0 [root@centos libxl]# xl list Name ID Mem VCPUs State Time(s) Domain-0 0 1024 2 r-- 22.2 [root@centos libxl]#> > J > >> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> >> >> diff -r a49dfaa986b0 tools/libxl/xl_cmdimpl.c >> --- a/tools/libxl/xl_cmdimpl.c Fri May 14 01:27:26 2010 +0800 >> +++ b/tools/libxl/xl_cmdimpl.c Fri May 14 01:29:36 2010 +0800 >> @@ -137,7 +137,11 @@ >> fprintf(stderr, "%s is an invalid domain identifier (rc=%d)\n", p, rc); >> exit(2); >> } >> - common_domname = was_name ? p : 0; >> + common_domname = was_name ? p : libxl_domid_to_name(&ctx, domid); >> + if (!common_domname) { >> + fprintf(stderr, "%s is an invalid domain identifier.\n", p); >> + exit(2); >> + } >> } >> >> #define LOG(_f, _a...) dolog(__FILE__, __LINE__, __func__, _f "\n", ##_a) >> >> > > >-- Regards Yang Hongyang _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-May-18 17:01 UTC
Re: [Xen-devel] [PATCH 2/2]xl: Check invalid domid in find_domain()
On 05/17/2010 06:18 PM, Yang Hongyang wrote:> Hi jeremy, > > On 05/18/2010 07:27 AM, Jeremy Fitzhardinge wrote: > >> On 05/13/2010 02:36 AM, Yang Hongyang wrote: >> >>> If domain id is invalid, find_domain should return error because >>> the specified domain does not exist. >>> >>> >> I think this patch is causing some regressions. For example, I can no >> longer rename dom0: >> >> sh-4.0# xl list 0 >> Name ID Mem VCPUs State Time(s) >> (null) 0 3321 4 r-- 741.7 >> sh-4.0# xl rename 0 dom0 >> 0 is an invalid domain identifier. >> >> I haven''t specifically worked out what''s failing here... >> > I think there''s some other problems in the kernel or xl.The kernel has nothing to do with domain management, so the problem must either be in usermode or in Xen (but since Xen is known to work...).> First of all, the > (null) name should not appera in the list, if there is, then this > VM is not fullly destoried I think, but when I''m trying to destory the vm, libxl > reported errors and the VM still exists. Second of all, I notice that your > DOM0''s name is (null), and State is running, how could you do that? I manully > tryed out ''xl rename "(null)"'' to rename DOM0''s name to (null), but after > that I can still rename the name back with my patch. >I boot my machine without ever having run xend. When I first log in: # oxenstored # xl list Name ID Mem VCPUs State Time(s) (null) 0 3321 4 r-- 143.1 But the problem is that "xl" doesn''t let me operate on dom0 at all. For example, if I do: # xl mem-set 0 2000000 0 is an invalid domain identifier.> We often met the "(null)" problem''s either, but never met that problem on DOM0... > And the "(null)" problem often happened after "xl destory". We can not > figure out a method to reproduce this problem. >I see this regularly, especially after doing a "xl save" on a domain. J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel