From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Scott Subject: Re: [PATCH] libxl: stat the path for all non-qdisk backends (including unknown) Date: Fri, 10 May 2013 15:21:59 +0100 Message-ID: <518D0287.1030902@eu.citrix.com> References: <1366985563-10926-1-git-send-email-ian.campbell@citrix.com> <1366986572.3142.116.camel@zakaz.uk.xensource.com> <1368193752.27857.124.camel@zakaz.uk.xensource.com> <518CFC5F.9020008@eu.citrix.com> <1368194940.27857.133.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1368194940.27857.133.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: George Dunlap , Sylvain Munaut , Ian Jackson , Roger Pau Monne , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org 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 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 >>>>> 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