Roger Pau Monné
2011-Jul-22 12:38 UTC
[Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
Hello, This patch fixes the problems described in a previous thread regarding PV machines not booting on NetBSD with libxl toolstack. The patch also prevents libxl from adding PCI entries to xenstore if there is no PCI device configured in the guest. Regards, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2011-Jul-22 13:25 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
On 07/22/11 14:38, Roger Pau Monné wrote:> Hello, > > This patch fixes the problems described in a previous thread regarding > PV machines not booting on NetBSD with libxl toolstack. The patch also > prevents libxl from adding PCI entries to xenstore if there is no PCI > device configured in the guest. > > Regards, Roger.Thanks for this work. Just some coding style nits: - in xenbackend.c, replace C++ comments with C comments. - in libxl_device.c, start the preprecessor lines at column 1. Other than that: Acked-by: Christoph Egger <Christoph.Egger@amd.com> Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-22 13:39 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
On Fri, 2011-07-22 at 13:38 +0100, Roger Pau Monné wrote:> Hello, > > This patch fixes the problems described in a previous thread regarding > PV machines not booting on NetBSD with libxl toolstack. The patch also > prevents libxl from adding PCI entries to xenstore if there is no PCI > device configured in the guest.Thanks Roger. In general we prefer to see individual fixes split up into separate patches for review and commit. Also please can you add to your ~/.hgrc: [diff] showfunc = True It prints the function name in the hunk header which makes it easier to review. The change to the hotplug script seems odd, if the "xtype" paramter is not needed then just remove it altogether, otherwise we need to teach libxl what to write. The PCI change looks good, although you might as well throw the for() loop under the check too. "#ifdef" etc should be in column 0, but the content should be indented normally. The checks on LIBXL_DISK_FORMAT_EMPTY and S_ISBLK are not really related so I would not combine them even though the return is the same. Otherwise the HAVE_PHY_BACKEND_FILE_SUPPORT change itself looks ok What is DONT_REMOVE_VBD_FROM_STORE for? Is this because xenbackendd does it for you or something else? Does your changed xenbackendd still work with xend? Oh, I see what the hotplug script change was about now -- I guess just remove the commented out line? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2011-Jul-22 13:46 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
On 07/22/11 15:39, Ian Campbell wrote:> On Fri, 2011-07-22 at 13:38 +0100, Roger Pau Monné wrote: >> Hello, >> >> This patch fixes the problems described in a previous thread regarding >> PV machines not booting on NetBSD with libxl toolstack. The patch also >> prevents libxl from adding PCI entries to xenstore if there is no PCI >> device configured in the guest. > > Thanks Roger. > > In general we prefer to see individual fixes split up into separate > patches for review and commit. Also please can you add to your ~/.hgrc: > [diff] > showfunc = True > It prints the function name in the hunk header which makes it easier to > review. > > The change to the hotplug script seems odd, if the "xtype" paramter is > not needed then just remove it altogether, otherwise we need to teach > libxl what to write. > > The PCI change looks good, although you might as well throw the for() > loop under the check too. > > "#ifdef" etc should be in column 0, but the content should be indented > normally.Ack.> The checks on LIBXL_DISK_FORMAT_EMPTY and S_ISBLK are not really related > so I would not combine them even though the return is the same. > Otherwise the HAVE_PHY_BACKEND_FILE_SUPPORT change itself looks ok > > What is DONT_REMOVE_VBD_FROM_STORE for? Is this because xenbackendd does > it for you or something else?The hotplug script invoked by xenbackendd removes the vbd entry via xenstored-rm.> Does your changed xenbackendd still work with xend? Oh, I see what the > hotplug script change was about now -- I guess just remove the commented > out line?Ack. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-22 13:54 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
On Fri, 2011-07-22 at 14:46 +0100, Christoph Egger wrote:> > > The checks on LIBXL_DISK_FORMAT_EMPTY and S_ISBLK are not really > related > > so I would not combine them even though the return is the same. > > Otherwise the HAVE_PHY_BACKEND_FILE_SUPPORT change itself looks ok > > > > What is DONT_REMOVE_VBD_FROM_STORE for? Is this because xenbackendd > does > > it for you or something else? > > The hotplug script invoked by xenbackendd removes the vbd entry > via xenstored-rm.I thought that was something which was normally considered the toolstack''s job. Is there a special case for this in xend too? We certainly need to retain some rm''ing of backend directories in the toolstack in at least the forcible destroy code path, as opposed to the graceful shutdown case. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monné
2011-Jul-22 14:55 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
2011/7/22 Ian Campbell <Ian.Campbell@citrix.com>:> On Fri, 2011-07-22 at 13:38 +0100, Roger Pau Monné wrote: >> Hello, >> >> This patch fixes the problems described in a previous thread regarding >> PV machines not booting on NetBSD with libxl toolstack. The patch also >> prevents libxl from adding PCI entries to xenstore if there is no PCI >> device configured in the guest. > > Thanks Roger. > > In general we prefer to see individual fixes split up into separate > patches for review and commit. Also please can you add to your ~/.hgrc: > [diff] > showfunc = True > It prints the function name in the hunk header which makes it easier to > review.Ok done, sorry, I didn''t know about that.> > The change to the hotplug script seems odd, if the "xtype" paramter is > not needed then just remove it altogether, otherwise we need to teach > libxl what to write.The block script needs the parameter xtype because it needs to know if the vbd is a block device (phy) or a regular file (file). Since libxl sets the param type based on disk->backend, and it is always LIBXL_DISK_BACKEND_PHY (even if image is a regular file or a block device, libxl.c:998), the "type" atribute in the xenstore is no longer helpful to choose whether the "params" is a block device or a regular file. That''s why I asked to create a new DISK_BACKEND define, something like VND or LOOP. Or maybe if disk->backend is PHY, use disk->format to set "type" in xenstore (using RAW for "file" and EMPTY for "phy").> > The PCI change looks good, although you might as well throw the for() > loop under the check too. > > "#ifdef" etc should be in column 0, but the content should be indented > normally.Ok, will fix that.> > The checks on LIBXL_DISK_FORMAT_EMPTY and S_ISBLK are not really related > so I would not combine them even though the return is the same. > Otherwise the HAVE_PHY_BACKEND_FILE_SUPPORT change itself looks ok > > What is DONT_REMOVE_VBD_FROM_STORE for? Is this because xenbackendd does > it for you or something else?xenbackendd is a little tricky, and can cause trouble regarding sincronization. libxl and xenbackendd are not synchronized, so sometimes xenstore entries where removed from the store before xenbackend had time to read them, and perform the necessary operations, this resulted in vnd devices not being detached, so I just disabled the remove of vbd entries from libxl. I think it would be better to implement some kind of synchronization between libxl and xenbackend, so the flow would be like: Stop guest ----> xenbackend: detach devices -----> libxl: remove entries Don''t know about the best way to synchronize this, I can only thing about synchronizing them using the "hotplug-status" attribute, but that would mean changing the way libxl__device_destroy works.> > Does your changed xenbackendd still work with xend? Oh, I see what the > hotplug script change was about now -- I guess just remove the commented > out line?Yes, I missed that one, I will remove it.> > Ian. > >Thanks for the feedback, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-22 16:01 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
On Fri, 2011-07-22 at 18:36 +0100, Roger Pau Monne wrote:> @@ -455,6 +460,10 @@ int libxl__devices_destroy(libxl__gc *gc > fe_path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s", domid, l1[i], l2[j]); > be_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend", fe_path)); > if (be_path != NULL) { > +#ifdef DONT_REMOVE_VBD_FROM_STORE > + if (!strcmp(l1[i], "vbd")) > + continue; > +#endif > if (libxl__device_destroy(gc, be_path, force) > 0) > n_watches++; > } else {Most of this patch looks good but I''m still not convinced by this bit. It misses the libxl_device_disk_del case which goes via libxl__device_destroy path not libxl__devices_destroy, as well as (maybe?) breaking the forced-destroy case (where the toolstack is responsible for actually nuking everything without exceptions) but really it''s just that this special casing doesn''t really pass the sniff test and makes me suspect it is just papering over a more fundamental issue somewhere else. The Linux hotplug scripts also consults and then removes the backend dir and it doesn''t seem to cause visible issues, so what is it about xenbackendd and/or the NetBSD scripts which doesn''t like libxl''s behaviour I wonder? If there''s a race there then perhaps Linux also has the issue but in a benign form -- in which case it should be worth putting a generic fix in instead of special casing NetBSD. If this really is correct NetBSD specific behaviour then I think it needs a lot more rationale in the changelog etc. The libxl device teardown stuff is pretty convoluted and I''m reasonably sure it is wrong in several respects, I''ve been meaning to untangle it, perhaps I should get to that sooner rather than later and maybe fix this issue as a side effect. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monné
2011-Jul-22 17:07 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
2011/7/22 Ian Campbell <Ian.Campbell@citrix.com>:> On Fri, 2011-07-22 at 18:36 +0100, Roger Pau Monne wrote: >> @@ -455,6 +460,10 @@ int libxl__devices_destroy(libxl__gc *gc >> fe_path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s", domid, l1[i], l2[j]); >> be_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend", fe_path)); >> if (be_path != NULL) { >> +#ifdef DONT_REMOVE_VBD_FROM_STORE >> + if (!strcmp(l1[i], "vbd")) >> + continue; >> +#endif >> if (libxl__device_destroy(gc, be_path, force) > 0) >> n_watches++; >> } else { > > Most of this patch looks good but I''m still not convinced by this bit. > > It misses the libxl_device_disk_del case which goes via > libxl__device_destroy path not libxl__devices_destroy, as well as > (maybe?) breaking the forced-destroy case (where the toolstack is > responsible for actually nuking everything without exceptions) but > really it''s just that this special casing doesn''t really pass the sniff > test and makes me suspect it is just papering over a more fundamental > issue somewhere else. > > The Linux hotplug scripts also consults and then removes the backend dir > and it doesn''t seem to cause visible issues, so what is it about > xenbackendd and/or the NetBSD scripts which doesn''t like libxl''s > behaviour I wonder? If there''s a race there then perhaps Linux also has > the issue but in a benign form -- in which case it should be worth > putting a generic fix in instead of special casing NetBSD. If this > really is correct NetBSD specific behaviour then I think it needs a lot > more rationale in the changelog etc.I don''t like doing it this way either. How are Linux hotplug scripts called? I''ve been looking arround, and there seems to be a set of udev rules that call the scripts based on changes regarding devfs, the problem is that NetBSD doesn''t have devfs, and we have to watch the xenstore for changes in the status of the devices to trigger the correct scripts. I also see that Linux hotplug scripts also remove entries from xenstore after detaching (xen-hotplug-cleanup), isn''t it wrong if the entries are deleted in libxl? The basic problem with this (if I got it right) is synchronization, entries should be deleted after the corresponding hotplug scripts are done. This happens sometimes in NetBSD, but it is not guaranted, since there''s no rendezvous point to tell libxl that all is fine and xenstore can be cleaned. I think it would be good to set some kind of indication, that hotplug scripts have finished execution before libxl deletes the entries in xenstore, and possibly set a timeout, so libxl is not waiting forever if something goes wrong. Also with xend hotplug scripts where in charge of removing entries from xenstore, but in libxl Linux has no support for block-hotplug using the loop device (only ''phy'' which doesn''t involve any read from xenstore to unplug), that''s why I think this problem has not appeared before.> > The libxl device teardown stuff is pretty convoluted and I''m reasonably > sure it is wrong in several respects, I''ve been meaning to untangle it, > perhaps I should get to that sooner rather than later and maybe fix this > issue as a side effect. >That would be great, I would like to give a hand if I can.> Ian. > >Regards, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monne
2011-Jul-22 17:36 UTC
[Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
# HG changeset patch
# User royger
# Date 1311355770 -7200
# Node ID c3d4cad0fc2d34c8346c8a1c4d493a429939249d
# Parent bb2568713604f2eef45326c271132b06a0bff1cc
libxl: added libxl compatibility with physical backend file for NetBSD
This patch allows running PV machines on NetBSD using the libxenlight toolstack.
Added option for PHY backend to handle regular files and disable auto-remove of
vbd xenstore entries.
Updated xenbackend to detect if vbd is a regular file or a block partition and
pass the correct parameter to ''block'' script.
Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
diff -r bb2568713604 -r c3d4cad0fc2d tools/hotplug/NetBSD/block
--- a/tools/hotplug/NetBSD/block Fri Jul 22 19:24:34 2011 +0200
+++ b/tools/hotplug/NetBSD/block Fri Jul 22 19:29:30 2011 +0200
@@ -19,7 +19,7 @@ error() {
xpath=$1
xstatus=$2
-xtype=$(xenstore-read "$xpath/type")
+xtype=$3
xparams=$(xenstore-read "$xpath/params")
case $xstatus in
diff -r bb2568713604 -r c3d4cad0fc2d tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c Fri Jul 22 19:24:34 2011 +0200
+++ b/tools/libxl/libxl_device.c Fri Jul 22 19:29:30 2011 +0200
@@ -136,15 +136,20 @@ static int disk_try_backend(disk_try_bac
a->disk->format == LIBXL_DISK_FORMAT_EMPTY)) {
goto bad_format;
}
- if (a->disk->format != LIBXL_DISK_FORMAT_EMPTY &&
- !S_ISBLK(a->stab.st_mode)) {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend
phy"
- " unsuitable as phys path not a block device",
- a->disk->vdev);
- return 0;
- }
-
- return backend;
+ if (S_ISBLK(a->stab.st_mode))
+ return backend;
+#ifdef HAVE_PHY_BACKEND_FILE_SUPPORT
+ if (S_ISREG(a->stab.st_mode))
+ return backend;
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
+ " unsuitable as phys path not a block device or"
+ " raw image", a->disk->vdev);
+#else
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
+ " unsuitable as phys path not a block device",
+ a->disk->vdev);
+#endif
+ return 0;
case LIBXL_DISK_BACKEND_TAP:
if (!libxl__blktap_enabled(a->gc)) {
@@ -455,6 +460,10 @@ int libxl__devices_destroy(libxl__gc *gc
fe_path = libxl__sprintf(gc,
"/local/domain/%d/device/%s/%s", domid, l1[i], l2[j]);
be_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
"%s/backend", fe_path));
if (be_path != NULL) {
+#ifdef DONT_REMOVE_VBD_FROM_STORE
+ if (!strcmp(l1[i], "vbd"))
+ continue;
+#endif
if (libxl__device_destroy(gc, be_path, force) > 0)
n_watches++;
} else {
diff -r bb2568713604 -r c3d4cad0fc2d tools/libxl/libxl_osdeps.h
--- a/tools/libxl/libxl_osdeps.h Fri Jul 22 19:24:34 2011 +0200
+++ b/tools/libxl/libxl_osdeps.h Fri Jul 22 19:29:30 2011 +0200
@@ -25,6 +25,8 @@
#if defined(__NetBSD__) || defined(__OpenBSD__)
#include <util.h>
+#define HAVE_PHY_BACKEND_FILE_SUPPORT
+#define DONT_REMOVE_VBD_FROM_STORE
#elif defined(__linux__)
#include <pty.h>
#elif defined(__sun__)
diff -r bb2568713604 -r c3d4cad0fc2d tools/xenbackendd/xenbackendd.c
--- a/tools/xenbackendd/xenbackendd.c Fri Jul 22 19:24:34 2011 +0200
+++ b/tools/xenbackendd/xenbackendd.c Fri Jul 22 19:29:30 2011 +0200
@@ -89,15 +89,15 @@ dodebug(const char *fmt, ...)
}
static void
-doexec(const char *cmd, const char *arg1, const char *arg2)
+doexec(const char *cmd, const char *arg1, const char *arg2, const char *arg3)
{
- dodebug("exec %s %s %s", cmd, arg1, arg2);
+ dodebug("exec %s %s %s %s", cmd, arg1, arg2, arg3);
switch(vfork()) {
case -1:
dolog(LOG_ERR, "can''t vfork: %s", strerror(errno));
break;
case 0:
- execl(cmd, cmd, arg1, arg2, NULL);
+ execl(cmd, cmd, arg1, arg2, arg3, NULL);
dolog(LOG_ERR, "can''t exec %s: %s", cmd, strerror(errno));
exit(EXIT_FAILURE);
/* NOTREACHED */
@@ -145,11 +145,14 @@ xen_setup(void)
int
main(int argc, char * const argv[])
{
+ struct stat stab;
char **vec;
unsigned int num;
char *s;
int state;
char *sstate;
+ char *stype;
+ char *params;
char *p;
char buf[80];
int type;
@@ -169,7 +172,7 @@ main(int argc, char * const argv[])
log_file = optarg;
break;
case ''p'':
- pidfile = pidfile;
+ pidfile = optarg;
case ''s'':
vbd_script = optarg;
break;
@@ -297,11 +300,38 @@ main(int argc, char * const argv[])
strerror(errno));
goto next2;
}
- doexec(s, vec[XS_WATCH_PATH], sstate);
+ doexec(s, vec[XS_WATCH_PATH], sstate, NULL);
break;
case DEVTYPE_VBD:
- doexec(vbd_script, vec[XS_WATCH_PATH], sstate);
+ /* check if given file is a block device or a raw image */
+ snprintf(buf, sizeof(buf), "%s/params", vec[XS_WATCH_PATH]);
+ params = xs_read(xs, XBT_NULL, buf, 0);
+ if(params == NULL) {
+ dolog(LOG_ERR,
+ "Failed to read %s (%s)", buf, strerror(errno));
+ goto next2;
+ }
+ if (stat(params, &stab) < 0) {
+ dolog(LOG_ERR,
+ "Failed to get info about %s (%s)", params,
+ strerror(errno));
+ goto next3;
+ }
+ stype = NULL;
+ if (S_ISBLK(stab.st_mode))
+ stype = "phy";
+ if (S_ISREG(stab.st_mode))
+ stype = "file";
+ if (stype == NULL) {
+ dolog(LOG_ERR,
+ "Failed to attach %s (not a block device or raw image)",
+ params, strerror(errno));
+ goto next3;
+ }
+ doexec(vbd_script, vec[XS_WATCH_PATH], sstate, stype);
+next3:
+ free(params);
break;
default:
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-22 19:07 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
On Fri, 2011-07-22 at 18:07 +0100, Roger Pau Monné wrote:> 2011/7/22 Ian Campbell <Ian.Campbell@citrix.com>: > > On Fri, 2011-07-22 at 18:36 +0100, Roger Pau Monne wrote: > I don''t like doing it this way either. How are Linux hotplug scripts > called? I''ve been looking arround, and there seems to be a set of udev > rules that call the scripts based on changes regarding devfs, the > problem is that NetBSD doesn''t have devfs, and we have to watch the > xenstore for changes in the status of the devices to trigger the > correct scripts.The hotplugs scripts are called from a udev rule but the event is triggered (asynchronously) by the backend driver based upon the changing state node, so ultimately they are triggered from the same xenstore watch. It''s certainly possible that the sequencing is different between Linux and NetBSD or that in the Linux case there is some interlock (deliberate or otherwise) between the kernel side and the hotplug script side which saves our bacon. Who is responsible for driving the backend xenstore state machine under NetBSD? It doesn''t look like xenbackendd (which just reacts to state changes).> I also see that Linux hotplug scripts also remove > entries from xenstore after detaching (xen-hotplug-cleanup), isn''t it > wrong if the entries are deleted in libxl?It may be redundant but I don''t think it is wrong, it should be harmless for both to remove the backend but if it isn''t then I think it should be the toolstack''s responsibility rather than the hotplug script.> The basic problem with this (if I got it right) is synchronization, > entries should be deleted after the corresponding hotplug scripts are > done. This happens sometimes in NetBSD, but it is not guaranted, since > there''s no rendezvous point to tell libxl that all is fine and > xenstore can be cleaned. I think it would be good to set some kind of > indication, that hotplug scripts have finished execution before libxl > deletes the entries in xenstore, and possibly set a timeout, so libxl > is not waiting forever if something goes wrong. > > Also with xend hotplug scripts where in charge of removing entries > from xenstore, but in libxl Linux has no support for block-hotplug > using the loop device (only ''phy'' which doesn''t involve any read from > xenstore to unplug), that''s why I think this problem has not appeared > before.Ah, that may well be it. I had thought that vnd was all in kernel but now I see that it is setup and torn down in the hotplug script, much like for loopback on Linux with xend. On Linux we were able to make the simplifying assumption of not handling this for phy devices because we had blktap to fall back on. NetBSD doesn''t currently have that option. I think part of the problem is that the vbd lifecycle and the backing device''s lifecycle (vnd in this case, but could also be an iscsi login for example) are somewhat entwined via the hotplug scripts etc when really they should be layered and somewhat independent. Ian Jackson has the best handle on this stuff and I know he has plans to move the block script callout stuff forward to better support various more complex block device types, integrate better with the toolstack and generally rationalise the existing interfaces etc so I''d like to hear what he thinks before I suggesting any particular avenue of attack to this problem. He''s travelling right now but may be online intermittently next week (we''re both going to be at the Debian conference in Banja Luka), I think he''s back properly the week after that though.> > The libxl device teardown stuff is pretty convoluted and I''m reasonably > > sure it is wrong in several respects, I''ve been meaning to untangle it, > > perhaps I should get to that sooner rather than later and maybe fix this > > issue as a side effect. > > > > That would be great, I would like to give a hand if I can.Thanks. I''m travelling next week and on vacation the week after. But I need to get onto this soon so it is done for Xen 4.2, since it will an API incompatible change to libxl and we are hoping to declare the API stable at that point. Have you considered coming to the Munich Xen.org hackathon in September? http://wiki.xen.org/xenwiki/MUC_2011_hackathon. That would be a great opportunity to work through some of these issues and ensure that the future direction of libxl''s handling of this stuff works for NetBSD as well as Linux. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monné
2011-Jul-29 11:02 UTC
Re: [Xen-devel] [PATCH] libxl: added libxl compatibility with physical backend file for NetBSD
> The hotplugs scripts are called from a udev rule but the event is > triggered (asynchronously) by the backend driver based upon the changing > state node, so ultimately they are triggered from the same xenstore > watch. > > It''s certainly possible that the sequencing is different between Linux > and NetBSD or that in the Linux case there is some interlock (deliberate > or otherwise) between the kernel side and the hotplug script side which > saves our bacon. > > Who is responsible for driving the backend xenstore state machine under > NetBSD? It doesn''t look like xenbackendd (which just reacts to state > changes).xbdback is responsible for changing the state of xenstore, it performs the state change and attaches devices to guests. It is pretty similar to xenbackendd, but runs in the kernel space.> Ah, that may well be it. I had thought that vnd was all in kernel but > now I see that it is setup and torn down in the hotplug script, much > like for loopback on Linux with xend.It''s basically the same strategy as the one used on Linux with loopback devices> On Linux we were able to make the simplifying assumption of not handling > this for phy devices because we had blktap to fall back on. NetBSD > doesn''t currently have that option.Well, I''m trying to implement something similar to blktap2 in NetBSD, but all in user space, using pud, a driver that enables the implementation of block and character device drivers as userspace daemons (something similar to nbd in Linux I think), I''ve also looked into FUSE, but it required the use of vnd/loop device for the image to be presented as a block device.> I think part of the problem is that the vbd lifecycle and the backing > device''s lifecycle (vnd in this case, but could also be an iscsi login > for example) are somewhat entwined via the hotplug scripts etc when > really they should be layered and somewhat independent. > > Ian Jackson has the best handle on this stuff and I know he has plans to > move the block script callout stuff forward to better support various > more complex block device types, integrate better with the toolstack and > generally rationalise the existing interfaces etc so I''d like to hear > what he thinks before I suggesting any particular avenue of attack to > this problem. He''s travelling right now but may be online intermittently > next week (we''re both going to be at the Debian conference in Banja > Luka), I think he''s back properly the week after that though. > > I''m travelling next week and on vacation the week after. But I need to > get onto this soon so it is done for Xen 4.2, since it will an API > incompatible change to libxl and we are hoping to declare the API stable > at that point.Well, I hope you had a good trip, I will be off next week, and won''t be back until the 10th of August, so we will have to leave this on a hold until then, but I''m not going to let it die.> Have you considered coming to the Munich Xen.org hackathon in September? > http://wiki.xen.org/xenwiki/MUC_2011_hackathon. That would be a great > opportunity to work through some of these issues and ensure that the > future direction of libxl''s handling of this stuff works for NetBSD as > well as Linux.Thanks for the offer, I would really like to come, but I have some other stuff I also have to attend, so I don''t think I will be able to come (it took me so long to answer this email because I was trying to come to the hackathon). Anyway, I hope I can come to future meetings/hackathons! I don''t know if Christoph will we able to attend, if so he may be able to clarify some of the doubts about NetBSD device handling. Thanks and regards, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel