From 8b2f0abfd61237b301a29e814535b1e36d733aaa Mon Sep 17 00:00:00 2001 From: Yik Fang Date: Thu, 12 Feb 2015 06:21:51 +0000 Subject: [PATCH 01/19] nbd: Fix overflow return value The value of reply.error should be the type unsigned int. Signed-off-by: Yik Fang Message-Id: <1423722111-12902-1-git-send-email-eric.fangyi@huawei.com> Signed-off-by: Paolo Bonzini --- nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nbd.c b/nbd.c index 71159aff6a..02ba0fe252 100644 --- a/nbd.c +++ b/nbd.c @@ -1295,7 +1295,7 @@ static void nbd_trip(void *opaque) default: LOG("invalid request type (%u) received", request.type); invalid_request: - reply.error = -EINVAL; + reply.error = EINVAL; error_reply: if (nbd_co_send_reply(req, &reply, 0) < 0) { goto out; From 2b21233061696feed434317a70e0a8b74f956ec8 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:14 -0500 Subject: [PATCH 02/19] util/uri: Add overflow check to rfc3986_parse_port And while at it, replace tabs by eight spaces in this function. Signed-off-by: Max Reitz Message-Id: <1424887718-10800-2-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- util/uri.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/util/uri.c b/util/uri.c index 1cfd78bdb5..550b984587 100644 --- a/util/uri.c +++ b/util/uri.c @@ -320,19 +320,23 @@ static int rfc3986_parse_port(URI *uri, const char **str) { const char *cur = *str; + int port = 0; if (ISA_DIGIT(cur)) { - if (uri != NULL) - uri->port = 0; - while (ISA_DIGIT(cur)) { - if (uri != NULL) - uri->port = uri->port * 10 + (*cur - '0'); - cur++; - } - *str = cur; - return(0); + while (ISA_DIGIT(cur)) { + port = port * 10 + (*cur - '0'); + if (port > 65535) { + return 1; + } + cur++; + } + if (uri) { + uri->port = port; + } + *str = cur; + return 0; } - return(1); + return 1; } /** From 453b07b13443713f6a632005977c7ccab17e135d Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:15 -0500 Subject: [PATCH 03/19] qemu-nbd: Detect unused partitions by system == 0 Unused partitions do not necessarily have a total sector count of 0 (although they should have), but they always do have the system field set to 0, so use that for testing whether a partition is in use rather than the sector count field alone. Signed-off-by: Max Reitz Message-Id: <1424887718-10800-3-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- qemu-nbd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 064b000b43..d8daf1d3a6 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -167,8 +167,9 @@ static int find_partition(BlockBackend *blk, int partition, for (i = 0; i < 4; i++) { read_partition(&data[446 + 16 * i], &mbr[i]); - if (!mbr[i].nb_sectors_abs) + if (!mbr[i].system || !mbr[i].nb_sectors_abs) { continue; + } if (mbr[i].system == 0xF || mbr[i].system == 0x5) { struct partition_record ext[4]; @@ -182,8 +183,9 @@ static int find_partition(BlockBackend *blk, int partition, for (j = 0; j < 4; j++) { read_partition(&data1[446 + 16 * j], &ext[j]); - if (!ext[j].nb_sectors_abs) + if (!ext[j].system || !ext[j].nb_sectors_abs) { continue; + } if ((ext_partnum + j + 1) == partition) { *offset = (uint64_t)ext[j].start_sector_abs << 9; From 2b1f13b996c3a278ed3d4bf4ce0893f3506fb7cc Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:16 -0500 Subject: [PATCH 04/19] nbd: Fix nbd_establish_connection()'s return value unix_connect_opts() and inet_connect_opts() do not necessarily set errno (if at all); therefore, nbd_establish_connection() should not literally return -errno on error. Signed-off-by: Max Reitz Message-Id: <1424887718-10800-4-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/nbd.c b/block/nbd.c index 6634a69664..217618612d 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -248,7 +248,7 @@ static int nbd_establish_connection(BlockDriverState *bs, Error **errp) /* Failed to establish connection */ if (sock < 0) { logout("Failed to establish connection to NBD server\n"); - return -errno; + return -EIO; } return sock; From 892f5a5270f9f3cae4f384dffbf70679fa2a57b6 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:19 -0500 Subject: [PATCH 05/19] nbd: Pass return value from nbd_handle_list() While it does not make a difference in practice, nbd_receive_options() generally returns -errno, so it should do that here as well; and the easiest way to achieve this is by passing on the value returned by nbd_handle_list(). Signed-off-by: Max Reitz Message-Id: <1424887718-10800-7-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- nbd.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nbd.c b/nbd.c index 02ba0fe252..34f4dbb80d 100644 --- a/nbd.c +++ b/nbd.c @@ -351,7 +351,7 @@ fail: static int nbd_receive_options(NBDClient *client) { while (1) { - int csock = client->sock; + int csock = client->sock, ret; uint32_t tmp, length; uint64_t magic; @@ -398,8 +398,9 @@ static int nbd_receive_options(NBDClient *client) TRACE("Checking option"); switch (be32_to_cpu(tmp)) { case NBD_OPT_LIST: - if (nbd_handle_list(client, length) < 0) { - return 1; + ret = nbd_handle_list(client, length); + if (ret < 0) { + return ret; } break; From 98f44bbe70bb803e7be2421b7cc92a1c179afb87 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:21 -0500 Subject: [PATCH 06/19] nbd: Handle blk_getlength() failure Signed-off-by: Max Reitz Message-Id: <1424887718-10800-9-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- blockdev-nbd.c | 6 +++++- include/block/nbd.h | 3 ++- nbd.c | 16 ++++++++++++++-- qemu-nbd.c | 10 +++++++++- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 22e95d17ee..b29e456f1f 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -105,7 +105,11 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, writable = false; } - exp = nbd_export_new(blk, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, NULL); + exp = nbd_export_new(blk, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, NULL, + errp); + if (!exp) { + return; + } nbd_export_set_name(exp, device); diff --git a/include/block/nbd.h b/include/block/nbd.h index ca9a5ac5b3..2c20138588 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -86,7 +86,8 @@ typedef struct NBDExport NBDExport; typedef struct NBDClient NBDClient; NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, - uint32_t nbdflags, void (*close)(NBDExport *)); + uint32_t nbdflags, void (*close)(NBDExport *), + Error **errp); void nbd_export_close(NBDExport *exp); void nbd_export_get(NBDExport *exp); void nbd_export_put(NBDExport *exp); diff --git a/nbd.c b/nbd.c index 34f4dbb80d..8837e751d1 100644 --- a/nbd.c +++ b/nbd.c @@ -966,7 +966,8 @@ static void blk_aio_detach(void *opaque) } NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, - uint32_t nbdflags, void (*close)(NBDExport *)) + uint32_t nbdflags, void (*close)(NBDExport *), + Error **errp) { NBDExport *exp = g_malloc0(sizeof(NBDExport)); exp->refcount = 1; @@ -974,7 +975,14 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, exp->blk = blk; exp->dev_offset = dev_offset; exp->nbdflags = nbdflags; - exp->size = size == -1 ? blk_getlength(blk) : size; + exp->size = size < 0 ? blk_getlength(blk) : size; + if (exp->size < 0) { + error_setg_errno(errp, -exp->size, + "Failed to determine the NBD export's length"); + goto fail; + } + exp->size -= exp->size % BDRV_SECTOR_SIZE; + exp->close = close; exp->ctx = blk_get_aio_context(blk); blk_ref(blk); @@ -986,6 +994,10 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, */ blk_invalidate_cache(blk, NULL); return exp; + +fail: + g_free(exp); + return NULL; } NBDExport *nbd_export_find(const char *name) diff --git a/qemu-nbd.c b/qemu-nbd.c index d8daf1d3a6..0cb0e4e29a 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -717,6 +717,10 @@ int main(int argc, char **argv) bs->detect_zeroes = detect_zeroes; fd_size = blk_getlength(blk); + if (fd_size < 0) { + errx(EXIT_FAILURE, "Failed to determine the image length: %s", + strerror(-fd_size)); + } if (partition != -1) { ret = find_partition(blk, partition, &dev_offset, &fd_size); @@ -726,7 +730,11 @@ int main(int argc, char **argv) } } - exp = nbd_export_new(blk, dev_offset, fd_size, nbdflags, nbd_export_closed); + exp = nbd_export_new(blk, dev_offset, fd_size, nbdflags, nbd_export_closed, + &local_err); + if (!exp) { + errx(EXIT_FAILURE, "%s", error_get_pretty(local_err)); + } if (sockpath) { fd = unix_socket_incoming(sockpath); From 70d4739ef200760d8cac3355d05b4252f2f37fec Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:22 -0500 Subject: [PATCH 07/19] qemu-nbd: fork() can fail It is very unlikely, but it is possible. Signed-off-by: Max Reitz Message-Id: <1424887718-10800-10-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- qemu-nbd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 0cb0e4e29a..0c9e807a1a 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -635,7 +635,9 @@ int main(int argc, char **argv) * print errors and exit with the proper status code. */ pid = fork(); - if (pid == 0) { + if (pid < 0) { + err(EXIT_FAILURE, "Failed to fork"); + } else if (pid == 0) { close(stderr_fd[0]); ret = qemu_daemon(1, 0); From ac97393dc7c4761af6104fb8fca5f600899f687b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:23 -0500 Subject: [PATCH 08/19] nbd: Fix potential signed overflow issues Signed-off-by: Max Reitz Message-Id: <1424887718-10800-11-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- include/block/nbd.h | 4 ++-- qemu-nbd.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index 2c20138588..53726e82e9 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -54,8 +54,8 @@ struct nbd_reply { /* Reply types. */ #define NBD_REP_ACK (1) /* Data sending finished. */ #define NBD_REP_SERVER (2) /* Export description. */ -#define NBD_REP_ERR_UNSUP ((1 << 31) | 1) /* Unknown option. */ -#define NBD_REP_ERR_INVALID ((1 << 31) | 3) /* Invalid length. */ +#define NBD_REP_ERR_UNSUP ((UINT32_C(1) << 31) | 1) /* Unknown option. */ +#define NBD_REP_ERR_INVALID ((UINT32_C(1) << 31) | 3) /* Invalid length. */ #define NBD_CMD_MASK_COMMAND 0x0000ffff #define NBD_CMD_FLAG_FUA (1 << 16) diff --git a/qemu-nbd.c b/qemu-nbd.c index 0c9e807a1a..a4a9a0cf37 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -142,8 +142,9 @@ static void read_partition(uint8_t *p, struct partition_record *r) r->end_head = p[5]; r->end_cylinder = p[7] | ((p[6] << 2) & 0x300); r->end_sector = p[6] & 0x3f; - r->start_sector_abs = p[8] | p[9] << 8 | p[10] << 16 | p[11] << 24; - r->nb_sectors_abs = p[12] | p[13] << 8 | p[14] << 16 | p[15] << 24; + + r->start_sector_abs = le32_to_cpup((uint32_t *)(p + 8)); + r->nb_sectors_abs = le32_to_cpup((uint32_t *)(p + 12)); } static int find_partition(BlockBackend *blk, int partition, From 3f4726596dafd2e27485e51f4cc4a2363f48d4a3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:25 -0500 Subject: [PATCH 09/19] nbd: Set block size to BDRV_SECTOR_SIZE Signed-off-by: Max Reitz Message-Id: <1424887718-10800-13-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- block/nbd-client.c | 3 +-- block/nbd-client.h | 1 - include/block/nbd.h | 4 ++-- nbd.c | 15 +++++++-------- qemu-nbd.c | 5 ++--- 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index 259f5a3cb6..e1bb9198c5 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -386,8 +386,7 @@ int nbd_client_init(BlockDriverState *bs, int sock, const char *export, logout("session init %s\n", export); qemu_set_block(sock); ret = nbd_receive_negotiate(sock, export, - &client->nbdflags, &client->size, - &client->blocksize, errp); + &client->nbdflags, &client->size, errp); if (ret < 0) { logout("Failed to negotiate with the NBD server\n"); closesocket(sock); diff --git a/block/nbd-client.h b/block/nbd-client.h index fa4ff42d22..e8413408b5 100644 --- a/block/nbd-client.h +++ b/block/nbd-client.h @@ -20,7 +20,6 @@ typedef struct NbdClientSession { int sock; uint32_t nbdflags; off_t size; - size_t blocksize; CoMutex send_mutex; CoMutex free_sema; diff --git a/include/block/nbd.h b/include/block/nbd.h index 53726e82e9..65f409d804 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -75,8 +75,8 @@ enum { ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read); int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags, - off_t *size, size_t *blocksize, Error **errp); -int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize); + off_t *size, Error **errp); +int nbd_init(int fd, int csock, uint32_t flags, off_t size); ssize_t nbd_send_request(int csock, struct nbd_request *request); ssize_t nbd_receive_reply(int csock, struct nbd_reply *reply); int nbd_client(int fd); diff --git a/nbd.c b/nbd.c index 8837e751d1..3d550f750a 100644 --- a/nbd.c +++ b/nbd.c @@ -495,7 +495,7 @@ fail: } int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags, - off_t *size, size_t *blocksize, Error **errp) + off_t *size, Error **errp) { char buf[256]; uint64_t magic, s; @@ -603,7 +603,6 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags, goto fail; } *size = be64_to_cpu(s); - *blocksize = 1024; TRACE("Size is %" PRIu64, *size); if (!name) { @@ -630,7 +629,7 @@ fail: } #ifdef __linux__ -int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize) +int nbd_init(int fd, int csock, uint32_t flags, off_t size) { TRACE("Setting NBD socket"); @@ -640,17 +639,17 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize) return -serrno; } - TRACE("Setting block size to %lu", (unsigned long)blocksize); + TRACE("Setting block size to %lu", (unsigned long)BDRV_SECTOR_SIZE); - if (ioctl(fd, NBD_SET_BLKSIZE, blocksize) < 0) { + if (ioctl(fd, NBD_SET_BLKSIZE, (size_t)BDRV_SECTOR_SIZE) < 0) { int serrno = errno; LOG("Failed setting NBD block size"); return -serrno; } - TRACE("Setting size to %zd block(s)", (size_t)(size / blocksize)); + TRACE("Setting size to %zd block(s)", (size_t)(size / BDRV_SECTOR_SIZE)); - if (ioctl(fd, NBD_SET_SIZE_BLOCKS, size / blocksize) < 0) { + if (ioctl(fd, NBD_SET_SIZE_BLOCKS, size / (size_t)BDRV_SECTOR_SIZE) < 0) { int serrno = errno; LOG("Failed setting size (in blocks)"); return -serrno; @@ -715,7 +714,7 @@ int nbd_client(int fd) return ret; } #else -int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize) +int nbd_init(int fd, int csock, uint32_t flags, off_t size) { return -ENOTSUP; } diff --git a/qemu-nbd.c b/qemu-nbd.c index a4a9a0cf37..7e690fff7e 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -279,7 +279,6 @@ static void *nbd_client_thread(void *arg) { char *device = arg; off_t size; - size_t blocksize; uint32_t nbdflags; int fd, sock; int ret; @@ -292,7 +291,7 @@ static void *nbd_client_thread(void *arg) } ret = nbd_receive_negotiate(sock, NULL, &nbdflags, - &size, &blocksize, &local_error); + &size, &local_error); if (ret < 0) { if (local_error) { fprintf(stderr, "%s\n", error_get_pretty(local_error)); @@ -308,7 +307,7 @@ static void *nbd_client_thread(void *arg) goto out_socket; } - ret = nbd_init(fd, sock, nbdflags, size, blocksize); + ret = nbd_init(fd, sock, nbdflags, size); if (ret < 0) { goto out_fd; } From 9c122adadbf4377eb77195b3944be10a59d9484f Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:31 -0500 Subject: [PATCH 10/19] nbd: Fix nbd_receive_options() The client flags are sent exactly once overall, not once per option. Signed-off-by: Max Reitz Message-Id: <1424887718-10800-19-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- nbd.c | 49 +++++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/nbd.c b/nbd.c index 3d550f750a..fb8a4d47f3 100644 --- a/nbd.c +++ b/nbd.c @@ -350,30 +350,39 @@ fail: static int nbd_receive_options(NBDClient *client) { + int csock = client->sock; + uint32_t flags; + + /* Client sends: + [ 0 .. 3] client flags + + [ 0 .. 7] NBD_OPTS_MAGIC + [ 8 .. 11] NBD option + [12 .. 15] Data length + ... Rest of request + + [ 0 .. 7] NBD_OPTS_MAGIC + [ 8 .. 11] Second NBD option + [12 .. 15] Data length + ... Rest of request + */ + + if (read_sync(csock, &flags, sizeof(flags)) != sizeof(flags)) { + LOG("read failed"); + return -EIO; + } + TRACE("Checking client flags"); + be32_to_cpus(&flags); + if (flags != 0 && flags != NBD_FLAG_C_FIXED_NEWSTYLE) { + LOG("Bad client flags received"); + return -EIO; + } + while (1) { - int csock = client->sock, ret; + int ret; uint32_t tmp, length; uint64_t magic; - /* Client sends: - [ 0 .. 3] client flags - [ 4 .. 11] NBD_OPTS_MAGIC - [12 .. 15] NBD option - [16 .. 19] length - ... Rest of request - */ - - if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) { - LOG("read failed"); - return -EINVAL; - } - TRACE("Checking client flags"); - tmp = be32_to_cpu(tmp); - if (tmp != 0 && tmp != NBD_FLAG_C_FIXED_NEWSTYLE) { - LOG("Bad client flags received"); - return -EINVAL; - } - if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) { LOG("read failed"); return -EINVAL; From 48c7d80de8863e3436b3b5d5676018b2afaec161 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:32 -0500 Subject: [PATCH 11/19] nbd: Fix interpretation of the export flags The export flags are a 16 bit value, so be16_to_cpu() has to be used to interpret them correctly. This makes discard and flush actually work for named NBD exports (they did not work before, because the client always assumed them to be unsupported because of the bug fixed by this patch). Signed-off-by: Max Reitz Message-Id: <1424887718-10800-20-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nbd.c b/nbd.c index fb8a4d47f3..563e8207d1 100644 --- a/nbd.c +++ b/nbd.c @@ -625,7 +625,7 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags, error_setg(errp, "Failed to read export flags"); goto fail; } - *flags |= be32_to_cpu(tmp); + *flags |= be16_to_cpu(tmp); } if (read_sync(csock, &buf, 124) != 124) { error_setg(errp, "Failed to read reserved block"); From 0379f474ddebfc69f42fa8231d86687cf29d997b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:34 -0500 Subject: [PATCH 12/19] nbd: Drop unexpected data for NBD_OPT_LIST When requesting the list of exports, no data should be sent. If data is sent, the NBD server should not just inform the client of the invalid request, but also drop the data. Signed-off-by: Max Reitz Message-Id: <1424887718-10800-22-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- nbd.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/nbd.c b/nbd.c index 563e8207d1..91b7d56239 100644 --- a/nbd.c +++ b/nbd.c @@ -193,6 +193,26 @@ static ssize_t read_sync(int fd, void *buffer, size_t size) return nbd_wr_sync(fd, buffer, size, true); } +static ssize_t drop_sync(int fd, size_t size) +{ + ssize_t ret, dropped = size; + uint8_t *buffer = g_malloc(MIN(65536, size)); + + while (size > 0) { + ret = read_sync(fd, buffer, MIN(65536, size)); + if (ret < 0) { + g_free(buffer); + return ret; + } + + assert(ret <= size); + size -= ret; + } + + g_free(buffer); + return dropped; +} + static ssize_t write_sync(int fd, void *buffer, size_t size) { int ret; @@ -303,6 +323,9 @@ static int nbd_handle_list(NBDClient *client, uint32_t length) csock = client->sock; if (length) { + if (drop_sync(csock, length) != length) { + return -EIO; + } return nbd_send_rep(csock, NBD_REP_ERR_INVALID, NBD_OPT_LIST); } From 4adf4180f284caf4ea9cd83ce37085d50a52603b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 25 Feb 2015 13:08:28 -0500 Subject: [PATCH 13/19] coroutine-io: Return -errno in case of error In case qemu_co_sendv_recvv() fails without any data read, there is no reason not to return the perfectly fine error number retrieved from socket_error(). Signed-off-by: Max Reitz Message-Id: <1424887718-10800-16-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- qemu-coroutine-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-coroutine-io.c b/qemu-coroutine-io.c index d4049260da..28dc7351ac 100644 --- a/qemu-coroutine-io.c +++ b/qemu-coroutine-io.c @@ -45,7 +45,7 @@ qemu_co_sendv_recvv(int sockfd, struct iovec *iov, unsigned iov_cnt, if (err == EAGAIN || err == EWOULDBLOCK) { qemu_coroutine_yield(); } else if (done == 0) { - return -1; + return -err; } else { break; } From 15564d85afaf1d7b314c858a5a34bda599f4cd14 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 12 Mar 2015 16:00:05 +0100 Subject: [PATCH 14/19] build: pass .d file name to scripts/make_device_config.sh, fix makefile target The .d file name must match exactly what is used in the SUBDIR_DEVICES_MAK_DEP variable. Instead of making assumptions in the make_device_config.sh script, just pass it in. Similarly, the makefile target may not match the output file name, because Makefile uses a temporary file. Instead of making assumptions on what the Makefile does, emit the config-devices.mak file to stdout, and use the passed-in destination as the makefile target Reported-by: Peter Maydell Cc: Michael S. Tsirkin Signed-off-by: Paolo Bonzini --- Makefile | 3 ++- scripts/make_device_config.sh | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 884b59dc06..88bce561a8 100644 --- a/Makefile +++ b/Makefile @@ -112,7 +112,8 @@ endif -include $(SUBDIR_DEVICES_MAK_DEP) %/config-devices.mak: default-configs/%.mak - $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@.tmp $<, " GEN $@.tmp") + $(call quiet-command, \ + $(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $< $*-config-devices.mak.d $@ > $@.tmp, " GEN $@.tmp") $(call quiet-command, if test -f $@; then \ if cmp -s $@.old $@; then \ mv $@.tmp $@; \ diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh index 7958086132..c1afb3ffaa 100644 --- a/scripts/make_device_config.sh +++ b/scripts/make_device_config.sh @@ -1,10 +1,12 @@ #! /bin/sh -# Construct a target device config file from a default, pulling in any -# files from include directives. +# Writes a target device config file to stdout, from a default and from +# include directives therein. Also emits Makefile dependencies. +# +# Usage: make_device_config.sh SRC DEPFILE-NAME DEPFILE-TARGET > DEST -dest=$1 -dep=`dirname $1`-`basename $1`.d -src=$2 +src=$1 +dep=$2 +target=$3 src_dir=`dirname $src` all_includes= @@ -22,7 +24,7 @@ while [ -n "$f" ] ; do [ $? = 0 ] || exit 1 all_includes="$all_includes $f" done -process_includes $src > $dest +process_includes $src -cat $src $all_includes | grep -v '^include' > $dest -echo "$1: $all_includes" > $dep +cat $src $all_includes | grep -v '^include' +echo "$target: $all_includes" > $dep From 2034e324dabc55064553aaa07de1536ebf8ea497 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 13 Mar 2015 15:55:54 +0800 Subject: [PATCH 15/19] virtio-scsi: Fix assert in virtio_scsi_push_event Hotplugging a scsi-disk may trigger the assertion in qemu_sgl_concat. qemu-system-x86_64: qemu/hw/scsi/virtio-scsi.c:115: qemu_sgl_concat: Assertion `skip == 0' failed. This is introduced by commit 55783a55 (virtio-scsi: work around bug in old BIOSes) which didn't check out_num when accessing out_sg[0].iov_len (the same to in sg). For virtio_scsi_push_event, looking into out_sg doesn't make sense because 0 req_size is intended. Cc: qemu-stable@nongnu.org [Cc'ing qemu-stable because 55783a55 did it too] Signed-off-by: Fam Zheng Message-Id: <1426233354-525-1-git-send-email-famz@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index da0cff83f7..c9bea067e1 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -146,8 +146,12 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req, * TODO: always disable this workaround for virtio 1.0 devices. */ if (!virtio_has_feature(vdev, VIRTIO_F_ANY_LAYOUT)) { - req_size = req->elem.out_sg[0].iov_len; - resp_size = req->elem.in_sg[0].iov_len; + if (req->elem.out_num) { + req_size = req->elem.out_sg[0].iov_len; + } + if (req->elem.in_num) { + resp_size = req->elem.in_sg[0].iov_len; + } } out_size = qemu_sgl_concat(req, req->elem.out_sg, From b680c5ba54946ab205cdb5083bc0a17e3f2fb468 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 13 Mar 2015 22:23:37 +0100 Subject: [PATCH 16/19] kvm: fix ioeventfd endianness on bi-endian architectures KVM expects host endian values. Hosts that don't use the default endianness need to negate the swap performed in adjust_endianness(). Suggested-by: Paolo Bonzini Signed-off-by: Greg Kurz Message-Id: <20150313212337.31142.3991.stgit@bahia.local> Signed-off-by: Paolo Bonzini --- kvm-all.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 55025cc366..335438adb5 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -528,13 +528,33 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension) return ret; } +static uint32_t adjust_ioeventfd_endianness(uint32_t val, uint32_t size) +{ +#if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN) + /* The kernel expects ioeventfd values in HOST_WORDS_BIGENDIAN + * endianness, but the memory core hands them in target endianness. + * For example, PPC is always treated as big-endian even if running + * on KVM and on PPC64LE. Correct here. + */ + switch (size) { + case 2: + val = bswap16(val); + break; + case 4: + val = bswap32(val); + break; + } +#endif + return val; +} + static int kvm_set_ioeventfd_mmio(int fd, hwaddr addr, uint32_t val, bool assign, uint32_t size, bool datamatch) { int ret; struct kvm_ioeventfd iofd; - iofd.datamatch = datamatch ? val : 0; + iofd.datamatch = datamatch ? adjust_ioeventfd_endianness(val, size) : 0; iofd.addr = addr; iofd.len = size; iofd.flags = 0; @@ -564,7 +584,7 @@ static int kvm_set_ioeventfd_pio(int fd, uint16_t addr, uint16_t val, bool assign, uint32_t size, bool datamatch) { struct kvm_ioeventfd kick = { - .datamatch = datamatch ? val : 0, + .datamatch = datamatch ? adjust_ioeventfd_endianness(val, size) : 0, .addr = addr, .flags = KVM_IOEVENTFD_FLAG_PIO, .len = size, From 89d5cbddeeaf6bb4aa6a5ca4fbb443115abce4a2 Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Mon, 16 Mar 2015 14:57:38 +1100 Subject: [PATCH 17/19] profiler: Reenable built-in profiler 2ed1ebcf6 "timer: replace time() with QEMU_CLOCK_HOST" broke compile when configured with --enable-profiler. Turned out the profiler has been broken for a while. This does s/qemu_time/tcg_time/ as the profiler only works in a TCG mode. This also fixes the compile error. This changes profile_getclock() to return nanoseconds rather than CPU ticks as the "profile" HMP command prints seconds and there is no platform-independent way to get ticks-per-second rate. Since TCG is quite slow and get_clock() returns nanoseconds (fine enough), this should not affect precision much. This removes unused qemu_time_start and tlb_flush_time. Signed-off-by: Alexey Kardashevskiy Message-Id: <1426478258-29961-1-git-send-email-aik@ozlabs.ru> Signed-off-by: Paolo Bonzini --- cpus.c | 2 +- include/qemu/timer.h | 5 ++--- monitor.c | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cpus.c b/cpus.c index 1ce90a12e8..314df16190 100644 --- a/cpus.c +++ b/cpus.c @@ -1353,7 +1353,7 @@ static int tcg_cpu_exec(CPUArchState *env) } ret = cpu_exec(env); #ifdef CONFIG_PROFILER - qemu_time += profile_getclock() - ti; + tcg_time += profile_getclock() - ti; #endif if (use_icount) { /* Fold pending instructions back into the diff --git a/include/qemu/timer.h b/include/qemu/timer.h index eba8b2109c..e5bd494c07 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -999,11 +999,10 @@ static inline int64_t cpu_get_real_ticks (void) #ifdef CONFIG_PROFILER static inline int64_t profile_getclock(void) { - return cpu_get_real_ticks(); + return get_clock(); } -extern int64_t qemu_time, qemu_time_start; -extern int64_t tlb_flush_time; +extern int64_t tcg_time; extern int64_t dev_time; #endif diff --git a/monitor.c b/monitor.c index 42116a942e..65a63dfbbf 100644 --- a/monitor.c +++ b/monitor.c @@ -1975,7 +1975,7 @@ static void hmp_info_numa(Monitor *mon, const QDict *qdict) #ifdef CONFIG_PROFILER -int64_t qemu_time; +int64_t tcg_time; int64_t dev_time; static void hmp_info_profile(Monitor *mon, const QDict *qdict) @@ -1983,8 +1983,8 @@ static void hmp_info_profile(Monitor *mon, const QDict *qdict) monitor_printf(mon, "async time %" PRId64 " (%0.3f)\n", dev_time, dev_time / (double)get_ticks_per_sec()); monitor_printf(mon, "qemu time %" PRId64 " (%0.3f)\n", - qemu_time, qemu_time / (double)get_ticks_per_sec()); - qemu_time = 0; + tcg_time, tcg_time / (double)get_ticks_per_sec()); + tcg_time = 0; dev_time = 0; } #else From 196d4fc56d824ccbbb58714e9ad0793053ef8260 Mon Sep 17 00:00:00 2001 From: Bo Su Date: Wed, 18 Mar 2015 09:42:12 +0000 Subject: [PATCH 18/19] virtio-scsi-dataplane: fix memory leak in virtio_scsi_vring_init if k->set_host_notifier failed, VirtIOSCSIVring *r will leak Signed-off-by: Bo Su Message-Id: <1426671732-80213-1-git-send-email-subo7@huawei.com> Reviewed-by: Fam Zheng Reviewed-by: Gonglei Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi-dataplane.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index 3f40ff090f..c069cd764c 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -45,7 +45,7 @@ static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSI *s, { BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s))); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); - VirtIOSCSIVring *r = g_slice_new(VirtIOSCSIVring); + VirtIOSCSIVring *r; int rc; /* Set up virtqueue notify */ @@ -56,6 +56,8 @@ static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSI *s, s->dataplane_fenced = true; return NULL; } + + r = g_slice_new(VirtIOSCSIVring); r->host_notifier = *virtio_queue_get_host_notifier(vq); r->guest_notifier = *virtio_queue_get_guest_notifier(vq); aio_set_event_notifier(s->ctx, &r->host_notifier, handler); From c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Mon, 16 Mar 2015 22:35:54 -0700 Subject: [PATCH 19/19] exec: Respect as_tranlsate_internal length clamp address_space_translate_internal will clamp the *plen length argument based on the size of the memory region being queried. The iommu walker logic in addresss_space_translate was ignoring this by discarding the post fn call value of *plen. Fix by just always using *plen as the length argument throughout the fn, removing the len local variable. This fixes a bootloader bug when a single elf section spans multiple QEMU memory regions. Signed-off-by: Peter Crosthwaite Message-Id: <1426570554-15940-1-git-send-email-peter.crosthwaite@xilinx.com> Signed-off-by: Paolo Bonzini --- exec.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/exec.c b/exec.c index e97071a3ec..8b922db612 100644 --- a/exec.c +++ b/exec.c @@ -380,7 +380,6 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, IOMMUTLBEntry iotlb; MemoryRegionSection *section; MemoryRegion *mr; - hwaddr len = *plen; rcu_read_lock(); for (;;) { @@ -395,7 +394,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, iotlb = mr->iommu_ops->translate(mr, addr, is_write); addr = ((iotlb.translated_addr & ~iotlb.addr_mask) | (addr & iotlb.addr_mask)); - len = MIN(len, (addr | iotlb.addr_mask) - addr + 1); + *plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1); if (!(iotlb.perm & (1 << is_write))) { mr = &io_mem_unassigned; break; @@ -406,10 +405,9 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, if (xen_enabled() && memory_access_is_direct(mr, is_write)) { hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr; - len = MIN(page, len); + *plen = MIN(page, *plen); } - *plen = len; *xlat = addr; rcu_read_unlock(); return mr;