Re: [PATCH RFC] libxl: set disk defaults in remove/destroy functions

From: Wei Liu <wei.liu2@citrix.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>, Jim Fehlig <jfehlig@suse.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [PATCH RFC] libxl: set disk defaults in remove/destroy functions
Date: Mon, 2 Feb 2015 16:32:25 +0000	[thread overview]
Message-ID: <20150202163225.GB25824@zion.uk.xensource.com> (raw)
In-Reply-To: <1422883633.19293.19.camel@citrix.com>

On Mon, Feb 02, 2015 at 01:27:13PM +0000, Ian Campbell wrote:
> On Mon, 2015-01-26 at 16:14 -0700, Jim Fehlig wrote:
> 
> Cc-ing the other toolstack maintainers, both of whom have more
> familiarity with this part of libxl than I.
> 
> > The attached patch is a hack I cooked up to fix one of the libvirt-TCK
> > Xen failures.  The test (200-disk-hotplug.t) attempts to hot add and
> > remove a disk from a running domain.  The add works fine, but remove
> > fails with
> > 
> > libxl: debug: libxl.c:3858:libxl_device_disk_remove: ao 0x7f9b9c0015a0:
> > create: how=(nil) callback=(nil) poller=
> > 0x7f9ba0004590
> > libxl: error: libxl.c:2399:libxl__device_from_disk: unrecognized disk
> > backend type: 0
> > 
> > The test does not define a backend type, in which case the libvirt libxl
> > driver allows libxl to choose an appropriate backend via
> > libxl__device_disk_set_backend().  The backend type is never set on a
> > remove operation, hence it fails with the above error.
> > 
> > I spent some time trying to figure out the best place to set backend
> > type on remove, but in the end could only come up with this hack.  It
> > wouldn't feel so gross if I could have simply added
> > 'libxl__device_##type##_setdefault(gc, type);' to the existing
> > DEFINE_DEVICE_REMOVE macro, but alas libxl__device_nic_setdefault() has
> > a different prototype than the other devices.
> > 
> > Better suggestions welcomed!  One I considered was fixing this in
> > libvirt.  But the Xen community suggested allowing libxl to choose a
> > suitable backend when not specified, so I think this recommendation
> > should be symmetrical in the add and remove operations.
> > 

FWIW xl block-detach calls libxl_vdev_to_device_disk to convert a vdev
to disk. That function reads xenstore to get the actual backend of that
specific vdev. Don't know how useful it is to libvirt though.

Maybe we should look up disk's backend in libxl_device_disk_remove? Not
sure what's the best approach.

Wei.

> > Regards,
> > Jim
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 