A bunch of fixes for libxl

PATCH New Marek Marczykowski 5 participants 34 messages Jun 5, 2011 – Aug 30, 2011 1 message not in the archive Arm/actions semi architecture

lore

Share & tools

lore

Marek Marczykowski

author

2011-06-05 16:50 UTC #0 lore raw

PATCH 1/1 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).

Marek Marczykowski

author

2011-06-05 16:50 UTC #1 lore raw

PATCH 1/1 libxl: Remove frontend and backend devices from xenstore after destroy

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>

tools/libxl/libxl.c +0 -2

diff -r 37c77bacb52a -r c32797243a6b tools/libxl/libxl.c
@@ -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;

tools/libxl/libxl_device.c +5 ~~-10~~

diff -r 37c77bacb52a -r c32797243a6b tools/libxl/libxl_device.c
@@ -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:

Ian Campbell

Acked-by on v1 1/1

2011-06-06 10:00 UTC #2 lore raw

Re: [PATCH 1 of 6 RESENT] libxl: Remove frontend and backend devices from xenstore after destroy

Quoted 7 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# Date 1307143993 -7200
# Node ID c32797243a6ba61dd2942a0307151e42fb7bf157
# Parent 37c77bacb52aa7795978b994f9d371b979b2cb07
libxl: Remove frontend and backend devices from xenstore after destroy

Quoted 5 lines

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 <[email protected]>

Acked-by Ian Campbell <[email protected]>

Quoted 87 lines

diff -r 37c77bacb52a -r c32797243a6b tools/libxl/libxl.c

tools/libxl/libxl.c +0 -2

@@ -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

tools/libxl/libxl_device.c +5 ~~-10~~

@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Ian Jackson

Applied

2011-06-27 16:29 UTC #3 lore raw

Re: [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"):

Quoted 8 lines · Marek Marczykowski

Quoted 7 lines
# HG changeset patch
# User Marek Marczykowski <[email protected]>
# Date 1307143993 -7200
# Node ID c32797243a6ba61dd2942a0307151e42fb7bf157
# Parent 37c77bacb52aa7795978b994f9d371b979b2cb07
libxl: Remove frontend and backend devices from xenstore after destroy

Quoted 7 lines

Quoted 5 lines
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 <[email protected]>
Acked-by Ian Campbell <[email protected]>

Applied, thanks.

Ian.

Marek Marczykowski

author

2011-06-05 16:50 UTC #4 lore raw

PATCH 1/1 libxl: Accept disk name in libxl_devid_to_device_disk

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # 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 <[email protected]>

tools/libxl/libxl_device.c +1 -1

diff -r c32797243a6b -r 5231fa19f3e3 tools/libxl/libxl_device.c
@@ -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;

tools/libxl/libxl_internal.h +1 -1

diff -r c32797243a6b -r 5231fa19f3e3 tools/libxl/libxl_internal.h
@@ -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,

tools/libxl/libxl_utils.c +4 -4

diff -r c32797243a6b -r 5231fa19f3e3 tools/libxl/libxl_utils.c
@@ -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;
     }

Ian Campbell

Acked-by on v1 1/1

2011-06-06 11:24 UTC #5 lore raw

Re: [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk

Quoted 10 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # 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 <[email protected]>

Acked-by Ian Campbell <[email protected]>

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 <[email protected]>
# 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 <[email protected]>

tools/libxl/libxl_device.c +5 -1

diff -r 86009542df09 -r b5dfb12aa231 tools/libxl/libxl_device.c
@@ -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,

Ian Campbell

2011-06-06 11:31 UTC #6 lore raw

Re: [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk

Quoted 18 lines · Ian Campbell

Quoted 10 lines · Marek Marczykowski
# HG changeset patch
# User Marek Marczykowski <[email protected]> # 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 <[email protected]>
Acked-by Ian Campbell <[email protected]>
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.

Quoted 5 lines

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.

Ian Jackson

Applied

2011-06-27 16:28 UTC #7 lore raw

Re: [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk [and 1 more messages]

In reply to Marek Marczykowski

Marek Marczykowski writes ("[Xen-devel] [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk"):

Quoted 1 line

libxl: Accept disk name in libxl_devid_to_device_disk

Applied, thanks (long lines fixed up).

Ian.

Marek Marczykowski

author

2011-06-05 16:50 UTC #8 lore raw

PATCH 1/1 libxl: Allocate memory for strings in libxl_device_disk

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # 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 <[email protected]>

tools/libxl/libxl_utils.c +2 -2

diff -r 5231fa19f3e3 -r 9fe949c7ab96 tools/libxl/libxl_utils.c
@@ -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));

Ian Campbell

Acked-by on v1 1/1

2011-06-06 11:24 UTC #9 lore raw

Re: [PATCH 3 of 6 RESENT] libxl: Allocate memory for strings in libxl_device_disk

Quoted 7 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962954 -7200
# Node ID 9fe949c7ab9601bb5500a53c538f7a23b61e1bcb # Parent 5231fa19f3e39091ff29e2a6dca057ca54403092
libxl: Allocate memory for strings in libxl_device_disk

Quoted 6 lines

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 <[email protected]>

Acked-by Ian Campbell <[email protected]>

Quoted 24 lines

diff -r 5231fa19f3e3 -r 9fe949c7ab96 tools/libxl/libxl_utils.c

tools/libxl/libxl_utils.c +2 -2

@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Ian Jackson

Applied

2011-06-27 16:25 UTC #10 lore raw

Re: [PATCH 3 of 6 RESENT] libxl: Allocate memory for strings in libxl_device_disk

In reply to Marek Marczykowski

Marek Marczykowski writes ("[Xen-devel] [PATCH 3 of 6 RESENT] libxl: Allocate memory for strings in libxl_device_disk"):

Quoted 6 lines

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

Applied, thanks (long lines fixed up).

Ian.

Marek Marczykowski

author

2011-06-05 16:50 UTC #11 lore raw

PATCH 1/1 xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>

tools/libxl/xl_cmdimpl.c +38 ~~-25~~

diff -r 9fe949c7ab96 -r 0863dfd23b3a tools/libxl/xl_cmdimpl.c
@@ -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;

Stefano Stabellini

Acked-by on v1 1/1

2011-06-07 12:02 UTC #12 lore raw

Re: [PATCH 4 of 6 RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")

Quoted 7 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# Date 1307145042 -7200
# Node ID 0863dfd23b3a10bee6d3eda30415dff0eef2bee1
# Parent 9fe949c7ab9601bb5500a53c538f7a23b61e1bcb
xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")

Quoted 5 lines

'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 <[email protected]>

It seems correct to me and makes the code easier to read, so

Acked-by Stefano Stabellini <[email protected]>

Quoted 112 lines

diff -r 9fe949c7ab96 -r 0863dfd23b3a tools/libxl/xl_cmdimpl.c

tools/libxl/xl_cmdimpl.c +38 ~~-25~~

@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Marek Marczykowski

author

2011-06-05 16:50 UTC #13 lore raw

PATCH 1/1 xen.lowlevel.xl: Return None on empty domain name

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # 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 <[email protected]>

tools/python/xen/lowlevel/xl/xl.c +5 -3

diff -r 0863dfd23b3a -r 0c0f9e1bd140 tools/python/xen/lowlevel/xl/xl.c
@@ -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;
 }

Ian Jackson

Acked-by on v1 1/1

2011-06-27 16:34 UTC #14 lore raw

Re: [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"):

Quoted 6 lines

xen.lowlevel.xl: Return None on empty domain name
Previously PyString_FromString(NULL) was called, which caused assertion failure.
Signed-off-by Marek Marczykowski <[email protected]>

Acked-by Ian Jackson <[email protected]>

Committed-by Ian Jackson <[email protected]>

I modified it slightly to make the diff and complexity slightly smaller (since "free(NULL)" is legal).

Ian.

A message that is not in the archive (sent off-list or not yet received): [email protected]

Marek Marczykowski

author

2011-06-28 21:37 UTC #16 lore raw

Backport libxl bugfixes to 4.1 (was: Re: [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)

Quoted 1 line · Ian Jackson

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

Signature · 3 lines

Pozdrawiam / Best Regards,
Marek Marczykowski         | RLU #390519
marmarek at mimuw edu pl   | xmpp:marmarek at staszic waw pl

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2011-06-28 21:57 UTC #17 lore raw

Re: Backport libxl bugfixes to 4.1 (was: Re: [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)

Quoted 6 lines · Marek Marczykowski

Quoted 1 line · Ian Jackson
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

Quoted 12 lines

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

Ian Jackson

2011-06-30 16:47 UTC #18 lore raw

Backport libxl bugfixes to 4.1 (was: Re: [PATCH 5 of 6 RESENT] xen.lowlevel.xl: Return None on empty domain name)

In reply to Marek Marczykowski

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)"):

Quoted 6 lines · Ian Jackson

Quoted 1 line
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 ?

Quoted 1 line

I can also send patches ready for 4.1 tree if it helps.

That would certainly be useful.

Quoted 6 lines

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.

Quoted 1 line

23602:5c353f53c8e2 xl: Use macros for param parsing in network-attach

This one isn't, I think ?

Ian.

Ian Jackson

2011-08-25 11:05 UTC #19 lore raw

Backport libxl bugfixes to 4.1 (was: Re: [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)"):

Quoted 5 lines

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)"):
Quoted 1 line
I can also send patches ready for 4.1 tree if it helps.
That would certainly be useful.

Quoted 8 lines

Quoted 6 lines
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.

Marek Marczykowski

author

2011-08-25 11:12 UTC #20 lore raw

Re: Backport libxl bugfixes to 4.1

Quoted 13 lines · Ian Jackson

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)"):
Quoted 12 lines
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)"):
Quoted 1 line
I can also send patches ready for 4.1 tree if it helps.
That would certainly be useful.
Quoted 6 lines
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

Quoted 3 lines

Quoted 1 line
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).

