Re: [Qemu-devel] [PATCH] memsave: Add a space after address in error message
Cc: qemu-triv...@nongnu.org
On 07/02/2015 21:29, Borislav Petkov wrote:
From: Borislav Petkov b...@suse.de
Add the missing space to separate address from specified.
Cc: Anthony Liguori aligu...@amazon.com
Cc: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Borislav Petkov b...@suse.de
---
cpus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cpus.c b/cpus.c
index 0cdd1d71560b..4fa196d48207 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1489,7 +1489,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char
*filename,
if (l size)
l = size;
if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
-error_setg(errp, Invalid addr 0x%016 PRIx64 specified, addr);
+error_setg(errp, Invalid addr 0x%016 PRIx64 specified,
addr);
goto exit;
}
if (fwrite(buf, 1, l, f) != l) {
Re: [Qemu-devel] [question] the patch which affect performance of virtio-scsi
On 07/02/2015 12:26, Wangting (Kathy) wrote: OK, Thank you very much for your detailed explanation. But I have another question about the big change from qemu-1.5.3 to qemu-1.6.0-rc0. When I use ramdisk for IO performance testing, the result is as follows. [fio-test] rw bs iodepthjobs bw iops qemu-1.5.3 read 4k 32 1 285MB/s 73208 qemu-1.6.0-rc0 read 4k 32 1 253MB/s 64967 And virtio-blk is the same. I know there are so many differences between qemu-1.5 and qemu-1.6, but I am confused about what new features impact the performance so much. Do you know it? No, sorry. Please try newer versions first and see if this was fixed. QEMU 1.6 is more than a year old. Paolo
Re: [Qemu-devel] [PATCH v3] pc: acpi-build: make linker RSDP tables dynamic
On 02/04/2015 11:01 AM, Igor Mammedov wrote:
Linker and RSDP tables are build only once, so if later
during rebuild sizes of other ACPI tables change
pointers will be patched incorrectly due to wrong
offsets in RSDP and linker.
To fix it rebuild linker and RSDP tables along with
the rest of ACPI tables so that they would have
offsets that match just built tables.
Here is a simple reproducer:
1: hotplug bridge using command:
device_add pci-bridge,chassis_nr=1
2: reset system from monitor:
system_reset
As result pointers to ACPI tables are not correct
and guest can't read/parse ACPI tables and on top
of it linker corrupted them by patching at stale
offsets.
Windows guests just refuses to boot and
Linux guests are more resilient and try to boot without
ACPI, sometimes successfully.
Fix applies only to new machine types starting from 2.3,
so it won't break migration for old machine types.
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
hw/i386/acpi-build.c | 27 ---
hw/i386/pc_piix.c| 3 +++
hw/i386/pc_q35.c | 3 +++
include/hw/i386/pc.h | 1 +
4 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4944249..58cf8b7 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1523,6 +1523,10 @@ struct AcpiBuildState {
/* Copy of table in RAM (for patching). */
ram_addr_t table_ram;
uint32_t table_size;
+ram_addr_t linker_ram;
+uint32_t linker_size;
+ram_addr_t rsdp_ram;
+uint32_t rsdp_size;
/* Is table patched? */
uint8_t patched;
PcGuestInfo *guest_info;
@@ -1733,6 +1737,10 @@ static void acpi_build_update(void *build_opaque,
uint32_t offset)
memcpy(qemu_get_ram_ptr(build_state-table_ram), tables.table_data-data,
build_state-table_size);
+memcpy(qemu_get_ram_ptr(build_state-linker_ram), tables.linker-data,
+ build_state-linker_size);
+memcpy(qemu_get_ram_ptr(build_state-rsdp_ram), tables.rsdp-data,
+ build_state-rsdp_size);
cpu_physical_memory_set_dirty_range_nocode(build_state-table_ram,
build_state-table_size);
@@ -1799,17 +1807,22 @@ void acpi_setup(PcGuestInfo *guest_info)
assert(build_state-table_ram != RAM_ADDR_MAX);
build_state-table_size = acpi_data_len(tables.table_data);
-acpi_add_rom_blob(NULL, tables.linker, etc/table-loader, 0);
+build_state-linker_ram = acpi_add_rom_blob(build_state, tables.linker,
+etc/table-loader, 0);
+build_state-linker_size = acpi_data_len(tables.linker);
fw_cfg_add_file(guest_info-fw_cfg, ACPI_BUILD_TPMLOG_FILE,
tables.tcpalog-data, acpi_data_len(tables.tcpalog));
-/*
- * RSDP is small so it's easy to keep it immutable, no need to
- * bother with ROM blobs.
- */
-fw_cfg_add_file(guest_info-fw_cfg, ACPI_BUILD_RSDP_FILE,
-tables.rsdp-data, acpi_data_len(tables.rsdp));
+if (guest_info-has_imutable_rsdp) {
+/* Keep for compatibility with old machine types */
+fw_cfg_add_file(guest_info-fw_cfg, ACPI_BUILD_RSDP_FILE,
+tables.rsdp-data, acpi_data_len(tables.rsdp));
+} else {
+build_state-rsdp_ram = acpi_add_rom_blob(build_state, tables.rsdp,
+ ACPI_BUILD_RSDP_FILE, 0);
+build_state-rsdp_size = acpi_data_len(tables.rsdp);
+}
qemu_register_reset(acpi_build_reset, build_state);
acpi_build_reset(build_state);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 38b42b0..866b783 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,6 +60,7 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static bool has_acpi_build = true;
+static bool has_imutable_rsdp;
imutable - immutable
Other than that, it looks good and clean to me.
Thanks,
Marcel
static int legacy_acpi_table_size;
static bool smbios_defaults = true;
static bool smbios_legacy_mode;
@@ -168,6 +169,7 @@ static void pc_init1(MachineState *machine,
guest_info-isapc_ram_fw = !pci_enabled;
guest_info-has_reserved_memory = has_reserved_memory;
+guest_info-has_imutable_rsdp = has_imutable_rsdp;
if (smbios_defaults) {
MachineClass *mc = MACHINE_GET_CLASS(machine);
@@ -310,6 +312,7 @@ static void pc_init_pci(MachineState *machine)
static void pc_compat_2_2(MachineState *machine)
{
+has_imutable_rsdp = true;
x86_cpu_compat_set_features(kvm64, FEAT_1_EDX, 0, CPUID_VME);
x86_cpu_compat_set_features(kvm32, FEAT_1_EDX, 0, CPUID_VME);
x86_cpu_compat_set_features(Conroe, FEAT_1_EDX, 0, CPUID_VME);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 63027ee..6f649a1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -50,6 +50,7 @@
Re: [Qemu-devel] [PATCH 1/7] softfloat: Fix sNaN handling in FP conversion operations
On Fri, 6 Feb 2015, Maciej W. Rozycki wrote:
I think this means that:
(1) we want to put handling of silencing the signaling NaNs
into the NaN conversion functions themselves (since it's
too late to do it correctly once the commonNaNtoFloat16
function has thrown away data)
(2) that handling needs to be target specific, as most details
of quiet vs signaling NaNs are
I note in passing that the float*ToCommonNaN and commonNaNToFloat*
functions are used only in the back-to-back call sequence of
return commonNaNToFloatX(floatYToCommonNaN(...))
for handling the NaN inputs on FP-to-FP conversion.
I believe my proposal addresses your concerns in a burden-free way for
target maintainers who look after processors that implement the IEEE 754
standard as it stands.
I don't, which is why I made the comment above. If you're going to
fix up NaN handling in the float-to-float conversion routines we
should do it in a way that lets us handle the behaviour of
target CPUs we care about.
No argument about maintaining correct emulation where it already is such,
sure. Please note that `float64_maybe_silence_nan' and friends are
already target-specific, which should generally let us deal with things,
and I am sort of surprised ARM sets certain rules for sNaN silencing for
conversions and different ones for other arithmetic operations. Or is it
really that an input sNaN's sign bit is propagated in the single weird
corner case only?
OK, I see where the issue is -- this is specifically about the case where
dropping trailing significand bits of an sNaN in the process of format
conversion would result in an infinity being produced. This is handled by
`commonNaNToFloat*' functions, but not in a way the ARM architecture
requires. As a preexisting problem this has nothing to do with my
proposal and neither my proposal makes things worse, though I indeed
missed the opportunity to correct it.
I've thought of two general solutions:
1. Given that `*_default_nan' are now functions make them accept the sign
bit as input and propagate it to output if required by a given
implementation. Conversions would pass the input sign bit while
arithmetic operations would hardcode it to 0.
2. Quieten an sNaN being converted in the wider format operated on. For a
widening conversion it would be the output format just as in the
original patch proposed. For a narrowing conversion it would be the
input format.
I'm leaning towards the second option as being more elegant and robust.
So to give a specific example, `float32_to_float64' would have:
if (aSig) {
float64 nan;
nan = commonNaNToFloat64(float32ToCommonNaN(a, status), status);
return float64_maybe_silence_nan(nan);
}
but `float64_to_float32' would have:
if (aSig) {
float64 nan;
nan = float64_maybe_silence_nan(a);
return commonNaNToFloat32(float64ToCommonNaN(nan, status), status);
}
instead. This does the right thing as far as NaN quietening is concerned
and avoids the `!mantissa' case triggering in `commonNaNToFloat32'.
Based on the lone contents of fpu/softfloat-specialize.h this solution
looks correct to me, in particular for the SNAN_BIT_IS_ONE case -- are you
aware of any processor architectures for which this approach would not
produce the result required?
I've had a look at target-arm/helper.c and target-arm/helper-a64.c and
AFAICT the hacks they do for NaN quietening do not follow the architecture
specification in narrowing conversions (they merely reproduce what I
proposed with the original patch and therefore do not preserve the sign
bit in the case concerned), so the approach I outlined above will actually
progress these targets as well, and these hacks can then be removed.
This solution also meets the IEEE 754-2008 requirement for the way NaN
quietening is done where the preferred NaN encoding is implemented.
Maciej
[Qemu-devel] [PATCH v8] Support vhd type VHD_DIFFERENCING
Now qemu only supports vhd type VHD_FIXED and VHD_DYNAMIC, so qemu
can't read snapshot volume of vhd, and can't support other storage
features of vhd file.
This patch add read parent information in function vpc_open, read
bitmap in vpc_read, and change bitmap in vpc_write.
Signed-off-by: Xiaodong Gong gongxiaodo...@huawei.com
Reviewed-by: Ding xiao ssdx...@163.com
---
Changes since v7:
- use iconv to decode UTF-16LE(w2u) and UTF-8(macx) to ASCII
(Stefan Hajnoczi)
- change the return value of vpc_write back to 0 (Stefan Hajnoczi)
Changes since v6:
- remove include of iconv.h (Stefan Hajnoczi)
- make sure data_length length of backing_file (Stefan Hajnoczi)
- change big-ending of platform to cpu order (Stefan Hajnoczi)
Changes since v5:
- Change malloc to g_malloc. (Gonglei)(Stefan Hajnoczi)
- Fix the bug of free(null). (Gonglei)(Stefan Hajnoczi)
Changes since v4:
- Parse the batmap only when the version of VHD 1.2. (Lucian Petrut)
- Add support to parent location of W2RU. (Lucian Petrut) (Philipp Hahn)
Changes since v3:
- Remove the PARENT_MAX_LOC.
Changes since v2:
- Change MACX to PLATFAORM_MACX. (Kevin Wolf)
- Return with EINVAL to parent location is W2RU and W2KU. (Kevin Wolf)
- Change -1 == ret to a natrual order of ret == -1. (Kevin Wolf)
- Get rid of the get_sector_offset_diff, get_sector_offset
supports VHD_DIFF. (Kevin Wolf)
- Return code of get_sector_offset is set to, -1 for error,
-2 for not allocate, -3 for in parent. (Kevin Wolf)
- Fix un init ret of vpc_write, when nb_sector == 0. (Kevin Wolf)
- Change if (diff == ture) to if (diff) and so on. (Kevin Wolf)
- Add PARENT_MAX_LOC to more understand. (Kevin Wolf)
- Restore the boundary check to write on dynamic type in
get_sector_offset. (Kevin Wolf)
Changes since v1:
- Add Boundary check to any input. (Stefan Hajnoczi)
- Clean the code no used after in vpc_open. (Stefan Hajnoczi)
- Change bdrv_co_readv() to bdrv_preadv in vpc_read. (Stefan Hajnoczi)
- Added some code to make it easy to understand. (Stefan Hajnoczi)
---
block/vpc.c | 549
1 file changed, 483 insertions(+), 66 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 46803b1..c41fd39 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -29,17 +29,29 @@
#if defined(CONFIG_UUID)
#include uuid/uuid.h
#endif
+#include iconv.h
/**/
#define HEADER_SIZE 512
+#define DYNAMIC_HEADER_SIZE 1024
+#define PARENT_LOCATOR_NUM 8
+#define TBBATMAP_HEAD_SIZE 28
+
+#define MACX_PREFIX_LEN 7 /* file:// */
+
+#define PLATFORM_MACX 0x5863614d /* big endian */
+#define PLATFORM_W2RU 0x75723257
+#define PLATFORM_W2KU 0x756B3257
+
+#define VHD_VERSION(major, minor) (((major) 16) | ((minor) 0x))
//#define CACHE
enum vhd_type {
VHD_FIXED = 2,
VHD_DYNAMIC = 3,
-VHD_DIFFERENCING= 4,
+VHD_DIFF= 4,
};
// Seconds since Jan 1, 2000 0:00:00 (UTC)
@@ -138,6 +150,15 @@ typedef struct BDRVVPCState {
Error *migration_blocker;
} BDRVVPCState;
+typedef struct vhd_tdbatmap_header {
+char magic[8]; /* always tdbatmap */
+
+uint64_t batmap_offset;
+uint32_t batmap_size;
+uint32_t batmap_version;
+uint32_t checksum;
+} QEMU_PACKED VHDTdBatmapHeader;
+
static uint32_t vpc_checksum(uint8_t* buf, size_t size)
{
uint32_t res = 0;
@@ -157,6 +178,225 @@ static int vpc_probe(const uint8_t *buf, int buf_size,
const char *filename)
return 0;
}
+static int vpc_decode_maxc_loc(BlockDriverState *bs, char data_length)
+{
+iconv_t cd;
+char *inbuf, *outbuf, *buf;
+size_t inbyteleft, outbyteleft, outbyte;
+int ret;
+
+if (!bs || !bs-backing_file || !data_length) {
+return -1;
+}
+
+cd = iconv_open(ASCII, UTF8);
+if (cd == (iconv_t) -1) {
+return -1;
+}
+
+inbuf = bs-backing_file;
+outbuf = buf = (char *) g_malloc(data_length + 1);
+if (!outbuf) {
+ret = -1;
+goto fail2;
+}
+inbyteleft = outbyteleft = data_length;
+
+ret = iconv(cd, inbuf, inbyteleft, outbuf, outbyteleft);
+if (ret == (size_t) -1 || inbyteleft) {
+ret = -1;
+goto fail1;
+}
+outbyte = data_length - outbyteleft;
+if (outbyte sizeof(bs-backing_file) - 1) {
+ret = -1;
+goto fail1;
+}
+if (outbyte MACX_PREFIX_LEN) {
+ret = -1;
+goto fail1;
+}
+buf[outbyte] = '\0';
+
+strncpy(bs-backing_file, buf + MACX_PREFIX_LEN,
+outbyte - MACX_PREFIX_LEN);
+bs-backing_file[outbyte - MACX_PREFIX_LEN] = '\0';
+
+fail1:
+g_free(buf);
+buf = NULL;
+
+fail2:
+ret = iconv_close(cd);
+if (ret == -1) {
+return -1;
+}
+
+return 0;
+}
+
+static int vpc_decode_w2u_loc(BlockDriverState *bs, char data_length)
+{
+iconv_t cd;
+char *buf, *inbuf, *outbuf;
+size_t inbyteleft, outbyteleft, outbyte;
+char *ptr;
+
Re: [Qemu-devel] [Qemu-trivial] [PATCH 00/10] Fix warnings (undeclared global symbols)
07.02.2015 00:43, Stefan Weil wrote: All warnings were reported by the Sparse static analysis tool. [PATCH 01/10] disas/cris: Fix warning caused by missing 'static' [PATCH 02/10] disas/sh4: Fix warning caused by missing 'static' [PATCH 03/10] migration: Fix warning caused by missing declaration of [PATCH 04/10] migration: Fix warnings caused by missing 'static' [PATCH 05/10] moxie: Fix warning caused by missing include statement [PATCH 06/10] serial: Fix warnings caused by missing 'static' [PATCH 07/10] spice: Add missing 'static' attribute [PATCH 08/10] stubs: Fix warning caused by missing include statement [PATCH 09/10] vga: Fix warning caused by missing 'static' attribute [PATCH 10/10] virtio: Fix warning caused by missing 'static' Applied all 10 to -trivial, good stuff! Thanks, /mjt
Re: [Qemu-devel] [PATCH RESEND] Support vhd type VHD_DIFFERENCING
On Wed, Nov 26, 2014 at 11:32 PM, Stefan Hajnoczi stefa...@gmail.com
wrote:
On Thu, Nov 06, 2014 at 10:43:50PM +0800, Xiaodong Gong wrote:
+} else if (platform == PLATFORM_W2RU) {
+/* Must be UTF16-LE to ASCII */
+char *out, *optr;
+int j;
+
+optr = out = (char *) g_malloc(data_length + 1);
+if (out == NULL) {
+ret = -1;
+return ret;
+}
+
+for (j = 0; j data_length + 1; j++) {
+out[j] = bs-backing_file[2 * j];
+}
+out[data_length + 1] = '\0';
+
+while (*optr != '\0') {
+if (*optr == '\\') {
+*optr = '/';
+}
+optr++;
+}
+
+strncpy(bs-backing_file, out, data_length + 1);
+
+g_free(out);
+out = NULL;
+
+done = true;
+}
Please convert from UTF-16 LE to the local file system character set:
https://developer.gnome.org/glib/stable/glib-Character-Set-Conversion.html
Also, using -backing_file[] when the data is UTF-16 LE encoded is not
ideal since it halves the maximum size of the string! It would be
better to read into a temporary buffer that is 2 *
sizeof(backing_file[]) big before writing into -backing_file[].
Okay, I'll use iconv() to convert UTF-16LE and UTF-8 to ASCII. why g_iconv
in
glib ? And the data_length is the total length of location, 2 *
sizeof(backing_file[]) is no need.
@@ -286,6 +411,37 @@ static int vpc_open(BlockDriverState *bs, QDict
*options, int flags,
s-free_data_block_offset =
(s-bat_offset + (s-max_table_entries * 4) + 511) ~511;
+/* Read tdbatmap header by offset */
+if (footer-version = VHD_VERSION(1, 2)) {
Missing be32_to_cpu(footer-version)
Okay
+static int vpc_write(BlockDriverState *bs, int64_t sector_num,
+const uint8_t *buf, int nb_sectors)
+{
+BDRVVPCState *s = bs-opaque;
+VHDFooter *footer = (VHDFooter *) s-footer_buf;
+int64_t sectors_per_block = s-block_size BDRV_SECTOR_BITS;
+int64_t offset, sectors;
+bool diff = true;
+int ret = 0;
+
+switch (be32_to_cpu(footer-type)) {
+case VHD_FIXED:
+return bdrv_write(bs-file, sector_num, buf, nb_sectors);
+case VHD_DYNAMIC:
+case VHD_DIFF:
+if (be32_to_cpu(footer-type) == VHD_DYNAMIC) {
+diff = false;
}
This can be done with a fall-through case instead of checking
footer-type again.
case VHD_DYNAMIC:
diff = false;
/* fall-through */
case VHD_DIFF:
Okay, this is more clear
+ret = bdrv_pwrite(bs-file, offset, buf,
+sectors * BDRV_SECTOR_SIZE);
+if (ret != sectors * BDRV_SECTOR_SIZE) {
+return -1;
+}
-nb_sectors -= sectors;
-sector_num += sectors;
-buf += sectors * BDRV_SECTOR_SIZE;
-}
+if (diff) {
+ret = write_bitmap(bs, sector_num, sectors);
+if (ret 0) {
+return -1;
+}
+}
-return 0;
+nb_sectors -= sectors;
+sector_num += sectors;
+buf += sectors * BDRV_SECTOR_SIZE;
+}
+break;
+default:
+return -1;
+}
+return ret;
In the VHD_DYNAMIC case we must *not* return the number of bytes from
bdrv_pwrite(). Should this be return 0?
It is 0 originally, I' ll change it to back. But ret of Fixed format is
not 0 and
0 has no side effect.
[Qemu-devel] [PATCH] block:Fix FIXED base vpc be probed to raw format
When open the vpc snapshot based FIXED format, its backing file,
this FIXED vpc image, could be probed as a raw image, because that
the find_image_format just checkout the first sector.
Add a re-probe for the last sector to FIXED base vpc image,when get
a NULL or raw driver in first sector probe.
Signed-off-by: Xiaodong Gong gongxiaodo...@huawei.com
---
block.c | 55 ---
1 file changed, 48 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index d45e4dd..e183515 100644
--- a/block.c
+++ b/block.c
@@ -702,20 +702,24 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int
buf_size,
return drv;
}
-static int find_image_format(BlockDriverState *bs, const char *filename,
- BlockDriver **pdrv, Error **errp)
+static int probe_image_format(BlockDriverState *bs, const char *filename,
+ int64_t offset, BlockDriver **pdrv, Error **errp,
+ bool isfoot)
{
BlockDriver *drv;
uint8_t buf[BLOCK_PROBE_BUF_SIZE];
int ret = 0;
-/* Return the raw BlockDriver * to scsi-generic devices or empty drives */
-if (bs-sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
-*pdrv = bdrv_raw;
+if (!bs || !filename) {
+*pdrv = NULL;
return ret;
}
-ret = bdrv_pread(bs, 0, buf, sizeof(buf));
+if (offset + sizeof(buf) bs-total_sectors BDRV_SECTOR_BITS) {
+return ret;
+}
+
+ret = bdrv_pread(bs, offset, buf, sizeof(buf));
if (ret 0) {
error_setg_errno(errp, -ret, Could not read image for determining its
format);
@@ -724,12 +728,49 @@ static int find_image_format(BlockDriverState *bs, const
char *filename,
}
drv = bdrv_probe_all(buf, ret, filename);
-if (!drv) {
+if (!drv isfoot) {
error_setg(errp, Could not determine image format: No compatible
driver found);
ret = -ENOENT;
}
*pdrv = drv;
+
+return ret;
+}
+
+static int find_image_format(BlockDriverState *bs, const char *filename,
+ BlockDriver **pdrv, Error **errp)
+{
+int last_sector_offset;
+int ret = 0;
+
+*pdrv = NULL;
+
+/* Return the raw BlockDriver * to scsi-generic devices or empty drives */
+if (bs-sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
+*pdrv = bdrv_raw;
+return ret;
+}
+
+/* probe the header to guess image format */
+ret = probe_image_format(bs, filename, 0, pdrv, errp, false);
+if (ret 0) {
+return ret;
+}
+
+/* The FIXED base img of VHD only has the footer in tail. The drv could
+ * be NULL or raw */
+if (*pdrv *pdrv != bdrv_raw) {
+return ret;
+}
+
+last_sector_offset = (bs-total_sectors - 1) BDRV_SECTOR_BITS;
+ret = probe_image_format(bs, filename, last_sector_offset, pdrv,
+errp, true);
+if (ret 0) {
+return ret;
+}
+
return ret;
}
--
1.8.3.1
Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
On Fri, Feb 06, 2015 at 01:14:46PM +0100, Cornelia Huck wrote: On Fri, 6 Feb 2015 13:41:26 +0800 Tiejun Chen tiejun.c...@intel.com wrote: Actually we define these device IDs in virtio standard, so we'd better put them into one common place to manage conveniently. Here I also add VIRTIO_ID_RESERVE according to virtio spec. Signed-off-by: Tiejun Chen tiejun.c...@intel.com We really should just write a script to import the headers from the linux kernel. They will need some tweaks to avoid dependencies on linux/types, but this seems easy to do - better than trying to keep things in sync manually. -- MST
Re: [Qemu-devel] [PATCH v2 0/6] Trivial cleanups around g_malloc()
04.02.2015 13:26, Markus Armbruster wrote: I'm routing these patches through qemu-trivial, even though some of them touch actively maintained code, and could go through the respective tree instead: * PATCH 1 block (Kevin, Stefan) * PATCH 3 KVM (Paolo) * PATCH 4 migration (Juan, Amit) * PATCH 5 VNC (Gerd) If you want me to reroute any of them, let me know. v2: * Trivially rebased, R-bys retained Markus Armbruster (6): onenand: g_malloc() can't fail, bury dead error handling rtl8139: g_malloc() can't fail, bury dead error handling kvm: g_malloc() can't fail, bury dead error handling rdma: g_malloc0() can't fail, bury dead error handling vnc: g_realloc() can't fail, bury dead error handling translate-all: Use g_try_malloc() for dynamic translator buffer Applied all to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH 2/5] virtio: increase VIRITO_QUEUE_MAX to 513
On Fri, Feb 06, 2015 at 03:54:10PM +0800, Jason Wang wrote: Recent linux kernel supports up to 256 tap queues. Increase the limit to 513 (256 * 2 + 1(ctrl vq)). Signed-off-by: Jason Wang jasow...@redhat.com We have a bunch of loops over all possible VQ numbers in virtio pci. Doing this for 1000 VQs when most of them are inactive seems extreme - I suspect we need a list of active VQs now. --- include/hw/virtio/virtio.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 220c09d..21bb30f 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -92,7 +92,7 @@ typedef struct VirtQueueElement struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; } VirtQueueElement; -#define VIRTIO_QUEUE_MAX 64 +#define VIRTIO_QUEUE_MAX 513 #define VIRTIO_NO_VECTOR 0x -- 1.9.1
[Qemu-devel] [PATCH] memsave: Improve and disambiguate error message
On Sun, Feb 08, 2015 at 11:09:24AM +0100, Paolo Bonzini wrote:
Cc: qemu-triv...@nongnu.org
Thanks.
But, there's more b0rked with this error message so you might wanna take
the patch below instead.
Btw, what are the vim settings when doing patches for qemu, this must be
documented somewhere as coding style differs significantly from that of
the kernel?
:-)
---
From: Borislav Petkov b...@suse.de
Subject: [PATCH] memsave: Improve and disambiguate error message
When requesting a size which cannot be read, the error message shows
a different address which is misleading to the user and it looks like
something's wrong with the address parsing. This is because the input
@addr variable is incremented in the memory dumping loop:
(qemu) memsave 0x8418069c 0xb0 mem
Invalid addr 0x849ffe9c specified
Fix that by saving the original address and size and use them in the
error message:
(qemu) memsave 0x8418069c 0xb0 mem
Invalid addr 0x8418069c/size 11534336 specified
Cc: Anthony Liguori aligu...@amazon.com
Cc: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Borislav Petkov b...@suse.de
---
cpus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/cpus.c b/cpus.c
index 0cdd1d71560b..a5c7cad00fc9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1466,6 +1466,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char
*filename,
uint32_t l;
CPUState *cpu;
uint8_t buf[1024];
+int64_t orig_addr = addr, orig_size = size;
if (!has_cpu) {
cpu_index = 0;
@@ -1489,7 +1490,8 @@ void qmp_memsave(int64_t addr, int64_t size, const char
*filename,
if (l size)
l = size;
if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
-error_setg(errp, Invalid addr 0x%016 PRIx64 specified, addr);
+error_setg(errp, Invalid addr 0x%016 PRIx64 /size % PRId64
+ specified, orig_addr, orig_size);
goto exit;
}
if (fwrite(buf, 1, l, f) != l) {
--
2.2.0.33.gc18b867
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
Re: [Qemu-devel] [PATCH RESEND] Support vhd type VHD_DIFFERENCING
On Wed, Nov 26, 2014 at 11:57 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Thu, Nov 06, 2014 at 10:43:50PM +0800, Xiaodong Gong wrote: Now qemu only supports vhd type VHD_FIXED and VHD_DYNAMIC, so qemu can't read snapshot volume of vhd, and can't support other storage features of vhd file. This patch add read parent information in function vpc_open, read bitmap in vpc_read, and change bitmap in vpc_write. Signed-off-by: Xiaodong Gong gongxiaodo...@huawei.com Reviewed-by: Ding xiao ssdx...@163.com --- Have you run tests/qemu-iotests/check for all 3 subformats (fixed, dynamic, and differencing)? http://qemu-project.org/Documentation/QemuIoTests The tests need to pass before we can merge a new subforma There are 004, 006, 099 and 104 of test case that do not be passed, the same as the current master branch. 004 to sure that writing/reading offset after size of image fails, but not. 006 to sure that creating a 128G image fails, but not. 099 to sure that parsing args of qemu --drive is robust. 104 to sure that real size of image is 1024 for 1024, 1536 for 1025(not 1024 align), but all is 34816 And qemu-img could not create the snapshot of vpc, so _make_test_img is not works with args of '-b'. vhd_differencing format cloud not be tested with qemu-iostest now.
[Qemu-devel] [PATCH 2/7] vl.c: Remove unnecessary zero-initialization of NUMA globals
There's no need to zero-initialize globals, they are automatically
initialized to zero.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
vl.c | 8
1 file changed, 8 deletions(-)
diff --git a/vl.c b/vl.c
index 63ec996..8a32a4b 100644
--- a/vl.c
+++ b/vl.c
@@ -2818,14 +2818,6 @@ int main(int argc, char **argv, char **envp)
cyls = heads = secs = 0;
translation = BIOS_ATA_TRANSLATION_AUTO;
-for (i = 0; i MAX_NODES; i++) {
-numa_info[i].node_mem = 0;
-numa_info[i].present = false;
-bitmap_zero(numa_info[i].node_cpu, MAX_CPUMASK_BITS);
-}
-
-nb_numa_nodes = 0;
-max_numa_nodeid = 0;
nb_nics = 0;
bdrv_init_with_whitelist();
--
2.1.0
[Qemu-devel] [PATCH 7/7] numa: Rename set_numa_modes() to numa_post_machine_init()
This function does some initialization that needs to be done after
machine init. The function may be eventually removed if we move the
CPUState.numa_node initialization to the CPU init code, but while the
function exists, lets give it a name that makes sense.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
include/sysemu/numa.h | 2 +-
numa.c| 2 +-
vl.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index 453b49a..5633b85 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -17,7 +17,7 @@ typedef struct node_info {
} NodeInfo;
extern NodeInfo numa_info[MAX_NODES];
void parse_numa_opts(void);
-void set_numa_modes(void);
+void numa_post_machine_init(void);
void query_numa_node_mem(uint64_t node_mem[]);
extern QemuOptsList qemu_numa_opts;
diff --git a/numa.c b/numa.c
index d5b95e1..0d15375 100644
--- a/numa.c
+++ b/numa.c
@@ -246,7 +246,7 @@ void parse_numa_opts(void)
}
}
-void set_numa_modes(void)
+void numa_post_machine_init(void)
{
CPUState *cpu;
int i;
diff --git a/vl.c b/vl.c
index 7243216..f8e516f 100644
--- a/vl.c
+++ b/vl.c
@@ -4213,7 +4213,7 @@ int main(int argc, char **argv, char **envp)
cpu_synchronize_all_post_init();
-set_numa_modes();
+numa_post_machine_init();
/* init USB devices */
if (usb_enabled()) {
--
2.1.0
[Qemu-devel] [PATCH 5/7] numa: Move QemuOpts parsing to set_numa_nodes()
This allows us to make numa_init_func() static.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
include/sysemu/numa.h | 1 -
numa.c| 9 -
vl.c | 5 -
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index eae881e..d25ada9 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -20,6 +20,5 @@ void set_numa_nodes(void);
void set_numa_modes(void);
void query_numa_node_mem(uint64_t node_mem[]);
extern QemuOptsList qemu_numa_opts;
-int numa_init_func(QemuOpts *opts, void *opaque);
#endif
diff --git a/numa.c b/numa.c
index 84d4cb5..eb9259b 100644
--- a/numa.c
+++ b/numa.c
@@ -36,6 +36,8 @@
#include sysemu/hostmem.h
#include qmp-commands.h
#include hw/mem/pc-dimm.h
+#include qemu/option.h
+#include qemu/config-file.h
QemuOptsList qemu_numa_opts = {
.name = numa,
@@ -121,7 +123,7 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts
*opts, Error **errp)
max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
}
-int numa_init_func(QemuOpts *opts, void *opaque)
+static int numa_init_func(QemuOpts *opts, void *opaque)
{
NumaOptions *object = NULL;
Error *err = NULL;
@@ -168,6 +170,11 @@ void set_numa_nodes(void)
{
int i;
+if (qemu_opts_foreach(qemu_find_opts(numa), numa_init_func,
+ NULL, 1) != 0) {
+exit(1);
+}
+
assert(max_numa_nodeid = MAX_NODES);
/* No support for sparse NUMA node IDs yet: */
diff --git a/vl.c b/vl.c
index 5adea75..cfce820 100644
--- a/vl.c
+++ b/vl.c
@@ -4154,11 +4154,6 @@ int main(int argc, char **argv, char **envp)
default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
-if (qemu_opts_foreach(qemu_find_opts(numa), numa_init_func,
- NULL, 1) != 0) {
-exit(1);
-}
-
set_numa_nodes();
if (qemu_opts_foreach(qemu_find_opts(mon), mon_init_func, NULL, 1) != 0)
{
--
2.1.0
[Qemu-devel] [PATCH 1/7] numa: Move NUMA declarations from sysemu.h to numa.h
Not all sysemu.h users need the NUMA declarations, and keeping them in a
separate file makes easier to see what are the interfaces provided by
numa.c.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
hw/i386/pc.c| 1 +
hw/mem/pc-dimm.c| 1 +
hw/ppc/spapr.c | 1 +
include/sysemu/numa.h | 28
include/sysemu/sysemu.h | 18 --
monitor.c | 1 +
numa.c | 2 +-
vl.c| 1 +
8 files changed, 34 insertions(+), 19 deletions(-)
create mode 100644 include/sysemu/numa.h
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c7af6aa..0b31c14 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -41,6 +41,7 @@
#include hw/pci/msi.h
#include hw/sysbus.h
#include sysemu/sysemu.h
+#include sysemu/numa.h
#include sysemu/kvm.h
#include kvm_i386.h
#include hw/xen/xen.h
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 18cdc54..f27a087 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -22,6 +22,7 @@
#include qemu/config-file.h
#include qapi/visitor.h
#include qemu/range.h
+#include sysemu/numa.h
typedef struct pc_dimms_capacity {
uint64_t size;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b560459..e754262 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -25,6 +25,7 @@
*
*/
#include sysemu/sysemu.h
+#include sysemu/numa.h
#include hw/hw.h
#include hw/fw-path-provider.h
#include elf.h
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
new file mode 100644
index 000..514914f
--- /dev/null
+++ b/include/sysemu/numa.h
@@ -0,0 +1,28 @@
+#ifndef SYSEMU_NUMA_H
+#define SYSEMU_NUMA_H
+
+#include stdint.h
+#include qemu/bitmap.h
+#include qemu/option.h
+#include sysemu/sysemu.h
+#include sysemu/hostmem.h
+
+extern int nb_numa_nodes; /* Number of NUMA nodes */
+extern int max_numa_nodeid; /* Highest specified NUMA node ID, plus one.
+ * For all nodes, nodeid max_numa_nodeid
+ */
+
+typedef struct node_info {
+uint64_t node_mem;
+DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
+struct HostMemoryBackend *node_memdev;
+bool present;
+} NodeInfo;
+extern NodeInfo numa_info[MAX_NODES];
+void set_numa_nodes(void);
+void set_numa_modes(void);
+void query_numa_node_mem(uint64_t node_mem[]);
+extern QemuOptsList qemu_numa_opts;
+int numa_init_func(QemuOpts *opts, void *opaque);
+
+#endif
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 748d059..a726659 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -147,24 +147,6 @@ extern int mem_prealloc;
*/
#define MAX_CPUMASK_BITS 255
-extern int nb_numa_nodes; /* Number of NUMA nodes */
-extern int max_numa_nodeid; /* Highest specified NUMA node ID, plus one.
- * For all nodes, nodeid max_numa_nodeid
- */
-
-typedef struct node_info {
-uint64_t node_mem;
-DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
-struct HostMemoryBackend *node_memdev;
-bool present;
-} NodeInfo;
-extern NodeInfo numa_info[MAX_NODES];
-void set_numa_nodes(void);
-void set_numa_modes(void);
-void query_numa_node_mem(uint64_t node_mem[]);
-extern QemuOptsList qemu_numa_opts;
-int numa_init_func(QemuOpts *opts, void *opaque);
-
#define MAX_OPTION_ROMS 16
typedef struct QEMUOptionRom {
const char *name;
diff --git a/monitor.c b/monitor.c
index 5a24311..4de3a8c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -35,6 +35,7 @@
#include sysemu/char.h
#include ui/qemu-spice.h
#include sysemu/sysemu.h
+#include sysemu/numa.h
#include monitor/monitor.h
#include qemu/readline.h
#include ui/console.h
diff --git a/numa.c b/numa.c
index afd2866..40f3d36 100644
--- a/numa.c
+++ b/numa.c
@@ -22,7 +22,7 @@
* THE SOFTWARE.
*/
-#include sysemu/sysemu.h
+#include sysemu/numa.h
#include exec/cpu-common.h
#include qemu/bitmap.h
#include qom/cpu.h
diff --git a/vl.c b/vl.c
index 983259b..63ec996 100644
--- a/vl.c
+++ b/vl.c
@@ -78,6 +78,7 @@ int main(int argc, char **argv)
#include monitor/monitor.h
#include ui/console.h
#include sysemu/sysemu.h
+#include sysemu/numa.h
#include exec/gdbstub.h
#include qemu/timer.h
#include sysemu/char.h
--
2.1.0
Re: [Qemu-devel] [PATCH 0/4] gdbstub: improve query packet parsing, add qAttached support
On 02/07/2015 08:38 AM, Jan Kiszka wrote: This addresses the review comments on the previews two patches to add qAttached support. No longer trivial, so maybe you can pick it up, Peter. Excellent, thanks for doing this Jan. Looks good to me. Thanks, Pedro Alves
[Qemu-devel] [PATCH 6/7] numa: Rename option parsing functions
Renaming set_numa_nodes() and numa_init_func() to parse_numa_opts() and
parse_numa() makes the purpose of those functions clearer.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
include/sysemu/numa.h | 2 +-
numa.c| 6 +++---
vl.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index d25ada9..453b49a 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -16,7 +16,7 @@ typedef struct node_info {
bool present;
} NodeInfo;
extern NodeInfo numa_info[MAX_NODES];
-void set_numa_nodes(void);
+void parse_numa_opts(void);
void set_numa_modes(void);
void query_numa_node_mem(uint64_t node_mem[]);
extern QemuOptsList qemu_numa_opts;
diff --git a/numa.c b/numa.c
index eb9259b..d5b95e1 100644
--- a/numa.c
+++ b/numa.c
@@ -123,7 +123,7 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts
*opts, Error **errp)
max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
}
-static int numa_init_func(QemuOpts *opts, void *opaque)
+static int parse_numa(QemuOpts *opts, void *opaque)
{
NumaOptions *object = NULL;
Error *err = NULL;
@@ -166,11 +166,11 @@ error:
return -1;
}
-void set_numa_nodes(void)
+void parse_numa_opts(void)
{
int i;
-if (qemu_opts_foreach(qemu_find_opts(numa), numa_init_func,
+if (qemu_opts_foreach(qemu_find_opts(numa), parse_numa,
NULL, 1) != 0) {
exit(1);
}
diff --git a/vl.c b/vl.c
index cfce820..7243216 100644
--- a/vl.c
+++ b/vl.c
@@ -4154,7 +4154,7 @@ int main(int argc, char **argv, char **envp)
default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
-set_numa_nodes();
+parse_numa_opts();
if (qemu_opts_foreach(qemu_find_opts(mon), mon_init_func, NULL, 1) != 0)
{
exit(1);
--
2.1.0
[Qemu-devel] [PATCH 0/7] NUMA code cleanup
This cleans up some of the NUMA code: moves declarations to numa.h, rename some functions, and remove some existing code that was inside main(). Eduardo Habkost (7): numa: Move NUMA declarations from sysemu.h to numa.h vl.c: Remove unnecessary zero-initialization of NUMA globals numa: Move NUMA globals to numa.c numa: Make max_numa_nodeid static numa: Move QemuOpts parsing to set_numa_nodes() numa: Rename option parsing functions numa: Rename set_numa_modes() to numa_post_machine_init() hw/i386/pc.c| 1 + hw/mem/pc-dimm.c| 1 + hw/ppc/spapr.c | 1 + include/sysemu/numa.h | 24 include/sysemu/sysemu.h | 18 -- monitor.c | 1 + numa.c | 20 vl.c| 22 +++--- 8 files changed, 47 insertions(+), 41 deletions(-) create mode 100644 include/sysemu/numa.h -- 2.1.0
[Qemu-devel] [PATCH 4/7] numa: Make max_numa_nodeid static
Now the only code that uses the variable is inside numa.c.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
include/sysemu/numa.h | 3 ---
numa.c| 4 +++-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index 514914f..eae881e 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -8,9 +8,6 @@
#include sysemu/hostmem.h
extern int nb_numa_nodes; /* Number of NUMA nodes */
-extern int max_numa_nodeid; /* Highest specified NUMA node ID, plus one.
- * For all nodes, nodeid max_numa_nodeid
- */
typedef struct node_info {
uint64_t node_mem;
diff --git a/numa.c b/numa.c
index 61531db..84d4cb5 100644
--- a/numa.c
+++ b/numa.c
@@ -45,8 +45,10 @@ QemuOptsList qemu_numa_opts = {
};
static int have_memdevs = -1;
+static int max_numa_nodeid; /* Highest specified NUMA node ID, plus one.
+ * For all nodes, nodeid max_numa_nodeid
+ */
int nb_numa_nodes;
-int max_numa_nodeid;
NodeInfo numa_info[MAX_NODES];
static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error
**errp)
--
2.1.0
Re: [Qemu-devel] [PATCH v3 0/3] vfio: free data and unmap BARs in instance_finalize
On 08/02/2015 18:22, Alex Williamson wrote: Ok, I went back to 83761b9244ad, applied 3a4dbe6aa934 to get the object_unparent() fix, then applied this series. Everything seems to work ok. Then I manually applied and bisected the commits that came in via d5fbb4c9ed52. I land on 374f2981d1f1 as introducing the segfault in memory_listener_register(). I guess I was mis-remembering where I did my testing for the last vfio pull request. My tag was based on ec6f25e, but I remember that I had to test based on a commit before the RCU merge. Ouch. memory_listeners needs mutex protection. Paolo
Re: [Qemu-devel] [PATCH v3 0/3] vfio: free data and unmap BARs in instance_finalize
On Sat, 2015-02-07 at 21:00 +0100, Paolo Bonzini wrote: On 07/02/2015 02:39, Alex Williamson wrote: I'm not sure where it's coming from yet, but I did extensive testing for my last pull request based on ec6f25e because if I updated to d5fbb4c vfio hotplug broke immediately. I'll keep looking, but I thought I'd share in case you have some ideas. Thanks, I'm not sure I understand: d5fbb4c9ed52d97aebe5994d8a857c74c0d95a92 (RCU merge) is an ancestor of ec6f25e788ef57ce1e9f734984ef8885172fd9e2 (s390 merge) and the only patches in the middle are for s390. Ok, I went back to 83761b9244ad, applied 3a4dbe6aa934 to get the object_unparent() fix, then applied this series. Everything seems to work ok. Then I manually applied and bisected the commits that came in via d5fbb4c9ed52. I land on 374f2981d1f1 as introducing the segfault in memory_listener_register(). I guess I was mis-remembering where I did my testing for the last vfio pull request. My tag was based on ec6f25e, but I remember that I had to test based on a commit before the RCU merge. My test is to simply do virsh detach-device, attach-device in a loop for a vfio assigned VF NIC, 1s delay between ops. It typically fails within 100 to 150 iterations, I call 500 a pass. Thanks, Alex
[Qemu-devel] [PATCH] linux-user: Remove type casts to union type
Casting to a union type is a gcc (and clang) extension. Other compilers
might not support it. This is not a problem today, but the type casts
can be removed easily. Smatch now no longer complains like before:
linux-user/syscall.c:3190:18: warning: cast to non-scalar
linux-user/syscall.c:7348:44: warning: cast to non-scalar
Cc: Riku Voipio riku.voi...@iki.fi
Signed-off-by: Stefan Weil s...@weilnetz.de
---
linux-user/syscall.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 852308e..ec137db 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2663,8 +2663,9 @@ static inline abi_long host_to_target_semarray(int semid,
abi_ulong target_addr,
}
static inline abi_long do_semctl(int semid, int semnum, int cmd,
- union target_semun target_su)
+ abi_ulong target_arg)
{
+union target_semun target_su;
union semun arg;
struct semid_ds dsarg;
unsigned short *array = NULL;
@@ -2673,6 +2674,8 @@ static inline abi_long do_semctl(int semid, int semnum,
int cmd,
abi_long err;
cmd = 0xff;
+target_su.buf = target_arg;
+
switch( cmd ) {
case GETVAL:
case SETVAL:
@@ -3186,8 +3189,7 @@ static abi_long do_ipc(unsigned int call, abi_long first,
* ptr argument. */
abi_ulong atptr;
get_user_ual(atptr, ptr);
-ret = do_semctl(first, second, third,
-(union target_semun) atptr);
+ret = do_semctl(first, second, third, atptr);
break;
}
@@ -7345,7 +7347,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
#endif
#ifdef TARGET_NR_semctl
case TARGET_NR_semctl:
-ret = do_semctl(arg1, arg2, arg3, (union target_semun)(abi_ulong)arg4);
+ret = do_semctl(arg1, arg2, arg3, arg4);
break;
#endif
#ifdef TARGET_NR_msgctl
--
1.7.10.4
[Qemu-devel] [PATCH] hmp: Fix warning from smatch (wrong argument in function call)
Fix this warning:
hmp.c:414:38: warning: Using plain integer as NULL pointer
qmp_query_block expects a pointer argument, so passing false is wrong.
Cc: Luiz Capitulino lcapitul...@redhat.com
Signed-off-by: Stefan Weil s...@weilnetz.de
---
hmp.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index a42c5c0..6926960 100644
--- a/hmp.c
+++ b/hmp.c
@@ -411,7 +411,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
/* Print BlockBackend information */
if (!nodes) {
-block_list = qmp_query_block(false);
+block_list = qmp_query_block(NULL);
} else {
block_list = NULL;
}
--
1.7.10.4
Re: [Qemu-devel] [PATCH] memsave: Improve and disambiguate error message
On 08/02/2015 13:14, Borislav Petkov wrote: On Sun, Feb 08, 2015 at 11:09:24AM +0100, Paolo Bonzini wrote: Cc: qemu-triv...@nongnu.org Thanks. But, there's more b0rked with this error message so you might wanna take the patch below instead. Btw, what are the vim settings when doing patches for qemu, this must be documented somewhere as coding style differs significantly from that of the kernel? I use :set shiftwidth=4 expandtab and c-indent mode. Quick googling for vim four spaces suggests the following alternatives: * :set tabstop=8 expandtab shiftwidth=4 softtabstop=4 * set tabstop=8 softtabstop=0 expandtab shiftwidth=4 smarttab. Paolo
Re: [Qemu-devel] [question] the patch which affect performance of virtio-scsi
On 2015-2-8 18:10, Paolo Bonzini wrote: On 07/02/2015 12:26, Wangting (Kathy) wrote: OK, Thank you very much for your detailed explanation. But I have another question about the big change from qemu-1.5.3 to qemu-1.6.0-rc0. When I use ramdisk for IO performance testing, the result is as follows. [fio-test] rw bs iodepthjobs bw iops qemu-1.5.3 read 4k 32 1 285MB/s 73208 qemu-1.6.0-rc0 read 4k 32 1 253MB/s 64967 And virtio-blk is the same. I know there are so many differences between qemu-1.5 and qemu-1.6, but I am confused about what new features impact the performance so much. Do you know it? No, sorry. Please try newer versions first and see if this was fixed. QEMU 1.6 is more than a year old. Paolo OK, thanks. Ting
[Qemu-devel] [PATCH v2] qemu-coroutine: segfault when restarting co_queue
From: Bin Wu wu.wu...@huawei.com
We tested VMs migration with their disk images by drive_mirror. With
migration, two VMs copyed large files between each other. During the
test, a segfault occured. The stack was as follow:
(gdb) bt
qemu-coroutine-lock.c:66
to=0x7fa5a1798648) at qemu-coroutine.c:97
request=0x7fa28c2ffa10, reply=0x7fa28c2ffa30, qiov=0x0, offset=0) at
block/nbd-client.c:165
sector_num=8552704, nb_sectors=2040, qiov=0x7fa5a1757468, offset=0) at
block/nbd-client.c:262
sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468) at
block/nbd-client.c:296
nb_sectors=2048, qiov=0x7fa5a1757468) at block/nbd.c:291
req=0x7fa28c2ffbb0, offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468,
flags=0) at block.c:3321
offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, flags=(unknown: 0)) at
block.c:3447
sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468, flags=(unknown: 0)) at
block.c:3471
nb_sectors=2048, qiov=0x7fa5a1757468) at block.c:3480
nb_sectors=2048, qiov=0x7fa5a1757468) at block/raw_bsd.c:62
req=0x7fa28c2ffe30, offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468,
flags=0) at block.c:3321
offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, flags=(unknown: 0)) at
block.c:3447
sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468, flags=(unknown: 0)) at
block.c:3471
coroutine-ucontext.c:121
After analyzing the stack and reviewing the code, we find the
qemu_co_queue_run_restart should not be put in the coroutine_swap function which
can be invoked by qemu_coroutine_enter or qemu_coroutine_yield. Only
qemu_coroutine_enter needs to restart the co_queue.
The error scenario is as follow: coroutine C1 enters C2, C2 yields
back to C1, then C1 ternimates and the related coroutine memory
becomes invalid. After a while, the C2 coroutine is entered again.
At this point, C1 is used as a parameter passed to
qemu_co_queue_run_restart. Therefore, qemu_co_queue_run_restart
accesses an invalid memory and a segfault error ocurrs.
The qemu_co_queue_run_restart function re-enters coroutines waiting
in the co_queue. However, this function should be only used int the
qemu_coroutine_enter context. Only in this context, when the current
coroutine gets execution control again(after the execution of
qemu_coroutine_switch), we can restart the target coutine because the
target coutine has yielded back to the current coroutine or it has
terminated.
First we want to put qemu_co_queue_run_restart in qemu_coroutine_enter,
but we find we can not access the target coroutine if it terminates.
Signed-off-by: Bin Wu wu.wu...@huawei.com
---
qemu-coroutine.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 525247b..cc0bdfa 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -99,29 +99,31 @@ static void coroutine_delete(Coroutine *co)
qemu_coroutine_delete(co);
}
-static void coroutine_swap(Coroutine *from, Coroutine *to)
+static CoroutineAction coroutine_swap(Coroutine *from, Coroutine *to)
{
CoroutineAction ret;
ret = qemu_coroutine_switch(from, to, COROUTINE_YIELD);
-qemu_co_queue_run_restart(to);
-
switch (ret) {
case COROUTINE_YIELD:
-return;
+break;
case COROUTINE_TERMINATE:
trace_qemu_coroutine_terminate(to);
+qemu_co_queue_run_restart(to);
coroutine_delete(to);
-return;
+break;
default:
abort();
}
+
+return ret;
}
void qemu_coroutine_enter(Coroutine *co, void *opaque)
{
Coroutine *self = qemu_coroutine_self();
+CoroutineAction ret;
trace_qemu_coroutine_enter(self, co, opaque);
@@ -132,7 +134,9 @@ void qemu_coroutine_enter(Coroutine *co, void *opaque)
co-caller = self;
co-entry_arg = opaque;
-coroutine_swap(self, co);
+ret = coroutine_swap(self, co);
+if (ret == COROUTINE_YIELD)
+qemu_co_queue_run_restart(co);
}
void coroutine_fn qemu_coroutine_yield(void)
--
1.7.12.4
Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
On 2015/2/6 20:14, Cornelia Huck wrote:
On Fri, 6 Feb 2015 13:41:26 +0800
Tiejun Chen tiejun.c...@intel.com wrote:
Actually we define these device IDs in virtio standard, so
we'd better put them into one common place to manage conveniently.
Here I also add VIRTIO_ID_RESERVE according to virtio spec.
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
---
hw/9pfs/virtio-9p.h| 2 --
include/hw/virtio/virtio-balloon.h | 3 ---
include/hw/virtio/virtio-blk.h | 3 ---
include/hw/virtio/virtio-rng.h | 3 ---
include/hw/virtio/virtio-scsi.h| 3 ---
include/hw/virtio/virtio-serial.h | 3 ---
include/hw/virtio/virtio.h | 16
pc-bios/s390-ccw/virtio.h | 8 +---
8 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index f24997d..9ad6bb2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -23,6 +23,22 @@
#include hw/virtio/virtio-9p.h
#endif
+/* Refer to Linux's linux/virtio_ids.h */
Why not refer to the virtio spec instead? :) And maybe add in the ids
Actually they're same now but this really is a potential risk.
that already have been reserved.
So what about this?
@@ -23,6 +23,22 @@
#include hw/virtio/virtio-9p.h
#endif
+/* Refer to VirtIO Spec 1.0. */
+
+#define VIRTIO_ID_RESERVED 0 /* reserved (invalid)*/
+#define VIRTIO_ID_NET 1 /* network card */
+#define VIRTIO_ID_BLOCK 2 /* block device */
+#define VIRTIO_ID_CONSOLE 3 /* console */
+#define VIRTIO_ID_RNG 4 /* entropy source */
+#define VIRTIO_ID_BALLOON 5 /* memory ballooning */
+#define VIRTIO_ID_IOMEMORY 6 /* ioMemory */
+#define VIRTIO_ID_RPMSG 7 /* rpmsg */
+#define VIRTIO_ID_SCSI 8 /* SCSI host */
+#define VIRTIO_ID_9P9 /* 9P transport */
+#define VIRTIO_ID_MAC80211_WALN 10 /* mac80211 wlan */
+#define VIRTIO_ID_RPROC_SERIAL 11 /* rproc seria */
+#define VIRTIO_ID_CAIF 12 /* virtio CAIF */
+
/* from Linux's linux/virtio_config.h */
/* Status byte for guest to report progress, and synchronize features. */
+
+enum virtio_dev_type {
+VIRTIO_ID_RESERVED = 0, /* invalid virtio device */
+VIRTIO_ID_NET = 1, /* virtio net */
+VIRTIO_ID_BLOCK= 2, /* virtio block */
+VIRTIO_ID_CONSOLE = 3, /* virtio console */
+VIRTIO_ID_RNG = 4, /* virtio rng */
+VIRTIO_ID_BALLOON = 5, /* virtio balloon */
/* virtio balloon (legacy) */
+VIRTIO_ID_RPMSG= 7, /* virtio remote processor messaging */
+VIRTIO_ID_SCSI = 8, /* virtio scsi */
+VIRTIO_ID_9P = 9, /* 9p virtio console */
+VIRTIO_ID_RPROC_SERIAL = 11, /* virtio remoteproc serial link */
+VIRTIO_ID_CAIF = 12, /* Virtio caif */
+};
+
/* from Linux's linux/virtio_config.h */
/* Status byte for guest to report progress, and synchronize features. */
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index c23466b..2eabcb4 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -11,6 +11,7 @@
#ifndef VIRTIO_H
#define VIRTIO_H
+#include hw/virtio/virtio.h
This won't work, the bios can't use the common headers.
Thanks for your caught.
#include s390-ccw.h
/* Status byte for guest to report progress, and synchronize features. */
@@ -23,13 +24,6 @@
/* We've given up on this device. */
#define VIRTIO_CONFIG_S_FAILED 0x80
-enum virtio_dev_type {
-VIRTIO_ID_NET = 1,
-VIRTIO_ID_BLOCK = 2,
-VIRTIO_ID_CONSOLE = 3,
-VIRTIO_ID_BALLOON = 5,
-};
Even though this one is incomplete; but we don't need anything but the
block id anyway.
-
struct virtio_dev_header {
enum virtio_dev_type type : 8;
u8 num_vq;
I will remove all s390 stuff in this patch.
Thanks
Tiejun
Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
On 2015/2/7 0:28, Stefan Hajnoczi wrote: On Fri, Feb 06, 2015 at 01:41:26PM +0800, Tiejun Chen wrote: Actually we define these device IDs in virtio standard, so we'd better put them into one common place to manage conveniently. Here I also add VIRTIO_ID_RESERVE according to virtio spec. Signed-off-by: Tiejun Chen tiejun.c...@intel.com --- hw/9pfs/virtio-9p.h| 2 -- include/hw/virtio/virtio-balloon.h | 3 --- include/hw/virtio/virtio-blk.h | 3 --- include/hw/virtio/virtio-rng.h | 3 --- include/hw/virtio/virtio-scsi.h| 3 --- include/hw/virtio/virtio-serial.h | 3 --- include/hw/virtio/virtio.h | 16 pc-bios/s390-ccw/virtio.h | 8 +--- 8 files changed, 17 insertions(+), 24 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com Thanks for you review. Just a little change will be introduced in next revision. Tiejun
Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
On 2015/2/8 18:48, Michael S. Tsirkin wrote: On Fri, Feb 06, 2015 at 01:14:46PM +0100, Cornelia Huck wrote: On Fri, 6 Feb 2015 13:41:26 +0800 Tiejun Chen tiejun.c...@intel.com wrote: Actually we define these device IDs in virtio standard, so we'd better put them into one common place to manage conveniently. Here I also add VIRTIO_ID_RESERVE according to virtio spec. Signed-off-by: Tiejun Chen tiejun.c...@intel.com We really should just write a script to import the headers from the linux kernel. They will need some tweaks to avoid dependencies on linux/types, but this seems easy to do - better than trying to keep things in sync manually. I prefer Cornelia's comment since actually we're trying to define a little bit according to a spec, so the following may be enough? diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index f24997d..4afb0b7 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -23,6 +23,22 @@ #include hw/virtio/virtio-9p.h #endif +/* Refer to VirtIO Spec 1.0. */ + +#define VIRTIO_ID_RESERVED 0 /* reserved (invalid)*/ +#define VIRTIO_ID_NET 1 /* network card */ +#define VIRTIO_ID_BLOCK 2 /* block device */ +#define VIRTIO_ID_CONSOLE 3 /* console */ +#define VIRTIO_ID_RNG 4 /* entropy source */ +#define VIRTIO_ID_BALLOON 5 /* memory ballooning */ +#define VIRTIO_ID_IOMEMORY 6 /* ioMemory */ +#define VIRTIO_ID_RPMSG 7 /* rpmsg */ +#define VIRTIO_ID_SCSI 8 /* SCSI host */ +#define VIRTIO_ID_9P9 /* 9P transport */ +#define VIRTIO_ID_MAC80211_WALN 10 /* mac80211 wlan */ +#define VIRTIO_ID_RPROC_SERIAL 11 /* rproc seria */ +#define VIRTIO_ID_CAIF 12 /* virtio CAIF */ + /* from Linux's linux/virtio_config.h */ /* Status byte for guest to report progress, and synchronize features. */ Thanks Tiejun
Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
On Mon, Feb 09, 2015 at 03:01:15PM +0800, Chen, Tiejun wrote: On 2015/2/8 18:48, Michael S. Tsirkin wrote: On Fri, Feb 06, 2015 at 01:14:46PM +0100, Cornelia Huck wrote: On Fri, 6 Feb 2015 13:41:26 +0800 Tiejun Chen tiejun.c...@intel.com wrote: Actually we define these device IDs in virtio standard, so we'd better put them into one common place to manage conveniently. Here I also add VIRTIO_ID_RESERVE according to virtio spec. Signed-off-by: Tiejun Chen tiejun.c...@intel.com We really should just write a script to import the headers from the linux kernel. They will need some tweaks to avoid dependencies on linux/types, but this seems easy to do - better than trying to keep things in sync manually. I prefer Cornelia's comment since actually we're trying to define a little bit according to a spec, so the following may be enough? diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index f24997d..4afb0b7 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -23,6 +23,22 @@ #include hw/virtio/virtio-9p.h #endif +/* Refer to VirtIO Spec 1.0. */ + +#define VIRTIO_ID_RESERVED 0 /* reserved (invalid)*/ +#define VIRTIO_ID_NET 1 /* network card */ +#define VIRTIO_ID_BLOCK 2 /* block device */ +#define VIRTIO_ID_CONSOLE 3 /* console */ +#define VIRTIO_ID_RNG 4 /* entropy source */ +#define VIRTIO_ID_BALLOON 5 /* memory ballooning */ +#define VIRTIO_ID_IOMEMORY 6 /* ioMemory */ +#define VIRTIO_ID_RPMSG 7 /* rpmsg */ +#define VIRTIO_ID_SCSI 8 /* SCSI host */ +#define VIRTIO_ID_9P9 /* 9P transport */ +#define VIRTIO_ID_MAC80211_WALN 10 /* mac80211 wlan */ +#define VIRTIO_ID_RPROC_SERIAL 11 /* rproc seria */ +#define VIRTIO_ID_CAIF 12 /* virtio CAIF */ + /* from Linux's linux/virtio_config.h */ /* Status byte for guest to report progress, and synchronize features. */ Thanks Tiejun This still means each change has to be done in two places. An automated script for copying headers would be much better IMHO. -- MST
Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
On 2015/2/9 15:02, Michael S. Tsirkin wrote: On Mon, Feb 09, 2015 at 03:01:15PM +0800, Chen, Tiejun wrote: On 2015/2/8 18:48, Michael S. Tsirkin wrote: On Fri, Feb 06, 2015 at 01:14:46PM +0100, Cornelia Huck wrote: On Fri, 6 Feb 2015 13:41:26 +0800 Tiejun Chen tiejun.c...@intel.com wrote: Actually we define these device IDs in virtio standard, so we'd better put them into one common place to manage conveniently. Here I also add VIRTIO_ID_RESERVE according to virtio spec. Signed-off-by: Tiejun Chen tiejun.c...@intel.com We really should just write a script to import the headers from the linux kernel. They will need some tweaks to avoid dependencies on linux/types, but this seems easy to do - better than trying to keep things in sync manually. I prefer Cornelia's comment since actually we're trying to define a little bit according to a spec, so the following may be enough? diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index f24997d..4afb0b7 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -23,6 +23,22 @@ #include hw/virtio/virtio-9p.h #endif +/* Refer to VirtIO Spec 1.0. */ + +#define VIRTIO_ID_RESERVED 0 /* reserved (invalid)*/ +#define VIRTIO_ID_NET 1 /* network card */ +#define VIRTIO_ID_BLOCK 2 /* block device */ +#define VIRTIO_ID_CONSOLE 3 /* console */ +#define VIRTIO_ID_RNG 4 /* entropy source */ +#define VIRTIO_ID_BALLOON 5 /* memory ballooning */ +#define VIRTIO_ID_IOMEMORY 6 /* ioMemory */ +#define VIRTIO_ID_RPMSG 7 /* rpmsg */ +#define VIRTIO_ID_SCSI 8 /* SCSI host */ +#define VIRTIO_ID_9P9 /* 9P transport */ +#define VIRTIO_ID_MAC80211_WALN 10 /* mac80211 wlan */ +#define VIRTIO_ID_RPROC_SERIAL 11 /* rproc seria */ +#define VIRTIO_ID_CAIF 12 /* virtio CAIF */ + /* from Linux's linux/virtio_config.h */ /* Status byte for guest to report progress, and synchronize features. */ Thanks Tiejun This still means each change has to be done in two places. Are you saying another head file, pc-bios/s390-ccw/virtio.h? But seems Cornelia thought in case of s390-ccw, -quote- Even though this one is incomplete; but we don't need anything but the block id anyway. So I'm not sure we really should do this :) Thanks Tiejun
Re: [Qemu-devel] [PATCH 2/5] virtio: increase VIRITO_QUEUE_MAX to 513
On Sun, Feb 8, 2015 at 6:51 PM, Michael S. Tsirkin m...@redhat.com wrote: On Fri, Feb 06, 2015 at 03:54:10PM +0800, Jason Wang wrote: Recent linux kernel supports up to 256 tap queues. Increase the limit to 513 (256 * 2 + 1(ctrl vq)). Signed-off-by: Jason Wang jasow...@redhat.com We have a bunch of loops over all possible VQ numbers in virtio pci. Doing this for 1000 VQs when most of them are inactive seems extreme - I suspect we need a list of active VQs now. We can do it on top of this series. But not sure it was really needed since none of those loops happen on data path. So we were probably ok. --- include/hw/virtio/virtio.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 220c09d..21bb30f 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -92,7 +92,7 @@ typedef struct VirtQueueElement struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; } VirtQueueElement; -#define VIRTIO_QUEUE_MAX 64 +#define VIRTIO_QUEUE_MAX 513 #define VIRTIO_NO_VECTOR 0x -- 1.9.1
Re: [Qemu-devel] [PATCH] qemu-coroutine: fix qemu_co_queue_run_restart error
sorry, there is a mistake in this patch: the ret variable is not
defined :
I will send a new patch to fix this problem.
On 2015/2/9 12:09, Bin Wu wrote:
From: Bin Wu wu.wu...@huawei.com
The error scenario is as follow: coroutine C1 enters C2, C2 yields
back to C1, then C1 ternimates and the related coroutine memory
becomes invalid. After a while, the C2 coroutine is entered again.
At this point, C1 is used as a parameter passed to
qemu_co_queue_run_restart. Therefore, qemu_co_queue_run_restart
accesses an invalid memory and a segfault error ocurrs.
The qemu_co_queue_run_restart function re-enters coroutines waiting
in the co_queue. However, this function should be only used int the
qemu_coroutine_enter context. Only in this context, when the current
coroutine gets execution control again(after the execution of
qemu_coroutine_switch), we can restart the target coutine because the
target coutine has yielded back to the current coroutine or it has
terminated.
First we want to put qemu_co_queue_run_restart in qemu_coroutine_enter,
but we find we can not access the target coroutine if it terminates.
Signed-off-by: Bin Wu wu.wu...@huawei.com
---
qemu-coroutine.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 525247b..9a294c4 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -99,24 +99,25 @@ static void coroutine_delete(Coroutine *co)
qemu_coroutine_delete(co);
}
-static void coroutine_swap(Coroutine *from, Coroutine *to)
+static CoroutineAction coroutine_swap(Coroutine *from, Coroutine *to)
{
CoroutineAction ret;
ret = qemu_coroutine_switch(from, to, COROUTINE_YIELD);
-qemu_co_queue_run_restart(to);
-
switch (ret) {
case COROUTINE_YIELD:
-return;
+break;
case COROUTINE_TERMINATE:
trace_qemu_coroutine_terminate(to);
+qemu_co_queue_run_restart(to);
coroutine_delete(to);
-return;
+break;
default:
abort();
}
+
+return ret;
}
void qemu_coroutine_enter(Coroutine *co, void *opaque)
@@ -133,6 +134,8 @@ void qemu_coroutine_enter(Coroutine *co, void *opaque)
co-caller = self;
co-entry_arg = opaque;
coroutine_swap(self, co);
+if (ret == COROUTINE_YIELD)
+qemu_co_queue_run_restart(co);
}
void coroutine_fn qemu_coroutine_yield(void)
--
Bin Wu
Re: [Qemu-devel] [PATCH 1/5] virtio: rename VIRTIO_PCI_QUEUE_MAX to VIRTIO_QUEUE_MAX
On Fri, Feb 6, 2015 at 7:54 PM, Cornelia Huck cornelia.h...@de.ibm.com wrote: On Fri, 6 Feb 2015 15:54:09 +0800 Jason Wang jasow...@redhat.com wrote: VIRTIO_PCI_QUEUE_MAX was not specific to pci, so rename it to VIRTIO_QUEUE_MAX. Cc: Amit Shah amit.s...@redhat.com Cc: Anthony Liguori aligu...@amazon.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Alexander Graf ag...@suse.de Cc: Richard Henderson r...@twiddle.net Cc: Cornelia Huck cornelia.h...@de.ibm.com Cc: Christian Borntraeger borntrae...@de.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Jason Wang jasow...@redhat.com --- hw/char/virtio-serial-bus.c | 2 +- hw/s390x/s390-virtio-bus.c | 4 ++-- hw/s390x/s390-virtio-ccw.c | 2 +- hw/s390x/virtio-ccw.c| 10 +- hw/scsi/virtio-scsi.c| 4 ++-- hw/virtio/virtio-mmio.c | 4 ++-- hw/virtio/virtio-pci.c | 10 +- hw/virtio/virtio.c | 26 +- include/hw/s390x/s390_flic.h | 2 +- include/hw/virtio/virtio.h | 2 +- 10 files changed, 33 insertions(+), 33 deletions(-) While I certainly agree with the patch on its own, I read through your complete patch series as well and I have some additional comments. Your patch series bumps up the maximum number of virtqueues for all transports, but I think we need to consider carefully what this means for each transport. For virtio-ccw, for example, we'll run into trouble when the guest uses classic queue indicators: These are limited to 64 per device (as I assumed 64 virtqueues per device initially and they fit quite nicely into a 64 bit integer). If the guest uses adapter interrupts with two-stage indicators, we should probably be fine. The guest will hopefully care about that, but we need to describe failure modes for this in the spec. For s390-virtio, I frankly have no idea what large numbers of queues will do :) Maybe we just run out of space for devices much earlier. I see the issue, thanks. So looks like we need a transport specific limitation for this. You seem to need some changes for this to work for virtio-pci as well, don't you? PCI allows at most 2048 MSI-X entires, and also allow one entry to be shared by several virtqueues which looks fine for even thousands of virtqueues. If there's a architecture limitation (e.g x86 only allow 240 interrupt vectors for MSI-X), the rest work was in the driver. If not, probably no much work needs to be done for PCI. Thanks
Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough
On 2015/2/6 9:01, Chen, Tiejun wrote:
On 2015/2/5 17:52, Ian Campbell wrote:
On Thu, 2015-02-05 at 09:22 +0800, Chen, Tiejun wrote:
Indeed this is not something workaround, and I think in any type of VGA
devices, we'd like to diminish this sort of thing gradually, right?
This mightn't come true in real world :)
It's not really something we can control, the h/w guys will do what they
do, including integrating on-board graphics tightly with the N/S-bridges
etc.
Yeah.
I think there are three ways to achieve that:
* Make the libxl/xl option something which is not generic e.g.
igd_passthru=1
* Add a second option to allow the user to configure the
kind of
graphics device being passed thru (e.g. gfx_passthru=1,
passthru_device=igd), or combine the two by making the
gfx_passthru option a string instead of a boolean.
It makes more sense but this mean we're going to change that existing
rule in qemu-traditional. But here I guess we shouldn't consider that
case.
qemu-trad is frozen so we wouldn't be adding new features such as
support for new graphics passthru devices, so we can ignore it's
deficiencies in this area and just improve things in the qemu-xen case.
(we may want to add a compat handling for any new option we add to libxl
to translate to -gfx_passthru, but that's about it)
Understood.
* Make libxl detect the type of graphics device somehow and
therefore automatically determine when gfx_passthru=1 =
igd-passthru
This way confounds me all. Can libxl detect the graphics device *before*
we intend to pass a parameter to active qemu?
We know the BDF of all devices which we are going to pass to the guest
and we can observe various properties of that device
via /sys/bus/pci/devices/:B:D:F.
So sounds what you're saying is Xen already have this sort of example in
libxl.
Currently, we have to set
something as follows,
gfx_passthru=1
pci=[00:02.0]
This always works for qemu-xen-traditional.
But you should know '00:02.0' doesn't mean we are passing IGD through.
But by looking at the device 00:02.0 (e.g. its PCI vendor and device ID
and other properties) we can unambiguously determine if it is an IGD
device or not, can't we?
Again, like what I said above, I'm not sure if its possible in my case.
If I'm wrong please correct me.
Is my reply above sufficient?
Yes, I can understand what you mean but I need to take close look at
exactly what should be done in libxl :)
In high level, this way may come out as follows,
#1 libxl parse 'pci=[]' to get SBDF
#2 Scan SBDF by accessing sysfs to get vendor/device IDs.
#3 If this pair of IDs is identified to our target device, IGD,
igd-passthru would be delivered to qemu.
Right?
What about this?
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 8599a6a..507034f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -710,9 +710,6 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_append(dm_args, -net);
flexarray_append(dm_args, none);
}
-if (libxl_defbool_val(b_info-u.hvm.gfx_passthru)) {
-flexarray_append(dm_args, -gfx_passthru);
-}
} else {
if (!sdl !vnc) {
flexarray_append(dm_args, -nographic);
@@ -757,6 +754,11 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
machinearg, max_ram_below_4g);
}
}
+
+if (libxl__is_igd_vga_passthru(gc, guest_config)) {
+machinearg = GCSPRINTF(%s,igd-passthru=on, machinearg);
+}
+
flexarray_append(dm_args, machinearg);
for (i = 0; b_info-extra_hvm b_info-extra_hvm[i] != NULL;
i++)
flexarray_append(dm_args, b_info-extra_hvm[i]);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 934465a..35ec5fc 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1177,6 +1177,9 @@ _hidden int libxl__create_pci_backend(libxl__gc
*gc, uint32_t domid,
libxl_device_pci *pcidev, int num);
_hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc,
+ const libxl_domain_config
*d_config);
+
/*- xswait: wait for a xenstore node to be suitable -*/
typedef struct libxl__xswait_state libxl__xswait_state;
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index f3ae132..9cccbfe 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -491,6 +491,108 @@ static int sysfs_dev_unbind(libxl__gc *gc,
libxl_device_pci *pcidev,
return 0;
}
+/* Currently we only support HSW and BDW in qemu upstream.*/
+static const uint16_t igd_ids[] = {
+/* HSW Classic */
+0x0402, /* HSWGT1D, HSWD_w7 */
+
Re: [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects
On 2015/1/31 15:27, Gonglei (Arei) wrote: From: Gonglei arei.gong...@huawei.com Gonglei (2): xen-pt: fix Negative array index read xen-pt: fix Out-of-bounds read hw/xen/xen_pt_config_init.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) Ping... Regards, -Gonglei
Re: [Qemu-devel] [PATCH v4] sheepdog: selectable object size support
On Fri, Feb 06, 2015 at 04:57:55PM +0900, Teruaki Ishizaki wrote: (2015/02/06 11:18), Liu Yuan wrote: On Wed, Feb 04, 2015 at 01:54:19PM +0900, Teruaki Ishizaki wrote: (2015/02/02 15:52), Liu Yuan wrote: On Tue, Jan 27, 2015 at 05:35:27PM +0900, Teruaki Ishizaki wrote: Previously, qemu block driver of sheepdog used hard-coded VDI object size. This patch enables users to handle block_size_shift value for calculating VDI object size. When you start qemu, you don't need to specify additional command option. But when you create the VDI which doesn't have default object size with qemu-img command, you specify block_size_shift option. If you want to create a VDI of 8MB(1 23) object size, you need to specify following command option. # qemu-img create -o block_size_shift=23 sheepdog:test1 100M Is it possible to make this option more user friendly? such as $ qemu-img create -o object_size=8M sheepdog:test 1G At first, I thought that the object_size was user friendly. But, Sheepdog has already the value of block_size_shift in the inode layout that means like object_size. 'object_size' doesn't always fit right in 'block_size_shift'. On the other hands, 'block_size_shift' always fit right in 'object_size'. I think that existing layout shouldn't be changed easily and it seems that it is difficult for users to specify the object_size value that fit right in 'block_size_shift'. I don't think we need to change the layout. block_size_shift is what QEMU talks to sheep, and QEMU options is what users talks to QEMU. We can convert the user friendly object size into block_size_shift internally in the driver before sending it tosheep daemon, no? For example, users specify 12MB for object size, block_size_shift doesn't fit exactly to an integer. In this case, we should abort and print the error message and notify the users the acceptable range like erasure coding option. I suppose that normally an administrator do format sheepdog cluster with specifying block_size_shift and users usually do qemu-img command without a block_size_shift option. block_size_shift is too developer centered and has a direct relation to the underlying algorithm. If in the future, e.g, we change the underlying algorithm about how we represent block size, block_size_shift might not even exsit. So use object_size would be more generic and won't have this kind of problem. Secondly, it is not hard to parse the object_size into block_size_shift, so I'd suggest we'd better try our best to make it user friendly. Thanks Yuan
[Qemu-devel] [PATCH] qemu-coroutine: segfault when restarting co_queue
From: Bin Wu wu.wu...@huawei.com We tested VMs migration with their disk images by drive_mirror. With migration, two VMs copyed large files between each other. During the test, a segfault occured. The stack was as follow: (gdb) bt #0 0x7fa5a0c63fc5 in qemu_co_queue_run_restart (co=0x7fa5a1798648) at qemu-coroutine-lock.c:66 #1 0x7fa5a0c63bed in coroutine_swap (from=0x7fa5a178f160, to=0x7fa5a1798648) at qemu-coroutine.c:97 #2 0x7fa5a0c63dbf in qemu_coroutine_yield () at qemu-coroutine.c:140 #3 0x7fa5a0c9e474 in nbd_co_receive_reply (s=0x7fa5a1a3cfd0, request=0x7fa28c2ffa10, reply=0x7fa28c2ffa30, qiov=0x0, offset=0) at block/nbd-client.c:165 #4 0x7fa5a0c9e8b5 in nbd_co_writev_1 (client=0x7fa5a1a3cfd0, sector_num=8552704, nb_sectors=2040, qiov=0x7fa5a1757468, offset=0) at block/nbd-client.c:262 #5 0x7fa5a0c9e9dd in nbd_client_session_co_writev (client=0x7fa5a1a3cfd0, sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468) at block/nbd-client.c:296 #6 0x7fa5a0c9dda1 in nbd_co_writev (bs=0x7fa5a198fcb0, sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468) at block/nbd.c:291 #7 0x7fa5a0c509a4 in bdrv_aligned_pwritev (bs=0x7fa5a198fcb0, req=0x7fa28c2ffbb0, offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, flags=0) at block.c:3321 #8 0x7fa5a0c50f3f in bdrv_co_do_pwritev (bs=0x7fa5a198fcb0, offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, flags=(unknown: 0)) at block.c:3447 #9 0x7fa5a0c51007 in bdrv_co_do_writev (bs=0x7fa5a198fcb0, sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468, flags=(unknown: 0)) at block.c:3471 #10 0x7fa5a0c51074 in bdrv_co_writev (bs=0x7fa5a198fcb0, sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468) at block.c:3480 #11 0x7fa5a0c652ec in raw_co_writev (bs=0x7fa5a198c110, sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468) at block/raw_bsd.c:62 #12 0x7fa5a0c509a4 in bdrv_aligned_pwritev (bs=0x7fa5a198c110, req=0x7fa28c2ffe30, offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, flags=0) at block.c:3321 #13 0x7fa5a0c50f3f in bdrv_co_do_pwritev (bs=0x7fa5a198c110, offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, flags=(unknown: 0)) at block.c:3447 #14 0x7fa5a0c51007 in bdrv_co_do_writev (bs=0x7fa5a198c110, sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468, flags=(unknown: 0)) at block.c:3471 #15 0x7fa5a0c542b3 in bdrv_co_do_rw (opaque=0x7fa5a17a) at block.c:4706 #16 0x7fa5a0c64e6e in coroutine_trampoline (i0=-1585909408, i1=32677) at coroutine-ucontext.c:121 #17 0x7fa59dc5aa50 in __correctly_grouped_prefixwc () from /lib64/libc.so.6 #18 0x in ?? () After analyzing the stack and reviewing the code, we find the qemu_co_queue_run_restart should not be put in the coroutine_swap function which can be invoked by qemu_coroutine_enter or qemu_coroutine_yield. Only qemu_coroutine_enter needs to restart the co_queue. The error scenario is given in the following patch log. Bin Wu (1): qemu-coroutine: fix qemu_co_queue_run_restart error qemu-coroutine.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH] qemu-coroutine: fix qemu_co_queue_run_restart error
From: Bin Wu wu.wu...@huawei.com
The error scenario is as follow: coroutine C1 enters C2, C2 yields
back to C1, then C1 ternimates and the related coroutine memory
becomes invalid. After a while, the C2 coroutine is entered again.
At this point, C1 is used as a parameter passed to
qemu_co_queue_run_restart. Therefore, qemu_co_queue_run_restart
accesses an invalid memory and a segfault error ocurrs.
The qemu_co_queue_run_restart function re-enters coroutines waiting
in the co_queue. However, this function should be only used int the
qemu_coroutine_enter context. Only in this context, when the current
coroutine gets execution control again(after the execution of
qemu_coroutine_switch), we can restart the target coutine because the
target coutine has yielded back to the current coroutine or it has
terminated.
First we want to put qemu_co_queue_run_restart in qemu_coroutine_enter,
but we find we can not access the target coroutine if it terminates.
Signed-off-by: Bin Wu wu.wu...@huawei.com
---
qemu-coroutine.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 525247b..9a294c4 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -99,24 +99,25 @@ static void coroutine_delete(Coroutine *co)
qemu_coroutine_delete(co);
}
-static void coroutine_swap(Coroutine *from, Coroutine *to)
+static CoroutineAction coroutine_swap(Coroutine *from, Coroutine *to)
{
CoroutineAction ret;
ret = qemu_coroutine_switch(from, to, COROUTINE_YIELD);
-qemu_co_queue_run_restart(to);
-
switch (ret) {
case COROUTINE_YIELD:
-return;
+break;
case COROUTINE_TERMINATE:
trace_qemu_coroutine_terminate(to);
+qemu_co_queue_run_restart(to);
coroutine_delete(to);
-return;
+break;
default:
abort();
}
+
+return ret;
}
void qemu_coroutine_enter(Coroutine *co, void *opaque)
@@ -133,6 +134,8 @@ void qemu_coroutine_enter(Coroutine *co, void *opaque)
co-caller = self;
co-entry_arg = opaque;
coroutine_swap(self, co);
+if (ret == COROUTINE_YIELD)
+qemu_co_queue_run_restart(co);
}
void coroutine_fn qemu_coroutine_yield(void)
--
1.7.12.4
Re: [Qemu-devel] [RFC] pseries: Enable in-kernel H_LOGICAL_CI_{LOAD, STORE} implementations
On 09.02.15 01:37, David Gibson wrote:
On Fri, 06 Feb 2015 08:56:32 +0100
Alexander Graf ag...@suse.de wrote:
On 06.02.15 03:54, David Gibson wrote:
On Thu, Feb 05, 2015 at 12:55:45PM +0100, Alexander Graf wrote:
On 05.02.15 12:30, David Gibson wrote:
On Thu, Feb 05, 2015 at 11:22:13AM +0100, Alexander Graf wrote:
[snip]
[snip]
+ret1 = kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_LOAD);
+if (ret1 != 0) {
+fprintf(stderr, Warning: error enabling
H_LOGICAL_CI_LOAD in KVM:
+ %s\n, strerror(errno));
+}
+
+ret2 = kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_STORE);
+if (ret2 != 0) {
+fprintf(stderr, Warning: error enabling
H_LOGICAL_CI_STORE in KVM:
+ %s\n, strerror(errno));
+ }
+
+if ((ret1 != 0) || (ret2 != 0)) {
+fprintf(stderr, Warning: Couldn't enable H_LOGICAL_CI_*
in KVM, SLOF
+ may be unable to operate devices with
in-kernel emulation\n);
+}
You'll always get these warnings if you're running on an old
(meaning
current upstream) kernel, which could be annoying.
True.
Is there any way
to tell whether you have configured any devices which need the
in-kernel MMIO emulation and only warn if you have?
In theory, I guess so. In practice I can't see how you'd enumerate
all devices that might require kernel intervention without something
horribly invasive.
We could WARN_ONCE in QEMU if we emulate such a hypercall, but its
handler is io_mem_unassigned (or we add another minimum priority huge
memory region on all 64bits of address space that reports the
breakage).
Would that work for the virtio+iothread case? I had the impression
the kernel handled notification region was layered over the qemu
emulated region in that case.
IIRC we don't have a way to call back into kvm saying please write to
this in-kernel device. But we could at least defer the warning to a
point where we know that we actually hit it.
Right, but I'm saying we might miss the warning in cases where we want
it, because the KVM device is shadowed by a qemu device, so qemu won't
see the IO as unassigned or unhandled.
In particular, I think that will happen in the case of virtio-blk with
iothread, which is the simplest case in which to observe the problem.
The virtio-blk device exists in qemu and is functional, but we rely on
KVM catching the queue notification MMIO before it reaches the qemu
implementation of the rest of the device's IO space.
But in that case the VM stays functional and will merely see a
performance hit when using virtio in SLOF, no? I don't think that's
a problem worth worrying users about.
Alas, no. The iothread stuff *relies* on the in-kernel notification,
so it will not work if the IO gets punted to qemu. This is the whole
reason for the in-kernel hcall implementation.
So at least with vhost-net the in-kernel trapping is optional. If we
happen to get MMIO into QEMU, we'll just handle it there.
Enlighten me why the iothread stuff can't handle it that way too.
So, as I understand it, it could, but it doesn't. Working out how to
fix it properly requires better understanding of the dataplane code
than I currently possess,
So, using virtio-blk as the example case. Normally the queue notify
mmio will get routed by the general virtio code to
virtio_blk_handle_output().
In the case of dataplane, that just calls
virtio_blk_data_plane_start(). So the first time we get a vq notify,
the dataplane is started. That sets up the host notifier
(VirtioBusClass::set_host_notifier - virtio_pci_set_host_notifier -
virtio_pci_set_host_notifier_internal - memory_region_add_eventfd()
- memory_region_transaction_commit() -
address_space_update_ioeventfds - address_space_add_del_ioeventfds -
kvm_mem_ioeventfd_add - kvm_set_ioeventfd_mmio - KVM_IOEVENTFD
ioctl)
From this point on further calls to virtio_blk_handle_output() are
IIUC a can't happen, because vq notifies should go to the eventfd
instead, where they will kick the iothread.
So, with SLOF, the first request is ok - it hits
virtio_blk_handle_output() which starts the iothread which goes on to
process the request.
On the second request, however, we get back into
virtio_blk_data_plane_start() which sees the iothread is already
running and aborts. I think it is assuming that this must be the
result of a race with another vcpu starting the dataplane, and so
assumes the racing thread will have woken the dataplane which will
then handle this vcpu's request as well.
In our case, however, the IO hcalls go through to
virtio_blk_handle_output() when the dataplane already going, and
become no-ops without waking it up again to handle the new request.
Enlightened enough yet?
So reading this, it sounds like we could just add logic in the virtio
dataplane code that allows for a graceful fallback to QEMU based MMIO by
triggering the eventfd itself in the MMIO handler.
Re: [Qemu-devel] [PATCH] quorum: don't share qiov
On 02/03/2015 07:01 PM, Paolo Bonzini wrote: On 03/02/2015 10:22, Kevin Wolf wrote: Paolo, I think it's rather surprising that iov_send_recv() modifies its iov. The modification is undone at the end, so you seem to have considered that a caller might be reusing it after and you can't use it up, but we still get problems with concurrent accesses. Yes, I wasn't thinking of concurrent accesses indeed. But I wasn't the author of iov_send_recv(), I just took it from sheepdog. :) Was it an intentional design decision that iov_send_recv() is the sole owner of the iov and the caller must duplicate it if it's used elsewhere concurrently? Otherwise I would suggest to fix iov_send_recv(), and possibly try and make all the qiov/iov arguments in the block layer const. I agree. However, it's not a small change. I think Wen's patch is okay with a FIXME comment added. Hi, kevin What should I do next? Add a comment and resend it? Thanks Wen Congyang Paolo .
[Qemu-devel] [PATCH] win64: perform correct setjmp calls
On w64, setjmp is implemented by _setjmp which needs a second parameter.
This parameter should be NULL to allow using longjump from generated code.
This patch replaces all usages of setjmp.h with new header files which
replaces setjmp with _setjmp function on win64 platform.
Signed-off-by: Pavel Dovgalyuk pavel.dovga...@ispras.ru
---
coroutine-sigaltstack.c |2 +-
coroutine-ucontext.c |2 +-
disas/i386.c |2 +-
disas/m68k.c |2 +-
include/qom/cpu.h |2 +-
include/sysemu/os-win32.h | 17 -
include/sysemu/setjmp.h | 35 +++
util/oslib-posix.c|2 +-
8 files changed, 41 insertions(+), 23 deletions(-)
create mode 100755 include/sysemu/setjmp.h
diff --git a/coroutine-sigaltstack.c b/coroutine-sigaltstack.c
index 63519ff..c890496 100644
--- a/coroutine-sigaltstack.c
+++ b/coroutine-sigaltstack.c
@@ -26,7 +26,7 @@
#undef _FORTIFY_SOURCE
#endif
#include stdlib.h
-#include setjmp.h
+#include sysemu/setjmp.h
#include stdint.h
#include pthread.h
#include signal.h
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 259fcb4..12e2826 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -23,7 +23,7 @@
#undef _FORTIFY_SOURCE
#endif
#include stdlib.h
-#include setjmp.h
+#include sysemu/setjmp.h
#include stdint.h
#include ucontext.h
#include qemu-common.h
diff --git a/disas/i386.c b/disas/i386.c
index 00ceca9..fdb1ec6 100644
--- a/disas/i386.c
+++ b/disas/i386.c
@@ -153,7 +153,7 @@
/* opcodes/i386-dis.c r1.126 */
#include qemu-common.h
-#include setjmp.h
+#include sysemu/setjmp.h
static int fetch_data2(struct disassemble_info *, bfd_byte *);
static int fetch_data(struct disassemble_info *, bfd_byte *);
diff --git a/disas/m68k.c b/disas/m68k.c
index cc0db96..312351e 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -616,7 +616,7 @@ static const char *const reg_half_names[] =
/* Maximum length of an instruction. */
#define MAXLEN 22
-#include setjmp.h
+#include sysemu/setjmp.h
struct private
{
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 2098f1c..16dc2f7 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -21,7 +21,7 @@
#define QEMU_CPU_H
#include signal.h
-#include setjmp.h
+#include sysemu/setjmp.h
#include hw/qdev-core.h
#include exec/hwaddr.h
#include qemu/queue.h
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index af3fbc4..0387313 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -55,23 +55,6 @@
# define EWOULDBLOCK WSAEWOULDBLOCK
#endif
-#if defined(_WIN64)
-/* On w64, setjmp is implemented by _setjmp which needs a second parameter.
- * If this parameter is NULL, longjump does no stack unwinding.
- * That is what we need for QEMU. Passing the value of register rsp (default)
- * lets longjmp try a stack unwinding which will crash with generated code. */
-# undef setjmp
-# define setjmp(env) _setjmp(env, NULL)
-#endif
-/* QEMU uses sigsetjmp()/siglongjmp() as the portable way to specify
- * longjmp and don't touch the signal masks. Since we know that the
- * savemask parameter will always be zero we can safely define these
- * in terms of setjmp/longjmp on Win32.
- */
-#define sigjmp_buf jmp_buf
-#define sigsetjmp(env, savemask) setjmp(env)
-#define siglongjmp(env, val) longjmp(env, val)
-
/* Declaration of ffs() is missing in MinGW's strings.h. */
int ffs(int i);
diff --git a/include/sysemu/setjmp.h b/include/sysemu/setjmp.h
new file mode 100755
index 000..ffd804a
--- /dev/null
+++ b/include/sysemu/setjmp.h
@@ -0,0 +1,35 @@
+/*
+ * setjmp.h
+ *
+ * Copyright (c) 2015 Institute for System Programming
+ *of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#ifndef QEMU_SETJMP_H
+#define QEMU_SETJMP_H
+
+#include setjmp.h
+
+#if defined(_WIN32)
+#if defined(_WIN64)
+/* On w64, setjmp is implemented by _setjmp which needs a second parameter.
+ * If this parameter is NULL, longjump does no stack unwinding.
+ * That is what we need for QEMU. Passing the value of register rsp (default)
+ * lets longjmp try a stack unwinding which will crash with generated code. */
+# undef setjmp
+# define setjmp(env) _setjmp(env, NULL)
+#endif
+/* QEMU uses sigsetjmp()/siglongjmp() as the portable way to specify
+ * longjmp and don't touch the signal masks. Since we know that the
+ * savemask parameter will always be zero we can safely define these
+ * in terms of setjmp/longjmp on Win32.
+ */
+#define sigjmp_buf jmp_buf
+#define sigsetjmp(env, savemask) setjmp(env)
+#define siglongjmp(env, val) longjmp(env, val)
+#endif
+
+#endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 16fcec2..23b7af8 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -59,7 +59,7 @@ extern int daemon(int, int);
#include qemu/sockets.h
[Qemu-devel] GSOC 2015 Project Proposal
Hi I would like to do GSOC this summer. The project i have in mind is to implement a set of facilities to make implementing Hardware transactional memory (HTM) easier in QEMU. HTM has become available in many architecture supported by QEMU, e.g. i386, PowerPC, etc. Currently, necessary memory tracking. conflict detection and transaction rollbak/commit are not available in QEMU. As a result, HTM is supported in a very rudimentary fashion in PowerPC, i.e. the transaction begins (tbegin in PowerPC) always trigger a fault to the fallback code path. Even though HTM is supported by different architectures, the underlying principle are very similar and therefore it is beneficial to provide a set of facilities to make implementing HTM easier in QEMU. These facilities should include. A modified software TLB to make memory address and value tracking simple. A performant and memory efficient value/address tracking facility to detect read/write conflicts for transactions. A performant and memory efficient mechanism to rollback and commit memory accesses. A mechanism to abort transactions on the current processor as well as remote processor. I will come up with a more detailed proposal as application time draws close. Any suggestions are appreciated at the moment. Thanks, Xin
Re: [Qemu-devel] Virtio Disk drivers and Microsoft clustering
On Fri, 02/06 11:23, Fam Zheng wrote:
On Thu, 02/05 15:29, massimo buscato wrote:
Hi all!
About virtio-scsi driver:
There are many problem to use it on windows 2012 cluster service.
Every time you try to validate a Virtio disk under W2012 cluster tool,
you have this errors:
with VIRTIO DISK device:
The port driver used by the disk does not support clustering. Disk
bus type does not support clustering. Disk partition style is MBR.
Disk type is BASIC.
with VIRTIO SCSI DISK device:
The port driver used by the disk does not support clustering. Disk
bus type does not support clustering. Disk partition style is MBR.
Disk type is BASIC. The required inquiry data (SCSI page 83h VPD
descriptor) was reported as not being supported.
Hi Massimo,
I don't know much about Windows cluster service but I think this error is
because virtio-scsi is a direct attached controller, like explained in:
http://support.microsoft.com/kb/2839292?wa=wsignin1.0
Discussed a bit with Massimo off-list, this feature basically means two systems
can see the same lun at the same time, and one of them accesses it. When one
system is down, the other takes over. In VMware a disk with RDM (raw device
mapping) could be added, so I think scsi-block and/or scsi-generic should work
too.
If I understand correctly, Massimo uses image based scsi-disk.
scsi-{block,generic} requires a scsi target on host side. In order to use an
image file, we could use iscsi service to export the host image and use the
iscsi driver in QEMU. Not sure if there are simpler ways.
Fam
