David Scott
2013-Apr-23 09:36 UTC
[PATCH] Only call stat() when adding a disk if we expect a device to exist.
We consider calling stat() a helpful error check in the following circumstances only: 1. the disk backend type must be PHYsical 2. the disk backend domain must be the same as the running libxl code (assumed to be 0) 3. there must not be a hotplug script because this would imply that the device won''t be created until after the hotplug script has run. With this fix, it is possible to use qemu''s built-in block drivers such as ceph, with a xl config disk spec like this: disk=[ ''backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img'' ] Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- tools/libxl/libxl_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index eeea9d9..ec0fec2 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -236,7 +236,7 @@ int libxl__device_disk_set_backend(libxl__gc *gc, libxl_device_disk *disk) { return ERROR_INVAL; } memset(&a.stab, 0, sizeof(a.stab)); - } else if (!disk->script) { + } else if (disk->backend == LIBXL_DISK_BACKEND_PHY && disk->backend_domid == 0 && !disk->script) { if (stat(disk->pdev_path, &a.stab)) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Disk vdev=%s " "failed to stat: %s", -- 1.8.1.2
Ian Campbell
2013-Apr-23 09:44 UTC
Re: [PATCH] Only call stat() when adding a disk if we expect a device to exist.
On Tue, 2013-04-23 at 10:36 +0100, David Scott wrote:> We consider calling stat() a helpful error check in the following > circumstances only: > 1. the disk backend type must be PHYsical > 2. the disk backend domain must be the same as the running libxl > code (assumed to be 0)AKA LIBXL_TOOLSTACK_DOMID. That line could also do with wrapping but I can do that as I commit if you like.> 3. there must not be a hotplug script because this would imply that > the device won''t be created until after the hotplug script has > run. > > With this fix, it is possible to use qemu''s built-in block drivers > such as ceph, with a xl config disk spec like this: > > disk=[ ''backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img'' ] > > Signed-off-by: David Scott <dave.scott@eu.citrix.com>With the above changes: Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > tools/libxl/libxl_device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c > index eeea9d9..ec0fec2 100644 > --- a/tools/libxl/libxl_device.c > +++ b/tools/libxl/libxl_device.c > @@ -236,7 +236,7 @@ int libxl__device_disk_set_backend(libxl__gc *gc, libxl_device_disk *disk) { > return ERROR_INVAL; > } > memset(&a.stab, 0, sizeof(a.stab)); > - } else if (!disk->script) { > + } else if (disk->backend == LIBXL_DISK_BACKEND_PHY && disk->backend_domid == 0 && !disk->script) { > if (stat(disk->pdev_path, &a.stab)) { > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Disk vdev=%s " > "failed to stat: %s",
David Scott
2013-Apr-23 09:52 UTC
Re: [PATCH] Only call stat() when adding a disk if we expect a device to exist.
On 23/04/13 10:44, Ian Campbell wrote:> On Tue, 2013-04-23 at 10:36 +0100, David Scott wrote: >> >We consider calling stat() a helpful error check in the following >> >circumstances only: >> > 1. the disk backend type must be PHYsical >> > 2. the disk backend domain must be the same as the running libxl >> > code (assumed to be 0) > AKA LIBXL_TOOLSTACK_DOMID. That line could also do with wrapping but I > can do that as I commit if you like.Ah, I grepped around for stuff like "DOMID_SELF" but missed that one :-) I''ll resend... Cheers, Dave