Re: [Xen-devel] [PATCH v8 08/11] xl/libxl: implement QDISK libxl_device_disk_local_attach

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("Re: [Xen-devel] [PATCH v8 08/11] xl/libxl: implement 
QDISK libxl_device_disk_local_attach"):
> Stefano Stabellini wrote:
> >   int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk 
> > *disk)
> >   {
> > -    /* Nothing to do for PHYSTYPE_PHY. */
> > +    int rc = 0;
> >
> > -    /*
> > -     * For other device types assume that the blktap2 process is
> > -     * needed by the soon to be started domain and do nothing.
> > -     */
> > +    switch (disk->backend) {
> > +        case LIBXL_DISK_BACKEND_QDISK:
> > +            if (disk->vdev != NULL) {
> > +                libxl_device_disk_remove(gc->owner, LIBXL_TOOLSTACK_DOMID,
> > +                        disk, 0);
> 
> I was just looking at this code, and I have the feeling this could be 
> dangerous, since you call libxl__device_disk_local_detach from an 
> already running ao (the bootloader ao), and libxl_device_disk_remove 
> will initiate another ao, and set it to completed inside of an already 
> running ao.

This is indeed forbidden.  The inner ao function exit will reenter the
libxl event loop, which might do almost anything, including
unexpectedly recursively reentering the other asynchronous operation
(the lock doesn't prevent this since it's the same thread).

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel