Daniel Stodden
2010-Jun-07 21:54 UTC
[Xen-devel] [PATCH 0 of 7] blktap2: The tap-ctl userspace control utility and library
Hi.
Apart from removing some dead code and tidying up tapdisk-vbd a
little, the bigger introduces our new tap-ctl utility code.
The code aims to be a complete replacement for the original blktap2
control path, run through sysfs. Fully implemented in userspace, all
tapdisks now gained a small IPC layer on top of Unix domain sockets.
Sample usage:
# tap-ctl allocate
/dev/xen/blktap-2/tapdev0
# tap-ctl spawn
tapdisk spawned with pid 4168
# tap-ctl list
4168 - - - -
- 0 - - -
# tap-ctl attach -p 4168 -m 0
# tap-ctl list
4168 0 0 - -
# tap-ctl open -p 4168 -m 0 -a aio:/var/tmp/lenny.ext
# tap-ctl list
4168 0 0 aio /var/tmp/lenny.ext
# tap-ctl close -p 4168 -m 0
# tap-ctl detach -p 4168 -m 0
# tap-ctl free -m 0
The above example is a bit noisy, because it''s mediating between minor
number (block devices), tapdisks and tapdisk VBDs (the tapdisk I/O
queue running a bdev) in detail.
There are shortcuts. At the same time, the low-level interface should
be general enough to stay extensible, and help accomodate some of the
more esoteric features, like shared images and/or multiple VBDs
sharing the same tapdisk.
Cheers,
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-07 21:54 UTC
[Xen-devel] [PATCH 1 of 7] blktap2: Remove tapdisk-ipc module
Obsoleted with blktapctrl. Signed-off-by: Jake Wires <jake.wires@citrix.com> Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
These only mattered for XCP''s LVHD with blktap1. Signed-off-by: Jake Wires <jake.wires@citrix.com> Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-07 21:54 UTC
[Xen-devel] [PATCH 3 of 7] blktap2: Fix tapdisk disktype issues
Stop coercing drivers/disktype code into the tool stack. Make both blktapctrl and tap-ctl transfer type/path pairs as "<type>:<path>" strings. Remove the message.disktype integer altogether. Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> Signed-off-by: Jake Wires <jake.wires@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-07 21:54 UTC
[Xen-devel] [PATCH 4 of 7] blktap2: Fix E/DPRINTF defs all around the driver/ subdir
This is just to avoid macro madness among subdirs sharing blktaplib.h. Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> Signed-off-by: Jake Wires <jake.wires@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-07 21:54 UTC
[Xen-devel] [PATCH 5 of 7] blktap2: Cleanup vdi stacking code
Removes some rough edges, memory leakage (?), fixes __list_splice, ... Signed-off-by: Jake Wires <jake.wires@citrix.com> Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-07 21:54 UTC
[Xen-devel] [PATCH 6 of 7] blktap2: The tap-ctl userspace control utility and library
Tapdisk control in userspace, a replacement for the original blktap2 control stack, which had to pass a kernel space interface based on sysfs nodes. All tapdisk processes listen for commands on a unix stream socket. The control library supports scanning the socket namespace for running tapdisks, VBD minors allocated, associated images and state inquiry. Control operations include allocating/releasing devices, spawning tapdisks, opening/closing images, attaching disk images to devices. disk pause/resume operations and runtime switching of disk images. Signed-off-by: Jake Wires <jake.wires@citrix.com> Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-07 21:54 UTC
[Xen-devel] [PATCH 7 of 7] blktap2: Port Xend to the tap-ctl interface
Signed-off-by: Jake Wires <jake.wires@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Jun-09 20:25 UTC
Re: [Xen-devel] [PATCH 7 of 7] blktap2: Port Xend to the tap-ctl interface
On 06/07/2010 02:54 PM, Daniel Stodden wrote:> Signed-off-by: Jake Wires <jake.wires@citrix.com> > >Presumably libxl needs corresponding changes? At the moment all my tapdisk processes are dying complaining about being passed a "-n" option. J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-09 20:57 UTC
Re: [Xen-devel] [PATCH 7 of 7] blktap2: Port Xend to the tap-ctl interface
On Wed, 2010-06-09 at 16:25 -0400, Jeremy Fitzhardinge wrote:> On 06/07/2010 02:54 PM, Daniel Stodden wrote: > > Signed-off-by: Jake Wires <jake.wires@citrix.com> > > > > > > Presumably libxl needs corresponding changes? At the moment all my > tapdisk processes are dying complaining about being passed a "-n" option.Uh, oh. I never looked into that stuff. Will see what it takes. Anyone else maybe interested in doing that? No? Daniel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-09 22:11 UTC
Re: [Xen-devel] [PATCH 7 of 7] blktap2: Port Xend to the tap-ctl interface
On Wed, 2010-06-09 at 16:57 -0400, Daniel Stodden wrote:> On Wed, 2010-06-09 at 16:25 -0400, Jeremy Fitzhardinge wrote: > > On 06/07/2010 02:54 PM, Daniel Stodden wrote: > > > Signed-off-by: Jake Wires <jake.wires@citrix.com> > > > > > > > > > > Presumably libxl needs corresponding changes? At the moment all my > > tapdisk processes are dying complaining about being passed a "-n" option. > > Uh, oh. I never looked into that stuff. > > Will see what it takes. > > Anyone else maybe interested in doing that? No?Does anyone have a sample config file flying around? Thanks. Daniel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Jun-09 22:32 UTC
Re: [Xen-devel] [PATCH 7 of 7] blktap2: Port Xend to the tap-ctl interface
On 06/09/2010 03:11 PM, Daniel Stodden wrote:> On Wed, 2010-06-09 at 16:57 -0400, Daniel Stodden wrote: > >> On Wed, 2010-06-09 at 16:25 -0400, Jeremy Fitzhardinge wrote: >> >>> On 06/07/2010 02:54 PM, Daniel Stodden wrote: >>> >>>> Signed-off-by: Jake Wires <jake.wires@citrix.com> >>>> >>>> >>>> >>> Presumably libxl needs corresponding changes? At the moment all my >>> tapdisk processes are dying complaining about being passed a "-n" option. >>> >> Uh, oh. I never looked into that stuff. >> >> Will see what it takes. >> >> Anyone else maybe interested in doing that? No? >> > Does anyone have a sample config file flying around? >Stock xm ones should work almost as-is with xl, except you: * need to specify a path to the config, as it doesn''t default to /etc/xen * remove all explicit python code and just leave the assignments * use full paths for things like hvmloader J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Cheers, Daniel tools/blktap2/control/tap-ctl.c | 4 +- tools/blktap2/control/tap-ctl-list.c | 30 +++++++++++ tools/blktap2/control/tap-ctl.h | 1 + tools/Rules.mk | 4 + tools/blktap2/control/Makefile | 23 ++++++-- tools/blktap2/control/tap-ctl.h | 3 +- tools/libxl/Makefile | 4 +- tools/libxl/libxl.c | 93 ++++++++++------------------------- 8 files changed, 83 insertions(+), 79 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-10 03:26 UTC
[Xen-devel] [PATCH 1 of 4] blktap2: Fix broken tap-ctl-list type/path filter logic
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> tools/blktap2/control/tap-ctl.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-10 03:26 UTC
[Xen-devel] [PATCH 2 of 4] blktap2: Add tap_ctl_find_minor
Slack ''tap-ctl find -t <type> -f <path>''. Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> tools/blktap2/control/tap-ctl-list.c | 30 ++++++++++++++++++++++++++++++ tools/blktap2/control/tap-ctl.h | 1 + 2 files changed, 31 insertions(+), 0 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-10 03:26 UTC
[Xen-devel] [PATCH 3 of 4] blktap2: Build libblktapctl.so
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> tools/Rules.mk | 4 ++++ tools/blktap2/control/Makefile | 23 ++++++++++++++++------- tools/blktap2/control/tap-ctl.h | 3 +-- 3 files changed, 21 insertions(+), 9 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-10 03:26 UTC
[Xen-devel] [PATCH 4 of 4] libxl: Use libblktapctl.so
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> tools/libxl/Makefile | 4 +- tools/libxl/libxl.c | 93 +++++++++++++++------------------------------------ 2 files changed, 29 insertions(+), 68 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jun-10 15:43 UTC
Re: [Xen-devel] [PATCH 4 of 4] libxl: Use libblktapctl.so
On Thu, 10 Jun 2010, Daniel Stodden wrote:> Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>This patch broke file: and tap:aio support in xl/libxl. Did you actually test it? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jun-10 16:25 UTC
Re: [Xen-devel] [PATCH 4 of 4] libxl: Use libblktapctl.so
> -static char *get_blktap2_device(struct libxl_ctx *ctx, char *name, char *type) > +static char *make_blktap2_device(struct libxl_ctx *ctx, > + const char *name, const char *type) > { > - char buf[1024]; > - char *p; > - int devnum; > - FILE *f = fopen("/sys/class/blktap2/devices", "r"); > - > - > - while (!feof(f)) { > - if (fscanf(f, "%d %s", &devnum, buf) != 2) > - continue; > - p = strchr(buf, '':''); > - if (p == NULL) > - continue; > - p++; > - if (!strcmp(p, name) && !strncmp(buf, type, 3)) { > - fclose(f); > - return libxl_sprintf(ctx, "/dev/xen/blktap-2/tapdev%d", devnum); > - } > - } > - fclose(f); > - return NULL; > + char *params, *devname; > + int err; > + params = libxl_sprintf(ctx, "%s:%s", type, name); > + devname = NULL; > + err = tap_ctl_create(params, &devname); > + free(params); > + return err ? NULL : devname; > } >you shouldn''t free pointers returned by libxl internal functions, because libxl will take care of free''ing them.> int libxl_device_disk_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk) > @@ -1328,38 +1310,16 @@ > case PHYSTYPE_FILE: > /* let''s pretend is tap:aio for the moment */ > disk->phystype = PHYSTYPE_AIO; > - case PHYSTYPE_AIO: case PHYSTYPE_QCOW: case PHYSTYPE_QCOW2: case PHYSTYPE_VHD: > - if (is_blktap2_supported()) { > - int rc, c, p[2], tot; > - char buf[1024], *dev; > - dev = get_blktap2_device(ctx, disk->physpath, device_disk_string_of_phystype(disk->phystype)); > - if (dev == NULL) { > - rc= libxl_pipe(ctx, p); if (rc==-1) return -1; > - rc= libxl_fork(ctx); if (rc==-1) return -1; > - if (!rc) { /* child */ > - int null_r, null_w; > - char *args[4]; > - args[0] = "tapdisk2"; > - args[1] = "-n"; > - args[2] = libxl_sprintf(ctx, "%s:%s", device_disk_string_of_phystype(disk->phystype), disk->physpath); > - args[3] = NULL; > - > - null_r = open("/dev/null", O_RDONLY); > - null_w = open("/dev/null", O_WRONLY); > - libxl_exec(null_r, p[1], null_w, > - libxl_abs_path(ctx, "tapdisk2", > - libxl_sbindir_path()), > - args); > - XL_LOG(ctx, XL_LOG_ERROR, "Error execing tapdisk2"); > - } > - close(p[1]); > - tot = 0; > - while ((c = read(p[0], buf + tot, sizeof(buf) - tot)) > 0) > - tot = tot + c; > - close(p[0]); > - buf[tot - 1] = ''\0''; > - dev = buf; > - } > + case PHYSTYPE_AIO: case PHYSTYPE_QCOW: case PHYSTYPE_QCOW2: case PHYSTYPE_VHD: { > + const char *msg; > + if (!tap_ctl_check(&msg)) { > + const char *type = device_disk_string_of_phystype(disk->phystype); > + char *dev; > + dev = get_blktap2_device(ctx, disk->physpath, type); > + if (!dev) > + dev = make_blktap2_device(ctx, disk->physpath, type); > + if (!dev) > + return -1; > flexarray_set(back, boffset++, "tapdisk-params"); > flexarray_set(back, boffset++, libxl_sprintf(ctx, "%s:%s", device_disk_string_of_phystype(disk->phystype), disk->physpath)); > flexarray_set(back, boffset++, "params"); >you are calling make_blktap2_device only if(!dev), are you sure it is correct? get_blktap2_device returns a pointer on success... _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-10 20:11 UTC
Re: [Xen-devel] [PATCH 4 of 4] libxl: Use libblktapctl.so
On Thu, 2010-06-10 at 11:43 -0400, Stefano Stabellini wrote:> On Thu, 10 Jun 2010, Daniel Stodden wrote: > > Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com> > > This patch broke file: and tap:aio support in xl/libxl. > Did you actually test it?Yes. Worked for for a couple runs. Daniel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2010-Jun-10 22:27 UTC
Re: [Xen-devel] [PATCH 4 of 4] libxl: Use libblktapctl.so
On Thu, 2010-06-10 at 12:25 -0400, Stefano Stabellini wrote:> > -static char *get_blktap2_device(struct libxl_ctx *ctx, char *name, char *type) > > +static char *make_blktap2_device(struct libxl_ctx *ctx, > > + const char *name, const char *type) > > { > > - char buf[1024]; > > - char *p; > > - int devnum; > > - FILE *f = fopen("/sys/class/blktap2/devices", "r"); > > - > > - > > - while (!feof(f)) { > > - if (fscanf(f, "%d %s", &devnum, buf) != 2) > > - continue; > > - p = strchr(buf, '':''); > > - if (p == NULL) > > - continue; > > - p++; > > - if (!strcmp(p, name) && !strncmp(buf, type, 3)) { > > - fclose(f); > > - return libxl_sprintf(ctx, "/dev/xen/blktap-2/tapdev%d", devnum); > > - } > > - } > > - fclose(f); > > - return NULL; > > + char *params, *devname; > > + int err; > > + params = libxl_sprintf(ctx, "%s:%s", type, name); > > + devname = NULL; > > + err = tap_ctl_create(params, &devname); > > + free(params); > > + return err ? NULL : devname; > > } > > > > you shouldn''t free pointers returned by libxl internal functions, > because libxl will take care of free''ing them.I''ll send an update. Sorry for that. Please don''t hesitate to submit corrections right away if you spot more issues.> dev = buf; > > - } > > + case PHYSTYPE_AIO: case PHYSTYPE_QCOW: case PHYSTYPE_QCOW2: case PHYSTYPE_VHD: { > > + const char *msg; > > + if (!tap_ctl_check(&msg)) { > > + const char *type = device_disk_string_of_phystype(disk->phystype); > > + char *dev; > > + dev = get_blktap2_device(ctx, disk->physpath, type); > > + if (!dev) > > + dev = make_blktap2_device(ctx, disk->physpath, type); > > + if (!dev) > > + return -1; > > flexarray_set(back, boffset++, "tapdisk-params"); > > flexarray_set(back, boffset++, libxl_sprintf(ctx, "%s:%s", device_disk_string_of_phystype(disk->phystype), disk->physpath)); > > flexarray_set(back, boffset++, "params"); > > > > you are calling make_blktap2_device only if(!dev), are you sure it is > correct? get_blktap2_device returns a pointer on success...I don''t quite manage to follow this comment. The behavior was meant to match the original version of that code. It still looks like it does. Is that what you question? Unless you''re worried about null pointer handling with massively broken compilers? Did you ever manage to find one? I didn''t %-) But again, if you''re not happy with it, please just go ahead and make it suit your coding preferences. I got zero stakes in that code. Btw, someone probably should also fix the device teardown path as well, it seems to be consistently leaking device nodes. I guess enumerating backend physical-device nodes to count vbds would probably do for a ''light'' control layer. Thanks for the input. Daniel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jun-11 10:42 UTC
Re: [Xen-devel] [PATCH 4 of 4] libxl: Use libblktapctl.so
On Thu, 10 Jun 2010, Daniel Stodden wrote:> I don''t quite manage to follow this comment. The behavior was meant to > match the original version of that code. It still looks like it does. Is > that what you question? > > Unless you''re worried about null pointer handling with massively broken > compilers? Did you ever manage to find one? I didn''t %-) > > But again, if you''re not happy with it, please just go ahead and make it > suit your coding preferences. I got zero stakes in that code. >Nope, you are right, after a second look the code seems correct. However it still doesn''t work for me. The device returned by make_blktap2_device is /dev/xen/blktap-2/tapdev0, but "cat /sys/class/blktap2/devices" returns nothing and my VM doesn''t start. Am I missing something?> Btw, someone probably should also fix the device teardown path as well, > it seems to be consistently leaking device nodes. I guess enumerating > backend physical-device nodes to count vbds would probably do for a > ''light'' control layer. >you are right, are you volunteering ? ;-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jun-11 13:00 UTC
Re: [Xen-devel] [PATCH 4 of 4] libxl: Use libblktapctl.so
On Fri, 11 Jun 2010, Stefano Stabellini wrote:> On Thu, 10 Jun 2010, Daniel Stodden wrote: > > I don''t quite manage to follow this comment. The behavior was meant to > > match the original version of that code. It still looks like it does. Is > > that what you question? > > > > Unless you''re worried about null pointer handling with massively broken > > compilers? Did you ever manage to find one? I didn''t %-) > > > > But again, if you''re not happy with it, please just go ahead and make it > > suit your coding preferences. I got zero stakes in that code. > > > > Nope, you are right, after a second look the code seems correct. > However it still doesn''t work for me. > The device returned by make_blktap2_device is /dev/xen/blktap-2/tapdev0, > but "cat /sys/class/blktap2/devices" returns nothing and my VM doesn''t > start. > Am I missing something?Yes, I was missing something: my tapdisk2 was outdated, everything works now. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Łukasz Oleś
2010-Aug-03 19:14 UTC
Re: [Xen-devel] [PATCH 0 of 7] blktap2: The tap-ctl userspace control utility and library
On Monday 07 June 2010 23:54:47 Daniel Stodden wrote:> Hi. > > Apart from removing some dead code and tidying up tapdisk-vbd a > little, the bigger introduces our new tap-ctl utility code. > > The code aims to be a complete replacement for the original blktap2 > control path, run through sysfs. Fully implemented in userspace, all > tapdisks now gained a small IPC layer on top of Unix domain sockets. > > Sample usage: > > # tap-ctl allocate > /dev/xen/blktap-2/tapdev0 > > # tap-ctl spawn > tapdisk spawned with pid 4168 > > # tap-ctl list > 4168 - - - - > - 0 - - - > > # tap-ctl attach -p 4168 -m 0 > # tap-ctl list > 4168 0 0 - - > > # tap-ctl open -p 4168 -m 0 -a aio:/var/tmp/lenny.ext > > # tap-ctl list > 4168 0 0 aio /var/tmp/lenny.ext > > # tap-ctl close -p 4168 -m 0 > # tap-ctl detach -p 4168 -m 0 > # tap-ctl free -m 0 > > The above example is a bit noisy, because it''s mediating between minor > number (block devices), tapdisks and tapdisk VBDs (the tapdisk I/O > queue running a bdev) in detail. > > There are shortcuts. At the same time, the low-level interface should > be general enough to stay extensible, and help accomodate some of the > more esoteric features, like shared images and/or multiple VBDs > sharing the same tapdisk.I think README file should be also updated. And maybe its good idea to merge it to xen 4.0? -- Łukasz Oleś _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel