Xentop encounters error in pv-ops domain0, because the VBD path in sysfs changes to "/sys/devices" Also delete the macro in xenstat_netbsd.c, because it is not used. Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com> Best Regards, -- Dongxiao _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2009-Jul-06 13:34 UTC
Re: [Xen-devel] [PATCH] Fix xentop on pv-ops domain0
On Monday 06 July 2009 15:08:06 Xu, Dongxiao wrote:> Xentop encounters error in pv-ops domain0, because the VBD path in sysfs > changes to "/sys/devices" Also delete the macro in xenstat_netbsd.c, > because it is not used. > > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>Can you move the Linux specific code you add in xentop.c into xenstat_linux.c, please? This is an abstraction violation, otherwise. Thanks, Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Thanks for comments. The revised patch is attached! Best Regards, -- Dongxiao -----Original Message----- From: Christoph Egger [mailto:Christoph.Egger@amd.com] Sent: Monday, July 06, 2009 9:35 PM To: xen-devel@lists.xensource.com Cc: Xu, Dongxiao; keir.fraser@eu.citrix.com Subject: Re: [Xen-devel] [PATCH] Fix xentop on pv-ops domain0 On Monday 06 July 2009 15:08:06 Xu, Dongxiao wrote:> Xentop encounters error in pv-ops domain0, because the VBD path in sysfs > changes to "/sys/devices" Also delete the macro in xenstat_netbsd.c, > because it is not used. > > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>Can you move the Linux specific code you add in xentop.c into xenstat_linux.c, please? This is an abstraction violation, otherwise. Thanks, Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hello,>Xentop encounters error in pv-ops domain0, because the VBD path in sysfs changes to "/sys/devices" >Also delete the macro in xenstat_netbsd.c, because it is not used. > >Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com> > >Best Regards, >-- DongxiaoThe patch you''ve provided explicitly checks for a .18-xen kernel; doing so runs the risk of breaking behavior on every forward port after .18. This is quite fragile. It is my suggestion that, instead, we might utilize the patch Ian Campbell suggested some time ago. See http://lists.xensource.com/archives/html/xen-devel/2009-05/msg00299.html That patch is a small tweak: #define SYSFS_VBD_PATH "/sys/bus/xen-backend/devices" Luckily, this change satisfies legacy .18-xen, pvops, (and even older .16-xen kernels), Xenstat - while useful - is brittle enough as it is, so keeping things generic would be best. -Steve Maresca _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2009-Jul-06 15:49 UTC
Re: [Xen-devel] [PATCH] Fix xentop on pv-ops domain0
This patch is good. Christoph On Monday 06 July 2009 16:08:10 Xu, Dongxiao wrote:> Thanks for comments. The revised patch is attached! > > Best Regards, > -- Dongxiao > > -----Original Message----- > From: Christoph Egger [mailto:Christoph.Egger@amd.com] > Sent: Monday, July 06, 2009 9:35 PM > To: xen-devel@lists.xensource.com > Cc: Xu, Dongxiao; keir.fraser@eu.citrix.com > Subject: Re: [Xen-devel] [PATCH] Fix xentop on pv-ops domain0 > > On Monday 06 July 2009 15:08:06 Xu, Dongxiao wrote: > > Xentop encounters error in pv-ops domain0, because the VBD path in sysfs > > changes to "/sys/devices" Also delete the macro in xenstat_netbsd.c, > > because it is not used. > > > > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com> > > Can you move the Linux specific code you add in xentop.c into > xenstat_linux.c, please? This is an abstraction violation, otherwise. > > Thanks, > Christoph-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi, On Mon, 2009-07-06 at 17:49 +0200, Christoph Egger wrote:> This patch is good. >The following is going to break xenstat on almost every forward port of 2.6.18.8 (including Andy''s 2.6.30 vanilla port): + if (strncmp(system_info.release, "2.6.18.8-xen", + sizeof("2.6.18.8-xen")) == 0) + sysfs_vbd_path = "/sys/devices/xen-backend/"; + else + sysfs_vbd_path = "/sys/devices/"; + I would really recommend considering just using /sys/bus/xen-backend/devices, as Stephen noted. Cheers, --Tim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Using "/sys/bus/xen-backend/devices/" is fine, formerly I didn''t notice there was a link in this directory. Thanks! Best Regards, -- Dongxiao -----Original Message----- From: Tim Post [mailto:echo@echoreply.us] Sent: Tuesday, July 07, 2009 12:02 AM To: Christoph Egger Cc: Xu, Dongxiao; xen-devel@lists.xensource.com; keir.fraser@eu.citrix.com Subject: Re: [Xen-devel] [PATCH] Fix xentop on pv-ops domain0 Hi, On Mon, 2009-07-06 at 17:49 +0200, Christoph Egger wrote:> This patch is good. >The following is going to break xenstat on almost every forward port of 2.6.18.8 (including Andy''s 2.6.30 vanilla port): + if (strncmp(system_info.release, "2.6.18.8-xen", + sizeof("2.6.18.8-xen")) == 0) + sysfs_vbd_path = "/sys/devices/xen-backend/"; + else + sysfs_vbd_path = "/sys/devices/"; + I would really recommend considering just using /sys/bus/xen-backend/devices, as Stephen noted. Cheers, --Tim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Please send a final patch. Thanks, Keir On 07/07/2009 01:37, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:> Using "/sys/bus/xen-backend/devices/" is fine, formerly I didn''t notice there > was a link in this directory. Thanks! > > Best Regards, > -- Dongxiao > > -----Original Message----- > From: Tim Post [mailto:echo@echoreply.us] > Sent: Tuesday, July 07, 2009 12:02 AM > To: Christoph Egger > Cc: Xu, Dongxiao; xen-devel@lists.xensource.com; keir.fraser@eu.citrix.com > Subject: Re: [Xen-devel] [PATCH] Fix xentop on pv-ops domain0 > > Hi, > > On Mon, 2009-07-06 at 17:49 +0200, Christoph Egger wrote: >> This patch is good. >> > > The following is going to break xenstat on almost every forward port of > 2.6.18.8 (including Andy''s 2.6.30 vanilla port): > > > + if (strncmp(system_info.release, "2.6.18.8-xen", > + sizeof("2.6.18.8-xen")) == 0) > + sysfs_vbd_path = "/sys/devices/xen-backend/"; > + else > + sysfs_vbd_path = "/sys/devices/"; > + > > I would really recommend considering just > using /sys/bus/xen-backend/devices, as Stephen noted. > > Cheers, > --Tim > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi, Keir,
The following patch is from Ian Campbell, sent at Fri 5/8/2009 5:03 PM.
This path doesn''t exist in the pvops kernel. Looks
like /sys/bus/xen-backend/devices is just as good and exists in both
pvops and 2.6.18 kernels.
Subject: xenstat: Use backend path which is compatible with pvops and 2.6.18-xen
kernels.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 07812e857e67 tools/xenstat/libxenstat/src/xenstat_linux.c
--- a/tools/xenstat/libxenstat/src/xenstat_linux.c Thu Apr 09 12:09:14 2009
+0100
+++ b/tools/xenstat/libxenstat/src/xenstat_linux.c Fri May 08 09:40:45 2009
+0100
@@ -31,7 +31,7 @@
#include "xenstat_priv.h"
-#define SYSFS_VBD_PATH "/sys/devices/xen-backend/"
+#define SYSFS_VBD_PATH "/sys/bus/xen-backend/devices"
struct priv_data {
FILE *procnetdev;
Best Regards,
-- Dongxiao
-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
Sent: Tuesday, July 07, 2009 3:16 PM
To: Xu, Dongxiao; echo@echoreply.us; Christoph Egger; Steven Maresca
Cc: xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] Fix xentop on pv-ops domain0
Please send a final patch.
Thanks,
Keir
On 07/07/2009 01:37, "Xu, Dongxiao" <dongxiao.xu@intel.com>
wrote:
> Using "/sys/bus/xen-backend/devices/" is fine, formerly I
didn''t notice there
> was a link in this directory. Thanks!
>
> Best Regards,
> -- Dongxiao
>
> -----Original Message-----
> From: Tim Post [mailto:echo@echoreply.us]
> Sent: Tuesday, July 07, 2009 12:02 AM
> To: Christoph Egger
> Cc: Xu, Dongxiao; xen-devel@lists.xensource.com; keir.fraser@eu.citrix.com
> Subject: Re: [Xen-devel] [PATCH] Fix xentop on pv-ops domain0
>
> Hi,
>
> On Mon, 2009-07-06 at 17:49 +0200, Christoph Egger wrote:
>> This patch is good.
>>
>
> The following is going to break xenstat on almost every forward port of
> 2.6.18.8 (including Andy''s 2.6.30 vanilla port):
>
>
> + if (strncmp(system_info.release, "2.6.18.8-xen",
> + sizeof("2.6.18.8-xen")) == 0)
> + sysfs_vbd_path = "/sys/devices/xen-backend/";
> + else
> + sysfs_vbd_path = "/sys/devices/";
> +
>
> I would really recommend considering just
> using /sys/bus/xen-backend/devices, as Stephen noted.
>
> Cheers,
> --Tim
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Reasonably Related Threads
- [PATCH] xenstat: Correct copy of network device name
- [PATCH] Enable xenstat to use xenstore & fix bugzilla #311
- Regarding xenstat [test.c:3:24: fatal error: xenstat.h: No such file or directory]
- [REQEST] about xentop -b
- [PATCH] SIGTERM and SIGINT handler to flush xentop -b outputs