On Tue, 2017-06-20 at 17:45 +0200, Michal Privoznik wrote:
On 06/20/2017 05:21 PM, Dawid Zamirski wrote:
On Tue, 2017-06-20 at 17:03 +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.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 36 ++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 2 ++ 3 files changed, 39 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c1e9471c5..53878a30f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1698,6 +1698,7 @@ virFileStripSuffix; virFileTouch; virFileUnlock; virFileUpdatePerm; +virFileWaitForAccess; virFileWrapperFdClose; virFileWrapperFdFree; virFileWrapperFdNew; 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). + */ +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); + return -1; + } else if (tries == 10) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to access '%s' after %zu tries"), + path, tries); + return -1; + } else { + VIR_DEBUG("Failed to access '%s', re-try in %zu ms", path, ms); + usleep(ms * 1000); + } + } + + return 0; +}
Just FYI, there's another way to address it by calling udevadm settle before and after "touching" a block device, libguestfs is using this approach and it works very well:
https://github.com/libguestfs/libguestfs/search?utf8=%E2%9C%93&q=ud ev_s ettle&type=
Does it? udevadm settle waits for all the events to be processed, not just the one that we want. The wait time would be unpredictable IMO.
Michal
Well it's a kind of brute-force approach but at least guarantees the device will be accessible once it exits. Why would setting a custom arbitrary timeout be better which may be too short? Is the predictable wait time here more important than being able to open the device at all?