Re: [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union
Wenchao Xia xiaw...@linux.vnet.ibm.com writes:
于 2014/2/14 17:23, Markus Armbruster 写道:
Wenchao Xia xiaw...@linux.vnet.ibm.com writes:
于 2014/2/13 23:14, Markus Armbruster 写道:
Wenchao Xia xiaw...@linux.vnet.ibm.com writes:
It will check whether the values specified are written correctly,
and whether all enum values are covered, when discriminator is a
pre-defined enum type
Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
scripts/qapi-visit.py | 17 +
scripts/qapi.py | 31 +++
2 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 65f1a54..c0efb5f 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -255,6 +255,23 @@ def generate_visit_union(expr):
assert not base
return generate_visit_anon_union(name, members)
+# If discriminator is specified and it is a pre-defined enum in
schema,
+# check its correctness
+enum_define = discriminator_find_enum_define(expr)
+if enum_define:
+for key in members:
+if not key in enum_define[enum_values]:
+sys.stderr.write(Discriminator value '%s' is not found
in
+ enum '%s'\n %
+ (key, enum_define[enum_name]))
+sys.exit(1)
Can this happen? If yes, why isn't it diagnosed in qapi.py, like all
the other semantic errors?
I think the parse procedure contains two part:
1 read qapi-schema.json and parse it into exprs.
2 translate exprs into final output.
Looking at qapi.py, qapi-visit.py, qapi-types.py, it seems qapi.py is
in charge of step 1 handling literal error, and other two script are in
charge of step 2. The above error can be only detected in step 2 after
all enum defines are remembered in step 1, so I didn't add those things
into qapi.py.
The distribution of work between the qapi*py isn't spelled out anywhere,
but my working hypothesis is qapi.py is the frontend, and the
qapi-{commands,types,visit}.py are backends.
The frontend's job is lexical, syntax and semantic analysis.
The backends' job is source code generation.
This isn't the only possible split, but it's the orthodox way to split
compilers.
I guess you want to place the check inside parse_schema() to let
test case detect it easier, one way to go is, let qapi.py do checks
for step 2:
def parse_schema(fp):
try:
schema = QAPISchema(fp)
except QAPISchemaError, e:
print sys.stderr, e
exit(1)
exprs = []
for expr in schema.exprs:
if expr.has_key('enum'):
add_enum(expr['enum'])
elif expr.has_key('union'):
add_union(expr)
add_enum('%sKind' % expr['union'])
elif expr.has_key('type'):
add_struct(expr)
exprs.append(expr)
+for expr in schema.exprs:
+if expr.has_key('union'):
+#check code
return exprs
This way qapi.py can detect such errors. Disadvantage is that,
qapi.py is invloved for step 2 things, so some code in qapi.py
and qapi-visit.py may be dupicated, here the if union...
discriminator code may appear in both qapi.py and qapi-visit.py.
How much code would be duplicated?
Not many now, my concern is it may becomes more complex
when more check introduced in future.
However, your distribution of qapi*.py as complier make
sense, so I am OK to respin this series.
I'll gladly review your respin. If you need help rebasing, don't
hesitate to ask me.
Luiz, could you apply or push Markus's series, so I
can pull it as my working base?
I pushed my series trivially rebased to current master to
git://repo.or.cz/qemu/armbru.git branch qapi-scripts. It applies fine
to Luiz's qmp-unstable branch.
Re: [Qemu-devel] [PATCH 3/5] query-command-line-options: query all the options in qemu-options.hx
On Tue, Feb 11, 2014 at 01:03:05PM +0100, Markus Armbruster wrote:
Amos Kong ak...@redhat.com writes:
vm_config_groups[] contain the options which have parameter, but some
legcy options haven't been added to vm_config_groups[].
All the options can be found in qemu-options.hx, this patch used two
new marcos to generate two tables, we can check if the option name is
valid and if the option has arguments.
This patch also try to visit all the options in option_names[], then
we won't lost the legacy options that weren't added to vm_config_groups[].
The options have no arguments will also be returned (eg: -enable-fips)
Signed-off-by: Amos Kong ak...@redhat.com
---
util/qemu-config.c | 41 +++--
1 file changed, 35 insertions(+), 6 deletions(-)
diff --git a/util/qemu-config.c b/util/qemu-config.c
index d624d92..de233d8 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -78,6 +78,17 @@ static CommandLineParameterInfoList
*get_param_infolist(const QemuOptDesc *desc)
return param_list;
}
+static int get_group_index(const char *name)
+{
+int i;
+
+for (i = 0; vm_config_groups[i] != NULL; i++) {
+if (!strcmp(vm_config_groups[i]-name, name)) {
+return i;
+}
+}
+return -1;
+}
/* remove repeated entry from the info list */
static void cleanup_infolist(CommandLineParameterInfoList *head)
{
@@ -139,16 +150,34 @@ CommandLineOptionInfoList
*qmp_query_command_line_options(bool has_option,
CommandLineOptionInfo *info;
int i;
-for (i = 0; vm_config_groups[i] != NULL; i++) {
-if (!has_option || !strcmp(option, vm_config_groups[i]-name)) {
+char const *option_names[] = {
+#define QEMU_OPTIONS_GENERATE_NAME
+#include qemu-options-wrapper.h
+};
+
+char const *option_hasargs[] = {
+#define QEMU_OPTIONS_GENERATE_HASARG
+#include qemu-options-wrapper.h
+};
Both tables are technically redundant. The same information is already
in vl.c's qemu_options[]. That one also includes -h, which the tables
here miss.
Duplicating tables can be okay, but I suspect using the existing one
would be simpler. Have you tried?
Right, it's redundant work.
+
+for (i = 0; i sizeof(option_names) / sizeof(char *); i++) {
+if (!has_option || !strcmp(option, option_names[i])) {
info = g_malloc0(sizeof(*info));
-info-option = g_strdup(vm_config_groups[i]-name);
-if (!strcmp(drive, vm_config_groups[i]-name)) {
+info-option = g_strdup(option_names[i]);
+
+int idx = get_group_index(option_names[i]);
Variable declaration follows statement. Please declare int idx at the
beginning of a block. I'd declare it right at the beginning, together
with int i.
Ok
+
+if (!strcmp(HAS_ARG, option_hasargs[i])) {
+info-has_parameters = true;
+}
qemu/util/qemu-config.c:171:21: error: ‘CommandLineOptionInfo’ has no member
named ‘has_parameters’
Did you forget to include a schema change?
Schema change was wrongly included to patch 5/5.
Awfully roundabout way to test whether the option takes an argument.
Here's the other part, from PATCH 2/5:
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)\
+stringify(opt_arg),
Please do it more like vl.c:
#define HAS_ARG true
bool option_has_arg[] = {
#define QEMU_OPTIONS_GENERATE_HASARG
#include qemu-options-wrapper.h
}
I touched an error in the past, so convert the has_arg to string.
| In file included from util/qemu-config.c:160:0:
| ./qemu-options.def: In function ‘qmp_query_command_line_options’:
| ./qemu-options.def:10:16: error: ‘HAS_ARG’ undeclared (first use in this
function)
| DEF(machine, HAS_ARG, QEMU_OPTION_machine, \
| ^
| ./qemu-options-wrapper.h:32:5: note: in definition of macro ‘DEF’
| opt_arg,
| ^
| ./qemu-options.def:10:16: note: each undeclared identifier is reported only
once for each function it appears in
| DEF(machine, HAS_ARG, QEMU_OPTION_machine, \
| ^
| ./qemu-options-wrapper.h:32:5: note: in definition of macro ‘DEF’
| opt_arg,
| ^
This problem can be solved by defining HAS_ARG in qemu-config.c as in vl.c
+ #define HAS_ARG 0x0001
Then you can simply test option_has_arg[i].
Or reuse vl.c's table.
OK.
+
+if (!strcmp(drive, option_names[i])) {
info-parameters = get_drive_infolist();
-} else {
+} else if (idx = 0) {
info-parameters =
-get_param_infolist(vm_config_groups[i]-desc);
+get_param_infolist(vm_config_groups[idx]-desc);
}
+
entry = g_malloc0(sizeof(*entry));
entry-value = info;
Re: [Qemu-devel] [PATCH V6 02/10] qapi script: add check for duplicated key
Wenchao Xia xiaw...@linux.vnet.ibm.com writes:
于 2014/2/13 23:14, Markus Armbruster 写道:
Wenchao Xia xiaw...@linux.vnet.ibm.com writes:
It is bad that same key was specified twice, especially when a union have
two branches with same condition. This patch can prevent it.
Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
scripts/qapi.py |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index aec6bbb..cf34768 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -116,6 +116,8 @@ class QAPISchema:
if self.tok != ':':
raise QAPISchemaError(self, 'Expected :')
self.accept()
+if key in expr:
+raise QAPISchemaError(self, 'Duplicated key %s' % key)
expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
All errors should have a test in tests/qapi-schema/. I can try to add
tests for you when I rebase your 09/10.
I considered error path test before but didn't find a good place to
go. It would be great if you can add one.
Here's the test for your commit, feel free to squash it into yours.
From ce842b83cd999ee76e4221e24313a5c447e40bac Mon Sep 17 00:00:00 2001
From: Markus Armbruster arm...@redhat.com
Date: Fri, 14 Feb 2014 13:15:46 +0100
Subject: [PATCH] qapi: Polish error message for duplicate key, and add test
Signed-off-by: Markus Armbruster arm...@redhat.com
---
scripts/qapi.py | 2 +-
tests/Makefile | 4 ++--
tests/qapi-schema/duplicate-key.err | 1 +
tests/qapi-schema/duplicate-key.exit | 1 +
tests/qapi-schema/duplicate-key.json | 2 ++
tests/qapi-schema/duplicate-key.out | 0
6 files changed, 7 insertions(+), 3 deletions(-)
create mode 100644 tests/qapi-schema/duplicate-key.err
create mode 100644 tests/qapi-schema/duplicate-key.exit
create mode 100644 tests/qapi-schema/duplicate-key.json
create mode 100644 tests/qapi-schema/duplicate-key.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0663c2e..3732fe1 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -117,7 +117,7 @@ class QAPISchema:
raise QAPISchemaError(self, 'Expected :')
self.accept()
if key in expr:
-raise QAPISchemaError(self, 'Duplicated key %s' % key)
+raise QAPISchemaError(self, 'Duplicate key %s' % key)
expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
diff --git a/tests/Makefile b/tests/Makefile
index fd36eee..e3ddbcc 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -119,8 +119,8 @@ check-qtest-xtensa-y += tests/qom-test$(EXESUF)
check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
-comments.json empty.json funny-char.json indented-expr.json \
-missing-colon.json missing-comma-list.json \
+comments.json duplicate-key.json empty.json funny-char.json \
+indented-expr.json missing-colon.json missing-comma-list.json \
missing-comma-object.json non-objects.json \
qapi-schema-test.json quoted-structural-chars.json \
trailing-comma-list.json trailing-comma-object.json \
diff --git a/tests/qapi-schema/duplicate-key.err
b/tests/qapi-schema/duplicate-key.err
new file mode 100644
index 000..0801c6a
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.err
@@ -0,0 +1 @@
+stdin:2:10: Duplicate key key
diff --git a/tests/qapi-schema/duplicate-key.exit
b/tests/qapi-schema/duplicate-key.exit
new file mode 100644
index 000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/duplicate-key.json
b/tests/qapi-schema/duplicate-key.json
new file mode 100644
index 000..1b55d88
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.json
@@ -0,0 +1,2 @@
+{ 'key': 'value',
+ 'key': 'value' }
diff --git a/tests/qapi-schema/duplicate-key.out
b/tests/qapi-schema/duplicate-key.out
new file mode 100644
index 000..e69de29
--
1.8.1.4
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On Fr, 2014-02-07 at 13:51 +0100, Igor Mammedov wrote: Since introduction of PCIHP, it became problematic to punch hole in PCI0._CRS statically since PCI hotplug region size became runtime changeable. So replace static hole punching with dynamically consumed resources in a child device on PCI0 bus. i.e generate PNP0C02 device as a child of PCI0 bus at runtime and consume GPE0, PCI/CPU hotplug IO resources in it instead of punching holes in static PCI0._CRS. Nice. Can you try to do that for the mmconf xbar in memory space too while being at it? cheers, Gerd
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin wrote: On Fri, Feb 07, 2014 at 01:51:27PM +0100, Igor Mammedov wrote: Since introduction of PCIHP, it became problematic to punch hole in PCI0._CRS statically since PCI hotplug region size became runtime changeable. What makes it runtime changeable? machine type. q35 / piix map them at different locations. Also we might want to this also for devices which are runtime-configurable (isa-debugcon, pvpanic, ...). cheers, Gerd
Re: [Qemu-devel] Trying to write a new device / virtio-i2c ?
Thanks for all your answers. I understand that what I want to achieve seemed pretty confused. I will try to clarify : On real hardware, I have an I2C device used to get temperatures, pressure etc... and it works on x86 and there were no QEMU virtualized hardware corresponding. I don't really need to simulate the I2C hardware in QEMU. Indeed, there are few of them just sending regular i2c data. For some tests, I want to be able to send/receive these ones from my daemon on the host. After some researches, what I was thinking about was : 1) Use virtio-serial and write an I2C driver (guest kernel) that would give/take data to/from i2c-1 and read/write to vportp0... Seemed a bit ugly, so I wanted to try something else. 2) Write virtio-i2c (using i2c-driver and virtio kernel basics) that would register, for example, i2c-1 and get/send data from my guest app, and use virtio to send these data to host. What I have done for now : I used virtio-serial / virtio-console in linux kernel and inspired from virtio-pci to try to registers these vport0pX as i2c-1, i2c-2 etc... and as i2c devices. I also wrote a virtio-i2c QEMU-side to register as this hardware using virtio-i2c drivers. I think my understanding of the architecture is probably not complete, as it seems that this QEMU device doesn't automatically registers as a virtio-i2c hardware that would launch my guest kernel driver. My printk's in the probe are not printed. My driver is then never used. My questions were then : - My solution might be a bit complicated assuming I don't have that much knowledge in the architecture (however I'm interested into learning...) - Are there solutions that seems more adapted to my case ? Like using USB-I2C bridge ? 2014-02-14 17:45 GMT+01:00 Paolo Bonzini pbonz...@redhat.com: Il 14/02/2014 17:31, Andreas Färber ha scritto: While that is certainly possible in case host passthrough was desired, maybe virtio was mixed up with VFIO? I don't think so, VFIO is mostly about IOMMUs and protecting from DMA. Paolo
Re: [Qemu-devel] [PATCH 5/5] query-command-line-options: return help message for legacy options
On Tue, Feb 11, 2014 at 01:19:16PM +0100, Markus Armbruster wrote:
[Note cc: Eric]
Hi Markus,
Amos Kong ak...@redhat.com writes:
Some legacy options that have arguments weren't added to
vm_config_groups[], so query-command-line-options returns a
NULL parameters infolist. This patch try to return help message
for this kind of legacy options.
Example:
{
helpmsg: \-vnc displaystart a VNC server on display\\n\,
parameters: [
],
option: vnc
},
Do we have prospective users for this feature?
I had posted a RFC mail to discuss about fix/redo query-command-line-options,
this patch is a solution for legacy options that have not arguments.
Current QEMU returns NULL list for this kind of options, this patch
tries to return option name and help message to the libvirt. (it's
better)
You can find some examples in the end.
Signed-off-by: Amos Kong ak...@redhat.com
---
qapi-schema.json | 5 -
util/qemu-config.c | 3 +++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 05ced9d..b3e6f46 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3943,13 +3943,16 @@
# Details about a command line option, including its list of parameter
details
#
# @option: option name
+# @helpmsg: help message for legacy options
#
# @parameters: an array of @CommandLineParameterInfo
#
# Since 1.5
##
{ 'type': 'CommandLineOptionInfo',
- 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } }
+ 'data': { 'option': 'str',
+'*parameters': ['CommandLineParameterInfo'],
+'*helpmsg': 'str' } }
##
# @query-command-line-options:
Aha, here's the schema change missing in PATCH 3/5.
Yeah
The schema needs to cover these kinds of options:
1. No argument
{ 'option': OPT-NAME }
2. QemuOpts argument
2a. with known parameters (QemuOptsList member desc[] not empty)
{ 'option': OPT-NAME,
'parameters': [ { 'name': PAR-NAME, 'type': PAR-TYPE }, ... ] }
2b. with unknown parameters (desc[] empty)
{ 'option': OPT-NAME, parameters: [] }
3. Other argument
{ 'option': OPT-NAME, ? }
This patch adds 3. We need to decide what we want there.
Any particular reason why we have to invent something new, and can't
simply fold it into 2b?
I thinks it's ok, it's just the output format, then the 'parameters'
isn't optional.
Note that I completely left out help strings, both on the option and on
the parameter level. Both for brevity of presentation, and because I'd
like to see a use before we add them.
Something was _wrong_ in this patch, I want to additionally output
helpmsg only for this case:
If option has parameter, and we didn't convert the option to use
QemuOpts (with good desc table), then help message is helpful.
This only effects some legacy options.
|-set group.id.arg=value
|set arg parameter for item id of type group
|i.e. -set drive.$id.file=/path/to/image
|-qtest-log /dev/null
|-qtest
|-writeconfig file
| read/write config file
|
|
diff --git a/util/qemu-config.c b/util/qemu-config.c
index de233d8..a2def03 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -176,6 +176,9 @@ CommandLineOptionInfoList
*qmp_query_command_line_options(bool has_option,
} else if (idx = 0) {
info-parameters =
get_param_infolist(vm_config_groups[idx]-desc);
+} else if (info-has_parameters) {
+info-has_helpmsg = true;
+info-helpmsg = g_strdup(option_helpmsgs[i]);
}
entry = g_malloc0(sizeof(*entry));
--
Amos.
Re: [Qemu-devel] [PATCH v3 21/31] target-arm: Implement AArch64 DAIF system register
On 17 February 2014 00:17, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
On Sun, Feb 16, 2014 at 2:07 AM, Peter Maydell peter.mayd...@linaro.org
wrote:
Implement the DAIF system register which is a view of the
DAIF bits in PSTATE.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
target-arm/helper.c | 24
1 file changed, 24 insertions(+)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 367fbbe..c50ca5a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1589,6 +1589,25 @@ static void aa64_fpsr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
vfp_set_fpsr(env, value);
}
+static CPAccessResult aa64_daif_access(CPUARMState *env, const ARMCPRegInfo
*ri)
+{
+if (arm_current_pl(env) == 0 !(env-cp15.c1_sys SCTLR_UMA)) {
+return CP_ACCESS_TRAP;
+}
+return CP_ACCESS_OK;
+}
+
+static uint64_t aa64_daif_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+return env-daif;
+}
Is it better to just define the .fieldoffset and do away with the
default-behaving read handler? My understanding is this will avoid a
call out to helper context when running under TCG as well, leading to
a slight perf increase.
Yeah; I think this was just a holdover from when it was reading
from pstate and so had to mask out the other bits.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 5/5] query-command-line-options: return help message for legacy options
On Wed, Feb 12, 2014 at 02:00:51PM -0700, Eric Blake wrote:
On 01/27/2014 08:53 PM, Amos Kong wrote:
Some legacy options that have arguments weren't added to
vm_config_groups[], so query-command-line-options returns a
NULL parameters infolist. This patch try to return help message
for this kind of legacy options.
Example:
{
helpmsg: \-vnc displaystart a VNC server on display\\n\,
parameters: [
],
option: vnc
},
Signed-off-by: Amos Kong ak...@redhat.com
---
qapi-schema.json | 5 -
util/qemu-config.c | 3 +++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 05ced9d..b3e6f46 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3943,13 +3943,16 @@
# Details about a command line option, including its list of parameter
details
#
# @option: option name
+# @helpmsg: help message for legacy options
Missing #optional and (since 2.0) designations
#
# @parameters: an array of @CommandLineParameterInfo
Might be worth documenting #optional since 2.0 (we don't yet have good
precedent for documenting when a formerly mandatory field became optional).
Groan. This is an output struct. On input structs, changing a
mandatory field to optional is safe - old callers will always supply the
field. But on output structs, changing a mandatory field to optional is
backwards-incompatible. Old callers may be blindly expecting the field,
and crash when it is not present.
Your approach needs to be modified.
#
# Since 1.5
##
{ 'type': 'CommandLineOptionInfo',
- 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } }
+ 'data': { 'option': 'str',
+'*parameters': ['CommandLineParameterInfo'],
+'*helpmsg': 'str' } }
I suggest:
# @parameters: array of @CommandLineParameterInfo, possibly empty
# @argument: @optional present if the @parameters array is empty. If
#true, then the option takes unspecified arguments, if
#false, then the option is merely a boolean flag (since 2.0)
{ 'type': 'CommandLineOptionInfo',
'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'],
'*argument': 'bool' } }
used as:
[
{ option:enable-fips, parameters:[], argument:false },
{ option:smbios, parameters:[], argument:true },
{ option:iscsi, paramters:[ ... ] },
...
]
It's ok to use a split argument to indicate if option has
parameters. and the parameters will not be optional, it will
be NULL list in the case of no parameters.
Thanks.
which adequately differentiates between -iscsi taking arguments (but
where we haven't yet hooked it in to introspect those arguments) vs.
-enable-fips being boolean.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
Amos.
[Qemu-devel] [PATCH] trace-events: Fix typo in offset
s/offet/offset/ Signed-off-by: Kevin Wolf kw...@redhat.com --- trace-events | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/trace-events b/trace-events index ab11f97..3713063 100644 --- a/trace-events +++ b/trace-events @@ -495,10 +495,10 @@ qcow2_writev_done_part(void *co, int cur_nr_sectors) co %p cur_nr_sectors %d qcow2_writev_data(void *co, uint64_t offset) co %p offset % PRIx64 # block/qcow2-cluster.c -qcow2_alloc_clusters_offset(void *co, uint64_t offset, int num) co %p offet % PRIx64 num %d -qcow2_handle_copied(void *co, uint64_t guest_offset, uint64_t host_offset, uint64_t bytes) co %p guest_offet % PRIx64 host_offset % PRIx64 bytes % PRIx64 -qcow2_handle_alloc(void *co, uint64_t guest_offset, uint64_t host_offset, uint64_t bytes) co %p guest_offet % PRIx64 host_offset % PRIx64 bytes % PRIx64 -qcow2_do_alloc_clusters_offset(void *co, uint64_t guest_offset, uint64_t host_offset, int nb_clusters) co %p guest_offet % PRIx64 host_offset % PRIx64 nb_clusters %d +qcow2_alloc_clusters_offset(void *co, uint64_t offset, int num) co %p offset % PRIx64 num %d +qcow2_handle_copied(void *co, uint64_t guest_offset, uint64_t host_offset, uint64_t bytes) co %p guest_offset % PRIx64 host_offset % PRIx64 bytes % PRIx64 +qcow2_handle_alloc(void *co, uint64_t guest_offset, uint64_t host_offset, uint64_t bytes) co %p guest_offset % PRIx64 host_offset % PRIx64 bytes % PRIx64 +qcow2_do_alloc_clusters_offset(void *co, uint64_t guest_offset, uint64_t host_offset, int nb_clusters) co %p guest_offset % PRIx64 host_offset % PRIx64 nb_clusters %d qcow2_cluster_alloc_phys(void *co) co %p qcow2_cluster_link_l2(void *co, int nb_clusters) co %p nb_clusters %d -- 1.8.1.4
Re: [Qemu-devel] Trying to write a new device / virtio-i2c ?
Il 17/02/2014 09:35, Alex David ha scritto: - Are there solutions that seems more adapted to my case ? Like using USB-I2C bridge ? From an upstream point of view, a host passthrough device pair (one object talking to /dev/i2c-N on the host, and one device per sensor talking to the other object) would be the best. But if you can make I2C-over-parallel work, that would also be interesting. And not much new code to write: all you need to parse the bitbanging I2C protocol is already in hw/i2c/bitbang_i2c.c. Paolo
[Qemu-devel] Fwd: Trying to write a new device / virtio-i2c ?
2014-02-17 10:19 GMT+01:00 Paolo Bonzini pbonz...@redhat.com: Il 17/02/2014 09:35, Alex David ha scritto: - Are there solutions that seems more adapted to my case ? Like using USB-I2C bridge ? From an upstream point of view, a host passthrough device pair (one object talking to /dev/i2c-N on the host, and one device per sensor talking to the other object) would be the best. i2c-N is on the guest, that's why I want to virtualize an i2c hardware on QEMU. My idea was getting the data on a socket on the host = read these data from my daemon. That's why virtio-serial seemed like a good thing to use / adapt as i2c. I'll look on the parallel solution, I don't think there's a virtio-parallel, right ?
Re: [Qemu-devel] qemu_rdma_cleanup seg - related to 5a91337?
* Michael R. Hines (mrhi...@linux.vnet.ibm.com) wrote:
On 02/06/2014 08:26 PM, Dr. David Alan Gilbert wrote:
Hi Isaku,
I hit a seg in qemu_rdma_cleanup in the code changed by your
'[PATCH] rdma: clean up of qemu_rdma_cleanup()'
migration-rdma.c ~ 2241
if (rdma-qp) {
rdma_destroy_qp(rdma-cm_id);
rdma-qp = NULL;
}
Your patch changed that to free cm_id at that point rather than
qp; but in my case cm_id is NULL and so rdma_destroy_qp segs.
given that there is a :
if (rdma-cm_id) {
rdma_destroy_id(rdma-cm_id);
rdma-cm_id = NULL;
}
later down, and there is now no longer any destroy of rdma-qp
I don't understand your change.
Your change text says:
'- RDMAContext::qp is created by rdma_create_qp() so that it should be
destroyed
by rdma_destroy_qp(). not ibv_destroy_qp()'
but the diff is:
if (rdma-qp) {
-ibv_destroy_qp(rdma-qp);
+rdma_destroy_qp(rdma-cm_id);
rdma-qp = NULL;
should that have been rdma_destroy_qp(rdma-qp)?
Dave (who doesn't yet know enough RDMA to be dangerous)
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Hi Michael,
Responding for Isaku. Thanks for reporting the bug, but I need some help
in tracking down the cause of the bug, see below.
Actually, the parameter rdma-cm_id to the function is correct, it's just
that the variable never got initialized in the first place, which
means that either
the connection never got established or an early error happened during
the migration that required cleaning up the identifier.
Can you describe the conditions of the migration and the environment?
1. Did you migrate only one VM? Was the host under heavy load?
2. Did your migration lose connectivity? Did one of the hosts crash?
3. Was the connection abruptly broken for some reason?
4. Did you ever cancel the migration at some point and restart?
5. Did you use libvirt?
This is my 1st attempt with RDMA and I'm using softiwarp and
getting an early error, I've not tracked down why yet, hence why I'm
only really reporting the cleanup seg.
outgoing:
rdma_get_cm_event != EVENT_ESTABLISHED after rdma_connect: No such file or
directory
RDMA ERROR: connecting to destination!
migration: setting error state
migration: setting error state
migrate: RDMA ERROR: connecting to destination!
(qemu)
incoming:
ibv_poll_cq wc.status=5 Work Request Flushed Error!
ibv_poll_cq wrid=CONTROL RECV!
messages from qemu_rdma_poll
I've not 100% sure which side fails first yet, but I believe that
incoming fails after outgoing calls rdma_connect but before it calls
rdma_get_cm_event; but as I say I'm new to RDMA and it's my 1st time
trying to debug it.
A simple fix would be to surround the rdma_destroy_qp() call with a check
to see if rdma-cm_id is valid, but that doesn't answer why
rdma-cm_id would be invalid
in the first place.
Yeh, I think just adding the NULL check is best.
I need some additional information to try to reproduce the
conditions of the bug.
Thanks!
- Michael Hines
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] Fwd: Trying to write a new device / virtio-i2c ?
Il 17/02/2014 10:38, Alex David ha scritto: From an upstream point of view, a host passthrough device pair (one object talking to /dev/i2c-N on the host, and one device per sensor talking to the other object) would be the best. i2c-N is on the guest, that's why I want to virtualize an i2c hardware on QEMU. My idea was getting the data on a socket on the host = read these data from my daemon. That's why virtio-serial seemed like a good thing to use / adapt as i2c. I'll look on the parallel solution, I don't think there's a virtio-parallel, right ? You do not need to use virtio at all costs. Paravirtualization is just where existing hardware doesn't have the required performance or feature set. If you don't use paravirtualization, you have the big advantage of not having to write a guest driver! You have not answered my question. Are you emulating a bunch of sensors/actuators? Or do the sensors/actuators physically exist in the host? So far I assumed the latter. If that is not correct, and you just need to emulate a temperature sensor or something like that, there is already an I2C bus in the guest. If you do modprobe i2c-dev in the guest you'll see it. Just use -device and QEMU will attach the emulated sensor to that bus. Something like USB-I2C is only needed if you need more than one I2C bus (e.g. because of two devices with the same address). And parallel port I2C is probably useless for anything except passing through a host /dev/i2c-N to the guest. Paolo
Re: [Qemu-devel] [PATCH] block: Unlink temporary file
On Sat, Feb 15, 2014 at 06:03:21PM +0100, Max Reitz wrote:
If the image file cannot be opened and was created as a temporary file,
it should be deleted; thus, in this case, we should jump to the
unlink_and_fail label and not just to fail.
Reported-by: Benoît Canet ben...@irqsave.net
Signed-off-by: Max Reitz mre...@redhat.com
---
This patch's context depends on my bdrv_open()/bdrv_file_open() series
([PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open()).
---
block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 62161bd..51c77b0 100644
--- a/block.c
+++ b/block.c
@@ -1308,7 +1308,7 @@ int bdrv_open(BlockDriverState **pbs, const char
*filename,
bdrv_open_flags(bs, flags | BDRV_O_UNMAP) |
BDRV_O_PROTOCOL, true, local_err);
if (ret 0) {
-goto fail;
+goto unlink_and_fail;
}
/* Find the right image format driver */
Acked-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [PATCH] target-i386: Fix I/O bitmap checks for in/out
Am 24.01.2014 um 11:19 hat Kevin Wolf geschrieben: Commit 1b90d56e changed the implementation of in/out imm to not assign the accessed port number to cpu_T[0] as it appeared unnecessary. However, currently gen_check_io() makes use of cpu_T[0] to implement the I/O bitmap checks, so it's in fact still used and the change broke the check, leading to #GP in legitimate cases (and probably also allowing access to ports that shouldn't be allowed). This patch reintroduces the missing assignment for these cases. Signed-off-by: Kevin Wolf kw...@redhat.com Reviewed-by: Richard Henderson r...@twiddle.net Ping? /me considers sending a one-patch pull request for an area he's absolutely not maintaining, but if this is the only way to get patches applied to qemu... Kevin --- target-i386/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-i386/translate.c b/target-i386/translate.c index b0f2279..5dd2450 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -6284,6 +6284,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, case 0xe5: ot = mo_b_d32(b, dflag); val = cpu_ldub_code(env, s-pc++); +tcg_gen_movi_tl(cpu_T[0], val); gen_check_io(s, ot, pc_start - s-cs_base, SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes)); if (use_icount) @@ -6300,6 +6301,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, case 0xe7: ot = mo_b_d32(b, dflag); val = cpu_ldub_code(env, s-pc++); +tcg_gen_movi_tl(cpu_T[0], val); gen_check_io(s, ot, pc_start - s-cs_base, svm_is_rep(prefixes)); gen_op_mov_v_reg(ot, cpu_T[1], R_EAX); -- 1.8.1.4
Re: [Qemu-devel] [PATCH v2] qdev: add the device to the QOM tree before using it to set a link
On Thu, Jan 02, 2014 at 09:02:11AM +0800, Amos Kong wrote:
Test steps:
(qemu) device_add e1000,addr=adsf
Property 'e1000.addr' doesn't take value 'adsf'
(qemu) info qtree
Then qemu crashed.
Currently we set a link to the new device for qdev parent bus, but the
device hasn't been added to QOM tree. When it fails to set properties,
object_unparent() can't cleanup the device.
This patch moves qdev_set_parent_bus() back to object_property_add_child(),
we only needs to unref the object if setting properties fails.
Signed-off-by: Amos Kong ak...@redhat.com
---
V2: fix bz by adjust the initialization order (Paolo)
Hi Anthony, other maintainer
The V2 already reviewed and tested by Markus. Can you help to review apply it?
Thanks, Amos
---
qdev-monitor.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index dc37a43..4070b0a 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -518,16 +518,11 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* create device, set properties */
dev = DEVICE(object_new(driver));
-if (bus) {
-qdev_set_parent_bus(dev, bus);
-}
-
id = qemu_opts_id(opts);
if (id) {
dev-id = id;
}
if (qemu_opt_foreach(opts, set_property, dev, 1) != 0) {
-object_unparent(OBJECT(dev));
object_unref(OBJECT(dev));
return NULL;
}
@@ -541,6 +536,11 @@ DeviceState *qdev_device_add(QemuOpts *opts)
OBJECT(dev), NULL);
g_free(name);
}
+
+if (bus) {
+qdev_set_parent_bus(dev, bus);
+}
+
object_property_set_bool(OBJECT(dev), true, realized, err);
if (err != NULL) {
qerror_report_err(err);
--
1.8.4.2
Re: [Qemu-devel] [PATCH v2] RFC: Add blockdev-del QMP command
Am 14.02.2014 um 20:17 hat Ian Main geschrieben:
On Thu, Feb 13, 2014 at 09:59:40AM +0100, Kevin Wolf wrote:
Am 12.02.2014 um 18:36 hat Ian Main geschrieben:
This is the sister command to blockdev-add. In Fam's example he uses
the drive_del HMP command to clean up but it would be much nicer to
have a way to do this via QMP.
Signed-off-by: Ian Main im...@redhat.com
diff --git a/qapi-schema.json b/qapi-schema.json
index d22651c..01186cd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4469,3 +4469,14 @@
# Since: 1.7
##
{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
+
+##
+# @blockdev-del:
+#
+# Delete a block device.
+#
+# @device: Identifier for the block device to be deleted.
+#
+# Since: 2.0
+##
+{ 'command': 'blockdev-del', 'data': { 'device': 'str' } }
I believe the full documentation should go here as well, not just in
qmp-commands.hx.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c3ee46a..f08045d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3442,6 +3442,36 @@ Example (2):
EQMP
{
+.name = blockdev-del,
+.args_type = device:s,
+.mhandler.cmd_new = qmp_marshal_input_blockdev_del,
+},
+
+SQMP
+blockdev-del
+
+
+Remove host block device. The result is that guest generated IO is no
+longer submitted against the host device underlying the disk. Once a
+drive has been deleted, the QEMU Block layer returns -EIO which results
+in IO errors in the guest for applications that are reading/writing to
+the device. These errors are always reported to the guest, regardless
+of the drive's error actions (drive options rerror, werror).
I think we wanted to have different semantics for blockdev-del.
Specifically, it is a backend command that should have no effect on
users of that backend.
Let me paste and comment on some notes I made in a previous blockdev
discussion:
* Make sure that an explicit blockdev-del is needed to remove the
BDS; it shouldn't happen automagically just because the guest
device was unplugged
[done]
* By default, return an error for blockdev-del if reference count 1
[ The assumption is here that one reference is held by the
monitor/external user, which isn't true today to my knowledge ]
- But have a force option that closes the image file, even if it
breaks the remaining users (e.g. uncooperative guest that doesn't
release its PCI device)
[ Here we need working refcounting including the external user,
because otherwise we don't free the (closed) BDS even when the
guest device is unplugged. It is an open question whether and
how BDSes without an external reference are shown in the
monitor. ]
* Prevent mixing blockdev-add with drive_del and vice versa
- Ideally drive_add BDSes are exactly those with a DriveInfo
[ not true today, but DriveInfo.enable_auto_del can be used to
distinguish them ]
So I believe we still have some design work to do before we can actually
implement this. I would prefer not to merge this for 2.0.
Kevin
Are we only changing the semantics/implementation of the API, or is the
API itself going to change with these improvements? If that were the
case wouldn't it make some sense to get people using blockdev-del now
and update the semantics later? As it is now consumers will just end
up using blockdev-add/drive_del.
It should be obvious that you can't change the semantics after the fact.
The API is not just a command name and a set of arguments, but clearly
also what happens when you invoke the command.
Doing one thing now and changing it later to behave differently will
break any management tool because it wouldn't be able to know what the
command means for the specific qemu version it's talking to.
Kevin
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On Mon, Feb 17, 2014 at 09:32:35AM +0100, Gerd Hoffmann wrote: On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin wrote: On Fri, Feb 07, 2014 at 01:51:27PM +0100, Igor Mammedov wrote: Since introduction of PCIHP, it became problematic to punch hole in PCI0._CRS statically since PCI hotplug region size became runtime changeable. What makes it runtime changeable? machine type. q35 / piix map them at different locations. That's not dynamic. We can load the correct ones per DSDT. Also we might want to this also for devices which are runtime-configurable (isa-debugcon, pvpanic, ...). That's more convincing, but I don't want knowledge of all these devices in acpi-build. Also we need to make seabios avoid these ranges when enumerating devices. How does it know to avoid them ATM? cheers, Gerd
Re: [Qemu-devel] Exposing and calculating CPU APIC IDs (was Re: [RFC 1/3] target-i386: moving registers of vmstate from cpu_exec_init() to x86_cpu_realizefn())
On Thu, 2014-02-13 at 10:44 +0100, Igor Mammedov wrote: On Thu, 13 Feb 2014 14:14:08 +0800 Chen Fan chen.fan.f...@cn.fujitsu.com wrote: On Tue, 2014-01-21 at 11:10 +0100, Andreas Färber wrote: Am 21.01.2014 10:51, schrieb Chen Fan: On Tue, 2014-01-21 at 10:31 +0100, Igor Mammedov wrote: On Tue, 21 Jan 2014 15:12:45 +0800 Chen Fan chen.fan.f...@cn.fujitsu.com wrote: On Mon, 2014-01-20 at 13:29 +0100, Igor Mammedov wrote: On Fri, 17 Jan 2014 17:13:55 -0200 Eduardo Habkost ehabk...@redhat.com wrote: On Wed, Jan 15, 2014 at 03:37:04PM +0100, Igor Mammedov wrote: I recall there were objections to it since APIC ID contains topology information and it's not trivial for user to get it right. The last idea that was discussed to fix it was not expose APIC ID to user but rather introduce QOM hierarchy like: /machine/node/N/socket/X/core/Y/thread/Z and use it in user interface as a means to specify an arbitrary CPU and let QEMU calculate APIC ID based on this path. But nobody took on implementing it yet. We're taking so long to get a decent interface implemented, that part of me is considering exposing the APIC ID directly like suggested before, and requiring libvirt to calculate topology-aware APIC IDs[1] to properly implement CPU hotplug (and possibly for other tasks). If you are speaking about 'qemu will core dump with -smp 254, sockets=2, cores=3, threads=2' http://patchwork.ozlabs.org/patch/301272/ bug then it's limitation of ACPI implementation, I'm going to refactor it to use full APIC ids instead of using bitmap, so that we won't ever run into issue regardless of cpu supported CPU count. Another part of me is hoping that the libvirt developers ask us to please not do that, so I can use it as argument against exposing the APIC IDs directly the next time we discuss this. :) why not try your /machine/node/N/socket/X/core/Y/thread/Z idea first. It will benefit not only cpu hotplug but also '-numa' and topology description in general. have there been any plan/model of the idea? Need to add a new option to qemu command? I suppose we can start with internal default implementation first. one way could be 1. let machine prebuild empty QOM tree /machine/node/N/socket/X/core/Y/thread/Z 2. add node, socket, core, thread properties to CPU and link CPU into respective link created by #1 Thanks, I hope I can take some time to make some patches to implement it. Please give us a few hours to reply. :) /machine/node seems too broad a term to me. You can't prebuild the full tree, you can only prepare the nodes. core[Y]/thread[Z] was previously discussed as syntax. The important part to decide on will be what is going to be child and what link. Has anyone played with the Intel Quark platform for instance? (Galileo board or upcoming Edison card) On a regular mainboard, we would have socket[X] as a linkx86_64-cpu, which might point to a childcpu /machine/memory-node[W]/cpu[X]. But if we do so we can't reassign it to another memory node - acceptable? With Quark (or Qseven modules etc.) there would be a container object rather than the /machine itself that has a childi386-cpu instead of a linki386-cpu. I guess the memory nodes could still be on the /machine though. The other point of discussion between Anthony and me was whether core[Y] should be a link or child, same for thread. I believe a child is better as it enforces that unrealizing the CPU will unrealize all its cores and all its threads in the future. More issues may pop up when thinking about it longer than a few minutes. But yes, we need to start investigating this, and so far I had other priorities like getting the CPUState mess I created cleaned up. Hi, Igor, Andreas, In addition, I want to know what way user could use to specify an arbitrary CPU if using /machine/node/N/socket/X/core/Y/thread/Z idea? -device qemu64,socket=X,core=Y,thread=Z? or add a new optional command line? Definitely not another CLU option. I see a couple of options, 1. as you suggest with additional 'numa=N' so that QEMU could know where to attach a new CPU. 2. add 'parent' like option tied to linkcpu property and specify full QOM path on CLI: -device cpufoo,parent=/machine/node[N]/socket[X]/... Hi, Igor, Currently, we know, after adding an arbitrary CPU then do migration, on target, there will be not aware that which CPU have been added. in order to notify target of the cpu topo, can we specify full QOM path that you mentioned 2th point on target? if we can simply make smp n + 1 work as well at target to be better, but target how to know the cpu topo on source side? Thanks, Chen Regards, Andreas
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On Sun, 16 Feb 2014 17:53:45 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Feb 07, 2014 at 01:51:27PM +0100, Igor Mammedov wrote: Since introduction of PCIHP, it became problematic to punch hole in PCI0._CRS statically since PCI hotplug region size became runtime changeable. What makes it runtime changeable? piix machine acpi-pci-hotplug-with-bridge-support=on effectively changes size of pcihp MMIO region So replace static hole punching with dynamically consumed resources in a child device on PCI0 bus. i.e generate PNP0C02 device as a child of PCI0 bus at runtime and consume GPE0, PCI/CPU hotplug IO resources in it instead of punching holes in static PCI0._CRS. It seems that we are being too exact with IO resources here. Can't we roughly reserve 0xae00 to 0xafff and be done with it? that would be easiest way for this specific case if we could agree for ranges on PIIX/Q35 machines. But I also use it as excuse to introduce ASL like macros so that building dynamic SSDT would be easier i.e. replace template patching with single place where dynamic device is defined and its values are set in simple and straightforward manner. Tested with Windows XPsp3, Vista, Windows Server 2003, 2008, 2012r2. PS: Series adds several ASL like macros to simplify code for dynamic generation of AML structures. Igor Mammedov (9): Revert pc: Q35 DSDT: exclude CPU hotplug IO range from PCI bus resources Revert pc: PIIX DSDT: exclude CPU/PCI hotplug GPE0 IO range from PCI bus resources Partial revert pc: ACPI: expose PRST IO range via _CRS acpi: replace opencoded opcodes with defines acpi: add PNP0C02 to PCI0 bus acpi: consume GPE0 IO resources in PNP0C02 device acpi: consume CPU hotplug IO resource in PNP0C02 device pcihp: expose PCI hotplug MMIO base/length as properties of piix4pm acpi: consume PCIHP IO resource in PNP0C02 device hw/acpi/pcihp.c | 28 ++ hw/acpi/piix4.c |1 + hw/i386/acpi-build.c | 177 ++-- hw/i386/acpi-dsdt-cpu-hotplug.dsl | 11 --- hw/i386/acpi-dsdt-pci-crs.dsl | 15 +++- hw/i386/acpi-dsdt.dsl | 39 hw/i386/q35-acpi-dsdt.dsl | 16 include/hw/acpi/pcihp.h |4 + 8 files changed, 214 insertions(+), 77 deletions(-) -- Regards, Igor
Re: [Qemu-devel] Fwd: Trying to write a new device / virtio-i2c ?
Il 17/02/2014 11:01, Alex David ha scritto:
I indeed don't use paravirtualization.
Virtio _is_ paravirtualization. :)
I'm emulating a bunch of sensors/actuators.
If I virtualize my sensors and attach them to the i2c-dev with -device,
how do I get those data on the host then ?
It depends on your use case.
It could be that you can make them return a constant value.
Otherwise, you may want to use a chardev for that purpose, or finally a
QOM (QEMU Object Model) property. For example, add
CONFIG_TMP105=y
to default-configs/x86_64-softmmu.mak before building QEMU, then do the
following (indented = in the guest):
$ x86_64-softmmu/qemu-system-x86_64 --enable-kvm ~/test2.img -m 256 \
-device tmp105,id=sensor,address=0x50 \
-qmp unix:$HOME/qmp.sock,server,nowait
$ qmp/qom-list -s ~/qmp.sock /machine/peripheral/sensor
temperature
@parent_bus/
address
hotpluggable
realized
type
$ scripts/qmp/qmp-shell ~/qmp.sock
(QEMU) qom-get path=/machine/peripheral/sensor property=temperature
{u'return': 0}
(QEMU) qom-get path=sensor property=address
{u'return': 80}
# modprobe i2c-dev
# i2cget -y 0 0x50 0 w
0x
(QEMU) qom-set path=sensor property=temperature value=2
{u'return': {}}
# i2cget -y 0 0x50 0 w
0x0014
For this particular sensor, you have to swap the two bytes and the
result is 8.8 fixed-point.
Paolo
Thanks for your help. As you may see, I'm not that experienced in
QEMU/Linux kernel.
Re: [Qemu-devel] Exposing and calculating CPU APIC IDs (was Re: [RFC 1/3] target-i386: moving registers of vmstate from cpu_exec_init() to x86_cpu_realizefn())
On Mon, 17 Feb 2014 18:24:09 +0800 Chen Fan chen.fan.f...@cn.fujitsu.com wrote: On Thu, 2014-02-13 at 10:44 +0100, Igor Mammedov wrote: On Thu, 13 Feb 2014 14:14:08 +0800 Chen Fan chen.fan.f...@cn.fujitsu.com wrote: On Tue, 2014-01-21 at 11:10 +0100, Andreas Färber wrote: Am 21.01.2014 10:51, schrieb Chen Fan: On Tue, 2014-01-21 at 10:31 +0100, Igor Mammedov wrote: On Tue, 21 Jan 2014 15:12:45 +0800 Chen Fan chen.fan.f...@cn.fujitsu.com wrote: On Mon, 2014-01-20 at 13:29 +0100, Igor Mammedov wrote: On Fri, 17 Jan 2014 17:13:55 -0200 Eduardo Habkost ehabk...@redhat.com wrote: On Wed, Jan 15, 2014 at 03:37:04PM +0100, Igor Mammedov wrote: I recall there were objections to it since APIC ID contains topology information and it's not trivial for user to get it right. The last idea that was discussed to fix it was not expose APIC ID to user but rather introduce QOM hierarchy like: /machine/node/N/socket/X/core/Y/thread/Z and use it in user interface as a means to specify an arbitrary CPU and let QEMU calculate APIC ID based on this path. But nobody took on implementing it yet. We're taking so long to get a decent interface implemented, that part of me is considering exposing the APIC ID directly like suggested before, and requiring libvirt to calculate topology-aware APIC IDs[1] to properly implement CPU hotplug (and possibly for other tasks). If you are speaking about 'qemu will core dump with -smp 254, sockets=2, cores=3, threads=2' http://patchwork.ozlabs.org/patch/301272/ bug then it's limitation of ACPI implementation, I'm going to refactor it to use full APIC ids instead of using bitmap, so that we won't ever run into issue regardless of cpu supported CPU count. Another part of me is hoping that the libvirt developers ask us to please not do that, so I can use it as argument against exposing the APIC IDs directly the next time we discuss this. :) why not try your /machine/node/N/socket/X/core/Y/thread/Z idea first. It will benefit not only cpu hotplug but also '-numa' and topology description in general. have there been any plan/model of the idea? Need to add a new option to qemu command? I suppose we can start with internal default implementation first. one way could be 1. let machine prebuild empty QOM tree /machine/node/N/socket/X/core/Y/thread/Z 2. add node, socket, core, thread properties to CPU and link CPU into respective link created by #1 Thanks, I hope I can take some time to make some patches to implement it. Please give us a few hours to reply. :) /machine/node seems too broad a term to me. You can't prebuild the full tree, you can only prepare the nodes. core[Y]/thread[Z] was previously discussed as syntax. The important part to decide on will be what is going to be child and what link. Has anyone played with the Intel Quark platform for instance? (Galileo board or upcoming Edison card) On a regular mainboard, we would have socket[X] as a linkx86_64-cpu, which might point to a childcpu /machine/memory-node[W]/cpu[X]. But if we do so we can't reassign it to another memory node - acceptable? With Quark (or Qseven modules etc.) there would be a container object rather than the /machine itself that has a childi386-cpu instead of a linki386-cpu. I guess the memory nodes could still be on the /machine though. The other point of discussion between Anthony and me was whether core[Y] should be a link or child, same for thread. I believe a child is better as it enforces that unrealizing the CPU will unrealize all its cores and all its threads in the future. More issues may pop up when thinking about it longer than a few minutes. But yes, we need to start investigating this, and so far I had other priorities like getting the CPUState mess I created cleaned up. Hi, Igor, Andreas, In addition, I want to know what way user could use to specify an arbitrary CPU if using /machine/node/N/socket/X/core/Y/thread/Z idea? -device qemu64,socket=X,core=Y,thread=Z? or add a new optional command line? Definitely not another CLU option. I see a couple of options, 1. as you suggest with additional 'numa=N' so that QEMU could know where to attach a new CPU. 2. add 'parent' like option tied to linkcpu property and specify full QOM path on CLI: -device cpufoo,parent=/machine/node[N]/socket[X]/... Hi, Igor, Currently, we know, after adding an arbitrary CPU then do migration, on target, there will be not aware that which CPU have been added. in order to notify target of the cpu topo, can we specify full QOM path that you
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On Mo, 2014-02-17 at 12:28 +0200, Michael S. Tsirkin wrote: On Mon, Feb 17, 2014 at 09:32:35AM +0100, Gerd Hoffmann wrote: On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin wrote: On Fri, Feb 07, 2014 at 01:51:27PM +0100, Igor Mammedov wrote: Since introduction of PCIHP, it became problematic to punch hole in PCI0._CRS statically since PCI hotplug region size became runtime changeable. What makes it runtime changeable? machine type. q35 / piix map them at different locations. That's not dynamic. We can load the correct ones per DSDT. Also we might want to this also for devices which are runtime-configurable (isa-debugcon, pvpanic, ...). That's more convincing, but I don't want knowledge of all these devices in acpi-build. Also we need to make seabios avoid these ranges when enumerating devices. How does it know to avoid them ATM? seabios maps io ports @ 0xc000 up. recently it has changed to use 0x1000 - 0xa000 region in case the hole above 0xc000 is too small. In other words: It doesn't map anything below 0x1000 and it avoids 0xa000 - 0xbfff. Hardcoded. I want lift the later restriction on q35, by moving pmbase (0xb000 atm) out of the way, so seabios can use the whole 0x1000 - 0x range, but that is still wip. cheers, Gerd
Re: [Qemu-devel] [PATCH] target-i386: Fix I/O bitmap checks for in/out
On 17 February 2014 10:14, Kevin Wolf kw...@redhat.com wrote: Am 24.01.2014 um 11:19 hat Kevin Wolf geschrieben: Commit 1b90d56e changed the implementation of in/out imm to not assign the accessed port number to cpu_T[0] as it appeared unnecessary. However, currently gen_check_io() makes use of cpu_T[0] to implement the I/O bitmap checks, so it's in fact still used and the change broke the check, leading to #GP in legitimate cases (and probably also allowing access to ports that shouldn't be allowed). This patch reintroduces the missing assignment for these cases. Signed-off-by: Kevin Wolf kw...@redhat.com Reviewed-by: Richard Henderson r...@twiddle.net Ping? /me considers sending a one-patch pull request for an area he's absolutely not maintaining, but if this is the only way to get patches applied to qemu... I don't currently have a workflow for identifying and applying patches which aren't in pull requests (apart from obvious fixes build breakage patches, and even there it's depending on my happening to notice them). In this case I'd expect rth to put together a pull request, I guess. Suggestions for better workflows welcome; we have had issues with patches falling through the gaps between maintained subsystems for a long time. thanks -- PMM
Re: [Qemu-devel] [PATCH V17 05/12] quorum: Add quorum_aio_readv.
Am 15.02.2014 um 02:40 hat Max Reitz geschrieben:
On 12.02.2014 23:06, Benoît Canet wrote:
From: Benoît Canet ben...@irqsave.net
Add code to do num_children reads in parallel and cleanup the structures
afterward.
afterwards
Signed-off-by: Benoit Canet ben...@irqsave.net
Reviewed-by: Max Reitz mre...@redhat.com
---
block/quorum.c | 39 ++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/block/quorum.c b/block/quorum.c
index 197cdca..c7a5d79 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -91,10 +91,18 @@ static AIOCBInfo quorum_aiocb_info = {
static void quorum_aio_finalize(QuorumAIOCB *acb)
{
-int ret = 0;
+BDRVQuorumState *s = acb-common.bs-opaque;
+int i, ret = 0;
acb-common.cb(acb-common.opaque, ret);
+if (acb-is_read) {
+for (i = 0; i s-num_children; i++) {
+qemu_vfree(acb-qcrs[i].buf);
+qemu_iovec_destroy(acb-qcrs[i].qiov);
+}
+}
+
g_free(acb-qcrs);
qemu_aio_release(acb);
}
@@ -149,6 +157,34 @@ static void quorum_aio_cb(void *opaque, int ret)
quorum_aio_finalize(acb);
}
+static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs,
+ int64_t sector_num,
+ QEMUIOVector *qiov,
+ int nb_sectors,
+ BlockDriverCompletionFunc *cb,
+ void *opaque)
+{
+BDRVQuorumState *s = bs-opaque;
+QuorumAIOCB *acb = quorum_aio_get(s, bs, qiov, sector_num,
+ nb_sectors, cb, opaque);
+int i;
+
+acb-is_read = true;
+
+for (i = 0; i s-num_children; i++) {
+acb-qcrs[i].buf = qemu_blockalign(s-bs[i], qiov-size);
+qemu_iovec_init(acb-qcrs[i].qiov, qiov-niov);
+qemu_iovec_clone(acb-qcrs[i].qiov, qiov, acb-qcrs[i].buf);
+}
+
+for (i = 0; i s-num_children; i++) {
+bdrv_aio_readv(s-bs[i], sector_num, acb-qcrs[i].qiov, nb_sectors,
+ quorum_aio_cb, acb-qcrs[i]);
I know Kevin told you to, but using the child's own QIOV here means
quorum_aio_readv() won't work after this patch but only after patch
6. Just pointing it out, I don't like it, but Kevin told you to, so:
Sorry, but this is the entirely wrong attitude. Just because I am a
maintainer that doesn't mean that I don't get things wrong. When I talk
bullshit (and probably I do more often than others), just point it out.
In this case, I didn't even tell him to do the change, but I asked in
patch 6:
Why don't you do this from the beginning?
The answer appears to be that the read data isn't copied back to the
original qiov yet, but this happens only in the quorum_copy_qiov() calls
in quorum_vote().
So this is a bug in this patch, which can be fixed either by reverting
to the old state of using the original qiov here, or (perhaps more
nicely) by introducing quorum_copy_qiov() already in this patch.
Kevin
[Qemu-devel] [PATCH] tmp105: read temperature in milli-celsius
Right now, the temperature property must be written in milli-celsius, but it
reads back the value in 8.8 fixed point. Fix this by letting the property
read back the original value (possibly rounded). Also simplify the code that
does the conversion.
Before:
(QEMU) qom-set path=/machine/peripheral/sensor property=temperature
value=2
{u'return': {}}
(QEMU) qom-get path=sensor property=temperature
{u'return': 5120}
After:
(QEMU) qom-set path=/machine/peripheral/sensor property=temperature
value=2
{u'return': {}}
(QEMU) qom-get path=sensor property=temperature
{u'return': 2}
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/misc/tmp105.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c
index 155e03d..63aa3d6 100644
--- a/hw/misc/tmp105.c
+++ b/hw/misc/tmp105.c
@@ -56,12 +56,14 @@ static void tmp105_get_temperature(Object *obj, Visitor *v,
void *opaque,
const char *name, Error **errp)
{
TMP105State *s = TMP105(obj);
-int64_t value = s-temperature;
+int64_t value = s-temperature * 1000 / 256;
visit_type_int(v, value, name, errp);
}
-/* Units are 0.001 centigrades relative to 0 C. */
+/* Units are 0.001 centigrades relative to 0 C. s-temperature is 8.8
+ * fixed point, so units are 1/256 centigrades. A simple ratio will do.
+ */
static void tmp105_set_temperature(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
@@ -78,7 +80,7 @@ static void tmp105_set_temperature(Object *obj, Visitor *v,
void *opaque,
return;
}
-s-temperature = ((int16_t) (temp * 0x800 / 128000)) 4;
+s-temperature = (int16_t) (temp * 256 / 1000);
tmp105_alarm_update(s);
}
--
1.8.5.3
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On Mon, Feb 17, 2014 at 11:33:23AM +0100, Igor Mammedov wrote:
On Sun, 16 Feb 2014 17:53:45 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Fri, Feb 07, 2014 at 01:51:27PM +0100, Igor Mammedov wrote:
Since introduction of PCIHP, it became problematic to
punch hole in PCI0._CRS statically since PCI hotplug
region size became runtime changeable.
What makes it runtime changeable?
piix machine acpi-pci-hotplug-with-bridge-support=on effectively
changes size of pcihp MMIO region
It adds 4 bytes. Let's just reserve a reasonably sized region,
like 512 bytes.
So replace static hole punching with dynamically consumed
resources in a child device on PCI0 bus. i.e generate
PNP0C02 device as a child of PCI0 bus at runtime and
consume GPE0, PCI/CPU hotplug IO resources in it instead
of punching holes in static PCI0._CRS.
It seems that we are being too exact with
IO resources here.
Can't we roughly reserve 0xae00 to 0xafff
and be done with it?
that would be easiest way for this specific case if we could agree
for ranges on PIIX/Q35 machines.
But I also use it as excuse to introduce ASL like macros so that
building dynamic SSDT would be easier i.e. replace template patching
with single place where dynamic device is defined and its values
are set in simple and straightforward manner.
We don't need excuses really :)
But the approach itself needs some work IMHO, in particular
ASL like syntax by using macros and varargs is not something to strive
for IMHO :)
Why don't we just pass GArrays around?
crs = build_alloc_crs();
build_append_resource(crs, )
Or if you really want this, using array of GArray:
build_append_crs(ssdt,
{
build_alloc_resource(...),
build_alloc_resource(...),
build_alloc_resource(...),
build_alloc_resource(...),
NULL
})
Tested with Windows XPsp3, Vista, Windows Server 2003, 2008, 2012r2.
PS:
Series adds several ASL like macros to simplify
code for dynamic generation of AML structures.
Igor Mammedov (9):
Revert pc: Q35 DSDT: exclude CPU hotplug IO range from PCI bus
resources
Revert pc: PIIX DSDT: exclude CPU/PCI hotplug GPE0 IO range from
PCI bus resources
Partial revert pc: ACPI: expose PRST IO range via _CRS
acpi: replace opencoded opcodes with defines
acpi: add PNP0C02 to PCI0 bus
acpi: consume GPE0 IO resources in PNP0C02 device
acpi: consume CPU hotplug IO resource in PNP0C02 device
pcihp: expose PCI hotplug MMIO base/length as properties of piix4pm
acpi: consume PCIHP IO resource in PNP0C02 device
hw/acpi/pcihp.c | 28 ++
hw/acpi/piix4.c |1 +
hw/i386/acpi-build.c | 177
++--
hw/i386/acpi-dsdt-cpu-hotplug.dsl | 11 ---
hw/i386/acpi-dsdt-pci-crs.dsl | 15 +++-
hw/i386/acpi-dsdt.dsl | 39
hw/i386/q35-acpi-dsdt.dsl | 16
include/hw/acpi/pcihp.h |4 +
8 files changed, 214 insertions(+), 77 deletions(-)
--
Regards,
Igor
Re: [Qemu-devel] [RFC 0/9] generate dynamic _CRS for motherboard resources
On Mon, Feb 17, 2014 at 11:46:13AM +0100, Gerd Hoffmann wrote: On Mo, 2014-02-17 at 12:28 +0200, Michael S. Tsirkin wrote: On Mon, Feb 17, 2014 at 09:32:35AM +0100, Gerd Hoffmann wrote: On So, 2014-02-16 at 17:53 +0200, Michael S. Tsirkin wrote: On Fri, Feb 07, 2014 at 01:51:27PM +0100, Igor Mammedov wrote: Since introduction of PCIHP, it became problematic to punch hole in PCI0._CRS statically since PCI hotplug region size became runtime changeable. What makes it runtime changeable? machine type. q35 / piix map them at different locations. That's not dynamic. We can load the correct ones per DSDT. Also we might want to this also for devices which are runtime-configurable (isa-debugcon, pvpanic, ...). That's more convincing, but I don't want knowledge of all these devices in acpi-build. Also we need to make seabios avoid these ranges when enumerating devices. How does it know to avoid them ATM? seabios maps io ports @ 0xc000 up. recently it has changed to use 0x1000 - 0xa000 region in case the hole above 0xc000 is too small. In other words: It doesn't map anything below 0x1000 and it avoids 0xa000 - 0xbfff. Hardcoded. I want lift the later restriction on q35, by moving pmbase (0xb000 atm) out of the way, so seabios can use the whole 0x1000 - 0x range, but that is still wip. cheers, Gerd okay so we'll want some fwcfg for that correct? whatever fills that, can share logic with acpi generation :) That might or might not make it dynamic enough to make it worth bothering - alternative is just two version of acpi depending on machine type. -- MST
Re: [Qemu-devel] [PATCH] target-i386: Fix I/O bitmap checks for in/out
Am 17.02.2014 um 11:47 hat Peter Maydell geschrieben: On 17 February 2014 10:14, Kevin Wolf kw...@redhat.com wrote: Am 24.01.2014 um 11:19 hat Kevin Wolf geschrieben: Commit 1b90d56e changed the implementation of in/out imm to not assign the accessed port number to cpu_T[0] as it appeared unnecessary. However, currently gen_check_io() makes use of cpu_T[0] to implement the I/O bitmap checks, so it's in fact still used and the change broke the check, leading to #GP in legitimate cases (and probably also allowing access to ports that shouldn't be allowed). This patch reintroduces the missing assignment for these cases. Signed-off-by: Kevin Wolf kw...@redhat.com Reviewed-by: Richard Henderson r...@twiddle.net Ping? /me considers sending a one-patch pull request for an area he's absolutely not maintaining, but if this is the only way to get patches applied to qemu... I don't currently have a workflow for identifying and applying patches which aren't in pull requests (apart from obvious fixes build breakage patches, and even there it's depending on my happening to notice them). In this case I'd expect rth to put together a pull request, I guess. The problem is the I guess part, especially if Richard guesses otherwise. target-i386 happens to be an officially unmaintained area. This is the get_maintainer.pl output: qemu-devel@nongnu.org (odd fixer:X86) Richard Henderson r...@twiddle.net (commit_signer:123/126=98%) Peter Maydell peter.mayd...@linaro.org (commit_signer:51/126=40%) Paolo Bonzini pbonz...@redhat.com (commit_signer:32/126=25%) Blue Swirl blauwir...@gmail.com (commit_signer:13/126=10%) Richard, would you be willing to take up official maintainership to solve at least this uncertainty? Suggestions for better workflows welcome; we have had issues with patches falling through the gaps between maintained subsystems for a long time. Yes, we have a lot of code that doesn't fall in any subsystem with a subtree maintainer. This is the really worrying part here. I'm pretty sure I would get this specific patch merged the one or the other way (after all, my pull requests are generally accepted), but if even I fail to get it in using the normal way, it probably also means that contributors outside of the core team have no chance at all getting any patches in. This is alarming and certainly can't be healthy. I think Anthony did try to apply such patches that don't belong to any submaintainer's area (even though often with considerable delays), but I'm not sure how much time it cost him and how he managed to filter them. Anthony, any hints? Kevin
[Qemu-devel] [PATCH 0/3] pm_smbus: correctly report unclaimed cycles
Prodded by Alex David's i2c thread, I tried running i2cdetect on an x86 machine, and it reported all addresses as occupied. This small series fixes it. Paolo Bonzini (3): smbus: allow returning an error from reads smbus: return -1 if nothing found at the given address pm_smbus: correctly report unclaimed cycles hw/i2c/pm_smbus.c | 63 ++ hw/i2c/smbus.c | 68 ++ include/hw/i2c/smbus.h | 18 ++--- 3 files changed, 97 insertions(+), 52 deletions(-) -- 1.8.5.3
[Qemu-devel] [PATCH 2/3] smbus: return -1 if nothing found at the given address
Fix coding standards in the meanwhile.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i2c/smbus.c | 62 +++---
include/hw/i2c/smbus.h | 12 +-
2 files changed, 50 insertions(+), 24 deletions(-)
diff --git a/hw/i2c/smbus.c b/hw/i2c/smbus.c
index a8931b7..18fb4d1 100644
--- a/hw/i2c/smbus.c
+++ b/hw/i2c/smbus.c
@@ -208,34 +208,44 @@ static int smbus_device_init(I2CSlave *i2c)
}
/* Master device commands. */
-void smbus_quick_command(i2c_bus *bus, uint8_t addr, int read)
+int smbus_quick_command(i2c_bus *bus, uint8_t addr, int read)
{
-i2c_start_transfer(bus, addr, read);
+if (i2c_start_transfer(bus, addr, read)) {
+return -1;
+}
i2c_end_transfer(bus);
+return 0;
}
int smbus_receive_byte(i2c_bus *bus, uint8_t addr)
{
uint8_t data;
-i2c_start_transfer(bus, addr, 1);
+if (i2c_start_transfer(bus, addr, 1)) {
+return -1;
+}
data = i2c_recv(bus);
i2c_nack(bus);
i2c_end_transfer(bus);
return data;
}
-void smbus_send_byte(i2c_bus *bus, uint8_t addr, uint8_t data)
+int smbus_send_byte(i2c_bus *bus, uint8_t addr, uint8_t data)
{
-i2c_start_transfer(bus, addr, 0);
+if (i2c_start_transfer(bus, addr, 0)) {
+return -1;
+}
i2c_send(bus, data);
i2c_end_transfer(bus);
+return 0;
}
int smbus_read_byte(i2c_bus *bus, uint8_t addr, uint8_t command)
{
uint8_t data;
-i2c_start_transfer(bus, addr, 0);
+if (i2c_start_transfer(bus, addr, 0)) {
+return -1;
+}
i2c_send(bus, command);
i2c_start_transfer(bus, addr, 1);
data = i2c_recv(bus);
@@ -244,18 +254,23 @@ int smbus_read_byte(i2c_bus *bus, uint8_t addr, uint8_t
command)
return data;
}
-void smbus_write_byte(i2c_bus *bus, uint8_t addr, uint8_t command, uint8_t
data)
+int smbus_write_byte(i2c_bus *bus, uint8_t addr, uint8_t command, uint8_t data)
{
-i2c_start_transfer(bus, addr, 0);
+if (i2c_start_transfer(bus, addr, 0)) {
+return -1;
+}
i2c_send(bus, command);
i2c_send(bus, data);
i2c_end_transfer(bus);
+return 0;
}
int smbus_read_word(i2c_bus *bus, uint8_t addr, uint8_t command)
{
uint16_t data;
-i2c_start_transfer(bus, addr, 0);
+if (i2c_start_transfer(bus, addr, 0)) {
+return -1;
+}
i2c_send(bus, command);
i2c_start_transfer(bus, addr, 1);
data = i2c_recv(bus);
@@ -265,13 +280,16 @@ int smbus_read_word(i2c_bus *bus, uint8_t addr, uint8_t
command)
return data;
}
-void smbus_write_word(i2c_bus *bus, uint8_t addr, uint8_t command, uint16_t
data)
+int smbus_write_word(i2c_bus *bus, uint8_t addr, uint8_t command, uint16_t
data)
{
-i2c_start_transfer(bus, addr, 0);
+if (i2c_start_transfer(bus, addr, 0)) {
+return -1;
+}
i2c_send(bus, command);
i2c_send(bus, data 0xff);
i2c_send(bus, data 8);
i2c_end_transfer(bus);
+return 0;
}
int smbus_read_block(i2c_bus *bus, uint8_t addr, uint8_t command, uint8_t
*data)
@@ -279,33 +297,41 @@ int smbus_read_block(i2c_bus *bus, uint8_t addr, uint8_t
command, uint8_t *data)
int len;
int i;
-i2c_start_transfer(bus, addr, 0);
+if (i2c_start_transfer(bus, addr, 0)) {
+return -1;
+}
i2c_send(bus, command);
i2c_start_transfer(bus, addr, 1);
len = i2c_recv(bus);
-if (len 32)
+if (len 32) {
len = 0;
-for (i = 0; i len; i++)
+}
+for (i = 0; i len; i++) {
data[i] = i2c_recv(bus);
+}
i2c_nack(bus);
i2c_end_transfer(bus);
return len;
}
-void smbus_write_block(i2c_bus *bus, uint8_t addr, uint8_t command, uint8_t
*data,
- int len)
+int smbus_write_block(i2c_bus *bus, uint8_t addr, uint8_t command, uint8_t
*data,
+ int len)
{
int i;
if (len 32)
len = 32;
-i2c_start_transfer(bus, addr, 0);
+if (i2c_start_transfer(bus, addr, 0)) {
+return -1;
+}
i2c_send(bus, command);
i2c_send(bus, len);
-for (i = 0; i len; i++)
+for (i = 0; i len; i++) {
i2c_send(bus, data[i]);
+}
i2c_end_transfer(bus);
+return 0;
}
static void smbus_device_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/i2c/smbus.h b/include/hw/i2c/smbus.h
index 4293733..358f35c 100644
--- a/include/hw/i2c/smbus.h
+++ b/include/hw/i2c/smbus.h
@@ -66,16 +66,16 @@ struct SMBusDevice {
};
/* Master device commands. */
-void smbus_quick_command(i2c_bus *bus, uint8_t addr, int read);
+int smbus_quick_command(i2c_bus *bus, uint8_t addr, int read);
int smbus_receive_byte(i2c_bus *bus, uint8_t addr);
-void smbus_send_byte(i2c_bus *bus, uint8_t addr, uint8_t data);
+int smbus_send_byte(i2c_bus *bus, uint8_t addr, uint8_t data);
int smbus_read_byte(i2c_bus *bus, uint8_t addr, uint8_t command);
-void smbus_write_byte(i2c_bus *bus, uint8_t addr, uint8_t command,
[Qemu-devel] [PATCH 1/3] smbus: allow returning an error from reads
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i2c/smbus.c | 6 +++---
include/hw/i2c/smbus.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/i2c/smbus.c b/hw/i2c/smbus.c
index 25d2d04..a8931b7 100644
--- a/hw/i2c/smbus.c
+++ b/hw/i2c/smbus.c
@@ -214,7 +214,7 @@ void smbus_quick_command(i2c_bus *bus, uint8_t addr, int
read)
i2c_end_transfer(bus);
}
-uint8_t smbus_receive_byte(i2c_bus *bus, uint8_t addr)
+int smbus_receive_byte(i2c_bus *bus, uint8_t addr)
{
uint8_t data;
@@ -232,7 +232,7 @@ void smbus_send_byte(i2c_bus *bus, uint8_t addr, uint8_t
data)
i2c_end_transfer(bus);
}
-uint8_t smbus_read_byte(i2c_bus *bus, uint8_t addr, uint8_t command)
+int smbus_read_byte(i2c_bus *bus, uint8_t addr, uint8_t command)
{
uint8_t data;
i2c_start_transfer(bus, addr, 0);
@@ -252,7 +252,7 @@ void smbus_write_byte(i2c_bus *bus, uint8_t addr, uint8_t
command, uint8_t data)
i2c_end_transfer(bus);
}
-uint16_t smbus_read_word(i2c_bus *bus, uint8_t addr, uint8_t command)
+int smbus_read_word(i2c_bus *bus, uint8_t addr, uint8_t command)
{
uint16_t data;
i2c_start_transfer(bus, addr, 0);
diff --git a/include/hw/i2c/smbus.h b/include/hw/i2c/smbus.h
index d764d75..4293733 100644
--- a/include/hw/i2c/smbus.h
+++ b/include/hw/i2c/smbus.h
@@ -67,11 +67,11 @@ struct SMBusDevice {
/* Master device commands. */
void smbus_quick_command(i2c_bus *bus, uint8_t addr, int read);
-uint8_t smbus_receive_byte(i2c_bus *bus, uint8_t addr);
+int smbus_receive_byte(i2c_bus *bus, uint8_t addr);
void smbus_send_byte(i2c_bus *bus, uint8_t addr, uint8_t data);
-uint8_t smbus_read_byte(i2c_bus *bus, uint8_t addr, uint8_t command);
+int smbus_read_byte(i2c_bus *bus, uint8_t addr, uint8_t command);
void smbus_write_byte(i2c_bus *bus, uint8_t addr, uint8_t command, uint8_t
data);
-uint16_t smbus_read_word(i2c_bus *bus, uint8_t addr, uint8_t command);
+int smbus_read_word(i2c_bus *bus, uint8_t addr, uint8_t command);
void smbus_write_word(i2c_bus *bus, uint8_t addr, uint8_t command, uint16_t
data);
int smbus_read_block(i2c_bus *bus, uint8_t addr, uint8_t command, uint8_t
*data);
void smbus_write_block(i2c_bus *bus, uint8_t addr, uint8_t command, uint8_t
*data,
--
1.8.5.3
[Qemu-devel] [PATCH 3/3] pm_smbus: correctly report unclaimed cycles
Without this patch, i2cdetect will report all addresses as present.
With it, only 0x50..0x57 are present.
Before:
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30: 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50: 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60: 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f
70: 70 71 72 73 74 75 76 77
After:
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: 50 51 52 53 54 55 56 57 -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- --
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i2c/pm_smbus.c | 63 ---
1 file changed, 41 insertions(+), 22 deletions(-)
diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index c98e447..9dccd5f 100644
--- a/hw/i2c/pm_smbus.c
+++ b/hw/i2c/pm_smbus.c
@@ -60,59 +60,78 @@ static void smb_transaction(PMSMBus *s)
uint8_t cmd = s-smb_cmd;
uint8_t addr = s-smb_addr 1;
i2c_bus *bus = s-smbus;
+int ret;
SMBUS_DPRINTF(SMBus trans addr=0x%02x prot=0x%02x\n, addr, prot);
/* Transaction isn't exec if STS_DEV_ERR bit set */
if ((s-smb_stat STS_DEV_ERR) != 0) {
-goto error;
-}
+goto error;
+}
switch(prot) {
case 0x0:
-smbus_quick_command(bus, addr, read);
-s-smb_stat |= STS_BYTE_DONE | STS_INTR;
-break;
+ret = smbus_quick_command(bus, addr, read);
+goto done;
case 0x1:
if (read) {
-s-smb_data0 = smbus_receive_byte(bus, addr);
+ret = smbus_receive_byte(bus, addr);
+goto data0;
} else {
-smbus_send_byte(bus, addr, cmd);
+ret = smbus_send_byte(bus, addr, cmd);
+goto done;
}
-s-smb_stat |= STS_BYTE_DONE | STS_INTR;
-break;
case 0x2:
if (read) {
-s-smb_data0 = smbus_read_byte(bus, addr, cmd);
+ret = smbus_read_byte(bus, addr, cmd);
+goto data0;
} else {
-smbus_write_byte(bus, addr, cmd, s-smb_data0);
+ret = smbus_write_byte(bus, addr, cmd, s-smb_data0);
+goto done;
}
-s-smb_stat |= STS_BYTE_DONE | STS_INTR;
break;
case 0x3:
if (read) {
-uint16_t val;
-val = smbus_read_word(bus, addr, cmd);
-s-smb_data0 = val;
-s-smb_data1 = val 8;
+ret = smbus_read_word(bus, addr, cmd);
+goto data01;
} else {
-smbus_write_word(bus, addr, cmd, (s-smb_data1 8) |
s-smb_data0);
+ret = smbus_write_word(bus, addr, cmd, (s-smb_data1 8) |
s-smb_data0);
+goto done;
}
-s-smb_stat |= STS_BYTE_DONE | STS_INTR;
break;
case 0x5:
if (read) {
-s-smb_data0 = smbus_read_block(bus, addr, cmd, s-smb_data);
+ret = smbus_read_block(bus, addr, cmd, s-smb_data);
+goto data0;
} else {
-smbus_write_block(bus, addr, cmd, s-smb_data, s-smb_data0);
+ret = smbus_write_block(bus, addr, cmd, s-smb_data, s-smb_data0);
+goto done;
}
-s-smb_stat |= STS_BYTE_DONE | STS_INTR;
break;
default:
goto error;
}
+abort();
+
+data01:
+if (ret 0) {
+goto error;
+}
+s-smb_data1 = ret 8;
+data0:
+if (ret 0) {
+goto error;
+}
+s-smb_data0 = ret;
+done:
+if (ret 0) {
+goto error;
+}
+s-smb_stat |= STS_BYTE_DONE | STS_INTR;
return;
- error:
+error:
s-smb_stat |= STS_DEV_ERR;
+return;
+
}
static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
--
1.8.5.3
Re: [Qemu-devel] [PATCH v2] qdev: add the device to the QOM tree before using it to set a link
Am 17.02.2014 11:23, schrieb Amos Kong:
On Thu, Jan 02, 2014 at 09:02:11AM +0800, Amos Kong wrote:
Test steps:
(qemu) device_add e1000,addr=adsf
Property 'e1000.addr' doesn't take value 'adsf'
(qemu) info qtree
Then qemu crashed.
Currently we set a link to the new device for qdev parent bus, but the
device hasn't been added to QOM tree. When it fails to set properties,
object_unparent() can't cleanup the device.
This patch moves qdev_set_parent_bus() back to object_property_add_child(),
we only needs to unref the object if setting properties fails.
Signed-off-by: Amos Kong ak...@redhat.com
---
V2: fix bz by adjust the initialization order (Paolo)
Hi Anthony, other maintainer
The V2 already reviewed and tested by Markus. Can you help to review apply
it?
Amos, I had pointed out to Paolo (IRC?) that this differs from how all
legacy devices are being created, so I consider it a bad idea.
qdev_set_parent_bus() is called from qdev_try_create(), which is called
by qdev_create(). Devices may thus assume that the bus is set early,
e.g. in their property setters invoked by qemu_opt_foreach(), and some
functions have special behavior for a NULL bus (thinking of ISA here),
so the change may lead to silent functional changes.
Long-term we will have to move the code adding the device out of realize
because we want to make realize work recursively on the composition
tree. So what about rather moving the code adding the device to
periph-anon / periph between dev-id and qemu_opt_foreach() so that the
original unparenting works as expected?
Regards,
Andreas
Thanks, Amos
---
qdev-monitor.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index dc37a43..4070b0a 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -518,16 +518,11 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* create device, set properties */
dev = DEVICE(object_new(driver));
-if (bus) {
-qdev_set_parent_bus(dev, bus);
-}
-
id = qemu_opts_id(opts);
if (id) {
dev-id = id;
}
if (qemu_opt_foreach(opts, set_property, dev, 1) != 0) {
-object_unparent(OBJECT(dev));
object_unref(OBJECT(dev));
return NULL;
}
@@ -541,6 +536,11 @@ DeviceState *qdev_device_add(QemuOpts *opts)
OBJECT(dev), NULL);
g_free(name);
}
+
+if (bus) {
+qdev_set_parent_bus(dev, bus);
+}
+
object_property_set_bool(OBJECT(dev), true, realized, err);
if (err != NULL) {
qerror_report_err(err);
--
1.8.4.2
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC] char: fix avail_connections init in qemu_chr_open_eventfd()
Hello all, On Fri, Feb 14, 2014 at 1:16 PM, David Marchand david.march...@6wind.comwrote: In HEAD, ivshmem seems to be the only place where qemu_chr_open_eventfd() is used : $ git grep qemu_chr_open_eventfd hw/misc/ivshmem.c:chr = qemu_chr_open_eventfd(eventfd); include/sysemu/char.h:CharDriverState *qemu_chr_open_eventfd(int eventfd); qemu-char.c:CharDriverState *qemu_chr_open_eventfd(int eventfd) I suppose my change is not that impacting :-) Since this change has no impact, can it be pulled in current git tree ? Thanks. -- David Marchand
[Qemu-devel] [PATCH] qapi: output visitor crashes qemu if it encounters a NULL value
A NULL value is not added to visitor's stack, but there
is no check for that when the visitor tries to return
that value, leading to Qemu crash.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
qapi/qmp-output-visitor.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 74a5684..0562f49 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -66,6 +66,11 @@ static QObject *qmp_output_pop(QmpOutputVisitor *qov)
static QObject *qmp_output_first(QmpOutputVisitor *qov)
{
QStackEntry *e = QTAILQ_LAST(qov-stack, QStack);
+
+if (!e) {
+return NULL;
+}
+
return e-value;
}
--
1.8.3.1
Re: [Qemu-devel] [PULL 0/3] Update seabios to 1.7.4
On 8 February 2014 13:12, Peter Maydell peter.mayd...@linaro.org wrote: On 3 February 2014 14:45, Gerd Hoffmann kra...@redhat.com wrote: Hi, Sorry for the delay folks, totally forgot that one. Here is the seabios update to 1.7.4 final. please pull, Gerd The following changes since commit 2f61120c10da9128357510debc8e66880cd2bfdc: Merge remote-tracking branch 'qmp-unstable/queue/qmp' into staging (2014-02-01 23:32:31 +) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-roms-1 Applied, thanks. It looks like this pull request updates our seabios module to a revision that isn't actually present in git://git.qemu-project.org/seabios.git/. Gerd, do you have a fix for that or should I just revert this? thanks -- PMM
Re: [Qemu-devel] [PATCH] tmp105: read temperature in milli-celsius
Am 17.02.2014 11:57, schrieb Paolo Bonzini:
Right now, the temperature property must be written in milli-celsius, but it
reads back the value in 8.8 fixed point. Fix this by letting the property
read back the original value (possibly rounded). Also simplify the code that
does the conversion.
Before:
(QEMU) qom-set path=/machine/peripheral/sensor property=temperature
value=2
{u'return': {}}
(QEMU) qom-get path=sensor property=temperature
{u'return': 5120}
After:
(QEMU) qom-set path=/machine/peripheral/sensor property=temperature
value=2
{u'return': {}}
(QEMU) qom-get path=sensor property=temperature
{u'return': 2}
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/misc/tmp105.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c
index 155e03d..63aa3d6 100644
--- a/hw/misc/tmp105.c
+++ b/hw/misc/tmp105.c
@@ -56,12 +56,14 @@ static void tmp105_get_temperature(Object *obj, Visitor
*v, void *opaque,
const char *name, Error **errp)
{
TMP105State *s = TMP105(obj);
-int64_t value = s-temperature;
+int64_t value = s-temperature * 1000 / 256;
Hmm, I'll have to check history, but I guess the setter was there and I
wrongly added the getter. That would be easier to ack of course if I
didn't have to think about a complete new formula in both places... ;)
visit_type_int(v, value, name, errp);
}
-/* Units are 0.001 centigrades relative to 0 C. */
+/* Units are 0.001 centigrades relative to 0 C. s-temperature is 8.8
+ * fixed point, so units are 1/256 centigrades. A simple ratio will do.
+ */
static void tmp105_set_temperature(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
@@ -78,7 +80,7 @@ static void tmp105_set_temperature(Object *obj, Visitor *v,
void *opaque,
return;
}
-s-temperature = ((int16_t) (temp * 0x800 / 128000)) 4;
+s-temperature = (int16_t) (temp * 256 / 1000);
Did you check whether those magic 4 bits shift were for some other
purpose such as flags possibly? CC'ing Alex Horn.
Since we do have a tmp105-test, we should also add a regression test for
the getter bug.
Regards,
Andreas
tmp105_alarm_update(s);
}
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] Delegating tasks for the GitHub mirror
Hi Anthony, I would like to get the Travis tests up and running on the official QEMU GitHub mirror to try and shorten the loop between the build breaking and people being notified. I'm quite happy to set this up if you add me to the QEMU organisation (my GitHub username is: stsquad). There are also hooks I can set-up into IRC so the #qemu channel can be notified when the build breaks. We can also experiment with auto-testing pull requests but I think the first step is to ensure master is fixed as soon as possible when the build breaks. Cheers, -- Alex Bennée QEMU/KVM Hacker for Linaro
Re: [Qemu-devel] [PATCH 1/2] usb-hid: Add high speed mouse configuration
On So, 2014-02-16 at 00:41 -0500, Jan Vesely wrote: Signed-off-by: Jan Vesely jano.ves...@gmail.com This should have a usb_version property, like usb-tablet has. cheers, Gerd
Re: [Qemu-devel] [PATCH] tmp105: read temperature in milli-celsius
@@ -78,7 +80,7 @@ static void tmp105_set_temperature(Object *obj, Visitor *v, void *opaque, return; } -s-temperature = ((int16_t) (temp * 0x800 / 128000)) 4; +s-temperature = (int16_t) (temp * 256 / 1000); Did you check whether those magic 4 bits shift were for some other purpose such as flags possibly? CC'ing Alex Horn. From the data sheet, the temperature register is 9-12 bits, depending on the configuration. So the low 4 bits will always read back as zero. However, they are already masked away when reading the temperature in tmp105_read: s-buf[s-len ++] = (((uint16_t) s-temperature) 8); s-buf[s-len ++] = (((uint16_t) s-temperature) 0) (0xf0 ((~s-config 5) 3)); /* R */ The equivalence of the formula can be proved as follows: ((int16_t) (temp * 0x800 / 128000)) 4 = (int16_t) (temp * 0x8000 / 128000) ~15 = (int16_t) (temp * (0x100 * 0x80) / (1000 * 0x80)) ~15 = (int16_t) (temp * 0x100 / 1000) ~15 and the AND can be removed as mentioned above. Since we do have a tmp105-test, we should also add a regression test for the getter bug. Yeah, if only tmp105-test already tested the setter. :) Paolo
Re: [Qemu-devel] Delegating tasks for the GitHub mirror
On 17 February 2014 12:09, Alex Bennée alex.ben...@linaro.org wrote: I would like to get the Travis tests up and running on the official QEMU GitHub mirror to try and shorten the loop between the build breaking and people being notified. I'm quite happy to set this up if you add me to the QEMU organisation (my GitHub username is: stsquad). As a side-note, I think it would be useful if we documented somewhere who has relevant admin rights for various bits of QEMU infrastructure: * direct commit access * QEMU wiki admin/bureaucrat rights * github mirror admin * sysadmin contacts for whatever host is running qemu-project.org * mailing list admin * other semi-official things if relevant (eg we have a project set up on the coverity scan website) Does anybody think there's a good reason for this info not to simply be public on the wiki? thanks -- PMM
Re: [Qemu-devel] Fwd: Trying to write a new device / virtio-i2c ?
2014-02-17 11:38 GMT+01:00 Paolo Bonzini pbonz...@redhat.com:
Il 17/02/2014 11:01, Alex David ha scritto:
I indeed don't use paravirtualization.
Virtio _is_ paravirtualization. :)
Ok, now that seems much more understandable... I missed that point ha.
I'm emulating a bunch of sensors/actuators.
If I virtualize my sensors and attach them to the i2c-dev with -device,
how do I get those data on the host then ?
It depends on your use case.
It could be that you can make them return a constant value.
Otherwise, you may want to use a chardev for that purpose, or finally a
QOM (QEMU Object Model) property. For example, add
CONFIG_TMP105=y
to default-configs/x86_64-softmmu.mak before building QEMU, then do the
following (indented = in the guest):
$ x86_64-softmmu/qemu-system-x86_64 --enable-kvm ~/test2.img -m 256 \
-device tmp105,id=sensor,address=0x50 \
-qmp unix:$HOME/qmp.sock,server,nowait
$ qmp/qom-list -s ~/qmp.sock /machine/peripheral/sensor
temperature
@parent_bus/
address
hotpluggable
realized
type
$ scripts/qmp/qmp-shell ~/qmp.sock
(QEMU) qom-get path=/machine/peripheral/sensor property=temperature
{u'return': 0}
(QEMU) qom-get path=sensor property=address
{u'return': 80}
# modprobe i2c-dev
# i2cget -y 0 0x50 0 w
0x
(QEMU) qom-set path=sensor property=temperature value=2
{u'return': {}}
# i2cget -y 0 0x50 0 w
0x0014
For this particular sensor, you have to swap the two bytes and the result
is 8.8 fixed-point.
Paolo
Thanks for your help. As you may see, I'm not that experienced in
QEMU/Linux kernel.
Ok, I'm gonna try these things. I might try to use a chardev also. Thanks
for your help !
Re: [Qemu-devel] [PATCHv2 2/2] sun4m: Add Sun CG3 framebuffer initialisation function
On 09/02/14 15:32, Andreas Färber wrote:
IIUC SUNW,cgthree is an optional device, so it's not covered by my
qom-test. Please follow-up with a tests/cg3-test.c so that it gets
covered. Compare my recent PCI NIC series for how such a stub could look
like, in particular vmxnet3-test.c since this will be sparc-only.
Okay - should this be a totally separate patch or appended to the end of
this patchset?
---
hw/sparc/sun4m.c| 60 +--
include/sysemu/sysemu.h |1 +
vl.c| 24 +++
3 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 94f7950..4c6c450 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -561,6 +561,31 @@ static void tcx_init(hwaddr addr, int vram_size, int width,
}
}
+static void cg3_init(hwaddr addr, qemu_irq irq, int vram_size, int width,
+int height, int depth)
Indentation?
Fixed.
+{
+DeviceState *dev;
+SysBusDevice *s;
+
+dev = qdev_create(NULL, SUNW,cgthree);
+qdev_prop_set_uint32(dev, vram_size, vram_size);
+qdev_prop_set_uint16(dev, width, width);
+qdev_prop_set_uint16(dev, height, height);
+qdev_prop_set_uint16(dev, depth, depth);
+qdev_prop_set_uint64(dev, prom_addr, addr);
+qdev_init_nofail(dev);
+s = SYS_BUS_DEVICE(dev);
+
+/* FCode ROM */
+sysbus_mmio_map(s, 0, addr);
+/* DAC */
+sysbus_mmio_map(s, 1, addr + 0x40ULL);
+/* 8-bit plane */
+sysbus_mmio_map(s, 2, addr + 0x80ULL);
+
+sysbus_connect_irq(s, 0, irq);
+}
+
/* NCR89C100/MACIO Internal ID register */
#define TYPE_MACIO_ID_REGISTER macio_idreg
@@ -918,8 +943,39 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
}
num_vsimms = 0;
if (num_vsimms == 0) {
-tcx_init(hwdef-tcx_base, 0x0010, graphic_width, graphic_height,
- graphic_depth);
+if (vga_interface_type == VGA_CG3) {
+if (graphic_depth != 8) {
+fprintf(stderr, qemu: Unsupported depth: %d\n,
graphic_depth);
error_report() please - without qemu: and without trailing \n then.
Fixed.
+exit(1);
+}
+
+if (!(graphic_width == 1024 graphic_height == 768)
+!(graphic_width == 1152 graphic_height == 900)) {
+fprintf(stderr, qemu: Unsupported resolution: %d x %d\n,
+graphic_width, graphic_height);
Dito.
Fixed.
+exit(1);
+}
+
+/* sbus irq 5 */
+cg3_init(hwdef-tcx_base, slavio_irq[11], 0x0010,
+ graphic_width, graphic_height, graphic_depth);
+} else {
+/* If no display specified, default to TCX */
+if (graphic_depth != 8 graphic_depth != 24) {
+fprintf(stderr, qemu: Unsupported depth: %d\n,
+graphic_depth);
Dito.
Fixed.
+exit(1);
+}
+
+if (!(graphic_width == 1024 graphic_height == 768)) {
+fprintf(stderr, qemu: Unsupported resolution: %d x %d\n,
+graphic_width, graphic_height);
Dito.
Fixed.
+exit(1);
+}
These checks are new, right? Would deserve a mention in the commit message.
Yes that's a good point. Sun's OBP boots in the higher resolution so
I've altered the commit message to make this clearer.
+
+tcx_init(hwdef-tcx_base, 0x0010, graphic_width,
graphic_height,
+ graphic_depth);
+}
}
for (i = num_vsimms; i MAX_VSIMMS; i++) {
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 495dae8..b90df9a 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -104,6 +104,7 @@ extern int autostart;
typedef enum {
VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL,
+VGA_TCX, VGA_CG3,
} VGAInterfaceType;
extern int vga_interface_type;
diff --git a/vl.c b/vl.c
index 383be1b..61c8212 100644
--- a/vl.c
+++ b/vl.c
@@ -2084,6 +2084,16 @@ static bool qxl_vga_available(void)
return object_class_by_name(qxl-vga);
}
+static bool tcx_vga_available(void)
+{
+return object_class_by_name(SUNW,tcx);
+}
+
+static bool cg3_vga_available(void)
+{
+return object_class_by_name(SUNW,cgthree);
+}
+
static void select_vgahw (const char *p)
{
const char *opts;
@@ -2119,6 +2129,20 @@ static void select_vgahw (const char *p)
fprintf(stderr, Error: QXL VGA not available\n);
exit(0);
}
+} else if (strstart(p, tcx,opts)) {
+if (tcx_vga_available()) {
+vga_interface_type = VGA_TCX;
+} else {
+fprintf(stderr, Error: TCX framebuffer not available\n);
+exit(0);
I note that these two and the below two are copied from QXL above, but
shouldn't that
Re: [Qemu-devel] [PATCHv2 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM
On 09/02/14 15:33, Peter Maydell wrote: It's been a little while since I looked, however this was my interpretation of the table 3-12 on p.66 of the SBus specification. While that particular table refers to the acknowledgment cycle from the slave back to the master, it seems to work here in the same way when transferring between the master and the slave. It's not that I think your comment is wrong about how SBus does byte transfers, or that the code is wrong, I just don't think that the reason for the code is the SBus behaviour; it's the device behaviour. QEMU abstracts a bus's mechanisms for transferring data into the MemoryRegion ops, which means that for a byte access you'll always get called with the byte value in the low 8 bits of the input argument. This would be true whether SBus passed byte values in lines 31..28, 7..0 or by sending them one bit at a time on D13. The relation between a byte write and a word write here is a property of the device, not the bus. (You'll notice that 3-12 allows the slave to send the byte data in various different lanes depending on which size acknowlegement it chooses to send. The documentation of what the master does is on page 53 and actually says you have to send it in two lanes for a non-aligned address, and certainly QEMU won't do anything corresponding to that. In any case I don't think it's relevant here.) (goes and thinks about this for a bit) Okay I can agree with that it's more an artifact of the implementation, rather than dependent upon the behaviour of the bus. I'll alter the comment for the next revision which I hope to post to the list over the next day or so. ATB, Mark.
Re: [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open()
Am 15.02.2014 um 04:30 hat Max Reitz geschrieben: bdrv_file_open() is now nearly a subset of bdrv_open(), except for the fact that bdrv_file_open() is for protocols and bdrv_open() for block drivers. It is possible to use bdrv_file_open() with a block driver, but in that case that block driver must be explicitly specified. Due to these great similarities, bdrv_file_open() can be integrated and made a special case of bdrv_open(). If the flag BDRV_O_PROTOCOL is specified, bdrv_open() will now do what bdrv_file_open() used to do: Auto-detecting a protocol instead of a block driver. This series implements this and changes all calls to bdrv_file_open() to bdrv_open() calls with BDRV_O_PROTOCOL specified. Note that this flag cannot be discerned automatically since it is impossible for bdrv_open() to know by itself whether a given file should be opened with or without the format layer involved: Both are valid alternatives. Therefore, it still has to be specified by the user. This series conflicts with Benoît's patches that have been merged into master. When rebasing, please be careful with the code motion patch so that you don't accidentally revert Benoît's changes. (It's an easy conflict to resolve, but not trivial enough for me to do it while applying the patch, with no additional review.) Kevin
Re: [Qemu-devel] [PATCHv2 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM
On 10/02/14 08:20, Paolo Bonzini wrote: Il 09/02/2014 16:24, Mark Cave-Ayland ha scritto: Alright I can change those for the next version of the patch. Does that mean the use of hex output is now a display option rather than a separate property type? info qtree will always print both decimal and hex. If I try that here then with the CG3 patch applied the prom_addr property is always just displayed in decimal...? Or have I missed something obvious here? (You can see the same in vanilla QEMU by changing the same property in hw/display/tcx.c from DEFINE_PROP_HEX64 to DEFINE_PROP_UINT64 and then running info qtree from qemu-system-sparc) ATB, Mark.
Re: [Qemu-devel] [PATCHv2 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM
On 14/02/14 14:54, Peter Crosthwaite wrote: The short answer is we don't know because we don't have any documentation. Sigh This has happened quite a lot lately. If the kernel driver has macros, re-use them as much as possible. If you have a vague idea on whats, what, a few well invented names would help the device self-documentation. Okay. I now have a revised version which borrows macro names from the Linux and BSD drivers which I think should be more readable. I'll post the revised version to the list shortly. Your hander switch statements stride in 4, are you only doing this for your one exception case of that one-byte big-endian access I commented earlier. Yes, that is correct. Should you trap misaligned accesses then? Over the weekend I found out that the non-BT458 accesses (addr = 0x10) are done as byte accesses and so byte accesses do need to be allowed to these registers. My interpretation of reading the SBus documentation is that on real hardware the bus converts accesses for you, and so I don't think a trap would be suitable here. Also I've not found an image (yet) that attempts bad accesses in this way across my OpenBIOS ISO test suite... ATB, Mark.
Re: [Qemu-devel] [PATCHv2 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM
info qtree will always print both decimal and hex. If I try that here then with the CG3 patch applied the prom_addr property is always just displayed in decimal...? Or have I missed something obvious here? Grammar sucks. :) It *will* always print both decimal and hex once HEX64 is gone. It doesn't yet; right now, the exact printed format depends on UINT64 vs. HEX64 as you found out. But it's still easier for you to use UINT64 and avoid future conflicts. Paolo
Re: [Qemu-devel] [PATCH v8 01/17] Convert -mem-path to QemuOpts and add prealloc and share properties
On Mon, Feb 17, 2014 at 8:56 AM, Michael S. Tsirkin m...@redhat.com wrote: On Mon, Feb 17, 2014 at 12:42:45AM +0100, Paolo Bonzini wrote: Il 15/02/2014 19:10, Michael Tokarev ha scritto: 13 февраля 2014 г. 16:03:12 GMT+04:00, Antonios Motakis a.mota...@virtualopensystems.com пишет: Extend -mem-path with additional properties: - prealloc=on|off - default off, same as -mem-prealloc - share=on|off - default off, memory is mmapped with MAP_SHARED Maybe we should combine -m and -mem-path options together to form something more sane? It's on the way: it would be something like -object mem-file,size=1024M,path=/path/to/foo,share=on,prealloc=on,id=mem \ -numa node,memdev=mem using the same host/guest split model that is already in use in many other places. Not 2.0 material though. Paolo Hmm in that case, let's not add prealloc as a property here. Stick to existing flag for that, this way we don't need to support 3 ways to do this. We'll remove the prealloc property then for the next version. Otherwise, I wonder if the long term memdev discussion can impose additional constraints on our current implementation. It seems to me that this shouldn't be the case, right? -- Antonios Motakis Virtual Open Systems
Re: [Qemu-devel] Fwd: Trying to write a new device / virtio-i2c ?
I'm emulating a bunch of sensors/actuators.
If I virtualize my sensors and attach them to the i2c-dev with -device,
how do I get those data on the host then ?
It depends on your use case.
It could be that you can make them return a constant value.
Otherwise, you may want to use a chardev for that purpose, or finally a
QOM (QEMU Object Model) property. For example, add
CONFIG_TMP105=y
to default-configs/x86_64-softmmu.mak before building QEMU, then do the
following (indented = in the guest):
$ x86_64-softmmu/qemu-system-x86_64 --enable-kvm ~/test2.img -m 256 \
-device tmp105,id=sensor,address=0x50 \
-qmp unix:$HOME/qmp.sock,server,nowait
$ qmp/qom-list -s ~/qmp.sock /machine/peripheral/sensor
temperature
@parent_bus/
address
hotpluggable
realized
type
$ scripts/qmp/qmp-shell ~/qmp.sock
(QEMU) qom-get path=/machine/peripheral/sensor property=temperature
{u'return': 0}
(QEMU) qom-get path=sensor property=address
{u'return': 80}
# modprobe i2c-dev
# i2cget -y 0 0x50 0 w
0x
(QEMU) qom-set path=sensor property=temperature value=2
{u'return': {}}
# i2cget -y 0 0x50 0 w
0x0014
For this particular sensor, you have to swap the two bytes and the result
is 8.8 fixed-point.
Paolo
Thanks for your help. As you may see, I'm not that experienced in
QEMU/Linux kernel.
Ok, I'm gonna try these things. I might try to use a chardev also. Thanks
for your help !
I've tried using tmp105. As my linux isn't 64bits, i'm using
qemu-system-i386... It crashes my computer when I use it with my linux
image (it's a debian .qcow2..., easy to do some tests...).
I will most probably need a chardev anyways, I will need to read/write data
when I want to. I'm gonna be annoying and ask another basic question : I
wrote this in my i2c_device_test.c (QEMU device) :
static const TypeInfo mydevice_i2c_type_info = {
.name= TYPE_MYDEVICE_I2C,
.parent= TYPE_I2C_SLAVE,
.instance_size= sizeof(MYDEVICEI2CState),
.instance_init= MYDEVICE_i2c_init,
.class_init= mydevice_i2c_class_init,
};
I will be able to add a chardev using the properties, right ?
static void mydevice_i2c_class_init(ObjectClass *klass, void *data)
{
printf(Test init2\n);
I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
sc-init = mydevice_i2c_init;
sc-event = mydevice_i2c_event;
sc-recv = mydevice_i2c_recv;
sc-send = mydevice_i2c_send;
dc-vmsd = mydevice_i2c_vmstate;
dc-props = mydevice_i2c_properties;
dc-realize = mydevice_i2c_realize;
}
Does this seems ok for you ? So far, I understood the props are needed
for when I'm gonna declare the device at QEMU launch.
I am not sure if it's needed (my i2c-0 should be created anyways), but in
that case, how do I plug it on my socket on the host ?
Thanks !
Re: [Qemu-devel] [PATCH v2 09/20] cow: correctly propagate errors
On Sat, 02/15 11:01, Markus Armbruster wrote:
Jeff Cody jc...@redhat.com writes:
On Fri, Feb 14, 2014 at 05:45:40PM +0100, Kevin Wolf wrote:
Am 11.02.2014 um 18:03 hat Paolo Bonzini geschrieben:
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/cow.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index 7fc0b12..43a2150 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -82,7 +82,7 @@ static int cow_open(BlockDriverState *bs, QDict
*options, int flags,
char version[64];
snprintf(version, sizeof(version),
COW version %d, cow_header.version);
-qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
bs-device_name, cow, version);
ret = -ENOTSUP;
goto fail;
@@ -330,7 +330,6 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options,
struct stat st;
int64_t image_sectors = 0;
const char *image_filename = NULL;
-Error *local_err = NULL;
int ret;
BlockDriverState *cow_bs;
@@ -344,18 +343,13 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options,
options++;
}
-ret = bdrv_create_file(filename, options, local_err);
+ret = bdrv_create_file(filename, options, errp);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
return ret;
}
-ret = bdrv_file_open(cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
- local_err);
+ret = bdrv_file_open(cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
errp);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
return ret;
}
This is technically correct, but I think general policy is that using
the local_err pattern is preferred anyway.
If I recall correct, I think there are several places that pass errp
along. How about this for a rule of thumb policy: use the local_err
method if the function does not indicate error outside of the passed
Error pointer.
Use local_err when you need to examine the error object. Passing errp
directly is no good then, because it may be null.
When you're forwarding errors without examining them, then passing errp
directly is just fine.
Does this mean that error_is_set() is always used by programmer to check a
non-NULL error pointer? Is there any case to call error_is_set(errp) without
knowing if errp is NULL or not? If no, should we enforce the rule and add
assert(errp) in error_is_set()?
Thanks,
Fam
Re: [Qemu-devel] [PATCH v20 00/11] Shared library module support
Ping?
Thanks,
Fam
On Mon, 02/10 14:48, Fam Zheng wrote:
Rewrote the executable directory patch and added Darwin API in
qemu_init_exec_dir().
v20:
Dropped the argv0 passing patch from v19. Refactored qemu_get_exec_dir()
in
patch 01. Three patches are affected:
[01/11] util: Split out exec_dir from os_find_datadir
Rewritten. The exec_dir is a static global in os-{win32,posix}.c
that is initialized in main() and later used with
qemu_get_exec_dir().
[07/11] module: implement module loading
Use glue(). Prefix hash with _, so the preceding digit won't
cause compile error now.
[11/11] oslib: port qemu_init_exec_dir to Darwin
New.
Fam Zheng (10):
util: Split out exec_dir from os_find_datadir
rules.mak: fix $(obj) to a real relative path
rules.mak: allow per object cflags and libs
block: use per-object cflags and libs
rules.mak: introduce DSO rules
module: implement module loading
Makefile: install modules with make install
Makefile: introduce common-obj-m and block-obj-m for DSO
block: convert block drivers linked with libs to modules
oslib: port qemu_init_exec_dir to Darwin
Paolo Bonzini (1):
darwin: do not use -mdynamic-no-pic
.gitignore| 3 ++
Makefile | 29 +-
Makefile.objs | 19 ++-
Makefile.target | 21 ++--
block/Makefile.objs | 13 -
configure | 93 +---
include/qemu-common.h | 2 +-
include/qemu/module.h | 23 +++-
include/qemu/osdep.h | 9
module-common.c | 10
os-posix.c| 42 +++
os-win32.c| 21 +---
qemu-img.c| 1 +
qemu-io.c | 1 +
qemu-nbd.c| 1 +
rules.mak | 80 +++-
scripts/create_config | 3 ++
util/module.c | 145
+-
util/oslib-posix.c| 69
util/oslib-win32.c| 30 +++
vl.c | 3 +-
21 files changed, 494 insertions(+), 124 deletions(-)
create mode 100644 module-common.c
--
1.8.5.4
Re: [Qemu-devel] [PULL 0/3] Update seabios to 1.7.4
On Mo, 2014-02-17 at 11:56 +, Peter Maydell wrote: On 8 February 2014 13:12, Peter Maydell peter.mayd...@linaro.org wrote: On 3 February 2014 14:45, Gerd Hoffmann kra...@redhat.com wrote: Hi, Sorry for the delay folks, totally forgot that one. Here is the seabios update to 1.7.4 final. please pull, Gerd The following changes since commit 2f61120c10da9128357510debc8e66880cd2bfdc: Merge remote-tracking branch 'qmp-unstable/queue/qmp' into staging (2014-02-01 23:32:31 +) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-roms-1 Applied, thanks. It looks like this pull request updates our seabios module to a revision that isn't actually present in git://git.qemu-project.org/seabios.git/. Gerd, do you have a fix for that or should I just revert this? The seabios mirroring doesn't seem to be automatic (or the automatic mirroring is broken). Usually Anthony updates the seabios.git tree manually. Upstream git repo is git://git.seabios.org/seabios.git If you have write access to the seabios repo you can just pull latest master from seabios.org and push it to qemu-project.org. If not bug anthony about it. Or we can update the git submodule url to point the upstream repo directly. Not fully sure this is a good idea from the gpl point of view (we ship blobs, so we should ship the source for them too, even if all we provide is just a mirror of upstream ...). cheers, Gerd
Re: [Qemu-devel] Fwd: Trying to write a new device / virtio-i2c ?
Il 17/02/2014 14:11, Alex David ha scritto:
I've tried using tmp105. As my linux isn't 64bits, i'm using
qemu-system-i386... It crashes my computer when I use it with my linux
image (it's a debian .qcow2..., easy to do some tests...).
You mean crashes your host?
I will most probably need a chardev anyways, I will need to read/write
data when I want to.
Depends on how much data. If it's just one or two ints, like tmp105,
QOM would work too. qmp-shell talks to QEMU via a simple JSON protocol,
and it has simple Python bindings too.
static const TypeInfo mydevice_i2c_type_info = {
.name= TYPE_MYDEVICE_I2C,
.parent= TYPE_I2C_SLAVE,
.instance_size= sizeof(MYDEVICEI2CState),
.instance_init= MYDEVICE_i2c_init,
.class_init= mydevice_i2c_class_init,
};
I will be able to add a chardev using the properties, right ?
Yes, using dc-props.
Does this seems ok for you ? So far, I understood the props are needed
for when I'm gonna declare the device at QEMU launch.
I am not sure if it's needed (my i2c-0 should be created anyways), but
in that case, how do I plug it on my socket on the host ?
Your device is not i2c-0. i2c-0 is provided by the PC board. Your
device will get one address, e.g. 0x42, on the i2c bus. You'll be able
to access it with i2cget and i2cset using 0 as the bus address (for
i2c-0) and 0x42 as the device address on bus 0.
So you'll indeed have to do something like -chardev socket,id=foo
-device myi2c,address=0x42,chr=foo if you go for sockets, or just
-device myi2c,address=0x42 if you go for QOM. The chr property
should go into dc-props, while address is provided by the abstract
class TYPE_I2C_SLAVE.
Paolo
Re: [Qemu-devel] [PATCH v2 09/20] cow: correctly propagate errors
Il 17/02/2014 14:15, Fam Zheng ha scritto: Does this mean that error_is_set() is always used by programmer to check a non-NULL error pointer? Is there any case to call error_is_set(errp) without knowing if errp is NULL or not? If no, should we enforce the rule and add assert(errp) in error_is_set()? I think we shouldn't need error_is_set() at all... Paolo
[Qemu-devel] [PATCH 0/4] target-ppc: htab fixes
Hi, This is a new tentative for the patches 2/5 to 5/5 from the target-ppc: Add support for dumping guest memory using qemu gdb server patchset: https://lists.nongnu.org/archive/html/qemu-ppc/2014-01/msg00380.html All patches have been rebased on the current ppc-next head (72c798d7dccc). To ensure proper bisectability, the following was verified for each individual patch: •- 32 and 64 bit build of ppc-softmmu and ppc64-softmmu (fedora 19 ppc64) •- 64 bit pseries guest with KVM on a POWER7 host (fedora 19 ppc64) •- 64 bit pseries guest with 64 bit TCG on a x86_64 host (fedora 19 ppc64) •- 64 bit pseries guest with 32 bit TCG on a x86_64 host (fedora 19 ppc64) •- 32 bit mac99 guest with 64 bit TCG on a x86_64 host (wheezy ppc) •- 32 bit mac99 guest with 32 bit TCG on a x86_64 host (wheezy ppc) Alex, This should address all the requirements you expressed in your last mail. Please tell me if something is missing. Best regards. --- Aneesh Kumar K.V (4): target-ppc: Fix htab_mask calculation target-ppc: Fix page table lookup with kvm enabled target-ppc: Change the hpte store API target-ppc: Update ppc_hash64_store_hpte to support updating in-kernel htab hw/ppc/spapr.c |9 hw/ppc/spapr_hcall.c | 81 ++--- target-ppc/cpu.h |1 target-ppc/kvm.c | 93 ++ target-ppc/kvm_ppc.h | 29 + target-ppc/machine.c | 11 +++-- target-ppc/misc_helper.c |4 +- target-ppc/mmu-hash64.c | 101 +++--- target-ppc/mmu-hash64.h | 47 - target-ppc/mmu_helper.c |3 + 10 files changed, 293 insertions(+), 86 deletions(-) -- Greg
[Qemu-devel] [PATCH 1/4] target-ppc: Fix htab_mask calculation
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Correctly update the htab_mask using the return value of
KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
on GET_SREGS for HV. We check for external htab and if
found true, we don't need to update sdr1
[ fixed pte group offset computation in ppc_hash64_htab_lookup() that
caused TCG to fail, Greg Kurz gk...@linux.vnet.ibm.com ]
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Alexander Graf ag...@suse.de
---
hw/ppc/spapr.c |8 +++-
hw/ppc/spapr_hcall.c | 19 +++
target-ppc/cpu.h |1 +
target-ppc/kvm.c |4 +++-
target-ppc/machine.c | 11 +++
target-ppc/misc_helper.c |4 +++-
target-ppc/mmu-hash64.c |4 ++--
target-ppc/mmu_helper.c |3 ++-
8 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0989ed6..8ac4d8a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -749,7 +749,13 @@ static void spapr_cpu_reset(void *opaque)
env-external_htab = (void *)1;
}
env-htab_base = -1;
-env-htab_mask = HTAB_SIZE(spapr) - 1;
+/*
+ * htab_mask is the mask used to normalize hash value to PTEG index.
+ * htab_shift is log2 of hash table size.
+ * We have 8 hpte per group, and each hpte is 16 bytes.
+ * ie have 128 bytes per hpte entry.
+ */
+env-htab_mask = (1ULL ((spapr)-htab_shift - 7)) - 1;
env-spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr-htab |
(spapr-htab_shift - 18);
}
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 3ffcc65..d19e3fc 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -40,6 +40,17 @@ static target_ulong compute_tlbie_rb(target_ulong v,
target_ulong r,
return rb;
}
+static inline bool valid_pte_index(CPUPPCState *env, target_ulong pte_index)
+{
+/*
+ * hash value/pteg group index is normalized by htab_mask
+ */
+if (((pte_index ~7ULL) / HPTES_PER_GROUP) ~env-htab_mask) {
+return false;
+}
+return true;
+}
+
static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong opcode, target_ulong *args)
{
@@ -91,7 +102,7 @@ static target_ulong h_enter(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
pteh = ~0x60ULL;
-if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+if (!valid_pte_index(env, pte_index)) {
return H_PARAMETER;
}
if (likely((flags H_EXACT) == 0)) {
@@ -136,7 +147,7 @@ static RemoveResult remove_hpte(CPUPPCState *env,
target_ulong ptex,
hwaddr hpte;
target_ulong v, r, rb;
-if ((ptex * HASH_PTE_SIZE_64) ~env-htab_mask) {
+if (!valid_pte_index(env, ptex)) {
return REMOVE_PARM;
}
@@ -262,7 +273,7 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
hwaddr hpte;
target_ulong v, r, rb;
-if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+if (!valid_pte_index(env, pte_index)) {
return H_PARAMETER;
}
@@ -299,7 +310,7 @@ static target_ulong h_read(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
uint8_t *hpte;
int i, ridx, n_entries = 1;
-if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+if (!valid_pte_index(env, pte_index)) {
return H_PARAMETER;
}
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index bb299d7..14783b6 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -961,6 +961,7 @@ struct CPUPPCState {
#endif
/* segment registers */
hwaddr htab_base;
+/* mask used to normalize hash value to PTEG index */
hwaddr htab_mask;
target_ulong sr[32];
/* externally stored hash table */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 33d69d2..969ebdd 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1031,7 +1031,9 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
-ppc_store_sdr1(env, sregs.u.s.sdr1);
+if (!env-external_htab) {
+ppc_store_sdr1(env, sregs.u.s.sdr1);
+}
/* Sync SLB */
#ifdef TARGET_PPC64
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 12c174f..2d46cec 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -70,7 +70,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int
version_id)
qemu_get_betls(f, env-pb[i]);
for (i = 0; i 1024; i++)
qemu_get_betls(f, env-spr[i]);
-ppc_store_sdr1(env, sdr1);
+if (!env-external_htab) {
+ppc_store_sdr1(env, sdr1);
+}
qemu_get_be32s(f, env-vscr);
qemu_get_be64s(f, env-spe_acc);
qemu_get_be32s(f, env-spe_fscr);
@@ -179,9 +181,10 @@ static int cpu_post_load(void *opaque, int version_id)
env-IBAT[1][i+4] = env-spr[SPR_IBAT4U + 2*i + 1];
}
-/* Restore htab_base and htab_mask variables */
-
[Qemu-devel] [PATCH 4/4] target-ppc: Update ppc_hash64_store_hpte to support updating in-kernel htab
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
This support updating htab managed by the hypervisor. Currently we don't have
any user for this feature. This actually bring the store_hpte interface
in-line with the load_hpte one. We may want to use this when we want to
emulate henter hcall in qemu for HV kvm.
[ folded fix for the warn_unused_result build break in
kvmppc_hash64_write_pte(), Greg Kurz gk...@linux.vnet.ibm.com ]
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/kvm.c| 36
target-ppc/kvm_ppc.h| 10 ++
target-ppc/mmu-hash64.c | 20
target-ppc/mmu-hash64.h | 17 ++---
4 files changed, 68 insertions(+), 15 deletions(-)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index e50a50e..84aa4e4 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1991,3 +1991,39 @@ void kvmppc_hash64_free_pteg(uint64_t token)
g_free(htab_buf);
return;
}
+
+void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1)
+{
+int htab_fd;
+struct kvm_get_htab_fd ghf;
+struct kvm_get_htab_buf hpte_buf;
+
+ghf.flags = 0;
+ghf.start_index = 0; /* Ignored */
+htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, ghf);
+if (htab_fd 0) {
+goto error_out;
+}
+
+hpte_buf.header.n_valid = 1;
+hpte_buf.header.n_invalid = 0;
+hpte_buf.header.index = pte_index;
+hpte_buf.hpte[0] = pte0;
+hpte_buf.hpte[1] = pte1;
+/*
+ * Write the hpte entry.
+ * CAUTION: write() has the warn_unused_result attribute. Hence we
+ * need to check the return value, even though we do nothing.
+ */
+if (write(htab_fd, hpte_buf, sizeof(hpte_buf)) 0) {
+goto out_close;
+}
+
+out_close:
+close(htab_fd);
+return;
+
+error_out:
+return;
+}
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 800e1ad..a65d345 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -47,6 +47,9 @@ int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t
index,
uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index);
void kvmppc_hash64_free_pteg(uint64_t token);
+void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1);
+
#else
static inline uint32_t kvmppc_get_tbfreq(void)
@@ -207,6 +210,13 @@ static inline void kvmppc_hash64_free_pteg(uint64_t token)
abort();
}
+static inline void kvmppc_hash64_write_pte(CPUPPCState *env,
+ target_ulong pte_index,
+ target_ulong pte0, target_ulong
pte1)
+{
+abort();
+}
+
#endif
#ifndef CONFIG_KVM
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 8dd5d22..f2af4fb 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -603,3 +603,23 @@ hwaddr ppc_hash64_get_phys_page_debug(CPUPPCState *env,
target_ulong addr)
return ppc_hash64_pte_raddr(slb, pte, addr) TARGET_PAGE_MASK;
}
+
+void ppc_hash64_store_hpte(CPUPPCState *env,
+ target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1)
+{
+CPUState *cs = ENV_GET_CPU(env);
+
+if (kvmppc_kern_htab) {
+return kvmppc_hash64_write_pte(env, pte_index, pte0, pte1);
+}
+
+pte_index *= HASH_PTE_SIZE_64;
+if (env-external_htab) {
+stq_p(env-external_htab + pte_index, pte0);
+stq_p(env-external_htab + pte_index + HASH_PTE_SIZE_64/2, pte1);
+} else {
+stq_phys(cs-as, env-htab_base + pte_index, pte0);
+stq_phys(cs-as, env-htab_base + pte_index + HASH_PTE_SIZE_64/2,
pte1);
+}
+}
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index 49d866b..1746b3e 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -9,6 +9,8 @@ int ppc_store_slb (CPUPPCState *env, target_ulong rb,
target_ulong rs);
hwaddr ppc_hash64_get_phys_page_debug(CPUPPCState *env, target_ulong addr);
int ppc_hash64_handle_mmu_fault(CPUPPCState *env, target_ulong address, int rw,
int mmu_idx);
+void ppc_hash64_store_hpte(CPUPPCState *env, target_ulong index,
+ target_ulong pte0, target_ulong pte1);
#endif
/*
@@ -106,21 +108,6 @@ static inline target_ulong
ppc_hash64_load_hpte1(CPUPPCState *env,
}
}
-static inline void ppc_hash64_store_hpte(CPUPPCState *env,
- target_ulong pte_index,
- target_ulong pte0, target_ulong pte1)
-{
-CPUState *cs = ENV_GET_CPU(env);
-pte_index *= HASH_PTE_SIZE_64;
-if (env-external_htab) {
-
Re: [Qemu-devel] [PATCH v2 09/20] cow: correctly propagate errors
On Mon, Feb 17, 2014 at 02:20:10PM +0100, Paolo Bonzini wrote: Il 17/02/2014 14:15, Fam Zheng ha scritto: Does this mean that error_is_set() is always used by programmer to check a non-NULL error pointer? Is there any case to call error_is_set(errp) without knowing if errp is NULL or not? If no, should we enforce the rule and add assert(errp) in error_is_set()? I think we shouldn't need error_is_set() at all... By this do you mean the caller should dereference errp explicitly to check to see if an error is set, or that there should not be void functions that only indicate error via errp?
[Qemu-devel] [PATCH 2/4] target-ppc: Fix page table lookup with kvm enabled
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
With kvm enabled, we store the hash page table information in the hypervisor.
Use ioctl to read the htab contents. Without this we get the below error when
trying to read the guest address
(gdb) x/10 do_fork
0xc0098660 do_fork: Cannot access memory at address
0xc0098660
(gdb)
[ folded fixes for 32 bit build (casts!), ldq_phys() API change,
Greg Kurz gk...@linux.vnet.ibm.com ]
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Alexander Graf ag...@suse.de
---
hw/ppc/spapr.c |1 +
hw/ppc/spapr_hcall.c| 50 +++---
target-ppc/kvm.c| 53
target-ppc/kvm_ppc.h| 19 +++
target-ppc/mmu-hash64.c | 78 +++
target-ppc/mmu-hash64.h | 22 +
6 files changed, 183 insertions(+), 40 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8ac4d8a..94cf520 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -686,6 +686,7 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
if (shift 0) {
/* Kernel handles htab, we don't need to allocate one */
spapr-htab_shift = shift;
+kvmppc_kern_htab = true;
} else {
if (!spapr-htab) {
/* Allocate an htab if we don't yet have one */
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index d19e3fc..7493302 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -61,8 +61,9 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment
*spapr,
target_ulong ptel = args[3];
target_ulong page_shift = 12;
target_ulong raddr;
-target_ulong i;
+target_ulong index;
hwaddr hpte;
+uint64_t token;
/* only handle 4k and 16M pages for now */
if (pteh HPTE64_V_LARGE) {
@@ -105,30 +106,37 @@ static target_ulong h_enter(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
if (!valid_pte_index(env, pte_index)) {
return H_PARAMETER;
}
+
+index = 0;
+hpte = pte_index * HASH_PTE_SIZE_64;
if (likely((flags H_EXACT) == 0)) {
pte_index = ~7ULL;
-hpte = pte_index * HASH_PTE_SIZE_64;
-for (i = 0; ; ++i) {
-if (i == 8) {
+token = ppc_hash64_start_access(cpu, pte_index);
+do {
+if (index == 8) {
+ppc_hash64_stop_access(token);
return H_PTEG_FULL;
}
-if ((ppc_hash64_load_hpte0(env, hpte) HPTE64_V_VALID) == 0) {
+if ((ppc_hash64_load_hpte0(env, token, index) HPTE64_V_VALID) ==
0) {
break;
}
-hpte += HASH_PTE_SIZE_64;
-}
+} while (index++);
+ppc_hash64_stop_access(token);
} else {
-i = 0;
-hpte = pte_index * HASH_PTE_SIZE_64;
-if (ppc_hash64_load_hpte0(env, hpte) HPTE64_V_VALID) {
+token = ppc_hash64_start_access(cpu, pte_index);
+if (ppc_hash64_load_hpte0(env, token, 0) HPTE64_V_VALID) {
+ppc_hash64_stop_access(token);
return H_PTEG_FULL;
}
+ppc_hash64_stop_access(token);
}
+hpte += index * HASH_PTE_SIZE_64;
+
ppc_hash64_store_hpte1(env, hpte, ptel);
/* eieio(); FIXME: need some sort of barrier for smp? */
ppc_hash64_store_hpte0(env, hpte, pteh | HPTE64_V_HPTE_DIRTY);
-args[0] = pte_index + i;
+args[0] = pte_index + index;
return H_SUCCESS;
}
@@ -145,16 +153,17 @@ static RemoveResult remove_hpte(CPUPPCState *env,
target_ulong ptex,
target_ulong *vp, target_ulong *rp)
{
hwaddr hpte;
+uint64_t token;
target_ulong v, r, rb;
if (!valid_pte_index(env, ptex)) {
return REMOVE_PARM;
}
-hpte = ptex * HASH_PTE_SIZE_64;
-
-v = ppc_hash64_load_hpte0(env, hpte);
-r = ppc_hash64_load_hpte1(env, hpte);
+token = ppc_hash64_start_access(ppc_env_get_cpu(env), ptex);
+v = ppc_hash64_load_hpte0(env, token, 0);
+r = ppc_hash64_load_hpte1(env, token, 0);
+ppc_hash64_stop_access(token);
if ((v HPTE64_V_VALID) == 0 ||
((flags H_AVPN) (v ~0x7fULL) != avpn) ||
@@ -163,6 +172,7 @@ static RemoveResult remove_hpte(CPUPPCState *env,
target_ulong ptex,
}
*vp = v;
*rp = r;
+hpte = ptex * HASH_PTE_SIZE_64;
ppc_hash64_store_hpte0(env, hpte, HPTE64_V_HPTE_DIRTY);
rb = compute_tlbie_rb(v, r, ptex);
ppc_tlb_invalidate_one(env, rb);
@@ -271,16 +281,17 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
target_ulong pte_index = args[1];
target_ulong avpn = args[2];
hwaddr hpte;
+uint64_t token;
target_ulong v, r, rb;
if (!valid_pte_index(env, pte_index)) {
return H_PARAMETER;
}
-hpte = pte_index * HASH_PTE_SIZE_64;
-
-v =
[Qemu-devel] [PATCH 3/4] target-ppc: Change the hpte store API
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
For updating in kernel htab we need to provide both pte0 and pte1, hence update
the interface to take pte0 and pte1 together
[ ldq_phys() API change, Greg Kurz gk...@linux.vnet.ibm.com ]
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Alexander Graf ag...@suse.de
---
hw/ppc/spapr_hcall.c| 20 ++--
target-ppc/mmu-hash64.c |3 ++-
target-ppc/mmu-hash64.h | 24
3 files changed, 16 insertions(+), 31 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 7493302..b0f2529 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -62,7 +62,6 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment
*spapr,
target_ulong page_shift = 12;
target_ulong raddr;
target_ulong index;
-hwaddr hpte;
uint64_t token;
/* only handle 4k and 16M pages for now */
@@ -108,7 +107,6 @@ static target_ulong h_enter(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
}
index = 0;
-hpte = pte_index * HASH_PTE_SIZE_64;
if (likely((flags H_EXACT) == 0)) {
pte_index = ~7ULL;
token = ppc_hash64_start_access(cpu, pte_index);
@@ -130,11 +128,9 @@ static target_ulong h_enter(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
}
ppc_hash64_stop_access(token);
}
-hpte += index * HASH_PTE_SIZE_64;
-ppc_hash64_store_hpte1(env, hpte, ptel);
-/* eieio(); FIXME: need some sort of barrier for smp? */
-ppc_hash64_store_hpte0(env, hpte, pteh | HPTE64_V_HPTE_DIRTY);
+ppc_hash64_store_hpte(env, pte_index + index,
+ pteh | HPTE64_V_HPTE_DIRTY, ptel);
args[0] = pte_index + index;
return H_SUCCESS;
@@ -152,7 +148,6 @@ static RemoveResult remove_hpte(CPUPPCState *env,
target_ulong ptex,
target_ulong flags,
target_ulong *vp, target_ulong *rp)
{
-hwaddr hpte;
uint64_t token;
target_ulong v, r, rb;
@@ -172,8 +167,7 @@ static RemoveResult remove_hpte(CPUPPCState *env,
target_ulong ptex,
}
*vp = v;
*rp = r;
-hpte = ptex * HASH_PTE_SIZE_64;
-ppc_hash64_store_hpte0(env, hpte, HPTE64_V_HPTE_DIRTY);
+ppc_hash64_store_hpte(env, ptex, HPTE64_V_HPTE_DIRTY, 0);
rb = compute_tlbie_rb(v, r, ptex);
ppc_tlb_invalidate_one(env, rb);
return REMOVE_SUCCESS;
@@ -280,7 +274,6 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
target_ulong flags = args[0];
target_ulong pte_index = args[1];
target_ulong avpn = args[2];
-hwaddr hpte;
uint64_t token;
target_ulong v, r, rb;
@@ -304,12 +297,11 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
r |= (flags 48) HPTE64_R_KEY_HI;
r |= flags (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
rb = compute_tlbie_rb(v, r, pte_index);
-hpte = pte_index * HASH_PTE_SIZE_64;
-ppc_hash64_store_hpte0(env, hpte, (v ~HPTE64_V_VALID) |
HPTE64_V_HPTE_DIRTY);
+ppc_hash64_store_hpte(env, pte_index,
+ (v ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
ppc_tlb_invalidate_one(env, rb);
-ppc_hash64_store_hpte1(env, hpte, r);
/* Don't need a memory barrier, due to qemu's global lock */
-ppc_hash64_store_hpte0(env, hpte, v | HPTE64_V_HPTE_DIRTY);
+ppc_hash64_store_hpte(env, pte_index, v | HPTE64_V_HPTE_DIRTY, r);
return H_SUCCESS;
}
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 68a6f69..8dd5d22 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -566,7 +566,8 @@ int ppc_hash64_handle_mmu_fault(CPUPPCState *env,
target_ulong eaddr,
}
if (new_pte1 != pte.pte1) {
-ppc_hash64_store_hpte1(env, pte_offset, new_pte1);
+ppc_hash64_store_hpte(env, pte_offset / HASH_PTE_SIZE_64,
+ pte.pte0, new_pte1);
}
/* 7. Determine the real address from the PTE */
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index e7cb96f..49d866b 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -106,26 +106,18 @@ static inline target_ulong
ppc_hash64_load_hpte1(CPUPPCState *env,
}
}
-static inline void ppc_hash64_store_hpte0(CPUPPCState *env,
- hwaddr pte_offset, target_ulong pte0)
+static inline void ppc_hash64_store_hpte(CPUPPCState *env,
+ target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1)
{
CPUState *cs = ENV_GET_CPU(env);
+pte_index *= HASH_PTE_SIZE_64;
if (env-external_htab) {
-stq_p(env-external_htab + pte_offset, pte0);
+stq_p(env-external_htab + pte_index, pte0);
+stq_p(env-external_htab + pte_index + HASH_PTE_SIZE_64/2,
Re: [Qemu-devel] [PULL 0/3] Update seabios to 1.7.4
On 17 February 2014 13:17, Gerd Hoffmann kra...@redhat.com wrote: On Mo, 2014-02-17 at 11:56 +, Peter Maydell wrote: It looks like this pull request updates our seabios module to a revision that isn't actually present in git://git.qemu-project.org/seabios.git/. Gerd, do you have a fix for that or should I just revert this? The seabios mirroring doesn't seem to be automatic (or the automatic mirroring is broken). Usually Anthony updates the seabios.git tree manually. Upstream git repo is git://git.seabios.org/seabios.git If you have write access to the seabios repo you can just pull latest master from seabios.org and push it to qemu-project.org. If not bug anthony about it. I do have write access, but I'm not completely confident what the right command line rune to do this is. (It looks like our mirror of seabios has all its branches and maybe also tags, so a simple 'push one branch' is probably wrong). Anthony? Or we can update the git submodule url to point the upstream repo directly. Not fully sure this is a good idea from the gpl point of view (we ship blobs, so we should ship the source for them too, even if all we provide is just a mirror of upstream ...). Yep, not pointing straight at the upstream repo is deliberate and for this reason AIUI. thanks -- PMM
Re: [Qemu-devel] [PATCH v13 03/14] block: Replace in_use with operation blocker
On Thu, 02/13 13:34, Benoît Canet wrote:
The Wednesday 29 Jan 2014 à 13:07:30 (+0800), Fam Zheng wrote :
@@ -368,6 +371,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev,
VirtIOBlkConf *blk,
{
VirtIOBlockDataPlane *s;
int fd;
+Error *local_err = NULL;
*dataplane = NULL;
@@ -390,9 +394,10 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev,
VirtIOBlkConf *blk,
/* If dataplane is (re-)enabled while the guest is running there could
be
* block jobs that can conflict.
*/
-if (bdrv_in_use(blk-conf.bs)) {
-error_setg(errp,
- cannot start dataplane thread while device is in use);
+if (bdrv_op_is_blocked(blk-conf.bs, BLOCK_OP_TYPE_DATAPLANE,
local_err)) {
+error_report(cannot start dataplane thread: %s,
+ error_get_pretty(local_err));
+error_free(local_err);
return;
}
@@ -407,9 +412,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev,
VirtIOBlkConf *blk,
s-vdev = vdev;
s-fd = fd;
s-blk = blk;
-
-/* Prevent block operations that conflict with data plane thread */
The comment was not really an extra.
But the code is even more self-explanatory now :)
Fam
-bdrv_set_in_use(blk-conf.bs, 1);
+error_setg(s-blocker, block device is in use by data plane);
+bdrv_op_block_all(blk-conf.bs, s-blocker);
*dataplane = s;
}
Re: [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static
Am 15.02.2014 um 04:30 hat Max Reitz geschrieben: Add the bdrv_open() option BDRV_O_PROTOCOL which results in passing the call to bdrv_file_open(). Additionally, make bdrv_file_open() static and therefore bdrv_open() the only way to call it. Consequently, all existing calls to bdrv_file_open() have to be adjusted to use bdrv_open() with the BDRV_O_PROTOCOL flag instead. Signed-off-by: Max Reitz mre...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH v13 02/14] block: Introduce op_blockers to BlockDriverState
On Thu, 02/13 13:24, Benoît Canet wrote:
The Wednesday 29 Jan 2014 à 13:07:29 (+0800), Fam Zheng wrote :
+bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
+{
+BdrvOpBlocker *blocker;
+assert((int) op = 0 op BLOCK_OP_TYPE_MAX);
+if (!QLIST_EMPTY(bs-op_blockers[op])) {
+blocker = QLIST_FIRST(bs-op_blockers[op]);
+if (errp) {
+*errp = error_copy(blocker-reason);
When an operation is blocked the first reason found is returned as **errp.
I think that this could lead to some randomization of the error messages
depending on the bdrv_op_block call order.
This is not randomization IMO.
I think for a program that could fail with more than one reasons, it fails on
and reports the first failure. This behavior is not uncommon. Being verbose
here does not show much more help, but it's still very easy to add later when
users asks for it.
Fam
Re: [Qemu-devel] [PATCH v13 05/14] block: Add bdrv_set_backing_hd()
On Thu, 02/13 13:49, Benoît Canet wrote:
The Wednesday 29 Jan 2014 à 13:07:32 (+0800), Fam Zheng wrote :
This is the common but non-trivial steps to assign or change the
backing_hd of BDS.
Signed-off-by: Fam Zheng f...@redhat.com
---
block.c | 34 +-
include/block/block.h | 1 +
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index 8000ac0..c4eaa37 100644
--- a/block.c
+++ b/block.c
@@ -1072,6 +1072,26 @@ fail:
return ret;
}
+void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState
*backing_hd)
+{
+if (bs-backing_hd) {
+bdrv_unref(bs-backing_hd);
+}
+
+bs-backing_hd = backing_hd;
+if (!backing_hd) {
+bs-backing_file[0] = '\0';
+bs-backing_format[0] = '\0';
+goto out;
+}
+pstrcpy(bs-backing_file, sizeof(bs-backing_file),
backing_hd-filename);
+pstrcpy(bs-backing_format, sizeof(bs-backing_file),
I don't understand the sizeof(bs-backing_file). Is it
sizeof(bs-backing_format) ?
Yes.
+backing_hd-drv ? backing_hd-drv-format_name : );
+bdrv_ref(bs-backing_hd);
+out:
+bdrv_refresh_limits(bs);
+}
+
/*
* Opens the backing file for a BlockDriverState if not yet open
*
@@ -1085,6 +1105,7 @@ int bdrv_open_backing_file(BlockDriverState *bs,
QDict *options, Error **errp)
char backing_filename[PATH_MAX];
int back_flags, ret;
BlockDriver *back_drv = NULL;
+BlockDriverState *backing_hd;
Error *local_err = NULL;
if (bs-backing_hd != NULL) {
@@ -1108,7 +1129,7 @@ int bdrv_open_backing_file(BlockDriverState *bs,
QDict *options, Error **errp)
sizeof(backing_filename));
}
-bs-backing_hd = bdrv_new();
+backing_hd = bdrv_new();
if (bs-backing_format[0] != '\0') {
back_drv = bdrv_find_format(bs-backing_format);
@@ -1118,23 +1139,18 @@ int bdrv_open_backing_file(BlockDriverState *bs,
QDict *options, Error **errp)
back_flags = bs-open_flags ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT |
BDRV_O_COPY_ON_READ);
-ret = bdrv_open(bs-backing_hd,
+ret = bdrv_open(backing_hd,
*backing_filename ? backing_filename : NULL, options,
back_flags, back_drv, local_err);
if (ret 0) {
-bdrv_unref(bs-backing_hd);
-bs-backing_hd = NULL;
+bdrv_unref(backing_hd);
Here I wonder if this way of calling bdrv_open and doing a bdrv_unref confict
with Max latests bdrv_open patches.
You probably need to rebase and get rid of this bdrv_unref() : check Max
series
to be sure.
I'll check it.
bs-open_flags |= BDRV_O_NO_BACKING;
error_setg(errp, Could not open backing file: %s,
error_get_pretty(local_err));
error_free(local_err);
return ret;
}
-
-if (bs-backing_hd-file) {
-pstrcpy(bs-backing_file, sizeof(bs-backing_file),
-bs-backing_hd-file-filename);
-}
+bdrv_set_backing_hd(bs, backing_hd);
/* Recalculate the BlockLimits with the backing file */
bdrv_refresh_limits(bs);
diff --git a/include/block/block.h b/include/block/block.h
index 9125bbe..f449753 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -208,6 +208,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char
*filename,
int bdrv_open_image(BlockDriverState **pbs, const char *filename,
QDict *options, const char *bdref_key, int flags,
bool force_raw, bool allow_none, Error **errp);
+void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState
*backing_hd);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error
**errp);
int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
int flags, BlockDriver *drv, Error **errp);
--
1.8.5.3
bdrv_set_backing_hd seems to be handy since some people want to dynamically
change backing_file using QMP to move them from slow storage to fast storage
and
the reverse.
Thanks,
Fam
Re: [Qemu-devel] [PATCH v13 10/14] qmp: Add command 'blockdev-backup'
On Thu, 02/13 14:48, Benoît Canet wrote:
The Wednesday 29 Jan 2014 à 13:07:37 (+0800), Fam Zheng wrote :
Similar to drive-backup, but this command uses a device id as target
instead of creating/opening an image file.
Also add blocker on target bs, since the target is also a named device
now.
Add check and report error for bs == target which became possible but is
an illegal case with introduction of blockdev-backup.
Signed-off-by: Fam Zheng f...@redhat.com
---
block/backup.c | 26 ++
blockdev.c | 47 +++
qapi-schema.json | 49 +
qmp-commands.hx | 44
4 files changed, 166 insertions(+)
diff --git a/block/backup.c b/block/backup.c
index 15a2e55..ea46340 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -344,6 +344,7 @@ static void coroutine_fn backup_run(void *opaque)
hbitmap_free(job-bitmap);
bdrv_iostatus_disable(target);
+bdrv_op_unblock_all(target, job-common.blocker);
bdrv_unref(target);
block_job_completed(job-common, ret);
@@ -362,6 +363,11 @@ void backup_start(BlockDriverState *bs,
BlockDriverState *target,
assert(target);
assert(cb);
+if (bs == target) {
+error_setg(errp, Source and target cannot be the same);
+return;
+}
+
if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
on_source_error == BLOCKDEV_ON_ERROR_ENOSPC)
!bdrv_iostatus_is_enabled(bs)) {
@@ -369,6 +375,24 @@ void backup_start(BlockDriverState *bs,
BlockDriverState *target,
return;
}
+if (!bdrv_is_inserted(bs)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bs-device_name);
+return;
+}
+
+if (!bdrv_is_inserted(target)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, target-device_name);
+return;
+}
+
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+return;
+}
+
+if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARGET, errp)) {
+return;
+}
+
len = bdrv_getlength(bs);
if (len 0) {
error_setg_errno(errp, -len, unable to get length for '%s',
@@ -382,6 +406,8 @@ void backup_start(BlockDriverState *bs,
BlockDriverState *target,
return;
}
+bdrv_op_block_all(target, job-common.blocker);
+
job-on_source_error = on_source_error;
job-on_target_error = on_target_error;
job-target = target;
diff --git a/blockdev.c b/blockdev.c
index ffaa6a9..b346cc1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1940,6 +1940,8 @@ void qmp_drive_backup(const char *device, const char
*target,
return;
}
+/* Although backup_run has this check too, we need to use bs-drv
below, so
+ * do an early check redundantly. */
if (!bdrv_is_inserted(bs)) {
error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
return;
@@ -1956,6 +1958,7 @@ void qmp_drive_backup(const char *device, const char
*target,
}
}
+/* Early check to avoid creating target */
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
return;
}
@@ -2019,6 +2022,50 @@ BlockDeviceInfoList
*qmp_query_named_block_nodes(Error **errp)
return bdrv_named_nodes_list();
}
+void qmp_blockdev_backup(const char *device, const char *target,
+ enum MirrorSyncMode sync,
+ bool has_speed, int64_t speed,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ Error **errp)
+{
+BlockDriverState *bs;
+BlockDriverState *target_bs;
+Error *local_err = NULL;
+
+if (!has_speed) {
+speed = 0;
+}
+if (!has_on_source_error) {
+on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+}
+if (!has_on_target_error) {
+on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+}
+
+bs = bdrv_find(device);
This and the qmp prototype should be upgraded to the new device and node-name
alternative and make use of bdrv_lookup_bs().
Can we do it later in a separate patch?
Fam
Re: [Qemu-devel] Fwd: Trying to write a new device / virtio-i2c ?
2014-02-17 14:19 GMT+01:00 Paolo Bonzini pbonz...@redhat.com:
Il 17/02/2014 14:11, Alex David ha scritto:
I've tried using tmp105. As my linux isn't 64bits, i'm using
qemu-system-i386... It crashes my computer when I use it with my linux
image (it's a debian .qcow2..., easy to do some tests...).
You mean crashes your host?
Yes, my host crashes... I need to hard-reboot my computer.
I will most probably need a chardev anyways, I will need to read/write
data when I want to.
Depends on how much data. If it's just one or two ints, like tmp105, QOM
would work too. qmp-shell talks to QEMU via a simple JSON protocol, and it
has simple Python bindings too.
static const TypeInfo mydevice_i2c_type_info = {
.name= TYPE_MYDEVICE_I2C,
.parent= TYPE_I2C_SLAVE,
.instance_size= sizeof(MYDEVICEI2CState),
.instance_init= MYDEVICE_i2c_init,
.class_init= mydevice_i2c_class_init,
};
I will be able to add a chardev using the properties, right ?
Yes, using dc-props.
Does this seems ok for you ? So far, I understood the props are needed
for when I'm gonna declare the device at QEMU launch.
I am not sure if it's needed (my i2c-0 should be created anyways), but
in that case, how do I plug it on my socket on the host ?
Your device is not i2c-0. i2c-0 is provided by the PC board. Your device
will get one address, e.g. 0x42, on the i2c bus. You'll be able to access
it with i2cget and i2cset using 0 as the bus address (for i2c-0) and 0x42
as the device address on bus 0.
So you'll indeed have to do something like -chardev socket,id=foo -device
myi2c,address=0x42,chr=foo if you go for sockets, or just -device
myi2c,address=0x42 if you go for QOM. The chr property should go into
dc-props, while address is provided by the abstract class TYPE_I2C_SLAVE.
Paolo
That seems fairly easy.
But that leaves me with another problem as I now understand how I2C works
on linux... I, in fact, need at least 3 busses (for my at least 3 devices)
- so i2c-0, i2c-1, i2c-2 ... There are applications I can't change on the
guest and opening / writing / reading on i2c-0, i2c-1 etc...
Can I just declare these busses using i2c-dev ?
Re: [Qemu-devel] [PATCH v20 00/11] Shared library module support
Il 17/02/2014 14:16, Fam Zheng ha scritto: Ping? I'll send a pull request for patches 1-10 later. Paolo
Re: [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open()
Am 15.02.2014 um 04:30 hat Max Reitz geschrieben: The fail and success paths of bdrv_file_open() may be further shortened by reusing code already existent in bdrv_open(). This includes bdrv_file_open() not taking the reference to options which allows the removal of QDECREF(options) in that function. Signed-off-by: Max Reitz mre...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH v20 00/11] Shared library module support
On Mon, 02/17 14:34, Paolo Bonzini wrote: Il 17/02/2014 14:16, Fam Zheng ha scritto: Ping? I'll send a pull request for patches 1-10 later. Bravo, thanks! Fam
Re: [Qemu-devel] [PATCH v13 02/14] block: Introduce op_blockers to BlockDriverState
On Thu, 02/13 13:37, Benoît Canet wrote:
The Wednesday 29 Jan 2014 à 13:07:29 (+0800), Fam Zheng wrote :
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0bcf1c9..4e558d0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -270,6 +270,8 @@ typedef struct BlockLimits {
size_t opt_mem_alignment;
} BlockLimits;
+typedef struct BdrvOpBlocker BdrvOpBlocker;
+
/*
* Note: the function bdrv_append() copies and swaps contents of
* BlockDriverStates, so if you add new fields to this struct, please
@@ -361,6 +363,9 @@ struct BlockDriverState {
QLIST_HEAD(, BdrvTrackedRequest) tracked_requests;
+/* operation blockers */
+QLIST_HEAD(, BdrvOpBlocker) op_blockers[BLOCK_OP_TYPE_MAX];
Is a loop doing QLIST_INIT on this array elements required ?
Yes, we better do it.
Thanks,
Fam
Re: [Qemu-devel] [PATCH] qtest: Unlink pid file before reading from QMP
On Fri, Feb 14, 2014 at 4:38 PM, Andreas Färber afaer...@suse.de wrote:
Am 14.02.2014 15:43, schrieb Stefan Hajnoczi:
On Sun, Feb 09, 2014 at 12:21:41PM +0100, Andreas Färber wrote:
Despite 1ad3c6abc0d67e00b84abaa5527bc64b70ca2205, supplying invalid
arguments to the QEMU process still leaked a /tmp/qtest-*.pid file.
Fix this by reordering the reading and unlinking to before reading from
QMP socket, which relies on a running process.
Cc: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Andreas Färber afaer...@suse.de
---
tests/libqtest.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index c9a4f89..9433782 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -157,14 +157,14 @@ QTestState *qtest_init(const char *extra_args)
s-irq_level[i] = false;
}
-/* Read the QMP greeting and then do the handshake */
-qtest_qmp_discard_response(s, );
-qtest_qmp_discard_response(s, { 'execute': 'qmp_capabilities' });
-
s-qemu_pid = read_pid_file(pid_file);
unlink(pid_file);
g_free(pid_file);
+/* Read the QMP greeting and then do the handshake */
+qtest_qmp_discard_response(s, );
+qtest_qmp_discard_response(s, { 'execute': 'qmp_capabilities' });
+
Hmm...the original ordering was intentional.
In order to avoid race conditions between QEMU creating the pid file and
qtest reading the pid file, we wait until socket communication with QEMU
has been performed. That way we're sure the pid file already exists.
I think the tests can now fail due to the race condition.
I just verified that this patch did not slip into my pull. :)
Can you please propose an alternative solution or patch?
I have an idea to get rid of the pid file entirely. Will send a patch.
Stefan
[Qemu-devel] Call for testing QEMU aarch64-linux-user emulation
Hi, After a solid few months of work the QEMU master branch [1] has now reached instruction feature parity with the suse-1.6 [6] tree that a lot of people have been using to build various aarch64 binaries. In addition to the SUSE work we have fixed numerous edge cases and finished off classes of instructions. All instructions have been verified with Peter's RISU random instruction testing tool. I have also built and run many packages as well as built gcc and passed most of the aarch64 specific tests. I've tested against the following aarch64 rootfs: * SUSE [2] * Debian [3] * Ubuntu Saucy [4] In my tree the remaining insns that the GCC aarch64 tests need to implement are: FRECPE FRECPX CLS (2 misc variant) CLZ (2 misc variant) FSQRT FRINTZ FCVTZS Which I'm currently working though now. However for most build tasks I expect the instructions in master [1] will be enough. If you want the latest instructions working their way to mainline you are free to use my tree [5] which currently has: * Additional NEON/SIMD instructions * sendmsg syscall * Improved helper scripts for setting up binfmt_misc * The ability to set QEMU_LOG_FILENAME to /path/to/something-%d.log - this is useful when tests are failing N-levels deep as %d is replaced with the pid Feedback I'm interested in == * Any instruction failure (please include the log line with the unsupported message) * Any aarch64 specific failures (i.e. not generic QEMU threading flakeiness). If you need to catch me in real time I'm available on #qemu (stsquad) and #linaro-virtualization (ajb-linaro). Many thanks to the SUSE guys for getting the aarch64 train rolling. I hope your happy with the final result ;-) Cheers, -- Alex Bennée QEMU/KVM Hacker for Linaro [1] git://git.qemu.org/qemu.git master [2] http://download.opensuse.org/ports/aarch64/distribution/13.1/appliances/openSUSE-13.1-ARM-JeOS.aarch64-rootfs.aarch64-1.12.1-Build32.1.tbz [3] http://people.debian.org/~wookey/bootstrap/rootfs/debian-unstable-arm64.tar.gz [4] http://people.debian.org/~wookey/bootstrap/rootfs/saucy-arm64.tar.gz [5] https://github.com/stsquad/qemu/tree/ajb-a64-working [6] https://github.com/susematz/qemu/tree/aarch64-1.6
[Qemu-devel] [PATCH v3 07/20] gluster: default scheme to gluster:// and host to localhost.
Currently, gluster:///volname/img and (using file. options)
file.driver=gluster,file.filename=foo will segfault. Also,
//host/volname/img will be rejected, but it is a valid URL
that should be accepted just fine with file.driver=gluster.
Accept all of these, by inferring missing transport and host
as TCP and localhost respectively.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/gluster.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index a009b15..f9dd37f 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -127,7 +127,7 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const
char *filename)
}
/* transport */
-if (!strcmp(uri-scheme, gluster)) {
+if (!uri-scheme || !strcmp(uri-scheme, gluster)) {
gconf-transport = g_strdup(tcp);
} else if (!strcmp(uri-scheme, gluster+tcp)) {
gconf-transport = g_strdup(tcp);
@@ -163,7 +163,7 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const
char *filename)
}
gconf-server = g_strdup(qp-p[0].value);
} else {
-gconf-server = g_strdup(uri-server);
+gconf-server = g_strdup(uri-server ? uri-server : localhost);
gconf-port = uri-port;
}
--
1.8.5.3
[Qemu-devel] [PATCH v3 04/20] nbd: move socket wrappers to qemu-nbd
qemu-nbd is one of the few valid users of qerror_report_err. Move
the error-reporting socket wrappers there.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
include/block/nbd.h | 4
nbd.c | 50 --
qemu-nbd.c | 52
3 files changed, 52 insertions(+), 54 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 1b39c06..79502a0 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -62,10 +62,6 @@ enum {
#define NBD_MAX_BUFFER_SIZE (32 * 1024 * 1024)
ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
-int tcp_socket_incoming(const char *address, uint16_t port);
-int unix_socket_outgoing(const char *path);
-int unix_socket_incoming(const char *path);
-
int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
off_t *size, size_t *blocksize);
int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize);
diff --git a/nbd.c b/nbd.c
index 2fc1f1f..e5084b6 100644
--- a/nbd.c
+++ b/nbd.c
@@ -188,56 +188,6 @@ static ssize_t write_sync(int fd, void *buffer, size_t
size)
return ret;
}
-static void combine_addr(char *buf, size_t len, const char* address,
- uint16_t port)
-{
-/* If the address-part contains a colon, it's an IPv6 IP so needs [] */
-if (strstr(address, :)) {
-snprintf(buf, len, [%s]:%u, address, port);
-} else {
-snprintf(buf, len, %s:%u, address, port);
-}
-}
-
-int tcp_socket_incoming(const char *address, uint16_t port)
-{
-char address_and_port[128];
-Error *local_err = NULL;
-
-combine_addr(address_and_port, 128, address, port);
-int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0,
local_err);
-
-if (local_err != NULL) {
-qerror_report_err(local_err);
-error_free(local_err);
-}
-return fd;
-}
-
-int unix_socket_incoming(const char *path)
-{
-Error *local_err = NULL;
-int fd = unix_listen(path, NULL, 0, local_err);
-
-if (local_err != NULL) {
-qerror_report_err(local_err);
-error_free(local_err);
-}
-return fd;
-}
-
-int unix_socket_outgoing(const char *path)
-{
-Error *local_err = NULL;
-int fd = unix_connect(path, local_err);
-
-if (local_err != NULL) {
-qerror_report_err(local_err);
-error_free(local_err);
-}
-return fd;
-}
-
/* Basic flow for negotiation
Server Client
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 136e8c9..8138435 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -20,6 +20,8 @@
#include block/block.h
#include block/nbd.h
#include qemu/main-loop.h
+#include qemu/sockets.h
+#include qemu/error-report.h
#include block/snapshot.h
#include stdarg.h
@@ -201,6 +203,56 @@ static void termsig_handler(int signum)
qemu_notify_event();
}
+static void combine_addr(char *buf, size_t len, const char* address,
+ uint16_t port)
+{
+/* If the address-part contains a colon, it's an IPv6 IP so needs [] */
+if (strstr(address, :)) {
+snprintf(buf, len, [%s]:%u, address, port);
+} else {
+snprintf(buf, len, %s:%u, address, port);
+}
+}
+
+static int tcp_socket_incoming(const char *address, uint16_t port)
+{
+char address_and_port[128];
+Error *local_err = NULL;
+
+combine_addr(address_and_port, 128, address, port);
+int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0,
local_err);
+
+if (local_err != NULL) {
+qerror_report_err(local_err);
+error_free(local_err);
+}
+return fd;
+}
+
+static int unix_socket_incoming(const char *path)
+{
+Error *local_err = NULL;
+int fd = unix_listen(path, NULL, 0, local_err);
+
+if (local_err != NULL) {
+qerror_report_err(local_err);
+error_free(local_err);
+}
+return fd;
+}
+
+static int unix_socket_outgoing(const char *path)
+{
+Error *local_err = NULL;
+int fd = unix_connect(path, local_err);
+
+if (local_err != NULL) {
+qerror_report_err(local_err);
+error_free(local_err);
+}
+return fd;
+}
+
static void *show_parts(void *arg)
{
char *device = arg;
--
1.8.5.3
[Qemu-devel] [PATCH v3 02/20] nbd: correctly propagate errors
Before:
$ ./qemu-io-old
qemu-io-old open -r -o file.driver=nbd
one of path and host must be specified.
qemu-io-old: can't open device (null): Could not open image: Invalid
argument
$ ./qemu-io-old
qemu-io-old open -r -o file.driver=nbd,file.host=foo,file.path=bar
path and host may not be used at the same time.
qemu-io-old: can't open device (null): Could not open image: Invalid
argument
After:
$ ./qemu-io
qemu-io open -r -o file.driver=nbd
qemu-io: can't open device (null): one of path and host must be specified.
$ ./qemu-io
qemu-io open -r -o file.driver=nbd,file.host=foo,file.path=bar
qemu-io: can't open device (null): path and host may not be used at the
same time.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/nbd.c| 34 --
include/block/nbd.h| 1 -
nbd.c | 12
tests/qemu-iotests/051.out | 6 ++
4 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index fd89083..2378802 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -188,19 +188,18 @@ out:
g_free(file);
}
-static int nbd_config(BDRVNBDState *s, QDict *options, char **export)
+static void nbd_config(BDRVNBDState *s, QDict *options, char **export,
+ Error **errp)
{
Error *local_err = NULL;
if (qdict_haskey(options, path) == qdict_haskey(options, host)) {
if (qdict_haskey(options, path)) {
-qerror_report(ERROR_CLASS_GENERIC_ERROR, path and host may not
- be used at the same time.);
+error_setg(errp, path and host may not be used at the same
time.);
} else {
-qerror_report(ERROR_CLASS_GENERIC_ERROR, one of path and host
- must be specified.);
+error_setg(errp, one of path and host must be specified.);
}
-return -EINVAL;
+return;
}
s-client.is_unix = qdict_haskey(options, path);
@@ -209,9 +208,8 @@ static int nbd_config(BDRVNBDState *s, QDict *options, char
**export)
qemu_opts_absorb_qdict(s-socket_opts, options, local_err);
if (error_is_set(local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -EINVAL;
+error_propagate(errp, local_err);
+return;
}
if (!qemu_opt_get(s-socket_opts, port)) {
@@ -222,19 +220,17 @@ static int nbd_config(BDRVNBDState *s, QDict *options,
char **export)
if (*export) {
qdict_del(options, export);
}
-
-return 0;
}
-static int nbd_establish_connection(BlockDriverState *bs)
+static int nbd_establish_connection(BlockDriverState *bs, Error **errp)
{
BDRVNBDState *s = bs-opaque;
int sock;
if (s-client.is_unix) {
-sock = unix_socket_outgoing(qemu_opt_get(s-socket_opts, path));
+sock = unix_connect_opts(s-socket_opts, errp, NULL, NULL);
} else {
-sock = tcp_socket_outgoing_opts(s-socket_opts);
+sock = inet_connect_opts(s-socket_opts, errp, NULL, NULL);
if (sock = 0) {
socket_set_nodelay(sock);
}
@@ -255,17 +251,19 @@ static int nbd_open(BlockDriverState *bs, QDict *options,
int flags,
BDRVNBDState *s = bs-opaque;
char *export = NULL;
int result, sock;
+Error *local_err = NULL;
/* Pop the config into our state object. Exit if invalid. */
-result = nbd_config(s, options, export);
-if (result != 0) {
-return result;
+nbd_config(s, options, export, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return -EINVAL;
}
/* establish TCP connection, return error if it fails
* TODO: Configurable retry-until-timeout behaviour.
*/
-sock = nbd_establish_connection(bs);
+sock = nbd_establish_connection(bs, errp);
if (sock 0) {
return sock;
}
diff --git a/include/block/nbd.h b/include/block/nbd.h
index c90f5e4..e10ab82 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -64,7 +64,6 @@ enum {
ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
int tcp_socket_incoming(const char *address, uint16_t port);
int tcp_socket_incoming_spec(const char *address_and_port);
-int tcp_socket_outgoing_opts(QemuOpts *opts);
int unix_socket_outgoing(const char *path);
int unix_socket_incoming(const char *path);
diff --git a/nbd.c b/nbd.c
index 030f56b..17ca95b 100644
--- a/nbd.c
+++ b/nbd.c
@@ -199,18 +199,6 @@ static void combine_addr(char *buf, size_t len, const
char* address,
}
}
-int tcp_socket_outgoing_opts(QemuOpts *opts)
-{
-Error *local_err = NULL;
-int fd = inet_connect_opts(opts, local_err, NULL, NULL);
-if (local_err != NULL) {
-qerror_report_err(local_err);
-error_free(local_err);
-}
-
-return fd;
-}
-
int tcp_socket_incoming(const char *address,
[Qemu-devel] [PATCH v3 00/20] Improve bdrv_open error messages
Most of the block drivers are not using the Error** argument to bdrv_open, and instead just printing errors to stderr. This series improves that, and as a consequence it also avoids abuse of errno numbers. The only hurdle (caught by qemu-iotests, too) is VMDK, where we currently parse a file first as image, and second as a descriptor. This makes it hard to pick a good error message, because you do not know which attempt gave the most reasonable error message. For this reason patches 15-17 modify this approach and push up the detection of vmdk image file magic numbers. Apart from this, and from a segfault fixed by patch 7, the series consists of mostly trivial patches. Paolo v2-v3: Rebase for new qemu-nbd test (Jeff, 01-02/20) Fix memory leak (Stefan, 06/20) Use local_err for bdrv_file_open (Kevin, 09-11-12/20) Paolo Bonzini (20): nbd: produce a better error if neither host nor port is passed nbd: correctly propagate errors nbd: inline tcp_socket_incoming_spec into sole caller nbd: move socket wrappers to qemu-nbd iscsi: fix indentation iscsi: correctly propagate errors in iscsi_open gluster: default scheme to gluster:// and host to localhost. gluster: correctly propagate errors cow: correctly propagate errors curl: correctly propagate errors qcow: correctly propagate errors qed: correctly propagate errors vhdx: correctly propagate errors vvfat: correctly propagate errors vmdk: extract vmdk_read_desc vmdk: push vmdk_read_desc up to caller vmdk: do not try opening a file as both image and descriptor vmdk: correctly propagate errors block: do not abuse EMEDIUMTYPE vdi: say why an image is bad block/bochs.c | 3 +- block/cow.c| 11 ++-- block/curl.c | 13 ++--- block/gluster.c| 28 - block/iscsi.c | 142 +++-- block/nbd.c| 43 +++--- block/parallels.c | 3 +- block/qcow.c | 15 ++--- block/qcow2.c | 2 +- block/qed.c| 16 ++--- block/vdi.c| 29 + block/vhdx.c | 21 +++ block/vmdk.c | 123 +-- block/vpc.c| 3 +- block/vvfat.c | 9 +-- include/block/nbd.h| 6 -- nbd.c | 66 - qemu-nbd.c | 52 + tests/qemu-iotests/051.out | 4 +- tests/qemu-iotests/059.out | 6 +- 20 files changed, 307 insertions(+), 288 deletions(-) -- 1.8.5.3
[Qemu-devel] [PATCH v3 01/20] nbd: produce a better error if neither host nor port is passed
Before:
$ qemu-io-old
qemu-io-old open -r -o file.driver=nbd
qemu-io-old: can't open device (null): Could not open image: Invalid
argument
$ ./qemu-io-old
qemu-io-old open -r -o file.driver=nbd,file.host=foo,file.path=bar
path and host may not be used at the same time.
qemu-io-old: can't open device (null): Could not open image: Invalid
argument
After:
$ ./qemu-io
qemu-io open -r -o file.driver=nbd
one of path and host must be specified.
qemu-io: can't open device (null): Could not open image: Invalid argument
$ ./qemu-io
qemu-io open -r -o file.driver=nbd,file.host=foo,file.path=bar
path and host may not be used at the same time.
qemu-io: can't open device (null): Could not open image: Invalid argument
Next patch will fix the error propagation.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/nbd.c| 13 ++---
tests/qemu-iotests/051.out | 2 ++
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 327e913..fd89083 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -192,19 +192,18 @@ static int nbd_config(BDRVNBDState *s, QDict *options,
char **export)
{
Error *local_err = NULL;
-if (qdict_haskey(options, path)) {
-if (qdict_haskey(options, host)) {
+if (qdict_haskey(options, path) == qdict_haskey(options, host)) {
+if (qdict_haskey(options, path)) {
qerror_report(ERROR_CLASS_GENERIC_ERROR, path and host may not
be used at the same time.);
-return -EINVAL;
+} else {
+qerror_report(ERROR_CLASS_GENERIC_ERROR, one of path and host
+ must be specified.);
}
-s-client.is_unix = true;
-} else if (qdict_haskey(options, host)) {
-s-client.is_unix = false;
-} else {
return -EINVAL;
}
+s-client.is_unix = qdict_haskey(options, path);
s-socket_opts = qemu_opts_create(socket_optslist, NULL, 0,
error_abort);
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 30e2dbd..3e8d962 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -231,6 +231,7 @@ Testing: -drive driver=file
QEMU_PROG: -drive driver=file: could not open disk image ide0-hd0: The 'file'
block driver requires a file name
Testing: -drive driver=nbd
+QEMU_PROG: -drive driver=nbd: one of path and host must be specified.
QEMU_PROG: -drive driver=nbd: could not open disk image ide0-hd0: Could not
open image: Invalid argument
Testing: -drive driver=raw
@@ -240,6 +241,7 @@ Testing: -drive file.driver=file
QEMU_PROG: -drive file.driver=file: could not open disk image ide0-hd0: The
'file' block driver requires a file name
Testing: -drive file.driver=nbd
+QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified.
QEMU_PROG: -drive file.driver=nbd: could not open disk image ide0-hd0: Could
not open image: Invalid argument
Testing: -drive file.driver=raw
--
1.8.5.3
[Qemu-devel] [PATCH v3 08/20] gluster: correctly propagate errors
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/gluster.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index f9dd37f..bc9c59f 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -175,7 +175,8 @@ out:
return ret;
}
-static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename)
+static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
+ Error **errp)
{
struct glfs *glfs = NULL;
int ret;
@@ -183,8 +184,8 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf,
const char *filename)
ret = qemu_gluster_parseuri(gconf, filename);
if (ret 0) {
-error_report(Usage: file=gluster[+transport]://[server[:port]]/
-volname/image[?socket=...]);
+error_setg(errp, Usage: file=gluster[+transport]://[server[:port]]/
+ volname/image[?socket=...]);
errno = -ret;
goto out;
}
@@ -211,9 +212,11 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf,
const char *filename)
ret = glfs_init(glfs);
if (ret) {
-error_report(Gluster connection failed for server=%s port=%d
- volume=%s image=%s transport=%s, gconf-server, gconf-port,
- gconf-volname, gconf-image, gconf-transport);
+error_setg_errno(errp, errno,
+ Gluster connection failed for server=%s port=%d
+ volume=%s image=%s transport=%s, gconf-server,
+ gconf-port, gconf-volname, gconf-image,
+ gconf-transport);
goto out;
}
return glfs;
@@ -283,15 +286,14 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict
*options,
opts = qemu_opts_create(runtime_opts, NULL, 0, error_abort);
qemu_opts_absorb_qdict(opts, options, local_err);
if (error_is_set(local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
ret = -EINVAL;
goto out;
}
filename = qemu_opt_get(opts, filename);
-s-glfs = qemu_gluster_init(gconf, filename);
+s-glfs = qemu_gluster_init(gconf, filename, errp);
if (!s-glfs) {
ret = -errno;
goto out;
@@ -389,9 +391,9 @@ static int qemu_gluster_create(const char *filename,
int64_t total_size = 0;
GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
-glfs = qemu_gluster_init(gconf, filename);
+glfs = qemu_gluster_init(gconf, filename, errp);
if (!glfs) {
-ret = -errno;
+ret = -EINVAL;
goto out;
}
--
1.8.5.3
[Qemu-devel] [PATCH v3 03/20] nbd: inline tcp_socket_incoming_spec into sole caller
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
include/block/nbd.h | 1 -
nbd.c | 8 ++--
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index e10ab82..1b39c06 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -63,7 +63,6 @@ enum {
ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
int tcp_socket_incoming(const char *address, uint16_t port);
-int tcp_socket_incoming_spec(const char *address_and_port);
int unix_socket_outgoing(const char *path);
int unix_socket_incoming(const char *path);
diff --git a/nbd.c b/nbd.c
index 17ca95b..2fc1f1f 100644
--- a/nbd.c
+++ b/nbd.c
@@ -202,13 +202,9 @@ static void combine_addr(char *buf, size_t len, const
char* address,
int tcp_socket_incoming(const char *address, uint16_t port)
{
char address_and_port[128];
-combine_addr(address_and_port, 128, address, port);
-return tcp_socket_incoming_spec(address_and_port);
-}
-
-int tcp_socket_incoming_spec(const char *address_and_port)
-{
Error *local_err = NULL;
+
+combine_addr(address_and_port, 128, address, port);
int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0,
local_err);
if (local_err != NULL) {
--
1.8.5.3
[Qemu-devel] [PATCH v3 10/20] curl: correctly propagate errors
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/curl.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index a807584..7edb3cc 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -456,30 +456,27 @@ static int curl_open(BlockDriverState *bs, QDict
*options, int flags,
static int inited = 0;
if (flags BDRV_O_RDWR) {
-qerror_report(ERROR_CLASS_GENERIC_ERROR,
- curl block device does not support writes);
+error_setg(errp, curl block device does not support writes);
return -EROFS;
}
opts = qemu_opts_create(runtime_opts, NULL, 0, error_abort);
qemu_opts_absorb_qdict(opts, options, local_err);
if (error_is_set(local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
goto out_noclean;
}
s-readahead_size = qemu_opt_get_size(opts, readahead, READ_AHEAD_SIZE);
if ((s-readahead_size 0x1ff) != 0) {
-fprintf(stderr, HTTP_READAHEAD_SIZE %zd is not a multiple of 512\n,
-s-readahead_size);
+error_setg(errp, HTTP_READAHEAD_SIZE %zd is not a multiple of 512,
+ s-readahead_size);
goto out_noclean;
}
file = qemu_opt_get(opts, url);
if (file == NULL) {
-qerror_report(ERROR_CLASS_GENERIC_ERROR, curl block driver requires
- an 'url' option);
+error_setg(errp, curl block driver requires an 'url' option);
goto out_noclean;
}
--
1.8.5.3
[Qemu-devel] [PATCH v3 09/20] cow: correctly propagate errors
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/cow.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index 7fc0b12..0564744 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -82,7 +82,7 @@ static int cow_open(BlockDriverState *bs, QDict *options, int
flags,
char version[64];
snprintf(version, sizeof(version),
COW version %d, cow_header.version);
-qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
bs-device_name, cow, version);
ret = -ENOTSUP;
goto fail;
@@ -346,16 +346,14 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options,
ret = bdrv_create_file(filename, options, local_err);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
return ret;
}
ret = bdrv_file_open(cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
local_err);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
return ret;
}
--
1.8.5.3
[Qemu-devel] [PATCH v3 11/20] qcow: correctly propagate errors
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/qcow.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 948b0c5..a915bc3 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -119,17 +119,19 @@ static int qcow_open(BlockDriverState *bs, QDict
*options, int flags,
if (header.version != QCOW_VERSION) {
char version[64];
snprintf(version, sizeof(version), QCOW version %d, header.version);
-qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-bs-device_name, qcow, version);
+error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+ bs-device_name, qcow, version);
ret = -ENOTSUP;
goto fail;
}
if (header.size = 1 || header.cluster_bits 9) {
+error_setg(errp, invalid value in qcow header);
ret = -EINVAL;
goto fail;
}
if (header.crypt_method QCOW_CRYPT_AES) {
+error_setg(errp, invalid encryption method in qcow header);
ret = -EINVAL;
goto fail;
}
@@ -686,16 +688,14 @@ static int qcow_create(const char *filename,
QEMUOptionParameter *options,
ret = bdrv_create_file(filename, options, local_err);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
return ret;
}
ret = bdrv_file_open(qcow_bs, filename, NULL, NULL, BDRV_O_RDWR,
local_err);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
return ret;
}
--
1.8.5.3
[Qemu-devel] [PATCH v3 13/20] vhdx: correctly propagate errors
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/vhdx.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index 55689cf..bd3081b 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -402,9 +402,10 @@ int vhdx_update_headers(BlockDriverState *bs,
BDRVVHDXState *s,
}
/* opens the specified header block from the VHDX file header section */
-static int vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s)
+static void vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s,
+ Error **errp)
{
-int ret = 0;
+int ret;
VHDXHeader *header1;
VHDXHeader *header2;
bool h1_valid = false;
@@ -462,7 +463,6 @@ static int vhdx_parse_header(BlockDriverState *bs,
BDRVVHDXState *s)
} else if (!h1_valid h2_valid) {
s-curr_header = 1;
} else if (!h1_valid !h2_valid) {
-ret = -EINVAL;
goto fail;
} else {
/* If both headers are valid, then we choose the active one by the
@@ -473,27 +473,22 @@ static int vhdx_parse_header(BlockDriverState *bs,
BDRVVHDXState *s)
} else if (h2_seq h1_seq) {
s-curr_header = 1;
} else {
-ret = -EINVAL;
goto fail;
}
}
vhdx_region_register(s, s-headers[s-curr_header]-log_offset,
s-headers[s-curr_header]-log_length);
-
-ret = 0;
-
goto exit;
fail:
-qerror_report(ERROR_CLASS_GENERIC_ERROR, No valid VHDX header found);
+error_setg_errno(errp, -ret, No valid VHDX header found);
qemu_vfree(header1);
qemu_vfree(header2);
s-headers[0] = NULL;
s-headers[1] = NULL;
exit:
qemu_vfree(buffer);
-return ret;
}
@@ -878,7 +873,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options,
int flags,
int ret = 0;
uint32_t i;
uint64_t signature;
-
+Error *local_err = NULL;
s-bat = NULL;
s-first_visible_write = true;
@@ -901,8 +896,10 @@ static int vhdx_open(BlockDriverState *bs, QDict *options,
int flags,
* header update */
vhdx_guid_generate(s-session_guid);
-ret = vhdx_parse_header(bs, s);
-if (ret 0) {
+vhdx_parse_header(bs, s, local_err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
goto fail;
}
--
1.8.5.3
[Qemu-devel] [PATCH v3 05/20] iscsi: fix indentation
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/iscsi.c | 45 +++--
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index c97c040..95a1030 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1065,35 +1065,36 @@ static QemuOptsList runtime_opts = {
},
};
-static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi,
- int lun, int evpd, int pc) {
-int full_size;
-struct scsi_task *task = NULL;
-task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, 64);
+static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun,
+ int evpd, int pc)
+{
+int full_size;
+struct scsi_task *task = NULL;
+task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, 64);
+if (task == NULL || task-status != SCSI_STATUS_GOOD) {
+goto fail;
+}
+full_size = scsi_datain_getfullsize(task);
+if (full_size task-datain.size) {
+scsi_free_scsi_task(task);
+
+/* we need more data for the full list */
+task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, full_size);
if (task == NULL || task-status != SCSI_STATUS_GOOD) {
goto fail;
}
-full_size = scsi_datain_getfullsize(task);
-if (full_size task-datain.size) {
-scsi_free_scsi_task(task);
-
-/* we need more data for the full list */
-task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, full_size);
-if (task == NULL || task-status != SCSI_STATUS_GOOD) {
-goto fail;
-}
-}
+}
-return task;
+return task;
fail:
-error_report(iSCSI: Inquiry command failed : %s,
- iscsi_get_error(iscsi));
-if (task) {
-scsi_free_scsi_task(task);
-return NULL;
-}
+error_report(iSCSI: Inquiry command failed : %s,
+ iscsi_get_error(iscsi));
+if (task) {
+scsi_free_scsi_task(task);
return NULL;
+}
+return NULL;
}
/*
--
1.8.5.3
[Qemu-devel] [PATCH v3 06/20] iscsi: correctly propagate errors in iscsi_open
Before:
$ ./qemu-io-old
qemu-io-old open -r -o file.driver=iscsi,file.filename=foo
Failed to parse URL : foo
qemu-io-old: can't open device (null): Could not open 'foo': Invalid
argument
After:
$ ./qemu-io
qemu-io open -r -o file.driver=iscsi,file.filename=foo
qemu-io: can't open device (null): Failed to parse URL : foo
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/iscsi.c | 103 ++
1 file changed, 53 insertions(+), 50 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 95a1030..05dd50d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -856,7 +856,8 @@ retry:
#endif /* SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED */
-static int parse_chap(struct iscsi_context *iscsi, const char *target)
+static void parse_chap(struct iscsi_context *iscsi, const char *target,
+ Error **errp)
{
QemuOptsList *list;
QemuOpts *opts;
@@ -865,37 +866,35 @@ static int parse_chap(struct iscsi_context *iscsi, const
char *target)
list = qemu_find_opts(iscsi);
if (!list) {
-return 0;
+return;
}
opts = qemu_opts_find(list, target);
if (opts == NULL) {
opts = QTAILQ_FIRST(list-head);
if (!opts) {
-return 0;
+return;
}
}
user = qemu_opt_get(opts, user);
if (!user) {
-return 0;
+return;
}
password = qemu_opt_get(opts, password);
if (!password) {
-error_report(CHAP username specified but no password was given);
-return -1;
+error_setg(errp, CHAP username specified but no password was given);
+return;
}
if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
-error_report(Failed to set initiator username and password);
-return -1;
+error_setg(errp, Failed to set initiator username and password);
}
-
-return 0;
}
-static void parse_header_digest(struct iscsi_context *iscsi, const char
*target)
+static void parse_header_digest(struct iscsi_context *iscsi, const char
*target,
+Error **errp)
{
QemuOptsList *list;
QemuOpts *opts;
@@ -928,7 +927,7 @@ static void parse_header_digest(struct iscsi_context
*iscsi, const char *target)
} else if (!strcmp(digest, NONE-CRC32C)) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
} else {
-error_report(Invalid header-digest setting : %s, digest);
+error_setg(errp, Invalid header-digest setting : %s, digest);
}
}
@@ -986,12 +985,11 @@ static void iscsi_nop_timed_event(void *opaque)
}
#endif
-static int iscsi_readcapacity_sync(IscsiLun *iscsilun)
+static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
{
struct scsi_task *task = NULL;
struct scsi_readcapacity10 *rc10 = NULL;
struct scsi_readcapacity16 *rc16 = NULL;
-int ret = 0;
int retries = ISCSI_CMD_RETRIES;
do {
@@ -1006,8 +1004,7 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun)
if (task != NULL task-status == SCSI_STATUS_GOOD) {
rc16 = scsi_datain_unmarshall(task);
if (rc16 == NULL) {
-error_report(iSCSI: Failed to unmarshall readcapacity16
data.);
-ret = -EINVAL;
+error_setg(errp, iSCSI: Failed to unmarshall
readcapacity16 data.);
} else {
iscsilun-block_size = rc16-block_length;
iscsilun-num_blocks = rc16-returned_lba + 1;
@@ -1021,8 +1018,7 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun)
if (task != NULL task-status == SCSI_STATUS_GOOD) {
rc10 = scsi_datain_unmarshall(task);
if (rc10 == NULL) {
-error_report(iSCSI: Failed to unmarshall readcapacity10
data.);
-ret = -EINVAL;
+error_setg(errp, iSCSI: Failed to unmarshall
readcapacity10 data.);
} else {
iscsilun-block_size = rc10-block_size;
if (rc10-lba == 0) {
@@ -1035,20 +1031,18 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun)
}
break;
default:
-return 0;
+return;
}
} while (task != NULL task-status == SCSI_STATUS_CHECK_CONDITION
task-sense.key == SCSI_SENSE_UNIT_ATTENTION
retries-- 0);
if (task == NULL || task-status != SCSI_STATUS_GOOD) {
-error_report(iSCSI: failed to send readcapacity10 command.);
-ret = -EINVAL;
+error_setg(errp, iSCSI: failed to send readcapacity10 command.);
}
if (task) {
scsi_free_scsi_task(task);
}
-return ret;
}
/* TODO Convert to fine grained options */
@@ -1066,7 +1060,7 @@ static QemuOptsList runtime_opts = {
};
[Qemu-devel] [PATCH v3 20/20] vdi: say why an image is bad
Instead of just putting it in debugging output, we can now put the
value in an Error.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/vdi.c | 25 -
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index f3c6acf..1966d62 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -399,39 +399,46 @@ static int vdi_open(BlockDriverState *bs, QDict *options,
int flags,
ret = -EINVAL;
goto fail;
} else if (header.version != VDI_VERSION_1_1) {
-logout(unsupported version %u.%u\n,
- header.version 16, header.version 0x);
+error_setg(errp, unsupported VDI image (version %u.%u),
+ header.version 16, header.version 0x);
ret = -ENOTSUP;
goto fail;
} else if (header.offset_bmap % SECTOR_SIZE != 0) {
/* We only support block maps which start on a sector boundary. */
-logout(unsupported block map offset 0x%x B\n, header.offset_bmap);
+error_setg(errp, unsupported VDI image (unaligned block map offset
0x%x),
+ header.offset_bmap);
ret = -ENOTSUP;
goto fail;
} else if (header.offset_data % SECTOR_SIZE != 0) {
/* We only support data blocks which start on a sector boundary. */
-logout(unsupported data offset 0x%x B\n, header.offset_data);
+error_setg(errp, unsupported VDI image (unaligned data offset 0x%x),
+ header.offset_data);
ret = -ENOTSUP;
goto fail;
} else if (header.sector_size != SECTOR_SIZE) {
-logout(unsupported sector size %u B\n, header.sector_size);
+error_setg(errp, unsupported VDI image (sector size %u is not %u),
+ header.sector_size, SECTOR_SIZE);
ret = -ENOTSUP;
goto fail;
} else if (header.block_size != 1 * MiB) {
-logout(unsupported block size %u B\n, header.block_size);
+error_setg(errp, unsupported VDI image (sector size %u is not %u),
+ header.block_size, 1 * MiB);
ret = -ENOTSUP;
goto fail;
} else if (header.disk_size
(uint64_t)header.blocks_in_image * header.block_size) {
-logout(unsupported disk size % PRIu64 B\n, header.disk_size);
+error_setg(errp, unsupported VDI image (disk size % PRIu64 ,
+ image bitmap has room for % PRIu64 ),
+ header.disk_size,
+ (uint64_t)header.blocks_in_image * header.block_size);
ret = -ENOTSUP;
goto fail;
} else if (!uuid_is_null(header.uuid_link)) {
-logout(link uuid != 0, unsupported\n);
+error_setg(errp, unsupported VDI image (non-NULL link UUID));
ret = -ENOTSUP;
goto fail;
} else if (!uuid_is_null(header.uuid_parent)) {
-logout(parent uuid != 0, unsupported\n);
+error_setg(errp, unsupported VDI image (non-NULL parent UUID));
ret = -ENOTSUP;
goto fail;
}
--
1.8.5.3
[Qemu-devel] [PATCH v3 19/20] block: do not abuse EMEDIUMTYPE
Returning Wrong medium type for an image that does not have a valid
header is a bit weird. Improve the error by mentioning what format
was trying to open it.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/bochs.c | 3 ++-
block/cow.c | 3 ++-
block/parallels.c | 3 ++-
block/qcow.c | 3 ++-
block/qcow2.c | 2 +-
block/qed.c | 3 ++-
block/vdi.c | 4 ++--
block/vmdk.c | 6 --
block/vpc.c | 3 ++-
9 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/block/bochs.c b/block/bochs.c
index 51d9a90..4d6403f 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -129,7 +129,8 @@ static int bochs_open(BlockDriverState *bs, QDict *options,
int flags,
strcmp(bochs.subtype, GROWING_TYPE) ||
((le32_to_cpu(bochs.version) != HEADER_VERSION)
(le32_to_cpu(bochs.version) != HEADER_V1))) {
-return -EMEDIUMTYPE;
+error_setg(errp, Image not in Bochs format);
+return -EINVAL;
}
if (le32_to_cpu(bochs.version) == HEADER_V1) {
diff --git a/block/cow.c b/block/cow.c
index 0564744..9603347 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -74,7 +74,8 @@ static int cow_open(BlockDriverState *bs, QDict *options, int
flags,
}
if (be32_to_cpu(cow_header.magic) != COW_MAGIC) {
-ret = -EMEDIUMTYPE;
+error_setg(errp, Image not in COW format);
+ret = -EINVAL;
goto fail;
}
diff --git a/block/parallels.c b/block/parallels.c
index 2121e43..3f588f5 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -85,7 +85,8 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
if (memcmp(ph.magic, HEADER_MAGIC, 16) ||
(le32_to_cpu(ph.version) != HEADER_VERSION)) {
-ret = -EMEDIUMTYPE;
+error_setg(errp, Image not in Parallels format);
+ret = -EINVAL;
goto fail;
}
diff --git a/block/qcow.c b/block/qcow.c
index a915bc3..b273b2f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -113,7 +113,8 @@ static int qcow_open(BlockDriverState *bs, QDict *options,
int flags,
be64_to_cpus(header.l1_table_offset);
if (header.magic != QCOW_MAGIC) {
-ret = -EMEDIUMTYPE;
+error_setg(errp, Image not in qcow format);
+ret = -EINVAL;
goto fail;
}
if (header.version != QCOW_VERSION) {
diff --git a/block/qcow2.c b/block/qcow2.c
index 0b4335c..4f7a3d1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -449,7 +449,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options,
int flags,
if (header.magic != QCOW_MAGIC) {
error_setg(errp, Image is not in qcow2 format);
-ret = -EMEDIUMTYPE;
+ret = -EINVAL;
goto fail;
}
if (header.version 2 || header.version 3) {
diff --git a/block/qed.c b/block/qed.c
index b13ef8a..236d985 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -391,7 +391,8 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict
*options, int flags,
qed_header_le_to_cpu(le_header, s-header);
if (s-header.magic != QED_MAGIC) {
-return -EMEDIUMTYPE;
+error_setg(errp, Image not in QED format);
+return -EINVAL;
}
if (s-header.features ~QED_FEATURE_MASK) {
/* image uses unsupported feature bits */
diff --git a/block/vdi.c b/block/vdi.c
index 2d7490f..f3c6acf 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -395,8 +395,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options,
int flags,
}
if (header.signature != VDI_SIGNATURE) {
-logout(bad vdi signature %08x\n, header.signature);
-ret = -EMEDIUMTYPE;
+error_setg(errp, Image not in VDI format (bad signature %08x),
header.signature);
+ret = -EINVAL;
goto fail;
} else if (header.version != VDI_VERSION_1_1) {
logout(unsupported version %u.%u\n,
diff --git a/block/vmdk.c b/block/vmdk.c
index ba2b6f5..54ecbd6 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -748,7 +748,8 @@ static int vmdk_open_sparse(BlockDriverState *bs,
return vmdk_open_vmdk4(bs, file, flags, errp);
break;
default:
-return -EMEDIUMTYPE;
+error_setg(errp, Image not in VMDK format);
+return -EINVAL;
break;
}
}
@@ -861,7 +862,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int
flags, char *buf,
BDRVVmdkState *s = bs-opaque;
if (vmdk_parse_description(buf, createType, ct, sizeof(ct))) {
-ret = -EMEDIUMTYPE;
+error_setg(errp, invalid VMDK image descriptor);
+ret = -EINVAL;
goto exit;
}
if (strcmp(ct, monolithicFlat)
diff --git a/block/vpc.c b/block/vpc.c
index 1d326cb..82bf248 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -190,7 +190,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options,
int flags,
goto fail;
}
if (strncmp(footer-creator, conectix, 8)) {
-ret = -EMEDIUMTYPE;
+
[Qemu-devel] [PATCH v3 15/20] vmdk: extract vmdk_read_desc
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/vmdk.c | 41 ++---
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index ff6f5ee..c834512 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -529,6 +529,32 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
uint64_t desc_offset, Error **errp);
+static char *vmdk_read_desc(BlockDriverState *file, uint64_t desc_offset,
+Error **errp)
+{
+int64_t size;
+char *buf;
+int ret;
+
+size = bdrv_getlength(file);
+if (size 0) {
+error_setg_errno(errp, -size, Could not access file);
+return NULL;
+}
+
+size = MIN(size, 1 20); /* avoid unbounded allocation */
+buf = g_malloc0(size + 1);
+
+ret = bdrv_pread(file, desc_offset, buf, size);
+if (ret 0) {
+error_setg_errno(errp, -ret, Could not read from file);
+g_free(buf);
+return NULL;
+}
+
+return buf;
+}
+
static int vmdk_open_vmdk4(BlockDriverState *bs,
BlockDriverState *file,
int flags, Error **errp)
@@ -822,23 +848,16 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int
flags,
uint64_t desc_offset, Error **errp)
{
int ret;
-char *buf = NULL;
+char *buf;
char ct[128];
BDRVVmdkState *s = bs-opaque;
-int64_t size;
-size = bdrv_getlength(bs-file);
-if (size 0) {
+buf = vmdk_read_desc(bs-file, desc_offset, errp);
+if (!buf) {
return -EINVAL;
-}
-
-size = MIN(size, 1 20); /* avoid unbounded allocation */
-buf = g_malloc0(size + 1);
-
-ret = bdrv_pread(bs-file, desc_offset, buf, size);
-if (ret 0) {
goto exit;
}
+
if (vmdk_parse_description(buf, createType, ct, sizeof(ct))) {
ret = -EMEDIUMTYPE;
goto exit;
--
1.8.5.3
[Qemu-devel] [PATCH v3 14/20] vvfat: correctly propagate errors
Before:
$ ./qemu-io-old
qemu-io-old open -r -o driver=vvfat,fat-type=24,dir=i386-softmmu
Valid FAT types are only 12, 16 and 32
qemu-io-old: can't open device (null): Could not open image: Invalid
argument
After:
$ ./qemu-io
qemu-io open -r -o driver=vvfat,fat-type=24,dir=i386-softmmu
qemu-io: can't open device (null): Valid FAT types are only 12, 16 and 32
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/vvfat.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 664941c..7c3521a 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1086,16 +1086,14 @@ DLOG(if (stderr == NULL) {
opts = qemu_opts_create(runtime_opts, NULL, 0, error_abort);
qemu_opts_absorb_qdict(opts, options, local_err);
if (error_is_set(local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
ret = -EINVAL;
goto fail;
}
dirname = qemu_opt_get(opts, dir);
if (!dirname) {
-qerror_report(ERROR_CLASS_GENERIC_ERROR, vvfat block driver requires
- a 'dir' option);
+error_setg(errp, vvfat block driver requires a 'dir' option);
ret = -EINVAL;
goto fail;
}
@@ -1135,8 +1133,7 @@ DLOG(if (stderr == NULL) {
case 12:
break;
default:
-qerror_report(ERROR_CLASS_GENERIC_ERROR, Valid FAT types are only
- 12, 16 and 32);
+error_setg(errp, Valid FAT types are only 12, 16 and 32);
ret = -EINVAL;
goto fail;
}
--
1.8.5.3
[Qemu-devel] [PATCH v3 12/20] qed: correctly propagate errors
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/qed.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/block/qed.c b/block/qed.c
index b9ca7ac..b13ef8a 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -398,7 +398,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict
*options, int flags,
char buf[64];
snprintf(buf, sizeof(buf), % PRIx64,
s-header.features ~QED_FEATURE_MASK);
-qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
bs-device_name, QED, buf);
return -ENOTSUP;
}
@@ -545,7 +545,8 @@ static void bdrv_qed_close(BlockDriverState *bs)
static int qed_create(const char *filename, uint32_t cluster_size,
uint64_t image_size, uint32_t table_size,
- const char *backing_file, const char *backing_fmt)
+ const char *backing_file, const char *backing_fmt,
+ Error **errp)
{
QEDHeader header = {
.magic = QED_MAGIC,
@@ -566,16 +567,14 @@ static int qed_create(const char *filename, uint32_t
cluster_size,
ret = bdrv_create_file(filename, NULL, local_err);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
return ret;
}
ret = bdrv_file_open(bs, filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB, local_err);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
+error_propagate(errp, local_err);
return ret;
}
@@ -665,7 +664,7 @@ static int bdrv_qed_create(const char *filename,
QEMUOptionParameter *options,
}
return qed_create(filename, cluster_size, image_size, table_size,
- backing_file, backing_fmt);
+ backing_file, backing_fmt, errp);
}
typedef struct {
--
1.8.5.3
Re: [Qemu-devel] [PATCH v2 09/20] cow: correctly propagate errors
Am 17.02.2014 um 14:15 hat Fam Zheng geschrieben:
On Sat, 02/15 11:01, Markus Armbruster wrote:
Jeff Cody jc...@redhat.com writes:
On Fri, Feb 14, 2014 at 05:45:40PM +0100, Kevin Wolf wrote:
Am 11.02.2014 um 18:03 hat Paolo Bonzini geschrieben:
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/cow.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index 7fc0b12..43a2150 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -82,7 +82,7 @@ static int cow_open(BlockDriverState *bs, QDict
*options, int flags,
char version[64];
snprintf(version, sizeof(version),
COW version %d, cow_header.version);
-qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
bs-device_name, cow, version);
ret = -ENOTSUP;
goto fail;
@@ -330,7 +330,6 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options,
struct stat st;
int64_t image_sectors = 0;
const char *image_filename = NULL;
-Error *local_err = NULL;
int ret;
BlockDriverState *cow_bs;
@@ -344,18 +343,13 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options,
options++;
}
-ret = bdrv_create_file(filename, options, local_err);
+ret = bdrv_create_file(filename, options, errp);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
return ret;
}
-ret = bdrv_file_open(cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
- local_err);
+ret = bdrv_file_open(cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
errp);
if (ret 0) {
-qerror_report_err(local_err);
-error_free(local_err);
return ret;
}
This is technically correct, but I think general policy is that using
the local_err pattern is preferred anyway.
If I recall correct, I think there are several places that pass errp
along. How about this for a rule of thumb policy: use the local_err
method if the function does not indicate error outside of the passed
Error pointer.
Use local_err when you need to examine the error object. Passing errp
directly is no good then, because it may be null.
When you're forwarding errors without examining them, then passing errp
directly is just fine.
Does this mean that error_is_set() is always used by programmer to check a
non-NULL error pointer? Is there any case to call error_is_set(errp) without
knowing if errp is NULL or not? If no, should we enforce the rule and add
assert(errp) in error_is_set()?
Sounds like a good idea to me, it would catch bugs where you forget to
use a local_err. Of course, it requires that error_is_set() is used
instead of just using errp as a boolean, but such an assertion could
actually be a reason to make this the policy.
Kevin
[Qemu-devel] [PATCH v3 17/20] vmdk: do not try opening a file as both image and descriptor
This prepares for propagating errors from vmdk_open_sparse and
vmdk_open_desc_file up to the caller of vmdk_open.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/vmdk.c | 22 +++---
tests/qemu-iotests/059.out | 4 ++--
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 0ebb732..d3858b0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -885,20 +885,28 @@ static int vmdk_open(BlockDriverState *bs, QDict
*options, int flags,
char *buf = NULL;
int ret;
BDRVVmdkState *s = bs-opaque;
+uint32_t magic;
buf = vmdk_read_desc(bs-file, 0, errp);
if (!buf) {
return -EINVAL;
}
-if (vmdk_open_sparse(bs, bs-file, flags, buf, errp) == 0) {
-s-desc_offset = 0x200;
-} else {
-ret = vmdk_open_desc_file(bs, flags, buf, errp);
-if (ret) {
-goto fail;
-}
+magic = ldl_be_p(buf);
+switch (magic) {
+case VMDK3_MAGIC:
+case VMDK4_MAGIC:
+ret = vmdk_open_sparse(bs, bs-file, flags, buf, errp);
+s-desc_offset = 0x200;
+break;
+default:
+ret = vmdk_open_desc_file(bs, flags, buf, errp);
+break;
}
+if (ret) {
+goto fail;
+}
+
/* try to open parent images, if exist */
ret = vmdk_parent_open(bs);
if (ret) {
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 4ffeb54..4600670 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -8,7 +8,7 @@ no file open, try 'help open'
=== Testing too big L2 table size ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
L2 table size too big
-qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk':
Wrong medium type
+qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk':
Invalid argument
no file open, try 'help open'
=== Testing too big L1 table size ===
@@ -2046,7 +2046,7 @@ RW 12582912 VMFS dummy.IMGFMT 1
=== Testing truncated sparse ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
qemu-img: File truncated, expecting at least 13172736 bytes
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Could not open
'TEST_DIR/t.IMGFMT': Wrong medium type
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Could not open
'TEST_DIR/t.IMGFMT': Invalid argument
=== Testing version 3 ===
image: TEST_DIR/iotest-version3.IMGFMT
--
1.8.5.3
[Qemu-devel] [PATCH v3 16/20] vmdk: push vmdk_read_desc up to caller
Currently, we just try reading a VMDK file as both image and descriptor.
This makes it hard to choose which of the two attempts gave the best error.
We'll decide in advance if the file looks like an image or a descriptor,
and this patch is the first step to that end.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/vmdk.c | 55 +++
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index c834512..0ebb732 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -526,8 +526,8 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
return ret;
}
-static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
- uint64_t desc_offset, Error **errp);
+static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
+ Error **errp);
static char *vmdk_read_desc(BlockDriverState *file, uint64_t desc_offset,
Error **errp)
@@ -576,7 +576,13 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
if (header.capacity == 0) {
uint64_t desc_offset = le64_to_cpu(header.desc_offset);
if (desc_offset) {
-return vmdk_open_desc_file(bs, flags, desc_offset 9, errp);
+char *buf = vmdk_read_desc(file, desc_offset 9, errp);
+if (!buf) {
+return -EINVAL;
+}
+ret = vmdk_open_desc_file(bs, flags, buf, errp);
+g_free(buf);
+return ret;
}
}
@@ -727,16 +733,12 @@ static int vmdk_parse_description(const char *desc, const
char *opt_name,
/* Open an extent file and append to bs array */
static int vmdk_open_sparse(BlockDriverState *bs,
-BlockDriverState *file,
-int flags, Error **errp)
+BlockDriverState *file, int flags,
+char *buf, Error **errp)
{
uint32_t magic;
-if (bdrv_pread(file, 0, magic, sizeof(magic)) != sizeof(magic)) {
-return -EIO;
-}
-
-magic = be32_to_cpu(magic);
+magic = ldl_be_p(buf);
switch (magic) {
case VMDK3_MAGIC:
return vmdk_open_vmfs_sparse(bs, file, flags, errp);
@@ -820,8 +822,14 @@ static int vmdk_parse_extents(const char *desc,
BlockDriverState *bs,
extent-flat_start_offset = flat_offset 9;
} else if (!strcmp(type, SPARSE) || !strcmp(type, VMFSSPARSE)) {
/* SPARSE extent and VMFSSPARSE extent are both COWD sparse
file*/
-ret = vmdk_open_sparse(bs, extent_file, bs-open_flags, errp);
+char *buf = vmdk_read_desc(extent_file, 0, errp);
+if (!buf) {
+ret = -EINVAL;
+} else {
+ret = vmdk_open_sparse(bs, extent_file, bs-open_flags, buf,
errp);
+}
if (ret) {
+g_free(buf);
bdrv_unref(extent_file);
return ret;
}
@@ -844,20 +852,13 @@ next_line:
return 0;
}
-static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
- uint64_t desc_offset, Error **errp)
+static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
+ Error **errp)
{
int ret;
-char *buf;
char ct[128];
BDRVVmdkState *s = bs-opaque;
-buf = vmdk_read_desc(bs-file, desc_offset, errp);
-if (!buf) {
-return -EINVAL;
-goto exit;
-}
-
if (vmdk_parse_description(buf, createType, ct, sizeof(ct))) {
ret = -EMEDIUMTYPE;
goto exit;
@@ -875,20 +876,25 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int
flags,
s-desc_offset = 0;
ret = vmdk_parse_extents(buf, bs, bs-file-filename, errp);
exit:
-g_free(buf);
return ret;
}
static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
+char *buf = NULL;
int ret;
BDRVVmdkState *s = bs-opaque;
-if (vmdk_open_sparse(bs, bs-file, flags, errp) == 0) {
+buf = vmdk_read_desc(bs-file, 0, errp);
+if (!buf) {
+return -EINVAL;
+}
+
+if (vmdk_open_sparse(bs, bs-file, flags, buf, errp) == 0) {
s-desc_offset = 0x200;
} else {
-ret = vmdk_open_desc_file(bs, flags, 0, errp);
+ret = vmdk_open_desc_file(bs, flags, buf, errp);
if (ret) {
goto fail;
}
@@ -907,10 +913,11 @@ static int vmdk_open(BlockDriverState *bs, QDict
*options, int flags,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
vmdk, bs-device_name, live migration);
migrate_add_blocker(s-migration_blocker);
-
+g_free(buf);
return 0;
fail:
+g_free(buf);
g_free(s-create_type);
s-create_type = NULL;
vmdk_free_extents(bs);
--
1.8.5.3
[Qemu-devel] [PATCH v3 18/20] vmdk: correctly propagate errors
Now that we can return the right errors, use the Error** parameter
to pass them back instead of just printing them.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/vmdk.c | 11 ++-
tests/qemu-iotests/059.out | 6 ++
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index d3858b0..ba2b6f5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -572,6 +572,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
error_setg_errno(errp, -ret,
Could not read header from file '%s',
file-filename);
+return -EINVAL;
}
if (header.capacity == 0) {
uint64_t desc_offset = le64_to_cpu(header.desc_offset);
@@ -641,8 +642,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
char buf[64];
snprintf(buf, sizeof(buf), VMDK version %d,
le32_to_cpu(header.version));
-qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-bs-device_name, vmdk, buf);
+error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+ bs-device_name, vmdk, buf);
return -ENOTSUP;
} else if (le32_to_cpu(header.version) == 3 (flags BDRV_O_RDWR)) {
/* VMware KB 2064959 explains that version 3 added support for
@@ -654,7 +655,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
}
if (le32_to_cpu(header.num_gtes_per_gt) 512) {
-error_report(L2 table size too big);
+error_setg(errp, L2 table size too big);
return -EINVAL;
}
@@ -670,8 +671,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
}
if (bdrv_getlength(file)
le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE) {
-error_report(File truncated, expecting at least %lld bytes,
-le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE);
+error_setg(errp, File truncated, expecting at least %lld bytes,
+ le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE);
return -EINVAL;
}
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 4600670..3371c86 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -7,8 +7,7 @@ no file open, try 'help open'
=== Testing too big L2 table size ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-L2 table size too big
-qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk':
Invalid argument
+qemu-io: can't open device TEST_DIR/t.vmdk: L2 table size too big
no file open, try 'help open'
=== Testing too big L1 table size ===
@@ -2045,8 +2044,7 @@ RW 12582912 VMFS dummy.IMGFMT 1
=== Testing truncated sparse ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
-qemu-img: File truncated, expecting at least 13172736 bytes
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Could not open
'TEST_DIR/t.IMGFMT': Invalid argument
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated, expecting at
least 13172736 bytes
=== Testing version 3 ===
image: TEST_DIR/iotest-version3.IMGFMT
--
1.8.5.3
[Qemu-devel] [PATCH] Properly check if 'log dirty' flags have changed
The test (!!(mem-flags KVM_MEM_LOG_DIRTY_PAGES) == enable) is not
good because the condition is valid when enable == 0 and current dirty log
memory flag is set.
As a consequence kvm_log_global_stop() does not stop the KVM dirty log
tracking: kvm_set_migration_log(0) didn't do its job.
So instead I propose to use kvm_slot_dirty_pages_log_change() which correctly
compare the memory flags (old/new).
Signed-off-by: Vincent KHERBACHE vincent.kherba...@inria.fr
---
kvm-all.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index 2ca9143..f104f87 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -355,7 +355,7 @@ static int kvm_set_migration_log(int enable)
{
KVMState *s = kvm_state;
KVMSlot *mem;
-int i, err;
+int i, err = 0;
s-migration_log = enable;
@@ -365,15 +365,9 @@ static int kvm_set_migration_log(int enable)
if (!mem-memory_size) {
continue;
}
-if (!!(mem-flags KVM_MEM_LOG_DIRTY_PAGES) == enable) {
-continue;
-}
-err = kvm_set_user_memory_region(s, mem);
-if (err) {
-return err;
-}
+err = kvm_slot_dirty_pages_log_change(mem, (bool)enable);
}
-return 0;
+return err;
}
/* get kvm's dirty pages bitmap and update qemu's */
--
1.8.3.1
[Qemu-devel] [PATCH] dirty log: fix kvm_set_migration_log() behavior
Vincent KHERBACHE (1): Properly check if 'log dirty' flags have changed kvm-all.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) -- 1.8.3.1
