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

Re: [Xen-devel] blktap, qdisk, xl cd-eject, and xencommons



On 04/30/2013 04:09 PM, George Dunlap wrote:
On 04/30/2013 03:12 PM, Ian Campbell wrote:
On Tue, 2013-04-30 at 13:27 +0100, George Dunlap wrote:
On 04/30/2013 11:21 AM, Ian Campbell wrote:
On Tue, 2013-04-30 at 11:02 +0100, George Dunlap wrote:
The second is problems with xl cd-insert and eject, initially reported
by Fabio Fantoni, and then (accidentally) reproduced by me.  The
problem turns out to be libxl using blktap for cdroms.  Basically,
AFAICT, the whole cd-insert cd-eject thing completely doen't work if
blktap is used to provide it; and it's not a simple fix.

cd-insert/eject are suppose to operate on emulated CDROM, which blktap
simply isn't in a position to provide, even if it was capable of doing
so. So this is certainly wrong IMHO at some level.

All emulated disks have a PV counterpart. Many (all?) PVHVM drivers
choose not to unplug the emulated CDROM and use it in preference to the
PV version (since this way they get proper media change events etc) but
I don't think they are required to behave like this.

In principal it's not wrong to provide the PV face of a device from a
different backend to the emulated one (e.g. we do blkback+qdisk all the
time for disks), but in the interests of simplicity it seems like the
obvious thing to do is to unconditionally use qdisk for both faces of an
HVM guest's CDROM.

Right -- so I guess from a release perspective the best thing to do
would just be to have tools/libxl/libxl_device.c:disk_try_backend() fail
for TAP if disk->is_cdrom is true?

Without having consulted the code that Sounds Plausible, yes.

Is the underling problem understood though? In principal PV=blktap
+EMU=qemu should work, is this just some confusion on libxl's side when
reading the xenstore entries? I we sure we can't trigger that confusion
for other uses of blktap -- e.g. disks.

Well the whole thing is a bit of a tangled mess right now.

For one thing, disk_try_backend() will return an error if disk->format
is EMPTY.  (Which is why in 4.2 if you hit this path, it breaks
everything -- libxl__device_disk_set_backend() sets it to qdev because
TAP fails, so it tries to write new qdev stuff and fails halfway through.)

Is there anyone who actually knows what's supposed to be going on here,
that we can ask to help with this?

I suppose it might have been a mistake that TAP fails for EMPTY -- PHY
will take EMPTY, so maybe TAP can too?

Thanos, do you have any idea about that?  It looks like for an empty
cdrom, it just sets "params" equal to "" in xenstore
(tools/libxl/libxl.c:libxl_cdrom_insert()) -- will that work for blktap,
or cause a problem?

And, can changing the the "params" change the actual disk in the current
version of blktap?

And the other thing is, it looks like for qemu-xen, libxl_cdrom_insert() unconditionally calls libxl__qmp_insert_cdrom(), which passes the path of the file in to qemu anyway -- so is blktap even actually being used in that case? Or is it really just qemu directly accessing the file anyway?

 -George

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


 


Rackspace

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