On Tue, Jun 20, 2017 at 05:22:52PM +0200, Peter Krempa wrote: > On Tue, Jun 20, 2017 at 17:03:31 +0200, Erik Skultety wrote: > > Since we have a number of places where we workaround timing issues with > > devices, attributes (files in general) not being available at the time > > of processing them by calling usleep in a loop for a fixed number of > > tries, we could as well have a utility function that would do that. > > Therefore we won't have to duplicate this ugly workaround even more. > > > > This is a prerequisite for > > https://bugzilla.redhat.com/show_bug.cgi?id=1463285. > > This statement is useless. The helper function can be reused elsewhere. > > > > > Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx> > > --- > > src/libvirt_private.syms | 1 + > > src/util/virfile.c | 36 ++++++++++++++++++++++++++++++++++++ > > src/util/virfile.h | 2 ++ > > 3 files changed, 39 insertions(+) > > [...] > > > diff --git a/src/util/virfile.c b/src/util/virfile.c > > index 6bbcc3d15..0b1a91699 100644 > > --- a/src/util/virfile.c > > +++ b/src/util/virfile.c > > @@ -4164,3 +4164,39 @@ virFileReadValueString(char **value, const char *format, ...) > > VIR_FREE(str); > > return ret; > > } > > + > > + > > +/** > > + * virFileWaitForAccess: > > + * @path: absolute path to a sysfs attribute (can be a symlink) > > + * @ms: how long to wait (in milliseconds) > > + * @tries: how many times should we try to wait for @path to become accessible > > + * > > + * Checks the existence of @path. In case the file defined by @path > > + * doesn't exist, we wait for it to appear in 100ms (for up to @tries times). > > + * > > + * Returns 0 on success, -1 on error (ENOENT is fine here). > > This description does not make sense. You don't state that this reports > errors. Also the mention of ENOENT does not make sense. > > This function in fact sometimes returns enoent if the file does not > appear until timeout. > > > + */ > > +int > > +virFileWaitForAccess(const char *path, size_t ms, size_t tries) > > +{ > > + errno = 0; > > + > > + /* wait for @path to be accessible in @ms milliseconds, up to @tries */ > > + while (tries-- > 0 && !virFileExists(path)) { > > + if (errno != ENOENT) { > > + virReportSystemError(errno, "%s", path); > > This does not really explain stuff to users. You might want to add a > more comprehensive error message or leave error reporting to users. > > > + return -1; > > + } else if (tries == 10) { > > This does not make any sense. The while loop counts down and checks if > tries is more than 10. And this checks that tries is equal to 10. > > So if somebody passes 11 as @tries he/she gets this error? If tries is > set to < 10 you get success even on timeout? > > Did you modify the code without testing it? Damn :/, I actually tested it (after those 3+ modifications I did to it in an iterative manner) and it worked, but surely didn't hit the issue you describe. Nevertheless, yep, it's wrong and I need to rework it. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
