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

[David Scott <dave.scott@xxxxxxxxxxxxx>]

On 10/05/13 15:09, Ian Campbell wrote:
> On Fri, 2013-05-10 at 14:55 +0100, George Dunlap wrote:
> > On 10/05/13 14:49, Ian Campbell wrote:
> > > On Fri, 2013-05-10 at 14:46 +0100, 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:
> > > > >
> > > > > 8<------------------------------------------------------------
> > > > >
> > > > >   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 
> > > > > unknown)
> > > > >
> > > > > 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.
> > > A kit if these issues will come back with disk driver domains I think.
> > >
> > > > 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...
> > > That might well be the correct answer, but not for 4.3 I suspect?
> > I can't resolve this into an unambiguous meaning that makes sense. :-)
> > Are you saying that ceph *should* be "raw" long-term, but that it
> > shoudln't be for 4.3?  Or that right now ceph *is* raw, but we should
> > change it for 4.3?  Or that we should change it long-term but leave it
> > for 4.3?
> A ceph type might be the correct long term answer, but I don't think we
> want to be making that change now, do we?.
I think if we add an explicit ceph "format" we'll end up adding 
"formats" for all the other kinds of storage supported by qemu/tapdisk-- 
this seems like avoidable work. The way things stand atm, if someone 
adds a cool new storage type to qemu/tapdisk it should just work with libxl.
Perhaps we could use 'qemu-img' (and some tapdisk equivalent) to verify 
the existence of the disk in place of stat()?
Cheers,
Dave

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