* [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support
@ 2017-01-25 17:42 Jeff Cody
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 1/7] iscsi: Split URL into individual options Jeff Cody
` (8 more replies)
0 siblings, 9 replies; 22+ messages in thread
From: Jeff Cody @ 2017-01-25 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
This adds blockdev-add support to the iscsi block driver.
Picked this series up from Kevin. I've tested it on my local iscsi setup.
There are only a few minor changes:
* In patch 2, fixed the segfault pointed out by Daniel
* In patch 6, placed the ':' after the command header as now required
* New patch 7, to fix some out of date documentation in the qapi schema
Jeff Cody (1):
QAPI: Fix blockdev-add example documentation
Kevin Wolf (6):
iscsi: Split URL into individual options
iscsi: Handle -iscsi user/password in bdrv_parse_filename()
iscsi: Add initiator-name option
iscsi: Add header-digest option
iscsi: Add timeout option
iscsi: Add blockdev-add support
block/iscsi.c | 349 +++++++++++++++++++++++++++++++--------------------
qapi/block-core.json | 92 +++++++++++---
2 files changed, 288 insertions(+), 153 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 1/7] iscsi: Split URL into individual options
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
@ 2017-01-25 17:42 ` Jeff Cody
2017-02-07 10:06 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename() Jeff Cody
` (7 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Jeff Cody @ 2017-01-25 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
From: Kevin Wolf <kwolf@redhat.com>
This introduces a .bdrv_parse_filename handler for iscsi which parses an
URL if given and translates it to individual options.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/iscsi.c | 189 ++++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 136 insertions(+), 53 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 6aeeb9e..97cff30 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1470,20 +1470,6 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
}
}
-/* TODO Convert to fine grained options */
-static QemuOptsList runtime_opts = {
- .name = "iscsi",
- .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
- .desc = {
- {
- .name = "filename",
- .type = QEMU_OPT_STRING,
- .help = "URL to the iscsi image",
- },
- { /* end of list */ }
- },
-};
-
static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun,
int evpd, int pc, void **inq, Error **errp)
{
@@ -1605,20 +1591,98 @@ out:
* We support iscsi url's on the form
* iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
*/
+static void iscsi_parse_filename(const char *filename, QDict *options,
+ Error **errp)
+{
+ struct iscsi_url *iscsi_url;
+ const char *transport_name;
+ char *lun_str;
+
+ iscsi_url = iscsi_parse_full_url(NULL, filename);
+ if (iscsi_url == NULL) {
+ error_setg(errp, "Failed to parse URL : %s", filename);
+ return;
+ }
+
+#if LIBISCSI_API_VERSION >= (20160603)
+ switch (iscsi_url->transport) {
+ case TCP_TRANSPORT: transport_name = "tcp"; break;
+ case ISER_TRANSPORT: transport_name = "iser"; break;
+ default:
+ error_setg(errp, "Unknown transport type (%d)",
+ iscsi_url->transport);
+ return;
+ }
+#else
+ transport_name = "tcp";
+#endif
+
+ qdict_set_default_str(options, "transport", transport_name);
+ qdict_set_default_str(options, "portal", iscsi_url->portal);
+ qdict_set_default_str(options, "target", iscsi_url->target);
+
+ lun_str = g_strdup_printf("%d", iscsi_url->lun);
+ qdict_set_default_str(options, "lun", lun_str);
+ g_free(lun_str);
+
+ if (iscsi_url->user[0] != '\0') {
+ qdict_set_default_str(options, "user", iscsi_url->user);
+ qdict_set_default_str(options, "password", iscsi_url->passwd);
+ }
+
+ iscsi_destroy_url(iscsi_url);
+}
+
+/* TODO Add -iscsi options */
+static QemuOptsList runtime_opts = {
+ .name = "iscsi",
+ .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
+ .desc = {
+ {
+ .name = "transport",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "portal",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "target",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "user",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "password",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "lun",
+ .type = QEMU_OPT_NUMBER,
+ },
+ { /* end of list */ }
+ },
+};
+
static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
struct iscsi_context *iscsi = NULL;
- struct iscsi_url *iscsi_url = NULL;
struct scsi_task *task = NULL;
struct scsi_inquiry_standard *inq = NULL;
struct scsi_inquiry_supported_pages *inq_vpd;
char *initiator_name = NULL;
QemuOpts *opts;
Error *local_err = NULL;
- const char *filename;
- int i, ret = 0, timeout = 0;
+ const char *transport_name, *portal, *target;
+ const char *user, *password;
+#if LIBISCSI_API_VERSION >= (20160603)
+ enum iscsi_transport_type transport;
+#endif
+ int i, ret = 0, timeout = 0, lun;
opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
@@ -1628,18 +1692,41 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
- filename = qemu_opt_get(opts, "filename");
+ transport_name = qemu_opt_get(opts, "transport");
+ portal = qemu_opt_get(opts, "portal");
+ target = qemu_opt_get(opts, "target");
+ user = qemu_opt_get(opts, "user");
+ password = qemu_opt_get(opts, "password");
+ lun = qemu_opt_get_number(opts, "lun", 0);
- iscsi_url = iscsi_parse_full_url(iscsi, filename);
- if (iscsi_url == NULL) {
- error_setg(errp, "Failed to parse URL : %s", filename);
+ if (!transport_name || !portal || !target) {
+ error_setg(errp, "Need all of transport, portal and target options");
+ ret = -EINVAL;
+ goto out;
+ }
+ if (user && !password) {
+ error_setg(errp, "If a user name is given, a password is required");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!strcmp(transport_name, "tcp")) {
+#if LIBISCSI_API_VERSION >= (20160603)
+ transport = TCP_TRANSPORT;
+ } else if (!strcmp(transport_name, "iser")) {
+ transport = ISER_TRANSPORT;
+#else
+ /* TCP is what older libiscsi versions always use */
+#endif
+ } else {
+ error_setg(errp, "Unknown transport: %s", transport_name);
ret = -EINVAL;
goto out;
}
memset(iscsilun, 0, sizeof(IscsiLun));
- initiator_name = parse_initiator_name(iscsi_url->target);
+ initiator_name = parse_initiator_name(target);
iscsi = iscsi_create_context(initiator_name);
if (iscsi == NULL) {
@@ -1648,21 +1735,20 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
#if LIBISCSI_API_VERSION >= (20160603)
- if (iscsi_init_transport(iscsi, iscsi_url->transport)) {
+ if (iscsi_init_transport(iscsi, transport)) {
error_setg(errp, ("Error initializing transport."));
ret = -EINVAL;
goto out;
}
#endif
- if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
+ if (iscsi_set_targetname(iscsi, target)) {
error_setg(errp, "iSCSI: Failed to set target name.");
ret = -EINVAL;
goto out;
}
- if (iscsi_url->user[0] != '\0') {
- ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user,
- iscsi_url->passwd);
+ if (user) {
+ ret = iscsi_set_initiator_username_pwd(iscsi, user, password);
if (ret != 0) {
error_setg(errp, "Failed to set initiator username and password");
ret = -EINVAL;
@@ -1671,7 +1757,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
}
/* check if we got CHAP username/password via the options */
- parse_chap(iscsi, iscsi_url->target, &local_err);
+ parse_chap(iscsi, target, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
ret = -EINVAL;
@@ -1687,7 +1773,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
/* check if we got HEADER_DIGEST via the options */
- parse_header_digest(iscsi, iscsi_url->target, &local_err);
+ parse_header_digest(iscsi, target, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
ret = -EINVAL;
@@ -1695,7 +1781,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
}
/* timeout handling is broken in libiscsi before 1.15.0 */
- timeout = parse_timeout(iscsi_url->target);
+ timeout = parse_timeout(target);
#if LIBISCSI_API_VERSION >= 20150621
iscsi_set_timeout(iscsi, timeout);
#else
@@ -1704,7 +1790,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
}
#endif
- if (iscsi_full_connect_sync(iscsi, iscsi_url->portal, iscsi_url->lun) != 0) {
+ if (iscsi_full_connect_sync(iscsi, portal, lun) != 0) {
error_setg(errp, "iSCSI: Failed to connect to LUN : %s",
iscsi_get_error(iscsi));
ret = -EINVAL;
@@ -1713,7 +1799,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
iscsilun->iscsi = iscsi;
iscsilun->aio_context = bdrv_get_aio_context(bs);
- iscsilun->lun = iscsi_url->lun;
+ iscsilun->lun = lun;
iscsilun->has_write_same = true;
task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 0, 0,
@@ -1816,9 +1902,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
out:
qemu_opts_del(opts);
g_free(initiator_name);
- if (iscsi_url != NULL) {
- iscsi_destroy_url(iscsi_url);
- }
if (task != NULL) {
scsi_free_scsi_task(task);
}
@@ -2027,15 +2110,15 @@ static BlockDriver bdrv_iscsi = {
.format_name = "iscsi",
.protocol_name = "iscsi",
- .instance_size = sizeof(IscsiLun),
- .bdrv_needs_filename = true,
- .bdrv_file_open = iscsi_open,
- .bdrv_close = iscsi_close,
- .bdrv_create = iscsi_create,
- .create_opts = &iscsi_create_opts,
- .bdrv_reopen_prepare = iscsi_reopen_prepare,
- .bdrv_reopen_commit = iscsi_reopen_commit,
- .bdrv_invalidate_cache = iscsi_invalidate_cache,
+ .instance_size = sizeof(IscsiLun),
+ .bdrv_parse_filename = iscsi_parse_filename,
+ .bdrv_file_open = iscsi_open,
+ .bdrv_close = iscsi_close,
+ .bdrv_create = iscsi_create,
+ .create_opts = &iscsi_create_opts,
+ .bdrv_reopen_prepare = iscsi_reopen_prepare,
+ .bdrv_reopen_commit = iscsi_reopen_commit,
+ .bdrv_invalidate_cache = iscsi_invalidate_cache,
.bdrv_getlength = iscsi_getlength,
.bdrv_get_info = iscsi_get_info,
@@ -2062,15 +2145,15 @@ static BlockDriver bdrv_iser = {
.format_name = "iser",
.protocol_name = "iser",
- .instance_size = sizeof(IscsiLun),
- .bdrv_needs_filename = true,
- .bdrv_file_open = iscsi_open,
- .bdrv_close = iscsi_close,
- .bdrv_create = iscsi_create,
- .create_opts = &iscsi_create_opts,
- .bdrv_reopen_prepare = iscsi_reopen_prepare,
- .bdrv_reopen_commit = iscsi_reopen_commit,
- .bdrv_invalidate_cache = iscsi_invalidate_cache,
+ .instance_size = sizeof(IscsiLun),
+ .bdrv_parse_filename = iscsi_parse_filename,
+ .bdrv_file_open = iscsi_open,
+ .bdrv_close = iscsi_close,
+ .bdrv_create = iscsi_create,
+ .create_opts = &iscsi_create_opts,
+ .bdrv_reopen_prepare = iscsi_reopen_prepare,
+ .bdrv_reopen_commit = iscsi_reopen_commit,
+ .bdrv_invalidate_cache = iscsi_invalidate_cache,
.bdrv_getlength = iscsi_getlength,
.bdrv_get_info = iscsi_get_info,
--
2.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename()
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 1/7] iscsi: Split URL into individual options Jeff Cody
@ 2017-01-25 17:42 ` Jeff Cody
2017-02-07 10:13 ` Fam Zheng
2017-02-17 14:10 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 3/7] iscsi: Add initiator-name option Jeff Cody
` (6 subsequent siblings)
8 siblings, 2 replies; 22+ messages in thread
From: Jeff Cody @ 2017-01-25 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
From: Kevin Wolf <kwolf@redhat.com>
This splits the logic in the old parse_chap() function into a part that
parses the -iscsi options into the new driver-specific options, and
another part that actually applies those options (called apply_chap()
now).
Note that this means that username and password specified with -iscsi
only take effect when a URL is provided. This is intentional, -iscsi is
a legacy interface only supported for compatibility, new users should
use the proper driver-specific options.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/iscsi.c | 78 +++++++++++++++++++++++++++++++++--------------------------
1 file changed, 44 insertions(+), 34 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 97cff30..fc91d0f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1236,29 +1236,14 @@ retry:
return 0;
}
-static void parse_chap(struct iscsi_context *iscsi, const char *target,
+static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
Error **errp)
{
- QemuOptsList *list;
- QemuOpts *opts;
const char *user = NULL;
const char *password = NULL;
const char *secretid;
char *secret = NULL;
- list = qemu_find_opts("iscsi");
- if (!list) {
- return;
- }
-
- opts = qemu_opts_find(list, target);
- if (opts == NULL) {
- opts = QTAILQ_FIRST(&list->head);
- if (!opts) {
- return;
- }
- }
-
user = qemu_opt_get(opts, "user");
if (!user) {
return;
@@ -1587,6 +1572,41 @@ out:
}
}
+static void iscsi_parse_iscsi_option(const char *target, QDict *options)
+{
+ QemuOptsList *list;
+ QemuOpts *opts;
+ const char *user, *password, *password_secret;
+
+ list = qemu_find_opts("iscsi");
+ if (!list) {
+ return;
+ }
+
+ opts = qemu_opts_find(list, target);
+ if (opts == NULL) {
+ opts = QTAILQ_FIRST(&list->head);
+ if (!opts) {
+ return;
+ }
+ }
+
+ user = qemu_opt_get(opts, "user");
+ if (user) {
+ qdict_set_default_str(options, "user", user);
+ }
+
+ password = qemu_opt_get(opts, "password");
+ if (password) {
+ qdict_set_default_str(options, "password", password);
+ }
+
+ password_secret = qemu_opt_get(opts, "password-secret");
+ if (password_secret) {
+ qdict_set_default_str(options, "password-secret", password_secret);
+ }
+}
+
/*
* We support iscsi url's on the form
* iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
@@ -1625,6 +1645,9 @@ static void iscsi_parse_filename(const char *filename, QDict *options,
qdict_set_default_str(options, "lun", lun_str);
g_free(lun_str);
+ /* User/password from -iscsi take precedence over those from the URL */
+ iscsi_parse_iscsi_option(iscsi_url->target, options);
+
if (iscsi_url->user[0] != '\0') {
qdict_set_default_str(options, "user", iscsi_url->user);
qdict_set_default_str(options, "password", iscsi_url->passwd);
@@ -1659,6 +1682,10 @@ static QemuOptsList runtime_opts = {
.type = QEMU_OPT_STRING,
},
{
+ .name = "password-secret",
+ .type = QEMU_OPT_STRING,
+ },
+ {
.name = "lun",
.type = QEMU_OPT_NUMBER,
},
@@ -1678,7 +1705,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
QemuOpts *opts;
Error *local_err = NULL;
const char *transport_name, *portal, *target;
- const char *user, *password;
#if LIBISCSI_API_VERSION >= (20160603)
enum iscsi_transport_type transport;
#endif
@@ -1695,8 +1721,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
transport_name = qemu_opt_get(opts, "transport");
portal = qemu_opt_get(opts, "portal");
target = qemu_opt_get(opts, "target");
- user = qemu_opt_get(opts, "user");
- password = qemu_opt_get(opts, "password");
lun = qemu_opt_get_number(opts, "lun", 0);
if (!transport_name || !portal || !target) {
@@ -1704,11 +1728,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
ret = -EINVAL;
goto out;
}
- if (user && !password) {
- error_setg(errp, "If a user name is given, a password is required");
- ret = -EINVAL;
- goto out;
- }
if (!strcmp(transport_name, "tcp")) {
#if LIBISCSI_API_VERSION >= (20160603)
@@ -1747,17 +1766,8 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
- if (user) {
- ret = iscsi_set_initiator_username_pwd(iscsi, user, password);
- if (ret != 0) {
- error_setg(errp, "Failed to set initiator username and password");
- ret = -EINVAL;
- goto out;
- }
- }
-
/* check if we got CHAP username/password via the options */
- parse_chap(iscsi, target, &local_err);
+ apply_chap(iscsi, opts, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
ret = -EINVAL;
--
2.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 3/7] iscsi: Add initiator-name option
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 1/7] iscsi: Split URL into individual options Jeff Cody
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename() Jeff Cody
@ 2017-01-25 17:42 ` Jeff Cody
2017-02-07 10:16 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 4/7] iscsi: Add header-digest option Jeff Cody
` (5 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Jeff Cody @ 2017-01-25 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
From: Kevin Wolf <kwolf@redhat.com>
This was previously only available with -iscsi. Again, after this patch,
the -iscsi option only takes effect if an URL is given. New users are
supposed to use the new driver-specific option.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/iscsi.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index fc91d0f..3401b7e 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1312,26 +1312,15 @@ static void parse_header_digest(struct iscsi_context *iscsi, const char *target,
}
}
-static char *parse_initiator_name(const char *target)
+static char *get_initiator_name(QemuOpts *opts)
{
- QemuOptsList *list;
- QemuOpts *opts;
const char *name;
char *iscsi_name;
UuidInfo *uuid_info;
- list = qemu_find_opts("iscsi");
- if (list) {
- opts = qemu_opts_find(list, target);
- if (!opts) {
- opts = QTAILQ_FIRST(&list->head);
- }
- if (opts) {
- name = qemu_opt_get(opts, "initiator-name");
- if (name) {
- return g_strdup(name);
- }
- }
+ name = qemu_opt_get(opts, "initiator-name");
+ if (name) {
+ return g_strdup(name);
}
uuid_info = qmp_query_uuid(NULL);
@@ -1576,7 +1565,7 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
{
QemuOptsList *list;
QemuOpts *opts;
- const char *user, *password, *password_secret;
+ const char *user, *password, *password_secret, *initiator_name;
list = qemu_find_opts("iscsi");
if (!list) {
@@ -1605,6 +1594,11 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
if (password_secret) {
qdict_set_default_str(options, "password-secret", password_secret);
}
+
+ initiator_name = qemu_opt_get(opts, "initiator-name");
+ if (initiator_name) {
+ qdict_set_default_str(options, "initiator-name", initiator_name);
+ }
}
/*
@@ -1689,6 +1683,10 @@ static QemuOptsList runtime_opts = {
.name = "lun",
.type = QEMU_OPT_NUMBER,
},
+ {
+ .name = "initiator-name",
+ .type = QEMU_OPT_STRING,
+ },
{ /* end of list */ }
},
};
@@ -1745,7 +1743,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
memset(iscsilun, 0, sizeof(IscsiLun));
- initiator_name = parse_initiator_name(target);
+ initiator_name = get_initiator_name(opts);
iscsi = iscsi_create_context(initiator_name);
if (iscsi == NULL) {
--
2.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 4/7] iscsi: Add header-digest option
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
` (2 preceding siblings ...)
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 3/7] iscsi: Add initiator-name option Jeff Cody
@ 2017-01-25 17:42 ` Jeff Cody
2017-02-07 10:18 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 5/7] iscsi: Add timeout option Jeff Cody
` (4 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Jeff Cody @ 2017-01-25 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
From: Kevin Wolf <kwolf@redhat.com>
This was previously only available with -iscsi. Again, after this patch,
the -iscsi option only takes effect if an URL is given. New users are
supposed to use the new driver-specific option.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/iscsi.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 3401b7e..a989b52 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1274,32 +1274,15 @@ static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
g_free(secret);
}
-static void parse_header_digest(struct iscsi_context *iscsi, const char *target,
+static void apply_header_digest(struct iscsi_context *iscsi, QemuOpts *opts,
Error **errp)
{
- QemuOptsList *list;
- QemuOpts *opts;
const char *digest = NULL;
- list = qemu_find_opts("iscsi");
- if (!list) {
- return;
- }
-
- opts = qemu_opts_find(list, target);
- if (opts == NULL) {
- opts = QTAILQ_FIRST(&list->head);
- if (!opts) {
- return;
- }
- }
-
digest = qemu_opt_get(opts, "header-digest");
if (!digest) {
- return;
- }
-
- if (!strcmp(digest, "CRC32C")) {
+ iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
+ } else if (!strcmp(digest, "CRC32C")) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
} else if (!strcmp(digest, "NONE")) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
@@ -1565,7 +1548,8 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
{
QemuOptsList *list;
QemuOpts *opts;
- const char *user, *password, *password_secret, *initiator_name;
+ const char *user, *password, *password_secret, *initiator_name,
+ *header_digest;
list = qemu_find_opts("iscsi");
if (!list) {
@@ -1599,6 +1583,11 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
if (initiator_name) {
qdict_set_default_str(options, "initiator-name", initiator_name);
}
+
+ header_digest = qemu_opt_get(opts, "header-digest");
+ if (header_digest) {
+ qdict_set_default_str(options, "header-digest", header_digest);
+ }
}
/*
@@ -1687,6 +1676,10 @@ static QemuOptsList runtime_opts = {
.name = "initiator-name",
.type = QEMU_OPT_STRING,
},
+ {
+ .name = "header-digest",
+ .type = QEMU_OPT_STRING,
+ },
{ /* end of list */ }
},
};
@@ -1778,10 +1771,8 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
- iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
-
/* check if we got HEADER_DIGEST via the options */
- parse_header_digest(iscsi, target, &local_err);
+ apply_header_digest(iscsi, opts, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
ret = -EINVAL;
--
2.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 5/7] iscsi: Add timeout option
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
` (3 preceding siblings ...)
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 4/7] iscsi: Add header-digest option Jeff Cody
@ 2017-01-25 17:42 ` Jeff Cody
2017-02-07 10:21 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support Jeff Cody
` (3 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Jeff Cody @ 2017-01-25 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
From: Kevin Wolf <kwolf@redhat.com>
This was previously only available with -iscsi. Again, after this patch,
the -iscsi option only takes effect if an URL is given. New users are
supposed to use the new driver-specific option.
All -iscsi options have a corresponding driver-specific option for the
iscsi block driver now.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/iscsi.c | 37 +++++++++++--------------------------
1 file changed, 11 insertions(+), 26 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index a989b52..4701a27 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1318,29 +1318,6 @@ static char *get_initiator_name(QemuOpts *opts)
return iscsi_name;
}
-static int parse_timeout(const char *target)
-{
- QemuOptsList *list;
- QemuOpts *opts;
- const char *timeout;
-
- list = qemu_find_opts("iscsi");
- if (list) {
- opts = qemu_opts_find(list, target);
- if (!opts) {
- opts = QTAILQ_FIRST(&list->head);
- }
- if (opts) {
- timeout = qemu_opt_get(opts, "timeout");
- if (timeout) {
- return atoi(timeout);
- }
- }
- }
-
- return 0;
-}
-
static void iscsi_nop_timed_event(void *opaque)
{
IscsiLun *iscsilun = opaque;
@@ -1549,7 +1526,7 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
QemuOptsList *list;
QemuOpts *opts;
const char *user, *password, *password_secret, *initiator_name,
- *header_digest;
+ *header_digest, *timeout;
list = qemu_find_opts("iscsi");
if (!list) {
@@ -1588,6 +1565,11 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
if (header_digest) {
qdict_set_default_str(options, "header-digest", header_digest);
}
+
+ timeout = qemu_opt_get(opts, "timeout");
+ if (timeout) {
+ qdict_set_default_str(options, "timeout", timeout);
+ }
}
/*
@@ -1639,7 +1621,6 @@ static void iscsi_parse_filename(const char *filename, QDict *options,
iscsi_destroy_url(iscsi_url);
}
-/* TODO Add -iscsi options */
static QemuOptsList runtime_opts = {
.name = "iscsi",
.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
@@ -1680,6 +1661,10 @@ static QemuOptsList runtime_opts = {
.name = "header-digest",
.type = QEMU_OPT_STRING,
},
+ {
+ .name = "timeout",
+ .type = QEMU_OPT_NUMBER,
+ },
{ /* end of list */ }
},
};
@@ -1780,7 +1765,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
}
/* timeout handling is broken in libiscsi before 1.15.0 */
- timeout = parse_timeout(target);
+ timeout = qemu_opt_get_number(opts, "timeout", 0);
#if LIBISCSI_API_VERSION >= 20150621
iscsi_set_timeout(iscsi, timeout);
#else
--
2.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
` (4 preceding siblings ...)
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 5/7] iscsi: Add timeout option Jeff Cody
@ 2017-01-25 17:42 ` Jeff Cody
2017-02-07 10:23 ` Fam Zheng
2017-02-17 21:40 ` Eric Blake
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 7/7] QAPI: Fix blockdev-add example documentation Jeff Cody
` (2 subsequent siblings)
8 siblings, 2 replies; 22+ messages in thread
From: Jeff Cody @ 2017-01-25 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
From: Kevin Wolf <kwolf@redhat.com>
This adds blockdev-add support for iscsi devices.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/iscsi.c | 14 ++++++----
qapi/block-core.json | 74 ++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 78 insertions(+), 10 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 4701a27..65484f0 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1282,13 +1282,13 @@ static void apply_header_digest(struct iscsi_context *iscsi, QemuOpts *opts,
digest = qemu_opt_get(opts, "header-digest");
if (!digest) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
- } else if (!strcmp(digest, "CRC32C")) {
+ } else if (!strcmp(digest, "crc32c")) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
- } else if (!strcmp(digest, "NONE")) {
+ } else if (!strcmp(digest, "none")) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
- } else if (!strcmp(digest, "CRC32C-NONE")) {
+ } else if (!strcmp(digest, "crc32c-none")) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C_NONE);
- } else if (!strcmp(digest, "NONE-CRC32C")) {
+ } else if (!strcmp(digest, "none-crc32c")) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
} else {
error_setg(errp, "Invalid header-digest setting : %s", digest);
@@ -1563,7 +1563,11 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
header_digest = qemu_opt_get(opts, "header-digest");
if (header_digest) {
- qdict_set_default_str(options, "header-digest", header_digest);
+ /* -iscsi takes upper case values, but QAPI only supports lower case
+ * enum constant names, so we have to convert here. */
+ char *qapi_value = g_ascii_strdown(header_digest, -1);
+ qdict_set_default_str(options, "header-digest", qapi_value);
+ g_free(qapi_value);
}
timeout = qemu_opt_get(opts, "timeout");
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1b3e6eb..4ebb8d8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2116,10 +2116,10 @@
{ 'enum': 'BlockdevDriver',
'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
- 'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 'null-aio',
- 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
- 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc',
- 'vvfat' ] }
+ 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',
+ 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',
+ 'quorum', 'raw', 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk',
+ 'vpc', 'vvfat' ] }
##
# @BlockdevOptionsFile:
@@ -2601,6 +2601,70 @@
'*logfile': 'str' } }
##
+# @IscsiTransport:
+#
+# An enumeration of libiscsi transport types
+#
+# Since: 2.9
+##
+{ 'enum': 'IscsiTransport',
+ 'data': [ 'tcp', 'iser' ] }
+
+##
+# @IscsiHeaderDigest:
+#
+# An enumeration of header digests supported by libiscsi
+#
+# Since: 2.9
+##
+{ 'enum': 'IscsiHeaderDigest',
+ 'prefix': 'QAPI_ISCSI_HEADER_DIGEST',
+ 'data': [ 'crc32c', 'none', 'crc32c-none', 'none-crc32c' ] }
+
+##
+# @BlockdevOptionsIscsi:
+#
+# @transport The iscsi transport type
+#
+# @portal The address of the iscsi portal
+#
+# @target The target iqn name
+#
+# @lun #optional LUN to connect to. Defaults to 0.
+#
+# @user #optional User name to log in with. If omitted, no CHAP
+# authentication is performed.
+#
+# @password-secret #optional The ID of a QCryptoSecret object providing
+# the password for the login. This option is required if
+# @user is specified.
+#
+# @initiator-name #optional The iqn name we want to identify to the target
+# as. If this option is not specified, an initiator name is
+# generated automatically.
+#
+# @header-digest #optional The desired header digest. Defaults to
+# none-crc32c.
+#
+# @timeout #optional Timeout in seconds after which a request will
+# timeout. 0 means no timeout and is the default.
+#
+# Driver specific block device options for iscsi
+#
+# Since: 2.9
+##
+{ 'struct': 'BlockdevOptionsIscsi',
+ 'data': { 'transport': 'IscsiTransport',
+ 'portal': 'str',
+ 'target': 'str',
+ '*lun': 'int',
+ '*user': 'str',
+ '*password-secret': 'str',
+ '*initiator-name': 'str',
+ '*header-digest': 'IscsiHeaderDigest',
+ '*timeout': 'int' } }
+
+##
# @ReplicationMode:
#
# An enumeration of replication modes.
@@ -2786,7 +2850,7 @@
'host_device':'BlockdevOptionsFile',
'http': 'BlockdevOptionsCurl',
'https': 'BlockdevOptionsCurl',
-# TODO iscsi: Wait for structured options
+ 'iscsi': 'BlockdevOptionsIscsi',
'luks': 'BlockdevOptionsLUKS',
'nbd': 'BlockdevOptionsNbd',
'nfs': 'BlockdevOptionsNfs',
--
2.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 7/7] QAPI: Fix blockdev-add example documentation
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
` (5 preceding siblings ...)
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support Jeff Cody
@ 2017-01-25 17:42 ` Jeff Cody
2017-02-07 10:29 ` Fam Zheng
2017-01-25 17:57 ` [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support no-reply
2017-02-17 21:12 ` Jeff Cody
8 siblings, 1 reply; 22+ messages in thread
From: Jeff Cody @ 2017-01-25 17:42 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
qapi/block-core.json | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4ebb8d8..adc089f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2909,21 +2909,24 @@
# 1.
# -> { "execute": "blockdev-add",
# "arguments": {
-# "options" : { "driver": "qcow2",
-# "file": { "driver": "file",
-# "filename": "test.qcow2" } } } }
+# "driver": "qcow2",
+# "node-name": "test1",
+# "file": {
+# "driver": "file",
+# "filename": "test.qcow2"
+# }
+# }
+# }
# <- { "return": {} }
#
# 2.
# -> { "execute": "blockdev-add",
# "arguments": {
-# "options": {
# "driver": "qcow2",
# "node-name": "node0",
# "discard": "unmap",
# "cache": {
-# "direct": true,
-# "writeback": true
+# "direct": true
# },
# "file": {
# "driver": "file",
@@ -2936,7 +2939,6 @@
# "filename": "/dev/fdset/4"
# }
# }
-# }
# }
# }
#
@@ -2964,14 +2966,12 @@
#
# -> { "execute": "blockdev-add",
# "arguments": {
-# "options": {
# "driver": "qcow2",
# "node-name": "node0",
# "file": {
# "driver": "file",
# "filename": "test.qcow2"
# }
-# }
# }
# }
# <- { "return": {} }
--
2.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
` (6 preceding siblings ...)
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 7/7] QAPI: Fix blockdev-add example documentation Jeff Cody
@ 2017-01-25 17:57 ` no-reply
2017-02-17 21:12 ` Jeff Cody
8 siblings, 0 replies; 22+ messages in thread
From: no-reply @ 2017-01-25 17:57 UTC (permalink / raw)
To: jcody; +Cc: famz, qemu-devel, kwolf, pbonzini, qemu-block
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support
Message-id: cover.1485365834.git.jcody@redhat.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/cover.1485365834.git.jcody@redhat.com -> patchew/cover.1485365834.git.jcody@redhat.com
Switched to a new branch 'test'
b51085c QAPI: Fix blockdev-add example documentation
f3be4b6 iscsi: Add blockdev-add support
714b5f9 iscsi: Add timeout option
c5ee895 iscsi: Add header-digest option
8047699 iscsi: Add initiator-name option
a9e68ca iscsi: Handle -iscsi user/password in bdrv_parse_filename()
c38bdab iscsi: Split URL into individual options
=== OUTPUT BEGIN ===
Checking PATCH 1/7: iscsi: Split URL into individual options...
ERROR: switch and case should be at the same indent
#56: FILE: block/iscsi.c:1608:
+ switch (iscsi_url->transport) {
+ case TCP_TRANSPORT: transport_name = "tcp"; break;
+ case ISER_TRANSPORT: transport_name = "iser"; break;
+ default:
ERROR: trailing statements should be on next line
#57: FILE: block/iscsi.c:1609:
+ case TCP_TRANSPORT: transport_name = "tcp"; break;
ERROR: trailing statements should be on next line
#58: FILE: block/iscsi.c:1610:
+ case ISER_TRANSPORT: transport_name = "iser"; break;
total: 3 errors, 0 warnings, 289 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/7: iscsi: Handle -iscsi user/password in bdrv_parse_filename()...
Checking PATCH 3/7: iscsi: Add initiator-name option...
Checking PATCH 4/7: iscsi: Add header-digest option...
Checking PATCH 5/7: iscsi: Add timeout option...
Checking PATCH 6/7: iscsi: Add blockdev-add support...
Checking PATCH 7/7: QAPI: Fix blockdev-add example documentation...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/7] iscsi: Split URL into individual options
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 1/7] iscsi: Split URL into individual options Jeff Cody
@ 2017-02-07 10:06 ` Fam Zheng
0 siblings, 0 replies; 22+ messages in thread
From: Fam Zheng @ 2017-02-07 10:06 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Wed, 01/25 12:42, Jeff Cody wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> This introduces a .bdrv_parse_filename handler for iscsi which parses an
> URL if given and translates it to individual options.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/iscsi.c | 189 ++++++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 136 insertions(+), 53 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 6aeeb9e..97cff30 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1470,20 +1470,6 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
> }
> }
>
> -/* TODO Convert to fine grained options */
> -static QemuOptsList runtime_opts = {
> - .name = "iscsi",
> - .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
> - .desc = {
> - {
> - .name = "filename",
> - .type = QEMU_OPT_STRING,
> - .help = "URL to the iscsi image",
> - },
> - { /* end of list */ }
> - },
> -};
> -
> static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun,
> int evpd, int pc, void **inq, Error **errp)
> {
> @@ -1605,20 +1591,98 @@ out:
> * We support iscsi url's on the form
> * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> */
> +static void iscsi_parse_filename(const char *filename, QDict *options,
> + Error **errp)
> +{
> + struct iscsi_url *iscsi_url;
> + const char *transport_name;
> + char *lun_str;
> +
> + iscsi_url = iscsi_parse_full_url(NULL, filename);
> + if (iscsi_url == NULL) {
> + error_setg(errp, "Failed to parse URL : %s", filename);
> + return;
> + }
> +
> +#if LIBISCSI_API_VERSION >= (20160603)
> + switch (iscsi_url->transport) {
> + case TCP_TRANSPORT: transport_name = "tcp"; break;
> + case ISER_TRANSPORT: transport_name = "iser"; break;
> + default:
> + error_setg(errp, "Unknown transport type (%d)",
> + iscsi_url->transport);
> + return;
checkpatch.pl doesn't like how this switch block is formatted. If you fix it:
Reviewed-by: Fam Zheng <famz@redhat.com>
> + }
> +#else
> + transport_name = "tcp";
> +#endif
> +
> + qdict_set_default_str(options, "transport", transport_name);
> + qdict_set_default_str(options, "portal", iscsi_url->portal);
> + qdict_set_default_str(options, "target", iscsi_url->target);
> +
> + lun_str = g_strdup_printf("%d", iscsi_url->lun);
> + qdict_set_default_str(options, "lun", lun_str);
> + g_free(lun_str);
> +
> + if (iscsi_url->user[0] != '\0') {
> + qdict_set_default_str(options, "user", iscsi_url->user);
> + qdict_set_default_str(options, "password", iscsi_url->passwd);
> + }
> +
> + iscsi_destroy_url(iscsi_url);
> +}
> +
> +/* TODO Add -iscsi options */
> +static QemuOptsList runtime_opts = {
> + .name = "iscsi",
> + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
> + .desc = {
> + {
> + .name = "transport",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "portal",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "target",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "user",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "password",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "lun",
> + .type = QEMU_OPT_NUMBER,
> + },
> + { /* end of list */ }
> + },
> +};
> +
> static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> Error **errp)
> {
> IscsiLun *iscsilun = bs->opaque;
> struct iscsi_context *iscsi = NULL;
> - struct iscsi_url *iscsi_url = NULL;
> struct scsi_task *task = NULL;
> struct scsi_inquiry_standard *inq = NULL;
> struct scsi_inquiry_supported_pages *inq_vpd;
> char *initiator_name = NULL;
> QemuOpts *opts;
> Error *local_err = NULL;
> - const char *filename;
> - int i, ret = 0, timeout = 0;
> + const char *transport_name, *portal, *target;
> + const char *user, *password;
> +#if LIBISCSI_API_VERSION >= (20160603)
> + enum iscsi_transport_type transport;
> +#endif
> + int i, ret = 0, timeout = 0, lun;
>
> opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> qemu_opts_absorb_qdict(opts, options, &local_err);
> @@ -1628,18 +1692,41 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> goto out;
> }
>
> - filename = qemu_opt_get(opts, "filename");
> + transport_name = qemu_opt_get(opts, "transport");
> + portal = qemu_opt_get(opts, "portal");
> + target = qemu_opt_get(opts, "target");
> + user = qemu_opt_get(opts, "user");
> + password = qemu_opt_get(opts, "password");
> + lun = qemu_opt_get_number(opts, "lun", 0);
>
> - iscsi_url = iscsi_parse_full_url(iscsi, filename);
> - if (iscsi_url == NULL) {
> - error_setg(errp, "Failed to parse URL : %s", filename);
> + if (!transport_name || !portal || !target) {
> + error_setg(errp, "Need all of transport, portal and target options");
> + ret = -EINVAL;
> + goto out;
> + }
> + if (user && !password) {
> + error_setg(errp, "If a user name is given, a password is required");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (!strcmp(transport_name, "tcp")) {
> +#if LIBISCSI_API_VERSION >= (20160603)
> + transport = TCP_TRANSPORT;
> + } else if (!strcmp(transport_name, "iser")) {
> + transport = ISER_TRANSPORT;
> +#else
> + /* TCP is what older libiscsi versions always use */
> +#endif
> + } else {
> + error_setg(errp, "Unknown transport: %s", transport_name);
> ret = -EINVAL;
> goto out;
> }
>
> memset(iscsilun, 0, sizeof(IscsiLun));
>
> - initiator_name = parse_initiator_name(iscsi_url->target);
> + initiator_name = parse_initiator_name(target);
>
> iscsi = iscsi_create_context(initiator_name);
> if (iscsi == NULL) {
> @@ -1648,21 +1735,20 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> goto out;
> }
> #if LIBISCSI_API_VERSION >= (20160603)
> - if (iscsi_init_transport(iscsi, iscsi_url->transport)) {
> + if (iscsi_init_transport(iscsi, transport)) {
> error_setg(errp, ("Error initializing transport."));
> ret = -EINVAL;
> goto out;
> }
> #endif
> - if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
> + if (iscsi_set_targetname(iscsi, target)) {
> error_setg(errp, "iSCSI: Failed to set target name.");
> ret = -EINVAL;
> goto out;
> }
>
> - if (iscsi_url->user[0] != '\0') {
> - ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user,
> - iscsi_url->passwd);
> + if (user) {
> + ret = iscsi_set_initiator_username_pwd(iscsi, user, password);
> if (ret != 0) {
> error_setg(errp, "Failed to set initiator username and password");
> ret = -EINVAL;
> @@ -1671,7 +1757,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> }
>
> /* check if we got CHAP username/password via the options */
> - parse_chap(iscsi, iscsi_url->target, &local_err);
> + parse_chap(iscsi, target, &local_err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> ret = -EINVAL;
> @@ -1687,7 +1773,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
>
> /* check if we got HEADER_DIGEST via the options */
> - parse_header_digest(iscsi, iscsi_url->target, &local_err);
> + parse_header_digest(iscsi, target, &local_err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> ret = -EINVAL;
> @@ -1695,7 +1781,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> }
>
> /* timeout handling is broken in libiscsi before 1.15.0 */
> - timeout = parse_timeout(iscsi_url->target);
> + timeout = parse_timeout(target);
> #if LIBISCSI_API_VERSION >= 20150621
> iscsi_set_timeout(iscsi, timeout);
> #else
> @@ -1704,7 +1790,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> }
> #endif
>
> - if (iscsi_full_connect_sync(iscsi, iscsi_url->portal, iscsi_url->lun) != 0) {
> + if (iscsi_full_connect_sync(iscsi, portal, lun) != 0) {
> error_setg(errp, "iSCSI: Failed to connect to LUN : %s",
> iscsi_get_error(iscsi));
> ret = -EINVAL;
> @@ -1713,7 +1799,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>
> iscsilun->iscsi = iscsi;
> iscsilun->aio_context = bdrv_get_aio_context(bs);
> - iscsilun->lun = iscsi_url->lun;
> + iscsilun->lun = lun;
> iscsilun->has_write_same = true;
>
> task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 0, 0,
> @@ -1816,9 +1902,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> out:
> qemu_opts_del(opts);
> g_free(initiator_name);
> - if (iscsi_url != NULL) {
> - iscsi_destroy_url(iscsi_url);
> - }
> if (task != NULL) {
> scsi_free_scsi_task(task);
> }
> @@ -2027,15 +2110,15 @@ static BlockDriver bdrv_iscsi = {
> .format_name = "iscsi",
> .protocol_name = "iscsi",
>
> - .instance_size = sizeof(IscsiLun),
> - .bdrv_needs_filename = true,
> - .bdrv_file_open = iscsi_open,
> - .bdrv_close = iscsi_close,
> - .bdrv_create = iscsi_create,
> - .create_opts = &iscsi_create_opts,
> - .bdrv_reopen_prepare = iscsi_reopen_prepare,
> - .bdrv_reopen_commit = iscsi_reopen_commit,
> - .bdrv_invalidate_cache = iscsi_invalidate_cache,
> + .instance_size = sizeof(IscsiLun),
> + .bdrv_parse_filename = iscsi_parse_filename,
> + .bdrv_file_open = iscsi_open,
> + .bdrv_close = iscsi_close,
> + .bdrv_create = iscsi_create,
> + .create_opts = &iscsi_create_opts,
> + .bdrv_reopen_prepare = iscsi_reopen_prepare,
> + .bdrv_reopen_commit = iscsi_reopen_commit,
> + .bdrv_invalidate_cache = iscsi_invalidate_cache,
>
> .bdrv_getlength = iscsi_getlength,
> .bdrv_get_info = iscsi_get_info,
> @@ -2062,15 +2145,15 @@ static BlockDriver bdrv_iser = {
> .format_name = "iser",
> .protocol_name = "iser",
>
> - .instance_size = sizeof(IscsiLun),
> - .bdrv_needs_filename = true,
> - .bdrv_file_open = iscsi_open,
> - .bdrv_close = iscsi_close,
> - .bdrv_create = iscsi_create,
> - .create_opts = &iscsi_create_opts,
> - .bdrv_reopen_prepare = iscsi_reopen_prepare,
> - .bdrv_reopen_commit = iscsi_reopen_commit,
> - .bdrv_invalidate_cache = iscsi_invalidate_cache,
> + .instance_size = sizeof(IscsiLun),
> + .bdrv_parse_filename = iscsi_parse_filename,
> + .bdrv_file_open = iscsi_open,
> + .bdrv_close = iscsi_close,
> + .bdrv_create = iscsi_create,
> + .create_opts = &iscsi_create_opts,
> + .bdrv_reopen_prepare = iscsi_reopen_prepare,
> + .bdrv_reopen_commit = iscsi_reopen_commit,
> + .bdrv_invalidate_cache = iscsi_invalidate_cache,
>
> .bdrv_getlength = iscsi_getlength,
> .bdrv_get_info = iscsi_get_info,
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename()
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename() Jeff Cody
@ 2017-02-07 10:13 ` Fam Zheng
2017-02-17 13:26 ` Kevin Wolf
2017-02-17 14:10 ` Fam Zheng
1 sibling, 1 reply; 22+ messages in thread
From: Fam Zheng @ 2017-02-07 10:13 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Wed, 01/25 12:42, Jeff Cody wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> This splits the logic in the old parse_chap() function into a part that
> parses the -iscsi options into the new driver-specific options, and
> another part that actually applies those options (called apply_chap()
> now).
>
> Note that this means that username and password specified with -iscsi
> only take effect when a URL is provided. This is intentional, -iscsi is
> a legacy interface only supported for compatibility, new users should
> use the proper driver-specific options.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/iscsi.c | 78 +++++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 44 insertions(+), 34 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 97cff30..fc91d0f 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1236,29 +1236,14 @@ retry:
> return 0;
> }
>
> -static void parse_chap(struct iscsi_context *iscsi, const char *target,
> +static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
> Error **errp)
> {
> - QemuOptsList *list;
> - QemuOpts *opts;
> const char *user = NULL;
> const char *password = NULL;
> const char *secretid;
> char *secret = NULL;
>
> - list = qemu_find_opts("iscsi");
> - if (!list) {
> - return;
> - }
> -
> - opts = qemu_opts_find(list, target);
> - if (opts == NULL) {
> - opts = QTAILQ_FIRST(&list->head);
> - if (!opts) {
> - return;
> - }
> - }
> -
> user = qemu_opt_get(opts, "user");
> if (!user) {
> return;
> @@ -1587,6 +1572,41 @@ out:
> }
> }
>
> +static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> +{
> + QemuOptsList *list;
> + QemuOpts *opts;
> + const char *user, *password, *password_secret;
> +
> + list = qemu_find_opts("iscsi");
> + if (!list) {
> + return;
> + }
> +
> + opts = qemu_opts_find(list, target);
> + if (opts == NULL) {
> + opts = QTAILQ_FIRST(&list->head);
> + if (!opts) {
> + return;
> + }
> + }
> +
> + user = qemu_opt_get(opts, "user");
> + if (user) {
> + qdict_set_default_str(options, "user", user);
> + }
> +
> + password = qemu_opt_get(opts, "password");
> + if (password) {
> + qdict_set_default_str(options, "password", password);
> + }
> +
> + password_secret = qemu_opt_get(opts, "password-secret");
> + if (password_secret) {
> + qdict_set_default_str(options, "password-secret", password_secret);
> + }
> +}
> +
> /*
> * We support iscsi url's on the form
> * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> @@ -1625,6 +1645,9 @@ static void iscsi_parse_filename(const char *filename, QDict *options,
> qdict_set_default_str(options, "lun", lun_str);
> g_free(lun_str);
>
> + /* User/password from -iscsi take precedence over those from the URL */
> + iscsi_parse_iscsi_option(iscsi_url->target, options);
> +
Isn't more natural to let the local option take precedence over the global one?
> if (iscsi_url->user[0] != '\0') {
> qdict_set_default_str(options, "user", iscsi_url->user);
> qdict_set_default_str(options, "password", iscsi_url->passwd);
> @@ -1659,6 +1682,10 @@ static QemuOptsList runtime_opts = {
> .type = QEMU_OPT_STRING,
> },
> {
> + .name = "password-secret",
> + .type = QEMU_OPT_STRING,
> + },
> + {
I think this added password-secret is not the one looked up in
iscsi_parse_iscsi_option(), which is from -iscsi
(block/iscsi-opts.c:qemu_iscsi_opts). Is this intended? Or does it belong to a
different patch?
Fam
> .name = "lun",
> .type = QEMU_OPT_NUMBER,
> },
> @@ -1678,7 +1705,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> QemuOpts *opts;
> Error *local_err = NULL;
> const char *transport_name, *portal, *target;
> - const char *user, *password;
> #if LIBISCSI_API_VERSION >= (20160603)
> enum iscsi_transport_type transport;
> #endif
> @@ -1695,8 +1721,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> transport_name = qemu_opt_get(opts, "transport");
> portal = qemu_opt_get(opts, "portal");
> target = qemu_opt_get(opts, "target");
> - user = qemu_opt_get(opts, "user");
> - password = qemu_opt_get(opts, "password");
> lun = qemu_opt_get_number(opts, "lun", 0);
>
> if (!transport_name || !portal || !target) {
> @@ -1704,11 +1728,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> ret = -EINVAL;
> goto out;
> }
> - if (user && !password) {
> - error_setg(errp, "If a user name is given, a password is required");
> - ret = -EINVAL;
> - goto out;
> - }
>
> if (!strcmp(transport_name, "tcp")) {
> #if LIBISCSI_API_VERSION >= (20160603)
> @@ -1747,17 +1766,8 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> goto out;
> }
>
> - if (user) {
> - ret = iscsi_set_initiator_username_pwd(iscsi, user, password);
> - if (ret != 0) {
> - error_setg(errp, "Failed to set initiator username and password");
> - ret = -EINVAL;
> - goto out;
> - }
> - }
> -
> /* check if we got CHAP username/password via the options */
> - parse_chap(iscsi, target, &local_err);
> + apply_chap(iscsi, opts, &local_err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> ret = -EINVAL;
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/7] iscsi: Add initiator-name option
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 3/7] iscsi: Add initiator-name option Jeff Cody
@ 2017-02-07 10:16 ` Fam Zheng
0 siblings, 0 replies; 22+ messages in thread
From: Fam Zheng @ 2017-02-07 10:16 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Wed, 01/25 12:42, Jeff Cody wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> This was previously only available with -iscsi. Again, after this patch,
> the -iscsi option only takes effect if an URL is given. New users are
> supposed to use the new driver-specific option.
>
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/iscsi.c | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index fc91d0f..3401b7e 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1312,26 +1312,15 @@ static void parse_header_digest(struct iscsi_context *iscsi, const char *target,
> }
> }
>
> -static char *parse_initiator_name(const char *target)
> +static char *get_initiator_name(QemuOpts *opts)
> {
> - QemuOptsList *list;
> - QemuOpts *opts;
> const char *name;
> char *iscsi_name;
> UuidInfo *uuid_info;
>
> - list = qemu_find_opts("iscsi");
> - if (list) {
> - opts = qemu_opts_find(list, target);
> - if (!opts) {
> - opts = QTAILQ_FIRST(&list->head);
> - }
> - if (opts) {
> - name = qemu_opt_get(opts, "initiator-name");
> - if (name) {
> - return g_strdup(name);
> - }
> - }
> + name = qemu_opt_get(opts, "initiator-name");
> + if (name) {
> + return g_strdup(name);
> }
>
> uuid_info = qmp_query_uuid(NULL);
> @@ -1576,7 +1565,7 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> {
> QemuOptsList *list;
> QemuOpts *opts;
> - const char *user, *password, *password_secret;
> + const char *user, *password, *password_secret, *initiator_name;
>
> list = qemu_find_opts("iscsi");
> if (!list) {
> @@ -1605,6 +1594,11 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> if (password_secret) {
> qdict_set_default_str(options, "password-secret", password_secret);
> }
> +
> + initiator_name = qemu_opt_get(opts, "initiator-name");
> + if (initiator_name) {
> + qdict_set_default_str(options, "initiator-name", initiator_name);
> + }
> }
>
> /*
> @@ -1689,6 +1683,10 @@ static QemuOptsList runtime_opts = {
> .name = "lun",
> .type = QEMU_OPT_NUMBER,
> },
> + {
> + .name = "initiator-name",
> + .type = QEMU_OPT_STRING,
> + },
> { /* end of list */ }
> },
> };
> @@ -1745,7 +1743,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>
> memset(iscsilun, 0, sizeof(IscsiLun));
>
> - initiator_name = parse_initiator_name(target);
> + initiator_name = get_initiator_name(opts);
>
> iscsi = iscsi_create_context(initiator_name);
> if (iscsi == NULL) {
> --
> 2.9.3
>
>
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] iscsi: Add header-digest option
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 4/7] iscsi: Add header-digest option Jeff Cody
@ 2017-02-07 10:18 ` Fam Zheng
0 siblings, 0 replies; 22+ messages in thread
From: Fam Zheng @ 2017-02-07 10:18 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Wed, 01/25 12:42, Jeff Cody wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> This was previously only available with -iscsi. Again, after this patch,
> the -iscsi option only takes effect if an URL is given. New users are
> supposed to use the new driver-specific option.
>
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/iscsi.c | 39 +++++++++++++++------------------------
> 1 file changed, 15 insertions(+), 24 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 3401b7e..a989b52 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1274,32 +1274,15 @@ static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
> g_free(secret);
> }
>
> -static void parse_header_digest(struct iscsi_context *iscsi, const char *target,
> +static void apply_header_digest(struct iscsi_context *iscsi, QemuOpts *opts,
> Error **errp)
> {
> - QemuOptsList *list;
> - QemuOpts *opts;
> const char *digest = NULL;
>
> - list = qemu_find_opts("iscsi");
> - if (!list) {
> - return;
> - }
> -
> - opts = qemu_opts_find(list, target);
> - if (opts == NULL) {
> - opts = QTAILQ_FIRST(&list->head);
> - if (!opts) {
> - return;
> - }
> - }
> -
> digest = qemu_opt_get(opts, "header-digest");
> if (!digest) {
> - return;
> - }
> -
> - if (!strcmp(digest, "CRC32C")) {
> + iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
> + } else if (!strcmp(digest, "CRC32C")) {
> iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
> } else if (!strcmp(digest, "NONE")) {
> iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
> @@ -1565,7 +1548,8 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> {
> QemuOptsList *list;
> QemuOpts *opts;
> - const char *user, *password, *password_secret, *initiator_name;
> + const char *user, *password, *password_secret, *initiator_name,
> + *header_digest;
>
> list = qemu_find_opts("iscsi");
> if (!list) {
> @@ -1599,6 +1583,11 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> if (initiator_name) {
> qdict_set_default_str(options, "initiator-name", initiator_name);
> }
> +
> + header_digest = qemu_opt_get(opts, "header-digest");
> + if (header_digest) {
> + qdict_set_default_str(options, "header-digest", header_digest);
> + }
> }
>
> /*
> @@ -1687,6 +1676,10 @@ static QemuOptsList runtime_opts = {
> .name = "initiator-name",
> .type = QEMU_OPT_STRING,
> },
> + {
> + .name = "header-digest",
> + .type = QEMU_OPT_STRING,
> + },
> { /* end of list */ }
> },
> };
> @@ -1778,10 +1771,8 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> goto out;
> }
>
> - iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
> -
> /* check if we got HEADER_DIGEST via the options */
> - parse_header_digest(iscsi, target, &local_err);
> + apply_header_digest(iscsi, opts, &local_err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> ret = -EINVAL;
> --
> 2.9.3
>
>
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/7] iscsi: Add timeout option
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 5/7] iscsi: Add timeout option Jeff Cody
@ 2017-02-07 10:21 ` Fam Zheng
0 siblings, 0 replies; 22+ messages in thread
From: Fam Zheng @ 2017-02-07 10:21 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Wed, 01/25 12:42, Jeff Cody wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> This was previously only available with -iscsi. Again, after this patch,
> the -iscsi option only takes effect if an URL is given. New users are
> supposed to use the new driver-specific option.
>
> All -iscsi options have a corresponding driver-specific option for the
> iscsi block driver now.
>
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/iscsi.c | 37 +++++++++++--------------------------
> 1 file changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index a989b52..4701a27 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1318,29 +1318,6 @@ static char *get_initiator_name(QemuOpts *opts)
> return iscsi_name;
> }
>
> -static int parse_timeout(const char *target)
> -{
> - QemuOptsList *list;
> - QemuOpts *opts;
> - const char *timeout;
> -
> - list = qemu_find_opts("iscsi");
> - if (list) {
> - opts = qemu_opts_find(list, target);
> - if (!opts) {
> - opts = QTAILQ_FIRST(&list->head);
> - }
> - if (opts) {
> - timeout = qemu_opt_get(opts, "timeout");
> - if (timeout) {
> - return atoi(timeout);
> - }
> - }
> - }
> -
> - return 0;
> -}
> -
> static void iscsi_nop_timed_event(void *opaque)
> {
> IscsiLun *iscsilun = opaque;
> @@ -1549,7 +1526,7 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> QemuOptsList *list;
> QemuOpts *opts;
> const char *user, *password, *password_secret, *initiator_name,
> - *header_digest;
> + *header_digest, *timeout;
>
> list = qemu_find_opts("iscsi");
> if (!list) {
> @@ -1588,6 +1565,11 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> if (header_digest) {
> qdict_set_default_str(options, "header-digest", header_digest);
> }
> +
> + timeout = qemu_opt_get(opts, "timeout");
> + if (timeout) {
> + qdict_set_default_str(options, "timeout", timeout);
> + }
> }
>
> /*
> @@ -1639,7 +1621,6 @@ static void iscsi_parse_filename(const char *filename, QDict *options,
> iscsi_destroy_url(iscsi_url);
> }
>
> -/* TODO Add -iscsi options */
> static QemuOptsList runtime_opts = {
> .name = "iscsi",
> .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
> @@ -1680,6 +1661,10 @@ static QemuOptsList runtime_opts = {
> .name = "header-digest",
> .type = QEMU_OPT_STRING,
> },
> + {
> + .name = "timeout",
> + .type = QEMU_OPT_NUMBER,
> + },
> { /* end of list */ }
> },
> };
> @@ -1780,7 +1765,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> }
>
> /* timeout handling is broken in libiscsi before 1.15.0 */
> - timeout = parse_timeout(target);
> + timeout = qemu_opt_get_number(opts, "timeout", 0);
> #if LIBISCSI_API_VERSION >= 20150621
> iscsi_set_timeout(iscsi, timeout);
> #else
> --
> 2.9.3
>
>
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support Jeff Cody
@ 2017-02-07 10:23 ` Fam Zheng
2017-02-17 21:40 ` Eric Blake
1 sibling, 0 replies; 22+ messages in thread
From: Fam Zheng @ 2017-02-07 10:23 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Wed, 01/25 12:42, Jeff Cody wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> This adds blockdev-add support for iscsi devices.
>
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/iscsi.c | 14 ++++++----
> qapi/block-core.json | 74 ++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 78 insertions(+), 10 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 4701a27..65484f0 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1282,13 +1282,13 @@ static void apply_header_digest(struct iscsi_context *iscsi, QemuOpts *opts,
> digest = qemu_opt_get(opts, "header-digest");
> if (!digest) {
> iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
> - } else if (!strcmp(digest, "CRC32C")) {
> + } else if (!strcmp(digest, "crc32c")) {
> iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
> - } else if (!strcmp(digest, "NONE")) {
> + } else if (!strcmp(digest, "none")) {
> iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
> - } else if (!strcmp(digest, "CRC32C-NONE")) {
> + } else if (!strcmp(digest, "crc32c-none")) {
> iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C_NONE);
> - } else if (!strcmp(digest, "NONE-CRC32C")) {
> + } else if (!strcmp(digest, "none-crc32c")) {
> iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
> } else {
> error_setg(errp, "Invalid header-digest setting : %s", digest);
> @@ -1563,7 +1563,11 @@ static void iscsi_parse_iscsi_option(const char *target, QDict *options)
>
> header_digest = qemu_opt_get(opts, "header-digest");
> if (header_digest) {
> - qdict_set_default_str(options, "header-digest", header_digest);
> + /* -iscsi takes upper case values, but QAPI only supports lower case
> + * enum constant names, so we have to convert here. */
> + char *qapi_value = g_ascii_strdown(header_digest, -1);
> + qdict_set_default_str(options, "header-digest", qapi_value);
> + g_free(qapi_value);
> }
>
> timeout = qemu_opt_get(opts, "timeout");
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 1b3e6eb..4ebb8d8 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2116,10 +2116,10 @@
> { 'enum': 'BlockdevDriver',
> 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
> 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
> - 'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 'null-aio',
> - 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> - 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc',
> - 'vvfat' ] }
> + 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',
> + 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',
> + 'quorum', 'raw', 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk',
> + 'vpc', 'vvfat' ] }
>
> ##
> # @BlockdevOptionsFile:
> @@ -2601,6 +2601,70 @@
> '*logfile': 'str' } }
>
> ##
> +# @IscsiTransport:
> +#
> +# An enumeration of libiscsi transport types
> +#
> +# Since: 2.9
> +##
> +{ 'enum': 'IscsiTransport',
> + 'data': [ 'tcp', 'iser' ] }
> +
> +##
> +# @IscsiHeaderDigest:
> +#
> +# An enumeration of header digests supported by libiscsi
> +#
> +# Since: 2.9
> +##
> +{ 'enum': 'IscsiHeaderDigest',
> + 'prefix': 'QAPI_ISCSI_HEADER_DIGEST',
> + 'data': [ 'crc32c', 'none', 'crc32c-none', 'none-crc32c' ] }
> +
> +##
> +# @BlockdevOptionsIscsi:
> +#
> +# @transport The iscsi transport type
> +#
> +# @portal The address of the iscsi portal
> +#
> +# @target The target iqn name
> +#
> +# @lun #optional LUN to connect to. Defaults to 0.
> +#
> +# @user #optional User name to log in with. If omitted, no CHAP
> +# authentication is performed.
> +#
> +# @password-secret #optional The ID of a QCryptoSecret object providing
> +# the password for the login. This option is required if
> +# @user is specified.
> +#
> +# @initiator-name #optional The iqn name we want to identify to the target
> +# as. If this option is not specified, an initiator name is
> +# generated automatically.
> +#
> +# @header-digest #optional The desired header digest. Defaults to
> +# none-crc32c.
> +#
> +# @timeout #optional Timeout in seconds after which a request will
> +# timeout. 0 means no timeout and is the default.
> +#
> +# Driver specific block device options for iscsi
> +#
> +# Since: 2.9
> +##
> +{ 'struct': 'BlockdevOptionsIscsi',
> + 'data': { 'transport': 'IscsiTransport',
> + 'portal': 'str',
> + 'target': 'str',
> + '*lun': 'int',
> + '*user': 'str',
> + '*password-secret': 'str',
> + '*initiator-name': 'str',
> + '*header-digest': 'IscsiHeaderDigest',
> + '*timeout': 'int' } }
> +
> +##
> # @ReplicationMode:
> #
> # An enumeration of replication modes.
> @@ -2786,7 +2850,7 @@
> 'host_device':'BlockdevOptionsFile',
> 'http': 'BlockdevOptionsCurl',
> 'https': 'BlockdevOptionsCurl',
> -# TODO iscsi: Wait for structured options
> + 'iscsi': 'BlockdevOptionsIscsi',
> 'luks': 'BlockdevOptionsLUKS',
> 'nbd': 'BlockdevOptionsNbd',
> 'nfs': 'BlockdevOptionsNfs',
> --
> 2.9.3
>
>
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/7] QAPI: Fix blockdev-add example documentation
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 7/7] QAPI: Fix blockdev-add example documentation Jeff Cody
@ 2017-02-07 10:29 ` Fam Zheng
0 siblings, 0 replies; 22+ messages in thread
From: Fam Zheng @ 2017-02-07 10:29 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Wed, 01/25 12:42, Jeff Cody wrote:
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> qapi/block-core.json | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 4ebb8d8..adc089f 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2909,21 +2909,24 @@
> # 1.
> # -> { "execute": "blockdev-add",
> # "arguments": {
> -# "options" : { "driver": "qcow2",
> -# "file": { "driver": "file",
> -# "filename": "test.qcow2" } } } }
> +# "driver": "qcow2",
> +# "node-name": "test1",
> +# "file": {
> +# "driver": "file",
> +# "filename": "test.qcow2"
> +# }
> +# }
> +# }
> # <- { "return": {} }
> #
> # 2.
> # -> { "execute": "blockdev-add",
> # "arguments": {
> -# "options": {
> # "driver": "qcow2",
> # "node-name": "node0",
> # "discard": "unmap",
> # "cache": {
> -# "direct": true,
> -# "writeback": true
> +# "direct": true
> # },
> # "file": {
> # "driver": "file",
> @@ -2936,7 +2939,6 @@
> # "filename": "/dev/fdset/4"
> # }
> # }
> -# }
> # }
> # }
> #
> @@ -2964,14 +2966,12 @@
> #
> # -> { "execute": "blockdev-add",
> # "arguments": {
> -# "options": {
> # "driver": "qcow2",
> # "node-name": "node0",
> # "file": {
> # "driver": "file",
> # "filename": "test.qcow2"
> # }
> -# }
> # }
> # }
> # <- { "return": {} }
> --
> 2.9.3
>
>
The alignments are a bit odd now but the syntax looks correct. Is it worth to
reindent while we are at it?
Fam
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename()
2017-02-07 10:13 ` Fam Zheng
@ 2017-02-17 13:26 ` Kevin Wolf
2017-02-17 14:09 ` Fam Zheng
0 siblings, 1 reply; 22+ messages in thread
From: Kevin Wolf @ 2017-02-17 13:26 UTC (permalink / raw)
To: Fam Zheng; +Cc: Jeff Cody, qemu-devel, pbonzini, qemu-block
Am 07.02.2017 um 11:13 hat Fam Zheng geschrieben:
> On Wed, 01/25 12:42, Jeff Cody wrote:
> > From: Kevin Wolf <kwolf@redhat.com>
> >
> > This splits the logic in the old parse_chap() function into a part that
> > parses the -iscsi options into the new driver-specific options, and
> > another part that actually applies those options (called apply_chap()
> > now).
> >
> > Note that this means that username and password specified with -iscsi
> > only take effect when a URL is provided. This is intentional, -iscsi is
> > a legacy interface only supported for compatibility, new users should
> > use the proper driver-specific options.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Signed-off-by: Jeff Cody <jcody@redhat.com>
> > ---
> > block/iscsi.c | 78 +++++++++++++++++++++++++++++++++--------------------------
> > 1 file changed, 44 insertions(+), 34 deletions(-)
> >
> > diff --git a/block/iscsi.c b/block/iscsi.c
> > index 97cff30..fc91d0f 100644
> > --- a/block/iscsi.c
> > +++ b/block/iscsi.c
> > @@ -1236,29 +1236,14 @@ retry:
> > return 0;
> > }
> >
> > -static void parse_chap(struct iscsi_context *iscsi, const char *target,
> > +static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
> > Error **errp)
> > {
> > - QemuOptsList *list;
> > - QemuOpts *opts;
> > const char *user = NULL;
> > const char *password = NULL;
> > const char *secretid;
> > char *secret = NULL;
> >
> > - list = qemu_find_opts("iscsi");
> > - if (!list) {
> > - return;
> > - }
> > -
> > - opts = qemu_opts_find(list, target);
> > - if (opts == NULL) {
> > - opts = QTAILQ_FIRST(&list->head);
> > - if (!opts) {
> > - return;
> > - }
> > - }
> > -
> > user = qemu_opt_get(opts, "user");
> > if (!user) {
> > return;
> > @@ -1587,6 +1572,41 @@ out:
> > }
> > }
> >
> > +static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> > +{
> > + QemuOptsList *list;
> > + QemuOpts *opts;
> > + const char *user, *password, *password_secret;
> > +
> > + list = qemu_find_opts("iscsi");
> > + if (!list) {
> > + return;
> > + }
> > +
> > + opts = qemu_opts_find(list, target);
> > + if (opts == NULL) {
> > + opts = QTAILQ_FIRST(&list->head);
> > + if (!opts) {
> > + return;
> > + }
> > + }
> > +
> > + user = qemu_opt_get(opts, "user");
> > + if (user) {
> > + qdict_set_default_str(options, "user", user);
> > + }
> > +
> > + password = qemu_opt_get(opts, "password");
> > + if (password) {
> > + qdict_set_default_str(options, "password", password);
> > + }
> > +
> > + password_secret = qemu_opt_get(opts, "password-secret");
> > + if (password_secret) {
> > + qdict_set_default_str(options, "password-secret", password_secret);
> > + }
> > +}
> > +
> > /*
> > * We support iscsi url's on the form
> > * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> > @@ -1625,6 +1645,9 @@ static void iscsi_parse_filename(const char *filename, QDict *options,
> > qdict_set_default_str(options, "lun", lun_str);
> > g_free(lun_str);
> >
> > + /* User/password from -iscsi take precedence over those from the URL */
> > + iscsi_parse_iscsi_option(iscsi_url->target, options);
> > +
>
> Isn't more natural to let the local option take precedence over the global one?
One would think so, but that's how the option work today, so we can't
change it without breaking compatibility. We can only newly define the
precedence of the new driver-specific options vs. the existing ones, and
there the local driver-specific ones do take precedence.
> > if (iscsi_url->user[0] != '\0') {
> > qdict_set_default_str(options, "user", iscsi_url->user);
> > qdict_set_default_str(options, "password", iscsi_url->passwd);
> > @@ -1659,6 +1682,10 @@ static QemuOptsList runtime_opts = {
> > .type = QEMU_OPT_STRING,
> > },
> > {
> > + .name = "password-secret",
> > + .type = QEMU_OPT_STRING,
> > + },
> > + {
>
> I think this added password-secret is not the one looked up in
> iscsi_parse_iscsi_option(), which is from -iscsi
> (block/iscsi-opts.c:qemu_iscsi_opts). Is this intended? Or does it belong to a
> different patch?
It is the one that it put into the QDict by iscsi_parse_iscsi_option(),
which is supposed to be the value from -iscsi.
Why do you think it's not the one? Maybe I'm missing something.
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename()
2017-02-17 13:26 ` Kevin Wolf
@ 2017-02-17 14:09 ` Fam Zheng
0 siblings, 0 replies; 22+ messages in thread
From: Fam Zheng @ 2017-02-17 14:09 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pbonzini, Jeff Cody, qemu-devel, qemu-block
On Fri, 02/17 14:26, Kevin Wolf wrote:
> It is the one that it put into the QDict by iscsi_parse_iscsi_option(),
> which is supposed to be the value from -iscsi.
OK! This is what I was missing. :)
Fam
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename()
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename() Jeff Cody
2017-02-07 10:13 ` Fam Zheng
@ 2017-02-17 14:10 ` Fam Zheng
1 sibling, 0 replies; 22+ messages in thread
From: Fam Zheng @ 2017-02-17 14:10 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Wed, 01/25 12:42, Jeff Cody wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> This splits the logic in the old parse_chap() function into a part that
> parses the -iscsi options into the new driver-specific options, and
> another part that actually applies those options (called apply_chap()
> now).
>
> Note that this means that username and password specified with -iscsi
> only take effect when a URL is provided. This is intentional, -iscsi is
> a legacy interface only supported for compatibility, new users should
> use the proper driver-specific options.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
` (7 preceding siblings ...)
2017-01-25 17:57 ` [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support no-reply
@ 2017-02-17 21:12 ` Jeff Cody
8 siblings, 0 replies; 22+ messages in thread
From: Jeff Cody @ 2017-02-17 21:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, berrange, pbonzini
On Wed, Jan 25, 2017 at 12:42:01PM -0500, Jeff Cody wrote:
> This adds blockdev-add support to the iscsi block driver.
>
> Picked this series up from Kevin. I've tested it on my local iscsi setup.
>
> There are only a few minor changes:
>
> * In patch 2, fixed the segfault pointed out by Daniel
> * In patch 6, placed the ':' after the command header as now required
> * New patch 7, to fix some out of date documentation in the qapi schema
>
>
> Jeff Cody (1):
> QAPI: Fix blockdev-add example documentation
>
> Kevin Wolf (6):
> iscsi: Split URL into individual options
> iscsi: Handle -iscsi user/password in bdrv_parse_filename()
> iscsi: Add initiator-name option
> iscsi: Add header-digest option
> iscsi: Add timeout option
> iscsi: Add blockdev-add support
>
> block/iscsi.c | 349 +++++++++++++++++++++++++++++++--------------------
> qapi/block-core.json | 92 +++++++++++---
> 2 files changed, 288 insertions(+), 153 deletions(-)
>
> --
> 2.9.3
>
Fixed up the formatting issues pointed out by Fam & patchew, and applied to
my block branch:
git://github.com/codyprime/qemu-kvm-jtc.git block
-Jeff
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support Jeff Cody
2017-02-07 10:23 ` Fam Zheng
@ 2017-02-17 21:40 ` Eric Blake
2017-02-17 21:47 ` Jeff Cody
1 sibling, 1 reply; 22+ messages in thread
From: Eric Blake @ 2017-02-17 21:40 UTC (permalink / raw)
To: Jeff Cody, qemu-devel; +Cc: kwolf, pbonzini, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]
On 01/25/2017 11:42 AM, Jeff Cody wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> This adds blockdev-add support for iscsi devices.
>
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/iscsi.c | 14 ++++++----
> qapi/block-core.json | 74 ++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 78 insertions(+), 10 deletions(-)
> +++ b/qapi/block-core.json
> @@ -2116,10 +2116,10 @@
> { 'enum': 'BlockdevDriver',
> 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
> 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
> - 'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 'null-aio',
> - 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> - 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc',
> - 'vvfat' ] }
> + 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',
> + 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',
> + 'quorum', 'raw', 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk',
> + 'vpc', 'vvfat' ] }
Are we missing a since 2.9 documentation designation here?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support
2017-02-17 21:40 ` Eric Blake
@ 2017-02-17 21:47 ` Jeff Cody
0 siblings, 0 replies; 22+ messages in thread
From: Jeff Cody @ 2017-02-17 21:47 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, kwolf, pbonzini, qemu-block
On Fri, Feb 17, 2017 at 03:40:21PM -0600, Eric Blake wrote:
> On 01/25/2017 11:42 AM, Jeff Cody wrote:
> > From: Kevin Wolf <kwolf@redhat.com>
> >
> > This adds blockdev-add support for iscsi devices.
> >
> > Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Signed-off-by: Jeff Cody <jcody@redhat.com>
> > ---
> > block/iscsi.c | 14 ++++++----
> > qapi/block-core.json | 74 ++++++++++++++++++++++++++++++++++++++++++++++++----
> > 2 files changed, 78 insertions(+), 10 deletions(-)
>
> > +++ b/qapi/block-core.json
> > @@ -2116,10 +2116,10 @@
> > { 'enum': 'BlockdevDriver',
> > 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
> > 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
> > - 'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 'null-aio',
> > - 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> > - 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc',
> > - 'vvfat' ] }
> > + 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',
> > + 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',
> > + 'quorum', 'raw', 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk',
> > + 'vpc', 'vvfat' ] }
>
> Are we missing a since 2.9 documentation designation here?
>
Yes, thanks for catching that. I've applied it to my branch already, but I
will go ahead and fix this and rebase with the following squashed into the
patch:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f00fc9d..5f82d35 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2110,6 +2110,7 @@
# @nfs: Since 2.8
# @replication: Since 2.8
# @ssh: Since 2.8
+# @iscsi: Since 2.9
#
# Since: 2.0
##
^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-02-17 21:48 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-25 17:42 [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support Jeff Cody
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 1/7] iscsi: Split URL into individual options Jeff Cody
2017-02-07 10:06 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename() Jeff Cody
2017-02-07 10:13 ` Fam Zheng
2017-02-17 13:26 ` Kevin Wolf
2017-02-17 14:09 ` Fam Zheng
2017-02-17 14:10 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 3/7] iscsi: Add initiator-name option Jeff Cody
2017-02-07 10:16 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 4/7] iscsi: Add header-digest option Jeff Cody
2017-02-07 10:18 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 5/7] iscsi: Add timeout option Jeff Cody
2017-02-07 10:21 ` Fam Zheng
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support Jeff Cody
2017-02-07 10:23 ` Fam Zheng
2017-02-17 21:40 ` Eric Blake
2017-02-17 21:47 ` Jeff Cody
2017-01-25 17:42 ` [Qemu-devel] [PATCH v2 7/7] QAPI: Fix blockdev-add example documentation Jeff Cody
2017-02-07 10:29 ` Fam Zheng
2017-01-25 17:57 ` [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support no-reply
2017-02-17 21:12 ` Jeff Cody
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).