|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 08 of 11] blktap2: configurable driver chains
Hey Ryan.
The command line idea is ok, but how about "!"?
Also, please check the line width.
Also, please don't alter the kernel list macros [..or submit yours to
lkml].
Daniel
On Thu, 2009-11-05 at 17:58 -0500, Brendan Cully wrote:
> # HG changeset patch
> # User Ryan O'Connor <rjo@xxxxxxxxx>
> # Date 1252530408 25200
> # Node ID 4ca00ee41ce9731b8725c736b27c841b484dce5d
> # Parent 6ca67fe3514ada809a62282c30fe46d5df5ce265
> blktap2: configurable driver chains
>
> Blktap2 allows block device drivers to be layered to create more
> advanced virtual block devices. However, composing a layered driver is
> not exposed to the user. This patch fixes this, and allows the user to
> explicitly specify a driver chain when starting a tapdisk process, using
> the pipe character ('|') to explicitly seperate layers in a blktap2
> configuration string.
>
> for example, the command:
> ~$ tapdisk2 -n "log:|aio:/path/to/file.img"
> will create a blktap2 device where read and write requests are passed to
> the 'log' driver, then forwarded to the 'aio' driver.
>
> Signed-off-by: Ryan O'Connor <rjo@xxxxxxxxx>
>
> diff --git a/tools/blktap2/drivers/tapdisk-vbd.c
> b/tools/blktap2/drivers/tapdisk-vbd.c
> --- a/tools/blktap2/drivers/tapdisk-vbd.c
> +++ b/tools/blktap2/drivers/tapdisk-vbd.c
> @@ -106,6 +106,7 @@
> vbd->callback = tapdisk_vbd_callback;
> vbd->argument = vbd;
>
> + INIT_LIST_HEAD(&vbd->driver_stack);
> INIT_LIST_HEAD(&vbd->images);
> INIT_LIST_HEAD(&vbd->new_requests);
> INIT_LIST_HEAD(&vbd->pending_requests);
> @@ -541,6 +542,105 @@
> goto out;
> }
>
> +/* TODO: ugh, lets not call it parent info... */
> +static struct list_head *
> +tapdisk_vbd_open_level(td_vbd_t *vbd, char* params, int driver_type,
> td_disk_info_t *parent_info, td_flag_t flags)
> +{
> + char *name;
> + int type, err;
> + td_image_t *image;
> + td_disk_id_t id;
> + struct list_head *images;
> + td_driver_t *driver;
> +
> + images = calloc(1, sizeof(struct list_head));
> + INIT_LIST_HEAD(images);
> +
> + name = params;
> + type = driver_type;
> +
> + for (;;) {
> + err = -ENOMEM;
> + image = tapdisk_image_allocate(name, type,
> + vbd->storage, flags, vbd);
> +
> + /* free 'name' if it was created by td_get_parent_id() */
> + if (name != params) {
> + free(name);
> + name = NULL;
> + }
> +
> + if (!image)
> + return NULL;
> +
> +
> + /* We have to do this to set the driver info for child
> drivers. this conflicts with td_open */
> + driver = image->driver;
> + if (!driver) {
> + driver = tapdisk_driver_allocate(image->type,
> + image->name,
> + image->flags,
> + image->storage);
> + if (!driver)
> + return NULL;
> + }
> + /* the image has a driver, set the info and driver */
> + image->driver = driver;
> + image->info = driver->info;
> +
> + /* XXX: we don't touch driver->refcount, broken? */
> + /* XXX: we've replicated about 90% of td_open() gross! */
> + /* XXX: this breaks if a driver modifies its info within a
> layer */
> +
> + /* if the parent info is set, pass it to the child */
> + if(parent_info)
> + {
> + image->driver->info = *parent_info;
> + }
> +
> + err = td_load(image);
> + if (err) {
> + if (err != -ENODEV)
> + return NULL;
> +
> + err = td_open(image);
> + if (err)
> + return NULL;
> + }
> +
> + /* TODO: non-sink drivers that don't care about their child
> + * currently return EINVAL. Could return TD_PARENT_OK or
> + * TD_ANY_PARENT */
> +
> + err = td_get_parent_id(image, &id);
> + if (err && (err != TD_NO_PARENT && err != -EINVAL)) {
> + td_close(image);
> + return NULL;
> + }
> +
> + if (!image->storage)
> + image->storage = vbd->storage;
> +
> + /* add this image to the end of the list */
> + list_add_tail(&image->next, images);
> +
> + image = NULL;
> +
> + /* if the image does not have a parent we return the
> + * list of images generated by this level of the stack */
> + if (err == TD_NO_PARENT || err == -EINVAL)
> + break;
> +
> + name = id.name;
> + type = id.drivertype;
> +#if 0
> + /* catch this by validate, not here */
> + flags |= (TD_OPEN_RDONLY | TD_OPEN_SHAREABLE);
> +#endif
> + }
> + return images;
> +}
> +
> static int
> __tapdisk_vbd_open_vdi(td_vbd_t *vbd, td_flag_t extra_flags)
> {
> @@ -548,58 +648,36 @@
> int err, type;
> td_flag_t flags;
> td_disk_id_t id;
> - td_image_t *image, *tmp;
> + td_image_t *tmp;
> struct tfilter *filter = NULL;
> + td_vbd_driver_info_t *driver_info;
> + struct list_head *images;
> + td_disk_info_t *parent_info = NULL;
>
> err = tapdisk_vbd_reactivate_volumes(vbd, 0);
> if (err)
> return err;
>
> flags = (vbd->flags & ~TD_OPEN_SHAREABLE) | extra_flags;
> - file = vbd->name;
> - type = vbd->type;
>
> - for (;;) {
> - err = -ENOMEM;
> - image = tapdisk_image_allocate(file, type,
> - vbd->storage, flags, vbd);
>
> - if (file != vbd->name) {
> - free(file);
> - file = NULL;
> - }
> + /* loop on each user specified driver.
> + * NOTE: driver_info is in reverse order. That is, the first
> + * item is the 'parent' or 'sink' driver */
> + list_for_each_entry(driver_info, &vbd->driver_stack, next) {
> + file = driver_info->params;
> + type = driver_info->type;
> + images = tapdisk_vbd_open_level(vbd, file, type, parent_info,
> flags);
> + if (!images)
> + return -EINVAL;
>
> - if (!image)
> - goto fail;
> + /* after each loop, append the created stack to the result
> stack */
> + list_splice(images, &vbd->images);
> + free(images);
>
> - err = td_load(image);
> - if (err) {
> - if (err != -ENODEV)
> - goto fail;
> -
> - err = td_open(image);
> - if (err)
> - goto fail;
> - }
> -
> - err = td_get_parent_id(image, &id);
> - if (err && err != TD_NO_PARENT) {
> - td_close(image);
> - goto fail;
> - }
> -
> - if (!image->storage)
> - image->storage = vbd->storage;
> -
> - tapdisk_vbd_add_image(vbd, image);
> - image = NULL;
> -
> - if (err == TD_NO_PARENT)
> - break;
> -
> - file = id.name;
> - type = id.drivertype;
> - flags |= (TD_OPEN_RDONLY | TD_OPEN_SHAREABLE);
> + /* set the parent_info to the first diskinfo on the stack */
> + tmp = tapdisk_vbd_first_image(vbd);
> + parent_info = &tmp->info;
> }
>
> if (td_flag_test(vbd->flags, TD_OPEN_LOG_DIRTY)) {
> @@ -623,14 +701,91 @@
> return 0;
>
> fail:
> +
> +/* TODO: loop over vbd to free images? maybe do that in vbd_close_vdi */
> +#if 0
> if (image)
> tapdisk_image_free(image);
> +#endif
>
> + /* TODO: handle partial stack createion? */
> tapdisk_vbd_close_vdi(vbd);
>
> return err;
> }
>
> +/* this populates a vbd type based on path */
> +int
> +tapdisk_vbd_parse_stack(td_vbd_t *vbd, const char *path)
> +{
> + int err;
> + char *params, *driver_str;
> + td_vbd_driver_info_t *driver;
> +
> + /* make a copy of path */
> + /* TODO: check against MAX_NAME_LEM ? */
> + err = tapdisk_namedup(¶ms, path);
> + if(err)
> + goto error;
> +
> +
> + /* tokenize params based on pipe '|' */
> + driver_str = strtok(params, "|");
> + while(driver_str != NULL)
> + {
> + /* parse driver info and add to vbd */
> + driver = calloc(1, sizeof(td_vbd_driver_info_t));
> + INIT_LIST_HEAD(&driver->next);
> + err = tapdisk_parse_disk_type(driver_str, &driver->params,
> &driver->type);
> + if(err)
> + goto error;
> +
> + /* build the list backwards as the last driver will be the
> first
> + * driver to open in the stack */
> + list_add(&driver->next, &vbd->driver_stack);
> +
> + /* get next driver string */
> + driver_str = strtok(NULL, "|");
> + }
> +
> + return 0;
> +
> + /* error: free any driver_info's and params */
> +error:
> + while(!list_empty(&vbd->driver_stack)) {
> + driver = list_entry(vbd->driver_stack.next,
> td_vbd_driver_info_t, next);
> + list_del(&driver->next);
> + free(driver);
> + }
> +
> + return err;
> +}
> +
> +/* NOTE: driver type, etc. must be set */
> +static int
> +tapdisk_vbd_open_stack(td_vbd_t *vbd, uint16_t storage, td_flag_t flags)
> +{
> + int i, err;
> +
> + vbd->flags = flags;
> + vbd->storage = storage;
> +
> + for (i = 0; i < TD_VBD_EIO_RETRIES; i++) {
> + err = __tapdisk_vbd_open_vdi(vbd, 0);
> + if (err != -EIO)
> + break;
> +
> + sleep(TD_VBD_EIO_SLEEP);
> + }
> + if (err)
> + goto fail;
> +
> + return 0;
> +
> +fail:
> + return err;
> +}
> +
> int
> tapdisk_vbd_open_vdi(td_vbd_t *vbd, const char *path,
> uint16_t drivertype, uint16_t storage, td_flag_t flags)
> @@ -759,7 +914,7 @@
> {
> int err;
>
> - err = tapdisk_vbd_open_vdi(vbd, name, type, storage, flags);
> + err = tapdisk_vbd_open_stack(vbd, storage, flags);
> if (err)
> goto out;
>
> diff --git a/tools/blktap2/drivers/tapdisk-vbd.h
> b/tools/blktap2/drivers/tapdisk-vbd.h
> --- a/tools/blktap2/drivers/tapdisk-vbd.h
> +++ b/tools/blktap2/drivers/tapdisk-vbd.h
> @@ -53,6 +53,7 @@
>
> typedef struct td_ring td_ring_t;
> typedef struct td_vbd_request td_vbd_request_t;
> +typedef struct td_vbd_driver_info td_vbd_driver_info_t;
> typedef struct td_vbd_handle td_vbd_t;
> typedef void (*td_vbd_cb_t) (void *, blkif_response_t *);
>
> @@ -79,12 +80,20 @@
> struct list_head next;
> };
>
> +struct td_vbd_driver_info {
> + char *params;
> + int type;
> + struct list_head next;
> +};
> +
> struct td_vbd_handle {
> char *name;
>
> td_uuid_t uuid;
> int type;
>
> + struct list_head driver_stack;
> +
> int storage;
>
> uint8_t reopened;
> @@ -164,6 +173,7 @@
>
> int tapdisk_vbd_initialize(int, int, td_uuid_t);
> void tapdisk_vbd_set_callback(td_vbd_t *, td_vbd_cb_t, void *);
> +int tapdisk_vbd_parse_stack(td_vbd_t *vbd, const char *path);
> int tapdisk_vbd_open(td_vbd_t *, const char *, uint16_t,
> uint16_t, const char *, td_flag_t);
> int tapdisk_vbd_close(td_vbd_t *);
> diff --git a/tools/blktap2/drivers/tapdisk2.c
> b/tools/blktap2/drivers/tapdisk2.c
> --- a/tools/blktap2/drivers/tapdisk2.c
> +++ b/tools/blktap2/drivers/tapdisk2.c
> @@ -264,6 +264,13 @@
> return err;
> }
>
> + err = tapdisk_vbd_parse_stack(vbd, name);
> + if (err) {
> + CHILD_ERR(err, "vbd_parse_stack failed: %d\n", err);
> + return err;
> + }
> +
> + /* TODO: clean this up */
> err = tapdisk_vbd_open(vbd, path, type,
> TAPDISK_STORAGE_TYPE_DEFAULT,
> devname, 0);
> diff --git a/tools/blktap2/include/list.h b/tools/blktap2/include/list.h
> --- a/tools/blktap2/include/list.h
> +++ b/tools/blktap2/include/list.h
> @@ -87,6 +87,26 @@
> return list->next == head;
> }
>
> +static inline void __list_splice(struct list_head *list,
> + struct list_head *head)
> +{
> + struct list_head *first = list->next;
> + struct list_head *last = list->prev;
> + struct list_head *at = head->next;
> +
> + first->prev = head;
> + head->next = first;
> +
> + last->next = at;
> + at->prev = last;
> +}
> +
> +static inline void list_splice(struct list_head *list, struct list_head
> *head)
> +{
> + if (!list_empty(list))
> + __list_splice(list, head);
> +}
> +
> #define list_entry(ptr, type, member) \
> ((type *)((char *)(ptr)-(unsigned long)(&((type *)0)->member)))
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |