Re: [Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
Amos Kong ak...@redhat.com writes: On Tue, Apr 22, 2014 at 09:20:41AM +0200, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Il 18/04/2014 23:56, Amos Kong ha scritto: Currently we always add a space after c_type in mcgen(), there is some redundant space in generated code. The space isn't needed for points by the coding style. char * value; ^ qapi_free_NameInfo(NameInfo * obj) ^ It's fussy to add checking in each mcgen(), this patch just addes the necessary space in c_type(), and remove original space in mcgen(). It also makes the generator harder to read to though, and the improvement in the generated code is minor enough that I think the change is overall negative. But I won't oppose the patch if the maintainers are fine with it. Hi Markus, The readability hit could be avoided: instead of moving the space from mcgen()'s argument to c_type()'s result, add a hungry character to c_type()'s result, then make it eat space to the right in mcgen(). '\b' is used to eat char in left, but what's the hungry character to each char in right? When I added a '\b' to c_type()'s return, there is an arror: error: stray '\10' in program I'm afraid you misunderstood me. Pick a character that cannot occur in valid C code. Make c_type()'s return value end in that character in the cases where you don't want space. Then make mcgen() strip it from its return value along with immediately following space. Questions? [...]
Re: [Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
Kevin Wolf kw...@redhat.com writes:
Am 24.04.2014 um 09:27 hat Markus Armbruster geschrieben:
Kevin Wolf kw...@redhat.com writes:
Am 23.04.2014 um 17:34 hat Eric Blake geschrieben:
On 04/23/2014 09:12 AM, Kevin Wolf wrote:
All the functionality to use the host_device, host_cdrom and host_floppy
drivers is already there, they just need to be added to the schema.
Signed-off-by: Kevin Wolf kw...@redhat.com
---
qapi-schema.json | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 391356f..0fc0f12 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4288,7 +4288,8 @@
# Since: 2.0
We haven't been good at tracking enum growth, but it can't hurt to try.
It might be worth changing this line to read:
# Since: 2.0, 'host_device', 'host_cdrom', 'host_floppy' since 2.1
I'm fine with documenting the changes, but this format doesn't look like
it works very well in the long term.
##
{ 'enum': 'BlockdevDriver',
- 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp',
vvfat', 'blkdebug',
+ 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
Any reason you used _ instead of - in these names? Newer QMP tends to
prefer - unless there is a good reason why _ has already been baked in
due to back-compat.
The block driver has always been called host_device with an underscore.
We can't change it because that would break compatibility on the command
line.
A simple indirection could buy us a little more consistency in QMP. If
you say that's not worth the trouble because there's so much
inconsistency already, you have a point. I still hate it, though :)
Is inconsistency between block device configuration in QMP and block
device configuration on the command line any better than inconsistency
between one QMP command and part of the other QMP commands?
Matter of taste :)
The command line is internally inconsistent, too: -add-fd, -mem-path,
-machine dump-guest-core=..., but also -virtfs_synth, -machine
kernel_irqchip...
No adult supervision...
So, similar problem, similar solution space:
Probably the only way to stop the proliferation of this '-' vs. '_'
nuisance is to accept both '-' and '_' everywhere in QMP, and use only
'-' in documentation. Clearly beyond the scope of your patch.
Not sure how hard this would be to implement, but I agree that this
would be the best solution.
Re: [Qemu-devel] [PATCH microblaze v1 1/1] net: xilinx_axienet.c: Add phy soft reset bit clearing
On Thu, Apr 24, 2014 at 02:03:45PM +0200, Stefan Hajnoczi wrote: On Tue, Apr 08, 2014 at 06:52:39PM -0700, Peter Crosthwaite wrote: From: Nathan Rossi nathan.ro...@xilinx.com Clear the BMCR Reset when writing to registers. Signed-off-by: Nathan Rossi nathan.ro...@xilinx.com [ PC: * Trivial style fixes to commit message ] Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/net/xilinx_axienet.c | 3 +++ 1 file changed, 3 insertions(+) Will this patch go through Edgar's MicroBlaze tree? Just wanted to check I'm not holding it up :). Hi Stefan, Feel free to take it via your tree. Reviewed-by: Edgar E. Iglesias edgar.igles...@gmail.com Thanks, Edgar
Re: [Qemu-devel] [PATCH 0/3] virtio: Eliminate exit(1) upon invalid request in virtio-blk and virtio-scsi
Michael S. Tsirkin m...@redhat.com writes: On Thu, Apr 24, 2014 at 12:43:56PM +0200, Kevin Wolf wrote: Am 24.04.2014 um 09:55 hat Michael S. Tsirkin geschrieben: On Thu, Apr 24, 2014 at 09:15:25AM +0200, Markus Armbruster wrote: Michael S. Tsirkin m...@redhat.com writes: On Wed, Apr 23, 2014 at 11:22:51AM +0800, Fam Zheng wrote: This series is a step towards getting rid of such code, Sure, incremental patches are good. At this point I think it's a good idea to clearly mark this as RFC - I don't think we should yet merge this upstream until the solution is a bit more complete. Changing virtio is the easy part though, so I'm not sure it's a good idea to start there. Does this series hinder work on the harder parts in any way? Does it pick a specific solution that may not work for the harder parts? If not, then I can't see what keeping it out of tree can buy us. Less code churn. It's more code apparently for no real benefit since buggy drivers will still abort qemu. Depends on what bugs the driver has. Not fixing bugs because there are still other bugs that can crash qemu isn't productive. If we went this way consistently, we would reject any bug fix unless the commit message contains a mathematical proof that all of qemu is correct now. We could stop development right now. This is an easy bug to fix, we have the patches, so let's just fix it. The solution won't become any better by waiting for independent fixes. Making nested virt work well is a big project, I'd like to see some progress on the hard parts before trying to address easy corner cases like this one. Addressing the easy parts doesn't make the hard parts any harder. Exactly. Thanks for putting it so pithily; I rather like that :) Fam's work is an incremental step towards rectifying a certain kind of mistake in device model code. Nested virt just happens to be hurt by these mistakes. Making it depend on the big, complex, ambitious project like make nested virt work is *backwards*, and all that can achieve is moving the problem from incrementally solvable towards boil the ocean. Regarding the malicious guest, protecting D.O.S. attack is also valuable, isn't it? Thanks, Fam Guest denying itself a service? I'm not sure why it's valuable. If I remember correctly, the DOS involved passthrough of a virtual device to a nested guest or something like that. Guest killing itself is unexciting, nested guest killing its host qualifies as DOS. I guess our current answer to that is don't do that then. Yes. virtio doesn't support that for a variety of other reasons, one of which is that it doesn't go through an mmu. Now, before someone sends a trivial patch converting it to mmu aware calls, that's not yet possible without teaching vhost and dataplane about MMU. Nested virt is really just one example for a userspace virtio driver. Userspace shouldn't be able to kill the whole guest. Kevin Without an MMIO this is fundamentally unavoidable. Really? Why is it fundamentally impossible to put the device into an error state when we detect invalid device use by the guest? Honest question; please excuse my ignorance here...
Re: [Qemu-devel] [GSoC] Wanted: small warmup tasks
Andreas Färber afaer...@suse.de writes: Am 24.04.2014 08:19, schrieb Jan Kiszka: On 2014-04-23 11:25, Stefan Hajnoczi wrote: Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. To follow up on this: I'm currently looking for optional tiny warmup tasks for our QEMU students during the bonding period (till May 18). If you have any trivial issues or extensions in mind that someone could address within a few days or even hours, that would be perfect. It could even be something like reformat the printing of these messages or so. Replacing some more fprintf(stderr, foo\n) with error_report(foo) comes to mind. :) A logical next step after this kind of warmup would be a simple conversion to error_set(). Perhaps a good place to start would be qmp_chardev_add(). The switch there has some cases converted already. Convert one or more of the remaining ones. Beware of the rabbit holes! In case of vertigo, contact your mentor immediately ;) [...]
Re: [Qemu-devel] [PATCH] qxl: Fix initial screenshot with spice
Hi, ui/console.c:qmp_screendump attempts to handle this by calling graphic_hw_update, but qxl handle's that asynchronously, and it isn't run until after the first screendump is performed. That's why the second screendump is correct. Known issue. The graphic_hw_update makes sure it works sort-of ok when you do snapshorts regularly (popular use case: autotest). It's band aid, but still better than nothing. Fix this by triggering qxl_render_update whenever a non-vga surface is created. That only catches a small fraction of the problem. You'll still face the very same issue shortly thereafter. Start firefox in your guest, then do two snapshots again. Different variant of the same bug: no firefox window on the first snapshot. And this patch doesn't help. We'll basically need a better snapshot command, but qmp lacks infrastructure. We'll need something like blockjobs, but more general and not limited to the block layer. Then you can kick off the screenshot request as job, get notified on completion, and we can get the qxl case right then. cheers, Gerd
Re: [Qemu-devel] [PATCH for-2.0 v2] tests: Don't run qom-test twice
Andreas Färber afaer...@suse.de writes: Am 24.04.2014 16:34, schrieb Peter Maydell: On 24 April 2014 15:29, Stefan Hajnoczi stefa...@redhat.com wrote: On Mon, Apr 07, 2014 at 04:13:00PM +0200, Andreas Färber wrote: Commit 3687d5325 accidentally resulted in running qom-test twice for x86_64, once directly via the wildcard, and once because x86_64 includes all the i386 qtests (which includes qom-test). Filter out x86_64 as well as microblazeel and xtensaeb to fix this. Cc: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Andreas Färber afaer...@suse.de --- v1 (PMM) - v2: * Instead of sorting all qtests, leave the order intact and just filter the three affected architectures out. tests/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Didn't make it into 2.0 but... Reviewed-by: Stefan Hajnoczi stefa...@redhat.com Personally I prefer the just sort-and-uniquify approach. This patch introduces an explicit list of architectures which need to be special case, which means that if we ever add a future arch variant which also needs this special casing we need to update the list. The sort patch doesn't have this requirement -- it will just work for both this and for any other reason why we might end up with a test in the list multiple times. Your dislike is why I didn't apply it for 2.0. However, a) the inheritance of tests for these three architectures is already an explicit statement in the Makefile and more severely b) by sorting the list, tests newly added get lost in the gcov stdout chatter (leading to even less verification of correctness) and we cannot sensibly order tests to first cover, say, PCI host bridge and then PCI devices depending on it. I already asked you for an alternative way of automatically stripping duplicates without changing their order, to no avail. Or append a new element to a list only if not already present. Entirely untested: $(if $(word $(words $(sort $(list) $(elt)), $(list))), \ $(list), $(list) $(elt))
Re: [Qemu-devel] [RFC PATCH v2 00/16] visitor+BER migration format
Michael S. Tsirkin m...@redhat.com writes: On Thu, Apr 24, 2014 at 09:21:00AM +0100, Dr. David Alan Gilbert wrote: * Markus Armbruster (arm...@redhat.com) wrote: Dr. David Alan Gilbert dgilb...@redhat.com writes: * Eric Blake (ebl...@redhat.com) wrote: On 04/23/2014 10:37 AM, Dr. David Alan Gilbert (git) wrote: From: Dr. David Alan Gilbert dgilb...@redhat.com 4) At the moment you select BER output format by setting an environment variable ( export QEMUMIGFORMAT=BER ) , I need to put more thought in to the right way to do this, there are some harder questions like what happens to devices that are still using pre-vmstate encodings (that are currently sent as blobs) when they eventually convert over and thus how to keep compatibility with earlier BER output versions where they were blobs. I don't have good advice on how to address intra-version design (what happens when an old version of BER sends a blob but a new version on the receiving side expects formatted data instead of a blob), other than it's going to be similar to any other intra-version design that we already have to consider when upgrading from old to new qemu. But for how to select BER format, I _do_ have an idea: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg00782.html Basically, I think that the choice of migration format should be selected via a new extended capability added to migrate-set-capabilities. Setting the choice at the environment variable is too inflexible (it's locked down for the duration of the entire qemu process), whereas setting it via QMP is desirable (for example, it would let us choose at the time of migration whether we are migrating to an older host and want the old format, or migrating to a file for checkpointing reasons and want the new format). Yep, that would certainly be easy to do - and I can do that for the next version. It's more the intra-version I'm worried about, primarily because I don't want to have to wait until every device is vmstate'd before moving this code forward. The one thing that the environment variable does make nice and easy, for dev, is using it with existing test setups - e.g. running virt-test in BER mode or existing mode. Sounds like a useful hack to speed up development, but not so much like a useful permanent API :) Yep, I think what I'll do is go with Eric's suggestion of the migration-capability, but initialise it based on the environment variable; then I can take that out once it all settles out. Dave OK but for a new machine type, let's default to BER, right? I see no reason to keep supporting when non-BER when -M specifies 2.1 compatibility, do you? I fail to see the relation between machine type and migration's wire encoding.
[Qemu-devel] Discussion: xen hvm guest direct kernel boot
Hi, I'm looking at xen hvm guest direct kernel boot and interested to do it. I found there were some discussions about it and an early work around by Daniel (based on xen qemu-dm). [1]http://old-list-archives.xenproject.org/xen-devel/2011-08/msg00414.html [2]http://old-list-archives.xenproject.org/archives/html/xen-devel/2007-12/msg00723.html In xen, the BIOS is provided by hvmloader, which is loaded to RAM in libxc. When hvmloader finishes, it jumps to BIOS entry point. BIOS will probe for MBR from qemu disk, through grub loading kernel and initrd to correct address and then start the guest kernel. In [2], the work around implementation is to pass kernel and initrd to qemu, qemu reads kernel and initrd to certain address and generate a boot sector on the 1st disk for BIOS probing. But in current qemu code, the load_linux() way is different. It reads kernel and initrd to certain address, then put linuxboot.bin and multiboot.bin to option roms which will intercept boot process, so that boot process will jump to linuxboot.bin/multiboot.bin instead of normal int19 probing MBR stage. This sounds much cleaner then generating a boot sector. And in current xen hvmloader, it uses seabios, which is also the one upstream qemu uses. So, back to xen direct kernel boot, I wonder if not generate a boot sector for BIOS probing, could we break hvmloader limitation and borrow qemu load_linux() way to achieve the goal? Then, where is the right place to load the kernel and initrd, any way to intercept boot process? Could we touch hvmloader? My knowledge is limited in this part. Welcome experts' clues or ideas or discussions. Thanks. Regards, Chunyan
Re: [Qemu-devel] [PATCH 0/3] disas/libvixl: update to upstream 1.3
On 24 April 2014 21:11, Peter Maydell peter.mayd...@linaro.org wrote: This patchset updates our copy of libvixl to the upstream 1.3 release. I don't think there's anything particularly earthshattering in 1.3 compared to what we had before. I had a bit of a dilemma with this patchset: * separate out the pristine upstream files from the reapply local fixes commits * retain bisectability of compilation on all hosts I've opted for the former, because I think it will make future upgrades easier -- we can easily see what our local-to-QEMU changes are and reapply them next time round if required. However it does have the disadvantage that between the commits in this series the 32 bit hosts won't compile :-( Possible options: (a) apply these patches, and live with the bisection break on 32 bit hosts (b) squash all these patches together into a single commit, avoiding the bisection break but losing the ability to see the local changes (c) add a patch at the start which nobbles configure to never set CONFIG_ARM_A64_DIS=y, and then another at the end which reenables it Opinions? I think being able to easily distinguish the pristine upstream files from the QEMU versions and point out local changes is absolutely important to maintain some sort of integration between these two open source projects. I can live with (a), but option (c) does seem to be an improvement on that with no additional disadvantages. -Christoffer
Re: [Qemu-devel] [GSoC] Wanted: small warmup tasks
On 2014-04-24 08:19, Jan Kiszka wrote: On 2014-04-23 11:25, Stefan Hajnoczi wrote: Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. To follow up on this: I'm currently looking for optional tiny warmup tasks for our QEMU students during the bonding period (till May 18). If you have any trivial issues or extensions in mind that someone could address within a few days or even hours, that would be perfect. It could even be something like reformat the printing of these messages or so. We used this mechanism last year with the KVM student quite successfully. The idea is to give the student very early a chance to get in contact with the community and with the patch submission review procedure. So the focus is more on dealing with patches than on solving a technical problem in QEMU. If all works fine, this should encourage her/him to work with the community right from the beginning, ask question, post things early etc. Thanks for all these suggestion so far! I personally wasn't able to look into them in details yet. A wish to the mentors and students: if someone picks up a task, please drop a note here so that we can avoid duplicate work! Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] mirror: Check for bdrv_get_info result
bdrv_get_info could fail. Add check before using the returned value.
Signed-off-by: Fam Zheng f...@redhat.com
---
block/mirror.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 0ef41f9..fafcc93 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -339,8 +339,8 @@ static void coroutine_fn mirror_run(void *opaque)
bdrv_get_backing_filename(s-target, backing_filename,
sizeof(backing_filename));
if (backing_filename[0] !s-target-backing_hd) {
-bdrv_get_info(s-target, bdi);
-if (s-granularity bdi.cluster_size) {
+ret = bdrv_get_info(s-target, bdi);
+if (ret == 0 s-granularity bdi.cluster_size) {
s-buf_size = MAX(s-buf_size, bdi.cluster_size);
s-cow_bitmap = bitmap_new(length);
}
--
1.9.2
Re: [Qemu-devel] [PATCH] monitor: fix qmp_getfd() fd leak in error case
Stefan Hajnoczi stefa...@redhat.com writes: qemu_chr_fe_get_msgfd() transfers ownership of the file descriptor to the caller. Therefore all code paths in qmp_getfd() should either register the file descriptor somewhere or close it. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [GSoC] Wanted: small warmup tasks
I'm now looking at the conditional fprintf's. I'll need a bit of help later in sending the patches :D. Marc 2014-04-25 9:24 GMT+02:00 Jan Kiszka jan.kis...@web.de: On 2014-04-24 08:19, Jan Kiszka wrote: On 2014-04-23 11:25, Stefan Hajnoczi wrote: Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. To follow up on this: I'm currently looking for optional tiny warmup tasks for our QEMU students during the bonding period (till May 18). If you have any trivial issues or extensions in mind that someone could address within a few days or even hours, that would be perfect. It could even be something like reformat the printing of these messages or so. We used this mechanism last year with the KVM student quite successfully. The idea is to give the student very early a chance to get in contact with the community and with the patch submission review procedure. So the focus is more on dealing with patches than on solving a technical problem in QEMU. If all works fine, this should encourage her/him to work with the community right from the beginning, ask question, post things early etc. Thanks for all these suggestion so far! I personally wasn't able to look into them in details yet. A wish to the mentors and students: if someone picks up a task, please drop a note here so that we can avoid duplicate work! Jan
[Qemu-devel] [PATCH v2 0/2] qapi: fix coding style in generated code
Not a serious issue, but it's helpful if we can fix it. V2: split change of scripts/qapi-visit.py to a split patch, eat space by using a special char as Markus suggested Amos Kong (2): qapi: fix coding style in parameters list qapi: add a special string in c_type()'s result to each redundant space scripts/qapi-commands.py | 2 +- scripts/qapi-visit.py| 20 ++-- scripts/qapi.py | 11 ++- 3 files changed, 17 insertions(+), 16 deletions(-) -- 1.9.0
[Qemu-devel] [PATCH v2 2/2] qapi: add a special string in c_type()'s result to each redundant space
Currently we always add a space after c_type in mcgen(), there is
some redundant space in generated code. The space isn't needed for
points by the coding style.
char * value;
^
qapi_free_NameInfo(NameInfo * obj)
^
This patch added a special string 'EATSPACE' in the end of c_type()'s
result only for point type. The string and the following space will be
striped in mcgen().
Signed-off-by: Amos Kong ak...@redhat.com
---
scripts/qapi-commands.py | 2 +-
scripts/qapi.py | 11 ++-
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 9734ab0..0ebb1b9 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -26,7 +26,7 @@ def type_visitor(name):
def generate_command_decl(name, args, ret_type):
arglist=
for argname, argtype, optional, structured in parse_args(args):
-argtype = c_type(argtype)
+argtype = c_type(argtype).replace('EATSPACE', '')
if argtype == char *:
argtype = const char *
if optional:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index b474c39..b46c518 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -423,7 +423,7 @@ def is_enum(name):
def c_type(name):
if name == 'str':
-return 'char *'
+return 'char *EATSPACE'
elif name == 'int':
return 'int64_t'
elif (name == 'int8' or name == 'int16' or name == 'int32' or
@@ -437,15 +437,15 @@ def c_type(name):
elif name == 'number':
return 'double'
elif type(name) == list:
-return '%s *' % c_list_type(name[0])
+return '%s *EATSPACE' % c_list_type(name[0])
elif is_enum(name):
return name
elif name == None or len(name) == 0:
return 'void'
elif name == name.upper():
-return '%sEvent *' % camel_case(name)
+return '%sEvent *EATSPACE' % camel_case(name)
else:
-return '%s *' % name
+return '%s *EATSPACE' % name
def genindent(count):
ret =
@@ -470,7 +470,8 @@ def cgen(code, **kwds):
return '\n'.join(lines) % kwds + '\n'
def mcgen(code, **kwds):
-return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
+raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
+return raw.replace('*EATSPACE ', '*')
def basename(filename):
return filename.split(/)[-1]
--
1.9.0
[Qemu-devel] [PATCH v2 1/2] qapi: fix coding style in parameters list
The space before point is redundant.
Signed-off-by: Amos Kong ak...@redhat.com
---
scripts/qapi-visit.py | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 45ce3a9..8ffed3d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -38,7 +38,7 @@ def generate_visit_struct_fields(name, field_prefix,
fn_prefix, members, base =
ret += mcgen('''
-static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error
**errp)
+static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s **obj, Error
**errp)
{
Error *err = NULL;
''',
@@ -149,7 +149,7 @@ def generate_visit_struct(expr):
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
''',
name=name)
@@ -166,7 +166,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const
char *name, Error **
def generate_visit_list(name, members):
return mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp)
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name,
Error **errp)
{
GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
@@ -193,7 +193,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
def generate_visit_enum(name, members):
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error
**errp)
{
visit_type_enum(m, (int *)obj, %(name)s_lookup, %(name)s, name, errp);
}
@@ -203,7 +203,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const
char *name, Error **e
def generate_visit_anon_union(name, members):
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
Error *err = NULL;
@@ -279,7 +279,7 @@ def generate_visit_union(expr):
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
Error *err = NULL;
@@ -367,13 +367,13 @@ def generate_declaration(name, members, genlist=True,
builtin_type=False):
if not builtin_type:
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp);
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp);
''',
name=name)
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp);
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name,
Error **errp);
''',
name=name)
@@ -383,7 +383,7 @@ def generate_enum_declaration(name, members, genlist=True):
ret =
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp);
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name,
Error **errp);
''',
name=name)
@@ -392,7 +392,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
def generate_decl_enum(name, members, genlist=True):
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp);
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error
**errp);
''',
name=name)
--
1.9.0
[Qemu-devel] [PATCH v3 0/4] Fix conversion from ISO to VMDK streamOptimized
Previouly, convert from ISO to VMDK with subformat=streamOptimized fails: $ ./qemu-img convert -O vmdk -o subformat=streamOptimized foo.iso bar.vmdk VMDK: can't write to allocated cluster for streamOptimized qemu-img: error while writing sector 64: Input/output error Because current code in qemu-img.c uses the normal convert loop, rather than the compressed == true loop. In VMDK streamOptimized, writes must be in cluster granularity, because overlapped write to an allocated cluster is -EIO. This series adds is_compressed into BlockDriverInfo, and uses compressed convertion loop if the target block driver sets this field to true. It also implements .bdrv_get_info and .bdrv_write_compressed in VMDK driver to fit into this framework. Adds a test case to cover the code path in question: source image cluster size is smaller. v3: Finally revisit and address Stefan's review comments on v2 from 2 months ago. Sorry for being so slow on this one. The general approach is the same, changes include: - Split originial [PATCH v2 5/5] mirror: Check for bdrv_get_info result and send to list separately, because it's irrelevant with this series. - Don't report cluster_size in bdrv_get_info if it's a flat extent, no need to change type of BlockDriverInfo.cluster_size to 64 bits. So drop [PATCH v2 3/5] block: Change BlockDriverInfo.cluster_size to 64 bits. - Add qemu-iotests case as [PATCH 4/4]. Fam Zheng (4): qemu-img: Convert by cluster size if target is compressed vmdk: Implement .bdrv_write_compressed vmdk: Implement .bdrv_get_info() qemu-iotests: Test converting to streamOptimized from small cluster size block/vmdk.c | 35 +++ include/block/block.h | 4 qemu-img.c | 1 + tests/qemu-iotests/059 | 7 +++ tests/qemu-iotests/059.out | 8 5 files changed, 55 insertions(+) -- 1.9.2
[Qemu-devel] [PATCH v3 3/4] vmdk: Implement .bdrv_get_info()
This will return cluster_size and is_compressed to caller, if all the
extents has the same value (or there's only one extent). Otherwise
return -ENOTSUP.
cluster_size is only reported for sparse formats.
Signed-off-by: Fam Zheng f...@redhat.com
---
block/vmdk.c | 21 +
tests/qemu-iotests/059.out | 1 +
2 files changed, 22 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index ced914b..d2b8894 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2075,6 +2075,26 @@ static ImageInfoSpecific
*vmdk_get_specific_info(BlockDriverState *bs)
return spec_info;
}
+static int vmdk_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+int i;
+BDRVVmdkState *s = bs-opaque;
+assert(s-num_extents);
+bdi-is_compressed = s-extents[0].compressed;
+if (!s-extents[0].flat) {
+bdi-cluster_size = s-extents[0].cluster_sectors BDRV_SECTOR_BITS;
+}
+/* See if we have multiple extents but they have different cases */
+for (i = 1; i s-num_extents; i++) {
+if (bdi-is_compressed != s-extents[i].compressed ||
+(bdi-cluster_size bdi-cluster_size !=
+s-extents[i].cluster_sectors BDRV_SECTOR_BITS)) {
+return -ENOTSUP;
+}
+}
+return 0;
+}
+
static QEMUOptionParameter vmdk_create_options[] = {
{
.name = BLOCK_OPT_SIZE,
@@ -2131,6 +2151,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_has_zero_init = vmdk_has_zero_init,
.bdrv_get_specific_info = vmdk_get_specific_info,
.bdrv_refresh_limits = vmdk_refresh_limits,
+.bdrv_get_info= vmdk_get_info,
.create_options = vmdk_create_options,
};
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 3371c86..14c0957 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2050,6 +2050,7 @@ qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File
truncated, expecting at least
image: TEST_DIR/iotest-version3.IMGFMT
file format: IMGFMT
virtual size: 1.0G (1073741824 bytes)
+cluster_size: 65536
=== Testing 4TB monolithicFlat creation and IO ===
Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=4398046511104
--
1.9.2
[Qemu-devel] [PATCH v3 2/4] vmdk: Implement .bdrv_write_compressed
Add a wrapper function to support compressed path in qemu-img convert.
Only support streamOptimized subformat case for now (num_extents == 1
and extent compression is true).
Signed-off-by: Fam Zheng f...@redhat.com
---
block/vmdk.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index b69988d..ced914b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1495,6 +1495,19 @@ static coroutine_fn int vmdk_co_write(BlockDriverState
*bs, int64_t sector_num,
return ret;
}
+static int vmdk_write_compressed(BlockDriverState *bs,
+ int64_t sector_num,
+ const uint8_t *buf,
+ int nb_sectors)
+{
+BDRVVmdkState *s = bs-opaque;
+if (s-num_extents == 1 s-extents[0].compressed) {
+return vmdk_write(bs, sector_num, buf, nb_sectors, false, false);
+} else {
+return -ENOTSUP;
+}
+}
+
static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors,
@@ -2108,6 +2121,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_reopen_prepare = vmdk_reopen_prepare,
.bdrv_read= vmdk_co_read,
.bdrv_write = vmdk_co_write,
+.bdrv_write_compressed= vmdk_write_compressed,
.bdrv_co_write_zeroes = vmdk_co_write_zeroes,
.bdrv_close = vmdk_close,
.bdrv_create = vmdk_create,
--
1.9.2
[Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test converting to streamOptimized from small cluster size
Signed-off-by: Fam Zheng f...@redhat.com --- tests/qemu-iotests/059 | 7 +++ tests/qemu-iotests/059.out | 7 +++ 2 files changed, 14 insertions(+) diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index ca5aa16..26a2fd3 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -104,6 +104,13 @@ truncate -s 10M $TEST_IMG _img_info echo +echo === Converting to streamOptimized from image with small cluster size=== +TEST_IMG=$TEST_IMG.qcow2 IMGFMT=qcow2 IMGOPTS=cluster_size=4096 _make_test_img 1G +$QEMU_IO -c write -P 0xa 0 512 $TEST_IMG.qcow2 | _filter_qemu_io +$QEMU_IO -c write -P 0xb 10240 512 $TEST_IMG.qcow2 | _filter_qemu_io +$QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized $TEST_IMG.qcow2 $TEST_IMG 21 + +echo echo === Testing version 3 === _use_sample_img iotest-version3.vmdk.bz2 _img_info diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 14c0957..eba0ded 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -2046,6 +2046,13 @@ RW 12582912 VMFS dummy.IMGFMT 1 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400 qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated, expecting at least 13172736 bytes +=== Converting to streamOptimized from image with small cluster size=== +Formatting 'TEST_DIR/t.vmdk.IMGFMT', fmt=IMGFMT size=1073741824 +wrote 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 10240 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + === Testing version 3 === image: TEST_DIR/iotest-version3.IMGFMT file format: IMGFMT -- 1.9.2
[Qemu-devel] [PATCH] improve emulation correctness
There is a set of test, that checks QEMU CPU for similar behavior with real
hardware (http://roberto.greyhats.it/projects/pills.html). Test reg/pill2579.c
can detect, that program is execute in emulated environment. It is related with
behavior of rcl instruction. If the number of shifted bits more than 1, OF of
eflags become undefined. Real CPUs does not change OF, if it is undefined. QEMU
do it anyway.
Emulated program can execute that test and after that can understand
environment not real.
Signed-off-by: Dmitry Poletaev poletaev-q...@yandex.ru
diff --git a/target-i386/shift_helper_template.h
b/target-i386/shift_helper_template.h
index cf91a2d..d5bd321 100644
--- a/target-i386/shift_helper_template.h
+++ b/target-i386/shift_helper_template.h
@@ -64,8 +64,10 @@ target_ulong glue(helper_rcl, SUFFIX)(CPUX86State *env,
target_ulong t0,
}
t0 = res;
env-cc_src = (eflags ~(CC_C | CC_O)) |
-(lshift(src ^ t0, 11 - (DATA_BITS - 1)) CC_O) |
((src (DATA_BITS - count)) CC_C);
+if (count == 1) {
+env-cc_src |= (lshift(src ^ t0, 11 - (DATA_BITS - 1)) CC_O);
+}
}
return t0;
}
This patch improve correctness of emulator behavior.
Re: [Qemu-devel] [PATCH v2 1/2] qapi: fix coding style in parameters list
On Fri, 04/25 15:56, Amos Kong wrote:
The space before point is redundant.
s/point/pointer
Fam
Signed-off-by: Amos Kong ak...@redhat.com
---
scripts/qapi-visit.py | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 45ce3a9..8ffed3d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -38,7 +38,7 @@ def generate_visit_struct_fields(name, field_prefix,
fn_prefix, members, base =
ret += mcgen('''
-static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj,
Error **errp)
+static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s **obj,
Error **errp)
{
Error *err = NULL;
''',
@@ -149,7 +149,7 @@ def generate_visit_struct(expr):
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name,
Error **errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
''',
name=name)
@@ -166,7 +166,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj,
const char *name, Error **
def generate_visit_list(name, members):
return mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp)
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char
*name, Error **errp)
{
GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
@@ -193,7 +193,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
def generate_visit_enum(name, members):
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error
**errp)
{
visit_type_enum(m, (int *)obj, %(name)s_lookup, %(name)s, name, errp);
}
@@ -203,7 +203,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj,
const char *name, Error **e
def generate_visit_anon_union(name, members):
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name,
Error **errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
Error *err = NULL;
@@ -279,7 +279,7 @@ def generate_visit_union(expr):
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name,
Error **errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
Error *err = NULL;
@@ -367,13 +367,13 @@ def generate_declaration(name, members, genlist=True,
builtin_type=False):
if not builtin_type:
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name,
Error **errp);
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp);
''',
name=name)
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp);
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char
*name, Error **errp);
''',
name=name)
@@ -383,7 +383,7 @@ def generate_enum_declaration(name, members,
genlist=True):
ret =
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp);
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char
*name, Error **errp);
''',
name=name)
@@ -392,7 +392,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
def generate_decl_enum(name, members, genlist=True):
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp);
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error
**errp);
''',
name=name)
--
1.9.0
Re: [Qemu-devel] [PATCH 0/3] virtio: Eliminate exit(1) upon invalid request in virtio-blk and virtio-scsi
Am 25.04.2014 um 08:29 hat Markus Armbruster geschrieben: Michael S. Tsirkin m...@redhat.com writes: On Thu, Apr 24, 2014 at 12:43:56PM +0200, Kevin Wolf wrote: Am 24.04.2014 um 09:55 hat Michael S. Tsirkin geschrieben: On Thu, Apr 24, 2014 at 09:15:25AM +0200, Markus Armbruster wrote: If I remember correctly, the DOS involved passthrough of a virtual device to a nested guest or something like that. Guest killing itself is unexciting, nested guest killing its host qualifies as DOS. I guess our current answer to that is don't do that then. Yes. virtio doesn't support that for a variety of other reasons, one of which is that it doesn't go through an mmu. Now, before someone sends a trivial patch converting it to mmu aware calls, that's not yet possible without teaching vhost and dataplane about MMU. Nested virt is really just one example for a userspace virtio driver. Userspace shouldn't be able to kill the whole guest. Kevin Without an MMIO this is fundamentally unavoidable. s/MMIO/IOMMU/, I guess Really? Why is it fundamentally impossible to put the device into an error state when we detect invalid device use by the guest? Honest question; please excuse my ignorance here... I think what Michael means is that without an IOMMU, a buggy or malicious userspace guest driver (which could be a nested VM, in fact) can always kill the guest kernel by DMAing to the right places. This is true, without an IOMMU the protection won't be perfect. But fixing what can easily be fixed is still an improvement and protects at least against some forms of buggy drivers. It doesn't immediately achieve the goal userspace can't kill the guest, but it does bring us closer to it. Kevin
[Qemu-devel] [PATCH v3 1/4] qemu-img: Convert by cluster size if target is compressed
If target block driver forces compression, qemu-img convert needs to
write by cluster size as well as -c option.
Particularly, this applies for converting to VMDK streamOptimized
format.
Signed-off-by: Fam Zheng f...@redhat.com
---
include/block/block.h | 4
qemu-img.c| 1 +
2 files changed, 5 insertions(+)
diff --git a/include/block/block.h b/include/block/block.h
index b3230a2..8a1cb83 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -34,6 +34,10 @@ typedef struct BlockDriverInfo {
* opened with BDRV_O_UNMAP flag for this to work.
*/
bool can_write_zeroes_with_unmap;
+/*
+ * True if the driver is writing data clusters compressed
+ */
+bool is_compressed;
} BlockDriverInfo;
typedef struct BlockFragInfo {
diff --git a/qemu-img.c b/qemu-img.c
index 8455994..ace4c21 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1458,6 +1458,7 @@ static int img_convert(int argc, char **argv)
goto out;
}
} else {
+compress = compress || bdi.is_compressed;
cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
}
--
1.9.2
Re: [Qemu-devel] Monitor Readline - no terminal echo after exit
Mike Day ncm...@ncultra.org writes: On Thu, Apr 24, 2014 at 3:31 AM, Markus Armbruster arm...@redhat.com wrote: I believe someone on the list mentioned they are seeing a couple problems entering and exiting the Monitor. I'd like to look at this more closely, starting with my most pending issue: losing the terminal echo after exiting the Monitor. Thanks for the reply Markus. Reproducer? I've found a couple of ways to reproduce the problem. The easiest is to use -nographic on the qemu command line when starting a qemu session. In this case the monitor opens stdio but there is no visible input or output. -nographic without -nodefaults sets up a default serial line and a monitor, multiplexed onto stdio. Another way to get that is -serial mon:stdio. Maybe the multiplexing has regressed. I'm not sure, as I don't use it myself. Gerd (cc'ed) might know more. You could try old versions to confirm it's a regression, and if it is, bisect it. Another way is to use -nographic along with -mon chardev = stdio,mode=readline. In this case the monitor works, but when you exit from the monitor your terminal will not echo characters. I can't reproduce that problem. Probably because I'm not taking the exact same steps as you do. Common problem with reproducers lacking detail. For reference, here are the chardev and mon options I use: -chardev stdio,id=mon0 -mon chardev=mon0,mode=readline I see that -nographic is a deprecated option, fwiw. Yes, because it does several losely related things, what exactly it does depends on many conditions, and nobody can predict what it does without reading the code (and sometimes not even then). The recommended way to suppress the display is --display none. Then set up monitor and serial to taste with --mon and --chardev. If the default magic gets in the way, whack it with --nodefaults. The monitor runs on top of a QEMU chardev. Suggest to start digging at monitor_init(), both into the monitor itself, and into the CharDriverState object. Thus far I've confirmed that when the -nographic option is passed, the mon_init_func does not get called (as it does for readline mode). I know why this is, but I'm not yet sure the right way to fix it. Also, with -nographic and mon:stdio monitor_flush is called for every line entered execpt for the last line. Normally monitor_flush is called for every line including the last line, at least in readline mode. I've run out of time looking at this today, but would but would be happy if anyone has further ideas. Hope this helps a bit.
Re: [Qemu-devel] [PATCH 2/5] gtk: Fix monitor greeting
Hi, Apparently requesting the vte terminal size isn't sufficient, we need to force a size_request so text doesn't line wrap. We use slightly different APIs for gtk3, since on 3.10 the size_request trick doesn't seem to work. Something is fishy there indeed. I somehow feel like we are doing something wrong somewhere else though. vte_terminal_set_size() should be enough, the parent widgets should respect the request ... vte widget placing looks odd too. First text line is missing, and I have a white bar at the bottom, looks like the vte widget is misplaced. Maybe those two isses are related ... cheers, Gerd
Re: [Qemu-devel] [PATCH 3/5] gtk: Fix -serial vc
On Do, 2014-04-24 at 13:35 -0400, Cole Robinson wrote: Drop use of a pty and just use vte infrastructure for reading and writing. Makes perfect sense. pty is poinless in our vte usage szenario. cheers, Gerd
[Qemu-devel] [PATCH 2/3] char: Clean up fragile use of error_is_set()
Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP
may be null, because errors go undetected when it is. It's fragile
when proving ERRP non-null involves a non-local argument. Else, it's
unnecessarily opaque (see commit 84d18f0).
The error_is_set(errp) in qemu_chr_new_from_opts() is merely fragile,
because the callers never pass a null errp argument.
Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
qemu-char.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 3eaefc9..5a7975f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3204,6 +3204,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
void (*init)(struct CharDriverState *s),
Error **errp)
{
+Error *local_err = NULL;
CharDriver *cd;
CharDriverState *chr;
GSList *i;
@@ -3245,8 +3246,9 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
chr = NULL;
backend-kind = cd-kind;
if (cd-parse) {
-cd-parse(opts, backend, errp);
-if (error_is_set(errp)) {
+cd-parse(opts, backend, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
goto qapi_out;
}
}
--
1.8.1.4
[Qemu-devel] [PATCH 1/3] char: Use return values instead of error_is_set(errp)
Using error_is_set(errp) to check whether a function call failed is
fragile: it breaks when errp is null. Check perfectly suitable return
values instead when possible. As far as I can tell, errp can't be
null there, but this is more robust and more obviously correct
Signed-off-by: Markus Armbruster arm...@redhat.com
---
qemu-char.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 54ed244..3eaefc9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3251,7 +3251,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
}
}
ret = qmp_chardev_add(bid ? bid : id, backend, errp);
-if (error_is_set(errp)) {
+if (!ret) {
goto qapi_out;
}
@@ -3263,7 +3263,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
backend-kind = CHARDEV_BACKEND_KIND_MUX;
backend-mux-chardev = g_strdup(bid);
ret = qmp_chardev_add(id, backend, errp);
-if (error_is_set(errp)) {
+if (!ret) {
chr = qemu_chr_find(bid);
qemu_chr_delete(chr);
chr = NULL;
@@ -3620,18 +3620,18 @@ static int qmp_chardev_open_file_source(char *src, int
flags,
static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
{
-int flags, in = -1, out = -1;
+int flags, in = -1, out;
flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY;
out = qmp_chardev_open_file_source(file-out, flags, errp);
-if (error_is_set(errp)) {
+if (out 0) {
return NULL;
}
if (file-has_in) {
flags = O_RDONLY;
in = qmp_chardev_open_file_source(file-in, flags, errp);
-if (error_is_set(errp)) {
+if (in 0) {
qemu_close(out);
return NULL;
}
@@ -3647,7 +3647,7 @@ static CharDriverState
*qmp_chardev_open_serial(ChardevHostdev *serial,
int fd;
fd = qmp_chardev_open_file_source(serial-device, O_RDWR, errp);
-if (error_is_set(errp)) {
+if (fd 0) {
return NULL;
}
qemu_set_nonblock(fd);
@@ -3665,7 +3665,7 @@ static CharDriverState
*qmp_chardev_open_parallel(ChardevHostdev *parallel,
int fd;
fd = qmp_chardev_open_file_source(parallel-device, O_RDWR, errp);
-if (error_is_set(errp)) {
+if (fd 0) {
return NULL;
}
return qemu_chr_open_pp_fd(fd);
@@ -3692,7 +3692,7 @@ static CharDriverState
*qmp_chardev_open_socket(ChardevSocket *sock,
} else {
fd = socket_connect(addr, errp, NULL, NULL);
}
-if (error_is_set(errp)) {
+if (fd 0) {
return NULL;
}
return qemu_chr_open_socket_fd(fd, do_nodelay, is_listen,
@@ -3705,7 +3705,7 @@ static CharDriverState *qmp_chardev_open_udp(ChardevUdp
*udp,
int fd;
fd = socket_dgram(udp-remote, udp-local, errp);
-if (error_is_set(errp)) {
+if (fd 0) {
return NULL;
}
return qemu_chr_open_udp_fd(fd);
--
1.8.1.4
[Qemu-devel] [PATCH 0/3] char: Purge error_is_set()
I got a private branch getting rid of it entirely. This is the third part, covering character backends. Markus Armbruster (3): char: Use return values instead of error_is_set(errp) char: Clean up fragile use of error_is_set() char: Explain qmp_chardev_add()'s unusual error handling qemu-char.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH 3/3] char: Explain qmp_chardev_add()'s unusual error handling
Character backend open hasn't been fully converted to the Error API.
Some opens fail without setting an error. qmp_chardev_add() needs to
detect when that happens, and set a generic error. Explain that in a
comment, and inline error_is_set() for clarity.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
qemu-char.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/qemu-char.c b/qemu-char.c
index 5a7975f..17b476e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3798,7 +3798,13 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
break;
}
-if (chr == NULL !error_is_set(errp)) {
+/*
+ * Character backend open hasn't been fully converted to the Error
+ * API. Some opens fail without setting an error. Set a generic
+ * error then.
+ * TODO full conversion to Error API
+ */
+if (chr == NULL errp !*errp) {
error_setg(errp, Failed to create chardev);
}
if (chr) {
--
1.8.1.4
[Qemu-devel] [PATCH] migration: remove duplicate code
From: ChenLiang chenlian...@huawei.com
version_id is checked twice in the ram_load.
Signed-off-by: ChenLiang chenlian...@huawei.com
Signed-off-by: Gonglei arei.gong...@huawei.com
---
arch_init.c | 68 ++---
1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 60c975d..85c6d6e 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -997,7 +997,7 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
seq_iter++;
-if (version_id 4 || version_id 4) {
+if (version_id != 4) {
return -EINVAL;
}
@@ -1008,44 +1008,42 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
addr = TARGET_PAGE_MASK;
if (flags RAM_SAVE_FLAG_MEM_SIZE) {
-if (version_id == 4) {
-/* Synchronize RAM block list */
-char id[256];
-ram_addr_t length;
-ram_addr_t total_ram_bytes = addr;
-
-while (total_ram_bytes) {
-RAMBlock *block;
-uint8_t len;
-
-len = qemu_get_byte(f);
-qemu_get_buffer(f, (uint8_t *)id, len);
-id[len] = 0;
-length = qemu_get_be64(f);
-
-QTAILQ_FOREACH(block, ram_list.blocks, next) {
-if (!strncmp(id, block-idstr, sizeof(id))) {
-if (block-length != length) {
-fprintf(stderr,
-Length mismatch: %s: RAM_ADDR_FMT
- in != RAM_ADDR_FMT \n, id,
length,
-block-length);
-ret = -EINVAL;
-goto done;
-}
-break;
+/* Synchronize RAM block list */
+char id[256];
+ram_addr_t length;
+ram_addr_t total_ram_bytes = addr;
+
+while (total_ram_bytes) {
+RAMBlock *block;
+uint8_t len;
+
+len = qemu_get_byte(f);
+qemu_get_buffer(f, (uint8_t *)id, len);
+id[len] = 0;
+length = qemu_get_be64(f);
+
+QTAILQ_FOREACH(block, ram_list.blocks, next) {
+if (!strncmp(id, block-idstr, sizeof(id))) {
+if (block-length != length) {
+fprintf(stderr,
+Length mismatch: %s: RAM_ADDR_FMT
+ in != RAM_ADDR_FMT \n, id, length,
+block-length);
+ret = -EINVAL;
+goto done;
}
+break;
}
+}
-if (!block) {
-fprintf(stderr, Unknown ramblock \%s\, cannot
-accept migration\n, id);
-ret = -EINVAL;
-goto done;
-}
-
-total_ram_bytes -= length;
+if (!block) {
+fprintf(stderr, Unknown ramblock \%s\, cannot
+accept migration\n, id);
+ret = -EINVAL;
+goto done;
}
+
+total_ram_bytes -= length;
}
}
--
1.7.12.4
Re: [Qemu-devel] [RFC 4/4] MemoryRegion with EOI callbacks for VFIO Platform devices
Hi Eric, Thank you for reviewing it. On 23/04/2014 17:00, Eric Auger wrote: Hi Alvise, Thank you for the patch. Indeed I am very interested in further discussing the vfio-platform integration with you. On 04/17/2014 07:29 PM, Alvise Rigo wrote: The user can specify the location of the memory region (register) used by the guest driver to clear the pending interrupt; if enable, this mechanism will substitute the default one (timer based). actually the current mechanism is that any read/write access to the device memory region (currently #0) after an IRQ hit is trapped and interpreted as an attempt of the guest driver to reset the interrupt status register. This is quite coarse! To me the timer mechanism is another thing that makes possible to turn off this trapping after a while if no IRQ is pending anymore. The region is provided as command line property intclr-region of the vfio-platform device. The property is a string region_index;offset;size where: region_index: is the index of the memory region where the register lives, offset: offset of the register in the region, size: size of the register. example: -device vfio-platform,...,intclr-region=0;0x2c;4 I fully understand the rationale behind reducing the memory region that is trapped for detecting device IRQ status reset. However the code as of today does not always reduce the trapped region size. Let me share with you my understanding: given the granularity of the MMU settings (4kB or 64kB granule) it is not possible to trap just a register. In practice defining such IO region will force a whole page to be unmapped from stage2. and both Midway xgmac and PL330 node region is 4kB. This means that when providing such option, this is the whole register space (for those IPs) that is ALWAYS unmapped from stage2 point of view. This means upon guest access KVM_RUN exists and handle_mmio is entered. Then in cpu_physical_memory_rw, address_space_rw, memory_access_is_direct checks whether the memory region is IO or RAM leading either to io_mem_read/write (calling QEMU device callbacks) or qemu_get_ram_ptr and memcpy (using direct host access). You are right, this solution will cause the guest to trap every time it will access the device memory: I was not considering the MMU stage2 unmapping behaviour. However, if we keep the time mechanism together with the interrupt clear memory region, some good things could happen: in a scenario with high throughput of interrupts, such a solution could definitively be an improvement, allowing us to reflect the behaviour of the real interrupt with the emulated one. Moreover, in case of a high throughput of IRQ/s and accesses as well, at every access the callback will not check every IRQ/s to see if it's pending, saving a lot of work. On the contrary, in a scenario where there is a mild traffic of interrupts, we should try to keep the mmap region as more as possible like you suggested, relaying however on the interrupt clear memory region to trap only the access that is in charge to disable the interrupt. In fact, what I noticed with the DMA330 is that the kernel driver was accessing two registers of the device during the timer window before actually writing to the interrupt clear register: this means that the EOI is made at the first access and not a the third. In the first scenario the timer could play a fundamental role, which is to define the temporal window where interrupts could possibly hit again and renew the timer (in order to avoid subsequent enable/disable of the mmap memory region). Is then needed a timer for each IRQ? I will update the patch with these last ideas to have in case a base for further discussion. Best regards, alvise My conclusion is your patch improves the precision of the EOI however it may decrease the perf in case where the guest could benefit from a direct nested stage1 + stage2 access to the HW device memory. This is where the timer mechanism can play a role, restoring the mmap after a while. So to me your patch is not antagonist with timer mechanism and you should restore it. Besides, as documented in PCI code also, in case IPs are generating a very high throughput of IRQ/s, the mmap region is not so much helpful. However I guess that for less intensive IRQ IPs, the mmap region definitively brings all its value and shall remain. Let me know if you share the same understanding. A last comment on the option passing. Philosophy of the previous patch was to hide much of the complexity to the end-user. Passing the IRQ status register address and size requires some knowledge from the end-user. I don't know what will be the community drift. There is a separate thread related to vl.c: Enable adding devices to the system bus patch, https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg01797.html. Best Regards Eric Signed-off-by: Alvise Rigo
Re: [Qemu-devel] [PATCH 4/5] gtk: Fix zoom in accelerator
On Do, 2014-04-24 at 13:35 -0400, Cole Robinson wrote: The accelerator was ctrl+shift+'+', but '+' required a shift key already, so the accelerator didn't trigger. Switch it to ctrl+shift+'=' Hmm? For me the accelerator is ctrl+alt. Also which keys need shift and which don't depends on the keyboard layout. My german keyboard has a unshifted '+' ... cheers, Gerd
Re: [Qemu-devel] [PATCH 5/5] gtk: Fix accelerators being triggered twice with gtk3
On Do, 2014-04-24 at 13:35 -0400, Cole Robinson wrote: When keyboard focus is grabbed, current qemu wants to pass every keypress to the VM, unless the user is pressing a UI accelerator. That's exactly how things work without any of the fancy handling. Drop the special handling, which seems to trigger accelerators twice on gtk3. Is this tested on gtk2? cheers, Gerd
Re: [Qemu-devel] [RFC 2/4] Add EXEC_FLAG to VFIO DMA mappings
On 24/04/2014 02:25, Peter Crosthwaite wrote:
On Fri, Apr 18, 2014 at 3:29 AM, Alvise Rigo
a.r...@virtualopensystems.com wrote:
The flag is mandatory for the ARM SMMU, add it.
When VFIO will be able to tell about the IOMMU being used, we will add
it only if necessary.
Signed-off-by: Alvise Rigo a.r...@virtualopensystems.com
---
hw/vfio/common.c | 3 +++
linux-headers/linux/vfio.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 9d1f723..f4c1fec 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -107,6 +107,9 @@ static int vfio_dma_map(VFIOContainer *container, hwaddr
iova,
map.flags |= VFIO_DMA_MAP_FLAG_WRITE;
}
+/* add exec flag */
+map.flags |= VFIO_DMA_MAP_FLAG_WRITE;
+
Did you mean FLAG_EXEC?
Definitively yes, the flag is VFIO_DMA_MAP_FLAG_EXEC.
I will fix this in the next version.
Thank you,
alvise
Regards,
Peter
/*
* Try the mapping, if it fails with EBUSY, unmap the region and try
* again. This shouldn't be necessary, but we sometimes see it in
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 17c58e0..e0b404e 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -392,6 +392,7 @@ struct vfio_iommu_type1_dma_map {
__u32 flags;
#define VFIO_DMA_MAP_FLAG_READ (1 0)/* readable from
device */
#define VFIO_DMA_MAP_FLAG_WRITE (1 1) /* writable from device */
+#define VFIO_DMA_MAP_FLAG_EXEC (1 2)/* executable from device */
__u64 vaddr; /* Process virtual address */
__u64 iova; /* IO virtual address */
__u64 size; /* Size of mapping (bytes) */
--
1.8.3.2
Re: [Qemu-devel] [PATCH 0/5] gtk: Misc fixes
On Do, 2014-04-24 at 13:35 -0400, Cole Robinson wrote: A collection of fixes related to the gtk UI. See individual patches for details. cherry-picked #1 + #3 for now. FYI: current gtk patch queue looks like this: https://www.kraxel.org/cgit/qemu/log/?h=rebase/ui-gtk-next cheers, Gerd
Re: [Qemu-devel] [RFC 0/4] AMBA platform device passthrough
Il 24/04/2014 02:16, Peter Crosthwaite ha scritto: On Thu, Apr 24, 2014 at 1:21 AM, Eric Auger eric.au...@linaro.org wrote: On 04/17/2014 07:29 PM, Alvise Rigo wrote: These patches were born after trying the current work on device passthrough (see thread [Qemu-devel] [RFC v2 0/6] KVM platform device passthrough) with a DMA330 and FastModels. This work want to be the starting point for dicussion on some aspects related to the next VFIO platform development: - [PATCH 1/4] Add a way to allocate memory region from a user pointer for devices, and not exclusively for RAM. This is useful to exclude the devices bound to VFIO from being DMA mapped by the VFIO memory listener. - [PATCH 2/4] Addition of the exec flag to the DMA mappings. This is mandatory only for the ARM SMMU: in its next version VFIO will be able to tell if the flag is supported. - [PATCH 3/4] Possibility to bind a wider class of devices. In particular the patch proposes a solution to enhance a bit the device tree nodes generation, allowing to specify more than one compatibility property (handy for AMBA devices). I Alvise, This improvement was indeed needed. Do you want me to integrate that one on next RFC version or do you maintain it on your side? I think its probably best taken as a squash to your series. Agreed. Regards, alvise Regards, Peter This was required by the DMA330 that needs “arm,pl330”,”arm,primecell” as compatibility string, together with a clock source defined in the device tree. In the future VFIO will be able to tell if we are dealing with an AMBA device; Any idea of how you want to achieve this? Is it something that can be generalized to other devices? before that, we have to rely on the compatibility string to state that. - [PATCH 4/4] Another approach to handle the EOI, starting from the assumption that we know in advance the location of the interrupt clear register of the device. Testing the reference patches I was noticing that the EOI mechanism was disabling the interrupt three times, because the kernel driver was reading some registers in the timer window before actually clearing the interrupt. can you elaborate on the last sentence and especially EOI mechanism was disabling the interrupt 3 times. I do not get what you mean here. Best Regards Eric This is of course another workaround and not the definitive solution until we can rely on a proper callback from the VGIC (something that we will see in a future version of VFIO for platform devices). These patches are based on the QEMU branch mentioned in the original thread ([Qemu-devel] [RFC v2 0/6] KVM platform device passthrough). Alvise Rigo (4): Allocate non-RAM MemoryRegion from user pointer Add EXEC_FLAG to VFIO DMA mappings Add AMBA devices support to VFIO MemoryRegion with EOI callbacks for VFIO Platform devices exec.c | 2 +- hw/arm/virt.c | 39 +-- hw/vfio/common.c | 3 + hw/vfio/platform.c | 158 ++--- include/exec/memory.h | 16 + linux-headers/linux/vfio.h | 1 + memory.c | 14 memory_mapping.c | 2 +- 8 files changed, 220 insertions(+), 15 deletions(-)
Re: [Qemu-devel] Monitor Readline - no terminal echo after exit
Hi, -nographic without -nodefaults sets up a default serial line and a monitor, multiplexed onto stdio. Another way to get that is -serial mon:stdio. Maybe the multiplexing has regressed. I'm not sure, as I don't use it myself. Gerd (cc'ed) might know more. Works for me. Serial line is active by default. 'Ctrl-a c' switches from serial to monitor (and prints the monitor greeting). Same key sequence again switches back. 'Ctrl-a h' prints the chardev mux hotkeys. cheers, Gerd
Re: [Qemu-devel] [GSoC] Wanted: small warmup tasks
On Fri, Apr 25, 2014 at 5:55 PM, Marc Marí marc.mari.barc...@gmail.com wrote: I'm now looking at the conditional fprintf's. I'll need a bit of help later in sending the patches :D. CC me in on the results. Regards, Peter Marc 2014-04-25 9:24 GMT+02:00 Jan Kiszka jan.kis...@web.de: On 2014-04-24 08:19, Jan Kiszka wrote: On 2014-04-23 11:25, Stefan Hajnoczi wrote: Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. To follow up on this: I'm currently looking for optional tiny warmup tasks for our QEMU students during the bonding period (till May 18). If you have any trivial issues or extensions in mind that someone could address within a few days or even hours, that would be perfect. It could even be something like reformat the printing of these messages or so. We used this mechanism last year with the KVM student quite successfully. The idea is to give the student very early a chance to get in contact with the community and with the patch submission review procedure. So the focus is more on dealing with patches than on solving a technical problem in QEMU. If all works fine, this should encourage her/him to work with the community right from the beginning, ask question, post things early etc. Thanks for all these suggestion so far! I personally wasn't able to look into them in details yet. A wish to the mentors and students: if someone picks up a task, please drop a note here so that we can avoid duplicate work! Jan
Re: [Qemu-devel] [RFC 0/4] AMBA platform device passthrough
On 23/04/2014 17:21, Eric Auger wrote: On 04/17/2014 07:29 PM, Alvise Rigo wrote: These patches were born after trying the current work on device passthrough (see thread [Qemu-devel] [RFC v2 0/6] KVM platform device passthrough) with a DMA330 and FastModels. This work want to be the starting point for dicussion on some aspects related to the next VFIO platform development: - [PATCH 1/4] Add a way to allocate memory region from a user pointer for devices, and not exclusively for RAM. This is useful to exclude the devices bound to VFIO from being DMA mapped by the VFIO memory listener. - [PATCH 2/4] Addition of the exec flag to the DMA mappings. This is mandatory only for the ARM SMMU: in its next version VFIO will be able to tell if the flag is supported. - [PATCH 3/4] Possibility to bind a wider class of devices. In particular the patch proposes a solution to enhance a bit the device tree nodes generation, allowing to specify more than one compatibility property (handy for AMBA devices). I Alvise, This improvement was indeed needed. Do you want me to integrate that one on next RFC version or do you maintain it on your side? This was required by the DMA330 that needs “arm,pl330”,”arm,primecell” as compatibility string, together with a clock source defined in the device tree. In the future VFIO will be able to tell if we are dealing with an AMBA device; Any idea of how you want to achieve this? The VFIO API at some point will be able to tell if we are dealing with an AMBA device. In general these devices still need some work from the VFIO API side to be properly supported. Is it something that can be generalized to other devices? Yes, it will be a matter of checking the flags returned by the VFIO API. before that, we have to rely on the compatibility string to state that. - [PATCH 4/4] Another approach to handle the EOI, starting from the assumption that we know in advance the location of the interrupt clear register of the device. Testing the reference patches I was noticing that the EOI mechanism was disabling the interrupt three times, because the kernel driver was reading some registers in the timer window before actually clearing the interrupt. can you elaborate on the last sentence and especially EOI mechanism was disabling the interrupt 3 times. I do not get what you mean here. I should have said that after clearing the interrupt, there are two more accesses in the timer window that check for a pending interrupt. More on this in the [RFC 4/4] thread. Best regards, alvise Best Regards Eric This is of course another workaround and not the definitive solution until we can rely on a proper callback from the VGIC (something that we will see in a future version of VFIO for platform devices). These patches are based on the QEMU branch mentioned in the original thread ([Qemu-devel] [RFC v2 0/6] KVM platform device passthrough). Alvise Rigo (4): Allocate non-RAM MemoryRegion from user pointer Add EXEC_FLAG to VFIO DMA mappings Add AMBA devices support to VFIO MemoryRegion with EOI callbacks for VFIO Platform devices exec.c | 2 +- hw/arm/virt.c | 39 +-- hw/vfio/common.c | 3 + hw/vfio/platform.c | 158 ++--- include/exec/memory.h | 16 + linux-headers/linux/vfio.h | 1 + memory.c | 14 memory_mapping.c | 2 +- 8 files changed, 220 insertions(+), 15 deletions(-)
Re: [Qemu-devel] [PATCH for-2.1 v1 2/3] qdev: Implement named GPIOs
On Fri, Apr 25, 2014 at 12:02 AM, Andreas Färber afaer...@suse.de wrote: Am 09.04.2014 01:45, schrieb Peter Crosthwaite: Implement named GPIOs on the Device layer. Listifies the existing GPIOs stuff using string keys. Legacy un-named GPIOs are preserved by using a NULL name string - they are just a single matchable element in the name list. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/core/qdev.c | 82 ++ include/hw/qdev-core.h | 25 --- qdev-monitor.c | 14 ++--- qtest.c| 15 ++--- 4 files changed, 110 insertions(+), 26 deletions(-) Please don't roll your own list implementation, use qemu/queue.h. Fair enough. Will respin. While we are here, what do you think of the idea of using this to get rid of sysbus IRQs completely? Just make all existing sysbus IRQs back onto a fixed-name named GPIO and remove the qemu_irq stuffs from the state struct completely? (one step closer to sysbus dismantlement). Regards, Peter Regards, Andreas -- 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] [GSoC] Wanted: small warmup tasks
On Thu, Apr 24, 2014 at 08:19:19AM +0200, Jan Kiszka wrote: On 2014-04-23 11:25, Stefan Hajnoczi wrote: Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. To follow up on this: I'm currently looking for optional tiny warmup tasks for our QEMU students during the bonding period (till May 18). If you have any trivial issues or extensions in mind that someone could address within a few days or even hours, that would be perfect. It could even be something like reformat the printing of these messages or so. We used this mechanism last year with the KVM student quite successfully. The idea is to give the student very early a chance to get in contact with the community and with the patch submission review procedure. So the focus is more on dealing with patches than on solving a technical problem in QEMU. If all works fine, this should encourage her/him to work with the community right from the beginning, ask question, post things early etc. Currently QEMU uses a handful of GCC warning flags: -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits There are a lot more gcc warning flags that are potentially useful to QEMU. For example libvirt builds with this (possibly insanely) large set: -W -Waddress -Waggressive-loop-optimizations -Wall -Warray-bounds -Wattributes -Wbad-function-cast -Wbuiltin-macro-redefined -Wcast-align -Wchar-subscripts -Wclobbered -Wcomment -Wcomments -Wcoverage-mismatch -Wcpp -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wdouble-promotion -Wempty-body -Wendif-labels -Wextra -Wformat-contains-nul -Wformat-extra-args -Wformat-security -Wformat-y2k -Wformat-zero-length -Wfree-nonheap-object -Wignored-qualifiers -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Winit-self -Winline -Wint-to-pointer-cast -Winvalid-memory-model -Winvalid-pch -Wjump-misses-init -Wlogical-op -Wmain -Wmaybe-uninitialized -Wmissing-braces -Wmissing-declarations -Wmissing-field-initializers -Wmissing-include-dirs -Wmissing-parameter-type -Wmissing-prototypes -Wmultichar -Wnarrowing -Wnested-externs -Wnonnull -Wnormalized=nfc -Wold-style-declaration -Wold-style-definition -Woverflow -Woverride-init -Wpacked-bitfield-compat -Wparentheses -Wpointer-arith -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wreturn-local-addr -Wreturn-type -Wsequence-point -Wshadow -Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes -Wsuggest-attribute=const -Wsuggest-attribute=format -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wswitch -Wsync-nand -Wtrampolines -Wtrigraphs -Wtype-limits -Wuninitialized -Wunknown-pragmas -Wunused -Wunused-but-set-parameter -Wunused-but-set-variable -Wunused-function -Wunused-label -Wunused-local-typedefs -Wunused-parameter -Wunused-result -Wunused-value -Wunused-variable -Wvarargs -Wvariadic-macros -Wvector-operation-performance -Wvolatile-register-var -Wwrite-strings -Wno-sign-compare -Wjump-misses-init -Wno-format-nonliteral -Wframe-larger-than=4096 -Wno-suggest-attribute=pure -Wno-suggest-attribute=const I've previously submitted a patch series which attempted to enable some more of these for QEMU, but I never got time to address the feedback I received on it. Might be something someone can pick up from where I left off... http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg00085.html Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 0/3] hw/arm/virt: Support Cortex-A57
On Thu, Apr 24, 2014 at 06:54:40PM +0100, Peter Maydell wrote: This patchset wires up our new Cortex-A57 emulation into the virt machine model. Rather than the somewhat hacky approach in the previous system emulation patchsets, I've decided that our best approach is to have the board model create the GIC itself. This essentially corresponds to modelling a board with a standalone GIC -- you can see this in some real hardware which uses a GIC-400. I think this makes more sense than either borrowing the a15mpcore_priv device, or creating an a57mpcore_priv device (since the A57 does not actually have a built-in GICv2). So we're adding an incompletely modelled A57 to the virt board, and adding a board mounted GICv2. Doesn't sound like something that could ever exist in real life, but I agree that the way we are tying things together, semantically letting the board decide how to place the GIC makes the most sense. Oh well. Let's implement GICv3 support some time. -Christoffer
Re: [Qemu-devel] [PULL v2 2/2] usb: mtp filesharing
Hi, +trace_usb_mtp_op_get_device_info(s-dev.addr); + +usb_mtp_add_u16(d, 0x0100); Sect. 5.1.1.1 says: This identifies the PTP version this device can support in hundredths. For MTP devices implemented under this specification, this shall contain the value 100 (representing 1.00). Is it an error in the spec (missing 0x) or should the value here really be 0x0100 instead of 0x0064? It's probably /me not reading careful enough and assuming bcd version encoding which is frequently used elsewhere in usb ... Thanks for the careful review, as the patch got merged I'll go send out a series of incremental fixes. cheers, Gerd
[Qemu-devel] [PATCH 1/4] hw: Consistently name Error * objects err, and not errp
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/core/qdev-properties-system.c | 10 +-
hw/dma/xilinx_axidma.c | 16
hw/net/xilinx_axienet.c | 16
3 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index de83561..404cf18 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -338,13 +338,13 @@ PropertyInfo qdev_prop_vlan = {
int qdev_prop_set_drive(DeviceState *dev, const char *name,
BlockDriverState *value)
{
-Error *errp = NULL;
+Error *err = NULL;
const char *bdrv_name = value ? bdrv_get_device_name(value) : ;
object_property_set_str(OBJECT(dev), bdrv_name,
-name, errp);
-if (errp) {
-qerror_report_err(errp);
-error_free(errp);
+name, err);
+if (err) {
+qerror_report_err(err);
+error_free(err);
return -1;
}
return 0;
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 14b887b..cc90eb5 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -534,24 +534,24 @@ static void xilinx_axidma_realize(DeviceState *dev, Error
**errp)
XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(s-rx_data_dev);
XilinxAXIDMAStreamSlave *cs = XILINX_AXI_DMA_CONTROL_STREAM(
s-rx_control_dev);
-Error *local_errp = NULL;
+Error *local_err = NULL;
object_property_add_link(OBJECT(ds), dma, TYPE_XILINX_AXI_DMA,
(Object **)ds-dma,
object_property_allow_set_link,
OBJ_PROP_LINK_UNREF_ON_RELEASE,
- local_errp);
+ local_err);
object_property_add_link(OBJECT(cs), dma, TYPE_XILINX_AXI_DMA,
(Object **)cs-dma,
object_property_allow_set_link,
OBJ_PROP_LINK_UNREF_ON_RELEASE,
- local_errp);
-if (local_errp) {
+ local_err);
+if (local_err) {
goto xilinx_axidma_realize_fail;
}
-object_property_set_link(OBJECT(ds), OBJECT(s), dma, local_errp);
-object_property_set_link(OBJECT(cs), OBJECT(s), dma, local_errp);
-if (local_errp) {
+object_property_set_link(OBJECT(ds), OBJECT(s), dma, local_err);
+object_property_set_link(OBJECT(cs), OBJECT(s), dma, local_err);
+if (local_err) {
goto xilinx_axidma_realize_fail;
}
@@ -567,7 +567,7 @@ static void xilinx_axidma_realize(DeviceState *dev, Error
**errp)
xilinx_axidma_realize_fail:
if (!*errp) {
-*errp = local_errp;
+*errp = local_err;
}
}
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 839d97c..c24e38c 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -942,24 +942,24 @@ static void xilinx_enet_realize(DeviceState *dev, Error
**errp)
XilinxAXIEnetStreamSlave *ds =
XILINX_AXI_ENET_DATA_STREAM(s-rx_data_dev);
XilinxAXIEnetStreamSlave *cs = XILINX_AXI_ENET_CONTROL_STREAM(
s-rx_control_dev);
-Error *local_errp = NULL;
+Error *local_err = NULL;
object_property_add_link(OBJECT(ds), enet, xlnx.axi-ethernet,
(Object **) ds-enet,
object_property_allow_set_link,
OBJ_PROP_LINK_UNREF_ON_RELEASE,
- local_errp);
+ local_err);
object_property_add_link(OBJECT(cs), enet, xlnx.axi-ethernet,
(Object **) cs-enet,
object_property_allow_set_link,
OBJ_PROP_LINK_UNREF_ON_RELEASE,
- local_errp);
-if (local_errp) {
+ local_err);
+if (local_err) {
goto xilinx_enet_realize_fail;
}
-object_property_set_link(OBJECT(ds), OBJECT(s), enet, local_errp);
-object_property_set_link(OBJECT(cs), OBJECT(s), enet, local_errp);
-if (local_errp) {
+object_property_set_link(OBJECT(ds), OBJECT(s), enet, local_err);
+object_property_set_link(OBJECT(cs), OBJECT(s), enet, local_err);
+if (local_err) {
goto xilinx_enet_realize_fail;
}
@@ -978,7 +978,7 @@ static void xilinx_enet_realize(DeviceState *dev, Error
**errp)
xilinx_enet_realize_fail:
if (!*errp) {
-*errp = local_errp;
+*errp = local_err;
}
}
--
1.8.1.4
[Qemu-devel] [PATCH 4/4] arm: Clean up fragile use of error_is_set() in realize() methods
Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP
may be null, because errors go undetected when it is. It's fragile
when proving ERRP non-null involves a non-local argument. Else, it's
unnecessarily opaque (see commit 84d18f0).
I guess the error_is_set(errp) in the DeviceClass realize() methods
are merely fragile right now, because I can't find a call chain that
passes a null errp argument.
Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/intc/arm_gic.c | 6 --
hw/intc/arm_gic_kvm.c | 6 --
hw/intc/armv7m_nvic.c | 6 --
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 955b8d4..f6c7144 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -793,13 +793,15 @@ void gic_init_irqs_and_distributor(GICState *s, int
num_irq)
static void arm_gic_realize(DeviceState *dev, Error **errp)
{
/* Device instance realize function for the GIC sysbus device */
+Error *local_err = NULL;
int i;
GICState *s = ARM_GIC(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
ARMGICClass *agc = ARM_GIC_GET_CLASS(s);
-agc-parent_realize(dev, errp);
-if (error_is_set(errp)) {
+agc-parent_realize(dev, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
return;
}
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 719d227..35d79a6 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -513,14 +513,16 @@ static void kvm_arm_gic_reset(DeviceState *dev)
static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
{
+Error *local_err = NULL;
int i;
GICState *s = KVM_ARM_GIC(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
KVMARMGICClass *kgc = KVM_ARM_GIC_GET_CLASS(s);
int ret;
-kgc-parent_realize(dev, errp);
-if (error_is_set(errp)) {
+kgc-parent_realize(dev, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
return;
}
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 6066fa6..0d79def 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -472,6 +472,7 @@ static void armv7m_nvic_reset(DeviceState *dev)
static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
{
+Error *local_err = NULL;
nvic_state *s = NVIC(dev);
NVICClass *nc = NVIC_GET_CLASS(s);
@@ -480,8 +481,9 @@ static void armv7m_nvic_realize(DeviceState *dev, Error
**errp)
/* Tell the common code we're an NVIC */
s-gic.revision = 0x;
s-num_irq = s-gic.num_irq;
-nc-parent_realize(dev, errp);
-if (error_is_set(errp)) {
+nc-parent_realize(dev, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
return;
}
gic_init_irqs_and_distributor(s-gic, s-num_irq);
--
1.8.1.4
[Qemu-devel] [PATCH 0/4] hw: Purge error_is_set()
I got a private branch getting rid of it entirely. This is the fourth part, covering devices, except for two places I need to cover together with QAPI, in a future series. Could this one go through Andreas's tree? Markus Armbruster (4): hw: Consistently name Error * objects err, and not errp hw: Consistently name Error ** objects errp, and not err qom: Clean up fragile use of error_is_set() in set() methods arm: Clean up fragile use of error_is_set() in realize() methods backends/rng.c | 11 +++ backends/tpm.c | 11 +++ hw/core/qdev-properties-system.c | 10 +- hw/core/qdev-properties.c| 11 +++ hw/core/qdev.c | 20 ++-- hw/dma/xilinx_axidma.c | 16 hw/intc/arm_gic.c| 6 -- hw/intc/arm_gic_kvm.c| 6 -- hw/intc/armv7m_nvic.c| 6 -- hw/intc/i8259.c | 4 ++-- hw/misc/tmp105.c | 6 -- hw/net/xilinx_axienet.c | 16 hw/timer/i8254.c | 4 ++-- hw/virtio/virtio-balloon.c | 6 -- target-i386/cpu.c| 24 15 files changed, 92 insertions(+), 65 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH 2/4] hw: Consistently name Error ** objects errp, and not err
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/core/qdev.c | 20 ++--
hw/intc/i8259.c | 4 ++--
hw/timer/i8254.c | 4 ++--
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 60f9df1..2fd5100 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -174,14 +174,14 @@ int qdev_init(DeviceState *dev)
return 0;
}
-static void device_realize(DeviceState *dev, Error **err)
+static void device_realize(DeviceState *dev, Error **errp)
{
DeviceClass *dc = DEVICE_GET_CLASS(dev);
if (dc-init) {
int rc = dc-init(dev);
if (rc 0) {
-error_setg(err, Device initialization failed.);
+error_setg(errp, Device initialization failed.);
return;
}
}
@@ -504,14 +504,14 @@ static void bus_unparent(Object *obj)
}
}
-static bool bus_get_realized(Object *obj, Error **err)
+static bool bus_get_realized(Object *obj, Error **errp)
{
BusState *bus = BUS(obj);
return bus-realized;
}
-static void bus_set_realized(Object *obj, bool value, Error **err)
+static void bus_set_realized(Object *obj, bool value, Error **errp)
{
BusState *bus = BUS(obj);
BusClass *bc = BUS_GET_CLASS(bus);
@@ -540,7 +540,7 @@ static void bus_set_realized(Object *obj, bool value, Error
**err)
return;
error:
-error_propagate(err, local_err);
+error_propagate(errp, local_err);
}
void qbus_create_inplace(void *bus, size_t size, const char *typename,
@@ -724,13 +724,13 @@ void qdev_property_add_static(DeviceState *dev, Property
*prop,
}
}
-static bool device_get_realized(Object *obj, Error **err)
+static bool device_get_realized(Object *obj, Error **errp)
{
DeviceState *dev = DEVICE(obj);
return dev-realized;
}
-static void device_set_realized(Object *obj, bool value, Error **err)
+static void device_set_realized(Object *obj, bool value, Error **errp)
{
DeviceState *dev = DEVICE(obj);
DeviceClass *dc = DEVICE_GET_CLASS(dev);
@@ -738,7 +738,7 @@ static void device_set_realized(Object *obj, bool value,
Error **err)
Error *local_err = NULL;
if (dev-hotplugged !dc-hotpluggable) {
-error_set(err, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
+error_set(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
return;
}
@@ -797,14 +797,14 @@ static void device_set_realized(Object *obj, bool value,
Error **err)
}
if (local_err != NULL) {
-error_propagate(err, local_err);
+error_propagate(errp, local_err);
return;
}
dev-realized = value;
}
-static bool device_get_hotpluggable(Object *obj, Error **err)
+static bool device_get_hotpluggable(Object *obj, Error **errp)
{
DeviceClass *dc = DEVICE_GET_CLASS(obj);
DeviceState *dev = DEVICE(obj);
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
index c6f248b..ec01393 100644
--- a/hw/intc/i8259.c
+++ b/hw/intc/i8259.c
@@ -412,7 +412,7 @@ static const MemoryRegionOps pic_elcr_ioport_ops = {
},
};
-static void pic_realize(DeviceState *dev, Error **err)
+static void pic_realize(DeviceState *dev, Error **errp)
{
PICCommonState *s = PIC_COMMON(dev);
PICClass *pc = PIC_GET_CLASS(dev);
@@ -425,7 +425,7 @@ static void pic_realize(DeviceState *dev, Error **err)
qdev_init_gpio_out(dev, s-int_out, ARRAY_SIZE(s-int_out));
qdev_init_gpio_in(dev, pic_set_irq, 8);
-pc-parent_realize(dev, err);
+pc-parent_realize(dev, errp);
}
void pic_info(Monitor *mon, const QDict *qdict)
diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c
index 28152d8..3450c98 100644
--- a/hw/timer/i8254.c
+++ b/hw/timer/i8254.c
@@ -322,7 +322,7 @@ static void pit_post_load(PITCommonState *s)
}
}
-static void pit_realizefn(DeviceState *dev, Error **err)
+static void pit_realizefn(DeviceState *dev, Error **errp)
{
PITCommonState *pit = PIT_COMMON(dev);
PITClass *pc = PIT_GET_CLASS(dev);
@@ -338,7 +338,7 @@ static void pit_realizefn(DeviceState *dev, Error **err)
qdev_init_gpio_in(dev, pit_irq_control, 1);
-pc-parent_realize(dev, err);
+pc-parent_realize(dev, errp);
}
static Property pit_properties[] = {
--
1.8.1.4
[Qemu-devel] [PATCH 3/4] qom: Clean up fragile use of error_is_set() in set() methods
Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP
may be null, because errors go undetected when it is. It's fragile
when proving ERRP non-null involves a non-local argument. Else, it's
unnecessarily opaque (see commit 84d18f0).
I guess the error_is_set(errp) in the ObjectProperty set() methods are
merely fragile right now, because I can't find a call chain that
passes a null errp argument.
Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
backends/rng.c | 11 +++
backends/tpm.c | 11 +++
hw/core/qdev-properties.c | 11 +++
hw/misc/tmp105.c | 6 --
hw/virtio/virtio-balloon.c | 6 --
target-i386/cpu.c | 24
6 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/backends/rng.c b/backends/rng.c
index 8b8d5a4..0f2fc11 100644
--- a/backends/rng.c
+++ b/backends/rng.c
@@ -50,6 +50,7 @@ static void rng_backend_prop_set_opened(Object *obj, bool
value, Error **errp)
{
RngBackend *s = RNG_BACKEND(obj);
RngBackendClass *k = RNG_BACKEND_GET_CLASS(s);
+Error *local_err = NULL;
if (value == s-opened) {
return;
@@ -61,12 +62,14 @@ static void rng_backend_prop_set_opened(Object *obj, bool
value, Error **errp)
}
if (k-opened) {
-k-opened(s, errp);
+k-opened(s, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
}
-if (!error_is_set(errp)) {
-s-opened = value;
-}
+s-opened = true;
}
static void rng_backend_init(Object *obj)
diff --git a/backends/tpm.c b/backends/tpm.c
index b735801..01860c4 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -112,6 +112,7 @@ static void tpm_backend_prop_set_opened(Object *obj, bool
value, Error **errp)
{
TPMBackend *s = TPM_BACKEND(obj);
TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
+Error *local_err = NULL;
if (value == s-opened) {
return;
@@ -123,12 +124,14 @@ static void tpm_backend_prop_set_opened(Object *obj, bool
value, Error **errp)
}
if (k-opened) {
-k-opened(s, errp);
+k-opened(s, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
}
-if (!error_is_set(errp)) {
-s-opened = value;
-}
+s-opened = true;
}
static void tpm_backend_instance_init(Object *obj)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index c67acf5..5c08611 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -746,6 +746,7 @@ static void set_prop_arraylen(Object *obj, Visitor *v, void
*opaque,
* array-length field in the device struct, we have to create the
* array itself and dynamically add the corresponding properties.
*/
+Error *local_err = NULL;
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
uint32_t *alenptr = qdev_get_prop_ptr(dev, prop);
@@ -763,8 +764,9 @@ static void set_prop_arraylen(Object *obj, Visitor *v, void
*opaque,
name);
return;
}
-visit_type_uint32(v, alenptr, name, errp);
-if (error_is_set(errp)) {
+visit_type_uint32(v, alenptr, name, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
return;
}
if (!*alenptr) {
@@ -801,8 +803,9 @@ static void set_prop_arraylen(Object *obj, Visitor *v, void
*opaque,
arrayprop-prop.info-get,
arrayprop-prop.info-set,
array_element_release,
-arrayprop, errp);
-if (error_is_set(errp)) {
+arrayprop, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
return;
}
}
diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c
index 63aa3d6..e11657d 100644
--- a/hw/misc/tmp105.c
+++ b/hw/misc/tmp105.c
@@ -67,11 +67,13 @@ static void tmp105_get_temperature(Object *obj, Visitor *v,
void *opaque,
static void tmp105_set_temperature(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
+Error *local_err = NULL;
TMP105State *s = TMP105(obj);
int64_t temp;
-visit_type_int(v, temp, name, errp);
-if (error_is_set(errp)) {
+visit_type_int(v, temp, name, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
return;
}
if (temp = 128000 || temp -128000) {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index a470a0b..51f02eb 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -141,11 +141,13 @@
[Qemu-devel] [PATCH 5/9] usb: mtp: fix error path memory leak
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/dev-mtp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 775dc8d..45f9562 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -669,6 +669,7 @@ static MTPData *usb_mtp_get_object(MTPState *s, MTPControl
*c,
d-fd = open(o-path, O_RDONLY);
if (d-fd == -1) {
+usb_mtp_data_free(d);
return NULL;
}
d-length = o-stat.st_size;
@@ -688,6 +689,7 @@ static MTPData *usb_mtp_get_partial_object(MTPState *s,
MTPControl *c,
d-fd = open(o-path, O_RDONLY);
if (d-fd == -1) {
+usb_mtp_data_free(d);
return NULL;
}
--
1.8.3.1
[Qemu-devel] [PATCH 2/9] usb: mtp: fix usb_mtp_add_u64
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/dev-mtp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 17df447..063a426 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -416,7 +416,7 @@ static void usb_mtp_add_u32(MTPData *data, uint32_t val)
static void usb_mtp_add_u64(MTPData *data, uint64_t val)
{
-usb_mtp_realloc(data, 4);
+usb_mtp_realloc(data, 8);
data-data[data-length++] = (val 0) 0xff;
data-data[data-length++] = (val 8) 0xff;
data-data[data-length++] = (val 16) 0xff;
@@ -424,7 +424,7 @@ static void usb_mtp_add_u64(MTPData *data, uint64_t val)
data-data[data-length++] = (val 32) 0xff;
data-data[data-length++] = (val 40) 0xff;
data-data[data-length++] = (val 48) 0xff;
-data-data[data-length++] = (val 54) 0xff;
+data-data[data-length++] = (val 56) 0xff;
}
static void usb_mtp_add_u16_array(MTPData *data, uint32_t len,
--
1.8.3.1
[Qemu-devel] [PATCH 7/9] usb: mtp: avoid empty description string
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/dev-mtp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 82d5b64..536a23d 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1049,7 +1049,7 @@ static int usb_mtp_initfn(USBDevice *dev)
QTAILQ_INIT(s-objects);
if (s-desc == NULL) {
s-desc = strrchr(s-root, '/');
-if (s-desc) {
+if (s-desc s-desc[0]) {
s-desc = g_strdup(s-desc + 1);
} else {
s-desc = g_strdup(none);
--
1.8.3.1
[Qemu-devel] [PATCH 1/9] usb: mtp: replace debug printfs with trace points
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/dev-mtp.c | 8
trace-events | 3 +++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 8b44032..17df447 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -294,7 +294,7 @@ static MTPObject *usb_mtp_object_alloc(MTPState *s,
uint32_t handle,
goto ignore;
}
-fprintf(stderr, %s: 0x%x %s\n, __func__, o-handle, o-path);
+trace_usb_mtp_object_alloc(s-dev.addr, o-handle, o-path);
QTAILQ_INSERT_TAIL(s-objects, o, next);
return o;
@@ -310,7 +310,7 @@ static void usb_mtp_object_free(MTPState *s, MTPObject *o)
{
int i;
-fprintf(stderr, %s: 0x%x %s\n, __func__, o-handle, o-path);
+trace_usb_mtp_object_free(s-dev.addr, o-handle, o-path);
QTAILQ_REMOVE(s-objects, o, next);
for (i = 0; i o-nchildren; i++) {
@@ -843,8 +843,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
res0 = data_in-length;
break;
default:
-fprintf(stderr, %s: unknown command code 0x%04x\n,
-__func__, c-code);
+trace_usb_mtp_op_unknown(s-dev.addr, c-code);
usb_mtp_queue_result(s, RES_OPERATION_NOT_SUPPORTED,
c-trans, 0, 0, 0);
return;
@@ -892,6 +891,7 @@ static void usb_mtp_handle_control(USBDevice *dev,
USBPacket *p,
static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
{
+/* we don't use async packets, so this should never be called */
fprintf(stderr, %s\n, __func__);
}
diff --git a/trace-events b/trace-events
index 6ecaab2..7c17c8b 100644
--- a/trace-events
+++ b/trace-events
@@ -453,6 +453,9 @@ usb_mtp_op_get_object_handles(int dev, uint32_t handle,
const char *path) dev %
usb_mtp_op_get_object_info(int dev, uint32_t handle, const char *path) dev
%d, handle 0x%x, path %s
usb_mtp_op_get_object(int dev, uint32_t handle, const char *path) dev %d,
handle 0x%x, path %s
usb_mtp_op_get_partial_object(int dev, uint32_t handle, const char *path,
uint32_t offset, uint32_t length) dev %d, handle 0x%x, path %s, off %d, len %d
+usb_mtp_op_unknown(int dev, uint32_t code) dev %d, command code 0x%x
+usb_mtp_object_alloc(int dev, uint32_t handle, const char *path) dev %d,
handle 0x%x, path %s
+usb_mtp_object_free(int dev, uint32_t handle, const char *path) dev %d,
handle 0x%x, path %s
# hw/usb/host-libusb.c
usb_host_open_started(int bus, int addr) dev %d:%d
--
1.8.3.1
[Qemu-devel] [PATCH 8/9] usb: mtp: drop data-out hexdump
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/usb/dev-mtp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c index 536a23d..6bd6a82 100644 --- a/hw/usb/dev-mtp.c +++ b/hw/usb/dev-mtp.c @@ -1014,8 +1014,7 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket *p) usb_mtp_command(s, cmd); break; default: -iov_hexdump(p-iov.iov, p-iov.niov, stderr, mtp-out, 32); -trace_usb_mtp_stall(s-dev.addr, TODO: implement data-out); +/* not needed as long as the mtp device is read-only */ p-status = USB_RET_STALL; return; } -- 1.8.3.1
[Qemu-devel] [PATCH 3/9] usb: mtp: fix version (is decimal not bcd)
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/usb/dev-mtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c index 063a426..dff2618 100644 --- a/hw/usb/dev-mtp.c +++ b/hw/usb/dev-mtp.c @@ -533,7 +533,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, MTPControl *c) trace_usb_mtp_op_get_device_info(s-dev.addr); -usb_mtp_add_u16(d, 0x0100); +usb_mtp_add_u16(d, 100); usb_mtp_add_u32(d, 0x); usb_mtp_add_u16(d, 0x0101); usb_mtp_add_wstr(d, L); -- 1.8.3.1
[Qemu-devel] [PATCH 0/9] usb: mtp: a bunch of fixes
Hi, Here is a bunch of incremental fixes for the just merged usb mtp device, coming from patch review by Peter and Stefan. cheers, Gerd Gerd Hoffmann (9): usb: mtp: replace debug printfs with trace points usb: mtp: fix usb_mtp_add_u64 usb: mtp: fix version (is decimal not bcd) usb: mtp: fix serial (must be exact 32 chars) usb: mtp: fix error path memory leak usb: mtp: fix possible buffer overflow usb: mtp: avoid empty description string usb: mtp: drop data-out hexdump usb: mtp: reply INCOMPLETE_TRANSFER on read errors hw/usb/dev-mtp.c | 29 + trace-events | 3 +++ 2 files changed, 20 insertions(+), 12 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 9/9] usb: mtp: reply INCOMPLETE_TRANSFER on read errors
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/dev-mtp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6bd6a82..8e1a1b7 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -50,6 +50,7 @@ enum mtp_code {
RES_INVALID_TRANSACTION_ID = 0x2004,
RES_OPERATION_NOT_SUPPORTED= 0x2005,
RES_PARAMETER_NOT_SUPPORTED= 0x2006,
+RES_INCOMPLETE_TRANSFER= 0x2007,
RES_INVALID_STORAGE_ID = 0x2008,
RES_INVALID_OBJECT_HANDLE = 0x2009,
RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
@@ -946,7 +947,7 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket
*p)
}
rc = read(d-fd, d-data, dlen);
if (rc != dlen) {
-fprintf(stderr, %s: TODO: handle read error\n, __func__);
+s-result-code = RES_INCOMPLETE_TRANSFER;
}
usb_packet_copy(p, d-data, dlen);
}
--
1.8.3.1
[Qemu-devel] [PATCH 6/9] usb: mtp: fix possible buffer overflow
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/dev-mtp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 45f9562..82d5b64 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -998,6 +998,9 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket
*p)
cmd.argc = (le32_to_cpu(container.length) - sizeof(container))
/ sizeof(uint32_t);
cmd.trans = le32_to_cpu(container.trans);
+if (cmd.argc ARRAY_SIZE(cmd.argv)) {
+cmd.argc = ARRAY_SIZE(cmd.argv);
+}
usb_packet_copy(p, params, cmd.argc * sizeof(uint32_t));
for (i = 0; i cmd.argc; i++) {
cmd.argv[i] = le32_to_cpu(params[i]);
--
1.8.3.1
[Qemu-devel] [PATCH 4/9] usb: mtp: fix serial (must be exact 32 chars)
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/usb/dev-mtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c index dff2618..775dc8d 100644 --- a/hw/usb/dev-mtp.c +++ b/hw/usb/dev-mtp.c @@ -548,7 +548,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, MTPControl *c) usb_mtp_add_wstr(d, L MTP_MANUFACTURER); usb_mtp_add_wstr(d, L MTP_PRODUCT); usb_mtp_add_wstr(d, L0.1); -usb_mtp_add_wstr(d, L123456789abcdef123456789abcdef); +usb_mtp_add_wstr(d, L0123456789abcdef0123456789abcdef); return d; } -- 1.8.3.1
Re: [Qemu-devel] [PULL v2 2/2] usb: mtp filesharing
On Thursday 24 April 2014 17:01:47 Gerd Hoffmann wrote: PS: Funny thing that the reviews start coming in when I send pull requests. The patches have been on the list a few weeks back already (during 2.0 freeze, thats why the long delay between [patch] and [pull]). No comments. Should I consider going straight for a pull requests to get reviews faster? Well, I am interested in the USB subsystem (working on usb monitoring functionality[1] and a virtual Logitech Unifying device[2]), but only just noticed this subject on the list. (I am subscribed since somewhere in the end of March.) The messages volume is also large, so it is easy to miss some. It would be nice if someone could make Launchpad not send out duplicate mails to the list. Regards, Peter [1]: https://git.lekensteyn.nl/peter/qemu/log/?h=usbdump-usbhid [2]: https://git.lekensteyn.nl/peter/qemu/log/?h=logitech-unifying
Re: [Qemu-devel] [PATCH 1/4] hw: Consistently name Error * objects err, and not errp
Am 25.04.2014 12:44, schrieb Markus Armbruster: Signed-off-by: Markus Armbruster arm...@redhat.com Thanks a lot, Reviewed-by: Andreas Färber afaer...@suse.de Andreas -- 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 2/4] hw: Consistently name Error ** objects errp, and not err
Am 25.04.2014 12:44, schrieb Markus Armbruster: Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/core/qdev.c | 20 ++-- hw/intc/i8259.c | 4 ++-- hw/timer/i8254.c | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) Reviewed-by: Andreas Färber afaer...@suse.de Andreas -- 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 6/9] usb: mtp: fix possible buffer overflow
On Friday 25 April 2014 12:48:11 Gerd Hoffmann wrote:
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/dev-mtp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 45f9562..82d5b64 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -998,6 +998,9 @@ static void usb_mtp_handle_data(USBDevice *dev,
USBPacket *p) cmd.argc = (le32_to_cpu(container.length) -
sizeof(container)) / sizeof(uint32_t);
cmd.trans = le32_to_cpu(container.trans);
+if (cmd.argc ARRAY_SIZE(cmd.argv)) {
+cmd.argc = ARRAY_SIZE(cmd.argv);
+}
This is not sufficient, you must also check that the iov is large enough to
hold this many args.
Kind regards,
Peter
usb_packet_copy(p, params, cmd.argc * sizeof(uint32_t));
for (i = 0; i cmd.argc; i++) {
cmd.argv[i] = le32_to_cpu(params[i]);
Re: [Qemu-devel] [PATCH 9/9] usb: mtp: reply INCOMPLETE_TRANSFER on read errors
On Friday 25 April 2014 12:48:14 Gerd Hoffmann wrote:
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/dev-mtp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6bd6a82..8e1a1b7 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -50,6 +50,7 @@ enum mtp_code {
RES_INVALID_TRANSACTION_ID = 0x2004,
RES_OPERATION_NOT_SUPPORTED= 0x2005,
RES_PARAMETER_NOT_SUPPORTED= 0x2006,
+RES_INCOMPLETE_TRANSFER= 0x2007,
RES_INVALID_STORAGE_ID = 0x2008,
RES_INVALID_OBJECT_HANDLE = 0x2009,
RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
@@ -946,7 +947,7 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket
*p) }
rc = read(d-fd, d-data, dlen);
if (rc != dlen) {
-fprintf(stderr, %s: TODO: handle read error\n,
__func__);
+s-result-code = RES_INCOMPLETE_TRANSFER;
}
usb_packet_copy(p, d-data, dlen);
}
The bogus data packet is sent with usb_packet_copy, shouldn't you return
USB_RET_NAK for now? Something like:
if (rc == dlen) {
usb_packet_copy(p, d-data, dlen);
} else {
s-result-code = RES_INCOMPLETE_TRANSFER;
dlen = 0;
}
dlen = 0 to avoid messing with the offset.
Kind regards,
Peter
Re: [Qemu-devel] [PATCH 0/3] hw/arm/virt: Support Cortex-A57
On 25 April 2014 11:42, Christoffer Dall christoffer.d...@linaro.org wrote: So we're adding an incompletely modelled A57 to the virt board, and adding a board mounted GICv2. Doesn't sound like something that could ever exist in real life, but I agree that the way we are tying things together, semantically letting the board decide how to place the GIC makes the most sense. Actually I think a lot of A57 hardware right now is using a standalone GIC. The ARM Server Base System Architecture spec mandates a v2 GIC for level 0/1 compliance, GICv3 only comes in at level 2. So it's not that far from reality (and it leaves us the obvious space to put in a v3 GIC emulation eventually). Oh well. Let's implement GICv3 support some time. You first :-) thanks -- PMM
Re: [Qemu-devel] [PATCH 4/4] arm: Clean up fragile use of error_is_set() in realize() methods
Am 25.04.2014 12:44, schrieb Markus Armbruster:
Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP
may be null, because errors go undetected when it is. It's fragile
when proving ERRP non-null involves a non-local argument. Else, it's
unnecessarily opaque (see commit 84d18f0).
I guess the error_is_set(errp) in the DeviceClass realize() methods
are merely fragile right now, because I can't find a call chain that
passes a null errp argument.
Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/intc/arm_gic.c | 6 --
hw/intc/arm_gic_kvm.c | 6 --
hw/intc/armv7m_nvic.c | 6 --
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 955b8d4..f6c7144 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -793,13 +793,15 @@ void gic_init_irqs_and_distributor(GICState *s, int
num_irq)
static void arm_gic_realize(DeviceState *dev, Error **errp)
{
/* Device instance realize function for the GIC sysbus device */
+Error *local_err = NULL;
int i;
GICState *s = ARM_GIC(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
ARMGICClass *agc = ARM_GIC_GET_CLASS(s);
[snip]
Here and in the previous patch I'm not so thrilled about placing this
new variable first. We've usually been using the system of casting the
arguments first, from base type to most specific type, then any local
variables in the end. Here, i breaks the system already, not your fault,
but in the next hunk there's an int ret as final variable or int64_t
temp in TMP105. Can we (me if through my tree) move them down?
Otherwise both patches look fine, thanks for your efforts!
I do wonder, and maybe Peter can comment as native speaker, whether it
should be uses (plural) or usage in both subjects?
Regards,
Andreas
--
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 0/9] usb: mtp: a bunch of fixes
On Friday 25 April 2014 12:48:05 Gerd Hoffmann wrote: Hi, Here is a bunch of incremental fixes for the just merged usb mtp device, coming from patch review by Peter and Stefan. cheers, Gerd Gerd Hoffmann (9): usb: mtp: replace debug printfs with trace points usb: mtp: fix usb_mtp_add_u64 usb: mtp: fix version (is decimal not bcd) usb: mtp: fix serial (must be exact 32 chars) usb: mtp: fix error path memory leak usb: mtp: fix possible buffer overflow usb: mtp: avoid empty description string usb: mtp: drop data-out hexdump usb: mtp: reply INCOMPLETE_TRANSFER on read errors hw/usb/dev-mtp.c | 29 + trace-events | 3 +++ 2 files changed, 20 insertions(+), 12 deletions(-) For patches 1-5 and 7-8: Reviewed-by: Peter Wu pe...@lekensteyn.nl Patch 6 and 9 have some comments. Kind regards, Peter
Re: [Qemu-devel] [PATCH v3] ps2: set ps/2 output buffer size as the same as kernel
On Do, 2014-04-24 at 20:06 +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com According to the PS/2 Mouse/Keyboard Protocol, the keyboard outupt buffer size is 16 bytes. And the PS2_QUEUE_SIZE 256 was introduced in Qemu from the very beginning. Hmm, seems there is no maintainer for ps2.c, guess I better pick that into my input patches queue so it doesn't get lost. Thanks for your effort. cheers, Gerd
Re: [Qemu-devel] [PATCH v3 0/5] Add common QEMU control functionality to qemu-iotests
On Thu, Apr 10, 2014 at 04:47:35PM -0400, Jeff Cody wrote: Changes from v2: Updated Reviewed-by for Fam and Benoit (Benoit's from the v1 patch, I forgot to add those to v2) Patch 1: * updated commit message (Thanks Fam) * Addded '-machine accel=qtest' to qemu launch args (Thanks Fam) Patch 3: * Moved from test 089 - test 090 to avoid collision with Fam's series (Thanks Fam) Changes from v1: Patch 1: * Fixed commit message, clarified comments (Thanks Benoît) * Changed 'shift' line to be POSIX-friendly, instead of relying on bashism (Thanks Eric) * Added ability to repeat qmp or hmp commands an arbitrary number of times Patch 3: New patch, for live migration Original Description: This adds some common functionality to control QEMU for qemu-iotests. Additionally, test 085 is updated to use this new functionality. Some minor fixups along the way, to clear up spaced pathname issues, for common.rc, test 019, and test 086. Jeff Cody (5): block: qemu-iotests - add common.qemu, for bash-controlled qemu tests block: qemu-iotests - update 085 to use common.qemu block: qemu-iotests - test for live migration block: qemu-iotests - fix image cleanup when using spaced pathnames block: qemu-iotests: make test 019 and 086 work with spaced pathnames tests/qemu-iotests/019 | 2 +- tests/qemu-iotests/085 | 73 +++ tests/qemu-iotests/086 | 8 +- tests/qemu-iotests/090 | 97 tests/qemu-iotests/090.out | 20 + tests/qemu-iotests/common.qemu | 195 + tests/qemu-iotests/common.rc | 4 +- tests/qemu-iotests/group | 1 + 8 files changed, 332 insertions(+), 68 deletions(-) create mode 100755 tests/qemu-iotests/090 create mode 100644 tests/qemu-iotests/090.out create mode 100644 tests/qemu-iotests/common.qemu -- 1.8.3.1 Ping?
Re: [Qemu-devel] [PATCH microblaze v1 1/1] net: xilinx_axienet.c: Add phy soft reset bit clearing
On Tue, Apr 08, 2014 at 06:52:39PM -0700, Peter Crosthwaite wrote: From: Nathan Rossi nathan.ro...@xilinx.com Clear the BMCR Reset when writing to registers. Signed-off-by: Nathan Rossi nathan.ro...@xilinx.com [ PC: * Trivial style fixes to commit message ] Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/net/xilinx_axienet.c | 3 +++ 1 file changed, 3 insertions(+) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan
Re: [Qemu-devel] [PATCH 4/4] arm: Clean up fragile use of error_is_set() in realize() methods
On 25 April 2014 12:25, Andreas Färber afaer...@suse.de wrote: I do wonder, and maybe Peter can comment as native speaker, whether it should be uses (plural) or usage in both subjects? In this subject I would consider all of use, uses and usage as acceptable variations. thanks -- PMM
Re: [Qemu-devel] [GSoC] Wanted: small warmup tasks
Am 25.04.2014 08:38, schrieb Markus Armbruster: Andreas Färber afaer...@suse.de writes: Am 24.04.2014 08:19, schrieb Jan Kiszka: On 2014-04-23 11:25, Stefan Hajnoczi wrote: Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. To follow up on this: I'm currently looking for optional tiny warmup tasks for our QEMU students during the bonding period (till May 18). If you have any trivial issues or extensions in mind that someone could address within a few days or even hours, that would be perfect. It could even be something like reformat the printing of these messages or so. Replacing some more fprintf(stderr, foo\n) with error_report(foo) comes to mind. :) A logical next step after this kind of warmup would be a simple conversion to error_set(). Perhaps a good place to start would be qmp_chardev_add(). The switch there has some cases converted already. Convert one or more of the remaining ones. Beware of the rabbit holes! [snip] Jumping in on error_set*(), a cute idea might be to evaluate improving error_propagate(). Right now, it just pointer-wise (or contents-wise?) propagates the supplied Error object. This may lead to the root error cause being badly understandable for the end user in a long chain of propagations. Think of realizing a SoC device or a PCI host bridge, which in turn has child objects getting realized, doing things in their realize function that might fail: We may end up getting a standard error such as Permission denied without any context of where or why. Compare that to Java where you'd get (too verbose) recursive Exception objects telling you that this failed because that failed because that failed. While it might not be handy to introduce recursively allocated Error objects, simply prepending something to the Error's description should be more easily doable, i.e. error_propagate_foo(errp, local_err, Foo failed) - Foo failed: Original message. On that matter, I had once sprouted the idea of introducing a statically allocated error_oom, similar to error_abort, for avoiding aborts on out-of-memory for user-initiated operations such as hot-add. Unlike the scenario described above, the idea here is to avoid any dynamic allocations, to make it safer to work with guests in low memory situations. This also involves avoiding the aborting object_new() in favor of g_try_malloc() together with a proposed QOM API extension for obtaining the .instance_size of a type: http://patchwork.ozlabs.org/patch/269591/ Cheers, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PULL 0/4] current s390x queue
No further comments had been made on the patches below, so sending a pull request. The following changes since commit 0e96643c98eb22a5f2e11ac500852133032d38e4: Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-5' into staging (2014-04-24 16:16:57 +0100) are available in the git repository at: https://github.com/cohuck/qemu.git tags/s390x-20140425 for you to fetch changes up to 44b0c0bbb50818e995702cf76d8e07dd36f479bf: s390x/kvm: sync gbea and pp register (2014-04-25 12:59:57 +0200) Some s390x patches: - gdb stubs to make it compile if gdb support is pulled in - linux-headers update for new oneregs - two onereg enhancements Christian Borntraeger (2): s390x/kvm: rework KVM synchronize to tracing for some ONEREGS s390x/kvm: sync gbea and pp register Cornelia Huck (1): linux-headers update David Hildenbrand (1): s390x: empty function stubs in preparation for __KVM_HAVE_GUEST_DEBUG linux-headers/asm-s390/kvm.h | 24 ++ linux-headers/linux/kvm.h| 17 linux-headers/linux/vfio.h |6 ++ target-s390x/cpu.h |3 + target-s390x/kvm.c | 175 +++--- trace-events |4 + 6 files changed, 151 insertions(+), 78 deletions(-) -- 1.7.9.5
[Qemu-devel] [PULL 3/4] s390x/kvm: rework KVM synchronize to tracing for some ONEREGS
From: Christian Borntraeger borntrae...@de.ibm.com
Some ONE_REGS on s390 are not protected by a capability. Older kernels
might not provide those and return an error. Fortunately these registers
are only critical for the migration path. There is no need to error out
on reset and normal runtime. Furthermore, these kernels don't provide
a proper dirty bitmap anyway, so let's use tracing for those errors.
Also provide generic one reg helper to simplify the code.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
---
target-s390x/kvm.c | 143
trace-events |4 ++
2 files changed, 69 insertions(+), 78 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 2b2dcdc..daaabbd 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -36,6 +36,7 @@
#include sysemu/device_tree.h
#include qapi/qmp/qjson.h
#include monitor/monitor.h
+#include trace.h
/* #define DEBUG_KVM */
@@ -128,14 +129,42 @@ void kvm_arch_reset_vcpu(CPUState *cpu)
}
}
+static int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source)
+{
+struct kvm_one_reg reg;
+int r;
+
+reg.id = id;
+reg.addr = (uint64_t) source;
+r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
+if (r) {
+trace_kvm_failed_reg_set(id, strerror(errno));
+}
+return r;
+}
+
+static int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target)
+{
+struct kvm_one_reg reg;
+int r;
+
+reg.id = id;
+reg.addr = (uint64_t) target;
+r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, reg);
+if (r) {
+trace_kvm_failed_reg_get(id, strerror(errno));
+}
+return r;
+}
+
+
int kvm_arch_put_registers(CPUState *cs, int level)
{
S390CPU *cpu = S390_CPU(cs);
CPUS390XState *env = cpu-env;
-struct kvm_one_reg reg;
struct kvm_sregs sregs;
struct kvm_regs regs;
-int ret;
+int r;
int i;
/* always save the PSW and the GPRS*/
@@ -151,9 +180,9 @@ int kvm_arch_put_registers(CPUState *cs, int level)
for (i = 0; i 16; i++) {
regs.gprs[i] = env-regs[i];
}
-ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, regs);
-if (ret 0) {
-return ret;
+r = kvm_vcpu_ioctl(cs, KVM_SET_REGS, regs);
+if (r 0) {
+return r;
}
}
@@ -162,47 +191,27 @@ int kvm_arch_put_registers(CPUState *cs, int level)
return 0;
}
-reg.id = KVM_REG_S390_CPU_TIMER;
-reg.addr = (__u64)(env-cputm);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
-if (ret 0) {
-return ret;
-}
-
-reg.id = KVM_REG_S390_CLOCK_COMP;
-reg.addr = (__u64)(env-ckc);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
-if (ret 0) {
-return ret;
-}
-
-reg.id = KVM_REG_S390_TODPR;
-reg.addr = (__u64)(env-todpr);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
-if (ret 0) {
-return ret;
-}
+/*
+ * These ONE_REGS are not protected by a capability. As they are only
+ * necessary for migration we just trace a possible error, but don't
+ * return with an error return code.
+ */
+kvm_set_one_reg(cs, KVM_REG_S390_CPU_TIMER, env-cputm);
+kvm_set_one_reg(cs, KVM_REG_S390_CLOCK_COMP, env-ckc);
+kvm_set_one_reg(cs, KVM_REG_S390_TODPR, env-todpr);
if (cap_async_pf) {
-reg.id = KVM_REG_S390_PFTOKEN;
-reg.addr = (__u64)(env-pfault_token);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
-if (ret 0) {
-return ret;
+r = kvm_set_one_reg(cs, KVM_REG_S390_PFTOKEN, env-pfault_token);
+if (r 0) {
+return r;
}
-
-reg.id = KVM_REG_S390_PFCOMPARE;
-reg.addr = (__u64)(env-pfault_compare);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
-if (ret 0) {
-return ret;
+r = kvm_set_one_reg(cs, KVM_REG_S390_PFCOMPARE, env-pfault_compare);
+if (r 0) {
+return r;
}
-
-reg.id = KVM_REG_S390_PFSELECT;
-reg.addr = (__u64)(env-pfault_select);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg);
-if (ret 0) {
-return ret;
+r = kvm_set_one_reg(cs, KVM_REG_S390_PFSELECT, env-pfault_select);
+if (r 0) {
+return r;
}
}
@@ -220,9 +229,9 @@ int kvm_arch_put_registers(CPUState *cs, int level)
sregs.acrs[i] = env-aregs[i];
sregs.crs[i] = env-cregs[i];
}
-ret = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, sregs);
-if (ret 0) {
-return ret;
+r = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, sregs);
+if (r 0) {
+return r;
}
}
@@ -240,7 +249,6 @@ int kvm_arch_get_registers(CPUState *cs)
{
S390CPU *cpu = S390_CPU(cs);
CPUS390XState
[Qemu-devel] [PULL 4/4] s390x/kvm: sync gbea and pp register
From: Christian Borntraeger borntrae...@de.ibm.com
We also need to sync guest breaking event address and program parameter
register for migration support.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
---
target-s390x/cpu.h |3 +++
target-s390x/kvm.c |4
2 files changed, 7 insertions(+)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index f332d41..41903a9 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -126,6 +126,9 @@ typedef struct CPUS390XState {
uint64_t pfault_compare;
uint64_t pfault_select;
+uint64_t gbea;
+uint64_t pp;
+
CPU_COMMON
/* reset does memset(0) up to here */
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index daaabbd..a30d1bc 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -199,6 +199,8 @@ int kvm_arch_put_registers(CPUState *cs, int level)
kvm_set_one_reg(cs, KVM_REG_S390_CPU_TIMER, env-cputm);
kvm_set_one_reg(cs, KVM_REG_S390_CLOCK_COMP, env-ckc);
kvm_set_one_reg(cs, KVM_REG_S390_TODPR, env-todpr);
+kvm_set_one_reg(cs, KVM_REG_S390_GBEA, env-gbea);
+kvm_set_one_reg(cs, KVM_REG_S390_PP, env-pp);
if (cap_async_pf) {
r = kvm_set_one_reg(cs, KVM_REG_S390_PFTOKEN, env-pfault_token);
@@ -304,6 +306,8 @@ int kvm_arch_get_registers(CPUState *cs)
kvm_get_one_reg(cs, KVM_REG_S390_CPU_TIMER, env-cputm);
kvm_get_one_reg(cs, KVM_REG_S390_CLOCK_COMP, env-ckc);
kvm_get_one_reg(cs, KVM_REG_S390_TODPR, env-todpr);
+kvm_get_one_reg(cs, KVM_REG_S390_GBEA, env-gbea);
+kvm_get_one_reg(cs, KVM_REG_S390_PP, env-pp);
if (cap_async_pf) {
r = kvm_get_one_reg(cs, KVM_REG_S390_PFTOKEN, env-pfault_token);
--
1.7.9.5
[Qemu-devel] [PULL 2/4] linux-headers update
linux-headers update against v3.15-rc2 (a798c10f)
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
---
linux-headers/asm-s390/kvm.h | 24
linux-headers/linux/kvm.h| 17 +
linux-headers/linux/vfio.h |6 ++
3 files changed, 47 insertions(+)
diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index cb4c1eb..c003c6a 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -22,6 +22,8 @@
#define KVM_DEV_FLIC_CLEAR_IRQS3
#define KVM_DEV_FLIC_APF_ENABLE4
#define KVM_DEV_FLIC_APF_DISABLE_WAIT 5
+#define KVM_DEV_FLIC_ADAPTER_REGISTER 6
+#define KVM_DEV_FLIC_ADAPTER_MODIFY7
/*
* We can have up to 4*64k pending subchannels + 8 adapter interrupts,
* as well as up to ASYNC_PF_PER_VCPU*KVM_MAX_VCPUS pfault done interrupts.
@@ -32,6 +34,26 @@
#define KVM_S390_MAX_FLOAT_IRQS266250
#define KVM_S390_FLIC_MAX_BUFFER 0x200
+struct kvm_s390_io_adapter {
+ __u32 id;
+ __u8 isc;
+ __u8 maskable;
+ __u8 swap;
+ __u8 pad;
+};
+
+#define KVM_S390_IO_ADAPTER_MASK 1
+#define KVM_S390_IO_ADAPTER_MAP 2
+#define KVM_S390_IO_ADAPTER_UNMAP 3
+
+struct kvm_s390_io_adapter_req {
+ __u32 id;
+ __u8 type;
+ __u8 mask;
+ __u16 pad0;
+ __u64 addr;
+};
+
/* for KVM_GET_REGS and KVM_SET_REGS */
struct kvm_regs {
/* general purpose regs for s390 */
@@ -76,4 +98,6 @@ struct kvm_sync_regs {
#define KVM_REG_S390_PFTOKEN (KVM_REG_S390 | KVM_REG_SIZE_U64 | 0x5)
#define KVM_REG_S390_PFCOMPARE (KVM_REG_S390 | KVM_REG_SIZE_U64 | 0x6)
#define KVM_REG_S390_PFSELECT (KVM_REG_S390 | KVM_REG_SIZE_U64 | 0x7)
+#define KVM_REG_S390_PP(KVM_REG_S390 | KVM_REG_SIZE_U64 | 0x8)
+#define KVM_REG_S390_GBEA (KVM_REG_S390 | KVM_REG_SIZE_U64 | 0x9)
#endif
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index e27a4b3..b278ab3 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -740,6 +740,9 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_SPAPR_MULTITCE 94
#define KVM_CAP_EXT_EMUL_CPUID 95
#define KVM_CAP_HYPERV_TIME 96
+#define KVM_CAP_IOAPIC_POLARITY_IGNORED 97
+#define KVM_CAP_ENABLE_CAP_VM 98
+#define KVM_CAP_S390_IRQCHIP 99
#ifdef KVM_CAP_IRQ_ROUTING
@@ -755,9 +758,18 @@ struct kvm_irq_routing_msi {
__u32 pad;
};
+struct kvm_irq_routing_s390_adapter {
+ __u64 ind_addr;
+ __u64 summary_addr;
+ __u64 ind_offset;
+ __u32 summary_offset;
+ __u32 adapter_id;
+};
+
/* gsi routing entry types */
#define KVM_IRQ_ROUTING_IRQCHIP 1
#define KVM_IRQ_ROUTING_MSI 2
+#define KVM_IRQ_ROUTING_S390_ADAPTER 3
struct kvm_irq_routing_entry {
__u32 gsi;
@@ -767,6 +779,7 @@ struct kvm_irq_routing_entry {
union {
struct kvm_irq_routing_irqchip irqchip;
struct kvm_irq_routing_msi msi;
+ struct kvm_irq_routing_s390_adapter adapter;
__u32 pad[8];
} u;
};
@@ -1075,6 +1088,10 @@ struct kvm_s390_ucas_mapping {
/* Available with KVM_CAP_DEBUGREGS */
#define KVM_GET_DEBUGREGS _IOR(KVMIO, 0xa1, struct kvm_debugregs)
#define KVM_SET_DEBUGREGS _IOW(KVMIO, 0xa2, struct kvm_debugregs)
+/*
+ * vcpu version available with KVM_ENABLE_CAP
+ * vm version available with KVM_CAP_ENABLE_CAP_VM
+ */
#define KVM_ENABLE_CAP_IOW(KVMIO, 0xa3, struct kvm_enable_cap)
/* Available with KVM_CAP_XSAVE */
#define KVM_GET_XSAVE_IOR(KVMIO, 0xa4, struct kvm_xsave)
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 17c58e0..26c218e 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -23,6 +23,12 @@
#define VFIO_TYPE1_IOMMU 1
#define VFIO_SPAPR_TCE_IOMMU 2
+#define VFIO_TYPE1v2_IOMMU 3
+/*
+ * IOMMU enforces DMA cache coherence (ex. PCIe NoSnoop stripping). This
+ * capability is subject to change as groups are added or removed.
+ */
+#define VFIO_DMA_CC_IOMMU 4
/*
* The IOCTL interface is designed for extensibility by embedding the
--
1.7.9.5
[Qemu-devel] [PULL 1/4] s390x: empty function stubs in preparation for __KVM_HAVE_GUEST_DEBUG
From: David Hildenbrand d...@linux.vnet.ibm.com
This patch creates empty function stubs (used by the gdbserver) in preparation
for the hw debugging support by kvm on s390, which will enable the
__KVM_HAVE_GUEST_DEBUG define in the linux headers and require these methods on
the qemu side.
Signed-off-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Cc: qemu-sta...@nongnu.org
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
---
target-s390x/kvm.c | 28
1 file changed, 28 insertions(+)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 56b9af7..2b2dcdc 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -383,6 +383,26 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct
kvm_sw_breakpoint *bp)
return 0;
}
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type)
+{
+return -ENOSYS;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type)
+{
+return -ENOSYS;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+}
+
+void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
+{
+}
+
void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
{
}
@@ -844,6 +864,11 @@ static int handle_tsch(S390CPU *cpu)
return ret;
}
+static int kvm_arch_handle_debug_exit(S390CPU *cpu)
+{
+return -ENOSYS;
+}
+
int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
{
S390CPU *cpu = S390_CPU(cs);
@@ -859,6 +884,9 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
case KVM_EXIT_S390_TSCH:
ret = handle_tsch(cpu);
break;
+case KVM_EXIT_DEBUG:
+ret = kvm_arch_handle_debug_exit(cpu);
+break;
default:
fprintf(stderr, Unknown KVM exit: %d\n, run-exit_reason);
break;
--
1.7.9.5
Re: [Qemu-devel] [PATCH 3/5] gtk: Fix -serial vc
Am 24.04.2014 19:35, schrieb Cole Robinson:
Try kicking off a rhel5 text install over serial, the text menu navigation
is all messed up, and some of the kernel boot messages are randomly
corrupted.
Drop use of a pty and just use vte infrastructure for reading and writing.
This fixes the above corruption, and is simpler to boot.
(I don't know what was wrong with the original code though. FWIW this is
what virt-manager has done for years).
Signed-off-by: Cole Robinson crobi...@redhat.com
---
ui/gtk.c | 41 +
1 file changed, 9 insertions(+), 32 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 816ef15..117b0eb 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
[...]
@@ -1194,18 +1196,11 @@ void early_gtk_display_init(void)
}
#if defined(CONFIG_VTE)
-static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
+static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
+ gpointer user_data)
{
-VirtualConsole *vc = opaque;
-uint8_t buffer[1024];
-ssize_t len;
-
-len = read(vc-fd, buffer, sizeof(buffer));
-if (len = 0) {
-return FALSE;
-}
-
-qemu_chr_be_write(vc-chr, buffer, len);
+VirtualConsole *vc = user_data;
Gerd, can we retain the white line here please? :)
Andreas
+qemu_chr_be_write(vc-chr, (uint8_t *)text, (unsigned int)size);
return TRUE;
}
[snip]
--
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 9/9] usb: mtp: reply INCOMPLETE_TRANSFER on read errors
Hi, The bogus data packet is sent with usb_packet_copy, shouldn't you return USB_RET_NAK for now? I don't think so. The transfer must be completed, even if we don't send valid data, because the guest expects a certain number of data bytes before the result packet with the status code. If we don't send them we are out of sync. We might memset(d-data, 0, dlen) so the guest gets zeros instead of the data block from the last successful read. The guest is supposed to discard the data though, so it should not be needed. Ahem, well, realloc doesn't clear memory, so I guess we better _do_ memset, so we don't leak random qemu memory to the guest in case the very first read call fails. I'll fix it up. cheers, Gerd
Re: [Qemu-devel] [PATCH 3/3] s390x/helper: Added format control bit to MMU translation
Hi Alex,
On Thu, 24 Apr 2014 18:36:18 +0200
Alexander Graf ag...@suse.de wrote:
On 24.04.14 17:34, Jens Freimann wrote:
From: Thomas Huth th...@linux.vnet.ibm.com
With the EDAT-1 facility, the MMU translation can stop at the
segment table already, pointing to a 1 MB block.
...
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index aa628b8..89dc6e7 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -217,6 +217,10 @@ static int mmu_translate_asce(CPUS390XState *env,
target_ulong vaddr,
offs = (vaddr 17) 0x3ff8;
break;
case _ASCE_TYPE_SEGMENT:
+if (env (env-cregs[0] CR0_EDAT) (asce
_SEGMENT_ENTRY_FC)) {
+*raddr = (asce 0xfff0ULL) | (vaddr 0xf);
+return 0;
+}
This is missing the page flags.
D'oh, missed that :-(
I also think we should rather align the
code with the PTE handling somehow. This way it gets pretty confusing to
follow. How about something like this (untested)?
I gave it a try ... works fine with two small modifications...
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index aa628b8..96c1c66 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -170,6 +170,48 @@ static void trigger_page_fault(CPUS390XState *env,
target_ulong vaddr,
trigger_pgm_exception(env, type, ilen);
}
+static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
+ uint64_t asc, uint64_t asce,
+ target_ulong *raddr, int *flags, int rw)
+{
+if (asce _PAGE_INVALID) {
+DPRINTF(%s: PTE=0x% PRIx64 invalid\n, __func__, asce);
+trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
+return -1;
+}
+
+if (asce _PAGE_RO) {
+*flags = ~PAGE_WRITE;
+}
+
+*raddr = asce _ASCE_ORIGIN;
+
+PTE_DPRINTF(%s: PTE=0x% PRIx64 \n, __func__, asce);
+
+return 0;
+}
+
+static int mmu_translate_segpte(CPUS390XState *env, target_ulong vaddr,
+uint64_t asc, uint64_t asce,
+ target_ulong *raddr, int *flags, int rw)
+{
+if (asce _SEGMENT_ENTRY_INV) {
+DPRINTF(%s: SEG=0x% PRIx64 invalid\n, __func__, asce);
+trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw);
+return -1;
+}
+
+if (asce _PAGE_RO) { /* XXX is this correct? */
You can remove the XXX comment, the protection bit is the same for
both, page table entries and segment table entries.
+*flags = ~PAGE_WRITE;
+}
+
+*raddr = (asce 0xfff0ULL) | (vaddr 0xf);
+
+PTE_DPRINTF(%s: SEG=0x% PRIx64 \n, __func__, asce);
+
+return 0;
+}
+
static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
uint64_t asc, uint64_t asce, int level,
target_ulong *raddr, int *flags, int rw)
@@ -229,28 +271,19 @@ static int mmu_translate_asce(CPUS390XState *env,
target_ulong vaddr,
PTE_DPRINTF(%s: 0x% PRIx64 + 0x% PRIx64 = 0x%016 PRIx64 \n,
__func__, origin, offs, new_asce);
-if (level != _ASCE_TYPE_SEGMENT) {
+} if (level == _ASCE_TYPE_SEGMENT) {
I had to remove the } at the beginning of above line.
+/* 4KB page */
+return mmu_translate_pte(env, vaddr, asc, new_asce, raddr,
flags, rw);
+} else if (((level - 4) == _ASCE_TYPE_SEGMENT)
+ (env-cregs[0] CR0_EDAT)
+(asce _SEGMENT_ENTRY_FC)) {
That's got to be (new_asce _SEGMENT_ENTRY_FC) instead.
+/* 1MB page */
+return mmu_translate_segpte(env, vaddr, asc, new_asce, raddr,
flags, rw);
+} else {
/* yet another region */
return mmu_translate_asce(env, vaddr, asc, new_asce, level -
4, raddr,
flags, rw);
}
-
-/* PTE */
-if (new_asce _PAGE_INVALID) {
-DPRINTF(%s: PTE=0x% PRIx64 invalid\n, __func__, new_asce);
-trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
-return -1;
-}
-
-if (new_asce _PAGE_RO) {
-*flags = ~PAGE_WRITE;
-}
-
-*raddr = new_asce _ASCE_ORIGIN;
-
-PTE_DPRINTF(%s: PTE=0x% PRIx64 \n, __func__, new_asce);
-
-return 0;
}
static int mmu_translate_asc(CPUS390XState *env, target_ulong vaddr,
I'm fine with these changes, too. So how shall we continue? Could you
assemble a complete patch or shall I prepare one?
Thomas
Re: [Qemu-devel] [PATCH RFC qom-next for-next v2 6/6] pci: Move VMSTATE_MSIX() into vmstate_pci_device
On Apr 16, 2014, at 17:22 PM, Andreas Färber afaer...@suse.de wrote:
Am 02.09.2013 13:31, schrieb Michael S. Tsirkin:
On Mon, Jul 29, 2013 at 02:27:01AM +0200, Andreas Färber wrote:
Use it conditional on msix_present() and drop msix_{save,load}() calls
following pci_device_{save,load}().
This reorders the msix_save() and msix_unuse_all_vectors() calls for
virtio-pci, but they seem independent of each other.
No, that's a bug. msix_unuse_all_vectors clears pending state
if any, it will lose state if called before load.
Michael, you were just saying no here, now MegaSAS is forgetting to add
appropriate VMState. How do you envision solving that bug? Can we move
msix_unuse_all_vectors() to common code or something?
FTR, Stefan had requested on IRC that vmxnet state not be changed
incompatibly. What we discussed there was to register the legacy savevm
handler only for reading incoming state (NULL for writing new state);
but I am no longer sure that could work due to new VMSTATE_PCI().
Dmitry, why is vmxnet using such a non-standard way of registering
VMState for MSI-X, and can you please help to fix that for the benefit
of all PCI devices?
It was a log time ago so I don’t really remember. Probably got the template
from some other device’s code.
Sure, I’ll put this to my queue.
Thanks,
Andreas
Signed-off-by: Andreas Färber afaer...@suse.de
---
hw/misc/ivshmem.c | 7 ++-
hw/net/vmxnet3.c | 27 +++
hw/pci/pci.c | 8
hw/usb/hcd-xhci.c | 1 -
hw/virtio/virtio-pci.c | 19 +++
include/hw/pci/msix.h | 7 ---
6 files changed, 28 insertions(+), 41 deletions(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 4a74856..de997cd 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -599,9 +599,7 @@ static void ivshmem_save(QEMUFile* f, void *opaque)
IVSHMEM_DPRINTF(ivshmem_save\n);
pci_device_save(pci_dev, f);
-if (ivshmem_has_feature(proxy, IVSHMEM_MSI)) {
-msix_save(pci_dev, f);
-} else {
+if (!ivshmem_has_feature(proxy, IVSHMEM_MSI)) {
qemu_put_be32(f, proxy-intrstatus);
qemu_put_be32(f, proxy-intrmask);
}
@@ -631,8 +629,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int
version_id)
}
if (ivshmem_has_feature(proxy, IVSHMEM_MSI)) {
-msix_load(pci_dev, f);
- ivshmem_use_msix(proxy);
+ivshmem_use_msix(proxy);
} else {
proxy-intrstatus = qemu_get_be32(f);
proxy-intrmask = qemu_get_be32(f);
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 3bad83c..471ca24 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2031,21 +2031,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
}
}
-static void
-vmxnet3_msix_save(QEMUFile *f, void *opaque)
-{
-PCIDevice *d = PCI_DEVICE(opaque);
-msix_save(d, f);
-}
-
-static int
-vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
-{
-PCIDevice *d = PCI_DEVICE(opaque);
-msix_load(d, f);
-return 0;
-}
-
static const MemoryRegionOps b0_ops = {
.read = vmxnet3_io_bar0_read,
.write = vmxnet3_io_bar0_write,
@@ -2103,9 +2088,6 @@ static int vmxnet3_pci_init(PCIDevice *pci_dev)
vmxnet3_net_init(s);
-register_savevm(dev, vmxnet3-msix, -1, 1,
-vmxnet3_msix_save, vmxnet3_msix_load, s);
-
add_boot_device_path(s-conf.bootindex, dev, /ethernet-phy@0);
return 0;
@@ -2114,13 +2096,10 @@ static int vmxnet3_pci_init(PCIDevice *pci_dev)
static void vmxnet3_pci_uninit(PCIDevice *pci_dev)
{
-DeviceState *dev = DEVICE(pci_dev);
VMXNET3State *s = VMXNET3(pci_dev);
VMW_CBPRN(Starting uninit...);
-unregister_savevm(dev, vmxnet3-msix, s);
-
vmxnet3_net_uninit(s);
vmxnet3_cleanup_msix(s);
@@ -2372,9 +2351,9 @@ const VMStateInfo int_state_info = {
static const VMStateDescription vmstate_vmxnet3 = {
.name = vmxnet3,
-.version_id = 1,
-.minimum_version_id = 1,
-.minimum_version_id_old = 1,
+.version_id = 2,
+.minimum_version_id = 2,
+.minimum_version_id_old = 2,
.pre_save = vmxnet3_pre_save,
.post_load = vmxnet3_post_load,
.fields = (VMStateField[]) {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b790d98..bd6078b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -481,6 +481,13 @@ static bool pci_device_aer_needed(void *opaque, int
version_id)
return pci_is_express(s) s-exp.aer_log.log != NULL;
}
+static bool pci_device_msix_needed(void *opaque, int version_id)
+{
+PCIDevice *s = opaque;
+
+return msix_present(s);
+}
+
const VMStateDescription vmstate_pci_device = {
.name = PCIDevice,
.version_id = 2,
@@ -499,6 +506,7 @@ const VMStateDescription vmstate_pci_device = {
VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
vmstate_info_pci_irq_state,
[Qemu-devel] [PULL 2/2] linux-user: Add support for SCM_CREDENTIALS.
Signed-off-by: Huw Davies h...@codeweavers.com
---
linux-user/syscall.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index cf4372e..e9ae9c5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1269,6 +1269,17 @@ static inline abi_long host_to_target_cmsg(struct
target_msghdr *target_msgh,
target_tv-tv_usec = tswapal(tv-tv_usec);
break;
}
+case SCM_CREDENTIALS:
+{
+struct ucred *cred = (struct ucred *)data;
+struct target_ucred *target_cred =
+(struct target_ucred *)target_data;
+
+__put_user(cred-pid, target_cred-pid);
+__put_user(cred-uid, target_cred-uid);
+__put_user(cred-gid, target_cred-gid);
+break;
+}
default:
goto unimplemented;
}
--
1.8.0
[Qemu-devel] [PULL 0/2] linux-user: Add support for SCM_CREDENTIALS.
This set has been on the mailing list unreviewed for more than a week. It implements copying of the SCM_CREDENTIALS msg payload. Patch 1 changes an if-else block to a more expandable switch block. Patch 2 is the actual implementation. The following changes since commit 0e96643c98eb22a5f2e11ac500852133032d38e4: Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-5' into staging (2014-04-24 16:16:57 +0100) are available in the git repository at: git://github.com/hdmdavies/qemu.git scm_credentials for you to fetch changes up to ddbcd08d0f0f6c008c83bab58d9f2a831565c9dd: linux-user: Add support for SCM_CREDENTIALS. (2014-04-25 12:32:40 +0100) Huw Davies (2): linux-user: Move if-elses to a switch statement. linux-user: Add support for SCM_CREDENTIALS. linux-user/syscall.c | 62 +--- 1 file changed, 44 insertions(+), 18 deletions(-)
[Qemu-devel] [PULL 1/2] linux-user: Move if-elses to a switch statement.
This makes adding more message types cleaner.
Signed-off-by: Huw Davies h...@codeweavers.com
---
linux-user/syscall.c | 51 +--
1 file changed, 33 insertions(+), 18 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9864813..cf4372e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1242,25 +1242,40 @@ static inline abi_long host_to_target_cmsg(struct
target_msghdr *target_msgh,
target_cmsg-cmsg_type = tswap32(cmsg-cmsg_type);
target_cmsg-cmsg_len = tswapal(TARGET_CMSG_LEN(len));
-if ((cmsg-cmsg_level == SOL_SOCKET)
-(cmsg-cmsg_type == SCM_RIGHTS)) {
-int *fd = (int *)data;
-int *target_fd = (int *)target_data;
-int i, numfds = len / sizeof(int);
+switch (cmsg-cmsg_level) {
+case SOL_SOCKET:
+switch (cmsg-cmsg_type) {
+case SCM_RIGHTS:
+{
+int *fd = (int *)data;
+int *target_fd = (int *)target_data;
+int i, numfds = len / sizeof(int);
-for (i = 0; i numfds; i++)
-target_fd[i] = tswap32(fd[i]);
-} else if ((cmsg-cmsg_level == SOL_SOCKET)
-(cmsg-cmsg_type == SO_TIMESTAMP)
-(len == sizeof(struct timeval))) {
-/* copy struct timeval to target */
-struct timeval *tv = (struct timeval *)data;
-struct target_timeval *target_tv =
-(struct target_timeval *)target_data;
-
-target_tv-tv_sec = tswapal(tv-tv_sec);
-target_tv-tv_usec = tswapal(tv-tv_usec);
-} else {
+for (i = 0; i numfds; i++)
+target_fd[i] = tswap32(fd[i]);
+break;
+}
+case SO_TIMESTAMP:
+{
+struct timeval *tv = (struct timeval *)data;
+struct target_timeval *target_tv =
+(struct target_timeval *)target_data;
+
+if (len != sizeof(struct timeval))
+goto unimplemented;
+
+/* copy struct timeval to target */
+target_tv-tv_sec = tswapal(tv-tv_sec);
+target_tv-tv_usec = tswapal(tv-tv_usec);
+break;
+}
+default:
+goto unimplemented;
+}
+break;
+
+default:
+unimplemented:
gemu_log(Unsupported ancillary data: %d/%d\n,
cmsg-cmsg_level, cmsg-cmsg_type);
memcpy(target_data, data, len);
--
1.8.0
Re: [Qemu-devel] [GSoC] Wanted: small warmup tasks
Am 25.04.2014 11:22, schrieb Peter Crosthwaite: On Fri, Apr 25, 2014 at 5:55 PM, Marc Marí marc.mari.barc...@gmail.com wrote: I'm now looking at the conditional fprintf's. I'll need a bit of help later in sending the patches :D. For starters, please use plain text format mails and don't top-post. :) CC me in on the results. Me, too, please. If you search the archives, you will find that I already tried this for target-* and there were a lot of discussions surrounding DOs and DON'Ts. https://github.com/afaerber/qemu-cpu/commits/dprintf is still in a state I was instructed to, but that was later considered too performance-intrusive due to variable usage. In particular I believe we concluded that unlike Peter's pseudocode we need separate preprocessor macro naming between how the user enables it and how we check it inside the macro. Also macros should not end in a semicolon. Regards, Andreas P.S. Here probably didn't strictly mean replying to Jan's message. ;) -- 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] [PULL 3/4] s390x/kvm: rework KVM synchronize to tracing for some ONEREGS
On 25.04.14 14:02, Cornelia Huck wrote: From: Christian Borntraeger borntrae...@de.ibm.com Some ONE_REGS on s390 are not protected by a capability. Older kernels might not provide those and return an error. Fortunately these registers are only critical for the migration path. There is no need to error out on reset and normal runtime. Furthermore, these kernels don't provide a proper dirty bitmap anyway, so let's use tracing for those errors. Also provide generic one reg helper to simplify the code. Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com CC stable? Alex
Re: [Qemu-devel] [PATCH 3/5] gtk: Fix -serial vc
-qemu_chr_be_write(vc-chr, buffer, len); +VirtualConsole *vc = user_data; Gerd, can we retain the white line here please? :) Andreas +qemu_chr_be_write(vc-chr, (uint8_t *)text, (unsigned int)size); Fixed in my local copy. cheers, Gerd
Re: [Qemu-devel] [PULL 3/4] s390x/kvm: rework KVM synchronize to tracing for some ONEREGS
On 25.04.14 14:24, Christian Borntraeger wrote: On 25/04/14 14:04, Alexander Graf wrote: On 25.04.14 14:02, Cornelia Huck wrote: From: Christian Borntraeger borntrae...@de.ibm.com Some ONE_REGS on s390 are not protected by a capability. Older kernels might not provide those and return an error. Fortunately these registers are only critical for the migration path. There is no need to error out on reset and normal runtime. Furthermore, these kernels don't provide a proper dirty bitmap anyway, so let's use tracing for those errors. Also provide generic one reg helper to simplify the code. Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com CC stable? Makes sense. get_registers is fine as the ONE_REGS are at the end of the function, but put_registers will not store the SREGS and the prefix in that case. I guess its ok to send the patch and the commit id to qemu stable after this hits master? The normal process is that the maintainer adds a CC: line to the commit when he puts it into his queue. That way git-send-email automatically CCs stable and there's proper documentation of this in the master tree. But of course doing it manually also works. Alex
Re: [Qemu-devel] [PATCH v25 10/31] QemuOpts: check NULL input for qemu_opts_del
On Fri, Apr 11, 2014 at 01:54:06AM +0800, Chunyan Liu wrote: To simplify later using of qemu_opts_del, accept NULL input. Reviewed-by: Eric Blake ebl...@redhat.com Signed-off-by: Chunyan Liu cy...@suse.com --- util/qemu-option.c | 4 1 file changed, 4 insertions(+) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [PULL 00/16] Block patches
On 23 April 2014 11:04, Kevin Wolf kw...@redhat.com wrote: The following changes since commit 2d03b49c3f225994c4b0b46146437d8c887d6774: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140417-1' into staging (2014-04-17 21:37:26 +0100) are available in the git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 370e681629da587af7592a7b83ebc7ec4955461e: block/cloop: use PRIu32 format specifier for uint32_t (2014-04-23 11:34:10 +0200) Applied, thanks. -- PMM
Re: [Qemu-devel] [PULL 3/4] s390x/kvm: rework KVM synchronize to tracing for some ONEREGS
On 25/04/14 14:04, Alexander Graf wrote: On 25.04.14 14:02, Cornelia Huck wrote: From: Christian Borntraeger borntrae...@de.ibm.com Some ONE_REGS on s390 are not protected by a capability. Older kernels might not provide those and return an error. Fortunately these registers are only critical for the migration path. There is no need to error out on reset and normal runtime. Furthermore, these kernels don't provide a proper dirty bitmap anyway, so let's use tracing for those errors. Also provide generic one reg helper to simplify the code. Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com CC stable? Makes sense. get_registers is fine as the ONE_REGS are at the end of the function, but put_registers will not store the SREGS and the prefix in that case. I guess its ok to send the patch and the commit id to qemu stable after this hits master? Christian
Re: [Qemu-devel] [PATCH] mirror: Check for bdrv_get_info result
On Fri, Apr 25, 2014 at 02:50:29PM +0800, Fam Zheng wrote:
bdrv_get_info could fail. Add check before using the returned value.
Signed-off-by: Fam Zheng f...@redhat.com
---
block/mirror.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 0ef41f9..fafcc93 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -339,8 +339,8 @@ static void coroutine_fn mirror_run(void *opaque)
bdrv_get_backing_filename(s-target, backing_filename,
sizeof(backing_filename));
if (backing_filename[0] !s-target-backing_hd) {
-bdrv_get_info(s-target, bdi);
-if (s-granularity bdi.cluster_size) {
+ret = bdrv_get_info(s-target, bdi);
+if (ret == 0 s-granularity bdi.cluster_size) {
s-buf_size = MAX(s-buf_size, bdi.cluster_size);
s-cow_bitmap = bitmap_new(length);
}
Please explain why it's okay to ignore errors.
Stefan
Re: [Qemu-devel] [PATCH 3/3] s390x/helper: Added format control bit to MMU translation
On 25.04.14 14:15, Thomas Huth wrote:
Hi Alex,
On Thu, 24 Apr 2014 18:36:18 +0200
Alexander Graf ag...@suse.de wrote:
On 24.04.14 17:34, Jens Freimann wrote:
From: Thomas Huth th...@linux.vnet.ibm.com
With the EDAT-1 facility, the MMU translation can stop at the
segment table already, pointing to a 1 MB block.
...
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index aa628b8..89dc6e7 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -217,6 +217,10 @@ static int mmu_translate_asce(CPUS390XState *env,
target_ulong vaddr,
offs = (vaddr 17) 0x3ff8;
break;
case _ASCE_TYPE_SEGMENT:
+if (env (env-cregs[0] CR0_EDAT) (asce _SEGMENT_ENTRY_FC)) {
+*raddr = (asce 0xfff0ULL) | (vaddr 0xf);
+return 0;
+}
This is missing the page flags.
D'oh, missed that :-(
I also think we should rather align the
code with the PTE handling somehow. This way it gets pretty confusing to
follow. How about something like this (untested)?
I gave it a try ... works fine with two small modifications...
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index aa628b8..96c1c66 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -170,6 +170,48 @@ static void trigger_page_fault(CPUS390XState *env,
target_ulong vaddr,
trigger_pgm_exception(env, type, ilen);
}
+static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
+ uint64_t asc, uint64_t asce,
+ target_ulong *raddr, int *flags, int rw)
+{
+if (asce _PAGE_INVALID) {
+DPRINTF(%s: PTE=0x% PRIx64 invalid\n, __func__, asce);
+trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
+return -1;
+}
+
+if (asce _PAGE_RO) {
+*flags = ~PAGE_WRITE;
+}
+
+*raddr = asce _ASCE_ORIGIN;
+
+PTE_DPRINTF(%s: PTE=0x% PRIx64 \n, __func__, asce);
+
+return 0;
+}
+
+static int mmu_translate_segpte(CPUS390XState *env, target_ulong vaddr,
+uint64_t asc, uint64_t asce,
+ target_ulong *raddr, int *flags, int rw)
+{
+if (asce _SEGMENT_ENTRY_INV) {
+DPRINTF(%s: SEG=0x% PRIx64 invalid\n, __func__, asce);
+trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw);
+return -1;
+}
+
+if (asce _PAGE_RO) { /* XXX is this correct? */
You can remove the XXX comment, the protection bit is the same for
both, page table entries and segment table entries.
+*flags = ~PAGE_WRITE;
+}
+
+*raddr = (asce 0xfff0ULL) | (vaddr 0xf);
+
+PTE_DPRINTF(%s: SEG=0x% PRIx64 \n, __func__, asce);
+
+return 0;
+}
+
static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
uint64_t asc, uint64_t asce, int level,
target_ulong *raddr, int *flags, int rw)
@@ -229,28 +271,19 @@ static int mmu_translate_asce(CPUS390XState *env,
target_ulong vaddr,
PTE_DPRINTF(%s: 0x% PRIx64 + 0x% PRIx64 = 0x%016 PRIx64 \n,
__func__, origin, offs, new_asce);
-if (level != _ASCE_TYPE_SEGMENT) {
+} if (level == _ASCE_TYPE_SEGMENT) {
I had to remove the } at the beginning of above line.
+/* 4KB page */
+return mmu_translate_pte(env, vaddr, asc, new_asce, raddr,
flags, rw);
+} else if (((level - 4) == _ASCE_TYPE_SEGMENT)
+ (env-cregs[0] CR0_EDAT)
+(asce _SEGMENT_ENTRY_FC)) {
That's got to be (new_asce _SEGMENT_ENTRY_FC) instead.
+/* 1MB page */
+return mmu_translate_segpte(env, vaddr, asc, new_asce, raddr,
flags, rw);
+} else {
/* yet another region */
return mmu_translate_asce(env, vaddr, asc, new_asce, level -
4, raddr,
flags, rw);
}
-
-/* PTE */
-if (new_asce _PAGE_INVALID) {
-DPRINTF(%s: PTE=0x% PRIx64 invalid\n, __func__, new_asce);
-trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
-return -1;
-}
-
-if (new_asce _PAGE_RO) {
-*flags = ~PAGE_WRITE;
-}
-
-*raddr = new_asce _ASCE_ORIGIN;
-
-PTE_DPRINTF(%s: PTE=0x% PRIx64 \n, __func__, new_asce);
-
-return 0;
}
static int mmu_translate_asc(CPUS390XState *env, target_ulong vaddr,
I'm fine with these changes, too. So how shall we continue? Could you
assemble a complete patch or shall I prepare one?
You go ahead and do one. If you can think of a good way to combine
mmu_translate_pte and mmu_translate_seg into a single function I'd be
happy to see that too :).
With the RO flag identical the only differences left are the page mask
(4k is implicit for QEMU's TLB, that's why it's omitted for normal PTEs)
and the page fault injection. Do 1MB PTEs fault with a page fault or a
segment fault? If they fault with a page fault, it's probably the best
to
Re: [Qemu-devel] [PATCH v2 1/2] qapi: fix coding style in parameters list
On 04/25/2014 01:56 AM, Amos Kong wrote: The space before point is redundant. Would sound better as: A space after * when declaring a pointer type is redundant. Signed-off-by: Amos Kong ak...@redhat.com --- scripts/qapi-visit.py | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 0/2] linux-user: Add support for SCM_CREDENTIALS.
Am 25.04.2014 14:19, schrieb Huw Davies: This set has been on the mailing list unreviewed for more than a week. It implements copying of the SCM_CREDENTIALS msg payload. Patch 1 changes an if-else block to a more expandable switch block. Patch 2 is the actual implementation. a) Patches being unreviewed does not indicate they are good to go, b) only maintainers send PULLs for QEMU, c) we just did a v2.0.0 release which incurred a Hard Freeze, plus there were Easter holidays in some parts of the world. Rather reply with a ping to your original patches so that they get reviewed and picked up by the appropriate people, whom you are forgetting to CC here (maybe that also explains lack of review?). git config sendemail.cccmd scripts/get_maintainer.pl --nogit-fallback Regards, Andreas -- 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 v2 2/2] qapi: add a special string in c_type()'s result to each redundant space
On 04/25/2014 01:56 AM, Amos Kong wrote: Currently we always add a space after c_type in mcgen(), there is some redundant space in generated code. The space isn't needed for points by the coding style. s/points/pointers/ char * value; ^ qapi_free_NameInfo(NameInfo * obj) ^ This patch added a special string 'EATSPACE' in the end of c_type()'s Present tense for what a patch does: s/added/adds/ result only for point type. The string and the following space will be s/point type/pointer types/ striped in mcgen(). s/striped/stripped/ Signed-off-by: Amos Kong ak...@redhat.com --- scripts/qapi-commands.py | 2 +- scripts/qapi.py | 11 ++- 2 files changed, 7 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v25 30/31] cleanup QEMUOptionParameter
On Fri, Apr 11, 2014 at 01:54:26AM +0800, Chunyan Liu wrote: Now that all backend drivers are using QemuOpts, remove all QEMUOptionParameter related codes. Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com Signed-off-by: Chunyan Liu cy...@suse.com --- block.c | 86 ++ block/cow.c | 4 +- block/gluster.c | 8 +- block/iscsi.c | 2 +- block/nfs.c | 2 +- block/qcow.c | 4 +- block/qcow2.c | 6 +- block/qed.c | 4 +- block/raw-posix.c | 10 +- block/raw-win32.c | 2 +- block/raw_bsd.c | 4 +- block/rbd.c | 2 +- block/sheepdog.c | 6 +- block/ssh.c | 2 +- block/vdi.c | 2 +- block/vhdx.c | 4 +- block/vmdk.c | 6 +- block/vpc.c | 2 +- block/vvfat.c | 12 +- include/block/block.h | 8 +- include/block/block_int.h | 16 +- include/qemu/option.h | 48 +- qemu-img.c| 19 +-- util/qemu-option.c| 426 +- 24 files changed, 68 insertions(+), 617 deletions(-) In block/gluster.c s/\OPT_STRING/QEMU_OPT_STRING/g: CCblock/gluster.o block/gluster.c:701:21: error: ‘OPT_STRING’ undeclared here (not in a function) .type = OPT_STRING,
Re: [Qemu-devel] 答复: report a suspect bug about arm gic
On Wed, Apr 16, 2014 at 02:46:52AM +, zhuxiaodong wrote:
I don't doubt about the algorithm of level interrupts.
The problem may come from the level value tested by GIC_TEST_LEVEL(irq, cm).
It is set in gic_set_irq_generic():
115 static void gic_set_irq_generic(GICState *s, int irq, int level,
116 int cm, int target)
117 {
118 if (level) {
119 GIC_SET_LEVEL(irq, cm);
120 DPRINTF(Set %d pending mask %x\n, irq, target);
121 if (GIC_TEST_EDGE_TRIGGER(irq)) {
122 GIC_SET_PENDING(irq, target);
123 }
124 } else {
125 GIC_CLEAR_LEVEL(irq, cm);
126 }
127 }
At line 119 we can see that it is always set to cm.
Actually the bits of the cm is or'ed onto the level bitmask.
And gic_set_irq_generic is called by gic_set_irq (void *opaque, int irq, int
level):
132 static void gic_set_irq(void *opaque, int irq, int level)
133 {
134 /* Meaning of the 'irq' parameter:
135 * [0..N-1] : external interrupts
136 * [N..N+31] : PPI (internal) interrupts for CPU 0
137 * [N+32..N+63] : PPI (internal interrupts for CPU 1
138 * ...
139 */
140 GICState *s = (GICState *)opaque;
141 int cm, target;
142 if (irq (s-num_irq - GIC_INTERNAL)) {
143 /* The first external input line is internal interrupt 32. */
144 cm = ALL_CPU_MASK;
145 irq += GIC_INTERNAL;
146 target = GIC_TARGET(irq);
At line 144 we can see that cm is always set to ALL_CPU_MASK if irq is a SPI.
That means GIC_TEST_LEVEL can success for any cpu!
So when GIC_TEST_LEVEL(irq,cm) is invoked by gic_update, at the first loop
for cpu 0, the irq will be choosen ahead of time even if cpu 0 is not the
target cpu.
Please consider about this case.
I think what you're trying to say is that gic_set_irq_...() does not
respect settings in the GICD_ITARGETSR (which is what GIC_TARGET(irq)
checks for). In fact I don't think it should, but we should check the
GICD_ITARGETSR value in gic_update to comply with the architecture
specification that states that changes to GICD_ITARGETSR have an effect
on pending interrupts (but not on active, or on active and pending
interrupts until the status is cleared on the latter).
I haven't checked the 11mpcore or the nvic docs yet on how the behavior
should be for those.
-Christoffer
