From b321afbefd2279a8c93b31a2b13f202c017b974c Mon Sep 17 00:00:00 2001 From: Chen Gang Date: Fri, 4 Apr 2014 17:39:33 +0800 Subject: vl: Report accelerator not supported for target more nicely When you ask for an accelerator not supported for your target, you get a bogus "accelerator does not exist" message: $ qemu-system-arm -machine none,accel=kvm KVM not supported for this target "kvm" accelerator does not exist. No accelerator found! Suppress it. Signed-off-by: Chen Gang Reviewed-by: Markus Armbruster Signed-off-by: Michael Tokarev --- vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 9975e5a4e3..db9ea90a1d 100644 --- a/vl.c +++ b/vl.c @@ -2740,7 +2740,7 @@ static int configure_accelerator(QEMUMachine *machine) if (!accel_list[i].available()) { printf("%s not supported for this target\n", accel_list[i].name); - continue; + break; } *(accel_list[i].allowed) = true; ret = accel_list[i].init(machine); -- cgit v1.2.1 From 1634df567d1997099bc9abd9312c8c77f09293da Mon Sep 17 00:00:00 2001 From: Amos Kong Date: Fri, 4 Apr 2014 23:25:02 +0800 Subject: qga: trivial fix for unclear documentation of guest-set-time We mixed the use of "guest time", "system time", "hardware time", "RTC" in documentation, it's unclear. This patch just added two remarks of RTC and replace two "guest time" by "guest's system time". Signed-off-by: Amos Kong Reviewed-by: Michal Privoznik Reviewed-by: Eric Blake Signed-off-by: Michael Tokarev --- qga/commands-posix.c | 2 +- qga/qapi-schema.json | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 6b5f11f83f..935a4ec14d 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -171,7 +171,7 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) /* Now, if user has passed a time to set and the system time is set, we * just need to synchronize the hardware clock. However, if no time was * passed, user is requesting the opposite: set the system time from the - * hardware clock. */ + * hardware clock (RTC). */ pid = fork(); if (pid == 0) { setsid(); diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 80edca143a..a8cdcb35c4 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -96,8 +96,8 @@ ## # @guest-get-time: # -# Get the information about guest time relative to the Epoch -# of 1970-01-01 in UTC. +# Get the information about guest's System Time relative to +# the Epoch of 1970-01-01 in UTC. # # Returns: Time in nanoseconds. # @@ -117,11 +117,11 @@ # gap was, NTP might not be able to resynchronize the # guest. # -# This command tries to set guest time to the given value, -# then sets the Hardware Clock to the current System Time. -# This will make it easier for a guest to resynchronize -# without waiting for NTP. If no @time is specified, then -# the time to set is read from RTC. +# This command tries to set guest's System Time to the +# given value, then sets the Hardware Clock (RTC) to the +# current System Time. This will make it easier for a guest +# to resynchronize without waiting for NTP. If no @time is +# specified, then the time to set is read from RTC. # # @time: #optional time of nanoseconds, relative to the Epoch # of 1970-01-01 in UTC. -- cgit v1.2.1 From d61ce900b968829666336843df957b86815c08ab Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 31 Mar 2014 19:51:55 +0100 Subject: configure: Fix indentation of help for --enable/disable-debug-info The help text for the --enable-debug-info and --disable-debug-info command line options was misindented: delete the stray extra space and bring it in to line with everything else. Signed-off-by: Peter Maydell Signed-off-by: Michael Tokarev --- configure | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 69b9f56847..f073072d38 100755 --- a/configure +++ b/configure @@ -1217,8 +1217,8 @@ Advanced options (experts only): --enable-modules enable modules support --enable-debug-tcg enable TCG debugging --disable-debug-tcg disable TCG debugging (default) - --enable-debug-info enable debugging information (default) - --disable-debug-info disable debugging information + --enable-debug-info enable debugging information (default) + --disable-debug-info disable debugging information --enable-debug enable common debug build options --enable-sparse enable sparse checker --disable-sparse disable sparse checker (default) -- cgit v1.2.1 From 86e117724a463b865accfd31eed383c2652c3d17 Mon Sep 17 00:00:00 2001 From: Hani Benhabiles Date: Tue, 1 Apr 2014 00:05:14 +0100 Subject: net: Report error when device / hub combo is not found. Also convert nearby monitor_printf() call to error_report(). Signed-off-by: Hani Benhabiles Signed-off-by: Michael Tokarev --- net/net.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/net.c b/net/net.c index e3ef1e4f1d..a4aadffc11 100644 --- a/net/net.c +++ b/net/net.c @@ -952,10 +952,12 @@ void net_host_device_remove(Monitor *mon, const QDict *qdict) nc = net_hub_find_client_by_name(vlan_id, device); if (!nc) { + error_report("Host network device '%s' on hub '%d' not found", + device, vlan_id); return; } if (!net_host_check_device(nc->model)) { - monitor_printf(mon, "invalid host network device %s\n", device); + error_report("invalid host network device '%s'", device); return; } qemu_del_net_client(nc); -- cgit v1.2.1 From ee25595f0126de0f83da86cc29ba2365be7a50d2 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 28 Mar 2014 15:12:55 +0000 Subject: hw/ide/ahci.c: Avoid shift left into sign bit Add U suffix to avoid shifting left into the sign bit, which is undefined behaviour. Signed-off-by: Peter Maydell Signed-off-by: Michael Tokarev --- hw/ide/ahci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index bfe633f3a5..50327ffdf1 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -438,9 +438,9 @@ static void check_cmd(AHCIState *s, int port) if ((pr->cmd & PORT_CMD_START) && pr->cmd_issue) { for (slot = 0; (slot < 32) && pr->cmd_issue; slot++) { - if ((pr->cmd_issue & (1 << slot)) && + if ((pr->cmd_issue & (1U << slot)) && !handle_cmd(s, port, slot)) { - pr->cmd_issue &= ~(1 << slot); + pr->cmd_issue &= ~(1U << slot); } } } -- cgit v1.2.1 From 423d00c857ebc814ef6b5fc63f1d6c595cdc005d Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 28 Mar 2014 15:12:56 +0000 Subject: int128.h: Avoid undefined behaviours involving signed arithmetic Add casts when we're performing arithmetic on the .hi parts of an Int128, to avoid undefined behaviour. Signed-off-by: Peter Maydell Signed-off-by: Michael Tokarev --- include/qemu/int128.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/qemu/int128.h b/include/qemu/int128.h index 9ed47aafd3..f59703143a 100644 --- a/include/qemu/int128.h +++ b/include/qemu/int128.h @@ -53,7 +53,7 @@ static inline Int128 int128_rshift(Int128 a, int n) if (n >= 64) { return (Int128) { h, h >> 63 }; } else { - return (Int128) { (a.lo >> n) | (a.hi << (64 - n)), h }; + return (Int128) { (a.lo >> n) | ((uint64_t)a.hi << (64 - n)), h }; } } @@ -78,7 +78,7 @@ static inline Int128 int128_neg(Int128 a) static inline Int128 int128_sub(Int128 a, Int128 b) { - return (Int128){ a.lo - b.lo, a.hi - b.hi - (a.lo < b.lo) }; + return (Int128){ a.lo - b.lo, (uint64_t)a.hi - b.hi - (a.lo < b.lo) }; } static inline bool int128_nonneg(Int128 a) -- cgit v1.2.1 From 968fc24d843c9e9b24231ca1960b47ef2fc724ea Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 28 Mar 2014 15:12:57 +0000 Subject: xbzrle.c: Avoid undefined behaviour with signed arithmetic Use unsigned types for doing bitwise arithmetic in the xzbrle calculations, to avoid undefined behaviour: xbzrle.c:99:49: runtime error: left shift of 72340172838076673 by 7 places cannot be represented in type 'long' Signed-off-by: Peter Maydell Signed-off-by: Michael Tokarev --- xbzrle.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/xbzrle.c b/xbzrle.c index fbcb35d0e3..8e220bf25b 100644 --- a/xbzrle.c +++ b/xbzrle.c @@ -28,7 +28,7 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen, { uint32_t zrun_len = 0, nzrun_len = 0; int d = 0, i = 0; - long res, xor; + long res; uint8_t *nzrun_start = NULL; g_assert(!(((uintptr_t)old_buf | (uintptr_t)new_buf | slen) % @@ -93,9 +93,11 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen, /* word at a time for speed, use of 32-bit long okay */ if (!res) { /* truncation to 32-bit long okay */ - long mask = (long)0x0101010101010101ULL; + unsigned long mask = (unsigned long)0x0101010101010101ULL; while (i < slen) { - xor = *(long *)(old_buf + i) ^ *(long *)(new_buf + i); + unsigned long xor; + xor = *(unsigned long *)(old_buf + i) + ^ *(unsigned long *)(new_buf + i); if ((xor - mask) & ~xor & (mask << 7)) { /* found the end of an nzrun within the current long */ while (old_buf[i] != new_buf[i]) { -- cgit v1.2.1 From e40cdb0e6efb795e4d19368987d53e3e4ae19cf7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 26 Mar 2014 12:45:49 +0100 Subject: scripts: add sample model file for Coverity Scan This is the model file that is being used for the QEMU project's scans on scan.coverity.com. It fixed about 30 false positives (10% of the total) and exposed about 60 new memory leaks. The file is not automatically used; changes to it must be propagated to the website manually by an admin (right now Markus, Peter and me are admins). Signed-off-by: Paolo Bonzini Signed-off-by: Markus Armbruster Signed-off-by: Michael Tokarev --- scripts/coverity-model.c | 183 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 scripts/coverity-model.c diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c new file mode 100644 index 0000000000..4c99a85cfc --- /dev/null +++ b/scripts/coverity-model.c @@ -0,0 +1,183 @@ +/* Coverity Scan model + * + * Copyright (C) 2014 Red Hat, Inc. + * + * Authors: + * Markus Armbruster + * Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or, at your + * option, any later version. See the COPYING file in the top-level directory. + */ + + +/* + * This is the source code for our Coverity user model file. The + * purpose of user models is to increase scanning accuracy by explaining + * code Coverity can't see (out of tree libraries) or doesn't + * sufficiently understand. Better accuracy means both fewer false + * positives and more true defects. Memory leaks in particular. + * + * - A model file can't import any header files. Some built-in primitives are + * available but not wchar_t, NULL etc. + * - Modeling doesn't need full structs and typedefs. Rudimentary structs + * and similar types are sufficient. + * - An uninitialized local variable signifies that the variable could be + * any value. + * + * The model file must be uploaded by an admin in the analysis settings of + * http://scan.coverity.com/projects/378 + */ + +#define NULL ((void *)0) + +typedef unsigned char uint8_t; +typedef char int8_t; +typedef unsigned int uint32_t; +typedef int int32_t; +typedef long ssize_t; +typedef unsigned long long uint64_t; +typedef long long int64_t; +typedef _Bool bool; + +/* exec.c */ + +typedef struct AddressSpace AddressSpace; +typedef uint64_t hwaddr; + +static void __write(uint8_t *buf, ssize_t len) +{ + int first, last; + __coverity_negative_sink__(len); + if (len == 0) return; + buf[0] = first; + buf[len-1] = last; + __coverity_writeall__(buf); +} + +static void __read(uint8_t *buf, ssize_t len) +{ + __coverity_negative_sink__(len); + if (len == 0) return; + int first = buf[0]; + int last = buf[len-1]; +} + +bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf, + int len, bool is_write) +{ + bool result; + + // TODO: investigate impact of treating reads as producing + // tainted data, with __coverity_tainted_data_argument__(buf). + if (is_write) __write(buf, len); else __read(buf, len); + + return result; +} + +/* Tainting */ + +typedef struct {} name2keysym_t; +static int get_keysym(const name2keysym_t *table, + const char *name) +{ + int result; + if (result > 0) { + __coverity_tainted_string_sanitize_content__(name); + return result; + } else { + return 0; + } +} + +/* glib memory allocation functions. + * + * Note that we ignore the fact that g_malloc of 0 bytes returns NULL, + * and g_realloc of 0 bytes frees the pointer. + * + * Modeling this would result in Coverity flagging a lot of memory + * allocations as potentially returning NULL, and asking us to check + * whether the result of the allocation is NULL or not. However, the + * resulting pointer should never be dereferenced anyway, and in fact + * it is not in the vast majority of cases. + * + * If a dereference did happen, this would suppress a defect report + * for an actual null pointer dereference. But it's too unlikely to + * be worth wading through the false positives, and with some luck + * we'll get a buffer overflow reported anyway. + */ + +void *malloc(size_t); +void *calloc(size_t, size_t); +void *realloc(void *, size_t); +void free(void *); + +void * +g_malloc(size_t n_bytes) +{ + void *mem; + __coverity_negative_sink__(n_bytes); + mem = malloc(n_bytes == 0 ? 1 : n_bytes); + if (!mem) __coverity_panic__(); + return mem; +} + +void * +g_malloc0(size_t n_bytes) +{ + void *mem; + __coverity_negative_sink__(n_bytes); + mem = calloc(1, n_bytes == 0 ? 1 : n_bytes); + if (!mem) __coverity_panic__(); + return mem; +} + +void g_free(void *mem) +{ + free(mem); +} + +void *g_realloc(void * mem, size_t n_bytes) +{ + __coverity_negative_sink__(n_bytes); + mem = realloc(mem, n_bytes == 0 ? 1 : n_bytes); + if (!mem) __coverity_panic__(); + return mem; +} + +void *g_try_malloc(size_t n_bytes) +{ + __coverity_negative_sink__(n_bytes); + return malloc(n_bytes == 0 ? 1 : n_bytes); +} + +void *g_try_malloc0(size_t n_bytes) +{ + __coverity_negative_sink__(n_bytes); + return calloc(1, n_bytes == 0 ? 1 : n_bytes); +} + +void *g_try_realloc(void *mem, size_t n_bytes) +{ + __coverity_negative_sink__(n_bytes); + return realloc(mem, n_bytes == 0 ? 1 : n_bytes); +} + +/* Other glib functions */ + +typedef struct _GIOChannel GIOChannel; +GIOChannel *g_io_channel_unix_new(int fd) +{ + GIOChannel *c = g_malloc0(sizeof(GIOChannel)); + __coverity_escape__(fd); + return c; +} + +void g_assertion_message_expr(const char *domain, + const char *file, + int line, + const char *func, + const char *expr) +{ + __coverity_panic__(); +} -- cgit v1.2.1 From 2300aed15d704de102b5577cd0a125bb59d2030a Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Fri, 14 Mar 2014 21:11:13 +0100 Subject: configure: Remove redundant message for -Werror The compiler flag -Werror is printed (or not printed) as any other compiler flag which is part of QEMU_CFLAGS. Therefore an extra output line for -Werror is redundant and can be removed. Signed-off-by: Stefan Weil Signed-off-by: Michael Tokarev --- configure | 1 - 1 file changed, 1 deletion(-) diff --git a/configure b/configure index f073072d38..b08afc3fb8 100755 --- a/configure +++ b/configure @@ -4095,7 +4095,6 @@ echo "sparse enabled $sparse" echo "strip binaries $strip_opt" echo "profiler $profiler" echo "static build $static" -echo "-Werror enabled $werror" if test "$darwin" = "yes" ; then echo "Cocoa support $cocoa" fi -- cgit v1.2.1 From 9d85d557326df69fe0570e7de84b2f57e133c7e7 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Mon, 7 Apr 2014 13:34:58 +0400 Subject: doc: grammify "allows to" English language grammar does not allow usage of the word "allows" directly followed by an infinitive, declaring constructs like "something allows to do somestuff" un-grammatical. Often it is possible to just insert "one" between "allows" and "to" to make the construct grammatical, but usually it is better to re-phrase the statement. This patch tries to fix 4 examples of "allows to" usage in qemu doc, but does not address comments in the code with similar constructs. It also adds missing "the" in the same line. Signed-off-by: Michael Tokarev --- qemu-doc.texi | 2 +- qemu-options.hx | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/qemu-doc.texi b/qemu-doc.texi index e6e20ebbd3..88ec9bb133 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -823,7 +823,7 @@ In this case, the block device must be exported using qemu-nbd: qemu-nbd --socket=/tmp/my_socket my_disk.qcow2 @end example -The use of qemu-nbd allows to share a disk between several guests: +The use of qemu-nbd allows sharing of a disk between several guests: @example qemu-nbd --socket=/tmp/my_socket --share=2 my_disk.qcow2 @end example diff --git a/qemu-options.hx b/qemu-options.hx index 2d33815fb9..6457034b8c 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -444,7 +444,8 @@ This option defines the type of the media: disk or cdrom. @item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}] These options have the same definition as they have in @option{-hdachs}. @item snapshot=@var{snapshot} -@var{snapshot} is "on" or "off" and allows to enable snapshot for given drive (see @option{-snapshot}). +@var{snapshot} is "on" or "off" and controls snapshot mode for the given drive +(see @option{-snapshot}). @item cache=@var{cache} @var{cache} is "none", "writeback", "unsafe", "directsync" or "writethrough" and controls how the host cache is used to access block data. @item aio=@var{aio} @@ -1242,7 +1243,7 @@ Disable adaptive encodings. Adaptive encodings are enabled by default. An adaptive encoding will try to detect frequently updated screen regions, and send updates in these regions using a lossy encoding (like JPEG). This can be really helpful to save bandwidth when playing videos. Disabling -adaptive encodings allows to restore the original static behavior of encodings +adaptive encodings restores the original static behavior of encodings like Tight. @item share=[allow-exclusive|force-shared|ignore] @@ -2805,7 +2806,7 @@ UTC or local time, respectively. @code{localtime} is required for correct date i MS-DOS or Windows. To start at a specific point in time, provide @var{date} in the format @code{2006-06-17T16:01:21} or @code{2006-06-17}. The default base is UTC. -By default the RTC is driven by the host system time. This allows to use the +By default the RTC is driven by the host system time. This allows using of the RTC as accurate reference clock inside the guest, specifically if the host time is smoothly following an accurate external reference clock, e.g. via NTP. If you want to isolate the guest time from the host, you can set @option{clock} -- cgit v1.2.1 From b36dc67b95dedcece8757ec23bf42625a7ccda34 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Mon, 7 Apr 2014 19:42:59 +0200 Subject: Fix grammar in comment Signed-off-by: Stefan Weil Reviewed-by: Peter Crosthwaite Signed-off-by: Michael Tokarev --- hw/i2c/smbus_eeprom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 86f35c11de..72c09cba6b 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -71,7 +71,7 @@ static void eeprom_write_data(SMBusDevice *dev, uint8_t cmd, uint8_t *buf, int l printf("eeprom_write_byte: addr=0x%02x cmd=0x%02x val=0x%02x\n", dev->i2c.address, cmd, buf[0]); #endif - /* An page write operation is not a valid SMBus command. + /* A page write operation is not a valid SMBus command. It is a block write without a length byte. Fortunately we get the full block anyway. */ /* TODO: Should this set the current location? */ -- cgit v1.2.1