* [Qemu-devel] [PATCH 0/2] nbd: add zero-init parameter
@ 2016-10-05 9:33 Denis V. Lunev
2016-10-05 9:33 ` [Qemu-devel] [PATCH 1/2] nbd: change option parsing scheme Denis V. Lunev
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Denis V. Lunev @ 2016-10-05 9:33 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: den, Denis Plotnikov, Paolo Bonzini, Kevin Wolf, Max Reitz
When using a nbd block device, the info about necessity of prior disk
zeroing could significantly improve the speed of certain operations
(e.g. backups).
This patch also will allow to preserve QCOW2 images during migration.
Management software now may specify zero-init option and thus abscent
areas in the QCOW2 areas will not be marked as zeroes and will remain
empty. This is tight distiction but it is here.
Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
Denis Plotnikov (2):
nbd: change option parsing scheme
nbd: add zero-init parameter
block/nbd.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 184 insertions(+), 20 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] nbd: change option parsing scheme
2016-10-05 9:33 [Qemu-devel] [PATCH 0/2] nbd: add zero-init parameter Denis V. Lunev
@ 2016-10-05 9:33 ` Denis V. Lunev
2016-10-05 9:57 ` Paolo Bonzini
2016-10-05 9:33 ` [Qemu-devel] [PATCH 2/2] nbd: add zero-init parameter Denis V. Lunev
2016-10-05 9:55 ` [Qemu-devel] [PATCH 0/2] " Paolo Bonzini
2 siblings, 1 reply; 6+ messages in thread
From: Denis V. Lunev @ 2016-10-05 9:33 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: den, Denis Plotnikov, Paolo Bonzini, Kevin Wolf, Max Reitz
From: Denis Plotnikov <dplotnikov@virtuozzo.com>
This is a preparatory commit to make code more generic. We are going to
add more options in the next patch.
Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
---
block/nbd.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 124 insertions(+), 19 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 6bc06d6..3b133ed 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -38,7 +38,9 @@
#include "qapi/qmp/qstring.h"
#include "qemu/cutils.h"
-#define EN_OPTSTR ":exportname="
+#define EN_OPTSTR "exportname"
+
+#define PATH_PARAM (1u << 0)
typedef struct BDRVNBDState {
NbdClientSession client;
@@ -47,6 +49,46 @@ typedef struct BDRVNBDState {
char *path, *host, *port, *export, *tlscredsid;
} BDRVNBDState;
+/*
+ * helpers for dealing with option parsing
+ * to ease futher params adding and managing
+ */
+
+/*
+ * @param_flags - bit flags defining a set of param names to be parsed
+ */
+static bool parse_query_params(QueryParams *qp, QDict *options,
+ unsigned int param_flags)
+{
+ int i;
+ for (i = 0; i < qp->n; i++) {
+ QueryParam *param = &qp->p[i];
+
+ if ((PATH_PARAM & param_flags) &&
+ strcmp(param->name, "socket") == 0) {
+ qdict_put(options, "path", qstring_from_str(param->value));
+ continue;
+ }
+
+ }
+ return true;
+}
+
+static bool find_prohibited_params(QueryParams *qp, unsigned int param_flags)
+{
+ int i;
+ for (i = 0; i < qp->n; i++) {
+ QueryParam *param = &qp->p[i];
+
+ if ((PATH_PARAM & param_flags) &&
+ strcmp(param->name, "socket") == 0) {
+ return true;
+ }
+ }
+ return false;
+}
+
+
static int nbd_parse_uri(const char *filename, QDict *options)
{
URI *uri;
@@ -79,18 +121,18 @@ static int nbd_parse_uri(const char *filename, QDict *options)
}
qp = query_params_parse(uri->query);
- if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) {
- ret = -EINVAL;
- goto out;
- }
if (is_unix) {
/* nbd+unix:///export?socket=path */
- if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) {
+ if (uri->server || uri->port) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!parse_query_params(qp, options, PATH_PARAM)) {
ret = -EINVAL;
goto out;
}
- qdict_put(options, "path", qstring_from_str(qp->p[0].value));
} else {
QString *host;
/* nbd[+tcp]://host[:port]/export */
@@ -113,6 +155,11 @@ static int nbd_parse_uri(const char *filename, QDict *options)
qdict_put(options, "port", qstring_from_str(port_str));
g_free(port_str);
}
+
+ if (find_prohibited_params(qp, PATH_PARAM)) {
+ ret = -EINVAL;
+ goto out;
+ }
}
out:
@@ -127,7 +174,7 @@ static void nbd_parse_filename(const char *filename, QDict *options,
Error **errp)
{
char *file;
- char *export_name;
+ char *opt_str;
const char *host_spec;
const char *unixpath;
@@ -150,17 +197,6 @@ static void nbd_parse_filename(const char *filename, QDict *options,
file = g_strdup(filename);
- export_name = strstr(file, EN_OPTSTR);
- if (export_name) {
- if (export_name[strlen(EN_OPTSTR)] == 0) {
- goto out;
- }
- export_name[0] = 0; /* truncate 'file' */
- export_name += strlen(EN_OPTSTR);
-
- qdict_put(options, "export", qstring_from_str(export_name));
- }
-
/* extract the host_spec - fail if it's not nbd:... */
if (!strstart(file, "nbd:", &host_spec)) {
error_setg(errp, "File name string for NBD must start with 'nbd:'");
@@ -173,8 +209,40 @@ static void nbd_parse_filename(const char *filename, QDict *options,
/* are we a UNIX or TCP socket? */
if (strstart(host_spec, "unix:", &unixpath)) {
+ opt_str = (char *) unixpath;
+
+ /* do we have any options? */
+ /* unixpath could be unix: or unix:something:options */
+ opt_str = strchr(opt_str, ':');
+
+ /* if we have any options then "divide" */
+ /* the path and the options by replacing the last colon with "\0" */
+ if (opt_str != NULL) {
+ /* truncate 'unixpath' replacing the last ":" */
+ char *colon_pos = opt_str;
+ colon_pos[0] = '\0';
+ opt_str++;
+ }
qdict_put(options, "path", qstring_from_str(unixpath));
} else {
+ /* host_spec could be ip:port or ip:port:options */
+ int i;
+ opt_str = (char *)host_spec;
+ for (i = 0; i < 2; i++) {
+ opt_str = strchr(opt_str, ':');
+ if (opt_str == NULL) {
+ break;
+ }
+ opt_str++;
+ }
+
+ /* the same idea with dividing as above */
+ if (opt_str != NULL) {
+ /* truncate 'host_name' replacing the last ":" */
+ char *second_colon_pos = opt_str - 1;
+ second_colon_pos[0] = '\0';
+ }
+
InetSocketAddress *addr = NULL;
addr = inet_parse(host_spec, errp);
@@ -187,6 +255,43 @@ static void nbd_parse_filename(const char *filename, QDict *options,
qapi_free_InetSocketAddress(addr);
}
+ /* opt_str == NULL means no options given */
+ if (opt_str != NULL) {
+ static QemuOptsList file_opts = {
+ .name = "file_opts",
+ .head = QTAILQ_HEAD_INITIALIZER(file_opts.head),
+ .desc = {
+ {
+ .name = EN_OPTSTR,
+ .type = QEMU_OPT_STRING,
+ .help = "Name of the NBD export to open",
+ },
+ },
+ };
+
+ QemuOpts *opts = qemu_opts_create(&file_opts, NULL, 0, errp);
+ if (opts == NULL) {
+ error_setg(errp, "Can't parse file options");
+ goto out;
+ }
+
+ Error *local_err = NULL;
+ qemu_opts_do_parse(opts, opt_str, NULL, &local_err);
+ if (local_err) {
+ error_setg(errp, "Can't parse file options");
+ qemu_opts_del(opts);
+ goto out;
+ }
+
+ const char *value;
+ value = qemu_opt_get(opts, EN_OPTSTR);
+ if (value) {
+ qdict_put(options, "export", qstring_from_str(value));
+ }
+
+ qemu_opts_del(opts);
+ }
+
out:
g_free(file);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] nbd: add zero-init parameter
2016-10-05 9:33 [Qemu-devel] [PATCH 0/2] nbd: add zero-init parameter Denis V. Lunev
2016-10-05 9:33 ` [Qemu-devel] [PATCH 1/2] nbd: change option parsing scheme Denis V. Lunev
@ 2016-10-05 9:33 ` Denis V. Lunev
2016-10-05 9:55 ` [Qemu-devel] [PATCH 0/2] " Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Denis V. Lunev @ 2016-10-05 9:33 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: den, Denis Plotnikov, Paolo Bonzini, Kevin Wolf, Max Reitz
From: Denis Plotnikov <dplotnikov@virtuozzo.com>
When using a nbd block device, the info about necessity of prior disk
zeroing could significantly improve the speed of certain operations
(e.g. backups).
This patch also will allow to preserve QCOW2 images during migration.
Management software now may specify zero-init option and thus abscent
areas in the original QCOW2 image will not be marked as zeroes in the
target image. This is tight distiction but it is here.
Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
---
block/nbd.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 63 insertions(+), 4 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 3b133ed..f9dfe57 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -36,19 +36,35 @@
#include "qapi/qmp/qjson.h"
#include "qapi/qmp/qint.h"
#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qbool.h"
#include "qemu/cutils.h"
#define EN_OPTSTR "exportname"
+#define ZI_OPTSTR "zero-init"
-#define PATH_PARAM (1u << 0)
+#define PATH_PARAM (1u << 0)
+#define ZERO_INIT_PARAM (1u << 1)
typedef struct BDRVNBDState {
NbdClientSession client;
/* For nbd_refresh_filename() */
char *path, *host, *port, *export, *tlscredsid;
+ bool zero_init;
} BDRVNBDState;
+static bool set_option_zero_init(QDict *options, const char *value)
+{
+ if (strcmp(value, "on") == 0) {
+ qdict_put(options, "zero-init", qbool_from_bool(true));
+ } else if (strcmp(value, "off") == 0) {
+ qdict_put(options, "zero-init", qbool_from_bool(false));
+ } else {
+ return false;
+ }
+ return true;
+}
+
/*
* helpers for dealing with option parsing
* to ease futher params adding and managing
@@ -69,7 +85,13 @@ static bool parse_query_params(QueryParams *qp, QDict *options,
qdict_put(options, "path", qstring_from_str(param->value));
continue;
}
-
+ if ((ZERO_INIT_PARAM & param_flags) &&
+ strcmp(param->name, "zero-init") == 0) {
+ if (!set_option_zero_init(options, param->value)) {
+ return false;
+ }
+ continue;
+ }
}
return true;
}
@@ -123,13 +145,13 @@ static int nbd_parse_uri(const char *filename, QDict *options)
qp = query_params_parse(uri->query);
if (is_unix) {
- /* nbd+unix:///export?socket=path */
+ /* nbd+unix:///export?socket=path{,zero-init=[on|off]} */
if (uri->server || uri->port) {
ret = -EINVAL;
goto out;
}
- if (!parse_query_params(qp, options, PATH_PARAM)) {
+ if (!parse_query_params(qp, options, PATH_PARAM | ZERO_INIT_PARAM)) {
ret = -EINVAL;
goto out;
}
@@ -160,6 +182,11 @@ static int nbd_parse_uri(const char *filename, QDict *options)
ret = -EINVAL;
goto out;
}
+
+ if (!parse_query_params(qp, options, ZERO_INIT_PARAM)) {
+ ret = -EINVAL;
+ goto out;
+ }
}
out:
@@ -187,6 +214,8 @@ static void nbd_parse_filename(const char *filename, QDict *options,
return;
}
+ set_option_zero_init(options, "off");
+
if (strstr(filename, "://")) {
int ret = nbd_parse_uri(filename, options);
if (ret < 0) {
@@ -266,6 +295,11 @@ static void nbd_parse_filename(const char *filename, QDict *options,
.type = QEMU_OPT_STRING,
.help = "Name of the NBD export to open",
},
+ {
+ .name = ZI_OPTSTR,
+ .type = QEMU_OPT_BOOL,
+ .help = "Zero-initialized image flag",
+ },
},
};
@@ -289,6 +323,11 @@ static void nbd_parse_filename(const char *filename, QDict *options,
qdict_put(options, "export", qstring_from_str(value));
}
+ value = qemu_opt_get(opts, ZI_OPTSTR);
+ if (value) {
+ qdict_put(options, ZI_OPTSTR,
+ qbool_from_bool(strcmp(value, "on") == 0));
+ }
qemu_opts_del(opts);
}
@@ -337,6 +376,8 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
s->export = g_strdup(qemu_opt_get(opts, "export"));
+ s->zero_init = strcmp(qemu_opt_get(opts, "zero-init"), "on") == 0;
+
return saddr;
}
@@ -427,6 +468,11 @@ static QemuOptsList nbd_runtime_opts = {
.type = QEMU_OPT_STRING,
.help = "ID of the TLS credentials to use",
},
+ {
+ .name = "zero-init",
+ .type = QEMU_OPT_BOOL,
+ .help = "Zero-initialized image flag",
+ },
},
};
@@ -585,9 +631,19 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
QOBJECT(qstring_from_str(s->tlscredsid)));
}
+ if (s->zero_init) {
+ qdict_put(opts, "zero-init", qbool_from_bool(true));
+ }
+
bs->full_open_options = opts;
}
+static int nbd_has_zero_init(BlockDriverState *bs)
+{
+ BDRVNBDState *s = bs->opaque;
+ return s->zero_init;
+}
+
static BlockDriver bdrv_nbd = {
.format_name = "nbd",
.protocol_name = "nbd",
@@ -604,6 +660,7 @@ static BlockDriver bdrv_nbd = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_has_zero_init = nbd_has_zero_init,
};
static BlockDriver bdrv_nbd_tcp = {
@@ -622,6 +679,7 @@ static BlockDriver bdrv_nbd_tcp = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_has_zero_init = nbd_has_zero_init,
};
static BlockDriver bdrv_nbd_unix = {
@@ -640,6 +698,7 @@ static BlockDriver bdrv_nbd_unix = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_has_zero_init = nbd_has_zero_init,
};
static void bdrv_nbd_init(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd: add zero-init parameter
2016-10-05 9:33 [Qemu-devel] [PATCH 0/2] nbd: add zero-init parameter Denis V. Lunev
2016-10-05 9:33 ` [Qemu-devel] [PATCH 1/2] nbd: change option parsing scheme Denis V. Lunev
2016-10-05 9:33 ` [Qemu-devel] [PATCH 2/2] nbd: add zero-init parameter Denis V. Lunev
@ 2016-10-05 9:55 ` Paolo Bonzini
2016-10-05 10:56 ` Denis V. Lunev
2 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2016-10-05 9:55 UTC (permalink / raw)
To: Denis V. Lunev, qemu-block, qemu-devel
Cc: Denis Plotnikov, Kevin Wolf, Max Reitz
On 05/10/2016 11:33, Denis V. Lunev wrote:
> When using a nbd block device, the info about necessity of prior disk
> zeroing could significantly improve the speed of certain operations
> (e.g. backups).
>
> This patch also will allow to preserve QCOW2 images during migration.
> Management software now may specify zero-init option and thus abscent
> areas in the QCOW2 areas will not be marked as zeroes and will remain
> empty. This is tight distiction but it is here.
I think Kevin said he wanted to avoid the URI shortcut, and instead
preferred to use JSON specifications?
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] nbd: change option parsing scheme
2016-10-05 9:33 ` [Qemu-devel] [PATCH 1/2] nbd: change option parsing scheme Denis V. Lunev
@ 2016-10-05 9:57 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2016-10-05 9:57 UTC (permalink / raw)
To: Denis V. Lunev, qemu-block, qemu-devel
Cc: Denis Plotnikov, Kevin Wolf, Max Reitz
I reviewed this patch before noticing that the overall idea is not what
Kevin suggested, so I'm sending it out anyway. Further comments from
Kevin and Max might come since I am not familiar with the current
conventions on parsing block device options.
On 05/10/2016 11:33, Denis V. Lunev wrote:
> From: Denis Plotnikov <dplotnikov@virtuozzo.com>
>
> This is a preparatory commit to make code more generic. We are going to
> add more options in the next patch.
>
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> ---
> block/nbd.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 124 insertions(+), 19 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index 6bc06d6..3b133ed 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -38,7 +38,9 @@
> #include "qapi/qmp/qstring.h"
> #include "qemu/cutils.h"
>
> -#define EN_OPTSTR ":exportname="
> +#define EN_OPTSTR "exportname"
Please replace the #define with the "exportname" string; same for
ZI_OPTSTR in patch 2.
> +#define PATH_PARAM (1u << 0)
>
> typedef struct BDRVNBDState {
> NbdClientSession client;
> @@ -47,6 +49,46 @@ typedef struct BDRVNBDState {
> char *path, *host, *port, *export, *tlscredsid;
> } BDRVNBDState;
>
> +/*
> + * helpers for dealing with option parsing
> + * to ease futher params adding and managing
> + */
> +
> +/*
> + * @param_flags - bit flags defining a set of param names to be parsed
> + */
> +static bool parse_query_params(QueryParams *qp, QDict *options,
> + unsigned int param_flags)
> +{
> + int i;
> + for (i = 0; i < qp->n; i++) {
> + QueryParam *param = &qp->p[i];
> +
> + if ((PATH_PARAM & param_flags) &&
> + strcmp(param->name, "socket") == 0) {
> + qdict_put(options, "path", qstring_from_str(param->value));
> + continue;
> + }
> +
> + }
> + return true;
> +}
Please remove the param_flags argument. In patch 2 you do:
if (find_prohibited_params(qp, PATH_PARAM)) {
ret = -EINVAL;
goto out;
}
if (!parse_query_params(qp, options, ZERO_INIT_PARAM)) {
ret = -EINVAL;
goto out;
}
Because you've filtered out the socket parameter, it's okay if
parse_query_params parses it always.
Also, please change nbd_parse_uri to return errors via Error**. Then
parse_query_params can do the same, and we get better error messages.
> +
> +static bool find_prohibited_params(QueryParams *qp, unsigned int param_flags)
> +{
> + int i;
> + for (i = 0; i < qp->n; i++) {
> + QueryParam *param = &qp->p[i];
> +
> + if ((PATH_PARAM & param_flags) &&
> + strcmp(param->name, "socket") == 0) {
> + return true;
> + }
> + }
> + return false;
> +}
Please change this function to something like
static QueryParam *find_query_param(QueryParams *qp, const char *name)
> + if (!parse_query_params(qp, options, PATH_PARAM)) {
> ret = -EINVAL;
> goto out;
> }
> - qdict_put(options, "path", qstring_from_str(qp->p[0].value));
> } else {
> QString *host;
> /* nbd[+tcp]://host[:port]/export */
> @@ -113,6 +155,11 @@ static int nbd_parse_uri(const char *filename, QDict *options)
> qdict_put(options, "port", qstring_from_str(port_str));
> g_free(port_str);
> }
> +
> + if (find_prohibited_params(qp, PATH_PARAM)) {
> + ret = -EINVAL;
> + goto out;
> + }
As mentioned above, using Error** will let you return a better error
message, such as "The 'socket' parameter is only valid for NBD over Unix
domain sockets".
> }
>
> out:
> @@ -127,7 +174,7 @@ static void nbd_parse_filename(const char *filename, QDict *options,
> Error **errp)
> {
> char *file;
> - char *export_name;
> + char *opt_str;
> const char *host_spec;
> const char *unixpath;
>
> @@ -150,17 +197,6 @@ static void nbd_parse_filename(const char *filename, QDict *options,
>
> file = g_strdup(filename);
>
> - export_name = strstr(file, EN_OPTSTR);
> - if (export_name) {
> - if (export_name[strlen(EN_OPTSTR)] == 0) {
> - goto out;
> - }
> - export_name[0] = 0; /* truncate 'file' */
> - export_name += strlen(EN_OPTSTR);
> -
> - qdict_put(options, "export", qstring_from_str(export_name));
> - }
> -
> /* extract the host_spec - fail if it's not nbd:... */
> if (!strstart(file, "nbd:", &host_spec)) {
> error_setg(errp, "File name string for NBD must start with 'nbd:'");
> @@ -173,8 +209,40 @@ static void nbd_parse_filename(const char *filename, QDict *options,
>
> /* are we a UNIX or TCP socket? */
> if (strstart(host_spec, "unix:", &unixpath)) {
> + opt_str = (char *) unixpath;
> +
> + /* do we have any options? */
> + /* unixpath could be unix: or unix:something:options */
> + opt_str = strchr(opt_str, ':');
> +
> + /* if we have any options then "divide" */
> + /* the path and the options by replacing the last colon with "\0" */
> + if (opt_str != NULL) {
> + /* truncate 'unixpath' replacing the last ":" */
> + char *colon_pos = opt_str;
> + colon_pos[0] = '\0';
> + opt_str++;
Just this:
if (opt_str != NULL) {
*opt_str++ = 0;
}
> + }
> qdict_put(options, "path", qstring_from_str(unixpath));
> } else {
> + /* host_spec could be ip:port or ip:port:options */
> + int i;
> + opt_str = (char *)host_spec;
> + for (i = 0; i < 2; i++) {
> + opt_str = strchr(opt_str, ':');
> + if (opt_str == NULL) {
> + break;
> + }
> + opt_str++;
> + }
> +
> + /* the same idea with dividing as above */
> + if (opt_str != NULL) {
> + /* truncate 'host_name' replacing the last ":" */
> + char *second_colon_pos = opt_str - 1;
> + second_colon_pos[0] = '\0';
> + }
Again, simpler:
opt_str = strchr((char *)host_spec, ':');
if (opt_str != NULL) {
opt_str = strchr(opt_str + 1, ':');
if (opt_str != NULL) {
*opt_str++ = 0;
}
}
> InetSocketAddress *addr = NULL;
Please avoid declarations in the middle of statements.
>
> addr = inet_parse(host_spec, errp);
> @@ -187,6 +255,43 @@ static void nbd_parse_filename(const char *filename, QDict *options,
> qapi_free_InetSocketAddress(addr);
> }
>
> + /* opt_str == NULL means no options given */
> + if (opt_str != NULL) {
> + static QemuOptsList file_opts = {
Please put this declaration outside the function, and call it nbd_opts.
> + .name = "file_opts",
.name = "nbd",
> + .head = QTAILQ_HEAD_INITIALIZER(file_opts.head),
> + .desc = {
> + {
> + .name = EN_OPTSTR,
> + .type = QEMU_OPT_STRING,
> + .help = "Name of the NBD export to open",
> + },
> + },
> + };
> +
> + QemuOpts *opts = qemu_opts_create(&file_opts, NULL, 0, errp);
> + if (opts == NULL) {
> + error_setg(errp, "Can't parse file options");
> + goto out;
> + }
> +
> + Error *local_err = NULL;
> + qemu_opts_do_parse(opts, opt_str, NULL, &local_err);
> + if (local_err) {
> + error_setg(errp, "Can't parse file options");
> + qemu_opts_del(opts);
> + goto out;
> + }
> +
> + const char *value;
> + value = qemu_opt_get(opts, EN_OPTSTR);
> + if (value) {
> + qdict_put(options, "export", qstring_from_str(value));
"export" should be changed to "exportname" in the QDict. This would
probably simplify the code but I don't know the exact function to use.
> + }
> +
> + qemu_opts_del(opts);
> + }
> +
> out:
> g_free(file);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd: add zero-init parameter
2016-10-05 9:55 ` [Qemu-devel] [PATCH 0/2] " Paolo Bonzini
@ 2016-10-05 10:56 ` Denis V. Lunev
0 siblings, 0 replies; 6+ messages in thread
From: Denis V. Lunev @ 2016-10-05 10:56 UTC (permalink / raw)
To: Paolo Bonzini, qemu-block, qemu-devel
Cc: Denis Plotnikov, Kevin Wolf, Max Reitz
On 10/05/2016 12:55 PM, Paolo Bonzini wrote:
>
> On 05/10/2016 11:33, Denis V. Lunev wrote:
>> When using a nbd block device, the info about necessity of prior disk
>> zeroing could significantly improve the speed of certain operations
>> (e.g. backups).
>>
>> This patch also will allow to preserve QCOW2 images during migration.
>> Management software now may specify zero-init option and thus abscent
>> areas in the QCOW2 areas will not be marked as zeroes and will remain
>> empty. This is tight distiction but it is here.
> I think Kevin said he wanted to avoid the URI shortcut, and instead
> preferred to use JSON specifications?
>
> Paolo
yep. filling myself stupid. Will resend, the patch will look MUSH shorter.
Den
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-05 10:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-05 9:33 [Qemu-devel] [PATCH 0/2] nbd: add zero-init parameter Denis V. Lunev
2016-10-05 9:33 ` [Qemu-devel] [PATCH 1/2] nbd: change option parsing scheme Denis V. Lunev
2016-10-05 9:57 ` Paolo Bonzini
2016-10-05 9:33 ` [Qemu-devel] [PATCH 2/2] nbd: add zero-init parameter Denis V. Lunev
2016-10-05 9:55 ` [Qemu-devel] [PATCH 0/2] " Paolo Bonzini
2016-10-05 10:56 ` Denis V. Lunev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.