diff options
author | Keir Fraser <keir.fraser@citrix.com> | 2010-06-08 08:04:36 +0100 |
---|---|---|
committer | Keir Fraser <keir.fraser@citrix.com> | 2010-06-08 08:04:36 +0100 |
commit | c6913b5aaf48107f7bb9fb4e01c7f74c863b3713 (patch) | |
tree | fc54da78886f20023cd57fcc188f255f624b36ed /tools/blktap2 | |
parent | 08a07a95dd601b4fe686e4e2d8ef41930652d2bb (diff) | |
download | xen-c6913b5aaf48107f7bb9fb4e01c7f74c863b3713.tar.gz xen-c6913b5aaf48107f7bb9fb4e01c7f74c863b3713.tar.bz2 xen-c6913b5aaf48107f7bb9fb4e01c7f74c863b3713.zip |
blktap2: Cleanup vdi stacking code.
Removes some rough edges, memory leakage (?), fixes __list_splice, ...
Signed-off-by: Jake Wires <jake.wires@citrix.com>
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Diffstat (limited to 'tools/blktap2')
-rw-r--r-- | tools/blktap2/drivers/tapdisk-interface.c | 11 | ||||
-rw-r--r-- | tools/blktap2/drivers/tapdisk-interface.h | 1 | ||||
-rw-r--r-- | tools/blktap2/drivers/tapdisk-vbd.c | 199 | ||||
-rw-r--r-- | tools/blktap2/drivers/tapdisk-vbd.h | 3 | ||||
-rw-r--r-- | tools/blktap2/include/list.h | 19 |
5 files changed, 126 insertions, 107 deletions
diff --git a/tools/blktap2/drivers/tapdisk-interface.c b/tools/blktap2/drivers/tapdisk-interface.c index fedb17a3aa..2e518833db 100644 --- a/tools/blktap2/drivers/tapdisk-interface.c +++ b/tools/blktap2/drivers/tapdisk-interface.c @@ -59,7 +59,7 @@ td_load(td_image_t *image) } int -td_open(td_image_t *image) +__td_open(td_image_t *image, td_disk_info_t *info) { int err; td_driver_t *driver; @@ -72,6 +72,9 @@ td_open(td_image_t *image) image->storage); if (!driver) return -ENOMEM; + + if (info) /* pre-seed driver->info for virtual drivers */ + driver->info = *info; } if (!td_flag_test(driver->state, TD_DRIVER_OPEN)) { @@ -95,6 +98,12 @@ td_open(td_image_t *image) } int +td_open(td_image_t *image) +{ + return __td_open(image, NULL); +} + +int td_close(td_image_t *image) { td_driver_t *driver; diff --git a/tools/blktap2/drivers/tapdisk-interface.h b/tools/blktap2/drivers/tapdisk-interface.h index 1e48e5811a..adc4376910 100644 --- a/tools/blktap2/drivers/tapdisk-interface.h +++ b/tools/blktap2/drivers/tapdisk-interface.h @@ -32,6 +32,7 @@ #include "tapdisk-queue.h" int td_open(td_image_t *); +int __td_open(td_image_t *, td_disk_info_t *); int td_load(td_image_t *); int td_close(td_image_t *); int td_get_parent_id(td_image_t *, td_disk_id_t *); diff --git a/tools/blktap2/drivers/tapdisk-vbd.c b/tools/blktap2/drivers/tapdisk-vbd.c index 65300acbf9..f39fdadb1a 100644 --- a/tools/blktap2/drivers/tapdisk-vbd.c +++ b/tools/blktap2/drivers/tapdisk-vbd.c @@ -82,6 +82,17 @@ tapdisk_vbd_initialize_vreq(td_vbd_request_t *vreq) INIT_LIST_HEAD(&vreq->next); } +void +tapdisk_vbd_free(td_vbd_t *vbd) +{ + if (vbd) { + tapdisk_vbd_free_stack(vbd); + list_del_init(&vbd->next); + free(vbd->name); + free(vbd); + } +} + int tapdisk_vbd_initialize(uint16_t uuid) { @@ -281,23 +292,21 @@ fail: return err; } -/* TODO: ugh, lets not call it parent info... */ -static struct list_head * -tapdisk_vbd_open_level(td_vbd_t *vbd, const char* params, int driver_type, td_disk_info_t *parent_info, td_flag_t flags) +static int +tapdisk_vbd_open_level(td_vbd_t *vbd, struct list_head *head, + const char *params, int driver_type, + td_disk_info_t *driver_info, td_flag_t flags) { const 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; id.name = NULL; type = driver_type; + INIT_LIST_HEAD(head); for (;;) { err = -ENOMEM; @@ -307,42 +316,13 @@ tapdisk_vbd_open_level(td_vbd_t *vbd, const char* params, int driver_type, td_di free(id.name); if (!image) - return NULL; - + goto out; - /* 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; - } + /* this breaks if a driver modifies its info within a layer */ + err = __td_open(image, driver_info); + if (err) + goto out; /* TODO: non-sink drivers that don't care about their child * currently return EINVAL. Could return TD_PARENT_OK or @@ -351,60 +331,73 @@ tapdisk_vbd_open_level(td_vbd_t *vbd, const char* params, int driver_type, td_di err = td_get_parent_id(image, &id); if (err && (err != TD_NO_PARENT && err != -EINVAL)) { td_close(image); - return NULL; + goto out; } - if (!image->storage) - image->storage = vbd->storage; - /* add this image to the end of the list */ - list_add_tail(&image->next, images); - + list_add_tail(&image->next, head); 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; + if (err == TD_NO_PARENT || err == -EINVAL) { + err = 0; + goto out; + } name = id.name; type = id.drivertype; -#if 0 - /* catch this by validate, not here */ + flags |= (TD_OPEN_RDONLY | TD_OPEN_SHAREABLE); -#endif } - return images; + +out: + if (err) { + if (image) { + td_close(image); + tapdisk_image_free(image); + } + while (!list_empty(head)) { + image = list_entry(&head->next, td_image_t, next); + td_close(image); + tapdisk_image_free(image); + } + } + + return err; } static int __tapdisk_vbd_open_vdi(td_vbd_t *vbd, td_flag_t extra_flags) { - const char *file; - int err, type; + int err; td_flag_t flags; - td_disk_id_t id; 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; + if (list_empty(&vbd->driver_stack)) + return -ENOENT; + flags = (vbd->flags & ~TD_OPEN_SHAREABLE) | extra_flags; /* 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; + LIST_HEAD(images); - /* after each loop, append the created stack to the result stack */ - list_splice(images, &vbd->images); - free(images); + err = tapdisk_vbd_open_level(vbd, &images, + driver_info->params, + driver_info->type, + parent_info, flags); + if (err) + goto fail; + + /* after each loop, + * append the created stack to the result stack */ + list_splice(&images, &vbd->images); /* set the parent_info to the first diskinfo on the stack */ tmp = tapdisk_vbd_first_image(vbd); @@ -421,7 +414,7 @@ __tapdisk_vbd_open_vdi(td_vbd_t *vbd, td_flag_t extra_flags) err = tapdisk_vbd_add_block_cache(vbd); if (err) goto fail; - } + } err = tapdisk_vbd_validate_chain(vbd); if (err) @@ -432,16 +425,7 @@ __tapdisk_vbd_open_vdi(td_vbd_t *vbd, td_flag_t extra_flags) 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 creation? */ tapdisk_vbd_close_vdi(vbd); - return err; } @@ -453,47 +437,71 @@ tapdisk_vbd_parse_stack(td_vbd_t *vbd, const char *path) 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; - + if (err) + return err; /* tokenize params based on pipe '|' */ driver_str = strtok(params, "|"); - while(driver_str != NULL) - { + while (driver_str != NULL) { + const char *path; + int type; + /* parse driver info and add to vbd */ driver = calloc(1, sizeof(td_vbd_driver_info_t)); + if (!driver) { + PERROR("malloc"); + err = -errno; + goto out; + } 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 */ + err = tapdisk_parse_disk_type(driver_str, &path, &type); + if (err) { + free(driver); + goto out; + } + + driver->type = type; + driver->params = strdup(path); + if (!driver->params) { + err = -ENOMEM; + free(driver); + goto out; + } + + /* 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; +out: + free(params); + if (err) + tapdisk_vbd_free_stack(vbd); + + return err; +} + +void +tapdisk_vbd_free_stack(td_vbd_t *vbd) +{ + td_vbd_driver_info_t *driver; - /* 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); + 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->params); free(driver); } - - return err; } /* NOTE: driver type, etc. must be set */ -static int +int tapdisk_vbd_open_stack(td_vbd_t *vbd, uint16_t storage, td_flag_t flags) { int i, err; @@ -536,7 +544,6 @@ tapdisk_vbd_open_vdi(td_vbd_t *vbd, const char *path, vbd->flags = flags; vbd->storage = storage; - vbd->type = drivertype; for (i = 0; i < TD_VBD_EIO_RETRIES; i++) { err = __tapdisk_vbd_open_vdi(vbd, 0); diff --git a/tools/blktap2/drivers/tapdisk-vbd.h b/tools/blktap2/drivers/tapdisk-vbd.h index 3d26942582..0fea53e4f9 100644 --- a/tools/blktap2/drivers/tapdisk-vbd.h +++ b/tools/blktap2/drivers/tapdisk-vbd.h @@ -80,7 +80,7 @@ struct td_vbd_request { }; struct td_vbd_driver_info { - const char *params; + char *params; int type; struct list_head next; }; @@ -174,6 +174,7 @@ 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 *); +void tapdisk_vbd_free_stack(td_vbd_t *); int tapdisk_vbd_open_vdi(td_vbd_t *, const char *, uint16_t, uint16_t, td_flag_t); diff --git a/tools/blktap2/include/list.h b/tools/blktap2/include/list.h index 2a5f855743..cbd0050dc5 100644 --- a/tools/blktap2/include/list.h +++ b/tools/blktap2/include/list.h @@ -87,24 +87,25 @@ static inline int list_is_last(const struct list_head *list, return list->next == head; } -static inline void __list_splice(struct list_head *list, - struct list_head *head) +static inline void __list_splice(const struct list_head *list, + struct list_head *prev, + struct list_head *next) { struct list_head *first = list->next; struct list_head *last = list->prev; - struct list_head *at = head->next; - first->prev = head; - head->next = first; + first->prev = prev; + prev->next = first; - last->next = at; - at->prev = last; + last->next = next; + next->prev = last; } -static inline void list_splice(struct list_head *list, struct list_head *head) +static inline void list_splice(const struct list_head *list, + struct list_head *head) { if (!list_empty(list)) - __list_splice(list, head); + __list_splice(list, head, head->next); } #define list_entry(ptr, type, member) \ |