Re: [Qemu-devel] [PATCH] pc: q35: disable CPU hotplug handler on machines less than 2.0
On Tue, May 06, 2014 at 03:09:04PM +0200, Igor Mammedov wrote:
> CPU hotplug handling for Q35 machine types was added
> only since 2.0 but commit wrongly c649983b58 added
> CPU hotplug handler to Q35 1.5 machine without
> adding ACPI support for it. As result user can
> create VCPU but guest couldn't use it since it
> hasn't any data on it (missing ACPI implementation).
>
> Signed-off-by: Igor Mammedov
> ---
> hw/i386/pc_q35.c |8
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index c844dc2..687a397 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -308,7 +308,9 @@ static QEMUMachine pc_q35_machine_v2_0 = {
> .init = pc_q35_init,
> };
>
> -#define PC_Q35_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
> +#define PC_Q35_1_7_MACHINE_OPTIONS \
> +PC_Q35_MACHINE_OPTIONS, \
> +.hot_add_cpu = NULL
>
> static QEMUMachine pc_q35_machine_v1_7 = {
> PC_Q35_1_7_MACHINE_OPTIONS,
> @@ -342,9 +344,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
> },
> };
>
> -#define PC_Q35_1_4_MACHINE_OPTIONS \
> -PC_Q35_1_6_MACHINE_OPTIONS, \
> -.hot_add_cpu = NULL
> +#define PC_Q35_1_4_MACHINE_OPTIONS PC_Q35_1_6_MACHINE_OPTIONS
If I'm looking at the correct code, pc-q35-1.6 still enables cpu
hotplug:
#define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
You can remove PC_Q35_1_6_MACHINE_OPTIONS and use
PC_Q35_1_7_MACHINE_OPTIONS in all versions less then 1.7.
>
> static QEMUMachine pc_q35_machine_v1_4 = {
> PC_Q35_1_4_MACHINE_OPTIONS,
> --
> 1.7.1
>
Re: [Qemu-devel] [PATCH] spapr: pci: clean msi info when releasing it
On 05/07/2014 04:51 PM, Liu Ping Fan wrote:
> In current code, we use phb->msi_table[ndev].nvec to indicate whether
> this msi entries are used by a device or not. So when unplug a pci
> device, we should reset nvec to zero.
>
> Signed-off-by: Liu Ping Fan
> ---
> hw/ppc/spapr_pci.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index cbef095..7b1dfe1 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -316,6 +316,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
> sPAPREnvironment *spapr,
> rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> return;
> }
> +phb->msi_table[ndev].nvec = 0;
> trace_spapr_pci_msi("Released MSIs", ndev, config_addr);
> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> rtas_st(rets, 1, 0);
ibm,change-msi is called with 0 to disable MSIs. If later the guest decides
to reenable MSI on the same device (rmmod + modprobe in the guest can do
that I suppose), new block will be allocated because of this patch which is
bad.
And there is no PCI hotplug for SPAPR in upstream QEMU so this patch cannot
possibly fix it :)
--
Alexey
Re: [Qemu-devel] [PATCH v3 00/12] block/json: Add JSON protocol driver
On Tue, May 06, 2014 at 07:51:10PM +0200, Max Reitz wrote: > On 06.05.2014 10:10, Stefan Hajnoczi wrote: > >On Mon, May 05, 2014 at 06:19:07PM +0200, Max Reitz wrote: > >>On 10.04.2014 20:43, Max Reitz wrote: > >>>This series adds a passthrough JSON protocol block driver. Its filenames > >>>are JSON objects prefixed by "json:". The objects are used as options > >>>for opening another block device which will be the child of the JSON > >>>device. Regarding this child device, the JSON driver behaves nearly the > >>>same as raw_bsd in that it is just a passthrough driver. The only > >>>difference is probably that the JSON driver identifies itself as a block > >>>filter, in contrast to raw_bsd. > >>> > >>>The purpose of this driver is that it may sometimes be desirable to > >>>specify options for a block device where only a filename can be given, > >>>e.g., for backing files. Using this should obviously be the exception, > >>>but it is nice to have if actually needed. > >>Ping – I do understand that Kevin has reservations against this > >>series, but as long as he doesn't explicitly ask me to reimplement > >>this in bdrv_open() without an own block driver (which I'd more or > >>less gladly do), I do not see issues why this series should not be > >>merged. > >I haven't reviewed it further because it seems like a kludge (that we > >have to keep supporting once it's merged). Was hoping you and Kevin > >will come up with a long-term fix instead. > > Okay, if you think the same, I guess I'll have to rewrite this > series. I agree that including this functionality in bdrv_open() is > the nicer alternative from the user's point of view, whereas I think > using a separate block driver results in nicer code. > > I'll rewrite this series and then we'll see how bad it actually looks. ;-) Thanks! Stefan
Re: [Qemu-devel] [PATCH 01/13] qapi: Update qapi-code-gen.txt example to match current code
Eric Blake writes: > On 05/05/2014 12:49 AM, Markus Armbruster wrote: >> Eric Blake writes: >> > Example: mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qmp-marshal.c >>> >>> The file was missing the example command line that generated this file; >>> I figured it out: >>> >>> mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-commands.py \ >>> --output-dir="qapi-generated" --prefix="example-" < >>> example-schema.json >>> >>> if you want to add that to this patch. >> >> Sure, why not, but if there's nothing else to correct in this series, >> I'd prefer a separate commit to a respin. > > Followup commit is just fine, if I don't turn up anything else while > reviewing the rest of the series. Followup commit. because my fix conflicts with a patch in Luiz's queue. I'll wait for that to land, then post my fix on top.
[Qemu-devel] [PATCH v2 03/12] qapi: Remove unused Visitor callbacks start_handle(), end_handle()
These have never been called or implemented by anything, and their
intended use is undocumented, like all of the visitor API.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
include/qapi/visitor-impl.h | 3 ---
qapi/qapi-visit-core.c | 15 ---
2 files changed, 18 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index f3fa420..166aadd 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -46,9 +46,6 @@ struct Visitor
Error **errp);
void (*end_optional)(Visitor *v, Error **errp);
-void (*start_handle)(Visitor *v, void **obj, const char *kind,
- const char *name, Error **errp);
-void (*end_handle)(Visitor *v, Error **errp);
void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error
**errp);
void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error
**errp);
void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error
**errp);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 6451a21..1f7475c 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -17,21 +17,6 @@
#include "qapi/visitor.h"
#include "qapi/visitor-impl.h"
-void visit_start_handle(Visitor *v, void **obj, const char *kind,
-const char *name, Error **errp)
-{
-if (!error_is_set(errp) && v->start_handle) {
-v->start_handle(v, obj, kind, name, errp);
-}
-}
-
-void visit_end_handle(Visitor *v, Error **errp)
-{
-if (!error_is_set(errp) && v->end_handle) {
-v->end_handle(v, errp);
-}
-}
-
void visit_start_struct(Visitor *v, void **obj, const char *kind,
const char *name, size_t size, Error **errp)
{
--
1.8.1.4
[Qemu-devel] [PATCH v2 01/12] qapi: Update qapi-code-gen.txt example to match current code
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
docs/qapi-code-gen.txt | 146 ++---
1 file changed, 90 insertions(+), 56 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index d78921f..923565e 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -223,11 +223,23 @@ Example:
mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \
--output-dir="qapi-generated" --prefix="example-" < example-schema.json
mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c
-/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
+[Uninteresting stuff omitted...]
+
+void qapi_free_UserDefOneList(UserDefOneList * obj)
+{
+QapiDeallocVisitor *md;
+Visitor *v;
+
+if (!obj) {
+return;
+}
+
+md = qapi_dealloc_visitor_new();
+v = qapi_dealloc_get_visitor(md);
+visit_type_UserDefOneList(v, &obj, NULL, NULL);
+qapi_dealloc_visitor_cleanup(md);
+}
-#include "qapi/qapi-dealloc-visitor.h"
-#include "example-qapi-types.h"
-#include "example-qapi-visit.h"
void qapi_free_UserDefOne(UserDefOne * obj)
{
@@ -245,31 +257,37 @@ Example:
}
mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.h
-/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
-#ifndef QAPI_GENERATED_EXAMPLE_QAPI_TYPES
-#define QAPI_GENERATED_EXAMPLE_QAPI_TYPES
+[Uninteresting stuff omitted...]
+
+#ifndef EXAMPLE_QAPI_TYPES_H
+#define EXAMPLE_QAPI_TYPES_H
-#include "qapi/qapi-types-core.h"
+[Builtin types omitted...]
typedef struct UserDefOne UserDefOne;
typedef struct UserDefOneList
{
-UserDefOne *value;
+union {
+UserDefOne *value;
+uint64_t padding;
+};
struct UserDefOneList *next;
} UserDefOneList;
+[Functions on builtin types omitted...]
+
struct UserDefOne
{
int64_t integer;
char * string;
};
+void qapi_free_UserDefOneList(UserDefOneList * obj);
void qapi_free_UserDefOne(UserDefOne * obj);
#endif
-
=== scripts/qapi-visit.py ===
Used to generate the visitor functions used to walk through and convert
@@ -293,39 +311,63 @@ Example:
mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-visit.py \
--output-dir="qapi-generated" --prefix="example-" < example-schema.json
mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.c
-/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+[Uninteresting stuff omitted...]
-#include "example-qapi-visit.h"
+static void visit_type_UserDefOne_fields(Visitor *m, UserDefOne ** obj,
Error **errp)
+{
+Error *err = NULL;
+visit_type_int(m, &(*obj)->integer, "integer", &err);
+visit_type_str(m, &(*obj)->string, "string", &err);
+
+error_propagate(errp, err);
+}
void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char
*name, Error **errp)
{
-visit_start_struct(m, (void **)obj, "UserDefOne", name,
sizeof(UserDefOne), errp);
-visit_type_int(m, (obj && *obj) ? &(*obj)->integer : NULL, "integer",
errp);
-visit_type_str(m, (obj && *obj) ? &(*obj)->string : NULL, "string",
errp);
-visit_end_struct(m, errp);
+if (!error_is_set(errp)) {
+Error *err = NULL;
+visit_start_struct(m, (void **)obj, "UserDefOne", name,
sizeof(UserDefOne), &err);
+if (!err) {
+if (*obj) {
+visit_type_UserDefOne_fields(m, obj, &err);
+error_propagate(errp, err);
+err = NULL;
+}
+/* Always call end_struct if start_struct succeeded. */
+visit_end_struct(m, &err);
+}
+error_propagate(errp, err);
+}
}
void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const
char *name, Error **errp)
{
GenericList *i, **prev = (GenericList **)obj;
-
-visit_start_list(m, name, errp);
-
-for (; (i = visit_next_list(m, prev, errp)) != NULL; prev = &i) {
-UserDefOneList *native_i = (UserDefOneList *)i;
-visit_type_UserDefOne(m, &native_i->value, NULL, errp);
+Error *err = NULL;
+
+if (!error_is_set(errp)) {
+visit_start_list(m, name, &err);
+if (!err) {
+for (; (i = visit_next_list(m, prev, &err)) != NULL; prev =
&i) {
+UserDefOneList *native_i = (UserDefOneList *)i;
+visit_type_UserDefOne(m, &native_i->value, NULL, &err);
+}
+error_propagate(errp, err);
+err = NULL;
+
+/* Always call end_list if start_list succeeded. */
+visit_end_list(m, &err);
+}
+error_propagate(err
[Qemu-devel] [PATCH v2 00/12] qapi: Purge error_is_set()
This is the sixth part, covering QAPI and its users. Luiz agreed to take this through his tree. PATCH 01-08 are preparatory cleanups. PATCH 09-11 fix misuses of the visitor API in hand-written code. Generated code uses the API correctly. PATCH 12 converts QAPI and its users to the common use of the error API, purging error_is_set() along the way. v1 has a PATCH 13 that drops error_is_set(). This depends on all five prior parts of the purge, of which only the first two have been committed already. Luiz asked me to drop it from this series. My series conflicts with Lluís's "qapi: Allow modularization of QAPI schema files" and Amos's "qapi: fix coding style in generated code", but the conflicts are trivial, and 3-way merge can take care of them. v2: * Fix pasto in commit messages of PATCH 10+11 [Eric] * Fix logic error in PATCH 12 [Eric] * Update copyright notice in PATCH 12 * Unbundle PATCH 13 Markus Armbruster (12): qapi: Update qapi-code-gen.txt example to match current code qapi: Normalize marshalling's visitor initialization and cleanup qapi: Remove unused Visitor callbacks start_handle(), end_handle() qapi: Replace start_optional()/end_optional() by optional() qapi-visit.py: Clean up confusing push_indent() / pop_indent() use qapi: Clean up shadowing of parameters and locals in inner scopes qapi-visit.py: Clean up a sloppy use of field prefix qapi: Un-inline visit of implicit struct hmp: Call visit_end_struct() after visit_start_struct() succeeds hw: Don't call visit_end_struct() after visit_start_struct() fails tests: Don't call visit_end_struct() after visit_start_struct() fails qapi: Replace uncommon use of the error API by the common one docs/qapi-code-gen.txt | 165 ++- hmp.c | 16 +-- hw/timer/mc146818rtc.c | 41 +- hw/virtio/virtio-balloon.c | 33 +++-- include/qapi/visitor-impl.h| 8 +- include/qapi/visitor.h | 5 +- qapi/opts-visitor.c| 5 +- qapi/qapi-visit-core.c | 259 +++-- qapi/qmp-input-visitor.c | 6 +- qapi/string-input-visitor.c| 6 +- scripts/qapi-commands.py | 89 - scripts/qapi-visit.py | 232 +++-- tests/test-qmp-input-strict.c | 28 +++- tests/test-qmp-input-visitor.c | 26 ++-- tests/test-qmp-output-visitor.c| 28 +++- tests/test-visitor-serialization.c | 26 +++- 16 files changed, 558 insertions(+), 415 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH v2 07/12] qapi-visit.py: Clean up a sloppy use of field prefix
generate_visit_struct_fields() generates the base type's struct member
name both with and without the field prefix. Harmless, because the
field prefix is always empty there: only unboxed complex members have
a prefix, and those can't have a base type.
Clean it up anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi-visit.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 222ce62..23ae5f2 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -60,7 +60,7 @@ static void visit_type_%(full_name)s_fields(Visitor *m,
%(name)s ** obj, Error *
if base:
ret += mcgen('''
-visit_start_implicit_struct(m, (void**) &(*obj)->%(c_name)s, sizeof(%(type)s),
&err);
+visit_start_implicit_struct(m, (void**) &(*obj)->%(c_prefix)s%(c_name)s,
sizeof(%(type)s), &err);
if (!err) {
visit_type_%(type)s_fields(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
error_propagate(errp, err);
--
1.8.1.4
[Qemu-devel] [PATCH v2 11/12] tests: Don't call visit_end_struct() after visit_start_struct() fails
When visit_start_struct() fails, visit_end_struct() must not be
called. Three out of four visit_type_TestStruct() call it anyway. As
far as I can tell, visit_start_struct() doesn't actually fail there.
Fix them anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
tests/test-qmp-input-strict.c | 18 +-
tests/test-qmp-output-visitor.c| 18 +-
tests/test-visitor-serialization.c | 18 +-
3 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 449d285..ec798c2 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -72,14 +72,22 @@ typedef struct TestStruct
static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
const char *name, Error **errp)
{
+Error *err = NULL;
+
visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
- errp);
+ &err);
+if (err) {
+goto out;
+}
+
+visit_type_int(v, &(*obj)->integer, "integer", &err);
+visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+visit_type_str(v, &(*obj)->string, "string", &err);
-visit_type_int(v, &(*obj)->integer, "integer", errp);
-visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-visit_type_str(v, &(*obj)->string, "string", errp);
+visit_end_struct(v, &err);
-visit_end_struct(v, errp);
+out:
+error_propagate(errp, err);
}
static void test_validate_struct(TestInputVisitorData *data,
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 2580f3d..dfd597c 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -176,14 +176,22 @@ typedef struct TestStruct
static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
const char *name, Error **errp)
{
+Error *err = NULL;
+
visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
- errp);
+ &err);
+if (err) {
+goto out;
+}
+
+visit_type_int(v, &(*obj)->integer, "integer", &err);
+visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+visit_type_str(v, &(*obj)->string, "string", &err);
-visit_type_int(v, &(*obj)->integer, "integer", errp);
-visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-visit_type_str(v, &(*obj)->string, "string", errp);
+visit_end_struct(v, &err);
-visit_end_struct(v, errp);
+out:
+error_propagate(errp, err);
}
static void test_visitor_out_struct(TestOutputVisitorData *data,
diff --git a/tests/test-visitor-serialization.c
b/tests/test-visitor-serialization.c
index 8166cf1..85170e5 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -195,13 +195,21 @@ typedef struct TestStruct
static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
const char *name, Error **errp)
{
-visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), errp);
+Error *err= NULL;
-visit_type_int(v, &(*obj)->integer, "integer", errp);
-visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-visit_type_str(v, &(*obj)->string, "string", errp);
+visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), &err);
+if (err) {
+goto out;
+}
+
+visit_type_int(v, &(*obj)->integer, "integer", &err);
+visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+visit_type_str(v, &(*obj)->string, "string", &err);
+
+visit_end_struct(v, &err);
-visit_end_struct(v, errp);
+out:
+error_propagate(errp, err);
}
static TestStruct *struct_create(void)
--
1.8.1.4
[Qemu-devel] [PATCH v2 08/12] qapi: Un-inline visit of implicit struct
In preparation of error handling changes. Bonus: generates less
duplicated code.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi-visit.py | 48 ++--
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 23ae5f2..3e161bf 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -17,6 +17,31 @@ import os
import getopt
import errno
+implicit_structs = []
+
+def generate_visit_implicit_struct(type):
+global implicit_structs
+if type in implicit_structs:
+return ''
+implicit_structs.append(type)
+return mcgen('''
+
+static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error
**errp)
+{
+Error *err = NULL;
+
+visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err);
+if (!err) {
+visit_type_%(c_type)s_fields(m, obj, &err);
+error_propagate(errp, err);
+err = NULL;
+visit_end_implicit_struct(m, &err);
+}
+error_propagate(errp, err);
+}
+''',
+ c_type=type_name(type))
+
def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base
= None):
substructs = []
ret = ''
@@ -49,6 +74,9 @@ static void visit_type_%(full_name)s_field_%(c_name)s(Visitor
*m, %(name)s **obj
}
''')
+if base:
+ret += generate_visit_implicit_struct(base)
+
ret += mcgen('''
static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error
**errp)
@@ -60,13 +88,7 @@ static void visit_type_%(full_name)s_fields(Visitor *m,
%(name)s ** obj, Error *
if base:
ret += mcgen('''
-visit_start_implicit_struct(m, (void**) &(*obj)->%(c_prefix)s%(c_name)s,
sizeof(%(type)s), &err);
-if (!err) {
-visit_type_%(type)s_fields(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
-error_propagate(errp, err);
-err = NULL;
-visit_end_implicit_struct(m, &err);
-}
+visit_type_implicit_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
''',
c_prefix=c_var(field_prefix),
type=type_name(base), c_name=c_var('base'))
@@ -292,6 +314,10 @@ def generate_visit_union(expr):
del base_fields[discriminator]
ret += generate_visit_struct_fields(name, "", "", base_fields)
+if discriminator:
+for key in members:
+ret += generate_visit_implicit_struct(members[key])
+
ret += mcgen('''
void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
@@ -330,13 +356,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj,
const char *name, Error **
if not discriminator:
fmt = 'visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data",
&err);'
else:
-fmt = '''visit_start_implicit_struct(m, (void**)
&(*obj)->%(c_name)s, sizeof(%(c_type)s), &err);
-if (!err) {
-visit_type_%(c_type)s_fields(m, &(*obj)->%(c_name)s, &err);
-error_propagate(errp, err);
-err = NULL;
-visit_end_implicit_struct(m, &err);
-}'''
+fmt = 'visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s,
&err);'
enum_full_value = generate_enum_full_value(disc_type, key)
ret += mcgen('''
--
1.8.1.4
[Qemu-devel] [PATCH v2 05/12] qapi-visit.py: Clean up confusing push_indent() / pop_indent() use
Changing implicit indentation in the middle of generating a block
makes following the code being generated unnecessarily hard.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi-visit.py | 32 ++--
1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index b38d62e..3eeb435 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -128,12 +128,14 @@ if (!err) {
''',
name=full_name)
+ret += mcgen('''
+/* Always call end_struct if start_struct succeeded. */
+visit_end_struct(m, &err);
+}
+error_propagate(errp, err);
+''')
pop_indent()
ret += mcgen('''
-/* Always call end_struct if start_struct succeeded. */
-visit_end_struct(m, &err);
-}
-error_propagate(errp, err);
}
''')
return ret
@@ -289,19 +291,15 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj,
const char *name, Error **
''',
name=name)
-
-push_indent()
push_indent()
push_indent()
if base:
ret += mcgen('''
-visit_type_%(name)s_fields(m, obj, &err);
+visit_type_%(name)s_fields(m, obj, &err);
''',
name=name)
-pop_indent()
-
if not discriminator:
disc_key = "type"
else:
@@ -343,19 +341,17 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj,
const char *name, Error **
}
error_propagate(errp, err);
err = NULL;
-}
''')
pop_indent()
-ret += mcgen('''
-/* Always call end_struct if start_struct succeeded. */
-visit_end_struct(m, &err);
-}
-error_propagate(errp, err);
-}
-''')
+pop_indent()
-pop_indent();
ret += mcgen('''
+}
+/* Always call end_struct if start_struct succeeded. */
+visit_end_struct(m, &err);
+}
+error_propagate(errp, err);
+}
}
''')
--
1.8.1.4
[Qemu-devel] [PATCH v2 02/12] qapi: Normalize marshalling's visitor initialization and cleanup
Input and output marshalling functions do it differently. Change them
to work the same: initialize the I/O visitor, use it, clean it up,
initialize the dealloc visitor, use it, clean it up.
This delays dealloc visitor initialization in output marshalling
functions, and input visitor cleanup in input marshalling functions.
No functional change, but the latter will be convenient when I change
the error handling.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
docs/qapi-code-gen.txt | 8
scripts/qapi-commands.py | 27 ---
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 923565e..ac951ef 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -399,8 +399,8 @@ Example:
static void qmp_marshal_output_my_command(UserDefOne * ret_in, QObject
**ret_out, Error **errp)
{
-QapiDeallocVisitor *md = qapi_dealloc_visitor_new();
QmpOutputVisitor *mo = qmp_output_visitor_new();
+QapiDeallocVisitor *md;
Visitor *v;
v = qmp_output_get_visitor(mo);
@@ -409,6 +409,7 @@ Example:
*ret_out = qmp_output_get_qobject(mo);
}
qmp_output_visitor_cleanup(mo);
+md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
visit_type_UserDefOne(v, &ret_in, "unused", NULL);
qapi_dealloc_visitor_cleanup(md);
@@ -417,15 +418,13 @@ Example:
static void qmp_marshal_input_my_command(QDict *args, QObject **ret, Error
**errp)
{
UserDefOne * retval = NULL;
-QmpInputVisitor *mi;
+QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
QapiDeallocVisitor *md;
Visitor *v;
UserDefOne * arg1 = NULL;
-mi = qmp_input_visitor_new_strict(QOBJECT(args));
v = qmp_input_get_visitor(mi);
visit_type_UserDefOne(v, &arg1, "arg1", errp);
-qmp_input_visitor_cleanup(mi);
if (error_is_set(errp)) {
goto out;
@@ -436,6 +435,7 @@ Example:
}
out:
+qmp_input_visitor_cleanup(mi);
md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
visit_type_UserDefOne(v, &arg1, "arg1", NULL);
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 9734ab0..f56cc1c 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -69,16 +69,17 @@ def gen_marshal_output_call(name, ret_type):
return ""
return "qmp_marshal_output_%s(retval, ret, errp);" % c_fun(name)
-def gen_visitor_input_containers_decl(args):
+def gen_visitor_input_containers_decl(args, obj):
ret = ""
push_indent()
if len(args) > 0:
ret += mcgen('''
-QmpInputVisitor *mi;
+QmpInputVisitor *mi = qmp_input_visitor_new_strict(%(obj)s);
QapiDeallocVisitor *md;
Visitor *v;
-''')
+''',
+ obj=obj)
pop_indent()
return ret.rstrip()
@@ -106,7 +107,7 @@ bool has_%(argname)s = false;
pop_indent()
return ret.rstrip()
-def gen_visitor_input_block(args, obj, dealloc=False):
+def gen_visitor_input_block(args, dealloc=False):
ret = ""
errparg = 'errp'
@@ -118,15 +119,14 @@ def gen_visitor_input_block(args, obj, dealloc=False):
if dealloc:
errparg = 'NULL'
ret += mcgen('''
+qmp_input_visitor_cleanup(mi);
md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
''')
else:
ret += mcgen('''
-mi = qmp_input_visitor_new_strict(%(obj)s);
v = qmp_input_get_visitor(mi);
-''',
- obj=obj)
+''')
for argname, argtype, optional, structured in parse_args(args):
if optional:
@@ -152,10 +152,6 @@ visit_end_optional(v, %(errp)s);
ret += mcgen('''
qapi_dealloc_visitor_cleanup(md);
''')
-else:
-ret += mcgen('''
-qmp_input_visitor_cleanup(mi);
-''')
pop_indent()
return ret.rstrip()
@@ -166,8 +162,8 @@ def gen_marshal_output(name, args, ret_type, middle_mode):
ret = mcgen('''
static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject
**ret_out, Error **errp)
{
-QapiDeallocVisitor *md = qapi_dealloc_visitor_new();
QmpOutputVisitor *mo = qmp_output_visitor_new();
+QapiDeallocVisitor *md;
Visitor *v;
v = qmp_output_get_visitor(mo);
@@ -176,6 +172,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s
ret_in, QObject **ret_o
*ret_out = qmp_output_get_qobject(mo);
}
qmp_output_visitor_cleanup(mo);
+md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
%(visitor)s(v, &ret_in, "unused", NULL);
qapi_dealloc_visitor_cleanup(md);
@@ -228,9 +225,9 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
%(visitor_input_block)s
''',
-
visitor_input_containers_decl=gen_visitor_input_containers_decl(args),
+
visitor_input_conta
[Qemu-devel] [PATCH v2 04/12] qapi: Replace start_optional()/end_optional() by optional()
Semantics of end_optional() differ subtly from the other end_FOO()
callbacks: when start_FOO() succeeds, the matching end_FOO() gets
called regardless of what happens in between. end_optional() gets
called only when everything in between succeeds as well. Entirely
undocumented, like all of the visitor API.
The only user of Visitor Callback end_optional() never did anything,
and was removed in commit 9f9ab46.
I'm about to clean up error handling in the generated visitor code,
and end_optional() is in my way. No users mean no test cases, and
making non-trivial cleanup transformations without test cases doesn't
strike me as a good idea.
Drop end_optional(), and rename start_optional() to optional(). We
can always go back to a pair of callbacks when we have an actual need.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
include/qapi/visitor-impl.h | 5 ++---
include/qapi/visitor.h | 5 ++---
qapi/opts-visitor.c | 5 ++---
qapi/qapi-visit-core.c | 15 ---
qapi/qmp-input-visitor.c| 6 +++---
qapi/string-input-visitor.c | 6 +++---
scripts/qapi-commands.py| 5 ++---
scripts/qapi-visit.py | 3 +--
8 files changed, 19 insertions(+), 31 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 166aadd..ecc0183 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -42,9 +42,8 @@ struct Visitor
Error **errp);
/* May be NULL */
-void (*start_optional)(Visitor *v, bool *present, const char *name,
- Error **errp);
-void (*end_optional)(Visitor *v, Error **errp);
+void (*optional)(Visitor *v, bool *present, const char *name,
+ Error **errp);
void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error
**errp);
void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error
**errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 29da211..4a0178f 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -39,9 +39,8 @@ void visit_end_implicit_struct(Visitor *v, Error **errp);
void visit_start_list(Visitor *v, const char *name, Error **errp);
GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
void visit_end_list(Visitor *v, Error **errp);
-void visit_start_optional(Visitor *v, bool *present, const char *name,
- Error **errp);
-void visit_end_optional(Visitor *v, Error **errp);
+void visit_optional(Visitor *v, bool *present, const char *name,
+Error **errp);
void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
const char *name, Error **errp);
void visit_type_enum(Visitor *v, int *obj, const char *strings[],
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 5d830a2..1632c54 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -483,8 +483,7 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name,
Error **errp)
static void
-opts_start_optional(Visitor *v, bool *present, const char *name,
- Error **errp)
+opts_optional(Visitor *v, bool *present, const char *name, Error **errp)
{
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
@@ -527,7 +526,7 @@ opts_visitor_new(const QemuOpts *opts)
/* type_number() is not filled in, but this is not the first visitor to
* skip some mandatory methods... */
-ov->visitor.start_optional = &opts_start_optional;
+ov->visitor.optional = &opts_optional;
ov->opts_root = opts;
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 1f7475c..ffd7637 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -69,18 +69,11 @@ void visit_end_list(Visitor *v, Error **errp)
v->end_list(v, errp);
}
-void visit_start_optional(Visitor *v, bool *present, const char *name,
- Error **errp)
+void visit_optional(Visitor *v, bool *present, const char *name,
+Error **errp)
{
-if (!error_is_set(errp) && v->start_optional) {
-v->start_optional(v, present, name, errp);
-}
-}
-
-void visit_end_optional(Visitor *v, Error **errp)
-{
-if (!error_is_set(errp) && v->end_optional) {
-v->end_optional(v, errp);
+if (!error_is_set(errp) && v->optional) {
+v->optional(v, present, name, errp);
}
}
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index a2bed1e..d861206 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -286,8 +286,8 @@ static void qmp_input_type_number(Visitor *v, double *obj,
const char *name,
}
}
-static void qmp_input_start_optional(Visitor *v, bool *present,
- const char *name, Error **errp)
+static void qmp_input_optional(Visitor *v, bool *present, const char *name,
+ Error **errp)
{
QmpInputVisit
[Qemu-devel] [PATCH v2 10/12] hw: Don't call visit_end_struct() after visit_start_struct() fails
When visit_start_struct() fails, visit_end_struct() must not be
called. rtc_get_date() and balloon_stats_all() call it anyway. As
far as I can tell, they're only used with the string output visitor,
which doesn't care. Fix them anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
hw/timer/mc146818rtc.c | 23 +++
hw/virtio/virtio-balloon.c | 25 +++--
2 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 8509309..6c3e3b6 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -793,19 +793,26 @@ static const MemoryRegionOps cmos_ops = {
static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
+Error *err = NULL;
RTCState *s = MC146818_RTC(obj);
struct tm current_tm;
rtc_update_time(s);
rtc_get_time(s, ¤t_tm);
-visit_start_struct(v, NULL, "struct tm", name, 0, errp);
-visit_type_int32(v, ¤t_tm.tm_year, "tm_year", errp);
-visit_type_int32(v, ¤t_tm.tm_mon, "tm_mon", errp);
-visit_type_int32(v, ¤t_tm.tm_mday, "tm_mday", errp);
-visit_type_int32(v, ¤t_tm.tm_hour, "tm_hour", errp);
-visit_type_int32(v, ¤t_tm.tm_min, "tm_min", errp);
-visit_type_int32(v, ¤t_tm.tm_sec, "tm_sec", errp);
-visit_end_struct(v, errp);
+visit_start_struct(v, NULL, "struct tm", name, 0, &err);
+if (err) {
+goto out;
+}
+visit_type_int32(v, ¤t_tm.tm_year, "tm_year", &err);
+visit_type_int32(v, ¤t_tm.tm_mon, "tm_mon", &err);
+visit_type_int32(v, ¤t_tm.tm_mday, "tm_mday", &err);
+visit_type_int32(v, ¤t_tm.tm_hour, "tm_hour", &err);
+visit_type_int32(v, ¤t_tm.tm_min, "tm_min", &err);
+visit_type_int32(v, ¤t_tm.tm_sec, "tm_sec", &err);
+visit_end_struct(v, &err);
+
+out:
+error_propagate(errp, err);
}
static void rtc_realizefn(DeviceState *dev, Error **errp)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 51f02eb..65d05c8 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -108,6 +108,7 @@ static void balloon_stats_poll_cb(void *opaque)
static void balloon_stats_get_all(Object *obj, struct Visitor *v,
void *opaque, const char *name, Error **errp)
{
+Error *err = NULL;
VirtIOBalloon *s = opaque;
int i;
@@ -116,17 +117,29 @@ static void balloon_stats_get_all(Object *obj, struct
Visitor *v,
return;
}
-visit_start_struct(v, NULL, "guest-stats", name, 0, errp);
-visit_type_int(v, &s->stats_last_update, "last-update", errp);
+visit_start_struct(v, NULL, "guest-stats", name, 0, &err);
+if (err) {
+goto out;
+}
+
+visit_type_int(v, &s->stats_last_update, "last-update", &err);
-visit_start_struct(v, NULL, NULL, "stats", 0, errp);
+visit_start_struct(v, NULL, NULL, "stats", 0, &err);
+if (err) {
+goto out_end;
+}
+
for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
visit_type_int64(v, (int64_t *) &s->stats[i], balloon_stat_names[i],
- errp);
+ &err);
}
-visit_end_struct(v, errp);
+visit_end_struct(v, &err);
+
+out_end:
+visit_end_struct(v, &err);
-visit_end_struct(v, errp);
+out:
+error_propagate(errp, err);
}
static void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,
--
1.8.1.4
[Qemu-devel] [PATCH v2 09/12] hmp: Call visit_end_struct() after visit_start_struct() succeeds
When visit_start_struct() succeeds, visit_end_struct() must be called.
hmp_object_add() doesn't when a member visit fails. As far as I can
tell, the opts visitor copes okay with the misuse. Fix it anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
hmp.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/hmp.c b/hmp.c
index ad31ceb..a7cd59e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1384,6 +1384,7 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
+Error *err_end = NULL;
QemuOpts *opts;
char *type = NULL;
char *id = NULL;
@@ -1407,24 +1408,23 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
qdict_del(pdict, "qom-type");
visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
if (err) {
-goto out_clean;
+goto out_end;
}
qdict_del(pdict, "id");
visit_type_str(opts_get_visitor(ov), &id, "id", &err);
if (err) {
-goto out_clean;
+goto out_end;
}
object_add(type, id, pdict, opts_get_visitor(ov), &err);
-if (err) {
-goto out_clean;
-}
-visit_end_struct(opts_get_visitor(ov), &err);
-if (err) {
+
+out_end:
+visit_end_struct(opts_get_visitor(ov), &err_end);
+if (!err && err_end) {
qmp_object_del(id, NULL);
}
-
+error_propagate(&err, err_end);
out_clean:
opts_visitor_cleanup(ov);
--
1.8.1.4
[Qemu-devel] [PATCH v2 12/12] qapi: Replace uncommon use of the error API by the common one
We commonly use the error API like this:
err = NULL;
foo(..., &err);
if (err) {
goto out;
}
bar(..., &err);
Every error source is checked separately. The second function is only
called when the first one succeeds. Both functions are free to pass
their argument to error_set(). Because error_set() asserts no error
has been set, this effectively means they must not be called with an
error set.
The qapi-generated code uses the error API differently:
// *errp was initialized to NULL somewhere up the call chain
frob(..., errp);
gnat(..., errp);
Errors accumulate in *errp: first error wins, subsequent errors get
dropped. To make this work, the second function does nothing when
called with an error set. Requires non-null errp, or else the second
function can't see the first one fail.
This usage has also bled into visitor tests, and two device model
object property getters rtc_get_date() and balloon_stats_get_all().
With the "accumulate" technique, you need fewer error checks in
callers, and buy that with an error check in every callee. Can be
nice.
However, mixing the two techniques is confusing. You can't use the
"accumulate" technique with functions designed for the "check
separately" technique. You can use the "check separately" technique
with functions designed for the "accumulate" technique, but then
error_set() can't catch you setting an error more than once.
Standardize on the "check separately" technique for now, because it's
overwhelmingly prevalent.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
docs/qapi-code-gen.txt | 87 --
hw/timer/mc146818rtc.c | 24 +++-
hw/virtio/virtio-balloon.c | 12 +-
qapi/qapi-visit-core.c | 231 -
scripts/qapi-commands.py | 57 ++---
scripts/qapi-visit.py | 171 +--
tests/test-qmp-input-strict.c | 10 +-
tests/test-qmp-input-visitor.c | 26 +++--
tests/test-qmp-output-visitor.c| 10 +-
tests/test-visitor-serialization.c | 12 +-
10 files changed, 354 insertions(+), 286 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index ac951ef..eb32981 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -240,7 +240,6 @@ Example:
qapi_dealloc_visitor_cleanup(md);
}
-
void qapi_free_UserDefOne(UserDefOne * obj)
{
QapiDeallocVisitor *md;
@@ -317,49 +316,54 @@ Example:
{
Error *err = NULL;
visit_type_int(m, &(*obj)->integer, "integer", &err);
+if (err) {
+goto out;
+}
visit_type_str(m, &(*obj)->string, "string", &err);
+if (err) {
+goto out;
+}
+out:
error_propagate(errp, err);
}
void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char
*name, Error **errp)
{
-if (!error_is_set(errp)) {
-Error *err = NULL;
-visit_start_struct(m, (void **)obj, "UserDefOne", name,
sizeof(UserDefOne), &err);
-if (!err) {
-if (*obj) {
-visit_type_UserDefOne_fields(m, obj, &err);
-error_propagate(errp, err);
-err = NULL;
-}
-/* Always call end_struct if start_struct succeeded. */
-visit_end_struct(m, &err);
+Error *err = NULL;
+
+visit_start_struct(m, (void **)obj, "UserDefOne", name,
sizeof(UserDefOne), &err);
+if (!err) {
+if (*obj) {
+visit_type_UserDefOne_fields(m, obj, errp);
}
-error_propagate(errp, err);
+visit_end_struct(m, &err);
}
+error_propagate(errp, err);
}
void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const
char *name, Error **errp)
{
-GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
+GenericList *i, **prev;
-if (!error_is_set(errp)) {
-visit_start_list(m, name, &err);
-if (!err) {
-for (; (i = visit_next_list(m, prev, &err)) != NULL; prev =
&i) {
-UserDefOneList *native_i = (UserDefOneList *)i;
-visit_type_UserDefOne(m, &native_i->value, NULL, &err);
-}
-error_propagate(errp, err);
-err = NULL;
-
-/* Always call end_list if start_list succeeded. */
-visit_end_list(m, &err);
-}
-error_propagate(errp, err);
+visit_start_list(m, name, &err);
+if (err) {
+goto out;
+}
+
+for (prev = (GenericList **)obj;
+ !err && (i = visit_next_list(m, prev, &err)) != NULL;
+ prev = &i) {
+UserDefOneList *native_i = (UserDefOneList *)i;
+visit_ty
[Qemu-devel] [PATCH v2 06/12] qapi: Clean up shadowing of parameters and locals in inner scopes
By un-inlining the visit of nested complex types.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi-visit.py | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 3eeb435..222ce62 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -35,6 +35,19 @@ def generate_visit_struct_fields(name, field_prefix,
fn_prefix, members, base =
nested_field_prefix = "%s%s." % (field_prefix, argname)
ret += generate_visit_struct_fields(name, nested_field_prefix,
nested_fn_prefix, argentry)
+ret += mcgen('''
+
+static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s
**obj, Error **errp)
+{
+Error *err = NULL;
+''',
+ name=name, full_name=full_name, c_name=c_var(argname))
+push_indent()
+ret += generate_visit_struct_body(full_name, argname, argentry)
+pop_indent()
+ret += mcgen('''
+}
+''')
ret += mcgen('''
@@ -69,7 +82,10 @@ if ((*obj)->%(prefix)shas_%(c_name)s) {
push_indent()
if structured:
-ret += generate_visit_struct_body(full_name, argname, argentry)
+ret += mcgen('''
+visit_type_%(full_name)s_field_%(c_name)s(m, obj, &err);
+''',
+ full_name=full_name, c_name=c_var(argname))
else:
ret += mcgen('''
visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
@@ -106,8 +122,6 @@ if (!error_is_set(errp)) {
if len(field_prefix):
ret += mcgen('''
-Error **errp = &err; /* from outer scope */
-Error *err = NULL;
visit_start_struct(m, NULL, "", "%(name)s", 0, &err);
''',
name=name)
--
1.8.1.4
Re: [Qemu-devel] [PATCH v3] glib: fix g_poll early timeout on windows
On Fri, Apr 18, 2014 at 08:24:03PM +0400, Stanislav Vorobiov wrote:
Please fix the following compiler warning with gcc 4.8.2:
> +} else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) ||
> + ((int)res < WAIT_OBJECT_0) ||
> + (res >= (WAIT_OBJECT_0 + nhandles))) {
> +break;
> +}
util/oslib-win32.c: In function 'g_poll_fixed':
util/oslib-win32.c:324:21: warning: comparison of unsigned expression < 0 is
always false [-Wtype-limits]
((int)res < WAIT_OBJECT_0) ||
^
Re: [Qemu-devel] [PATCH for-2.0 0/7] SMBus and tmp105 fixes
Il 02/04/2014 17:55, Michael S. Tsirkin ha scritto: > > smbus: allow returning an error from reads > > smbus: return -1 if nothing found at the given address > > pm_smbus: correctly report unclaimed cycles > > I've reviewed these and they look sane and safe for 2.0. > mst, could you have a second look as PC maintainer and take them? I'd rather delay to 2.1. It's not a regression is it? Ping? Paolo
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
On Wed, May 07, 2014 at 09:45:17AM +0800, Fam Zheng wrote:
> On Tue, 05/06 10:32, Fam Zheng wrote:
> > @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState *bs,
> > }
> >
> > /* Avoid the L2 tables update for the images that have snapshots.
> > */
> > -*cluster_offset = bdrv_getlength(extent->file);
> > +ret = bdrv_getlength(extent->file);
> > +if (ret < 0 ||
> > +ret & ((extent->cluster_sectors << BDRV_SECTOR_BITS) - 1)) {
> > +return VMDK_ERROR;
> > +}
> > +*cluster_offset = ret;
> > if (!extent->compressed) {
> > -bdrv_truncate(
> > -extent->file,
> > -*cluster_offset + (extent->cluster_sectors << 9)
> > -);
> > +ret = bdrv_write_zeroes(extent->file,
> > +*cluster_offset >> BDRV_SECTOR_BITS,
> > +extent->cluster_sectors,
> > +0);
>
> Hi Stefan,
>
> By considering a bdrv_write_zeroes as a pre-write, it in general doubles the
> write for the whole image, so it's not a good solution.
>
> A better way would be removing the bdrv_truncate and require the caller to do
> full cluster write (with a bounce buffer if necessary).
>
> So let's drop this patch.
Okay, thanks for investigating this.
Stefan
Re: [Qemu-devel] Configure virtio-scsi options via libvirt
On Tue, May 06, 2014 at 04:13:50PM -0700, Mike Perez wrote: > I would like be able to configure virtio-scsi options num_queues, max_sectors, > and cmd_per_lun via libvirt. Are there any plans to have this support? Hi Mike, I'm not sure about the status of libvirt support for virtio-scsi options but in the meantime you can use passthrough: http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html Stefan
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On Tue, 6 May 2014 19:37:22 +0100 Peter Maydell wrote: > On 5 May 2014 09:07, Greg Kurz wrote: > > POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR > > special purpose register to decide the endianness to use when > > entering interrupt handlers. When running a Linux guest, this > > provides a hint on the endianness used by the kernel. From a > > QEMU point of view, the information is needed for legacy virtio > > support and crash dump support as well. > > Do you care about the case of: > * kernel bigendian Yes. FWIW, ppc64 is still widely used in big endian mode we don't want to break. > * userspace littleendian (or vice-versa) We don't care about userspace here. We assume that virtio structures are owned by the guest kernel. > * guest kernel passes virtio device through to guest userspace Not sure to understand... could you please point me to an example ? > * guest userspace is doing the manipulation of the device > Hmm... you mean we would have virtio drivers implemented in the guest userspace ? Does that exist ? Please elaborate. > ? > > (Will Deacon just suggested this as a possibility on the > kvm-arm mailing list...) > Just discovered some virtio endian threads in the kvm-arm@ archives... I'll take some time to read. > Also, are we documenting what the process should be for a > virtio implementation to decide the endianness for a particular > architecture? I assume we'd like kvmtool and QEMU to do > the same thing rather than subtly different things... > Sure ! > thanks > -- PMM > Thanks. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 "Anarchy is about taking complete responsibility for yourself." Alan Moore.
Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64
On Mon, 05 May 2014 13:04:35 +0200
Alexander Graf wrote:
> On 05/05/2014 10:05 AM, Greg Kurz wrote:
> > From: Bharata B Rao
> >
> > Fix ppc64 arch specific dump code to work correctly for little endian
> > guests.
> >
> > We introduce a NoteFuncArg type to avoid adding extra arguments to all note
> > functions.
> >
> > Signed-off-by: Bharata B Rao
> > [ rebased on top of current master branch,
> >introduced NoteFuncArg,
> >use new cpu_to_dump{16,32,64} endian helpers,
> >Greg Kurz ]
> > Reviewed-by: Alexander Graf
> > Signed-off-by: Greg Kurz
> > ---
> >
> > Changes in v3:
> > - better taste with the endian helpers naming
> >
> > target-ppc/arch_dump.c | 82
> > +---
> > 1 file changed, 49 insertions(+), 33 deletions(-)
> >
> > diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
> > index 9dccf1a..5487b61 100644
> > --- a/target-ppc/arch_dump.c
> > +++ b/target-ppc/arch_dump.c
> > @@ -79,94 +79,109 @@ typedef struct noteStruct {
> > } contents;
> > } QEMU_PACKED Note;
> >
> > +typedef struct NoteFuncArg {
> > +Note note;
> > +DumpState *state;
> > +} NoteFuncArg;
> >
> > -static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
> > +static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
> > {
> > int i;
> > uint64_t cr;
> > struct PPC64ElfPrstatus *prstatus;
> > struct PPC64UserRegStruct *reg;
> > +Note *note = &arg->note;
> > +DumpState *s = arg->state;
> >
> > -note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
> > +note->hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
> >
> > prstatus = ¬e->contents.prstatus;
> > memset(prstatus, 0, sizeof(*prstatus));
> > reg = &prstatus->pr_reg;
> >
> > for (i = 0; i < 32; i++) {
> > -reg->gpr[i] = cpu_to_be64(cpu->env.gpr[i]);
> > +reg->gpr[i] = cpu_to_dump64(s, cpu->env.gpr[i]);
> > }
> > -reg->nip = cpu_to_be64(cpu->env.nip);
> > -reg->msr = cpu_to_be64(cpu->env.msr);
> > -reg->ctr = cpu_to_be64(cpu->env.ctr);
> > -reg->link = cpu_to_be64(cpu->env.lr);
> > -reg->xer = cpu_to_be64(cpu_read_xer(&cpu->env));
> > +reg->nip = cpu_to_dump64(s, cpu->env.nip);
> > +reg->msr = cpu_to_dump64(s, cpu->env.msr);
> > +reg->ctr = cpu_to_dump64(s, cpu->env.ctr);
> > +reg->link = cpu_to_dump64(s, cpu->env.lr);
> > +reg->xer = cpu_to_dump64(s, cpu_read_xer(&cpu->env));
> >
> > cr = 0;
> > for (i = 0; i < 8; i++) {
> > cr |= (cpu->env.crf[i] & 15) << (4 * (7 - i));
> > }
> > -reg->ccr = cpu_to_be64(cr);
> > +reg->ccr = cpu_to_dump64(s, cr);
> > }
> >
> > -static void ppc64_write_elf64_fpregset(Note *note, PowerPCCPU *cpu)
> > +static void ppc64_write_elf64_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> > {
> > int i;
> > struct PPC64ElfFpregset *fpregset;
> > +Note *note = &arg->note;
> > +DumpState *s = arg->state;
> >
> > -note->hdr.n_type = cpu_to_be32(NT_PRFPREG);
> > +note->hdr.n_type = cpu_to_dump32(s, NT_PRFPREG);
> >
> > fpregset = ¬e->contents.fpregset;
> > memset(fpregset, 0, sizeof(*fpregset));
> >
> > for (i = 0; i < 32; i++) {
> > -fpregset->fpr[i] = cpu_to_be64(cpu->env.fpr[i]);
> > +fpregset->fpr[i] = cpu_to_dump64(s, cpu->env.fpr[i]);
> > }
> > -fpregset->fpscr = cpu_to_be64(cpu->env.fpscr);
> > +fpregset->fpscr = cpu_to_dump64(s, cpu->env.fpscr);
> > }
> >
> > -static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu)
> > +static void ppc64_write_elf64_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> > {
> > int i;
> > struct PPC64ElfVmxregset *vmxregset;
> > +Note *note = &arg->note;
> > +DumpState *s = arg->state;
> >
> > -note->hdr.n_type = cpu_to_be32(NT_PPC_VMX);
> > +note->hdr.n_type = cpu_to_dump32(s, NT_PPC_VMX);
> > vmxregset = ¬e->contents.vmxregset;
> > memset(vmxregset, 0, sizeof(*vmxregset));
> >
> > for (i = 0; i < 32; i++) {
> > -vmxregset->avr[i].u64[0] = cpu_to_be64(cpu->env.avr[i].u64[0]);
> > -vmxregset->avr[i].u64[1] = cpu_to_be64(cpu->env.avr[i].u64[1]);
> > +vmxregset->avr[i].u64[0] = cpu_to_dump64(s,
> > cpu->env.avr[i].u64[0]);
> > +vmxregset->avr[i].u64[1] = cpu_to_dump64(s,
> > cpu->env.avr[i].u64[1]);
>
> Is this correct? Tom, could you please ack if it is?
>
>
> Alex
>
>
Tom,
Can you comment plz ?
Thanks.
--
Gregory Kurz kurzg...@fr.ibm.com
gk...@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
Am 07.05.2014 um 03:45 hat Fam Zheng geschrieben:
> On Tue, 05/06 10:32, Fam Zheng wrote:
> > On mounted NFS filesystem, ftruncate is much much slower than doing a
> > zero write. Changing this significantly speeds up cluster allocation.
> >
> > Comparing by converting a cirros image (296M) to VMDK on an NFS mount
> > point, over 1Gbe LAN:
> >
> > $ time qemu-img convert cirros-0.3.1.img /mnt/a.raw -O vmdk
> >
> > Before:
> > real0m26.464s
> > user0m0.133s
> > sys 0m0.527s
> >
> > After:
> > real0m2.120s
> > user0m0.080s
> > sys 0m0.197s
> >
> > Signed-off-by: Fam Zheng
> >
> > ---
> > V2: Fix cluster_offset check. (Kevin)
> >
> > Signed-off-by: Fam Zheng
> > ---
> > block/vmdk.c | 19 ++-
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 06a1f9f..98d2d56 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -1037,6 +1037,7 @@ static int get_cluster_offset(BlockDriverState *bs,
> > int min_index, i, j;
> > uint32_t min_count, *l2_table;
> > bool zeroed = false;
> > +int64_t ret;
> >
> > if (m_data) {
> > m_data->valid = 0;
> > @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState *bs,
> > }
> >
> > /* Avoid the L2 tables update for the images that have snapshots.
> > */
> > -*cluster_offset = bdrv_getlength(extent->file);
> > +ret = bdrv_getlength(extent->file);
> > +if (ret < 0 ||
> > +ret & ((extent->cluster_sectors << BDRV_SECTOR_BITS) - 1)) {
> > +return VMDK_ERROR;
> > +}
> > +*cluster_offset = ret;
> > if (!extent->compressed) {
> > -bdrv_truncate(
> > -extent->file,
> > -*cluster_offset + (extent->cluster_sectors << 9)
> > -);
> > +ret = bdrv_write_zeroes(extent->file,
> > +*cluster_offset >> BDRV_SECTOR_BITS,
> > +extent->cluster_sectors,
> > +0);
>
> Hi Stefan,
>
> By considering a bdrv_write_zeroes as a pre-write, it in general doubles the
> write for the whole image, so it's not a good solution.
>
> A better way would be removing the bdrv_truncate and require the caller to do
> full cluster write (with a bounce buffer if necessary).
Doesn't get_whole_cluster() already ensure that you already write a full
cluster to the image file?
However, it might be better to not use bdrv_getlength() each time you
need a new cluster, but instead use a field in VmdkExtent to keep the
next free cluster offset (which is rounded up in vmdk_open). This will
ensure that we don't overlap the next cluster allocation in case
get_whole_cluster() fails halfway through.
(In fact, the L2 table should only be updated after get_whole_cluster()
has succeeded, but we can do both to be on the safe side...)
Kevin
Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
Am 06.05.2014 um 23:03 hat Max Reitz geschrieben: > On 06.05.2014 13:10, Jan Kiszka wrote: > >On 2014-05-06 12:19, Kevin Wolf wrote: > >>The immediately visible effect of this patch is that it fixes committing > >>a temporary snapshot to its backing file. Previously, it would fail with > >>a "permission denied" error because bdrv_inherited_flags() forced the > >>backing file to be read-only, ignoring the r/w reopen of bdrv_commit(). > >> > >>The bigger problem this releaved is that the original open flags must > >>actually only be applied to the temporary snapshot, and the original > >>image file must be treated as a backing file of the temporary snapshot > >>and get the right flags for that. > >> > >>Reported-by: Jan Kiszka > >>Signed-off-by: Kevin Wolf > >>--- > >> block.c| 34 +++--- > >> include/block/block.h | 2 +- > >> tests/qemu-iotests/051 | 4 > >> tests/qemu-iotests/051.out | 10 ++ > >> 4 files changed, 34 insertions(+), 16 deletions(-) > >Works fine here! > > > >(For unknown reason, applying the iotest hunk didn't work for me, though.) > > The lines are too long and therefore split in this mail, they need > to be joined manually before applying the patch. Perhaps the monitor should be changed to avoid printing so many useless control characters, then we'd hit the limit less often... Stefan, didn't you plan to do something like this? Or was it unrelated? Kevin
Re: [Qemu-devel] [libvirt] Configure virtio-scsi options via libvirt
On 05/07/2014 01:13 AM, Mike Perez wrote: > Hi everyone, > > I would like be able to configure virtio-scsi options num_queues, max_sectors, > and cmd_per_lun via libvirt. Are there any plans to have this support? > num_queues is supported for virtio-scsi controllers since libvirt 1.0.5: http://libvirt.org/git/?p=libvirt.git;a=commit;h=d4bf0a9 Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files
Am 06.05.2014 um 21:53 hat Max Reitz geschrieben:
> On 06.05.2014 15:30, Kevin Wolf wrote:
> >bdrv_is_allocated() shouldn't return true for sectors that are
> >unallocated, but after the end of a short backing file, even though
> >such sectors are (correctly) marked as containing zeros.
> >
> >Signed-off-by: Kevin Wolf
> >---
> > block.c | 8 +---
> > include/block/block.h | 11 +++
> > 2 files changed, 12 insertions(+), 7 deletions(-)
> >
> >diff --git a/block.c b/block.c
> >index c90c71a..d3a9906 100644
> >--- a/block.c
> >+++ b/block.c
> >@@ -3883,6 +3883,10 @@ static int64_t coroutine_fn
> >bdrv_co_get_block_status(BlockDriverState *bs,
> > *pnum, pnum);
> > }
> >+if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
> >+ret |= BDRV_BLOCK_ALLOCATED;
> >+}
> >+
>
> Shouldn't BDRV_BLOCK_ALLOCATED be set in the
> !bs->drv->bdrv_co_get_block_status case as well?
It should. Thanks, I'll send a v2.
> > if (!(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO)) {
> > if (bdrv_unallocated_blocks_are_zero(bs)) {
> > ret |= BDRV_BLOCK_ZERO;
> >@@ -3959,9 +3963,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState
> >*bs, int64_t sector_num,
> > if (ret < 0) {
> > return ret;
> > }
> >-return
> >-(ret & BDRV_BLOCK_DATA) ||
> >-((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
> >+return (ret & BDRV_BLOCK_ALLOCATED);
> > }
> > /*
> >diff --git a/include/block/block.h b/include/block/block.h
> >index 2fda81c..ad4c7e8 100644
> >--- a/include/block/block.h
> >+++ b/include/block/block.h
> >@@ -116,6 +116,8 @@ typedef enum {
> > /* BDRV_BLOCK_DATA: data is read from bs->file or another file
> > * BDRV_BLOCK_ZERO: sectors read as zero
> > * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
> >+ * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
> >+ * layer (as opposed to the backing file)
>
> I guess this is above BDRV_BLOCK_RAW (albeit having a greater value)
> because it is not only used internally? (to pick up on the topic of
> OCD :-P)
Because it felt right. This may or may not be equivalent.
Kevin
[Qemu-devel] [PATCH v2] block: Fix bdrv_is_allocated() for short backing files
bdrv_is_allocated() shouldn't return true for sectors that are
unallocated, but after the end of a short backing file, even though
such sectors are (correctly) marked as containing zeros.
Signed-off-by: Kevin Wolf
---
v2:
- Set BDRV_BLOCK_ALLOCATED for !drv->bdrv_co_get_block_status (Max)
block.c | 10 ++
include/block/block.h | 11 +++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index c90c71a..65e8191 100644
--- a/block.c
+++ b/block.c
@@ -3864,7 +3864,7 @@ static int64_t coroutine_fn
bdrv_co_get_block_status(BlockDriverState *bs,
if (!bs->drv->bdrv_co_get_block_status) {
*pnum = nb_sectors;
-ret = BDRV_BLOCK_DATA;
+ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
if (bs->drv->protocol_name) {
ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE);
}
@@ -3883,6 +3883,10 @@ static int64_t coroutine_fn
bdrv_co_get_block_status(BlockDriverState *bs,
*pnum, pnum);
}
+if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
+ret |= BDRV_BLOCK_ALLOCATED;
+}
+
if (!(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO)) {
if (bdrv_unallocated_blocks_are_zero(bs)) {
ret |= BDRV_BLOCK_ZERO;
@@ -3959,9 +3963,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs,
int64_t sector_num,
if (ret < 0) {
return ret;
}
-return
-(ret & BDRV_BLOCK_DATA) ||
-((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
+return (ret & BDRV_BLOCK_ALLOCATED);
}
/*
diff --git a/include/block/block.h b/include/block/block.h
index 2fda81c..ad4c7e8 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -116,6 +116,8 @@ typedef enum {
/* BDRV_BLOCK_DATA: data is read from bs->file or another file
* BDRV_BLOCK_ZERO: sectors read as zero
* BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
+ * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
+ * layer (as opposed to the backing file)
* BDRV_BLOCK_RAW: used internally to indicate that the request
* was answered by the raw driver and that one
* should look in bs->file directly.
@@ -137,10 +139,11 @@ typedef enum {
* ftf not allocated or unknown offset, read as zero
* fff not allocated or unknown offset, read from backing_hd
*/
-#define BDRV_BLOCK_DATA 1
-#define BDRV_BLOCK_ZERO 2
-#define BDRV_BLOCK_OFFSET_VALID 4
-#define BDRV_BLOCK_RAW 8
+#define BDRV_BLOCK_DATA 0x01
+#define BDRV_BLOCK_ZERO 0x02
+#define BDRV_BLOCK_OFFSET_VALID 0x04
+#define BDRV_BLOCK_RAW 0x08
+#define BDRV_BLOCK_ALLOCATED0x10
#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK
typedef enum {
--
1.8.3.1
Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
On Tue, May 06, 2014 at 12:19:10PM +0200, Kevin Wolf wrote: > The immediately visible effect of this patch is that it fixes committing > a temporary snapshot to its backing file. Previously, it would fail with > a "permission denied" error because bdrv_inherited_flags() forced the > backing file to be read-only, ignoring the r/w reopen of bdrv_commit(). > > The bigger problem this releaved is that the original open flags must > actually only be applied to the temporary snapshot, and the original > image file must be treated as a backing file of the temporary snapshot > and get the right flags for that. > > Reported-by: Jan Kiszka > Signed-off-by: Kevin Wolf > --- > block.c| 34 +++--- > include/block/block.h | 2 +- > tests/qemu-iotests/051 | 4 > tests/qemu-iotests/051.out | 10 ++ > 4 files changed, 34 insertions(+), 16 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v3] glib: fix g_poll early timeout on windows
Hi,
Hm, but (int)res expression is not unsigned, it's signed. I've also had this
warning,
but with this expression: (res < WAIT_OBJECT_0), that's why I put (int) there.
Could it be that
for some reason your compiler treats "int" and "unsigned int", that would be
really strange though...
On 05/07/2014 12:02 PM, Stefan Hajnoczi wrote:
> On Fri, Apr 18, 2014 at 08:24:03PM +0400, Stanislav Vorobiov wrote:
>
> Please fix the following compiler warning with gcc 4.8.2:
>
>> +} else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) ||
>> + ((int)res < WAIT_OBJECT_0) ||
>> + (res >= (WAIT_OBJECT_0 + nhandles))) {
>> +break;
>> +}
>
> util/oslib-win32.c: In function 'g_poll_fixed':
> util/oslib-win32.c:324:21: warning: comparison of unsigned expression < 0 is
> always false [-Wtype-limits]
> ((int)res < WAIT_OBJECT_0) ||
>^
>
[Qemu-devel] [PULL 1/3] s390x/helper: Fixed real-to-absolute address translation
From: Thomas Huth
The real-to-absolute address translation in mmu_translate() was
missing the second part for translating the page at the prefix
address back to the 0 page. And while we're at it, also moved the
code into a separate helper function since this might come in
handy for other parts of the code, too.
Signed-off-by: Thomas Huth
Reviewed-by: David Hildenbrand
Reviewed-by: Alexander Graf
Signed-off-by: Jens Freimann
Signed-off-by: Cornelia Huck
---
target-s390x/helper.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index aa628b8..ddf268e 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -170,6 +170,20 @@ static void trigger_page_fault(CPUS390XState *env,
target_ulong vaddr,
trigger_pgm_exception(env, type, ilen);
}
+/**
+ * Translate real address to absolute (= physical)
+ * address by taking care of the prefix mapping.
+ */
+static target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr)
+{
+if (raddr < 0x2000) {
+return raddr + env->psa;/* Map the lowcore. */
+} else if (raddr >= env->psa && raddr < env->psa + 0x2000) {
+return raddr - env->psa;/* Map the 0 page. */
+}
+return raddr;
+}
+
static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
uint64_t asc, uint64_t asce, int level,
target_ulong *raddr, int *flags, int rw)
@@ -363,9 +377,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr,
int rw, uint64_t asc,
out:
/* Convert real address -> absolute address */
-if (*raddr < 0x2000) {
-*raddr = *raddr + env->psa;
-}
+*raddr = mmu_real2abs(env, *raddr);
if (*raddr <= ram_size) {
sk = &env->storage_keys[*raddr / TARGET_PAGE_SIZE];
--
1.7.9.5
[Qemu-devel] [PULL 0/3] s390 fixes
The following changes since commit 9898370497da3f18e0c9555b65c858eabc78ab50: Merge remote-tracking branch 'remotes/kraxel/tags/pull-smbios-2' into staging (2014-05-06 12:23:05 +0100) are available in the git repository at: https://github.com/cohuck/qemu.git tags/s390x-20140507 for you to fetch changes up to 56bf1a8e90e76701428885656743515af53f19ef: s390x/css: Don't save orb in subchannel. (2014-05-07 10:17:35 +0200) Some improvements for s390. Two patches deal with address translation, one fixes a problem in the channel subsystem code. Cornelia Huck (1): s390x/css: Don't save orb in subchannel. Thomas Huth (2): s390x/helper: Fixed real-to-absolute address translation s390x/helper: Added format control bit to MMU translation hw/s390x/css.c| 21 +--- hw/s390x/css.h|1 - hw/s390x/virtio-ccw.c |1 - target-s390x/cpu.h|4 +++ target-s390x/helper.c | 88 + 5 files changed, 79 insertions(+), 36 deletions(-) -- 1.7.9.5
[Qemu-devel] [PULL 3/3] s390x/css: Don't save orb in subchannel.
Current css code saves the operation request block (orb) in the
subchannel structure for later consumption by the start function
handler. This might make sense for asynchronous execution of the
start function (which qemu doesn't support), but not in our case;
it would even be wrong since orb contains a reference to a local
variable in the base ssch handler.
Let's just pass the orb through the start function call chain for
ssch; for rsch, we can pass NULL as the backend function does not
use any information passed via the orb there.
Acked-by: Christian Borntraeger
Signed-off-by: Cornelia Huck
---
hw/s390x/css.c| 21 -
hw/s390x/css.h|1 -
hw/s390x/virtio-ccw.c |1 -
3 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 7074d2b..122cc7e 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -140,7 +140,6 @@ static void sch_handle_clear_func(SubchDev *sch)
s->flags &= ~SCSW_FLAGS_MASK_PNO;
/* We always 'attempt to issue the clear signal', and we always succeed. */
-sch->orb = NULL;
sch->channel_prog = 0x0;
sch->last_cmd_valid = false;
s->ctrl &= ~SCSW_ACTL_CLEAR_PEND;
@@ -163,7 +162,6 @@ static void sch_handle_halt_func(SubchDev *sch)
path = 0x80;
/* We always 'attempt to issue the halt signal', and we always succeed. */
-sch->orb = NULL;
sch->channel_prog = 0x0;
sch->last_cmd_valid = false;
s->ctrl &= ~SCSW_ACTL_HALT_PEND;
@@ -317,12 +315,11 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr
ccw_addr)
return ret;
}
-static void sch_handle_start_func(SubchDev *sch)
+static void sch_handle_start_func(SubchDev *sch, ORB *orb)
{
PMCW *p = &sch->curr_status.pmcw;
SCSW *s = &sch->curr_status.scsw;
-ORB *orb = sch->orb;
int path;
int ret;
@@ -331,6 +328,7 @@ static void sch_handle_start_func(SubchDev *sch)
if (!(s->ctrl & SCSW_ACTL_SUSP)) {
/* Look at the orb and try to execute the channel program. */
+assert(orb != NULL); /* resume does not pass an orb */
p->intparm = orb->intparm;
if (!(orb->lpm & path)) {
/* Generate a deferred cc 3 condition. */
@@ -406,7 +404,7 @@ static void sch_handle_start_func(SubchDev *sch)
* read/writes) asynchronous later on if we start supporting more than
* our current very simple devices.
*/
-static void do_subchannel_work(SubchDev *sch)
+static void do_subchannel_work(SubchDev *sch, ORB *orb)
{
SCSW *s = &sch->curr_status.scsw;
@@ -416,7 +414,7 @@ static void do_subchannel_work(SubchDev *sch)
} else if (s->ctrl & SCSW_FCTL_HALT_FUNC) {
sch_handle_halt_func(sch);
} else if (s->ctrl & SCSW_FCTL_START_FUNC) {
-sch_handle_start_func(sch);
+sch_handle_start_func(sch, orb);
} else {
/* Cannot happen. */
return;
@@ -594,7 +592,6 @@ int css_do_xsch(SubchDev *sch)
SCSW_ACTL_SUSP);
sch->channel_prog = 0x0;
sch->last_cmd_valid = false;
-sch->orb = NULL;
s->dstat = 0;
s->cstat = 0;
ret = 0;
@@ -618,7 +615,7 @@ int css_do_csch(SubchDev *sch)
s->ctrl &= ~(SCSW_CTRL_MASK_FCTL | SCSW_CTRL_MASK_ACTL);
s->ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_FCTL_CLEAR_FUNC;
-do_subchannel_work(sch);
+do_subchannel_work(sch, NULL);
ret = 0;
out:
@@ -659,7 +656,7 @@ int css_do_hsch(SubchDev *sch)
}
s->ctrl |= SCSW_ACTL_HALT_PEND;
-do_subchannel_work(sch);
+do_subchannel_work(sch, NULL);
ret = 0;
out:
@@ -721,13 +718,12 @@ int css_do_ssch(SubchDev *sch, ORB *orb)
if (channel_subsys->chnmon_active) {
css_update_chnmon(sch);
}
-sch->orb = orb;
sch->channel_prog = orb->cpa;
/* Trigger the start function. */
s->ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND);
s->flags &= ~SCSW_FLAGS_MASK_PNO;
-do_subchannel_work(sch);
+do_subchannel_work(sch, orb);
ret = 0;
out:
@@ -957,7 +953,7 @@ int css_do_rsch(SubchDev *sch)
}
s->ctrl |= SCSW_ACTL_RESUME_PEND;
-do_subchannel_work(sch);
+do_subchannel_work(sch, NULL);
ret = 0;
out:
@@ -1267,7 +1263,6 @@ void css_reset_sch(SubchDev *sch)
sch->channel_prog = 0x0;
sch->last_cmd_valid = false;
-sch->orb = NULL;
sch->thinint_active = false;
}
diff --git a/hw/s390x/css.h b/hw/s390x/css.h
index e9b4405..220169e 100644
--- a/hw/s390x/css.h
+++ b/hw/s390x/css.h
@@ -76,7 +76,6 @@ struct SubchDev {
hwaddr channel_prog;
CCW1 last_cmd;
bool last_cmd_valid;
-ORB *orb;
bool thinint_active;
/* transport-provided data: */
int (*ccw_cb) (SubchDev *, CCW1);
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 2bf0af8..1cb4e2c 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -559,7 +559,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev,
VirtIODevice *vdev)
/* Initialize subchannel structure. *
[Qemu-devel] [PULL 2/3] s390x/helper: Added format control bit to MMU translation
From: Thomas Huth
With the EDAT-1 facility, the MMU translation can stop at the
segment table already, pointing to a 1 MB block. And while we're
at it, move the page table entry handling to a separate function,
too, as suggested by Alexander Graf.
Acked-by: Alexander Graf
Signed-off-by: Thomas Huth
Signed-off-by: Cornelia Huck
---
target-s390x/cpu.h|4 +++
target-s390x/helper.c | 70 -
2 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 41903a9..aad277a 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -270,6 +270,9 @@ typedef struct CPUS390XState {
#define FLAG_MASK_64(PSW_MASK_64 >> 32)
#define FLAG_MASK_320x1000
+/* Control register 0 bits */
+#define CR0_EDAT0x0080ULL
+
static inline int cpu_mmu_index (CPUS390XState *env)
{
if (env->psw.mask & PSW_MASK_PSTATE) {
@@ -927,6 +930,7 @@ struct sysib_322 {
#define _REGION_ENTRY_LENGTH0x03 /* region third length
*/
#define _SEGMENT_ENTRY_ORIGIN ~0x7ffULL /* segment table origin
*/
+#define _SEGMENT_ENTRY_FC 0x400 /* format control
*/
#define _SEGMENT_ENTRY_RO 0x200 /* page protection bit
*/
#define _SEGMENT_ENTRY_INV 0x20 /* invalid segment table entry
*/
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index ddf268e..7c76fc1 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -184,6 +184,50 @@ static target_ulong mmu_real2abs(CPUS390XState *env,
target_ulong raddr)
return raddr;
}
+/* Decode page table entry (normal 4KB page) */
+static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
+ uint64_t asc, uint64_t asce,
+ target_ulong *raddr, int *flags, int rw)
+{
+if (asce & _PAGE_INVALID) {
+DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, asce);
+trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
+return -1;
+}
+
+if (asce & _PAGE_RO) {
+*flags &= ~PAGE_WRITE;
+}
+
+*raddr = asce & _ASCE_ORIGIN;
+
+PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, asce);
+
+return 0;
+}
+
+/* Decode EDAT1 segment frame absolute address (1MB page) */
+static int mmu_translate_sfaa(CPUS390XState *env, target_ulong vaddr,
+ uint64_t asc, uint64_t asce, target_ulong *raddr,
+ int *flags, int rw)
+{
+if (asce & _SEGMENT_ENTRY_INV) {
+DPRINTF("%s: SEG=0x%" PRIx64 " invalid\n", __func__, asce);
+trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw);
+return -1;
+}
+
+if (asce & _SEGMENT_ENTRY_RO) {
+*flags &= ~PAGE_WRITE;
+}
+
+*raddr = (asce & 0xfff0ULL) | (vaddr & 0xf);
+
+PTE_DPRINTF("%s: SEG=0x%" PRIx64 "\n", __func__, asce);
+
+return 0;
+}
+
static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
uint64_t asc, uint64_t asce, int level,
target_ulong *raddr, int *flags, int rw)
@@ -243,28 +287,18 @@ static int mmu_translate_asce(CPUS390XState *env,
target_ulong vaddr,
PTE_DPRINTF("%s: 0x%" PRIx64 " + 0x%" PRIx64 " => 0x%016" PRIx64 "\n",
__func__, origin, offs, new_asce);
-if (level != _ASCE_TYPE_SEGMENT) {
+if (level == _ASCE_TYPE_SEGMENT) {
+/* 4KB page */
+return mmu_translate_pte(env, vaddr, asc, new_asce, raddr, flags, rw);
+} else if (level - 4 == _ASCE_TYPE_SEGMENT &&
+ (new_asce & _SEGMENT_ENTRY_FC) && (env->cregs[0] & CR0_EDAT)) {
+/* 1MB page */
+return mmu_translate_sfaa(env, vaddr, asc, new_asce, raddr, flags, rw);
+} else {
/* yet another region */
return mmu_translate_asce(env, vaddr, asc, new_asce, level - 4, raddr,
flags, rw);
}
-
-/* PTE */
-if (new_asce & _PAGE_INVALID) {
-DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, new_asce);
-trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
-return -1;
-}
-
-if (new_asce & _PAGE_RO) {
-*flags &= ~PAGE_WRITE;
-}
-
-*raddr = new_asce & _ASCE_ORIGIN;
-
-PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, new_asce);
-
-return 0;
}
static int mmu_translate_asc(CPUS390XState *env, target_ulong vaddr,
--
1.7.9.5
Re: [Qemu-devel] [PATCH v3] glib: fix g_poll early timeout on windows
On 7 May 2014, at 09:36, Stanislav Vorobiov wrote:
> Hi,
>
> Hm, but (int)res expression is not unsigned, it's signed. I've also had this
> warning,
> but with this expression: (res < WAIT_OBJECT_0), that's why I put (int)
> there. Could it be that
> for some reason your compiler treats "int" and "unsigned int", that would be
> really strange though...
I suspect the problem is that WAIT_OBJECT_0 is defined as an unsigned long:
#define WAIT_OBJECT_0 ((STATUS_WAIT_0 ) + 0 )
#define STATUS_WAIT_0 ((DWORD)0xL)
So IIRC under the 'usual conversions', and int compared with it will be cast to
an unsigned long too.
I think you want to cast WAIT_OBJECT_0 to a long or similar.
Alex
> On 05/07/2014 12:02 PM, Stefan Hajnoczi wrote:
>> On Fri, Apr 18, 2014 at 08:24:03PM +0400, Stanislav Vorobiov wrote:
>>
>> Please fix the following compiler warning with gcc 4.8.2:
>>
>>> +} else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) ||
>>> + ((int)res < WAIT_OBJECT_0) ||
>>> + (res >= (WAIT_OBJECT_0 + nhandles))) {
>>> +break;
>>> +}
>>
>> util/oslib-win32.c: In function 'g_poll_fixed':
>> util/oslib-win32.c:324:21: warning: comparison of unsigned expression < 0 is
>> always false [-Wtype-limits]
>>((int)res < WAIT_OBJECT_0) ||
>> ^
>>
>
>
>
--
Alex Bligh
Re: [Qemu-devel] [PATCH v1 01/22] target-arm: A64: Add friendly logging of PSTATE A and I flags
On 6 May 2014 07:08, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" > > Signed-off-by: Edgar E. Iglesias > --- > target-arm/translate-a64.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index b62db4d..4f8246f 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -137,8 +137,10 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f, > cpu_fprintf(f, " "); > } > } > -cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n", > +cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c%c%c)\n", > psr, > +psr & PSTATE_A ? 'A' : '-', > +psr & PSTATE_I ? 'I' : '-', > psr & PSTATE_N ? 'N' : '-', > psr & PSTATE_Z ? 'Z' : '-', > psr & PSTATE_C ? 'C' : '-', Why A and I ? In particular in QEMU the A bit is always zero because we don't do System Errors (aka asynchronous external aborts), and it's weird to show I but not F. The idea of splitting out NZCV is really that (as with the A32/T32 state dump) they're the most useful bits for immediately figuring out code flow); anything else you can fish out of the hex value by hand if you really need it. I think you can make a case for "decode only a small set of key bits" or for "completely decode the whole register", but I'm not sure adding only two more bits makes sense. thanks -- PMM
Re: [Qemu-devel] [PATCH] vl.c: remove init_clocks call from main
On Tue, May 06, 2014 at 04:59:53PM +0400, Kirill Batuzov wrote: > Clocks are initialized in qemu_init_main_loop. They are not needed before it. > Initializing them twice is not only unnecessary but is harmful: it results in > memory leak and potentially can lead to a situation where different parts of > QEMU use different sets of timers. > > To avoid it remove init_clocks call from main and add an assertion to > qemu_clock_init that corresponding clock has not been initialized yet. > > Signed-off-by: Kirill Batuzov > --- > qemu-timer.c |3 +++ > vl.c |1 - > 2 files changed, 3 insertions(+), 1 deletion(-) > > The init_clocks call was added to qemu_init_main_loop in commit > 172061a0a0d98c974ea8d5ed715195237bc44225. init_clocks was made idempotent > in prior commit 744ca8e3754e6808c6b5331d287adc533fca0ad3. Back then it was > not possible to remove init_clocks call from main because rtc_clock variable > was of type QEMUClock * and was used in option processing. So clocks needed to > be initialized before command line options could be processed. > > Commit 7bf8fbde449600926370f744b2b3d9cc819ca74f removed idempotence property > from init_clocks. Commit 884f17c235095af99b92dd8cd10bb824a5b0f777 removed > the need to call init_clocks from main, but did not remove the call. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
On Wed, 05/07 10:20, Kevin Wolf wrote:
> Am 07.05.2014 um 03:45 hat Fam Zheng geschrieben:
> > On Tue, 05/06 10:32, Fam Zheng wrote:
> > > On mounted NFS filesystem, ftruncate is much much slower than doing a
> > > zero write. Changing this significantly speeds up cluster allocation.
> > >
> > > Comparing by converting a cirros image (296M) to VMDK on an NFS mount
> > > point, over 1Gbe LAN:
> > >
> > > $ time qemu-img convert cirros-0.3.1.img /mnt/a.raw -O vmdk
> > >
> > > Before:
> > > real0m26.464s
> > > user0m0.133s
> > > sys 0m0.527s
> > >
> > > After:
> > > real0m2.120s
> > > user0m0.080s
> > > sys 0m0.197s
> > >
> > > Signed-off-by: Fam Zheng
> > >
> > > ---
> > > V2: Fix cluster_offset check. (Kevin)
> > >
> > > Signed-off-by: Fam Zheng
> > > ---
> > > block/vmdk.c | 19 ++-
> > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/block/vmdk.c b/block/vmdk.c
> > > index 06a1f9f..98d2d56 100644
> > > --- a/block/vmdk.c
> > > +++ b/block/vmdk.c
> > > @@ -1037,6 +1037,7 @@ static int get_cluster_offset(BlockDriverState *bs,
> > > int min_index, i, j;
> > > uint32_t min_count, *l2_table;
> > > bool zeroed = false;
> > > +int64_t ret;
> > >
> > > if (m_data) {
> > > m_data->valid = 0;
> > > @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState
> > > *bs,
> > > }
> > >
> > > /* Avoid the L2 tables update for the images that have
> > > snapshots. */
> > > -*cluster_offset = bdrv_getlength(extent->file);
> > > +ret = bdrv_getlength(extent->file);
> > > +if (ret < 0 ||
> > > +ret & ((extent->cluster_sectors << BDRV_SECTOR_BITS) - 1)) {
> > > +return VMDK_ERROR;
> > > +}
> > > +*cluster_offset = ret;
> > > if (!extent->compressed) {
> > > -bdrv_truncate(
> > > -extent->file,
> > > -*cluster_offset + (extent->cluster_sectors << 9)
> > > -);
> > > +ret = bdrv_write_zeroes(extent->file,
> > > +*cluster_offset >> BDRV_SECTOR_BITS,
> > > +extent->cluster_sectors,
> > > +0);
> >
> > Hi Stefan,
> >
> > By considering a bdrv_write_zeroes as a pre-write, it in general doubles the
> > write for the whole image, so it's not a good solution.
> >
> > A better way would be removing the bdrv_truncate and require the caller to
> > do
> > full cluster write (with a bounce buffer if necessary).
>
> Doesn't get_whole_cluster() already ensure that you already write a full
> cluster to the image file?
That one is actually called get_backing_cluster(), if you look at the code it
has. :)
>
> However, it might be better to not use bdrv_getlength() each time you
> need a new cluster, but instead use a field in VmdkExtent to keep the
> next free cluster offset (which is rounded up in vmdk_open).
Yes, indeed. We should do that.
Thanks,
Fam
Re: [Qemu-devel] [PATCH v1 16/22] target-arm: A64: Forbid ERET to unimplemented ELs
On 6 May 2014 07:08, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias"
>
> Check for EL2 support before returning to it.
>
> Signed-off-by: Edgar E. Iglesias
> ---
> target-arm/op_helper.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 770c776..f1ae05e 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -411,12 +411,10 @@ void HELPER(exception_return)(CPUARMState *env)
> env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
> } else {
> new_el = extract32(spsr, 2, 2);
> -if (new_el > cur_el) {
> +if (new_el > cur_el
> +|| (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
> /* Disallow returns to higher ELs than the current one. */
> -goto illegal_return;
> -}
> -if (new_el > 1) {
> -/* Return to unimplemented EL */
> +/* Disallow returns to unimplemented ELs. */
Merge the comments rather than having two one-liners one after
the other, please.
/* Disallow return to an EL which is unimplemented or higher
* than the current one.
*/
thanks
-- PMM
Re: [Qemu-devel] [PATCH] target-i386: fix handling of ZF in btx instructions
On 6 May 2014 14:33, Dmitry Poletaev wrote: > Eflags for bt/bts/btr/btc instructions compute as for shift(SAR) > instructions. According to Intel A2 manual, for btx instructions zf is > unaffected under any condition, but for SAR group, when evaluate eflags, we > compute zf. Isn't this the same bug that's fixed by Richard Henderson's patch from last month? http://patchwork.ozlabs.org/patch/337929/ thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.0 0/7] SMBus and tmp105 fixes
On Wed, May 07, 2014 at 10:03:58AM +0200, Paolo Bonzini wrote: > Il 02/04/2014 17:55, Michael S. Tsirkin ha scritto: > > smbus: allow returning an error from reads > > smbus: return -1 if nothing found at the given address > > pm_smbus: correctly report unclaimed cycles > >>> > >>> I've reviewed these and they look sane and safe for 2.0. > >>> mst, could you have a second look as PC maintainer and take them? > >I'd rather delay to 2.1. > >It's not a regression is it? > > > > Ping? > > Paolo Applied, thanks!
Re: [Qemu-devel] [PATCH v6 0/2] apic: bump emulated lapic version to 0x14
On Tue, May 06, 2014 at 11:17:23AM -0400, Gabriel L. Somlo wrote: > This patch set changes the software-emulated local apic version > to 0x14 starting with pc machine types 2.1 and newer. This should > be particularly helpful when running OS X guests with TCG, since > XNU appears to have a hardcoded requirement that lapic version >= 0x14. > > Changelog: > > v6: - rebased to apply cleanly (no fuzz) against latest qemu git > - opportunity to practice dealing with Acked-by and Reviewed-by :) > > v5: convert lapic version to uint8_t (only 8 bits dedicated to > "implementation version" in the apic version register, according to > the Intel spec). > > v4: - split into a two-patch series with cover letter > - 1/2: - introduces empty 2.0 compat_props > - depends on 3458b2b075f92f163ccb9a1f24733eb5705947f0 to add >2.1 machine type and move aliases (now already upstream, but >not at the time v4 went out :) > - 2/2: - adds lapic version as a machine property defaulting to 0x14 > - set to 0x11 in compat_props for machines 2.0 and older > > v3 and older: single patch, lapic version is global, no cover letter > > Thanks again, > Gabriel Applied, thanks for your patience. > PS. Funny, now that I'm getting close to having figured out the qemu > contributor netiquette, I'm just about done submitting all the changes > I set out to contribute... :) > > Gabriel L. Somlo (2): > pc: add compat_props placeholder for 2.0 machine type > pic: use emulated lapic version 0x14 on pc machines >= 2.1 > > hw/i386/pc_piix.c | 4 > hw/i386/pc_q35.c| 4 > hw/intc/apic.c | 2 +- > hw/intc/apic_common.c | 1 + > include/hw/i386/apic_internal.h | 1 + > include/hw/i386/pc.h| 12 > 6 files changed, 23 insertions(+), 1 deletion(-) > > -- > 1.9.0
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
Am 07.05.2014 um 10:57 hat Fam Zheng geschrieben:
> On Wed, 05/07 10:20, Kevin Wolf wrote:
> > Am 07.05.2014 um 03:45 hat Fam Zheng geschrieben:
> > > On Tue, 05/06 10:32, Fam Zheng wrote:
> > > > On mounted NFS filesystem, ftruncate is much much slower than doing a
> > > > zero write. Changing this significantly speeds up cluster allocation.
> > > >
> > > > Comparing by converting a cirros image (296M) to VMDK on an NFS mount
> > > > point, over 1Gbe LAN:
> > > >
> > > > $ time qemu-img convert cirros-0.3.1.img /mnt/a.raw -O vmdk
> > > >
> > > > Before:
> > > > real0m26.464s
> > > > user0m0.133s
> > > > sys 0m0.527s
> > > >
> > > > After:
> > > > real0m2.120s
> > > > user0m0.080s
> > > > sys 0m0.197s
> > > >
> > > > Signed-off-by: Fam Zheng
> > > >
> > > > ---
> > > > V2: Fix cluster_offset check. (Kevin)
> > > >
> > > > Signed-off-by: Fam Zheng
> > > > ---
> > > > block/vmdk.c | 19 ++-
> > > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/block/vmdk.c b/block/vmdk.c
> > > > index 06a1f9f..98d2d56 100644
> > > > --- a/block/vmdk.c
> > > > +++ b/block/vmdk.c
> > > > @@ -1037,6 +1037,7 @@ static int get_cluster_offset(BlockDriverState
> > > > *bs,
> > > > int min_index, i, j;
> > > > uint32_t min_count, *l2_table;
> > > > bool zeroed = false;
> > > > +int64_t ret;
> > > >
> > > > if (m_data) {
> > > > m_data->valid = 0;
> > > > @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState
> > > > *bs,
> > > > }
> > > >
> > > > /* Avoid the L2 tables update for the images that have
> > > > snapshots. */
> > > > -*cluster_offset = bdrv_getlength(extent->file);
> > > > +ret = bdrv_getlength(extent->file);
> > > > +if (ret < 0 ||
> > > > +ret & ((extent->cluster_sectors << BDRV_SECTOR_BITS) - 1))
> > > > {
> > > > +return VMDK_ERROR;
> > > > +}
> > > > +*cluster_offset = ret;
> > > > if (!extent->compressed) {
> > > > -bdrv_truncate(
> > > > -extent->file,
> > > > -*cluster_offset + (extent->cluster_sectors << 9)
> > > > -);
> > > > +ret = bdrv_write_zeroes(extent->file,
> > > > +*cluster_offset >>
> > > > BDRV_SECTOR_BITS,
> > > > +extent->cluster_sectors,
> > > > +0);
> > >
> > > Hi Stefan,
> > >
> > > By considering a bdrv_write_zeroes as a pre-write, it in general doubles
> > > the
> > > write for the whole image, so it's not a good solution.
> > >
> > > A better way would be removing the bdrv_truncate and require the caller
> > > to do
> > > full cluster write (with a bounce buffer if necessary).
> >
> > Doesn't get_whole_cluster() already ensure that you already write a full
> > cluster to the image file?
>
> That one is actually called get_backing_cluster(), if you look at the code it
> has. :)
Right, it doesn't do anything without a backing file. This is different
from qcow2, whose mechanism I assumed without reading the code in
detail. :-)
I think it would make sense to rewrite get_whole_cluster() to write the
cluster for both image with a backing file and standalone images; just
that without a backing file it would use memset() to fill the buffer
instead of bdrv_read().
Not sure how easy it would be, but it might be an opportunity to also
change it to write only those parts of the cluster that aren't written
to anyway by the cluster.
Kevin
Re: [Qemu-devel] [PATCH v3] glib: fix g_poll early timeout on windows
Hi,
Yes it's probably the cause, thanks.
On 05/07/2014 12:49 PM, Alex Bligh wrote:
>
> On 7 May 2014, at 09:36, Stanislav Vorobiov wrote:
>
>> Hi,
>>
>> Hm, but (int)res expression is not unsigned, it's signed. I've also had this
>> warning,
>> but with this expression: (res < WAIT_OBJECT_0), that's why I put (int)
>> there. Could it be that
>> for some reason your compiler treats "int" and "unsigned int", that would be
>> really strange though...
>
> I suspect the problem is that WAIT_OBJECT_0 is defined as an unsigned long:
>
> #define WAIT_OBJECT_0 ((STATUS_WAIT_0 ) + 0 )
>
> #define STATUS_WAIT_0 ((DWORD)0xL)
>
> So IIRC under the 'usual conversions', and int compared with it will be cast
> to an unsigned long too.
>
> I think you want to cast WAIT_OBJECT_0 to a long or similar.
>
> Alex
>
>
>> On 05/07/2014 12:02 PM, Stefan Hajnoczi wrote:
>>> On Fri, Apr 18, 2014 at 08:24:03PM +0400, Stanislav Vorobiov wrote:
>>>
>>> Please fix the following compiler warning with gcc 4.8.2:
>>>
+} else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) ||
+ ((int)res < WAIT_OBJECT_0) ||
+ (res >= (WAIT_OBJECT_0 + nhandles))) {
+break;
+}
>>>
>>> util/oslib-win32.c: In function 'g_poll_fixed':
>>> util/oslib-win32.c:324:21: warning: comparison of unsigned expression < 0
>>> is always false [-Wtype-limits]
>>>((int)res < WAIT_OBJECT_0) ||
>>> ^
>>>
>>
>>
>>
>
[Qemu-devel] [PATCH v4] glib: fix g_poll early timeout on windows
From: Sangho Park
g_poll has a problem on Windows when using
timeouts < 10ms, in glib/gpoll.c:
/* If not, and we have a significant timeout, poll again with
* timeout then. Note that this will return indication for only
* one event, or only for messages. We ignore timeouts less than
* ten milliseconds as they are mostly pointless on Windows, the
* MsgWaitForMultipleObjectsEx() call will timeout right away
* anyway.
*/
if (retval == 0 && (timeout == INFINITE || timeout >= 10))
retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout);
so whenever g_poll is called with timeout < 10ms it does
a quick poll instead of wait, this causes significant performance
degradation of QEMU, thus we should use WaitForMultipleObjectsEx
directly
Signed-off-by: Stanislav Vorobiov
---
include/glib-compat.h | 19 +
include/qemu-common.h | 12 --
util/oslib-win32.c| 112 +
3 files changed, 131 insertions(+), 12 deletions(-)
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 8aa77af..1280fb2 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -24,4 +24,23 @@ static inline guint g_timeout_add_seconds(guint interval,
GSourceFunc function,
}
#endif
+#ifdef _WIN32
+/*
+ * g_poll has a problem on Windows when using
+ * timeouts < 10ms, so use wrapper.
+ */
+#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
+gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
+#elif !GLIB_CHECK_VERSION(2, 20, 0)
+/*
+ * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly
+ * on older systems.
+ */
+static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout)
+{
+GMainContext *ctx = g_main_context_default();
+return g_main_context_get_poll_func(ctx)(fds, nfds, timeout);
+}
+#endif
+
#endif
diff --git a/include/qemu-common.h b/include/qemu-common.h
index a998e8d..3f3fd60 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -124,18 +124,6 @@ int qemu_main(int argc, char **argv, char **envp);
void qemu_get_timedate(struct tm *tm, int offset);
int qemu_timedate_diff(struct tm *tm);
-#if !GLIB_CHECK_VERSION(2, 20, 0)
-/*
- * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly
- * on older systems.
- */
-static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout)
-{
-GMainContext *ctx = g_main_context_default();
-return g_main_context_get_poll_func(ctx)(fds, nfds, timeout);
-}
-#endif
-
/**
* is_help_option:
* @s: string to test
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 93f7d35..69552f7 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -238,3 +238,115 @@ char *qemu_get_exec_dir(void)
{
return g_strdup(exec_dir);
}
+
+/*
+ * g_poll has a problem on Windows when using
+ * timeouts < 10ms, in glib/gpoll.c:
+ *
+ * // If not, and we have a significant timeout, poll again with
+ * // timeout then. Note that this will return indication for only
+ * // one event, or only for messages. We ignore timeouts less than
+ * // ten milliseconds as they are mostly pointless on Windows, the
+ * // MsgWaitForMultipleObjectsEx() call will timeout right away
+ * // anyway.
+ *
+ * if (retval == 0 && (timeout == INFINITE || timeout >= 10))
+ * retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout);
+ *
+ * So whenever g_poll is called with timeout < 10ms it does
+ * a quick poll instead of wait, this causes significant performance
+ * degradation of QEMU, thus we should use WaitForMultipleObjectsEx
+ * directly
+ */
+gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout)
+{
+guint i;
+HANDLE handles[MAXIMUM_WAIT_OBJECTS];
+gint nhandles = 0;
+int num_completed = 0;
+
+for (i = 0; i < nfds; i++) {
+gint j;
+
+if (fds[i].fd <= 0) {
+continue;
+}
+
+/* don't add same handle several times
+ */
+for (j = 0; j < nhandles; j++) {
+if (handles[j] == (HANDLE)fds[i].fd) {
+break;
+}
+}
+
+if (j == nhandles) {
+if (nhandles == MAXIMUM_WAIT_OBJECTS) {
+fprintf(stderr, "Too many handles to wait for!\n");
+break;
+} else {
+handles[nhandles++] = (HANDLE)fds[i].fd;
+}
+}
+}
+
+for (i = 0; i < nfds; ++i) {
+fds[i].revents = 0;
+}
+
+if (timeout == -1) {
+timeout = INFINITE;
+}
+
+if (nhandles == 0) {
+if (timeout == INFINITE) {
+return -1;
+} else {
+SleepEx(timeout, TRUE);
+return 0;
+}
+}
+
+while (1) {
+DWORD res;
+gint j;
+
+res = WaitForMultipleObjectsEx(nhandles, handles, FALSE,
+timeout, TRUE);
+
+if (res == WAIT_FAILED) {
+for (i = 0; i < nfds; ++i) {
+fds[i].revents = 0;
+}
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On 7 May 2014 09:14, Greg Kurz wrote: > On Tue, 6 May 2014 19:37:22 +0100 > Peter Maydell wrote: >> On 5 May 2014 09:07, Greg Kurz wrote: >> > POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR >> > special purpose register to decide the endianness to use when >> > entering interrupt handlers. When running a Linux guest, this >> > provides a hint on the endianness used by the kernel. From a >> > QEMU point of view, the information is needed for legacy virtio >> > support and crash dump support as well. >> >> Do you care about the case of: >> * kernel bigendian > > Yes. FWIW, ppc64 is still widely used in big endian mode we don't > want to break. > >> * userspace littleendian (or vice-versa) > > We don't care about userspace here. We assume that virtio structures are > owned by the guest kernel. > >> * guest kernel passes virtio device through to guest userspace > > Not sure to understand... could you please point me to an example ? Consider PCI passthrough of a virtio device to userspace Linux or to a nested KVM/QEMU instance running inside the outermost KVM. It's a bit of an odd corner case, but we should either accommodate it or definitely say it's not expected to work consistently across architectures I think. thanks -- PMM
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On 07.05.14 10:14, Greg Kurz wrote: On Tue, 6 May 2014 19:37:22 +0100 Peter Maydell wrote: On 5 May 2014 09:07, Greg Kurz wrote: POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well. Do you care about the case of: * kernel bigendian Yes. FWIW, ppc64 is still widely used in big endian mode we don't want to break. * userspace littleendian (or vice-versa) We don't care about userspace here. We assume that virtio structures are owned by the guest kernel. * guest kernel passes virtio device through to guest userspace Not sure to understand... could you please point me to an example ? * guest userspace is doing the manipulation of the device Hmm... you mean we would have virtio drivers implemented in the guest userspace ? Does that exist ? Please elaborate. Virtio bypasses the IOMMU by design, so user space drivers don't make sense here :). I don't think we should overengineer hacks for legacy virtio. Alex
Re: [Qemu-devel] [PATCH 00/35] pc: ACPI memory hotplug
Hello Igor,
while testing your patchset i came to a very stupid problem.
I wanted to test migration and it cames out that the migration works
fine after plugging in memory only if i run the target VM without the
-daemonize option.
If i enable the -daemonize option the target vm tries to read from non
readable memory.
proc maps shows:
7f9334021000-7f933800 ---p 00:00 0
where it tries to read from.
Also the memory layout is different in daemonize mode than in non
daemonize mode.
Stefan
Am 04.04.2014 15:36, schrieb Igor Mammedov:
> What's new since v7:
>
> * Per Andreas' suggestion dropped DIMMBus concept.
>
> * Added hotplug binding for bus-less devices
>
> * DIMM device is split to backend and frontend. Therefore following
> command/options were added for supporting it:
>
> For memory-ram backend:
> CLI: -object-add memory-ram,
> with options: 'id' and 'size'
> For dimm frontend:
> option "size" became readonly, pulling it's size from attached backend
> added option "memdev" for specifying backend by 'id'
>
> * dropped support for 32 bit guests
>
> * failed hotplug action doesn't consume 1 slot anymore
>
> * vaious fixes adressing reviewer's comments most of them in ACPI part
> ---
>
> This series allows to hotplug 'arbitrary' DIMM devices specifying size,
> NUMA node mapping (guest side), slot and address where to map it, at runtime.
>
> Due to ACPI limitation there is need to specify a number of possible
> DIMM devices. For this task -m option was extended to support
> following format:
>
> -m [mem=]RamSize[,slots=N,maxmem=M]
>
> To allow memory hotplug user must specify a pair of additional parameters:
> 'slots' - number of possible increments
> 'maxmem' - max possible total memory size QEMU is allowed to use,
>including RamSize.
>
> minimal monitor command syntax to hotplug DIMM device:
>
> object_add memory-ram,id=memX,size=1G
> device_add dimm,id=dimmX,memdev=memX
>
> DIMM device provides following properties that could be used with
> device_add / -device to alter default behavior:
>
> id- unique string identifying device [mandatory]
> slot - number in range [0-slots) [optional], if not specified
> the first free slot is used
> node - NUMA node id [optional] (default: 0)
> size - amount of memory to add, readonly derived from backing memdev
> start - guest's physical address where to plug DIMM [optional],
> if not specified the first gap in hotplug memory region
> that fits DIMM is used
>
> -device option could be used for adding potentially hotunplugable DIMMs
> and also for specifying hotplugged DIMMs in migration case.
>
> Tested guests:
> - RHEL 6x64
> - Windows 2012DCx64
> - Windows 2008DCx64
>
> Known limitations/bugs/TODOs:
> - hot-remove is not supported, yet
> - max number of supported DIMM devices 255 (due to ACPI object name
>limit), could be increased creating several containers and putting
>DIMMs there. (exercise for future)
> - e820 table doesn't include DIMM devices added with -device /
>(or after reboot devices added with device_add)
> - Windows 2008 remembers DIMM configuration, so if DIMM with other
>start/size is added into the same slot, it refuses to use it insisting
>on old mapping.
>
> QEMU git tree for testing is available at:
> https://github.com/imammedo/qemu/commits/memory-hotplug-v8
>
> Example QEMU cmd line:
> qemu-system-x86_64 -enable-kvm -monitor unix:/tmp/mon,server,nowait \
> -m 4096,slots=4,maxmem=8G guest.img
>
> PS:
> Windows guest requires SRAT table for hotplug to work so add an extra
> option:
>-numa node
> to QEMU command line.
>
>
> Igor Mammedov (34):
> vl: convert -m to QemuOpts
> object_add: allow completion handler to get canonical path
> add memdev backend infrastructure
> vl.c: extend -m option to support options for memory hotplug
> add pc-{i440fx,q35}-2.1 machine types
> pc: create custom generic PC machine type
> qdev: hotplug for buss-less devices
> qdev: expose DeviceState.hotplugged field as a property
> dimm: implement dimm device abstraction
> memory: add memory_region_is_mapped() API
> dimm: do not allow to set already busy memdev
> pc: initialize memory hotplug address space
> pc: exit QEMU if slots > 256
> pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS
> pc: add memory hotplug handler to PC_MACHINE
> dimm: add busy address check and address auto-allocation
> dimm: add busy slot check and slot auto-allocation
> acpi: rename cpu_hotplug_defs.h to acpi_defs.h
> acpi: memory hotplug ACPI hardware implementation
> trace: add acpi memory hotplug IO region events
> trace: add DIMM slot & address allocation for target-i386
> acpi:piix4: make plug/unlug callbacks generic
> acpi:piix4: add memory hotplug handling
> pc: ich9 lpc: make it work with global/compat properties
> acpi:ich9: add m
[Qemu-devel] [PATCH net v1 1/4] net: cadence_gem: Fix Tx descriptor update
The local variable "desc" was being used to read-modify-write the
first descriptor (of a multi-desc packet) upon packet completion.
desc however continues to be used by the code as the current
descriptor. Give this first desc RMW it's own local variable to
avoid trampling.
Signed-off-by: Peter Crosthwaite
---
hw/net/cadence_gem.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index e34b25e..1c36e48 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -917,14 +917,16 @@ static void gem_transmit(GemState *s)
/* Last descriptor for this packet; hand the whole thing off */
if (tx_desc_get_last(desc)) {
+unsigneddesc_first[2];
+
/* Modify the 1st descriptor of this packet to be owned by
* the processor.
*/
cpu_physical_memory_read(s->tx_desc_addr,
- (uint8_t *)&desc[0], sizeof(desc));
-tx_desc_set_used(desc);
+ (uint8_t *)&desc_first[0], sizeof(desc));
+tx_desc_set_used(desc_first);
cpu_physical_memory_write(s->tx_desc_addr,
- (uint8_t *)&desc[0], sizeof(desc));
+ (uint8_t *)&desc_first[0], sizeof(desc));
/* Advance the hardare current descriptor past this packet */
if (tx_desc_get_wrap(desc)) {
s->tx_desc_addr = s->regs[GEM_TXQBASE];
--
1.9.2.1.g06c4abd
[Qemu-devel] [PATCH net v1 0/4] Cadence GEM patches
Hi Stefan, Peter, Found a bug in the Cadence GEM. Fixed in P1. Have some follow us trivials as well (P2-4). Regards, Peter Peter Crosthwaite (4): net: cadence_gem: Fix Tx descriptor update net: cadence_gem: Add Tx descriptor fetch printf net: cadence_gem: Fix top comment net: cadence_gem: Comment spelling sweep hw/net/cadence_gem.c | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) -- 1.9.2.1.g06c4abd
Re: [Qemu-devel] [PATCH 5/7] monitor: Add set_link arguments completion.
On Sun, Apr 27, 2014 at 05:00:06PM +0100, Hani Benhabiles wrote: > Make it possible to query all net clients without specifying an ID when > calling > qemu_find_net_clients_except(). > > This also adds the add_completion_option() function which is to be used for > other commands completions as well. > > Signed-off-by: Hani Benhabiles > --- > hmp-commands.hx | 1 + > hmp.h | 1 + > monitor.c | 34 ++ > net/net.c | 2 +- > 4 files changed, 37 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi
[Qemu-devel] [PATCH net v1 2/4] net: cadence_gem: Add Tx descriptor fetch printf
Add a debug printf for TX descriptor fetching. This helpful to anyone
needing to debug TX ring buffer traversal. Its also now consistent with
the RX code which has a similar printf.
Signed-off-by: Peter Crosthwaite
---
hw/net/cadence_gem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 1c36e48..f999886 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -886,6 +886,8 @@ static void gem_transmit(GemState *s)
/* read current descriptor */
packet_desc_addr = s->tx_desc_addr;
+
+DB_PRINT("read descriptor 0x%x\n", (unsigned)packet_desc_addr);
cpu_physical_memory_read(packet_desc_addr,
(uint8_t *)&desc[0], sizeof(desc));
/* Handle all descriptors owned by hardware */
@@ -968,6 +970,7 @@ static void gem_transmit(GemState *s)
} else {
packet_desc_addr += 8;
}
+DB_PRINT("read descriptor 0x%x\n", (unsigned)packet_desc_addr);
cpu_physical_memory_read(packet_desc_addr,
(uint8_t *)&desc[0], sizeof(desc));
}
--
1.9.2.1.g06c4abd
[Qemu-devel] [PATCH net v1 3/4] net: cadence_gem: Fix top comment
To indicate Cadence GEM not Xilinx. Signed-off-by: Peter Crosthwaite --- hw/net/cadence_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index f999886..7d13e7c 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -1,5 +1,5 @@ /* - * QEMU Xilinx GEM emulation + * QEMU Cadence GEM emulation * * Copyright (c) 2011 Xilinx, Inc. * -- 1.9.2.1.g06c4abd
[Qemu-devel] [PATCH net v1 4/4] net: cadence_gem: Comment spelling sweep
Fix some typos in comments.
Signed-off-by: Peter Crosthwaite
---
hw/net/cadence_gem.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 7d13e7c..4e49f07 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -50,7 +50,7 @@
#define GEM_IER (0x0028/4) /* Interrupt Enable reg */
#define GEM_IDR (0x002C/4) /* Interrupt Disable reg */
#define GEM_IMR (0x0030/4) /* Interrupt Mask reg */
-#define GEM_PHYMNTNC (0x0034/4) /* Phy Maintaince reg */
+#define GEM_PHYMNTNC (0x0034/4) /* Phy Maintenance reg */
#define GEM_RXPAUSE (0x0038/4) /* RX Pause Time reg */
#define GEM_TXPAUSE (0x003C/4) /* TX Pause Time reg */
#define GEM_TXPARTIALSF (0x0040/4) /* TX Partial Store and Forward */
@@ -150,7 +150,7 @@
#define GEM_NWCTRL_LOCALLOOP 0x0002 /* Local Loopback */
#define GEM_NWCFG_STRIP_FCS0x0002 /* Strip FCS field */
-#define GEM_NWCFG_LERR_DISC0x0001 /* Discard RX frames with lenth err
*/
+#define GEM_NWCFG_LERR_DISC0x0001 /* Discard RX frames with len err */
#define GEM_NWCFG_BUFF_OFST_M 0xC000 /* Receive buffer offset mask */
#define GEM_NWCFG_BUFF_OFST_S 14 /* Receive buffer offset shift */
#define GEM_NWCFG_UCAST_HASH 0x0080 /* accept unicast if hash match */
@@ -397,7 +397,7 @@ const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF };
*/
static void gem_init_register_masks(GemState *s)
{
-/* Mask of register bits which are read only*/
+/* Mask of register bits which are read only */
memset(&s->regs_ro[0], 0, sizeof(s->regs_ro));
s->regs_ro[GEM_NWCTRL] = 0xFFF8;
s->regs_ro[GEM_NWSTATUS] = 0x;
@@ -720,7 +720,7 @@ static ssize_t gem_receive(NetClientState *nc, const
uint8_t *buf, size_t size)
int crc_offset;
/* The application wants the FCS field, which QEMU does not provide.
- * We must try and caclculate one.
+ * We must try and calculate one.
*/
memcpy(rxbuf, buf, size);
@@ -877,7 +877,7 @@ static void gem_transmit(GemState *s)
DB_PRINT("\n");
-/* The packet we will hand off to qemu.
+/* The packet we will hand off to QEMU.
* Packets scattered across multiple descriptors are gathered to this
* one contiguous buffer first.
*/
@@ -929,7 +929,7 @@ static void gem_transmit(GemState *s)
tx_desc_set_used(desc_first);
cpu_physical_memory_write(s->tx_desc_addr,
(uint8_t *)&desc_first[0], sizeof(desc));
-/* Advance the hardare current descriptor past this packet */
+/* Advance the hardware current descriptor past this packet */
if (tx_desc_get_wrap(desc)) {
s->tx_desc_addr = s->regs[GEM_TXQBASE];
} else {
--
1.9.2.1.g06c4abd
Re: [Qemu-devel] [PATCH 6/7] monitor: Add netdev_add type argument completion.
On Sun, Apr 27, 2014 at 05:00:07PM +0100, Hani Benhabiles wrote: > Also update the command's documentation. > > Signed-off-by: Hani Benhabiles > --- > hmp-commands.hx | 3 ++- > hmp.h | 1 + > monitor.c | 15 +++ > 3 files changed, 18 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
On Wed, 05/07 11:06, Kevin Wolf wrote:
> Am 07.05.2014 um 10:57 hat Fam Zheng geschrieben:
> > On Wed, 05/07 10:20, Kevin Wolf wrote:
> > > Am 07.05.2014 um 03:45 hat Fam Zheng geschrieben:
> > > > On Tue, 05/06 10:32, Fam Zheng wrote:
> > > > > On mounted NFS filesystem, ftruncate is much much slower than doing a
> > > > > zero write. Changing this significantly speeds up cluster allocation.
> > > > >
> > > > > Comparing by converting a cirros image (296M) to VMDK on an NFS mount
> > > > > point, over 1Gbe LAN:
> > > > >
> > > > > $ time qemu-img convert cirros-0.3.1.img /mnt/a.raw -O vmdk
> > > > >
> > > > > Before:
> > > > > real0m26.464s
> > > > > user0m0.133s
> > > > > sys 0m0.527s
> > > > >
> > > > > After:
> > > > > real0m2.120s
> > > > > user0m0.080s
> > > > > sys 0m0.197s
> > > > >
> > > > > Signed-off-by: Fam Zheng
> > > > >
> > > > > ---
> > > > > V2: Fix cluster_offset check. (Kevin)
> > > > >
> > > > > Signed-off-by: Fam Zheng
> > > > > ---
> > > > > block/vmdk.c | 19 ++-
> > > > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/block/vmdk.c b/block/vmdk.c
> > > > > index 06a1f9f..98d2d56 100644
> > > > > --- a/block/vmdk.c
> > > > > +++ b/block/vmdk.c
> > > > > @@ -1037,6 +1037,7 @@ static int get_cluster_offset(BlockDriverState
> > > > > *bs,
> > > > > int min_index, i, j;
> > > > > uint32_t min_count, *l2_table;
> > > > > bool zeroed = false;
> > > > > +int64_t ret;
> > > > >
> > > > > if (m_data) {
> > > > > m_data->valid = 0;
> > > > > @@ -1110,12 +,20 @@ static int
> > > > > get_cluster_offset(BlockDriverState *bs,
> > > > > }
> > > > >
> > > > > /* Avoid the L2 tables update for the images that have
> > > > > snapshots. */
> > > > > -*cluster_offset = bdrv_getlength(extent->file);
> > > > > +ret = bdrv_getlength(extent->file);
> > > > > +if (ret < 0 ||
> > > > > +ret & ((extent->cluster_sectors << BDRV_SECTOR_BITS) -
> > > > > 1)) {
> > > > > +return VMDK_ERROR;
> > > > > +}
> > > > > +*cluster_offset = ret;
> > > > > if (!extent->compressed) {
> > > > > -bdrv_truncate(
> > > > > -extent->file,
> > > > > -*cluster_offset + (extent->cluster_sectors << 9)
> > > > > -);
> > > > > +ret = bdrv_write_zeroes(extent->file,
> > > > > +*cluster_offset >>
> > > > > BDRV_SECTOR_BITS,
> > > > > +extent->cluster_sectors,
> > > > > +0);
> > > >
> > > > Hi Stefan,
> > > >
> > > > By considering a bdrv_write_zeroes as a pre-write, it in general
> > > > doubles the
> > > > write for the whole image, so it's not a good solution.
> > > >
> > > > A better way would be removing the bdrv_truncate and require the caller
> > > > to do
> > > > full cluster write (with a bounce buffer if necessary).
> > >
> > > Doesn't get_whole_cluster() already ensure that you already write a full
> > > cluster to the image file?
> >
> > That one is actually called get_backing_cluster(), if you look at the code
> > it
> > has. :)
>
> Right, it doesn't do anything without a backing file. This is different
> from qcow2, whose mechanism I assumed without reading the code in
> detail. :-)
>
> I think it would make sense to rewrite get_whole_cluster() to write the
> cluster for both image with a backing file and standalone images; just
> that without a backing file it would use memset() to fill the buffer
> instead of bdrv_read().
>
> Not sure how easy it would be, but it might be an opportunity to also
> change it to write only those parts of the cluster that aren't written
> to anyway by the cluster.
>
I think that shouldn't be hard . I'll make the change and send another patch
later.
Thanks,
Fam
Re: [Qemu-devel] [RFC PATCH v2 00/16] visitor+BER migration format
* Markus Armbruster (arm...@redhat.com) wrote: > "Michael S. Tsirkin" writes: > > > On Tue, May 06, 2014 at 07:58:07PM +0100, Dr. David Alan Gilbert wrote: > >> * Markus Armbruster (arm...@redhat.com) wrote: > >> > "Michael S. Tsirkin" writes: > >> > >> > >> > >> > > OK but for a new machine type, let's default to BER, right? > >> > > I see no reason to keep supporting when non-BER when -M specifies 2.1 > >> > > compatibility, do you? > >> > > >> > I fail to see the relation between machine type and migration's wire > >> > encoding. > >> > >> New machine types are a useful but not definitive line in the sand. If > >> you enable something/change the default on a new machine type you know > >> it won't break any existing users since there aren't any. > >> > >> Dve > > The purpose of machine types is to keep the guest ABI stable. I don't > like tacking random crap unrelated to guest ABI to machine types. > They're hard enough to grasp for users as they are. > > > Exactly. And on the other hand, someone enabling old machine type > > and doing live migration is likely to want to be compatible with old > > qemu wrt migration. > > Machine types let you migrate to a newer QEMU (forward migration) > without messing up the guest ABI. Migrating to an older QEMU (backward > migration) basically doesn't work, and as long as that's the case, > picking the older wire format by default is worthless. Anyway, we seem to have had a long conversation about the least complicated part of this patch set. I'd love some thoughts on the actual visitor interface, which IMHO is the bit that's actually messy and needs some rethinking). Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 7/7] monitor: Add netdev_del id argument completion.
On Sun, Apr 27, 2014 at 05:00:08PM +0100, Hani Benhabiles wrote: > Signed-off-by: Hani Benhabiles > --- > hmp-commands.hx | 1 + > hmp.h | 1 + > monitor.c | 26 ++ > 3 files changed, 28 insertions(+) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH 3/4] block: Allow JSON filenames
Am 06.05.2014 um 21:57 hat Eric Blake geschrieben:
> On 05/06/2014 01:30 PM, Max Reitz wrote:
> > If the filename given to bdrv_open() is prefixed with "json:", parse the
> > rest as a JSON object and use the result as the options QDict.
> >
> > Signed-off-by: Max Reitz
> > ---
> > block.c | 41 +
> > 1 file changed, 41 insertions(+)
> >
>
> > /*
> > * Opens a disk image (raw, qcow2, vmdk, ...)
> > *
> > @@ -1337,6 +1364,20 @@ int bdrv_open(BlockDriverState **pbs, const char
> > *filename,
> > options = qdict_new();
> > }
> >
> > +if (filename && g_str_has_prefix(filename, "json:")) {
> > +QDict *json_options = parse_json_filename(filename, &local_err);
> > +if (local_err) {
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +
> > +qdict_join(options, json_options, true);
> > +assert(qdict_size(json_options) == 0);
>
> Would it be better to pass false to qdict_join(), and then raise an
> error if the user specified conflicting options? For example (untested,
> just typing off the top of my head here),
>
> -drive
> file='json:{"driver":"qcow2","file.filename":"foo","backing.file.driver":"raw"}',backing.file.driver=qcow2
>
> looks like it specifies conflicting backing.file.driver options.
> Passing true means that qdict_join silently overwrites the value in
> options to instead be the value in the json string; passing false means
> you could flag the user error.
Isn't the more realistic case, that 'file' is actually the backing file
string stored in an image, and the overwriting option comes from the
command line? In this case, I think we want to allow overriding the
option stored in the qcow2 file.
Kevin
pgpwYyU85KJK3.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On 7 May 2014 10:09, Alexander Graf wrote: > I don't think we should overengineer hacks for legacy virtio. Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes? thanks -- PMM
Re: [Qemu-devel] [SeaBIOS [PATCH V5 0/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Thu, Apr 10, 2014 at 09:27:44PM +0300, Marcel Apfelbaum wrote: > v4 -> v5 > - Addressed Michael S. Tsirkin's comments (patch 2/2): >- Open-coded pci_config_is_reserved() method. > > v3 -> v4: > - Addressed Kevin O'Connor's comments: >- Refactored a for loop in patch 1/2. > - Addressed Michael S. Tsirkin's comments (patch 2/2): >- Removed not needed method >- Test only base registers (dropped limits tests) >- Renamed a helper method >- Used 0xFF to test if the memory is reserved >- Simplified code in pci_bridge_has_region > - I did keep the code that restores base's address as I don't want >to modify the registers in a 'query' method. (as replied on the mail > thread) > > v2 -> v3: > - Addressed Michael S. Tsirkin's comments: >- I/O and Prefetchable Memory are optional. Do not allocate ranges > if they are not implemented (2/2). > - Note that 2/2 patch can be seen as a separate fix. However, it >is related to ranges reservation. > > v1 -> v2: > - Thanks Gerd Hoffmann for the review. > - Addressed Michael S. Tsirkin's comments: >- Limit capabilities query to 256 iterations, to make sure we > don't get into an infinite loop with a broken device. > > > If a pci-2-pci bridge supports hot-plug functionality but there are no devices > connected to it, reserve IO/mem in order to be able to attach devices > later. Do not waste space, use minimum allowed. > > Marcel Apfelbaum (2): > hw/pci: reserve IO and mem for pci-2-pci bridges with no devices > attached > hw/pci: check if pci2pci bridges implement optional limit registers > > src/fw/pciinit.c | 12 +--- > src/hw/pci.c | 45 + > src/hw/pci.h | 10 ++ > 3 files changed, 60 insertions(+), 7 deletions(-) It would be nice to have a seabios release with these patches included in QEMU: make it easier for people to use hotplug. Gerd? > -- > 1.8.3.1
Re: [Qemu-devel] [SeaBIOS [PATCH V5 0/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
Hi, > It would be nice to have a seabios release with these patches > included in QEMU: make it easier for people to use hotplug. New release from master is planned already. cheers, Gerd
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
> Am 07.05.2014 um 11:26 schrieb Peter Maydell : > >> On 7 May 2014 10:09, Alexander Graf wrote: >> I don't think we should overengineer hacks for legacy virtio. > > Agreed. So what's our final conclusion: virtio endianness > is the endianness of the guest kernel at the point where > it triggers a reset of the virtio device, yes? The interrupt endianness for book3s PPC. Since that's an arch specific thing I think we should just make the determination mechanism arch dependent and list it in the spec. Booke for example would be vastly different since there is no global LE flag - it's a bit in the TLB entries. Alex
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On 7 May 2014 10:37, Alexander Graf wrote: > > >> Am 07.05.2014 um 11:26 schrieb Peter Maydell : >> >>> On 7 May 2014 10:09, Alexander Graf wrote: >>> I don't think we should overengineer hacks for legacy virtio. >> >> Agreed. So what's our final conclusion: virtio endianness >> is the endianness of the guest kernel at the point where >> it triggers a reset of the virtio device, yes? > > The interrupt endianness for book3s PPC. Since that's an arch > specific thing I think we should just make the determination > mechanism arch dependent and list it in the spec. > > Booke for example would be vastly different since there is no > global LE flag - it's a bit in the TLB entries. Sure, but I think we should also state the general principle we're aiming to implement with the arch specific detail, so that people figuring out what a new arch should do have a guide to follow. thanks -- PMM
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
> Am 07.05.2014 um 11:26 schrieb Peter Maydell : > >> On 7 May 2014 10:09, Alexander Graf wrote: >> I don't think we should overengineer hacks for legacy virtio. > > Agreed. So what's our final conclusion: virtio endianness > is the endianness of the guest kernel at the point where > it triggers a reset of the virtio device, yes? I just realized we're talking about virtio in a non-virtio thread. This patch set is about core dump support which is different from virtio bi-endian support. While both may end up at the same logic, I don't like the idea to mix them. This function is PPC internal. Alex > > thanks > -- PMM
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
> Am 07.05.2014 um 11:40 schrieb Peter Maydell : > >> On 7 May 2014 10:37, Alexander Graf wrote: >> >> Am 07.05.2014 um 11:26 schrieb Peter Maydell : On 7 May 2014 10:09, Alexander Graf wrote: I don't think we should overengineer hacks for legacy virtio. >>> >>> Agreed. So what's our final conclusion: virtio endianness >>> is the endianness of the guest kernel at the point where >>> it triggers a reset of the virtio device, yes? >> >> The interrupt endianness for book3s PPC. Since that's an arch >> specific thing I think we should just make the determination >> mechanism arch dependent and list it in the spec. >> >> Booke for example would be vastly different since there is no >> global LE flag - it's a bit in the TLB entries. > > Sure, but I think we should also state the general principle > we're aiming to implement with the arch specific detail, > so that people figuring out what a new arch should do > have a guide to follow. The general principle is "Try to find the Linux guest compile time endianness" :). Alex
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi, all
Ping...anyone? Thanks!
Best regards,
-Gonglei
> -Original Message-
> From: Gonglei (Arei)
> Sent: Sunday, May 04, 2014 5:25 PM
> To: qemu-devel@nongnu.org; xen-de...@lists.xen.org
> Cc: ian.campb...@citrix.com; jbeul...@suse.com;
> stefano.stabell...@eu.citrix.com; fabio.fant...@m2r.biz;
> anthony.per...@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei
> (UVP); Gonglei (Arei)
> Subject: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for
> PCIslots that support hotplug by runtime patching
>
> From: Gaowei
>
> In Xen platform, after using upstream qemu, the all of pci devices will show
> hotplug in the windows guest. In this situation, the windows guest may occur
> blue screen when VM' user click the icon of VGA card for trying unplug VGA
> card.
> However, we don't hope VM's user can do such dangerous operation, and
> showing
> all pci devices inside the guest OS is unfriendly.
>
> This is done by runtime patching:
> - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that this has the
> same checksum, but is ignored by OSPM.
> - At compile time, look for these methods in ASL source,find the matching
> AML,
> and store the offsets of these methods in a table named aml_ej0_data.
> - At run time, go over aml_ej0_data, check which slots not support hotplug
> and
> patch the ACPI table, replacing _EJ0 with EJ0_.
>
> Signed-off-by: Gaowei
> Signed-off-by: Gonglei
> ---
> v3->v2:
> reformat on the latest master
> v2->v1:
> rework by Jan Beulich's suggestion:
> - adjust the code style
>
> tools/firmware/hvmloader/acpi/Makefile | 44 ++-
> tools/firmware/hvmloader/acpi/acpi2_0.h| 4 +
> tools/firmware/hvmloader/acpi/build.c | 22 ++
> tools/firmware/hvmloader/acpi/dsdt.asl | 1 +
> tools/firmware/hvmloader/acpi/mk_dsdt.c| 2 +
> tools/firmware/hvmloader/ovmf.c| 6 +-
> tools/firmware/hvmloader/rombios.c | 4 +
> tools/firmware/hvmloader/seabios.c | 8 +
> tools/firmware/hvmloader/tools/acpi_extract.py | 308
> +
> .../hvmloader/tools/acpi_extract_preprocess.py | 41 +++
> 10 files changed, 428 insertions(+), 12 deletions(-)
> create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py
> create mode 100644
> tools/firmware/hvmloader/tools/acpi_extract_preprocess.py
>
> diff --git a/tools/firmware/hvmloader/acpi/Makefile
> b/tools/firmware/hvmloader/acpi/Makefile
> index 2c50851..f79ecc3 100644
> --- a/tools/firmware/hvmloader/acpi/Makefile
> +++ b/tools/firmware/hvmloader/acpi/Makefile
> @@ -24,30 +24,46 @@ OBJS = $(patsubst %.c,%.o,$(C_SRC))
> CFLAGS += $(CFLAGS_xeninclude)
>
> vpath iasl $(PATH)
> +vpath python $(PATH)
> +
> +.DELETE_ON_ERROR:$(filter dsdt_%.c,$(C_SRC))
> +
> all: acpi.a
>
> ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl
> iasl -vs -p $* -tc $<
> - sed -e 's/AmlCode/$*/g' $*.hex >$@
> + sed -e 's/AmlCode/$*/g' $*.hex >$@.tmp
> + $(call move-if-changed,$@.tmp $@)
> rm -f $*.hex $*.aml
>
> mk_dsdt: mk_dsdt.c
> $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
>
> dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
> - awk 'NR > 1 {print s} {s=$$0}' $< > $@
> - ./mk_dsdt --dm-version qemu-xen >> $@
> + awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp
> + sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@.tmp
> + ./mk_dsdt --dm-version qemu-xen >> $@.tmp
> + sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g'
> $@.tmp
> + sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g'
> $@.tmp
> + $(call move-if-changed,$@.tmp $@)
>
> # NB. awk invocation is a portable alternative to 'head -n -1'
> dsdt_%cpu.asl: dsdt.asl mk_dsdt
> - awk 'NR > 1 {print s} {s=$$0}' $< > $@
> - ./mk_dsdt --maxcpu $* >> $@
> + awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp
> + sed -i 's/AmlCode/dsdt_$*cpu/g' $@.tmp
> + ./mk_dsdt --maxcpu $* >> $@.tmp
> + $(call move-if-changed,$@.tmp $@)
>
> -$(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
> - iasl -vs -p $* -tc $*.asl
> - sed -e 's/AmlCode/$*/g' $*.hex >$@
> - echo "int $*_len=sizeof($*);" >>$@
> - rm -f $*.aml $*.hex
> +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
> + cpp -P $*.asl > $*.asl.i.orig
> + python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
> + iasl -vs -l -tc -p $* $*.asl.i
> + python ../tools/acpi_extract.py $*.lst > $@.tmp
> + echo "int $*_len=sizeof($*);" >>$@.tmp
> + if grep -q "$*_aml_ej0_name" $@.tmp; then echo "int
> $*_aml_ej0_name_len=sizeof($*_aml_ej0_name);" >>$@.tmp; fi
> + if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int
> $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
> + $(call move-if-changed,$@.tmp $@)
> + rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
>
> iasl:
> @echo
> @@ -57,6 +73,12 @@ iasl:
Re: [Qemu-devel] [RFC PATCH v2 05/16] Header/constant/types fixes for visitors
On Wed, Apr 23, 2014 at 05:37:38PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> Move constants around and add types to allow file structure to move into
> visitors.
>
> Signed-off-by: Dr. David Alan Gilbert
> ---
> arch_init.c | 12
> include/migration/migration.h | 17 +
> include/migration/vmstate.h | 20 +---
> include/qemu/typedefs.h | 4 ++--
> 4 files changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 60c975d..73b9303 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -110,18 +110,6 @@ static bool mig_throttle_on;
> static int dirty_rate_high_cnt;
> static void check_guest_throttling(void);
>
> -/***/
> -/* ram save/restore */
> -
> -#define RAM_SAVE_FLAG_FULL 0x01 /* Obsolete, not used anymore */
> -#define RAM_SAVE_FLAG_COMPRESS 0x02
> -#define RAM_SAVE_FLAG_MEM_SIZE 0x04
> -#define RAM_SAVE_FLAG_PAGE 0x08
> -#define RAM_SAVE_FLAG_EOS 0x10
> -#define RAM_SAVE_FLAG_CONTINUE 0x20
> -#define RAM_SAVE_FLAG_XBZRLE 0x40
> -/* 0x80 is reserved in migration.h start with 0x100 next */
> -
> static struct defconfig_file {
> const char *filename;
> /* Indicates it is an user config file (disabled by -no-user-config) */
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 3e1e6c7..825 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -165,7 +165,16 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags);
> * side. This lets before_ram_iterate/after_ram_iterate add
> * transport-specific sections to the RAM migration data.
> */
> +/* ram save/restore */
> +#define RAM_SAVE_FLAG_FULL 0x01 /* Obsolete, not used anymore */
> +#define RAM_SAVE_FLAG_COMPRESS 0x02
> +#define RAM_SAVE_FLAG_MEM_SIZE 0x04
> +#define RAM_SAVE_FLAG_PAGE 0x08
> +#define RAM_SAVE_FLAG_EOS 0x10
> +#define RAM_SAVE_FLAG_CONTINUE 0x20
> +#define RAM_SAVE_FLAG_XBZRLE 0x40
> #define RAM_SAVE_FLAG_HOOK 0x80
> +#define RAM_SAVE_FLAG_MASK0x1ff
>
> #define RAM_SAVE_CONTROL_NOT_SUPP -1000
> #define RAM_SAVE_CONTROL_DELAYED -2000
> @@ -174,4 +183,12 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
> block_offset,
> ram_addr_t offset, size_t size,
> int *bytes_sent);
>
> +typedef struct {
> +uint64_t addr;
> +uint16_t flags;
> +char idstr[256];
> +char ch; /* Used for filled pages (normally 0 fill) */
> +size_t len; /* Uses include xbzrle's data len */
> +} ramsecentry_header;
> +
RamSecEntryHeader?
and maybe we should make this 256 a named constant too.
> #endif
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index e7e1705..a5e4b0b 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -26,6 +26,7 @@
> #ifndef QEMU_VMSTATE_H
> #define QEMU_VMSTATE_H 1
>
> +#include "qemu/typedefs.h"
> #ifndef CONFIG_USER_ONLY
> #include
> #endif
> @@ -49,15 +50,27 @@ typedef struct SaveVMHandlers {
> * use data that is local to the migration thread or protected
> * by other locks.
> */
> -int (*save_live_iterate)(QEMUFile *f, void *opaque);
> +int (*save_live_iterate)(Visitor *v, void *opaque);
>
> /* This runs outside the iothread lock! */
> -int (*save_live_setup)(QEMUFile *f, void *opaque);
> -uint64_t (*save_live_pending)(QEMUFile *f, void *opaque, uint64_t
> max_size);
> +int (*save_live_setup)(Visitor *v, void *opaque);
> +uint64_t (*save_live_pending)(void *opaque, uint64_t max_size);
>
> LoadStateHandler *load_state;
> } SaveVMHandlers;
>
> +/* This is the data used to identify a section as passed
> + * into the section version of the compat sequence visitor
> + * (TODO: Probably want to move the whole name lookup into there
> + *and keep the section_id wrapped inside the binary visitor)
> + */
> +typedef struct SectionHeader {
> +uint32_t section_id;
> +uint32_t instance_id; /* Below only used for full version */
> +uint32_t version_id;
> +char idstr[256];
> +} SectionHeader;
> +
> int register_savevm(DeviceState *dev,
> const char *idstr,
> int instance_id,
> @@ -134,6 +147,7 @@ struct VMStateDescription {
> void (*pre_save)(void *opaque);
> VMStateField *fields;
> const VMStateSubsection *subsections;
> +uint32_t ber_tag;
> };
>
> #ifdef CONFIG_USER_ONLY
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index bf8daac..3fea88e 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -10,8 +10,6 @@ typedef struct QEMUBH QEMUBH;
>
> typedef struct AioContext AioContext;
>
> -typedef struct Visitor Visitor;
> -
> struct Monitor;
> typedef struct Monitor M
Re: [Qemu-devel] [RFC PATCH v2 01/16] Visitor: Add methods for migration format use
On Wed, Apr 23, 2014 at 05:37:34PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> array types
> From https://lists.gnu.org/archive/html/qemu-devel/2011-09/msg02465.html
>
> str256 type
> For the upto 256byte strings QEMU commonly uses for IDs
>
> buffer type
> For a blob of data that the caller wants to deliver whole (e.g.
> a page of RAM or block of disk)
>
> Load/save flags to let a user perform pre-save/post-load checking
>
> An accessor to get the underlying QEMUFile* (for compatibility)
>
> compat-sequences
> Provide enough information for a visitor providing compatibility
> with the old format to generate it's byte stream, while allowing a new
> visitor to do something sensible.
>
> destroy
> Allows the caller to destroy a visitor without knowing what type of
> visitor it is.
>
> Signed-off-by: Dr. David Alan Gilbert
> ---
> include/qapi/visitor-impl.h | 23 +
> include/qapi/visitor.h | 51 +
> qapi/qapi-visit-core.c | 80
> +
> 3 files changed, 154 insertions(+)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index f3fa420..10cdbf7 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -15,6 +15,9 @@
> #include "qapi/error.h"
> #include "qapi/visitor.h"
>
> +#define VISITOR_SAVING (1<<0)
> +#define VISITOR_LOADING (1<<1)
> +
> struct Visitor
> {
> /* Must be set */
> @@ -29,6 +32,10 @@ struct Visitor
> void (*start_list)(Visitor *v, const char *name, Error **errp);
> GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp);
> void (*end_list)(Visitor *v, Error **errp);
> +void (*start_array)(Visitor *v, void **obj, const char *name,
> +size_t elem_count, size_t elem_size, Error **errp);
> +void (*next_array)(Visitor *v, Error **errp);
> +void (*end_array)(Visitor *v, Error **errp);
>
> void (*type_enum)(Visitor *v, int *obj, const char *strings[],
>const char *kind, const char *name, Error **errp);
> @@ -38,6 +45,7 @@ struct Visitor
> void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error
> **errp);
> void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp);
> void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
> +void (*type_str256)(Visitor *v, char *obj, const char *name, Error
> **errp);
> void (*type_number)(Visitor *v, double *obj, const char *name,
> Error **errp);
>
> @@ -49,6 +57,8 @@ struct Visitor
> void (*start_handle)(Visitor *v, void **obj, const char *kind,
> const char *name, Error **errp);
> void (*end_handle)(Visitor *v, Error **errp);
> +void (*type_buffer)(Visitor *v, void *data, size_t len, bool async,
> +const char *name, Error **errp);
> void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error
> **errp);
> void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error
> **errp);
> void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error
> **errp);
> @@ -59,6 +69,19 @@ struct Visitor
> void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error
> **errp);
> /* visit_type_size() falls back to (*type_uint64)() if type_size is
> unset */
> void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error
> **errp);
> +
> +void (*destroy)(Visitor *v, Error **errp);
> +
> +void (*start_sequence_compat)(Visitor *v, const char *name,
> + Visit_seq_compat_mode compat_mode,
> + void *opaque, Error **errp);
> +void (*end_sequence_compat)(Visitor *v, const char *name,
> +Visit_seq_compat_mode compat_mode,
> +Error **errp);
> +
> +QEMUFile* (*get_qemufile)(Visitor *v);
> +
> +uint64_t flags;
> };
>
> void input_type_enum(Visitor *v, int *obj, const char *strings[],
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 29da211..70c20df 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -39,11 +39,22 @@ void visit_end_implicit_struct(Visitor *v, Error **errp);
> void visit_start_list(Visitor *v, const char *name, Error **errp);
> GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
> void visit_end_list(Visitor *v, Error **errp);
> +void visit_start_array(Visitor *v, void **obj, const char *name,
> + size_t elem_count, size_t elem_size, Error **errp);
> +void visit_next_array(Visitor *v, Error **errp);
> +void visit_end_array(Visitor *v, Error **errp);
> +
> void visit_start_optional(Visitor *v, bool *present, const char *name,
>Error **errp);
> void vi
[Qemu-devel] [PATCH v27 01/33] QemuOpts: move find_desc_by_name ahead for later calling
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Signed-off-by: Chunyan Liu
---
util/qemu-option.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 8bbc3ad..808aef4 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -173,6 +173,20 @@ static void parse_option_number(const char *name, const
char *value,
}
}
+static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
+const char *name)
+{
+int i;
+
+for (i = 0; desc[i].name != NULL; i++) {
+if (strcmp(desc[i].name, name) == 0) {
+return &desc[i];
+}
+}
+
+return NULL;
+}
+
void parse_option_size(const char *name, const char *value,
uint64_t *ret, Error **errp)
{
@@ -637,20 +651,6 @@ static bool opts_accepts_any(const QemuOpts *opts)
return opts->list->desc[0].name == NULL;
}
-static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
-const char *name)
-{
-int i;
-
-for (i = 0; desc[i].name != NULL; i++) {
-if (strcmp(desc[i].name, name) == 0) {
-return &desc[i];
-}
-}
-
-return NULL;
-}
-
int qemu_opt_unset(QemuOpts *opts, const char *name)
{
QemuOpt *opt = qemu_opt_find(opts, name);
--
1.7.12.4
[Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts
This patch series is to replace QEMUOptionParameter with QemuOpts, so that only one Qemu Option structure is kept in QEMU code. --- Changes to v26: * Following Eric's comment, backward split 2/33, 3/33. (repurpose qemu_opts_print first, add def_value_str to QemuOptDesc later). * Fix memory free in qemu_opts_append to solve iotest issue. 10/33 * Following Eric's comment, remove the end '.' in error message. And update qemu-iotests .out file. 12/33 * Following Eric's comment, fix memory free in vvfat.c 13/33 * Following Eric's comment, split qcow2 patch into two. 19/33, 20/33: export qemu_opt_find first, add qcow2 driver patch later. * rebase to git master All patches are also available from: https://github.com/chunyanliu/qemu/commits/QemuOpts Chunyan Liu (33): QemuOpts: move find_desc_by_name ahead for later calling QemuOpts: repurpose qemu_opts_print to replace print_option_parameters QemuOpts: add def_value_str to QemuOptDesc qapi: output def_value_str when query command line options QemuOpts: change opt->name|str from (const char *) to (char *) QemuOpts: move qemu_opt_del ahead for later calling QemuOpts: add qemu_opt_get_*_del functions for replace work QemuOpts: add qemu_opts_print_help to replace print_option_help QemuOpts: add conversion between QEMUOptionParameter to QemuOpts QemuOpts: add qemu_opts_append to replace append_option_parameters QemuOpts: check NULL input for qemu_opts_del change block layer to support both QemuOpts and QEMUOptionParamter vvfat.c: handle cross_driver's create_options and create_opts cow.c: replace QEMUOptionParameter with QemuOpts gluster.c: replace QEMUOptionParameter with QemuOpts iscsi.c: replace QEMUOptionParameter with QemuOpts nfs.c: replace QEMUOptionParameter with QemuOpts qcow.c: replace QEMUOptionParameter with QemuOpts QemuOpts: export qemu_opt_find qcow2.c: replace QEMUOptionParameter with QemuOpts qed.c: replace QEMUOptionParameter with QemuOpts raw-posix.c: replace QEMUOptionParameter with QemuOpts raw-win32.c: replace QEMUOptionParameter with QemuOpts raw_bsd.c: replace QEMUOptionParameter with QemuOpts rbd.c: replace QEMUOptionParameter with QemuOpts sheepdog.c: replace QEMUOptionParameter with QemuOpts ssh.c: replace QEMUOptionParameter with QemuOpts vdi.c: replace QEMUOptionParameter with QemuOpts vhdx.c: replace QEMUOptionParameter with QemuOpts vmdk.c: replace QEMUOptionParameter with QemuOpts vpc.c: replace QEMUOptionParameter with QemuOpts cleanup QEMUOptionParameter QemuOpts: cleanup tmp 'allocated' member from QemuOptsList block.c| 99 block/cow.c| 52 ++-- block/gluster.c| 73 +++--- block/iscsi.c | 32 ++- block/nfs.c| 10 +- block/qcow.c | 72 +++--- block/qcow2.c | 259 ++-- block/qed.c| 112 + block/qed.h| 3 +- block/raw-posix.c | 55 ++--- block/raw-win32.c | 38 +-- block/raw_bsd.c| 25 +- block/rbd.c| 61 +++-- block/sheepdog.c | 102 block/ssh.c| 30 ++- block/vdi.c| 71 +++--- block/vhdx.c | 97 block/vhdx.h | 1 + block/vmdk.c | 121 +- block/vpc.c| 60 ++--- block/vvfat.c | 13 +- include/block/block.h | 7 +- include/block/block_int.h | 9 +- include/qemu/option.h | 53 +--- include/qemu/option_int.h | 4 +- qapi-schema.json | 5 +- qapi/opts-visitor.c| 10 +- qemu-img.c | 89 +++ qmp-commands.hx| 2 + tests/qemu-iotests/049.out | 2 +- tests/qemu-iotests/061.out | 2 +- util/qemu-config.c | 4 + util/qemu-option.c | 588 - 33 files changed, 1033 insertions(+), 1128 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH v27 05/33] QemuOpts: change opt->name|str from (const char *) to (char *)
qemu_opt_del() already assumes that all QemuOpt instances contain
malloc'd name and value; but it had to cast away const because
opts_start_struct() was doing its own thing and using static storage
instead. By using the correct type and malloced strings everywhere, the
usage of this struct becomes clearer.
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Chunyan Liu
---
include/qemu/option_int.h | 4 ++--
qapi/opts-visitor.c | 10 +++---
util/qemu-option.c| 4 ++--
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
index 8212fa4..6432c1a 100644
--- a/include/qemu/option_int.h
+++ b/include/qemu/option_int.h
@@ -30,8 +30,8 @@
#include "qemu/error-report.h"
struct QemuOpt {
-const char *name;
-const char *str;
+char *name;
+char *str;
const QemuOptDesc *desc;
union {
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 5d830a2..7a64f4e 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -143,8 +143,8 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
if (ov->opts_root->id != NULL) {
ov->fake_id_opt = g_malloc0(sizeof *ov->fake_id_opt);
-ov->fake_id_opt->name = "id";
-ov->fake_id_opt->str = ov->opts_root->id;
+ov->fake_id_opt->name = g_strdup("id");
+ov->fake_id_opt->str = g_strdup(ov->opts_root->id);
opts_visitor_insert(ov->unprocessed_opts, ov->fake_id_opt);
}
}
@@ -177,7 +177,11 @@ opts_end_struct(Visitor *v, Error **errp)
}
g_hash_table_destroy(ov->unprocessed_opts);
ov->unprocessed_opts = NULL;
-g_free(ov->fake_id_opt);
+if (ov->fake_id_opt) {
+g_free(ov->fake_id_opt->name);
+g_free(ov->fake_id_opt->str);
+g_free(ov->fake_id_opt);
+}
ov->fake_id_opt = NULL;
}
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 2be6995..69cdf3f 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -664,8 +664,8 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
static void qemu_opt_del(QemuOpt *opt)
{
QTAILQ_REMOVE(&opt->opts->head, opt, next);
-g_free((/* !const */ char*)opt->name);
-g_free((/* !const */ char*)opt->str);
+g_free(opt->name);
+g_free(opt->str);
g_free(opt);
}
--
1.7.12.4
[Qemu-devel] [PATCH v27 10/33] QemuOpts: add qemu_opts_append to replace append_option_parameters
For later merge .create_opts of drv and proto_drv in qemu-img commands.
Signed-off-by: Chunyan Liu
---
Changes:
* fix memory free:
if (param) {
- g_free(list);
+ qemu_opts_free(list); //free allocated memory too
}
include/qemu/option.h | 5
util/qemu-option.c| 67 +++
2 files changed, 72 insertions(+)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index e98e8ef..44d9961 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -176,5 +176,10 @@ void qemu_opts_print_help(QemuOptsList *list);
void qemu_opts_free(QemuOptsList *list);
QEMUOptionParameter *opts_to_params(QemuOpts *opts);
QemuOptsList *params_to_opts(QEMUOptionParameter *list);
+/* FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
+ */
+QemuOptsList *qemu_opts_append(QemuOptsList *dst,
+ QemuOptsList *list,
+ QEMUOptionParameter *param);
#endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 93ffcd5..e15723a 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1498,3 +1498,70 @@ void qemu_opts_free(QemuOptsList *list)
g_free(list);
}
+
+/* Realloc dst option list and append options either from an option list (list)
+ * or a QEMUOptionParameter (param) to it. dst could be NULL or a malloced
list.
+ * FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
+ */
+QemuOptsList *qemu_opts_append(QemuOptsList *dst,
+ QemuOptsList *list,
+ QEMUOptionParameter *param)
+{
+size_t num_opts, num_dst_opts;
+QemuOptDesc *desc;
+bool need_init = false;
+
+assert(!(list && param));
+if (!param && !list) {
+return dst;
+}
+
+if (param) {
+list = params_to_opts(param);
+}
+
+/* If dst is NULL, after realloc, some area of dst should be initialized
+ * before adding options to it.
+ */
+if (!dst) {
+need_init = true;
+}
+
+num_opts = count_opts_list(dst);
+num_dst_opts = num_opts;
+num_opts += count_opts_list(list);
+dst = g_realloc(dst, sizeof(QemuOptsList) +
+(num_opts + 1) * sizeof(QemuOptDesc));
+if (need_init) {
+dst->name = NULL;
+dst->implied_opt_name = NULL;
+QTAILQ_INIT(&dst->head);
+dst->allocated = true;
+dst->merge_lists = false;
+}
+dst->desc[num_dst_opts].name = NULL;
+
+/* (const char *) members of result dst are malloced, need free. */
+assert(dst->allocated);
+/* append list->desc to dst->desc */
+if (list) {
+desc = list->desc;
+while (desc && desc->name) {
+if (find_desc_by_name(dst->desc, desc->name) == NULL) {
+dst->desc[num_dst_opts].name = g_strdup(desc->name);
+dst->desc[num_dst_opts].type = desc->type;
+dst->desc[num_dst_opts].help = g_strdup(desc->help);
+dst->desc[num_dst_opts].def_value_str =
+ g_strdup(desc->def_value_str);
+num_dst_opts++;
+dst->desc[num_dst_opts].name = NULL;
+}
+desc++;
+}
+}
+
+if (param) {
+qemu_opts_free(list);
+}
+return dst;
+}
--
1.7.12.4
[Qemu-devel] [PATCH v27 04/33] qapi: output def_value_str when query command line options
Change qapi interfaces to output the newly added def_value_str when querying
command line options.
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
qapi-schema.json | 5 -
qmp-commands.hx| 2 ++
util/qemu-config.c | 4
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 0b00427..880829d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4088,12 +4088,15 @@
#
# @help: #optional human readable text string, not suitable for parsing.
#
+# @default: #optional default value string (since 2.1)
+#
# Since 1.5
##
{ 'type': 'CommandLineParameterInfo',
'data': { 'name': 'str',
'type': 'CommandLineParameterType',
-'*help': 'str' } }
+'*help': 'str',
+'*default': 'str' } }
##
# @CommandLineOptionInfo:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ed3ab92..1271332 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2895,6 +2895,8 @@ Each array entry contains the following:
or 'size')
- "help": human readable description of the parameter
(json-string, optional)
+- "default": default value string for the parameter
+ (json-string, optional)
Example:
diff --git a/util/qemu-config.c b/util/qemu-config.c
index f4e4f38..ba375c0 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -82,6 +82,10 @@ static CommandLineParameterInfoList
*query_option_descs(const QemuOptDesc *desc)
info->has_help = true;
info->help = g_strdup(desc[i].help);
}
+if (desc[i].def_value_str) {
+info->has_q_default = true;
+info->q_default = g_strdup(desc[i].def_value_str);
+}
entry = g_malloc0(sizeof(*entry));
entry->value = info;
--
1.7.12.4
[Qemu-devel] [PATCH v27 06/33] QemuOpts: move qemu_opt_del ahead for later calling
In later patch, qemu_opt_get_del functions will be added, they will
first get the option value, then call qemu_opt_del to remove the option
from opt list. To prepare for that purpose, move qemu_opt_del ahead first.
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Chunyan Liu
---
util/qemu-option.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 69cdf3f..4d2d4d1 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -567,6 +567,14 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char
*name)
return NULL;
}
+static void qemu_opt_del(QemuOpt *opt)
+{
+QTAILQ_REMOVE(&opt->opts->head, opt, next);
+g_free(opt->name);
+g_free(opt->str);
+g_free(opt);
+}
+
const char *qemu_opt_get(QemuOpts *opts, const char *name)
{
QemuOpt *opt = qemu_opt_find(opts, name);
@@ -661,14 +669,6 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
}
}
-static void qemu_opt_del(QemuOpt *opt)
-{
-QTAILQ_REMOVE(&opt->opts->head, opt, next);
-g_free(opt->name);
-g_free(opt->str);
-g_free(opt);
-}
-
static bool opts_accepts_any(const QemuOpts *opts)
{
return opts->list->desc[0].name == NULL;
--
1.7.12.4
[Qemu-devel] [PATCH v27 02/33] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters
Currently this function is not used anywhere. In later patches, it will
replace print_option_parameters. To avoid print info changes, change
qemu_opts_print from fprintf stderr to printf, and remove last printf.
Signed-off-by: Chunyan Liu
---
Changes:
* backward split than v26 (2/33, 3/33).
include/qemu/option.h | 2 +-
util/qemu-option.c| 10 --
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 8c0ac34..1077b69 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -156,7 +156,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp);
typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
-int qemu_opts_print(QemuOpts *opts, void *dummy);
+void qemu_opts_print(QemuOpts *opts);
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void
*opaque,
int abort_on_failure);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 808aef4..290ed54 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -895,17 +895,15 @@ void qemu_opts_del(QemuOpts *opts)
g_free(opts);
}
-int qemu_opts_print(QemuOpts *opts, void *dummy)
+void qemu_opts_print(QemuOpts *opts)
{
QemuOpt *opt;
-fprintf(stderr, "%s: %s:", opts->list->name,
-opts->id ? opts->id : "");
+printf("%s: %s:", opts->list->name,
+ opts->id ? opts->id : "");
QTAILQ_FOREACH(opt, &opts->head, next) {
-fprintf(stderr, " %s=\"%s\"", opt->name, opt->str);
+printf(" %s=\"%s\"", opt->name, opt->str);
}
-fprintf(stderr, "\n");
-return 0;
}
static int opts_do_parse(QemuOpts *opts, const char *params,
--
1.7.12.4
[Qemu-devel] [PATCH v27 03/33] QemuOpts: add def_value_str to QemuOptDesc
Add def_value_str (default value) to QemuOptDesc, to replace function of the
default value in QEMUOptionParameter.
Improve qemu_opts_get_* functions: if find opt, return opt->str; otherwise,
if desc->def_value_str is set, return desc->def_value_str; otherwise, return
input defval.
Improve qemu_opts_print: if option is set, print opt->str; otherwise, if
desc->def_value_str is set, print it.
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
Changes:
* backward split than v26. (2/33, 3/33)
include/qemu/option.h | 1 +
util/qemu-option.c| 56 ---
2 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 1077b69..c3b0a91 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -99,6 +99,7 @@ typedef struct QemuOptDesc {
const char *name;
enum QemuOptType type;
const char *help;
+const char *def_value_str;
} QemuOptDesc;
struct QemuOptsList {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 290ed54..2be6995 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -570,6 +570,13 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char
*name)
const char *qemu_opt_get(QemuOpts *opts, const char *name)
{
QemuOpt *opt = qemu_opt_find(opts, name);
+
+if (!opt) {
+const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+return desc->def_value_str;
+}
+}
return opt ? opt->str : NULL;
}
@@ -589,8 +596,13 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name,
bool defval)
{
QemuOpt *opt = qemu_opt_find(opts, name);
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+parse_option_bool(name, desc->def_value_str, &defval,
&error_abort);
+}
return defval;
+}
assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL);
return opt->value.boolean;
}
@@ -599,8 +611,14 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char
*name, uint64_t defval)
{
QemuOpt *opt = qemu_opt_find(opts, name);
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+parse_option_number(name, desc->def_value_str, &defval,
+&error_abort);
+}
return defval;
+}
assert(opt->desc && opt->desc->type == QEMU_OPT_NUMBER);
return opt->value.uint;
}
@@ -609,8 +627,13 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char
*name, uint64_t defval)
{
QemuOpt *opt = qemu_opt_find(opts, name);
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+parse_option_size(name, desc->def_value_str, &defval,
&error_abort);
+}
return defval;
+}
assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE);
return opt->value.uint;
}
@@ -898,11 +921,30 @@ void qemu_opts_del(QemuOpts *opts)
void qemu_opts_print(QemuOpts *opts)
{
QemuOpt *opt;
+QemuOptDesc *desc = opts->list->desc;
-printf("%s: %s:", opts->list->name,
- opts->id ? opts->id : "");
-QTAILQ_FOREACH(opt, &opts->head, next) {
-printf(" %s=\"%s\"", opt->name, opt->str);
+if (desc[0].name == NULL) {
+QTAILQ_FOREACH(opt, &opts->head, next) {
+printf("%s=\"%s\" ", opt->name, opt->str);
+}
+return;
+}
+for (; desc && desc->name; desc++) {
+const char *value;
+QemuOpt *opt = qemu_opt_find(opts, desc->name);
+
+value = opt ? opt->str : desc->def_value_str;
+if (!value) {
+continue;
+}
+if (desc->type == QEMU_OPT_STRING) {
+printf("%s='%s' ", desc->name, value);
+} else if ((desc->type == QEMU_OPT_SIZE ||
+desc->type == QEMU_OPT_NUMBER) && opt) {
+printf("%s=%" PRId64 " ", desc->name, opt->value.uint);
+} else {
+printf("%s=%s ", desc->name, value);
+}
}
}
--
1.7.12.4
[Qemu-devel] [PATCH v27 09/33] QemuOpts: add conversion between QEMUOptionParameter to QemuOpts
Add two temp conversion functions between QEMUOptionParameter to QemuOpts,
so that next patch can use it. It will simplify later patch for easier
review. And will be finally removed after all backend drivers switch to
QemuOpts.
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Chunyan Liu
---
include/qemu/option.h | 9 +++
util/qemu-option.c| 153 ++
2 files changed, 162 insertions(+)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index fbf5dc2..e98e8ef 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -103,6 +103,12 @@ typedef struct QemuOptDesc {
} QemuOptDesc;
struct QemuOptsList {
+/* FIXME: Temp used for QEMUOptionParamter->QemuOpts conversion to
+ * indicate the need to free memory. Will remove after all drivers
+ * switch to QemuOpts.
+ */
+bool allocated;
+
const char *name;
const char *implied_opt_name;
bool merge_lists; /* Merge multiple uses of option into a single list? */
@@ -167,5 +173,8 @@ void qemu_opts_print(QemuOpts *opts);
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void
*opaque,
int abort_on_failure);
void qemu_opts_print_help(QemuOptsList *list);
+void qemu_opts_free(QemuOptsList *list);
+QEMUOptionParameter *opts_to_params(QemuOpts *opts);
+QemuOptsList *params_to_opts(QEMUOptionParameter *list);
#endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index adb7c3c..93ffcd5 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1345,3 +1345,156 @@ int qemu_opts_foreach(QemuOptsList *list,
qemu_opts_loopfunc func, void *opaque,
loc_pop(&loc);
return rc;
}
+
+static size_t count_opts_list(QemuOptsList *list)
+{
+QemuOptDesc *desc = NULL;
+size_t num_opts = 0;
+
+if (!list) {
+return 0;
+}
+
+desc = list->desc;
+while (desc && desc->name) {
+num_opts++;
+desc++;
+}
+
+return num_opts;
+}
+
+/* Convert QEMUOptionParameter to QemuOpts
+ * FIXME: this function will be removed after all drivers
+ * switch to QemuOpts
+ */
+QemuOptsList *params_to_opts(QEMUOptionParameter *list)
+{
+QemuOptsList *opts = NULL;
+size_t num_opts, i = 0;
+
+if (!list) {
+return NULL;
+}
+
+num_opts = count_option_parameters(list);
+opts = g_malloc0(sizeof(QemuOptsList) +
+ (num_opts + 1) * sizeof(QemuOptDesc));
+QTAILQ_INIT(&opts->head);
+/* (const char *) members will point to malloced space and need to free */
+opts->allocated = true;
+
+while (list && list->name) {
+opts->desc[i].name = g_strdup(list->name);
+opts->desc[i].help = g_strdup(list->help);
+switch (list->type) {
+case OPT_FLAG:
+opts->desc[i].type = QEMU_OPT_BOOL;
+opts->desc[i].def_value_str =
+g_strdup(list->value.n ? "on" : "off");
+break;
+
+case OPT_NUMBER:
+opts->desc[i].type = QEMU_OPT_NUMBER;
+if (list->value.n) {
+opts->desc[i].def_value_str =
+g_strdup_printf("%" PRIu64, list->value.n);
+}
+break;
+
+case OPT_SIZE:
+opts->desc[i].type = QEMU_OPT_SIZE;
+if (list->value.n) {
+opts->desc[i].def_value_str =
+g_strdup_printf("%" PRIu64, list->value.n);
+}
+break;
+
+case OPT_STRING:
+opts->desc[i].type = QEMU_OPT_STRING;
+opts->desc[i].def_value_str = g_strdup(list->value.s);
+break;
+}
+
+i++;
+list++;
+}
+
+return opts;
+}
+
+/* convert QemuOpts to QEMUOptionParameter
+ * Note: result QEMUOptionParameter has shorter lifetime than
+ * input QemuOpts.
+ * FIXME: this function will be removed after all drivers
+ * switch to QemuOpts
+ */
+QEMUOptionParameter *opts_to_params(QemuOpts *opts)
+{
+QEMUOptionParameter *dest = NULL;
+QemuOptDesc *desc;
+size_t num_opts, i = 0;
+const char *tmp;
+
+if (!opts || !opts->list || !opts->list->desc) {
+return NULL;
+}
+assert(!opts_accepts_any(opts));
+
+num_opts = count_opts_list(opts->list);
+dest = g_malloc0((num_opts + 1) * sizeof(QEMUOptionParameter));
+
+desc = opts->list->desc;
+while (desc && desc->name) {
+dest[i].name = desc->name;
+dest[i].help = desc->help;
+dest[i].assigned = qemu_opt_find(opts, desc->name) ? true : false;
+switch (desc->type) {
+case QEMU_OPT_STRING:
+dest[i].type = OPT_STRING;
+tmp = qemu_opt_get(opts, desc->name);
+dest[i].value.s = g_strdup(tmp);
+break;
+
+case QEMU_OPT_BOOL:
+dest[i].type = OPT_FLAG;
+dest[i].value.n = qemu_opt_get_bool(opts, desc->name, 0) ? 1 : 0;
+break;
+
+c
[Qemu-devel] [PATCH v27 11/33] QemuOpts: check NULL input for qemu_opts_del
To simplify later using of qemu_opts_del, accept NULL input.
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Chunyan Liu
---
util/qemu-option.c | 4
1 file changed, 4 insertions(+)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index e15723a..71415ee 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1010,6 +1010,10 @@ void qemu_opts_del(QemuOpts *opts)
{
QemuOpt *opt;
+if (opts == NULL) {
+return;
+}
+
for (;;) {
opt = QTAILQ_FIRST(&opts->head);
if (opt == NULL)
--
1.7.12.4
[Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts
This patch series is to replace QEMUOptionParameter with QemuOpts, so that only one Qemu Option structure is kept in QEMU code. --- Changes to v26: * Following Eric's comment, backward split 2/33, 3/33. (repurpose qemu_opts_print first, add def_value_str to QemuOptDesc later). * Fix memory free in qemu_opts_append to solve iotest issue. 10/33 * Following Eric's comment, remove the end '.' in error message. And update qemu-iotests .out file. 12/33 * Following Eric's comment, fix memory free in vvfat.c 13/33 * Following Eric's comment, split qcow2 patch into two. 19/33, 20/33: export qemu_opt_find first, add qcow2 driver patch later. * rebase to git master All patches are also available from: https://github.com/chunyanliu/qemu/commits/QemuOpts Chunyan Liu (33): QemuOpts: move find_desc_by_name ahead for later calling QemuOpts: repurpose qemu_opts_print to replace print_option_parameters QemuOpts: add def_value_str to QemuOptDesc qapi: output def_value_str when query command line options QemuOpts: change opt->name|str from (const char *) to (char *) QemuOpts: move qemu_opt_del ahead for later calling QemuOpts: add qemu_opt_get_*_del functions for replace work QemuOpts: add qemu_opts_print_help to replace print_option_help QemuOpts: add conversion between QEMUOptionParameter to QemuOpts QemuOpts: add qemu_opts_append to replace append_option_parameters QemuOpts: check NULL input for qemu_opts_del change block layer to support both QemuOpts and QEMUOptionParamter vvfat.c: handle cross_driver's create_options and create_opts cow.c: replace QEMUOptionParameter with QemuOpts gluster.c: replace QEMUOptionParameter with QemuOpts iscsi.c: replace QEMUOptionParameter with QemuOpts nfs.c: replace QEMUOptionParameter with QemuOpts qcow.c: replace QEMUOptionParameter with QemuOpts QemuOpts: export qemu_opt_find qcow2.c: replace QEMUOptionParameter with QemuOpts qed.c: replace QEMUOptionParameter with QemuOpts raw-posix.c: replace QEMUOptionParameter with QemuOpts raw-win32.c: replace QEMUOptionParameter with QemuOpts raw_bsd.c: replace QEMUOptionParameter with QemuOpts rbd.c: replace QEMUOptionParameter with QemuOpts sheepdog.c: replace QEMUOptionParameter with QemuOpts ssh.c: replace QEMUOptionParameter with QemuOpts vdi.c: replace QEMUOptionParameter with QemuOpts vhdx.c: replace QEMUOptionParameter with QemuOpts vmdk.c: replace QEMUOptionParameter with QemuOpts vpc.c: replace QEMUOptionParameter with QemuOpts cleanup QEMUOptionParameter QemuOpts: cleanup tmp 'allocated' member from QemuOptsList block.c| 99 block/cow.c| 52 ++-- block/gluster.c| 73 +++--- block/iscsi.c | 32 ++- block/nfs.c| 10 +- block/qcow.c | 72 +++--- block/qcow2.c | 259 ++-- block/qed.c| 112 + block/qed.h| 3 +- block/raw-posix.c | 55 ++--- block/raw-win32.c | 38 +-- block/raw_bsd.c| 25 +- block/rbd.c| 61 +++-- block/sheepdog.c | 102 block/ssh.c| 30 ++- block/vdi.c| 71 +++--- block/vhdx.c | 97 block/vhdx.h | 1 + block/vmdk.c | 121 +- block/vpc.c| 60 ++--- block/vvfat.c | 13 +- include/block/block.h | 7 +- include/block/block_int.h | 9 +- include/qemu/option.h | 53 +--- include/qemu/option_int.h | 4 +- qapi-schema.json | 5 +- qapi/opts-visitor.c| 10 +- qemu-img.c | 89 +++ qmp-commands.hx| 2 + tests/qemu-iotests/049.out | 2 +- tests/qemu-iotests/061.out | 2 +- util/qemu-config.c | 4 + util/qemu-option.c | 588 - 33 files changed, 1033 insertions(+), 1128 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH v27 14/33] cow.c: replace QEMUOptionParameter with QemuOpts
Reviewed-by: Eric Blake
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
block/cow.c | 54 ++
1 file changed, 26 insertions(+), 28 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index 7e61024..af85753 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -324,31 +324,24 @@ static void cow_close(BlockDriverState *bs)
{
}
-static int cow_create(const char *filename, QEMUOptionParameter *options,
- Error **errp)
+static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
{
struct cow_header_v2 cow_header;
struct stat st;
int64_t image_sectors = 0;
-const char *image_filename = NULL;
+char *image_filename = NULL;
Error *local_err = NULL;
int ret;
BlockDriverState *cow_bs;
/* Read out options */
-while (options && options->name) {
-if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-image_sectors = options->value.n / 512;
-} else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
-image_filename = options->value.s;
-}
-options++;
-}
+image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
-ret = bdrv_create_file(filename, options, NULL, &local_err);
+ret = bdrv_create_file(filename, NULL, opts, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
-return ret;
+goto exit;
}
cow_bs = NULL;
@@ -356,7 +349,7 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options,
BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
-return ret;
+goto exit;
}
memset(&cow_header, 0, sizeof(cow_header));
@@ -389,22 +382,27 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options,
}
exit:
+g_free(image_filename);
bdrv_unref(cow_bs);
return ret;
}
-static QEMUOptionParameter cow_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = "Virtual disk size"
-},
-{
-.name = BLOCK_OPT_BACKING_FILE,
-.type = OPT_STRING,
-.help = "File name of a base image"
-},
-{ NULL }
+static QemuOptsList cow_create_opts = {
+.name = "cow-create-opts",
+.head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Virtual disk size"
+},
+{
+.name = BLOCK_OPT_BACKING_FILE,
+.type = QEMU_OPT_STRING,
+.help = "File name of a base image"
+},
+{ /* end of list */ }
+}
};
static BlockDriver bdrv_cow = {
@@ -414,14 +412,14 @@ static BlockDriver bdrv_cow = {
.bdrv_probe = cow_probe,
.bdrv_open = cow_open,
.bdrv_close = cow_close,
-.bdrv_create= cow_create,
+.bdrv_create2 = cow_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_read = cow_co_read,
.bdrv_write = cow_co_write,
.bdrv_co_get_block_status = cow_co_get_block_status,
-.create_options = cow_create_options,
+.create_opts= &cow_create_opts,
};
static void bdrv_cow_init(void)
--
1.7.12.4
[Qemu-devel] [PATCH v27 08/33] QemuOpts: add qemu_opts_print_help to replace print_option_help
print_option_help takes QEMUOptionParameter as parameter, add
qemu_opts_print_help to take QemuOptsList as parameter for later
replace work.
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
include/qemu/option.h | 1 +
util/qemu-option.c| 13 +
2 files changed, 14 insertions(+)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 6653e43..fbf5dc2 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -166,5 +166,6 @@ typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void
*opaque);
void qemu_opts_print(QemuOpts *opts);
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void
*opaque,
int abort_on_failure);
+void qemu_opts_print_help(QemuOptsList *list);
#endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 32e1d50..adb7c3c 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -553,6 +553,19 @@ void print_option_help(QEMUOptionParameter *list)
}
}
+void qemu_opts_print_help(QemuOptsList *list)
+{
+QemuOptDesc *desc;
+
+assert(list);
+desc = list->desc;
+printf("Supported options:\n");
+while (desc && desc->name) {
+printf("%-16s %s\n", desc->name,
+ desc->help ? desc->help : "No description available");
+desc++;
+}
+}
/* -- */
static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
--
1.7.12.4
[Qemu-devel] [PATCH v27 09/33] QemuOpts: add conversion between QEMUOptionParameter to QemuOpts
Add two temp conversion functions between QEMUOptionParameter to QemuOpts,
so that next patch can use it. It will simplify later patch for easier
review. And will be finally removed after all backend drivers switch to
QemuOpts.
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Chunyan Liu
---
include/qemu/option.h | 9 +++
util/qemu-option.c| 153 ++
2 files changed, 162 insertions(+)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index fbf5dc2..e98e8ef 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -103,6 +103,12 @@ typedef struct QemuOptDesc {
} QemuOptDesc;
struct QemuOptsList {
+/* FIXME: Temp used for QEMUOptionParamter->QemuOpts conversion to
+ * indicate the need to free memory. Will remove after all drivers
+ * switch to QemuOpts.
+ */
+bool allocated;
+
const char *name;
const char *implied_opt_name;
bool merge_lists; /* Merge multiple uses of option into a single list? */
@@ -167,5 +173,8 @@ void qemu_opts_print(QemuOpts *opts);
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void
*opaque,
int abort_on_failure);
void qemu_opts_print_help(QemuOptsList *list);
+void qemu_opts_free(QemuOptsList *list);
+QEMUOptionParameter *opts_to_params(QemuOpts *opts);
+QemuOptsList *params_to_opts(QEMUOptionParameter *list);
#endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index adb7c3c..93ffcd5 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1345,3 +1345,156 @@ int qemu_opts_foreach(QemuOptsList *list,
qemu_opts_loopfunc func, void *opaque,
loc_pop(&loc);
return rc;
}
+
+static size_t count_opts_list(QemuOptsList *list)
+{
+QemuOptDesc *desc = NULL;
+size_t num_opts = 0;
+
+if (!list) {
+return 0;
+}
+
+desc = list->desc;
+while (desc && desc->name) {
+num_opts++;
+desc++;
+}
+
+return num_opts;
+}
+
+/* Convert QEMUOptionParameter to QemuOpts
+ * FIXME: this function will be removed after all drivers
+ * switch to QemuOpts
+ */
+QemuOptsList *params_to_opts(QEMUOptionParameter *list)
+{
+QemuOptsList *opts = NULL;
+size_t num_opts, i = 0;
+
+if (!list) {
+return NULL;
+}
+
+num_opts = count_option_parameters(list);
+opts = g_malloc0(sizeof(QemuOptsList) +
+ (num_opts + 1) * sizeof(QemuOptDesc));
+QTAILQ_INIT(&opts->head);
+/* (const char *) members will point to malloced space and need to free */
+opts->allocated = true;
+
+while (list && list->name) {
+opts->desc[i].name = g_strdup(list->name);
+opts->desc[i].help = g_strdup(list->help);
+switch (list->type) {
+case OPT_FLAG:
+opts->desc[i].type = QEMU_OPT_BOOL;
+opts->desc[i].def_value_str =
+g_strdup(list->value.n ? "on" : "off");
+break;
+
+case OPT_NUMBER:
+opts->desc[i].type = QEMU_OPT_NUMBER;
+if (list->value.n) {
+opts->desc[i].def_value_str =
+g_strdup_printf("%" PRIu64, list->value.n);
+}
+break;
+
+case OPT_SIZE:
+opts->desc[i].type = QEMU_OPT_SIZE;
+if (list->value.n) {
+opts->desc[i].def_value_str =
+g_strdup_printf("%" PRIu64, list->value.n);
+}
+break;
+
+case OPT_STRING:
+opts->desc[i].type = QEMU_OPT_STRING;
+opts->desc[i].def_value_str = g_strdup(list->value.s);
+break;
+}
+
+i++;
+list++;
+}
+
+return opts;
+}
+
+/* convert QemuOpts to QEMUOptionParameter
+ * Note: result QEMUOptionParameter has shorter lifetime than
+ * input QemuOpts.
+ * FIXME: this function will be removed after all drivers
+ * switch to QemuOpts
+ */
+QEMUOptionParameter *opts_to_params(QemuOpts *opts)
+{
+QEMUOptionParameter *dest = NULL;
+QemuOptDesc *desc;
+size_t num_opts, i = 0;
+const char *tmp;
+
+if (!opts || !opts->list || !opts->list->desc) {
+return NULL;
+}
+assert(!opts_accepts_any(opts));
+
+num_opts = count_opts_list(opts->list);
+dest = g_malloc0((num_opts + 1) * sizeof(QEMUOptionParameter));
+
+desc = opts->list->desc;
+while (desc && desc->name) {
+dest[i].name = desc->name;
+dest[i].help = desc->help;
+dest[i].assigned = qemu_opt_find(opts, desc->name) ? true : false;
+switch (desc->type) {
+case QEMU_OPT_STRING:
+dest[i].type = OPT_STRING;
+tmp = qemu_opt_get(opts, desc->name);
+dest[i].value.s = g_strdup(tmp);
+break;
+
+case QEMU_OPT_BOOL:
+dest[i].type = OPT_FLAG;
+dest[i].value.n = qemu_opt_get_bool(opts, desc->name, 0) ? 1 : 0;
+break;
+
+c
[Qemu-devel] [PATCH v27 03/33] QemuOpts: add def_value_str to QemuOptDesc
Add def_value_str (default value) to QemuOptDesc, to replace function of the
default value in QEMUOptionParameter.
Improve qemu_opts_get_* functions: if find opt, return opt->str; otherwise,
if desc->def_value_str is set, return desc->def_value_str; otherwise, return
input defval.
Improve qemu_opts_print: if option is set, print opt->str; otherwise, if
desc->def_value_str is set, print it.
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
Changes:
* backward split than v26. (2/33, 3/33)
include/qemu/option.h | 1 +
util/qemu-option.c| 56 ---
2 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 1077b69..c3b0a91 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -99,6 +99,7 @@ typedef struct QemuOptDesc {
const char *name;
enum QemuOptType type;
const char *help;
+const char *def_value_str;
} QemuOptDesc;
struct QemuOptsList {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 290ed54..2be6995 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -570,6 +570,13 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char
*name)
const char *qemu_opt_get(QemuOpts *opts, const char *name)
{
QemuOpt *opt = qemu_opt_find(opts, name);
+
+if (!opt) {
+const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+return desc->def_value_str;
+}
+}
return opt ? opt->str : NULL;
}
@@ -589,8 +596,13 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name,
bool defval)
{
QemuOpt *opt = qemu_opt_find(opts, name);
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+parse_option_bool(name, desc->def_value_str, &defval,
&error_abort);
+}
return defval;
+}
assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL);
return opt->value.boolean;
}
@@ -599,8 +611,14 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char
*name, uint64_t defval)
{
QemuOpt *opt = qemu_opt_find(opts, name);
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+parse_option_number(name, desc->def_value_str, &defval,
+&error_abort);
+}
return defval;
+}
assert(opt->desc && opt->desc->type == QEMU_OPT_NUMBER);
return opt->value.uint;
}
@@ -609,8 +627,13 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char
*name, uint64_t defval)
{
QemuOpt *opt = qemu_opt_find(opts, name);
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+parse_option_size(name, desc->def_value_str, &defval,
&error_abort);
+}
return defval;
+}
assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE);
return opt->value.uint;
}
@@ -898,11 +921,30 @@ void qemu_opts_del(QemuOpts *opts)
void qemu_opts_print(QemuOpts *opts)
{
QemuOpt *opt;
+QemuOptDesc *desc = opts->list->desc;
-printf("%s: %s:", opts->list->name,
- opts->id ? opts->id : "");
-QTAILQ_FOREACH(opt, &opts->head, next) {
-printf(" %s=\"%s\"", opt->name, opt->str);
+if (desc[0].name == NULL) {
+QTAILQ_FOREACH(opt, &opts->head, next) {
+printf("%s=\"%s\" ", opt->name, opt->str);
+}
+return;
+}
+for (; desc && desc->name; desc++) {
+const char *value;
+QemuOpt *opt = qemu_opt_find(opts, desc->name);
+
+value = opt ? opt->str : desc->def_value_str;
+if (!value) {
+continue;
+}
+if (desc->type == QEMU_OPT_STRING) {
+printf("%s='%s' ", desc->name, value);
+} else if ((desc->type == QEMU_OPT_SIZE ||
+desc->type == QEMU_OPT_NUMBER) && opt) {
+printf("%s=%" PRId64 " ", desc->name, opt->value.uint);
+} else {
+printf("%s=%s ", desc->name, value);
+}
}
}
--
1.7.12.4
[Qemu-devel] [PATCH v27 15/33] gluster.c: replace QEMUOptionParameter with QemuOpts
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
block/gluster.c | 81 ++---
1 file changed, 42 insertions(+), 39 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 8836085..c72ca77 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -471,13 +471,14 @@ static inline int qemu_gluster_zerofill(struct glfs_fd
*fd, int64_t offset,
#endif
static int qemu_gluster_create(const char *filename,
-QEMUOptionParameter *options, Error **errp)
+ QemuOpts *opts, Error **errp)
{
struct glfs *glfs;
struct glfs_fd *fd;
int ret = 0;
int prealloc = 0;
int64_t total_size = 0;
+char *tmp = NULL;
GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
glfs = qemu_gluster_init(gconf, filename, errp);
@@ -486,24 +487,21 @@ static int qemu_gluster_create(const char *filename,
goto out;
}
-while (options && options->name) {
-if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-total_size = options->value.n / BDRV_SECTOR_SIZE;
-} else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
-if (!options->value.s || !strcmp(options->value.s, "off")) {
-prealloc = 0;
-} else if (!strcmp(options->value.s, "full") &&
-gluster_supports_zerofill()) {
-prealloc = 1;
-} else {
-error_setg(errp, "Invalid preallocation mode: '%s'"
-" or GlusterFS doesn't support zerofill API",
- options->value.s);
-ret = -EINVAL;
-goto out;
-}
-}
-options++;
+total_size =
+qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
+
+tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+if (!tmp || !strcmp(tmp, "off")) {
+prealloc = 0;
+} else if (!strcmp(tmp, "full") &&
+ gluster_supports_zerofill()) {
+prealloc = 1;
+} else {
+error_setg(errp, "Invalid preallocation mode: '%s'"
+" or GlusterFS doesn't support zerofill API",
+tmp);
+ret = -EINVAL;
+goto out;
}
fd = glfs_creat(glfs, gconf->image,
@@ -525,6 +523,7 @@ static int qemu_gluster_create(const char *filename,
}
}
out:
+g_free(tmp);
qemu_gluster_gconf_free(gconf);
if (glfs) {
glfs_fini(glfs);
@@ -688,18 +687,22 @@ static int qemu_gluster_has_zero_init(BlockDriverState
*bs)
return 0;
}
-static QEMUOptionParameter qemu_gluster_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = "Virtual disk size"
-},
-{
-.name = BLOCK_OPT_PREALLOC,
-.type = OPT_STRING,
-.help = "Preallocation mode (allowed values: off, full)"
-},
-{ NULL }
+static QemuOptsList qemu_gluster_create_opts = {
+.name = "qemu-gluster-create-opts",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Virtual disk size"
+},
+{
+.name = BLOCK_OPT_PREALLOC,
+.type = QEMU_OPT_STRING,
+.help = "Preallocation mode (allowed values: off, full)"
+},
+{ /* end of list */ }
+}
};
static BlockDriver bdrv_gluster = {
@@ -712,7 +715,7 @@ static BlockDriver bdrv_gluster = {
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
.bdrv_reopen_abort= qemu_gluster_reopen_abort,
.bdrv_close = qemu_gluster_close,
-.bdrv_create = qemu_gluster_create,
+.bdrv_create2 = qemu_gluster_create,
.bdrv_getlength = qemu_gluster_getlength,
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
.bdrv_truncate= qemu_gluster_truncate,
@@ -726,7 +729,7 @@ static BlockDriver bdrv_gluster = {
#ifdef CONFIG_GLUSTERFS_ZEROFILL
.bdrv_co_write_zeroes = qemu_gluster_co_write_zeroes,
#endif
-.create_options = qemu_gluster_create_options,
+.create_opts = &qemu_gluster_create_opts,
};
static BlockDriver bdrv_gluster_tcp = {
@@ -739,7 +742,7 @@ static BlockDriver bdrv_gluster_tcp = {
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
.bdrv_reopen_abort= qemu_gluster_reopen_abort,
.bdrv_close = qemu_gluster_close,
-.bdrv_create = qemu_gluster_create,
+.bdrv_create2 = qemu_gluster_create,
.bdrv_getlength = qemu_gluster_getlength,
.bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
.bdrv_truncate= qemu_gluster_tru
[Qemu-devel] [PATCH v27 01/33] QemuOpts: move find_desc_by_name ahead for later calling
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Signed-off-by: Chunyan Liu
---
util/qemu-option.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 8bbc3ad..808aef4 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -173,6 +173,20 @@ static void parse_option_number(const char *name, const
char *value,
}
}
+static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
+const char *name)
+{
+int i;
+
+for (i = 0; desc[i].name != NULL; i++) {
+if (strcmp(desc[i].name, name) == 0) {
+return &desc[i];
+}
+}
+
+return NULL;
+}
+
void parse_option_size(const char *name, const char *value,
uint64_t *ret, Error **errp)
{
@@ -637,20 +651,6 @@ static bool opts_accepts_any(const QemuOpts *opts)
return opts->list->desc[0].name == NULL;
}
-static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
-const char *name)
-{
-int i;
-
-for (i = 0; desc[i].name != NULL; i++) {
-if (strcmp(desc[i].name, name) == 0) {
-return &desc[i];
-}
-}
-
-return NULL;
-}
-
int qemu_opt_unset(QemuOpts *opts, const char *name)
{
QemuOpt *opt = qemu_opt_find(opts, name);
--
1.7.12.4
[Qemu-devel] [PATCH v27 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work
Add qemu_opt_get_del, qemu_opt_get_bool_del, qemu_opt_get_number_del and
qemu_opt_get_size_del to replace the same handling of QEMUOptionParameter
(get and delete).
Several drivers are coded to parse a known subset of options, then
remove them from the list before handing all remaining options to a
second driver for further option processing. get_*_del makes it easier
to retrieve a known option (or its default) and remove it from the list
all in one action.
Share common helper function:
For qemu_opt_get_bool/size/number, they and their get_*_del counterpart
could share most of the code except whether or not deleting the opt from
option list, so generate common helper functions.
For qemu_opt_get and qemu_opt_get_del, keep code duplication, since
1. qemu_opt_get_del returns malloc'd memory while qemu_opt_get returns
in-place memory
2. qemu_opt_get_del returns (char *), qemu_opt_get returns (const char *),
and could not change to (char *), since in one case, it will return
desc->def_value_str, which is (const char *).
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
include/qemu/option.h | 6 +++
util/qemu-option.c| 116 --
2 files changed, 109 insertions(+), 13 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index c3b0a91..6653e43 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -111,6 +111,7 @@ struct QemuOptsList {
};
const char *qemu_opt_get(QemuOpts *opts, const char *name);
+char *qemu_opt_get_del(QemuOpts *opts, const char *name);
/**
* qemu_opt_has_help_opt:
* @opts: options to search for a help request
@@ -126,6 +127,11 @@ bool qemu_opt_has_help_opt(QemuOpts *opts);
bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t
defval);
uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
+bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval);
+uint64_t qemu_opt_get_number_del(QemuOpts *opts, const char *name,
+ uint64_t defval);
+uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name,
+ uint64_t defval);
int qemu_opt_unset(QemuOpts *opts, const char *name);
int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 4d2d4d1..32e1d50 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -575,6 +575,19 @@ static void qemu_opt_del(QemuOpt *opt)
g_free(opt);
}
+/* qemu_opt_set allows many settings for the same option.
+ * This function deletes all settings for an option.
+ */
+static void qemu_opt_del_all(QemuOpts *opts, const char *name)
+{
+QemuOpt *opt, *next_opt;
+
+QTAILQ_FOREACH_SAFE(opt, &opts->head, next, next_opt) {
+if (!strcmp(opt->name, name))
+qemu_opt_del(opt);
+}
+}
+
const char *qemu_opt_get(QemuOpts *opts, const char *name)
{
QemuOpt *opt = qemu_opt_find(opts, name);
@@ -588,6 +601,34 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
return opt ? opt->str : NULL;
}
+/* Get a known option (or its default) and remove it from the list
+ * all in one action. Return a malloced string of the option value.
+ * Result must be freed by caller with g_free().
+ */
+char *qemu_opt_get_del(QemuOpts *opts, const char *name)
+{
+QemuOpt *opt;
+const QemuOptDesc *desc;
+char *str = NULL;
+
+if (opts == NULL) {
+return NULL;
+}
+
+opt = qemu_opt_find(opts, name);
+if (!opt) {
+desc = find_desc_by_name(opts->list->desc, name);
+if (desc && desc->def_value_str) {
+str = g_strdup(desc->def_value_str);
+}
+return str;
+}
+str = opt->str;
+opt->str = NULL;
+qemu_opt_del_all(opts, name);
+return str;
+}
+
bool qemu_opt_has_help_opt(QemuOpts *opts)
{
QemuOpt *opt;
@@ -600,50 +641,99 @@ bool qemu_opt_has_help_opt(QemuOpts *opts)
return false;
}
-bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
+static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
+ bool defval, bool del)
{
QemuOpt *opt = qemu_opt_find(opts, name);
+bool ret = defval;
if (opt == NULL) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
-parse_option_bool(name, desc->def_value_str, &defval,
&error_abort);
+parse_option_bool(name, desc->def_value_str, &ret, &error_abort);
}
-return defval;
+return ret;
}
assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL);
-return opt->value.boolean;
+ret = opt->value.boolean;
+
[Qemu-devel] [PATCH v27 08/33] QemuOpts: add qemu_opts_print_help to replace print_option_help
print_option_help takes QEMUOptionParameter as parameter, add
qemu_opts_print_help to take QemuOptsList as parameter for later
replace work.
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
include/qemu/option.h | 1 +
util/qemu-option.c| 13 +
2 files changed, 14 insertions(+)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 6653e43..fbf5dc2 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -166,5 +166,6 @@ typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void
*opaque);
void qemu_opts_print(QemuOpts *opts);
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void
*opaque,
int abort_on_failure);
+void qemu_opts_print_help(QemuOptsList *list);
#endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 32e1d50..adb7c3c 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -553,6 +553,19 @@ void print_option_help(QEMUOptionParameter *list)
}
}
+void qemu_opts_print_help(QemuOptsList *list)
+{
+QemuOptDesc *desc;
+
+assert(list);
+desc = list->desc;
+printf("Supported options:\n");
+while (desc && desc->name) {
+printf("%-16s %s\n", desc->name,
+ desc->help ? desc->help : "No description available");
+desc++;
+}
+}
/* -- */
static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
--
1.7.12.4
[Qemu-devel] [PATCH v27 02/33] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters
Currently this function is not used anywhere. In later patches, it will
replace print_option_parameters. To avoid print info changes, change
qemu_opts_print from fprintf stderr to printf, and remove last printf.
Signed-off-by: Chunyan Liu
---
Changes:
* backward split than v26 (2/33, 3/33).
include/qemu/option.h | 2 +-
util/qemu-option.c| 10 --
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 8c0ac34..1077b69 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -156,7 +156,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp);
typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
-int qemu_opts_print(QemuOpts *opts, void *dummy);
+void qemu_opts_print(QemuOpts *opts);
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void
*opaque,
int abort_on_failure);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 808aef4..290ed54 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -895,17 +895,15 @@ void qemu_opts_del(QemuOpts *opts)
g_free(opts);
}
-int qemu_opts_print(QemuOpts *opts, void *dummy)
+void qemu_opts_print(QemuOpts *opts)
{
QemuOpt *opt;
-fprintf(stderr, "%s: %s:", opts->list->name,
-opts->id ? opts->id : "");
+printf("%s: %s:", opts->list->name,
+ opts->id ? opts->id : "");
QTAILQ_FOREACH(opt, &opts->head, next) {
-fprintf(stderr, " %s=\"%s\"", opt->name, opt->str);
+printf(" %s=\"%s\"", opt->name, opt->str);
}
-fprintf(stderr, "\n");
-return 0;
}
static int opts_do_parse(QemuOpts *opts, const char *params,
--
1.7.12.4
[Qemu-devel] [PATCH v27 28/33] vdi.c: replace QEMUOptionParameter with QemuOpts
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
block/vdi.c | 73 +
1 file changed, 35 insertions(+), 38 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 27737af..fe5cad2 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -673,8 +673,7 @@ static int vdi_co_write(BlockDriverState *bs,
return ret;
}
-static int vdi_create(const char *filename, QEMUOptionParameter *options,
- Error **errp)
+static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
{
int fd;
int result = 0;
@@ -689,25 +688,18 @@ static int vdi_create(const char *filename,
QEMUOptionParameter *options,
logout("\n");
/* Read out options. */
-while (options && options->name) {
-if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-bytes = options->value.n;
+bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
#if defined(CONFIG_VDI_BLOCK_SIZE)
-} else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) {
-if (options->value.n) {
-/* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
-block_size = options->value.n;
-}
+/* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
+block_size = qemu_opt_get_size_del(opts,
+ BLOCK_OPT_CLUSTER_SIZE,
+ DEFAULT_CLUSTER_SIZE);
#endif
#if defined(CONFIG_VDI_STATIC_IMAGE)
-} else if (!strcmp(options->name, BLOCK_OPT_STATIC)) {
-if (options->value.n) {
-image_type = VDI_TYPE_STATIC;
-}
-#endif
-}
-options++;
+if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, false)) {
+image_type = VDI_TYPE_STATIC;
}
+#endif
if (bytes > VDI_DISK_SIZE_MAX) {
result = -ENOTSUP;
@@ -802,29 +794,34 @@ static void vdi_close(BlockDriverState *bs)
error_free(s->migration_blocker);
}
-static QEMUOptionParameter vdi_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = "Virtual disk size"
-},
+static QemuOptsList vdi_create_opts = {
+.name = "vdi-create-opts",
+.head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Virtual disk size"
+},
#if defined(CONFIG_VDI_BLOCK_SIZE)
-{
-.name = BLOCK_OPT_CLUSTER_SIZE,
-.type = OPT_SIZE,
-.help = "VDI cluster (block) size",
-.value = { .n = DEFAULT_CLUSTER_SIZE },
-},
+{
+.name = BLOCK_OPT_CLUSTER_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "VDI cluster (block) size",
+.def_value_str = stringify(DEFAULT_CLUSTER_SIZE)
+},
#endif
#if defined(CONFIG_VDI_STATIC_IMAGE)
-{
-.name = BLOCK_OPT_STATIC,
-.type = OPT_FLAG,
-.help = "VDI static (pre-allocated) image"
-},
+{
+.name = BLOCK_OPT_STATIC,
+.type = QEMU_OPT_BOOL,
+.help = "VDI static (pre-allocated) image",
+.def_value_str = "off"
+},
#endif
-/* TODO: An additional option to set UUID values might be useful. */
-{ NULL }
+/* TODO: An additional option to set UUID values might be useful. */
+{ /* end of list */ }
+}
};
static BlockDriver bdrv_vdi = {
@@ -834,7 +831,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_open = vdi_open,
.bdrv_close = vdi_close,
.bdrv_reopen_prepare = vdi_reopen_prepare,
-.bdrv_create = vdi_create,
+.bdrv_create2 = vdi_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_get_block_status = vdi_co_get_block_status,
.bdrv_make_empty = vdi_make_empty,
@@ -846,7 +843,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_get_info = vdi_get_info,
-.create_options = vdi_create_options,
+.create_opts = &vdi_create_opts,
.bdrv_check = vdi_check,
};
--
1.7.12.4
[Qemu-devel] [PATCH v27 13/33] vvfat.c: handle cross_driver's create_options and create_opts
vvfat shares create options of qcow driver. To avoid vvfat breaking when
qcow driver changes from QEMUOptionParameter to QemuOpts, let it able
to handle both cases.
Signed-off-by: Chunyan Liu
---
Change:
* Fix memory free:
- qemu_opts_free(create_opts);
+ if (bdrv_qcow->create_options) {
+ qemu_opts_free(create_opts);
+ }
block/vvfat.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 155fc9b..41fe623 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2906,8 +2906,9 @@ static BlockDriver vvfat_write_target = {
static int enable_write_target(BDRVVVFATState *s)
{
-BlockDriver *bdrv_qcow;
-QEMUOptionParameter *options;
+BlockDriver *bdrv_qcow = NULL;
+QemuOptsList *create_opts = NULL;
+QemuOpts *opts = NULL;
Error *local_err = NULL;
int ret;
int size = sector2cluster(s, s->sector_count);
@@ -2922,11 +2923,17 @@ static int enable_write_target(BDRVVVFATState *s)
}
bdrv_qcow = bdrv_find_format("qcow");
-options = parse_option_parameters("", bdrv_qcow->create_options, NULL);
-set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512);
-set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:");
+assert(!(bdrv_qcow->create_opts && bdrv_qcow->create_options));
+if (bdrv_qcow->create_options) {
+create_opts = params_to_opts(bdrv_qcow->create_options);
+} else {
+create_opts = bdrv_qcow->create_opts;
+}
+opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512);
+qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:");
-ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, NULL, &local_err);
+ret = bdrv_create(bdrv_qcow, s->qcow_filename, NULL, opts, &local_err);
if (ret < 0) {
qerror_report_err(local_err);
error_free(local_err);
@@ -2955,6 +2962,10 @@ static int enable_write_target(BDRVVVFATState *s)
return 0;
err:
+qemu_opts_del(opts);
+if (bdrv_qcow->create_options) {
+qemu_opts_free(create_opts);
+}
g_free(s->qcow_filename);
s->qcow_filename = NULL;
return ret;
--
1.7.12.4
Re: [Qemu-devel] [RFC PATCH v2 00/16] visitor+BER migration format
On Wed, Apr 23, 2014 at 12:57:43PM -0600, Eric Blake wrote: > On 04/23/2014 11:54 AM, Michael S. Tsirkin wrote: > > On Wed, Apr 23, 2014 at 11:47:21AM -0600, Eric Blake wrote: > >> On 04/23/2014 11:16 AM, Dr. David Alan Gilbert wrote: > >>> The one thing that the environment variable does make nice and easy, > >>> for dev, is using it with existing test setups - e.g. running virt-test > >>> in BER mode or existing mode. > >> Hmm. Maybe a compromise - an environment variable determines the default > >> state of the QMP capability (for use in running the testsuite in one > >> mode or the other for all tests that don't explicitly set a mode), but > >> the QMP command still allows toggling the state at runtime (the > >> environment variable doesn't lock us out from changing away from the > >> startup default). > > > > Eric, could you explain the need for the runtime thing? > > I thought I did - when migrating across machines to what we know is an > older qemu, we want the old migration format; but when migrating to a > file to be reloaded by the current (or newer) qemu, we want the new > format since it is more robust. It's more robust for adding new functionality/new machine types but for old one I think we can assume it describes it sufficiently. > But libvirt can't know up front whether > the user starting a qemu instance has plans down the road of calling > 'virsh save' (migrate to file) or 'virsh migrate' (migrate across > machines) first. > > On the other hand, you have a point that upstream tends to not care > about migration from new qemu to old (that's one of the value-added > tasks of downstream distros), and that we could merely get away with > using the -m machine name as the witness of which form to use, with no > need for an environment variable or any other runtime switch. > > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
Re: [Qemu-devel] [PATCH 08/22] iscsi: implement .bdrv_detach/attach_aio_context()
On Fri, May 02, 2014 at 12:39:06AM +0200, Peter Lieven wrote:
> > +static void iscsi_attach_aio_context(BlockDriverState *bs,
> > + AioContext *new_context)
> > +{
> > +IscsiLun *iscsilun = bs->opaque;
> > +
> > +iscsilun->aio_context = new_context;
> > +iscsi_set_events(iscsilun);
> > +
> > +#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
> > +/* Set up a timer for sending out iSCSI NOPs */
> > +iscsilun->nop_timer = aio_timer_new(iscsilun->aio_context,
> > +QEMU_CLOCK_REALTIME, SCALE_MS,
> > +iscsi_nop_timed_event, iscsilun);
> > +timer_mod(iscsilun->nop_timer,
> > + qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
> > +#endif
> > +}
>
> Is it still guaranteed that iscsi_nop_timed_event for a target is not invoked
> while we are in another function/callback of the iscsi driver for the same
> target?
This is a good point.
Previously, the nop timer was deferred until the qemu_aio_wait() loop
terminates.
With this patch the nop timer fires during aio_poll() loops for any
synchronous emulation that QEMU does (including iscsi_aio_cancel() and
.bdrv_ioctl() in block/iscsi.c).
I don't know libiscsi well enough to understand the implications. I can
see that iscsi_reconnect() resends in-flight commands. So what's the
upshot of all this?
BTW, is iscsi_reconnect() the right libiscsi interface to use since it
is synchronous? It seems like this would block QEMU until the socket
has connected! The guest would be frozen.
Stefan
[Qemu-devel] [PATCH v27 05/33] QemuOpts: change opt->name|str from (const char *) to (char *)
qemu_opt_del() already assumes that all QemuOpt instances contain
malloc'd name and value; but it had to cast away const because
opts_start_struct() was doing its own thing and using static storage
instead. By using the correct type and malloced strings everywhere, the
usage of this struct becomes clearer.
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Chunyan Liu
---
include/qemu/option_int.h | 4 ++--
qapi/opts-visitor.c | 10 +++---
util/qemu-option.c| 4 ++--
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
index 8212fa4..6432c1a 100644
--- a/include/qemu/option_int.h
+++ b/include/qemu/option_int.h
@@ -30,8 +30,8 @@
#include "qemu/error-report.h"
struct QemuOpt {
-const char *name;
-const char *str;
+char *name;
+char *str;
const QemuOptDesc *desc;
union {
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 5d830a2..7a64f4e 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -143,8 +143,8 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
if (ov->opts_root->id != NULL) {
ov->fake_id_opt = g_malloc0(sizeof *ov->fake_id_opt);
-ov->fake_id_opt->name = "id";
-ov->fake_id_opt->str = ov->opts_root->id;
+ov->fake_id_opt->name = g_strdup("id");
+ov->fake_id_opt->str = g_strdup(ov->opts_root->id);
opts_visitor_insert(ov->unprocessed_opts, ov->fake_id_opt);
}
}
@@ -177,7 +177,11 @@ opts_end_struct(Visitor *v, Error **errp)
}
g_hash_table_destroy(ov->unprocessed_opts);
ov->unprocessed_opts = NULL;
-g_free(ov->fake_id_opt);
+if (ov->fake_id_opt) {
+g_free(ov->fake_id_opt->name);
+g_free(ov->fake_id_opt->str);
+g_free(ov->fake_id_opt);
+}
ov->fake_id_opt = NULL;
}
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 2be6995..69cdf3f 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -664,8 +664,8 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
static void qemu_opt_del(QemuOpt *opt)
{
QTAILQ_REMOVE(&opt->opts->head, opt, next);
-g_free((/* !const */ char*)opt->name);
-g_free((/* !const */ char*)opt->str);
+g_free(opt->name);
+g_free(opt->str);
g_free(opt);
}
--
1.7.12.4
[Qemu-devel] [PATCH v27 19/33] QemuOpts: export qemu_opt_find
Export qemu_opt_find for qcow2 driver using it.
After replacing QEMUOptionParameter with QemuOpts, qcow2 driver will
use qemu_opt_find to judge if an option is explicitly set, to replace
the usage of .assigned in QEMUOptionParameter.
Signed-off-by: Chunyan Liu
---
Changes:
* split qcow2 patch into two: 19/33, 20/33
- export qemu_opt_find in a separate commit
- then patch qcow2 driver
include/qemu/option.h | 1 +
util/qemu-option.c| 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 44d9961..3455267 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -130,6 +130,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name);
* Returns: true if @opts includes 'help' or equivalent.
*/
bool qemu_opt_has_help_opt(QemuOpts *opts);
+QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name);
bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t
defval);
uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 71415ee..55f1726 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -568,7 +568,7 @@ void qemu_opts_print_help(QemuOptsList *list)
}
/* -- */
-static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
+QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
{
QemuOpt *opt;
--
1.7.12.4
[Qemu-devel] [PATCH v27 12/33] change block layer to support both QemuOpts and QEMUOptionParamter
Change block layer to support both QemuOpts and QEMUOptionParameter.
After this patch, it will change backend drivers one by one. At the end,
QEMUOptionParameter will be removed and only QemuOpts is kept.
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
Changes:
* Fix Eric's comments:
- remove the end '.' in two error messages.
- update related iotests .out file.
block.c| 160 +++--
block/cow.c| 2 +-
block/qcow.c | 2 +-
block/qcow2.c | 2 +-
block/qed.c| 2 +-
block/raw_bsd.c| 2 +-
block/vhdx.c | 2 +-
block/vmdk.c | 4 +-
block/vvfat.c | 2 +-
include/block/block.h | 7 +-
include/block/block_int.h | 13 +++-
qemu-img.c | 94 +-
tests/qemu-iotests/049.out | 2 +-
tests/qemu-iotests/061.out | 2 +-
14 files changed, 183 insertions(+), 113 deletions(-)
diff --git a/block.c b/block.c
index b749d31..d417190 100644
--- a/block.c
+++ b/block.c
@@ -328,6 +328,13 @@ void bdrv_register(BlockDriver *bdrv)
}
}
+if (bdrv->bdrv_create) {
+assert(!bdrv->bdrv_create2 && !bdrv->create_opts);
+assert(!bdrv->bdrv_amend_options2);
+} else if (bdrv->bdrv_create2) {
+assert(!bdrv->bdrv_create && !bdrv->create_options);
+assert(!bdrv->bdrv_amend_options);
+}
QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
}
@@ -419,6 +426,7 @@ typedef struct CreateCo {
BlockDriver *drv;
char *filename;
QEMUOptionParameter *options;
+QemuOpts *opts;
int ret;
Error *err;
} CreateCo;
@@ -430,8 +438,28 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
CreateCo *cco = opaque;
assert(cco->drv);
+assert(!(cco->options && cco->opts));
-ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+if (cco->drv->bdrv_create2) {
+QemuOptsList *opts_list = NULL;
+if (cco->options) {
+opts_list = params_to_opts(cco->options);
+cco->opts = qemu_opts_create(opts_list, NULL, 0, &error_abort);
+}
+ret = cco->drv->bdrv_create2(cco->filename, cco->opts, &local_err);
+if (cco->options) {
+qemu_opts_del(cco->opts);
+qemu_opts_free(opts_list);
+}
+} else {
+if (cco->opts) {
+cco->options = opts_to_params(cco->opts);
+}
+ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+if (cco->opts) {
+free_option_parameters(cco->options);
+}
+}
if (local_err) {
error_propagate(&cco->err, local_err);
}
@@ -439,7 +467,8 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
}
int bdrv_create(BlockDriver *drv, const char* filename,
-QEMUOptionParameter *options, Error **errp)
+QEMUOptionParameter *options,
+QemuOpts *opts, Error **errp)
{
int ret;
@@ -448,11 +477,12 @@ int bdrv_create(BlockDriver *drv, const char* filename,
.drv = drv,
.filename = g_strdup(filename),
.options = options,
+.opts = opts,
.ret = NOT_DONE,
.err = NULL,
};
-if (!drv->bdrv_create) {
+if (!drv->bdrv_create && !drv->bdrv_create2) {
error_setg(errp, "Driver '%s' does not support image creation",
drv->format_name);
ret = -ENOTSUP;
goto out;
@@ -484,7 +514,7 @@ out:
}
int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
- Error **errp)
+ QemuOpts *opts, Error **errp)
{
BlockDriver *drv;
Error *local_err = NULL;
@@ -496,7 +526,7 @@ int bdrv_create_file(const char* filename,
QEMUOptionParameter *options,
return -ENOENT;
}
-ret = bdrv_create(drv, filename, options, &local_err);
+ret = bdrv_create(drv, filename, options, opts, &local_err);
if (local_err) {
error_propagate(errp, local_err);
}
@@ -1212,7 +1242,8 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs,
Error **errp)
char *tmp_filename = g_malloc0(PATH_MAX + 1);
int64_t total_size;
BlockDriver *bdrv_qcow2;
-QEMUOptionParameter *create_options;
+QemuOptsList *create_opts = NULL;
+QemuOpts *opts = NULL;
QDict *snapshot_options;
BlockDriverState *bs_snapshot;
Error *local_err;
@@ -1237,13 +1268,20 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs,
Error **errp)
}
bdrv_qcow2 = bdrv_find_format("qcow2");
-create_options = parse_option_parameters("", bdrv_qcow2->create_options,
- NULL);
-
-set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
-ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
-free_option_parameters(create_
[Qemu-devel] [PATCH v27 06/33] QemuOpts: move qemu_opt_del ahead for later calling
In later patch, qemu_opt_get_del functions will be added, they will
first get the option value, then call qemu_opt_del to remove the option
from opt list. To prepare for that purpose, move qemu_opt_del ahead first.
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Chunyan Liu
---
util/qemu-option.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 69cdf3f..4d2d4d1 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -567,6 +567,14 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char
*name)
return NULL;
}
+static void qemu_opt_del(QemuOpt *opt)
+{
+QTAILQ_REMOVE(&opt->opts->head, opt, next);
+g_free(opt->name);
+g_free(opt->str);
+g_free(opt);
+}
+
const char *qemu_opt_get(QemuOpts *opts, const char *name)
{
QemuOpt *opt = qemu_opt_find(opts, name);
@@ -661,14 +669,6 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
}
}
-static void qemu_opt_del(QemuOpt *opt)
-{
-QTAILQ_REMOVE(&opt->opts->head, opt, next);
-g_free(opt->name);
-g_free(opt->str);
-g_free(opt);
-}
-
static bool opts_accepts_any(const QemuOpts *opts)
{
return opts->list->desc[0].name == NULL;
--
1.7.12.4
[Qemu-devel] [PATCH v27 18/33] qcow.c: replace QEMUOptionParameter with QemuOpts
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Dong Xu Wang
Signed-off-by: Chunyan Liu
---
block/qcow.c | 72 ++--
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 3bac591..e8fe874 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -663,35 +663,29 @@ static void qcow_close(BlockDriverState *bs)
error_free(s->migration_blocker);
}
-static int qcow_create(const char *filename, QEMUOptionParameter *options,
- Error **errp)
+static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
{
int header_size, backing_filename_len, l1_size, shift, i;
QCowHeader header;
uint8_t *tmp;
int64_t total_size = 0;
-const char *backing_file = NULL;
+char *backing_file = NULL;
int flags = 0;
Error *local_err = NULL;
int ret;
BlockDriverState *qcow_bs;
/* Read out options */
-while (options && options->name) {
-if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-total_size = options->value.n / 512;
-} else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
-backing_file = options->value.s;
-} else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) {
-flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0;
-}
-options++;
+total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
+flags |= BLOCK_FLAG_ENCRYPT;
}
-ret = bdrv_create_file(filename, options, NULL, &local_err);
+ret = bdrv_create_file(filename, NULL, opts, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
-return ret;
+goto cleanup;
}
qcow_bs = NULL;
@@ -699,7 +693,7 @@ static int qcow_create(const char *filename,
QEMUOptionParameter *options,
BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
-return ret;
+goto cleanup;
}
ret = bdrv_truncate(qcow_bs, 0);
@@ -770,6 +764,8 @@ static int qcow_create(const char *filename,
QEMUOptionParameter *options,
ret = 0;
exit:
bdrv_unref(qcow_bs);
+cleanup:
+g_free(backing_file);
return ret;
}
@@ -882,24 +878,28 @@ static int qcow_get_info(BlockDriverState *bs,
BlockDriverInfo *bdi)
return 0;
}
-
-static QEMUOptionParameter qcow_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = "Virtual disk size"
-},
-{
-.name = BLOCK_OPT_BACKING_FILE,
-.type = OPT_STRING,
-.help = "File name of a base image"
-},
-{
-.name = BLOCK_OPT_ENCRYPT,
-.type = OPT_FLAG,
-.help = "Encrypt the image"
-},
-{ NULL }
+static QemuOptsList qcow_create_opts = {
+.name = "qcow-create-opts",
+.head = QTAILQ_HEAD_INITIALIZER(qcow_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Virtual disk size"
+},
+{
+.name = BLOCK_OPT_BACKING_FILE,
+.type = QEMU_OPT_STRING,
+.help = "File name of a base image"
+},
+{
+.name = BLOCK_OPT_ENCRYPT,
+.type = QEMU_OPT_BOOL,
+.help = "Encrypt the image",
+.def_value_str = "off"
+},
+{ /* end of list */ }
+}
};
static BlockDriver bdrv_qcow = {
@@ -908,8 +908,8 @@ static BlockDriver bdrv_qcow = {
.bdrv_probe= qcow_probe,
.bdrv_open = qcow_open,
.bdrv_close= qcow_close,
-.bdrv_reopen_prepare = qcow_reopen_prepare,
-.bdrv_create = qcow_create,
+.bdrv_reopen_prepare= qcow_reopen_prepare,
+.bdrv_create2 = qcow_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_readv = qcow_co_readv,
@@ -921,7 +921,7 @@ static BlockDriver bdrv_qcow = {
.bdrv_write_compressed = qcow_write_compressed,
.bdrv_get_info = qcow_get_info,
-.create_options = qcow_create_options,
+.create_opts= &qcow_create_opts,
};
static void bdrv_qcow_init(void)
--
1.7.12.4
Re: [Qemu-devel] [PATCH v2] mac99: Added FW_CFG_PPC_BUSFREQ to match CLOCKFREQ and TBFREQ already there
Ping?
On Thu, 17 Apr 2014, BALATON Zoltan wrote:
While there, also moved the hard coded value for CLOCKFREQ to a #define.
Signed-off-by: BALATON Zoltan
---
v2: Also include mac_oldworld that I missed in the first version and
fix commit message
hw/ppc/mac_newworld.c | 5 -
hw/ppc/mac_oldworld.c | 5 -
include/hw/ppc/ppc.h | 2 ++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index a533ec7..c937ff1 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -72,6 +72,8 @@
#define MAX_IDE_BUS 2
#define CFG_ADDR 0xf510
#define TBFREQ (100UL * 1000UL * 1000UL)
+#define CLOCKFREQ (266UL * 1000UL * 1000UL)
+#define BUSFREQ (100UL * 1000UL * 1000UL)
/* debug UniNorth */
//#define DEBUG_UNIN
@@ -469,7 +471,8 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, TBFREQ);
}
/* Mac OS X requires a "known good" clock-frequency value; pass it one. */
-fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, 26600);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
}
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 2f27754..5de9223 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -46,6 +46,8 @@
#define MAX_IDE_BUS 2
#define CFG_ADDR 0xf510
#define TBFREQ 1660UL
+#define CLOCKFREQ 26600UL
+#define BUSFREQ 6600UL
static int fw_cfg_boot_set(void *opaque, const char *boot_device)
{
@@ -337,7 +339,8 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, TBFREQ);
}
/* Mac OS X requires a "known good" clock-frequency value; pass it one. */
-fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, 26600);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
}
diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index d71bd07..7e16e2e 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -92,6 +92,8 @@ enum {
#define FW_CFG_PPC_IS_KVM (FW_CFG_ARCH_LOCAL + 0x05)
#define FW_CFG_PPC_KVM_HC (FW_CFG_ARCH_LOCAL + 0x06)
#define FW_CFG_PPC_KVM_PID (FW_CFG_ARCH_LOCAL + 0x07)
+/* OpenBIOS has FW_CFG_PPC_NVRAM_ADDR as +0x08 */
+#define FW_CFG_PPC_BUSFREQ (FW_CFG_ARCH_LOCAL + 0x09)
#define PPC_SERIAL_MM_BAUDBASE 399193
--
1.8.1.5
[Qemu-devel] [PATCH v27 11/33] QemuOpts: check NULL input for qemu_opts_del
To simplify later using of qemu_opts_del, accept NULL input.
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Reviewed-by: Leandro Dorileo
Signed-off-by: Chunyan Liu
---
util/qemu-option.c | 4
1 file changed, 4 insertions(+)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index e15723a..71415ee 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1010,6 +1010,10 @@ void qemu_opts_del(QemuOpts *opts)
{
QemuOpt *opt;
+if (opts == NULL) {
+return;
+}
+
for (;;) {
opt = QTAILQ_FIRST(&opts->head);
if (opt == NULL)
--
1.7.12.4
