Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Xen: Devel

[PATCH v6 05/11] libxl: introduce libxl__device_disk_add

 

 

Xen devel RSS feed   Index | Next | Previous | View Threaded


stefano.stabellini at eu

May 18, 2012, 7:08 AM

Post #1 of 7 (142 views)
Permalink
[PATCH v6 05/11] libxl: introduce libxl__device_disk_add

Introduce libxl__device_disk_add that takes an additional
xs_transaction_t paramter.
Implement libxl_device_disk_add using libxl__device_disk_add.

Signed-off-by: Stefano Stabellini <stefano.stabellini [at] eu>
---
tools/libxl/libxl.c | 113 +---------------------------------------
tools/libxl/libxl_internal.c | 119 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 2 +
3 files changed, 122 insertions(+), 112 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3f53da5..1bed2fe 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1284,118 +1284,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
{
GC_INIT(ctx);
- flexarray_t *front;
- flexarray_t *back;
- char *dev;
- libxl__device device;
- int major, minor, rc;
-
- rc = libxl__device_disk_setdefault(gc, disk);
- if (rc) goto out;
-
- front = flexarray_make(16, 1);
- if (!front) {
- rc = ERROR_NOMEM;
- goto out;
- }
- back = flexarray_make(16, 1);
- if (!back) {
- rc = ERROR_NOMEM;
- goto out_free;
- }
-
- if (disk->script) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
- " not yet supported, sorry");
- rc = ERROR_INVAL;
- goto out_free;
- }
-
- rc = libxl__device_from_disk(gc, domid, disk, &device);
- if (rc != 0) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
- " virtual disk identifier %s", disk->vdev);
- goto out_free;
- }
-
- switch (disk->backend) {
- case LIBXL_DISK_BACKEND_PHY:
- dev = disk->pdev_path;
- do_backend_phy:
- libxl__device_physdisk_major_minor(dev, &major, &minor);
- flexarray_append(back, "physical-device");
- flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
-
- flexarray_append(back, "params");
- flexarray_append(back, dev);
-
- assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
- break;
- case LIBXL_DISK_BACKEND_TAP:
- dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
- if (!dev) {
- LOG(ERROR, "failed to get blktap devpath for %p\n",
- disk->pdev_path);
- rc = ERROR_FAIL;
- goto out_free;
- }
- flexarray_append(back, "tapdisk-params");
- flexarray_append(back, libxl__sprintf(gc, "%s:%s",
- libxl__device_disk_string_of_format(disk->format),
- disk->pdev_path));
-
- /* now create a phy device to export the device to the guest */
- goto do_backend_phy;
- case LIBXL_DISK_BACKEND_QDISK:
- flexarray_append(back, "params");
- flexarray_append(back, libxl__sprintf(gc, "%s:%s",
- libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
- assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
- break;
- default:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
- rc = ERROR_INVAL;
- goto out_free;
- }
-
- flexarray_append(back, "frontend-id");
- flexarray_append(back, libxl__sprintf(gc, "%d", domid));
- flexarray_append(back, "online");
- flexarray_append(back, "1");
- flexarray_append(back, "removable");
- flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
- flexarray_append(back, "bootable");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
- flexarray_append(back, "state");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
- flexarray_append(back, "dev");
- flexarray_append(back, disk->vdev);
- flexarray_append(back, "type");
- flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
- flexarray_append(back, "mode");
- flexarray_append(back, disk->readwrite ? "w" : "r");
- flexarray_append(back, "device-type");
- flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
-
- flexarray_append(front, "backend-id");
- flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
- flexarray_append(front, "state");
- flexarray_append(front, libxl__sprintf(gc, "%d", 1));
- flexarray_append(front, "virtual-device");
- flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
- flexarray_append(front, "device-type");
- flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
-
- libxl__device_generic_add(gc, XBT_NULL, &device,
- libxl__xs_kvs_of_flexarray(gc, back, back->count),
- libxl__xs_kvs_of_flexarray(gc, front, front->count));
-
- rc = 0;
-
-out_free:
- flexarray_free(back);
- flexarray_free(front);
-out:
+ int rc = libxl__device_disk_add(gc, domid, XBT_NULL, disk);
GC_FREE;
return rc;
}
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 276429c..70a8c4b 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -323,6 +323,125 @@ out:
return rc;
}

+int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+ xs_transaction_t t, libxl_device_disk *disk)
+{
+ flexarray_t *front;
+ flexarray_t *back;
+ char *dev;
+ libxl__device device;
+ int major, minor, rc;
+ libxl_ctx *ctx = gc->owner;
+
+ rc = libxl__device_disk_setdefault(gc, disk);
+ if (rc) goto out;
+
+ front = flexarray_make(16, 1);
+ if (!front) {
+ rc = ERROR_NOMEM;
+ goto out;
+ }
+ back = flexarray_make(16, 1);
+ if (!back) {
+ rc = ERROR_NOMEM;
+ goto out_free;
+ }
+
+ if (disk->script) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
+ " not yet supported, sorry");
+ rc = ERROR_INVAL;
+ goto out_free;
+ }
+
+ rc = libxl__device_from_disk(gc, domid, disk, &device);
+ if (rc != 0) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
+ " virtual disk identifier %s", disk->vdev);
+ goto out_free;
+ }
+
+ switch (disk->backend) {
+ case LIBXL_DISK_BACKEND_PHY:
+ dev = disk->pdev_path;
+ do_backend_phy:
+ libxl__device_physdisk_major_minor(dev, &major, &minor);
+ flexarray_append(back, "physical-device");
+ flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
+
+ flexarray_append(back, "params");
+ flexarray_append(back, dev);
+
+ assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
+ break;
+ case LIBXL_DISK_BACKEND_TAP:
+ dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
+ if (!dev) {
+ LOG(ERROR, "failed to get blktap devpath for %p\n",
+ disk->pdev_path);
+ rc = ERROR_FAIL;
+ goto out_free;
+ }
+ flexarray_append(back, "tapdisk-params");
+ flexarray_append(back, libxl__sprintf(gc, "%s:%s",
+ libxl__device_disk_string_of_format(disk->format),
+ disk->pdev_path));
+
+ /* now create a phy device to export the device to the guest */
+ goto do_backend_phy;
+ case LIBXL_DISK_BACKEND_QDISK:
+ flexarray_append(back, "params");
+ flexarray_append(back, libxl__sprintf(gc, "%s:%s",
+ libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
+ assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
+ break;
+ default:
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
+ rc = ERROR_INVAL;
+ goto out_free;
+ }
+
+ flexarray_append(back, "frontend-id");
+ flexarray_append(back, libxl__sprintf(gc, "%d", domid));
+ flexarray_append(back, "online");
+ flexarray_append(back, "1");
+ flexarray_append(back, "removable");
+ flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
+ flexarray_append(back, "bootable");
+ flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(back, "state");
+ flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(back, "dev");
+ flexarray_append(back, disk->vdev);
+ flexarray_append(back, "type");
+ flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
+ flexarray_append(back, "mode");
+ flexarray_append(back, disk->readwrite ? "w" : "r");
+ flexarray_append(back, "device-type");
+ flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
+
+ flexarray_append(front, "backend-id");
+ flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
+ flexarray_append(front, "state");
+ flexarray_append(front, libxl__sprintf(gc, "%d", 1));
+ flexarray_append(front, "virtual-device");
+ flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
+ flexarray_append(front, "device-type");
+ flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
+
+ libxl__device_generic_add(gc, t, &device,
+ libxl__xs_kvs_of_flexarray(gc, back, back->count),
+ libxl__xs_kvs_of_flexarray(gc, front, front->count));
+
+ rc = 0;
+
+out_free:
+ flexarray_free(back);
+ flexarray_free(front);
+out:
+ return rc;
+}
+
char * libxl__device_disk_local_attach(libxl__gc *gc,
const libxl_device_disk *in_disk,
libxl_device_disk *disk)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3578414..98f9762 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1236,6 +1236,8 @@ _hidden void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path);
_hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
libxl_device_disk *disk,
libxl__device *device);
+_hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+ xs_transaction_t t, libxl_device_disk *disk);

/*
* Make a disk available in this (the control) domain. Returns path to
--
1.7.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


Ian.Jackson at eu

May 18, 2012, 7:36 AM

Post #2 of 7 (145 views)
Permalink
Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add [In reply to]

Stefano Stabellini writes ("[PATCH v6 05/11] libxl: introduce libxl__device_disk_add"):
> Introduce libxl__device_disk_add that takes an additional
> xs_transaction_t paramter.
> Implement libxl_device_disk_add using libxl__device_disk_add.

Can't this be done in such a way that the diff isn't "delete this
function completely" followed by "here is a new function" ?

I don't really understand why you want to move the function at all,
TBH. I think keeping the public wrapper and the internal
implementation together is fine. There is no need IMO to move the
internal function to libxl_internal.c.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


Ian.Campbell at citrix

May 18, 2012, 7:42 AM

Post #3 of 7 (145 views)
Permalink
Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add [In reply to]

On Fri, 2012-05-18 at 15:36 +0100, Ian Jackson wrote:
> There is no need IMO to move the
> internal function to libxl_internal.c.

I've always thought the existence libxl_internal.c was just begging for
people to group functions along the wrong criteria anyway...

The whole concept of what lives where inside tools/libxl/libxl*.c is all
pretty ad-hoc anyway.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


Ian.Jackson at eu

May 18, 2012, 7:47 AM

Post #4 of 7 (144 views)
Permalink
Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add [In reply to]

Ian Campbell writes ("Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add"):
> On Fri, 2012-05-18 at 15:36 +0100, Ian Jackson wrote:
> > There is no need IMO to move the
> > internal function to libxl_internal.c.
>
> I've always thought the existence libxl_internal.c was just begging for
> people to group functions along the wrong criteria anyway...

Yes.

> The whole concept of what lives where inside tools/libxl/libxl*.c is all
> pretty ad-hoc anyway.

Quite.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


roger.pau at citrix

May 18, 2012, 7:56 AM

Post #5 of 7 (144 views)
Permalink
Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add [In reply to]

Stefano Stabellini wrote:
> Introduce libxl__device_disk_add that takes an additional
> xs_transaction_t paramter.
> Implement libxl_device_disk_add using libxl__device_disk_add.
>
> Signed-off-by: Stefano Stabellini<stefano.stabellini [at] eu>
> ---
> tools/libxl/libxl.c | 113 +---------------------------------------
> tools/libxl/libxl_internal.c | 119 ++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_internal.h | 2 +
> 3 files changed, 122 insertions(+), 112 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 3f53da5..1bed2fe 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1284,118 +1284,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
> int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
> {
> GC_INIT(ctx);
> - flexarray_t *front;
> - flexarray_t *back;
> - char *dev;
> - libxl__device device;
> - int major, minor, rc;
> -
> - rc = libxl__device_disk_setdefault(gc, disk);
> - if (rc) goto out;
> -
> - front = flexarray_make(16, 1);
> - if (!front) {
> - rc = ERROR_NOMEM;
> - goto out;
> - }
> - back = flexarray_make(16, 1);
> - if (!back) {
> - rc = ERROR_NOMEM;
> - goto out_free;
> - }
> -
> - if (disk->script) {
> - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
> - " not yet supported, sorry");
> - rc = ERROR_INVAL;
> - goto out_free;
> - }
> -
> - rc = libxl__device_from_disk(gc, domid, disk,&device);
> - if (rc != 0) {
> - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
> - " virtual disk identifier %s", disk->vdev);
> - goto out_free;
> - }
> -
> - switch (disk->backend) {
> - case LIBXL_DISK_BACKEND_PHY:
> - dev = disk->pdev_path;
> - do_backend_phy:
> - libxl__device_physdisk_major_minor(dev,&major,&minor);
> - flexarray_append(back, "physical-device");
> - flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
> -
> - flexarray_append(back, "params");
> - flexarray_append(back, dev);
> -
> - assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
> - break;
> - case LIBXL_DISK_BACKEND_TAP:
> - dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
> - if (!dev) {
> - LOG(ERROR, "failed to get blktap devpath for %p\n",
> - disk->pdev_path);
> - rc = ERROR_FAIL;
> - goto out_free;
> - }
> - flexarray_append(back, "tapdisk-params");
> - flexarray_append(back, libxl__sprintf(gc, "%s:%s",
> - libxl__device_disk_string_of_format(disk->format),
> - disk->pdev_path));
> -
> - /* now create a phy device to export the device to the guest */
> - goto do_backend_phy;
> - case LIBXL_DISK_BACKEND_QDISK:
> - flexarray_append(back, "params");
> - flexarray_append(back, libxl__sprintf(gc, "%s:%s",
> - libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
> - assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
> - break;
> - default:
> - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
> - rc = ERROR_INVAL;
> - goto out_free;
> - }
> -
> - flexarray_append(back, "frontend-id");
> - flexarray_append(back, libxl__sprintf(gc, "%d", domid));
> - flexarray_append(back, "online");
> - flexarray_append(back, "1");
> - flexarray_append(back, "removable");
> - flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
> - flexarray_append(back, "bootable");
> - flexarray_append(back, libxl__sprintf(gc, "%d", 1));
> - flexarray_append(back, "state");
> - flexarray_append(back, libxl__sprintf(gc, "%d", 1));
> - flexarray_append(back, "dev");
> - flexarray_append(back, disk->vdev);
> - flexarray_append(back, "type");
> - flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
> - flexarray_append(back, "mode");
> - flexarray_append(back, disk->readwrite ? "w" : "r");
> - flexarray_append(back, "device-type");
> - flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
> -
> - flexarray_append(front, "backend-id");
> - flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
> - flexarray_append(front, "state");
> - flexarray_append(front, libxl__sprintf(gc, "%d", 1));
> - flexarray_append(front, "virtual-device");
> - flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
> - flexarray_append(front, "device-type");
> - flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
> -
> - libxl__device_generic_add(gc, XBT_NULL,&device,
> - libxl__xs_kvs_of_flexarray(gc, back, back->count),
> - libxl__xs_kvs_of_flexarray(gc, front, front->count));
> -
> - rc = 0;
> -
> -out_free:
> - flexarray_free(back);
> - flexarray_free(front);
> -out:
> + int rc = libxl__device_disk_add(gc, domid, XBT_NULL, disk);
> GC_FREE;
> return rc;
> }
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index 276429c..70a8c4b 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -323,6 +323,125 @@ out:
> return rc;
> }
>
> +int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
> + xs_transaction_t t, libxl_device_disk *disk)

