[Qemu-devel] buildbot failure in qemu on xen_x86_64_debian_6_0
The Buildbot has detected a new failure on builder xen_x86_64_debian_6_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/xen_x86_64_debian_6_0/builds/356 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_xen_next' triggered this build Build Source Stamp: [branch xen-next] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on xen_i386_debian_6_0
The Buildbot has detected a new failure on builder xen_i386_debian_6_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/xen_i386_debian_6_0/builds/357 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_xen_next' triggered this build Build Source Stamp: [branch xen-next] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on trivial-patches_i386_debian_6_0
The Buildbot has detected a new failure on builder trivial-patches_i386_debian_6_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/trivial-patches_i386_debian_6_0/builds/356 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_trivial-patches' triggered this build Build Source Stamp: [branch trivial-patches] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on trivial-patches_x86_64_debian_6_0
The Buildbot has detected a new failure on builder trivial-patches_x86_64_debian_6_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/trivial-patches_x86_64_debian_6_0/builds/356 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_trivial-patches' triggered this build Build Source Stamp: [branch trivial-patches] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on qmp_x86_64_debian_6_0
The Buildbot has detected a new failure on builder qmp_x86_64_debian_6_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/qmp_x86_64_debian_6_0/builds/355 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_qmp' triggered this build Build Source Stamp: [branch queue/qmp] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on qmp_i386_debian_6_0
The Buildbot has detected a new failure on builder qmp_i386_debian_6_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/qmp_i386_debian_6_0/builds/355 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_qmp' triggered this build Build Source Stamp: [branch queue/qmp] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
Re: [Qemu-devel] Is the return address of get_page_addr_code guest physical address?
On Fri, Aug 10, 2012 at 10:06 PM, Peter Maydell wrote: > On 10 August 2012 19:53, Steven wrote: >> On Fri, Aug 10, 2012 at 11:47 AM, Peter Maydell >> wrote: >>> On 10 August 2012 03:11, Steven wrote: The function definition has a return address type tb_page_addr_t. tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr) I am wondering is this address the guest physical address or the host virtual address. >>> >>> In linux-user mode the returned address is the guest virtual address. >>> In system mode it is a ram_addr_t. (the comment above the implementation >>> says "the returned address is not exactly the physical address: it >>> is the offset relative to phys_ram_base" but this is out of date I think). >>> A ram_addr_t is neither a host address nor a guest physical address >>> but it's closely related to a guest physaddr (you can think of it as >>> if all the RAM in the system was put into a straight line and then the >> >> My question is related to system mode. >> Is the RAM you mean the guest physical address, which is a continuous >> sequence of numbers beginning from 0 to the max of allocated RAM of >> the guest? >> >>> ram_addr_t is an index into that). >> >> If the returned value of get_page_addr_code is the index to that >> straight line, I am wondering if it is the guest physical address. > > No, it is definitely not the guest physical address. Consider > the case where there are two aliases of the same RAM in > guest physical memory -- two physical addresses might > map to a single ram_addr_t. Consider the case where there > is a 'hole' in memory -- ram_addr_t and physical address are > not identical there either. > >> So if I want to get the guest physical address (GPA) of a >> tb_page_addr_t, can I do >> tb_page_addr_t = returned value from get_page_addr_code + phys_ram_base >> Is this translation correct? > > This is wrong on several counts: (a) there's no such thing > as phys_ram_base any more (it was removed several years > ago) and (b) there is no single unique guest physical > address corresponding to a tb_page_addr_t, so what > you are trying to do is not well defined. > > What are you trying to do anyway and why do you want > to call get_page_addr_code() ? > > -- PMM I want to get the guest physical address of a pc. I note the part of the function cpu_x86_handle_mmu_fault will do something like page walking to convert a pc to its guest physical address. I think this is the guest physical address I need. However, there is no other function available to do this page walking. So I am thinking add a function to do the conversion. After you mentions about the memory region, do you think the following formula is correct guest_physical address = block->mr->addr + (pc's host virtual address - block->host) ^ ^ Base of the mapped memory block (offset in the memory block) Or do you have any suggestion? Thanks.
Re: [Qemu-devel] [PATCH] Fix QEMU
On 10 August 2012 20:48, Stefan Weil wrote: > One more... No problem with the patch but your commit message summary implies a rather wider effect than the patch actually has :-) -- PMM
Re: [Qemu-devel] Is the return address of get_page_addr_code guest physical address?
On 10 August 2012 19:53, Steven wrote: > On Fri, Aug 10, 2012 at 11:47 AM, Peter Maydell > wrote: >> On 10 August 2012 03:11, Steven wrote: >>> The function definition has a return address type tb_page_addr_t. >>> tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr) >>> >>> I am wondering is this address the guest physical address or the host >>> virtual address. >> >> In linux-user mode the returned address is the guest virtual address. >> In system mode it is a ram_addr_t. (the comment above the implementation >> says "the returned address is not exactly the physical address: it >> is the offset relative to phys_ram_base" but this is out of date I think). >> A ram_addr_t is neither a host address nor a guest physical address >> but it's closely related to a guest physaddr (you can think of it as >> if all the RAM in the system was put into a straight line and then the > > My question is related to system mode. > Is the RAM you mean the guest physical address, which is a continuous > sequence of numbers beginning from 0 to the max of allocated RAM of > the guest? > >> ram_addr_t is an index into that). > > If the returned value of get_page_addr_code is the index to that > straight line, I am wondering if it is the guest physical address. No, it is definitely not the guest physical address. Consider the case where there are two aliases of the same RAM in guest physical memory -- two physical addresses might map to a single ram_addr_t. Consider the case where there is a 'hole' in memory -- ram_addr_t and physical address are not identical there either. > So if I want to get the guest physical address (GPA) of a > tb_page_addr_t, can I do > tb_page_addr_t = returned value from get_page_addr_code + phys_ram_base > Is this translation correct? This is wrong on several counts: (a) there's no such thing as phys_ram_base any more (it was removed several years ago) and (b) there is no single unique guest physical address corresponding to a tb_page_addr_t, so what you are trying to do is not well defined. What are you trying to do anyway and why do you want to call get_page_addr_code() ? -- PMM
Re: [Qemu-devel] [PATCH 09/15] memory: prepare flatview and radix-tree for rcu style access
On Wed, Aug 8, 2012 at 5:41 PM, Avi Kivity wrote:
> On 08/08/2012 09:25 AM, Liu Ping Fan wrote:
>> From: Liu Ping Fan
>>
>> Flatview and radix view are all under the protection of pointer.
>> And this make sure the change of them seem to be atomic!
>>
>> The mr accessed by radix-tree leaf or flatview will be reclaimed
>> after the prev PhysMap not in use any longer
>>
>
> IMO this cleverness should come much later. Let's first take care of
> dropping the big qemu lock, then make swithcing memory maps more efficient.
>
> The initial paths could look like:
>
> lookup:
> take mem_map_lock
> lookup
> take ref
> drop mem_map_lock
>
> update:
> take mem_map_lock (in core_begin)
> do updates
> drop memo_map_lock
>
> Later we can replace mem_map_lock with either a rwlock or (real) rcu.
>
>
>>
>> #if !defined(CONFIG_USER_ONLY)
>>
>> -static void phys_map_node_reserve(unsigned nodes)
>> +static void phys_map_node_reserve(PhysMap *map, unsigned nodes)
>> {
>> -if (phys_map_nodes_nb + nodes > phys_map_nodes_nb_alloc) {
>> +if (map->phys_map_nodes_nb + nodes > map->phys_map_nodes_nb_alloc) {
>> typedef PhysPageEntry Node[L2_SIZE];
>> -phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc * 2, 16);
>> -phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc,
>> - phys_map_nodes_nb + nodes);
>> -phys_map_nodes = g_renew(Node, phys_map_nodes,
>> - phys_map_nodes_nb_alloc);
>> +map->phys_map_nodes_nb_alloc = MAX(map->phys_map_nodes_nb_alloc * 2,
>> +16);
>> +map->phys_map_nodes_nb_alloc = MAX(map->phys_map_nodes_nb_alloc,
>> + map->phys_map_nodes_nb + nodes);
>> +map->phys_map_nodes = g_renew(Node, map->phys_map_nodes,
>> + map->phys_map_nodes_nb_alloc);
>> }
>> }
>
> Please have a patch that just adds the map parameter to all these
> functions. This makes the later patch, that adds the copy, easier to read.
>
>> +
>> +void cur_map_update(PhysMap *next)
>> +{
>> +qemu_mutex_lock(&cur_map_lock);
>> +physmap_put(cur_map);
>> +cur_map = next;
>> +smp_mb();
>> +qemu_mutex_unlock(&cur_map_lock);
>> +}
>
> IMO this can be mem_map_lock.
>
> If we take my previous suggestion:
>
> lookup:
> take mem_map_lock
> lookup
> take ref
> drop mem_map_lock
>
> update:
> take mem_map_lock (in core_begin)
> do updates
> drop memo_map_lock
>
> And update it to
>
>
> update:
> prepare next_map (in core_begin)
> do updates
> take mem_map_lock (in core_commit)
> switch maps
> drop mem_map_lock
> free old map
>
>
> Note the lookup path copies the MemoryRegionSection instead of
> referencing it. Thus we can destroy the old map without worrying; the
> only pointers will point to MemoryRegions, which will be protected by
> the refcounts on their Objects.
>
Just find there may be a leak here. If mrs points to subpage, then the
subpage_t could be crashed by destroy.
To avoid such situation, we can walk down the chain to pin us on the
Object based mr, but then we must expose the address convert in
subpage_read() right here. Right?
Regards,
pingfan
> This can be easily switched to rcu:
>
> update:
> prepare next_map (in core_begin)
> do updates
> switch maps - rcu_assign_pointer
> call_rcu(free old map) (or synchronize_rcu; free old maps)
>
> Again, this should be done after the simplictic patch that enables
> parallel lookup but keeps just one map.
>
>
>
> --
> error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error.
On Mon, Aug 6, 2012 at 7:41 PM, Peter Maydell wrote: > On 6 August 2012 03:16, Peter A. G. Crosthwaite > wrote: >> To be more consistent with the newer ways of error signalling. That and >> SIGABT >> is easier to debug with than exit(1). >> >> Signed-off-by: Peter A. G. Crosthwaite > > Reviewed-by: Peter Maydell > This is independent of the rest of the series, do we want to en-queue to arm-devs and ill remove from series? One less diff for me and one less spam for the mailing list :) Regards, Peter > -- PMM
[Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser. The command is here for
reference, and can be issued against upstream QMP to reproduce (failure
occurs before any qmp command routing/execution):
http://pastebin.com/mJrZ3Ctg
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices. With this patch applied the above QMP/JSON request
can be parsed in under a second.
Tested with valgrind, make check, and QMP.
Signed-off-by: Michael Roth
---
json-parser.c | 234 +++--
1 file changed, 146 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..b4e6a60 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+struct {
+QObject **buf;
+size_t pos;
+size_t count;
+} tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list
*ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,115 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+ctxt->tokens.pos++;
+return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be * tracked and cleanup in parser_context_free()
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+JSONParserContext saved_ctxt;
+saved_ctxt.tokens.pos = ctxt->tokens.pos;
+saved_ctxt.tokens.count = ctxt->tokens.count;
+saved_ctxt.tokens.buf = ctxt->tokens.buf;
+return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ctxt->tokens.count = saved_ctxt.tokens.count;
+ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_count_from_iter(QObject *obj, void *opaque)
+{
+size_t *count = opaque;
+(*count)++;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+JSONParserContext *ctxt = opaque;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+JSONParserContext *ctxt;
+size_t count = 0;
+
+if (!tokens) {
+return NULL;
+}
+
+qlist_iter(tokens, tokens_count_from_iter, &count);
+if (count == 0) {
+return NULL;
+}
+
+ctxt = g_malloc0(sizeof(JSONParserContext));
+ctxt->tokens.pos = 0;
+ctxt->tokens.count = count;
+ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ctxt->tokens.pos = 0;
+
+return ctxt;
+}
+
+static void parser_context_free(JSONParserContext *ctxt)
+{
+int i;
+if (ctxt) {
+for (i = 0; i < ctxt->tokens.count; i++) {
+qobject_decref(ctxt->tokens.buf[i]);
+}
+g_free(ctxt->tokens.buf);
+g_free(ctxt);
+}
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens,
va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
-QList *working = qlist_copy(*tokens);
+JSONParserContext saved_ctxt = parser_context_save(ctxt);
-peek = qlist_peek(working);
+peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
-key = parse_value(ctxt, &working, ap);
+key = parse_value(ctxt, ap);
if (!key || qobject_type(key) !=
Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization
Hi Andreas, > > I find quilt patches easier to manage when I need to reorder them, > > revert, manually edit the diffs (that I routinely do), etc. Perhaps I'm > > just outdated, but that's the workflow I've found most efficient for me > > while not disturbing anyone else. I've used quilt patches with the Linux > > kernel including upstream submission successfully as well, for many years > > now, and I don't remember anyone complaining about their formatting. > > Also automatic patch retriever/tester scripts that some mailing lists have > > watching process them correctly. > > > > Let's therefore please focus on the technical value of these changes > > rather than their envelope. > > Both are important to those of us reviewing and working in maintenance. > > http://wiki.qemu.org/Contribute/SubmitAPatch > > For example, our usual convention would've been to use a "target-mips:" > rather than "MIPS:" prefix (the directory name), if you follow the list > and history. We also don't usually indent paragraphs within the commit > message, especially not with differing indentation, and our Coding Style > is without space before parenthesis. Patches that look odd may get less > review attention. Not everything is mandatory of course, but maybe you > can also see the other side of being flooded with patches. Sure, I'll take your suggestions into account as much as I can, that's not a problem, I see your point and I don't want to make your work harder than it already is -- I know a good technical review can take a lot of effort, especially with complicated changes. I just addressed the issue of the diff itself (or actually the heading only) that you specifically raised. I read the document before posting those proposals and I've thought I got all the points right, but it looks that I missed a bit here and there -- apologies for that. > > I don't recollect retracting any of my patches although I'll be making > > the adjustments previously requested and produce more. Any patches that > > may have overlapped with an earlier submission come from the same tree, > > except I regenerated them and retested against the then current top of the > > tree; I may have updated them too to address any problems spotted while > > processing them. > > OK, so you didn't retract them but Meador did comment: > > > I submitted a patch to fix this issue and the FCR0 issue a few months back > > [1]. > > Andreas reviewed it, but the patch never got committed. > > > > [1] http://patchwork.ozlabs.org/patch/144353/ > > They're definitely different in scope and changes, whatever process and > tools you guys use internally. And our policy is to give preference to > the earlier patch to not invite people to redo other people's patches > with different SoB (not saying you are, same company after all). I have had a look and it appears to me this is merely a number of patches that I posted as individual changes folded into one. Personally I try to follow the one-change-at-a-time principle whenever possible for easier tracking down any issues that may arise later on if nothing else (e.g. `git bisect'), so I maintain it's better if they're committed separately. > Either way, the committed patch is now missing the info that this issue > was originally Reported-by and attempted to fix by Khansa Butt, not > employed by Mentor nor using their tree. Hopefully that can be fixed up. Please note that Richard Henderson had concerns about these growing functions having been defined as static inline. I agree with that concern, technically; the original choice merely having been by following the convention observed elsewhere throughout QEMU's tree under the assumption the simulator is unusual enough a piece of software that there must have been a valid justification for such an arrangement. I'll be fixing this up with the repost. > >> Doing follow-ups based on this one or, in worst case, reverting is > >> certainly possible but the decision-making would best be done by someone > >> who actually uses mips - not that there's no users, just no volunteer > >> for a staging tree yet. > > > > I'm willing to provide assistance as time permits, in particular to move > > forward with any changes I have proposed either myself or on behalf of > > someone else, although owing to other commitments regrettably I can't > > commit to regular testing/mainentance. > > You could keep the status in MAINTAINERS as "Odd Fixes" but set up a git > branch where maintainers can pull from and that you / other contributors > can develop against. As I say, I feel I am too constrained, at least the moment, to give QEMU the level of attention it deserves, but certainly I am going to address any concerns anyone may have about my changes, be it at the submission time, or anytime in the future. Feel free to ask me about any hardware specifics as well. > Me, I'm still interested in modelling MIPS CPU models as proper QOM > su
Re: [Qemu-devel] [PATCH] device_tree: load_device_tree(): Allow NULL sizep
On Fri, Aug 10, 2012 at 11:42 PM, Stefan Hajnoczi wrote:
> On Fri, Aug 10, 2012 at 01:54:26PM +1000, Peter A. G. Crosthwaite wrote:
>> The sizep arg is populated with the size of the loaded device tree. Since
>> this
>> is one of those informational "please populate" type arguments it should be
>> optional. Guarded writes to *sizep against NULL accordingly.
>>
>> Signed-off-by: Peter A. G. Crosthwaite
>> Acked-by: Alexander Graf
>> ---
>> device_tree.c |8 ++--
>> 1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/device_tree.c b/device_tree.c
>> index d7a9b6b..641a48a 100644
>> --- a/device_tree.c
>> +++ b/device_tree.c
>> @@ -71,7 +71,9 @@ void *load_device_tree(const char *filename_path, int
>> *sizep)
>> int ret;
>> void *fdt = NULL;
>>
>> -*sizep = 0;
>> +if (sizep) {
>> +*sizep = 0;
>> +}
>> dt_size = get_image_size(filename_path);
>> if (dt_size < 0) {
>> printf("Unable to get size of device tree file '%s'\n",
>> @@ -104,7 +106,9 @@ void *load_device_tree(const char *filename_path, int
>> *sizep)
>> filename_path);
>> goto fail;
>> }
>> -*sizep = dt_size;
>> +if (sizep) {
>> +*sizep = dt_size;
>> +}
>
> What can the caller do with this void* buffer without knowing its size?
>
Sanity check the machine:
dtb = load_device_tree( ... ); //dont care how big it is
foo = fdt_gep_prop( dtb, ... );
if (foo != object_get_prop(foo_device, foo_prop, ... )) {
hw_error("your dtb is bad because ... !\n", ... );
}
...
boot();
This happens at machine init, which is quite separate from boot.
> They cannot hand the buffer to the guest unless they duplicate the
> get_image_size() call, which is pointless, or we're assuming a fixed
> size, which is unsafe. I'm asking what the legitimate use case for
> sizep == NULL is?
Sanity check the dtb without passing it to the guest. My CAD tools
emit a DTB for the hardware, but it wont always go through a linux
bootloader (E.G. I may want to boot random elfs). I still will pass
the CAD generated DTB to QEMU for sanity check however so qemu can
give me a nice report of what is and isn't modelled.
Regards,
Peter
>
> Stefan
Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization
On 08/10/2012 09:30 AM, Andreas Färber wrote: > OK, so you didn't retract them but Meador did comment: > >> > I submitted a patch to fix this issue and the FCR0 issue a few months back >> > [1]. >> > Andreas reviewed it, but the patch never got committed. >> > >> > [1] http://patchwork.ozlabs.org/patch/144353/ > They're definitely different in scope and changes, whatever process and > tools you guys use internally. And our policy is to give preference to > the earlier patch to not invite people to redo other people's patches > with different SoB (not saying you are, same company after all). Please consider my patch retracted. Maciej's work superseded mine. -- Meador Inge CodeSourcery / Mentor Embedded http://www.mentor.com/embedded-software
Re: [Qemu-devel] [PATCH v2 0/2] Probe the guest memory space when using -R
Ping ^ 2. On 08/01/2012 01:47 PM, Meador Inge wrote: > Ping. > > On 07/26/2012 09:50 PM, Meador Inge wrote: >> Hi, >> >> This patch series fixes an issue that was discussed here [1] where using -R >> can cause QEMU to fail to setup the guest address space because the guest >> base >> validation fails. I fixed this issue by (1) refactoring the guest space >> probing code into a single function for initializing the guest space and (2) >> by >> calling the guest space initialization code for both the case of reserving >> the >> guest space upfront (-R) and the case where the initial memory space >> base/size >> are gleaned from an ELF image. >> >> Tested by going through various combinations of -R , -B , >> -B -R , and neither -R or -B passed. I also ran the libstdc++ >> testsuite through the MIPS, ARM, and Power usermode emulators with -R set. >> No regressions. >> >> NOTE: This does not fix the problem that was raised concerning mapped the >> full 32-bit address space on a 64-bit system. That will need to be another >> patch. >> >> - Changes since v1: >> >>* Replaced '!host_start && !host_size' error check in 'init_guest_space' >> with an assert. >> >>* Ensured that 'guest_validate_base' is passed the true guest base instead >> of the current host start address. >> >>* s/init_guest_space(..., 0)/init_guest_space(..., false);/ >> >>* Fixed typo in 'init_guest_space' header comment. >> >> [1] http://lists.nongnu.org/archive/html/qemu-devel/2012-06/msg04508.html >> >> Signed-off-by: Meador Inge >> >> Meador Inge (2): >> linux-user: Factor out guest space probing into a function >> linux-user: Use init_guest_space when -R and -B are specified >> >> linux-user/elfload.c | 161 >> ++ >> linux-user/main.c| 35 ++- >> linux-user/qemu.h| 15 - >> 3 files changed, 140 insertions(+), 71 deletions(-) >> > > -- Meador Inge CodeSourcery / Mentor Embedded http://www.mentor.com/embedded-software
[Qemu-devel] [PATCH] srp: Don't use QEMU_PACKED for single elements of a structured type
QEMU_PACKED results in a MinGW compiler warning when it is
used for single structure elements:
warning: 'gcc_struct' attribute ignored
Using QEMU_PACKED for the whole structure avoids the compiler warning
without changing the memory layout.
Signed-off-by: Stefan Weil
---
hw/srp.h |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/srp.h b/hw/srp.h
index 3009bd5..5e0cad5 100644
--- a/hw/srp.h
+++ b/hw/srp.h
@@ -177,13 +177,13 @@ struct srp_tsk_mgmt {
uint8_treserved1[6];
uint64_t tag;
uint8_treserved2[4];
-uint64_t lun QEMU_PACKED;
+uint64_t lun;
uint8_treserved3[2];
uint8_ttsk_mgmt_func;
uint8_treserved4;
uint64_t task_tag;
uint8_treserved5[8];
-};
+} QEMU_PACKED;
/*
* We need the packed attribute because the SRP spec only aligns the
@@ -198,14 +198,14 @@ struct srp_cmd {
uint8_tdata_in_desc_cnt;
uint64_t tag;
uint8_treserved2[4];
-uint64_t lun QEMU_PACKED;
+uint64_t lun;
uint8_treserved3;
uint8_ttask_attr;
uint8_treserved4;
uint8_tadd_cdb_len;
uint8_tcdb[16];
uint8_tadd_data[0];
-};
+} QEMU_PACKED;
enum {
SRP_RSP_FLAG_RSPVALID = 1 << 0,
--
1.7.0.4
[Qemu-devel] [PATCH] Spelling fixes in comments and documentation
These wrong spellings were detected by codespell:
* successully -> successfully
* alot -> a lot
* wanna -> want to
* infomation -> information
* occured -> occurred
Signed-off-by: Stefan Weil
---
docs/specs/ppc-spapr-hcalls.txt |2 +-
docs/usb2.txt |4 ++--
hw/xen_pt.h |4 ++--
hw/xen_pt_config_init.c | 14 +++---
qemu-img.c |2 +-
qemu-img.texi |2 +-
6 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/docs/specs/ppc-spapr-hcalls.txt b/docs/specs/ppc-spapr-hcalls.txt
index 52ba8d4..667b3fa 100644
--- a/docs/specs/ppc-spapr-hcalls.txt
+++ b/docs/specs/ppc-spapr-hcalls.txt
@@ -31,7 +31,7 @@ Arguments:
Returns:
- H_SUCCESS : Successully called the RTAS function (RTAS result
+ H_SUCCESS : Successfully called the RTAS function (RTAS result
will have been stored in the parameter block)
H_PARAMETER : Unknown token
diff --git a/docs/usb2.txt b/docs/usb2.txt
index d17e3c0..21f6d14 100644
--- a/docs/usb2.txt
+++ b/docs/usb2.txt
@@ -58,11 +58,11 @@ try ...
xhci controller support
---
-There also is xhci host controller support available. It got alot
+There also is xhci host controller support available. It got a lot
less testing than ehci and there are a bunch of known limitations, so
ehci may work better for you. On the other hand the xhci hardware
design is much more virtualization-friendly, thus xhci emulation uses
-less ressources (especially cpu). If you wanna give xhci a try
+less ressources (especially cpu). If you want to give xhci a try
use this to add the host controller ...
qemu -device nec-usb-xhci,id=xhci
diff --git a/hw/xen_pt.h b/hw/xen_pt.h
index 41904ec..112477a 100644
--- a/hw/xen_pt.h
+++ b/hw/xen_pt.h
@@ -96,7 +96,7 @@ typedef struct XenPTRegion {
* - do NOT use ALL F for init_val, otherwise the tbl will not be registered.
*/
-/* emulated register infomation */
+/* emulated register information */
struct XenPTRegInfo {
uint32_t offset;
uint32_t size;
@@ -140,7 +140,7 @@ typedef int (*xen_pt_reg_size_init_fn)
(XenPCIPassthroughState *, const XenPTRegGroupInfo *,
uint32_t base_offset, uint8_t *size);
-/* emulated register group infomation */
+/* emulated register group information */
struct XenPTRegGroupInfo {
uint8_t grp_id;
XenPTRegisterGroupType grp_type;
diff --git a/hw/xen_pt_config_init.c b/hw/xen_pt_config_init.c
index 00eb3d9..e524a40 100644
--- a/hw/xen_pt_config_init.c
+++ b/hw/xen_pt_config_init.c
@@ -562,7 +562,7 @@ static int
xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
return 0;
}
-/* Header Type0 reg static infomation table */
+/* Header Type0 reg static information table */
static XenPTRegInfo xen_pt_emu_reg_header0[] = {
/* Vendor ID reg */
{
@@ -753,7 +753,7 @@ static XenPTRegInfo xen_pt_emu_reg_header0[] = {
* Vital Product Data Capability
*/
-/* Vital Product Data Capability Structure reg static infomation table */
+/* Vital Product Data Capability Structure reg static information table */
static XenPTRegInfo xen_pt_emu_reg_vpd[] = {
{
.offset = PCI_CAP_LIST_NEXT,
@@ -775,7 +775,7 @@ static XenPTRegInfo xen_pt_emu_reg_vpd[] = {
* Vendor Specific Capability
*/
-/* Vendor Specific Capability Structure reg static infomation table */
+/* Vendor Specific Capability Structure reg static information table */
static XenPTRegInfo xen_pt_emu_reg_vendor[] = {
{
.offset = PCI_CAP_LIST_NEXT,
@@ -866,7 +866,7 @@ static int xen_pt_linkctrl2_reg_init(XenPCIPassthroughState
*s,
return 0;
}
-/* PCI Express Capability Structure reg static infomation table */
+/* PCI Express Capability Structure reg static information table */
static XenPTRegInfo xen_pt_emu_reg_pcie[] = {
/* Next Pointer reg */
{
@@ -981,7 +981,7 @@ static int xen_pt_pmcsr_reg_write(XenPCIPassthroughState *s,
return 0;
}
-/* Power Management Capability reg static infomation table */
+/* Power Management Capability reg static information table */
static XenPTRegInfo xen_pt_emu_reg_pm[] = {
/* Next Pointer reg */
{
@@ -1259,7 +1259,7 @@ static int
xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
return 0;
}
-/* MSI Capability Structure reg static infomation table */
+/* MSI Capability Structure reg static information table */
static XenPTRegInfo xen_pt_emu_reg_msi[] = {
/* Next Pointer reg */
{
@@ -1396,7 +1396,7 @@ static int
xen_pt_msixctrl_reg_write(XenPCIPassthroughState *s,
return 0;
}
-/* MSI-X Capability Structure reg static infomation table */
+/* MSI-X Capability Structure reg static information table */
static XenPTRegInfo xen_pt_emu_reg_msix[] = {
/* Next Pointer reg */
{
diff --git a/qemu-img.c b/qemu-img.c
index f20116a..3a21cc1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -89,7 +89,7 @@ static void help(void)
[Qemu-devel] [PATCH] Fix spelling (licenced -> licensed) in GPL
The patch also fixes the case of "written". Signed-off-by: Stefan Weil --- hw/imx_avic.c |4 ++-- hw/imx_timer.c |4 ++-- hw/kzm.c |2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/imx_avic.c b/hw/imx_avic.c index 4f010e8..b1a8fe6 100644 --- a/hw/imx_avic.c +++ b/hw/imx_avic.c @@ -6,9 +6,9 @@ * * Copyright (c) 2008 OKL * Copyright (c) 2011 NICTA Pty Ltd - * Originally Written by Hans Jiang + * Originally written by Hans Jiang * - * This code is licenced under the GPL version 2 or later. See + * This code is licensed under the GPL version 2 or later. See * the COPYING file in the top-level directory. * * TODO: implement vectors. diff --git a/hw/imx_timer.c b/hw/imx_timer.c index 16215cc..c28c537 100644 --- a/hw/imx_timer.c +++ b/hw/imx_timer.c @@ -3,10 +3,10 @@ * * Copyright (c) 2008 OK Labs * Copyright (c) 2011 NICTA Pty Ltd - * Originally Written by Hans Jiang + * Originally written by Hans Jiang * Updated by Peter Chubb * - * This code is licenced under GPL version 2 or later. See + * This code is licensed under GPL version 2 or later. See * the COPYING file in the top-level directory. * */ diff --git a/hw/kzm.c b/hw/kzm.c index 6a5e9df..68cd1b4 100644 --- a/hw/kzm.c +++ b/hw/kzm.c @@ -5,7 +5,7 @@ * Written by Hans at OK-Labs * Updated by Peter Chubb. * - * This code is licenced under the GPL, version 2 or later. + * This code is licensed under the GPL, version 2 or later. * See the file `COPYING' in the top level directory. * * It (partially) emulates a Kyoto Microcomputer -- 1.7.0.4
[Qemu-devel] [PATCH] Spelling fix in comment (peripherans -> peripherals)
Signed-off-by: Stefan Weil --- hw/versatilepb.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/versatilepb.c b/hw/versatilepb.c index 4fd5d9b..7a92034 100644 --- a/hw/versatilepb.c +++ b/hw/versatilepb.c @@ -162,7 +162,7 @@ static int vpb_sic_init(SysBusDevice *dev) /* Board init. */ /* The AB and PB boards both use the same core, just with different - peripherans and expansion busses. For now we emulate a subset of the + peripherals and expansion busses. For now we emulate a subset of the PB peripherals and just change the board ID. */ static struct arm_boot_info versatile_binfo; -- 1.7.10
[Qemu-devel] [PATCH] docs: Fix spelling (propery -> property)
Signed-off-by: Stefan Weil --- docs/bootindex.txt |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/bootindex.txt b/docs/bootindex.txt index 16083b3..803ebfc 100644 --- a/docs/bootindex.txt +++ b/docs/bootindex.txt @@ -1,4 +1,4 @@ -= Bootindex propery = += Bootindex property = Block and net devices have bootindex property. This property is used to determine the order in which firmware will consider devices for booting -- 1.7.10
[Qemu-devel] [PATCH] vnc: Improve comment
It's still not good, but hopefully better than before. Signed-off-by: Stefan Weil --- ui/vnc-enc-zywrle.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/vnc-enc-zywrle.h b/ui/vnc-enc-zywrle.h index 188e247..addee55 100644 --- a/ui/vnc-enc-zywrle.h +++ b/ui/vnc-enc-zywrle.h @@ -305,8 +305,8 @@ static inline void harr(int8_t *px0, int8_t *px1) |L1H0H1H0|L1H0H1H0|L1H0H1H0|L1H0H1H0| : level 1 In this method, H/L and X0/X1 is always same position. - This lead us to more speed and less memory. - Of cause, the result of both methods is quite same + This leads us to more speed and less memory. + Of course, the result of both methods is quite same because its only difference is that coefficient position. */ static inline void wavelet_level(int *data, int size, int l, int skip_pixel) -- 1.7.10
[Qemu-devel] [PATCH] Fix QEMU
One more...
Signed-off-by: Stefan Weil
---
scripts/simpletrace.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
index 1e74e75..9fce4bb 100755
--- a/scripts/simpletrace.py
+++ b/scripts/simpletrace.py
@@ -71,7 +71,7 @@ def read_trace_file(edict, fobj):
log_version = header[2]
if log_version == 0:
-raise ValueError('Older log format, not supported with this Qemu
release!')
+raise ValueError('Older log format, not supported with this QEMU
release!')
while True:
rec = read_record(edict, fobj)
--
1.7.10
[Qemu-devel] [PATCH] w64: Fix compiler warning [-Wformat]
Glib2 uses __printf__ in macro G_GNUC_PRINTF for printf like
functions. For MinGW, we want __gnu_printf__ because we use
POSIX format specifiers instead of the MS format specifiers.
Signed-off-by: Stefan Weil
---
configure |2 ++
1 file changed, 2 insertions(+)
diff --git a/configure b/configure
index 7cbf4b4..7e69574 100755
--- a/configure
+++ b/configure
@@ -512,6 +512,8 @@ if test "$mingw32" = "yes" ; then
QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS"
# enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
+ # GLIB2 and other libraries also support C99/POSIX format strings.
+ QEMU_CFLAGS="-D__printf__=__gnu_printf__ $QEMU_CFLAGS"
LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
cat > $TMPC << EOF
int main(void) { return 0; }
--
1.7.10
Re: [Qemu-devel] [PATCH 28/35] error: drop unused functions
On 08/10/2012 11:44 AM, Luiz Capitulino wrote: > Besides of being unused, they operate on the current error format, s/ of// > which is going to be replaced soon. > -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 07/35] qerror: QError: drop file, linenr, func
They have never been fully used and conflict with future error
improvements.
Also makes qerror_report() a proper function, as there's no point
in having it as a macro anymore.
Signed-off-by: Luiz Capitulino
---
qerror.c | 20 +++-
qerror.h | 8 +---
2 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/qerror.c b/qerror.c
index e717496..6f9f49c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -404,27 +404,14 @@ static const QErrorStringTable *get_desc_no_fail(const
char *fmt)
/**
* qerror_from_info(): Create a new QError from error information
*
- * The information consists of:
- *
- * - file the file name of where the error occurred
- * - linenr the line number of where the error occurred
- * - func the function name of where the error occurred
- * - fmtJSON printf-like dictionary, there must exist keys 'class' and
- * 'data'
- * - va va_list of all arguments specified by fmt
- *
* Return strong reference.
*/
-static QError *qerror_from_info(const char *file, int linenr, const char *func,
-const char *fmt, va_list *va)
+static QError *qerror_from_info(const char *fmt, va_list *va)
{
QError *qerr;
qerr = qerror_new();
loc_save(&qerr->loc);
-qerr->linenr = linenr;
-qerr->file = file;
-qerr->func = func;
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
qerr->entry = get_desc_no_fail(fmt);
@@ -545,14 +532,13 @@ static void qerror_print(QError *qerror)
QDECREF(qstring);
}
-void qerror_report_internal(const char *file, int linenr, const char *func,
-const char *fmt, ...)
+void qerror_report(const char *fmt, ...)
{
va_list va;
QError *qerror;
va_start(va, fmt);
-qerror = qerror_from_info(file, linenr, func, fmt, &va);
+qerror = qerror_from_info(fmt, &va);
va_end(va);
if (monitor_cur_is_qmp()) {
diff --git a/qerror.h b/qerror.h
index fe8870c..3c0b14c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -27,20 +27,14 @@ typedef struct QError {
QObject_HEAD;
QDict *error;
Location loc;
-int linenr;
-const char *file;
-const char *func;
const QErrorStringTable *entry;
} QError;
QString *qerror_human(const QError *qerror);
-void qerror_report_internal(const char *file, int linenr, const char *func,
-const char *fmt, ...) GCC_FMT_ATTR(4, 5);
+void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
void qerror_report_err(Error *err);
void assert_no_error(Error *err);
QString *qerror_format(const char *fmt, QDict *error);
-#define qerror_report(fmt, ...) \
-qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 16/35] qerror: drop QERR_SOCKET_CONNECT_IN_PROGRESS
Unused since last commit.
Signed-off-by: Luiz Capitulino
---
qerror.c | 4
qerror.h | 3 ---
2 files changed, 7 deletions(-)
diff --git a/qerror.c b/qerror.c
index 5d38428..452ec69 100644
--- a/qerror.c
+++ b/qerror.c
@@ -309,10 +309,6 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Could not start VNC server on %(target)",
},
{
-.error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
-.desc = "Connection can not be completed immediately",
-},
-{
.error_fmt = QERR_SOCKET_CONNECT_FAILED,
.desc = "Failed to connect to socket",
},
diff --git a/qerror.h b/qerror.h
index de8497d..52ce58d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -240,9 +240,6 @@ char *qerror_format(const char *fmt, QDict *error);
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
-#define QERR_SOCKET_CONNECT_IN_PROGRESS \
-"{ 'class': 'SockConnectInprogress', 'data': {} }"
-
#define QERR_SOCKET_CONNECT_FAILED \
"{ 'class': 'SockConnectFailed', 'data': {} }"
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 17/35] block: block_int: include qerror.h
Several block/ files are relying on qerror.h being provided by qapi-types.h. Fix this, as a future commit will change qapi-types.h not to provide qerror.h. Signed-off-by: Luiz Capitulino --- block_int.h | 1 + 1 file changed, 1 insertion(+) diff --git a/block_int.h b/block_int.h index 6c1d9ca..4452f6f 100644 --- a/block_int.h +++ b/block_int.h @@ -30,6 +30,7 @@ #include "qemu-coroutine.h" #include "qemu-timer.h" #include "qapi-types.h" +#include "qerror.h" #define BLOCK_FLAG_ENCRYPT 1 #define BLOCK_FLAG_COMPAT6 4 -- 1.7.11.2.249.g31c7954.dirty
Re: [Qemu-devel] [PATCH 00/10] Quorum disk image corruption resiliency
Le Wednesday 08 Aug 2012 à 15:55:58 (+0100), Stefan Hajnoczi a écrit : > On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote: > > This patchset create a block driver implementing a quorum using three qemu > > disk > > images. Writes are mirrored on the three files. > > For the reading part the three files are read at the same time and a vote is > > done to determine which is the majority qiov version. It then return this > > majority version to the upper layers. > > When three differents versions of the data are returned by the lower layer > > the > > quorum is broken and the read return -EIO. > > If you make the quorum setting configurable, then this can replace > blkverify. n=2 is blkverify, n=3 is your current patch series, n/m is > also possible where n=number of mirrors and m=threshold needed to > achieve quorum. I am working on making the quorum settings configurable. I will post the new patchset next week. Benoît > > Stefan >
[Qemu-devel] [PATCH 11/35] qmp: query-block: add 'encryption_key_missing' field
Signed-off-by: Luiz Capitulino
---
block.c | 1 +
qapi-schema.json | 9 ++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 24323c1..016858b 100644
--- a/block.c
+++ b/block.c
@@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
info->value->inserted->ro = bs->read_only;
info->value->inserted->drv = g_strdup(bs->drv->format_name);
info->value->inserted->encrypted = bs->encrypted;
+info->value->inserted->encryption_key_missing =
bdrv_key_required(bs);
if (bs->backing_file[0]) {
info->value->inserted->has_backing_file = true;
info->value->inserted->backing_file =
g_strdup(bs->backing_file);
diff --git a/qapi-schema.json b/qapi-schema.json
index bd9c450..a62bf68 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -402,6 +402,9 @@
#
# @encrypted: true if the backing device is encrypted
#
+# @encryption_key_missing: true if the backing device is encrypted but an
+# valid encryption key is missing
+#
# @bps: total throughput limit in bytes per second is specified
#
# @bps_rd: read throughput limit in bytes per second is specified
@@ -421,9 +424,9 @@
{ 'type': 'BlockDeviceInfo',
'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
'*backing_file': 'str', 'backing_file_depth': 'int',
-'encrypted': 'bool', 'bps': 'int', 'bps_rd': 'int',
-'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int',
-'iops_wr': 'int'} }
+'encrypted': 'bool', 'encryption_key_missing': 'bool',
+'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
+'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
##
# @BlockDeviceIoStatus:
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 32/35] error, qerror: pass desc string to error calls
This commit changes all QERR_ macros to contain a human message (ie.
the desc string found in qerr_table[]) instead of a json dictionary
in string format.
Before this commit, error_set() and qerror_report() would receive
a json dictionary in string format and build a qobject from it. Now,
both function receive a human message instead and the qobject is
not built anymore.
Signed-off-by: Luiz Capitulino
---
error.c | 3 +-
error.h | 10 ++---
qerror.c | 42 +--
qerror.h | 141 +++
4 files changed, 77 insertions(+), 119 deletions(-)
diff --git a/error.c b/error.c
index 9d7b35f..0e10373 100644
--- a/error.c
+++ b/error.c
@@ -37,9 +37,8 @@ void error_set(Error **errp, ErrorClass err_class, const char
*fmt, ...)
err = g_malloc0(sizeof(*err));
va_start(ap, fmt);
-err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
+err->msg = g_strdup_vprintf(fmt, ap);
va_end(ap);
-err->msg = qerror_format(fmt, err->obj);
err->err_class = err_class;
*errp = err;
diff --git a/error.h b/error.h
index 5336fc5..96fc203 100644
--- a/error.h
+++ b/error.h
@@ -17,15 +17,15 @@
#include
/**
- * A class representing internal errors within QEMU. An error has a string
- * typename and optionally a set of named string parameters.
+ * A class representing internal errors within QEMU. An error has a ErrorClass
+ * code and a human message.
*/
typedef struct Error Error;
/**
- * Set an indirect pointer to an error given a printf-style format parameter.
- * Currently, qerror.h defines these error formats. This function is not
- * meant to be used outside of QEMU.
+ * Set an indirect pointer to an error given a ErrorClass value and a
+ * printf-style human message. This function is not meant to be used outside
+ * of QEMU.
*/
void error_set(Error **err, ErrorClass err_class, const char *fmt, ...)
GCC_FMT_ATTR(3, 4);
diff --git a/qerror.c b/qerror.c
index 0bf8aec..dda1427 100644
--- a/qerror.c
+++ b/qerror.c
@@ -342,45 +342,6 @@ static QError *qerror_new(void)
return qerr;
}
-static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
-{
-QObject *obj;
-QDict *ret;
-
-obj = qobject_from_jsonv(fmt, va);
-if (!obj) {
-fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QDICT) {
-fprintf(stderr, "error is not a dict '%s'\n", fmt);
-abort();
-}
-
-ret = qobject_to_qdict(obj);
-obj = qdict_get(ret, "class");
-if (!obj) {
-fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QSTRING) {
-fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
-abort();
-}
-
-obj = qdict_get(ret, "data");
-if (!obj) {
-fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QDICT) {
-fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
-abort();
-}
-
-return ret;
-}
-
/**
* qerror_from_info(): Create a new QError from error information
*
@@ -394,9 +355,8 @@ static QError *qerror_from_info(ErrorClass err_class, const
char *fmt,
qerr = qerror_new();
loc_save(&qerr->loc);
+qerr->err_msg = g_strdup_vprintf(fmt, *va);
qerr->err_class = err_class;
-qerr->error = error_obj_from_fmt_no_fail(fmt, va);
-qerr->err_msg = qerror_format(fmt, qerr->error);
return qerr;
}
diff --git a/qerror.h b/qerror.h
index 4f92218..057a8f2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -45,214 +45,213 @@ char *qerror_format(const char *fmt, QDict *error);
* Use scripts/check-qerror.sh to check.
*/
#define QERR_ADD_CLIENT_FAILED \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "Could not add client"
#define QERR_AMBIGUOUS_PATH \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path':
%s } }"
+ERROR_CLASS_GENERIC_ERROR, "Path '%s' does not uniquely identify an object"
#define QERR_BAD_BUS_FOR_DEVICE \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': {
'device': %s, 'bad_bus_type': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "Device '%s' can't go on a %s bus"
#define QERR_BASE_NOT_FOUND \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': { 'base':
%s } }"
+ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BlockFormatFeatureNotSupported',
'data': { 'format': %s, 'name': %s, 'feature': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not
support feature '%s'"
#define QERR_BUFFER_OVERRUN \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BufferOverrun', 'data': {} }"
+ERROR_CLASS_GE
[Qemu-devel] [PATCH 30/35] qemu-ga: switch to the new error format on the wire
IMPORTANT: this BREAKS qemu-ga compatibility for the error response.
Instead of returning something like:
{ "error": { "class": "InvalidParameterValue",
"data": {"name": "mode", "expected": "halt|powerdown|reboot" } } }
qemu-ga now returns:
{ "error": { "class": "GenericError",
"desc": "Parameter 'mode' expects halt|powerdown|reboot" } }
Notice that this is also a bug fix, as qemu-ga wasn't returning the
human message.
Signed-off-by: Luiz Capitulino
---
Makefile.objs | 1 +
qapi/qmp-core.h | 1 +
qapi/qmp-dispatch.c | 10 +-
qemu-ga.c | 4 ++--
4 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/Makefile.objs b/Makefile.objs
index 5ebbcfa..8454b53 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -211,6 +211,7 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y)
# qapi
qapi-obj-y = qapi/
+qapi-obj-y += qapi-types.o qapi-visit.o
common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o
common-obj-y += qmp.o hmp.o
diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h
index b0f64ba..00446cf 100644
--- a/qapi/qmp-core.h
+++ b/qapi/qmp-core.h
@@ -49,6 +49,7 @@ void qmp_disable_command(const char *name);
void qmp_enable_command(const char *name);
bool qmp_command_is_enabled(const char *name);
char **qmp_get_command_list(void);
+QObject *qmp_build_error_object(Error *errp);
#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 122c1a2..ec613f8 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -14,6 +14,7 @@
#include "qemu-objects.h"
#include "qapi/qmp-core.h"
#include "json-parser.h"
+#include "qapi-types.h"
#include "error.h"
#include "error_int.h"
#include "qerror.h"
@@ -109,6 +110,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error
**errp)
return ret;
}
+QObject *qmp_build_error_object(Error *errp)
+{
+return qobject_from_jsonf("{ 'class': %s, 'desc': %s }",
+ ErrorClass_lookup[error_get_class(errp)],
+ error_get_pretty(errp));
+}
+
QObject *qmp_dispatch(QObject *request)
{
Error *err = NULL;
@@ -119,7 +127,7 @@ QObject *qmp_dispatch(QObject *request)
rsp = qdict_new();
if (err) {
-qdict_put_obj(rsp, "error", error_get_qobject(err));
+qdict_put_obj(rsp, "error", qmp_build_error_object(err));
error_free(err);
} else if (ret) {
qdict_put_obj(rsp, "return", ret);
diff --git a/qemu-ga.c b/qemu-ga.c
index f1a39ec..39abc50 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -515,7 +515,7 @@ static void process_event(JSONMessageParser *parser, QList
*tokens)
} else {
g_warning("failed to parse event: %s", error_get_pretty(err));
}
-qdict_put_obj(qdict, "error", error_get_qobject(err));
+qdict_put_obj(qdict, "error", qmp_build_error_object(err));
error_free(err);
} else {
qdict = qobject_to_qdict(obj);
@@ -532,7 +532,7 @@ static void process_event(JSONMessageParser *parser, QList
*tokens)
qdict = qdict_new();
g_warning("unrecognized payload format");
error_set(&err, QERR_UNSUPPORTED);
-qdict_put_obj(qdict, "error", error_get_qobject(err));
+qdict_put_obj(qdict, "error", qmp_build_error_object(err));
error_free(err);
}
ret = send_response(s, QOBJECT(qdict));
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 33/35] qerror: drop qerror_table and qerror_format()
They are unused since last commit.
Signed-off-by: Luiz Capitulino
---
qerror.c | 400 ---
qerror.h | 7 --
2 files changed, 407 deletions(-)
diff --git a/qerror.c b/qerror.c
index dda1427..ccc52be 100644
--- a/qerror.c
+++ b/qerror.c
@@ -23,311 +23,6 @@ static const QType qerror_type = {
};
/**
- * The 'desc' parameter is a printf-like string, the format of the format
- * string is:
- *
- * %(KEY)
- *
- * Where KEY is a QDict key, which has to be passed to qerror_from_info().
- *
- * Example:
- *
- * "foo error on device: %(device) slot: %(slot_nr)"
- *
- * A single percent sign can be printed if followed by a second one,
- * for example:
- *
- * "running out of foo: %(foo)%%"
- *
- * Please keep the entries in alphabetical order.
- * Use scripts/check-qerror.sh to check.
- */
-static const QErrorStringTable qerror_table[] = {
-{
- QERR_ADD_CLIENT_FAILED,
- "Could not add client",
-},
-{
- QERR_AMBIGUOUS_PATH,
- "Path '%(path)' does not uniquely identify an object"
-},
-{
- QERR_BAD_BUS_FOR_DEVICE,
- "Device '%(device)' can't go on a %(bad_bus_type) bus",
-},
-{
- QERR_BASE_NOT_FOUND,
- "Base '%(base)' not found",
-},
-{
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "Block format '%(format)' used by device '%(name)' does not support
feature '%(feature)'",
-},
-{
- QERR_BUS_NO_HOTPLUG,
- "Bus '%(bus)' does not support hotplugging",
-},
-{
- QERR_BUS_NOT_FOUND,
- "Bus '%(bus)' not found",
-},
-{
- QERR_COMMAND_DISABLED,
- "The command %(name) has been disabled for this instance",
-},
-{
- QERR_COMMAND_NOT_FOUND,
- "The command %(name) has not been found",
-},
-{
- QERR_DEVICE_ENCRYPTED,
- "'%(device)' (%(filename)) is encrypted",
-},
-{
- QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
- "Migration is disabled when using feature '%(feature)' in device
'%(device)'",
-},
-{
- QERR_DEVICE_HAS_NO_MEDIUM,
- "Device '%(device)' has no medium",
-},
-{
- QERR_DEVICE_INIT_FAILED,
- "Device '%(device)' could not be initialized",
-},
-{
- QERR_DEVICE_IN_USE,
- "Device '%(device)' is in use",
-},
-{
- QERR_DEVICE_IS_READ_ONLY,
- "Device '%(device)' is read only",
-},
-{
- QERR_DEVICE_LOCKED,
- "Device '%(device)' is locked",
-},
-{
- QERR_DEVICE_MULTIPLE_BUSSES,
- "Device '%(device)' has multiple child busses",
-},
-{
- QERR_DEVICE_NO_BUS,
- "Device '%(device)' has no child bus",
-},
-{
- QERR_DEVICE_NO_HOTPLUG,
- "Device '%(device)' does not support hotplugging",
-},
-{
- QERR_DEVICE_NOT_ACTIVE,
- "Device '%(device)' has not been activated",
-},
-{
- QERR_DEVICE_NOT_ENCRYPTED,
- "Device '%(device)' is not encrypted",
-},
-{
- QERR_DEVICE_NOT_FOUND,
- "Device '%(device)' not found",
-},
-{
- QERR_DEVICE_NOT_REMOVABLE,
- "Device '%(device)' is not removable",
-},
-{
- QERR_DUPLICATE_ID,
- "Duplicate ID '%(id)' for %(object)",
-},
-{
- QERR_FD_NOT_FOUND,
- "File descriptor named '%(name)' not found",
-},
-{
- QERR_FD_NOT_SUPPLIED,
- "No file descriptor supplied via SCM_RIGHTS",
-},
-{
- QERR_FEATURE_DISABLED,
- "The feature '%(name)' is not enabled",
-},
-{
- QERR_INVALID_BLOCK_FORMAT,
- "Invalid block format '%(name)'",
-},
-{
- QERR_INVALID_OPTION_GROUP,
- "There is no option group '%(group)'",
-},
-{
- QERR_INVALID_PARAMETER,
- "Invalid parameter '%(name)'",
-},
-{
- QERR_INVALID_PARAMETER_COMBINATION,
- "Invalid parameter combination",
-},
-{
- QERR_INVALID_PARAMETER_TYPE,
- "Invalid parameter type for '%(name)', expected: %(expected)",
-},
-{
- QERR_INVALID_PARAMETER_VALUE,
- "Parameter '%(name)' expects %(expected)",
-},
-{
- QERR_INVALID_PASSWORD,
- "Password incorrect",
-},
-{
- QERR_IO_ERROR,
- "An IO error has occurred",
-},
-{
- QERR_JSON_PARSE_ERROR,
- "JSON parse error, %(message)",
-
-},
-{
- QERR_JSON_PARSING,
- "Invalid JSON syntax",
-},
-{
- QERR_KVM_MISSING_CAP,
- "Using KVM without %(capability), %(feature) unavailable",
-},
-{
- QERR_MIGRATION_ACTIVE,
- "There's a migration process in progress",
-},
-{
- QERR_MIGRATION_NOT_SUPPORTED,
- "State blocked by non-migratable device '%(device)'",
-},
-{
-
[Qemu-devel] [PATCH 08/35] qerror: qerror_format(): return an allocated string
Simplifies current and future users.
Signed-off-by: Luiz Capitulino
---
error.c | 5 +
qerror.c | 10 --
qerror.h | 2 +-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/error.c b/error.c
index 58f55a0..3a62592 100644
--- a/error.c
+++ b/error.c
@@ -65,10 +65,7 @@ bool error_is_set(Error **errp)
const char *error_get_pretty(Error *err)
{
if (err->msg == NULL) {
-QString *str;
-str = qerror_format(err->fmt, err->obj);
-err->msg = g_strdup(qstring_get_str(str));
-QDECREF(str);
+err->msg = qerror_format(err->fmt, err->obj);
}
return err->msg;
diff --git a/qerror.c b/qerror.c
index 6f9f49c..d073ed7 100644
--- a/qerror.c
+++ b/qerror.c
@@ -493,9 +493,11 @@ static QString *qerror_format_desc(QDict *error,
return qstring;
}
-QString *qerror_format(const char *fmt, QDict *error)
+char *qerror_format(const char *fmt, QDict *error)
{
const QErrorStringTable *entry = NULL;
+QString *qstr;
+char *ret;
int i;
for (i = 0; qerror_table[i].error_fmt; i++) {
@@ -505,7 +507,11 @@ QString *qerror_format(const char *fmt, QDict *error)
}
}
-return qerror_format_desc(error, entry);
+qstr = qerror_format_desc(error, entry);
+ret = g_strdup(qstring_get_str(qstr));
+QDECREF(qstr);
+
+return ret;
}
/**
diff --git a/qerror.h b/qerror.h
index 3c0b14c..aec76b2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -34,7 +34,7 @@ QString *qerror_human(const QError *qerror);
void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
void qerror_report_err(Error *err);
void assert_no_error(Error *err);
-QString *qerror_format(const char *fmt, QDict *error);
+char *qerror_format(const char *fmt, QDict *error);
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 18/35] hmp: hmp.h: include qdict.h
hmp.h is relying on qdict.h being provided by qapi-types.h. Fix this, as a future commit will change qapi-types.h not to provide qdict.h. Signed-off-by: Luiz Capitulino --- hmp.h | 1 + 1 file changed, 1 insertion(+) diff --git a/hmp.h b/hmp.h index 8d2b0d7..3275522 100644 --- a/hmp.h +++ b/hmp.h @@ -16,6 +16,7 @@ #include "qemu-common.h" #include "qapi-types.h" +#include "qdict.h" void hmp_info_name(Monitor *mon); void hmp_info_version(Monitor *mon); -- 1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 21/35] qapi: don't convert enum strings to lowercase
Next commit will introduce enum strings in camel case.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9b7da96..cf601ae 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -70,7 +70,7 @@ const char *%(name)s_lookup[] = {
ret += mcgen('''
"%(value)s",
''',
- value=value.lower())
+ value=value)
ret += mcgen('''
NULL,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 22/35] qapi-schema: add ErrorClass enum
Signed-off-by: Luiz Capitulino
---
qapi-schema.json | 30 ++
1 file changed, 30 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index a62bf68..b513935 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3,6 +3,36 @@
# QAPI Schema
##
+# @ErrorClass
+#
+# QEMU error classes
+#
+# @GenericError: this is used for errors that don't require a specific error
+#class. This should be the default case for most errors
+#
+# @CommandNotFound: the requested command has not been found
+#
+# @DeviceEncrypted: the requested operation can't be fulfilled because the
+# selected device is encrypted
+#
+# @DeviceNotActive: a device has failed to be become active
+#
+# @DeviceNotFound: the requested device has not been found
+#
+# @KVMMissingCap: the requested operation can't be fulfilled because a
+# required KVM capability is missing
+#
+# @MigrationExpected: the requested operation can't be fulfilled because a
+# migration process is expected
+#
+# Since: 1.2
+##
+{ 'enum': 'ErrorClass',
+ 'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
+'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
+'MigrationExpected' ] }
+
+##
# @NameInfo:
#
# Guest name information.
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 27/35] hmp: hmp_change(): use error_get_class()
The error_is_type() function is going to be dropped.
Signed-off-by: Luiz Capitulino
---
hmp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index 54c37d7..9b44dfc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -793,7 +793,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
qmp_change(device, target, !!arg, arg, &err);
-if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
+if (error_is_set(&err) &&
+error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
error_free(err);
monitor_read_block_device_key(mon, device, NULL, NULL);
return;
--
1.7.11.2.249.g31c7954.dirty
Re: [Qemu-devel] Is the return address of get_page_addr_code guest physical address?
On Fri, Aug 10, 2012 at 11:47 AM, Peter Maydell wrote: > On 10 August 2012 03:11, Steven wrote: >> The function definition has a return address type tb_page_addr_t. >> tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr) >> >> I am wondering is this address the guest physical address or the host >> virtual address. > > In linux-user mode the returned address is the guest virtual address. > In system mode it is a ram_addr_t. (the comment above the implementation > says "the returned address is not exactly the physical address: it > is the offset relative to phys_ram_base" but this is out of date I think). > A ram_addr_t is neither a host address nor a guest physical address > but it's closely related to a guest physaddr (you can think of it as > if all the RAM in the system was put into a straight line and then the My question is related to system mode. Is the RAM you mean the guest physical address, which is a continuous sequence of numbers beginning from 0 to the max of allocated RAM of the guest? > ram_addr_t is an index into that). If the returned value of get_page_addr_code is the index to that straight line, I am wondering if it is the guest physical address. For example, a 512MB guest has two RAMBlocks block hva: 139980450037760, offset: 0, length: 536870912 block hva: 139981262475264, offset: 537001984, length: 131072 Then I print out the value of pc and its phys_pc returned value from get_page_addr_code in tb_gen_code(...). pc and phys_pc: 0x000f207f 537075839 We can see that the phys_pc is within the second RAMBlock. So if we consider the beginning of the RAM line is 0, the pays_pc value is the offset to 0. In other word the phys_pc is exactly the guest physical address. Is this correct? - Steven > >> If it it is the guest physical address, why does Qemu waste guest >> physical space to store these address for tb? Thanks. > > I'm not sure what you're asking here. This function returns a > physical address because we store TCG translated code blocks in > a hash table indexed by guest physaddr. Given the information > "the CPU is trying to execute code from this physaddr" we need to > be able to find out whether we already have a code block translated > for that. (there is also a fast code path so we can avoid doing > a complete lookup from physaddr most of the time.) > > -- PMM
[Qemu-devel] [PATCH 25/35] qerror: add proper ErrorClass value for QERR_ macros
This commit replaces the place holder value for the ErrorClass
argument with a proper ErrorClass value for all QERR_ macros.
All current errors are mapped to GenericError, except for errors
CommandNotFound, DeviceEncrypted, DeviceNotActive, DeviceNotFound,
KVMMissingCap and MigrationExpected, which are maintained as they
are today.
Signed-off-by: Luiz Capitulino
---
qerror.h | 140 +++
1 file changed, 70 insertions(+), 70 deletions(-)
diff --git a/qerror.h b/qerror.h
index bcc93f8..4f92218 100644
--- a/qerror.h
+++ b/qerror.h
@@ -45,214 +45,214 @@ char *qerror_format(const char *fmt, QDict *error);
* Use scripts/check-qerror.sh to check.
*/
#define QERR_ADD_CLIENT_FAILED \
--1, "{ 'class': 'AddClientFailed', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
#define QERR_AMBIGUOUS_PATH \
--1, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path':
%s } }"
#define QERR_BAD_BUS_FOR_DEVICE \
--1, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type':
%s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': {
'device': %s, 'bad_bus_type': %s } }"
#define QERR_BASE_NOT_FOUND \
--1, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': { 'base':
%s } }"
#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
--1, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s,
'name': %s, 'feature': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BlockFormatFeatureNotSupported',
'data': { 'format': %s, 'name': %s, 'feature': %s } }"
#define QERR_BUFFER_OVERRUN \
--1, "{ 'class': 'BufferOverrun', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BufferOverrun', 'data': {} }"
#define QERR_BUS_NO_HOTPLUG \
--1, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s
} }"
#define QERR_BUS_NOT_FOUND \
--1, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNotFound', 'data': { 'bus': %s
} }"
#define QERR_COMMAND_DISABLED \
--1, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'CommandDisabled', 'data': {
'name': %s } }"
#define QERR_COMMAND_NOT_FOUND \
--1, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
+ERROR_CLASS_COMMAND_NOT_FOUND, "{ 'class': 'CommandNotFound', 'data': {
'name': %s } }"
#define QERR_DEVICE_ENCRYPTED \
--1, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s
} }"
+ERROR_CLASS_DEVICE_ENCRYPTED, "{ 'class': 'DeviceEncrypted', 'data': {
'device': %s, 'filename': %s } }"
#define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
--1, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s,
'feature': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceFeatureBlocksMigration',
'data': { 'device': %s, 'feature': %s } }"
#define QERR_DEVICE_HAS_NO_MEDIUM \
--1, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceHasNoMedium', 'data': {
'device': %s } }"
#define QERR_DEVICE_INIT_FAILED \
--1, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInitFailed', 'data': {
'device': %s } }"
#define QERR_DEVICE_IN_USE \
--1, "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInUse', 'data': { 'device':
%s } }"
#define QERR_DEVICE_IS_READ_ONLY \
--1, "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceIsReadOnly', 'data': {
'device': %s } }"
#define QERR_DEVICE_LOCKED \
--1, "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceLocked', 'data': { 'device':
%s } }"
#define QERR_DEVICE_MULTIPLE_BUSSES \
--1, "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceMultipleBusses', 'data': {
'device': %s } }"
#define QERR_DEVICE_NO_BUS \
--1, "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoBus', 'data': { 'device':
%s } }"
#define QERR_DEVICE_NO_HOTPLUG \
--1, "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoHotplug', 'data': {
'device': %s } }"
#define QERR_DEVICE_NOT_ACTIVE \
--1, "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
+ERROR_CLASS_DEVICE_NOT_ACTIVE, "{ 'class': 'DeviceNotActive', 'data': {
'device': %s } }"
#define QERR_DEVICE_NOT_ENCRYPTED \
--
[Qemu-devel] [PATCH 03/35] qerror: QERR_DEVICE_ENCRYPTED: change error message
Match what HMP commands print on DeviceEncrypted errors.
Signed-off-by: Luiz Capitulino
---
qerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qerror.c b/qerror.c
index 082de98..de0a79e 100644
--- a/qerror.c
+++ b/qerror.c
@@ -81,7 +81,7 @@ static const QErrorStringTable qerror_table[] = {
},
{
.error_fmt = QERR_DEVICE_ENCRYPTED,
-.desc = "Device '%(device)' is encrypted",
+.desc = "'%(device)' (%(filename)) is encrypted",
},
{
.error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 29/35] qmp: switch to the new error format on the wire
IMPORTANT: this BREAKS QMP's compatibility for the error response.
This commit changes QMP's wire protocol to make use of the simpler
error format introduced by previous commits.
There are two important (and mostly incompatible) changes:
1. Almost all error classes have been replaced by GenericError. The
only classes that are still supported for compatibility with
libvirt are: CommandNotFound, DeviceNotActive, KVMMissingCap,
DeviceNotFound and MigrationExpected
2. The 'data' field of the error dictionary is gone
As an example, an error response like:
{ "error": { "class": "DeviceNotRemovable",
"data": { "device": "virtio0" },
"desc": "Device 'virtio0' is not removable" } }
Will now be emitted as:
{ "error": { "class": "GenericError",
"desc": "Device 'virtio0' is not removable" } }
Signed-off-by: Luiz Capitulino
---
QMP/qmp-spec.txt | 10 +++---
monitor.c| 18 +-
qapi-schema.json | 55 ---
qmp-commands.hx | 4 ++--
4 files changed, 18 insertions(+), 69 deletions(-)
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 1ba916c..a277896 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -106,14 +106,11 @@ completed because of an error condition.
The format is:
-{ "error": { "class": json-string, "data": json-object, "desc": json-string },
- "id": json-value }
+{ "error": { "class": json-string, "desc": json-string }, "id": json-value }
Where,
-- The "class" member contains the error class name (eg. "ServiceUnavailable")
-- The "data" member contains specific error data and is defined in a
- per-command basis, it will be an empty json-object if the error has no data
+- The "class" member contains the error class name (eg. "GenericError")
- The "desc" member is a human-readable error message. Clients should
not attempt to parse this message.
- The "id" member contains the transaction identification associated with
@@ -173,8 +170,7 @@ S: {"return": {"enabled": true, "present": true}, "id":
"example"}
--
C: { "execute": }
-S: {"error": {"class": "JSONParsing", "desc": "Invalid JSON syntax", "data":
-{}}}
+S: {"error": {"class": "GenericError", "desc": "Invalid JSON syntax" } }
3.5 Powerdown event
---
diff --git a/monitor.c b/monitor.c
index aa57167..3694590 100644
--- a/monitor.c
+++ b/monitor.c
@@ -353,16 +353,26 @@ static void monitor_json_emitter(Monitor *mon, const
QObject *data)
QDECREF(json);
}
+static QDict *build_qmp_error_dict(const QError *err)
+{
+QObject *obj;
+
+obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %p } }",
+ ErrorClass_lookup[err->err_class],
+ qerror_human(err));
+
+return qobject_to_qdict(obj);
+}
+
static void monitor_protocol_emitter(Monitor *mon, QObject *data)
{
QDict *qmp;
trace_monitor_protocol_emitter(mon);
-qmp = qdict_new();
-
if (!monitor_has_error(mon)) {
/* success response */
+qmp = qdict_new();
if (data) {
qobject_incref(data);
qdict_put_obj(qmp, "return", data);
@@ -372,9 +382,7 @@ static void monitor_protocol_emitter(Monitor *mon, QObject
*data)
}
} else {
/* error response */
-qdict_put(mon->error->error, "desc", qerror_human(mon->error));
-qdict_put(qmp, "error", mon->error->error);
-QINCREF(mon->error->error);
+qmp = build_qmp_error_dict(mon->error);
QDECREF(mon->error);
mon->error = NULL;
}
diff --git a/qapi-schema.json b/qapi-schema.json
index b513935..ec8d919 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -658,7 +658,6 @@
# Returns information about the current VNC server
#
# Returns: @VncInfo
-# If VNC support is not compiled in, FeatureDisabled
#
# Since: 0.14.0
##
@@ -1042,9 +1041,6 @@
# virtual address (defaults to CPU 0)
#
# Returns: Nothing on success
-# If @cpu is not a valid VCPU, InvalidParameterValue
-# If @filename cannot be opened, OpenFileFailed
-# If an I/O error occurs while writing the file, IOError
#
# Since: 0.14.0
#
@@ -1065,8 +1061,6 @@
# @filename: the file to save the memory to as binary data
#
# Returns: Nothing on success
-# If @filename cannot be opened, OpenFileFailed
-# If an I/O error occurs while writing the file, IOError
#
# Since: 0.14.0
#
@@ -1108,7 +1102,6 @@
# Injects an Non-Maskable Interrupt into all guest's VCPUs.
#
# Returns: If successful, nothing
-# If the Virtual Machine doesn't support NMI injection, Unsupported
#
# Since: 0.14.0
#
@@ -1159,7 +1152,6 @@
# Returns: nothing on success
# If @device is not a valid block device, DeviceNotFound
# If @device is not encrypted, DeviceNotEncrypted
-# If @password is not valid f
[Qemu-devel] [PATCH 34/35] error, qerror: drop QDict member
Used to store error information, but it's unused now.
Signed-off-by: Luiz Capitulino
---
error.c | 4
qerror.c | 4
qerror.h | 1 -
3 files changed, 9 deletions(-)
diff --git a/error.c b/error.c
index 0e10373..1f05fc4 100644
--- a/error.c
+++ b/error.c
@@ -19,7 +19,6 @@
struct Error
{
-QDict *obj;
char *msg;
ErrorClass err_class;
};
@@ -51,8 +50,6 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
err_new->err_class = err->err_class;
-err_new->obj = err->obj;
-QINCREF(err_new->obj);
return err_new;
}
@@ -75,7 +72,6 @@ const char *error_get_pretty(Error *err)
void error_free(Error *err)
{
if (err) {
-QDECREF(err->obj);
g_free(err->msg);
g_free(err);
}
diff --git a/qerror.c b/qerror.c
index ccc52be..0818504 100644
--- a/qerror.c
+++ b/qerror.c
@@ -100,7 +100,6 @@ void qerror_report(ErrorClass eclass, const char *fmt, ...)
/* Evil... */
struct Error
{
-QDict *obj;
char *msg;
ErrorClass err_class;
};
@@ -111,8 +110,6 @@ void qerror_report_err(Error *err)
qerr = qerror_new();
loc_save(&qerr->loc);
-QINCREF(err->obj);
-qerr->error = err->obj;
qerr->err_msg = g_strdup(err->msg);
qerr->err_class = err->err_class;
@@ -154,7 +151,6 @@ static void qerror_destroy_obj(QObject *obj)
assert(obj != NULL);
qerr = qobject_to_qerror(obj);
-QDECREF(qerr->error);
g_free(qerr->err_msg);
g_free(qerr);
}
diff --git a/qerror.h b/qerror.h
index c5ad29f..d0a76a4 100644
--- a/qerror.h
+++ b/qerror.h
@@ -21,7 +21,6 @@
typedef struct QError {
QObject_HEAD;
-QDict *error;
Location loc;
char *err_msg;
ErrorClass err_class;
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 05/35] qerror: drop qerror_abort()
qerror_abort() depends on the 'file', 'func' and 'linenr' members of
QError. However, these members are going to be dropped by the next
commit, so let's drop qerror_abort() in favor of printing an error
message to stderr plus a call to abort().
Signed-off-by: Luiz Capitulino
---
qerror.c | 41 ++---
1 file changed, 14 insertions(+), 27 deletions(-)
diff --git a/qerror.c b/qerror.c
index bfb875a..7cb7c12 100644
--- a/qerror.c
+++ b/qerror.c
@@ -346,22 +346,6 @@ static QError *qerror_new(void)
return qerr;
}
-static void GCC_FMT_ATTR(2, 3) qerror_abort(const QError *qerr,
-const char *fmt, ...)
-{
-va_list ap;
-
-fprintf(stderr, "qerror: bad call in function '%s':\n", qerr->func);
-fprintf(stderr, "qerror: -> ");
-
-va_start(ap, fmt);
-vfprintf(stderr, fmt, ap);
-va_end(ap);
-
-fprintf(stderr, "\nqerror: call at %s:%d\n", qerr->file, qerr->linenr);
-abort();
-}
-
static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
const char *fmt, va_list *va)
{
@@ -369,28 +353,34 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError
*qerr,
obj = qobject_from_jsonv(fmt, va);
if (!obj) {
-qerror_abort(qerr, "invalid format '%s'", fmt);
+fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_abort(qerr, "error format is not a QDict '%s'", fmt);
+fprintf(stderr, "error is not a dict '%s'\n", fmt);
+abort();
}
qerr->error = qobject_to_qdict(obj);
obj = qdict_get(qerr->error, "class");
if (!obj) {
-qerror_abort(qerr, "missing 'class' key in '%s'", fmt);
+fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QSTRING) {
-qerror_abort(qerr, "'class' key value should be a QString");
+fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
+abort();
}
obj = qdict_get(qerr->error, "data");
if (!obj) {
-qerror_abort(qerr, "missing 'data' key in '%s'", fmt);
+fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_abort(qerr, "'data' key value should be a QDICT");
+fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
+abort();
}
}
@@ -407,7 +397,8 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
}
}
-qerror_abort(qerr, "error format '%s' not found", fmt);
+fprintf(stderr, "error format '%s' not found\n", fmt);
+abort();
}
/**
@@ -435,10 +426,6 @@ static QError *qerror_from_info(const char *file, int
linenr, const char *func,
qerr->file = file;
qerr->func = func;
-if (!fmt) {
-qerror_abort(qerr, "QDict not specified");
-}
-
qerror_set_data(qerr, fmt, va);
qerror_set_desc(qerr, fmt);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 04/35] qerror: reduce public exposure
qerror will be dropped in a near future, let's reduce its public
exposure by making functions only used in qerror.c static.
Signed-off-by: Luiz Capitulino
---
qerror.c | 10 +-
qerror.h | 5 -
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/qerror.c b/qerror.c
index de0a79e..bfb875a 100644
--- a/qerror.c
+++ b/qerror.c
@@ -336,7 +336,7 @@ static const QErrorStringTable qerror_table[] = {
*
* Return strong reference.
*/
-QError *qerror_new(void)
+static QError *qerror_new(void)
{
QError *qerr;
@@ -424,8 +424,8 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
*
* Return strong reference.
*/
-QError *qerror_from_info(const char *file, int linenr, const char *func,
- const char *fmt, va_list *va)
+static QError *qerror_from_info(const char *file, int linenr, const char *func,
+const char *fmt, va_list *va)
{
QError *qerr;
@@ -549,7 +549,7 @@ QString *qerror_human(const QError *qerror)
* it uses error_report() for this, so that the output is routed to the right
* place (ie. stderr or Monitor's device).
*/
-void qerror_print(QError *qerror)
+static void qerror_print(QError *qerror)
{
QString *qstring = qerror_human(qerror);
loc_push_restore(&qerror->loc);
@@ -620,7 +620,7 @@ void assert_no_error(Error *err)
/**
* qobject_to_qerror(): Convert a QObject into a QError
*/
-QError *qobject_to_qerror(const QObject *obj)
+static QError *qobject_to_qerror(const QObject *obj)
{
if (qobject_type(obj) != QTYPE_QERROR) {
return NULL;
diff --git a/qerror.h b/qerror.h
index b4c8758..fe8870c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -33,11 +33,7 @@ typedef struct QError {
const QErrorStringTable *entry;
} QError;
-QError *qerror_new(void);
-QError *qerror_from_info(const char *file, int linenr, const char *func,
- const char *fmt, va_list *va) GCC_FMT_ATTR(4, 0);
QString *qerror_human(const QError *qerror);
-void qerror_print(QError *qerror);
void qerror_report_internal(const char *file, int linenr, const char *func,
const char *fmt, ...) GCC_FMT_ATTR(4, 5);
void qerror_report_err(Error *err);
@@ -45,7 +41,6 @@ void assert_no_error(Error *err);
QString *qerror_format(const char *fmt, QDict *error);
#define qerror_report(fmt, ...) \
qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
-QError *qobject_to_qerror(const QObject *obj);
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 15/35] migration: don't rely on any QERR_SOCKET_*
Use the in_progress argument for QERR_SOCKET_CONNECT_IN_PROGRESS. The
other errors are handled the same by checking if the error is set and
then calling migrate_fd_error() if it's.
It's also necessary to change inet_connect_opts() not to set
QERR_SOCKET_CONNECT_IN_PROGRESS. This error is only used by
tcp_start_outgoing_migration() and not changing it along with the
usage of in_progress would break migration.
Furthermore this commit fixes a bug. Today, there's a spurious error
report when migration succeeds:
(qemu) migrate tcp:0:
migrate: Connection can not be completed immediately
(qemu)
After this commit no spurious error is reported anymore.
Signed-off-by: Luiz Capitulino
---
migration-tcp.c | 22 +-
qemu-sockets.c | 2 --
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 18944a4..ac891c3 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -82,27 +82,23 @@ static void tcp_wait_for_connect(void *opaque)
int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
Error **errp)
{
+bool in_progress;
+
s->get_error = socket_errno;
s->write = socket_write;
s->close = tcp_close;
-s->fd = inet_connect(host_port, false, NULL, errp);
+s->fd = inet_connect(host_port, false, &in_progress, errp);
+if (error_is_set(errp)) {
+migrate_fd_error(s);
+return -1;
+}
-if (!error_is_set(errp)) {
-migrate_fd_connect(s);
-} else if (error_is_type(*errp, QERR_SOCKET_CONNECT_IN_PROGRESS)) {
+if (in_progress) {
DPRINTF("connect in progress\n");
qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
-} else if (error_is_type(*errp, QERR_SOCKET_CREATE_FAILED)) {
-DPRINTF("connect failed\n");
-return -1;
-} else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
-DPRINTF("connect failed\n");
-migrate_fd_error(s);
-return -1;
} else {
-DPRINTF("unknown error\n");
-return -1;
+migrate_fd_connect(s);
}
return 0;
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 9cb47d4..361d890 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -284,8 +284,6 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress,
Error **errp)
if (in_progress) {
*in_progress = true;
}
-
-error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
} else if (rc < 0) {
if (NULL == e->ai_next)
fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 19/35] qapi: qapi-types.h: don't include qapi/qapi-types-core.h
qapi-types.h needs only qemu-common.h. Including qapi-types-core.h
causes problems when qerror.h or error.h includes qapi-types.h.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4a734f5..3ed9f04 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -253,7 +253,8 @@ fdecl.write(mcgen('''
#ifndef %(guard)s
#define %(guard)s
-#include "qapi/qapi-types-core.h"
+#include "qemu-common.h"
+
''',
guard=guardname(h_file)))
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 06/35] qerror: avoid passing qerr pointer
Helps dropping/modifying qerror functions.
Signed-off-by: Luiz Capitulino
---
qerror.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/qerror.c b/qerror.c
index 7cb7c12..e717496 100644
--- a/qerror.c
+++ b/qerror.c
@@ -346,10 +346,10 @@ static QError *qerror_new(void)
return qerr;
}
-static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
- const char *fmt, va_list *va)
+static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
{
QObject *obj;
+QDict *ret;
obj = qobject_from_jsonv(fmt, va);
if (!obj) {
@@ -361,9 +361,8 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
abort();
}
-qerr->error = qobject_to_qdict(obj);
-
-obj = qdict_get(qerr->error, "class");
+ret = qobject_to_qdict(obj);
+obj = qdict_get(ret, "class");
if (!obj) {
fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
abort();
@@ -372,8 +371,8 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
abort();
}
-
-obj = qdict_get(qerr->error, "data");
+
+obj = qdict_get(ret, "data");
if (!obj) {
fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
abort();
@@ -382,9 +381,11 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError
*qerr,
fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
abort();
}
+
+return ret;
}
-static void qerror_set_desc(QError *qerr, const char *fmt)
+static const QErrorStringTable *get_desc_no_fail(const char *fmt)
{
int i;
@@ -392,8 +393,7 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
for (i = 0; qerror_table[i].error_fmt; i++) {
if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-qerr->entry = &qerror_table[i];
-return;
+return &qerror_table[i];
}
}
@@ -426,8 +426,8 @@ static QError *qerror_from_info(const char *file, int
linenr, const char *func,
qerr->file = file;
qerr->func = func;
-qerror_set_data(qerr, fmt, va);
-qerror_set_desc(qerr, fmt);
+qerr->error = error_obj_from_fmt_no_fail(fmt, va);
+qerr->entry = get_desc_no_fail(fmt);
return qerr;
}
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 20/35] qapi: generate correct enum names for camel case enums
An enum like GenericError in the schema, should generate
GENERIC_ERROR and not GENERICERROR.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 3ed9f04..9b7da96 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -79,6 +79,16 @@ const char *%(name)s_lookup[] = {
''')
return ret
+def generate_enum_name(name):
+if name.isupper():
+return c_fun(name)
+new_name = ''
+for c in c_fun(name):
+if c.isupper():
+new_name += '_'
+new_name += c
+return new_name.lstrip('_').upper()
+
def generate_enum(name, values):
lookup_decl = mcgen('''
extern const char *%(name)s_lookup[];
@@ -100,7 +110,7 @@ typedef enum %(name)s
%(abbrev)s_%(value)s = %(i)d,
''',
abbrev=de_camel_case(name).upper(),
- value=c_fun(value).upper(),
+ value=generate_enum_name(value),
i=i)
i += 1
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 12/35] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED
This commit changes hmp_cont() to loop through all block devices
and proactively set an encryption key for any encrypted device
missing a key.
This change is needed because QERR_DEVICE_ENCRYPTED is going to be
dropped by a future commit.
Signed-off-by: Luiz Capitulino
---
hmp.c | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/hmp.c b/hmp.c
index 25688ab..4efaf51 100644
--- a/hmp.c
+++ b/hmp.c
@@ -612,34 +612,35 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
static void hmp_cont_cb(void *opaque, int err)
{
-Monitor *mon = opaque;
-
if (!err) {
-hmp_cont(mon, NULL);
+qmp_cont(NULL);
}
}
+static bool key_is_missing(const BlockInfo *bdev)
+{
+return (bdev->inserted && bdev->inserted->encryption_key_missing);
+}
+
void hmp_cont(Monitor *mon, const QDict *qdict)
{
+BlockInfoList *bdev_list, *bdev;
Error *errp = NULL;
-qmp_cont(&errp);
-if (error_is_set(&errp)) {
-if (error_is_type(errp, QERR_DEVICE_ENCRYPTED)) {
-const char *device;
-
-/* The device is encrypted. Ask the user for the password
- and retry */
-
-device = error_get_field(errp, "device");
-assert(device != NULL);
-
-monitor_read_block_device_key(mon, device, hmp_cont_cb, mon);
-error_free(errp);
-return;
+bdev_list = qmp_query_block(NULL);
+for (bdev = bdev_list; bdev; bdev = bdev->next) {
+if (key_is_missing(bdev->value)) {
+monitor_read_block_device_key(mon, bdev->value->device,
+ hmp_cont_cb, NULL);
+goto out;
}
-hmp_handle_error(mon, &errp);
}
+
+qmp_cont(&errp);
+hmp_handle_error(mon, &errp);
+
+out:
+qapi_free_BlockInfoList(bdev_list);
}
void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
--
1.7.11.2.249.g31c7954.dirty
Re: [Qemu-devel] [PATCH 2/2] cpu: for cpu-user and cpu-softmmu and make cpu-softmmu a child of DeviceState
Eduardo Habkost writes:
> On Fri, Aug 10, 2012 at 12:00:44PM -0500, Anthony Liguori wrote:
>> The line between linux-user and softmmu is not very well defined right now.
>> linux-user really don't want to include devices and making CpuState a child
>> of
>> DeviceState would require pulling lots of stuff into linux-user.
>>
>> To solve this, we simply fork cpu-user and cpu-softmmu letting them evolve
>> independently.
>>
>> We also need to push a qdev_init_nofail() into all callers of cpu_init().
>> This
>> is needed eventually anyway so might as well do this now.
>>
>> Signed-off-by: Anthony Liguori
>> ---
> [...]
>> --- /dev/null
>> +++ b/qom/cpu-user.c
> [...]
>> +static TypeInfo cpu_type_info = {
>> +.name = TYPE_CPU,
>> +#ifdef CONFIG_USER_ONLY
>> +.parent = TYPE_OBJECT,
>> +#else
>> +.parent = TYPE_DEVICE,
>> +#endif
>
> Is this #ifdef supposed to be here?
Nope. Thanks.
Regards,
Anthony Liguori
>
>> +.instance_size = sizeof(CPUState),
>> +.abstract = true,
>> +.class_size = sizeof(CPUClass),
>> +.class_init = cpu_class_init,
>> +};
>> +
>> +static void cpu_register_types(void)
>> +{
>> +type_register_static(&cpu_type_info);
>> +}
>> +
>> +type_init(cpu_register_types)
>> --
>> 1.7.5.4
>>
>
> --
> Eduardo
Re: [Qemu-devel] [PATCH 2/7] qapi: mark QOM commands stable
Eric Blake writes: > On 08/10/2012 10:04 AM, Anthony Liguori wrote: >> We've had a cycle to tweak. It is time to commit to supporting them. >> >> Signed-off-by: Anthony Liguori >> --- >> qapi-schema.json | 19 --- >> 1 files changed, 4 insertions(+), 15 deletions(-) >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 191a889..a938c8d 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -1363,9 +1363,7 @@ >> #4) A link type in the form 'link' where subtype is a qdev >> # device type name. Link properties form the device model graph. >> # >> -# Since: 1.1 >> -# >> -# Notes: This type is experimental. Its syntax may change in future >> releases. >> +# Since: 1.2 > > Per https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01416.html, > this should be 1.2.0 (throughout the series). I'll do a follow up to fix this across the board for the entire file. That's what I took away from your previous comment. Regards, Anthony Liguori > >> @@ -1382,10 +1380,7 @@ >> # Returns: a list of @ObjectPropertyInfo that describe the properties of the >> # object. >> # >> -# Since: 1.1 >> -# >> -# Notes: This command is experimental. It's syntax may change in future > > Yay, getting rid of bad grammar in the process (s/It's/Its/ if the > comment were to remain). > > -- > Eric Blake ebl...@redhat.com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org
[Qemu-devel] [PATCH 23/35] qerror: qerror_table: don't use C99 struct initializers
This allows for changing QERR_ macros to initialize two struct members
at the same time. See next commit for more details.
Signed-off-by: Luiz Capitulino
---
qerror.c | 276 +++
qerror.h | 2 +-
2 files changed, 139 insertions(+), 139 deletions(-)
diff --git a/qerror.c b/qerror.c
index 452ec69..ff460b0 100644
--- a/qerror.c
+++ b/qerror.c
@@ -44,285 +44,285 @@ static const QType qerror_type = {
*/
static const QErrorStringTable qerror_table[] = {
{
-.error_fmt = QERR_ADD_CLIENT_FAILED,
-.desc = "Could not add client",
+ QERR_ADD_CLIENT_FAILED,
+ "Could not add client",
},
{
-.error_fmt = QERR_AMBIGUOUS_PATH,
-.desc = "Path '%(path)' does not uniquely identify an object"
+ QERR_AMBIGUOUS_PATH,
+ "Path '%(path)' does not uniquely identify an object"
},
{
-.error_fmt = QERR_BAD_BUS_FOR_DEVICE,
-.desc = "Device '%(device)' can't go on a %(bad_bus_type) bus",
+ QERR_BAD_BUS_FOR_DEVICE,
+ "Device '%(device)' can't go on a %(bad_bus_type) bus",
},
{
-.error_fmt = QERR_BASE_NOT_FOUND,
-.desc = "Base '%(base)' not found",
+ QERR_BASE_NOT_FOUND,
+ "Base '%(base)' not found",
},
{
-.error_fmt = QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-.desc = "Block format '%(format)' used by device '%(name)' does
not support feature '%(feature)'",
+ QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ "Block format '%(format)' used by device '%(name)' does not support
feature '%(feature)'",
},
{
-.error_fmt = QERR_BUS_NO_HOTPLUG,
-.desc = "Bus '%(bus)' does not support hotplugging",
+ QERR_BUS_NO_HOTPLUG,
+ "Bus '%(bus)' does not support hotplugging",
},
{
-.error_fmt = QERR_BUS_NOT_FOUND,
-.desc = "Bus '%(bus)' not found",
+ QERR_BUS_NOT_FOUND,
+ "Bus '%(bus)' not found",
},
{
-.error_fmt = QERR_COMMAND_DISABLED,
-.desc = "The command %(name) has been disabled for this instance",
+ QERR_COMMAND_DISABLED,
+ "The command %(name) has been disabled for this instance",
},
{
-.error_fmt = QERR_COMMAND_NOT_FOUND,
-.desc = "The command %(name) has not been found",
+ QERR_COMMAND_NOT_FOUND,
+ "The command %(name) has not been found",
},
{
-.error_fmt = QERR_DEVICE_ENCRYPTED,
-.desc = "'%(device)' (%(filename)) is encrypted",
+ QERR_DEVICE_ENCRYPTED,
+ "'%(device)' (%(filename)) is encrypted",
},
{
-.error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
-.desc = "Migration is disabled when using feature '%(feature)' in
device '%(device)'",
+ QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
+ "Migration is disabled when using feature '%(feature)' in device
'%(device)'",
},
{
-.error_fmt = QERR_DEVICE_HAS_NO_MEDIUM,
-.desc = "Device '%(device)' has no medium",
+ QERR_DEVICE_HAS_NO_MEDIUM,
+ "Device '%(device)' has no medium",
},
{
-.error_fmt = QERR_DEVICE_INIT_FAILED,
-.desc = "Device '%(device)' could not be initialized",
+ QERR_DEVICE_INIT_FAILED,
+ "Device '%(device)' could not be initialized",
},
{
-.error_fmt = QERR_DEVICE_IN_USE,
-.desc = "Device '%(device)' is in use",
+ QERR_DEVICE_IN_USE,
+ "Device '%(device)' is in use",
},
{
-.error_fmt = QERR_DEVICE_IS_READ_ONLY,
-.desc = "Device '%(device)' is read only",
+ QERR_DEVICE_IS_READ_ONLY,
+ "Device '%(device)' is read only",
},
{
-.error_fmt = QERR_DEVICE_LOCKED,
-.desc = "Device '%(device)' is locked",
+ QERR_DEVICE_LOCKED,
+ "Device '%(device)' is locked",
},
{
-.error_fmt = QERR_DEVICE_MULTIPLE_BUSSES,
-.desc = "Device '%(device)' has multiple child busses",
+ QERR_DEVICE_MULTIPLE_BUSSES,
+ "Device '%(device)' has multiple child busses",
},
{
-.error_fmt = QERR_DEVICE_NO_BUS,
-.desc = "Device '%(device)' has no child bus",
+ QERR_DEVICE_NO_BUS,
+ "Device '%(device)' has no child bus",
},
{
-.error_fmt = QERR_DEVICE_NO_HOTPLUG,
-.desc = "Device '%(device)' does not support hotplugging",
+ QERR_DEVICE_NO_HOTPLUG,
+ "Device '%(device)' does not support hotplugging",
},
{
-.error_fmt = QERR_DEVICE_NOT_ACTIVE,
-.desc = "Device '%(device)' has not been activated",
+ QERR_DEVICE_NOT_ACTIVE,
+ "Device '%(device)' has not been activated",
},
{
-.error_fmt = QERR_DEVICE_NOT_ENCRYPTED,
-.desc = "De
[Qemu-devel] [PATCH 09/35] qerror: don't delay error message construction
Today, the error message is only constructed when it's used. This commit
changes qerror to construct the error message when the error object is
built (ie. when the error is reported).
This eliminates the need of storing a pointer to qerror_table[], which
will be dropped soon, and also simplifies the code.
Signed-off-by: Luiz Capitulino
---
qerror.c | 29 -
qerror.h | 2 +-
2 files changed, 5 insertions(+), 26 deletions(-)
diff --git a/qerror.c b/qerror.c
index d073ed7..a254f88 100644
--- a/qerror.c
+++ b/qerror.c
@@ -385,22 +385,6 @@ static QDict *error_obj_from_fmt_no_fail(const char *fmt,
va_list *va)
return ret;
}
-static const QErrorStringTable *get_desc_no_fail(const char *fmt)
-{
-int i;
-
-// FIXME: inefficient loop
-
-for (i = 0; qerror_table[i].error_fmt; i++) {
-if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-return &qerror_table[i];
-}
-}
-
-fprintf(stderr, "error format '%s' not found\n", fmt);
-abort();
-}
-
/**
* qerror_from_info(): Create a new QError from error information
*
@@ -414,7 +398,7 @@ static QError *qerror_from_info(const char *fmt, va_list
*va)
loc_save(&qerr->loc);
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
-qerr->entry = get_desc_no_fail(fmt);
+qerr->err_msg = qerror_format(fmt, qerr->error);
return qerr;
}
@@ -519,7 +503,7 @@ char *qerror_format(const char *fmt, QDict *error)
*/
QString *qerror_human(const QError *qerror)
{
-return qerror_format_desc(qerror->error, qerror->entry);
+return qstring_from_str(qerror->err_msg);
}
/**
@@ -566,19 +550,13 @@ struct Error
void qerror_report_err(Error *err)
{
QError *qerr;
-int i;
qerr = qerror_new();
loc_save(&qerr->loc);
QINCREF(err->obj);
qerr->error = err->obj;
-for (i = 0; qerror_table[i].error_fmt; i++) {
-if (strcmp(qerror_table[i].error_fmt, err->fmt) == 0) {
-qerr->entry = &qerror_table[i];
-break;
-}
-}
+qerr->err_msg = qerror_format(err->fmt, qerr->error);
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
@@ -619,5 +597,6 @@ static void qerror_destroy_obj(QObject *obj)
qerr = qobject_to_qerror(obj);
QDECREF(qerr->error);
+g_free(qerr->err_msg);
g_free(qerr);
}
diff --git a/qerror.h b/qerror.h
index aec76b2..de8497d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -27,7 +27,7 @@ typedef struct QError {
QObject_HEAD;
QDict *error;
Location loc;
-const QErrorStringTable *entry;
+char *err_msg;
} QError;
QString *qerror_human(const QError *qerror);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH v3 00/35]: add new error format
v3
o rebase on top of master
o replace 'valid_encryption_key' with 'encryption_key_missing', fixes
a bug found by Markus
o minor changes (changelogs, white-space fix and others)
Only the following patches have changed:
o [PATCH 11/35] qmp: query-block: add 'encryption_key_missing' field
o [PATCH 12/35] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED
o [PATCH 15/35] migration: don't rely on any QERR_SOCKET_*
o [PATCH 29/35] qmp: switch to the new error format on the wire
o [PATCH 35/35] docs: writing-qmp-commands.txt: update error section
This series implements the 'Plan for error handling in QMP' as described
by Anthony in this email:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03764.html
Basically, this replaces almost all error classes by GenericError (the
exception are a few error classes used by libvirt) and drops the error
data memeber. This also adds a free form string to error_set().
On the wire, we go from:
{ "error": { "class": "DeviceNotRemovable",
"data": { "device": "virtio0" },
"desc": "Device 'virtio0' is not removable" } }
to:
{ "error": { "class": "GenericError",
"desc": "Device 'virtio0' is not removable" } }
Internally, we go from:
void error_set(Error **err, const char *fmt, ...);
to:
void error_set(Error **err, ErrorClass err_class, const char *fmt, ...);
Makefile.objs | 1 +
QMP/qmp-spec.txt | 10 +-
block.c | 1 +
block_int.h | 1 +
configure | 10 -
docs/writing-qmp-commands.txt | 47 ++--
error.c | 93 +---
error.h | 34 +--
error_int.h | 29 ---
hmp.c | 69 ++
hmp.h | 1 +
migration-tcp.c | 22 +-
monitor.c | 83 ++-
nbd.c | 2 +-
qapi-schema.json | 94 +++-
qapi/qmp-core.h | 1 +
qapi/qmp-dispatch.c | 11 +-
qemu-char.c | 2 +-
qemu-ga.c | 5 +-
qemu-sockets.c| 14 +-
qemu_socket.h | 4 +-
qerror.c | 516 ++
qerror.h | 168 ++
qmp-commands.hx | 4 +-
scripts/qapi-types.py | 17 +-
ui/vnc.c | 2 +-
26 files changed, 268 insertions(+), 973 deletions(-)
Re: [Qemu-devel] [PATCH 2/7] qapi: mark QOM commands stable
On 08/10/2012 10:04 AM, Anthony Liguori wrote: > We've had a cycle to tweak. It is time to commit to supporting them. > > Signed-off-by: Anthony Liguori > --- > qapi-schema.json | 19 --- > 1 files changed, 4 insertions(+), 15 deletions(-) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 191a889..a938c8d 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1363,9 +1363,7 @@ > #4) A link type in the form 'link' where subtype is a qdev > # device type name. Link properties form the device model graph. > # > -# Since: 1.1 > -# > -# Notes: This type is experimental. Its syntax may change in future > releases. > +# Since: 1.2 Per https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01416.html, this should be 1.2.0 (throughout the series). > @@ -1382,10 +1380,7 @@ > # Returns: a list of @ObjectPropertyInfo that describe the properties of the > # object. > # > -# Since: 1.1 > -# > -# Notes: This command is experimental. It's syntax may change in future Yay, getting rid of bad grammar in the process (s/It's/Its/ if the comment were to remain). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 26/35] error: add error_get_class()
Signed-off-by: Luiz Capitulino
---
error.c | 5 +
error.h | 5 +
2 files changed, 10 insertions(+)
diff --git a/error.c b/error.c
index 648706a..2d34cde 100644
--- a/error.c
+++ b/error.c
@@ -64,6 +64,11 @@ bool error_is_set(Error **errp)
return (errp && *errp);
}
+ErrorClass error_get_class(const Error *err)
+{
+return err->err_class;
+}
+
const char *error_get_pretty(Error *err)
{
return err->msg;
diff --git a/error.h b/error.h
index 9678752..114e24b 100644
--- a/error.h
+++ b/error.h
@@ -35,6 +35,11 @@ void error_set(Error **err, ErrorClass err_class, const char
*fmt, ...) GCC_FMT_
*/
bool error_is_set(Error **err);
+/*
+ * Get the error class of an error object.
+ */
+ErrorClass error_get_class(const Error *err);
+
/**
* Returns an exact copy of the error passed as an argument.
*/
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 24/35] error, qerror: add ErrorClass argument to error functions
The new argument is added to functions qerror_report() and error_set().
It's stored in Error and QError. qerror_report_err() is also updated to
take care of it.
The QERR_ macros are changed to contain a place holder value for the
new argument, so that the value is used on all current calls to
qerror_report() and error_set() (and also to initialize qerror_table[]).
Next commit will update the QERR_ macros with a proper ErrorClass
value.
Signed-off-by: Luiz Capitulino
---
error.c | 8 +++-
error.h | 5 ++-
qerror.c | 10 +++--
qerror.h | 145 ---
4 files changed, 90 insertions(+), 78 deletions(-)
diff --git a/error.c b/error.c
index 2ade99b..648706a 100644
--- a/error.c
+++ b/error.c
@@ -14,6 +14,7 @@
#include "error.h"
#include "qjson.h"
#include "qdict.h"
+#include "qapi-types.h"
#include "error_int.h"
#include "qerror.h"
@@ -21,9 +22,10 @@ struct Error
{
QDict *obj;
char *msg;
+ErrorClass err_class;
};
-void error_set(Error **errp, const char *fmt, ...)
+void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
{
Error *err;
va_list ap;
@@ -39,6 +41,7 @@ void error_set(Error **errp, const char *fmt, ...)
err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
va_end(ap);
err->msg = qerror_format(fmt, err->obj);
+err->err_class = err_class;
*errp = err;
}
@@ -49,6 +52,7 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
+err_new->err_class = err->err_class;
err_new->obj = err->obj;
QINCREF(err_new->obj);
@@ -97,7 +101,7 @@ void error_free(Error *err)
}
}
-bool error_is_type(Error *err, const char *fmt)
+bool error_is_type(Error *err, ErrorClass err_class, const char *fmt)
{
const char *error_class;
char *ptr;
diff --git a/error.h b/error.h
index 3d9d96d..9678752 100644
--- a/error.h
+++ b/error.h
@@ -13,6 +13,7 @@
#define ERROR_H
#include "compiler.h"
+#include "qapi-types.h"
#include
/**
@@ -26,7 +27,7 @@ typedef struct Error Error;
* Currently, qerror.h defines these error formats. This function is not
* meant to be used outside of QEMU.
*/
-void error_set(Error **err, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+void error_set(Error **err, ErrorClass err_class, const char *fmt, ...)
GCC_FMT_ATTR(3, 4);
/**
* Returns true if an indirect pointer to an error is pointing to a valid
@@ -70,6 +71,6 @@ void error_free(Error *err);
* Determine if an error is of a speific type (based on the qerror format).
* Non-QEMU users should get the `class' field to identify the error type.
*/
-bool error_is_type(Error *err, const char *fmt);
+bool error_is_type(Error *err, ErrorClass err_class, const char *fmt);
#endif
diff --git a/qerror.c b/qerror.c
index ff460b0..0bf8aec 100644
--- a/qerror.c
+++ b/qerror.c
@@ -386,13 +386,15 @@ static QDict *error_obj_from_fmt_no_fail(const char *fmt,
va_list *va)
*
* Return strong reference.
*/
-static QError *qerror_from_info(const char *fmt, va_list *va)
+static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
+va_list *va)
{
QError *qerr;
qerr = qerror_new();
loc_save(&qerr->loc);
+qerr->err_class = err_class;
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
qerr->err_msg = qerror_format(fmt, qerr->error);
@@ -518,13 +520,13 @@ static void qerror_print(QError *qerror)
QDECREF(qstring);
}
-void qerror_report(const char *fmt, ...)
+void qerror_report(ErrorClass eclass, const char *fmt, ...)
{
va_list va;
QError *qerror;
va_start(va, fmt);
-qerror = qerror_from_info(fmt, &va);
+qerror = qerror_from_info(eclass, fmt, &va);
va_end(va);
if (monitor_cur_is_qmp()) {
@@ -540,6 +542,7 @@ struct Error
{
QDict *obj;
char *msg;
+ErrorClass err_class;
};
void qerror_report_err(Error *err)
@@ -551,6 +554,7 @@ void qerror_report_err(Error *err)
QINCREF(err->obj);
qerr->error = err->obj;
qerr->err_msg = g_strdup(err->msg);
+qerr->err_class = err->err_class;
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
diff --git a/qerror.h b/qerror.h
index 2e6a49d..bcc93f8 100644
--- a/qerror.h
+++ b/qerror.h
@@ -16,9 +16,11 @@
#include "qstring.h"
#include "qemu-error.h"
#include "error.h"
+#include "qapi-types.h"
#include
typedef struct QErrorStringTable {
+ErrorClass err_class;
const char *error_fmt;
const char *desc;
} QErrorStringTable;
@@ -28,10 +30,11 @@ typedef struct QError {
QDict *error;
Location loc;
char *err_msg;
+ErrorClass err_class;
} QError;
QString *qerror_human(const QError *qerror);
-void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
+void qerror_report(ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(2,
3);
void qerror_report_err(Error *err)
[Qemu-devel] [PATCH 31/35] error: drop error_get_qobject()/error_set_qobject()
error_get_qobject() is unused since last commit, error_set_qobject()
has never been used. Also drops error_int.h.
Signed-off-by: Luiz Capitulino
---
error.c | 20
error_int.h | 28
qapi/qmp-dispatch.c | 1 -
qemu-ga.c | 1 -
4 files changed, 50 deletions(-)
delete mode 100644 error_int.h
diff --git a/error.c b/error.c
index b1d5131..9d7b35f 100644
--- a/error.c
+++ b/error.c
@@ -15,7 +15,6 @@
#include "qjson.h"
#include "qdict.h"
#include "qapi-types.h"
-#include "error_int.h"
#include "qerror.h"
struct Error
@@ -91,22 +90,3 @@ void error_propagate(Error **dst_err, Error *local_err)
error_free(local_err);
}
}
-
-QObject *error_get_qobject(Error *err)
-{
-QINCREF(err->obj);
-return QOBJECT(err->obj);
-}
-
-void error_set_qobject(Error **errp, QObject *obj)
-{
-Error *err;
-if (errp == NULL) {
-return;
-}
-err = g_malloc0(sizeof(*err));
-err->obj = qobject_to_qdict(obj);
-qobject_incref(obj);
-
-*errp = err;
-}
diff --git a/error_int.h b/error_int.h
deleted file mode 100644
index 4b00d08..000
--- a/error_int.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * QEMU Error Objects
- *
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- * Anthony Liguori
- *
- * This work is licensed under the terms of the GNU LGPL, version 2. See
- * the COPYING.LIB file in the top-level directory.
- */
-#ifndef QEMU_ERROR_INT_H
-#define QEMU_ERROR_INT_H
-
-#include "qemu-common.h"
-#include "qobject.h"
-#include "qdict.h"
-#include "error.h"
-
-/**
- * Internal QEMU functions for working with Error.
- *
- * These are used to convert QErrors to Errors
- */
-QObject *error_get_qobject(Error *err);
-void error_set_qobject(Error **errp, QObject *obj);
-
-#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index ec613f8..4085994 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -16,7 +16,6 @@
#include "json-parser.h"
#include "qapi-types.h"
#include "error.h"
-#include "error_int.h"
#include "qerror.h"
static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
diff --git a/qemu-ga.c b/qemu-ga.c
index 39abc50..8f87621 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -28,7 +28,6 @@
#include "module.h"
#include "signal.h"
#include "qerror.h"
-#include "error_int.h"
#include "qapi/qmp-core.h"
#include "qga/channel.h"
#ifdef _WIN32
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 13/35] hmp_change(): don't access DeviceEncrypted's data
It's not needed. As the device name is already known, we can replace
the duplicated password prompting code by monitor_read_block_device_key().
This overly simplifies hmp_change().
Signed-off-by: Luiz Capitulino
---
hmp.c | 29 ++---
1 file changed, 2 insertions(+), 27 deletions(-)
diff --git a/hmp.c b/hmp.c
index 4efaf51..54c37d7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -776,22 +776,6 @@ static void hmp_change_read_arg(Monitor *mon, const char
*password,
monitor_read_command(mon, 1);
}
-static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
- void *opaque)
-{
-Error *encryption_err = opaque;
-Error *err = NULL;
-const char *device;
-
-device = error_get_field(encryption_err, "device");
-
-qmp_block_passwd(device, password, &err);
-hmp_handle_error(mon, &err);
-error_free(encryption_err);
-
-monitor_read_command(mon, 1);
-}
-
void hmp_change(Monitor *mon, const QDict *qdict)
{
const char *device = qdict_get_str(qdict, "device");
@@ -810,17 +794,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
qmp_change(device, target, !!arg, arg, &err);
if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
-monitor_printf(mon, "%s (%s) is encrypted.\n",
- error_get_field(err, "device"),
- error_get_field(err, "filename"));
-if (!monitor_get_rs(mon)) {
-monitor_printf(mon,
-"terminal does not support password prompting\n");
-error_free(err);
-return;
-}
-readline_start(monitor_get_rs(mon), "Password: ", 1,
- cb_hmp_change_bdrv_pwd, err);
+error_free(err);
+monitor_read_block_device_key(mon, device, NULL, NULL);
return;
}
hmp_handle_error(mon, &err);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 35/35] docs: writing-qmp-commands.txt: update error section
Add information about the new error format and improve the text a bit.
Signed-off-by: Luiz Capitulino
---
docs/writing-qmp-commands.txt | 47 +--
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 0ad51aa..8349dec 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -210,19 +210,17 @@ if you don't see these strings, then something went wrong.
=== Errors ===
QMP commands should use the error interface exported by the error.h header
-file. The basic function used to set an error is the error_set() one.
+file. Basically, errors are set by calling the error_set() function.
Let's say we don't accept the string "message" to contain the word "love". If
-it does contain it, we want the "hello-world" command to the return the
-InvalidParameter error.
-
-Only one change is required, and it's in the C implementation:
+it does contain it, we want the "hello-world" command to return an error:
void qmp_hello_world(bool has_message, const char *message, Error **errp)
{
if (has_message) {
if (strstr(message, "love")) {
-error_set(errp, QERR_INVALID_PARAMETER, "message");
+error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+ "the word 'love' is not allowed");
return;
}
printf("%s\n", message);
@@ -231,30 +229,40 @@ void qmp_hello_world(bool has_message, const char
*message, Error **errp)
}
}
-Let's test it. Build qemu, run it as defined in the "Testing" section, and
-then issue the following command:
+The first argument to the error_set() function is the Error pointer to pointer,
+which is passed to all QMP functions. The second argument is a ErrorClass
+value, which should be ERROR_CLASS_GENERIC_ERROR most of the time (more
+details about error classes are given below). The third argument is a human
+description of the error, this is a free-form printf-like string.
+
+Let's test the example above. Build qemu, run it as defined in the "Testing"
+section, and then issue the following command:
-{ "execute": "hello-world", "arguments": { "message": "we love qemu" } }
+{ "execute": "hello-world", "arguments": { "message": "all you need is love" }
}
The QMP server's response should be:
{
"error": {
-"class": "InvalidParameter",
-"desc": "Invalid parameter 'message'",
-"data": {
-"name": "message"
-}
+"class": "GenericError",
+"desc": "the word 'love' is not allowed"
}
}
-Which is the InvalidParameter error.
+As a general rule, all QMP errors should use ERROR_CLASS_GENERIC_ERROR. There
+are two exceptions to this rule:
+
+ 1. A non-generic ErrorClass value exists* for the failure you want to report
+(eg. DeviceNotFound)
+
+ 2. Management applications have to take special action on the failure you
+want to report, hence you have to add a new ErrorClass value so that they
+can check for it
-When you have to return an error but you're unsure what error to return or
-which arguments an error takes, you should look at the qerror.h file. Note
-that you might be required to add new errors if needed.
+If the failure you want to report doesn't fall in one of the two cases above,
+just report ERROR_CLASS_GENERIC_ERROR.
-FIXME: describe better the error API and how to add new errors.
+ * All existing ErrorClass values are defined in the qapi-schema.json file
=== Command Documentation ===
@@ -275,7 +283,6 @@ here goes "hello-world"'s new entry for the
qapi-schema.json file:
# @message: #optional string to be printed
#
# Returns: Nothing on success.
-# If @message contains "love", InvalidParameter
#
# Notes: if @message is not provided, the "Hello, world" string will
#be printed instead
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 28/35] error: drop unused functions
Besides of being unused, they operate on the current error format,
which is going to be replaced soon.
Signed-off-by: Luiz Capitulino
---
error.c | 48
error.h | 16
error_int.h | 1 -
3 files changed, 65 deletions(-)
diff --git a/error.c b/error.c
index 2d34cde..b1d5131 100644
--- a/error.c
+++ b/error.c
@@ -74,29 +74,6 @@ const char *error_get_pretty(Error *err)
return err->msg;
}
-const char *error_get_field(Error *err, const char *field)
-{
-if (strcmp(field, "class") == 0) {
-return qdict_get_str(err->obj, field);
-} else {
-QDict *dict = qdict_get_qdict(err->obj, "data");
-return qdict_get_str(dict, field);
-}
-}
-
-QDict *error_get_data(Error *err)
-{
-QDict *data = qdict_get_qdict(err->obj, "data");
-QINCREF(data);
-return data;
-}
-
-void error_set_field(Error *err, const char *field, const char *value)
-{
-QDict *dict = qdict_get_qdict(err->obj, "data");
-qdict_put(dict, field, qstring_from_str(value));
-}
-
void error_free(Error *err)
{
if (err) {
@@ -106,31 +83,6 @@ void error_free(Error *err)
}
}
-bool error_is_type(Error *err, ErrorClass err_class, const char *fmt)
-{
-const char *error_class;
-char *ptr;
-char *end;
-
-if (!err) {
-return false;
-}
-
-ptr = strstr(fmt, "'class': '");
-assert(ptr != NULL);
-ptr += strlen("'class': '");
-
-end = strchr(ptr, '\'');
-assert(end != NULL);
-
-error_class = error_get_field(err, "class");
-if (strlen(error_class) != end - ptr) {
-return false;
-}
-
-return strncmp(ptr, error_class, end - ptr) == 0;
-}
-
void error_propagate(Error **dst_err, Error *local_err)
{
if (dst_err && !*dst_err) {
diff --git a/error.h b/error.h
index 114e24b..5336fc5 100644
--- a/error.h
+++ b/error.h
@@ -51,16 +51,6 @@ Error *error_copy(const Error *err);
const char *error_get_pretty(Error *err);
/**
- * Get an individual named error field.
- */
-const char *error_get_field(Error *err, const char *field);
-
-/**
- * Get an individual named error field.
- */
-void error_set_field(Error *err, const char *field, const char *value);
-
-/**
* Propagate an error to an indirect pointer to an error. This function will
* always transfer ownership of the error reference and handles the case where
* dst_err is NULL correctly. Errors after the first are discarded.
@@ -72,10 +62,4 @@ void error_propagate(Error **dst_err, Error *local_err);
*/
void error_free(Error *err);
-/**
- * Determine if an error is of a speific type (based on the qerror format).
- * Non-QEMU users should get the `class' field to identify the error type.
- */
-bool error_is_type(Error *err, ErrorClass err_class, const char *fmt);
-
#endif
diff --git a/error_int.h b/error_int.h
index 5e39424..4b00d08 100644
--- a/error_int.h
+++ b/error_int.h
@@ -22,7 +22,6 @@
*
* These are used to convert QErrors to Errors
*/
-QDict *error_get_data(Error *err);
QObject *error_get_qobject(Error *err);
void error_set_qobject(Error **errp, QObject *obj);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 10/35] error: don't delay error message construction
Today, the error message is only constructed when it's used. This commit
changes that to construct the error message when the error object is
built (ie. when the error is reported).
This simplifies the Error object.
Signed-off-by: Luiz Capitulino
---
error.c | 8 +---
qerror.c | 4 +---
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/error.c b/error.c
index 3a62592..2ade99b 100644
--- a/error.c
+++ b/error.c
@@ -20,7 +20,6 @@
struct Error
{
QDict *obj;
-const char *fmt;
char *msg;
};
@@ -39,7 +38,7 @@ void error_set(Error **errp, const char *fmt, ...)
va_start(ap, fmt);
err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
va_end(ap);
-err->fmt = fmt;
+err->msg = qerror_format(fmt, err->obj);
*errp = err;
}
@@ -50,7 +49,6 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
-err_new->fmt = err->fmt;
err_new->obj = err->obj;
QINCREF(err_new->obj);
@@ -64,10 +62,6 @@ bool error_is_set(Error **errp)
const char *error_get_pretty(Error *err)
{
-if (err->msg == NULL) {
-err->msg = qerror_format(err->fmt, err->obj);
-}
-
return err->msg;
}
diff --git a/qerror.c b/qerror.c
index a254f88..5d38428 100644
--- a/qerror.c
+++ b/qerror.c
@@ -543,7 +543,6 @@ void qerror_report(const char *fmt, ...)
struct Error
{
QDict *obj;
-const char *fmt;
char *msg;
};
@@ -555,8 +554,7 @@ void qerror_report_err(Error *err)
loc_save(&qerr->loc);
QINCREF(err->obj);
qerr->error = err->obj;
-
-qerr->err_msg = qerror_format(err->fmt, qerr->error);
+qerr->err_msg = g_strdup(err->msg);
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [Bug 965327] Re: virtio-pci: can't reserve io 0x0000-0x001f
I'm probably the only person in the world who has this setup (ppc full emu on win32) but if anyone else out there was following this it was fixed in qemu.org git and the solution was another __attribute__ ((packed)) versus __attribute__ ((gcc_struct, packed)) -mms-bitfield issue in spapr_pci.c. You can mark this bug ticket as fix committed via Alexey. Thanks again! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/965327 Title: virtio-pci: can't reserve io 0x-0x001f Status in QEMU: New Bug description: Before 2012-03-05 I was able to successfully enable a virtio-pci block device from a sPAPR pseries ppc64 Linux guest. With the current git master branch after this date I get the following error: virtio-pci :00:00.0: device not available (can't reserve [io 0x-0x001f]) virtio-pci: probe of :00:00.0 failed with error -22 virtio-pci :00:01.0: device not available (can't reserve [io 0x-0x003f]) virtio-pci: probe of :00:01.0 failed with error -22 Full details: - command line: - ./testing/qemu/ppc64-softmmu/qemu-system-ppc64 \ -L ./testing/qemu/pc-bios \ -M pseries \ -m 1024 \ -rtc base=localtime \ -parallel none \ -netdev type=user,id=mynet0,hostfwd=tcp:127.0.0.1:9011-10.0.2.11:22 \ -device virtio-net-pci,netdev=mynet0 \ -drive file=images/suse-ppc.img,if=virtio,index=0,media=disk,cache=unsafe \ -kernel images/iso/suseboot/vmlinux \ -append "root=/dev/mapper/system-root ro audit=0 selinux=0 apparmor=0 console=tty0 console=ttyPZ0" \ -initrd images/iso/suseboot/initrd.img \ -gdb tcp::1234 -- BEFORE virtio-pci "bug/user error?" introduced: -- sPAPR memory map: RTAS : 0x3fff..3fff0013 FDT : 0x3ffe..3ffe Kernel : 0x0040..01abad7b Ramdisk : 0x01ad..02053df7 Firmware load: 0x..000d6ec0 Firmware runtime : 0x3d7e..3ffe sPAPR reset SLOF ** QEMU Starting Build Date = Mar 3 2012 21:46:40 FW Version = git-440e662879c4fc3c Press "s" to enter Open Firmware. Populating /vdevice methods Populating /vdevice/v-scsi@2000 VSCSI: Initializing VSCSI: Looking for disks SCSI ID 2 CD-ROM : "QEMU QEMU CD-ROM 1.0." Populating /vdevice/vty@3000 Populating /pci@0,0 Adapters on 00 (D) : 1af4 1000virtio [ net ] 00 0800 (D) : 1af4 1001virtio [ block ] No NVRAM common partition, re-initializing... Using default console: /vdevice/vty@3000 Detected RAM kernel at 40 (16bad7c bytes) Welcome to Open Firmware Copyright (c) 2004, 2011 IBM Corporation All rights reserved. This program and the accompanying materials are made available under the terms of the BSD License available at http://www.opensource.org/licenses/bsd-license.php Booting from memory... OF stdout device is: /vdevice/vty@3000 Preparing to boot Linux version 3.2.0-2-ppc64 (geeko@buildhost) (gcc version 4.6.2 20111212 [gcc-4_6-branch revision 18] (SUSE Linux) ) #1 SMP Wed Jan 25 10:51:08 UTC 2012 (2206a5c) Detected machine type: 0101 Max number of cores passed to firmware: 1024 (NR_CPUS = 1024) Calling ibm,client-architecture-support... not implemented couldn't open /packages/elf-loader command line: root=/dev/mapper/system-root ro audit=0 selinux=0 apparmor=0 console=tty0 console=ttyPZ0 memory layout at init: memory_limit : (16 MB aligned) alloc_bottom : 01ad alloc_top: 3000 alloc_top_hi : 4000 rmo_top : 3000 ram_top : 4000 instantiating rtas at 0x2fff... done Querying for OPAL presence... not there. boot cpu hw idx 0 copying OF device tree... Building dt strings... Building dt structure... Device tree strings 0x020e -> 0x020e0635 Device tree struct 0x020f -> 0x0210 Calling quiesce... returning from prom_init Using pSeries machine description Using 1TB segments Found initrd at 0xc1ad:0xc2053df8 bootconsole [udbg0] enabled CPU maps initialized for 1 thread per core Starting Linux PPC64 #1 SMP Wed Jan 25 10:51:08 UTC 2012 (2206a5c) - p
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
On Fri, 10 Aug 2012 19:17:22 +0200
Markus Armbruster wrote:
> Luiz Capitulino writes:
>
> > On Fri, 10 Aug 2012 18:35:26 +0200
> > Markus Armbruster wrote:
> >
> >> Luiz Capitulino writes:
> >>
> >> > On Fri, 10 Aug 2012 09:56:11 +0200
> >> > Markus Armbruster wrote:
> >> >
> >> >> Revisited this one on review of v2, replying here for context.
> >> >>
> >> >> Luiz Capitulino writes:
> >> >>
> >> >> > On Thu, 02 Aug 2012 13:35:54 +0200
> >> >> > Markus Armbruster wrote:
> >> >> >
> >> >> >> Luiz Capitulino writes:
> >> >> >>
> >> >> >> > Signed-off-by: Luiz Capitulino
> >> >> >> > ---
> >> >> >> > block.c | 1 +
> >> >> >> > qapi-schema.json | 7 +--
> >> >> >> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >> >> >> >
> >> >> >> > diff --git a/block.c b/block.c
> >> >> >> > index b38940b..9c113b8 100644
> >> >> >> > --- a/block.c
> >> >> >> > +++ b/block.c
> >> >> >> > @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
> >> >> >> > info->value->inserted->ro = bs->read_only;
> >> >> >> > info->value->inserted->drv =
> >> >> >> > g_strdup(bs->drv->format_name);
> >> >> >> > info->value->inserted->encrypted = bs->encrypted;
> >> >> >> > +info->value->inserted->valid_encryption_key =
> >> >> >> > bs->valid_key;
> >> >> >> > if (bs->backing_file[0]) {
> >> >> >> > info->value->inserted->has_backing_file = true;
> >> >> >> > info->value->inserted->backing_file =
> >> >> >> > g_strdup(bs->backing_file);
> >> >> >> > diff --git a/qapi-schema.json b/qapi-schema.json
> >> >> >> > index bc55ed2..1b2d7f5 100644
> >> >> >> > --- a/qapi-schema.json
> >> >> >> > +++ b/qapi-schema.json
> >> >> >> > @@ -400,6 +400,8 @@
> >> >> >> > #
> >> >> >> > # @encrypted: true if the backing device is encrypted
> >> >> >> > #
> >> >> >> > +# @valid_encryption_key: true if a valid encryption key has been
> >> >> >> > set
> >> >> >> > +#
> >> >> >> > # @bps: total throughput limit in bytes per second is specified
> >> >> >> > #
> >> >> >> > # @bps_rd: read throughput limit in bytes per second is specified
> >> >> >> > @@ -419,8 +421,9 @@
> >> >> >> > { 'type': 'BlockDeviceInfo',
> >> >> >> >'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
> >> >> >> > '*backing_file': 'str', 'encrypted': 'bool',
> >> >> >> > -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> >> >> >> > -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >> >> > +'valid_encryption_key': 'bool', 'bps': 'int',
> >> >> >> > +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
> >> >> >> > +'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >> >> >
> >> >> >> > ##
> >> >> >> > # @BlockDeviceIoStatus:
> >> >> >>
> >> >> >> BlockDeviceInfo is API, isn't it?
> >> >> >
> >> >> > Yes.
> >> >> >
> >> >> >> Note that bs->valid_key currently implies bs->encrypted.
> >> >> >> bs->valid_key
> >> >> >> && !bs->encrypted is impossible. Should we make valid_encryption_key
> >> >> >> only available when encrypted?
> >> >> >
> >> >> > I don't think so. It's a bool, so it's ok for it to be false when
> >> >> > encrypted is false.
> >> >>
> >> >> What bothers me is encrypted=false, valid_encryption_key=true.
> >> >
> >> > Disappearing keys is worse, IMHO (assuming that that situation is
> >> > impossible
> >> > in practice, of course).
> >>
> >> It's fundamentally three states: unencrypted, encrypted-no-key,
> >> encrypted-got-key. I'm fine with mapping these onto two bools, it's how
> >> the block layer does it. You may want to consider a single enumeration
> >> instead.
> >
> > That's arguable. But I like the bools slightly better because they allow
> > clients to do a true/false check vs. having to check against an enum value.
> >
> > Again, that's arguable.
> >
> >> >> >> valid_encryption_key is a bit long for my taste. Yours may be
> >> >> >> different.
> >> >> >
> >> >> > We should choose more descriptive and self-documenting names for the
> >> >> > protocol. Besides, I can't think of anything shorter that won't get
> >> >> > cryptic.
> >> >> >
> >> >> > Suggestions are always welcome though :)
> >> >>
> >> >> valid_encryption_key sounds like the value is the valid key.
> >> >
> >> > That's exactly what it is.
> >>
> >> Err, isn't the value bool? The key value is a string...
> >
> > Ah, sorry, I read "sounds like true means the key is valid even for an
> > invalid key". I've renamed it to encryption_key_missing, should be better
> > (although I could also do encryption_key_is_missing).
> >
> >> >> got_crypt_key? Also avoids "valid". Good, because current encrypted
> >> >> formats don't actually validate the key; they happily accept any key.
> >> >
> >> > That's a block layer bug, not QMP's.
> >> >
> >> > QMP clients are going to be misguided by valid_encryption_key the same
> >> > way
> >> > they are with the block_passwd command or h
Re: [Qemu-devel] [RFC V2 03/10] quorum: Add quorum_open().
Le Tuesday 07 Aug 2012 à 20:30:09 (+), Blue Swirl a écrit :
> On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet wrote:
> > Signed-off-by: Benoit Canet
> > ---
> > block/quorum.c | 62
> >
> > 1 file changed, 62 insertions(+)
> >
> > diff --git a/block/quorum.c b/block/quorum.c
> > index e0405b6..de58ab8 100644
> > --- a/block/quorum.c
> > +++ b/block/quorum.c
> > @@ -47,11 +47,73 @@ struct QuorumAIOCB {
> > int vote_ret;
> > };
> >
> > +/* Valid quorum filenames look like
> > + * quorum:path/to/a_image:path/to/b_image:path/to/c_image
>
> This syntax would mean that stacking for example curl or other network
> paths would not be possible. How about comma as separator?
I just tried comma but it fail because the qemu command line parsing
catch it believing that the string next to the coma is another "file="
like options.
Is there any other separator we can use ?
Benoît
[Qemu-devel] [PATCH 14/35] net: inet_connect(), inet_connect_opts(): add in_progress argument
It's used to indicate the special case where a valid file-descriptor
is returned (ie. success) but the connection can't be completed
w/o blocking.
This is needed because QERR_SOCKET_CONNECT_IN_PROGRESS is not
treated like an error and a future commit will drop it.
Signed-off-by: Luiz Capitulino
---
migration-tcp.c | 2 +-
nbd.c | 2 +-
qemu-char.c | 2 +-
qemu-sockets.c | 14 +++---
qemu_socket.h | 4 ++--
ui/vnc.c| 2 +-
6 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 440804d..18944a4 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -86,7 +86,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const
char *host_port,
s->write = socket_write;
s->close = tcp_close;
-s->fd = inet_connect(host_port, false, errp);
+s->fd = inet_connect(host_port, false, NULL, errp);
if (!error_is_set(errp)) {
migrate_fd_connect(s);
diff --git a/nbd.c b/nbd.c
index dc0adf9..0dd60c5 100644
--- a/nbd.c
+++ b/nbd.c
@@ -162,7 +162,7 @@ int tcp_socket_outgoing(const char *address, uint16_t port)
int tcp_socket_outgoing_spec(const char *address_and_port)
{
-return inet_connect(address_and_port, true, NULL);
+return inet_connect(address_and_port, true, NULL, NULL);
}
int tcp_socket_incoming(const char *address, uint16_t port)
diff --git a/qemu-char.c b/qemu-char.c
index c2aaaee..382c71e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2446,7 +2446,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts
*opts)
if (is_listen) {
fd = inet_listen_opts(opts, 0, NULL);
} else {
-fd = inet_connect_opts(opts, NULL);
+fd = inet_connect_opts(opts, NULL, NULL);
}
}
if (fd < 0) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index beb2bb6..9cb47d4 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -209,7 +209,7 @@ listen:
return slisten;
}
-int inet_connect_opts(QemuOpts *opts, Error **errp)
+int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
{
struct addrinfo ai,*res,*e;
const char *addr;
@@ -224,6 +224,10 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
ai.ai_family = PF_UNSPEC;
ai.ai_socktype = SOCK_STREAM;
+if (in_progress) {
+*in_progress = false;
+}
+
addr = qemu_opt_get(opts, "host");
port = qemu_opt_get(opts, "port");
block = qemu_opt_get_bool(opts, "block", 0);
@@ -277,6 +281,10 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
#else
if (!block && (rc == -EINPROGRESS)) {
#endif
+if (in_progress) {
+*in_progress = true;
+}
+
error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
} else if (rc < 0) {
if (NULL == e->ai_next)
@@ -487,7 +495,7 @@ int inet_listen(const char *str, char *ostr, int olen,
return sock;
}
-int inet_connect(const char *str, bool block, Error **errp)
+int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
{
QemuOpts *opts;
int sock = -1;
@@ -497,7 +505,7 @@ int inet_connect(const char *str, bool block, Error **errp)
if (block) {
qemu_opt_set(opts, "block", "on");
}
-sock = inet_connect_opts(opts, errp);
+sock = inet_connect_opts(opts, in_progress, errp);
} else {
error_set(errp, QERR_SOCKET_CREATE_FAILED);
}
diff --git a/qemu_socket.h b/qemu_socket.h
index 4689ff3..30ae6af 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -42,8 +42,8 @@ int send_all(int fd, const void *buf, int len1);
int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
int inet_listen(const char *str, char *ostr, int olen,
int socktype, int port_offset, Error **errp);
-int inet_connect_opts(QemuOpts *opts, Error **errp);
-int inet_connect(const char *str, bool block, Error **errp);
+int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp);
+int inet_connect(const char *str, bool block, bool *in_progress, Error **errp);
int inet_dgram_opts(QemuOpts *opts);
const char *inet_strfamily(int family);
diff --git a/ui/vnc.c b/ui/vnc.c
index 312ad7f..385e345 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3061,7 +3061,7 @@ int vnc_display_open(DisplayState *ds, const char
*display)
if (strncmp(display, "unix:", 5) == 0)
vs->lsock = unix_connect(display+5);
else
-vs->lsock = inet_connect(display, true, NULL);
+vs->lsock = inet_connect(display, true, NULL, NULL);
if (-1 == vs->lsock) {
g_free(vs->display);
vs->display = NULL;
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 02/35] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from human msg
Actually, renames it to 'object'. This must be what the original author
meant to write, as there's no 'object' in the error's data member.
Signed-off-by: Luiz Capitulino
---
qerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qerror.c b/qerror.c
index 92c4eff..082de98 100644
--- a/qerror.c
+++ b/qerror.c
@@ -49,7 +49,7 @@ static const QErrorStringTable qerror_table[] = {
},
{
.error_fmt = QERR_AMBIGUOUS_PATH,
-.desc = "Path '%(path)' does not uniquely identify a %(object)"
+.desc = "Path '%(path)' does not uniquely identify an object"
},
{
.error_fmt = QERR_BAD_BUS_FOR_DEVICE,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 01/35] monitor: drop unused monitor debug code
In the old QMP days, this code was used to find out QMP commands that
might be calling monitor_printf() down its call chain.
This is almost impossible to happen today, because the qapi converted
commands don't even have a monitor object. Besides, it's been more than
a year since I used this last time.
Let's just drop it.
Signed-off-by: Luiz Capitulino
---
configure | 10 --
monitor.c | 65 ---
2 files changed, 75 deletions(-)
diff --git a/configure b/configure
index 280726c..030d137 100755
--- a/configure
+++ b/configure
@@ -147,7 +147,6 @@ vhost_net="no"
kvm="no"
gprof="no"
debug_tcg="no"
-debug_mon="no"
debug="no"
strip_opt="yes"
tcg_interpreter="no"
@@ -633,14 +632,9 @@ for opt do
;;
--disable-debug-tcg) debug_tcg="no"
;;
- --enable-debug-mon) debug_mon="yes"
- ;;
- --disable-debug-mon) debug_mon="no"
- ;;
--enable-debug)
# Enable debugging options that aren't excessively noisy
debug_tcg="yes"
- debug_mon="yes"
debug="yes"
strip_opt="no"
;;
@@ -3039,7 +3033,6 @@ echo "host CPU $cpu"
echo "host big endian $bigendian"
echo "target list $target_list"
echo "tcg debug enabled $debug_tcg"
-echo "Mon debug enabled $debug_mon"
echo "gprof enabled $gprof"
echo "sparse enabled$sparse"
echo "strip binaries$strip_opt"
@@ -3132,9 +3125,6 @@ echo "ARCH=$ARCH" >> $config_host_mak
if test "$debug_tcg" = "yes" ; then
echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
fi
-if test "$debug_mon" = "yes" ; then
- echo "CONFIG_DEBUG_MONITOR=y" >> $config_host_mak
-fi
if test "$debug" = "yes" ; then
echo "CONFIG_DEBUG_EXEC=y" >> $config_host_mak
fi
diff --git a/monitor.c b/monitor.c
index 49dccfe..aa57167 100644
--- a/monitor.c
+++ b/monitor.c
@@ -172,41 +172,11 @@ struct Monitor {
CPUArchState *mon_cpu;
BlockDriverCompletionFunc *password_completion_cb;
void *password_opaque;
-#ifdef CONFIG_DEBUG_MONITOR
-int print_calls_nr;
-#endif
QError *error;
QLIST_HEAD(,mon_fd_t) fds;
QLIST_ENTRY(Monitor) entry;
};
-#ifdef CONFIG_DEBUG_MONITOR
-#define MON_DEBUG(fmt, ...) do {\
-fprintf(stderr, "Monitor: "); \
-fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-
-static inline void mon_print_count_inc(Monitor *mon)
-{
-mon->print_calls_nr++;
-}
-
-static inline void mon_print_count_init(Monitor *mon)
-{
-mon->print_calls_nr = 0;
-}
-
-static inline int mon_print_count_get(const Monitor *mon)
-{
-return mon->print_calls_nr;
-}
-
-#else /* !CONFIG_DEBUG_MONITOR */
-#define MON_DEBUG(fmt, ...) do { } while (0)
-static inline void mon_print_count_inc(Monitor *mon) { }
-static inline void mon_print_count_init(Monitor *mon) { }
-static inline int mon_print_count_get(const Monitor *mon) { return 0; }
-#endif /* CONFIG_DEBUG_MONITOR */
-
/* QMP checker flags */
#define QMP_ACCEPT_UNKNOWNS 1
@@ -299,8 +269,6 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list
ap)
if (!mon)
return;
-mon_print_count_inc(mon);
-
if (monitor_ctrl_mode(mon)) {
return;
}
@@ -3860,8 +3828,6 @@ void monitor_set_error(Monitor *mon, QError *qerror)
if (!mon->error) {
mon->error = qerror;
} else {
-MON_DEBUG("Additional error report at %s:%d\n",
- qerror->file, qerror->linenr);
QDECREF(qerror);
}
}
@@ -3875,36 +3841,7 @@ static void handler_audit(Monitor *mon, const mon_cmd_t
*cmd, int ret)
* Action: Report an internal error to the client if in QMP.
*/
qerror_report(QERR_UNDEFINED_ERROR);
-MON_DEBUG("command '%s' returned failure but did not pass an error\n",
- cmd->name);
}
-
-#ifdef CONFIG_DEBUG_MONITOR
-if (!ret && monitor_has_error(mon)) {
-/*
- * If it returns success, it must not have passed an error.
- *
- * Action: Report the passed error to the client.
- */
-MON_DEBUG("command '%s' returned success but passed an error\n",
- cmd->name);
-}
-
-if (mon_print_count_get(mon) > 0 && strcmp(cmd->name, "info") != 0) {
-/*
- * Handlers should not call Monitor print functions.
- *
- * Action: Ignore them in QMP.
- *
- * (XXX: we don't check any 'info' or 'query' command here
- * because the user print function _is_ called by do_info(), hence
- * we will trigger this check. This problem will go away when we
- * make 'query' commands real and kill do_info())
- */
-MON_DEBUG("command '%s' called print functions %d time(s)\n",
- cmd->name, mon_print_count_get(mon));
-}
-#endif
}
static void handle_user_command(Monitor *mon, const char *cmdline)
@@ -4433,8 +4370,6 @@ static void qmp_call_cmd(Monitor *mon, const mon_cmd_t
*cmd,
int ret;
QObject *data = NULL;
-mon_print_c
Re: [Qemu-devel] Is the return address of get_page_addr_code guest physical address?
On Fri, Aug 10, 2012 at 11:47 AM, Peter Maydell wrote: > On 10 August 2012 03:11, Steven wrote: >> The function definition has a return address type tb_page_addr_t. >> tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr) >> >> I am wondering is this address the guest physical address or the host >> virtual address. > > In linux-user mode the returned address is the guest virtual address. > In system mode it is a ram_addr_t. (the comment above the implementation > says "the returned address is not exactly the physical address: it > is the offset relative to phys_ram_base" but this is out of date I think). > A ram_addr_t is neither a host address nor a guest physical address > but it's closely related to a guest physaddr (you can think of it as > if all the RAM in the system was put into a straight line and then the > ram_addr_t is an index into that). So if I want to get the guest physical address (GPA) of a tb_page_addr_t, can I do tb_page_addr_t = returned value from get_page_addr_code + phys_ram_base Is this translation correct? > >> If it it is the guest physical address, why does Qemu waste guest >> physical space to store these address for tb? Thanks. > > I'm not sure what you're asking here. This function returns a > physical address because we store TCG translated code blocks in > a hash table indexed by guest physaddr. Given the information > "the CPU is trying to execute code from this physaddr" we need to > be able to find out whether we already have a code block translated > for that. (there is also a fast code path so we can avoid doing > a complete lookup from physaddr most of the time.) > > -- PMM
[Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpu-definitions (v2)
Signed-off-by: Anthony Liguori
---
v1 -> v2
- rename query-cpudefs -> query-cpu-definitions
---
target-i386/cpu.c | 22 ++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 880cfea..6d5d0d6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -28,6 +28,7 @@
#include "qemu-config.h"
#include "qapi/qapi-visit-core.h"
+#include "qmp-commands.h"
#include "hyperv.h"
@@ -1125,6 +1126,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf,
const char *optarg)
}
}
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+CpuDefinitionInfoList *cpu_list = NULL;
+x86_def_t *def;
+
+for (def = x86_defs; def; def = def->next) {
+CpuDefinitionInfoList *entry;
+CpuDefinitionInfo *info;
+
+info = g_malloc0(sizeof(*info));
+info->name = g_strdup(def->name);
+
+entry = g_malloc0(sizeof(*entry));
+entry->value = info;
+entry->next = cpu_list;
+cpu_list = entry;
+}
+
+return cpu_list;
+}
+
int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
{
CPUX86State *env = &cpu->env;
--
1.7.5.4
[Qemu-devel] [PATCH 04/11] qemu-iotests: Save some sed processes
Instead of building a huge pipeline, just pass all expressions to a single sed process. Suggested-by: Eric Blake Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- tests/qemu-iotests/common.rc | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 7782808..6b80516 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -105,16 +105,16 @@ _make_test_img() # XXX(hch): have global image options? $QEMU_IMG create -f $IMGFMT $extra_img_options $TEST_IMG $image_size | \ - sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" | \ - sed -e "s#$TEST_DIR#TEST_DIR#g" | \ - sed -e "s#$IMGFMT#IMGFMT#g" | \ - sed -e "s# encryption=off##g" | \ - sed -e "s# cluster_size=[0-9]\\+##g" | \ - sed -e "s# table_size=[0-9]\\+##g" | \ - sed -e "s# compat='[^']*'##g" | \ - sed -e "s# compat6=\\(on\\|off\\)##g" | \ - sed -e "s# static=\\(on\\|off\\)##g" | \ - sed -e "s# lazy_refcounts=\\(on\\|off\\)##g" +sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \ +-e "s#$TEST_DIR#TEST_DIR#g" \ +-e "s#$IMGFMT#IMGFMT#g" \ +-e "s# encryption=off##g" \ +-e "s# cluster_size=[0-9]\\+##g" \ +-e "s# table_size=[0-9]\\+##g" \ +-e "s# compat='[^']*'##g" \ +-e "s# compat6=\\(on\\|off\\)##g" \ +-e "s# static=\\(on\\|off\\)##g" \ +-e "s# lazy_refcounts=\\(on\\|off\\)##g" } _cleanup_test_img() -- 1.7.6.5
[Qemu-devel] [PATCH 02/11] ahci: Fix ahci cdrom read corruptions for reads > 128k
From: Jason Baron
While testing q35, which has its cdrom attached to the ahci controller, I found
that the Fedora 17 install would panic on boot. The panic occurs while
squashfs is trying to read from the cdrom. The errors are:
[8.622711] SQUASHFS error: xz_dec_run error, data probably corrupt
[8.625180] SQUASHFS error: squashfs_read_data failed to read block
0x20be48a
I was also able to produce corrupt data reads using an installed piix based
qemu machine, using 'dd'. I found that the corruptions were only occuring when
then read size was greater than 128k. For example, the following command
results in corrupted reads:
dd if=/dev/sr0 of=/tmp/blah bs=256k iflag=direct
The > 128k size reads exercise a different code path than 128k and below. In
ide_atapi_cmd_read_dma_cb() s->io_buffer_size is capped at 128k. Thus,
ide_atapi_cmd_read_dma_cb() is called a second time when the read is > 128k.
However, ahci_dma_rw_buf() restart the read from offset 0, instead of at 128k.
Thus, resulting in a corrupted read.
To fix this, I've introduced 'io_buffer_offset' field in IDEState to keep
track of the offset. I've also modified ahci_populate_sglist() to take a new
3rd offset argument, so that the sglist is property initialized.
I've tested this patch using 'dd' testing, and Fedora 17 now correctly boots
and installs on q35 with the cdrom ahci controller.
Signed-off-by: Jason Baron
Tested-by: Andreas Färber
Signed-off-by: Kevin Wolf
---
hw/ide/ahci.c | 41 ++---
hw/ide/internal.h |1 +
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index efea93f..de580a6 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -636,7 +636,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t
*cmd_fis)
}
}
-static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist)
+static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset)
{
AHCICmdHdr *cmd = ad->cur_cmd;
uint32_t opts = le32_to_cpu(cmd->opts);
@@ -647,6 +647,10 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList
*sglist)
uint8_t *prdt;
int i;
int r = 0;
+int sum = 0;
+int off_idx = -1;
+int off_pos = -1;
+int tbl_entry_size;
if (!sglist_alloc_hint) {
DPRINTF(ad->port_no, "no sg list given by guest: 0x%08x\n", opts);
@@ -669,10 +673,31 @@ static int ahci_populate_sglist(AHCIDevice *ad,
QEMUSGList *sglist)
/* Get entries in the PRDT, init a qemu sglist accordingly */
if (sglist_alloc_hint > 0) {
AHCI_SG *tbl = (AHCI_SG *)prdt;
-
-qemu_sglist_init(sglist, sglist_alloc_hint, ad->hba->dma);
+sum = 0;
for (i = 0; i < sglist_alloc_hint; i++) {
/* flags_size is zero-based */
+tbl_entry_size = (le32_to_cpu(tbl[i].flags_size) + 1);
+if (offset <= (sum + tbl_entry_size)) {
+off_idx = i;
+off_pos = offset - sum;
+break;
+}
+sum += tbl_entry_size;
+}
+if ((off_idx == -1) || (off_pos < 0) || (off_pos > tbl_entry_size)) {
+DPRINTF(ad->port_no, "%s: Incorrect offset! "
+"off_idx: %d, off_pos: %d\n",
+__func__, off_idx, off_pos);
+r = -1;
+goto out;
+}
+
+qemu_sglist_init(sglist, (sglist_alloc_hint - off_idx), ad->hba->dma);
+qemu_sglist_add(sglist, le64_to_cpu(tbl[off_idx].addr + off_pos),
+le32_to_cpu(tbl[off_idx].flags_size) + 1 - off_pos);
+
+for (i = off_idx + 1; i < sglist_alloc_hint; i++) {
+/* flags_size is zero-based */
qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr),
le32_to_cpu(tbl[i].flags_size) + 1);
}
@@ -745,7 +770,7 @@ static void process_ncq_command(AHCIState *s, int port,
uint8_t *cmd_fis,
ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 2,
s->dev[port].port.ifs[0].nb_sectors - 1);
-ahci_populate_sglist(&s->dev[port], &ncq_tfs->sglist);
+ahci_populate_sglist(&s->dev[port], &ncq_tfs->sglist, 0);
ncq_tfs->tag = tag;
switch(ncq_fis->command) {
@@ -970,7 +995,7 @@ static int ahci_start_transfer(IDEDMA *dma)
goto out;
}
-if (!ahci_populate_sglist(ad, &s->sg)) {
+if (!ahci_populate_sglist(ad, &s->sg, 0)) {
has_sglist = 1;
}
@@ -1015,6 +1040,7 @@ static void ahci_start_dma(IDEDMA *dma, IDEState *s,
DPRINTF(ad->port_no, "\n");
ad->dma_cb = dma_cb;
ad->dma_status |= BM_STATUS_DMAING;
+s->io_buffer_offset = 0;
dma_cb(s, 0);
}
@@ -1023,7 +1049,7 @@ static int ahci_dma_prepare_buf(IDEDMA *dma, int is_write)
AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
IDEState *s = &ad->port.ifs[0];
-ahci_populate_sglist(ad, &s->sg);
+ahci_populate_sglist(ad, &s->sg, 0);
s->i
[Qemu-devel] [PATCH 11/11] qemu-iotests: skip 039 with ./check -nocache
From: Stefan Hajnoczi
When the qemu-io --nocache option is used the 039 test case cannot abort
QEMU at a point where the image is dirty. Skip the test case.
Signed-off-by: Stefan Hajnoczi
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/039 |1 +
tests/qemu-iotests/common.rc | 14 ++
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index a749fcf..c5ae806 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -44,6 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto generic
_supported_os Linux
+_unsupported_qemu_io_options --nocache
size=128M
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 6b80516..d534e94 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -297,6 +297,20 @@ _supported_os()
_notrun "not suitable for this OS: $HOSTOS"
}
+_unsupported_qemu_io_options()
+{
+for bad_opt
+do
+for opt in $QEMU_IO_OPTIONS
+do
+if [ "$bad_opt" = "$opt" ]
+then
+_notrun "not suitable for qemu-io option: $bad_opt"
+fi
+done
+done
+}
+
# this test requires that a specified command (executable) exists
#
_require_command()
--
1.7.6.5
[Qemu-devel] [PATCH 09/11] qcow2: mark image clean after repair succeeds
From: Stefan Hajnoczi
The dirty bit is cleared after image repair succeeds in qcow2_open().
Move this into qcow2_check() so that all callers benefit from this
behavior when fix mode is enabled.
This is necessary so qemu-img check can call .bdrv_check() and mark the
image clean.
Signed-off-by: Stefan Hajnoczi
Signed-off-by: Kevin Wolf
---
block/qcow2.c | 28 +++-
1 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index fd5e214..5896fd6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -270,6 +270,20 @@ static int qcow2_mark_clean(BlockDriverState *bs)
return 0;
}
+static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
+ BdrvCheckMode fix)
+{
+int ret = qcow2_check_refcounts(bs, result, fix);
+if (ret < 0) {
+return ret;
+}
+
+if (fix && result->check_errors == 0 && result->corruptions == 0) {
+return qcow2_mark_clean(bs);
+}
+return ret;
+}
+
static int qcow2_open(BlockDriverState *bs, int flags)
{
BDRVQcowState *s = bs->opaque;
@@ -474,12 +488,7 @@ static int qcow2_open(BlockDriverState *bs, int flags)
!bs->read_only) {
BdrvCheckResult result = {0};
-ret = qcow2_check_refcounts(bs, &result, BDRV_FIX_ERRORS);
-if (ret < 0) {
-goto fail;
-}
-
-ret = qcow2_mark_clean(bs);
+ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
if (ret < 0) {
goto fail;
}
@@ -1568,13 +1577,6 @@ static int qcow2_get_info(BlockDriverState *bs,
BlockDriverInfo *bdi)
return 0;
}
-
-static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
- BdrvCheckMode fix)
-{
-return qcow2_check_refcounts(bs, result, fix);
-}
-
#if 0
static void dump_refcounts(BlockDriverState *bs)
{
--
1.7.6.5
Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs
On Fri, Aug 10, 2012 at 12:09:44PM -0500, Anthony Liguori wrote: > Eduardo Habkost writes: > > > On Fri, Aug 10, 2012 at 11:37:30AM -0500, Anthony Liguori wrote: > >> Eduardo Habkost writes: > >> >> > - add machine-type-specific cpudef compatibility changes? > >> >> > >> >> I think we've discussed this in IRC. I don't think we need to worry > >> >> about this. > >> > > >> > I remember discussing a lot about the mechanism we will use to add the > >> > compatibility changes, but I don t know how the query API will look > >> > like, after we implement this mechanism. > >> > >> 0) User-defined CPU definitions go away > >>- We already made a big step in this direction > >> > >> 1) CPU becomes a DeviceState > > > > 1.1) CPU models become classes > > > >> > >> 2) Features are expressed as properties > >> > >> 3) Same global mechanism used for everything else is used for CPUs > > > > This is basically the compatibility mechanism we agreed upon, yes, but > > what about the probing mechanism to allow libvirt to know what will be > > the result of "-machine M -cpu C"[1] before actually starting a VM? > > I think that the requirement of "before actually starting a VM" is > unreasonable. How is it unreasonable to expect an API where you can know what will be the results of an operation before actually running it? Maybe it doesn't fit in the beautiful and elegant model you are trying to push, but that doesn't make it unreasonable. > > Presumably migration compatibility checking would happen after launching > a guest so libvirt could surely delay querying the CPUID info until > after the guest has started. This is what I call unreasonable. A management layer needs to know if a host can run a VM before trying to migrate it, so the software or the user can take better decisions about migration before asking the VMs to be actually migrated. Note that I don't argue for every single CPUID bit to be available and queriable, but the (un)availability of some features need to be predictable. > > There's a lot of logic involved in deciding what gets exposed to the > guest. We don't really fully know until we've created the VCPU. It's a > whole lot easier and saner to just create the VCPU. If the logic is too complex and unpredictable, we have to make it clearer and more predictable. It's important to do so even if libvirt didn't need a probing interface, otherwise we would never be sure if the code is migration-safe. > > Regards, > > Anthony Liguori > > > > > [1] By "result" I mean: > >- Whether that combination can be run properly on that host; > >- Which CPU features will be visible to the guest in case it runs. > >Both items depend on CPU model _and_ machine-type, that's why we need > >some probing mechanism that depends on the machine-type or use the > >machine-type as input. > > > > > >> > >> Regards, > >> > >> Anthony Liguori > >> > >> >> > Would the command report different results depending on -machine? > >> >> > >> >> No. > >> > > >> > The problem is: > >> > > >> > 1) We need to introduce fixes on a CPU model that changes the set of > >> >guest-visible features (add or remove a feature)[1]; > >> > 2) The fix has to keep compatibility, so older machine-types will > >> >keep exposing the old set of gues-visible features; > >> >- That means different machine-types will have different CPU > >> > features being exposed. > >> > 3) libvirt needs to control/know which guest-visible CPU features are > >> >available to the guest (see above); > >> > 4) Because of (2), the querying system used by libvirt need to depend on > >> >the CPU model and machine-type. > >> > > >> > > >> > [1] Example: > >> > The SandyBridge model today has the "tsc-deadline" bit set, but > >> > QEMU-1.1 did not expose the tsc-deadline feature properly because of > >> > incorrect expectations about the GET_SUPPORTED_CPUID ioctl. This was > >> > fixed on qemu-1.2. > >> > > >> > That means "qemu-1.1 -machine pc-1.1 -cpu SandyBridge" does _not_ > >> > expose tsc-deadline to the guest, and we need to make "qemu-1.2 > >> > -machine pc-1.1 -cpu SandyBridge" _not_ expose it, too (otherwise > >> > migration from qemu-1.1 to qemu-1.2 will be broken). > >> > > >> >> > >> >> > > >> >> > Would the command return the latest cpudef without any machine-type > >> >> > hacks, and libvirt would have to query for the cpudef compatibility > >> >> > data > >> >> > for each machine-type and combine both pieces of information itself? > >> >> > >> >> I'm not sure what you mean by compatibility data. > >> > > >> > I mean any guest-visible compatibility bit that we will need to > >> > introduce on older machine-types, when making changes on CPU models (see > >> > the SandyBridge + tsc-deadline example above). > >> > > >> > I see two options: > >> > - Libvirt queries for a [f(machine_type, cpu_model) -> cpu_features] > >> > function, that will take into account the ma
[Qemu-devel] [PATCH 08/11] qed: mark image clean after repair succeeds
From: Stefan Hajnoczi
The dirty bit is cleared after image repair succeeds in qed_open().
Move this into qed_check() so that all callers benefit from this
behavior when fix=true.
This is necessary so qemu-img check can call .bdrv_check() and mark the
image clean.
Signed-off-by: Stefan Hajnoczi
Signed-off-by: Kevin Wolf
---
block/qed-check.c | 26 ++
block/qed.c |9 +
block/qed.h |5 +
3 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/block/qed-check.c b/block/qed-check.c
index 5edf607..b473dcd 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -194,6 +194,28 @@ static void qed_check_for_leaks(QEDCheck *check)
}
}
+/**
+ * Mark an image clean once it passes check or has been repaired
+ */
+static void qed_check_mark_clean(BDRVQEDState *s, BdrvCheckResult *result)
+{
+/* Skip if there were unfixable corruptions or I/O errors */
+if (result->corruptions > 0 || result->check_errors > 0) {
+return;
+}
+
+/* Skip if image is already marked clean */
+if (!(s->header.features & QED_F_NEED_CHECK)) {
+return;
+}
+
+/* Ensure fixes reach storage before clearing check bit */
+bdrv_flush(s->bs);
+
+s->header.features &= ~QED_F_NEED_CHECK;
+qed_write_header_sync(s);
+}
+
int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
{
QEDCheck check = {
@@ -215,6 +237,10 @@ int qed_check(BDRVQEDState *s, BdrvCheckResult *result,
bool fix)
if (ret == 0) {
/* Only check for leaks if entire image was scanned successfully */
qed_check_for_leaks(&check);
+
+if (fix) {
+qed_check_mark_clean(s, result);
+}
}
g_free(check.used_clusters);
diff --git a/block/qed.c b/block/qed.c
index 5f3eefa..226545d 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -89,7 +89,7 @@ static void qed_header_cpu_to_le(const QEDHeader *cpu,
QEDHeader *le)
le->backing_filename_size = cpu_to_le32(cpu->backing_filename_size);
}
-static int qed_write_header_sync(BDRVQEDState *s)
+int qed_write_header_sync(BDRVQEDState *s)
{
QEDHeader le;
int ret;
@@ -491,13 +491,6 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
if (ret) {
goto out;
}
-if (!result.corruptions && !result.check_errors) {
-/* Ensure fixes reach storage before clearing check bit */
-bdrv_flush(s->bs);
-
-s->header.features &= ~QED_F_NEED_CHECK;
-qed_write_header_sync(s);
-}
}
}
diff --git a/block/qed.h b/block/qed.h
index c716772..a063bf7 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -211,6 +211,11 @@ void *gencb_alloc(size_t len, BlockDriverCompletionFunc
*cb, void *opaque);
void gencb_complete(void *opaque, int ret);
/**
+ * Header functions
+ */
+int qed_write_header_sync(BDRVQEDState *s);
+
+/**
* L2 cache functions
*/
void qed_init_l2_cache(L2TableCache *l2_cache);
--
1.7.6.5
[Qemu-devel] [PATCH 06/11] virtio-blk: disable write cache if not negotiated
From: Paolo Bonzini
If the guest does not support flushes, we should run in writethrough mode.
The setting is temporary until the next reset, so that for example the
BIOS will run in writethrough mode while Linux will run with a writeback
cache.
Signed-off-by: Paolo Bonzini
Signed-off-by: Kevin Wolf
---
hw/virtio-blk.c | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 97bb4bd..fd8fa90 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -543,6 +543,19 @@ static uint32_t virtio_blk_get_features(VirtIODevice
*vdev, uint32_t features)
return features;
}
+static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
+{
+VirtIOBlock *s = to_virtio_blk(vdev);
+uint32_t features;
+
+if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+return;
+}
+
+features = vdev->guest_features;
+bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));
+}
+
static void virtio_blk_save(QEMUFile *f, void *opaque)
{
VirtIOBlock *s = opaque;
@@ -623,6 +636,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
VirtIOBlkConf *blk)
s->vdev.get_config = virtio_blk_update_config;
s->vdev.set_config = virtio_blk_set_config;
s->vdev.get_features = virtio_blk_get_features;
+s->vdev.set_status = virtio_blk_set_status;
s->vdev.reset = virtio_blk_reset;
s->bs = blk->conf.bs;
s->conf = &blk->conf;
--
1.7.6.5
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
Luiz Capitulino writes:
> On Fri, 10 Aug 2012 18:35:26 +0200
> Markus Armbruster wrote:
>
>> Luiz Capitulino writes:
>>
>> > On Fri, 10 Aug 2012 09:56:11 +0200
>> > Markus Armbruster wrote:
>> >
>> >> Revisited this one on review of v2, replying here for context.
>> >>
>> >> Luiz Capitulino writes:
>> >>
>> >> > On Thu, 02 Aug 2012 13:35:54 +0200
>> >> > Markus Armbruster wrote:
>> >> >
>> >> >> Luiz Capitulino writes:
>> >> >>
>> >> >> > Signed-off-by: Luiz Capitulino
>> >> >> > ---
>> >> >> > block.c | 1 +
>> >> >> > qapi-schema.json | 7 +--
>> >> >> > 2 files changed, 6 insertions(+), 2 deletions(-)
>> >> >> >
>> >> >> > diff --git a/block.c b/block.c
>> >> >> > index b38940b..9c113b8 100644
>> >> >> > --- a/block.c
>> >> >> > +++ b/block.c
>> >> >> > @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
>> >> >> > info->value->inserted->ro = bs->read_only;
>> >> >> > info->value->inserted->drv =
>> >> >> > g_strdup(bs->drv->format_name);
>> >> >> > info->value->inserted->encrypted = bs->encrypted;
>> >> >> > +info->value->inserted->valid_encryption_key =
>> >> >> > bs->valid_key;
>> >> >> > if (bs->backing_file[0]) {
>> >> >> > info->value->inserted->has_backing_file = true;
>> >> >> > info->value->inserted->backing_file =
>> >> >> > g_strdup(bs->backing_file);
>> >> >> > diff --git a/qapi-schema.json b/qapi-schema.json
>> >> >> > index bc55ed2..1b2d7f5 100644
>> >> >> > --- a/qapi-schema.json
>> >> >> > +++ b/qapi-schema.json
>> >> >> > @@ -400,6 +400,8 @@
>> >> >> > #
>> >> >> > # @encrypted: true if the backing device is encrypted
>> >> >> > #
>> >> >> > +# @valid_encryption_key: true if a valid encryption key has been set
>> >> >> > +#
>> >> >> > # @bps: total throughput limit in bytes per second is specified
>> >> >> > #
>> >> >> > # @bps_rd: read throughput limit in bytes per second is specified
>> >> >> > @@ -419,8 +421,9 @@
>> >> >> > { 'type': 'BlockDeviceInfo',
>> >> >> >'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
>> >> >> > '*backing_file': 'str', 'encrypted': 'bool',
>> >> >> > -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
>> >> >> > -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
>> >> >> > +'valid_encryption_key': 'bool', 'bps': 'int',
>> >> >> > +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
>> >> >> > +'iops_rd': 'int', 'iops_wr': 'int'} }
>> >> >> >
>> >> >> > ##
>> >> >> > # @BlockDeviceIoStatus:
>> >> >>
>> >> >> BlockDeviceInfo is API, isn't it?
>> >> >
>> >> > Yes.
>> >> >
>> >> >> Note that bs->valid_key currently implies bs->encrypted. bs->valid_key
>> >> >> && !bs->encrypted is impossible. Should we make valid_encryption_key
>> >> >> only available when encrypted?
>> >> >
>> >> > I don't think so. It's a bool, so it's ok for it to be false when
>> >> > encrypted is false.
>> >>
>> >> What bothers me is encrypted=false, valid_encryption_key=true.
>> >
>> > Disappearing keys is worse, IMHO (assuming that that situation is
>> > impossible
>> > in practice, of course).
>>
>> It's fundamentally three states: unencrypted, encrypted-no-key,
>> encrypted-got-key. I'm fine with mapping these onto two bools, it's how
>> the block layer does it. You may want to consider a single enumeration
>> instead.
>
> That's arguable. But I like the bools slightly better because they allow
> clients to do a true/false check vs. having to check against an enum value.
>
> Again, that's arguable.
>
>> >> >> valid_encryption_key is a bit long for my taste. Yours may be
>> >> >> different.
>> >> >
>> >> > We should choose more descriptive and self-documenting names for the
>> >> > protocol. Besides, I can't think of anything shorter that won't get
>> >> > cryptic.
>> >> >
>> >> > Suggestions are always welcome though :)
>> >>
>> >> valid_encryption_key sounds like the value is the valid key.
>> >
>> > That's exactly what it is.
>>
>> Err, isn't the value bool? The key value is a string...
>
> Ah, sorry, I read "sounds like true means the key is valid even for an
> invalid key". I've renamed it to encryption_key_missing, should be better
> (although I could also do encryption_key_is_missing).
>
>> >> got_crypt_key? Also avoids "valid". Good, because current encrypted
>> >> formats don't actually validate the key; they happily accept any key.
>> >
>> > That's a block layer bug, not QMP's.
>> >
>> > QMP clients are going to be misguided by valid_encryption_key the same way
>> > they are with the block_passwd command or how we suffer from it internally
>> > when calling bdrv_set_key() (which also manifests itself in HMP).
>> >
>> > Fixing the bug where it is will automatically fix all its instances.
>>
>> It's not fixable for existing image formats, and thus existing images.
>
> Why not? I'd expect that changing AES_set_decrypt_key() to fail for an
[Qemu-devel] [PATCH 10/11] block: add BLOCK_O_CHECK for qemu-img check
From: Stefan Hajnoczi
Image formats with a dirty bit, like qed and qcow2, repair dirty image
files upon open with BDRV_O_RDWR. Performing automatic repair when
qemu-img check runs is not ideal because the bdrv_open() call repairs
the image before the actual bdrv_check() call from qemu-img.c.
Fix this "double repair" since it leads to confusing output from
qemu-img check. Tell the block driver that this image is being opened
just for bdrv_check(). This skips automatic repair and qemu-img.c can
invoke it manually with bdrv_check().
Update the golden output for qemu-iotests 039 to reflect the new
qemu-img check output.
Signed-off-by: Stefan Hajnoczi
Signed-off-by: Kevin Wolf
---
block.h|1 +
block/qcow2.c |4 ++--
block/qed.c|2 +-
qemu-img.c |2 +-
tests/qemu-iotests/039.out |6 ++
5 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/block.h b/block.h
index 650d872..2e2be11 100644
--- a/block.h
+++ b/block.h
@@ -79,6 +79,7 @@ typedef struct BlockDevOps {
#define BDRV_O_NO_FLUSH0x0200 /* disable flushing on this disk */
#define BDRV_O_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
#define BDRV_O_INCOMING0x0800 /* consistency hint for incoming migration
*/
+#define BDRV_O_CHECK 0x1000 /* open solely for consistency check */
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
diff --git a/block/qcow2.c b/block/qcow2.c
index 5896fd6..8f183f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -484,8 +484,8 @@ static int qcow2_open(BlockDriverState *bs, int flags)
qemu_co_mutex_init(&s->lock);
/* Repair image if dirty */
-if ((s->incompatible_features & QCOW2_INCOMPAT_DIRTY) &&
-!bs->read_only) {
+if (!(flags & BDRV_O_CHECK) && !bs->read_only &&
+(s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) {
BdrvCheckResult result = {0};
ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
diff --git a/block/qed.c b/block/qed.c
index 226545d..a02dbfd 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -477,7 +477,7 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
}
/* If image was not closed cleanly, check consistency */
-if (s->header.features & QED_F_NEED_CHECK) {
+if (!(flags & BDRV_O_CHECK) && (s->header.features & QED_F_NEED_CHECK)) {
/* Read-only images cannot be fixed. There is no risk of corruption
* since write operations are not possible. Therefore, allow
* potentially inconsistent images to be opened read-only. This can
diff --git a/qemu-img.c b/qemu-img.c
index 94a31ad..b41e670 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -379,7 +379,7 @@ static int img_check(int argc, char **argv)
BlockDriverState *bs;
BdrvCheckResult result;
int fix = 0;
-int flags = BDRV_O_FLAGS;
+int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
fmt = NULL;
for(;;) {
diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 155a05e..cb510d6 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -26,6 +26,12 @@ incompatible_features 0x1
== Repairing the image file must succeed ==
ERROR OFLAG_COPIED: offset=8005 refcount=0
Repairing cluster 5 refcount=0 reference=1
+The following inconsistencies were found and repaired:
+
+0 leaked clusters
+1 corruptions
+
+Double checking the fixed image now...
No errors were found on the image.
incompatible_features 0x0
--
1.7.6.5
Re: [Qemu-devel] [PATCH RFC for-1.2] arm: Move some ARM devices into libhw
Am 02.08.2012 15:48, schrieb Peter Maydell: > On 2 August 2012 02:16, Andreas Färber wrote: >> +hw-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o > > Why just this stellaris device and not the others? So, turns out there was an actual reason: CClibhw64/hw/stellaris.o In file included from /home/andreas/QEMU/qemu-arm/hw/stellaris.c:12:0: /home/andreas/QEMU/qemu-arm/hw/arm-misc.h:19:28: error: unknown type name ‘ARMCPU’ /home/andreas/QEMU/qemu-arm/hw/arm-misc.h:53:34: error: unknown type name ‘ARMCPU’ /home/andreas/QEMU/qemu-arm/hw/arm-misc.h:55:38: error: unknown type name ‘ARMCPU’ /home/andreas/QEMU/qemu-arm/hw/arm-misc.h:62:22: error: unknown type name ‘ARMCPU’ make[1]: *** [hw/stellaris.o] Fehler 1 make: *** [subdir-libhw64] Fehler 2 Leave this part of the patch as is for now then? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH 01/11] virtio-blk: fix use-after-free while handling scsi commands
From: Avi Kivity The scsi passthrough handler falls through after completing a request into the failure path, resulting in a use after free. Reproducible by running a guest with aio=native on a block device. Reported-by: Stefan Priebe Signed-off-by: Avi Kivity Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/virtio-blk.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index f21757e..552b3b6 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -254,6 +254,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) virtio_blk_req_complete(req, status); g_free(req); +return; #else abort(); #endif -- 1.7.6.5
Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs
Eduardo Habkost writes: > On Fri, Aug 10, 2012 at 11:37:30AM -0500, Anthony Liguori wrote: >> Eduardo Habkost writes: >> >> > - add machine-type-specific cpudef compatibility changes? >> >> >> >> I think we've discussed this in IRC. I don't think we need to worry >> >> about this. >> > >> > I remember discussing a lot about the mechanism we will use to add the >> > compatibility changes, but I don t know how the query API will look >> > like, after we implement this mechanism. >> >> 0) User-defined CPU definitions go away >>- We already made a big step in this direction >> >> 1) CPU becomes a DeviceState > > 1.1) CPU models become classes > >> >> 2) Features are expressed as properties >> >> 3) Same global mechanism used for everything else is used for CPUs > > This is basically the compatibility mechanism we agreed upon, yes, but > what about the probing mechanism to allow libvirt to know what will be > the result of "-machine M -cpu C"[1] before actually starting a VM? I think that the requirement of "before actually starting a VM" is unreasonable. Presumably migration compatibility checking would happen after launching a guest so libvirt could surely delay querying the CPUID info until after the guest has started. There's a lot of logic involved in deciding what gets exposed to the guest. We don't really fully know until we've created the VCPU. It's a whole lot easier and saner to just create the VCPU. Regards, Anthony Liguori > > [1] By "result" I mean: >- Whether that combination can be run properly on that host; >- Which CPU features will be visible to the guest in case it runs. >Both items depend on CPU model _and_ machine-type, that's why we need >some probing mechanism that depends on the machine-type or use the >machine-type as input. > > >> >> Regards, >> >> Anthony Liguori >> >> >> > Would the command report different results depending on -machine? >> >> >> >> No. >> > >> > The problem is: >> > >> > 1) We need to introduce fixes on a CPU model that changes the set of >> >guest-visible features (add or remove a feature)[1]; >> > 2) The fix has to keep compatibility, so older machine-types will >> >keep exposing the old set of gues-visible features; >> >- That means different machine-types will have different CPU >> > features being exposed. >> > 3) libvirt needs to control/know which guest-visible CPU features are >> >available to the guest (see above); >> > 4) Because of (2), the querying system used by libvirt need to depend on >> >the CPU model and machine-type. >> > >> > >> > [1] Example: >> > The SandyBridge model today has the "tsc-deadline" bit set, but >> > QEMU-1.1 did not expose the tsc-deadline feature properly because of >> > incorrect expectations about the GET_SUPPORTED_CPUID ioctl. This was >> > fixed on qemu-1.2. >> > >> > That means "qemu-1.1 -machine pc-1.1 -cpu SandyBridge" does _not_ >> > expose tsc-deadline to the guest, and we need to make "qemu-1.2 >> > -machine pc-1.1 -cpu SandyBridge" _not_ expose it, too (otherwise >> > migration from qemu-1.1 to qemu-1.2 will be broken). >> > >> >> >> >> > >> >> > Would the command return the latest cpudef without any machine-type >> >> > hacks, and libvirt would have to query for the cpudef compatibility data >> >> > for each machine-type and combine both pieces of information itself? >> >> >> >> I'm not sure what you mean by compatibility data. >> > >> > I mean any guest-visible compatibility bit that we will need to >> > introduce on older machine-types, when making changes on CPU models (see >> > the SandyBridge + tsc-deadline example above). >> > >> > I see two options: >> > - Libvirt queries for a [f(machine_type, cpu_model) -> cpu_features] >> > function, that will take into account the machine-type-specific >> > compatibility bits. >> > - Libvirt queries for a [f(cpu_model) -> cpu_features] function and a >> > [f(machine_type) -> compatibility_changes] function, and combine both. >> > - I don't like this approach, I am just including it as a possible >> > alternative. >> > >> >> >> >> Regards, >> >> >> >> Anthony Liguori >> >> >> >> > >> >> > [1] Note that it doesn't have to be low-level leaf-by-leaf >> >> > register-by-register CPUID bits (I prefer a more high-level >> >> > interface, myself), but it has to at least say "feature FOO is >> >> > enabled/disabled" for a set of features libvirt cares about. >> >> > >> >> > -- >> >> > Eduardo >> >> >> > >> > -- >> > Eduardo >> > > -- > Eduardo
[Qemu-devel] [NOT RFC] target-i386: prepare for convertion to subclasses
crude attempt to show how to move out cpu_model string handling out of cpu.c
and make CPU more suitable to converting into subclasses.
Signed-off-by: Igor Mammedov
---
target-i386/cpu.c| 83
target-i386/helper.c | 89 +++-
2 files changed, 94 insertions(+), 78 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e266792..b5dcf56 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1099,76 +1099,15 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t
*def, Error **errp)
object_property_set_bool(OBJECT(cpu), true, "hypervisor", errp);
}
-/* convert legacy cpumodel string to string cpu_name and
- * a uniforms set of custom features that will be applied to CPU
- * using object_property_parse()
- */
-static void compat_normalize_cpu_model(const char *cpu_model, char **cpu_name,
-QDict **features, Error **errp)
-{
-
-char *s = g_strdup(cpu_model);
-char *featurestr, *sptr;
-
-*cpu_name = strtok_r(s, ",", &sptr);
-*features = qdict_new();
-
-featurestr = strtok_r(NULL, ",", &sptr);
-while (featurestr) {
-char *val;
-if (featurestr[0] == '+') {
-/*
- * preseve legacy behaviour, if feature was disabled once
- * do not allow to enable it again
- */
-if (!qdict_haskey(*features, featurestr + 1)) {
-qdict_put(*features, featurestr + 1, qstring_from_str("on"));
-}
-} else if (featurestr[0] == '-') {
-qdict_put(*features, featurestr + 1, qstring_from_str("off"));
-} else {
-val = strchr(featurestr, '=');
-if (val) {
-*val = 0; val++;
-if (!strcmp(featurestr, "vendor")) {
-qdict_put(*features, "vendor-override",
- qstring_from_str("on"));
-qdict_put(*features, featurestr, qstring_from_str(val));
-} else if (!strcmp(featurestr, "tsc_freq")) {
-qdict_put(*features, "tsc-frequency",
- qstring_from_str(val));
-} else {
-qdict_put(*features, featurestr, qstring_from_str(val));
-}
-} else {
-qdict_put(*features, featurestr, qstring_from_str("on"));
-}
-}
-
-featurestr = strtok_r(NULL, ",", &sptr);
-}
-
-return;
-}
-
static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
const char *cpu_model, Error **errp)
{
x86_def_t *def;
-QDict *features;
-const QDictEntry *ent;
-char *name;
-
-compat_normalize_cpu_model(cpu_model, &name, &features, errp);
-if (error_is_set(errp)) {
-goto error;
-}
-
for (def = x86_defs; def; def = def->next)
-if (name && !strcmp(name, def->name))
+if (cpu_model && !strcmp(cpu_model, def->name))
break;
-if (kvm_enabled() && name && strcmp(name, "host") == 0) {
+if (kvm_enabled() && cpu_model && strcmp(cpu_model, "host") == 0) {
cpu_x86_fill_host(x86_cpu_def);
} else if (!def) {
goto error;
@@ -1176,23 +1115,9 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t
*x86_cpu_def,
memcpy(x86_cpu_def, def, sizeof(*def));
}
-cpudef_2_x86_cpu(cpu, def, errp);
-
-for (ent = qdict_first(features); ent; ent = qdict_next(features, ent)) {
-const QString *qval = qobject_to_qstring(qdict_entry_value(ent));
-object_property_parse(OBJECT(cpu), qstring_get_str(qval),
- qdict_entry_key(ent), errp);
-if (error_is_set(errp)) {
-goto error;
-}
-}
-QDECREF(features);
-
-g_free(name);
return 0;
error:
-g_free(name);
if (!error_is_set(errp)) {
error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
}
@@ -1302,6 +1227,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
if (cpu_x86_find_by_name(cpu, def, cpu_model, &error) < 0)
goto out;
+/* ==> should go into initfn */
+cpudef_2_x86_cpu(cpu, def, &error);
+/* <== */
+
out:
if (error_is_set(&error)) {
fprintf(stderr, "%s\n", error_get_pretty(error));
diff --git a/target-i386/helper.c b/target-i386/helper.c
index a0e4c89..17acb4e 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -23,6 +23,8 @@
#include "sysemu.h"
#include "monitor.h"
#endif
+#include "qdict.h"
+#include "qstring.h"
//#define DEBUG_MMU
@@ -1147,20 +1149,105 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned
int selector,
return 1;
}
+/* convert legacy cpumodel string to string cpu_name and
+ * a uniforms set of custom features that will be applied to CPU
+ * using object_property_parse()
+ */
+static void comp
Re: [Qemu-devel] [PATCH 2/2] cpu: for cpu-user and cpu-softmmu and make cpu-softmmu a child of DeviceState
On Fri, Aug 10, 2012 at 12:00:44PM -0500, Anthony Liguori wrote:
> The line between linux-user and softmmu is not very well defined right now.
> linux-user really don't want to include devices and making CpuState a child of
> DeviceState would require pulling lots of stuff into linux-user.
>
> To solve this, we simply fork cpu-user and cpu-softmmu letting them evolve
> independently.
>
> We also need to push a qdev_init_nofail() into all callers of cpu_init().
> This
> is needed eventually anyway so might as well do this now.
>
> Signed-off-by: Anthony Liguori
> ---
[...]
> --- /dev/null
> +++ b/qom/cpu-user.c
[...]
> +static TypeInfo cpu_type_info = {
> +.name = TYPE_CPU,
> +#ifdef CONFIG_USER_ONLY
> +.parent = TYPE_OBJECT,
> +#else
> +.parent = TYPE_DEVICE,
> +#endif
Is this #ifdef supposed to be here?
> +.instance_size = sizeof(CPUState),
> +.abstract = true,
> +.class_size = sizeof(CPUClass),
> +.class_init = cpu_class_init,
> +};
> +
> +static void cpu_register_types(void)
> +{
> +type_register_static(&cpu_type_info);
> +}
> +
> +type_init(cpu_register_types)
> --
> 1.7.5.4
>
--
Eduardo
[Qemu-devel] [PULL 00/11] Block patches
The following changes since commit 3d1d9652978ac5a32a0beb4bdf6065ca39440d89: handle device help before accelerator set up (2012-08-09 19:53:01 +) are available in the git repository at: http://repo.or.cz/r/qemu/kevin.git for-anthony Avi Kivity (1): virtio-blk: fix use-after-free while handling scsi commands Jason Baron (2): ahci: Fix ahci cdrom read corruptions for reads > 128k ahci: Fix sglist memleak in ahci_dma_rw_buf() Kevin Wolf (1): qemu-iotests: Save some sed processes Paolo Bonzini (3): virtio-blk: support VIRTIO_BLK_F_CONFIG_WCE virtio-blk: disable write cache if not negotiated blockdev: flip default cache mode from writethrough to writeback Stefan Hajnoczi (4): qed: mark image clean after repair succeeds qcow2: mark image clean after repair succeeds block: add BLOCK_O_CHECK for qemu-img check qemu-iotests: skip 039 with ./check -nocache block.h |1 + block/qcow2.c| 32 -- block/qed-check.c| 26 block/qed.c | 11 + block/qed.h |5 blockdev.c |1 + dma-helpers.c|1 + hw/ide/ahci.c| 44 +++-- hw/ide/internal.h|1 + hw/virtio-blk.c | 31 +++- hw/virtio-blk.h |4 ++- qemu-img.c |2 +- tests/qemu-iotests/039 |1 + tests/qemu-iotests/039.out |6 + tests/qemu-iotests/common.rc | 34 ++- 15 files changed, 155 insertions(+), 45 deletions(-)
Re: [Qemu-devel] [PATCH v8 7/7] block: Enable qemu_open/close to work with fd sets
On 08/10/2012 12:56 PM, Corey Bryant wrote:
On 08/10/2012 12:34 PM, Kevin Wolf wrote:
Am 10.08.2012 04:10, schrieb Corey Bryant:
When qemu_open is passed a filename of the "/dev/fdset/nnn"
format (where nnn is the fdset ID), an fd with matching access
mode flags will be searched for within the specified monitor
fd set. If the fd is found, a dup of the fd will be returned
from qemu_open.
Each fd set has a reference count. The purpose of the reference
count is to determine if an fd set contains file descriptors that
have open dup() references that have not yet been closed. It is
incremented on qemu_open and decremented on qemu_close. It is
not until the refcount is zero that file desriptors in an fd set
can be closed. If an fd set has dup() references open, then we
must keep the other fds in the fd set open in case a reopen
of the file occurs that requires an fd with a different access
mode.
Signed-off-by: Corey Bryant
@@ -78,6 +79,69 @@ int qemu_madvise(void *addr, size_t len, int advice)
#endif
}
+/*
+ * Dups an fd and sets the flags
+ */
+static int qemu_dup(int fd, int flags)
qemu_dup() is probably not a good name. We'll want to use it when we
need to get a wrapper around dup(). And I suspect that we will need it
for making bdrv_reopen() compatible with fdset refcounting.
Do you you have a suggestion for a name?
+{
+int ret;
+int serrno;
+int dup_flags;
+int setfl_flags;
+
+if (flags & O_CLOEXEC) {
+#ifdef F_DUPFD_CLOEXEC
+ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
+#else
+ret = dup(fd);
+if (ret != -1) {
+qemu_set_cloexec(ret);
+}
+#endif
+} else {
+ret = dup(fd);
+}
qemu_open() is supposed to add O_CLOEXEC by itself, so I think we should
execute the then branch unconditionally (or we would have to change the
qemu_dup() call below to add it - but the fact that O_CLOEXEC isn't even
necessarily defined doesn't help with that).
Sure I can modify this to always add O_CLOEXEC. (I know you mentioned
this before but I think I interpreted the comment as a question to others.)
Do you also want me to modify qemu_open to always add O_CLOEXEC?
My mistake.. it's always set in qemu_open already.
--
Regards,
Corey
[Qemu-devel] [PATCH 2/2] cpu: for cpu-user and cpu-softmmu and make cpu-softmmu a child of DeviceState
The line between linux-user and softmmu is not very well defined right now. linux-user really don't want to include devices and making CpuState a child of DeviceState would require pulling lots of stuff into linux-user. To solve this, we simply fork cpu-user and cpu-softmmu letting them evolve independently. We also need to push a qdev_init_nofail() into all callers of cpu_init(). This is needed eventually anyway so might as well do this now. Signed-off-by: Anthony Liguori --- Makefile.objs |6 --- hw/armv7m.c |1 + hw/axis_dev88.c |1 + hw/exynos4210.c |1 + hw/highbank.c |1 + hw/integratorcp.c |1 + hw/leon3.c|1 + hw/lm32_boards.c |2 + hw/milkymist.c|1 + hw/mips_fulong2e.c|1 + hw/mips_jazz.c|1 + hw/mips_malta.c |1 + hw/mips_mipssim.c |1 + hw/mips_r4k.c |1 + hw/musicpal.c |1 + hw/omap1.c|1 + hw/omap2.c|1 + hw/pc.c |1 + hw/petalogix_ml605_mmu.c |1 + hw/petalogix_s3adsp1800_mmu.c |1 + hw/ppc440_bamboo.c|1 + hw/ppc4xx_devs.c |1 + hw/ppc_newworld.c |1 + hw/ppc_oldworld.c |1 + hw/ppc_prep.c |1 + hw/ppce500_mpc8544ds.c|1 + hw/pxa2xx.c |1 + hw/r2d.c |1 + hw/realview.c |1 + hw/s390-virtio.c |1 + hw/spapr.c|1 + hw/strongarm.c|1 + hw/sun4m.c|1 + hw/sun4u.c|1 + hw/versatilepb.c |1 + hw/vexpress.c |2 + hw/virtex_ml507.c |1 + hw/xen_machine_pv.c |1 + hw/xilinx_zynq.c |1 + hw/xtensa_lx60.c |1 + hw/xtensa_sim.c |1 + include/qemu/cpu-softmmu.h| 82 +++ include/qemu/cpu-user.h | 75 include/qemu/cpu.h| 85 ++--- qemu-common.h |3 +- qom/Makefile.objs |5 +- qom/cpu-softmmu.c | 66 +++ qom/cpu-user.c| 70 + 48 files changed, 343 insertions(+), 91 deletions(-) create mode 100644 include/qemu/cpu-softmmu.h create mode 100644 include/qemu/cpu-user.h create mode 100644 qom/cpu-softmmu.c create mode 100644 qom/cpu-user.c diff --git a/Makefile.objs b/Makefile.objs index 5ebbcfa..f285926 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -12,12 +12,6 @@ qobject-obj-y += qerror.o error.o qemu-error.o universal-obj-y += $(qobject-obj-y) ### -# QOM -qom-obj-y = qom/ - -universal-obj-y += $(qom-obj-y) - -### # oslib-obj-y is code depending on the OS (win32 vs posix) oslib-obj-y = osdep.o oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o diff --git a/hw/armv7m.c b/hw/armv7m.c index 8cec78d..2e8fa06 100644 --- a/hw/armv7m.c +++ b/hw/armv7m.c @@ -188,6 +188,7 @@ qemu_irq *armv7m_init(MemoryRegion *address_space_mem, fprintf(stderr, "Unable to find CPU definition\n"); exit(1); } +qdev_init_nofail(DEVICE(cpu)); env = &cpu->env; #if 0 diff --git a/hw/axis_dev88.c b/hw/axis_dev88.c index eab6327..f34fe8a 100644 --- a/hw/axis_dev88.c +++ b/hw/axis_dev88.c @@ -265,6 +265,7 @@ void axisdev88_init (ram_addr_t ram_size, cpu_model = "crisv32"; } cpu = cpu_cris_init(cpu_model); +qdev_init_nofail(DEVICE(cpu)); env = &cpu->env; /* allocate RAM */ diff --git a/hw/exynos4210.c b/hw/exynos4210.c index 00d4db8..aca8738 100644 --- a/hw/exynos4210.c +++ b/hw/exynos4210.c @@ -122,6 +122,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem, fprintf(stderr, "Unable to find CPU %d definition\n", n); exit(1); } +qdev_init_nofail(DEVICE(s->cpu[n])); /* Create PIC controller for each processor instance */ irqp = arm_pic_init_cpu(s->cpu[n]); diff --git a/hw/highbank.c b/hw/highbank.c index 11aa131..8f66f70 100644 --- a/hw/highbank.c +++ b/hw/highbank.c @@ -214,6 +214,7 @@ static void highbank_init(ram_addr_t ram_size, fprintf(stderr, "Unable to find CPU definition\n"); exit(1); } +qdev_init_nofail(DEVICE(cpu)); /* This will become a QOM property eventually */ cpu->reset_cbar = GIC_BASE_ADDR; diff --git a/hw/integratorcp.c b/hw/integratorcp.c index d0e2e9
[Qemu-devel] [PATCH 1/2] qdev: split up header so it can be used in cpu.h
Header file dependency is a frickin' nightmare right now. cpu.h tends to get
included in our 'include everything' header files but qdev also needs to include
those headers mainly for qdev-properties since it knows about CharDriverState
and friends.
We can solve this for now by splitting out qdev.h along the same lines that we
previously split the C file. Then cpu.h just needs to include qdev-core.h
Signed-off-by: Anthony Liguori
Signed-off-by: Anthony Liguori
---
hw/irq.h |2 +
hw/mc146818rtc.c |1 +
hw/qdev-addr.c |1 +
hw/qdev-core.h | 240
hw/qdev-monitor.h| 16 ++
hw/qdev-properties.c |1 +
hw/qdev-properties.h | 128 +
hw/qdev.c|1 +
hw/qdev.h| 371 +-
9 files changed, 394 insertions(+), 367 deletions(-)
create mode 100644 hw/qdev-core.h
create mode 100644 hw/qdev-monitor.h
create mode 100644 hw/qdev-properties.h
diff --git a/hw/irq.h b/hw/irq.h
index 56c55f0..1339a3a 100644
--- a/hw/irq.h
+++ b/hw/irq.h
@@ -3,6 +3,8 @@
/* Generic IRQ/GPIO pin infrastructure. */
+typedef struct IRQState *qemu_irq;
+
typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
void qemu_set_irq(qemu_irq irq, int level);
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 3777f85..3780617 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -25,6 +25,7 @@
#include "qemu-timer.h"
#include "sysemu.h"
#include "mc146818rtc.h"
+#include "qapi/qapi-visit-core.h"
#ifdef TARGET_I386
#include "apic.h"
diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index b711b6b..5b5d38f 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -1,6 +1,7 @@
#include "qdev.h"
#include "qdev-addr.h"
#include "targphys.h"
+#include "qapi/qapi-visit-core.h"
/* --- target physical address --- */
diff --git a/hw/qdev-core.h b/hw/qdev-core.h
new file mode 100644
index 000..ca205fc
--- /dev/null
+++ b/hw/qdev-core.h
@@ -0,0 +1,240 @@
+#ifndef QDEV_CORE_H
+#define QDEV_CORE_H
+
+#include "qemu-queue.h"
+#include "qemu-option.h"
+#include "qemu/object.h"
+#include "hw/irq.h"
+#include "error.h"
+
+typedef struct Property Property;
+
+typedef struct PropertyInfo PropertyInfo;
+
+typedef struct CompatProperty CompatProperty;
+
+typedef struct BusState BusState;
+
+typedef struct BusClass BusClass;
+
+enum DevState {
+DEV_STATE_CREATED = 1,
+DEV_STATE_INITIALIZED,
+};
+
+enum {
+DEV_NVECTORS_UNSPECIFIED = -1,
+};
+
+#define TYPE_DEVICE "device"
+#define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)
+#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
+#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
+
+typedef int (*qdev_initfn)(DeviceState *dev);
+typedef int (*qdev_event)(DeviceState *dev);
+typedef void (*qdev_resetfn)(DeviceState *dev);
+
+struct VMStateDescription;
+
+typedef struct DeviceClass {
+ObjectClass parent_class;
+
+const char *fw_name;
+const char *desc;
+Property *props;
+int no_user;
+
+/* callbacks */
+void (*reset)(DeviceState *dev);
+
+/* device state */
+const struct VMStateDescription *vmsd;
+
+/* Private to qdev / bus. */
+qdev_initfn init;
+qdev_event unplug;
+qdev_event exit;
+const char *bus_type;
+} DeviceClass;
+
+/* This structure should not be accessed directly. We declare it here
+ so that it can be embedded in individual device state structures. */
+struct DeviceState {
+Object parent_obj;
+
+const char *id;
+enum DevState state;
+struct QemuOpts *opts;
+int hotplugged;
+BusState *parent_bus;
+int num_gpio_out;
+qemu_irq *gpio_out;
+int num_gpio_in;
+qemu_irq *gpio_in;
+QLIST_HEAD(, BusState) child_bus;
+int num_child_bus;
+int instance_id_alias;
+int alias_required_for_version;
+};
+
+/*
+ * This callback is used to create Open Firmware device path in accordance with
+ * OF spec http://forthworks.com/standards/of1275.pdf. Indicidual bus bindings
+ * can be found here http://playground.sun.com/1275/bindings/.
+ */
+
+#define TYPE_BUS "bus"
+#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
+#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
+#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS)
+
+struct BusClass {
+ObjectClass parent_class;
+
+/* FIXME first arg should be BusState */
+void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
+char *(*get_dev_path)(DeviceState *dev);
+char *(*get_fw_dev_path)(DeviceState *dev);
+int (*reset)(BusState *bus);
+};
+
+typedef struct BusChild {
+DeviceState *child;
+int index;
+QTAILQ_ENTRY(BusChild) sibling;
+} BusChild;
+
+/**
+ * BusState:
+ * @qom_allocated: Indicates whether the object was allocated by QOM.
+ * @glib_allocated: Indicates whether the object was initialized in
[Qemu-devel] [PATCH 0/2] cpu: make a child of DeviceState
This is just a rebase of this series that I posted back in June. Andreas had a different approach involving a #define but I think doing a full split ends up being nicer. For instance, the recently added thread information is only relevant to cpu-softmmu so we can limit that entirely to cpu-softmmu.
[Qemu-devel] [PATCH 03/11] ahci: Fix sglist memleak in ahci_dma_rw_buf()
From: Jason Baron
I noticed that in hw/ide/ahci:ahci_dma_rw_buf() we do not free the sglist. Thus,
I've added a call to qemu_sglist_destroy() to fix this memory leak.
In addition, I've adeed a call in qemu_sglist_destroy() to 0 all of the sglist
fields, in case there is some other codepath that tries to free the sglist.
Signed-off-by: Jason Baron
Signed-off-by: Kevin Wolf
---
dma-helpers.c |1 +
hw/ide/ahci.c |3 +++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/dma-helpers.c b/dma-helpers.c
index 35cb500..13593d1 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -65,6 +65,7 @@ void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base,
dma_addr_t len)
void qemu_sglist_destroy(QEMUSGList *qsg)
{
g_free(qsg->sg);
+memset(qsg, 0, sizeof(*qsg));
}
typedef struct {
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index de580a6..5ea3cad 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1073,6 +1073,9 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_write)
dma_buf_write(p, l, &s->sg);
}
+/* free sglist that was created in ahci_populate_sglist() */
+qemu_sglist_destroy(&s->sg);
+
/* update number of transferred bytes */
ad->cur_cmd->status = cpu_to_le32(le32_to_cpu(ad->cur_cmd->status) + l);
s->io_buffer_index += l;
--
1.7.6.5
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
On Fri, 10 Aug 2012 18:35:26 +0200
Markus Armbruster wrote:
> Luiz Capitulino writes:
>
> > On Fri, 10 Aug 2012 09:56:11 +0200
> > Markus Armbruster wrote:
> >
> >> Revisited this one on review of v2, replying here for context.
> >>
> >> Luiz Capitulino writes:
> >>
> >> > On Thu, 02 Aug 2012 13:35:54 +0200
> >> > Markus Armbruster wrote:
> >> >
> >> >> Luiz Capitulino writes:
> >> >>
> >> >> > Signed-off-by: Luiz Capitulino
> >> >> > ---
> >> >> > block.c | 1 +
> >> >> > qapi-schema.json | 7 +--
> >> >> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >> >> >
> >> >> > diff --git a/block.c b/block.c
> >> >> > index b38940b..9c113b8 100644
> >> >> > --- a/block.c
> >> >> > +++ b/block.c
> >> >> > @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
> >> >> > info->value->inserted->ro = bs->read_only;
> >> >> > info->value->inserted->drv =
> >> >> > g_strdup(bs->drv->format_name);
> >> >> > info->value->inserted->encrypted = bs->encrypted;
> >> >> > +info->value->inserted->valid_encryption_key =
> >> >> > bs->valid_key;
> >> >> > if (bs->backing_file[0]) {
> >> >> > info->value->inserted->has_backing_file = true;
> >> >> > info->value->inserted->backing_file =
> >> >> > g_strdup(bs->backing_file);
> >> >> > diff --git a/qapi-schema.json b/qapi-schema.json
> >> >> > index bc55ed2..1b2d7f5 100644
> >> >> > --- a/qapi-schema.json
> >> >> > +++ b/qapi-schema.json
> >> >> > @@ -400,6 +400,8 @@
> >> >> > #
> >> >> > # @encrypted: true if the backing device is encrypted
> >> >> > #
> >> >> > +# @valid_encryption_key: true if a valid encryption key has been set
> >> >> > +#
> >> >> > # @bps: total throughput limit in bytes per second is specified
> >> >> > #
> >> >> > # @bps_rd: read throughput limit in bytes per second is specified
> >> >> > @@ -419,8 +421,9 @@
> >> >> > { 'type': 'BlockDeviceInfo',
> >> >> >'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
> >> >> > '*backing_file': 'str', 'encrypted': 'bool',
> >> >> > -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> >> >> > -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >> > +'valid_encryption_key': 'bool', 'bps': 'int',
> >> >> > +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
> >> >> > +'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >> >
> >> >> > ##
> >> >> > # @BlockDeviceIoStatus:
> >> >>
> >> >> BlockDeviceInfo is API, isn't it?
> >> >
> >> > Yes.
> >> >
> >> >> Note that bs->valid_key currently implies bs->encrypted. bs->valid_key
> >> >> && !bs->encrypted is impossible. Should we make valid_encryption_key
> >> >> only available when encrypted?
> >> >
> >> > I don't think so. It's a bool, so it's ok for it to be false when
> >> > encrypted is false.
> >>
> >> What bothers me is encrypted=false, valid_encryption_key=true.
> >
> > Disappearing keys is worse, IMHO (assuming that that situation is impossible
> > in practice, of course).
>
> It's fundamentally three states: unencrypted, encrypted-no-key,
> encrypted-got-key. I'm fine with mapping these onto two bools, it's how
> the block layer does it. You may want to consider a single enumeration
> instead.
That's arguable. But I like the bools slightly better because they allow
clients to do a true/false check vs. having to check against an enum value.
Again, that's arguable.
> >> >> valid_encryption_key is a bit long for my taste. Yours may be
> >> >> different.
> >> >
> >> > We should choose more descriptive and self-documenting names for the
> >> > protocol. Besides, I can't think of anything shorter that won't get
> >> > cryptic.
> >> >
> >> > Suggestions are always welcome though :)
> >>
> >> valid_encryption_key sounds like the value is the valid key.
> >
> > That's exactly what it is.
>
> Err, isn't the value bool? The key value is a string...
Ah, sorry, I read "sounds like true means the key is valid even for an
invalid key". I've renamed it to encryption_key_missing, should be better
(although I could also do encryption_key_is_missing).
> >> got_crypt_key? Also avoids "valid". Good, because current encrypted
> >> formats don't actually validate the key; they happily accept any key.
> >
> > That's a block layer bug, not QMP's.
> >
> > QMP clients are going to be misguided by valid_encryption_key the same way
> > they are with the block_passwd command or how we suffer from it internally
> > when calling bdrv_set_key() (which also manifests itself in HMP).
> >
> > Fixing the bug where it is will automatically fix all its instances.
>
> It's not fixable for existing image formats, and thus existing images.
Why not? I'd expect that changing AES_set_decrypt_key() to fail for an
invalid key wouldn't affect images, am I wrong?
> You could even call it a feature that makes it (marginally) harder to
> brute-force keys (I don't buy that argum
Re: [Qemu-devel] [PATCH v8 0/7] file descriptor passing using fd sets
On 08/10/2012 12:36 PM, Kevin Wolf wrote: Am 10.08.2012 04:10, schrieb Corey Bryant: libvirt's sVirt security driver provides SELinux MAC isolation for Qemu guest processes and their corresponding image files. In other words, sVirt uses SELinux to prevent a QEMU process from opening files that do not belong to it. sVirt provides this support by labeling guests and resources with security labels that are stored in file system extended attributes. Some file systems, such as NFS, do not support the extended attribute security namespace, and therefore cannot support sVirt isolation. A solution to this problem is to provide fd passing support, where libvirt opens files and passes file descriptors to QEMU. This, along with SELinux policy to prevent QEMU from opening files, can provide image file isolation for NFS files stored on the same NFS mount. This patch series adds the add-fd, remove-fd, and query-fdsets QMP monitor commands, which allow file descriptors to be passed via SCM_RIGHTS, and assigned to specified fd sets. This allows fd sets to be created per file with fds having, for example, different access rights. When QEMU needs to reopen a file with different access rights, it can search for a matching fd in the fd set. Fd sets also allow for easy tracking of fds per file, helping to prevent fd leaks. Support is also added to the block layer to allow QEMU to dup an fd from an fdset when the filename is of the /dev/fdset/nnn format, where nnn is the fd set ID. No new SELinux policy is required to prevent open of NFS files (files with type nfs_t). The virt_use_nfs boolean type simply needs to be set to false, and open will be prevented (and dup will be allowed). For example: # setsebool virt_use_nfs 0 # getsebool virt_use_nfs virt_use_nfs --> off Corey Bryant (7): qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg qapi: Introduce add-fd, remove-fd, query-fdsets monitor: Clean up fd sets on monitor disconnect block: Prevent detection of /dev/fdset/ as floppy block: Convert open calls to qemu_open block: Convert close calls to qemu_close block: Enable qemu_open/close to work with fd sets block/raw-posix.c | 46 + block/raw-win32.c |6 +- block/vdi.c |5 +- block/vmdk.c | 25 ++--- block/vpc.c |4 +- block/vvfat.c | 16 +-- cutils.c |5 + monitor.c | 294 + monitor.h |5 + osdep.c | 117 + qapi-schema.json | 98 ++ qemu-char.c | 12 ++- qemu-common.h |2 + qemu-tool.c | 20 qmp-commands.hx | 117 + savevm.c |4 +- 16 files changed, 721 insertions(+), 55 deletions(-) Apart from the few comments I made, I like this series. Maybe v9 will be the last one. :-) Thanks, I hope so too! -- Regards, Corey
Re: [Qemu-devel] [PATCH v8 7/7] block: Enable qemu_open/close to work with fd sets
On 08/10/2012 12:34 PM, Kevin Wolf wrote:
Am 10.08.2012 04:10, schrieb Corey Bryant:
When qemu_open is passed a filename of the "/dev/fdset/nnn"
format (where nnn is the fdset ID), an fd with matching access
mode flags will be searched for within the specified monitor
fd set. If the fd is found, a dup of the fd will be returned
from qemu_open.
Each fd set has a reference count. The purpose of the reference
count is to determine if an fd set contains file descriptors that
have open dup() references that have not yet been closed. It is
incremented on qemu_open and decremented on qemu_close. It is
not until the refcount is zero that file desriptors in an fd set
can be closed. If an fd set has dup() references open, then we
must keep the other fds in the fd set open in case a reopen
of the file occurs that requires an fd with a different access
mode.
Signed-off-by: Corey Bryant
@@ -78,6 +79,69 @@ int qemu_madvise(void *addr, size_t len, int advice)
#endif
}
+/*
+ * Dups an fd and sets the flags
+ */
+static int qemu_dup(int fd, int flags)
qemu_dup() is probably not a good name. We'll want to use it when we
need to get a wrapper around dup(). And I suspect that we will need it
for making bdrv_reopen() compatible with fdset refcounting.
Do you you have a suggestion for a name?
+{
+int ret;
+int serrno;
+int dup_flags;
+int setfl_flags;
+
+if (flags & O_CLOEXEC) {
+#ifdef F_DUPFD_CLOEXEC
+ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
+#else
+ret = dup(fd);
+if (ret != -1) {
+qemu_set_cloexec(ret);
+}
+#endif
+} else {
+ret = dup(fd);
+}
qemu_open() is supposed to add O_CLOEXEC by itself, so I think we should
execute the then branch unconditionally (or we would have to change the
qemu_dup() call below to add it - but the fact that O_CLOEXEC isn't even
necessarily defined doesn't help with that).
Sure I can modify this to always add O_CLOEXEC. (I know you mentioned
this before but I think I interpreted the comment as a question to others.)
Do you also want me to modify qemu_open to always add O_CLOEXEC?
--
Regards,
Corey
[Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need (v2)
This series implements the necessary commands to implements danpb's idea to
remove -help parsing in libvirt. We would introduce all of these commands in
1.2 and then change the -help output starting in 1.3.
Here is Dan's plan from a previous thread:
Basically I'd sum up my new idea as "just use QMP".
* No new command line arguments like -capabilities
* libvirt invokes something like
$QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics
* libvirt then runs a number of QMP commands to find out
what it needs to know. I'd expect the following existing
commands would be used
- query-version - already supported
- query-commands- already supported
- query-events - already supported
- query-kvm - already supported
- qom-{list,list-types,get} - already supported
- query-spice/vnc - already supported
And add the following new commands
- query-devices - new, -device ?, and/or -device NAME,? data
in QMP
- query-machines- new, -M ? in QMP
- query-cpu-types - new, -cpu ? in QMP
The above would take care of probably 50% of the current libvirt
capabilities probing, including a portion of the -help stuff. Then
there is all the rest of the crap we detect from the -help. We could
just take the view, that "as of 1.2", we assume everything we previously
detected is just available by default, and thus don't need to probe
it. For stuff that is QOM based, I expect we'll be able to detect new
features in the future using the qom-XXX monitor commands. For stuff
that is non-qdev, and non-qom, libvirt can just do a plain version
number check, unless we decide there is specific info worth exposing
via other new 'query-XXX' monitor commands.
Basically I'd sum up my new idea as "just use QMP".
* No new command line arguments like -capabilities
* libvirt invokes something like
$QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics
* libvirt then runs a number of QMP commands to find out
what it needs to know. I'd expect the following existing
commands would be used
- query-version - already supported
- query-commands- already supported
- query-events - already supported
- query-kvm - already supported
- qom-{list,list-types,get} - already supported
- query-spice/vnc - already supported
And add the following new commands
- query-devices - new, -device ?, and/or -device NAME,? data
in QMP
- query-machines- new, -M ? in QMP
- query-cpu-types - new, -cpu ? in QMP
The above would take care of probably 50% of the current libvirt
capabilities probing, including a portion of the -help stuff. Then
there is all the rest of the crap we detect from the -help. We could
just take the view, that "as of 1.2", we assume everything we previously
detected is just available by default, and thus don't need to probe
it. For stuff that is QOM based, I expect we'll be able to detect new
features in the future using the qom-XXX monitor commands. For stuff
that is non-qdev, and non-qom, libvirt can just do a plain version
number check, unless we decide there is specific info worth exposing
via other new 'query-XXX' monitor commands.
The one thing to note is that I didn't add a query-devices command because you
can already do:
qmp query-devices --implements=device --abstract=False
To get the equivalent output of -device ?. Instead, I added a command to list
specific properties of a device which is the equivalent of -device FOO,?
---
v1 -> v2
- rename query-cpudefs -> query-cpu-definitions
[Qemu-devel] [PATCH 2/7] qapi: mark QOM commands stable
We've had a cycle to tweak. It is time to commit to supporting them.
Signed-off-by: Anthony Liguori
---
qapi-schema.json | 19 ---
1 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 191a889..a938c8d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1363,9 +1363,7 @@
#4) A link type in the form 'link' where subtype is a qdev
# device type name. Link properties form the device model graph.
#
-# Since: 1.1
-#
-# Notes: This type is experimental. Its syntax may change in future releases.
+# Since: 1.2
##
{ 'type': 'ObjectPropertyInfo',
'data': { 'name': 'str', 'type': 'str' } }
@@ -1382,10 +1380,7 @@
# Returns: a list of @ObjectPropertyInfo that describe the properties of the
# object.
#
-# Since: 1.1
-#
-# Notes: This command is experimental. It's syntax may change in future
-#releases.
+# Since: 1.2
##
{ 'command': 'qom-list',
'data': { 'path': 'str' },
@@ -1421,9 +1416,7 @@
# returns as #str pathnames. All integer property types (u8, u16,
etc)
# are returned as #int.
#
-# Since: 1.1
-#
-# Notes: This command is experimental and may change syntax in future releases.
+# Since: 1.2
##
{ 'command': 'qom-get',
'data': { 'path': 'str', 'property': 'str' },
@@ -1442,9 +1435,7 @@
# @value: a value who's type is appropriate for the property type. See
@qom-get
# for a description of type mapping.
#
-# Since: 1.1
-#
-# Notes: This command is experimental and may change syntax in future releases.
+# Since: 1.2
##
{ 'command': 'qom-set',
'data': { 'path': 'str', 'property': 'str', 'value': 'visitor' },
@@ -1721,8 +1712,6 @@
# Returns: a list of @ObjectTypeInfo or an empty list if no results are found
#
# Since: 1.1
-#
-# Notes: This command is experimental and may change syntax in future releases.
##
{ 'command': 'qom-list-types',
'data': { '*implements': 'str', '*abstract': 'bool' },
--
1.7.5.4
[Qemu-devel] [PATCH 07/11] blockdev: flip default cache mode from writethrough to writeback
From: Paolo Bonzini
Now all major device models (IDE, SCSI, virtio) can choose between
writethrough and writeback at run-time, and virtio will even revert
to writethrough if the guest is not capable of sending flushes. So
we can change the default to writeback at last.
Tested, for lack of a better idea, with a breakpoint on bdrv_open
and all cache choices one by one.
Signed-off-by: Paolo Bonzini
Signed-off-by: Kevin Wolf
---
blockdev.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 8669142..7c83baa 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -377,6 +377,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
}
}
+bdrv_flags |= BDRV_O_CACHE_WB;
if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
if (bdrv_parse_cache_flags(buf, &bdrv_flags) != 0) {
error_report("invalid cache option");
--
1.7.6.5
Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs
On Fri, Aug 10, 2012 at 11:37:30AM -0500, Anthony Liguori wrote:
> Eduardo Habkost writes:
>
> > On Fri, Aug 10, 2012 at 09:43:21AM -0500, Anthony Liguori wrote:
> >> Eduardo Habkost writes:
> >>
> >> > On Fri, Jul 27, 2012 at 08:37:18AM -0500, Anthony Liguori wrote:
> >> >> Signed-off-by: Anthony Liguori
> >> >> ---
> >> >> target-i386/cpu.c | 22 ++
> >> >> 1 files changed, 22 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >> >> index 6b9659f..b398439 100644
> >> >> --- a/target-i386/cpu.c
> >> >> +++ b/target-i386/cpu.c
> >> >> @@ -28,6 +28,7 @@
> >> >> #include "qemu-config.h"
> >> >>
> >> >> #include "qapi/qapi-visit-core.h"
> >> >> +#include "qmp-commands.h"
> >> >>
> >> >> #include "hyperv.h"
> >> >>
> >> >> @@ -1123,6 +1124,27 @@ void x86_cpu_list(FILE *f, fprintf_function
> >> >> cpu_fprintf, const char *optarg)
> >> >> }
> >> >> }
> >> >>
> >> >> +CpuDefInfoList *qmp_query_cpudefs(Error **errp)
> >> >> +{
> >> >> +CpuDefInfoList *cpu_list = NULL;
> >> >> +x86_def_t *def;
> >> >> +
> >> >> +for (def = x86_defs; def; def = def->next) {
> >> >> +CpuDefInfoList *entry;
> >> >> +CpuDefInfo *info;
> >> >> +
> >> >> +info = g_malloc0(sizeof(*info));
> >> >> +info->name = g_strdup(def->name);
> >> >> +
> >> >> +entry = g_malloc0(sizeof(*entry));
> >> >> +entry->value = info;
> >> >> +entry->next = cpu_list;
> >> >> +cpu_list = entry;
> >> >> +}
> >> >> +
> >> >> +return cpu_list;
> >> >> +}
> >> >
> >> > How would the interface look like once we:
> >> > - let libvirt know which features are available on each CPU model
> >> > (libvirt needs that information[1]); and
> >>
> >> I'm not sure I understand why libvirt needs this information. Can you
> >> elaborate?
> >
> > I see two reasons:
> >
> > - The libvirt API has functions to tell the user which features are
> > going to be enabled for each CPU model, so it needs to know which
> > features are enabled or not, for each machine-type + cpu-model
> > combination, so this information can be reported proeprly.
>
> Ok, step number one is that CPU 'features' need to be defined more
> formally. By formally, I mean via qapi-schema.json.
>
> Then we can extend this command to return the set of features supported
> by each CPU type.
>
> The first step will need to sort out how this maps across architectures.
>
> > - Also, if libvirt can enable/disable specific CPU features in the
> > command-line, it just makes sens to know which ones are already
> > enabled in each built-in CPU model.
> >
> > - Probing for migration: libvirt needs to know if a given CPU model on a
> > host can be migrated to another host. To know that, two pieces of
> > information are needed:
> > A) Which CPU features are visible to the guest for a specific
> > configuration;
> > B) Which of those features are really supported by the host
> > hardware+kernel+QEMU, on the destination host, so it can
> > know if migration is really possible.
>
> Note that what QEMU thinks it exposes is not necessarily what gets
> exposed. KVM may mask additional features. How is this handled today?
No, what QEMU thinks it exposes actually is what gets exposed (and if it
is not, it's a bug we have to fix). This is handled using the KVM
GET_SUPPORTED_CPUID ioctl().
>
> >> > - add machine-type-specific cpudef compatibility changes?
> >>
> >> I think we've discussed this in IRC. I don't think we need to worry
> >> about this.
> >
> > I remember discussing a lot about the mechanism we will use to add the
> > compatibility changes, but I don t know how the query API will look
> > like, after we implement this mechanism.
>
> 0) User-defined CPU definitions go away
>- We already made a big step in this direction
>
> 1) CPU becomes a DeviceState
1.1) CPU models become classes
>
> 2) Features are expressed as properties
>
> 3) Same global mechanism used for everything else is used for CPUs
This is basically the compatibility mechanism we agreed upon, yes, but
what about the probing mechanism to allow libvirt to know what will be
the result of "-machine M -cpu C"[1] before actually starting a VM?
[1] By "result" I mean:
- Whether that combination can be run properly on that host;
- Which CPU features will be visible to the guest in case it runs.
Both items depend on CPU model _and_ machine-type, that's why we need
some probing mechanism that depends on the machine-type or use the
machine-type as input.
>
> Regards,
>
> Anthony Liguori
>
> >> > Would the command report different results depending on -machine?
> >>
> >> No.
> >
> > The problem is:
> >
> > 1) We need to introduce fixes on a CPU model that changes the set of
> >guest-visible features (add or remove a feature)[1];
> > 2) The fix has to keep compatibility, so older machine-types will
> >
[Qemu-devel] [PATCH 05/11] virtio-blk: support VIRTIO_BLK_F_CONFIG_WCE
From: Paolo Bonzini
Also rename VIRTIO_BLK_F_WCACHE to VIRTIO_BLK_F_WCE for consistency with
the spec.
Signed-off-by: Paolo Bonzini
Signed-off-by: Kevin Wolf
---
hw/virtio-blk.c | 16 ++--
hw/virtio-blk.h |4 +++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 552b3b6..97bb4bd 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -510,9 +510,19 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
uint8_t *config)
blkcfg.size_max = 0;
blkcfg.physical_block_exp = get_physical_block_exp(s->conf);
blkcfg.alignment_offset = 0;
+blkcfg.wce = bdrv_enable_write_cache(s->bs);
memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
}
+static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+VirtIOBlock *s = to_virtio_blk(vdev);
+struct virtio_blk_config blkcfg;
+
+memcpy(&blkcfg, config, sizeof(blkcfg));
+bdrv_set_enable_write_cache(s->bs, blkcfg.wce != 0);
+}
+
static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
{
VirtIOBlock *s = to_virtio_blk(vdev);
@@ -523,9 +533,10 @@ static uint32_t virtio_blk_get_features(VirtIODevice
*vdev, uint32_t features)
features |= (1 << VIRTIO_BLK_F_BLK_SIZE);
features |= (1 << VIRTIO_BLK_F_SCSI);
+features |= (1 << VIRTIO_BLK_F_CONFIG_WCE);
if (bdrv_enable_write_cache(s->bs))
-features |= (1 << VIRTIO_BLK_F_WCACHE);
-
+features |= (1 << VIRTIO_BLK_F_WCE);
+
if (bdrv_is_read_only(s->bs))
features |= 1 << VIRTIO_BLK_F_RO;
@@ -610,6 +621,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
VirtIOBlkConf *blk)
sizeof(VirtIOBlock));
s->vdev.get_config = virtio_blk_update_config;
+s->vdev.set_config = virtio_blk_set_config;
s->vdev.get_features = virtio_blk_get_features;
s->vdev.reset = virtio_blk_reset;
s->bs = blk->conf.bs;
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 79ebccc..35834cf 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -31,8 +31,9 @@
#define VIRTIO_BLK_F_BLK_SIZE 6 /* Block size of disk is available*/
#define VIRTIO_BLK_F_SCSI 7 /* Supports scsi command passthru */
/* #define VIRTIO_BLK_F_IDENTIFY 8 ATA IDENTIFY supported, DEPRECATED
*/
-#define VIRTIO_BLK_F_WCACHE 9 /* write cache enabled */
+#define VIRTIO_BLK_F_WCE9 /* write cache enabled */
#define VIRTIO_BLK_F_TOPOLOGY 10 /* Topology information is available */
+#define VIRTIO_BLK_F_CONFIG_WCE 11 /* write cache configurable */
#define VIRTIO_BLK_ID_BYTES 20 /* ID string length */
@@ -49,6 +50,7 @@ struct virtio_blk_config
uint8_t alignment_offset;
uint16_t min_io_size;
uint32_t opt_io_size;
+uint8_t wce;
} QEMU_PACKED;
/* These two define direction. */
--
1.7.6.5
Re: [Qemu-devel] ARM patches for QEMU 1.2: final call
On 08/10/2012 08:21 PM, Peter Maydell wrote: Last call for any ARM related patches to go into 1.2. My current queue looks like this: 59cbd70 hw/sd.c: make sd_wp_addr() return bool 8b4cc14 hw/sd.c: make sd_dataready() return bool 025caa6 hw/sd.c: convert binary variables to bool 38d24e6 hw/sd.c: introduce wrapper for conversion address to wp group 1835455 hw/sd.c: make sd_wp_addr() accept 64 bit address argument 34f99a8 hw/sd.c: convert wp_groups in SDState to bitfield 0b7ede9 armv7m: Guard against no -kernel argument 62140f8 hw/armv7m_nvic: Fix incorrect default for num-irqs property http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/arm-devs.next and I plan to send a pullreq Monday. (My target-arm queue is currently empty.) -- PMM That would be nice to also pull "Exynos4210: license and RAM vmstate fixes" patchset I sent long time ago. I've just resent it to list: http://comments.gmane.org/gmane.comp.emulators.qemu/164752
