PR-1248: Shorten the tapdisk_vbd_open_vdi control path.

We used to have to pipe all additional driver activations through
tapdisk_vbd_open_vdi, back then mainly for blktap1 compat. This can be
done in the control callback now. Reduces the amount of flagging
involved, and simplifies passing additional parameters to drivers.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff --git a/drivers/tapdisk-control.c b/drivers/tapdisk-control.c
index 6d243a32eb9c67837545e429ad013f7c8358d360..67c5be86dc3b723d4e030cafb8c6b3394a363e24 100644 (file)
--- a/drivers/tapdisk-control.c
+++ b/drivers/tapdisk-control.c
@@ -683,13 +683,13 @@ static void
 tapdisk_control_open_image(struct tapdisk_ctl_conn *conn,
                           tapdisk_message_t *request)
 {
-       int err, type, secondary_type;
+       int err, type;
        td_disk_info_t image;
        td_vbd_t *vbd;
        td_flag_t flags;
        tapdisk_message_t response;
        struct blktap_device_info info;
-       const char *path, *secondary_path;
+       const char *path;
 
        vbd = tapdisk_server_get_vbd(request->cookie);
        if (!vbd) {
@@ -708,33 +708,14 @@ tapdisk_control_open_image(struct tapdisk_ctl_conn *conn,
        }
 
        flags = 0;
-       secondary_type = 0;
-       secondary_path = NULL;
        if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_RDONLY)
                flags |= TD_OPEN_RDONLY;
        if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_SHARED)
                flags |= TD_OPEN_SHAREABLE;
-       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_ADD_CACHE)
-               flags |= TD_OPEN_ADD_CACHE;
        if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_VHD_INDEX)
                flags |= TD_OPEN_VHD_INDEX;
-       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_LOG_DIRTY)
-               flags |= TD_OPEN_LOG_DIRTY;
-       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_ADD_LCACHE)
-               flags |= TD_OPEN_LOCAL_CACHE;
        if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_REUSE_PRT)
                flags |= TD_OPEN_REUSE_PARENT;
-       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_STANDBY)
-               flags |= TD_OPEN_STANDBY;
-       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_SECONDARY) {
-               flags |= TD_OPEN_SECONDARY;
-               secondary_type = tapdisk_disktype_parse_params(
-                               request->u.params.secondary, &secondary_path);
-               if (secondary_type < 0) {
-                       err = secondary_type;
-                       goto out;
-               }
-       }
 
        type = tapdisk_disktype_parse_params(request->u.params.path, &path);
        if (type < 0) {
@@ -744,11 +725,45 @@ tapdisk_control_open_image(struct tapdisk_ctl_conn *conn,
 
        err = tapdisk_vbd_open_vdi(vbd,
                                   type, path,
-                                  flags, request->u.params.prt_devnum,
-                                  secondary_type, secondary_path);
+                                  flags, request->u.params.prt_devnum);
        if (err)
                goto out;
 
+       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_LOG_DIRTY) {
+               err = tapdisk_vbd_add_dirty_log(vbd);
+               if (err)
+                       goto fail_close;
+       }
+
+       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_ADD_CACHE) {
+               err = tapdisk_vbd_add_block_cache(vbd);
+               if (err)
+                       goto fail_close;
+       }
+
+       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_ADD_LCACHE) {
+               err = tapdisk_vbd_add_local_cache(vbd);
+               if (err)
+                       goto fail_close;
+       }
+
+       if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_SECONDARY) {
+               const char *name;
+               int standby;
+               name     = request->u.params.secondary;
+               standby  = request->u.params.flags;
+               standby &= TAPDISK_MESSAGE_FLAG_STANDBY;
+               standby  = !!standby;
+
+               err = tapdisk_vbd_add_secondary(vbd, name, standby);
+               if (err)
+                       goto fail_close;
+       }
+
+       err = tapdisk_vbd_validate_chain(vbd);
+       if (err)
+               goto fail_close;
+
        err = tapdisk_vbd_get_disk_info(vbd, &image);
        if (err)
                goto fail_close;

diff --git a/drivers/tapdisk-diff.c b/drivers/tapdisk-diff.c
index bf23d61286608a66f83b1ba00e76367b445938fd..8256a79217bf4af64d0a882596143334b7029e83 100644 (file)
--- a/drivers/tapdisk-diff.c
+++ b/drivers/tapdisk-diff.c
@@ -572,7 +572,7 @@ tapdisk_stream_open_image(struct tapdisk_stream *s, const char *path, int type)
        tapdisk_vbd_set_callback(s->vbd, tapdisk_stream_dequeue, s);
 
        err = tapdisk_vbd_open_vdi(s->vbd, type, path,
-                                  TD_OPEN_RDONLY, -1, -1, NULL);
+                                  TD_OPEN_RDONLY, -1);
        if (err)
                goto out;
 

diff --git a/drivers/tapdisk-stream.c b/drivers/tapdisk-stream.c
index 5852eac87489ea5790c1b8c9e7a5ce8f4d7e4285..71c2238565d889333abf9b1a1ca666846c5ad660 100644 (file)
--- a/drivers/tapdisk-stream.c
+++ b/drivers/tapdisk-stream.c
@@ -352,7 +352,7 @@ tapdisk_stream_open_image(struct tapdisk_stream *s, const char *path, int type)
        tapdisk_vbd_set_callback(s->vbd, tapdisk_stream_dequeue, s);
 
        err = tapdisk_vbd_open_vdi(s->vbd, type, path,
-                                  TD_OPEN_RDONLY, -1, -1, NULL);
+                                  TD_OPEN_RDONLY, -1);
        if (err)
                goto out;
 

diff --git a/drivers/tapdisk-vbd.c b/drivers/tapdisk-vbd.c
index 1bdad262b8400a6bc5d1debc074cce2005e1966d..b50ff37bd8c6bdc3510816890f968a1390e84acf 100644 (file)
--- a/drivers/tapdisk-vbd.c
+++ b/drivers/tapdisk-vbd.c
@@ -152,7 +152,7 @@ tapdisk_vbd_set_callback(td_vbd_t *vbd, td_vbd_cb_t callback, void *argument)
        vbd->argument = argument;
 }
 
-static int
+int
 tapdisk_vbd_validate_chain(td_vbd_t *vbd)
 {
        int err;
@@ -208,7 +208,7 @@ tapdisk_vbd_close_vdi(td_vbd_t *vbd)
        td_flag_set(vbd->state, TD_VBD_CLOSED);
 }
 
-static int
+int
 tapdisk_vbd_add_block_cache(td_vbd_t *vbd)
 {
        int err;
@@ -270,7 +270,7 @@ done:
        return 0;
 }
 
-static int
+int
 tapdisk_vbd_add_local_cache(td_vbd_t *vbd)
 {
        int err;
@@ -298,7 +298,6 @@ tapdisk_vbd_add_local_cache(td_vbd_t *vbd)
        cache->driver = tapdisk_driver_allocate(cache->type,
                                                cache->name,
                                                cache->flags);
-
        if (!cache->driver) {
                err = -ENOMEM;
                goto fail;
@@ -323,22 +322,30 @@ done:
        return 0;
 }
 
-static int
-tapdisk_vbd_add_secondary(td_vbd_t *vbd)
+int
+tapdisk_vbd_add_secondary(td_vbd_t *vbd, const char *name, int standby)
 {
-       int err;
-       td_driver_t *driver;
-       td_image_t *leaf, *second;
+       td_image_t *leaf, *second = NULL;
+       const char *path;
+       int type, err;
+
+       DPRINTF("Adding secondary image: %s\n", name);
 
-       DPRINTF("Adding secondary image: %s\n", vbd->secondary_name);
+       type = tapdisk_disktype_parse_params(name, &path);
+       if (type < 0)
+               return type;
 
        leaf = tapdisk_vbd_first_image(vbd);
-       second = tapdisk_image_allocate(vbd->secondary_name,
-                                       vbd->secondary_type,
-                                       leaf->flags);
+       if (!leaf) {
+               err = -EINVAL;
+               goto fail;
+       }
 
-       if (!second)
-               return -ENOMEM;
+       second = tapdisk_image_allocate(path, type, leaf->flags);
+       if (!second) {
+               err = -ENOMEM;
+               goto fail;
+       }
 
        second->driver = tapdisk_driver_allocate(second->type,
                                                 second->name,
@@ -366,13 +373,14 @@ tapdisk_vbd_add_secondary(td_vbd_t *vbd)
        goto done;
 
 fail:
-       tapdisk_image_free(second);
+       if (second)
+               tapdisk_image_free(second);
        return err;
 
 done:
        vbd->secondary = second;
        leaf->flags |= TD_IGNORE_ENOSPC;
-       if (td_flag_test(vbd->flags, TD_OPEN_STANDBY)) {
+       if (standby) {
                DPRINTF("In standby mode\n");
                vbd->secondary_mode = TD_VBD_SECONDARY_STANDBY;
        } else {
@@ -380,7 +388,7 @@ done:
                vbd->secondary_mode = TD_VBD_SECONDARY_MIRROR;
                /* we actually need this image to also be part of the chain, 
                 * since it may already contain data */
-               list_add(&vbd->secondary->next, &leaf->next);
+               list_add(&second->next, &leaf->next);
        }
 
        DPRINTF("Added secondary image\n");
@@ -447,7 +455,7 @@ fail:
        return err;
 }
 
-static int
+int
 tapdisk_vbd_add_dirty_log(td_vbd_t *vbd)
 {
        int err;
@@ -559,34 +567,6 @@ __tapdisk_vbd_open_vdi(td_vbd_t *vbd, td_flag_t extra_flags)
                }
        }
 
-       if (td_flag_test(vbd->flags, TD_OPEN_LOG_DIRTY)) {
-               err = tapdisk_vbd_add_dirty_log(vbd);
-               if (err)
-                       goto fail;
-       }
-
-       if (td_flag_test(vbd->flags, TD_OPEN_ADD_CACHE)) {
-               err = tapdisk_vbd_add_block_cache(vbd);
-               if (err)
-                       goto fail;
-       }               
-
-       if (td_flag_test(vbd->flags, TD_OPEN_LOCAL_CACHE)) {
-               err = tapdisk_vbd_add_local_cache(vbd);
-               if (err)
-                       goto fail;
-       }               
-
-       err = tapdisk_vbd_validate_chain(vbd);
-       if (err)
-               goto fail;
-
-       if (td_flag_test(vbd->flags, TD_OPEN_SECONDARY)) {
-               err = tapdisk_vbd_add_secondary(vbd);
-               if (err)
-                       goto fail;
-       }
-
        td_flag_clear(vbd->state, TD_VBD_CLOSED);
 
        return 0;
@@ -602,8 +582,7 @@ fail:
 
 int
 tapdisk_vbd_open_vdi(td_vbd_t *vbd, int type, const char *path,
-                    td_flag_t flags, int prt_devnum,
-                    int secondary_type, const char *secondary_name)
+                    td_flag_t flags, int prt_devnum)
 {
        const disk_info_t *info;
        int i, err;
@@ -622,13 +601,6 @@ tapdisk_vbd_open_vdi(td_vbd_t *vbd, int type, const char *path,
        if (flags & TD_OPEN_REUSE_PARENT)
                vbd->parent_devnum = prt_devnum;
 
-       if (flags & TD_OPEN_SECONDARY) {
-               vbd->secondary_type = secondary_type;
-               err = tapdisk_namedup(&vbd->secondary_name, secondary_name);
-               if (err)
-                       return err;
-       }
-
        vbd->flags   = flags;
        vbd->type    = type;
 
@@ -764,7 +736,7 @@ tapdisk_vbd_open(td_vbd_t *vbd, int type, const char *path,
 {
        int err;
 
-       err = tapdisk_vbd_open_vdi(vbd, type, path, flags, -1, -1, NULL);
+       err = tapdisk_vbd_open_vdi(vbd, type, path, flags, -1);
        if (err)
                goto out;
 

diff --git a/drivers/tapdisk-vbd.h b/drivers/tapdisk-vbd.h
index 3559ec97cdbaf9b7879116ce2c8afe91882e6241..54ce0945f1800ff474439659044335fd8c230657 100644 (file)
--- a/drivers/tapdisk-vbd.h
+++ b/drivers/tapdisk-vbd.h
@@ -95,8 +95,6 @@ struct td_vbd_handle {
        struct list_head            images;
 
        int                         parent_devnum;
-       char                       *secondary_name;
-       int                         secondary_type;
        td_image_t                 *secondary;
        uint8_t                     secondary_mode;
 
@@ -191,8 +189,7 @@ int tapdisk_vbd_open(td_vbd_t *, int, const char *,
                     int, const char *, td_flag_t);
 int tapdisk_vbd_close(td_vbd_t *);
 
-int tapdisk_vbd_open_vdi(td_vbd_t *, int, const char *, td_flag_t,
-                        int, int, const char *);
+int tapdisk_vbd_open_vdi(td_vbd_t *, int, const char *, td_flag_t, int);
 void tapdisk_vbd_close_vdi(td_vbd_t *);
 
 int tapdisk_vbd_attach(td_vbd_t *, const char *, int);
@@ -214,4 +211,10 @@ void tapdisk_vbd_check_progress(td_vbd_t *);
 void tapdisk_vbd_debug(td_vbd_t *);
 void tapdisk_vbd_stats(td_vbd_t *, td_stats_t *);
 
+int tapdisk_vbd_add_dirty_log(td_vbd_t *);
+int tapdisk_vbd_add_block_cache(td_vbd_t *);
+int tapdisk_vbd_add_local_cache(td_vbd_t *);
+int tapdisk_vbd_add_secondary(td_vbd_t *, const char *, int);
+int tapdisk_vbd_validate_chain(td_vbd_t *);
+
 #endif

diff --git a/drivers/tapdisk.h b/drivers/tapdisk.h
index dd98c4e22507f57fec5c69ce4041eecd8d6cc71f..25e94b2f32e2a7eb8c843c0239f05864dc35d0af 100644 (file)
--- a/drivers/tapdisk.h
+++ b/drivers/tapdisk.h
@@ -85,13 +85,8 @@
 #define TD_OPEN_RDONLY               0x00004
 #define TD_OPEN_STRICT               0x00008
 #define TD_OPEN_SHAREABLE            0x00010
-#define TD_OPEN_ADD_CACHE            0x00020
 #define TD_OPEN_VHD_INDEX            0x00040
-#define TD_OPEN_LOG_DIRTY            0x00080
-#define TD_OPEN_LOCAL_CACHE          0x00100
 #define TD_OPEN_REUSE_PARENT         0x00200
-#define TD_OPEN_SECONDARY            0x00400
-#define TD_OPEN_STANDBY              0x00800
 #define TD_IGNORE_ENOSPC             0x01000
 
 #define TD_CREATE_SPARSE             0x00001