* [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11)
@ 2017-11-07 22:27 Jeff Cody
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init() Jeff Cody
` (7 more replies)
0 siblings, 8 replies; 26+ messages in thread
From: Jeff Cody @ 2017-11-07 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
Changes from v2 -> v3:
Patch 1: Return -EIO for ssh_state_init() (Thanks Eric)
Patch 4: fixed missed line (Thanks Eric)
Patch 7: fixed opaque pointer assignment (Thanks Eric)
git-backport-diff -r master.. -u 81fb084
001/7:[0007] [FC] 'block/ssh: don't call libssh2_init() in block_init()'
002/7:[----] [--] 'block/ssh: make compliant with coding guidelines'
003/7:[----] [--] 'block/sheepdog: remove spurious NULL check'
004/7:[0010] [FC] 'block/sheepdog: code beautification'
005/7:[----] [--] 'block/curl: check error return of curl_global_init()'
006/7:[----] [--] 'block/curl: fix minor memory leaks'
007/7:[0002] [FC] 'block/curl: code cleanup to comply with coding style'
Jeff Cody (7):
block/ssh: don't call libssh2_init() in block_init()
block/ssh: make compliant with coding guidelines
block/sheepdog: remove spurious NULL check
block/sheepdog: code beautification
block/curl: check error return of curl_global_init()
block/curl: fix minor memory leaks
block/curl: code cleanup to comply with coding style
block/curl.c | 124 +++++++++++++++++++++++------------------
block/sheepdog.c | 166 +++++++++++++++++++++++++++----------------------------
block/ssh.c | 69 ++++++++++++++---------
3 files changed, 197 insertions(+), 162 deletions(-)
--
2.9.5
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init()
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
@ 2017-11-07 22:27 ` Jeff Cody
2017-11-07 22:48 ` Eric Blake
2017-11-08 11:39 ` Richard W.M. Jones
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines Jeff Cody
` (6 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Jeff Cody @ 2017-11-07 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
We don't need libssh2 failure to be fatal (we could just opt to not
register the driver on failure). But, it is probably a good idea to
avoid external library calls during the block_init(), and call the
libssh2 global init function on the first usage, returning any errors.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/ssh.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index b049a16..de81ec8 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -83,12 +83,28 @@ typedef struct BDRVSSHState {
bool unsafe_flush_warning;
} BDRVSSHState;
-static void ssh_state_init(BDRVSSHState *s)
+static bool ssh_libinit_called;
+
+static int ssh_state_init(BDRVSSHState *s, Error **errp)
{
+ int ret;
+
+ if (!ssh_libinit_called) {
+ ret = libssh2_init(0);
+ if (ret) {
+ error_setg(errp, "libssh2 initialization failed with %d", ret);
+ return ret;
+ }
+ ssh_libinit_called = true;
+ }
+
+
memset(s, 0, sizeof *s);
s->sock = -1;
s->offset = -1;
qemu_co_mutex_init(&s->lock);
+
+ return 0;
}
static void ssh_state_free(BDRVSSHState *s)
@@ -773,7 +789,9 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
int ret;
int ssh_flags;
- ssh_state_init(s);
+ if (ssh_state_init(s, errp)) {
+ return -EIO;
+ }
ssh_flags = LIBSSH2_FXF_READ;
if (bdrv_flags & BDRV_O_RDWR) {
@@ -821,8 +839,13 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
BDRVSSHState s;
ssize_t r2;
char c[1] = { '\0' };
+ Error *local_err = NULL;
- ssh_state_init(&s);
+ ret = ssh_state_init(&s, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return ret;
+ }
/* Get desired file size. */
total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
@@ -1213,14 +1236,6 @@ static BlockDriver bdrv_ssh = {
static void bdrv_ssh_init(void)
{
- int r;
-
- r = libssh2_init(0);
- if (r != 0) {
- fprintf(stderr, "libssh2 initialization failed, %d\n", r);
- exit(EXIT_FAILURE);
- }
-
bdrv_register(&bdrv_ssh);
}
--
2.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init() Jeff Cody
@ 2017-11-07 22:27 ` Jeff Cody
2017-11-08 10:50 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-08 11:38 ` [Qemu-devel] " Richard W.M. Jones
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 3/7] block/sheepdog: remove spurious NULL check Jeff Cody
` (5 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Jeff Cody @ 2017-11-07 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/ssh.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index de81ec8..39cacc1 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -241,7 +241,7 @@ static int parse_uri(const char *filename, QDict *options, Error **errp)
goto err;
}
- if(uri->user && strcmp(uri->user, "") != 0) {
+ if (uri->user && strcmp(uri->user, "") != 0) {
qdict_put_str(options, "user", uri->user);
}
@@ -268,7 +268,7 @@ static int parse_uri(const char *filename, QDict *options, Error **errp)
err:
if (uri) {
- uri_free(uri);
+ uri_free(uri);
}
return -EINVAL;
}
@@ -342,7 +342,7 @@ static int check_host_key_knownhosts(BDRVSSHState *s,
libssh2_knownhost_readfile(knh, knh_file, LIBSSH2_KNOWNHOST_FILE_OPENSSH);
r = libssh2_knownhost_checkp(knh, host, port, hostkey, len,
- LIBSSH2_KNOWNHOST_TYPE_PLAIN|
+ LIBSSH2_KNOWNHOST_TYPE_PLAIN |
LIBSSH2_KNOWNHOST_KEYENC_RAW,
&found);
switch (r) {
@@ -405,15 +405,18 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len,
unsigned c;
while (len > 0) {
- while (*host_key_check == ':')
+ while (*host_key_check == ':') {
host_key_check++;
+ }
if (!qemu_isxdigit(host_key_check[0]) ||
- !qemu_isxdigit(host_key_check[1]))
+ !qemu_isxdigit(host_key_check[1])) {
return 1;
+ }
c = hex2decimal(host_key_check[0]) * 16 +
hex2decimal(host_key_check[1]);
- if (c - *fingerprint != 0)
+ if (c - *fingerprint != 0) {
return c - *fingerprint;
+ }
fingerprint++;
len--;
host_key_check += 2;
@@ -433,8 +436,8 @@ check_host_key_hash(BDRVSSHState *s, const char *hash,
return -EINVAL;
}
- if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_len,
- hash) != 0) {
+ if (compare_fingerprint((unsigned char *) fingerprint, fingerprint_len,
+ hash) != 0) {
error_setg(errp, "remote host key does not match host_key_check '%s'",
hash);
return -EPERM;
@@ -507,7 +510,7 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp)
goto out;
}
- for(;;) {
+ for (;;) {
r = libssh2_agent_get_identity(agent, &identity, prev_identity);
if (r == 1) { /* end of list */
break;
@@ -860,8 +863,8 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
}
r = connect_to_ssh(&s, uri_options,
- LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE|
- LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC,
+ LIBSSH2_FXF_READ | LIBSSH2_FXF_WRITE |
+ LIBSSH2_FXF_CREAT | LIBSSH2_FXF_TRUNC,
0644, errp);
if (r < 0) {
ret = r;
@@ -869,7 +872,7 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
}
if (total_size > 0) {
- libssh2_sftp_seek64(s.sftp_handle, total_size-1);
+ libssh2_sftp_seek64(s.sftp_handle, total_size - 1);
r2 = libssh2_sftp_write(s.sftp_handle, c, 1);
if (r2 < 0) {
sftp_error_setg(errp, &s, "truncate failed");
@@ -1108,7 +1111,7 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
* works for me.
*/
if (r == 0) {
- ssh_seek(s, offset + written, SSH_SEEK_WRITE|SSH_SEEK_FORCE);
+ ssh_seek(s, offset + written, SSH_SEEK_WRITE | SSH_SEEK_FORCE);
co_yield(s, bs);
goto again;
}
@@ -1122,8 +1125,9 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
end_of_vec = i->iov_base + i->iov_len;
}
- if (offset + written > s->attrs.filesize)
+ if (offset + written > s->attrs.filesize) {
s->attrs.filesize = offset + written;
+ }
}
return 0;
--
2.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v3 3/7] block/sheepdog: remove spurious NULL check
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init() Jeff Cody
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines Jeff Cody
@ 2017-11-07 22:27 ` Jeff Cody
2017-11-08 10:48 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 4/7] block/sheepdog: code beautification Jeff Cody
` (4 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Jeff Cody @ 2017-11-07 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
'tag' is already checked in the lines immediately preceding this check,
and set to non-NULL if NULL. No need to check again, it hasn't changed.
Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/sheepdog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 696a714..459d93a 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1632,7 +1632,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
if (!tag) {
tag = "";
}
- if (tag && strlen(tag) >= SD_MAX_VDI_TAG_LEN) {
+ if (strlen(tag) >= SD_MAX_VDI_TAG_LEN) {
error_setg(errp, "value of parameter 'tag' is too long");
ret = -EINVAL;
goto err_no_fd;
--
2.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v3 4/7] block/sheepdog: code beautification
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
` (2 preceding siblings ...)
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 3/7] block/sheepdog: remove spurious NULL check Jeff Cody
@ 2017-11-07 22:27 ` Jeff Cody
2017-11-08 10:52 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 5/7] block/curl: check error return of curl_global_init() Jeff Cody
` (3 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Jeff Cody @ 2017-11-07 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
No functional changes, just whitespace manipulation.
Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/sheepdog.c | 164 +++++++++++++++++++++++++++----------------------------
1 file changed, 82 insertions(+), 82 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 459d93a..488bad3 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -400,7 +400,7 @@ typedef struct BDRVSheepdogReopenState {
int cache_flags;
} BDRVSheepdogReopenState;
-static const char * sd_strerror(int err)
+static const char *sd_strerror(int err)
{
int i;
@@ -3078,111 +3078,111 @@ static QemuOptsList sd_create_opts = {
};
static BlockDriver bdrv_sheepdog = {
- .format_name = "sheepdog",
- .protocol_name = "sheepdog",
- .instance_size = sizeof(BDRVSheepdogState),
- .bdrv_parse_filename = sd_parse_filename,
- .bdrv_file_open = sd_open,
- .bdrv_reopen_prepare = sd_reopen_prepare,
- .bdrv_reopen_commit = sd_reopen_commit,
- .bdrv_reopen_abort = sd_reopen_abort,
- .bdrv_close = sd_close,
- .bdrv_create = sd_create,
- .bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_getlength = sd_getlength,
+ .format_name = "sheepdog",
+ .protocol_name = "sheepdog",
+ .instance_size = sizeof(BDRVSheepdogState),
+ .bdrv_parse_filename = sd_parse_filename,
+ .bdrv_file_open = sd_open,
+ .bdrv_reopen_prepare = sd_reopen_prepare,
+ .bdrv_reopen_commit = sd_reopen_commit,
+ .bdrv_reopen_abort = sd_reopen_abort,
+ .bdrv_close = sd_close,
+ .bdrv_create = sd_create,
+ .bdrv_has_zero_init = bdrv_has_zero_init_1,
+ .bdrv_getlength = sd_getlength,
.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
- .bdrv_truncate = sd_truncate,
+ .bdrv_truncate = sd_truncate,
- .bdrv_co_readv = sd_co_readv,
- .bdrv_co_writev = sd_co_writev,
- .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
- .bdrv_co_pdiscard = sd_co_pdiscard,
- .bdrv_co_get_block_status = sd_co_get_block_status,
+ .bdrv_co_readv = sd_co_readv,
+ .bdrv_co_writev = sd_co_writev,
+ .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
+ .bdrv_co_pdiscard = sd_co_pdiscard,
+ .bdrv_co_get_block_status = sd_co_get_block_status,
- .bdrv_snapshot_create = sd_snapshot_create,
- .bdrv_snapshot_goto = sd_snapshot_goto,
- .bdrv_snapshot_delete = sd_snapshot_delete,
- .bdrv_snapshot_list = sd_snapshot_list,
+ .bdrv_snapshot_create = sd_snapshot_create,
+ .bdrv_snapshot_goto = sd_snapshot_goto,
+ .bdrv_snapshot_delete = sd_snapshot_delete,
+ .bdrv_snapshot_list = sd_snapshot_list,
- .bdrv_save_vmstate = sd_save_vmstate,
- .bdrv_load_vmstate = sd_load_vmstate,
+ .bdrv_save_vmstate = sd_save_vmstate,
+ .bdrv_load_vmstate = sd_load_vmstate,
- .bdrv_detach_aio_context = sd_detach_aio_context,
- .bdrv_attach_aio_context = sd_attach_aio_context,
+ .bdrv_detach_aio_context = sd_detach_aio_context,
+ .bdrv_attach_aio_context = sd_attach_aio_context,
- .create_opts = &sd_create_opts,
+ .create_opts = &sd_create_opts,
};
static BlockDriver bdrv_sheepdog_tcp = {
- .format_name = "sheepdog",
- .protocol_name = "sheepdog+tcp",
- .instance_size = sizeof(BDRVSheepdogState),
- .bdrv_parse_filename = sd_parse_filename,
- .bdrv_file_open = sd_open,
- .bdrv_reopen_prepare = sd_reopen_prepare,
- .bdrv_reopen_commit = sd_reopen_commit,
- .bdrv_reopen_abort = sd_reopen_abort,
- .bdrv_close = sd_close,
- .bdrv_create = sd_create,
- .bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_getlength = sd_getlength,
+ .format_name = "sheepdog",
+ .protocol_name = "sheepdog+tcp",
+ .instance_size = sizeof(BDRVSheepdogState),
+ .bdrv_parse_filename = sd_parse_filename,
+ .bdrv_file_open = sd_open,
+ .bdrv_reopen_prepare = sd_reopen_prepare,
+ .bdrv_reopen_commit = sd_reopen_commit,
+ .bdrv_reopen_abort = sd_reopen_abort,
+ .bdrv_close = sd_close,
+ .bdrv_create = sd_create,
+ .bdrv_has_zero_init = bdrv_has_zero_init_1,
+ .bdrv_getlength = sd_getlength,
.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
- .bdrv_truncate = sd_truncate,
+ .bdrv_truncate = sd_truncate,
- .bdrv_co_readv = sd_co_readv,
- .bdrv_co_writev = sd_co_writev,
- .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
- .bdrv_co_pdiscard = sd_co_pdiscard,
- .bdrv_co_get_block_status = sd_co_get_block_status,
+ .bdrv_co_readv = sd_co_readv,
+ .bdrv_co_writev = sd_co_writev,
+ .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
+ .bdrv_co_pdiscard = sd_co_pdiscard,
+ .bdrv_co_get_block_status = sd_co_get_block_status,
- .bdrv_snapshot_create = sd_snapshot_create,
- .bdrv_snapshot_goto = sd_snapshot_goto,
- .bdrv_snapshot_delete = sd_snapshot_delete,
- .bdrv_snapshot_list = sd_snapshot_list,
+ .bdrv_snapshot_create = sd_snapshot_create,
+ .bdrv_snapshot_goto = sd_snapshot_goto,
+ .bdrv_snapshot_delete = sd_snapshot_delete,
+ .bdrv_snapshot_list = sd_snapshot_list,
- .bdrv_save_vmstate = sd_save_vmstate,
- .bdrv_load_vmstate = sd_load_vmstate,
+ .bdrv_save_vmstate = sd_save_vmstate,
+ .bdrv_load_vmstate = sd_load_vmstate,
- .bdrv_detach_aio_context = sd_detach_aio_context,
- .bdrv_attach_aio_context = sd_attach_aio_context,
+ .bdrv_detach_aio_context = sd_detach_aio_context,
+ .bdrv_attach_aio_context = sd_attach_aio_context,
- .create_opts = &sd_create_opts,
+ .create_opts = &sd_create_opts,
};
static BlockDriver bdrv_sheepdog_unix = {
- .format_name = "sheepdog",
- .protocol_name = "sheepdog+unix",
- .instance_size = sizeof(BDRVSheepdogState),
- .bdrv_parse_filename = sd_parse_filename,
- .bdrv_file_open = sd_open,
- .bdrv_reopen_prepare = sd_reopen_prepare,
- .bdrv_reopen_commit = sd_reopen_commit,
- .bdrv_reopen_abort = sd_reopen_abort,
- .bdrv_close = sd_close,
- .bdrv_create = sd_create,
- .bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_getlength = sd_getlength,
+ .format_name = "sheepdog",
+ .protocol_name = "sheepdog+unix",
+ .instance_size = sizeof(BDRVSheepdogState),
+ .bdrv_parse_filename = sd_parse_filename,
+ .bdrv_file_open = sd_open,
+ .bdrv_reopen_prepare = sd_reopen_prepare,
+ .bdrv_reopen_commit = sd_reopen_commit,
+ .bdrv_reopen_abort = sd_reopen_abort,
+ .bdrv_close = sd_close,
+ .bdrv_create = sd_create,
+ .bdrv_has_zero_init = bdrv_has_zero_init_1,
+ .bdrv_getlength = sd_getlength,
.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
- .bdrv_truncate = sd_truncate,
+ .bdrv_truncate = sd_truncate,
- .bdrv_co_readv = sd_co_readv,
- .bdrv_co_writev = sd_co_writev,
- .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
- .bdrv_co_pdiscard = sd_co_pdiscard,
- .bdrv_co_get_block_status = sd_co_get_block_status,
+ .bdrv_co_readv = sd_co_readv,
+ .bdrv_co_writev = sd_co_writev,
+ .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
+ .bdrv_co_pdiscard = sd_co_pdiscard,
+ .bdrv_co_get_block_status = sd_co_get_block_status,
- .bdrv_snapshot_create = sd_snapshot_create,
- .bdrv_snapshot_goto = sd_snapshot_goto,
- .bdrv_snapshot_delete = sd_snapshot_delete,
- .bdrv_snapshot_list = sd_snapshot_list,
+ .bdrv_snapshot_create = sd_snapshot_create,
+ .bdrv_snapshot_goto = sd_snapshot_goto,
+ .bdrv_snapshot_delete = sd_snapshot_delete,
+ .bdrv_snapshot_list = sd_snapshot_list,
- .bdrv_save_vmstate = sd_save_vmstate,
- .bdrv_load_vmstate = sd_load_vmstate,
+ .bdrv_save_vmstate = sd_save_vmstate,
+ .bdrv_load_vmstate = sd_load_vmstate,
- .bdrv_detach_aio_context = sd_detach_aio_context,
- .bdrv_attach_aio_context = sd_attach_aio_context,
+ .bdrv_detach_aio_context = sd_detach_aio_context,
+ .bdrv_attach_aio_context = sd_attach_aio_context,
- .create_opts = &sd_create_opts,
+ .create_opts = &sd_create_opts,
};
static void bdrv_sheepdog_init(void)
--
2.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v3 5/7] block/curl: check error return of curl_global_init()
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
` (3 preceding siblings ...)
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 4/7] block/sheepdog: code beautification Jeff Cody
@ 2017-11-07 22:27 ` Jeff Cody
2017-11-08 10:51 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-08 11:41 ` [Qemu-devel] " Richard W.M. Jones
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks Jeff Cody
` (2 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Jeff Cody @ 2017-11-07 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
If curl_global_init() fails, per the documentation no other curl
functions may be called, so make sure to check the return value.
Also, some minor changes to the initialization latch variable 'inited':
- Make it static in the file, for clarity
- Change the name for clarity
- Make it a bool
Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/curl.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index 2a244e2..00a9879 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -89,6 +89,8 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
struct BDRVCURLState;
+static bool libcurl_initialized;
+
typedef struct CURLAIOCB {
Coroutine *co;
QEMUIOVector *qiov;
@@ -686,14 +688,23 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
double d;
const char *secretid;
const char *protocol_delimiter;
+ int ret;
- static int inited = 0;
if (flags & BDRV_O_RDWR) {
error_setg(errp, "curl block device does not support writes");
return -EROFS;
}
+ if (!libcurl_initialized) {
+ ret = curl_global_init(CURL_GLOBAL_ALL);
+ if (ret) {
+ error_setg(errp, "libcurl initialization failed with %d", ret);
+ return -EIO;
+ }
+ libcurl_initialized = true;
+ }
+
qemu_mutex_init(&s->mutex);
opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
@@ -772,11 +783,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
}
}
- if (!inited) {
- curl_global_init(CURL_GLOBAL_ALL);
- inited = 1;
- }
-
DPRINTF("CURL: Opening %s\n", file);
QSIMPLEQ_INIT(&s->free_state_waitq);
s->aio_context = bdrv_get_aio_context(bs);
--
2.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
` (4 preceding siblings ...)
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 5/7] block/curl: check error return of curl_global_init() Jeff Cody
@ 2017-11-07 22:27 ` Jeff Cody
2017-11-08 0:13 ` Eric Blake
` (2 more replies)
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style Jeff Cody
2017-12-18 21:05 ` [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
7 siblings, 3 replies; 26+ messages in thread
From: Jeff Cody @ 2017-11-07 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/curl.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/block/curl.c b/block/curl.c
index 00a9879..35cf417 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -857,6 +857,9 @@ out_noclean:
qemu_mutex_destroy(&s->mutex);
g_free(s->cookie);
g_free(s->url);
+ g_free(s->username);
+ g_free(s->proxyusername);
+ g_free(s->proxypassword);
qemu_opts_del(opts);
return -EINVAL;
}
@@ -955,6 +958,9 @@ static void curl_close(BlockDriverState *bs)
g_free(s->cookie);
g_free(s->url);
+ g_free(s->username);
+ g_free(s->proxyusername);
+ g_free(s->proxypassword);
}
static int64_t curl_getlength(BlockDriverState *bs)
--
2.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
` (5 preceding siblings ...)
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks Jeff Cody
@ 2017-11-07 22:27 ` Jeff Cody
2017-11-08 10:47 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-08 11:46 ` [Qemu-devel] " Richard W.M. Jones
2017-12-18 21:05 ` [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
7 siblings, 2 replies; 26+ messages in thread
From: Jeff Cody @ 2017-11-07 22:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
This addresses non-functional changes to help curl.c better comply
with the coding styles (comments, indentation, brackets, etc.).
One minor code change is the combination of two if statements into
a single if statement.
Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/curl.c | 100 +++++++++++++++++++++++++++++++----------------------------
1 file changed, 52 insertions(+), 48 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index 35cf417..7a6dd44 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -32,8 +32,10 @@
#include <curl/curl.h>
#include "qemu/cutils.h"
-// #define DEBUG_CURL
-// #define DEBUG_VERBOSE
+/*
+ #define DEBUG_CURL
+ #define DEBUG_VERBOSE
+*/
#ifdef DEBUG_CURL
#define DEBUG_CURL_PRINT 1
@@ -76,15 +78,15 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
#define CURL_TIMEOUT_DEFAULT 5
#define CURL_TIMEOUT_MAX 10000
-#define CURL_BLOCK_OPT_URL "url"
-#define CURL_BLOCK_OPT_READAHEAD "readahead"
-#define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
-#define CURL_BLOCK_OPT_TIMEOUT "timeout"
-#define CURL_BLOCK_OPT_COOKIE "cookie"
-#define CURL_BLOCK_OPT_COOKIE_SECRET "cookie-secret"
-#define CURL_BLOCK_OPT_USERNAME "username"
-#define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
-#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
+#define CURL_BLOCK_OPT_URL "url"
+#define CURL_BLOCK_OPT_READAHEAD "readahead"
+#define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
+#define CURL_BLOCK_OPT_TIMEOUT "timeout"
+#define CURL_BLOCK_OPT_COOKIE "cookie"
+#define CURL_BLOCK_OPT_COOKIE_SECRET "cookie-secret"
+#define CURL_BLOCK_OPT_USERNAME "username"
+#define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
+#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
#define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
struct BDRVCURLState;
@@ -110,8 +112,7 @@ typedef struct CURLSocket {
QLIST_ENTRY(CURLSocket) next;
} CURLSocket;
-typedef struct CURLState
-{
+typedef struct CURLState {
struct BDRVCURLState *s;
CURLAIOCB *acb[CURL_NUM_ACB];
CURL *curl;
@@ -196,22 +197,22 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, (int)fd);
switch (action) {
- case CURL_POLL_IN:
- aio_set_fd_handler(s->aio_context, fd, false,
- curl_multi_read, NULL, NULL, state);
- break;
- case CURL_POLL_OUT:
- aio_set_fd_handler(s->aio_context, fd, false,
- NULL, curl_multi_do, NULL, state);
- break;
- case CURL_POLL_INOUT:
- aio_set_fd_handler(s->aio_context, fd, false,
- curl_multi_read, curl_multi_do, NULL, state);
- break;
- case CURL_POLL_REMOVE:
- aio_set_fd_handler(s->aio_context, fd, false,
- NULL, NULL, NULL, NULL);
- break;
+ case CURL_POLL_IN:
+ aio_set_fd_handler(s->aio_context, fd, false,
+ curl_multi_read, NULL, NULL, state);
+ break;
+ case CURL_POLL_OUT:
+ aio_set_fd_handler(s->aio_context, fd, false,
+ NULL, curl_multi_do, NULL, state);
+ break;
+ case CURL_POLL_INOUT:
+ aio_set_fd_handler(s->aio_context, fd, false,
+ curl_multi_read, curl_multi_do, NULL, state);
+ break;
+ case CURL_POLL_REMOVE:
+ aio_set_fd_handler(s->aio_context, fd, false,
+ NULL, NULL, NULL, NULL);
+ break;
}
return 0;
@@ -235,7 +236,7 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
/* Called from curl_multi_do_locked, with s->mutex held. */
static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
{
- CURLState *s = ((CURLState*)opaque);
+ CURLState *s = opaque;
size_t realsize = size * nmemb;
int i;
@@ -253,11 +254,12 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
memcpy(s->orig_buf + s->buf_off, ptr, realsize);
s->buf_off += realsize;
- for(i=0; i<CURL_NUM_ACB; i++) {
+ for (i = 0; i < CURL_NUM_ACB; i++) {
CURLAIOCB *acb = s->acb[i];
- if (!acb)
+ if (!acb) {
continue;
+ }
if ((s->buf_off >= acb->end)) {
size_t request_length = acb->bytes;
@@ -293,17 +295,16 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
uint64_t clamped_end = MIN(end, s->len);
uint64_t clamped_len = clamped_end - start;
- for (i=0; i<CURL_NUM_STATES; i++) {
+ for (i = 0; i < CURL_NUM_STATES; i++) {
CURLState *state = &s->states[i];
uint64_t buf_end = (state->buf_start + state->buf_off);
uint64_t buf_fend = (state->buf_start + state->buf_len);
- if (!state->orig_buf)
- continue;
- if (!state->buf_off)
+ if (!state->orig_buf || !state->buf_off) {
continue;
+ }
- // Does the existing buffer cover our section?
+ /* Does the existing buffer cover our section? */
if ((start >= state->buf_start) &&
(start <= buf_end) &&
(clamped_end >= state->buf_start) &&
@@ -319,7 +320,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
return true;
}
- // Wait for unfinished chunks
+ /* Wait for unfinished chunks */
if (state->in_use &&
(start >= state->buf_start) &&
(start <= buf_fend) &&
@@ -331,7 +332,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
acb->start = start - state->buf_start;
acb->end = acb->start + clamped_len;
- for (j=0; j<CURL_NUM_ACB; j++) {
+ for (j = 0; j < CURL_NUM_ACB; j++) {
if (!state->acb[j]) {
state->acb[j] = acb;
return true;
@@ -355,8 +356,9 @@ static void curl_multi_check_completion(BDRVCURLState *s)
msg = curl_multi_info_read(s->multi, &msgs_in_queue);
/* Quit when there are no more completions */
- if (!msg)
+ if (!msg) {
break;
+ }
if (msg->msg == CURLMSG_DONE) {
CURLState *state = NULL;
@@ -540,12 +542,14 @@ static void curl_clean_state(CURLState *s)
{
CURLAIOCB *next;
int j;
+
for (j = 0; j < CURL_NUM_ACB; j++) {
assert(!s->acb[j]);
}
- if (s->s->multi)
+ if (s->s->multi) {
curl_multi_remove_handle(s->s->multi, s->curl);
+ }
while (!QLIST_EMPTY(&s->sockets)) {
CURLSocket *socket = QLIST_FIRST(&s->sockets);
@@ -794,7 +798,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
goto out_noclean;
}
- // Get file size
+ /* Get file size */
if (curl_init_state(s, state) < 0) {
goto out;
@@ -802,11 +806,11 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
s->accept_range = false;
curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1);
- curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION,
- curl_header_cb);
+ curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION, curl_header_cb);
curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s);
- if (curl_easy_perform(state->curl))
+ if (curl_easy_perform(state->curl)) {
goto out;
+ }
if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d)) {
goto out;
}
@@ -876,13 +880,13 @@ static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
qemu_mutex_lock(&s->mutex);
- // In case we have the requested data already (e.g. read-ahead),
- // we can just call the callback and be done.
+ /* In case we have the requested data already (e.g. read-ahead),
+ we can just call the callback and be done. */
if (curl_find_buf(s, start, acb->bytes, acb)) {
goto out;
}
- // No cache found, so let's start a new request
+ /* No cache found, so let's start a new request */
for (;;) {
state = curl_find_state(s);
if (state) {
--
2.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init()
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init() Jeff Cody
@ 2017-11-07 22:48 ` Eric Blake
2017-11-08 11:39 ` Richard W.M. Jones
1 sibling, 0 replies; 26+ messages in thread
From: Eric Blake @ 2017-11-07 22:48 UTC (permalink / raw)
To: Jeff Cody, qemu-devel
Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf
[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]
On 11/07/2017 04:27 PM, Jeff Cody wrote:
> We don't need libssh2 failure to be fatal (we could just opt to not
> register the driver on failure). But, it is probably a good idea to
> avoid external library calls during the block_init(), and call the
> libssh2 global init function on the first usage, returning any errors.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/ssh.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> +static int ssh_state_init(BDRVSSHState *s, Error **errp)
> {
> + int ret;
> +
> + if (!ssh_libinit_called) {
> + ret = libssh2_init(0);
> + if (ret) {
> + error_setg(errp, "libssh2 initialization failed with %d", ret);
Maybe s/with %d/with status %d/
> + return ret;
This is returning a non-zero value, but not necessarily a negative errno...
> @@ -821,8 +839,13 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
> BDRVSSHState s;
> ssize_t r2;
> char c[1] = { '\0' };
> + Error *local_err = NULL;
>
> - ssh_state_init(&s);
> + ret = ssh_state_init(&s, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return ret;
...but this function wants to return a negative errno. I think you can
rewrite this to:
if (ssh_state_init(&s, errp)) {
return -EIO;
}
and skip out on local_err.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks Jeff Cody
@ 2017-11-08 0:13 ` Eric Blake
2017-11-08 5:08 ` Philippe Mathieu-Daudé
2017-11-08 11:45 ` Richard W.M. Jones
2 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2017-11-08 0:13 UTC (permalink / raw)
To: Jeff Cody, qemu-devel
Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf
[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]
On 11/07/2017 04:27 PM, Jeff Cody wrote:
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/curl.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
>
> diff --git a/block/curl.c b/block/curl.c
> index 00a9879..35cf417 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -857,6 +857,9 @@ out_noclean:
> qemu_mutex_destroy(&s->mutex);
> g_free(s->cookie);
> g_free(s->url);
> + g_free(s->username);
> + g_free(s->proxyusername);
> + g_free(s->proxypassword);
> qemu_opts_del(opts);
> return -EINVAL;
> }
> @@ -955,6 +958,9 @@ static void curl_close(BlockDriverState *bs)
>
> g_free(s->cookie);
> g_free(s->url);
> + g_free(s->username);
> + g_free(s->proxyusername);
> + g_free(s->proxypassword);
> }
>
> static int64_t curl_getlength(BlockDriverState *bs)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks Jeff Cody
2017-11-08 0:13 ` Eric Blake
@ 2017-11-08 5:08 ` Philippe Mathieu-Daudé
2017-11-08 11:45 ` Richard W.M. Jones
2 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-11-08 5:08 UTC (permalink / raw)
To: Jeff Cody, qemu-devel
Cc: kwolf, qemu-block, mitake.hitoshi, rjones, namei.unix
Hi Jeff,
On 11/07/2017 07:27 PM, Jeff Cody wrote:
"Missed in 1bff9606429."
> Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> block/curl.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/block/curl.c b/block/curl.c
> index 00a9879..35cf417 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -857,6 +857,9 @@ out_noclean:
> qemu_mutex_destroy(&s->mutex);
> g_free(s->cookie);
> g_free(s->url);
> + g_free(s->username);
> + g_free(s->proxyusername);
> + g_free(s->proxypassword);
> qemu_opts_del(opts);
> return -EINVAL;
> }
> @@ -955,6 +958,9 @@ static void curl_close(BlockDriverState *bs)
>
> g_free(s->cookie);
> g_free(s->url);
> + g_free(s->username);
> + g_free(s->proxyusername);
> + g_free(s->proxypassword);
> }
>
> static int64_t curl_getlength(BlockDriverState *bs)
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style Jeff Cody
@ 2017-11-08 10:47 ` Darren Kenny
2017-11-08 14:26 ` Eric Blake
2017-11-08 15:01 ` Paolo Bonzini
2017-11-08 11:46 ` [Qemu-devel] " Richard W.M. Jones
1 sibling, 2 replies; 26+ messages in thread
From: Darren Kenny @ 2017-11-08 10:47 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, kwolf, qemu-block, mitake.hitoshi, rjones, namei.unix
Hi Jeff,
While I'm relatively new to this community, I do have some comments
about the styling in this file.
I don't see anything in the CODING_STYLE file that tells me I'm
wrong here, but it's certainly possible...
More inline.
On Tue, Nov 07, 2017 at 05:27:24PM -0500, Jeff Cody wrote:
>This addresses non-functional changes to help curl.c better comply
>with the coding styles (comments, indentation, brackets, etc.).
>
>One minor code change is the combination of two if statements into
>a single if statement.
>
>Signed-off-by: Jeff Cody <jcody@redhat.com>
>Reviewed-by: Eric Blake <eblake@redhat.com>
>---
> block/curl.c | 100 +++++++++++++++++++++++++++++++----------------------------
> 1 file changed, 52 insertions(+), 48 deletions(-)
>
>diff --git a/block/curl.c b/block/curl.c
>index 35cf417..7a6dd44 100644
>--- a/block/curl.c
>+++ b/block/curl.c
>@@ -32,8 +32,10 @@
> #include <curl/curl.h>
> #include "qemu/cutils.h"
>
>-// #define DEBUG_CURL
>-// #define DEBUG_VERBOSE
>+/*
>+ #define DEBUG_CURL
>+ #define DEBUG_VERBOSE
>+*/
Is it not more common to use the style:
/*
* text
*/
This is visible in almost every file at the top, where the Copyright
and License is.
>
> #ifdef DEBUG_CURL
> #define DEBUG_CURL_PRINT 1
>@@ -76,15 +78,15 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
> #define CURL_TIMEOUT_DEFAULT 5
> #define CURL_TIMEOUT_MAX 10000
>
>-#define CURL_BLOCK_OPT_URL "url"
>-#define CURL_BLOCK_OPT_READAHEAD "readahead"
>-#define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
>-#define CURL_BLOCK_OPT_TIMEOUT "timeout"
>-#define CURL_BLOCK_OPT_COOKIE "cookie"
>-#define CURL_BLOCK_OPT_COOKIE_SECRET "cookie-secret"
>-#define CURL_BLOCK_OPT_USERNAME "username"
>-#define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
>-#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
>+#define CURL_BLOCK_OPT_URL "url"
>+#define CURL_BLOCK_OPT_READAHEAD "readahead"
>+#define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
>+#define CURL_BLOCK_OPT_TIMEOUT "timeout"
>+#define CURL_BLOCK_OPT_COOKIE "cookie"
>+#define CURL_BLOCK_OPT_COOKIE_SECRET "cookie-secret"
>+#define CURL_BLOCK_OPT_USERNAME "username"
>+#define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
>+#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
> #define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
>
> struct BDRVCURLState;
>@@ -110,8 +112,7 @@ typedef struct CURLSocket {
> QLIST_ENTRY(CURLSocket) next;
> } CURLSocket;
>
>-typedef struct CURLState
>-{
>+typedef struct CURLState {
> struct BDRVCURLState *s;
> CURLAIOCB *acb[CURL_NUM_ACB];
> CURL *curl;
>@@ -196,22 +197,22 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
>
> DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, (int)fd);
> switch (action) {
>- case CURL_POLL_IN:
>- aio_set_fd_handler(s->aio_context, fd, false,
>- curl_multi_read, NULL, NULL, state);
>- break;
>- case CURL_POLL_OUT:
>- aio_set_fd_handler(s->aio_context, fd, false,
>- NULL, curl_multi_do, NULL, state);
>- break;
>- case CURL_POLL_INOUT:
>- aio_set_fd_handler(s->aio_context, fd, false,
>- curl_multi_read, curl_multi_do, NULL, state);
>- break;
>- case CURL_POLL_REMOVE:
>- aio_set_fd_handler(s->aio_context, fd, false,
>- NULL, NULL, NULL, NULL);
>- break;
>+ case CURL_POLL_IN:
>+ aio_set_fd_handler(s->aio_context, fd, false,
>+ curl_multi_read, NULL, NULL, state);
>+ break;
>+ case CURL_POLL_OUT:
>+ aio_set_fd_handler(s->aio_context, fd, false,
>+ NULL, curl_multi_do, NULL, state);
>+ break;
>+ case CURL_POLL_INOUT:
>+ aio_set_fd_handler(s->aio_context, fd, false,
>+ curl_multi_read, curl_multi_do, NULL, state);
>+ break;
>+ case CURL_POLL_REMOVE:
>+ aio_set_fd_handler(s->aio_context, fd, false,
>+ NULL, NULL, NULL, NULL);
>+ break;
> }
>
> return 0;
>@@ -235,7 +236,7 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
> /* Called from curl_multi_do_locked, with s->mutex held. */
> static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
> {
>- CURLState *s = ((CURLState*)opaque);
>+ CURLState *s = opaque;
Not sure about this one - while it may not be strictly necessary to
do the casting, from a readability point of view it is helpful to
see the cast - possibly with the outer brackets removed:
CURLState *s = (CURLState*)opaque;
> size_t realsize = size * nmemb;
> int i;
>
>@@ -253,11 +254,12 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
> memcpy(s->orig_buf + s->buf_off, ptr, realsize);
> s->buf_off += realsize;
>
>- for(i=0; i<CURL_NUM_ACB; i++) {
>+ for (i = 0; i < CURL_NUM_ACB; i++) {
> CURLAIOCB *acb = s->acb[i];
>
>- if (!acb)
>+ if (!acb) {
> continue;
>+ }
>
> if ((s->buf_off >= acb->end)) {
> size_t request_length = acb->bytes;
>@@ -293,17 +295,16 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
> uint64_t clamped_end = MIN(end, s->len);
> uint64_t clamped_len = clamped_end - start;
>
>- for (i=0; i<CURL_NUM_STATES; i++) {
>+ for (i = 0; i < CURL_NUM_STATES; i++) {
> CURLState *state = &s->states[i];
> uint64_t buf_end = (state->buf_start + state->buf_off);
> uint64_t buf_fend = (state->buf_start + state->buf_len);
>
>- if (!state->orig_buf)
>- continue;
>- if (!state->buf_off)
>+ if (!state->orig_buf || !state->buf_off) {
> continue;
>+ }
>
>- // Does the existing buffer cover our section?
>+ /* Does the existing buffer cover our section? */
> if ((start >= state->buf_start) &&
> (start <= buf_end) &&
> (clamped_end >= state->buf_start) &&
>@@ -319,7 +320,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
> return true;
> }
>
>- // Wait for unfinished chunks
>+ /* Wait for unfinished chunks */
> if (state->in_use &&
> (start >= state->buf_start) &&
> (start <= buf_fend) &&
>@@ -331,7 +332,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
> acb->start = start - state->buf_start;
> acb->end = acb->start + clamped_len;
>
>- for (j=0; j<CURL_NUM_ACB; j++) {
>+ for (j = 0; j < CURL_NUM_ACB; j++) {
> if (!state->acb[j]) {
> state->acb[j] = acb;
> return true;
>@@ -355,8 +356,9 @@ static void curl_multi_check_completion(BDRVCURLState *s)
> msg = curl_multi_info_read(s->multi, &msgs_in_queue);
>
> /* Quit when there are no more completions */
>- if (!msg)
>+ if (!msg) {
> break;
>+ }
>
> if (msg->msg == CURLMSG_DONE) {
> CURLState *state = NULL;
>@@ -540,12 +542,14 @@ static void curl_clean_state(CURLState *s)
> {
> CURLAIOCB *next;
> int j;
>+
> for (j = 0; j < CURL_NUM_ACB; j++) {
> assert(!s->acb[j]);
> }
>
>- if (s->s->multi)
>+ if (s->s->multi) {
> curl_multi_remove_handle(s->s->multi, s->curl);
>+ }
>
> while (!QLIST_EMPTY(&s->sockets)) {
> CURLSocket *socket = QLIST_FIRST(&s->sockets);
>@@ -794,7 +798,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
> goto out_noclean;
> }
>
>- // Get file size
>+ /* Get file size */
>
> if (curl_init_state(s, state) < 0) {
> goto out;
>@@ -802,11 +806,11 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
>
> s->accept_range = false;
> curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1);
>- curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION,
>- curl_header_cb);
>+ curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION, curl_header_cb);
> curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s);
>- if (curl_easy_perform(state->curl))
>+ if (curl_easy_perform(state->curl)) {
> goto out;
>+ }
> if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d)) {
> goto out;
> }
>@@ -876,13 +880,13 @@ static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
>
> qemu_mutex_lock(&s->mutex);
>
>- // In case we have the requested data already (e.g. read-ahead),
>- // we can just call the callback and be done.
>+ /* In case we have the requested data already (e.g. read-ahead),
>+ we can just call the callback and be done. */
Please don't do a multi-line comment like this, it is much more
obvious that the second line is a comment if you use the more normal
format of as outlined earlier by me.
Thanks,
Darren.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 3/7] block/sheepdog: remove spurious NULL check
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 3/7] block/sheepdog: remove spurious NULL check Jeff Cody
@ 2017-11-08 10:48 ` Darren Kenny
0 siblings, 0 replies; 26+ messages in thread
From: Darren Kenny @ 2017-11-08 10:48 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, kwolf, qemu-block, mitake.hitoshi, rjones, namei.unix
On Tue, Nov 07, 2017 at 05:27:20PM -0500, Jeff Cody wrote:
>'tag' is already checked in the lines immediately preceding this check,
>and set to non-NULL if NULL. No need to check again, it hasn't changed.
>
>Signed-off-by: Jeff Cody <jcody@redhat.com>
>Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
>---
> block/sheepdog.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/block/sheepdog.c b/block/sheepdog.c
>index 696a714..459d93a 100644
>--- a/block/sheepdog.c
>+++ b/block/sheepdog.c
>@@ -1632,7 +1632,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
> if (!tag) {
> tag = "";
> }
>- if (tag && strlen(tag) >= SD_MAX_VDI_TAG_LEN) {
>+ if (strlen(tag) >= SD_MAX_VDI_TAG_LEN) {
> error_setg(errp, "value of parameter 'tag' is too long");
> ret = -EINVAL;
> goto err_no_fd;
>--
>2.9.5
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines Jeff Cody
@ 2017-11-08 10:50 ` Darren Kenny
2017-11-08 11:38 ` [Qemu-devel] " Richard W.M. Jones
1 sibling, 0 replies; 26+ messages in thread
From: Darren Kenny @ 2017-11-08 10:50 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, kwolf, qemu-block, mitake.hitoshi, rjones, namei.unix
On Tue, Nov 07, 2017 at 05:27:19PM -0500, Jeff Cody wrote:
>Signed-off-by: Jeff Cody <jcody@redhat.com>
>Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
>---
> block/ssh.c | 32 ++++++++++++++++++--------------
> 1 file changed, 18 insertions(+), 14 deletions(-)
>
>diff --git a/block/ssh.c b/block/ssh.c
>index de81ec8..39cacc1 100644
>--- a/block/ssh.c
>+++ b/block/ssh.c
>@@ -241,7 +241,7 @@ static int parse_uri(const char *filename, QDict *options, Error **errp)
> goto err;
> }
>
>- if(uri->user && strcmp(uri->user, "") != 0) {
>+ if (uri->user && strcmp(uri->user, "") != 0) {
> qdict_put_str(options, "user", uri->user);
> }
>
>@@ -268,7 +268,7 @@ static int parse_uri(const char *filename, QDict *options, Error **errp)
>
> err:
> if (uri) {
>- uri_free(uri);
>+ uri_free(uri);
> }
> return -EINVAL;
> }
>@@ -342,7 +342,7 @@ static int check_host_key_knownhosts(BDRVSSHState *s,
> libssh2_knownhost_readfile(knh, knh_file, LIBSSH2_KNOWNHOST_FILE_OPENSSH);
>
> r = libssh2_knownhost_checkp(knh, host, port, hostkey, len,
>- LIBSSH2_KNOWNHOST_TYPE_PLAIN|
>+ LIBSSH2_KNOWNHOST_TYPE_PLAIN |
> LIBSSH2_KNOWNHOST_KEYENC_RAW,
> &found);
> switch (r) {
>@@ -405,15 +405,18 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len,
> unsigned c;
>
> while (len > 0) {
>- while (*host_key_check == ':')
>+ while (*host_key_check == ':') {
> host_key_check++;
>+ }
> if (!qemu_isxdigit(host_key_check[0]) ||
>- !qemu_isxdigit(host_key_check[1]))
>+ !qemu_isxdigit(host_key_check[1])) {
> return 1;
>+ }
> c = hex2decimal(host_key_check[0]) * 16 +
> hex2decimal(host_key_check[1]);
>- if (c - *fingerprint != 0)
>+ if (c - *fingerprint != 0) {
> return c - *fingerprint;
>+ }
> fingerprint++;
> len--;
> host_key_check += 2;
>@@ -433,8 +436,8 @@ check_host_key_hash(BDRVSSHState *s, const char *hash,
> return -EINVAL;
> }
>
>- if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_len,
>- hash) != 0) {
>+ if (compare_fingerprint((unsigned char *) fingerprint, fingerprint_len,
>+ hash) != 0) {
> error_setg(errp, "remote host key does not match host_key_check '%s'",
> hash);
> return -EPERM;
>@@ -507,7 +510,7 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp)
> goto out;
> }
>
>- for(;;) {
>+ for (;;) {
> r = libssh2_agent_get_identity(agent, &identity, prev_identity);
> if (r == 1) { /* end of list */
> break;
>@@ -860,8 +863,8 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
> }
>
> r = connect_to_ssh(&s, uri_options,
>- LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE|
>- LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC,
>+ LIBSSH2_FXF_READ | LIBSSH2_FXF_WRITE |
>+ LIBSSH2_FXF_CREAT | LIBSSH2_FXF_TRUNC,
> 0644, errp);
> if (r < 0) {
> ret = r;
>@@ -869,7 +872,7 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
> }
>
> if (total_size > 0) {
>- libssh2_sftp_seek64(s.sftp_handle, total_size-1);
>+ libssh2_sftp_seek64(s.sftp_handle, total_size - 1);
> r2 = libssh2_sftp_write(s.sftp_handle, c, 1);
> if (r2 < 0) {
> sftp_error_setg(errp, &s, "truncate failed");
>@@ -1108,7 +1111,7 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
> * works for me.
> */
> if (r == 0) {
>- ssh_seek(s, offset + written, SSH_SEEK_WRITE|SSH_SEEK_FORCE);
>+ ssh_seek(s, offset + written, SSH_SEEK_WRITE | SSH_SEEK_FORCE);
> co_yield(s, bs);
> goto again;
> }
>@@ -1122,8 +1125,9 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
> end_of_vec = i->iov_base + i->iov_len;
> }
>
>- if (offset + written > s->attrs.filesize)
>+ if (offset + written > s->attrs.filesize) {
> s->attrs.filesize = offset + written;
>+ }
> }
>
> return 0;
>--
>2.9.5
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 5/7] block/curl: check error return of curl_global_init()
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 5/7] block/curl: check error return of curl_global_init() Jeff Cody
@ 2017-11-08 10:51 ` Darren Kenny
2017-11-08 11:41 ` [Qemu-devel] " Richard W.M. Jones
1 sibling, 0 replies; 26+ messages in thread
From: Darren Kenny @ 2017-11-08 10:51 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, kwolf, qemu-block, mitake.hitoshi, rjones, namei.unix
On Tue, Nov 07, 2017 at 05:27:22PM -0500, Jeff Cody wrote:
>If curl_global_init() fails, per the documentation no other curl
>functions may be called, so make sure to check the return value.
>
>Also, some minor changes to the initialization latch variable 'inited':
>
>- Make it static in the file, for clarity
>- Change the name for clarity
>- Make it a bool
>
>Signed-off-by: Jeff Cody <jcody@redhat.com>
>Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
>---
> block/curl.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
>diff --git a/block/curl.c b/block/curl.c
>index 2a244e2..00a9879 100644
>--- a/block/curl.c
>+++ b/block/curl.c
>@@ -89,6 +89,8 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
>
> struct BDRVCURLState;
>
>+static bool libcurl_initialized;
>+
> typedef struct CURLAIOCB {
> Coroutine *co;
> QEMUIOVector *qiov;
>@@ -686,14 +688,23 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
> double d;
> const char *secretid;
> const char *protocol_delimiter;
>+ int ret;
>
>- static int inited = 0;
>
> if (flags & BDRV_O_RDWR) {
> error_setg(errp, "curl block device does not support writes");
> return -EROFS;
> }
>
>+ if (!libcurl_initialized) {
>+ ret = curl_global_init(CURL_GLOBAL_ALL);
>+ if (ret) {
>+ error_setg(errp, "libcurl initialization failed with %d", ret);
>+ return -EIO;
>+ }
>+ libcurl_initialized = true;
>+ }
>+
> qemu_mutex_init(&s->mutex);
> opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> qemu_opts_absorb_qdict(opts, options, &local_err);
>@@ -772,11 +783,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
> }
> }
>
>- if (!inited) {
>- curl_global_init(CURL_GLOBAL_ALL);
>- inited = 1;
>- }
>-
> DPRINTF("CURL: Opening %s\n", file);
> QSIMPLEQ_INIT(&s->free_state_waitq);
> s->aio_context = bdrv_get_aio_context(bs);
>--
>2.9.5
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 4/7] block/sheepdog: code beautification
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 4/7] block/sheepdog: code beautification Jeff Cody
@ 2017-11-08 10:52 ` Darren Kenny
0 siblings, 0 replies; 26+ messages in thread
From: Darren Kenny @ 2017-11-08 10:52 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, kwolf, qemu-block, mitake.hitoshi, rjones, namei.unix
On Tue, Nov 07, 2017 at 05:27:21PM -0500, Jeff Cody wrote:
>No functional changes, just whitespace manipulation.
>
>Signed-off-by: Jeff Cody <jcody@redhat.com>
>Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
>---
> block/sheepdog.c | 164 +++++++++++++++++++++++++++----------------------------
> 1 file changed, 82 insertions(+), 82 deletions(-)
>
>diff --git a/block/sheepdog.c b/block/sheepdog.c
>index 459d93a..488bad3 100644
>--- a/block/sheepdog.c
>+++ b/block/sheepdog.c
>@@ -400,7 +400,7 @@ typedef struct BDRVSheepdogReopenState {
> int cache_flags;
> } BDRVSheepdogReopenState;
>
>-static const char * sd_strerror(int err)
>+static const char *sd_strerror(int err)
> {
> int i;
>
>@@ -3078,111 +3078,111 @@ static QemuOptsList sd_create_opts = {
> };
>
> static BlockDriver bdrv_sheepdog = {
>- .format_name = "sheepdog",
>- .protocol_name = "sheepdog",
>- .instance_size = sizeof(BDRVSheepdogState),
>- .bdrv_parse_filename = sd_parse_filename,
>- .bdrv_file_open = sd_open,
>- .bdrv_reopen_prepare = sd_reopen_prepare,
>- .bdrv_reopen_commit = sd_reopen_commit,
>- .bdrv_reopen_abort = sd_reopen_abort,
>- .bdrv_close = sd_close,
>- .bdrv_create = sd_create,
>- .bdrv_has_zero_init = bdrv_has_zero_init_1,
>- .bdrv_getlength = sd_getlength,
>+ .format_name = "sheepdog",
>+ .protocol_name = "sheepdog",
>+ .instance_size = sizeof(BDRVSheepdogState),
>+ .bdrv_parse_filename = sd_parse_filename,
>+ .bdrv_file_open = sd_open,
>+ .bdrv_reopen_prepare = sd_reopen_prepare,
>+ .bdrv_reopen_commit = sd_reopen_commit,
>+ .bdrv_reopen_abort = sd_reopen_abort,
>+ .bdrv_close = sd_close,
>+ .bdrv_create = sd_create,
>+ .bdrv_has_zero_init = bdrv_has_zero_init_1,
>+ .bdrv_getlength = sd_getlength,
> .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
>- .bdrv_truncate = sd_truncate,
>+ .bdrv_truncate = sd_truncate,
>
>- .bdrv_co_readv = sd_co_readv,
>- .bdrv_co_writev = sd_co_writev,
>- .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
>- .bdrv_co_pdiscard = sd_co_pdiscard,
>- .bdrv_co_get_block_status = sd_co_get_block_status,
>+ .bdrv_co_readv = sd_co_readv,
>+ .bdrv_co_writev = sd_co_writev,
>+ .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
>+ .bdrv_co_pdiscard = sd_co_pdiscard,
>+ .bdrv_co_get_block_status = sd_co_get_block_status,
>
>- .bdrv_snapshot_create = sd_snapshot_create,
>- .bdrv_snapshot_goto = sd_snapshot_goto,
>- .bdrv_snapshot_delete = sd_snapshot_delete,
>- .bdrv_snapshot_list = sd_snapshot_list,
>+ .bdrv_snapshot_create = sd_snapshot_create,
>+ .bdrv_snapshot_goto = sd_snapshot_goto,
>+ .bdrv_snapshot_delete = sd_snapshot_delete,
>+ .bdrv_snapshot_list = sd_snapshot_list,
>
>- .bdrv_save_vmstate = sd_save_vmstate,
>- .bdrv_load_vmstate = sd_load_vmstate,
>+ .bdrv_save_vmstate = sd_save_vmstate,
>+ .bdrv_load_vmstate = sd_load_vmstate,
>
>- .bdrv_detach_aio_context = sd_detach_aio_context,
>- .bdrv_attach_aio_context = sd_attach_aio_context,
>+ .bdrv_detach_aio_context = sd_detach_aio_context,
>+ .bdrv_attach_aio_context = sd_attach_aio_context,
>
>- .create_opts = &sd_create_opts,
>+ .create_opts = &sd_create_opts,
> };
>
> static BlockDriver bdrv_sheepdog_tcp = {
>- .format_name = "sheepdog",
>- .protocol_name = "sheepdog+tcp",
>- .instance_size = sizeof(BDRVSheepdogState),
>- .bdrv_parse_filename = sd_parse_filename,
>- .bdrv_file_open = sd_open,
>- .bdrv_reopen_prepare = sd_reopen_prepare,
>- .bdrv_reopen_commit = sd_reopen_commit,
>- .bdrv_reopen_abort = sd_reopen_abort,
>- .bdrv_close = sd_close,
>- .bdrv_create = sd_create,
>- .bdrv_has_zero_init = bdrv_has_zero_init_1,
>- .bdrv_getlength = sd_getlength,
>+ .format_name = "sheepdog",
>+ .protocol_name = "sheepdog+tcp",
>+ .instance_size = sizeof(BDRVSheepdogState),
>+ .bdrv_parse_filename = sd_parse_filename,
>+ .bdrv_file_open = sd_open,
>+ .bdrv_reopen_prepare = sd_reopen_prepare,
>+ .bdrv_reopen_commit = sd_reopen_commit,
>+ .bdrv_reopen_abort = sd_reopen_abort,
>+ .bdrv_close = sd_close,
>+ .bdrv_create = sd_create,
>+ .bdrv_has_zero_init = bdrv_has_zero_init_1,
>+ .bdrv_getlength = sd_getlength,
> .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
>- .bdrv_truncate = sd_truncate,
>+ .bdrv_truncate = sd_truncate,
>
>- .bdrv_co_readv = sd_co_readv,
>- .bdrv_co_writev = sd_co_writev,
>- .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
>- .bdrv_co_pdiscard = sd_co_pdiscard,
>- .bdrv_co_get_block_status = sd_co_get_block_status,
>+ .bdrv_co_readv = sd_co_readv,
>+ .bdrv_co_writev = sd_co_writev,
>+ .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
>+ .bdrv_co_pdiscard = sd_co_pdiscard,
>+ .bdrv_co_get_block_status = sd_co_get_block_status,
>
>- .bdrv_snapshot_create = sd_snapshot_create,
>- .bdrv_snapshot_goto = sd_snapshot_goto,
>- .bdrv_snapshot_delete = sd_snapshot_delete,
>- .bdrv_snapshot_list = sd_snapshot_list,
>+ .bdrv_snapshot_create = sd_snapshot_create,
>+ .bdrv_snapshot_goto = sd_snapshot_goto,
>+ .bdrv_snapshot_delete = sd_snapshot_delete,
>+ .bdrv_snapshot_list = sd_snapshot_list,
>
>- .bdrv_save_vmstate = sd_save_vmstate,
>- .bdrv_load_vmstate = sd_load_vmstate,
>+ .bdrv_save_vmstate = sd_save_vmstate,
>+ .bdrv_load_vmstate = sd_load_vmstate,
>
>- .bdrv_detach_aio_context = sd_detach_aio_context,
>- .bdrv_attach_aio_context = sd_attach_aio_context,
>+ .bdrv_detach_aio_context = sd_detach_aio_context,
>+ .bdrv_attach_aio_context = sd_attach_aio_context,
>
>- .create_opts = &sd_create_opts,
>+ .create_opts = &sd_create_opts,
> };
>
> static BlockDriver bdrv_sheepdog_unix = {
>- .format_name = "sheepdog",
>- .protocol_name = "sheepdog+unix",
>- .instance_size = sizeof(BDRVSheepdogState),
>- .bdrv_parse_filename = sd_parse_filename,
>- .bdrv_file_open = sd_open,
>- .bdrv_reopen_prepare = sd_reopen_prepare,
>- .bdrv_reopen_commit = sd_reopen_commit,
>- .bdrv_reopen_abort = sd_reopen_abort,
>- .bdrv_close = sd_close,
>- .bdrv_create = sd_create,
>- .bdrv_has_zero_init = bdrv_has_zero_init_1,
>- .bdrv_getlength = sd_getlength,
>+ .format_name = "sheepdog",
>+ .protocol_name = "sheepdog+unix",
>+ .instance_size = sizeof(BDRVSheepdogState),
>+ .bdrv_parse_filename = sd_parse_filename,
>+ .bdrv_file_open = sd_open,
>+ .bdrv_reopen_prepare = sd_reopen_prepare,
>+ .bdrv_reopen_commit = sd_reopen_commit,
>+ .bdrv_reopen_abort = sd_reopen_abort,
>+ .bdrv_close = sd_close,
>+ .bdrv_create = sd_create,
>+ .bdrv_has_zero_init = bdrv_has_zero_init_1,
>+ .bdrv_getlength = sd_getlength,
> .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
>- .bdrv_truncate = sd_truncate,
>+ .bdrv_truncate = sd_truncate,
>
>- .bdrv_co_readv = sd_co_readv,
>- .bdrv_co_writev = sd_co_writev,
>- .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
>- .bdrv_co_pdiscard = sd_co_pdiscard,
>- .bdrv_co_get_block_status = sd_co_get_block_status,
>+ .bdrv_co_readv = sd_co_readv,
>+ .bdrv_co_writev = sd_co_writev,
>+ .bdrv_co_flush_to_disk = sd_co_flush_to_disk,
>+ .bdrv_co_pdiscard = sd_co_pdiscard,
>+ .bdrv_co_get_block_status = sd_co_get_block_status,
>
>- .bdrv_snapshot_create = sd_snapshot_create,
>- .bdrv_snapshot_goto = sd_snapshot_goto,
>- .bdrv_snapshot_delete = sd_snapshot_delete,
>- .bdrv_snapshot_list = sd_snapshot_list,
>+ .bdrv_snapshot_create = sd_snapshot_create,
>+ .bdrv_snapshot_goto = sd_snapshot_goto,
>+ .bdrv_snapshot_delete = sd_snapshot_delete,
>+ .bdrv_snapshot_list = sd_snapshot_list,
>
>- .bdrv_save_vmstate = sd_save_vmstate,
>- .bdrv_load_vmstate = sd_load_vmstate,
>+ .bdrv_save_vmstate = sd_save_vmstate,
>+ .bdrv_load_vmstate = sd_load_vmstate,
>
>- .bdrv_detach_aio_context = sd_detach_aio_context,
>- .bdrv_attach_aio_context = sd_attach_aio_context,
>+ .bdrv_detach_aio_context = sd_detach_aio_context,
>+ .bdrv_attach_aio_context = sd_attach_aio_context,
>
>- .create_opts = &sd_create_opts,
>+ .create_opts = &sd_create_opts,
> };
>
> static void bdrv_sheepdog_init(void)
>--
>2.9.5
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines Jeff Cody
2017-11-08 10:50 ` [Qemu-devel] [Qemu-block] " Darren Kenny
@ 2017-11-08 11:38 ` Richard W.M. Jones
1 sibling, 0 replies; 26+ messages in thread
From: Richard W.M. Jones @ 2017-11-08 11:38 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, qemu-block, mitake.hitoshi, namei.unix, kwolf, eblake
On Tue, Nov 07, 2017 at 05:27:19PM -0500, Jeff Cody wrote:
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
This one is just simple coding style fixes, so:
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Rich.
> block/ssh.c | 32 ++++++++++++++++++--------------
> 1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index de81ec8..39cacc1 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -241,7 +241,7 @@ static int parse_uri(const char *filename, QDict *options, Error **errp)
> goto err;
> }
>
> - if(uri->user && strcmp(uri->user, "") != 0) {
> + if (uri->user && strcmp(uri->user, "") != 0) {
> qdict_put_str(options, "user", uri->user);
> }
>
> @@ -268,7 +268,7 @@ static int parse_uri(const char *filename, QDict *options, Error **errp)
>
> err:
> if (uri) {
> - uri_free(uri);
> + uri_free(uri);
> }
> return -EINVAL;
> }
> @@ -342,7 +342,7 @@ static int check_host_key_knownhosts(BDRVSSHState *s,
> libssh2_knownhost_readfile(knh, knh_file, LIBSSH2_KNOWNHOST_FILE_OPENSSH);
>
> r = libssh2_knownhost_checkp(knh, host, port, hostkey, len,
> - LIBSSH2_KNOWNHOST_TYPE_PLAIN|
> + LIBSSH2_KNOWNHOST_TYPE_PLAIN |
> LIBSSH2_KNOWNHOST_KEYENC_RAW,
> &found);
> switch (r) {
> @@ -405,15 +405,18 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len,
> unsigned c;
>
> while (len > 0) {
> - while (*host_key_check == ':')
> + while (*host_key_check == ':') {
> host_key_check++;
> + }
> if (!qemu_isxdigit(host_key_check[0]) ||
> - !qemu_isxdigit(host_key_check[1]))
> + !qemu_isxdigit(host_key_check[1])) {
> return 1;
> + }
> c = hex2decimal(host_key_check[0]) * 16 +
> hex2decimal(host_key_check[1]);
> - if (c - *fingerprint != 0)
> + if (c - *fingerprint != 0) {
> return c - *fingerprint;
> + }
> fingerprint++;
> len--;
> host_key_check += 2;
> @@ -433,8 +436,8 @@ check_host_key_hash(BDRVSSHState *s, const char *hash,
> return -EINVAL;
> }
>
> - if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_len,
> - hash) != 0) {
> + if (compare_fingerprint((unsigned char *) fingerprint, fingerprint_len,
> + hash) != 0) {
> error_setg(errp, "remote host key does not match host_key_check '%s'",
> hash);
> return -EPERM;
> @@ -507,7 +510,7 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp)
> goto out;
> }
>
> - for(;;) {
> + for (;;) {
> r = libssh2_agent_get_identity(agent, &identity, prev_identity);
> if (r == 1) { /* end of list */
> break;
> @@ -860,8 +863,8 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
> }
>
> r = connect_to_ssh(&s, uri_options,
> - LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE|
> - LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC,
> + LIBSSH2_FXF_READ | LIBSSH2_FXF_WRITE |
> + LIBSSH2_FXF_CREAT | LIBSSH2_FXF_TRUNC,
> 0644, errp);
> if (r < 0) {
> ret = r;
> @@ -869,7 +872,7 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
> }
>
> if (total_size > 0) {
> - libssh2_sftp_seek64(s.sftp_handle, total_size-1);
> + libssh2_sftp_seek64(s.sftp_handle, total_size - 1);
> r2 = libssh2_sftp_write(s.sftp_handle, c, 1);
> if (r2 < 0) {
> sftp_error_setg(errp, &s, "truncate failed");
> @@ -1108,7 +1111,7 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
> * works for me.
> */
> if (r == 0) {
> - ssh_seek(s, offset + written, SSH_SEEK_WRITE|SSH_SEEK_FORCE);
> + ssh_seek(s, offset + written, SSH_SEEK_WRITE | SSH_SEEK_FORCE);
> co_yield(s, bs);
> goto again;
> }
> @@ -1122,8 +1125,9 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
> end_of_vec = i->iov_base + i->iov_len;
> }
>
> - if (offset + written > s->attrs.filesize)
> + if (offset + written > s->attrs.filesize) {
> s->attrs.filesize = offset + written;
> + }
> }
>
> return 0;
> --
> 2.9.5
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init()
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init() Jeff Cody
2017-11-07 22:48 ` Eric Blake
@ 2017-11-08 11:39 ` Richard W.M. Jones
1 sibling, 0 replies; 26+ messages in thread
From: Richard W.M. Jones @ 2017-11-08 11:39 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, qemu-block, mitake.hitoshi, namei.unix, kwolf, eblake
On Tue, Nov 07, 2017 at 05:27:18PM -0500, Jeff Cody wrote:
> We don't need libssh2 failure to be fatal (we could just opt to not
> register the driver on failure). But, it is probably a good idea to
> avoid external library calls during the block_init(), and call the
> libssh2 global init function on the first usage, returning any errors.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/ssh.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index b049a16..de81ec8 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -83,12 +83,28 @@ typedef struct BDRVSSHState {
> bool unsafe_flush_warning;
> } BDRVSSHState;
>
> -static void ssh_state_init(BDRVSSHState *s)
> +static bool ssh_libinit_called;
> +
> +static int ssh_state_init(BDRVSSHState *s, Error **errp)
> {
> + int ret;
> +
> + if (!ssh_libinit_called) {
> + ret = libssh2_init(0);
> + if (ret) {
> + error_setg(errp, "libssh2 initialization failed with %d", ret);
> + return ret;
> + }
> + ssh_libinit_called = true;
> + }
> +
> +
> memset(s, 0, sizeof *s);
> s->sock = -1;
> s->offset = -1;
> qemu_co_mutex_init(&s->lock);
> +
> + return 0;
Is this thread safe? Is there a case where multiple ssh URL opens
could race off against each other and we could end up calling
libssh2_init in parallel? (According to the libssh2_init
documentation, the function is not thread-safe so should not be called
in parallel on multiple threads)
Rich.
> }
>
> static void ssh_state_free(BDRVSSHState *s)
> @@ -773,7 +789,9 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
> int ret;
> int ssh_flags;
>
> - ssh_state_init(s);
> + if (ssh_state_init(s, errp)) {
> + return -EIO;
> + }
>
> ssh_flags = LIBSSH2_FXF_READ;
> if (bdrv_flags & BDRV_O_RDWR) {
> @@ -821,8 +839,13 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
> BDRVSSHState s;
> ssize_t r2;
> char c[1] = { '\0' };
> + Error *local_err = NULL;
>
> - ssh_state_init(&s);
> + ret = ssh_state_init(&s, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return ret;
> + }
>
> /* Get desired file size. */
> total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> @@ -1213,14 +1236,6 @@ static BlockDriver bdrv_ssh = {
>
> static void bdrv_ssh_init(void)
> {
> - int r;
> -
> - r = libssh2_init(0);
> - if (r != 0) {
> - fprintf(stderr, "libssh2 initialization failed, %d\n", r);
> - exit(EXIT_FAILURE);
> - }
> -
> bdrv_register(&bdrv_ssh);
> }
>
> --
> 2.9.5
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/7] block/curl: check error return of curl_global_init()
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 5/7] block/curl: check error return of curl_global_init() Jeff Cody
2017-11-08 10:51 ` [Qemu-devel] [Qemu-block] " Darren Kenny
@ 2017-11-08 11:41 ` Richard W.M. Jones
1 sibling, 0 replies; 26+ messages in thread
From: Richard W.M. Jones @ 2017-11-08 11:41 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, qemu-block, mitake.hitoshi, namei.unix, kwolf, eblake
On Tue, Nov 07, 2017 at 05:27:22PM -0500, Jeff Cody wrote:
> If curl_global_init() fails, per the documentation no other curl
> functions may be called, so make sure to check the return value.
>
> Also, some minor changes to the initialization latch variable 'inited':
>
> - Make it static in the file, for clarity
> - Change the name for clarity
> - Make it a bool
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
This is just a simple evolution of the previous code, so:
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Rich.
> block/curl.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/block/curl.c b/block/curl.c
> index 2a244e2..00a9879 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -89,6 +89,8 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
>
> struct BDRVCURLState;
>
> +static bool libcurl_initialized;
> +
> typedef struct CURLAIOCB {
> Coroutine *co;
> QEMUIOVector *qiov;
> @@ -686,14 +688,23 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
> double d;
> const char *secretid;
> const char *protocol_delimiter;
> + int ret;
>
> - static int inited = 0;
>
> if (flags & BDRV_O_RDWR) {
> error_setg(errp, "curl block device does not support writes");
> return -EROFS;
> }
>
> + if (!libcurl_initialized) {
> + ret = curl_global_init(CURL_GLOBAL_ALL);
> + if (ret) {
> + error_setg(errp, "libcurl initialization failed with %d", ret);
> + return -EIO;
> + }
> + libcurl_initialized = true;
> + }
> +
> qemu_mutex_init(&s->mutex);
> opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> qemu_opts_absorb_qdict(opts, options, &local_err);
> @@ -772,11 +783,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
> }
> }
>
> - if (!inited) {
> - curl_global_init(CURL_GLOBAL_ALL);
> - inited = 1;
> - }
> -
> DPRINTF("CURL: Opening %s\n", file);
> QSIMPLEQ_INIT(&s->free_state_waitq);
> s->aio_context = bdrv_get_aio_context(bs);
> --
> 2.9.5
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks Jeff Cody
2017-11-08 0:13 ` Eric Blake
2017-11-08 5:08 ` Philippe Mathieu-Daudé
@ 2017-11-08 11:45 ` Richard W.M. Jones
2 siblings, 0 replies; 26+ messages in thread
From: Richard W.M. Jones @ 2017-11-08 11:45 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, qemu-block, mitake.hitoshi, namei.unix, kwolf, eblake
On Tue, Nov 07, 2017 at 05:27:23PM -0500, Jeff Cody wrote:
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/curl.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/block/curl.c b/block/curl.c
> index 00a9879..35cf417 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -857,6 +857,9 @@ out_noclean:
> qemu_mutex_destroy(&s->mutex);
> g_free(s->cookie);
> g_free(s->url);
> + g_free(s->username);
> + g_free(s->proxyusername);
> + g_free(s->proxypassword);
> qemu_opts_del(opts);
> return -EINVAL;
> }
> @@ -955,6 +958,9 @@ static void curl_close(BlockDriverState *bs)
>
> g_free(s->cookie);
> g_free(s->url);
> + g_free(s->username);
> + g_free(s->proxyusername);
> + g_free(s->proxypassword);
username & proxyusername are allocated with g_strdup and so
should obviously be freed.
proxypassword is returned by qcrypto_secret_lookup_as_utf8, and it's
not clear to me if we should free that or not. However examining the
code of qcrypto_secret_lookup it looks as if this string is allocated
by g_new0, which would indicate that it should also be freed.
Therefore:
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style Jeff Cody
2017-11-08 10:47 ` [Qemu-devel] [Qemu-block] " Darren Kenny
@ 2017-11-08 11:46 ` Richard W.M. Jones
1 sibling, 0 replies; 26+ messages in thread
From: Richard W.M. Jones @ 2017-11-08 11:46 UTC (permalink / raw)
To: Jeff Cody
Cc: qemu-devel, qemu-block, mitake.hitoshi, namei.unix, kwolf, eblake
On Tue, Nov 07, 2017 at 05:27:24PM -0500, Jeff Cody wrote:
> This addresses non-functional changes to help curl.c better comply
> with the coding styles (comments, indentation, brackets, etc.).
>
> One minor code change is the combination of two if statements into
> a single if statement.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
As you say, just simple coding style fixes except for the single
combined if statement. Therefore:
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Rich.
> block/curl.c | 100 +++++++++++++++++++++++++++++++----------------------------
> 1 file changed, 52 insertions(+), 48 deletions(-)
>
> diff --git a/block/curl.c b/block/curl.c
> index 35cf417..7a6dd44 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -32,8 +32,10 @@
> #include <curl/curl.h>
> #include "qemu/cutils.h"
>
> -// #define DEBUG_CURL
> -// #define DEBUG_VERBOSE
> +/*
> + #define DEBUG_CURL
> + #define DEBUG_VERBOSE
> +*/
>
> #ifdef DEBUG_CURL
> #define DEBUG_CURL_PRINT 1
> @@ -76,15 +78,15 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
> #define CURL_TIMEOUT_DEFAULT 5
> #define CURL_TIMEOUT_MAX 10000
>
> -#define CURL_BLOCK_OPT_URL "url"
> -#define CURL_BLOCK_OPT_READAHEAD "readahead"
> -#define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
> -#define CURL_BLOCK_OPT_TIMEOUT "timeout"
> -#define CURL_BLOCK_OPT_COOKIE "cookie"
> -#define CURL_BLOCK_OPT_COOKIE_SECRET "cookie-secret"
> -#define CURL_BLOCK_OPT_USERNAME "username"
> -#define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
> -#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
> +#define CURL_BLOCK_OPT_URL "url"
> +#define CURL_BLOCK_OPT_READAHEAD "readahead"
> +#define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
> +#define CURL_BLOCK_OPT_TIMEOUT "timeout"
> +#define CURL_BLOCK_OPT_COOKIE "cookie"
> +#define CURL_BLOCK_OPT_COOKIE_SECRET "cookie-secret"
> +#define CURL_BLOCK_OPT_USERNAME "username"
> +#define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
> +#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
> #define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
>
> struct BDRVCURLState;
> @@ -110,8 +112,7 @@ typedef struct CURLSocket {
> QLIST_ENTRY(CURLSocket) next;
> } CURLSocket;
>
> -typedef struct CURLState
> -{
> +typedef struct CURLState {
> struct BDRVCURLState *s;
> CURLAIOCB *acb[CURL_NUM_ACB];
> CURL *curl;
> @@ -196,22 +197,22 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
>
> DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, (int)fd);
> switch (action) {
> - case CURL_POLL_IN:
> - aio_set_fd_handler(s->aio_context, fd, false,
> - curl_multi_read, NULL, NULL, state);
> - break;
> - case CURL_POLL_OUT:
> - aio_set_fd_handler(s->aio_context, fd, false,
> - NULL, curl_multi_do, NULL, state);
> - break;
> - case CURL_POLL_INOUT:
> - aio_set_fd_handler(s->aio_context, fd, false,
> - curl_multi_read, curl_multi_do, NULL, state);
> - break;
> - case CURL_POLL_REMOVE:
> - aio_set_fd_handler(s->aio_context, fd, false,
> - NULL, NULL, NULL, NULL);
> - break;
> + case CURL_POLL_IN:
> + aio_set_fd_handler(s->aio_context, fd, false,
> + curl_multi_read, NULL, NULL, state);
> + break;
> + case CURL_POLL_OUT:
> + aio_set_fd_handler(s->aio_context, fd, false,
> + NULL, curl_multi_do, NULL, state);
> + break;
> + case CURL_POLL_INOUT:
> + aio_set_fd_handler(s->aio_context, fd, false,
> + curl_multi_read, curl_multi_do, NULL, state);
> + break;
> + case CURL_POLL_REMOVE:
> + aio_set_fd_handler(s->aio_context, fd, false,
> + NULL, NULL, NULL, NULL);
> + break;
> }
>
> return 0;
> @@ -235,7 +236,7 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
> /* Called from curl_multi_do_locked, with s->mutex held. */
> static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
> {
> - CURLState *s = ((CURLState*)opaque);
> + CURLState *s = opaque;
> size_t realsize = size * nmemb;
> int i;
>
> @@ -253,11 +254,12 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
> memcpy(s->orig_buf + s->buf_off, ptr, realsize);
> s->buf_off += realsize;
>
> - for(i=0; i<CURL_NUM_ACB; i++) {
> + for (i = 0; i < CURL_NUM_ACB; i++) {
> CURLAIOCB *acb = s->acb[i];
>
> - if (!acb)
> + if (!acb) {
> continue;
> + }
>
> if ((s->buf_off >= acb->end)) {
> size_t request_length = acb->bytes;
> @@ -293,17 +295,16 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
> uint64_t clamped_end = MIN(end, s->len);
> uint64_t clamped_len = clamped_end - start;
>
> - for (i=0; i<CURL_NUM_STATES; i++) {
> + for (i = 0; i < CURL_NUM_STATES; i++) {
> CURLState *state = &s->states[i];
> uint64_t buf_end = (state->buf_start + state->buf_off);
> uint64_t buf_fend = (state->buf_start + state->buf_len);
>
> - if (!state->orig_buf)
> - continue;
> - if (!state->buf_off)
> + if (!state->orig_buf || !state->buf_off) {
> continue;
> + }
>
> - // Does the existing buffer cover our section?
> + /* Does the existing buffer cover our section? */
> if ((start >= state->buf_start) &&
> (start <= buf_end) &&
> (clamped_end >= state->buf_start) &&
> @@ -319,7 +320,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
> return true;
> }
>
> - // Wait for unfinished chunks
> + /* Wait for unfinished chunks */
> if (state->in_use &&
> (start >= state->buf_start) &&
> (start <= buf_fend) &&
> @@ -331,7 +332,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, uint64_t len,
> acb->start = start - state->buf_start;
> acb->end = acb->start + clamped_len;
>
> - for (j=0; j<CURL_NUM_ACB; j++) {
> + for (j = 0; j < CURL_NUM_ACB; j++) {
> if (!state->acb[j]) {
> state->acb[j] = acb;
> return true;
> @@ -355,8 +356,9 @@ static void curl_multi_check_completion(BDRVCURLState *s)
> msg = curl_multi_info_read(s->multi, &msgs_in_queue);
>
> /* Quit when there are no more completions */
> - if (!msg)
> + if (!msg) {
> break;
> + }
>
> if (msg->msg == CURLMSG_DONE) {
> CURLState *state = NULL;
> @@ -540,12 +542,14 @@ static void curl_clean_state(CURLState *s)
> {
> CURLAIOCB *next;
> int j;
> +
> for (j = 0; j < CURL_NUM_ACB; j++) {
> assert(!s->acb[j]);
> }
>
> - if (s->s->multi)
> + if (s->s->multi) {
> curl_multi_remove_handle(s->s->multi, s->curl);
> + }
>
> while (!QLIST_EMPTY(&s->sockets)) {
> CURLSocket *socket = QLIST_FIRST(&s->sockets);
> @@ -794,7 +798,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
> goto out_noclean;
> }
>
> - // Get file size
> + /* Get file size */
>
> if (curl_init_state(s, state) < 0) {
> goto out;
> @@ -802,11 +806,11 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
>
> s->accept_range = false;
> curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1);
> - curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION,
> - curl_header_cb);
> + curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION, curl_header_cb);
> curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s);
> - if (curl_easy_perform(state->curl))
> + if (curl_easy_perform(state->curl)) {
> goto out;
> + }
> if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d)) {
> goto out;
> }
> @@ -876,13 +880,13 @@ static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
>
> qemu_mutex_lock(&s->mutex);
>
> - // In case we have the requested data already (e.g. read-ahead),
> - // we can just call the callback and be done.
> + /* In case we have the requested data already (e.g. read-ahead),
> + we can just call the callback and be done. */
> if (curl_find_buf(s, start, acb->bytes, acb)) {
> goto out;
> }
>
> - // No cache found, so let's start a new request
> + /* No cache found, so let's start a new request */
> for (;;) {
> state = curl_find_state(s);
> if (state) {
> --
> 2.9.5
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style
2017-11-08 10:47 ` [Qemu-devel] [Qemu-block] " Darren Kenny
@ 2017-11-08 14:26 ` Eric Blake
2017-11-08 14:50 ` Darren Kenny
2017-11-08 15:01 ` Paolo Bonzini
1 sibling, 1 reply; 26+ messages in thread
From: Eric Blake @ 2017-11-08 14:26 UTC (permalink / raw)
To: Jeff Cody, qemu-devel, kwolf, qemu-block, mitake.hitoshi, rjones,
namei.unix
[-- Attachment #1: Type: text/plain, Size: 2651 bytes --]
On 11/08/2017 04:47 AM, Darren Kenny wrote:
> Hi Jeff,
>
> While I'm relatively new to this community, I do have some comments
> about the styling in this file.
>
> I don't see anything in the CODING_STYLE file that tells me I'm
> wrong here, but it's certainly possible...
>
> More inline.
>
>> -// #define DEBUG_CURL
>> -// #define DEBUG_VERBOSE
This is definitely against style (checkpatch.pl flags it),
>> +/*
>> + #define DEBUG_CURL
>> + #define DEBUG_VERBOSE
>> +*/
>
while you are correct that this is not quite usual style.
> Is it not more common to use the style:
>
> /*
> * text
> */
But, since checkpatch.pl doesn't flag it, and since it is easier to
remove the leading and trailing /* and */ to enable the debug #defines
(compared to editing every single line of the comment), I don't see a
problem with the style chosen here.
>> @@ -235,7 +236,7 @@ static size_t curl_header_cb(void *ptr, size_t
>> size, size_t nmemb, void *opaque)
>> /* Called from curl_multi_do_locked, with s->mutex held. */
>> static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void
>> *opaque)
>> {
>> - CURLState *s = ((CURLState*)opaque);
>> + CURLState *s = opaque;
>
> Not sure about this one - while it may not be strictly necessary to
> do the casting, from a readability point of view it is helpful to
> see the cast - possibly with the outer brackets removed:
>
> CURLState *s = (CURLState*)opaque;
I _am_ sure about it. The cast from void* is pointless (this is C, not
C++), and this is one of the changes that Jeff specifically made in v3
that was not present in v2, because I requested that we lose the cast
(in general, we try to avoid as many casts as possible).
>> @@ -876,13 +880,13 @@ static void curl_setup_preadv(BlockDriverState
>> *bs, CURLAIOCB *acb)
>>
>> qemu_mutex_lock(&s->mutex);
>>
>> - // In case we have the requested data already (e.g. read-ahead),
>> - // we can just call the callback and be done.
>> + /* In case we have the requested data already (e.g. read-ahead),
>> + we can just call the callback and be done. */
>
> Please don't do a multi-line comment like this, it is much more
> obvious that the second line is a comment if you use the more normal
> format of as outlined earlier by me.
Indeed, while I don't mind the style used on the #define DEBUG_*, this
one could be touched up to be more idiomatic.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style
2017-11-08 14:26 ` Eric Blake
@ 2017-11-08 14:50 ` Darren Kenny
2017-11-08 15:04 ` Paolo Bonzini
0 siblings, 1 reply; 26+ messages in thread
From: Darren Kenny @ 2017-11-08 14:50 UTC (permalink / raw)
To: Eric Blake
Cc: Jeff Cody, qemu-devel, kwolf, qemu-block, mitake.hitoshi, rjones,
namei.unix
On Wed, Nov 08, 2017 at 08:26:57AM -0600, Eric Blake wrote:
>On 11/08/2017 04:47 AM, Darren Kenny wrote:
>> Hi Jeff,
>>
>> While I'm relatively new to this community, I do have some comments
>> about the styling in this file.
>>
>> I don't see anything in the CODING_STYLE file that tells me I'm
>> wrong here, but it's certainly possible...
>>
>> More inline.
>>
>
>>> -// #define DEBUG_CURL
>>> -// #define DEBUG_VERBOSE
>
>This is definitely against style (checkpatch.pl flags it),
>
>>> +/*
>>> + #define DEBUG_CURL
>>> + #define DEBUG_VERBOSE
>>> +*/
>>
>
>while you are correct that this is not quite usual style.
>
>> Is it not more common to use the style:
>>
>> /*
>> * text
>> */
>
>But, since checkpatch.pl doesn't flag it, and since it is easier to
>remove the leading and trailing /* and */ to enable the debug #defines
>(compared to editing every single line of the comment), I don't see a
>problem with the style chosen here.
>
If that is the purpose, maybe an #if 0 is more appropriate or #ifdef
DEBUG, or similar.
Isn't the purpose of styling to be consistent? As such should we not
be trying to use the multi-line style set out at the top of the file?
>
>>> @@ -235,7 +236,7 @@ static size_t curl_header_cb(void *ptr, size_t
>>> size, size_t nmemb, void *opaque)
>>> /* Called from curl_multi_do_locked, with s->mutex held. */
>>> static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void
>>> *opaque)
>>> {
>>> - CURLState *s = ((CURLState*)opaque);
>>> + CURLState *s = opaque;
>>
>> Not sure about this one - while it may not be strictly necessary to
>> do the casting, from a readability point of view it is helpful to
>> see the cast - possibly with the outer brackets removed:
>>
>> CURLState *s = (CURLState*)opaque;
>
>I _am_ sure about it. The cast from void* is pointless (this is C, not
>C++), and this is one of the changes that Jeff specifically made in v3
>that was not present in v2, because I requested that we lose the cast
>(in general, we try to avoid as many casts as possible).
Fair enough.
>
>>> @@ -876,13 +880,13 @@ static void curl_setup_preadv(BlockDriverState
>>> *bs, CURLAIOCB *acb)
>>>
>>> qemu_mutex_lock(&s->mutex);
>>>
>>> - // In case we have the requested data already (e.g. read-ahead),
>>> - // we can just call the callback and be done.
>>> + /* In case we have the requested data already (e.g. read-ahead),
>>> + we can just call the callback and be done. */
>>
>> Please don't do a multi-line comment like this, it is much more
>> obvious that the second line is a comment if you use the more normal
>> format of as outlined earlier by me.
>
>Indeed, while I don't mind the style used on the #define DEBUG_*, this
>one could be touched up to be more idiomatic.
>
Thanks,
Darren.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style
2017-11-08 10:47 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-08 14:26 ` Eric Blake
@ 2017-11-08 15:01 ` Paolo Bonzini
1 sibling, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2017-11-08 15:01 UTC (permalink / raw)
To: Jeff Cody, qemu-devel, kwolf, qemu-block, mitake.hitoshi, rjones,
namei.unix
On 08/11/2017 11:47, Darren Kenny wrote:
>>
>>
>> -// #define DEBUG_CURL
>> -// #define DEBUG_VERBOSE
>> +/*
>> + #define DEBUG_CURL
>> + #define DEBUG_VERBOSE
>> +*/
>
> Is it not more common to use the style:
>
> /*
> * text
> */
>
> This is visible in almost every file at the top, where the Copyright
> and License is.
The most common style in QEMU is probably
/* text
* more text
*/
But for DEBUG_* macros I think // are appropriate. checkpatch should
still warn about them because DEBUG_* macros should be replaced by
tracepoints; but unless we do that we should keep line comments.
>>
>> - CURLState *s = ((CURLState*)opaque);
>> + CURLState *s = opaque;
>
> Not sure about this one - while it may not be strictly necessary to
> do the casting, from a readability point of view it is helpful to
> see the cast - possibly with the outer brackets removed:
>
> CURLState *s = (CURLState*)opaque;
I think the most common idiom here is to omit the cast.
>> - // In case we have the requested data already (e.g. read-ahead),
>> - // we can just call the callback and be done.
>> + /* In case we have the requested data already (e.g. read-ahead),
>> + we can just call the callback and be done. */
>
> Please don't do a multi-line comment like this, it is much more
> obvious that the second line is a comment if you use the more normal
> format of as outlined earlier by me.
Agreed.
Paolo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style
2017-11-08 14:50 ` Darren Kenny
@ 2017-11-08 15:04 ` Paolo Bonzini
0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2017-11-08 15:04 UTC (permalink / raw)
To: Eric Blake, Jeff Cody, qemu-devel, kwolf, qemu-block,
mitake.hitoshi, rjones, namei.unix
On 08/11/2017 15:50, Darren Kenny wrote:
>> But, since checkpatch.pl doesn't flag it, and since it is easier to
>> remove the leading and trailing /* and */ to enable the debug #defines
>> (compared to editing every single line of the comment), I don't see a
>> problem with the style chosen here.
>
> If that is the purpose, maybe an #if 0 is more appropriate or #ifdef
> DEBUG, or similar.
>
> Isn't the purpose of styling to be consistent? As such should we not
> be trying to use the multi-line style set out at the top of the file?
Yes, and you're very welcome to submit a checkpatch.pl patch that warns
about that comment style without * at the beginning of each line.
On the other hand, style should not get in the way, and the version that
gets least in the way for debug #defines is //.
Paolo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11)
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
` (6 preceding siblings ...)
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style Jeff Cody
@ 2017-12-18 21:05 ` Jeff Cody
7 siblings, 0 replies; 26+ messages in thread
From: Jeff Cody @ 2017-12-18 21:05 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, mitake.hitoshi, namei.unix, rjones, kwolf, eblake
On Tue, Nov 07, 2017 at 05:27:17PM -0500, Jeff Cody wrote:
> Changes from v2 -> v3:
>
> Patch 1: Return -EIO for ssh_state_init() (Thanks Eric)
> Patch 4: fixed missed line (Thanks Eric)
> Patch 7: fixed opaque pointer assignment (Thanks Eric)
>
> git-backport-diff -r master.. -u 81fb084
>
> 001/7:[0007] [FC] 'block/ssh: don't call libssh2_init() in block_init()'
> 002/7:[----] [--] 'block/ssh: make compliant with coding guidelines'
> 003/7:[----] [--] 'block/sheepdog: remove spurious NULL check'
> 004/7:[0010] [FC] 'block/sheepdog: code beautification'
> 005/7:[----] [--] 'block/curl: check error return of curl_global_init()'
> 006/7:[----] [--] 'block/curl: fix minor memory leaks'
> 007/7:[0002] [FC] 'block/curl: code cleanup to comply with coding style'
>
>
> Jeff Cody (7):
> block/ssh: don't call libssh2_init() in block_init()
> block/ssh: make compliant with coding guidelines
> block/sheepdog: remove spurious NULL check
> block/sheepdog: code beautification
> block/curl: check error return of curl_global_init()
> block/curl: fix minor memory leaks
> block/curl: code cleanup to comply with coding style
>
> block/curl.c | 124 +++++++++++++++++++++++------------------
> block/sheepdog.c | 166 +++++++++++++++++++++++++++----------------------------
> block/ssh.c | 69 ++++++++++++++---------
> 3 files changed, 197 insertions(+), 162 deletions(-)
>
> --
> 2.9.5
>
Applied the non-controversial patches 3-6 to my block branch:
git://github.com/codyprime/qemu-kvm-jtc block
-Jeff
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2017-12-18 21:05 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 22:27 [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init() Jeff Cody
2017-11-07 22:48 ` Eric Blake
2017-11-08 11:39 ` Richard W.M. Jones
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines Jeff Cody
2017-11-08 10:50 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-08 11:38 ` [Qemu-devel] " Richard W.M. Jones
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 3/7] block/sheepdog: remove spurious NULL check Jeff Cody
2017-11-08 10:48 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 4/7] block/sheepdog: code beautification Jeff Cody
2017-11-08 10:52 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 5/7] block/curl: check error return of curl_global_init() Jeff Cody
2017-11-08 10:51 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-08 11:41 ` [Qemu-devel] " Richard W.M. Jones
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 6/7] block/curl: fix minor memory leaks Jeff Cody
2017-11-08 0:13 ` Eric Blake
2017-11-08 5:08 ` Philippe Mathieu-Daudé
2017-11-08 11:45 ` Richard W.M. Jones
2017-11-07 22:27 ` [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style Jeff Cody
2017-11-08 10:47 ` [Qemu-devel] [Qemu-block] " Darren Kenny
2017-11-08 14:26 ` Eric Blake
2017-11-08 14:50 ` Darren Kenny
2017-11-08 15:04 ` Paolo Bonzini
2017-11-08 15:01 ` Paolo Bonzini
2017-11-08 11:46 ` [Qemu-devel] " Richard W.M. Jones
2017-12-18 21:05 ` [Qemu-devel] [PATCH v3 0/7] Code cleanup and minor fixes (for 2.11) Jeff Cody
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.