Signature · 3 lines

Pozdrawiam / Best Regards,
Marek Marczykowski         | RLU #390519
marmarek at mimuw edu pl   | xmpp:marmarek at staszic waw pl

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Marek Marczykowski

author

2011-08-25 17:13 UTC #21 lore raw

PATCH 1/1 A bunch of fixes for libxl

Backorted version of patches.

Marek Marczykowski

author

2011-08-25 17:13 UTC #22 lore raw

PATCH 1/1 libxl: Remove frontend and backend devices from xenstore after destroy

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>

tools/libxl/libxl_device.c +5 ~~-11~~

@@ -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:

Marek Marczykowski

author

2011-08-25 17:13 UTC #23 lore raw

PATCH 1/1 libxl: Accept disk name in libxl_devid_to_device_disk

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # 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 <[email protected]>

tools/libxl/libxl_device.c +1 -1

@@ -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;

tools/libxl/libxl_internal.h +1 -1

@@ -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);

tools/libxl/libxl_utils.c +4 -4

@@ -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;
     }

Marek Marczykowski

author

2011-08-25 17:13 UTC #24 lore raw

PATCH 1/1 libxl: Allocate memory for strings in libxl_device_disk

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # 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 <[email protected]>

tools/libxl/libxl_utils.c +2 -2

@@ -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));

Marek Marczykowski

author

2011-08-25 17:13 UTC #25 lore raw

PATCH 1/1 xen.lowlevel.xl: Return None on empty domain name

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # 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 <[email protected]>

tools/python/xen/lowlevel/xl/xl.c +5 -3

@@ -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;
 }

Marek Marczykowski

author

2011-08-25 17:13 UTC #26 lore raw

PATCH 1/1 libxl: Do not SEGV when no 'removable' disk parameter in xenstore

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>

tools/libxl/libxl.c +6 -1

@@ -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

Ian Jackson

Applied

2011-08-30 17:19 UTC #27 lore raw

In reply to Marek Marczykowski

Marek Marczykowski writes ("[Xen-devel] [PATCH 0 of 5] A bunch of fixes for libxl"):

Quoted 1 line

Backorted version of patches.

Applied all 5, thanks.

Ian.

Marek Marczykowski

author

2011-06-05 16:50 UTC #28 lore raw

PATCH 1/1 libxl: Do not SEGV when no 'removable' disk parameter in xenstore

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>

tools/libxl/libxl.c +6 -0

diff -r 0c0f9e1bd140 -r 70fc2a0f0a60 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;

@@ -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

Stefano Stabellini

2011-06-07 11:57 UTC #29 lore raw

Re: [PATCH 6 of 6 RESENT] libxl: Do not SEGV when no 'removable' disk parameter in xenstore

Quoted 23 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>
diff -r 0c0f9e1bd140 -r 70fc2a0f0a60 tools/libxl/libxl.c
tools/libxl/libxl.c +1 -0
@@ -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;

tools/libxl/libxl.c static unsigned int libxl__append_disk_l · line 1584

Quoted 3 lines

tools/libxl/libxl.c +0 -0

@@ -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?

Quoted 16 lines

tools/libxl/libxl.c +5 -0

+            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
[email protected]
http://lists.xensource.com/xen-devel

Marek Marczykowski

author

2011-06-07 12:00 UTC #30 lore raw

Re: [PATCH 6 of 6 RESENT] libxl: Do not SEGV when no 'removable' disk parameter in xenstore

Quoted 21 lines · Stefano Stabellini

Quoted 20 lines · Marek Marczykowski
# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>
diff -r 0c0f9e1bd140 -r 70fc2a0f0a60 tools/libxl/libxl.c
tools/libxl/libxl.c +1 -0
@@ -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));

Quoted 8 lines

tools/libxl/libxl.c static unsigned int libxl__append_disk_l · line 1584

Quoted 6 lines

tools/libxl/libxl.c +0 -0

         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...

Quoted 17 lines

tools/libxl/libxl.c +5 -0

+            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
[email protected]
http://lists.xensource.com/xen-devel

Signature · 3 lines

Pozdrawiam / Best Regards,
Marek Marczykowski         | RLU #390519
marmarek at mimuw edu pl   | xmpp:marmarek at staszic waw pl

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Marek Marczykowski

author

2011-06-08 10:41 UTC #31 lore raw

PATCH 1/1 libxl: Do not SEGV when no 'removable' disk parameter in xenstore

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>

tools/libxl/libxl.c +6 -1

@@ -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

Stefano Stabellini

Acked-by on v1 1/1

2011-06-08 10:50 UTC #32 lore raw

Re: [PATCH] libxl: Do not SEGV when no 'removable' disk parameter in xenstore

Quoted 11 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# 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 <[email protected]>

Acked-by Stefano Stabellini <[email protected]>

Quoted 32 lines

tools/libxl/libxl.c +6 -1

@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Ian Jackson

Applied

2011-06-27 16:37 UTC #33 lore raw

Re: [PATCH] libxl: Do not SEGV when no 'removable' disk parameter in xenstore [and 1 more messages]

In reply to Marek Marczykowski

Marek Marczykowski writes ("[Xen-devel] [PATCH] libxl: Do not SEGV when no 'removable' disk parameter in xenstore"):

Quoted 5 lines

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 <[email protected]>

Thanks, applied.

(Fixed up conflict due to "unpluggable" rename, fixed up long lines).

Ian.

Marek Marczykowski

author

2011-06-08 10:42 UTC #34 lore raw

PATCH 1/1 libxl: Do not SEGV when no 'removable' disk parameter in xenstore

In reply to Marek Marczykowski

Just assume disk as not removable when no 'removable' paremeter

Signed-off-by Marek Marczykowski <[email protected]>

tools/libxl/libxl.c +6 -1

@@ -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