Re: [Xen-devel] [PATCH v2 14/18] xen: add implementations of xen-block connect and disconnect functions...

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@xxxxxxxxxx]
> Sent: 07 December 2018 18:21
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: qemu-devel@xxxxxxxxxx; qemu-block@xxxxxxxxxx; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; Stefano Stabellini <sstabellini@xxxxxxxxxx>;
> Kevin Wolf <kwolf@xxxxxxxxxx>; Max Reitz <mreitz@xxxxxxxxxx>
> Subject: Re: [PATCH v2 14/18] xen: add implementations of xen-block
> connect and disconnect functions...
> 
> On Thu, Dec 06, 2018 at 03:08:40PM +0000, Paul Durrant wrote:
> > ...and wire in the dataplane.
> >
> > This patch adds the remaining code to make the xen-block XenDevice
> > functional. The parameters that a block frontend expects to find are
> > populated in the backend xenstore area, and the 'ring-ref' and
> > 'event-channel' values specified in the frontend xenstore area are
> > mapped/bound and used to set up the dataplane.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> 
> With this patch, we should be able to have QEMU instantiate a new
> backend for a guest, right ? (via command line or QMP)
> 
> I've tried, and that doesn't work, the xenstore path for the frontend is
> wrong. In the qemu trace, I have:
>     xs_node_create /local/domain/0/backend/xen-disk/23/268572709
> Which is probably fine, even if not described in xenstore-paths.markdown.
>     xs_node_create /local/domain/23/device/xen-disk/268572709
> Which is not, instead of "xen-disk", we should have "vbd".
> 
> I know that this is fixed in "xen: automatically create
> XenBlockDevice-s", but at least the "vbd" type couldn't be added in this
> patch, or maybe a previous one.
> 
> 
> Another issue seems to be error handling. I've done a very simple test,
> I've added '-device xen-disk,vdev=d536p37,id=mydisk' to the command
> line (which is obvious wrong), and QEMU abort with:
>     qemu-system-i386: hw/block/xen-block.c:174: xen_block_realize:
> Assertion `conf->blk' failed.
> But I've pointed out the error in the code below.
> 
> 
> And just for fun, adding then removing a xen-disk via QMP. Adding works
> fine (once I've fixed the frontend name). I've run the following with
> ./scripts/qmp/qmp-shell:
> blockdev-add driver=file  filename=/root/vm/disk/testing-disk.qcow2 node-
> name=emptyfile
> blockdev-add driver=qcow2 node-name=emptyqcow2 file=emptyfile
> device_add driver=xen-disk vdev=xvdn id=fromqmp drive=emptyqcow2
> 
> But, then, remove doesn't work, running "device_del id=fromqmp" doesn't
> do anything. I guess we can try to fix that later if you don't find
> what's missing.

The missing piece is a hotplug controller 'unplug' or 'unplug_request' method 
implementation. I think the 'right' thing to do is implement 'unplug_request' 
and mimic the behaviour of libxl... i.e. set 'online' to 0 and 'state' to 
closing (i.e. 5) under transaction lock. This means that any connected frontend 
should go through a clean disconnect and then the device will get removed.
To that end I'm going to need to add transaction support to my helper functions 
(in patch #4) and then add in the unplug_request implementation in this patch.

  Paul

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel