Hi,
I''ve got a virtual disk stored on a ceph cluster and am trying to
use qemu''s built-in support for the ceph RBD protocol.
However I notice in libxl_device.c:libxl__device_disk_set_backend
there''s an attempt to stat() the VM''s disk, which fails
because the
disk is remote: there''s never any file or device on the local system:
} else if (!disk->script) {
if (stat(disk->pdev_path, &a.stab)) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Disk vdev=%s "
"failed to stat: %s",
disk->vdev, disk->pdev_path);
return ERROR_INVAL;
}
[At this point I should confess that I did my testing with xen-4.2.1
rather than -unstable, but I didn''t spot any relevant changes to
libxl_device.c]
My attempts to ''xl create'' the VM fail as expected with:
# xl create ubuntu/ceph-ubuntu1204.cfg
Parsing config from ubuntu/ceph-ubuntu1204.cfg
libxl: error: libxl_device.c:243:libxl__device_disk_set_backend: Disk
vdev=hda failed to stat: rbd:rbd/ubuntu1204.img: No such file or directory
I tried to work around the error by adding a dummy hotplug script, but
this is incompatible with the qdisk backend:
case LIBXL_DISK_BACKEND_QDISK:
if (a->disk->script) goto bad_script;
return backend;
However if I work around the stat() by creating a fake file...
# mkdir "rbd:rbd"
# touch rbd\:rbd/ubuntu1204.img
... then it now works:
# xl create ubuntu/ceph-ubuntu1204.cfg
Parsing config from ubuntu/ceph-ubuntu1204.cfg
xc: info: VIRTUAL MEMORY ARRANGEMENT:
Loader: 0000000000100000->000000000019bb24
TOTAL: 0000000000000000->000000003f800000
ENTRY ADDRESS: 0000000000100000
xc: info: PHYSICAL MEMORY ALLOCATION:
4KB PAGES: 0x0000000000000200
2MB PAGES: 0x00000000000001fb
1GB PAGES: 0x0000000000000000
Daemon running with PID 5895
The resulting qemu has the right command-line argument:
-drive
file=rbd:rbd/ubuntu1204.img,if=ide,index=0,media=disk,format=raw,cache=writeback
The qdisk backend also seems operational: (although the "aio:" prefix
is
slightly concerning)
# xenstore-ls /local/domain/0/backend/qdisk/35/768
frontend = "/local/domain/35/device/vbd/768"
params = "aio:rbd:rbd/ubuntu1204.img"
frontend-id = "35"
online = "1"
removable = "0"
bootable = "1"
state = "4"
dev = "hda"
type = "qdisk"
mode = "w"
device-type = "disk"
feature-barrier = "1"
info = "0"
sector-size = "512"
sectors = "33554432"
hotplug-status = "connected"
My PV on HVM linux VM seems happy: it boots up and successfully mounts
its root filesystem on /dev/xvda1.
Is it safe to remove the stat() from libxl_device.c?
For reference my xl config file looks like this:
name="ubuntu1204"
builder=''hvm''
boot=''dc''
vcpus=1
memory=1024
disk=[
''backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img''
]
#vif=[ "bridge=br0" ]
device_model_version=''qemu-xen''
Cheers,
Dave
On Mon, 2013-04-22 at 15:07 +0100, David Scott wrote:> Is it safe to remove the stat() from libxl_device.c?I expect it only makes sense when backendtype=phy and perhaps the disk->script check has served as an imperfect surrogate for that until now? Does changing } else if (!disk->script) { into } else if (disk->backendtype == ...PHY && !disk->script) { Work for you also? Ian.
On 22/04/13 15:36, Ian Campbell wrote:> On Mon, 2013-04-22 at 15:07 +0100, David Scott wrote: >> Is it safe to remove the stat() from libxl_device.c? > > I expect it only makes sense when backendtype=phy and perhaps the > disk->script check has served as an imperfect surrogate for that until > now? > > Does changing > } else if (!disk->script) { > into > } else if (disk->backendtype == ...PHY && !disk->script) { > > Work for you also?Yes, it works nicely against 4.2.1! The exact patch I applied was: --- tools/libxl/libxl_device.c.orig 2013-04-22 14:52:54.745001092 +0000 +++ tools/libxl/libxl_device.c 2013-04-22 14:54:11.566001097 +0000 @@ -236,7 +236,7 @@ return ERROR_INVAL; } memset(&a.stab, 0, sizeof(a.stab)); - } else if (!disk->script) { + } else if (disk->backend == LIBXL_DISK_BACKEND_PHY && !disk->script) { if (stat(disk->pdev_path, &a.stab)) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Disk vdev=%s " "failed to stat: %s", Would you like me to retest this against -unstable? Also would you like me to ''git format-patch/send-email'' or is it too trivial to bother? Thanks, Dave
On Mon, 2013-04-22 at 17:54 +0100, Dave Scott wrote:> On 22/04/13 15:36, Ian Campbell wrote: > > On Mon, 2013-04-22 at 15:07 +0100, David Scott wrote: > >> Is it safe to remove the stat() from libxl_device.c? > > > > I expect it only makes sense when backendtype=phy and perhaps the > > disk->script check has served as an imperfect surrogate for that until > > now? > > > > Does changing > > } else if (!disk->script) { > > into > > } else if (disk->backendtype == ...PHY && !disk->script) { > > > > Work for you also? > > Yes, it works nicely against 4.2.1! The exact patch I applied was: > > --- tools/libxl/libxl_device.c.orig 2013-04-22 14:52:54.745001092 +0000 > +++ tools/libxl/libxl_device.c 2013-04-22 14:54:11.566001097 +0000 > @@ -236,7 +236,7 @@ > return ERROR_INVAL; > } > memset(&a.stab, 0, sizeof(a.stab)); > - } else if (!disk->script) { > + } else if (disk->backend == LIBXL_DISK_BACKEND_PHY && !disk->script) { > if (stat(disk->pdev_path, &a.stab)) { > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Disk vdev=%s " > "failed to stat: %s", > > Would you like me to retest this against -unstable? Also would you like > me to ''git format-patch/send-email'' or is it too trivial to bother?A changelog and a S-o-b should be sufficient, thanks. However, I think this code is probably broken for driver domains as well. I think it probably needs a disk->backend_domid check in addition to what''s there -- if you wouldn''t mind folding that into the patch that would be awesome. There''s so many exclusions now I''m wondering if the stat is actually useful, oh well. I suppose it is still active in some of the more common cases. Ian,