Marek Marczykowski
2011-Jun-05 16:50 UTC
[Xen-devel] [PATCH 0 of 6 RESENT] A bunch of fixes for libxl
A bunch of simple fixes for libxl. Most of them fixes SEGV or other annoying bugs. This time for xen-unstable tree - only part of previous patches are applicable. All of them should be also backported to xen-4.1-testing tree (bugfixes). _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-05 16:50 UTC
[Xen-devel] [PATCH 1 of 6 RESENT] libxl: Remove frontend and backend devices from xenstore after destroy
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307143993 -7200 # Node ID c32797243a6ba61dd2942a0307151e42fb7bf157 # Parent 37c77bacb52aa7795978b994f9d371b979b2cb07 libxl: Remove frontend and backend devices from xenstore after destroy Cleanup frontend and backend devices from xenstore for all dev types - not only disks. Because backend cleanup moved to libxl__device_destroy, libxl__devices_destroy is somehow simpler. Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff -r 37c77bacb52a -r c32797243a6b tools/libxl/libxl.c --- a/tools/libxl/libxl.c Mon May 23 17:38:28 2011 +0100 +++ b/tools/libxl/libxl.c Sat Jun 04 01:33:13 2011 +0200 @@ -1105,8 +1105,6 @@ int libxl_device_disk_del(libxl_ctx *ctx device.devid = devid; device.kind = DEVICE_VBD; rc = libxl__device_del(&gc, &device, wait); - xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, &device)); - xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, &device)); out_free: libxl__free_all(&gc); return rc; diff -r 37c77bacb52a -r c32797243a6b tools/libxl/libxl_device.c --- a/tools/libxl/libxl_device.c Mon May 23 17:38:28 2011 +0100 +++ b/tools/libxl/libxl_device.c Sat Jun 04 01:33:13 2011 +0200 @@ -272,6 +272,8 @@ retry_transaction: if (!force) { xs_watch(ctx->xsh, state_path, be_path); rc = 1; + } else { + xs_rm(ctx->xsh, XBT_NULL, be_path); } out: return rc; @@ -311,10 +313,8 @@ int libxl__devices_destroy(libxl__gc *gc char *path, *be_path, *fe_path; unsigned int num1, num2; char **l1 = NULL, **l2 = NULL; - int i, j, n = 0, n_watches = 0; - flexarray_t *toremove; + int i, j, n_watches = 0; - toremove = flexarray_make(16, 1); path = libxl__sprintf(gc, "/local/domain/%d/device", domid); l1 = libxl__xs_directory(gc, XBT_NULL, path, &num1); if (!l1) { @@ -338,7 +338,6 @@ int libxl__devices_destroy(libxl__gc *gc if (be_path != NULL) { if (libxl__device_destroy(gc, be_path, force) > 0) n_watches++; - flexarray_set(toremove, n++, libxl__dirname(gc, be_path)); } else { xs_rm(ctx->xsh, XBT_NULL, path); } @@ -351,7 +350,6 @@ int libxl__devices_destroy(libxl__gc *gc if (be_path && strcmp(be_path, "")) { if (libxl__device_destroy(gc, be_path, force) > 0) n_watches++; - flexarray_set(toremove, n++, libxl__dirname(gc, be_path)); } if (!force) { @@ -371,17 +369,13 @@ int libxl__devices_destroy(libxl__gc *gc } } } - for (i = 0; i < n; i++) { - flexarray_get(toremove, i, (void**) &path); - xs_rm(ctx->xsh, XBT_NULL, path); - } out: - flexarray_free(toremove); return 0; } int libxl__device_del(libxl__gc *gc, libxl__device *dev, int wait) { + libxl_ctx *ctx = libxl__gc_owner(gc); char *backend_path; int rc; @@ -400,6 +394,7 @@ int libxl__device_del(libxl__gc *gc, lib (void)wait_for_dev_destroy(gc, &tv); } + xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev)); rc = 0; out: _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-05 16:50 UTC
[Xen-devel] [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1306962929 -7200 # Node ID 5231fa19f3e39091ff29e2a6dca057ca54403092 # Parent c32797243a6ba61dd2942a0307151e42fb7bf157 libxl: Accept disk name in libxl_devid_to_device_disk Accept disk name in xl block-detach. Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff -r c32797243a6b -r 5231fa19f3e3 tools/libxl/libxl_device.c --- a/tools/libxl/libxl_device.c Sat Jun 04 01:33:13 2011 +0200 +++ b/tools/libxl/libxl_device.c Wed Jun 01 23:15:29 2011 +0200 @@ -198,7 +198,7 @@ static int device_virtdisk_matches(const return 1; } -int libxl__device_disk_dev_number(char *virtpath, int *pdisk, int *ppartition) +int libxl__device_disk_dev_number(const char *virtpath, int *pdisk, int *ppartition) { int disk, partition; char *ep; diff -r c32797243a6b -r 5231fa19f3e3 tools/libxl/libxl_internal.h --- a/tools/libxl/libxl_internal.h Sat Jun 04 01:33:13 2011 +0200 +++ b/tools/libxl/libxl_internal.h Wed Jun 01 23:15:29 2011 +0200 @@ -205,7 +205,7 @@ _hidden char *libxl__device_disk_string_ _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format); _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor); -_hidden int libxl__device_disk_dev_number(char *virtpath, +_hidden int libxl__device_disk_dev_number(const char *virtpath, int *pdisk, int *ppartition); _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t domid, diff -r c32797243a6b -r 5231fa19f3e3 tools/libxl/libxl_utils.c --- a/tools/libxl/libxl_utils.c Sat Jun 04 01:33:13 2011 +0200 +++ b/tools/libxl/libxl_utils.c Wed Jun 01 23:15:29 2011 +0200 @@ -530,18 +530,18 @@ int libxl_devid_to_device_disk(libxl_ctx const char *devid, libxl_device_disk *disk) { libxl__gc gc = LIBXL_INIT_GC(ctx); - char *endptr, *val; + char *val; char *dompath, *diskpath, *be_path; unsigned int devid_n; int rc = ERROR_INVAL; - devid_n = strtoul(devid, &endptr, 10); - if (devid == endptr) { + devid_n = libxl__device_disk_dev_number(devid, NULL, NULL); + if (devid_n < 0) { goto out; } rc = ERROR_FAIL; dompath = libxl__xs_get_dompath(&gc, domid); - diskpath = libxl__sprintf(&gc, "%s/device/vbd/%s", dompath, devid); + diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, devid_n); if (!diskpath) { goto out; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-05 16:50 UTC
[Xen-devel] [PATCH 3 of 6 RESENT] libxl: Allocate memory for strings in libxl_device_disk
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1306962954 -7200 # Node ID 9fe949c7ab9601bb5500a53c538f7a23b61e1bcb # Parent 5231fa19f3e39091ff29e2a6dca057ca54403092 libxl: Allocate memory for strings in libxl_device_disk Memory for strings in libxl_device_disk must be allocated from outside of libxl__gc to not be freed at the end of function (by libxl__free_all). Fixes xl block-detach Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff -r 5231fa19f3e3 -r 9fe949c7ab96 tools/libxl/libxl_utils.c --- a/tools/libxl/libxl_utils.c Wed Jun 01 23:15:29 2011 +0200 +++ b/tools/libxl/libxl_utils.c Wed Jun 01 23:15:54 2011 +0200 @@ -551,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx goto out; disk->backend_domid = strtoul(val, NULL, 10); be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath)); - disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)); + disk->pdev_path = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path), NULL); val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path)); libxl_string_to_backend(ctx, val, &(disk->backend)); - disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)); + disk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path), NULL); val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path)); disk->unpluggable = !strcmp(val, "1"); val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path)); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-05 16:50 UTC
[Xen-devel] [PATCH 4 of 6 RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307145042 -7200 # Node ID 0863dfd23b3a10bee6d3eda30415dff0eef2bee1 # Parent 9fe949c7ab9601bb5500a53c538f7a23b61e1bcb xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param") ''script='' length was wrong... Replaced calls to strncmp("param", *argv, explicit sizeof("param")) with macro and helper function to extract parameter value. Also introduce replace_string function to simplify code. Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff -r 9fe949c7ab96 -r 0863dfd23b3a tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Wed Jun 01 23:15:54 2011 +0200 +++ b/tools/libxl/xl_cmdimpl.c Sat Jun 04 01:50:42 2011 +0200 @@ -1229,6 +1229,24 @@ static int handle_domain_death(libxl_ctx return restart; } +/* for now used only by main_networkattach, but can be reused elsewhere */ +static int match_option_size(const char *prefix, size_t len, + char *arg, char **argopt) +{ + int rc = strncmp(prefix, arg, len); + if (!rc) *argopt = arg+len; + return !rc; +} +#define match_option(_prefix, _arg, _oparg) \ + match_option_size((_prefix "="), sizeof((_prefix)) + 1, (_arg), &(_oparg)) + +static void replace_string(char **str, const char *val) +{ + free(*str); + *str = strdup(val); +} + + static int preserve_domain(libxl_ctx *ctx, uint32_t domid, libxl_event *event, libxl_domain_config *d_config, libxl_dominfo *info) { @@ -3925,7 +3943,7 @@ int main_networkattach(int argc, char ** { int opt; libxl_device_nic nic; - char *endptr; + char *endptr, *oparg; const char *tok; int i; unsigned int val; @@ -3944,17 +3962,17 @@ int main_networkattach(int argc, char ** } libxl_device_nic_init(&nic, -1); for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) { - if (!strncmp("type=", *argv, 5)) { - if (!strncmp("vif", (*argv) + 5, 4)) { + if (match_option("type", *argv, oparg)) { + if (!strcmp("vif", oparg)) { nic.nictype = LIBXL_NIC_TYPE_VIF; - } else if (!strncmp("ioemu", (*argv) + 5, 5)) { + } else if (!strcmp("ioemu", oparg)) { nic.nictype = LIBXL_NIC_TYPE_IOEMU; } else { fprintf(stderr, "Invalid parameter `type''.\n"); return 1; } - } else if (!strncmp("mac=", *argv, 4)) { - tok = strtok((*argv) + 4, ":"); + } else if (match_option("mac", *argv, oparg)) { + tok = strtok(oparg, ":"); for (i = 0; tok && i < 6; tok = strtok(NULL, ":"), ++i) { val = strtoul(tok, &endptr, 16); if ((tok == endptr) || (val > 255)) { @@ -3963,29 +3981,24 @@ int main_networkattach(int argc, char ** } nic.mac[i] = val; } - } else if (!strncmp("bridge=", *argv, 7)) { - free(nic.bridge); - nic.bridge = strdup((*argv) + 7); - } else if (!strncmp("ip=", *argv, 3)) { - free(nic.ip); - nic.ip = strdup((*argv) + 3); - } else if (!strncmp("script=", *argv, 6)) { - free(nic.script); - nic.script = strdup((*argv) + 6); - } else if (!strncmp("backend=", *argv, 8)) { - if(libxl_name_to_domid(ctx, ((*argv) + 8), &val)) { + } else if (match_option("bridge", *argv, oparg)) { + replace_string(&nic.bridge, oparg); + } else if (match_option("ip", *argv, oparg)) { + replace_string(&nic.ip, oparg); + } else if (match_option("script", *argv, oparg)) { + replace_string(&nic.script, oparg); + } else if (match_option("backend", *argv, oparg)) { + if(libxl_name_to_domid(ctx, oparg, &val)) { fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n"); val = 0; } nic.backend_domid = val; - } else if (!strncmp("vifname=", *argv, 8)) { - free(nic.ifname); - nic.ifname = strdup((*argv) + 8); - } else if (!strncmp("model=", *argv, 6)) { - free(nic.model); - nic.model = strdup((*argv) + 6); - } else if (!strncmp("rate=", *argv, 5)) { - } else if (!strncmp("accel=", *argv, 6)) { + } else if (match_option("vifname", *argv, oparg)) { + replace_string(&nic.ifname, oparg); + } else if (match_option("model", *argv, oparg)) { + replace_string(&nic.model, oparg); + } else if (match_option("rate", *argv, oparg)) { + } else if (match_option("accel", *argv, oparg)) { } else { fprintf(stderr, "unrecognized argument `%s''\n", *argv); return 1; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-05 16:50 UTC
[Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307145131 -7200 # Node ID 0c0f9e1bd14073b5cb1d4f58b6950d16128003fa # Parent 0863dfd23b3a10bee6d3eda30415dff0eef2bee1 xen.lowlevel.xl: Return None on empty domain name Previously PyString_FromString(NULL) was called, which caused assertion failure. Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff -r 0863dfd23b3a -r 0c0f9e1bd140 tools/python/xen/lowlevel/xl/xl.c --- a/tools/python/xen/lowlevel/xl/xl.c Sat Jun 04 01:50:42 2011 +0200 +++ b/tools/python/xen/lowlevel/xl/xl.c Sat Jun 04 01:52:11 2011 +0200 @@ -409,14 +409,16 @@ static PyObject *pyxl_domid_to_name(XlOb { char *domname; int domid; - PyObject *ret; + PyObject *ret = Py_None; if ( !PyArg_ParseTuple(args, "i", &domid) ) return NULL; domname = libxl_domid_to_name(self->ctx, domid); - ret = PyString_FromString(domname); - free(domname); + if (domname) { + ret = PyString_FromString(domname); + free(domname); + } return ret; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-05 16:50 UTC
[Xen-devel] [PATCH 6 of 6 RESENT] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307145395 -7200 # Node ID 70fc2a0f0a6003f1bf591cd941a841a9e6b69c01 # Parent 0c0f9e1bd14073b5cb1d4f58b6950d16128003fa libxl: Do not SEGV when no ''removable'' disk parameter in xenstore Just assume disk as not removable when no ''removable'' paremeter Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff -r 0c0f9e1bd140 -r 70fc2a0f0a60 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Sat Jun 04 01:52:11 2011 +0200 +++ b/tools/libxl/libxl.c Sat Jun 04 01:56:35 2011 +0200 @@ -1563,6 +1563,7 @@ static unsigned int libxl__append_disk_l libxl__xs_get_dompath(gc, 0), type, domid); dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n); if (dir) { + char *removable; *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n)); pdisk = *disks + *ndisks; *ndisks += n; @@ -1581,6 +1582,11 @@ static unsigned int libxl__append_disk_l &(pdisk->backend)); pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/dev", be_path, *dir), &len); pdisk->unpluggable = atoi(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir))); + removable = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir)); + if (removable) + pdisk->unpluggable = atoi(removable); + else + pdisk->unpluggable = 0; if (!strcmp(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/mode", be_path, *dir)), "w")) pdisk->readwrite = 1; else _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jun-06 10:00 UTC
Re: [Xen-devel] [PATCH 1 of 6 RESENT] libxl: Remove frontend and backend devices from xenstore after destroy
On Sun, 2011-06-05 at 17:50 +0100, Marek Marczykowski wrote:> # HG changeset patch > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > # Date 1307143993 -7200 > # Node ID c32797243a6ba61dd2942a0307151e42fb7bf157 > # Parent 37c77bacb52aa7795978b994f9d371b979b2cb07 > libxl: Remove frontend and backend devices from xenstore after destroy > > Cleanup frontend and backend devices from xenstore for all dev types - not only > disks. Because backend cleanup moved to libxl__device_destroy, > libxl__devices_destroy is somehow simpler. > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl>Acked-by: Ian Campbell <ian.campbell@citrix.com>> > diff -r 37c77bacb52a -r c32797243a6b tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Mon May 23 17:38:28 2011 +0100 > +++ b/tools/libxl/libxl.c Sat Jun 04 01:33:13 2011 +0200 > @@ -1105,8 +1105,6 @@ int libxl_device_disk_del(libxl_ctx *ctx > device.devid = devid; > device.kind = DEVICE_VBD; > rc = libxl__device_del(&gc, &device, wait); > - xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, &device)); > - xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, &device)); > out_free: > libxl__free_all(&gc); > return rc; > diff -r 37c77bacb52a -r c32797243a6b tools/libxl/libxl_device.c > --- a/tools/libxl/libxl_device.c Mon May 23 17:38:28 2011 +0100 > +++ b/tools/libxl/libxl_device.c Sat Jun 04 01:33:13 2011 +0200 > @@ -272,6 +272,8 @@ retry_transaction: > if (!force) { > xs_watch(ctx->xsh, state_path, be_path); > rc = 1; > + } else { > + xs_rm(ctx->xsh, XBT_NULL, be_path); > } > out: > return rc; > @@ -311,10 +313,8 @@ int libxl__devices_destroy(libxl__gc *gc > char *path, *be_path, *fe_path; > unsigned int num1, num2; > char **l1 = NULL, **l2 = NULL; > - int i, j, n = 0, n_watches = 0; > - flexarray_t *toremove; > + int i, j, n_watches = 0; > > - toremove = flexarray_make(16, 1); > path = libxl__sprintf(gc, "/local/domain/%d/device", domid); > l1 = libxl__xs_directory(gc, XBT_NULL, path, &num1); > if (!l1) { > @@ -338,7 +338,6 @@ int libxl__devices_destroy(libxl__gc *gc > if (be_path != NULL) { > if (libxl__device_destroy(gc, be_path, force) > 0) > n_watches++; > - flexarray_set(toremove, n++, libxl__dirname(gc, be_path)); > } else { > xs_rm(ctx->xsh, XBT_NULL, path); > } > @@ -351,7 +350,6 @@ int libxl__devices_destroy(libxl__gc *gc > if (be_path && strcmp(be_path, "")) { > if (libxl__device_destroy(gc, be_path, force) > 0) > n_watches++; > - flexarray_set(toremove, n++, libxl__dirname(gc, be_path)); > } > > if (!force) { > @@ -371,17 +369,13 @@ int libxl__devices_destroy(libxl__gc *gc > } > } > } > - for (i = 0; i < n; i++) { > - flexarray_get(toremove, i, (void**) &path); > - xs_rm(ctx->xsh, XBT_NULL, path); > - } > out: > - flexarray_free(toremove); > return 0; > } > > int libxl__device_del(libxl__gc *gc, libxl__device *dev, int wait) > { > + libxl_ctx *ctx = libxl__gc_owner(gc); > char *backend_path; > int rc; > > @@ -400,6 +394,7 @@ int libxl__device_del(libxl__gc *gc, lib > (void)wait_for_dev_destroy(gc, &tv); > } > > + xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev)); > rc = 0; > > out: > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jun-06 11:24 UTC
Re: [Xen-devel] [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk
On Sun, 2011-06-05 at 17:50 +0100, Marek Marczykowski wrote:> # HG changeset patch > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > # Date 1306962929 -7200 > # Node ID 5231fa19f3e39091ff29e2a6dca057ca54403092 > # Parent c32797243a6ba61dd2942a0307151e42fb7bf157 > libxl: Accept disk name in libxl_devid_to_device_disk > > Accept disk name in xl block-detach. > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl>Acked-by: Ian Campbell <ian.campbell@citrix.com> While reviewing I noticed a path out of libxl__device_disk_dev_number which would succeed without setting pdisk or ppartition, which would surprise a caller which provided them. I wasn''t immediately sure from docs/misc/vbd-interface.txt how to parse a vdev which is an arbitrary number. Many cases are covered in the document but there are others (e.g. NetBSD uses small integers iirc) which I was unsure of. (I expect that document needs expanding to cover these cases, but I''m not sure what to put...) For now I just returned an error to prevent a cascading failure. 8<------------------------------------------------------- # HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1307359172 -3600 # Node ID b5dfb12aa231c98a4dcf3372d3bf1cf9a85df2a4 # Parent 86009542df09c70ca8b4a95e4dd3de63cf95c9d6 libxl: fail to parse disk vpath if a disk+part number is required but unavailable libxl__device_disk_dev_number() can parse a virtpath which is an encoded unsigned long but does not set *pdisk or *ppartition in that case. Ideally we would parse the number but for now simply fail to prevent cascading failures. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r 86009542df09 -r b5dfb12aa231 tools/libxl/libxl_device.c --- a/tools/libxl/libxl_device.c Mon Jun 06 12:11:25 2011 +0100 +++ b/tools/libxl/libxl_device.c Mon Jun 06 12:19:32 2011 +0100 @@ -222,8 +222,12 @@ int libxl__device_disk_dev_number(char * errno = 0; ul = strtoul(virtpath, &ep, 0); - if (!errno && !*ep && ul <= INT_MAX) + if (!errno && !*ep && ul <= INT_MAX) { + /* FIXME: should parse ul to determine these. */ + if (pdisk || ppartition) + return -1; return ul; + } if (device_virtdisk_matches(virtpath, "hd", &disk, 3, _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jun-06 11:24 UTC
Re: [Xen-devel] [PATCH 3 of 6 RESENT] libxl: Allocate memory for strings in libxl_device_disk
On Sun, 2011-06-05 at 17:50 +0100, Marek Marczykowski wrote:> # HG changeset patch > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > # Date 1306962954 -7200 > # Node ID 9fe949c7ab9601bb5500a53c538f7a23b61e1bcb > # Parent 5231fa19f3e39091ff29e2a6dca057ca54403092 > libxl: Allocate memory for strings in libxl_device_disk > > Memory for strings in libxl_device_disk must be allocated from outside of > libxl__gc to not be freed at the end of function (by libxl__free_all). > > Fixes xl block-detach > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl>Acked-by: Ian Campbell <ian.campbell@citrix.com>> > diff -r 5231fa19f3e3 -r 9fe949c7ab96 tools/libxl/libxl_utils.c > --- a/tools/libxl/libxl_utils.c Wed Jun 01 23:15:29 2011 +0200 > +++ b/tools/libxl/libxl_utils.c Wed Jun 01 23:15:54 2011 +0200 > @@ -551,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx > goto out; > disk->backend_domid = strtoul(val, NULL, 10); > be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath)); > - disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)); > + disk->pdev_path = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path), NULL); > val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path)); > libxl_string_to_backend(ctx, val, &(disk->backend)); > - disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)); > + disk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path), NULL); > val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path)); > disk->unpluggable = !strcmp(val, "1"); > val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path)); > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jun-06 11:31 UTC
Re: [Xen-devel] [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk
On Mon, 2011-06-06 at 12:24 +0100, Ian Campbell wrote:> On Sun, 2011-06-05 at 17:50 +0100, Marek Marczykowski wrote: > > # HG changeset patch > > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > > # Date 1306962929 -7200 > > # Node ID 5231fa19f3e39091ff29e2a6dca057ca54403092 > > # Parent c32797243a6ba61dd2942a0307151e42fb7bf157 > > libxl: Accept disk name in libxl_devid_to_device_disk > > > > Accept disk name in xl block-detach. > > > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > While reviewing I noticed a path out of libxl__device_disk_dev_number > which would succeed without setting pdisk or ppartition, which would > surprise a caller which provided them. > > I wasn''t immediately sure from docs/misc/vbd-interface.txt how to parse > a vdev which is an arbitrary number. Many cases are covered in the > document but there are others (e.g. NetBSD uses small integers iirc) > which I was unsure of. (I expect that document needs expanding to cover > these cases, but I''m not sure what to put...)Actually the doc does say: To cope with guests which predate this specification we preserve the existing facility to specify the xenstore numerical value directly by putting a single number (hex, decimal or octal) in the domain config file instead of the disk identifier; this number is written directly to xenstore (after conversion to the canonical decimal format). so I guess it is explicit that these values cannot be reliably parsed. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jun-07 11:57 UTC
Re: [Xen-devel] [PATCH 6 of 6 RESENT] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore
On Sun, 5 Jun 2011, Marek Marczykowski wrote:> # HG changeset patch > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > # Date 1307145395 -7200 > # Node ID 70fc2a0f0a6003f1bf591cd941a841a9e6b69c01 > # Parent 0c0f9e1bd14073b5cb1d4f58b6950d16128003fa > libxl: Do not SEGV when no ''removable'' disk parameter in xenstore > > Just assume disk as not removable when no ''removable'' paremeter > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> > > diff -r 0c0f9e1bd140 -r 70fc2a0f0a60 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Sat Jun 04 01:52:11 2011 +0200 > +++ b/tools/libxl/libxl.c Sat Jun 04 01:56:35 2011 +0200 > @@ -1563,6 +1563,7 @@ static unsigned int libxl__append_disk_l > libxl__xs_get_dompath(gc, 0), type, domid); > dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n); > if (dir) { > + char *removable; > *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n)); > pdisk = *disks + *ndisks; > *ndisks += n; > @@ -1581,6 +1582,11 @@ static unsigned int libxl__append_disk_l > &(pdisk->backend)); > pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/dev", be_path, *dir), &len); > pdisk->unpluggable = atoi(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir)));shouldn''t you be removing this ^ line?> + removable = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir)); > + if (removable) > + pdisk->unpluggable = atoi(removable); > + else > + pdisk->unpluggable = 0; > if (!strcmp(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/mode", be_path, *dir)), "w")) > pdisk->readwrite = 1; > else > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-07 12:00 UTC
Re: [Xen-devel] [PATCH 6 of 6 RESENT] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore
On 07.06.2011 13:57, Stefano Stabellini wrote:> On Sun, 5 Jun 2011, Marek Marczykowski wrote: >> # HG changeset patch >> # User Marek Marczykowski <marmarek@mimuw.edu.pl> >> # Date 1307145395 -7200 >> # Node ID 70fc2a0f0a6003f1bf591cd941a841a9e6b69c01 >> # Parent 0c0f9e1bd14073b5cb1d4f58b6950d16128003fa >> libxl: Do not SEGV when no ''removable'' disk parameter in xenstore >> >> Just assume disk as not removable when no ''removable'' paremeter >> >> Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> >> >> diff -r 0c0f9e1bd140 -r 70fc2a0f0a60 tools/libxl/libxl.c >> --- a/tools/libxl/libxl.c Sat Jun 04 01:52:11 2011 +0200 >> +++ b/tools/libxl/libxl.c Sat Jun 04 01:56:35 2011 +0200 >> @@ -1563,6 +1563,7 @@ static unsigned int libxl__append_disk_l >> libxl__xs_get_dompath(gc, 0), type, domid); >> dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n); >> if (dir) { >> + char *removable; >> *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n)); >> pdisk = *disks + *ndisks; >> *ndisks += n; >> @@ -1581,6 +1582,11 @@ static unsigned int libxl__append_disk_l >> &(pdisk->backend)); >> pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/dev", be_path, *dir), &len); >> pdisk->unpluggable = atoi(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir))); > > shouldn''t you be removing this ^ line?Yes, should be...> >> + removable = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir)); >> + if (removable) >> + pdisk->unpluggable = atoi(removable); >> + else >> + pdisk->unpluggable = 0; >> if (!strcmp(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/mode", be_path, *dir)), "w")) >> pdisk->readwrite = 1; >> else >> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel >>-- Pozdrawiam / Best Regards, Marek Marczykowski | RLU #390519 marmarek at mimuw edu pl | xmpp:marmarek at staszic waw pl _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jun-07 12:02 UTC
Re: [Xen-devel] [PATCH 4 of 6 RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")
On Sun, 5 Jun 2011, Marek Marczykowski wrote:> # HG changeset patch > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > # Date 1307145042 -7200 > # Node ID 0863dfd23b3a10bee6d3eda30415dff0eef2bee1 > # Parent 9fe949c7ab9601bb5500a53c538f7a23b61e1bcb > xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param") > > ''script='' length was wrong... Replaced calls to strncmp("param", *argv, > explicit sizeof("param")) with macro and helper function to extract parameter > value. Also introduce replace_string function to simplify code. > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl>It seems correct to me and makes the code easier to read, so Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> diff -r 9fe949c7ab96 -r 0863dfd23b3a tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Wed Jun 01 23:15:54 2011 +0200 > +++ b/tools/libxl/xl_cmdimpl.c Sat Jun 04 01:50:42 2011 +0200 > @@ -1229,6 +1229,24 @@ static int handle_domain_death(libxl_ctx > return restart; > } > > +/* for now used only by main_networkattach, but can be reused elsewhere */ > +static int match_option_size(const char *prefix, size_t len, > + char *arg, char **argopt) > +{ > + int rc = strncmp(prefix, arg, len); > + if (!rc) *argopt = arg+len; > + return !rc; > +} > +#define match_option(_prefix, _arg, _oparg) \ > + match_option_size((_prefix "="), sizeof((_prefix)) + 1, (_arg), &(_oparg)) > + > +static void replace_string(char **str, const char *val) > +{ > + free(*str); > + *str = strdup(val); > +} > + > + > static int preserve_domain(libxl_ctx *ctx, uint32_t domid, libxl_event *event, > libxl_domain_config *d_config, libxl_dominfo *info) > { > @@ -3925,7 +3943,7 @@ int main_networkattach(int argc, char ** > { > int opt; > libxl_device_nic nic; > - char *endptr; > + char *endptr, *oparg; > const char *tok; > int i; > unsigned int val; > @@ -3944,17 +3962,17 @@ int main_networkattach(int argc, char ** > } > libxl_device_nic_init(&nic, -1); > for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) { > - if (!strncmp("type=", *argv, 5)) { > - if (!strncmp("vif", (*argv) + 5, 4)) { > + if (match_option("type", *argv, oparg)) { > + if (!strcmp("vif", oparg)) { > nic.nictype = LIBXL_NIC_TYPE_VIF; > - } else if (!strncmp("ioemu", (*argv) + 5, 5)) { > + } else if (!strcmp("ioemu", oparg)) { > nic.nictype = LIBXL_NIC_TYPE_IOEMU; > } else { > fprintf(stderr, "Invalid parameter `type''.\n"); > return 1; > } > - } else if (!strncmp("mac=", *argv, 4)) { > - tok = strtok((*argv) + 4, ":"); > + } else if (match_option("mac", *argv, oparg)) { > + tok = strtok(oparg, ":"); > for (i = 0; tok && i < 6; tok = strtok(NULL, ":"), ++i) { > val = strtoul(tok, &endptr, 16); > if ((tok == endptr) || (val > 255)) { > @@ -3963,29 +3981,24 @@ int main_networkattach(int argc, char ** > } > nic.mac[i] = val; > } > - } else if (!strncmp("bridge=", *argv, 7)) { > - free(nic.bridge); > - nic.bridge = strdup((*argv) + 7); > - } else if (!strncmp("ip=", *argv, 3)) { > - free(nic.ip); > - nic.ip = strdup((*argv) + 3); > - } else if (!strncmp("script=", *argv, 6)) { > - free(nic.script); > - nic.script = strdup((*argv) + 6); > - } else if (!strncmp("backend=", *argv, 8)) { > - if(libxl_name_to_domid(ctx, ((*argv) + 8), &val)) { > + } else if (match_option("bridge", *argv, oparg)) { > + replace_string(&nic.bridge, oparg); > + } else if (match_option("ip", *argv, oparg)) { > + replace_string(&nic.ip, oparg); > + } else if (match_option("script", *argv, oparg)) { > + replace_string(&nic.script, oparg); > + } else if (match_option("backend", *argv, oparg)) { > + if(libxl_name_to_domid(ctx, oparg, &val)) { > fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n"); > val = 0; > } > nic.backend_domid = val; > - } else if (!strncmp("vifname=", *argv, 8)) { > - free(nic.ifname); > - nic.ifname = strdup((*argv) + 8); > - } else if (!strncmp("model=", *argv, 6)) { > - free(nic.model); > - nic.model = strdup((*argv) + 6); > - } else if (!strncmp("rate=", *argv, 5)) { > - } else if (!strncmp("accel=", *argv, 6)) { > + } else if (match_option("vifname", *argv, oparg)) { > + replace_string(&nic.ifname, oparg); > + } else if (match_option("model", *argv, oparg)) { > + replace_string(&nic.model, oparg); > + } else if (match_option("rate", *argv, oparg)) { > + } else if (match_option("accel", *argv, oparg)) { > } else { > fprintf(stderr, "unrecognized argument `%s''\n", *argv); > return 1; > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-08 10:41 UTC
[Xen-devel] [PATCH] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307145395 -7200 # Node ID b2b8fef3732c10f012fc209d2850e80d95471582 # Parent 0c0f9e1bd14073b5cb1d4f58b6950d16128003fa libxl: Do not SEGV when no ''removable'' disk parameter in xenstore Just assume disk as not removable when no ''removable'' paremeter Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -1563,6 +1563,7 @@ static unsigned int libxl__append_disk_l libxl__xs_get_dompath(gc, 0), type, domid); dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n); if (dir) { + char *removable; *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n)); pdisk = *disks + *ndisks; *ndisks += n; @@ -1580,7 +1581,11 @@ static unsigned int libxl__append_disk_l libxl__sprintf(gc, "%s/%s/type", be_path, *dir)), &(pdisk->backend)); pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/dev", be_path, *dir), &len); - pdisk->unpluggable = atoi(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir))); + removable = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir)); + if (removable) + pdisk->unpluggable = atoi(removable); + else + pdisk->unpluggable = 0; if (!strcmp(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/mode", be_path, *dir)), "w")) pdisk->readwrite = 1; else _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-08 10:42 UTC
[Xen-devel] [PATCH RESENTv2] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307145395 -7200 # Node ID b2b8fef3732c10f012fc209d2850e80d95471582 # Parent 0c0f9e1bd14073b5cb1d4f58b6950d16128003fa libxl: Do not SEGV when no ''removable'' disk parameter in xenstore Just assume disk as not removable when no ''removable'' paremeter Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -1563,6 +1563,7 @@ static unsigned int libxl__append_disk_l libxl__xs_get_dompath(gc, 0), type, domid); dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n); if (dir) { + char *removable; *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n)); pdisk = *disks + *ndisks; *ndisks += n; @@ -1580,7 +1581,11 @@ static unsigned int libxl__append_disk_l libxl__sprintf(gc, "%s/%s/type", be_path, *dir)), &(pdisk->backend)); pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/dev", be_path, *dir), &len); - pdisk->unpluggable = atoi(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir))); + removable = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir)); + if (removable) + pdisk->unpluggable = atoi(removable); + else + pdisk->unpluggable = 0; if (!strcmp(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/mode", be_path, *dir)), "w")) pdisk->readwrite = 1; else _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jun-08 10:50 UTC
Re: [Xen-devel] [PATCH] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore
On Wed, 8 Jun 2011, Marek Marczykowski wrote:> # HG changeset patch > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > # Date 1307145395 -7200 > # Node ID b2b8fef3732c10f012fc209d2850e80d95471582 > # Parent 0c0f9e1bd14073b5cb1d4f58b6950d16128003fa > libxl: Do not SEGV when no ''removable'' disk parameter in xenstore > > Just assume disk as not removable when no ''removable'' paremeter > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> >Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -1563,6 +1563,7 @@ static unsigned int libxl__append_disk_l > libxl__xs_get_dompath(gc, 0), type, domid); > dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n); > if (dir) { > + char *removable; > *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n)); > pdisk = *disks + *ndisks; > *ndisks += n; > @@ -1580,7 +1581,11 @@ static unsigned int libxl__append_disk_l > libxl__sprintf(gc, "%s/%s/type", be_path, *dir)), > &(pdisk->backend)); > pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/dev", be_path, *dir), &len); > - pdisk->unpluggable = atoi(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir))); > + removable = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/removable", be_path, *dir)); > + if (removable) > + pdisk->unpluggable = atoi(removable); > + else > + pdisk->unpluggable = 0; > if (!strcmp(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/mode", be_path, *dir)), "w")) > pdisk->readwrite = 1; > else > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-27 16:25 UTC
Re: [Xen-devel] [PATCH 3 of 6 RESENT] libxl: Allocate memory for strings in libxl_device_disk
Marek Marczykowski writes ("[Xen-devel] [PATCH 3 of 6 RESENT] libxl: Allocate memory for strings in libxl_device_disk"):> libxl: Allocate memory for strings in libxl_device_disk > > Memory for strings in libxl_device_disk must be allocated from outside of > libxl__gc to not be freed at the end of function (by libxl__free_all). > > Fixes xl block-detachApplied, thanks (long lines fixed up). Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-27 16:28 UTC
Re: [Xen-devel] [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk [and 1 more messages]
Marek Marczykowski writes ("[Xen-devel] [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk"):> libxl: Accept disk name in libxl_devid_to_device_diskApplied, thanks (long lines fixed up). Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-27 16:29 UTC
Re: [Xen-devel] [PATCH 1 of 6 RESENT] libxl: Remove frontend and backend devices from xenstore after destroy
Ian Campbell writes ("Re: [Xen-devel] [PATCH 1 of 6 RESENT] libxl: Remove frontend and backend devices from xenstore after destroy"):> On Sun, 2011-06-05 at 17:50 +0100, Marek Marczykowski wrote: > > # HG changeset patch > > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > > # Date 1307143993 -7200 > > # Node ID c32797243a6ba61dd2942a0307151e42fb7bf157 > > # Parent 37c77bacb52aa7795978b994f9d371b979b2cb07 > > libxl: Remove frontend and backend devices from xenstore after destroy > > > > Cleanup frontend and backend devices from xenstore for all dev types - not only > > disks. Because backend cleanup moved to libxl__device_destroy, > > libxl__devices_destroy is somehow simpler. > > > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> > > Acked-by: Ian Campbell <ian.campbell@citrix.com>Applied, thanks. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-27 16:34 UTC
Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name
Marek Marczykowski writes ("[Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name"):> xen.lowlevel.xl: Return None on empty domain name > > Previously PyString_FromString(NULL) was called, which caused assertion > failure. > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl>Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com> I modified it slightly to make the diff and complexity slightly smaller (since "free(NULL)" is legal). Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-27 16:37 UTC
Re: [Xen-devel] [PATCH] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore [and 1 more messages]
Marek Marczykowski writes ("[Xen-devel] [PATCH] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore"):> libxl: Do not SEGV when no ''removable'' disk parameter in xenstore > > Just assume disk as not removable when no ''removable'' paremeter > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl>Thanks, applied. (Fixed up conflict due to "unpluggable" rename, fixed up long lines). Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-28 21:37 UTC
Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)
On 27.06.2011 18:40, Ian Jackson wrote:> Right, I think I''ve dealt with all your outstanding patches now.Looks good, thanks :) Keir, can you apply them to 4.1 tree? This require slight modifications, but all of them are fixing bugs present also in 4.1. I can also send patches ready for 4.1 tree if it helps. This is all about: 23607:2f63562df1c4 libxl: Do not SEGV when no ''removable'' disk parameter in xenstoredefault tip 23606:cc2f376d0cd9 xen.lowlevel.xl: Return None on empty domain name 23605:ff8d170852b3 libxl: Remove frontend and backend devices from xenstore after destroy 23604:5d7998be2252 libxl: Accept disk name in libxl_devid_to_device_disk 23603:6656d80b4de4 libxl: Allocate memory for strings in libxl_device_disk 23602:5c353f53c8e2 xl: Use macros for param parsing in network-attach -- Pozdrawiam / Best Regards, Marek Marczykowski | RLU #390519 marmarek at mimuw edu pl | xmpp:marmarek at staszic waw pl _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Jun-28 21:57 UTC
Re: Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)
On 28/06/2011 22:37, "Marek Marczykowski" <marmarek@mimuw.edu.pl> wrote:> On 27.06.2011 18:40, Ian Jackson wrote: >> Right, I think I''ve dealt with all your outstanding patches now. > > Looks good, thanks :) > Keir, can you apply them to 4.1 tree? This require slight modifications, > but all of them are fixing bugs present also in 4.1.Ian Jackson deals with libxl backports to 4.1. -- Keir> I can also send patches ready for 4.1 tree if it helps. > > This is all about: > 23607:2f63562df1c4 libxl: Do not SEGV when no ''removable'' disk parameter > in xenstoredefault tip > 23606:cc2f376d0cd9 xen.lowlevel.xl: Return None on empty domain name > 23605:ff8d170852b3 libxl: Remove frontend and backend devices from > xenstore after destroy > 23604:5d7998be2252 libxl: Accept disk name in libxl_devid_to_device_disk > 23603:6656d80b4de4 libxl: Allocate memory for strings in libxl_device_disk > 23602:5c353f53c8e2 xl: Use macros for param parsing in network-attach >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-30 16:47 UTC
Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)
Marek Marczykowski writes ("Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)"):> On 27.06.2011 18:40, Ian Jackson wrote: > > Right, I think I''ve dealt with all your outstanding patches now. > > Looks good, thanks :) > Keir, can you apply them to 4.1 tree? This require slight modifications, > but all of them are fixing bugs present also in 4.1.Can we let it all sit in -unstable for a few weeks ?> I can also send patches ready for 4.1 tree if it helps.That would certainly be useful.> This is all about: > 23607:2f63562df1c4 libxl: Do not SEGV when no ''removable'' disk parameter > 23606:cc2f376d0cd9 xen.lowlevel.xl: Return None on empty domain name > 23605:ff8d170852b3 libxl: Remove frontend and backend devices from > 23604:5d7998be2252 libxl: Accept disk name in libxl_devid_to_device_disk > 23603:6656d80b4de4 libxl: Allocate memory for strings in libxl_device_diskWithout checking each one, I think these are suitable for backport.> 23602:5c353f53c8e2 xl: Use macros for param parsing in network-attachThis one isn''t, I think ? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Aug-25 11:05 UTC
Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)
Ian Jackson writes ("Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)"):> Marek Marczykowski writes ("Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)"): > > I can also send patches ready for 4.1 tree if it helps. > > That would certainly be useful. > > > This is all about: > > 23607:2f63562df1c4 libxl: Do not SEGV when no ''removable'' disk parameter > > 23606:cc2f376d0cd9 xen.lowlevel.xl: Return None on empty domain name > > 23605:ff8d170852b3 libxl: Remove frontend and backend devices from > > 23604:5d7998be2252 libxl: Accept disk name in libxl_devid_to_device_disk > > 23603:6656d80b4de4 libxl: Allocate memory for strings in libxl_device_disk > > Without checking each one, I think these are suitable for backport.Did you send backports for these ? I think we may have missed them. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 25.08.2011 13:05, Ian Jackson wrote:> Ian Jackson writes ("Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)"): >> Marek Marczykowski writes ("Backport libxl bugfixes to 4.1 (was: Re: [Xen-devel] [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)"): >>> I can also send patches ready for 4.1 tree if it helps. >> >> That would certainly be useful. >> >>> This is all about: >>> 23607:2f63562df1c4 libxl: Do not SEGV when no ''removable'' disk parameter >>> 23606:cc2f376d0cd9 xen.lowlevel.xl: Return None on empty domain name >>> 23605:ff8d170852b3 libxl: Remove frontend and backend devices from >>> 23604:5d7998be2252 libxl: Accept disk name in libxl_devid_to_device_disk >>> 23603:6656d80b4de4 libxl: Allocate memory for strings in libxl_device_disk >> >> Without checking each one, I think these are suitable for backport. > > Did you send backports for these ? I think we may have missed them.Not yet. Will send in near future (today/tomorrow). -- Pozdrawiam / Best Regards, Marek Marczykowski | RLU #390519 marmarek at mimuw edu pl | xmpp:marmarek at staszic waw pl _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Aug-25 17:13 UTC
[Xen-devel] [PATCH 0 of 5] A bunch of fixes for libxl
Backorted version of patches. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Aug-25 17:13 UTC
[Xen-devel] [PATCH 1 of 5] libxl: Remove frontend and backend devices from xenstore after destroy
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307285159 -7200 # Node ID 969051f07ee813d2a556f50d37cf59d4e509bf67 # Parent 43acc031eb24945973dffda2b7caf976993bbd5f libxl: Remove frontend and backend devices from xenstore after destroy Cleanup frontend and backend devices from xenstore for all dev types - not only disks. Because backend cleanup moved to libxl__device_destroy, libxl__devices_destroy is somehow simpler. Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -269,7 +269,9 @@ retry_transaction: if (!force) { xs_watch(ctx->xsh, state_path, be_path); rc = 1; - } + } else { + xs_rm(ctx->xsh, XBT_NULL, be_path); + } out: libxl__free_all(&gc); return rc; @@ -310,10 +312,8 @@ int libxl__devices_destroy(libxl_ctx *ct char *path, *be_path, *fe_path; unsigned int num1, num2; char **l1 = NULL, **l2 = NULL; - int i, j, n = 0, n_watches = 0; - flexarray_t *toremove; + int i, j, n_watches = 0; - toremove = flexarray_make(16, 1); path = libxl__sprintf(&gc, "/local/domain/%d/device", domid); l1 = libxl__xs_directory(&gc, XBT_NULL, path, &num1); if (!l1) { @@ -337,7 +337,6 @@ int libxl__devices_destroy(libxl_ctx *ct if (be_path != NULL) { if (libxl__device_destroy(ctx, be_path, force) > 0) n_watches++; - flexarray_set(toremove, n++, libxl__dirname(&gc, be_path)); } else { xs_rm(ctx->xsh, XBT_NULL, path); } @@ -350,7 +349,6 @@ int libxl__devices_destroy(libxl_ctx *ct if (be_path && strcmp(be_path, "")) { if (libxl__device_destroy(ctx, be_path, force) > 0) n_watches++; - flexarray_set(toremove, n++, libxl__dirname(&gc, be_path)); } if (!force) { @@ -370,12 +368,7 @@ int libxl__devices_destroy(libxl_ctx *ct } } } - for (i = 0; i < n; i++) { - flexarray_get(toremove, i, (void**) &path); - xs_rm(ctx->xsh, XBT_NULL, path); - } out: - flexarray_free(toremove); libxl__free_all(&gc); return 0; } @@ -401,6 +394,7 @@ int libxl__device_del(libxl_ctx *ctx, li (void)wait_for_dev_destroy(ctx, &tv); } + xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, dev)); rc = 0; out: _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Aug-25 17:13 UTC
[Xen-devel] [PATCH 2 of 5] libxl: Accept disk name in libxl_devid_to_device_disk
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307285297 -7200 # Node ID 8080606fb87a751f7cde4c6a08e3945a21d397b0 # Parent 969051f07ee813d2a556f50d37cf59d4e509bf67 libxl: Accept disk name in libxl_devid_to_device_disk Accept disk name in xl block-detach. Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -201,7 +201,7 @@ static int device_virtdisk_matches(const return 1; } -int libxl__device_disk_dev_number(char *virtpath) +int libxl__device_disk_dev_number(const char *virtpath) { int disk, partition; char *ep; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -182,7 +182,7 @@ _hidden char *libxl__device_disk_string_ _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format); _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor); -_hidden int libxl__device_disk_dev_number(char *virtpath); +_hidden int libxl__device_disk_dev_number(const char *virtpath); _hidden int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device, char **bents, char **fents); diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -529,18 +529,18 @@ int libxl_devid_to_device_disk(libxl_ctx const char *devid, libxl_device_disk *disk) { libxl__gc gc = LIBXL_INIT_GC(ctx); - char *endptr, *val; + char *val; char *dompath, *diskpath, *be_path; unsigned int devid_n; int rc = ERROR_INVAL; - devid_n = strtoul(devid, &endptr, 10); - if (devid == endptr) { + devid_n = libxl__device_disk_dev_number(devid); + if (devid_n < 0) { goto out; } rc = ERROR_FAIL; dompath = libxl__xs_get_dompath(&gc, domid); - diskpath = libxl__sprintf(&gc, "%s/device/vbd/%s", dompath, devid); + diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, devid_n); if (!diskpath) { goto out; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Aug-25 17:13 UTC
[Xen-devel] [PATCH 3 of 5] libxl: Allocate memory for strings in libxl_device_disk
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1306962954 -7200 # Node ID aa6e0521bb3853b0e1d90e870220568020929cf5 # Parent 8080606fb87a751f7cde4c6a08e3945a21d397b0 libxl: Allocate memory for strings in libxl_device_disk Memory for strings in libxl_device_disk must be allocated from outside of libxl__gc to not be freed at the end of function (by libxl__free_all). Fixes xl block-detach Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -551,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx disk->backend_domid = strtoul(val, NULL, 10); disk->domid = domid; be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath)); - disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)); + disk->pdev_path = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path), NULL); val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path)); libxl_string_to_backend(ctx, val, &(disk->backend)); - disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)); + disk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path), NULL); val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path)); disk->unpluggable = !strcmp(val, "1"); val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path)); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Aug-25 17:13 UTC
[Xen-devel] [PATCH 4 of 5] xen.lowlevel.xl: Return None on empty domain name
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307285583 -7200 # Node ID b77de60c85431593d439f2c4ac46023c6f8e5ee2 # Parent f33981cd7d3df1718123cd364ee91a7bad064283 xen.lowlevel.xl: Return None on empty domain name Previously PyString_FromString(NULL) was called, which caused assertion failure. Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowlevel/xl/xl.c --- a/tools/python/xen/lowlevel/xl/xl.c +++ b/tools/python/xen/lowlevel/xl/xl.c @@ -412,14 +412,16 @@ static PyObject *pyxl_domid_to_name(XlOb { char *domname; int domid; - PyObject *ret; + PyObject *ret = Py_None; if ( !PyArg_ParseTuple(args, "i", &domid) ) return NULL; domname = libxl_domid_to_name(&self->ctx, domid); - ret = PyString_FromString(domname); - free(domname); + if (domname) { + ret = PyString_FromString(domname); + free(domname); + } return ret; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Aug-25 17:13 UTC
[Xen-devel] [PATCH 5 of 5] libxl: Do not SEGV when no ''removable'' disk parameter in xenstore
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307285721 -7200 # Node ID fed2015fe1745e5e712380960cc50a71629698fc # Parent b77de60c85431593d439f2c4ac46023c6f8e5ee2 libxl: Do not SEGV when no ''removable'' disk parameter in xenstore Just assume disk as not removable when no ''removable'' paremeter Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -1724,6 +1724,7 @@ static unsigned int libxl_append_disk_li libxl__xs_get_dompath(&gc, 0), type, domid); dir = libxl__xs_directory(&gc, XBT_NULL, be_path, &n); if (dir) { + char *removable; *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n)); pdisk = *disks + *ndisks; *ndisks += n; @@ -1742,7 +1743,11 @@ static unsigned int libxl_append_disk_li libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->backend)); pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len); - pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir))); + removable = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir)); + if (removable) + pdisk->unpluggable = atoi(removable); + else + pdisk->unpluggable = 0; if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w")) pdisk->readwrite = 1; else _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Aug-30 17:19 UTC
Re: [Xen-devel] [PATCH 0 of 5] A bunch of fixes for libxl
Marek Marczykowski writes ("[Xen-devel] [PATCH 0 of 5] A bunch of fixes for libxl"):> Backorted version of patches.Applied all 5, thanks. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel