Skip to contentSkip to conversation

A bunch of fixes (and one feature) for libxl

PATCH New Marek Marczykowski 3 participants 26 messages Jun 2, 2011 – Jun 4, 2011 Arm/actions semi architecture

lore
Share & tools
lore

Marek Marczykowski

author
#0 lore raw

PATCH 1/1 A bunch of fixes (and one feature) for libxl

A bunch of simple fixes for libxl. Most of them fixes SEGV or other annoying bugs.

The last patch implements simple way to reuse block* scripts in /etc/xen/scripts (called by udev). This most likely will be ignored because of more elegant way pending in paches on the list. But maybe someone will be interested in it.

author
#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 1306962865 -7200
# Node ID e3a3f5cc95349e92b7cb8b1448e999ffc16bd060
# 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.

Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/libxl.c +0 -2
@@ -1065,8 +1065,6 @@
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
     rc = libxl__device_del(ctx, &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));
     libxl__free_all(&gc);
     return rc;
 }
tools/libxl/libxl_device.c +2 -0
@@ -401,6 +401,8 @@
         (void)wait_for_dev_destroy(ctx, &tv);
     }
 
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, dev));
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, dev));
     rc = 0;
 
 out:
#2 lore raw

Re: [PATCH 01 of 10] libxl: Remove frontend and backend devices from xenstore after destroy

Hi Marek,

Please can you add:
[diff]
showfunc = True
to your ~/.hgrc. It makes patches much easier to read.

Quoted 11 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# Date 1306962865 -7200
# Node ID e3a3f5cc95349e92b7cb8b1448e999ffc16bd060
# 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.

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

Thanks!

The whole device destroy/teardown path is a bit of a twisty maze of confusingly named functions and parameters (e.g. force == !wait, destroy vs delete vs remove etc). Any attempt to try and untangle things is most welcome and I think this is a step in the right direction.

Quoted 17 lines
tools/libxl/libxl.c +0 -2
@@ -1065,8 +1065,6 @@
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
     rc = libxl__device_del(ctx, &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));
     libxl__free_all(&gc);
     return rc;
 }

tools/libxl/libxl_device.c line 405

Quoted 5 lines
tools/libxl/libxl_device.c +2 -0
@@ -401,6 +401,8 @@
         (void)wait_for_dev_destroy(ctx, &tv);
     }
 
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, dev));
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, dev));

In the case where wait == true libxl__device_destroy will add a watch on the be path and wait_for_dev_destroy will subsequently remove the path.

I think it would be better to add the xs_rm for be path to libxl__device_destroy as the else clause to the !force which adds the watch? I think this allow the other caller (libxl__devices_destroy) to be simplified too since all that mucking around with the toremove flexarray could be dropped.

Ian.

Quoted 10 lines
tools/libxl/libxl_device.c +0 -0
     rc = 0;
 
 out:

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

author
#3 lore raw

Re: [PATCH 01 of 10] libxl: Remove frontend and backend devices from xenstore after destroy

Quoted 47 lines · Ian Campbell

Hi Marek,

Please can you add:
[diff]
showfunc = True
to your ~/.hgrc. It makes patches much easier to read.

Quoted 11 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# Date 1306962865 -7200
# Node ID e3a3f5cc95349e92b7cb8b1448e999ffc16bd060
# 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.

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

Thanks!

The whole device destroy/teardown path is a bit of a twisty maze of confusingly named functions and parameters (e.g. force == !wait, destroy vs delete vs remove etc). Any attempt to try and untangle things is most welcome and I think this is a step in the right direction.

Quoted 20 lines
tools/libxl/libxl.c +0 -2
@@ -1065,8 +1065,6 @@
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
     rc = libxl__device_del(ctx, &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));
     libxl__free_all(&gc);
     return rc;
 }
tools/libxl/libxl_device.c +0 -0
@@ -401,6 +401,8 @@
         (void)wait_for_dev_destroy(ctx, &tv);
     }
Quoted 5 lines

tools/libxl/libxl_device.c line 405

Quoted 2 lines
tools/libxl/libxl_device.c +2 -0
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, dev));
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, dev));

In the case where wait == true libxl__device_destroy will add a watch on the be path and wait_for_dev_destroy will subsequently remove the path.

Only backend one...

Quoted 3 lines

I think it would be better to add the xs_rm for be path to libxl__device_destroy as the else clause to the !force which adds the watch?

Also - only backend one (libxl__device_destroy gets only backend path as parameter).

Quoted 3 lines

I think this allow the other caller (libxl__devices_destroy) to be simplified too since all that mucking around with the toremove flexarray could be dropped.

What about frontend device in xenstore? In most cases only backend is removed. In wait_for_dev_destroy we don't know frontend patch (wheel, not exactly true, but not always easy). Perhaps there should be two watches - on backend AND on frontend (with writing "5" to frontend state also?)? Or backend should be removed as you suggested, but frontend as in my patch (after wait_for_dev_destroy if was called)?

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

#4 lore raw

Re: [PATCH 01 of 10] libxl: Remove frontend and backend devices from xenstore after destroy

Quoted 67 lines · Marek Marczykowski
Quoted 47 lines · Ian Campbell

Hi Marek,

Please can you add:
[diff]
showfunc = True
to your ~/.hgrc. It makes patches much easier to read.

Quoted 11 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# Date 1306962865 -7200
# Node ID e3a3f5cc95349e92b7cb8b1448e999ffc16bd060
# 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.

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

Thanks!

The whole device destroy/teardown path is a bit of a twisty maze of confusingly named functions and parameters (e.g. force == !wait, destroy vs delete vs remove etc). Any attempt to try and untangle things is most welcome and I think this is a step in the right direction.

Quoted 20 lines
tools/libxl/libxl.c +0 -2
@@ -1065,8 +1065,6 @@
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
     rc = libxl__device_del(ctx, &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));
     libxl__free_all(&gc);
     return rc;
 }
tools/libxl/libxl_device.c +0 -0
@@ -401,6 +401,8 @@
         (void)wait_for_dev_destroy(ctx, &tv);
     }
Quoted 5 lines

tools/libxl/libxl_device.c line 405

Quoted 2 lines
tools/libxl/libxl_device.c +2 -0
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, dev));
+    xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, dev));

In the case where wait == true libxl__device_destroy will add a watch on the be path and wait_for_dev_destroy will subsequently remove the path.

Only backend one...

Quoted 3 lines

I think it would be better to add the xs_rm for be path to libxl__device_destroy as the else clause to the !force which adds the watch?

Also - only backend one (libxl__device_destroy gets only backend path as parameter).

Quoted 3 lines

I think this allow the other caller (libxl__devices_destroy) to be simplified too since all that mucking around with the toremove flexarray could be dropped.

Quoted 6 lines

What about frontend device in xenstore? In most cases only backend is removed. In wait_for_dev_destroy we don't know frontend patch (wheel, not exactly true, but not always easy). Perhaps there should be two watches - on backend AND on frontend (with writing "5" to frontend state also?)? Or backend should be removed as you suggested, but frontend as in my patch (after wait_for_dev_destroy if was called)?

My comments were only about the location of the rm of the backend path.

I think your original patch moved the rm of the frontend path to the right place.

an.

author
#5 lore raw

PATCH 1/1 libxl: Do not start stubdom when not needed

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962900 -7200
# Node ID 59cff6b471f3f0c1e91349ed6653a47a40d4fd55 # Parent e3a3f5cc95349e92b7cb8b1448e999ffc16bd060
libxl: Do not start stubdom when not needed

Do not start stubdom when there is any disk - only when there are disks requiring it.

Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/libxl_dm.c +8 -2
@@ -828,8 +828,14 @@
         goto out;
     }
 
-    if (nr_disks > 0 && !libxl__blktap_enabled(&gc))
-        ret = 1;
+    if (nr_disks > 0 && !libxl__blktap_enabled(&gc)) {
+        for (i = 0; i < nr_disks; i++) {
+            if (disks[i].backend == DISK_BACKEND_TAP || disks[i].backend == DISK_BACKEND_QDISK) {
+                ret = 1;
+                goto out;
+            }
+        }
+    }
 
 out:
     libxl__free_all(&gc);
#6 lore raw

Re: [PATCH 02 of 10] libxl: Do not start stubdom when not needed

Quoted 11 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962900 -7200
# Node ID 59cff6b471f3f0c1e91349ed6653a47a40d4fd55 # Parent e3a3f5cc95349e92b7cb8b1448e999ffc16bd060
libxl: Do not start stubdom when not needed

Do not start stubdom when there is any disk - only when there are disks requiring it.

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

What tree is your series against? This same issue was fixed by 23044:d4ca456c0c25 in mid-March.

In general you need to post patches against a reasonably recent xen-unstable. You can also recommend patches (either new ones or existing commits in xen-unstable) for backporting to the x.y-testing trees if you think that is necessary.

I though 23044:d4ca456c0c25 had already been backported to 4.1-testing but it seems not, Keir can we do so please?

Ian.

Quoted 28 lines
tools/libxl/libxl_dm.c +8 -2
@@ -828,8 +828,14 @@
         goto out;
     }
 
-    if (nr_disks > 0 && !libxl__blktap_enabled(&gc))
-        ret = 1;
+    if (nr_disks > 0 && !libxl__blktap_enabled(&gc)) {
+        for (i = 0; i < nr_disks; i++) {
+            if (disks[i].backend == DISK_BACKEND_TAP || disks[i].backend == DISK_BACKEND_QDISK) {
+                ret = 1;
+                goto out;
+            }
+        }
+    }
 
 out:
     libxl__free_all(&gc);

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

author
#7 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 81df382694fd6e208255197d8da19bfe435a2cbd # Parent 59cff6b471f3f0c1e91349ed6653a47a40d4fd55
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_utils.c +4 -4
@@ -529,18 +529,18 @@
                                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((char *)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;
     }
#8 lore raw

Re: [PATCH 03 of 10] libxl: Accept disk name in libxl_devid_to_device_disk

Quoted 17 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962929 -7200
# Node ID 81df382694fd6e208255197d8da19bfe435a2cbd # Parent 59cff6b471f3f0c1e91349ed6653a47a40d4fd55
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_utils.c +0 -0
@@ -529,18 +529,18 @@
                                const char *devid, libxl_device_disk *disk)
 {

tools/libxl/libxl_utils.c line 537

Quoted 10 lines
tools/libxl/libxl_utils.c +2 -3
     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((char *)devid);

This function takes 3 parameters in xen-unstable.hg so this won't work. Please can you rebase.

Also casting away the const like that is probably unsafe (it's likely a const for a reason) and should have raised alarm bells.

libxl__device_disk_dev_number should take a const, it looks like that would be a trivial change since the function doesn't appear to actually modify the string, although I could be mistaken, in which case libxl__device_disk_dev_number needs fixing to not modify the string.

Cheers,
Ian.

Quoted 17 lines
tools/libxl/libxl_utils.c +2 -1
+    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
[email protected]
http://lists.xensource.com/xen-devel

author
#9 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 df639d3eef683460b6d5ab38296cbd90b26f60f0 # Parent 81df382694fd6e208255197d8da19bfe435a2cbd
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 @@
     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 = strdup(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)));
     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 = strdup(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)));
     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));
#10 lore raw

Re: [PATCH 04 of 10] libxl: Allocate memory for strings in libxl_device_disk

Quoted 18 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962954 -7200
# Node ID df639d3eef683460b6d5ab38296cbd90b26f60f0 # Parent 81df382694fd6e208255197d8da19bfe435a2cbd
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 line 558

Quoted 10 lines
tools/libxl/libxl_utils.c +2 -2
@@ -551,10 +551,10 @@
     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 = strdup(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)));
 
     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 = strdup(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)));

I think it is acceptable to use xs_read() directly in both these cases and avoid the need to strdup.

Ian.

Quoted 10 lines
tools/libxl/libxl_utils.c +0 -0
     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

author
#11 lore raw

PATCH 1/1 libxl: Set libxl_device_nic->domid when looking up by devid

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962980 -7200
# Node ID 3e5e8eaf2fe8352e584e7498fde21d6e76c3475b # Parent df639d3eef683460b6d5ab38296cbd90b26f60f0
libxl: Set libxl_device_nic->domid when looking up by devid

Fixes xl network-detach with device specified by id, not MAC

Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/libxl_utils.c +1 -0
@@ -512,6 +512,7 @@
     }
     nic->backend_domid = strtoul(val, NULL, 10);
     nic->devid = strtoul(devid, NULL, 10);
+    nic->domid = domid;
 
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mac", nic_path_fe));
     for (i = 0, tok = strtok(val, ":"); tok && (i < 6);
#12 lore raw

Re: [PATCH 05 of 10] libxl: Set libxl_device_nic->domid when looking up by devid

Quoted 8 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962980 -7200
# Node ID 3e5e8eaf2fe8352e584e7498fde21d6e76c3475b # Parent df639d3eef683460b6d5ab38296cbd90b26f60f0
libxl: Set libxl_device_nic->domid when looking up by devid

Fixes xl network-detach with device specified by id, not MAC

Please rebase onto xen-unstable.hg, the libxl_device_* structures no longer contain a domid in that tree, it is passed as a parameter to the relevant functions now.

Ian.

Quoted 21 lines
Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/libxl_utils.c +1 -0
@@ -512,6 +512,7 @@
     }
     nic->backend_domid = strtoul(val, NULL, 10);
     nic->devid = strtoul(devid, NULL, 10);
+    nic->domid = domid;
 
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mac", nic_path_fe));
     for (i = 0, tok = strtok(val, ":"); tok && (i < 6);

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

author
#13 lore raw

Re: [PATCH 05 of 10] libxl: Set libxl_device_nic->domid when looking up by devid

Quoted 8 lines · Ian Campbell
Quoted 7 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962980 -7200
# Node ID 3e5e8eaf2fe8352e584e7498fde21d6e76c3475b # Parent df639d3eef683460b6d5ab38296cbd90b26f60f0
libxl: Set libxl_device_nic->domid when looking up by devid

Quoted 5 lines
Quoted 1 line

Fixes xl network-detach with device specified by id, not MAC

Please rebase onto xen-unstable.hg, the libxl_device_* structures no longer contain a domid in that tree, it is passed as a parameter to the relevant functions now.

Ok, but in 4.1 tree still domid field exists and is used. So this should be fixed somehow (by applying this patch, or by backporting some other patches from unstable)...

Quoted 26 lines

Ian.

Quoted 21 lines
Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/libxl_utils.c +1 -0
@@ -512,6 +512,7 @@
     }
     nic->backend_domid = strtoul(val, NULL, 10);
     nic->devid = strtoul(devid, NULL, 10);
+    nic->domid = domid;
 
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mac", nic_path_fe));
     for (i = 0, tok = strtok(val, ":"); tok && (i < 6);

_______________________________________________
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

#14 lore raw

Re: [PATCH 05 of 10] libxl: Set libxl_device_nic->domid when looking up by devid

Quoted 11 lines · Marek Marczykowski
Quoted 10 lines · Ian Campbell
Quoted 8 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306962980 -7200
# Node ID 3e5e8eaf2fe8352e584e7498fde21d6e76c3475b # Parent df639d3eef683460b6d5ab38296cbd90b26f60f0
libxl: Set libxl_device_nic->domid when looking up by devid

Fixes xl network-detach with device specified by id, not MAC

Quoted 7 lines
Quoted 3 lines

Please rebase onto xen-unstable.hg, the libxl_device_* structures no longer contain a domid in that tree, it is passed as a parameter to the relevant functions now.

Ok, but in 4.1 tree still domid field exists and is used. So this should be fixed somehow (by applying this patch, or by backporting some other patches from unstable)...

IMHO the patch which removes domid is not suitable for backporting (since it changes libxl API). So this patch seems suitable for 4.1 directly, I think.

Thanks,Ian.

Quoted 29 lines
Quoted 27 lines

Ian.

Quoted 21 lines
Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/libxl_utils.c +1 -0
@@ -512,6 +512,7 @@
     }
     nic->backend_domid = strtoul(val, NULL, 10);
     nic->devid = strtoul(devid, NULL, 10);
+    nic->domid = domid;
 
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mac", nic_path_fe));
     for (i = 0, tok = strtok(val, ":"); tok && (i < 6);

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

author
#15 lore raw

PATCH 1/1 xl: Allocate memory for libxl_device_nic string members

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306963069 -7200
# Node ID eb7216a75b7d7a5de93c717401b447545022b582 # Parent 3e5e8eaf2fe8352e584e7498fde21d6e76c3475b
xl: Allocate memory for libxl_device_nic string members

Do not use argv directly - libxl_device_nic_destroy will try to free() it.

Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/xl_cmdimpl.c +8 -4
@@ -4280,12 +4280,14 @@
                 nic.mac[i] = val;
             }
         } else if (!strncmp("bridge=", *argv, 7)) {
-            nic.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)) {
-            nic.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)) {
                 fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");

@@ -4293,9 +4295,11 @@
             }
             nic.backend_domid = val;
         } else if (!strncmp("vifname=", *argv, 8)) {
-            nic.ifname = (*argv) + 8;
+			free(nic.ifname);
+            nic.ifname = strdup((*argv) + 8);
         } else if (!strncmp("model=", *argv, 6)) {
-            nic.model = (*argv) + 6;
+            free(nic.model);
+            nic.model = strdup((*argv) + 6);
         } else if (!strncmp("rate=", *argv, 5)) {
         } else if (!strncmp("accel=", *argv, 6)) {
         } else {
#16 lore raw

Re: [PATCH 06 of 10] xl: Allocate memory for libxl_device_nic string members

Quoted 15 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306963069 -7200
# Node ID eb7216a75b7d7a5de93c717401b447545022b582 # Parent 3e5e8eaf2fe8352e584e7498fde21d6e76c3475b
xl: Allocate memory for libxl_device_nic string members

Do not use argv directly - libxl_device_nic_destroy will try to free() it.

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

tools/libxl/xl_cmdimpl.c line 4284

Quoted 6 lines
tools/libxl/xl_cmdimpl.c +2 -1
@@ -4280,12 +4280,14 @@
                 nic.mac[i] = val;
             }
         } else if (!strncmp("bridge=", *argv, 7)) {
-            nic.bridge = (*argv) + 7;
+            free(nic.bridge);
+            nic.bridge = strdup((*argv) + 7);

For parsing the cfg files (which has a similar pattern) we defined a helper, xlu_cfg_replace_string. I think a similar helper for the non xlu_cfg case (e.g. replace_string) would be good here too.

Ian.

Quoted 31 lines
tools/libxl/xl_cmdimpl.c +6 -3
         } else if (!strncmp("ip=", *argv, 3)) {
             free(nic.ip);
             nic.ip = strdup((*argv) + 3);
         } else if (!strncmp("script=", *argv, 6)) {
-            nic.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)) {
                 fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");

@@ -4293,9 +4295,11 @@
             }
             nic.backend_domid = val;
         } else if (!strncmp("vifname=", *argv, 8)) {
-            nic.ifname = (*argv) + 8;
+			free(nic.ifname);
+            nic.ifname = strdup((*argv) + 8);
         } else if (!strncmp("model=", *argv, 6)) {
-            nic.model = (*argv) + 6;
+            free(nic.model);
+            nic.model = strdup((*argv) + 6);
         } else if (!strncmp("rate=", *argv, 5)) {
         } else if (!strncmp("accel=", *argv, 6)) {
         } else {

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

author
#17 lore raw

PATCH 1/1 xl: Fix 'script' param parsing in network-attach

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306963105 -7200
# Node ID 6811aa543e69379557ff7391ea3db8a5e7f7dde0 # Parent eb7216a75b7d7a5de93c717401b447545022b582
xl: Fix 'script' param parsing in network-attach

Fix 'script=' string length

Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/xl_cmdimpl.c +2 -2
@@ -4285,9 +4285,9 @@
         } else if (!strncmp("ip=", *argv, 3)) {
             free(nic.ip);
             nic.ip = strdup((*argv) + 3);
-        } else if (!strncmp("script=", *argv, 6)) {
+        } else if (!strncmp("script=", *argv, 7)) {
             free(nic.script);
-            nic.script = strdup((*argv) + 6);
+            nic.script = strdup((*argv) + 7);
         } else if (!strncmp("backend=", *argv, 8)) {
             if(libxl_name_to_domid(&ctx, ((*argv) + 8), &val)) {
                 fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
#18 lore raw

Re: [PATCH 07 of 10] xl: Fix 'script' param parsing in network-attach

Quoted 15 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306963105 -7200
# Node ID 6811aa543e69379557ff7391ea3db8a5e7f7dde0 # Parent eb7216a75b7d7a5de93c717401b447545022b582
xl: Fix 'script' param parsing in network-attach

Fix 'script=' string length

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

tools/libxl/xl_cmdimpl.c line 4290

Quoted 8 lines
tools/libxl/xl_cmdimpl.c +2 -2
@@ -4285,9 +4285,9 @@
         } else if (!strncmp("ip=", *argv, 3)) {
             free(nic.ip);
             nic.ip = strdup((*argv) + 3);
-        } else if (!strncmp("script=", *argv, 6)) {
+        } else if (!strncmp("script=", *argv, 7)) {
             free(nic.script);
-            nic.script = strdup((*argv) + 6);
+            nic.script = strdup((*argv) + 7);

Good catch.

The pre-existing use of all those
strncmp(A, *argv, open-coded-sizeof(A))
must be a source of many such errors. A helper function (or macro) is probably the way to go. Do you fancy coding that up?

Ian.

Quoted 10 lines
tools/libxl/xl_cmdimpl.c +0 -0
         } else if (!strncmp("backend=", *argv, 8)) {
             if(libxl_name_to_domid(&ctx, ((*argv) + 8), &val)) {
                 fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");

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

author
#19 lore raw

Re: [PATCH 07 of 10] xl: Fix 'script' param parsing in network-attach

Quoted 27 lines · Ian Campbell
Quoted 15 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306963105 -7200
# Node ID 6811aa543e69379557ff7391ea3db8a5e7f7dde0 # Parent eb7216a75b7d7a5de93c717401b447545022b582
xl: Fix 'script' param parsing in network-attach

Fix 'script=' string length

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

tools/libxl/xl_cmdimpl.c line 4290

Quoted 8 lines
tools/libxl/xl_cmdimpl.c +2 -2
@@ -4285,9 +4285,9 @@
         } else if (!strncmp("ip=", *argv, 3)) {
             free(nic.ip);
             nic.ip = strdup((*argv) + 3);
-        } else if (!strncmp("script=", *argv, 6)) {
+        } else if (!strncmp("script=", *argv, 7)) {
             free(nic.script);
-            nic.script = strdup((*argv) + 6);
+            nic.script = strdup((*argv) + 7);

Good catch.

Quoted 4 lines

The pre-existing use of all those
strncmp(A, *argv, open-coded-sizeof(A))
must be a source of many such errors. A helper function (or macro) is probably the way to go. Do you fancy coding that up?

And the same size must be used in strdup offset... Maybe something like this (solving also problem in 06 patch):
-----------

#define COMPARE_AND_REPLACE(pattern, dest) \
    } else if (!strncmp(pattern, *argv, sizeof(pattern))) { \
        free(dest); \
        dest = strdup((*argv) + sizeof(pattern));

(...)

    COMPARE_AND_REPLACE("script=", nic.script)
    COMPARE_AND_REPLACE("ip=", nic.ip)

---------
Looks weird because of no semicolon at the end of lines, but should works. Unfortunately cannot be used in all places (like one below)... What do you think?

Quoted 6 lines

Ian.

Quoted 3 lines
         } else if (!strncmp("backend=", *argv, 8)) {
             if(libxl_name_to_domid(&ctx, ((*argv) + 8), &val)) {
                 fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
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

#20 lore raw

Re: [PATCH 07 of 10] xl: Fix 'script' param parsing in network-attach

Quoted 41 lines · Marek Marczykowski
Quoted 27 lines · Ian Campbell
Quoted 15 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306963105 -7200
# Node ID 6811aa543e69379557ff7391ea3db8a5e7f7dde0 # Parent eb7216a75b7d7a5de93c717401b447545022b582
xl: Fix 'script' param parsing in network-attach

Fix 'script=' string length

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

tools/libxl/xl_cmdimpl.c line 4290

Quoted 8 lines
tools/libxl/xl_cmdimpl.c +2 -2
@@ -4285,9 +4285,9 @@
         } else if (!strncmp("ip=", *argv, 3)) {
             free(nic.ip);
             nic.ip = strdup((*argv) + 3);
-        } else if (!strncmp("script=", *argv, 6)) {
+        } else if (!strncmp("script=", *argv, 7)) {
             free(nic.script);
-            nic.script = strdup((*argv) + 6);
+            nic.script = strdup((*argv) + 7);

Good catch.

Quoted 4 lines

The pre-existing use of all those
strncmp(A, *argv, open-coded-sizeof(A))
must be a source of many such errors. A helper function (or macro) is probably the way to go. Do you fancy coding that up?

And the same size must be used in strdup offset... Maybe something like this (solving also problem in 06 patch):
-----------

#define COMPARE_AND_REPLACE(pattern, dest) \
    } else if (!strncmp(pattern, *argv, sizeof(pattern))) { \
        free(dest); \
        dest = strdup((*argv) + sizeof(pattern));
Quoted 6 lines

(...)

    COMPARE_AND_REPLACE("script=", nic.script)
    COMPARE_AND_REPLACE("ip=", nic.ip)

---------
Looks weird because of no semicolon at the end of lines, but should works.

Generally you can fix the semi-colon thing by omitting the last one from the macro itself.

Quoted 2 lines

Unfortunately cannot be used in all places (like one below)... What do you think?

How about (untested), requires a local "char *oparg":

static int match_option_size(const char *prefix, size_t len,
char *arg, char **argopt)

{
    int rc = strncmp(prefix, argv, len);
    if (!rc) *argopt = argv+len;
    return !rc
}
#define match_option(_prefix, _arg, _oparg) \
    match_option_size((_prefix), (_arg), sizeof((_prefix)), &(_oparg));

static void replace_string(char **str, const char *val)

{
    free(*str);\
    *str = strdup(val);
}

then you have:

    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");
            ...
        }
    }

with a bit of macro pasting in match_option you can probably push the "=" down into the macro so each callsite doesn't need it.

Ian.

Quoted 10 lines
Quoted 7 lines

Ian.

Quoted 3 lines
         } else if (!strncmp("backend=", *argv, 8)) {
             if(libxl_name_to_domid(&ctx, ((*argv) + 8), &val)) {
                 fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
author
#21 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 1306963128 -7200
# Node ID d03bd7a830bff8162b6b0564d27b3f36ca52d8bb # Parent 6811aa543e69379557ff7391ea3db8a5e7f7dde0 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 -2
@@ -418,8 +418,11 @@
         return NULL;
 
     domname = libxl_domid_to_name(&self->ctx, domid);
-    ret = PyString_FromString(domname);
-    free(domname);
+    if (domname) {
+        ret = PyString_FromString(domname);
+        free(domname);
+    } else
+        return Py_None;
 
     return ret;
 }
#22 lore raw

Re: [PATCH 08 of 10] xen.lowlevel.xl: Return None on empty domain name

Quoted 16 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]> # Date 1306963128 -7200
# Node ID d03bd7a830bff8162b6b0564d27b3f36ca52d8bb # Parent 6811aa543e69379557ff7391ea3db8a5e7f7dde0 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 line 425

Quoted 10 lines
tools/python/xen/lowlevel/xl/xl.c +5 -2
@@ -418,8 +418,11 @@
         return NULL;
 
     domname = libxl_domid_to_name(&self->ctx, domid);
-    ret = PyString_FromString(domname);
-    free(domname);
+    if (domname) {
+        ret = PyString_FromString(domname);
+        free(domname);
+    } else
+        return Py_None;

Please do

           ret = Py_None;

instead. Having a mixture the single return location and individual returns isn't nice.

Personally I'd just initialise ret to Py_None in the first place and omit the else.

Ian.

Quoted 10 lines
tools/python/xen/lowlevel/xl/xl.c +0 -0
     return ret;
 }

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

author
#23 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 1306963174 -7200
# Node ID 10bdf8aa5b8a1cb2178f2b9cbcc9ce2fd5644403
# Parent d03bd7a830bff8162b6b0564d27b3f36ca52d8bb
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
@@ -1722,6 +1722,7 @@
                              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;

@@ -1740,7 +1741,11 @@
                 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
author
#24 lore raw

PATCH 1/1 libxl: "script:" prefix in block device description to setup vbd by hotplug scripts

In reply to Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# Date 1306963204 -7200
# Node ID 1e3bf3cb3944402e01c57c400a02fe4293153458
# Parent 10bdf8aa5b8a1cb2178f2b9cbcc9ce2fd5644403
libxl: "script:" prefix in block device description to setup vbd by hotplug scripts

Implements old behaviour of block-attach - only write parameters to xenstore and left all work for hotplug scripts. Allows to directly reuse xen block scripts (and maybe some custom ones). Example use:

xl block-attach vm "script:nbd 1.1.1.1 9999" xvdg w

Signed-off-by Marek Marczykowski <[email protected]>
tools/libxl/libxl.c +26 -1
@@ -896,6 +896,13 @@
         }
     }
 
+    if (disk->backend == DISK_BACKEND_SCRIPT) {
+        delimiter = strchr(file_name, ':');
+        if (!delimiter)
+            return ERROR_INVAL;
+        return 0;
+    }
+
     if ( stat(file_name, &stat_buf) != 0 ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;

@@ -965,6 +972,24 @@
 
             device.backend_kind = DEVICE_VBD;
             break;
+        case DISK_BACKEND_SCRIPT:
+			{
+				char *delimiter;
+				backend_type = libxl__strdup(&gc, disk->pdev_path);
+				delimiter = strchr(backend_type, ':');
+				if (!delimiter) {
+					rc = ERROR_FAIL;
+					goto out_free;
+				}
+				*delimiter = '\0';
+				flexarray_append(back, "params");
+				flexarray_append(back, delimiter+1);
+                flexarray_append(back, "scripted");
+                flexarray_append(back, "1");
+
+				device.backend_kind = DEVICE_VBD;
+			}
+            break;
         case DISK_BACKEND_TAP:
             if (libxl__blktap_enabled(&gc) && disk->format != DISK_FORMAT_EMPTY) {
                 const char *dev = libxl__blktap_devpath(&gc,

@@ -1060,7 +1085,7 @@
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->backend == DISK_BACKEND_PHY || disk->backend == DISK_BACKEND_SCRIPT) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
tools/libxl/libxl.h +1 -0
@@ -185,6 +185,7 @@
     DISK_BACKEND_PHY,
     DISK_BACKEND_TAP,
     DISK_BACKEND_QDISK,
+    DISK_BACKEND_SCRIPT,
 } libxl_disk_backend;
 
 typedef enum {
tools/libxl/libxl_device.c +1 -0
@@ -139,6 +139,7 @@
         case DISK_BACKEND_QDISK: return "qdisk";
         case DISK_BACKEND_TAP: return "tap";
         case DISK_BACKEND_PHY: return "phy";
+        case DISK_BACKEND_SCRIPT: return "script";
         default: return NULL;
     }
 }
