Re: [Xen-devel] [PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file
On Mon, 2015-05-18 at 15:23 +0100, Ian Jackson wrote: Perhaps the bulk should be made into libxl__read_file_contents_core which takes a boolean instructing whether to tolerate magically shrinking files ? Setting that boolean probably ought to arrange to insist that the function gets eof, in case the file is actually bigger rather than smaller than the size. Ian, Wei ? Sounds ok to me. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file
On Mon, May 18, 2015 at 03:23:38PM +0100, Ian Jackson wrote: Chunyan Liu writes ([PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file): Sysfs file has size=4096 but actual file content is less than that. Wow. Is there any danger that the actual size might be 4096 ? Current libxl_read_file_contents will treat it as error when file size and actual file content differs, so reading sysfs file content with this function always fails. Fix it so that we can reuse this function to get sysfs file content in later pvusb work. I'm uncomfortable with removing an error check from this function for all its call sites. I think, sadly, that we are going to need a new function - at least, a new entrypoint. We don't want to repeat the whole of libxl__read_file_contents. Perhaps the bulk should be made into libxl__read_file_contents_core which takes a boolean instructing whether to tolerate magically shrinking files ? Setting that boolean probably ought to arrange to insist that the function gets eof, in case the file is actually bigger rather than smaller than the size. Ian, Wei ? Yes, we need a new entry point. Wei. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file
Chunyan Liu writes ([PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file): Sysfs file has size=4096 but actual file content is less than that. Wow. Is there any danger that the actual size might be 4096 ? Current libxl_read_file_contents will treat it as error when file size and actual file content differs, so reading sysfs file content with this function always fails. Fix it so that we can reuse this function to get sysfs file content in later pvusb work. I'm uncomfortable with removing an error check from this function for all its call sites. I think, sadly, that we are going to need a new function - at least, a new entrypoint. We don't want to repeat the whole of libxl__read_file_contents. Perhaps the bulk should be made into libxl__read_file_contents_core which takes a boolean instructing whether to tolerate magically shrinking files ? Setting that boolean probably ought to arrange to insist that the function gets eof, in case the file is actually bigger rather than smaller than the size. Ian, Wei ? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file
On Sun, Apr 19, 2015 at 11:50:48AM +0800, Chunyan Liu wrote:
Sysfs file has size=4096 but actual file content is less than that.
Current libxl_read_file_contents will treat it as error when file size
and actual file content differs, so reading sysfs file content with
this function always fails. Fix it so that we can reuse this function
to get sysfs file content in later pvusb work.
I'm not sure if I should classify this as a bug in Linux's sysfs
interface.
In any case, we would still like to detect the error case that file size
is changed under our feet.
I have a dumb idea of having a dedicated function that is used to read
sysfs, but I'm not sure if it is too dumb.
I will wait for Ian and Ian's input on this.
Wei.
Signed-off-by: Chunyan Liu cy...@suse.com
---
tools/libxl/libxl_utils.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..18ad2b8 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -363,12 +363,9 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char
*filename,
if (!data) goto xe;
rs = fread(data, 1, datalen, f);
-if (rs != datalen) {
+if (rs != datalen !feof(f)) {
if (ferror(f))
LOGE(ERROR, failed to read %s, filename);
-else if (feof(f))
-LOG(ERROR, %s changed size while we were reading it,
- filename);
else
abort();
goto xe;
--
1.8.5.2
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file
On 5/18/2015 at 10:30 PM, in message 20150518143008.ge9...@zion.uk.xensource.com, Wei Liu wei.l...@citrix.com wrote: On Mon, May 18, 2015 at 03:23:38PM +0100, Ian Jackson wrote: Chunyan Liu writes ([PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file): Sysfs file has size=4096 but actual file content is less than that. Wow. Is there any danger that the actual size might be 4096 ? Current libxl_read_file_contents will treat it as error when file size and actual file content differs, so reading sysfs file content with this function always fails. Fix it so that we can reuse this function to get sysfs file content in later pvusb work. I'm uncomfortable with removing an error check from this function for all its call sites. I think, sadly, that we are going to need a new function - at least, a new entrypoint. We don't want to repeat the whole of libxl__read_file_contents. Perhaps the bulk should be made into libxl__read_file_contents_core which takes a boolean instructing whether to tolerate magically shrinking files ? Setting that boolean probably ought to arrange to insist that the function gets eof, in case the file is actually bigger rather than smaller than the size. Ian, Wei ? Yes, we need a new entry point. Will update. Thanks! Wei. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH V3 2/6] libxl_read_file_contents: fix reading sysfs file
Sysfs file has size=4096 but actual file content is less than that.
Current libxl_read_file_contents will treat it as error when file size
and actual file content differs, so reading sysfs file content with
this function always fails. Fix it so that we can reuse this function
to get sysfs file content in later pvusb work.
Signed-off-by: Chunyan Liu cy...@suse.com
---
tools/libxl/libxl_utils.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..18ad2b8 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -363,12 +363,9 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char
*filename,
if (!data) goto xe;
rs = fread(data, 1, datalen, f);
-if (rs != datalen) {
+if (rs != datalen !feof(f)) {
if (ferror(f))
LOGE(ERROR, failed to read %s, filename);
-else if (feof(f))
-LOG(ERROR, %s changed size while we were reading it,
- filename);
else
abort();
goto xe;
--
1.8.5.2
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
