[Libguestfs] [PATCH 3/3] daemon: Run udev_settle after pwrite-device finishes.

[Richard W.M. Jones]

On Thu, Jan 19, 2012 at 03:36:49PM +0100, Jim Meyering wrote:
> Richard W.M. Jones wrote:
> 
> > From: "Richard W.M. Jones" <rjones at redhat.com>
> >
> > When you call close on any block device, udev kicks off a rule which
> > runs blkid to reexamine the device.  We need to wait for this rule to
> > finish running since it holds the device open and can cause other
> > operations to fail, notably BLKRRPART.
> > ---
> >  daemon/file.c   |   17 ++++++++++++++---
> >  daemon/parted.c |    3 ++-
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/daemon/file.c b/daemon/file.c
> > index 91746e0..3e42c1e 100644
> > --- a/daemon/file.c
> > +++ b/daemon/file.c
> > @@ -525,7 +525,7 @@ do_pread_device (const char *device, int count, int64_t offset, size_t *size_r)
> >
> >  static int
> >  pwrite_fd (int fd, const char *content, size_t size, int64_t offset,
> > -           const char *display_path)
> > +           const char *display_path, int settle)
> >  {
> >    ssize_t r;
> >
> > @@ -541,6 +541,17 @@ pwrite_fd (int fd, const char *content, size_t size, int64_t offset,
> >      return -1;
> >    }
> >
> > +  /* When you call close on any block device, udev kicks off a rule
> > +   * which runs blkid to reexamine the device.  We need to wait for
> > +   * this rule to finish running since it holds the device open and
> > +   * can cause other operations to fail, notably BLKRRPART.
> > +   *
> > +   * XXX We should be smarter about when we do this or should get rid
> > +   * of the udev rules since we don't use blkid in cached mode.
> 
> This series looks fine.
> Only question is what about non-block devices?
> 
> How about honoring "settle" only when fd is open on a block device?
> 
>   if (fstat (fd, &st) == 0 && S_ISBLK (st.st_mode) && settle)
>     udev_settle ();

That's done already because the (generated) surrounding code ensures
that do_pwrite is only called with a path in the guest filesystem,
whereas do_pwrite_device is only called with a device name.  Hence the
'settle' flag initialized here:

> > +  if (settle)
> > +    udev_settle ();
> > +
> >    return r;
> >  }
> >
> > @@ -563,7 +574,7 @@ do_pwrite (const char *path, const char *content, size_t size, int64_t offset)
> >      return -1;
> >    }
> >
> > -  return pwrite_fd (fd, content, size, offset, path);
> > +  return pwrite_fd (fd, content, size, offset, path, 0);
> >  }
> >
> >  int
> > @@ -581,7 +592,7 @@ do_pwrite_device (const char *device, const char *content, size_t size,
> >      return -1;
> >    }
> >
> > -  return pwrite_fd (fd, content, size, offset, device);
> > +  return pwrite_fd (fd, content, size, offset, device, 1);
> >  }
> ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top