tools/libxl/libxl_utils.c +9 -2
@@ -282,6 +282,8 @@
 
     if (!strcmp(s, "phy")) {
         *backend = DISK_BACKEND_PHY;
+    } else if (!strcmp(s, "script")) {
+        *backend = DISK_BACKEND_SCRIPT;
     } else if (!strcmp(s, "file")) {
         *backend = DISK_BACKEND_TAP;
     } else if (!strcmp(s, "tap")) {

@@ -553,8 +555,13 @@
     disk->domid = domid;
     be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath));
     disk->pdev_path = strdup(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)));
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
-    libxl_string_to_backend(ctx, val, &(disk->backend));
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/scripted", be_path));
+    if (val && atoi(val))
+        disk->backend = DISK_BACKEND_SCRIPT;
+    else {
+        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
+        libxl_string_to_backend(ctx, val, &(disk->backend));
+    }
     disk->vdev = strdup(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path));
     disk->unpluggable = !strcmp(val, "1");
tools/libxl/xl_cmdimpl.c +6 -0
@@ -481,6 +481,10 @@
                     state = DSTATE_PHYSPATH;
                     disk->format = DISK_FORMAT_RAW;
                     disk->backend = DISK_BACKEND_PHY;
+                }else if ( !strcmp(tok, "script") ) {
+                    state = DSTATE_PHYSPATH;
+                    disk->format = DISK_FORMAT_RAW;
+                    disk->backend = DISK_BACKEND_SCRIPT;
                 }else if ( !strcmp(tok, "file") ) {
                     state = DSTATE_PHYSPATH;
                     disk->format = DISK_FORMAT_RAW;

@@ -4435,6 +4439,8 @@
     tok = strtok(argv[optind+1], ":");
     if (!strcmp(tok, "phy")) {
         disk.backend = DISK_BACKEND_PHY;
+    } else if (!strcmp(tok, "script")) {
+        disk.backend = DISK_BACKEND_SCRIPT;
     } else if (!strcmp(tok, "file")) {
         disk.backend = DISK_BACKEND_TAP;
     } else if (!strcmp(tok, "tap")) {
#25 lore raw

Re: [PATCH 10 of 10] libxl: "script:" prefix in block device description to setup vbd by hotplug scripts

Quoted 7 lines · Marek Marczykowski

# HG changeset patch
# User Marek Marczykowski <[email protected]>
# Date 1306963204 -7200
# Node ID 1e3bf3cb3944402e01c57c400a02fe4293153458
# Parent 10bdf8aa5b8a1cb2178f2b9cbcc9ce2fd5644403
libxl: "script:" prefix in block device description to setup vbd by hotplug scripts

Quoted 7 lines

Implements old behaviour of block-attach - only write parameters to xenstore and left all work for hotplug scripts. Allows to directly reuse xen block scripts (and maybe some custom ones). Example use:

xl block-attach vm "script:nbd 1.1.1.1 9999" xvdg w

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

I didn't review this one, I think it probably interacts badly with (or more likely is obsoleted by) Ian Jackson's recent disk parsing cleanup series.

Ian.

Quoted 132 lines
tools/libxl/libxl.c +26 -1
@@ -896,6 +896,13 @@
         }
     }
 
