On Tue, Jun 20, 2017 at 12:21:48PM -0400, Dawid Zamirski wrote: > 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@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/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? > So, there was this thing about udevadm settle that wasn't clear to me, because it says it waits for udevd to create all the device nodes - so since udev only manages "/dev", I assumed it wouldn't work for sysfs attributes, but gave it a try since I wasn't sure and, no, we can't use udevadm here, because udevd really doesn't care about sysfs (mdevs only live in sysfs - well, strictly speaking, a corresponding /dev/vfio/ device IS created for an mdev, but for that we get a separate event and because for node device purposes we don't report vfio devices, we ignore such event) hence the explicit, unpredictable, best-effort timeout here. Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
