Re: [libvirt] [PATCH]: Give /dev/disk/by-{id,path} a chance to exist
Chris Lalancette <clalance(a)redhat.com> wrote:
...
on-demand by udev. It's basically a race between udev creating
the directory
and libvirt scanning the directory.
The following patch just adds a retry to the code above so that if it
doesn't exist when we first get there, we give it time to come into being. If
it still hasn't shown up 5 seconds after we started, we give up and throw the error.
...
- if ((dh = opendir(pool->def->target.path)) == NULL) {
+ dh = NULL;
not needed?
+ opentries = 0;
+ while (opentries < 50) {
+ if ((dh = opendir(pool->def->target.path)) != NULL)
+ break;
Hi Chris,
Nice one. Must have been er,.. fun to track down.
The patch looks fine, but how about making it so that it retries
only for a missing final (ENOENT) or intermediate (ENOTDIR)
component. We'd rather not have it retry due to OOM or lack of
permission, e.g.,
dh = opendir(pool->def->target.path);
if (dh != NULL || (errno != ENOENT && errno != ENOTDIR))
break;
but that has so many 'not's that you might prefer to write it this way:
if (! (dh == NULL && (errno == ENOENT || errno == ENOTDIR)))
break;
You can save a two lines by using a for loop.
It's nice to put a name on the constant, e.g.,
enum { WAIT_FOR_UDEV_SECS = 5 };
(and enum is better than #define, because it's usable in a debugger)
for (open_tries = 0; open_tries < WAIT_FOR_UDEV_SECS * 10; open_tries++) {
dh = opendir(pool->def->target.path);
/* Stop if opendir succeeds, or if it fails for any
reason other than a missing file name component. */
if (dh != NULL || (errno != ENOENT && errno != ENOTDIR))
break;
usleep(100 * 1000);
}
Why "open_tries"?
Because I first read "opentries" as "op" "entries" ;-)