+    if (disk->backend == DISK_BACKEND_SCRIPT) {
+        delimiter = strchr(file_name, ':');
+        if (!delimiter)
+            return ERROR_INVAL;
+        return 0;
+    }
+
     if ( stat(file_name, &stat_buf) != 0 ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;

@@ -965,6 +972,24 @@
 
             device.backend_kind = DEVICE_VBD;
             break;
+        case DISK_BACKEND_SCRIPT:
+			{
+				char *delimiter;
+				backend_type = libxl__strdup(&gc, disk->pdev_path);
+				delimiter = strchr(backend_type, ':');
+				if (!delimiter) {
+					rc = ERROR_FAIL;
+					goto out_free;
+				}
+				*delimiter = '\0';
+				flexarray_append(back, "params");
+				flexarray_append(back, delimiter+1);
+                flexarray_append(back, "scripted");
+                flexarray_append(back, "1");
+
+				device.backend_kind = DEVICE_VBD;
+			}
+            break;
         case DISK_BACKEND_TAP:
             if (libxl__blktap_enabled(&gc) && disk->format != DISK_FORMAT_EMPTY) {
                 const char *dev = libxl__blktap_devpath(&gc,

@@ -1060,7 +1085,7 @@
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->backend == DISK_BACKEND_PHY || disk->backend == DISK_BACKEND_SCRIPT) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
tools/libxl/libxl.h +1 -0
@@ -185,6 +185,7 @@
     DISK_BACKEND_PHY,
     DISK_BACKEND_TAP,
     DISK_BACKEND_QDISK,
+    DISK_BACKEND_SCRIPT,
 } libxl_disk_backend;
 
 typedef enum {
tools/libxl/libxl_device.c +1 -0
@@ -139,6 +139,7 @@
         case DISK_BACKEND_QDISK: return "qdisk";
         case DISK_BACKEND_TAP: return "tap";
         case DISK_BACKEND_PHY: return "phy";
+        case DISK_BACKEND_SCRIPT: return "script";
         default: return NULL;
     }
 }
tools/libxl/libxl_utils.c +9 -2
@@ -282,6 +282,8 @@
 
     if (!strcmp(s, "phy")) {
         *backend = DISK_BACKEND_PHY;
+    } else if (!strcmp(s, "script")) {
+        *backend = DISK_BACKEND_SCRIPT;
     } else if (!strcmp(s, "file")) {
         *backend = DISK_BACKEND_TAP;
     } else if (!strcmp(s, "tap")) {

@@ -553,8 +555,13 @@
     disk->domid = domid;
     be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath));
     disk->pdev_path = strdup(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)));
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
-    libxl_string_to_backend(ctx, val, &(disk->backend));
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/scripted", be_path));
+    if (val && atoi(val))
+        disk->backend = DISK_BACKEND_SCRIPT;
+    else {
+        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
+        libxl_string_to_backend(ctx, val, &(disk->backend));
+    }
     disk->vdev = strdup(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path));
     disk->unpluggable = !strcmp(val, "1");
tools/libxl/xl_cmdimpl.c +6 -0
@@ -481,6 +481,10 @@
                     state = DSTATE_PHYSPATH;
                     disk->format = DISK_FORMAT_RAW;
                     disk->backend = DISK_BACKEND_PHY;
+                }else if ( !strcmp(tok, "script") ) {
+                    state = DSTATE_PHYSPATH;
+                    disk->format = DISK_FORMAT_RAW;
+                    disk->backend = DISK_BACKEND_SCRIPT;
                 }else if ( !strcmp(tok, "file") ) {
                     state = DSTATE_PHYSPATH;
                     disk->format = DISK_FORMAT_RAW;

@@ -4435,6 +4439,8 @@
     tok = strtok(argv[optind+1], ":");
     if (!strcmp(tok, "phy")) {
         disk.backend = DISK_BACKEND_PHY;
+    } else if (!strcmp(tok, "script")) {
+        disk.backend = DISK_BACKEND_SCRIPT;
     } else if (!strcmp(tok, "file")) {
         disk.backend = DISK_BACKEND_TAP;
     } else if (!strcmp(tok, "tap")) {

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