[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] libxl: stat the path for all non-qdisk backends (including unknown)

On 10/05/13 15:42, George Dunlap wrote:
On 10/05/13 15:31, Dave Scott wrote:
On 10/05/13 15:19, George Dunlap wrote:
On 10/05/13 15:09, Dave Scott wrote:
On 10/05/13 14:46, George Dunlap wrote:
On Fri, Apr 26, 2013 at 3:29 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
On Fri, 2013-04-26 at 15:17 +0100, Sylvain Munaut wrote:
Since the intention of that commit was to allow for qdisk backends with no
explicit file in dom0 (i.e. network remote backend such as ceph) the lowest
impact fix appears to be to make that explicit. This should probably be
revisited to rationalize the probing.
What about the remote disk case of blktap ?  blktap2.5 supports NBD
already AFAIK
and I'm pretty sure I'll hit that same stat issue soon for another
remote blktap case.
Right, sounds like I should have gone with my first instinct which was:


     From 884beff4a897d785f61705dcfa2f048536982d7c Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Fri, 26 Apr 2013 12:41:43 +0100
Subject: [PATCH] libxl: stat the path for all non-qdisk backends (including 

The commit a8a1f236a296 "libxl: Only call stat() when adding a disk if we
expect a device to exist." changed things to only stat the file when the phy
backend was explicitly requested. This broke the case where we are probing and
would normally be able to decide on the phy option.
So at the moment qdisk backends aren't checked at all with this --
which means that if you give a path to a file that doesn't exist via,
for example, xl cd-insert, things fail in weird ways:

1. In qemu-traditional, the command silently completes; the effect is
that the disk currently in the drive is ejected

2. in qemu-upstream, qmp returns an error which is reported.  The disk
is ejected from the guest, but the xenstore entries are not updated
(still contain the old values)

It seems like we should probably also at least check if disk_format is RAW.

OTOH, I don't seen an option for disk_format to be ceph; is it just
listed as "raw" as well?  That doesn't seem right...
AFAICT a ceph disk is in the "raw" format but it uses a custom network
protocol to actually read and write the blocks. I imagine on the ceph
servers the disk is stored in a cleverer format, but all qemu/tapdisk
see are plain blocks with no fancy encoding, no .vhd or .qcow.
Oh, hang on -- does qdisk / tapdisk then just open a plain file? Then it
*is* just a raw file for these purposes; and we should still be able to
call stat() on it.
There is no file :) All we have is a "URL"-like thing which looks like:


which qemu/tapdisk accesses via a C library called librados. The C
library takes the "URL" and makes a network connection to a storage
server and reads and writes blocks over TCP/IP. No filesystems are
involved, "rbd:rbd/foo.img" doesn't exist in the filesystem.

Right -- so I think this should be a separate type to RAW.  I agree that
making a new format for each new protocol isn't very scalable -- but
there should be a "URL" or "VIRTUAL" format to indicate that there is no
real file.

Something like that would be fine. I agree "format=raw" is a bit odd, when there's no real "format" involved, only a network protocol.

Right now RAW doesn't seem to be talking about the format, but about the
format string; i.e., "I pass this straight into qemu and it figures out
what it means."  That's not right.

For 4.3 -- I guess the simplest thing to fix the actual bug (xl
cd-insert giving weird results) is to put a stat in xl_cmdimpl.c before
calling libxl_cdrom_insert().

That works for me. Do non-CDROM cases fail in a more reasonable way?


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.