As with the other patch, I think this should go to libxl_device.

> +{
> + flexarray_t *front;
> + flexarray_t *back;
> + char *dev;
> + libxl__device device;
> + int major, minor, rc;
> + libxl_ctx *ctx = gc->owner;
> +
> + rc = libxl__device_disk_setdefault(gc, disk);
> + if (rc) goto out;
> +
> + front = flexarray_make(16, 1);
> + if (!front) {
> + rc = ERROR_NOMEM;
> + goto out;
> + }
> + back = flexarray_make(16, 1);
> + if (!back) {
> + rc = ERROR_NOMEM;
> + goto out_free;
> + }
> +
> + if (disk->script) {
> + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
> + " not yet supported, sorry");
> + rc = ERROR_INVAL;
> + goto out_free;
> + }
> +
> + rc = libxl__device_from_disk(gc, domid, disk,&device);
> + if (rc != 0) {
> + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
> + " virtual disk identifier %s", disk->vdev);
> + goto out_free;
> + }
> +
> + switch (disk->backend) {
> + case LIBXL_DISK_BACKEND_PHY:
> + dev = disk->pdev_path;
> + do_backend_phy:
> + libxl__device_physdisk_major_minor(dev,&major,&minor);
> + flexarray_append(back, "physical-device");
> + flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
> +
> + flexarray_append(back, "params");
> + flexarray_append(back, dev);
> +
> + assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
> + break;
> + case LIBXL_DISK_BACKEND_TAP:
> + dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
> + if (!dev) {
> + LOG(ERROR, "failed to get blktap devpath for %p\n",
> + disk->pdev_path);
> + rc = ERROR_FAIL;
> + goto out_free;
> + }
> + flexarray_append(back, "tapdisk-params");
> + flexarray_append(back, libxl__sprintf(gc, "%s:%s",
> + libxl__device_disk_string_of_format(disk->format),
> + disk->pdev_path));
> +
> + /* now create a phy device to export the device to the guest */
> + goto do_backend_phy;
> + case LIBXL_DISK_BACKEND_QDISK:
> + flexarray_append(back, "params");
> + flexarray_append(back, libxl__sprintf(gc, "%s:%s",
> + libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
> + assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
> + break;
> + default:
> + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
> + rc = ERROR_INVAL;
> + goto out_free;
> + }
> +
> + flexarray_append(back, "frontend-id");
> + flexarray_append(back, libxl__sprintf(gc, "%d", domid));
> + flexarray_append(back, "online");
> + flexarray_append(back, "1");
> + flexarray_append(back, "removable");
> + flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
> + flexarray_append(back, "bootable");
> + flexarray_append(back, libxl__sprintf(gc, "%d", 1));
> + flexarray_append(back, "state");
> + flexarray_append(back, libxl__sprintf(gc, "%d", 1));
> + flexarray_append(back, "dev");
> + flexarray_append(back, disk->vdev);
> + flexarray_append(back, "type");
> + flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
> + flexarray_append(back, "mode");
> + flexarray_append(back, disk->readwrite ? "w" : "r");
> + flexarray_append(back, "device-type");
> + flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
> +
> + flexarray_append(front, "backend-id");
> + flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
> + flexarray_append(front, "state");
> + flexarray_append(front, libxl__sprintf(gc, "%d", 1));
> + flexarray_append(front, "virtual-device");
> + flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
> + flexarray_append(front, "device-type");
> + flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
> +
> + libxl__device_generic_add(gc, t,&device,
> + libxl__xs_kvs_of_flexarray(gc, back, back->count),
> + libxl__xs_kvs_of_flexarray(gc, front, front->count));
> +
> + rc = 0;
> +
> +out_free:
> + flexarray_free(back);
> + flexarray_free(front);
> +out:
> + return rc;
> +}
> +
> char * libxl__device_disk_local_attach(libxl__gc *gc,
> const libxl_device_disk *in_disk,
> libxl_device_disk *disk)
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 3578414..98f9762 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1236,6 +1236,8 @@ _hidden void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path);
> _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
> libxl_device_disk *disk,
> libxl__device *device);
> +_hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
> + xs_transaction_t t, libxl_device_disk *disk);
>
> /*
> * Make a disk available in this (the control) domain. Returns path to
> --
> 1.7.2.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel [at] lists
> http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


stefano.stabellini at eu

May 21, 2012, 9:30 AM

Post #6 of 7 (137 views)
Permalink
Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add [In reply to]

On Fri, 18 May 2012, Ian Jackson wrote:
> Stefano Stabellini writes ("[PATCH v6 05/11] libxl: introduce libxl__device_disk_add"):
> > Introduce libxl__device_disk_add that takes an additional
> > xs_transaction_t paramter.
> > Implement libxl_device_disk_add using libxl__device_disk_add.
>
> Can't this be done in such a way that the diff isn't "delete this
> function completely" followed by "here is a new function" ?

I don't know. Do you have any suggestions? TBH if I were you, I would
just open two terminals and compare line by line, but let me know if you
want to do something specific..


> I don't really understand why you want to move the function at all,
> TBH. I think keeping the public wrapper and the internal
> implementation together is fine. There is no need IMO to move the
> internal function to libxl_internal.c.

ATM there are no hidden functions in libxl.c.
Do you want to change this "policy"?

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


Ian.Jackson at eu

May 21, 2012, 9:48 AM

Post #7 of 7 (135 views)
Permalink
Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add [In reply to]

Stefano Stabellini writes ("Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add"):
> On Fri, 18 May 2012, Ian Jackson wrote:
> > Can't this be done in such a way that the diff isn't "delete this
> > function completely" followed by "here is a new function" ?
>
> I don't know. Do you have any suggestions? TBH if I were you, I would
> just open two terminals and compare line by line, but let me know if you
> want to do something specific..

If you leave all the existing code for the internal function in place
and simply change the function declaration and prologue, and
corresponding epilogue, you should find that diff doesn't mention the
contents at all.

The new public wrapper will of course be new.

> ATM there are no hidden functions in libxl.c.
> Do you want to change this "policy"?

I think there is no reason for this and it is not something I want to
preserve. The distinction between internal and external is not a good
basis for which file something should be in.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel

Xen devel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.