Re: Patch files for xen-4.1's xl

[Roger Pau Monné <roger.pau%entel.upc.edu@localhost>]

On Tue, May 29, 2012 at 7:05 PM, Toby Karyadi 
<toby.karyadi%gmail.com@localhost> wrote:
> Hi,
>
> Since there's renewed interest in Xen 4 on netbsd, I thought it's timely to
> get these patches out. The only reason I looked into this is because xm and
> xend have been giving me grief lately requiring killing the xend child
> process every so often (this is with Xen 4.1 and netbsd current, Xen 3.3 is
> fine). Additionally the xen folks claims that they're going to deprecate xm
> and xend, so it's time to move on.

Thanks for the hard work, I'm going to comment on some of this
problems/solutions, but again thanks for sharing this patches with us.

> Okay, attached are two patch files for xentools41 that would correct
> problems in xl when dealing with PV domUs using file (vnd) based disks. To
> use them:
> 1. Save the attachments into the sysutils/xentools41/patches directory of
> your pkgsrc tree and make sure you retain the original file names to
> maintain sorting order during the 'make patch' phase.
> 2. cd sysutils/xentools41/ and do 'make makepatchsum'
> 3. make patch just to make sure that the patches get applied properly, or
> otherwise you can just to make build. I made my changes against the version
> 1 day before pkgsrc-2012Q1, but I think it will apply cleanly against pkgsrc
> HEAD, although I haven't checked it.
>
> There were two problems that I encountered:
> 1. Using xl, if you have a file based disk, it would decide that it needs to
> start a qemu-dm process. With xm and xend on netbsd, it would just setup vnd
> properly. I compared the xenstore entries between xm and xl and noticed that
> with xl the 'type' entry under
> /local/domain/0/backend/vbd/<domid>/<frontend_diskid>/ was always 'phy', as
> opposed to 'file' when a file based disk is specified. So, the changes were
> merely to convince xl to set 'type' to 'file' for file-based disks. I also
> added support for extended physical-like disk types so that you can define
> 'phy:<custom_block_device_type>:<custom_block_device_spec>,hda,w'. This is
> done again by setting the 'type' to whatever you set for
> <custom_block_device_type>. The /usr/pkg/etc/xen/scripts/block script needs
> to be adjusted to accommodate your <custom_block_device_type>, but now there
> is more flexibility, e.g. to provide iscsi, or pud/rump-based disks through
> dom0, or whatever, which is what blktap in linux-xen is trying to do but
> which we don't have. Note, that defining phy:file:... would execute the same
> code path in the block script as defining 'file:...' virtual disks. FYI, the
> block script is called by xenbackendd who watches changes in xenstore under
> /local/domain/0/backend/vbd, but not for anything related to tap or qdisk.
> These changes are in the attached file patch-libxl_vnd_phylike_support.

In upstream Xen we have split some of this backend checking functions
into two separate files, one for NetBSD and one for Linux, so we have
a little more liberty to decide what we allow, and what we deny.

Also, I'm not convinced of using phy:vnd:.., since it will break
Linux/NetBSD common syntax. The right solution to this from my point
of view seems to be to allow the user to define custom hotplug scripts
from the guest config file, so you can decide how to handle the disk
(keeping both "phy:..." and "file:.." nomenclatures, which merely
point to the type of disk being used, but not the backend to use to
attach it).

> 2. Another problem with xl, which I suspect is netbsd dom0 specific, is that
> the block script doesn't get called after domU shutdown, and so if you have
> vnd based disks, vnconfig -u won't get called. This problem didn't appear
> with xl block-attach and xl block-detach however.
> These changes are in patch-libxl_vbd_destroy_fixup. This patch might be
> readily submitted upstream to the Xen folks, but I don't know whether the
> intrusive file: support for netbsd and the unsanctioned phy:<subtype>:
> scheme in the previous patch file would be well received upstream.

This fix is racy, xl will destroy the backend xenstore entries when
the backend reaches state 6, but xenbackendd needs those entries to
disconnect the device, so although it might work, it is prone to
errors, and I guess you have been very lucky that xenbackendd has
always executed the scripts before xl has deleted the backend entries
.

I'm working on this, and we are going to make libxl udev agnostic for
4.2. Hotplug scripts will be called from xl (the toolstack), so we can
keep a strict control of when to execute them and the result of the
execution, this change will also get rid of xenbackendd, and Linux and
NetBSD will have the same mechanism for calling hotplug scripts.

> I've also tested my HVM domU file-based XP image against these patches and
> it also worked. I didn't have to change anything in my HVM domU config file.
> I don't expect to encounter any problem with a physical block device based
> HVM domU.
>
> I'm thinking of refactoring and modifying the block script such that a
> script called block-<custom_block_device_type> will get called if available,
> so that users can extend the block script easily. But, feel free to beat me
> to it.

That would be really nice, so we can choose from a variety of
backends, I would strongly recommend to try to upstream those :)

> Anyway, give it a whirl and let me know if there is any problem/suggestions.
>
> Cheers,
> Toby