Re: [Qemu-devel] QEMU ARM946 emulation, DIGIC, and MPU fault handling
On Thu, 23 Jan 2014 22:25:36 + Peter Maydell wrote: > Hi Antony; have you noticed any issues with QEMU's handling of > MPU faults (data or address faults) on our ARM946 model? > I ask because DIGIC is the only board we have that uses the 946, > and as far as I can tell from the QEMU source code we will > incorrectly trash the access permissions registers any time we > take an MPU fault. > > By 'access permission registers' I mean these: > > MRC p15, 0, Rd, c5, c0, 2; read data access permission bits > MRC p15, 0, Rd, c5, c0, 3; read instruction access permission bits > MRC p15, 0, Rd, c5, c0, 0; read data access permission bits > MRC p15, 0, Rd, c5, c0, 1; read instruction access permission bits > > as described in the 946 TRM: > http://infocenter.arm.com/help/topic/com.arm.doc.ddi0201d/Babfaiic.html > > Looking at the source code it seems like cpu_arm_handle_mmu_fault() > will write the fault status into c5_data/c5_insn, because that is where > we keep the DFSR and IFSR for an ARM with an MMU. > > Since the 946 doesn't provide any way to find out what the fault > address actually was (it has no DFAR or IFAR) I presume that all > guest software treats a data abort or prefetch abort as a fatal error, > which is probably part of why nobody's ever noticed this. > > This bug would also affect the ARMv7M CPU (Cortex-M3) we emulate, > except that as far as I can tell we don't implement its MPU interface at all! > (it uses memory mapped registers rather than cp15 regs, and they just > aren't wired up in armv7m_nvic.c...) Sorry for delay! The number of software, running on DIGIC is very limited: * Canon Camera firware + maybe Magic Lantern or CHDK; * barebox. I know nothing about MPU fault handling in barebox. I'm planning to merge current qemu DIGIC support with patches from MagicLantern qemu patches. May be during this work I'll be capable to answer your question, but just now I have no answer, sorry. May be Magic Lantern people (especiall g3gg0) can help? I have added them to Cc. -- Best regards, Antony Pavlov
Re: [Qemu-devel] [PATCH v4 0/5] QMP full introspection
On Thu, Jan 23, 2014 at 10:46:31PM +0800, Amos Kong wrote: > This is an implement of qmp full-introspection, > parse and convert the json schema to a dynamical tree, > return it to management through QMP command output. > Correct the URLs > The whole output of query-qmp-schema command: > http://i-kvm.rhcloud.com/static/pub/v4/qmp-introspection.output.txt > http://i-kvm.rhcloud.com/static/pub/v4/qmp-introspection.h https://i-kvm.rhcloud.com/static/pub/v4/qmp-introspection.output.txt https://i-kvm.rhcloud.com/static/pub/v4/qapi-introspect.h > > Welcome your comments! > > V2: use 'DataObject' to describe dynamic struct > V3: improve the metadata as suggested by eric > V4: use python to extend/parse schema for improving > the response speed and simple the code > > Amos Kong (5): > qapi: introduce DataObject to describe dynamic structs > qapi: add qapi-introspect.py code generator > qobject: introduce qobject_get_str() > qmp: full introspection support for QMP > update docs/qmp-full-introspection.txt
Re: [Qemu-devel] [PATCH target-arm v5 0/5] Reset and Halting modifications + Zynq SMP
ping! On Wed, Jan 15, 2014 at 7:12 PM, Peter Crosthwaite wrote: > Hi All, > > The clock controller module in the Zynq platform has the ability to halt > and reset arbitrary devices, including the CPU. We use this feature to > implement > SMP Linux - the kernel halts CPU1 then rewrites the vector table to the > secondary entry point and the resets+unhalts. > > This series adds SMP support to the Zynq machine, and patches the Zynq SLCR > (the clock controller) to have GPIOs connected to the CPUs. The GPIOs > cause and ARM CPU reset. > > Only the reset side is implemented (which is good enough for SMP linux > as it stands). Future work is to implement the halting behaviour as > well. > > changed since v4 (PMM review): > Convert to GPIO scheme > Implemented custom secondary cpu reset > OCM Macro cleanup > changed since v3: > Removed halting patches > Reduced to minimal change needed for SMP Zynq > > > Peter Crosthwaite (5): > arm: zynq: Macroify OCM Base and Size > arm: zynq: added SMP support > zynq_slcr: Implement CPU reset > arm: Implement reset GPIO. > arm: zynq: Connect CPU resets to SLCR > > hw/arm/xilinx_zynq.c | 87 > > hw/misc/zynq_slcr.c | 16 ++ > target-arm/cpu.c | 23 ++ > target-arm/cpu.h | 8 +++-- > 4 files changed, 112 insertions(+), 22 deletions(-) > > -- > 1.8.5.3 >
Re: [Qemu-devel] [PATCH v4 2/5] qapi: add qapi-introspect.py code generator
On Thu, 01/23 22:46, Amos Kong wrote: > This is a code generator for qapi introspection. It will parse > qapi-schema.json, extend schema definitions and generate a schema > table with metadata, it references to the new structs which we used > to describe dynamic data structs. The metadata will help C code to > allocate right structs and provide useful information to management > to checking suported feature and QMP commandline detail. The schema > table will be saved to qapi-introspect.h. > > The $(prefix) is used to as a namespace to keep the generated code s/used to as/used as/ > from one schema/code-generation separated from others so code and > be generated from multiple schemas with clobbering previously s/with/without/ > created code. > > Signed-off-by: Amos Kong > --- > .gitignore | 1 + > Makefile| 5 +- > docs/qmp-full-introspection.txt | 17 > scripts/qapi-introspect.py | 172 > > 4 files changed, 194 insertions(+), 1 deletion(-) > create mode 100644 scripts/qapi-introspect.py > > diff --git a/.gitignore b/.gitignore > index 1c9d63d..de3cb80 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -22,6 +22,7 @@ linux-headers/asm > qapi-generated > qapi-types.[ch] > qapi-visit.[ch] > +qapi-introspect.h > qmp-commands.h > qmp-marshal.c > qemu-doc.html > diff --git a/Makefile b/Makefile > index bdff4e4..1dac5e7 100644 > --- a/Makefile > +++ b/Makefile > @@ -45,7 +45,7 @@ endif > endif > > GENERATED_HEADERS = config-host.h qemu-options.def > -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h > +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h > qapi-introspect.h > GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c > > GENERATED_HEADERS += trace/generated-events.h > @@ -229,6 +229,9 @@ $(SRC_PATH)/qapi-schema.json > $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) > qmp-commands.h qmp-marshal.c :\ > $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py > $(gen-out-type) -m -o "." < $<, " GEN $@") > +qapi-introspect.h:\ > +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-introspect.py > $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py > $(gen-out-type) -o "." < $<, " GEN $@") > > QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h > qga-qapi-visit.h qga-qmp-commands.h) > $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) > diff --git a/docs/qmp-full-introspection.txt b/docs/qmp-full-introspection.txt > index d2cf7b3..8ecbc0c 100644 > --- a/docs/qmp-full-introspection.txt > +++ b/docs/qmp-full-introspection.txt > @@ -42,3 +42,20 @@ types. > > 'anonymous-struct' will be used to describe arbitrary structs > (dictionary, list or string). > + > +== Avoid dead loop in recursive extending == > + > +We have four types (ImageInfo, BlockStats, PciDeviceInfo, ObjectData) > +that uses themself in their own define data directly or indirectly, s/themself/themselves/ s/define data/definition/ > +we will not repeatedly extend them to avoid dead loop. > + > +We use a 'parents List' to record the visit path, type name of each > +extended node will be saved to the List. > + > +Append type name to the list before extending, and remove type name > +from the list after extending. > + > +If the type name is already extended in parents List, we won't extend > +it repeatedly for avoiding dead loop. This "parents" list detail is not reflected in the generated information, right? I think it's good enough to describe that "type will not be extented more than once in a schema, when there's direct or indirect recursive type composition". > + > +'recursive' indicates if the type is extended or not. > diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py > new file mode 100644 > index 000..03179fa > --- /dev/null > +++ b/scripts/qapi-introspect.py > @@ -0,0 +1,172 @@ > +# > +# QAPI introspection info generator > +# > +# Copyright (C) 2014 Red Hat, Inc. > +# > +# Authors: > +# Amos Kong > +# > +# This work is licensed under the terms of the GNU GPLv2. > +# See the COPYING.LIB file in the top-level directory. > + > +from ordereddict import OrderedDict > +from qapi import * > +import sys > +import os > +import getopt > +import errno > + > + > +try: > +opts, args = getopt.gnu_getopt(sys.argv[1:], "hp:o:", > + ["header", "prefix=", "output-dir="]) > +except getopt.GetoptError, err: > +print str(err) > +sys.exit(1) > + > +output_dir = "" > +prefix = "" > +h_file = 'qapi-introspect.h' > + > +do_h = False > + > +for o, a in opts: > +if o in ("-p", "--prefix"): > +prefix = a Is this option used in your series? Thanks, Fam > +elif o in ("-o", "--output-dir"): > +output_dir = a + "/" > +elif o in ("-h", "--header"): > +do_h = T
Re: [Qemu-devel] [PATCH v4 1/4] qcow2: remove n_start and n_end of qcow2_alloc_cluster_offset()
On Thu, Jan 23, 2014 at 03:29:04PM +0100, Kevin Wolf wrote: > Am 23.01.2014 um 04:04 hat Hu Tao geschrieben: > > n_start can be actually calculated from offset. The number of > > sectors to be allocated(n_end - n_start) can be passed in in > > num. By removing n_start and n_end, we can save two parameters. > > > > The side effect is there is a bug in qcow2.c:preallocate() that > > passes incorrect n_start to qcow2_alloc_cluster_offset() is > > fixed. The bug can be triggerred by a larger cluster size than > > the default value(65536), for example: > > > > ./qemu-img create -f qcow2 \ > > -o 'cluster_size=131072,preallocation=metadata' file.img 4G > > > > Reviewed-by: Max Reitz > > Signed-off-by: Hu Tao > > --- > > block/qcow2-cluster.c | 14 ++ > > block/qcow2.c | 11 +++ > > block/qcow2.h | 2 +- > > trace-events | 2 +- > > 4 files changed, 11 insertions(+), 18 deletions(-) > > > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > > index 8534084..c57f39d 100644 > > --- a/block/qcow2-cluster.c > > +++ b/block/qcow2-cluster.c > > @@ -1182,7 +1182,7 @@ fail: > > * Return 0 on success and -errno in error cases > > */ > > int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset, > > -int n_start, int n_end, int *num, uint64_t *host_offset, QCowL2Meta > > **m) > > +int *num, uint64_t *host_offset, QCowL2Meta **m) > > { > > BDRVQcowState *s = bs->opaque; > > uint64_t start, remaining; > > @@ -1190,15 +1190,13 @@ int qcow2_alloc_cluster_offset(BlockDriverState > > *bs, uint64_t offset, > > uint64_t cur_bytes; > > int ret; > > > > -trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset, > > - n_start, n_end); > > +trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset, *num); > > > > -assert(n_start * BDRV_SECTOR_SIZE == offset_into_cluster(s, offset)); > > -offset = start_of_cluster(s, offset); > > +assert((offset & ~BDRV_SECTOR_MASK) == 0); > > > > again: > > -start = offset + (n_start << BDRV_SECTOR_BITS); > > -remaining = (n_end - n_start) << BDRV_SECTOR_BITS; > > +start = offset; > > +remaining = *num << BDRV_SECTOR_BITS; > > cluster_offset = 0; > > *host_offset = 0; > > cur_bytes = 0; > > @@ -1284,7 +1282,7 @@ again: > > } > > } > > > > -*num = (n_end - n_start) - (remaining >> BDRV_SECTOR_BITS); > > +*num -= remaining >> BDRV_SECTOR_BITS; > > assert(*num > 0); > > assert(*host_offset != 0); > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index 8ec9db1..0a310cc 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > @@ -992,7 +992,6 @@ static coroutine_fn int > > qcow2_co_writev(BlockDriverState *bs, > > { > > BDRVQcowState *s = bs->opaque; > > int index_in_cluster; > > -int n_end; > > int ret; > > int cur_nr_sectors; /* number of sectors in current iteration */ > > uint64_t cluster_offset; > > @@ -1016,14 +1015,10 @@ static coroutine_fn int > > qcow2_co_writev(BlockDriverState *bs, > > > > trace_qcow2_writev_start_part(qemu_coroutine_self()); > > index_in_cluster = sector_num & (s->cluster_sectors - 1); > > -n_end = index_in_cluster + remaining_sectors; > > -if (s->crypt_method && > > -n_end > QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors) { > > -n_end = QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors; > > -} > > +cur_nr_sectors = remaining_sectors; > > You still need to limit cur_nr_sectors for the encrypted case, otherwise > you get a buffer overflow of cluster_data later in the function. My > complaint in v3 was not that you have the limiting, but that applying it > to n_end doesn't have any effect any more, you need to apply it to > cur_nr_sectors. Thanks! I didn't understand you completely:-P.
Re: [Qemu-devel] [PATCH v4 2/5] qapi: add qapi-introspect.py code generator
On Fri, Jan 24, 2014 at 05:12:12PM +0800, Fam Zheng wrote: > On Thu, 01/23 22:46, Amos Kong wrote: > > This is a code generator for qapi introspection. It will parse > > qapi-schema.json, extend schema definitions and generate a schema > > table with metadata, it references to the new structs which we used > > to describe dynamic data structs. The metadata will help C code to > > allocate right structs and provide useful information to management > > to checking suported feature and QMP commandline detail. The schema > > table will be saved to qapi-introspect.h. > > > > The $(prefix) is used to as a namespace to keep the generated code > > s/used to as/used as/ > > > from one schema/code-generation separated from others so code and > > be generated from multiple schemas with clobbering previously > > s/with/without/ > > > created code. > > > > Signed-off-by: Amos Kong > > --- > > .gitignore | 1 + > > Makefile| 5 +- > > docs/qmp-full-introspection.txt | 17 > > scripts/qapi-introspect.py | 172 > > > > 4 files changed, 194 insertions(+), 1 deletion(-) > > create mode 100644 scripts/qapi-introspect.py > > > > diff --git a/.gitignore b/.gitignore > > index 1c9d63d..de3cb80 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -22,6 +22,7 @@ linux-headers/asm > > qapi-generated > > qapi-types.[ch] > > qapi-visit.[ch] > > +qapi-introspect.h > > qmp-commands.h > > qmp-marshal.c > > qemu-doc.html > > diff --git a/Makefile b/Makefile > > index bdff4e4..1dac5e7 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -45,7 +45,7 @@ endif > > endif > > > > GENERATED_HEADERS = config-host.h qemu-options.def > > -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h > > +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h > > qapi-introspect.h > > GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c > > > > GENERATED_HEADERS += trace/generated-events.h > > @@ -229,6 +229,9 @@ $(SRC_PATH)/qapi-schema.json > > $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) > > qmp-commands.h qmp-marshal.c :\ > > $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py > > $(qapi-py) > > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py > > $(gen-out-type) -m -o "." < $<, " GEN $@") > > +qapi-introspect.h:\ > > +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-introspect.py > > $(qapi-py) > > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py > > $(gen-out-type) -o "." < $<, " GEN $@") > > > > QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h > > qga-qapi-visit.h qga-qmp-commands.h) > > $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) > > diff --git a/docs/qmp-full-introspection.txt > > b/docs/qmp-full-introspection.txt > > index d2cf7b3..8ecbc0c 100644 > > --- a/docs/qmp-full-introspection.txt > > +++ b/docs/qmp-full-introspection.txt > > @@ -42,3 +42,20 @@ types. > > > > 'anonymous-struct' will be used to describe arbitrary structs > > (dictionary, list or string). > > + > > +== Avoid dead loop in recursive extending == > > + > > +We have four types (ImageInfo, BlockStats, PciDeviceInfo, ObjectData) > > +that uses themself in their own define data directly or indirectly, > > s/themself/themselves/ > s/define data/definition/ > > > +we will not repeatedly extend them to avoid dead loop. > > + > > +We use a 'parents List' to record the visit path, type name of each > > +extended node will be saved to the List. > > + > > +Append type name to the list before extending, and remove type name > > +from the list after extending. > > + > > +If the type name is already extended in parents List, we won't extend > > +it repeatedly for avoiding dead loop. > > This "parents" list detail is not reflected in the generated information, > right? Not, it just used to help the extending. > I think it's good enough to describe that "type will not be extented > more than once in a schema, when there's direct or indirect recursive > type composition". It's more meaningful. > > + > > +'recursive' indicates if the type is extended or not. > > diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py > > new file mode 100644 > > index 000..03179fa > > --- /dev/null > > +++ b/scripts/qapi-introspect.py > > @@ -0,0 +1,172 @@ > > +# > > +# QAPI introspection info generator > > +# > > +# Copyright (C) 2014 Red Hat, Inc. > > +# > > +# Authors: > > +# Amos Kong > > +# > > +# This work is licensed under the terms of the GNU GPLv2. > > +# See the COPYING.LIB file in the top-level directory. > > + > > +from ordereddict import OrderedDict > > +from qapi import * > > +import sys > > +import os > > +import getopt > > +import errno > > + > > + > > +try: > > +opts, args = getopt.gnu_getopt(sys.argv[1:], "hp:o:", > > + ["header", "prefix=", "output-dir="]) > > +except getopt.GetoptErr
Re: [Qemu-devel] [PATCH v4 1/4] qcow2: remove n_start and n_end of qcow2_alloc_cluster_offset()
On Thu, Jan 23, 2014 at 06:02:08PM +0100, Benoît Canet wrote: > Le Thursday 23 Jan 2014 à 11:04:05 (+0800), Hu Tao a écrit : > > n_start can be actually calculated from offset. The number of > > sectors to be allocated(n_end - n_start) can be passed in in > > num. By removing n_start and n_end, we can save two parameters. > > > > The side effect is there is a bug in qcow2.c:preallocate() that > > passes incorrect n_start to qcow2_alloc_cluster_offset() is > > fixed. The bug can be triggerred by a larger cluster size than > > the default value(65536), for example: > > > > ./qemu-img create -f qcow2 \ > > -o 'cluster_size=131072,preallocation=metadata' file.img 4G > > > > Reviewed-by: Max Reitz > > Signed-off-by: Hu Tao > > --- > > block/qcow2-cluster.c | 14 ++ > > block/qcow2.c | 11 +++ > > block/qcow2.h | 2 +- > > trace-events | 2 +- > > 4 files changed, 11 insertions(+), 18 deletions(-) > > > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > > index 8534084..c57f39d 100644 > > --- a/block/qcow2-cluster.c > > +++ b/block/qcow2-cluster.c > > @@ -1182,7 +1182,7 @@ fail: > > * Return 0 on success and -errno in error cases > > */ > > int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset, > > -int n_start, int n_end, int *num, uint64_t *host_offset, QCowL2Meta > > **m) > > +int *num, uint64_t *host_offset, QCowL2Meta **m) > > { > > BDRVQcowState *s = bs->opaque; > > uint64_t start, remaining; > > @@ -1190,15 +1190,13 @@ int qcow2_alloc_cluster_offset(BlockDriverState > > *bs, uint64_t offset, > > uint64_t cur_bytes; > > int ret; > > > > -trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset, > > - n_start, n_end); > > +trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset, *num); > > > > -assert(n_start * BDRV_SECTOR_SIZE == offset_into_cluster(s, offset)); > > -offset = start_of_cluster(s, offset); > > +assert((offset & ~BDRV_SECTOR_MASK) == 0); > > Why replace something that would round gently an unaligned offset > (start_of_cluster) by an assert that would make QEMU exit ? It is equivalent to the removed assert().
Re: [Qemu-devel] [PATCH 07/13 v7] dump: add members to DumpState and init some of them
On 01/24/14 02:52, Qiao Nuohan wrote: > On 01/23/2014 01:04 AM, Laszlo Ersek wrote: >>> @@ -864,6 +884,16 @@ static int dump_init(DumpState *s, int fd, bool >>> paging, bool has_filter, >>> > >>> qemu_get_guest_simple_memory_mapping(&s->list,&s->guest_phys_blocks); >>> >} >>> > >>> > +s->nr_cpus = nr_cpus; >>> > +s->page_size = TARGET_PAGE_SIZE; >>> > +s->page_shift = ffs(s->page_size) - 1; >>> > + >>> > +get_max_mapnr(s); >> Again from v6 10/11, good. The flag_flatten assignment has been dropped. >> Initialization seems to happen in a good spot this time too. >> >>> > + >>> > +uint64_t tmp; >>> > +tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), >>> s->page_size); >>> > +s->len_dump_bitmap = tmp * s->page_size; >>> > + >>> >if (s->has_filter) { >>> >memory_mapping_filter(&s->list, s->begin, s->length); >>> >} >> Again from v6 10/11. >> >> These assignments now all occur without depending on a user request for >> a compressed dump (kept this way in v7 12/13 too), but they are not >> costly. The loop in get_max_mapnr() iterates over less than 10 mappings >> in the non-paging dump case, and in the paging dump case it also >> shouldn't be more than a hundred or so (as I recall from earlier >> testing). This might be worth some regression-testing (perf-wise), but >> it looks OK to me. >> > > I see, moving them into "if(format...) {...}" block would be better. But, I > have no idea of "regression-testing (perf-wise)", would you mind give > some hint? I meant comparing how long it would take to dump in paging mode before this patchset, vs. after this patchset. In order to see the difference that is introduced by get_max_mapnr() when paging is enabled. However, please ignore this point. First, the loop is most probably negligible even for a paging dump. Second, you could make it conditional on compressed dumps (which force non-paging + non-filtering), where the number of mappings is very low. And third, as I wrote in later, the loop should be replaced anyway with an O(1) QTAILQ_LAST() access. So please just ignore this "performance" remark. Ultimately, what I suggest for get_max_mapnr() is: - rebase it to guest_phys_blocks, just like the other two places (which are now calling get_next_page()), - use QTAILQ_LAST() in it, - don't bother making it conditional (ie. its current call site is fine), because: - It'll be fast with QTAILQ_LAST(), - guest_phys_blocks is available in any case, so you can access it always Thanks Laszlo
Re: [Qemu-devel] [PATCH v4 2/4] qcow2: fix offset overflow in qcow2_alloc_clusters_at()
On Thu, Jan 23, 2014 at 06:13:48PM +0100, Benoît Canet wrote: > Le Thursday 23 Jan 2014 à 11:04:06 (+0800), Hu Tao a écrit : > > When cluster size is big enough it can lead offset overflow > > Maybe "it can lead to an offset overflow in" > > in qcow2_alloc_clusters_at(). This patch fixes it. Sure. > > > > The allocation each time is stopped at L2 table boundary > "The allocation is stopped each time at" Sure. > > > (see handle_alloc()), so the possible maximum bytes could be > > > > 2^(cluster_bits - 3 + cluster_bits) > > So if understand cluster_bits - 3 is used to compute the number of entry by L2 > and the additional cluster_bits is to take into account each clusters > referenced > by the L2 entries. Exactly. This is clearer than just one calculation. Do you mind I put the sentence in commit message? > It makes sense. > > > > > so int is safe for cluster_bits<=17, unsafe otherwise. > > > > Reviewed-by: Max Reitz > > Signed-off-by: Hu Tao > > --- > > block/qcow2-refcount.c | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > > index c974abe..8712d8b 100644 > > --- a/block/qcow2-refcount.c > > +++ b/block/qcow2-refcount.c > > @@ -676,7 +676,13 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, > > uint64_t offset, > > BDRVQcowState *s = bs->opaque; > > uint64_t cluster_index; > > uint64_t old_free_cluster_index; > > -int i, refcount, ret; > > +uint64_t i; > > +int refcount, ret; > > + > > > > +assert(nb_clusters >= 0); > > +if (nb_clusters == 0) { > > +return 0; > > +} > ^ > Adding a a line on the commit message about this assertion and chunk of code > would be helpful. How about Add an assert to guard the comparation between signed and unsigned ? > > Best regards > > Benoît > > > > > /* Check how many clusters there are free */ > > cluster_index = offset >> s->cluster_bits; > > -- > > 1.8.5.2.229.g4448466 > > > >
Re: [Qemu-devel] [PATCH v11 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD
Ping? On Wed, 01/08 18:07, Fam Zheng wrote: > This series adds for point-in-time snapshot NBD exporting based on > blockdev-backup (variant of drive-backup with existing device as target). > > We get a thin point-in-time snapshot by COW mechanism of drive-backup, and > export it through built in NBD server. The steps are as below: > > 1. (SHELL) qemu-img create -f qcow2 BACKUP.qcow2 > > (Alternatively we can use -o backing_file=RUNNING-VM.img to omit > explicitly > providing the size by ourselves, but it's risky because RUNNING-VM.qcow2 > is > used r/w by guest. Whether or not setting backing file in the image file > doesn't matter, as we are going to override the backing hd in the next > step) > > 2. (QMP) blockdev-add backing=source-drive file.driver=file > file.filename=BACKUP.qcow2 id=target0 if=none driver=qcow2 > > (where source-drive is the running BlockDriverState name for > RUNNING-VM.img. This patch implements "backing=" option to override > backing_hd for added drive) > > 3. (QMP) blockdev-backup device=source-drive sync=none target=target0 > > (this is the QMP command introduced by this series, which use a named > device as target of drive-backup) > > 4. (QMP) nbd-server-add device=target0 > > When image fleecing done: > > 1. (QMP) block-job-cancel device=source-drive > > 2. (HMP) drive_del target0 > > 3. (SHELL) rm BACKUP.qcow2 > > v8 -> v11: Rebased to qemu.git. Address Stefan's comments: > (sorry for sending 3 revisions in a day) > > [01/11] block: Add BlockOpType enum > Change enum definition as internal. > > [05/11] block: Add bdrv_set_backing_hd() > Set bs->backing_file and bs->backing_format. > > [06/11] block: Add backing_blocker in BlockDriverState > Reuse bdrv_set_backing_hd(). > > [07/11] block: Parse "backing" option to reference existing BDS > Update commit message about bdrv_swap assertion removal. > Fix use-after-free. > Check for "backing=" and "backing.file=" conflict. > > [08/11] block: Support dropping active in bdrv_drop_intermediate > Fix function comment. > > [09/11] stream: Use bdrv_drop_intermediate and drop close_unused_images > > Fam Zheng (11): > block: Add BlockOpType enum > block: Introduce op_blockers to BlockDriverState > block: Replace in_use with operation blocker > block: Move op_blocker check from block_job_create to its caller > block: Add bdrv_set_backing_hd() > block: Add backing_blocker in BlockDriverState > block: Parse "backing" option to reference existing BDS > block: Support dropping active in bdrv_drop_intermediate > stream: Use bdrv_drop_intermediate and drop close_unused_images > qmp: Add command 'blockdev-backup' > block: Allow backup on referenced named BlockDriverState > > block-migration.c | 7 +- > block.c | 304 > +++- > block/backup.c | 21 +++ > block/commit.c | 1 + > block/stream.c | 28 +--- > blockdev.c | 70 +++-- > blockjob.c | 14 +- > hw/block/dataplane/virtio-blk.c | 19 ++- > include/block/block.h | 29 +++- > include/block/block_int.h | 9 +- > include/block/blockjob.h| 3 + > qapi-schema.json| 49 +++ > qmp-commands.hx | 44 ++ > 13 files changed, 444 insertions(+), 154 deletions(-) > > -- > 1.8.5.1 > >
Re: [Qemu-devel] Possible bug in monitor code
On 01/23/2014 08:28 PM, Luiz Capitulino wrote: > On Thu, 23 Jan 2014 17:33:33 +0200 > Stratos Psomadakis wrote: > >> On 01/23/2014 03:54 PM, Luiz Capitulino wrote: >>> On Thu, 23 Jan 2014 08:44:02 -0500 >>> Luiz Capitulino wrote: >>> On Thu, 23 Jan 2014 19:23:51 +0800 Fam Zheng wrote: > Bcc: > Subject: Re: [Qemu-devel] Possible bug in monitor code > Reply-To: > In-Reply-To: <52e0ec4b.7010...@grnet.gr> > > On Thu, 01/23 12:17, Stratos Psomadakis wrote: >> On 01/23/2014 05:07 AM, Fam Zheng wrote: >>> On Wed, 01/22 17:53, Stratos Psomadakis wrote: Hi, we've encountered a weird issue regarding monitor (qmp and hmp) behavior with qemu-1.7 (and qemu-1.5). The following steps will reproduce the issue: 1) Client A connects to qmp socket with socat 2) Client A gets greeting message {"QMP": {"version": ..} 3) Client A waits (select on the socket's fd) 4) Client B tries to connect to the *same* qmp socket with socat QMP/HMP can only handle a single client per connection, so this is not supported. What you could do is to create multiple QMP/HMP instances on the command-line, but this has never been fully tested so we don't officially support this either (although it may work). Now, not gracefully failing on step 4 is a real bug here. I think the best thing to do would be to close client's B connection. Patches are welcome :) >>> Thinking about this again, I think this should already be the case >>> (as I don't believe chardev code is sitting on accept()). So maybe >>> you triggered a race on chardev code or broke something there. >> Yes, the chardev code doesn't accept any more connections until the >> currently active connection is closed. >> >> However, when a new client tries to connect (while there's another qmp / >> hmp connection active) the kernel, as long as there's room in the socket >> backlog, will return to the client that the connection has been >> successfully established and will queue the connection to be accepted >> later, when qmp actually finishes with its active connection and >> re-executes accept(). >> >> The problem arises if the new client closes the connection before the >> older one is finished. When qmp runs accept() again, it will get a >> socket fd for a client who has now closed the connection. At this point, >> the monitor control event handler will get triggered and try to write / >> flush the greeting message to the client. The client however has closed >> its socket so the write will fail with SIGPIPE / EPIPE. Neither the >> qemu-char nor the monitor code seem to handle this situation. >> >> Btw, have you tried to reproduce it? > Not yet, I may have some time tomorrow. How reproducible is it for you? We can trigger it (by following the steps described in the first mail) consistently. > Another question: have you tried to reproduce with an old qemu version > (say v1.0) to see if this bug always existed? If the bug was introduced > in some recent QEMU version you could try to bisect it. v1.1 is not affected. I checked the code and it seems the monitor code has been refactored since v1.1. > Maybe you could try to reproduce with a different subsystem so that we > can rule out or confirm monitor's involvement? Like -serial? It's actually a fault of the monitor_flush() function. As far as I can understand, monitor_flush() calls qemu_chr_fe_write() and doesn't handle all of the return codes / error cases properly (as I described in a previous mail). If you check the function, you'll see that the final case (where it set ups a watch / callback) always assumes an EAGAIN / EWOULDBLOCK error. If you can verify / confirm that this is the case and that the patch sent resolves the issue in a sane / correct way, I'll resubmit it properly (with git-format-patch, a git log msg etc). Thanks, Stratos > >> Thanks, >> Stratos >> 5) Client B does *NOT* get any greating message 6) Client B waits (select on the socket's fd) 7) Client B closes connection (kill socat) 8) Client A quits too 9) Client C connects to qmp socket 10) Client C gets *two* greeting messages!!! >>> Hi Stratos, thank you for debugging and reporting this. >>> >>> I tested this sequence but can't fully reproduce this. What I see is 5) >>> but no >>> 10). Client C acts normally. And your patch below doesn't solve it for >>> me. >> Hm, which qemu version (or repo branch / tag) did you use? We did a >> quick scan of the master branch code / commits, but we didn't find >> anything that might fix the issue. > I tried on qemu.git master, and also 1.7. I think it is a bug: in my > test, step > 5), B not getting any greeting message. > > But I get only one greeting message in step 10), which is a bi
[Qemu-devel] [PATCH] target-i386: Fix I/O bitmap checks for in/out
Commit 1b90d56e changed the implementation of in/out imm to not assign the accessed port number to cpu_T[0] as it appeared unnecessary. However, currently gen_check_io() makes use of cpu_T[0] to implement the I/O bitmap checks, so it's in fact still used and the change broke the check, leading to #GP in legitimate cases (and probably also allowing access to ports that shouldn't be allowed). This patch reintroduces the missing assignment for these cases. Signed-off-by: Kevin Wolf Reviewed-by: Richard Henderson --- target-i386/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-i386/translate.c b/target-i386/translate.c index b0f2279..5dd2450 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -6284,6 +6284,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, case 0xe5: ot = mo_b_d32(b, dflag); val = cpu_ldub_code(env, s->pc++); +tcg_gen_movi_tl(cpu_T[0], val); gen_check_io(s, ot, pc_start - s->cs_base, SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes)); if (use_icount) @@ -6300,6 +6301,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, case 0xe7: ot = mo_b_d32(b, dflag); val = cpu_ldub_code(env, s->pc++); +tcg_gen_movi_tl(cpu_T[0], val); gen_check_io(s, ot, pc_start - s->cs_base, svm_is_rep(prefixes)); gen_op_mov_v_reg(ot, cpu_T[1], R_EAX); -- 1.8.1.4
Re: [Qemu-devel] [PATCH] osdep: drop unused #include "trace.h"
24.01.2014 00:27, Stefan Hajnoczi wrote: > osdep.c does not use trace_*() so we can just drop the include. Thanks, applied to the trivial-patches queue. /mjt
Re: [Qemu-devel] [PATCH v4 4/5] qmp: full introspection support for QMP
On Thu, 01/23 22:46, Amos Kong wrote: > This patch introduces a new monitor command to query QMP schema > information, the return data is a range of schema structs, which > contains the useful metadata to help management to check supported > features, QMP commands detail, etc. > > We use qapi-introspect.py to parse all json definition in > qapi-schema.json, and generate a range of dictionaries with metadata. > The query command will visit the dictionaries and fill the data > to allocated struct tree. Then QMP infrastructure will convert > the tree to json string and return to QMP client. > > TODO: > Wenchao Xia is working to convert QMP events to qapi-schema.json, > then event can also be queried by this interface. > > I will introduce another command 'query-qga-schema' to query QGA > schema information, it's easy to add this support based on this > patch. > > Signed-off-by: Amos Kong > --- > qapi-schema.json | 11 +++ > qmp-commands.hx | 42 +++ > qmp.c| 215 > +++ > 3 files changed, 268 insertions(+) > > diff --git a/qapi-schema.json b/qapi-schema.json > index c63f0ca..6033383 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4411,3 +4411,14 @@ > 'reference-type': 'String', > 'type': 'DataObjectType', > 'unionobj': 'DataObjectUnion' } } > + > +## > +# @query-qmp-schema > +# > +# Query QMP schema information > +# > +# @returns: list of @DataObject > +# > +# Since: 1.8 > +## > +{ 'command': 'query-qmp-schema', 'returns': ['DataObject'] } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 02cc815..b83762d 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3291,6 +3291,48 @@ Example: > } > > EQMP > +{ > +.name = "query-qmp-schema", > +.args_type = "", > +.mhandler.cmd_new = qmp_marshal_input_query_qmp_schema, > +}, > + > + > +SQMP > +query-qmp-schema > + > + > +query qmp schema information > + > +Return a json-object with the following information: > + > +- "name": qmp schema name (json-string) > +- "type": qmp schema type, it can be 'comand', 'type', 'enum', 'union' > +- "returns": return data of qmp command (json-object, optional) > + > +Example: > + > +-> { "execute": "query-qmp-schema" } > +-> { "return": [ > + { > + "name": "query-name", > + "type": "command", > + "returns": { > + "name": "NameInfo", > + "type": "type", > + "data": [ > + { > + "name": "name", > + "optional": true, > + "recursive": false, > + "type": "str" > + } > + ] > + } > + } > + } > + > +EQMP > > { > .name = "blockdev-add", > diff --git a/qmp.c b/qmp.c > index 0f46171..a64ae6d 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -27,6 +27,8 @@ > #include "qapi/qmp/qobject.h" > #include "qapi/qmp-input-visitor.h" > #include "hw/boards.h" > +#include "qapi/qmp/qjson.h" > +#include "qapi-introspect.h" > > NameInfo *qmp_query_name(Error **errp) > { > @@ -488,6 +490,219 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error > **errp) > return arch_query_cpu_definitions(errp); > } > > +static strList *qobject_to_strlist(QObject *data) > +{ > +strList *list = NULL; > +strList **plist = &list; > +QList *qlist; > +const QListEntry *lent; > + > +qlist = qobject_to_qlist(data); > +for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { > +strList *entry = g_malloc0(sizeof(strList)); > +entry->value = g_strdup(qobject_get_str(lent->value)); > +*plist = entry; > +plist = &entry->next; > +} > + > +return list; > +} > + > +static DataObject *qobject_to_dataobj(QObject *data); > + > +static DataObjectMember *qobject_to_dataobjmem(QObject *data) > +{ > + > +DataObjectMember *member = g_malloc0(sizeof(DataObjectMember)); > + > +member->type = g_malloc0(sizeof(DataObjectMemberType)); > +if (data->type->code == QTYPE_QDICT) { > +member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_EXTEND; > +member->type->extend = qobject_to_dataobj(data); > +} else { > +member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE; > +member->type->reference = g_strdup(qobject_get_str(data)); > +} > + > +return member; > +} > + > +static DataObjectMemberList *qobject_to_dict_memlist(QObject *data) > +{ > +DataObjectMemberList *list = NULL; > +DataObjectMemberList **plist = &list; > +QDict *qdict = qobject_to_qdict(data); > +const QDictEntry *dent; > + > +for (dent = qdict_first(qdict); dent; dent = qdict_next(qdict, dent)) { > +DataObjectMemberList *entry = > g_malloc0(sizeof(DataObjectMemberList)); > +entry->value = qobject_to_dataobjmem(dent->value); > + > +entry->value->has_optional = true
Re: [Qemu-devel] [PATCH v2] qapi: Add "backing" to BlockStats
Am 23.01.2014 um 03:03 hat Fam Zheng geschrieben: > Currently there is no way to query BlockStats of the backing chain. This > adds "backing" field into BlockStats to make it possible. > > The comment of "parent" is reworded. > > Signed-off-by: Fam Zheng Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory'
On 01/23/14 16:17, Ekaterina Tumanova wrote: > On 01/17/2014 11:46 AM, qiaonuohan wrote: >> Make monitor command 'dump-guest-memory' be able to dump in >> kdump-compressed >> format. The command's usage: >> >>dump [-p] protocol [begin] [length] [format] >> >> 'format' is used to specified the format of vmcore and can be: >> 1. 'elf': ELF format, without compression >> 2. 'kdump-zlib': kdump-compressed format, with zlib-compressed >> 3. 'kdump-lzo': kdump-compressed format, with lzo-compressed >> 4. 'kdump-snappy': kdump-compressed format, with snappy-compressed >> Without 'format' being set, it is same as 'elf'. And if non-elf format is >> specified, paging and filter is not allowed. >> >> Note: >>1. The kdump-compressed format is readable only with the crash >> utility and >> makedumpfile, and it can be smaller than the ELF format because >> of the >> compression support. >>2. The kdump-compressed format is the 6th edition. >> >> Signed-off-by: Qiao Nuohan >> --- >> dump.c | 129 >> +++--- >> hmp.c|5 ++- >> qapi-schema.json | 25 ++- >> qmp-commands.hx |7 ++- >> 4 files changed, 156 insertions(+), 10 deletions(-) >> >> diff --git a/dump.c b/dump.c >> index bb03ef7..dbf4bb6 100644 >> --- a/dump.c >> +++ b/dump.c >> @@ -1449,6 +1449,64 @@ out: >> return ret; >> } >> >> +static int create_kdump_vmcore(DumpState *s) >> +{ >> +int ret; >> + >> +/* >> + * the kdump-compressed format is: >> + * File offset >> + * +--+ 0x0 >> + * |main header (struct disk_dump_header) | >> + * |--+ block 1 >> + * |sub header (struct kdump_sub_header) | >> + * |--+ block 2 >> + * |1st-dump_bitmap | >> + * |--+ block 2 + X blocks >> + * |2nd-dump_bitmap | (aligned by block) >> + * |--+ block 2 + 2 * X >> blocks >> + * | page desc for pfn 0 (struct page_desc) | (aligned by block) >> + * | page desc for pfn 1 (struct page_desc) | >> + * |: | >> + * |--| (not aligned by >> block) >> + * | page data (pfn 0)| >> + * | page data (pfn 1)| >> + * |: | >> + * +--+ >> + */ >> + >> +ret = write_start_flat_header(s->fd); >> +if (ret < 0) { >> +dump_error(s, "dump: failed to write start flat header.\n"); >> +return -1; >> +} >> + >> +ret = write_dump_header(s); >> +if (ret < 0) { >> +return -1; >> +} >> + >> +ret = write_dump_bitmap(s); >> +if (ret < 0) { >> +return -1; >> +} >> + >> +ret = write_dump_pages(s); >> +if (ret < 0) { >> +return -1; >> +} >> + >> +ret = write_end_flat_header(s->fd); >> +if (ret < 0) { >> +dump_error(s, "dump: failed to write end flat header.\n"); >> +return -1; >> +} >> + >> +dump_completed(s); >> + >> +return 0; >> +} >> + >> static ram_addr_t get_start_block(DumpState *s) >> { >> GuestPhysBlock *block; >> @@ -1487,7 +1545,8 @@ static void get_max_mapnr(DumpState *s) >> } >> } >> >> -static int dump_init(DumpState *s, int fd, bool paging, bool has_filter, >> +static int dump_init(DumpState *s, int fd, bool has_format, >> + DumpGuestMemoryFormat format, bool paging, bool >> has_filter, >>int64_t begin, int64_t length, Error **errp) >> { >> CPUState *cpu; >> @@ -1495,6 +1554,11 @@ static int dump_init(DumpState *s, int fd, bool >> paging, bool has_filter, >> Error *err = NULL; >> int ret; >> >> +/* kdump-compressed is conflict with paging and filter */ >> +if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) { >> +assert(!paging && !has_filter); >> +} >> + >> if (runstate_is_running()) { >> vm_stop(RUN_STATE_SAVE_VM); >> s->resume = true; >> @@ -1565,6 +1629,28 @@ static int dump_init(DumpState *s, int fd, bool >> paging, bool has_filter, >> tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), >> s->page_size); >> s->len_dump_bitmap = tmp * s->page_size; >> >> +/* init for kdump-compressed format */ >> +if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) { >> +switch (format) { >> +case DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB: >> +s->flag_compress = DUMP_DH_COMPRESSED_ZLIB; >> +break; >> + >> +case DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO: >> +s-
Re: [Qemu-devel] [PATCH v4 1/3] block: resize backing file image during offline commit, if necessary
Am 23.01.2014 um 23:45 hat Jeff Cody geschrieben: > Currently, if an image file is logically larger than its backing file, > committing it via 'qemu-img commit' will fail. > > For instance, if we have a base image with a virtual size 10G, and a > snapshot image of size 20G, then committing the snapshot offline with > 'qemu-img commit' will likely fail. > > This will automatically attempt to resize the base image, if the > snapshot image to be committed is larger. > > Signed-off-by: Jeff Cody > Reviewed-by: Fam Zheng > Reviewed-by: Eric Blake > Reviewed-by: Benoit Canet > --- > block.c | 23 --- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/block.c b/block.c > index 64e7d22..dea7591 100644 > --- a/block.c > +++ b/block.c > @@ -1876,10 +1876,10 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult > *res, BdrvCheckMode fix) > int bdrv_commit(BlockDriverState *bs) > { > BlockDriver *drv = bs->drv; > -int64_t sector, total_sectors; > +int64_t sector, total_sectors, length, backing_length; > int n, ro, open_flags; > int ret = 0; > -uint8_t *buf; > +uint8_t *buf = NULL; > char filename[PATH_MAX]; > > if (!drv) > @@ -1904,7 +1904,24 @@ int bdrv_commit(BlockDriverState *bs) > } > } > > -total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS; > +length = bdrv_getlength(bs); > +backing_length = bdrv_getlength(bs->backing_hd); > + > +if (length < 0 || backing_length < 0) { > +goto ro_cleanup; > +} ret is initialised to 0, so this returns success despite not having committed anything to the backing file. Kevin
Re: [Qemu-devel] [PATCH v4 5/5] update docs/qmp-full-introspection.txt
Il 23/01/2014 15:46, Amos Kong ha scritto: +The whole schema information will be returned in one go, it contains +all the schema entries. It doesn't support to be filtered by type +or name. Currently it takes about 4 seconds to return about 1.7M string. +Management only needs to execute this command once after installing +QEMU package. + This is quite a lot. Without comments, qapi-schema.json is 27k. I'd expect the DataObject representation to be in the ballpark of 100-200k. I don't understand why is it necessary to expand all the types in the result? Paolo
Re: [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory'
Thanks for addressing my earlier comments. Some new ones below: On 01/17/14 08:46, qiaonuohan wrote: > +/* check whether lzo/snappy is supported */ > +#ifndef CONFIG_LZO > +if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) { > +error_setg(errp, "kdump-lzo is not available now"); > +} > +#endif > + > +#ifndef CONFIG_SNAPPY > +if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) { > +error_setg(errp, "kdump-snappy is not available now"); > +} > +#endif Ekaterina caught in testing that these two blocks must be complemented with a return statement each -- when you detect these errors, qmp_dump_guest_memory() must not proceed. Apologies for not noticing them in v6. > +## > # @dump-guest-memory > # > # Dump guest's memory to vmcore. It is a synchronous operation that can take > @@ -2712,13 +2730,18 @@ > # want to dump all guest's memory, please specify the start @begin > # and @length > # > +# @format: #optional if specified, the format of guest memory dump. But > non-elf > +# format is conflict with paging and filter, ie. @paging, @begin and > +# @length is not allowed to be specified with @format at the same > time Please make it more precise with "non-elf", as in: ... are not allowed to be specified with *non-elf* @format at the same time Not really relevant but since you'll respin anyway... :) > +# (since 2.0) > +# > # Returns: nothing on success > # > # Since: 1.2 > ## > { 'command': 'dump-guest-memory', >'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int', > -'*length': 'int' } } > +'*length': 'int', '*format': 'DumpGuestMemoryFormat' } } > > ## > # @netdev_add: > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 02cc815..9158f22 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -791,8 +791,8 @@ EQMP > > { > .name = "dump-guest-memory", > -.args_type = "paging:b,protocol:s,begin:i?,end:i?", > -.params = "-p protocol [begin] [length]", > +.args_type = "paging:b,protocol:s,begin:i?,end:i?,format:s?", > +.params = "-p protocol [begin] [length] [format]", > .help = "dump guest memory to file", > .user_print = monitor_user_noop, > .mhandler.cmd_new = qmp_marshal_input_dump_guest_memory, > @@ -813,6 +813,9 @@ Arguments: > with length together (json-int) > - "length": the memory size, in bytes. It's optional, and should be specified > with begin together (json-int) > +- "format": the format of guest memory dump. It's optional, and can be > +elf|kdump-zlib|kdump-lzo|kdump-snappy, but non-elf formats will > +conflict with paging and filter, ie. begin and > length(json-string) Please add a space between "length" and "(json-string)". Thank you! Laszlo
[Qemu-devel] Strange monitor behavior related to hotplug commands
Hi, After having heavily used device_add/netdev_add lately with QEMU 1.7, we encountered the following strange behavior: 1) Get network/PCI configuration for a VM # echo info network | socat STDIO UNIX-CONNECT:/path/to/monitor/socket QEMU 1.7.0 monitor - type 'help' for more information (qemu) info network hotnic-42932f20-pci-5: index=0,type=nic,model=virtio-net-pci,macaddr=aa:00:08:28:1b:3c \ hotnic-42932f20-pci-5: index=0,type=tap,fd=8 hotnic-ab323482-pci-6: index=0,type=nic,model=virtio-net-pci,macaddr=aa:0c:f8:9d:d9:52 \ hotnic-ab323482-pci-6: index=0,type=tap,fd=9 (qemu) # echo info pci | socat STDIO UNIX-CONNECT:/path/to/monitor/socket QEMU 1.7.0 monitor - type 'help' for more information (qemu) info pci Bus 0, device 0, function 0: Host bridge: PCI device 8086:1237 id "" Bus 0, device 1, function 0: ISA bridge: PCI device 8086:7000 id "" Bus 0, device 1, function 1: IDE controller: PCI device 8086:7010 BAR4: I/O at 0xc0c0 [0xc0cf]. id "" Bus 0, device 1, function 2: USB controller: PCI device 8086:7020 IRQ 11. BAR4: I/O at 0xc0a0 [0xc0bf]. id "" Bus 0, device 1, function 3: Bridge: PCI device 8086:7113 IRQ 9. id "" Bus 0, device 2, function 0: VGA controller: PCI device 1013:00b8 BAR0: 32 bit prefetchable memory at 0xfc00 [0xfdff]. BAR1: 32 bit memory at 0xfebf3000 [0xfebf3fff]. BAR6: 32 bit memory at 0x [0xfffe]. id "" Bus 0, device 3, function 0: Class 0255: PCI device 1af4:1002 IRQ 11. BAR0: I/O at 0xc080 [0xc09f]. id "" Bus 0, device 4, function 0: SCSI controller: PCI device 1af4:1001 IRQ 11. BAR0: I/O at 0xc000 [0xc03f]. BAR1: 32 bit memory at 0xfebf2000 [0xfebf2fff]. id "hotdisk-1d455fed-pci-4" Bus 0, device 5, function 0: Ethernet controller: PCI device 1af4:1000 IRQ 10. BAR0: I/O at 0xc060 [0xc07f]. BAR1: 32 bit memory at 0xfebf1000 [0xfebf1fff]. BAR6: 32 bit memory at 0x [0xfffe]. id "hotnic-42932f20-pci-5" Bus 0, device 6, function 0: Ethernet controller: PCI device 1af4:1000 IRQ 10. BAR0: I/O at 0xc040 [0xc05f]. BAR1: 32 bit memory at 0xfebf [0xfebf0fff]. BAR6: 32 bit memory at 0x [0xfffe]. id "hotnic-ab323482-pci-6" (qemu) 2) Monitor states that it has two Ethernet controllers (devices at PCI 5 and 6) with the corresponding netdev. 3) Try to remove the one on PCI 6 with id hotnic-ab323482-pci-6 # echo device_del hotnic-ab323482-pci-6 | socat STDIO UNIX-CONNECT:/path/to/monitor/socket/ QEMU 1.7.0 monitor - type 'help' for more information (qemu) device_del hotnic-ab323482-pci-6 (qemu) 4) Monitor output points out that `device_del` succeeded. 5) *Still* info pci/network returns the same as above! 6) Try to remove the corresponding netdev: # echo netdev_del hotnic-ab323482-pci-6 | socat STDIO UNIX-CONNECT:/path/to/monitor/socket QEMU 1.7.0 monitor - type 'help' for more information (qemu) netdev_del hotnic-ab323482-pci-6 Device 'hotnic-ab323482-pci-6' not found (qemu) 7) Monitor output points out that `netdev_del` has failed because the corresponding device (not netdev?) ID was not found! 8) *Still* info pci/network returns the same as above! Any ideas why this is happening? Is there another way to get further debug info or even another monitor command to work this around? Random thought: Is there any chance that a previous back-to-back netdev_add/device_add command has actually failed (due to a race or something) but the corresponding configuration not properly updated? Thanks in advance, dimara PS: Has commit 03060d on 1.6.2 anything to do with the aforementioned? qdev-monitor: Unref device when device_add fails qdev_device_add() leaks the created device upon failure. I suspect this problem crept in because qdev_free() unparents the device but does not drop a reference - confusing name. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH v2] cpu: implementing victim TLB for QEMUsystem emulated TLBB
trent.t...@gmail.com writes: > Attaching data in excel which could not be sent with the patch at the same > time. > If you can attach the summary of the data as plain text that would be useful. Not all of us have access to a Windows box with Excell! -- Alex Bennée
Re: [Qemu-devel] [PATCH v11 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD
Am 24.01.2014 um 11:10 hat Fam Zheng geschrieben: > Ping? This doesn't apply on top of the block branch any more. Can you rebase, please? Kevin > On Wed, 01/08 18:07, Fam Zheng wrote: > > This series adds for point-in-time snapshot NBD exporting based on > > blockdev-backup (variant of drive-backup with existing device as target). > > > > We get a thin point-in-time snapshot by COW mechanism of drive-backup, and > > export it through built in NBD server. The steps are as below: > > > > 1. (SHELL) qemu-img create -f qcow2 BACKUP.qcow2 > > > > (Alternatively we can use -o backing_file=RUNNING-VM.img to omit > > explicitly > > providing the size by ourselves, but it's risky because > > RUNNING-VM.qcow2 is > > used r/w by guest. Whether or not setting backing file in the image file > > doesn't matter, as we are going to override the backing hd in the next > > step) > > > > 2. (QMP) blockdev-add backing=source-drive file.driver=file > > file.filename=BACKUP.qcow2 id=target0 if=none driver=qcow2 > > > > (where source-drive is the running BlockDriverState name for > > RUNNING-VM.img. This patch implements "backing=" option to override > > backing_hd for added drive) > > > > 3. (QMP) blockdev-backup device=source-drive sync=none target=target0 > > > > (this is the QMP command introduced by this series, which use a named > > device as target of drive-backup) > > > > 4. (QMP) nbd-server-add device=target0 > > > > When image fleecing done: > > > > 1. (QMP) block-job-cancel device=source-drive > > > > 2. (HMP) drive_del target0 > > > > 3. (SHELL) rm BACKUP.qcow2 > > > > v8 -> v11: Rebased to qemu.git. Address Stefan's comments: > > (sorry for sending 3 revisions in a day) > > > > [01/11] block: Add BlockOpType enum > > Change enum definition as internal. > > > > [05/11] block: Add bdrv_set_backing_hd() > > Set bs->backing_file and bs->backing_format. > > > > [06/11] block: Add backing_blocker in BlockDriverState > > Reuse bdrv_set_backing_hd(). > > > > [07/11] block: Parse "backing" option to reference existing BDS > > Update commit message about bdrv_swap assertion removal. > > Fix use-after-free. > > Check for "backing=" and "backing.file=" conflict. > > > > [08/11] block: Support dropping active in bdrv_drop_intermediate > > Fix function comment. > > > > [09/11] stream: Use bdrv_drop_intermediate and drop close_unused_images > > > > Fam Zheng (11): > > block: Add BlockOpType enum > > block: Introduce op_blockers to BlockDriverState > > block: Replace in_use with operation blocker > > block: Move op_blocker check from block_job_create to its caller > > block: Add bdrv_set_backing_hd() > > block: Add backing_blocker in BlockDriverState > > block: Parse "backing" option to reference existing BDS > > block: Support dropping active in bdrv_drop_intermediate > > stream: Use bdrv_drop_intermediate and drop close_unused_images > > qmp: Add command 'blockdev-backup' > > block: Allow backup on referenced named BlockDriverState > > > > block-migration.c | 7 +- > > block.c | 304 > > +++- > > block/backup.c | 21 +++ > > block/commit.c | 1 + > > block/stream.c | 28 +--- > > blockdev.c | 70 +++-- > > blockjob.c | 14 +- > > hw/block/dataplane/virtio-blk.c | 19 ++- > > include/block/block.h | 29 +++- > > include/block/block_int.h | 9 +- > > include/block/blockjob.h| 3 + > > qapi-schema.json| 49 +++ > > qmp-commands.hx | 44 ++ > > 13 files changed, 444 insertions(+), 154 deletions(-) > > > > -- > > 1.8.5.1 > > > >
Re: [Qemu-devel] [PATCH 13/13 v7] dump: add 'query-dump-guest-memory-capability' command
two comments below On 01/17/14 08:46, qiaonuohan wrote: > 'query-dump-guest-memory-capability' is used to query whether option 'format' > is available for 'dump-guest-memory' and the available format. The output > of the command will be like: > > -> { "execute": "query-dump-guest-memory-capability" } > <- { "return": { > "format-option": "optional", > "capabilities": [ > {"available": true, "format": "elf"}, > {"available": true, "format": "kdump-zlib"}, > {"available": true, "format": "kdump-lzo"}, > {"available": true, "format": "kdump-snappy"} > ] > } > > Signed-off-by: Qiao Nuohan > --- > dump.c | 42 ++ > qapi-schema.json | 13 + > qmp-commands.hx | 31 +++ > 3 files changed, 86 insertions(+), 0 deletions(-) > > diff --git a/dump.c b/dump.c > index dbf4bb6..c288cd3 100644 > --- a/dump.c > +++ b/dump.c > @@ -1794,3 +1794,45 @@ void qmp_dump_guest_memory(bool paging, const char > *file, bool has_begin, > > g_free(s); > } > + > +DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error > **errp) > +{ > +int i; > +DumpGuestMemoryCapabilityStatusList *item; > +DumpGuestMemoryCapability *cap = > + > g_malloc0(sizeof(DumpGuestMemoryCapability)); > + > +cap->format_option = g_strdup_printf("optional"); > + > +for (i = 0; i < DUMP_GUEST_MEMORY_FORMAT_MAX; i++) { > +if (cap->capabilities == NULL) { > +item = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList)); > +cap->capabilities = item; > +} else { > +item->next = > g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList)); > +item = item->next; > +} > + > +item->value = g_malloc0(sizeof(struct > DumpGuestMemoryCapabilityStatus)); > +item->value->format = i; > + > +if (i == DUMP_GUEST_MEMORY_FORMAT_ELF || i == > +DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB) { Line wrapping hiccup. > +item->value->available = true; > +} > + > +#ifdef CONFIG_LZO > +if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) { > +item->value->available = true; > +} > +#endif > + > +#ifdef CONFIG_SNAPPY > +if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) { > +item->value->available = true; > +} > +#endif > +} > + > +return cap; > +} > diff --git a/qapi-schema.json b/qapi-schema.json > index 52df894..3085294 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -2744,6 +2744,19 @@ > '*length': 'int', '*format': 'DumpGuestMemoryFormat' } } > > ## > +# Since: 2.0 > +## > +{ 'type': 'DumpGuestMemoryCapabilityStatus', > + 'data': { 'format': 'DumpGuestMemoryFormat', 'available': 'bool' } } > + > +{ 'type': 'DumpGuestMemoryCapability', > + 'data': { > + 'format-option': 'str', > + 'capabilities': ['DumpGuestMemoryCapabilityStatus'] } } > + > +{ 'command': 'query-dump-guest-memory-capability', 'returns': > 'DumpGuestMemoryCapability' } > + > +## > # @netdev_add: > # > # Add a network backend. > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 9158f22..ad51de0 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -829,6 +829,37 @@ Notes: > EQMP > > { > +.name = "query-dump-guest-memory-capability", > +.args_type = "", > +.mhandler.cmd_new = > qmp_marshal_input_query_dump_guest_memory_capability, > +}, > + > +SQMP > +query-dump-guest-memory-capability > +-- > + > +Show whether option 'format' is available for 'dump-guest-memory' and the > +available format. > + > +Example: > + > +-> { "execute": "query-dump-guest-memory-capability" } > +<- { "return": { > +"format-option": "optional", > +"capabilities": [ > +{"available": true, "format": "elf"}, > +{"available": true, "format": "kdump-zlib"}, > +{"available": true, "format": "kdump-lzo"}, > +{"available": true, "format": "kdump-snappy"} > +] > +} > + > +Note: This is a light-weight introspection to let management know whether > format > + option is available and the supported compression format. > + > +EQMP > + > +{ > .name = "netdev_add", > .args_type = "netdev:O", > .mhandler.cmd_new = qmp_netdev_add, > Although in general I don't like to obsess about English grammar, this should say available / supported "formats", plural (two instances in the above text). I'm only proposing these because you'll respin anyway, but I don't insist. If you change nothing else in this patch, then you can add my Reviewed-by: Laszlo Ersek Thanks! Laszlo
[Qemu-devel] Native MinGW build crashes when partitioning hard disk in MS-DOS guest
I am building the official Qemu 1.7.0 release from source natively under MinGW for myself. I execute the i386 target. Qemu starts fine, but it crashes when I try to partition a 2G hard disk in an MS-DOS 6.22 guest. Details of the build and reproducing the crash are below. The problem also occurs for me with the 1.6.2 release. The problem does not occur with these binaries from other people: Eric Lassauge's Qemu-1.6.0-windows.zip Prashant Satish's qemu-1.6.0-win32-sdl.tar.lzma Stefan Weil's qemu-w32-setup-20131128, qemu-w32-setup-20140118.exe The problem does not occur when I add the --enable-debug flag to configure! Whatever optimisation is removed as a result seems to avoid the crash. How can I troubleshoot what the problem is? What should I change about my build to solve the problem? Host: Windows 7 64-bit Build environment: (32-bit) MinGW fresh install including: mingw32-gettext bin 0.18.3.1-1 mingw32-gettext dev 0.18.3.1-1 mingw32-gettext dll 0.18.3.1-1 mingw32-libintl dll 0.18.3.1-1 mingw32-libz dev 1.2.8-1 mingw32-libz dll 1.2.8-1 MSYS shell fresh install Glib libraries and dependencies: glib_2.34.3-1_win32 glib-dev_2.34.3-1_win32 pkg-config_0.28-1_win32 pkg-config-dev_0.28-1_win32 SDL-devel-1.2.15-mingw32 Qemu 1.7.0 source is in /mingw/build/qemu-1.7.0 Build process: (out-of-tree build) using MSYS shell, in /mingw/build/qemu-1.7.0-obj directory: ../qemu-1.7.0/configure --python=C:/Python27/python --prefix=/mingw/build/qemu-1.7.0-bin --target-list="i386-softmmu" --disable-coroutine-pool make make install To reproduce crash: have a 1.44 floppy image of MS-DOS 6.22 boot disk place the disk image with name dos.img in /mingw/build/qemu-test directory using MSYS shell, in /mingw/build/qemu-test directory: ../qemu-1.7.0-bin/qemu-img create -f qcow2 test.img 2G ../qemu-1.7.0-bin/qemu-system-i386 -fda dos.img -hda test.img -boot a once DOS boots up, type fdisk and try to create a single partition using all available space Qemu crashes Thanks, Legorol
Re: [Qemu-devel] [PATCH v4 5/5] update docs/qmp-full-introspection.txt
On 01/24/2014 04:43 AM, Paolo Bonzini wrote: > Il 23/01/2014 15:46, Amos Kong ha scritto: >> +The whole schema information will be returned in one go, it contains >> +all the schema entries. It doesn't support to be filtered by type >> +or name. Currently it takes about 4 seconds to return about 1.7M string. >> +Management only needs to execute this command once after installing >> +QEMU package. >> + But management has to call the command for every variant of the qemu binary that it plans on driving - on a system with multiple qemu-* for targetting multiple target types, this starts to border on unacceptably long for libvirt. And while libvirt might use this command to learn about what is supported for a handful of specific commands, making libvirt store 1.7M + additional memory for its JSON parse of that data per qemu starts to add up fast, if libvirt were to keep that data around for the life of libvirtd rather than just learning what it needs from the string and discarding the string up front. We've just made an argument for WHY we need filtering to just a given type/command. > > This is quite a lot. > > Without comments, qapi-schema.json is 27k. I'd expect the DataObject > representation to be in the ballpark of 100-200k. > > I don't understand why is it necessary to expand all the types in the > result? Any time a type is returned at the top level in the same query (such as in your global query), management can look up any unexpanded type in the same result. I could understand expanding the results when returning only a subset of the tree (that is, when filtering is added, asking for a single type should give me all information about that type, including the subtypes it references, without me having to make multiple calls to learn about the subtypes), but even then, it might be worth having an optional boolean flag on the query that says whether I want expansion vs. compact results. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] block: Fix bdrv_commit return value
bdrv_commit() could return 0 or 1 on success, depending on whether or now the last sector was allocated in the overlay and whether the overlay format had a .bdrv_make_empty callback. Most callers ignored it, but qemu-img commit would print an error message while the operation actually succeeded. Also clean up the handling of I/O errors to return the real error code instead of -EIO. Signed-off-by: Kevin Wolf --- block.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 3e0994b..7d22ca9 100644 --- a/block.c +++ b/block.c @@ -2061,13 +2061,13 @@ int bdrv_commit(BlockDriverState *bs) goto ro_cleanup; } if (ret) { -if (bdrv_read(bs, sector, buf, n) != 0) { -ret = -EIO; +ret = bdrv_read(bs, sector, buf, n); +if (ret < 0) { goto ro_cleanup; } -if (bdrv_write(bs->backing_hd, sector, buf, n) != 0) { -ret = -EIO; +ret = bdrv_write(bs->backing_hd, sector, buf, n); +if (ret < 0) { goto ro_cleanup; } } @@ -2075,6 +2075,9 @@ int bdrv_commit(BlockDriverState *bs) if (drv->bdrv_make_empty) { ret = drv->bdrv_make_empty(bs); +if (ret < 0) { +goto ro_cleanup; +} bdrv_flush(bs); } @@ -2082,9 +2085,11 @@ int bdrv_commit(BlockDriverState *bs) * Make sure all data we wrote to the backing device is actually * stable on disk. */ -if (bs->backing_hd) +if (bs->backing_hd) { bdrv_flush(bs->backing_hd); +} +ret = 0; ro_cleanup: g_free(buf); -- 1.8.1.4
Re: [Qemu-devel] [PATCH V6 8/8] block: Use graph node name as reference in bdrv_file_open().
Am 23.01.2014 um 21:31 hat Benoît Canet geschrieben: > Signed-off-by: Benoit Canet > --- > block.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) I'm not going to merge this one yet. It breaks qemu-iotests case 071, which would have to be adapted. However, first of all I'd like to hear the opinions of at least Eric and Max on what BlockRef should really refer to. I think node names make most sense, but perhaps it's a bit inconvenient and the command line should default to node-name = id when id is set, but node-name isn't? Kevin > diff --git a/block.c b/block.c > index 3e0994b..7726636 100644 > --- a/block.c > +++ b/block.c > @@ -908,15 +908,15 @@ int bdrv_file_open(BlockDriverState **pbs, const char > *filename, > > if (reference) { > if (filename || qdict_size(options)) { > -error_setg(errp, "Cannot reference an existing block device with > " > +error_setg(errp, "Cannot reference an existing graph node with " > "additional options or a new filename"); > return -EINVAL; > } > QDECREF(options); > > -bs = bdrv_find(reference); > +bs = bdrv_find_node(reference); > if (!bs) { > -error_setg(errp, "Cannot find block device '%s'", reference); > +error_setg(errp, "Cannot find graph node '%s'", reference); > return -ENODEV; > } > bdrv_ref(bs); > -- > 1.8.3.2 >
Re: [Qemu-devel] [PATCH V6 0/8] Giving names to graph's BlockDriverState
Am 23.01.2014 um 21:31 hat Benoît Canet geschrieben: > The following series introduce a new file.node-name property in order to be > able to give a name to each BlockDriverState of the graph. > > since v5: > rename to bdrv_assign_node_name [Fam] > add node-name to BlockdevOptionsBase [Kevin] > add query-named-block-nodes name in commit message [Fam] > make bdrv_lookup_bs interface more generic [Kevin] > s/check/checks/ [Fam] > s/implentation/implementation/ [Fam] > use node names for references [Kevin] > > > > Benoît Canet (8): > block: Add bs->node_name to hold the name of a bs node of the bs > graph. > block: Allow the user to define "node-name" option both on command > line and QMP. > qmp: Add QMP query-named-block-nodes to list the named > BlockDriverState nodes. > qmp: Allow to change password on named block driver states. > block: Create authorizations mechanism for external snapshot and > resize. > qmp: Allow block_resize to manipulate bs graph nodes. > qmp: Allow to take external snapshots on bs graphs node. > block: Use graph node name as reference in bdrv_file_open(). > > block.c | 214 > -- > block/blkverify.c | 2 +- > block/qapi.c | 109 +++ > blockdev.c| 93 > hmp.c | 8 +- > include/block/block.h | 23 +++-- > include/block/block_int.h | 21 - > include/block/qapi.h | 1 + > qapi-schema.json | 50 +-- > qmp-commands.hx | 78 - > 10 files changed, 474 insertions(+), 125 deletions(-) Thanks, applied patches 1-7 to the block branch. Kevin
Re: [Qemu-devel] [PATCH] block: do not allow read-only=on and snapshot=on to be used together
Am 14.01.2014 um 20:12 hat Jeff Cody geschrieben: > Having both read-only=on and snapshot=on together does not make sense; > currently, the read-only argument is effectively ignored for the > temporary snapshot. To prevent confusion, disallow the usage of both > 'snapshot=on' and 'read-only=on'. > > Signed-off-by: Jeff Cody This breaks in a surprising way: $ softmmu/qemu-system-x86_64 -drive file=~/images/hd.img,snapshot=on ... works fine ... $ qemu-system-x86_64 -drive file=~/images/hd.img -snapshot qemu-system-x86_64: invalid option combination: read-only and snapshot qemu-iotests case 051 catches this. I'll have to remove this patch and the follow-up from the queue for now. Kevin
Re: [Qemu-devel] [PATCH V6 8/8] block: Use graph node name as reference in bdrv_file_open().
On 24.01.2014 14:26, Kevin Wolf wrote: Am 23.01.2014 um 21:31 hat Benoît Canet geschrieben: Signed-off-by: Benoit Canet --- block.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) I'm not going to merge this one yet. It breaks qemu-iotests case 071, which would have to be adapted. However, first of all I'd like to hear the opinions of at least Eric and Max on what BlockRef should really refer to. I think node names make most sense, but perhaps it's a bit inconvenient and the command line should default to node-name = id when id is set, but node-name isn't? The QAPI schema is pretty clear about this: “references the ID of an existing block device.” However, if the ID cannot be found, I think we should interpret it as a reference to the node name. Therefore, I'd first try bdrv_find() and if that returns NULL, try again with bdrv_find_node(). Max
Re: [Qemu-devel] [PATCH] block: do not allow read-only=on and snapshot=on to be used together
On Fri, Jan 24, 2014 at 02:33:19PM +0100, Kevin Wolf wrote: > Am 14.01.2014 um 20:12 hat Jeff Cody geschrieben: > > Having both read-only=on and snapshot=on together does not make sense; > > currently, the read-only argument is effectively ignored for the > > temporary snapshot. To prevent confusion, disallow the usage of both > > 'snapshot=on' and 'read-only=on'. > > > > Signed-off-by: Jeff Cody > > This breaks in a surprising way: > > $ softmmu/qemu-system-x86_64 -drive file=~/images/hd.img,snapshot=on > ... works fine ... > > $ qemu-system-x86_64 -drive file=~/images/hd.img -snapshot > qemu-system-x86_64: invalid option combination: read-only and snapshot > > qemu-iotests case 051 catches this. I'll have to remove this patch and > the follow-up from the queue for now. > Odd - OK, I'll follow up on this and submit a series with both patches (well, likely squashed together), and whatever is needed to fix this as well.
[Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface
From: Peter Maydell libcurl versions 7.16.0 and later have a timer callback interface which must be implemented in order for libcurl to make forward progress (it will sometimes rely on being called back on the timeout if there are no file descriptors registered). Implement the callback, and use a QEMU AIO timer to ensure we prod libcurl again when it asks us to. Based on Peter's original patch plus my fix to add curl_multi_timeout_do. Should compile just fine even on older versions of libcurl. I also tried copy-on-read and streaming: $ ./qemu-img create -f qcow2 -o \ backing_file=http://download.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso \ foo.qcow2 1G $ x86_64-softmmu/qemu-system-x86_64 \ -drive if=none,file=foo.qcow2,copy-on-read=on,id=cd \ -device ide-cd,drive=cd --enable-kvm -m 1024 Direct http usage is probably too slow, but with copy-on-read ultimately the image does boot! After some time, streaming gets canceled by an EIO, which needs further investigation. Signed-off-by: Peter Maydell Signed-off-by: Paolo Bonzini --- block/curl.c | 81 +++- 1 file changed, 70 insertions(+), 11 deletions(-) diff --git a/block/curl.c b/block/curl.c index a603936..a807584 100644 --- a/block/curl.c +++ b/block/curl.c @@ -34,6 +34,11 @@ #define DPRINTF(fmt, ...) do { } while (0) #endif +#if LIBCURL_VERSION_NUM >= 0x071000 +/* The multi interface timer callback was introduced in 7.16.0 */ +#define NEED_CURL_TIMER_CALLBACK +#endif + #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \ CURLPROTO_FTP | CURLPROTO_FTPS | \ CURLPROTO_TFTP) @@ -77,6 +82,7 @@ typedef struct CURLState typedef struct BDRVCURLState { CURLM *multi; +QEMUTimer timer; size_t len; CURLState states[CURL_NUM_STATES]; char *url; @@ -87,6 +93,23 @@ typedef struct BDRVCURLState { static void curl_clean_state(CURLState *s); static void curl_multi_do(void *arg); +#ifdef NEED_CURL_TIMER_CALLBACK +static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque) +{ +BDRVCURLState *s = opaque; + +DPRINTF("CURL: timer callback timeout_ms %ld\n", timeout_ms); +if (timeout_ms == -1) { +timer_del(&s->timer); +} else { +int64_t timeout_ns = (int64_t)timeout_ms * 1000 * 1000; +timer_mod(&s->timer, + qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ns); +} +return 0; +} +#endif + static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action, void *s, void *sp) { @@ -209,20 +232,10 @@ static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len, return FIND_RET_NONE; } -static void curl_multi_do(void *arg) +static void curl_multi_read(BDRVCURLState *s) { -BDRVCURLState *s = (BDRVCURLState *)arg; -int running; -int r; int msgs_in_queue; -if (!s->multi) -return; - -do { -r = curl_multi_socket_all(s->multi, &running); -} while(r == CURLM_CALL_MULTI_PERFORM); - /* Try to find done transfers, so we can free the easy * handle again. */ do { @@ -266,6 +279,41 @@ static void curl_multi_do(void *arg) } while(msgs_in_queue); } +static void curl_multi_do(void *arg) +{ +BDRVCURLState *s = (BDRVCURLState *)arg; +int running; +int r; + +if (!s->multi) { +return; +} + +do { +r = curl_multi_socket_all(s->multi, &running); +} while(r == CURLM_CALL_MULTI_PERFORM); + +curl_multi_read(s); +} + +static void curl_multi_timeout_do(void *arg) +{ +#ifdef NEED_CURL_TIMER_CALLBACK +BDRVCURLState *s = (BDRVCURLState *)arg; +int running; + +if (!s->multi) { +return; +} + +curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running); + +curl_multi_read(s); +#else +abort(); +#endif +} + static CURLState *curl_init_state(BDRVCURLState *s) { CURLState *state = NULL; @@ -473,12 +521,20 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, curl_easy_cleanup(state->curl); state->curl = NULL; +aio_timer_init(bdrv_get_aio_context(bs), &s->timer, + QEMU_CLOCK_REALTIME, SCALE_NS, + curl_multi_timeout_do, s); + // Now we know the file exists and its size, so let's // initialize the multi interface! s->multi = curl_multi_init(); curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s); curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb); +#ifdef NEED_CURL_TIMER_CALLBACK +curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s); +curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb); +#endif curl_multi_do(s); qemu_opts_del(opts); @@ -597,6 +653,9 @@ static void curl_close(BlockDriverState *bs) } if (s->multi) curl_multi_cleanup(s->multi);
[Qemu-devel] [PATCH v5 3/3] block: update block commit documentation regarding image truncation
This updates the documentation for commiting snapshot images. Specifically, this highlights what happens when the base image is either smaller or larger than the snapshot image being committed. In the case of the base image being smaller, it is resized to the larger size of the snapshot image. In the case of the base image being larger, it is not resized automatically, but once the commit has completed it is safe for the user to truncate the base image. Signed-off-by: Jeff Cody Reviewed-by: Fam Zheng Reviewed-by: Eric Blake --- hmp-commands.hx | 5 + qapi-schema.json | 7 +++ qemu-img.texi| 7 ++- qmp-commands.hx | 39 +++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index feca084..f3fc514 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -35,6 +35,11 @@ STEXI @item commit @findex commit Commit changes to the disk images (if -snapshot is used) or backing files. +If the backing file is smaller than the snapshot, then the backing file will be +resized to be the same size as the snapshot. If the snapshot is smaller than +the backing file, the backing file will not be truncated. If you want the +backing file to match the size of the smaller snapshot, you can safely truncate +it yourself once the commit operation successfully completes. ETEXI { diff --git a/qapi-schema.json b/qapi-schema.json index f27c48a..64a108b 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1973,6 +1973,13 @@ #user needs to complete the job with the block-job-complete #command after getting the ready event. (Since 2.0) # +#If the base image is smaller than top, then the base image +#will be resized to be the same size as top. If top is +#smaller than the base image, the base will not be +#truncated. If you want the base image size to match the +#size of the smaller top, you can safely truncate it +#yourself once the commit operation successfully completes. +# # # @speed: #optional the maximum speed, in bytes per second # diff --git a/qemu-img.texi b/qemu-img.texi index 1bba91e..22556df 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -140,7 +140,12 @@ it doesn't need to be specified separately in this case. @item commit [-f @var{fmt}] [-t @var{cache}] @var{filename} -Commit the changes recorded in @var{filename} in its base image. +Commit the changes recorded in @var{filename} in its base image or backing file. +If the backing file is smaller than the snapshot, then the backing file will be +resized to be the same size as the snapshot. If the snapshot is smaller than +the backing file, the backing file will not be truncated. If you want the +backing file to match the size of the smaller snapshot, you can safely truncate +it yourself once the commit operation successfully completes. @item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2} diff --git a/qmp-commands.hx b/qmp-commands.hx index 02cc815..d7ed5ba 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -965,6 +965,45 @@ EQMP .mhandler.cmd_new = qmp_marshal_input_block_commit, }, +SQMP +block-commit + + +Live commit of data from overlay image nodes into backing nodes - i.e., writes +data between 'top' and 'base' into 'base'. + +Arguments: + +- "device": The device's ID, must be unique (json-string) +- "base": The file name of the backing image to write data into. + If not specified, this is the deepest backing image + (json-string, optional) +- "top": The file name of the backing image within the image chain, + which contains the topmost data to be committed down. + + If top == base, that is an error. + If top == active, the job will not be completed by itself, + user needs to complete the job with the block-job-complete + command after getting the ready event. (Since 2.0) + + If the base image is smaller than top, then the base image + will be resized to be the same size as top. If top is + smaller than the base image, the base will not be + truncated. If you want the base image size to match the + size of the smaller top, you can safely truncate it + yourself once the commit operation successfully completes. + (json-string) +- "speed": the maximum speed, in bytes per second (json-int, optional) + + +Example: + +-> { "execute": "block-commit", "arguments": { "device": "virtio0", + "top": "/tmp/snap1.qcow2" } } +<- { "return": {} } + +EQMP + { .name = "drive-backup", .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," -- 1.8.3.1
[Qemu-devel] [PATCH v5 1/3] block: resize backing file image during offline commit, if necessary
Currently, if an image file is logically larger than its backing file, committing it via 'qemu-img commit' will fail. For instance, if we have a base image with a virtual size 10G, and a snapshot image of size 20G, then committing the snapshot offline with 'qemu-img commit' will likely fail. This will automatically attempt to resize the base image, if the snapshot image to be committed is larger. Signed-off-by: Jeff Cody Reviewed-by: Fam Zheng Reviewed-by: Eric Blake Reviewed-by: Benoit Canet --- block.c | 28 +--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index 64e7d22..bc306da 100644 --- a/block.c +++ b/block.c @@ -1876,10 +1876,10 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix) int bdrv_commit(BlockDriverState *bs) { BlockDriver *drv = bs->drv; -int64_t sector, total_sectors; +int64_t sector, total_sectors, length, backing_length; int n, ro, open_flags; int ret = 0; -uint8_t *buf; +uint8_t *buf = NULL; char filename[PATH_MAX]; if (!drv) @@ -1904,7 +1904,29 @@ int bdrv_commit(BlockDriverState *bs) } } -total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS; +length = bdrv_getlength(bs); +if (length < 0) { +ret = length; +goto ro_cleanup; +} + +backing_length = bdrv_getlength(bs->backing_hd); +if (backing_length < 0) { +ret = backing_length; +goto ro_cleanup; +} + +/* If our top snapshot is larger than the backing file image, + * grow the backing file image if possible. If not possible, + * we must return an error */ +if (length > backing_length) { +ret = bdrv_truncate(bs->backing_hd, length); +if (ret < 0) { +goto ro_cleanup; +} +} + +total_sectors = length >> BDRV_SECTOR_BITS; buf = g_malloc(COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE); for (sector = 0; sector < total_sectors; sector += n) { -- 1.8.3.1
[Qemu-devel] [PATCH v5 2/3] block: resize backing image during active layer commit, if needed
If the top image to commit is the active layer, and also larger than the base image, then an I/O error will likely be returned during block-commit. For instance, if we have a base image with a virtual size 10G, and a active layer image of size 20G, then committing the snapshot via 'block-commit' will likely fail. This will automatically attempt to resize the base image, if the active layer image to be committed is larger. Signed-off-by: Jeff Cody Reviewed-by: Eric Blake Reviewed-by: Benoit Canet --- block/mirror.c | 38 ++ 1 file changed, 38 insertions(+) diff --git a/block/mirror.c b/block/mirror.c index 2932bab..10fd15d 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -630,11 +630,49 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { +int64_t length, base_length; +int orig_base_flags; + +orig_base_flags = bdrv_get_flags(base); + if (bdrv_reopen(base, bs->open_flags, errp)) { return; } + +length = bdrv_getlength(bs); +if (length < 0) { +error_setg(errp, "Unable to determine length of %s", bs->filename); +goto error_restore_flags; +} + +base_length = bdrv_getlength(base); +if (base_length < 0) { +error_setg(errp, "Unable to determine length of %s", base->filename); +goto error_restore_flags; +} + +if (length > base_length) { +if (bdrv_truncate(base, length) < 0) { +error_setg(errp, "Top image %s is larger than base image %s, and " + "resize of base image failed", + bs->filename, base->filename); +goto error_restore_flags; +} +} + bdrv_ref(base); mirror_start_job(bs, base, speed, 0, 0, on_error, on_error, cb, opaque, errp, &commit_active_job_driver, false, base); +if (error_is_set(errp)) { +goto error_restore_flags; +} + +return; + +error_restore_flags: +/* ignore error and errp for bdrv_reopen, because we want to propagate + * the original error */ +bdrv_reopen(base, orig_base_flags, NULL); +return; } -- 1.8.3.1
[Qemu-devel] [PATCH v5 0/3] block: commits of snapshots larger than backing files
Changes, v4->v5 Patch 1/3: Update ret in bdrv_commit() (Kevin) Patch 2/3: None Patch 3/3: None Changes, v3->v4 Patch 1/3: Commit message typo (Eric) Patch 2/3: Removed dead code (Benoît) Patch 3/3: None Changes, v2->v3: Patch 1/3: None Patch 2/3: Set errp for both bdrv_getlength() failure cases (Fam) Remove "." from error message (Eric Blake) Patch 3/3: Removed spurious double space (Eric Blake) Changes, v1->v2: Patch 1/3: Added error check for bdrv_getlength() return (Stefan) Patch 2/3: Restore flags in commit_active_start() on failures after the bdrv_reopen() (Fam) Patch 3/3: New patch, adds documentation clarification for the behavior of both offline commit, and live block commit, as it pertains to image truncation. (Stefan) If a snapshot is larger than a backing file, then the offline bdrv_commit and the live active layer commit will fail with an i/o error (usually). A live commit of a non-active layer will complete successfully, as it runs bdrv_truncate() on the backing image to resize it to the larger size. For both bdrv_commit() and commit_active_start(), this series will resize the underlying base image if needed. If the resize fails, an error will be returned. Jeff Cody (3): block: resize backing file image during offline commit, if necessary block: resize backing image during active layer commit, if needed block: update block commit documentation regarding image truncation block.c | 28 +--- block/mirror.c | 38 ++ hmp-commands.hx | 5 + qapi-schema.json | 7 +++ qemu-img.texi| 7 ++- qmp-commands.hx | 39 +++ 6 files changed, 120 insertions(+), 4 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] Possible bug in monitor code
On Fri, 24 Jan 2014 12:14:12 +0200 Stratos Psomadakis wrote: > On 01/23/2014 08:28 PM, Luiz Capitulino wrote: > > On Thu, 23 Jan 2014 17:33:33 +0200 > > Stratos Psomadakis wrote: > > > >> On 01/23/2014 03:54 PM, Luiz Capitulino wrote: > >>> On Thu, 23 Jan 2014 08:44:02 -0500 > >>> Luiz Capitulino wrote: > >>> > On Thu, 23 Jan 2014 19:23:51 +0800 > Fam Zheng wrote: > > > Bcc: > > Subject: Re: [Qemu-devel] Possible bug in monitor code > > Reply-To: > > In-Reply-To: <52e0ec4b.7010...@grnet.gr> > > > > On Thu, 01/23 12:17, Stratos Psomadakis wrote: > >> On 01/23/2014 05:07 AM, Fam Zheng wrote: > >>> On Wed, 01/22 17:53, Stratos Psomadakis wrote: > Hi, > > we've encountered a weird issue regarding monitor (qmp and hmp) > behavior > with qemu-1.7 (and qemu-1.5). The following steps will reproduce the > issue: > > 1) Client A connects to qmp socket with socat > 2) Client A gets greeting message {"QMP": {"version": ..} > 3) Client A waits (select on the socket's fd) > 4) Client B tries to connect to the *same* qmp socket with socat > QMP/HMP can only handle a single client per connection, so this is > not supported. What you could do is to create multiple QMP/HMP instances > on the command-line, but this has never been fully tested so we don't > officially support this either (although it may work). > > Now, not gracefully failing on step 4 is a real bug here. I think the > best thing to do would be to close client's B connection. Patches are > welcome :) > >>> Thinking about this again, I think this should already be the case > >>> (as I don't believe chardev code is sitting on accept()). So maybe > >>> you triggered a race on chardev code or broke something there. > >> Yes, the chardev code doesn't accept any more connections until the > >> currently active connection is closed. > >> > >> However, when a new client tries to connect (while there's another qmp / > >> hmp connection active) the kernel, as long as there's room in the socket > >> backlog, will return to the client that the connection has been > >> successfully established and will queue the connection to be accepted > >> later, when qmp actually finishes with its active connection and > >> re-executes accept(). > >> > >> The problem arises if the new client closes the connection before the > >> older one is finished. When qmp runs accept() again, it will get a > >> socket fd for a client who has now closed the connection. At this point, > >> the monitor control event handler will get triggered and try to write / > >> flush the greeting message to the client. The client however has closed > >> its socket so the write will fail with SIGPIPE / EPIPE. Neither the > >> qemu-char nor the monitor code seem to handle this situation. > >> > >> Btw, have you tried to reproduce it? > > Not yet, I may have some time tomorrow. How reproducible is it for you? > > We can trigger it (by following the steps described in the first mail) > consistently. > > > Another question: have you tried to reproduce with an old qemu version > > (say v1.0) to see if this bug always existed? If the bug was introduced > > in some recent QEMU version you could try to bisect it. > > v1.1 is not affected. I checked the code and it seems the monitor code > has been refactored since v1.1. > > > Maybe you could try to reproduce with a different subsystem so that we > > can rule out or confirm monitor's involvement? Like -serial? > > It's actually a fault of the monitor_flush() function. As far as I can > understand, monitor_flush() calls qemu_chr_fe_write() and doesn't handle > all of the return codes / error cases properly (as I described in a > previous mail). If you check the function, you'll see that the final > case (where it set ups a watch / callback) always assumes an EAGAIN / > EWOULDBLOCK error. > > If you can verify / confirm that this is the case and that the patch > sent resolves the issue in a sane / correct way, I'll resubmit it > properly (with git-format-patch, a git log msg etc). I'll check that later today.
Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
On Thu, Jan 23, 2014 at 10:30:16PM +, Peter Maydell wrote: > On 23 January 2014 22:16, Wei Liu wrote: > > As promised I hacked a prototype based on Paolo's disable TCG series. > > However I coded some stubs for TCG anyway. So this series in principle > > should work with / without Paolo's series. > > I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and Thanks for being blunt. ;-) > "the binary is smaller" isn't IMHO sufficient justification for breaking > QEMU's basic structure of "target-* define target CPUs and we have > a lot of compile time constants which are specific to a CPU which > get defined there". How would you support a bigendian Xen CPU, > just to pick one example of where this falls down? > I think about this deeper. From Xen's (and I speculate this applies to other hardware assisted virtulization solution as well) PoV only the native endianess is supported, does it make sense to have a target-native thing? Wei. > thanks -- PMM
Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
Il 23/01/2014 23:30, Peter Maydell ha scritto: > As promised I hacked a prototype based on Paolo's disable TCG series. > However I coded some stubs for TCG anyway. So this series in principle > should work with / without Paolo's series. I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and "the binary is smaller" isn't IMHO sufficient justification for breaking QEMU's basic structure of "target-* define target CPUs and we have a lot of compile time constants which are specific to a CPU which get defined there". How would you support a bigendian Xen CPU, just to pick one example of where this falls down? (1) decide that the Xen ring buffers are little-endian even on big-endian CPUs (2) communicate the endianness of the Xen ring buffers via Xenstore, just like we do for sizeof(long), and let the guest use either endianness on any architecture. Paolo
Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
On 24 January 2014 14:30, Paolo Bonzini wrote: > Il 23/01/2014 23:30, Peter Maydell ha scritto: >> I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and >> "the binary is smaller" isn't IMHO sufficient justification for breaking >> QEMU's basic structure of "target-* define target CPUs and we have >> a lot of compile time constants which are specific to a CPU which >> get defined there". How would you support a bigendian Xen CPU, >> just to pick one example of where this falls down? > > > (1) decide that the Xen ring buffers are little-endian even on big-endian > CPUs > > (2) communicate the endianness of the Xen ring buffers via Xenstore, just > like we do for sizeof(long), and let the guest use either endianness on any > architecture. You still have to make a choice about what you think TARGET_WORDS_BIGENDIAN should be, and it's still going to be wrong half the time and horribly confusing. I just think this is completely the wrong solution to the problem. If Xen really wants a totally standalone binary of the smallest possible size with just paravirtualized hardware and minimal to no dependency on guest CPU architecture then they should write one, along the lines of kvmtool :-) thanks -- PMM
Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
Il 24/01/2014 15:23, Wei Liu ha scritto: On Thu, Jan 23, 2014 at 10:30:16PM +, Peter Maydell wrote: On 23 January 2014 22:16, Wei Liu wrote: As promised I hacked a prototype based on Paolo's disable TCG series. However I coded some stubs for TCG anyway. So this series in principle should work with / without Paolo's series. I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and Thanks for being blunt. ;-) "the binary is smaller" isn't IMHO sufficient justification for breaking QEMU's basic structure of "target-* define target CPUs and we have a lot of compile time constants which are specific to a CPU which get defined there". How would you support a bigendian Xen CPU, just to pick one example of where this falls down? I think about this deeper. From Xen's (and I speculate this applies to other hardware assisted virtulization solution as well) PoV only the native endianess is supported, does it make sense to have a target-native thing? I think this is wrong, for a few reasons: (1) xenpv is not hardware assisted virtualization (2) supporting only native endianness leads to complications when systems are bi-endian, as is the case for PPC. For example, virtio 1.0 will always be little endian. (3) there's a precedent for supporting different guests between the guest and the host in blkback, you can do the same for endianness. Paolo
[Qemu-devel] [Bug 1272252] [NEW] qemu-img ftp/http convert
Public bug reported: Converting images with ftp or http as source could be done a lot faster. The way it works now (qemu 1.7.50) is significantly slower than the optimal way. FTP - how it works now 1. Connect and login to ftp-server. Ask for size of file. 2. Get a chunk of data using rest+retr 3. Goto step 1 again in a loop until all data is retrieved FTP - better solution 1. Connect and login to ftp-server. Dont ask for size of file. 2. Retrieve all remaining data 3. Goto step 1 again if disconnected/io error (max NN errors etc) Http - how it works now 1. Connect to webserver and ask for size of file / http HEAD. 2. Get a chunk of data using http Range. 3. Goto step 1 again in a loop until all data is retrieved. Http - better solution 1. Connect to webserver. 2. Retrieve all remaining data. 3. Goto step 1 again if disconnected/io error (max NN errors). ** Affects: qemu Importance: Undecided Status: New ** Tags: optimisation qemu-img -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1272252 Title: qemu-img ftp/http convert Status in QEMU: New Bug description: Converting images with ftp or http as source could be done a lot faster. The way it works now (qemu 1.7.50) is significantly slower than the optimal way. FTP - how it works now 1. Connect and login to ftp-server. Ask for size of file. 2. Get a chunk of data using rest+retr 3. Goto step 1 again in a loop until all data is retrieved FTP - better solution 1. Connect and login to ftp-server. Dont ask for size of file. 2. Retrieve all remaining data 3. Goto step 1 again if disconnected/io error (max NN errors etc) Http - how it works now 1. Connect to webserver and ask for size of file / http HEAD. 2. Get a chunk of data using http Range. 3. Goto step 1 again in a loop until all data is retrieved. Http - better solution 1. Connect to webserver. 2. Retrieve all remaining data. 3. Goto step 1 again if disconnected/io error (max NN errors). To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1272252/+subscriptions
Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
Il 24/01/2014 15:35, Peter Maydell ha scritto: > (1) decide that the Xen ring buffers are little-endian even on big-endian > CPUs > > (2) communicate the endianness of the Xen ring buffers via Xenstore, just > like we do for sizeof(long), and let the guest use either endianness on any > architecture. You still have to make a choice about what you think TARGET_WORDS_BIGENDIAN should be, and it's still going to be wrong half the time and horribly confusing. I just think this is completely the wrong solution to the problem. Theoretically the xenpv-softmmu machine shouldn't need any code that depends on TARGET_WORDS_BIGENDIAN. If we changed every #ifdef TARGET_WORDS_BIGENDIAN to if(), we could compile it with "#define TARGET_WORDS_BIGENDIAN abort()". Paolo
Re: [Qemu-devel] Possible bug in monitor code
Hi all, On 12:14 Fri 24 Jan , Stratos Psomadakis wrote: > On 01/23/2014 08:28 PM, Luiz Capitulino wrote: > > Not yet, I may have some time tomorrow. How reproducible is it for > > you? > > We can trigger it (by following the steps described in the first mail) > consistently. > > > Another question: have you tried to reproduce with an old qemu version > > (say v1.0) to see if this bug always existed? If the bug was introduced > > in some recent QEMU version you could try to bisect it. > > v1.1 is not affected. I checked the code and it seems the monitor code > has been refactored since v1.1. > > > Maybe you could try to reproduce with a different subsystem so that we > > can rule out or confirm monitor's involvement? Like -serial? > > It's actually a fault of the monitor_flush() function. As far as I can > understand, monitor_flush() calls qemu_chr_fe_write() and doesn't handle > all of the return codes / error cases properly (as I described in a > previous mail). If you check the function, you'll see that the final > case (where it set ups a watch / callback) always assumes an EAGAIN / > EWOULDBLOCK error. > > If you can verify / confirm that this is the case and that the patch > sent resolves the issue in a sane / correct way, I'll resubmit it > properly (with git-format-patch, a git log msg etc). Please see the attached testcase (python script) that programmatically reproduces this. Sample output with qemu 1.7.0: $ ./test-qmp.py Spawning qemu Connecting client 1 Monitor output: {"QMP": {"version": {"qemu": {"micro": 0, "minor": 7, "major": 1}, "package": " (Debian 1.7.0+dfsg-2)"}, "capabilities": []}} Connecting client 2 Monitor output: (timeout, disconnecting) Disconnecting client 1 Connecting client 3 Monitor output {"QMP": {"version": {"qemu": {"micro": 0, "minor": 7, "major": 1}, "package": " (Debian 1.7.0+dfsg-2)"}, "capabilities": []}} {"QMP": {"version": {"qemu": {"micro": 0, "minor": 7, "major": 1}, "package": " (Debian 1.7.0+dfsg-2)"}, "capabilities": []}} Terminating qemu qemu: terminating on signal 15 from pid 11269 Regards, Apollon #!/usr/bin/python import os import socket import tempfile import subprocess from time import sleep sock_path = tempfile.mktemp() print "Spawning qemu" print qemu = subprocess.Popen(["/usr/bin/qemu", "-chardev", "socket,id=mon0,path=%s,server,nowait" % sock_path, "-mon", "chardev=mon0,mode=control", "-display", "none"]) # Wait for qemu to initialize while not os.path.exists(sock_path): sleep(0.1) print "Connecting client 1\n" cl1 = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) cl1.connect(sock_path) print "Monitor output:" print cl1.recv(1024) print print "Connecting client 2\n" cl2 = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) cl2.settimeout(1) try: cl2.connect(sock_path) print "Monitor output:" print cl2.recv(1024) except socket.timeout: print "(timeout, disconnecting)\n" cl2.close() print "Disconnecting client 1\n" cl1.close() print "Connecting client 3\n" cl3 = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) cl3.connect(sock_path) print "Monitor output" print cl3.recv(1024) cl3.close() print "Terminating qemu" qemu.terminate() qemu.wait()
Re: [Qemu-devel] [PATCH V6 8/8] block: Use graph node name as reference in bdrv_file_open().
Am 24.01.2014 um 14:37 hat Max Reitz geschrieben: > On 24.01.2014 14:26, Kevin Wolf wrote: > >Am 23.01.2014 um 21:31 hat Benoît Canet geschrieben: > >>Signed-off-by: Benoit Canet > >>--- > >> block.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >I'm not going to merge this one yet. It breaks qemu-iotests case 071, > >which would have to be adapted. > > > >However, first of all I'd like to hear the opinions of at least Eric and > >Max on what BlockRef should really refer to. I think node names make > >most sense, but perhaps it's a bit inconvenient and the command line > >should default to node-name = id when id is set, but node-name isn't? > > The QAPI schema is pretty clear about this: “references the ID of an > existing block device.” Sure, that's because I wrote that text before we had a node name. However, in 1.7 references didn't work yet, so we still have all freedom to change the interface as we like. > However, if the ID cannot be found, I think > we should interpret it as a reference to the node name. > > Therefore, I'd first try bdrv_find() and if that returns NULL, try > again with bdrv_find_node(). I think I would prefer to avoid such ambiguities. Otherwise a management tool that wants to use the node name needs to check first if it's not already used as a device name somewhere else and would therefore operate on the wrong device. On the other hand, a management tool using the same names for devices and nodes just gets what it deserves. Perhaps we should use a common namespace for both, i.e. you get an error if you try to assign a node name that is already a device name and vice versa? Kevin
Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
On Fri, Jan 24, 2014 at 03:38:01PM +0100, Paolo Bonzini wrote: > Il 24/01/2014 15:23, Wei Liu ha scritto: > >On Thu, Jan 23, 2014 at 10:30:16PM +, Peter Maydell wrote: > >>On 23 January 2014 22:16, Wei Liu wrote: > >>>As promised I hacked a prototype based on Paolo's disable TCG series. > >>>However I coded some stubs for TCG anyway. So this series in principle > >>>should work with / without Paolo's series. > >> > >>I'm afraid I still think this is a terrible idea. "Xen" isn't a CPU, and > > > >Thanks for being blunt. ;-) > > > >>"the binary is smaller" isn't IMHO sufficient justification for breaking > >>QEMU's basic structure of "target-* define target CPUs and we have > >>a lot of compile time constants which are specific to a CPU which > >>get defined there". How would you support a bigendian Xen CPU, > >>just to pick one example of where this falls down? > >> > > > >I think about this deeper. From Xen's (and I speculate this applies to > >other hardware assisted virtulization solution as well) PoV only the > >native endianess is supported, does it make sense to have a > >target-native thing? > > I think this is wrong, for a few reasons: > > (1) xenpv is not hardware assisted virtualization > Correct... What I really meant was "those virtualization solutions don't care much about CPU emulation" (if there's any other than Xen). > (2) supporting only native endianness leads to complications when > systems are bi-endian, as is the case for PPC. For example, virtio > 1.0 will always be little endian. > OK, good to know. > (3) there's a precedent for supporting different guests between the > guest and the host in blkback, you can do the same for endianness. > Yes. But this is still open question at the moment, as there's no canonical protocol for this (it hasn't even been discussed). Your second point is the thing I missed though. Thanks for reminding. Wei. > Paolo
Re: [Qemu-devel] [PATCH V6 2/8] block: Allow the user to define "node-name" option both on command line and QMP.
Le Thursday 23 Jan 2014 à 21:31:33 (+0100), Benoît Canet a écrit : > From: Benoît Canet > > Signed-off-by: Benoit Canet > --- > block.c | 36 > qapi-schema.json | 2 ++ > 2 files changed, 38 insertions(+) > > diff --git a/block.c b/block.c > index 60b70bc..d9d02d2 100644 > --- a/block.c > +++ b/block.c > @@ -728,6 +728,33 @@ static int bdrv_open_flags(BlockDriverState *bs, int > flags) > return open_flags; > } > > +static int bdrv_assign_node_name(BlockDriverState *bs, > + const char *node_name, > + Error **errp) > +{ > +if (!node_name) { > +return 0; > +} > + > +/* empty string node name is invalid */ > +if (node_name[0] == '\0') { > +error_setg(errp, "Empty node name"); > +return -EINVAL; > +} > + > +/* takes care of avoiding duplicates node names */ > +if (bdrv_find_node(node_name)) { > +error_setg(errp, "Duplicate node name"); > +return -EINVAL; > +} > + > +/* copy node name into the bs and insert it into the graph list */ > +pstrcpy(bs->node_name, sizeof(bs->node_name), node_name); > +QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list); > + > +return 0; > +} > + > /* > * Common part for opening disk images and files > * > @@ -738,6 +765,7 @@ static int bdrv_open_common(BlockDriverState *bs, > BlockDriverState *file, > { > int ret, open_flags; > const char *filename; > +const char *node_name = NULL; > Error *local_err = NULL; > > assert(drv != NULL); > @@ -752,6 +780,14 @@ static int bdrv_open_common(BlockDriverState *bs, > BlockDriverState *file, > > trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name); > > +node_name = qdict_get_try_str(options, "node-name"); > +qdict_del(options, "node-name"); Kevin: I wonder if I delete the option too early hence zeroing node-name ? > + > +ret = bdrv_assign_node_name(bs, node_name, errp); > +if (ret < 0) { > +return ret; > +} > + > /* bdrv_open() with directly using a protocol as drv. This layer is > already > * opened, so assign it to bs (while file becomes a closed > BlockDriverState) > * and return immediately. */ > diff --git a/qapi-schema.json b/qapi-schema.json > index 35f7b34..04167da 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4090,6 +4090,7 @@ > # @id: #optional id by which the new block device can be referred > to. > # This is a required option on the top level of blockdev-add, > and > # currently not allowed on any other level. > +# @node-name: #optional the name of a block driver state node (Since 2.0) > # @discard: #optional discard-related options (default: ignore) > # @cache: #optional cache-related options > # @aio: #optional AIO backend (default: threads) > @@ -4105,6 +4106,7 @@ > { 'type': 'BlockdevOptionsBase', >'data': { 'driver': 'str', > '*id': 'str', > +'*node-name': 'str', > '*discard': 'BlockdevDiscardOptions', > '*cache': 'BlockdevCacheOptions', > '*aio': 'BlockdevAioOptions', > -- > 1.8.3.2 >
Re: [Qemu-devel] [PATCH V6 8/8] block: Use graph node name as reference in bdrv_file_open().
On 24.01.2014 15:48, Kevin Wolf wrote: Am 24.01.2014 um 14:37 hat Max Reitz geschrieben: On 24.01.2014 14:26, Kevin Wolf wrote: Am 23.01.2014 um 21:31 hat Benoît Canet geschrieben: Signed-off-by: Benoit Canet --- block.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) I'm not going to merge this one yet. It breaks qemu-iotests case 071, which would have to be adapted. However, first of all I'd like to hear the opinions of at least Eric and Max on what BlockRef should really refer to. I think node names make most sense, but perhaps it's a bit inconvenient and the command line should default to node-name = id when id is set, but node-name isn't? The QAPI schema is pretty clear about this: “references the ID of an existing block device.” Sure, that's because I wrote that text before we had a node name. However, in 1.7 references didn't work yet, so we still have all freedom to change the interface as we like. Yes, that's right. However, if the ID cannot be found, I think we should interpret it as a reference to the node name. Therefore, I'd first try bdrv_find() and if that returns NULL, try again with bdrv_find_node(). I think I would prefer to avoid such ambiguities. Otherwise a management tool that wants to use the node name needs to check first if it's not already used as a device name somewhere else and would therefore operate on the wrong device. On the other hand, a management tool using the same names for devices and nodes just gets what it deserves. Perhaps we should use a common namespace for both, i.e. you get an error if you try to assign a node name that is already a device name and vice versa? This is what I would go for. However, then I don't really know why we should separate the ID and the node name in the first place (although that's probably because I haven't followed the discussion around node names). Max
Re: [Qemu-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
On Fri, 24 Jan 2014, Paolo Bonzini wrote: > Il 24/01/2014 15:35, Peter Maydell ha scritto: > > > > (1) decide that the Xen ring buffers are little-endian even on > > > big-endian > > > > CPUs > > > > > > > > (2) communicate the endianness of the Xen ring buffers via Xenstore, > > > just > > > > like we do for sizeof(long), and let the guest use either endianness on > > > any > > > > architecture. > > You still have to make a choice about what you think > > TARGET_WORDS_BIGENDIAN should be, and it's still going > > to be wrong half the time and horribly confusing. > > I just think this is completely the wrong solution to > > the problem. > > Theoretically the xenpv-softmmu machine shouldn't need any code that depends > on TARGET_WORDS_BIGENDIAN. > > If we changed every #ifdef TARGET_WORDS_BIGENDIAN to if(), we could compile it > with "#define TARGET_WORDS_BIGENDIAN abort()". Right. All our PV protocols are little endian by definition. Besides I don't know how to state this more clearly but there are no big endian Xen guests. There have never been any big endian Xen guests. There are no big endian Xen guests on the roadmap.
Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface
Am 24.01.2014 um 14:56 hat Paolo Bonzini geschrieben: > From: Peter Maydell > > libcurl versions 7.16.0 and later have a timer callback interface which > must be implemented in order for libcurl to make forward progress (it > will sometimes rely on being called back on the timeout if there are > no file descriptors registered). Implement the callback, and use a > QEMU AIO timer to ensure we prod libcurl again when it asks us to. > > Based on Peter's original patch plus my fix to add curl_multi_timeout_do. > Should compile just fine even on older versions of libcurl. > > I also tried copy-on-read and streaming: > > $ ./qemu-img create -f qcow2 -o \ > > backing_file=http://download.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso > \ > foo.qcow2 1G > $ x86_64-softmmu/qemu-system-x86_64 \ > -drive if=none,file=foo.qcow2,copy-on-read=on,id=cd \ > -device ide-cd,drive=cd --enable-kvm -m 1024 > > Direct http usage is probably too slow, but with copy-on-read ultimately > the image does boot! > > After some time, streaming gets canceled by an EIO, which needs further > investigation. > > Signed-off-by: Peter Maydell > Signed-off-by: Paolo Bonzini Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH v4 4/4] qemu-iotests: add test for qcow2 preallocation with different cluster sizes
On 23.01.2014 04:04, Hu Tao wrote: Signed-off-by: Hu Tao --- tests/qemu-iotests/079 | 63 ++ tests/qemu-iotests/079.out | 32 +++ tests/qemu-iotests/group | 1 + 3 files changed, 96 insertions(+) create mode 100755 tests/qemu-iotests/079 create mode 100644 tests/qemu-iotests/079.out Reviewed-by: Max Reitz
Re: [Qemu-devel] [PATCH V6 2/8] block: Allow the user to define "node-name" option both on command line and QMP.
Am 24.01.2014 um 15:51 hat Benoît Canet geschrieben: > Le Thursday 23 Jan 2014 à 21:31:33 (+0100), Benoît Canet a écrit : > > From: Benoît Canet > > > > Signed-off-by: Benoit Canet > > --- > > block.c | 36 > > qapi-schema.json | 2 ++ > > 2 files changed, 38 insertions(+) > > > > diff --git a/block.c b/block.c > > index 60b70bc..d9d02d2 100644 > > --- a/block.c > > +++ b/block.c > > @@ -728,6 +728,33 @@ static int bdrv_open_flags(BlockDriverState *bs, int > > flags) > > return open_flags; > > } > > > > +static int bdrv_assign_node_name(BlockDriverState *bs, > > + const char *node_name, > > + Error **errp) > > +{ > > +if (!node_name) { > > +return 0; > > +} > > + > > +/* empty string node name is invalid */ > > +if (node_name[0] == '\0') { > > +error_setg(errp, "Empty node name"); > > +return -EINVAL; > > +} > > + > > +/* takes care of avoiding duplicates node names */ > > +if (bdrv_find_node(node_name)) { > > +error_setg(errp, "Duplicate node name"); > > +return -EINVAL; > > +} > > + > > +/* copy node name into the bs and insert it into the graph list */ > > +pstrcpy(bs->node_name, sizeof(bs->node_name), node_name); > > +QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list); > > + > > +return 0; > > +} > > + > > /* > > * Common part for opening disk images and files > > * > > @@ -738,6 +765,7 @@ static int bdrv_open_common(BlockDriverState *bs, > > BlockDriverState *file, > > { > > int ret, open_flags; > > const char *filename; > > +const char *node_name = NULL; > > Error *local_err = NULL; > > > > assert(drv != NULL); > > @@ -752,6 +780,14 @@ static int bdrv_open_common(BlockDriverState *bs, > > BlockDriverState *file, > > > > trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name); > > > > +node_name = qdict_get_try_str(options, "node-name"); > > > +qdict_del(options, "node-name"); > > Kevin: I wonder if I delete the option too early hence zeroing node-name ? For the record, we concluded on IRC that I would squash the following patch in: diff --git a/block.c b/block.c index 9dc1216..f043669 100644 --- a/block.c +++ b/block.c @@ -788,12 +788,11 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name); node_name = qdict_get_try_str(options, "node-name"); -qdict_del(options, "node-name"); - ret = bdrv_assign_node_name(bs, node_name, errp); if (ret < 0) { return ret; } +qdict_del(options, "node-name"); /* bdrv_open() with directly using a protocol as drv. This layer is already * opened, so assign it to bs (while file becomes a closed BlockDriverState) Kevin > > + > > +ret = bdrv_assign_node_name(bs, node_name, errp); > > +if (ret < 0) { > > +return ret; > > +} > > + > > /* bdrv_open() with directly using a protocol as drv. This layer is > > already > > * opened, so assign it to bs (while file becomes a closed > > BlockDriverState) > > * and return immediately. */ > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 35f7b34..04167da 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -4090,6 +4090,7 @@ > > # @id: #optional id by which the new block device can be referred > > to. > > # This is a required option on the top level of > > blockdev-add, and > > # currently not allowed on any other level. > > +# @node-name: #optional the name of a block driver state node (Since 2.0) > > # @discard: #optional discard-related options (default: ignore) > > # @cache: #optional cache-related options > > # @aio: #optional AIO backend (default: threads) > > @@ -4105,6 +4106,7 @@ > > { 'type': 'BlockdevOptionsBase', > >'data': { 'driver': 'str', > > '*id': 'str', > > +'*node-name': 'str', > > '*discard': 'BlockdevDiscardOptions', > > '*cache': 'BlockdevCacheOptions', > > '*aio': 'BlockdevAioOptions', > > -- > > 1.8.3.2 > >
Re: [Qemu-devel] [PATCH v5 0/3] block: commits of snapshots larger than backing files
Am 24.01.2014 um 15:02 hat Jeff Cody geschrieben: > Changes, v4->v5 > > Patch 1/3: Update ret in bdrv_commit() (Kevin) > Patch 2/3: None > Patch 3/3: None > > [...] > > If a snapshot is larger than a backing file, then the offline bdrv_commit and > the live active layer commit will fail with an i/o error (usually). A live > commit of a non-active layer will complete successfully, as it runs > bdrv_truncate() on the backing image to resize it to the larger size. > > For both bdrv_commit() and commit_active_start(), this series will resize > the underlying base image if needed. If the resize fails, an error will > be returned. > > > Jeff Cody (3): > block: resize backing file image during offline commit, if necessary > block: resize backing image during active layer commit, if needed > block: update block commit documentation regarding image truncation > > block.c | 28 +--- > block/mirror.c | 38 ++ > hmp-commands.hx | 5 + > qapi-schema.json | 7 +++ > qemu-img.texi| 7 ++- > qmp-commands.hx | 39 +++ > 6 files changed, 120 insertions(+), 4 deletions(-) Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH v4 2/4] qcow2: fix offset overflow in qcow2_alloc_clusters_at()
Le Friday 24 Jan 2014 à 18:01:20 (+0800), Hu Tao a écrit : > On Thu, Jan 23, 2014 at 06:13:48PM +0100, Benoît Canet wrote: > > Le Thursday 23 Jan 2014 à 11:04:06 (+0800), Hu Tao a écrit : > > > When cluster size is big enough it can lead offset overflow > > > > Maybe "it can lead to an offset overflow in" > > > in qcow2_alloc_clusters_at(). This patch fixes it. > > Sure. > > > > > > > The allocation each time is stopped at L2 table boundary > > "The allocation is stopped each time at" > > Sure. > > > > > > (see handle_alloc()), so the possible maximum bytes could be > > > > > > 2^(cluster_bits - 3 + cluster_bits) > > > > So if understand cluster_bits - 3 is used to compute the number of entry by > > L2 > > and the additional cluster_bits is to take into account each clusters > > referenced > > by the L2 entries. > > Exactly. This is clearer than just one calculation. Do you mind I > put the sentence in commit message? yes It would be fine for future reference. > > > It makes sense. > > > > > > > > so int is safe for cluster_bits<=17, unsafe otherwise. > > > > > > Reviewed-by: Max Reitz > > > Signed-off-by: Hu Tao > > > --- > > > block/qcow2-refcount.c | 8 +++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > > > index c974abe..8712d8b 100644 > > > --- a/block/qcow2-refcount.c > > > +++ b/block/qcow2-refcount.c > > > @@ -676,7 +676,13 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, > > > uint64_t offset, > > > BDRVQcowState *s = bs->opaque; > > > uint64_t cluster_index; > > > uint64_t old_free_cluster_index; > > > -int i, refcount, ret; > > > +uint64_t i; > > > +int refcount, ret; > > > + > > > > > > > +assert(nb_clusters >= 0); > > > +if (nb_clusters == 0) { > > > +return 0; > > > +} > > ^ > > Adding a a line on the commit message about this assertion and chunk of code > > would be helpful. > > How about > > Add an assert to guard the comparation between signed and unsigned yes > > ? > > > > > Best regards > > > > Benoît > > > > > > > > /* Check how many clusters there are free */ > > > cluster_index = offset >> s->cluster_bits; > > > -- > > > 1.8.5.2.229.g4448466 > > > > > > >
Re: [Qemu-devel] [Xen-devel] [PATCH RFC 0/5] Xen: introduce Xen PV target
On Fri, 2014-01-24 at 14:56 +, Stefano Stabellini wrote: > Besides I don't know how to state this more clearly but there are no big > endian Xen guests. There have never been any big endian Xen guests. > There are no big endian Xen guests on the roadmap. That is perhaps a bit strong, but what is 100% true is that none of the defined Xen PV protocols (listed at [0]) is big endian. If someone were to build e.g. an armbe guest then they would either have to do the swapping in the frontend (and continue to claim to be the l.e. XEN_IO_PROTO_ABI_ARM) or they would have to define XEN_IO_PROTO_ABI_ARMBE and arrange for the front and backend to negotiate as appropriate, and that would be the appropriate point in time for the backends to be taught about non-native endianess IMHO. [0] http://xenbits.xen.org/docs/unstable-staging/hypercall/x86_64/include,public,io,protocols.h.html Ian.
Re: [Qemu-devel] [PATCH v4 1/4] qcow2: remove n_start and n_end of qcow2_alloc_cluster_offset()
Le Friday 24 Jan 2014 à 17:32:40 (+0800), Hu Tao a écrit : > On Thu, Jan 23, 2014 at 06:02:08PM +0100, Benoît Canet wrote: > > Le Thursday 23 Jan 2014 à 11:04:05 (+0800), Hu Tao a écrit : > > > n_start can be actually calculated from offset. The number of > > > sectors to be allocated(n_end - n_start) can be passed in in > > > num. By removing n_start and n_end, we can save two parameters. > > > > > > The side effect is there is a bug in qcow2.c:preallocate() that > > > passes incorrect n_start to qcow2_alloc_cluster_offset() is > > > fixed. The bug can be triggerred by a larger cluster size than > > > the default value(65536), for example: > > > > > > ./qemu-img create -f qcow2 \ > > > -o 'cluster_size=131072,preallocation=metadata' file.img 4G > > > > > > Reviewed-by: Max Reitz > > > Signed-off-by: Hu Tao > > > --- > > > block/qcow2-cluster.c | 14 ++ > > > block/qcow2.c | 11 +++ > > > block/qcow2.h | 2 +- > > > trace-events | 2 +- > > > 4 files changed, 11 insertions(+), 18 deletions(-) > > > > > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > > > index 8534084..c57f39d 100644 > > > --- a/block/qcow2-cluster.c > > > +++ b/block/qcow2-cluster.c > > > @@ -1182,7 +1182,7 @@ fail: > > > * Return 0 on success and -errno in error cases > > > */ > > > int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset, > > > -int n_start, int n_end, int *num, uint64_t *host_offset, QCowL2Meta > > > **m) > > > +int *num, uint64_t *host_offset, QCowL2Meta **m) > > > { > > > BDRVQcowState *s = bs->opaque; > > > uint64_t start, remaining; > > > @@ -1190,15 +1190,13 @@ int qcow2_alloc_cluster_offset(BlockDriverState > > > *bs, uint64_t offset, > > > uint64_t cur_bytes; > > > int ret; > > > > > > -trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset, > > > - n_start, n_end); > > > +trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset, > > > *num); > > > > > > -assert(n_start * BDRV_SECTOR_SIZE == offset_into_cluster(s, offset)); > > > -offset = start_of_cluster(s, offset); > > > +assert((offset & ~BDRV_SECTOR_MASK) == 0); > > > > Why replace something that would round gently an unaligned offset > > (start_of_cluster) by an assert that would make QEMU exit ? > > It is equivalent to the removed assert(). Oh sorry I didn't see the removed assert() when reviewing :( > >
Re: [Qemu-devel] [PATCH] block: Fix bdrv_commit return value
Le Friday 24 Jan 2014 à 14:08:21 (+0100), Kevin Wolf a écrit : > bdrv_commit() could return 0 or 1 on success, depending on whether or > now the last sector was allocated in the overlay and whether the overlay > format had a .bdrv_make_empty callback. > > Most callers ignored it, but qemu-img commit would print an error > message while the operation actually succeeded. > > Also clean up the handling of I/O errors to return the real error code > instead of -EIO. > > Signed-off-by: Kevin Wolf > --- > block.c | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/block.c b/block.c > index 3e0994b..7d22ca9 100644 > --- a/block.c > +++ b/block.c > @@ -2061,13 +2061,13 @@ int bdrv_commit(BlockDriverState *bs) > goto ro_cleanup; > } > if (ret) { > -if (bdrv_read(bs, sector, buf, n) != 0) { > -ret = -EIO; > +ret = bdrv_read(bs, sector, buf, n); > +if (ret < 0) { > goto ro_cleanup; > } > > -if (bdrv_write(bs->backing_hd, sector, buf, n) != 0) { > -ret = -EIO; > +ret = bdrv_write(bs->backing_hd, sector, buf, n); > +if (ret < 0) { > goto ro_cleanup; > } > } > @@ -2075,6 +2075,9 @@ int bdrv_commit(BlockDriverState *bs) > > if (drv->bdrv_make_empty) { > ret = drv->bdrv_make_empty(bs); > +if (ret < 0) { > +goto ro_cleanup; > +} > bdrv_flush(bs); > } > > @@ -2082,9 +2085,11 @@ int bdrv_commit(BlockDriverState *bs) > * Make sure all data we wrote to the backing device is actually > * stable on disk. > */ > -if (bs->backing_hd) > +if (bs->backing_hd) { > bdrv_flush(bs->backing_hd); > +} > > +ret = 0; > ro_cleanup: > g_free(buf); > > -- > 1.8.1.4 > > the != 0 in "if (bdrv_read(bs, sector, buf, n) != 0) {" was really odd. Reviewed-by: Benoit Canet
Re: [Qemu-devel] [PATCH] block: Fix bdrv_commit return value
On Fri, Jan 24, 2014 at 02:08:21PM +0100, Kevin Wolf wrote: > bdrv_commit() could return 0 or 1 on success, depending on whether or > now the last sector was allocated in the overlay and whether the overlay > format had a .bdrv_make_empty callback. > > Most callers ignored it, but qemu-img commit would print an error > message while the operation actually succeeded. > > Also clean up the handling of I/O errors to return the real error code > instead of -EIO. > > Signed-off-by: Kevin Wolf > --- > block.c | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/block.c b/block.c > index 3e0994b..7d22ca9 100644 > --- a/block.c > +++ b/block.c > @@ -2061,13 +2061,13 @@ int bdrv_commit(BlockDriverState *bs) > goto ro_cleanup; > } > if (ret) { > -if (bdrv_read(bs, sector, buf, n) != 0) { > -ret = -EIO; > +ret = bdrv_read(bs, sector, buf, n); > +if (ret < 0) { > goto ro_cleanup; > } > > -if (bdrv_write(bs->backing_hd, sector, buf, n) != 0) { > -ret = -EIO; > +ret = bdrv_write(bs->backing_hd, sector, buf, n); > +if (ret < 0) { > goto ro_cleanup; > } > } > @@ -2075,6 +2075,9 @@ int bdrv_commit(BlockDriverState *bs) > > if (drv->bdrv_make_empty) { > ret = drv->bdrv_make_empty(bs); > +if (ret < 0) { > +goto ro_cleanup; > +} QED has a .bdrv_make_empty implementation, but it is a stub that always returns -ENOTSUP. Prior to this patch, a commit of a QED snapshot would complete OK, but return an error saying "Image is already committed" (Which almost seems like a non-error error). Now, we'll skip ahead to cleanup. I think we should either: 1) filter out -ENOTSUP here, or 2) remove the stub from QED > bdrv_flush(bs); > } > > @@ -2082,9 +2085,11 @@ int bdrv_commit(BlockDriverState *bs) > * Make sure all data we wrote to the backing device is actually > * stable on disk. > */ > -if (bs->backing_hd) > +if (bs->backing_hd) { > bdrv_flush(bs->backing_hd); > +} > > +ret = 0; > ro_cleanup: > g_free(buf); > > -- > 1.8.1.4 >
Re: [Qemu-devel] [PATCH] block: Fix bdrv_commit return value
Am 24.01.2014 um 16:56 hat Jeff Cody geschrieben: > On Fri, Jan 24, 2014 at 02:08:21PM +0100, Kevin Wolf wrote: > > bdrv_commit() could return 0 or 1 on success, depending on whether or > > now the last sector was allocated in the overlay and whether the overlay > > format had a .bdrv_make_empty callback. > > > > Most callers ignored it, but qemu-img commit would print an error > > message while the operation actually succeeded. > > > > Also clean up the handling of I/O errors to return the real error code > > instead of -EIO. > > > > Signed-off-by: Kevin Wolf > > --- > > block.c | 15 ++- > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > diff --git a/block.c b/block.c > > index 3e0994b..7d22ca9 100644 > > --- a/block.c > > +++ b/block.c > > @@ -2061,13 +2061,13 @@ int bdrv_commit(BlockDriverState *bs) > > goto ro_cleanup; > > } > > if (ret) { > > -if (bdrv_read(bs, sector, buf, n) != 0) { > > -ret = -EIO; > > +ret = bdrv_read(bs, sector, buf, n); > > +if (ret < 0) { > > goto ro_cleanup; > > } > > > > -if (bdrv_write(bs->backing_hd, sector, buf, n) != 0) { > > -ret = -EIO; > > +ret = bdrv_write(bs->backing_hd, sector, buf, n); > > +if (ret < 0) { > > goto ro_cleanup; > > } > > } > > @@ -2075,6 +2075,9 @@ int bdrv_commit(BlockDriverState *bs) > > > > if (drv->bdrv_make_empty) { > > ret = drv->bdrv_make_empty(bs); > > +if (ret < 0) { > > +goto ro_cleanup; > > +} > > QED has a .bdrv_make_empty implementation, but it is a stub that > always returns -ENOTSUP. > > Prior to this patch, a commit of a QED snapshot would complete OK, but > return an error saying "Image is already committed" (Which almost > seems like a non-error error). > > Now, we'll skip ahead to cleanup. > > I think we should either: 1) filter out -ENOTSUP here, or 2) remove > the stub from QED I vote for fixing QED, this stub doesn't make any sense and never matched the interface that was actually used. And I guess we can remove it in qcow2 as well, there it's a simple return 0 with the rest of the function #if'd out. Kevin
Re: [Qemu-devel] [PATCH v3 21/29] block: Assert serialisation assumptions in pwritev
Le Friday 17 Jan 2014 à 15:15:11 (+0100), Kevin Wolf a écrit : > If a request calls wait_serialising_requests() and actually has to wait > in this function (i.e. a coroutine yield), other requests can run and > previously read data (like the head or tail buffer) could become > outdated. In this case, we would have to restart from the beginning to > read in the updated data. > > However, we're lucky and don't actually need to do that: A request can > only wait in the first call of wait_serialising_requests() because we > mark it as serialising before that call, so any later requests would > wait. So as we don't wait in practice, we don't have to reload the data. > > This is an important assumption that may not be broken or data > corruption will happen. Document it with some assertions. > > Signed-off-by: Kevin Wolf > --- > block.c | 16 > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/block.c b/block.c > index 859e1aa..53d9bd5 100644 > --- a/block.c > +++ b/block.c > @@ -2123,14 +2123,15 @@ static bool > tracked_request_overlaps(BdrvTrackedRequest *req, > return true; > } > > -static void coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self) > +static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self) > { > BlockDriverState *bs = self->bs; > BdrvTrackedRequest *req; > bool retry; > +bool waited = false; > > if (!bs->serialising_in_flight) { > -return; > +return false; > } > > do { > @@ -2156,11 +2157,14 @@ static void coroutine_fn > wait_serialising_requests(BdrvTrackedRequest *self) > qemu_co_queue_wait(&req->wait_queue); > self->waiting_for = NULL; > retry = true; > +waited = true; > break; > } > } > } > } while (retry); > + > +return waited; > } > > /* > @@ -3011,6 +3015,7 @@ static int coroutine_fn > bdrv_aligned_pwritev(BlockDriverState *bs, > QEMUIOVector *qiov, int flags) > { > BlockDriver *drv = bs->drv; > +bool waited; > int ret; > > int64_t sector_num = offset >> BDRV_SECTOR_BITS; > @@ -3019,7 +3024,8 @@ static int coroutine_fn > bdrv_aligned_pwritev(BlockDriverState *bs, > assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > -wait_serialising_requests(req); > +waited = wait_serialising_requests(req); > +assert(!waited || !req->serialising); I we apply De Morgan's law to the expression we have: assert(!(waited && req->serialising)); Is it really the condition we want ? Best regards Benoît > > ret = notifier_with_return_list_notify(&bs->before_write_notifiers, req); > > @@ -3119,9 +3125,11 @@ static int coroutine_fn > bdrv_co_do_pwritev(BlockDriverState *bs, > QEMUIOVector tail_qiov; > struct iovec tail_iov; > size_t tail_bytes; > +bool waited; > > mark_request_serialising(&req, align); > -wait_serialising_requests(&req); > +waited = wait_serialising_requests(&req); > +assert(!waited || !use_local_qiov); > > tail_buf = qemu_blockalign(bs, align); > tail_iov = (struct iovec) { > -- > 1.8.1.4 > >
[Qemu-devel] [PATCH 5/5] kvm: add support for hyper-v timers
From: Vadim Rozenfeld http://msdn.microsoft.com/en-us/library/windows/hardware/ff541625%28v=vs.85%29.aspx This code is generic for activating reference time counter or virtual reference time stamp counter Signed-off-by: Vadim Rozenfeld Reviewed-by: Marcelo Tosatti Signed-off-by: Paolo Bonzini --- linux-headers/asm-x86/hyperv.h | 3 +++ linux-headers/linux/kvm.h | 1 + target-i386/cpu-qom.h | 1 + target-i386/cpu.c | 1 + target-i386/cpu.h | 1 + target-i386/kvm.c | 20 +++- target-i386/machine.c | 22 ++ 7 files changed, 48 insertions(+), 1 deletion(-) diff --git a/linux-headers/asm-x86/hyperv.h b/linux-headers/asm-x86/hyperv.h index b8f1c01..3b400ee 100644 --- a/linux-headers/asm-x86/hyperv.h +++ b/linux-headers/asm-x86/hyperv.h @@ -149,6 +149,9 @@ /* MSR used to read the per-partition time reference counter */ #define HV_X64_MSR_TIME_REF_COUNT 0x4020 +/* A partition's reference time stamp counter (TSC) page */ +#define HV_X64_MSR_REFERENCE_TSC 0x4021 + /* MSR used to retrieve the TSC frequency */ #define HV_X64_MSR_TSC_FREQUENCY 0x4022 diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h index 5a49671..999fb13 100644 --- a/linux-headers/linux/kvm.h +++ b/linux-headers/linux/kvm.h @@ -674,6 +674,7 @@ struct kvm_ppc_smmu_info { #define KVM_CAP_ARM_EL1_32BIT 93 #define KVM_CAP_SPAPR_MULTITCE 94 #define KVM_CAP_EXT_EMUL_CPUID 95 +#define KVM_CAP_HYPERV_TIME 96 #ifdef KVM_CAP_IRQ_ROUTING diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index d1751a4..722f11a 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -69,6 +69,7 @@ typedef struct X86CPU { bool hyperv_vapic; bool hyperv_relaxed_timing; int hyperv_spinlock_attempts; +bool hyperv_time; bool check_cpuid; bool enforce_cpuid; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 2e0be01..1f30efd 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2702,6 +2702,7 @@ static Property x86_cpu_properties[] = { { .name = "hv-spinlocks", .info = &qdev_prop_spinlocks }, DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false), DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false), +DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false), DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false), DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false), DEFINE_PROP_END_OF_LIST() diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 45bd554..1b94f0f 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -865,6 +865,7 @@ typedef struct CPUX86State { uint64_t msr_hv_hypercall; uint64_t msr_hv_guest_os_id; uint64_t msr_hv_vapic; +uint64_t msr_hv_tsc; /* exception/interrupt handling */ int error_code; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index ddd437f..e555040 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -74,6 +74,7 @@ static bool has_msr_kvm_steal_time; static int lm_capable_kernel; static bool has_msr_hv_hypercall; static bool has_msr_hv_vapic; +static bool has_msr_hv_tsc; static bool has_msr_architectural_pmu; static uint32_t num_architectural_pmu_counters; @@ -442,6 +443,7 @@ static bool hyperv_enabled(X86CPU *cpu) CPUState *cs = CPU(cpu); return kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV) > 0 && (hyperv_hypercall_available(cpu) || +cpu->hyperv_time || cpu->hyperv_relaxed_timing); } @@ -499,7 +501,13 @@ int kvm_arch_init_vcpu(CPUState *cs) c->eax |= HV_X64_MSR_APIC_ACCESS_AVAILABLE; has_msr_hv_vapic = true; } - +if (cpu->hyperv_time && +kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) { +c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE; +c->eax |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE; +c->eax |= 0x200; +has_msr_hv_tsc = true; +} c = &cpuid_data.entries[cpuid_i++]; c->function = HYPERV_CPUID_ENLIGHTMENT_INFO; if (cpu->hyperv_relaxed_timing) { @@ -1239,6 +1247,10 @@ static int kvm_put_msrs(X86CPU *cpu, int level) kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, env->msr_hv_vapic); } +if (has_msr_hv_tsc) { +kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_REFERENCE_TSC, + env->msr_hv_tsc); +} /* Note: MSR_IA32_FEATURE_CONTROL is written separately, see * kvm_put_msr_feature_control. */ @@ -1530,6 +1542,9 @@ static int kvm_get_msrs(X86CPU *cpu) if (has_msr_hv_vapic) { msrs[n++].index = HV_X64_MSR_APIC_ASSIST_PAGE; } +if (has_msr_hv_tsc) { +msrs[n++].index = HV_X64_MSR_REFERENCE_TSC; +} msr_data.info.nmsrs = n; ret = kvm
[Qemu-devel] [PATCH 4/5] kvm: make hyperv vapic assist page migratable
From: Vadim Rozenfeld Signed-off-by: Vadim Rozenfeld Signed-off-by: Paolo Bonzini --- target-i386/cpu.h | 1 + target-i386/kvm.c | 16 +--- target-i386/machine.c | 22 ++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 3dba5ef..45bd554 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -864,6 +864,7 @@ typedef struct CPUX86State { uint64_t msr_gp_evtsel[MAX_GP_COUNTERS]; uint64_t msr_hv_hypercall; uint64_t msr_hv_guest_os_id; +uint64_t msr_hv_vapic; /* exception/interrupt handling */ int error_code; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 8f2854a..ddd437f 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -73,6 +73,7 @@ static bool has_msr_bndcfgs; static bool has_msr_kvm_steal_time; static int lm_capable_kernel; static bool has_msr_hv_hypercall; +static bool has_msr_hv_vapic; static bool has_msr_architectural_pmu; static uint32_t num_architectural_pmu_counters; @@ -496,6 +497,7 @@ int kvm_arch_init_vcpu(CPUState *cs) if (cpu->hyperv_vapic) { c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE; c->eax |= HV_X64_MSR_APIC_ACCESS_AVAILABLE; +has_msr_hv_vapic = true; } c = &cpuid_data.entries[cpuid_i++]; @@ -503,7 +505,7 @@ int kvm_arch_init_vcpu(CPUState *cs) if (cpu->hyperv_relaxed_timing) { c->eax |= HV_X64_RELAXED_TIMING_RECOMMENDED; } -if (cpu->hyperv_vapic) { +if (has_msr_hv_vapic) { c->eax |= HV_X64_APIC_ACCESS_RECOMMENDED; } c->ebx = cpu->hyperv_spinlock_attempts; @@ -1233,8 +1235,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level) kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_HYPERCALL, env->msr_hv_hypercall); } -if (cpu->hyperv_vapic) { -kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0); +if (has_msr_hv_vapic) { +kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, + env->msr_hv_vapic); } /* Note: MSR_IA32_FEATURE_CONTROL is written separately, see @@ -1524,6 +1527,10 @@ static int kvm_get_msrs(X86CPU *cpu) msrs[n++].index = HV_X64_MSR_HYPERCALL; msrs[n++].index = HV_X64_MSR_GUEST_OS_ID; } +if (has_msr_hv_vapic) { +msrs[n++].index = HV_X64_MSR_APIC_ASSIST_PAGE; +} + msr_data.info.nmsrs = n; ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); if (ret < 0) { @@ -1637,6 +1644,9 @@ static int kvm_get_msrs(X86CPU *cpu) case HV_X64_MSR_GUEST_OS_ID: env->msr_hv_guest_os_id = msrs[i].data; break; +case HV_X64_MSR_APIC_ASSIST_PAGE: +env->msr_hv_vapic = msrs[i].data; +break; } } diff --git a/target-i386/machine.c b/target-i386/machine.c index 96fd045..e72e270 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -574,6 +574,25 @@ static const VMStateDescription vmstate_msr_hypercall_hypercall = { } }; +static bool hyperv_vapic_enable_needed(void *opaque) +{ +X86CPU *cpu = opaque; +CPUX86State *env = &cpu->env; + +return env->msr_hv_vapic != 0; +} + +static const VMStateDescription vmstate_msr_hyperv_vapic = { +.name = "cpu/msr_hyperv_vapic", +.version_id = 1, +.minimum_version_id = 1, +.minimum_version_id_old = 1, +.fields = (VMStateField []) { +VMSTATE_UINT64(env.msr_hv_vapic, X86CPU), +VMSTATE_END_OF_LIST() +} +}; + const VMStateDescription vmstate_x86_cpu = { .name = "cpu", .version_id = 12, @@ -711,6 +730,9 @@ const VMStateDescription vmstate_x86_cpu = { }, { .vmsd = &vmstate_msr_hypercall_hypercall, .needed = hyperv_hypercall_enable_needed, +}, { +.vmsd = &vmstate_msr_hyperv_vapic, +.needed = hyperv_vapic_enable_needed, } , { /* empty */ } -- 1.8.3.1
[Qemu-devel] [PATCH 3/5] kvm: make hyperv hypercall and guest os id MSRs migratable.
From: Vadim Rozenfeld Signed-off-by: Vadim Rozenfeld Signed-off-by: Paolo Bonzini --- target-i386/cpu.h | 2 ++ target-i386/kvm.c | 18 +++--- target-i386/machine.c | 23 +++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 1fcbc82..3dba5ef 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -862,6 +862,8 @@ typedef struct CPUX86State { uint64_t msr_fixed_counters[MAX_FIXED_COUNTERS]; uint64_t msr_gp_counters[MAX_GP_COUNTERS]; uint64_t msr_gp_evtsel[MAX_GP_COUNTERS]; +uint64_t msr_hv_hypercall; +uint64_t msr_hv_guest_os_id; /* exception/interrupt handling */ int error_code; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 08c47bb..8f2854a 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1227,9 +1227,11 @@ static int kvm_put_msrs(X86CPU *cpu, int level) kvm_msr_entry_set(&msrs[n++], MSR_CORE_PERF_GLOBAL_CTRL, env->msr_global_ctrl); } -if (hyperv_hypercall_available(cpu)) { -kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_GUEST_OS_ID, 0); -kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_HYPERCALL, 0); +if (has_msr_hv_hypercall) { +kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_GUEST_OS_ID, + env->msr_hv_guest_os_id); +kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_HYPERCALL, + env->msr_hv_hypercall); } if (cpu->hyperv_vapic) { kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0); @@ -1518,6 +1520,10 @@ static int kvm_get_msrs(X86CPU *cpu) } } +if (has_msr_hv_hypercall) { +msrs[n++].index = HV_X64_MSR_HYPERCALL; +msrs[n++].index = HV_X64_MSR_GUEST_OS_ID; +} msr_data.info.nmsrs = n; ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); if (ret < 0) { @@ -1625,6 +1631,12 @@ static int kvm_get_msrs(X86CPU *cpu) case MSR_P6_EVNTSEL0 ... MSR_P6_EVNTSEL0 + MAX_GP_COUNTERS - 1: env->msr_gp_evtsel[index - MSR_P6_EVNTSEL0] = msrs[i].data; break; +case HV_X64_MSR_HYPERCALL: +env->msr_hv_hypercall = msrs[i].data; +break; +case HV_X64_MSR_GUEST_OS_ID: +env->msr_hv_guest_os_id = msrs[i].data; +break; } } diff --git a/target-i386/machine.c b/target-i386/machine.c index 2de1964..96fd045 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -554,6 +554,26 @@ static const VMStateDescription vmstate_mpx = { } }; +static bool hyperv_hypercall_enable_needed(void *opaque) +{ +X86CPU *cpu = opaque; +CPUX86State *env = &cpu->env; + +return env->msr_hv_hypercall != 0 || env->msr_hv_guest_os_id != 0; +} + +static const VMStateDescription vmstate_msr_hypercall_hypercall = { +.name = "cpu/msr_hyperv_hypercall", +.version_id = 1, +.minimum_version_id = 1, +.minimum_version_id_old = 1, +.fields = (VMStateField []) { +VMSTATE_UINT64(env.msr_hv_hypercall, X86CPU), +VMSTATE_UINT64(env.msr_hv_guest_os_id, X86CPU), +VMSTATE_END_OF_LIST() +} +}; + const VMStateDescription vmstate_x86_cpu = { .name = "cpu", .version_id = 12, @@ -688,6 +708,9 @@ const VMStateDescription vmstate_x86_cpu = { } , { .vmsd = &vmstate_mpx, .needed = mpx_needed, +}, { +.vmsd = &vmstate_msr_hypercall_hypercall, +.needed = hyperv_hypercall_enable_needed, } , { /* empty */ } -- 1.8.3.1
Re: [Qemu-devel] [PATCH v3 21/29] block: Assert serialisation assumptions in pwritev
Am 24.01.2014 um 17:09 hat Benoît Canet geschrieben: > Le Friday 17 Jan 2014 à 15:15:11 (+0100), Kevin Wolf a écrit : > > If a request calls wait_serialising_requests() and actually has to wait > > in this function (i.e. a coroutine yield), other requests can run and > > previously read data (like the head or tail buffer) could become > > outdated. In this case, we would have to restart from the beginning to > > read in the updated data. > > > > However, we're lucky and don't actually need to do that: A request can > > only wait in the first call of wait_serialising_requests() because we > > mark it as serialising before that call, so any later requests would > > wait. So as we don't wait in practice, we don't have to reload the data. > > > > This is an important assumption that may not be broken or data > > corruption will happen. Document it with some assertions. > > > > Signed-off-by: Kevin Wolf > > --- > > block.c | 16 > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/block.c b/block.c > > index 859e1aa..53d9bd5 100644 > > --- a/block.c > > +++ b/block.c > > @@ -2123,14 +2123,15 @@ static bool > > tracked_request_overlaps(BdrvTrackedRequest *req, > > return true; > > } > > > > -static void coroutine_fn wait_serialising_requests(BdrvTrackedRequest > > *self) > > +static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest > > *self) > > { > > BlockDriverState *bs = self->bs; > > BdrvTrackedRequest *req; > > bool retry; > > +bool waited = false; > > > > if (!bs->serialising_in_flight) { > > -return; > > +return false; > > } > > > > do { > > @@ -2156,11 +2157,14 @@ static void coroutine_fn > > wait_serialising_requests(BdrvTrackedRequest *self) > > qemu_co_queue_wait(&req->wait_queue); > > self->waiting_for = NULL; > > retry = true; > > +waited = true; > > break; > > } > > } > > } > > } while (retry); > > + > > +return waited; > > } > > > > /* > > @@ -3011,6 +3015,7 @@ static int coroutine_fn > > bdrv_aligned_pwritev(BlockDriverState *bs, > > QEMUIOVector *qiov, int flags) > > { > > BlockDriver *drv = bs->drv; > > +bool waited; > > int ret; > > > > int64_t sector_num = offset >> BDRV_SECTOR_BITS; > > @@ -3019,7 +3024,8 @@ static int coroutine_fn > > bdrv_aligned_pwritev(BlockDriverState *bs, > > assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > > > -wait_serialising_requests(req); > > +waited = wait_serialising_requests(req); > > +assert(!waited || !req->serialising); > > I we apply De Morgan's law to the expression we have: > > assert(!(waited && req->serialising)); > > Is it really the condition we want ? Yes. If req->serialising is true here, it's an RMW case and we already had a mark_request_serialising() + wait_serialising_requests() pair. So we have already waited for earlier requests and newer requests must be waiting for us. Therefore, it can't happen that a serialising request has to wait here. Kevin
Re: [Qemu-devel] [PATCH] block: Fix bdrv_commit return value
On Fri, Jan 24, 2014 at 05:00:45PM +0100, Kevin Wolf wrote: > Am 24.01.2014 um 16:56 hat Jeff Cody geschrieben: > > On Fri, Jan 24, 2014 at 02:08:21PM +0100, Kevin Wolf wrote: > > > bdrv_commit() could return 0 or 1 on success, depending on whether or > > > now the last sector was allocated in the overlay and whether the overlay > > > format had a .bdrv_make_empty callback. > > > > > > Most callers ignored it, but qemu-img commit would print an error > > > message while the operation actually succeeded. > > > > > > Also clean up the handling of I/O errors to return the real error code > > > instead of -EIO. > > > > > > Signed-off-by: Kevin Wolf > > > --- > > > block.c | 15 ++- > > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > > > diff --git a/block.c b/block.c > > > index 3e0994b..7d22ca9 100644 > > > --- a/block.c > > > +++ b/block.c > > > @@ -2061,13 +2061,13 @@ int bdrv_commit(BlockDriverState *bs) > > > goto ro_cleanup; > > > } > > > if (ret) { > > > -if (bdrv_read(bs, sector, buf, n) != 0) { > > > -ret = -EIO; > > > +ret = bdrv_read(bs, sector, buf, n); > > > +if (ret < 0) { > > > goto ro_cleanup; > > > } > > > > > > -if (bdrv_write(bs->backing_hd, sector, buf, n) != 0) { > > > -ret = -EIO; > > > +ret = bdrv_write(bs->backing_hd, sector, buf, n); > > > +if (ret < 0) { > > > goto ro_cleanup; > > > } > > > } > > > @@ -2075,6 +2075,9 @@ int bdrv_commit(BlockDriverState *bs) > > > > > > if (drv->bdrv_make_empty) { > > > ret = drv->bdrv_make_empty(bs); > > > +if (ret < 0) { > > > +goto ro_cleanup; > > > +} > > > > QED has a .bdrv_make_empty implementation, but it is a stub that > > always returns -ENOTSUP. > > > > Prior to this patch, a commit of a QED snapshot would complete OK, but > > return an error saying "Image is already committed" (Which almost > > seems like a non-error error). > > > > Now, we'll skip ahead to cleanup. > > > > I think we should either: 1) filter out -ENOTSUP here, or 2) remove > > the stub from QED > > I vote for fixing QED, this stub doesn't make any sense and never > matched the interface that was actually used. And I guess we can remove > it in qcow2 as well, there it's a simple return 0 with the rest of the > function #if'd out. > I agree, that would be cleanest approach, and would hopefully avoid similar errors elsewhere later on. Jeff
[Qemu-devel] [PATCH v2 0/4] mips32r5 with UFR
Version 2 of the patch series to add mips32r5-generic model with UFR feature. It includes extra patch to add support for Config4. Petar Jovanovic (4): target-mips: add CPU definition for MIPS32R5 target-mips: add support for CP0_Config4 target-mips: add support for CP0_Config5 target-mips: add user-mode FR switch support for MIPS32r5 target-mips/cpu.h| 13 +++ target-mips/helper.h |4 +++- target-mips/mips-defs.h |8 +++ target-mips/op_helper.c | 53 +++--- target-mips/translate.c | 39 +++ target-mips/translate_init.c | 43 ++ 6 files changed, 152 insertions(+), 8 deletions(-) -- 1.7.9.5
[Qemu-devel] [PATCH v2 4/4] target-mips: add user-mode FR switch support for MIPS32r5
From: Petar Jovanovic Description of UFR feature: Required in MIPS32r5 if floating point is implemented and user-mode FR switching is supported. The UFR register allows user-mode to clear StatusFR by executing a CTC1 to UFR with GPR[0] as input, and read StatusFR by executing a CFC1 to UFR. helper_ctc1 has been extended with an additional parameter rt to check requirements for UFR feature. Definition of mips32r5-generic has been modified to include support for UFR. Signed-off-by: Petar Jovanovic --- target-mips/helper.h |2 +- target-mips/op_helper.c | 41 ++--- target-mips/translate.c | 14 -- target-mips/translate_init.c |9 + 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/target-mips/helper.h b/target-mips/helper.h index b82f8e8..8c7921a 100644 --- a/target-mips/helper.h +++ b/target-mips/helper.h @@ -179,7 +179,7 @@ DEF_HELPER_2(yield, tl, env, tl) /* CP1 functions */ DEF_HELPER_2(cfc1, tl, env, i32) -DEF_HELPER_3(ctc1, void, env, tl, i32) +DEF_HELPER_4(ctc1, void, env, tl, i32, i32) DEF_HELPER_2(float_cvtd_s, i64, env, i32) DEF_HELPER_2(float_cvtd_w, i64, env, i32) diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c index eaf4d26..2ef6633 100644 --- a/target-mips/op_helper.c +++ b/target-mips/op_helper.c @@ -2199,12 +2199,23 @@ static inline void restore_flush_mode(CPUMIPSState *env) target_ulong helper_cfc1(CPUMIPSState *env, uint32_t reg) { -target_ulong arg1; +target_ulong arg1 = 0; switch (reg) { case 0: arg1 = (int32_t)env->active_fpu.fcr0; break; +case 1: +/* UFR Support - Read Status FR */ +if (env->active_fpu.fcr0 & (1 << FCR0_UFRP)) { +if (env->CP0_Config5 & (1 << CP0C5_UFR)) { +arg1 = (int32_t) + ((env->CP0_Status & (1 << CP0St_FR)) >> CP0St_FR); +} else { +helper_raise_exception(env, EXCP_RI); +} +} +break; case 25: arg1 = ((env->active_fpu.fcr31 >> 24) & 0xfe) | ((env->active_fpu.fcr31 >> 23) & 0x1); break; @@ -,9 +2233,33 @@ target_ulong helper_cfc1(CPUMIPSState *env, uint32_t reg) return arg1; } -void helper_ctc1(CPUMIPSState *env, target_ulong arg1, uint32_t reg) +void helper_ctc1(CPUMIPSState *env, target_ulong arg1, uint32_t fs, uint32_t rt) { -switch(reg) { +switch (fs) { +case 1: +/* UFR Alias - Reset Status FR */ +if (!((env->active_fpu.fcr0 & (1 << FCR0_UFRP)) && (rt == 0))) { +return; +} +if (env->CP0_Config5 & (1 << CP0C5_UFR)) { +env->CP0_Status &= ~(1 << CP0St_FR); +compute_hflags(env); +} else { +helper_raise_exception(env, EXCP_RI); +} +break; +case 4: +/* UNFR Alias - Set Status FR */ +if (!((env->active_fpu.fcr0 & (1 << FCR0_UFRP)) && (rt == 0))) { +return; +} +if (env->CP0_Config5 & (1 << CP0C5_UFR)) { +env->CP0_Status |= (1 << CP0St_FR); +compute_hflags(env); +} else { +helper_raise_exception(env, EXCP_RI); +} +break; case 25: if (arg1 & 0xff00) return; diff --git a/target-mips/translate.c b/target-mips/translate.c index 02a90cb..083f6ab 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -6818,7 +6818,12 @@ static void gen_mttr(CPUMIPSState *env, DisasContext *ctx, int rd, int rt, break; case 3: /* XXX: For now we support only a single FPU context. */ -gen_helper_0e1i(ctc1, t0, rd); +{ +TCGv_i32 fs_tmp = tcg_const_i32(rd); + +gen_helper_0e2i(ctc1, t0, fs_tmp, rt); +tcg_temp_free_i32(fs_tmp); +} break; /* COP2: Not implemented. */ case 4: @@ -7254,7 +7259,12 @@ static void gen_cp1 (DisasContext *ctx, uint32_t opc, int rt, int fs) break; case OPC_CTC1: gen_load_gpr(t0, rt); -gen_helper_0e1i(ctc1, t0, fs); +{ +TCGv_i32 fs_tmp = tcg_const_i32(fs); + +gen_helper_0e2i(ctc1, t0, fs_tmp, rt); +tcg_temp_free_i32(fs_tmp); +} opn = "ctc1"; break; #if defined(TARGET_MIPS64) diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index 3d4dc88..29d39e2 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -358,18 +358,19 @@ static const mips_def_t mips_defs[] = .CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_M), .CP0_Config4 = MIPS_CONFIG4 | (1 << CP0C4_M), .CP0_Config4_rw_bitmask = 0, -.CP0_Config5 = MIPS_CONFIG5, +.CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_UFR), .CP0_Config5_rw_bitmask = (0 << CP0C5_M) | (1 << CP0C5_K) | (1 << CP0C5_CV) | (0 << CP0C5_EVA) | -
Re: [Qemu-devel] [PATCH v3 25/29] iscsi: Set bs->request_alignment
Le Friday 17 Jan 2014 à 15:15:15 (+0100), Kevin Wolf a écrit : > From: Paolo Bonzini > > The iSCSI backend already gets the block size from the READ CAPACITY > command it sends. Save it so that the generic block layer gets it > too. > > Signed-off-by: Paolo Bonzini > Signed-off-by: Kevin Wolf > Reviewed-by: Max Reitz > --- > block/iscsi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 3202dc5..23c3fc4 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1217,6 +1217,7 @@ static int iscsi_open(BlockDriverState *bs, QDict > *options, int flags, > goto out; > } > bs->total_sectors = sector_lun2qemu(iscsilun->num_blocks, iscsilun); > +bs->request_alignment = iscsilun->block_size; > > /* Medium changer or tape. We dont have any emulation for this so this > must > * be sg ioctl compatible. We force it to be sg, otherwise qemu will try > -- > 1.8.1.4 > > Reviewed-by: Benoit Canet
[Qemu-devel] [PATCH v2 1/4] target-mips: add CPU definition for MIPS32R5
From: Petar Jovanovic Add mips32r5-generic among CPU definitions for MIPS. Define ISA_MIPS32R3 and ISA_MIPS32R5. Signed-off-by: Petar Jovanovic --- target-mips/mips-defs.h |8 target-mips/translate_init.c | 25 + 2 files changed, 33 insertions(+) diff --git a/target-mips/mips-defs.h b/target-mips/mips-defs.h index bf094a3..9dfa516 100644 --- a/target-mips/mips-defs.h +++ b/target-mips/mips-defs.h @@ -29,6 +29,8 @@ #defineISA_MIPS32R20x0040 #defineISA_MIPS64 0x0080 #defineISA_MIPS64R20x0100 +#define ISA_MIPS32R3 0x0200 +#define ISA_MIPS32R5 0x0400 /* MIPS ASEs. */ #defineASE_MIPS16 0x1000 @@ -64,6 +66,12 @@ #defineCPU_MIPS32R2(CPU_MIPS32 | ISA_MIPS32R2) #defineCPU_MIPS64R2(CPU_MIPS64 | CPU_MIPS32R2 | ISA_MIPS64R2) +/* MIPS Technologies "Release 3" */ +#define CPU_MIPS32R3 (CPU_MIPS32R2 | ISA_MIPS32R3) + +/* MIPS Technologies "Release 5" */ +#define CPU_MIPS32R5 (CPU_MIPS32R3 | ISA_MIPS32R5) + /* Strictly follow the architecture standard: - Disallow "special" instruction handling for PMON/SPIM. Note that we still maintain Count/Compare to match the host clock. */ diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index c45b1b2..d74a0af 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -333,6 +333,31 @@ static const mips_def_t mips_defs[] = .insn_flags = CPU_MIPS32R2 | ASE_MIPS16 | ASE_DSP | ASE_DSPR2, .mmu_type = MMU_TYPE_R4000, }, +{ +/* A generic CPU providing MIPS32 Release 5 features. + FIXME: Eventually this should be replaced by a real CPU model. */ +.name = "mips32r5-generic", +.CP0_PRid = 0x00019700, +.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | +(MMU_TYPE_R4000 << CP0C0_MT), +.CP0_Config1 = MIPS_CONFIG1 | (1 << CP0C1_FP) | (15 << CP0C1_MMU) | + (0 << CP0C1_IS) | (3 << CP0C1_IL) | (1 << CP0C1_IA) | + (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) | + (1 << CP0C1_CA), +.CP0_Config2 = MIPS_CONFIG2, +.CP0_Config3 = MIPS_CONFIG3, +.CP0_LLAddr_rw_bitmask = 0, +.CP0_LLAddr_shift = 4, +.SYNCI_Step = 32, +.CCRes = 2, +.CP0_Status_rw_bitmask = 0x3778FF1F, +.CP1_fcr0 = (1 << FCR0_F64) | (1 << FCR0_L) | (1 << FCR0_W) | +(1 << FCR0_D) | (1 << FCR0_S) | (0x93 << FCR0_PRID), +.SEGBITS = 32, +.PABITS = 32, +.insn_flags = CPU_MIPS32R5 | ASE_MIPS16 | ASE_DSP | ASE_DSPR2, +.mmu_type = MMU_TYPE_R4000, +}, #if defined(TARGET_MIPS64) { .name = "R4000", -- 1.7.9.5
[Qemu-devel] [PATCH v2 2/4] target-mips: add support for CP0_Config4
From: Petar Jovanovic Add CP0_Config4, define rw_bitmask. Signed-off-by: Petar Jovanovic --- target-mips/cpu.h|3 +++ target-mips/helper.h |1 + target-mips/op_helper.c |6 ++ target-mips/translate.c | 15 +-- target-mips/translate_init.c |9 - 5 files changed, 31 insertions(+), 3 deletions(-) diff --git a/target-mips/cpu.h b/target-mips/cpu.h index 9caf447..e8216ab 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -368,6 +368,9 @@ struct CPUMIPSState { #define CP0C3_MT 2 #define CP0C3_SM 1 #define CP0C3_TL 0 +uint32_t CP0_Config4; +uint32_t CP0_Config4_rw_bitmask; +#define CP0C4_M31 int32_t CP0_Config6; int32_t CP0_Config7; /* XXX: Maybe make LLAddr per-TC? */ diff --git a/target-mips/helper.h b/target-mips/helper.h index 1a8b86d..9e4508b 100644 --- a/target-mips/helper.h +++ b/target-mips/helper.h @@ -134,6 +134,7 @@ DEF_HELPER_2(mtc0_ebase, void, env, tl) DEF_HELPER_2(mttc0_ebase, void, env, tl) DEF_HELPER_2(mtc0_config0, void, env, tl) DEF_HELPER_2(mtc0_config2, void, env, tl) +DEF_HELPER_2(mtc0_config4, void, env, tl) DEF_HELPER_2(mtc0_lladdr, void, env, tl) DEF_HELPER_3(mtc0_watchlo, void, env, tl, i32) DEF_HELPER_3(mtc0_watchhi, void, env, tl, i32) diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c index 8e3a6d7..ed8dde8 100644 --- a/target-mips/op_helper.c +++ b/target-mips/op_helper.c @@ -1489,6 +1489,12 @@ void helper_mtc0_config2(CPUMIPSState *env, target_ulong arg1) env->CP0_Config2 = (env->CP0_Config2 & 0x8FFF0FFF); } +void helper_mtc0_config4(CPUMIPSState *env, target_ulong arg1) +{ +env->CP0_Config4 = (env->CP0_Config4 & (~env->CP0_Config4_rw_bitmask)) | + (arg1 & env->CP0_Config4_rw_bitmask); +} + void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1) { target_long mask = env->CP0_LLAddr_rw_bitmask; diff --git a/target-mips/translate.c b/target-mips/translate.c index ef0a2c3..db2f430 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -4405,7 +4405,11 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_Config3)); rn = "Config3"; break; -/* 4,5 are reserved */ +case 4: +gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_Config4)); +rn = "Config4"; +break; +/* 5 is reserved */ /* 6,7 are implementation dependent */ case 6: gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_Config6)); @@ -4982,7 +4986,12 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) /* ignored, read only */ rn = "Config3"; break; -/* 4,5 are reserved */ +case 4: +gen_helper_mtc0_config4(cpu_env, arg); +rn = "Config4"; +ctx->bstate = BS_STOP; +break; +/* 5 is reserved */ /* 6,7 are implementation dependent */ case 6: /* ignored */ @@ -15916,6 +15925,8 @@ void cpu_state_reset(CPUMIPSState *env) env->CP0_Config1 = env->cpu_model->CP0_Config1; env->CP0_Config2 = env->cpu_model->CP0_Config2; env->CP0_Config3 = env->cpu_model->CP0_Config3; +env->CP0_Config4 = env->cpu_model->CP0_Config4; +env->CP0_Config4_rw_bitmask = env->cpu_model->CP0_Config4_rw_bitmask; env->CP0_Config6 = env->cpu_model->CP0_Config6; env->CP0_Config7 = env->cpu_model->CP0_Config7; env->CP0_LLAddr_rw_bitmask = env->cpu_model->CP0_LLAddr_rw_bitmask diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index d74a0af..a0398cd 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -45,6 +45,9 @@ (0 << CP0C3_VEIC) | (0 << CP0C3_VInt) | (0 << CP0C3_SP) |\ (0 << CP0C3_SM) | (0 << CP0C3_TL)) +#define MIPS_CONFIG4 \ +((0 << CP0C4_M)) + /* MMU types, the first four entries have the same layout as the CP0C0_MT field. */ enum mips_mmu_types { @@ -64,6 +67,8 @@ struct mips_def_t { int32_t CP0_Config1; int32_t CP0_Config2; int32_t CP0_Config3; +int32_t CP0_Config4; +int32_t CP0_Config4_rw_bitmask; int32_t CP0_Config6; int32_t CP0_Config7; target_ulong CP0_LLAddr_rw_bitmask; @@ -345,7 +350,9 @@ static const mips_def_t mips_defs[] = (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) | (1 << CP0C1_CA), .CP0_Config2 = MIPS_CONFIG2, -.CP0_Config3 = MIPS_CONFIG3, +.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_M), +.CP0_Config4 = MIPS_CONFIG4, +.CP0_Config4_rw_bitmask = 0, .CP0_LLAddr_rw_bitmask = 0, .CP0_LLAddr_shift = 4, .SYNCI_Step = 32, -- 1.7.9.5
[Qemu-devel] [PATCH v2 3/4] target-mips: add support for CP0_Config5
From: Petar Jovanovic Add CP0_Config5, define rw_bitmask and enable modifications. Signed-off-by: Petar Jovanovic --- target-mips/cpu.h| 10 ++ target-mips/helper.h |1 + target-mips/op_helper.c |6 ++ target-mips/translate.c | 14 -- target-mips/translate_init.c | 12 +++- 5 files changed, 40 insertions(+), 3 deletions(-) diff --git a/target-mips/cpu.h b/target-mips/cpu.h index e8216ab..60c8061 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -73,6 +73,7 @@ struct CPUMIPSFPUContext { float_status fp_status; /* fpu implementation/revision register (fir) */ uint32_t fcr0; +#define FCR0_UFRP 28 #define FCR0_F64 22 #define FCR0_L 21 #define FCR0_W 20 @@ -371,6 +372,15 @@ struct CPUMIPSState { uint32_t CP0_Config4; uint32_t CP0_Config4_rw_bitmask; #define CP0C4_M31 +uint32_t CP0_Config5; +uint32_t CP0_Config5_rw_bitmask; +#define CP0C5_M 31 +#define CP0C5_K 30 +#define CP0C5_CV 29 +#define CP0C5_EVA28 +#define CP0C5_MSAEn 27 +#define CP0C5_UFR2 +#define CP0C5_NFExists 0 int32_t CP0_Config6; int32_t CP0_Config7; /* XXX: Maybe make LLAddr per-TC? */ diff --git a/target-mips/helper.h b/target-mips/helper.h index 9e4508b..b82f8e8 100644 --- a/target-mips/helper.h +++ b/target-mips/helper.h @@ -135,6 +135,7 @@ DEF_HELPER_2(mttc0_ebase, void, env, tl) DEF_HELPER_2(mtc0_config0, void, env, tl) DEF_HELPER_2(mtc0_config2, void, env, tl) DEF_HELPER_2(mtc0_config4, void, env, tl) +DEF_HELPER_2(mtc0_config5, void, env, tl) DEF_HELPER_2(mtc0_lladdr, void, env, tl) DEF_HELPER_3(mtc0_watchlo, void, env, tl, i32) DEF_HELPER_3(mtc0_watchhi, void, env, tl, i32) diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c index ed8dde8..eaf4d26 100644 --- a/target-mips/op_helper.c +++ b/target-mips/op_helper.c @@ -1495,6 +1495,12 @@ void helper_mtc0_config4(CPUMIPSState *env, target_ulong arg1) (arg1 & env->CP0_Config4_rw_bitmask); } +void helper_mtc0_config5(CPUMIPSState *env, target_ulong arg1) +{ +env->CP0_Config5 = (env->CP0_Config5 & (~env->CP0_Config5_rw_bitmask)) | + (arg1 & env->CP0_Config5_rw_bitmask); +} + void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1) { target_long mask = env->CP0_LLAddr_rw_bitmask; diff --git a/target-mips/translate.c b/target-mips/translate.c index db2f430..02a90cb 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -4409,7 +4409,10 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_Config4)); rn = "Config4"; break; -/* 5 is reserved */ +case 5: +gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_Config5)); +rn = "Config5"; +break; /* 6,7 are implementation dependent */ case 6: gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_Config6)); @@ -4991,7 +4994,12 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) rn = "Config4"; ctx->bstate = BS_STOP; break; -/* 5 is reserved */ +case 5: +gen_helper_mtc0_config5(cpu_env, arg); +rn = "Config5"; +/* Stop translation as we may have switched the execution mode */ +ctx->bstate = BS_STOP; +break; /* 6,7 are implementation dependent */ case 6: /* ignored */ @@ -15927,6 +15935,8 @@ void cpu_state_reset(CPUMIPSState *env) env->CP0_Config3 = env->cpu_model->CP0_Config3; env->CP0_Config4 = env->cpu_model->CP0_Config4; env->CP0_Config4_rw_bitmask = env->cpu_model->CP0_Config4_rw_bitmask; +env->CP0_Config5 = env->cpu_model->CP0_Config5; +env->CP0_Config5_rw_bitmask = env->cpu_model->CP0_Config5_rw_bitmask; env->CP0_Config6 = env->cpu_model->CP0_Config6; env->CP0_Config7 = env->cpu_model->CP0_Config7; env->CP0_LLAddr_rw_bitmask = env->cpu_model->CP0_LLAddr_rw_bitmask diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index a0398cd..3d4dc88 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -48,6 +48,9 @@ #define MIPS_CONFIG4 \ ((0 << CP0C4_M)) +#define MIPS_CONFIG5 \ +((0 << CP0C5_M)) + /* MMU types, the first four entries have the same layout as the CP0C0_MT field. */ enum mips_mmu_types { @@ -69,6 +72,8 @@ struct mips_def_t { int32_t CP0_Config3; int32_t CP0_Config4; int32_t CP0_Config4_rw_bitmask; +int32_t CP0_Config5; +int32_t CP0_Config5_rw_bitmask; int32_t CP0_Config6; int32_t CP0_Config7; target_ulong CP0_LLAddr_rw_bitmask; @@ -351,8 +356,13 @@ static const mips_def_t mips_defs[] =
Re: [Qemu-devel] [PATCH] ACPI: Add IRQ resource to HPET._CRS on Mac OS X
On Tue, Jan 21, 2014 at 08:38:51PM +0200, Michael S. Tsirkin wrote: > On Tue, Jan 21, 2014 at 01:11:01PM -0500, Gabriel L. Somlo wrote: > > Apple hardware invariably adds "IRQNoFlags() {0, 8}" to HPET._CRS, > > and, at least on piix+smp, an OS X guest will panic unless IRQNoFlags > > is present. On the other hand, Windows XP bluescreens whenever > > IRQNoFlags is present. This patch conditionally includes IRQNoFlags > > only when detecting the presence of an AppleSMC (as a heuristic > > predictor for running a Mac OS X guest). Querying _OS and/or _OSI > > directly was considered and rejected because such queries MAY NOT > > be stateless from the OSPM standpoint, leading to potentially > > unpredictable behavior. > > > > Signed-off-by: Gabriel Somlo > > Fine with me. > Acked-by: Michael S. Tsirkin > > I'll pick this up if no one complains. In the mean time I updated the bootloader I was using (chameleon) to the latest svn (2345), and was able to bring up Lion in addition to SnowLeopard. On Lion, without this patch (i.e. without IRQNoFlags on the HPET), all four combinations (piix vs q35) X (up vs. smp) work fine. I'd like to hold off on having this applied for now, until I get a chance to figure out whether it matters moving forward (MountainLion, Mavericks, etc). I'm not sure it's worth polluting the HPET DSDT node with the "(LEqual(\_SB.PCI0.ISA.SMC._STA, 0x0B))" hack, only to support (piix + smp) on an old version of OS X. I'll follow up if anything new and interesting happens, but please ignore it it for now... Thanks, --Gabriel
Re: [Qemu-devel] [PATCH v2] cpu: implementing victim TLB for QEMUsystem emulated TLBB
On 01/24/2014 04:15 AM, Alex Bennée wrote: > If you can attach the summary of the data as plain text that would be > useful. Not all of us have access to a Windows box with Excell! To be fair, it can be opened with openoffice. r~
[Qemu-devel] [PATCH 1/5] KVM: fix coexistence of KVM and Hyper-V leaves
kvm_arch_init_vcpu's initialization of the KVM leaves at 0x4100 is broken, because KVM_CPUID_FEATURES is left at 0x4001. Move it to 0x4101 if Hyper-V is enabled. Signed-off-by: Paolo Bonzini --- target-i386/kvm.c | 47 +-- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 0a21c30..5738911 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -455,6 +455,7 @@ int kvm_arch_init_vcpu(CPUState *cs) uint32_t unused; struct kvm_cpuid_entry2 *c; uint32_t signature[3]; +int kvm_base = KVM_CPUID_SIGNATURE; int r; memset(&cpuid_data, 0, sizeof(cpuid_data)); @@ -462,26 +463,22 @@ int kvm_arch_init_vcpu(CPUState *cs) cpuid_i = 0; /* Paravirtualization CPUIDs */ -c = &cpuid_data.entries[cpuid_i++]; -c->function = KVM_CPUID_SIGNATURE; -if (!hyperv_enabled(cpu)) { -memcpy(signature, "KVMKVMKVM\0\0\0", 12); -c->eax = 0; -} else { +if (hyperv_enabled(cpu)) { +c = &cpuid_data.entries[cpuid_i++]; +c->function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS; memcpy(signature, "Microsoft Hv", 12); c->eax = HYPERV_CPUID_MIN; -} -c->ebx = signature[0]; -c->ecx = signature[1]; -c->edx = signature[2]; - -c = &cpuid_data.entries[cpuid_i++]; -c->function = KVM_CPUID_FEATURES; -c->eax = env->features[FEAT_KVM]; +c->ebx = signature[0]; +c->ecx = signature[1]; +c->edx = signature[2]; -if (hyperv_enabled(cpu)) { +c = &cpuid_data.entries[cpuid_i++]; +c->function = HYPERV_CPUID_INTERFACE; memcpy(signature, "Hv#1\0\0\0\0\0\0\0\0", 12); c->eax = signature[0]; +c->ebx = 0; +c->ecx = 0; +c->edx = 0; c = &cpuid_data.entries[cpuid_i++]; c->function = HYPERV_CPUID_VERSION; @@ -513,15 +510,21 @@ int kvm_arch_init_vcpu(CPUState *cs) c->eax = 0x40; c->ebx = 0x40; -c = &cpuid_data.entries[cpuid_i++]; -c->function = KVM_CPUID_SIGNATURE_NEXT; -memcpy(signature, "KVMKVMKVM\0\0\0", 12); -c->eax = 0; -c->ebx = signature[0]; -c->ecx = signature[1]; -c->edx = signature[2]; +kvm_base = KVM_CPUID_SIGNATURE_NEXT; } +memcpy(signature, "KVMKVMKVM\0\0\0", 12); +c = &cpuid_data.entries[cpuid_i++]; +c->function = KVM_CPUID_SIGNATURE | kvm_base; +c->eax = 0; +c->ebx = signature[0]; +c->ecx = signature[1]; +c->edx = signature[2]; + +c = &cpuid_data.entries[cpuid_i++]; +c->function = KVM_CPUID_FEATURES | kvm_base; +c->eax = env->features[FEAT_KVM]; + has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF); has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI); -- 1.8.3.1
[Qemu-devel] [PATCH uq/master 0/5] Hyper-V improvements and migratability
The first patch fixes the KVM leaves at 0x4100. Before, there is no leaf at 0x4101 (and the data of the highest Intel leaf is returned, e.g. 0xd on a Sandy Bridge). After this patch there is one. The second patch is extracted from Vadim's migration patches, which are patches 3-5. Review of the first two patches is particularly welcome. Paolo Bonzini (2): KVM: fix coexistence of KVM and Hyper-V leaves kvm: make availability of Hyper-V enlightenments dependent on KVM_CAP_HYPERV Vadim Rozenfeld (3): kvm: make hyperv hypercall and guest os id MSRs migratable. kvm: make hyperv vapic assist page migratable kvm: add support for hyper-v timers linux-headers/asm-x86/hyperv.h | 3 ++ linux-headers/linux/kvm.h | 1 + target-i386/cpu-qom.h | 1 + target-i386/cpu.c | 1 + target-i386/cpu.h | 4 ++ target-i386/kvm.c | 109 + target-i386/machine.c | 67 + 7 files changed, 155 insertions(+), 31 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 2/5] kvm: make availability of Hyper-V enlightenments dependent on KVM_CAP_HYPERV
The MS docs specify HV_X64_MSR_HYPERCALL as a mandatory interface, thus we must provide the MSRs even if the user only specified features that, like relaxed timing, in principle don't require them. And the MSRs are only there if the hypervisor has KVM_CAP_HYPERV. Signed-off-by: Paolo Bonzini --- target-i386/kvm.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 5738911..08c47bb 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -72,6 +72,7 @@ static bool has_msr_misc_enable; static bool has_msr_bndcfgs; static bool has_msr_kvm_steal_time; static int lm_capable_kernel; +static bool has_msr_hv_hypercall; static bool has_msr_architectural_pmu; static uint32_t num_architectural_pmu_counters; @@ -437,8 +438,10 @@ static bool hyperv_hypercall_available(X86CPU *cpu) static bool hyperv_enabled(X86CPU *cpu) { -return hyperv_hypercall_available(cpu) || - cpu->hyperv_relaxed_timing; +CPUState *cs = CPU(cpu); +return kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV) > 0 && + (hyperv_hypercall_available(cpu) || +cpu->hyperv_relaxed_timing); } #define KVM_MAX_CPUID_ENTRIES 100 @@ -511,6 +514,7 @@ int kvm_arch_init_vcpu(CPUState *cs) c->ebx = 0x40; kvm_base = KVM_CPUID_SIGNATURE_NEXT; +has_msr_hv_hypercall = true; } memcpy(signature, "KVMKVMKVM\0\0\0", 12); -- 1.8.3.1
Re: [Qemu-devel] [PATCH RFC 5/5] xen: introduce xenpv-softmmu.mak
On Fri, Jan 24, 2014 at 08:38:19AM +0100, Paolo Bonzini wrote: > Il 23/01/2014 23:16, Wei Liu ha scritto: > >-echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak" > >+if test "$target_name" != "xenpv"; then > >+echo "CONFIG_XEN_I386=y" >> $config_target_mak > >+if test "$xen_pci_passthrough" = yes; then > >+echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak" > >+fi > > You can add > > CONFIG_XEN_PCI_PASSTHROUGH=$(CONFIG_XEN) > > to i386-softmmu.mak and x86_64-softmmu.mak, and drop this setting > from config-target.mak too. > I'm afraid not. CONFIG_XEN is prerequiste for CONFIG_XEN_PCI_PASSTHROUGH but doens't imply the feature is enabled by the user. Wei. > Paolo
[Qemu-devel] [PULL v2 00/93] Block patches
This supersedes my pull request from two weeks ago. The following changes since commit 732c66ce641c69702a7e7fdb73b68f0c1b583ab5: Revert "error: Don't use error_report() for assertion msgs." (2014-01-17 09:50:11 +1000) are available in the git repository at: git://repo.or.cz/qemu/kevin.git tags/for-anthony for you to fetch changes up to d5103588aa39157c8eea3bb5fb6780bbd8be21b7: block: Switch bdrv_io_limits_intercept() to byte granularity (2014-01-24 17:40:28 +0100) Block patches Benoît Canet (7): block: Add bs->node_name to hold the name of a bs node of the bs graph. block: Allow the user to define "node-name" option both on command line and QMP. qmp: Add QMP query-named-block-nodes to list the named BlockDriverState nodes. qmp: Allow to change password on named block driver states. block: Create authorizations mechanism for external snapshot and resize. qmp: Allow block_resize to manipulate bs graph nodes. qmp: Allow to take external snapshots on bs graphs node. Bharata B Rao (3): gluster: Convert aio routines into coroutines gluster: Implement .bdrv_co_write_zeroes for gluster gluster: Add support for creating zero-filled image Fam Zheng (7): qemu-iotests: Introduce _unsupported_imgopts qemu-iotests: Add _unsupported_imgopts for vmdk subformats qemu-iotests: Clean up all extents for vmdk vmdk: Fix big flat extent IO vmdk: Check for overhead when opening vmdk: Fix format specific information (create type) for streamOptimized qapi: Add "backing" to BlockStats Hu Tao (1): qcow2: fix wrong value of L1E_OFFSET_MASK, L2E_OFFSET_MASK and REFT_OFFSET_MASK Jeff Cody (3): block: resize backing file image during offline commit, if necessary block: resize backing image during active layer commit, if needed block: update block commit documentation regarding image truncation Kevin Wolf (30): qemu-progress: Drop unused include qemu-progress: Fix progress printing on SIGUSR1 Documentation: qemu-img: Mention SIGUSR1 progress report block: Fix bdrv_commit return value block: Move initialisation of BlockLimits to bdrv_refresh_limits() block: Inherit opt_transfer_length block: Update BlockLimits when they might have changed qemu_memalign: Allow small alignments block: Detect unaligned length in bdrv_qiov_is_aligned() block: Don't use guest sector size for qemu_blockalign() block: Introduce bdrv_aligned_preadv() block: Introduce bdrv_co_do_preadv() block: Introduce bdrv_aligned_pwritev() block: write: Handle COR dependency after I/O throttling block: Introduce bdrv_co_do_pwritev() block: Switch BdrvTrackedRequest to byte granularity block: Allow waiting for overlapping requests between begin/end block: Make zero-after-EOF work with larger alignment block: Generalise and optimise COR serialisation block: Make overlap range for serialisation dynamic block: Allow wait_serialising_requests() at any point block: Align requests in bdrv_co_do_pwritev() block: Assert serialisation assumptions in pwritev block: Change coroutine wrapper to byte granularity block: Make bdrv_pread() a bdrv_prwv_co() wrapper block: Make bdrv_pwrite() a bdrv_prwv_co() wrapper blkdebug: Make required alignment configurable qemu-io: New command 'sleep' qemu-iotests: Test pwritev RMW logic block: Switch bdrv_io_limits_intercept() to byte granularity Kewei Yu (1): qtest: Fix the bug about disable vnc causes "make check" fail Liu Yuan (2): sheepdog: fix clone operation by 'qemu-img create -b' sheepdog: fix 'qemu-img map' Max Reitz (24): blkdebug: Use errp for read_config() blkdebug: Don't require sophisticated filename qdict: Add qdict_array_split() qapi: extend qdict_flatten() for QLists qemu-option: Add qemu_config_parse_qdict() blkdebug: Always call read_config() blkdebug: Use command-line in read_config() block: Allow reference for bdrv_file_open() block: Pass reference to bdrv_file_open() block: Allow block devices without files block: Add bdrv_open_image() block: Use bdrv_open_image() in bdrv_open() block: Allow recursive "file"s blockdev: Move "file" to legacy_opts blkdebug: Allow command-line file configuration blkverify: Allow command-line configuration blkverify: Don't require protocol filename qapi: Add "errno" to the list of polluted words qapi: QMP interface for blkdebug and blkverify qemu-io: Make filename optional tests: Add test for qdict_array_split() tests: Add test for qdict_flatten() iotests: Test new blkdebug/blkverify interface
[Qemu-devel] [PULL 04/93] qemu-iotests: Clean up all extents for vmdk
From: Fam Zheng This modifies _cleanup_test_img to remove all the extent files listed by "qemu-img info"'s format specific information. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.rc | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 2489c43..0f68156 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -170,6 +170,17 @@ _make_test_img() fi } +_rm_test_img() +{ +local img=$1 +if [ "$IMGFMT" = "vmdk" ]; then +# Remove all the extents for vmdk +$QEMU_IMG info $img 2>/dev/null | grep 'filename:' | cut -f 2 -d: \ +| xargs -I {} rm -f "{}" +fi +rm -f $img +} + _cleanup_test_img() { case "$IMGPROTO" in @@ -179,9 +190,9 @@ _cleanup_test_img() rm -f "$TEST_IMG_FILE" ;; file) -rm -f "$TEST_DIR/t.$IMGFMT" -rm -f "$TEST_DIR/t.$IMGFMT.orig" -rm -f "$TEST_DIR/t.$IMGFMT.base" +_rm_test_img "$TEST_DIR/t.$IMGFMT" +_rm_test_img "$TEST_DIR/t.$IMGFMT.orig" +_rm_test_img "$TEST_DIR/t.$IMGFMT.base" if [ -n "$SAMPLE_IMG_FILE" ] then rm -f "$TEST_DIR/$SAMPLE_IMG_FILE" -- 1.8.1.4
[Qemu-devel] [PULL 08/93] gluster: Add support for creating zero-filled image
From: Bharata B Rao GlusterFS supports creation of zero-filled file on GlusterFS volume by means of an API called glfs_zerofill(). Use this API from QEMU to create an image that is filled with zeroes by using the preallocation option of qemu-img. qemu-img create gluster://server/volume/image -o preallocation=full 10G The allowed values for preallocation are 'full' and 'off'. By default preallocation is off and image is not zero-filled. glfs_zerofill() offloads the writing of zeroes to the server and if the storage supports SCSI WRITESAME, GlusterFS server can issue BLKZEROOUT ioctl to achieve the zeroing. Signed-off-by: Bharata B Rao Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/gluster.c | 50 +- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/block/gluster.c b/block/gluster.c index c11f60c..a009b15 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -354,6 +354,29 @@ out: g_slice_free(GlusterAIOCB, acb); return ret; } + +static inline bool gluster_supports_zerofill(void) +{ +return 1; +} + +static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset, +int64_t size) +{ +return glfs_zerofill(fd, offset, size); +} + +#else +static inline bool gluster_supports_zerofill(void) +{ +return 0; +} + +static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset, +int64_t size) +{ +return 0; +} #endif static int qemu_gluster_create(const char *filename, @@ -362,6 +385,7 @@ static int qemu_gluster_create(const char *filename, struct glfs *glfs; struct glfs_fd *fd; int ret = 0; +int prealloc = 0; int64_t total_size = 0; GlusterConf *gconf = g_malloc0(sizeof(GlusterConf)); @@ -374,6 +398,19 @@ static int qemu_gluster_create(const char *filename, while (options && options->name) { if (!strcmp(options->name, BLOCK_OPT_SIZE)) { total_size = options->value.n / BDRV_SECTOR_SIZE; +} else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) { +if (!options->value.s || !strcmp(options->value.s, "off")) { +prealloc = 0; +} else if (!strcmp(options->value.s, "full") && +gluster_supports_zerofill()) { +prealloc = 1; +} else { +error_setg(errp, "Invalid preallocation mode: '%s'" +" or GlusterFS doesn't support zerofill API", + options->value.s); +ret = -EINVAL; +goto out; +} } options++; } @@ -383,9 +420,15 @@ static int qemu_gluster_create(const char *filename, if (!fd) { ret = -errno; } else { -if (glfs_ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) { +if (!glfs_ftruncate(fd, total_size * BDRV_SECTOR_SIZE)) { +if (prealloc && qemu_gluster_zerofill(fd, 0, +total_size * BDRV_SECTOR_SIZE)) { +ret = -errno; +} +} else { ret = -errno; } + if (glfs_close(fd) != 0) { ret = -errno; } @@ -560,6 +603,11 @@ static QEMUOptionParameter qemu_gluster_create_options[] = { .type = OPT_SIZE, .help = "Virtual disk size" }, +{ +.name = BLOCK_OPT_PREALLOC, +.type = OPT_STRING, +.help = "Preallocation mode (allowed values: off, full)" +}, { NULL } }; -- 1.8.1.4
[Qemu-devel] [PULL 02/93] qemu-iotests: Introduce _unsupported_imgopts
From: Fam Zheng Introduce _unsupported_imgopts that causes _notrun for specific image options. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.rc | 11 +++ 1 file changed, 11 insertions(+) diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 28ba0d9..2489c43 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -406,6 +406,17 @@ _default_cache_mode() fi } +_unsupported_imgopts() +{ +for bad_opt +do +if echo "$IMGOPTS" | grep -q 2>/dev/null "$bad_opt" +then +_notrun "not suitable for image option: $bad_opt" +fi +done +} + # this test requires that a specified command (executable) exists # _require_command() -- 1.8.1.4
[Qemu-devel] [PULL 06/93] gluster: Convert aio routines into coroutines
From: Bharata B Rao Convert the read, write, flush and discard implementations from aio-based ones to coroutine based ones. Signed-off-by: Bharata B Rao Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/gluster.c | 221 +++- 1 file changed, 74 insertions(+), 147 deletions(-) diff --git a/block/gluster.c b/block/gluster.c index 563d497..f9aea0e 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -21,19 +21,15 @@ #include "qemu/uri.h" typedef struct GlusterAIOCB { -BlockDriverAIOCB common; int64_t size; int ret; -bool *finished; QEMUBH *bh; +Coroutine *coroutine; } GlusterAIOCB; typedef struct BDRVGlusterState { struct glfs *glfs; -int fds[2]; struct glfs_fd *fd; -int event_reader_pos; -GlusterAIOCB *event_acb; } BDRVGlusterState; #define GLUSTER_FD_READ 0 @@ -231,46 +227,13 @@ out: return NULL; } -static void qemu_gluster_complete_aio(GlusterAIOCB *acb, BDRVGlusterState *s) +static void qemu_gluster_complete_aio(void *opaque) { -int ret; -bool *finished = acb->finished; -BlockDriverCompletionFunc *cb = acb->common.cb; -void *opaque = acb->common.opaque; - -if (!acb->ret || acb->ret == acb->size) { -ret = 0; /* Success */ -} else if (acb->ret < 0) { -ret = acb->ret; /* Read/Write failed */ -} else { -ret = -EIO; /* Partial read/write - fail it */ -} +GlusterAIOCB *acb = (GlusterAIOCB *)opaque; -qemu_aio_release(acb); -cb(opaque, ret); -if (finished) { -*finished = true; -} -} - -static void qemu_gluster_aio_event_reader(void *opaque) -{ -BDRVGlusterState *s = opaque; -ssize_t ret; - -do { -char *p = (char *)&s->event_acb; - -ret = read(s->fds[GLUSTER_FD_READ], p + s->event_reader_pos, - sizeof(s->event_acb) - s->event_reader_pos); -if (ret > 0) { -s->event_reader_pos += ret; -if (s->event_reader_pos == sizeof(s->event_acb)) { -s->event_reader_pos = 0; -qemu_gluster_complete_aio(s->event_acb, s); -} -} -} while (ret < 0 && errno == EINTR); +qemu_bh_delete(acb->bh); +acb->bh = NULL; +qemu_coroutine_enter(acb->coroutine, NULL); } /* TODO Convert to fine grained options */ @@ -309,7 +272,6 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, filename = qemu_opt_get(opts, "filename"); - s->glfs = qemu_gluster_init(gconf, filename); if (!s->glfs) { ret = -errno; @@ -329,17 +291,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, s->fd = glfs_open(s->glfs, gconf->image, open_flags); if (!s->fd) { ret = -errno; -goto out; -} - -ret = qemu_pipe(s->fds); -if (ret < 0) { -ret = -errno; -goto out; } -fcntl(s->fds[GLUSTER_FD_READ], F_SETFL, O_NONBLOCK); -qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ], -qemu_gluster_aio_event_reader, NULL, s); out: qemu_opts_del(opts); @@ -398,58 +350,37 @@ out: return ret; } -static void qemu_gluster_aio_cancel(BlockDriverAIOCB *blockacb) -{ -GlusterAIOCB *acb = (GlusterAIOCB *)blockacb; -bool finished = false; - -acb->finished = &finished; -while (!finished) { -qemu_aio_wait(); -} -} - -static const AIOCBInfo gluster_aiocb_info = { -.aiocb_size = sizeof(GlusterAIOCB), -.cancel = qemu_gluster_aio_cancel, -}; - +/* + * AIO callback routine called from GlusterFS thread. + */ static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg) { GlusterAIOCB *acb = (GlusterAIOCB *)arg; -BlockDriverState *bs = acb->common.bs; -BDRVGlusterState *s = bs->opaque; -int retval; - -acb->ret = ret; -retval = qemu_write_full(s->fds[GLUSTER_FD_WRITE], &acb, sizeof(acb)); -if (retval != sizeof(acb)) { -/* - * Gluster AIO callback thread failed to notify the waiting - * QEMU thread about IO completion. - */ -error_report("Gluster AIO completion failed: %s", strerror(errno)); -abort(); + +if (!ret || ret == acb->size) { +acb->ret = 0; /* Success */ +} else if (ret < 0) { +acb->ret = ret; /* Read/Write failed */ +} else { +acb->ret = -EIO; /* Partial read/write - fail it */ } + +acb->bh = qemu_bh_new(qemu_gluster_complete_aio, acb); +qemu_bh_schedule(acb->bh); } -static BlockDriverAIOCB *qemu_gluster_aio_rw(BlockDriverState *bs, -int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, -BlockDriverCompletionFunc *cb, void *opaque, int write) +static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs, +int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int write) { int ret; -GlusterAIOCB *acb; +GlusterAIOCB *acb = g_slice_new(GlusterAIOCB); BDR
[Qemu-devel] [PULL 07/93] gluster: Implement .bdrv_co_write_zeroes for gluster
From: Bharata B Rao Support .bdrv_co_write_zeroes() from gluster driver by using GlusterFS API glfs_zerofill() that off-loads the writing of zeroes to GlusterFS server. Signed-off-by: Bharata B Rao Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/gluster.c | 79 +++-- configure | 8 ++ 2 files changed, 68 insertions(+), 19 deletions(-) diff --git a/block/gluster.c b/block/gluster.c index f9aea0e..c11f60c 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -236,6 +236,25 @@ static void qemu_gluster_complete_aio(void *opaque) qemu_coroutine_enter(acb->coroutine, NULL); } +/* + * AIO callback routine called from GlusterFS thread. + */ +static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg) +{ +GlusterAIOCB *acb = (GlusterAIOCB *)arg; + +if (!ret || ret == acb->size) { +acb->ret = 0; /* Success */ +} else if (ret < 0) { +acb->ret = ret; /* Read/Write failed */ +} else { +acb->ret = -EIO; /* Partial read/write - fail it */ +} + +acb->bh = qemu_bh_new(qemu_gluster_complete_aio, acb); +qemu_bh_schedule(acb->bh); +} + /* TODO Convert to fine grained options */ static QemuOptsList runtime_opts = { .name = "gluster", @@ -308,6 +327,35 @@ out: return ret; } +#ifdef CONFIG_GLUSTERFS_ZEROFILL +static coroutine_fn int qemu_gluster_co_write_zeroes(BlockDriverState *bs, +int64_t sector_num, int nb_sectors, BdrvRequestFlags flags) +{ +int ret; +GlusterAIOCB *acb = g_slice_new(GlusterAIOCB); +BDRVGlusterState *s = bs->opaque; +off_t size = nb_sectors * BDRV_SECTOR_SIZE; +off_t offset = sector_num * BDRV_SECTOR_SIZE; + +acb->size = size; +acb->ret = 0; +acb->coroutine = qemu_coroutine_self(); + +ret = glfs_zerofill_async(s->fd, offset, size, &gluster_finish_aiocb, acb); +if (ret < 0) { +ret = -errno; +goto out; +} + +qemu_coroutine_yield(); +ret = acb->ret; + +out: +g_slice_free(GlusterAIOCB, acb); +return ret; +} +#endif + static int qemu_gluster_create(const char *filename, QEMUOptionParameter *options, Error **errp) { @@ -350,25 +398,6 @@ out: return ret; } -/* - * AIO callback routine called from GlusterFS thread. - */ -static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg) -{ -GlusterAIOCB *acb = (GlusterAIOCB *)arg; - -if (!ret || ret == acb->size) { -acb->ret = 0; /* Success */ -} else if (ret < 0) { -acb->ret = ret; /* Read/Write failed */ -} else { -acb->ret = -EIO; /* Partial read/write - fail it */ -} - -acb->bh = qemu_bh_new(qemu_gluster_complete_aio, acb); -qemu_bh_schedule(acb->bh); -} - static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int write) { @@ -552,6 +581,9 @@ static BlockDriver bdrv_gluster = { #ifdef CONFIG_GLUSTERFS_DISCARD .bdrv_co_discard = qemu_gluster_co_discard, #endif +#ifdef CONFIG_GLUSTERFS_ZEROFILL +.bdrv_co_write_zeroes = qemu_gluster_co_write_zeroes, +#endif .create_options = qemu_gluster_create_options, }; @@ -573,6 +605,9 @@ static BlockDriver bdrv_gluster_tcp = { #ifdef CONFIG_GLUSTERFS_DISCARD .bdrv_co_discard = qemu_gluster_co_discard, #endif +#ifdef CONFIG_GLUSTERFS_ZEROFILL +.bdrv_co_write_zeroes = qemu_gluster_co_write_zeroes, +#endif .create_options = qemu_gluster_create_options, }; @@ -594,6 +629,9 @@ static BlockDriver bdrv_gluster_unix = { #ifdef CONFIG_GLUSTERFS_DISCARD .bdrv_co_discard = qemu_gluster_co_discard, #endif +#ifdef CONFIG_GLUSTERFS_ZEROFILL +.bdrv_co_write_zeroes = qemu_gluster_co_write_zeroes, +#endif .create_options = qemu_gluster_create_options, }; @@ -615,6 +653,9 @@ static BlockDriver bdrv_gluster_rdma = { #ifdef CONFIG_GLUSTERFS_DISCARD .bdrv_co_discard = qemu_gluster_co_discard, #endif +#ifdef CONFIG_GLUSTERFS_ZEROFILL +.bdrv_co_write_zeroes = qemu_gluster_co_write_zeroes, +#endif .create_options = qemu_gluster_create_options, }; diff --git a/configure b/configure index 3782a6a..b472694 100755 --- a/configure +++ b/configure @@ -256,6 +256,7 @@ coroutine_pool="" seccomp="" glusterfs="" glusterfs_discard="no" +glusterfs_zerofill="no" virtio_blk_data_plane="" gtk="" gtkabi="2.0" @@ -2701,6 +2702,9 @@ if test "$glusterfs" != "no" ; then if $pkg_config --atleast-version=5 glusterfs-api; then glusterfs_discard="yes" fi +if $pkg_config --atleast-version=6 glusterfs-api; then + glusterfs_zerofill="yes" +fi else if test "$glusterfs" = "yes" ; then feature_not_found "GlusterFS backend support" @@ -4229,6 +4233,10 @@ if test "$glusterfs_discard" = "yes" ; then ec
[Qemu-devel] [PULL 03/93] qemu-iotests: Add _unsupported_imgopts for vmdk subformats
From: Fam Zheng Some cases are not applicable for vmdk subformats those don't support certain features, e.g. backing file, and some others can't run on mult-file image, e.g. monolithicFlat. This adds declaration in test cases to skip them automatically, so that iotests on vmdk can go more smoothly (without manually picking of cases for each subformat). Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/017 | 1 + tests/qemu-iotests/018 | 1 + tests/qemu-iotests/019 | 3 +++ tests/qemu-iotests/020 | 3 +++ tests/qemu-iotests/034 | 3 +++ tests/qemu-iotests/037 | 3 +++ tests/qemu-iotests/059 | 3 +++ tests/qemu-iotests/063 | 3 +++ tests/qemu-iotests/069 | 1 + 9 files changed, 21 insertions(+) diff --git a/tests/qemu-iotests/017 b/tests/qemu-iotests/017 index aba3faf..3af3cdf 100755 --- a/tests/qemu-iotests/017 +++ b/tests/qemu-iotests/017 @@ -43,6 +43,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow qcow2 vmdk qed _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" TEST_OFFSETS="0 4294967296" diff --git a/tests/qemu-iotests/018 b/tests/qemu-iotests/018 index 15fcfe5..6f7f054 100755 --- a/tests/qemu-iotests/018 +++ b/tests/qemu-iotests/018 @@ -43,6 +43,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow qcow2 vmdk qed _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" TEST_OFFSETS="0 4294967296" diff --git a/tests/qemu-iotests/019 b/tests/qemu-iotests/019 index 5bb18d0..b43e70f 100755 --- a/tests/qemu-iotests/019 +++ b/tests/qemu-iotests/019 @@ -47,6 +47,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow qcow2 vmdk qed _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" \ + "subformat=twoGbMaxExtentFlat" \ + "subformat=twoGbMaxExtentSparse" TEST_OFFSETS="0 4294967296" CLUSTER_SIZE=65536 diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020 index b3c86d8..73a0429 100755 --- a/tests/qemu-iotests/020 +++ b/tests/qemu-iotests/020 @@ -45,6 +45,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow qcow2 vmdk qed _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" \ + "subformat=twoGbMaxExtentFlat" \ + "subformat=twoGbMaxExtentSparse" TEST_OFFSETS="0 4294967296" diff --git a/tests/qemu-iotests/034 b/tests/qemu-iotests/034 index 67f1959..7349789 100755 --- a/tests/qemu-iotests/034 +++ b/tests/qemu-iotests/034 @@ -41,6 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow qcow2 vmdk qed _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" \ + "subformat=twoGbMaxExtentFlat" \ + "subformat=twoGbMaxExtentSparse" CLUSTER_SIZE=4k size=128M diff --git a/tests/qemu-iotests/037 b/tests/qemu-iotests/037 index 743bae3..e444349 100755 --- a/tests/qemu-iotests/037 +++ b/tests/qemu-iotests/037 @@ -41,6 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow qcow2 vmdk qed _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" \ + "subformat=twoGbMaxExtentFlat" \ + "subformat=twoGbMaxExtentSparse" CLUSTER_SIZE=4k size=128M diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index 65bea1d..d8215ae 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -42,6 +42,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt vmdk _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" \ + "subformat=twoGbMaxExtentFlat" \ + "subformat=twoGbMaxExtentSparse" capacity_offset=16 granularity_offset=20 diff --git a/tests/qemu-iotests/063 b/tests/qemu-iotests/063 index 2ab8f20..77503a2 100755 --- a/tests/qemu-iotests/063 +++ b/tests/qemu-iotests/063 @@ -44,6 +44,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow qcow2 vmdk qed raw _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" \ + "subformat=twoGbMaxExtentFlat" \ + "subformat=twoGbMaxExtentSparse" _make_test_img 4M diff --git a/tests/qemu-iotests/069 b/tests/qemu-iotests/069 index 3042803..50347d9 100755 --- a/tests/qemu-iotests/069 +++ b/tests/qemu-iotests/069 @@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt cow qed qcow qcow2 vmdk _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" IMG_SIZE=128K -- 1.8.1.4
[Qemu-devel] [PULL 05/93] block/iscsi: return -ENOMEM if an async call fails immediately
From: Peter Lieven if an async libiscsi call fails directly it can only be due to an out of memory condition. All other errors are returned through the callback. Signed-off-by: Peter Lieven Reviewed-by: Ronnie Sahlberg Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/iscsi.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index c0ea0c4..76b3c96 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -308,7 +308,7 @@ retry: iscsi_co_generic_cb, &iTask); if (iTask.task == NULL) { g_free(buf); -return -EIO; +return -ENOMEM; } #if defined(LIBISCSI_FEATURE_IOVECTOR) scsi_task_set_iov_out(iTask.task, (struct scsi_iovec *) iov->iov, @@ -376,7 +376,7 @@ retry: break; } if (iTask.task == NULL) { -return -EIO; +return -ENOMEM; } #if defined(LIBISCSI_FEATURE_IOVECTOR) scsi_task_set_iov_in(iTask.task, (struct scsi_iovec *) iov->iov, iov->niov); @@ -419,7 +419,7 @@ static int coroutine_fn iscsi_co_flush(BlockDriverState *bs) retry: if (iscsi_synchronizecache10_task(iscsilun->iscsi, iscsilun->lun, 0, 0, 0, 0, iscsi_co_generic_cb, &iTask) == NULL) { -return -EIO; +return -ENOMEM; } while (!iTask.complete) { @@ -669,7 +669,7 @@ retry: sector_qemu2lun(sector_num, iscsilun), 8 + 16, iscsi_co_generic_cb, &iTask) == NULL) { -ret = -EIO; +ret = -ENOMEM; goto out; } @@ -753,7 +753,7 @@ coroutine_fn iscsi_co_discard(BlockDriverState *bs, int64_t sector_num, retry: if (iscsi_unmap_task(iscsilun->iscsi, iscsilun->lun, 0, 0, &list, 1, iscsi_co_generic_cb, &iTask) == NULL) { -return -EIO; +return -ENOMEM; } while (!iTask.complete) { @@ -822,7 +822,7 @@ retry: iscsilun->zeroblock, iscsilun->block_size, nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP), 0, 0, iscsi_co_generic_cb, &iTask) == NULL) { -return -EIO; +return -ENOMEM; } while (!iTask.complete) { -- 1.8.1.4
[Qemu-devel] [PULL 12/93] vmdk: Fix big flat extent IO
From: Fam Zheng Local variable "n" as int64_t avoids overflow with large sector number calculation. See test case change for failure case. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 4 +-- tests/qemu-iotests/059 | 7 + tests/qemu-iotests/059.out | 74 ++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index c6b60b4..22b99b0 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1325,8 +1325,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num, { BDRVVmdkState *s = bs->opaque; VmdkExtent *extent = NULL; -int n, ret; -int64_t index_in_cluster; +int ret; +int64_t index_in_cluster, n; uint64_t extent_begin_sector, extent_relative_sector_num; uint64_t cluster_offset; VmdkMetaData m_data; diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index d8215ae..64ed04c 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -102,6 +102,13 @@ echo "=== Testing version 3 ===" _use_sample_img iotest-version3.vmdk.bz2 _img_info +echo +echo "=== Testing 4TB monolithicFlat creation and IO ===" +IMGOPTS="subformat=monolithicFlat" _make_test_img 4T +_img_info +$QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 16ab7c6..5e30e69 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -2047,4 +2047,78 @@ RW 12582912 VMFS "dummy.IMGFMT" 1 image: TEST_DIR/iotest-version3.IMGFMT file format: IMGFMT virtual size: 1.0G (1073741824 bytes) + +=== Testing 4TB monolithicFlat creation and IO === +Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=4398046511104 +image: TEST_DIR/iotest-version3.IMGFMT +file format: IMGFMT +virtual size: 4.0T (4398046511104 bytes) +wrote 512/512 bytes at offset 966367641600 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +e1: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10010: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10020: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10030: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10040: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10050: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10060: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10070: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10080: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10090: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e100a0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e100b0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e100c0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e100d0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e100e0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e100f0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10100: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10110: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10120: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10130: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10140: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10150: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10160: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10170: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10180: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10190: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e101a0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e101b0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e101c0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e101d0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e101e0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e101f0: 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a +e10200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 +e10210: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 +e10220: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 +e10230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[Qemu-devel] [PULL 09/93] sheepdog: fix clone operation by 'qemu-img create -b'
From: Liu Yuan We should pass base_inode->vdi_id to base_vdi_id of SheepdogVdiReq so that sheep can create a clone instead a fresh volume. This fixes following command: qemu-create -b sheepdog:base sheepdog:clone so users can boot sheepdog:clone as a normal volume. Cc: qemu-devel@nongnu.org Cc: Kevin Wolf Cc: Stefan Hajnoczi Signed-off-by: Liu Yuan Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/sheepdog.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index b94ab6e..6088fa5 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -161,7 +161,7 @@ typedef struct SheepdogVdiReq { uint32_t id; uint32_t data_length; uint64_t vdi_size; -uint32_t vdi_id; +uint32_t base_vdi_id; uint8_t copies; uint8_t copy_policy; uint8_t reserved[2]; @@ -1493,7 +1493,7 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot) memset(&hdr, 0, sizeof(hdr)); hdr.opcode = SD_OP_NEW_VDI; -hdr.vdi_id = s->inode.vdi_id; +hdr.base_vdi_id = s->inode.vdi_id; wlen = SD_MAX_VDI_LEN; @@ -1684,7 +1684,7 @@ static int sd_create(const char *filename, QEMUOptionParameter *options, if (backing_file) { BlockDriverState *bs; -BDRVSheepdogState *s; +BDRVSheepdogState *base; BlockDriver *drv; /* Currently, only Sheepdog backing image is supported. */ @@ -1702,15 +1702,15 @@ static int sd_create(const char *filename, QEMUOptionParameter *options, goto out; } -s = bs->opaque; +base = bs->opaque; -if (!is_snapshot(&s->inode)) { +if (!is_snapshot(&base->inode)) { error_report("cannot clone from a non snapshot vdi"); bdrv_unref(bs); ret = -EINVAL; goto out; } - +s->inode.vdi_id = base->inode.vdi_id; bdrv_unref(bs); } @@ -1743,7 +1743,7 @@ static void sd_close(BlockDriverState *bs) memset(&hdr, 0, sizeof(hdr)); hdr.opcode = SD_OP_RELEASE_VDI; -hdr.vdi_id = s->inode.vdi_id; +hdr.base_vdi_id = s->inode.vdi_id; wlen = strlen(s->name) + 1; hdr.data_length = wlen; hdr.flags = SD_FLAG_CMD_WRITE; @@ -1846,7 +1846,7 @@ static bool sd_delete(BDRVSheepdogState *s) unsigned int wlen = SD_MAX_VDI_LEN, rlen = 0; SheepdogVdiReq hdr = { .opcode = SD_OP_DEL_VDI, -.vdi_id = s->inode.vdi_id, +.base_vdi_id = s->inode.vdi_id, .data_length = wlen, .flags = SD_FLAG_CMD_WRITE, }; -- 1.8.1.4
[Qemu-devel] [PULL 11/93] docs: qcow2 compat=1.1 is now the default
From: Stefan Hajnoczi Commit 9117b47717ad208b12786ce88eacb013f9b3dd1c ("qcow2: Change default for new images to compat=1.1") changed the default qcow2 image format version but forgot to update qemu-doc.texi and qemu-img.texi. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- qemu-doc.texi | 8 qemu-img.texi | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/qemu-doc.texi b/qemu-doc.texi index 4e9c6e9..ce61f30 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -536,11 +536,11 @@ support of multiple VM snapshots. Supported options: @table @code @item compat -Determines the qcow2 version to use. @code{compat=0.10} uses the traditional -image format that can be read by any QEMU since 0.10 (this is the default). +Determines the qcow2 version to use. @code{compat=0.10} uses the +traditional image format that can be read by any QEMU since 0.10. @code{compat=1.1} enables image format extensions that only QEMU 1.1 and -newer understand. Amongst others, this includes zero clusters, which allow -efficient copy-on-read for sparse images. +newer understand (this is the default). Amongst others, this includes +zero clusters, which allow efficient copy-on-read for sparse images. @item backing_file File name of a base image (see @option{create} subcommand) diff --git a/qemu-img.texi b/qemu-img.texi index 1bba91e..778e967 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -391,11 +391,11 @@ support of multiple VM snapshots. Supported options: @table @code @item compat -Determines the qcow2 version to use. @code{compat=0.10} uses the traditional -image format that can be read by any QEMU since 0.10 (this is the default). +Determines the qcow2 version to use. @code{compat=0.10} uses the +traditional image format that can be read by any QEMU since 0.10. @code{compat=1.1} enables image format extensions that only QEMU 1.1 and -newer understand. Amongst others, this includes zero clusters, which allow -efficient copy-on-read for sparse images. +newer understand (this is the default). Amongst others, this includes zero +clusters, which allow efficient copy-on-read for sparse images. @item backing_file File name of a base image (see @option{create} subcommand) -- 1.8.1.4
[Qemu-devel] [PULL 10/93] qtest: Fix the bug about disable vnc causes "make check" fail
From: Kewei Yu When we disable vnc from "./configure", QEMU can't use the vnc option. So qtest can't use the "vnc -none ", otherwise "make check" fails. If QEMU uses "-display none", "-vnc none" is excrescent, So we just need to drop it. Signed-off-by: Kewei Yu Reviewed-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- tests/fdc-test.c | 5 + tests/ide-test.c | 3 --- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 38b5b17..37096dc 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -518,7 +518,6 @@ static void fuzz_registers(void) int main(int argc, char **argv) { const char *arch = qtest_get_arch(); -char *cmdline; int fd; int ret; @@ -538,9 +537,7 @@ int main(int argc, char **argv) /* Run the tests */ g_test_init(&argc, &argv, NULL); -cmdline = g_strdup_printf("-vnc none "); - -qtest_start(cmdline); +qtest_start(NULL); qtest_irq_intercept_in(global_qtest, "ioapic"); qtest_add_func("/fdc/cmos", test_cmos); qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start); diff --git a/tests/ide-test.c b/tests/ide-test.c index d5cec5a..4a0d97f 100644 --- a/tests/ide-test.c +++ b/tests/ide-test.c @@ -380,7 +380,6 @@ static void test_bmdma_no_busmaster(void) static void test_bmdma_setup(void) { ide_test_start( -"-vnc none " "-drive file=%s,if=ide,serial=%s,cache=writeback " "-global ide-hd.ver=%s", tmp_path, "testdisk", "version"); @@ -410,7 +409,6 @@ static void test_identify(void) int ret; ide_test_start( -"-vnc none " "-drive file=%s,if=ide,serial=%s,cache=writeback " "-global ide-hd.ver=%s", tmp_path, "testdisk", "version"); @@ -455,7 +453,6 @@ static void test_flush(void) uint8_t data; ide_test_start( -"-vnc none " "-drive file=blkdebug::%s,if=ide,cache=writeback", tmp_path); -- 1.8.1.4
[Qemu-devel] [PULL 16/93] qemu-io: use readline.c
From: Stefan Hajnoczi Use readline.c for command-line history. There was support for GNU Readline and BSD Editline but it was never compiled in. Since QEMU has its own readline.c, just use that when qemu-io runs with stdin attached to a terminal. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- qemu-io.c | 103 +++--- 1 file changed, 58 insertions(+), 45 deletions(-) diff --git a/qemu-io.c b/qemu-io.c index 3b3340a..d7c26d3 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -18,6 +18,7 @@ #include "qemu/main-loop.h" #include "qemu/option.h" #include "qemu/config-file.h" +#include "qemu/readline.h" #include "block/block_int.h" #include "trace/control.h" @@ -32,6 +33,8 @@ extern int qemuio_misalign; static int ncmdline; static char **cmdline; +static ReadLineState *readline_state; + static int close_f(BlockDriverState *bs, int argc, char **argv) { bdrv_unref(bs); @@ -203,14 +206,6 @@ static void usage(const char *name) name); } - -#if defined(ENABLE_READLINE) -# include -# include -#elif defined(ENABLE_EDITLINE) -# include -#endif - static char *get_prompt(void) { static char prompt[FILENAME_MAX + 2 /*"> "*/ + 1 /*"\0"*/ ]; @@ -222,52 +217,47 @@ static char *get_prompt(void) return prompt; } -#if defined(ENABLE_READLINE) -static char *fetchline(void) +static void readline_printf_func(void *opaque, const char *fmt, ...) { -char *line = readline(get_prompt()); -if (line && *line) { -add_history(line); -} -return line; +va_list ap; +va_start(ap, fmt); +vprintf(fmt, ap); +va_end(ap); } -#elif defined(ENABLE_EDITLINE) -static char *el_get_prompt(EditLine *e) + +static void readline_flush_func(void *opaque) { -return get_prompt(); +fflush(stdout); } -static char *fetchline(void) +static void readline_func(void *opaque, const char *str, void *readline_opaque) { -static EditLine *el; -static History *hist; -HistEvent hevent; -char *line; -int count; - -if (!el) { -hist = history_init(); -history(hist, &hevent, H_SETSIZE, 100); -el = el_init(progname, stdin, stdout, stderr); -el_source(el, NULL); -el_set(el, EL_SIGNAL, 1); -el_set(el, EL_PROMPT, el_get_prompt); -el_set(el, EL_HIST, history, (const char *)hist); -} -line = strdup(el_gets(el, &count)); -if (line) { -if (count > 0) { -line[count-1] = '\0'; -} -if (*line) { -history(hist, &hevent, H_ENTER, line); +char **line = readline_opaque; +*line = g_strdup(str); +} + +static void readline_completion_func(void *opaque, const char *str) +{ +/* No command or argument completion implemented yet */ +} + +static char *fetchline_readline(void) +{ +char *line = NULL; + +readline_start(readline_state, get_prompt(), 0, readline_func, &line); +while (!line) { +int ch = getchar(); +if (ch == EOF) { +break; } +readline_handle_byte(readline_state, ch); } return line; } -#else -# define MAXREADLINESZ 1024 -static char *fetchline(void) + +#define MAXREADLINESZ 1024 +static char *fetchline_fgets(void) { char *p, *line = g_malloc(MAXREADLINESZ); @@ -283,7 +273,15 @@ static char *fetchline(void) return line; } -#endif + +static char *fetchline(void) +{ +if (readline_state) { +return fetchline_readline(); +} else { +return fetchline_fgets(); +} +} static void prep_fetchline(void *opaque) { @@ -339,6 +337,11 @@ static void add_user_command(char *optarg) cmdline[ncmdline-1] = optarg; } +static void reenable_tty_echo(void) +{ +qemu_set_tty_echo(STDIN_FILENO, true); +} + int main(int argc, char **argv) { int readonly = 0; @@ -435,6 +438,15 @@ int main(int argc, char **argv) qemuio_add_command(&open_cmd); qemuio_add_command(&close_cmd); +if (isatty(STDIN_FILENO)) { +readline_state = readline_init(readline_printf_func, + readline_flush_func, + NULL, + readline_completion_func); +qemu_set_tty_echo(STDIN_FILENO, false); +atexit(reenable_tty_echo); +} + /* open the device */ if (!readonly) { flags |= BDRV_O_RDWR; @@ -453,5 +465,6 @@ int main(int argc, char **argv) if (qemuio_bs) { bdrv_unref(qemuio_bs); } +g_free(readline_state); return 0; } -- 1.8.1.4
[Qemu-devel] [PULL 13/93] readline: decouple readline from the monitor
From: Stefan Hajnoczi Make the readline.c functionality reusable. Instead of calling monitor_printf() and monitor_flush() directly, invoke function pointers provided by the user. This way readline.c does not know about Monitor and other users will be able to make use of readline.c. Note that there is already an "opaque" argument to the ReadLineFunc callback. Consistently call it "readline_opaque" from now on to distinguish from the ReadLinePrintfFunc/ReadLineFlushFunc "opaque" argument. I also dropped the printf macro trickery since it's now highly unlikely that anyone modifying readline.c would call printf(3) directly. We no longer need this protection. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hmp.c | 6 +++--- include/monitor/readline.h | 20 +--- monitor.c | 39 --- readline.c | 44 +++- 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/hmp.c b/hmp.c index 79f9c7d..468f97d 100644 --- a/hmp.c +++ b/hmp.c @@ -1092,11 +1092,11 @@ void hmp_eject(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &err); } -static void hmp_change_read_arg(Monitor *mon, const char *password, -void *opaque) +static void hmp_change_read_arg(void *opaque, const char *password, +void *readline_opaque) { qmp_change_vnc_password(password, NULL); -monitor_read_command(mon, 1); +monitor_read_command(opaque, 1); } void hmp_change(Monitor *mon, const QDict *qdict) diff --git a/include/monitor/readline.h b/include/monitor/readline.h index 0faf6e1..a89fe4a 100644 --- a/include/monitor/readline.h +++ b/include/monitor/readline.h @@ -1,14 +1,15 @@ #ifndef READLINE_H #define READLINE_H -#include "qemu-common.h" - #define READLINE_CMD_BUF_SIZE 4095 #define READLINE_MAX_CMDS 64 #define READLINE_MAX_COMPLETIONS 256 -typedef void ReadLineFunc(Monitor *mon, const char *str, void *opaque); -typedef void ReadLineCompletionFunc(Monitor *mon, +typedef void ReadLinePrintfFunc(void *opaque, const char *fmt, ...); +typedef void ReadLineFlushFunc(void *opaque); +typedef void ReadLineFunc(void *opaque, const char *str, + void *readline_opaque); +typedef void ReadLineCompletionFunc(void *opaque, const char *cmdline); typedef struct ReadLineState { @@ -35,7 +36,10 @@ typedef struct ReadLineState { void *readline_opaque; int read_password; char prompt[256]; -Monitor *mon; + +ReadLinePrintfFunc *printf_func; +ReadLineFlushFunc *flush_func; +void *opaque; } ReadLineState; void readline_add_completion(ReadLineState *rs, const char *str); @@ -46,11 +50,13 @@ const char *readline_get_history(ReadLineState *rs, unsigned int index); void readline_handle_byte(ReadLineState *rs, int ch); void readline_start(ReadLineState *rs, const char *prompt, int read_password, -ReadLineFunc *readline_func, void *opaque); +ReadLineFunc *readline_func, void *readline_opaque); void readline_restart(ReadLineState *rs); void readline_show_prompt(ReadLineState *rs); -ReadLineState *readline_init(Monitor *mon, +ReadLineState *readline_init(ReadLinePrintfFunc *printf_func, + ReadLineFlushFunc *flush_func, + void *opaque, ReadLineCompletionFunc *completion_finder); #endif /* !READLINE_H */ diff --git a/monitor.c b/monitor.c index 845f608..32d0264 100644 --- a/monitor.c +++ b/monitor.c @@ -217,8 +217,8 @@ static const mon_cmd_t qmp_cmds[]; Monitor *cur_mon; Monitor *default_mon; -static void monitor_command_cb(Monitor *mon, const char *cmdline, - void *opaque); +static void monitor_command_cb(void *opaque, const char *cmdline, + void *readline_opaque); static inline int qmp_cmd_mode(const Monitor *mon) { @@ -4338,9 +4338,10 @@ static void monitor_find_completion_by_table(Monitor *mon, } } -static void monitor_find_completion(Monitor *mon, +static void monitor_find_completion(void *opaque, const char *cmdline) { +Monitor *mon = opaque; char *args[MAX_ARGS]; int nb_args, len; @@ -4751,8 +4752,11 @@ static void monitor_read(void *opaque, const uint8_t *buf, int size) cur_mon = old_mon; } -static void monitor_command_cb(Monitor *mon, const char *cmdline, void *opaque) +static void monitor_command_cb(void *opaque, const char *cmdline, + void *readline_opaque) { +Monitor *mon = opaque; + monitor_suspend(mon); handle_user_command(mon, cmdline); monitor_resume(mon); @@ -4881,6 +4885,22 @@ static void sortcmdlist(void) * End: */ +/* These functions just adapt the readline interfac
[Qemu-devel] [PULL 15/93] osdep: add qemu_set_tty_echo()
From: Stefan Hajnoczi Using stdin with readline.c requires disabling echo and line buffering. Add a portable wrapper to set the terminal attributes under Linux and Windows. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- include/qemu/osdep.h | 2 ++ util/oslib-posix.c | 18 ++ util/oslib-win32.c | 19 +++ 3 files changed, 39 insertions(+) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index b3e2b6d..eac7172 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -240,4 +240,6 @@ static inline void qemu_init_auxval(char **envp) { } void qemu_init_auxval(char **envp); #endif +void qemu_set_tty_echo(int fd, bool echo); + #endif diff --git a/util/oslib-posix.c b/util/oslib-posix.c index e00a44c..f5c4016 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -47,6 +47,9 @@ extern int daemon(int, int); # define QEMU_VMALLOC_ALIGN getpagesize() #endif +#include +#include + #include #include "config-host.h" @@ -251,3 +254,18 @@ qemu_get_local_state_pathname(const char *relative_pathname) return g_strdup_printf("%s/%s", CONFIG_QEMU_LOCALSTATEDIR, relative_pathname); } + +void qemu_set_tty_echo(int fd, bool echo) +{ +struct termios tty; + +tcgetattr(fd, &tty); + +if (echo) { +tty.c_lflag |= ECHO | ECHONL | ICANON | IEXTEN; +} else { +tty.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN); +} + +tcsetattr(fd, TCSANOW, &tty); +} diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 776ccfa..50be044 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -189,3 +189,22 @@ qemu_get_local_state_pathname(const char *relative_pathname) return g_strdup_printf("%s" G_DIR_SEPARATOR_S "%s", base_path, relative_pathname); } + +void qemu_set_tty_echo(int fd, bool echo) +{ +HANDLE handle = (HANDLE)_get_osfhandle(fd); +DWORD dwMode = 0; + +if (handle == INVALID_HANDLE_VALUE) { +return; +} + +GetConsoleMode(handle, &dwMode); + +if (echo) { +SetConsoleMode(handle, dwMode | ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT); +} else { +SetConsoleMode(handle, + dwMode & ~(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT)); +} +} -- 1.8.1.4
[Qemu-devel] [PULL 14/93] readline: move readline to a generic location
From: Stefan Hajnoczi Now that the monitor and readline are decoupled, readline.h no longer belongs in include/monitor/. Put the header into include/qemu/. Move the source file into util/ so it can be linked as part of libqemuutil.a. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- Makefile.objs | 1 - include/monitor/monitor.h | 2 +- include/monitor/readline.h | 62 -- include/qemu/readline.h| 62 ++ monitor.c | 2 +- readline.c | 495 - util/Makefile.objs | 1 + util/readline.c| 495 + 8 files changed, 560 insertions(+), 560 deletions(-) delete mode 100644 include/monitor/readline.h create mode 100644 include/qemu/readline.h delete mode 100644 readline.c create mode 100644 util/readline.c diff --git a/Makefile.objs b/Makefile.objs index 857bb53..ac1d0e1 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -43,7 +43,6 @@ libcacard-y += libcacard/vcardt.o ifeq ($(CONFIG_SOFTMMU),y) common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o block/ common-obj-y += net/ -common-obj-y += readline.o common-obj-y += qdev-monitor.o device-hotplug.o common-obj-$(CONFIG_WIN32) += os-win32.o common-obj-$(CONFIG_POSIX) += os-posix.o diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index 22d8b8f..7e5f752 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -5,7 +5,7 @@ #include "qapi/qmp/qerror.h" #include "qapi/qmp/qdict.h" #include "block/block.h" -#include "monitor/readline.h" +#include "qemu/readline.h" extern Monitor *cur_mon; extern Monitor *default_mon; diff --git a/include/monitor/readline.h b/include/monitor/readline.h deleted file mode 100644 index a89fe4a..000 --- a/include/monitor/readline.h +++ /dev/null @@ -1,62 +0,0 @@ -#ifndef READLINE_H -#define READLINE_H - -#define READLINE_CMD_BUF_SIZE 4095 -#define READLINE_MAX_CMDS 64 -#define READLINE_MAX_COMPLETIONS 256 - -typedef void ReadLinePrintfFunc(void *opaque, const char *fmt, ...); -typedef void ReadLineFlushFunc(void *opaque); -typedef void ReadLineFunc(void *opaque, const char *str, - void *readline_opaque); -typedef void ReadLineCompletionFunc(void *opaque, -const char *cmdline); - -typedef struct ReadLineState { -char cmd_buf[READLINE_CMD_BUF_SIZE + 1]; -int cmd_buf_index; -int cmd_buf_size; - -char last_cmd_buf[READLINE_CMD_BUF_SIZE + 1]; -int last_cmd_buf_index; -int last_cmd_buf_size; - -int esc_state; -int esc_param; - -char *history[READLINE_MAX_CMDS]; -int hist_entry; - -ReadLineCompletionFunc *completion_finder; -char *completions[READLINE_MAX_COMPLETIONS]; -int nb_completions; -int completion_index; - -ReadLineFunc *readline_func; -void *readline_opaque; -int read_password; -char prompt[256]; - -ReadLinePrintfFunc *printf_func; -ReadLineFlushFunc *flush_func; -void *opaque; -} ReadLineState; - -void readline_add_completion(ReadLineState *rs, const char *str); -void readline_set_completion_index(ReadLineState *rs, int completion_index); - -const char *readline_get_history(ReadLineState *rs, unsigned int index); - -void readline_handle_byte(ReadLineState *rs, int ch); - -void readline_start(ReadLineState *rs, const char *prompt, int read_password, -ReadLineFunc *readline_func, void *readline_opaque); -void readline_restart(ReadLineState *rs); -void readline_show_prompt(ReadLineState *rs); - -ReadLineState *readline_init(ReadLinePrintfFunc *printf_func, - ReadLineFlushFunc *flush_func, - void *opaque, - ReadLineCompletionFunc *completion_finder); - -#endif /* !READLINE_H */ diff --git a/include/qemu/readline.h b/include/qemu/readline.h new file mode 100644 index 000..a89fe4a --- /dev/null +++ b/include/qemu/readline.h @@ -0,0 +1,62 @@ +#ifndef READLINE_H +#define READLINE_H + +#define READLINE_CMD_BUF_SIZE 4095 +#define READLINE_MAX_CMDS 64 +#define READLINE_MAX_COMPLETIONS 256 + +typedef void ReadLinePrintfFunc(void *opaque, const char *fmt, ...); +typedef void ReadLineFlushFunc(void *opaque); +typedef void ReadLineFunc(void *opaque, const char *str, + void *readline_opaque); +typedef void ReadLineCompletionFunc(void *opaque, +const char *cmdline); + +typedef struct ReadLineState { +char cmd_buf[READLINE_CMD_BUF_SIZE + 1]; +int cmd_buf_index; +int cmd_buf_size; + +char last_cmd_buf[READLINE_CMD_BUF_SIZE + 1]; +int last_cmd_buf_index; +int last_cmd_buf_size; + +int esc_state; +int esc_param; + +char *history[READLINE_MAX_CMDS]; +int hist_entry; + +ReadLineCompletionFunc *completion_finder; +char *completions[READLINE_MAX_COMPLETIONS]; +
[Qemu-devel] [PULL 23/93] blkdebug: Always call read_config()
From: Max Reitz Move the check whether there actually is a config file into the read_config() function. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/blkdebug.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index fab76ce..acf23f2 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -273,21 +273,23 @@ static void remove_rule(BlkdebugRule *rule) static int read_config(BDRVBlkdebugState *s, const char *filename, Error **errp) { -FILE *f; +FILE *f = NULL; int ret; struct add_rule_data d; -f = fopen(filename, "r"); -if (f == NULL) { -error_setg_errno(errp, errno, "Could not read blkdebug config file"); -return -errno; -} +if (filename) { +f = fopen(filename, "r"); +if (f == NULL) { +error_setg_errno(errp, errno, "Could not read blkdebug config file"); +return -errno; +} -ret = qemu_config_parse(f, config_groups, filename); -if (ret < 0) { -error_setg(errp, "Could not parse blkdebug config file"); -ret = -EINVAL; -goto fail; +ret = qemu_config_parse(f, config_groups, filename); +if (ret < 0) { +error_setg(errp, "Could not parse blkdebug config file"); +ret = -EINVAL; +goto fail; +} } d.s = s; @@ -301,7 +303,9 @@ static int read_config(BDRVBlkdebugState *s, const char *filename, Error **errp) fail: qemu_opts_reset(&inject_error_opts); qemu_opts_reset(&set_state_opts); -fclose(f); +if (f) { +fclose(f); +} return ret; } @@ -374,11 +378,9 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, /* Read rules from config file */ config = qemu_opt_get(opts, "config"); -if (config) { -ret = read_config(s, config, errp); -if (ret) { -goto fail; -} +ret = read_config(s, config, errp); +if (ret) { +goto fail; } /* Set initial state */ -- 1.8.1.4
[Qemu-devel] [PULL 26/93] block: Pass reference to bdrv_file_open()
From: Max Reitz With that now being possible, bdrv_open() should try to extract a block device reference from the options and pass it to bdrv_file_open(). Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index 1e53b3d..bef4f82 100644 --- a/block.c +++ b/block.c @@ -1056,6 +1056,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, char tmp_filename[PATH_MAX + 1]; BlockDriverState *file = NULL; QDict *file_options = NULL; +const char *file_reference; const char *drvname; Error *local_err = NULL; @@ -1142,9 +1143,11 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, } qdict_extract_subqdict(options, &file_options, "file."); +file_reference = qdict_get_try_str(options, "file"); -ret = bdrv_file_open(&file, filename, NULL, file_options, +ret = bdrv_file_open(&file, filename, file_reference, file_options, bdrv_open_flags(bs, flags | BDRV_O_UNMAP), &local_err); +qdict_del(options, "file"); if (ret < 0) { goto fail; } -- 1.8.1.4
[Qemu-devel] [PULL 25/93] block: Allow reference for bdrv_file_open()
From: Max Reitz Allow specifying a reference to an existing block device (by name) for bdrv_file_open() instead of a filename and/or options. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block.c | 25 ++--- block/blkdebug.c | 2 +- block/blkverify.c | 2 +- block/cow.c | 3 ++- block/qcow.c | 3 ++- block/qcow2.c | 2 +- block/qed.c | 4 ++-- block/sheepdog.c | 4 ++-- block/vhdx.c | 2 +- block/vmdk.c | 8 include/block/block.h | 3 ++- qemu-io.c | 2 +- 12 files changed, 41 insertions(+), 19 deletions(-) diff --git a/block.c b/block.c index 64e7d22..1e53b3d 100644 --- a/block.c +++ b/block.c @@ -858,9 +858,10 @@ free_and_fail: * dictionary, it needs to use QINCREF() before calling bdrv_file_open. */ int bdrv_file_open(BlockDriverState **pbs, const char *filename, - QDict *options, int flags, Error **errp) + const char *reference, QDict *options, int flags, + Error **errp) { -BlockDriverState *bs; +BlockDriverState *bs = NULL; BlockDriver *drv; const char *drvname; bool allow_protocol_prefix = false; @@ -872,6 +873,24 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, options = qdict_new(); } +if (reference) { +if (filename || qdict_size(options)) { +error_setg(errp, "Cannot reference an existing block device with " + "additional options or a new filename"); +return -EINVAL; +} +QDECREF(options); + +bs = bdrv_find(reference); +if (!bs) { +error_setg(errp, "Cannot find block device '%s'", reference); +return -ENODEV; +} +bdrv_ref(bs); +*pbs = bs; +return 0; +} + bs = bdrv_new(""); bs->options = options; options = qdict_clone_shallow(options); @@ -1124,7 +1143,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, qdict_extract_subqdict(options, &file_options, "file."); -ret = bdrv_file_open(&file, filename, file_options, +ret = bdrv_file_open(&file, filename, NULL, file_options, bdrv_open_flags(bs, flags | BDRV_O_UNMAP), &local_err); if (ret < 0) { goto fail; diff --git a/block/blkdebug.c b/block/blkdebug.c index 0bf3bb5..21a4931 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -403,7 +403,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } -ret = bdrv_file_open(&bs->file, filename, NULL, flags, &local_err); +ret = bdrv_file_open(&bs->file, filename, NULL, NULL, flags, &local_err); if (ret < 0) { error_propagate(errp, local_err); goto fail; diff --git a/block/blkverify.c b/block/blkverify.c index 1c1637f..e15ac4c 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -141,7 +141,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } -ret = bdrv_file_open(&bs->file, raw, NULL, flags, &local_err); +ret = bdrv_file_open(&bs->file, raw, NULL, NULL, flags, &local_err); if (ret < 0) { error_propagate(errp, local_err); goto fail; diff --git a/block/cow.c b/block/cow.c index dc15e46..7fc0b12 100644 --- a/block/cow.c +++ b/block/cow.c @@ -351,7 +351,8 @@ static int cow_create(const char *filename, QEMUOptionParameter *options, return ret; } -ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, &local_err); +ret = bdrv_file_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR, + &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); diff --git a/block/qcow.c b/block/qcow.c index c470e05..948b0c5 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -691,7 +691,8 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options, return ret; } -ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, &local_err); +ret = bdrv_file_open(&qcow_bs, filename, NULL, NULL, BDRV_O_RDWR, + &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); diff --git a/block/qcow2.c b/block/qcow2.c index 8ec9db1..e15a4dd 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1483,7 +1483,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, return ret; } -ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err); +ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err); if (ret < 0) { error_propagate(errp, local_err); return ret; diff --git a/block/qed.c b/block/qed.c index 450a1fa..0dd5c58 100644 --- a/block/qed.c +++ b/block/qed.c @@ -563
[Qemu-devel] [PULL 27/93] block: Allow block devices without files
From: Max Reitz blkdebug and blkverify will, in order to retain compatibility, not support the field "file" implicitly through bdrv_open(). In order to be able to use those drivers without giving a filename anyway, it is necessary to be able to have block devices without files implicitly opened by bdrv_open(). This is the case, if there was neither a file name, a reference to an existing block device to use as a file nor options specific to the file. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index bef4f82..7464fb2 100644 --- a/block.c +++ b/block.c @@ -1145,11 +1145,14 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, qdict_extract_subqdict(options, &file_options, "file."); file_reference = qdict_get_try_str(options, "file"); -ret = bdrv_file_open(&file, filename, file_reference, file_options, - bdrv_open_flags(bs, flags | BDRV_O_UNMAP), &local_err); -qdict_del(options, "file"); -if (ret < 0) { -goto fail; +if (filename || file_reference || qdict_size(file_options)) { +ret = bdrv_file_open(&file, filename, file_reference, file_options, + bdrv_open_flags(bs, flags | BDRV_O_UNMAP), + &local_err); +qdict_del(options, "file"); +if (ret < 0) { +goto fail; +} } /* Find the right image format driver */ @@ -1165,7 +1168,13 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, } if (!drv) { -ret = find_image_format(file, filename, &drv, &local_err); +if (file) { +ret = find_image_format(file, filename, &drv, &local_err); +} else { +error_setg(errp, "Must specify either driver or file"); +ret = -EINVAL; +goto unlink_and_fail; +} } if (!drv) { @@ -1178,7 +1187,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, goto unlink_and_fail; } -if (bs->file != file) { +if (file && (bs->file != file)) { bdrv_unref(file); file = NULL; } -- 1.8.1.4
[Qemu-devel] [PULL 31/93] blockdev: Move "file" to legacy_opts
From: Max Reitz Specifying the image filename through the "file" option is a legacy option and should not be supported by blockdev-add (in that case, giving a string for "file" references an existing block device). Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- blockdev.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/blockdev.c b/blockdev.c index e457494..386109a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -307,12 +307,11 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType; /* Takes the ownership of bs_opts */ -static DriveInfo *blockdev_init(QDict *bs_opts, +static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, BlockInterfaceType type, Error **errp) { const char *buf; -const char *file = NULL; const char *serial; int ro = 0; int bdrv_flags = 0; @@ -354,7 +353,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, ro = qemu_opt_get_bool(opts, "read-only", 0); copy_on_read = qemu_opt_get_bool(opts, "copy-on-read", false); -file = qemu_opt_get(opts, "file"); serial = qemu_opt_get(opts, "serial"); if ((buf = qemu_opt_get(opts, "discard")) != NULL) { @@ -599,6 +597,10 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "addr", .type = QEMU_OPT_STRING, .help = "pci address (virtio only)", +},{ +.name = "file", +.type = QEMU_OPT_STRING, +.help = "file name", }, /* Options that are passed on, but have special semantics with -drive */ @@ -629,6 +631,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) const char *devaddr; bool read_only = false; bool copy_on_read; +const char *filename; Error *local_err = NULL; /* Change legacy command line options into QMP ones */ @@ -867,8 +870,10 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) } } +filename = qemu_opt_get(legacy_opts, "file"); + /* Actual block device init: Functionality shared with blockdev-add */ -dinfo = blockdev_init(bs_opts, type, &local_err); +dinfo = blockdev_init(filename, bs_opts, type, &local_err); if (dinfo == NULL) { if (error_is_set(&local_err)) { qerror_report_err(local_err); @@ -2210,7 +2215,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) qdict_flatten(qdict); -blockdev_init(qdict, IF_NONE, &local_err); +blockdev_init(NULL, qdict, IF_NONE, &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); goto fail; @@ -2251,10 +2256,6 @@ QemuOptsList qemu_common_drive_opts = { .type = QEMU_OPT_BOOL, .help = "enable/disable snapshot mode", },{ -.name = "file", -.type = QEMU_OPT_STRING, -.help = "disk image", -},{ .name = "discard", .type = QEMU_OPT_STRING, .help = "discard operation (ignore/off, unmap/on)", -- 1.8.1.4
[Qemu-devel] [PULL 18/93] blkdebug: Use errp for read_config()
From: Max Reitz Use an Error variable in the read_config() function. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/blkdebug.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index ebc5f13..2eb2e8b 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -271,7 +271,7 @@ static void remove_rule(BlkdebugRule *rule) g_free(rule); } -static int read_config(BDRVBlkdebugState *s, const char *filename) +static int read_config(BDRVBlkdebugState *s, const char *filename, Error **errp) { FILE *f; int ret; @@ -279,11 +279,14 @@ static int read_config(BDRVBlkdebugState *s, const char *filename) f = fopen(filename, "r"); if (f == NULL) { +error_setg_errno(errp, errno, "Could not read blkdebug config file"); return -errno; } ret = qemu_config_parse(f, config_groups, filename); if (ret < 0) { +error_setg(errp, "Could not parse blkdebug config file"); +ret = -EINVAL; goto fail; } @@ -370,9 +373,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, /* Read rules from config file */ config = qemu_opt_get(opts, "config"); if (config) { -ret = read_config(s, config); -if (ret < 0) { -error_setg_errno(errp, -ret, "Could not read blkdebug config file"); +ret = read_config(s, config, errp); +if (ret) { goto fail; } } -- 1.8.1.4
[Qemu-devel] [PULL 38/93] tests: Add test for qdict_array_split()
From: Max Reitz Add a test case for qdict_array_split() in tests/check-qdict.c. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/check-qdict.c | 80 + 1 file changed, 80 insertions(+) diff --git a/tests/check-qdict.c b/tests/check-qdict.c index dc5f05a..cab7dd0 100644 --- a/tests/check-qdict.c +++ b/tests/check-qdict.c @@ -227,6 +227,85 @@ static void qdict_iterapi_test(void) QDECREF(tests_dict); } +static void qdict_array_split_test(void) +{ +QDict *test_dict = qdict_new(); +QDict *dict1, *dict2; +QList *test_list; + +/* + * Test the split of + * + * { + * "1.x": 0, + * "3.y": 1, + * "0.a": 42, + * "o.o": 7, + * "0.b": 23 + * } + * + * to + * + * [ + * { + * "a": 42, + * "b": 23 + * }, + * { + * "x": 0 + * } + * ] + * + * and + * + * { + * "3.y": 1, + * "o.o": 7 + * } + * + * (remaining in the old QDict) + * + * This example is given in the comment of qdict_array_split(). + */ + +qdict_put(test_dict, "1.x", qint_from_int(0)); +qdict_put(test_dict, "3.y", qint_from_int(1)); +qdict_put(test_dict, "0.a", qint_from_int(42)); +qdict_put(test_dict, "o.o", qint_from_int(7)); +qdict_put(test_dict, "0.b", qint_from_int(23)); + +qdict_array_split(test_dict, &test_list); + +dict1 = qobject_to_qdict(qlist_pop(test_list)); +dict2 = qobject_to_qdict(qlist_pop(test_list)); + +g_assert(dict1); +g_assert(dict2); +g_assert(qlist_empty(test_list)); + +QDECREF(test_list); + +g_assert(qdict_get_int(dict1, "a") == 42); +g_assert(qdict_get_int(dict1, "b") == 23); + +g_assert(qdict_size(dict1) == 2); + +QDECREF(dict1); + +g_assert(qdict_get_int(dict2, "x") == 0); + +g_assert(qdict_size(dict2) == 1); + +QDECREF(dict2); + +g_assert(qdict_get_int(test_dict, "3.y") == 1); +g_assert(qdict_get_int(test_dict, "o.o") == 7); + +g_assert(qdict_size(test_dict) == 2); + +QDECREF(test_dict); +} + /* * Errors test-cases */ @@ -365,6 +444,7 @@ int main(int argc, char **argv) g_test_add_func("/public/del", qdict_del_test); g_test_add_func("/public/to_qdict", qobject_to_qdict_test); g_test_add_func("/public/iterapi", qdict_iterapi_test); +g_test_add_func("/public/array_split", qdict_array_split_test); g_test_add_func("/errors/put_exists", qdict_put_exists_test); g_test_add_func("/errors/get_not_exists", qdict_get_not_exists_test); -- 1.8.1.4