[Qemu-devel] Problem with allocation of big files

2011-09-20 Thread Nikita A Menkovich
Hello,

I found a strange problem with allocationg big files on drive.

For example in guestfish I allocate large disk image
for example:

$ guestfish
> allocate test.img 20G

When an image allocating, there is a big slowdown of guest OSes,
launched on host machine, and on different drives.

For Linux guests with virtio drivers, there is no so big performance
penalty, but for FreeBSD guests with ide it seems like the OS locked.

Manipulating with ionice and niceness do not affect at all.

Manipulating with cfq, deadline schedulers also do not have any results.

OS Debian Squeeze
libvirtd (libvirt) 0.9.2
QEMU emulator version 0.14.0 (Debian 0.14.0+dfsg-5.1), Copyright (c)
2003-2008 Fabrice Bellard
guestfish 1.10.3 with some local patches, backported from main tree

Does anyone have ideas how to fix it?

-- 
Nikita A Menkovich
http://libc6.org/
JID: menkov...@gmail.com



Re: [Qemu-devel] [PATCH] rbd: allow escaping in config string

2011-09-20 Thread Kevin Wolf
Am 19.09.2011 22:35, schrieb Sage Weil:
> The config string is variously delimited by =, @, and /, depending on the
> field.  Allow these characters to be escaped by preceeding them with \.
> 
> Signed-off-by: Sage Weil 

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] [PATCH 1/3] trace: Fix print format for "mipsnet_write"

2011-09-20 Thread Stefan Hajnoczi
On Fri, Sep 16, 2011 at 06:59:33PM +0200, Lluís Vilanova wrote:
> diff --git a/trace-events b/trace-events
> index c09399f..9d1fbbb 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -454,7 +454,7 @@ milkymist_vgafb_memory_write(uint32_t addr, uint32_t 
> value) "addr %08x value %08
>  mipsnet_send(uint32_t size) "sending len=%u"
>  mipsnet_receive(uint32_t size) "receiving len=%u"
>  mipsnet_read(uint64_t addr, uint32_t val) "read addr=0x%" PRIx64 " val=0x%x"
> -mipsnet_write(uint64_t addr, uint64_t val) "write addr=0x%" PRIx64 " 
> val=0x%" PRIx64
> +mipsnet_write(uint64_t addr, uint64_t val) "write addr=0x%" PRIx64 " 
> val=0x%" PRIx64 ""

This limitation of the tracetool parser has been removed in commit
913540a376f01569227b9a6480a2e96ae9d1d406.

Stefan



Re: [Qemu-devel] [PATCH] isapc: give system address space when pci is disabled

2011-09-20 Thread Jan Kiszka
On 2011-09-19 20:49, Hervé Poussineau wrote:
> Jan Kiszka a écrit :
>> On 2011-09-18 18:04, Hervé Poussineau wrote:
>>> Signed-off-by: Hervé Poussineau 
>>> ---
>>>  hw/pc_piix.c |2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>>> index da6fa55..c0b8a3a 100644
>>> --- a/hw/pc_piix.c
>>> +++ b/hw/pc_piix.c
>>> @@ -121,7 +121,7 @@ static void pc_init1(MemoryRegion *system_memory,
>>>  pc_memory_init(system_memory,
>>> kernel_filename, kernel_cmdline, initrd_filename,
>>> below_4g_mem_size, above_4g_mem_size,
>>> -   pci_memory, &ram_memory);
>>> +   pci_enabled ? pci_memory : system_memory, 
>>> &ram_memory);
>>>  }
>>>  
>>>  if (!xen_enabled()) {
>>
>> Makes sense. I guess we should rename 'pci_memory' inside pc_memory_init
>> to reflect this - 'rom_memory'?
>>
>> But -M isapc still only gives me a black screen. Are there further fixes
>> required?
>>
> 
> Yes, you also need to rollback pc-bios/bios.bin to previous version (ie 
> 8b06c62ae48b67b320f7420dcd4854c5559e1532)

Did you already try to bisect that probable regression in SeaBIOS?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 00/14] qdev: assign unique names to all devices (part 1)

2011-09-20 Thread Jan Kiszka
On 2011-09-19 16:24, Kevin Wolf wrote:
> Am 19.09.2011 16:05, schrieb Anthony Liguori:
>> On 09/19/2011 02:26 AM, Jan Kiszka wrote:
>>> On 2011-09-16 20:03, Anthony Liguori wrote:
 So this is a simplification that I plan on running with.  For now, I think 
 this
 series is the right next step because it gives us a path name for the name
 (although different syntax) and let's us enforce that all devices has a
 canonical path.
>>>
>>> For something that changes lots of devices and, at the same time, is
>>> going to be removed again, I'm hesitating to call it the right direction.
>>>
>>> A right step would be, IMHO, to introduce a generic introspectable
>>> device link so that parent devices can reference their children and a
>>> visitor can derive a child's relative name from that link name. Then
>>> make sure this link type is consistently used.
>>>
>>> I really dislike this focusing on assigning names internally and using
>>> them in QEMU-internal APIs. They should just fall out of the core when
>>> external interaction is required.
>>
>> I thought a lot about this over the weekend and decided that I should go in 
>> a 
>> different direction based on this discussion.
>> [...]
> 
> Sounds like a good direction to me. (At least until someone brings up
> the details :-))

Yeah. Maybe we should keep this at concept level to avoid getting into
disagreement again. ;)

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

2011-09-20 Thread hkran

On 09/09/2011 05:37 PM, Stefan Hajnoczi wrote:

The 'simple' trace backend uses pthreads and does not work on Windows.  These
patches switch from pthreads to glib so that the code builds on all platforms
supported by glib.

Only one thing I'm unhappy about: the simpletrace write-out thread used to
block all signals.  I have removed that code and don't expect glib to do it for
me.  I'm not sure if there is a problem if signal handlers are invoked in the
write-out thread instead of a QEMU thread.  Any thoughts?

Stefan Hajnoczi (2):
   trace: portable simple trace backend using glib
   trace: use binary file open mode in simpletrace

  trace/simple.c |   58 ++-
  1 files changed, 27 insertions(+), 31 deletions(-)


Stefan,

I applied the patch and make &install it.

After a round of running of the qemu with the patch, a trace file is 
here, but when I want to open it like this,

./simpletrace.py trace-events trace-29948//trace-29948 is my tracefile
 an error occurs:

Traceback (most recent call last):
  File "./simpletrace.py", line 151, in 
run(Formatter())
  File "./simpletrace.py", line 131, in run
events = parse_events(open(sys.argv[1], 'r'))
IOError: [Errno 2] No such file or directory: 'trace-events'

Am I using it in a right way?

Additionally, There is something about WIN32 in patch, How can I compile 
a qemu running on windows? Could you give a reference? thanks





Re: [Qemu-devel] [PATCH 0/6] QED block conversion

2011-09-20 Thread Kevin Wolf
Am 12.09.2011 16:47, schrieb Devin Nakamura:
> This patch series adds support for block conversion to the qed driver.
> This depends on my precivious block conversion api series
> Devin Nakamura (6):
>   qed: add qed_find_cluster_sync()
>   qed: add bdrv_qed_get_conversion_options()
>   qed: add open_conversion_target()
>   qed: add qed_bdrv_get_mapping()
>   qed: add bdrv_qed_map()
>   qed: add bdrv_qed_copy_header()
> 
>  block/qed-cluster.c |   33 +++
>  block/qed.c |  161 
> +++
>  block/qed.h |4 +
>  3 files changed, 198 insertions(+), 0 deletions(-)

Stefan, can you have a look at this series? I'm reviewing it right now,
but I'm not that familiar with the details of the QED implementation.

Devin, in the previous series there were two patches that I had
commented on and that need a new version. For now I'll push everything
that has been reviewed into a separate branch in my git repo [1], and
flag patches with unadressed comments with "(NEEDS FIX)" in the subject.
I hope this will help us with managing the whole set of patch series.

Kevin

[1]
http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/inplace-conversion



Re: [Qemu-devel] [PATCH 3/6] qed: add open_conversion_target()

2011-09-20 Thread Kevin Wolf
Am 12.09.2011 16:47, schrieb Devin Nakamura:
> Signed-off-by: Devin Nakamura 
> ---
>  block/qed.c |   57 +
>  1 files changed, 57 insertions(+), 0 deletions(-)
> 
> diff --git a/block/qed.c b/block/qed.c
> index 16320f5..93827db 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -1456,6 +1456,62 @@ static int 
> bdrv_qed_get_conversion_options(BlockDriverState *bs,
>  return 0;
>  }
>  
> +static int bdrv_qed_open_conversion_target(BlockDriverState *bs,
> +   BlockConversionOptions 
> *drv_options,
> +   QEMUOptionParameter *usr_options,
> +   bool force)
> +{
> +BDRVQEDState *s = bs->opaque;
> +s->bs = bs;
> +if (drv_options->encryption_type != BLOCK_CRYPT_NONE) {
> +error_report("Encryption not supported");
> +return -ENOTSUP;
> +}
> +if(drv_options->nb_snapshots && !force) {
> +error_report("Snapshots are not supported");
> +return -ENOTSUP;
> +}
> +s->header.magic = QED_MAGIC;
> +s->header.table_size = QED_DEFAULT_TABLE_SIZE;
> +if(qed_is_cluster_size_valid(drv_options->cluster_size)) {
> +s->header.cluster_size = drv_options->cluster_size;
> +} else {
> +error_report("Invalid cluster size");
> +return -EINVAL;
> +}
> +if(qed_is_image_size_valid(drv_options->image_size, 
> s->header.cluster_size,
> +   s->header.table_size)) {
> +s->header.image_size = drv_options->image_size;
> +} else {
> +error_report("Invalid image size");
> +return -EINVAL;
> +}
> +s->file_size = qed_Start_of_cluster(s, bs->file->total_sectors +
> +drv_options->cluster_size -1);

This doesn't look quite right. With the capital S in
qemu_Start_of_cluster it actually shouldn't even compile.

drv->options->cluster_size is in bytes, but bs->file->total_sectors is
in sectors. I don't think the sum of them makes a lot of sense.

> +s->l1_table = qed_alloc_table(s);
> +s->header.l1_table_offset = qed_alloc_clusters(s, s->header.table_size);
> +QSIMPLEQ_INIT(&s->allocating_write_reqs);
> +
> +
> +if (!qed_check_table_offset(s, s->header.l1_table_offset)) {
> +error_report("Invalid L1 table offset");
> +return -EINVAL;

This leaks s->l1_table.

> +}
> +
> +s->table_nelems = (s->header.cluster_size * s->header.table_size) /
> +  sizeof(uint64_t);
> +s->l2_shift = ffs(s->header.cluster_size) - 1;
> +s->l2_mask = s->table_nelems - 1;
> +s->l1_shift = s->l2_shift + ffs(s->table_nelems) - 1;
> +
> +qed_init_l2_cache(&s->l2_cache);
> +
> +s->need_check_timer = qemu_new_timer_ns(vm_clock,
> +qed_need_check_timer_cb, s);
> +qed_write_l1_table_sync(s, 0, s->table_nelems);
> +return 0;
> +}
> +
>  static QEMUOptionParameter qed_create_options[] = {
>  {
>  .name = BLOCK_OPT_SIZE,
> @@ -1503,6 +1559,7 @@ static BlockDriver bdrv_qed = {
>  .bdrv_change_backing_file = bdrv_qed_change_backing_file,
>  .bdrv_check   = bdrv_qed_check,
>  .bdrv_get_conversion_options = bdrv_qed_get_conversion_options,
> +.bdrv_open_conversion_target = bdrv_qed_open_conversion_target,
>  };
>  
>  static void bdrv_qed_init(void)

Another question that came to my mind while reviewing this - although
mostly unrelated - is where we handle backing files. Is that completely
contained in the generic conversion code?

Kevin



Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

2011-09-20 Thread Stefan Hajnoczi
On Tue, Sep 20, 2011 at 05:05:45PM +0800, hkran wrote:
> On 09/09/2011 05:37 PM, Stefan Hajnoczi wrote:
> >The 'simple' trace backend uses pthreads and does not work on Windows.  These
> >patches switch from pthreads to glib so that the code builds on all platforms
> >supported by glib.
> >
> >Only one thing I'm unhappy about: the simpletrace write-out thread used to
> >block all signals.  I have removed that code and don't expect glib to do it 
> >for
> >me.  I'm not sure if there is a problem if signal handlers are invoked in the
> >write-out thread instead of a QEMU thread.  Any thoughts?
> >
> >Stefan Hajnoczi (2):
> >   trace: portable simple trace backend using glib
> >   trace: use binary file open mode in simpletrace
> >
> >  trace/simple.c |   58 
> > ++-
> >  1 files changed, 27 insertions(+), 31 deletions(-)
> >
> Stefan,
> 
> I applied the patch and make &install it.
> 
> After a round of running of the qemu with the patch, a trace file is
> here, but when I want to open it like this,
> ./simpletrace.py trace-events trace-29948//trace-29948 is my tracefile
>  an error occurs:
> 
> Traceback (most recent call last):
>   File "./simpletrace.py", line 151, in 
> run(Formatter())
>   File "./simpletrace.py", line 131, in run
> events = parse_events(open(sys.argv[1], 'r'))
> IOError: [Errno 2] No such file or directory: 'trace-events'
> 
> Am I using it in a right way?

Looks like your current working directory is scripts/ so simpletrace.py
will be unable to find the trace-events file which is in the parent
directory.

Usually I stay in QEMU's root directory and just run:
$ qemu # ...generate the trace
$ scripts/simpletrace.py trace-events trace-$PID

> Additionally, There is something about WIN32 in patch, How can I
> compile a qemu running on windows? Could you give a reference?

Search for 'mingw' in qemu-doc.texi for instructions.

Stefan



Re: [Qemu-devel] [PATCH 0/6] QED block conversion

2011-09-20 Thread Stefan Hajnoczi
On Tue, Sep 20, 2011 at 10:38:05AM +0200, Kevin Wolf wrote:
> Am 12.09.2011 16:47, schrieb Devin Nakamura:
> > This patch series adds support for block conversion to the qed driver.
> > This depends on my precivious block conversion api series
> > Devin Nakamura (6):
> >   qed: add qed_find_cluster_sync()
> >   qed: add bdrv_qed_get_conversion_options()
> >   qed: add open_conversion_target()
> >   qed: add qed_bdrv_get_mapping()
> >   qed: add bdrv_qed_map()
> >   qed: add bdrv_qed_copy_header()
> > 
> >  block/qed-cluster.c |   33 +++
> >  block/qed.c |  161 
> > +++
> >  block/qed.h |4 +
> >  3 files changed, 198 insertions(+), 0 deletions(-)
> 
> Stefan, can you have a look at this series? I'm reviewing it right now,
> but I'm not that familiar with the details of the QED implementation.

Sure it's in my review queue, will try to get around to it soon.

Stefan



Re: [Qemu-devel] [PATCH] pci: implement bridge filtering

2011-09-20 Thread Wen Congyang
At 09/14/2011 09:48 AM, Wen Congyang Write:
> At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:
>> Support bridge filtering on top of the memory
>> API as suggested by Avi Kivity:
>>
>> Create a memory region for the bridge's address space.  This region is
>> not directly added to system_memory or its descendants.  Devices under
>> the bridge see this region as its pci_address_space().  The region is
>> as large as the entire address space - it does not take into account
>> any windows.
>>
>> For each of the three windows (pref, non-pref, vga), create an alias
>> with the appropriate start and size.  Map the alias into the bridge's
>> parent's pci_address_space(), as subregions.
>>
>> Signed-off-by: Michael S. Tsirkin 
>> ---
>>
>> The below seems to work fine for me so I applied this.
>> Still need to test bridge filtering, any help with this
>> appreciated.
>>
> 
> 
> I test bridge filtering, and the BAR still can be visible on guest even if
> I change the memory region.

Hi Michael S. Tsirkin:
I test pci bridge filtering on real hardware, and I find that I can mmap
the resource after I change the memory base and memory limit(The BAR should
be not visible on OS after changing the memory region).

So I try to write and read to the BAR. Here is my test result:
1. Before changing the pci bridge's memory region, I can read and write to the 
memory, and
   I can get the same value that I write.

2. After changing the pci bridge's memory region, I can still read and write to 
the memory,
   but it is very slow, and I can not get the same value that I write(The value 
is always 0).

Does this result means that pci bridge filtering works fine?

Thanks
Wen Congyang



Re: [Qemu-devel] [PATCH 6/6] qed: add bdrv_qed_copy_header()

2011-09-20 Thread Kevin Wolf
Am 12.09.2011 16:59, schrieb Devin Nakamura:
> Signed-off-by: Devin Nakamura 
> ---
>  block/qed.c |   15 +++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/block/qed.c b/block/qed.c
> index 341cf9d..caecdff 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -1586,6 +1586,20 @@ static int bdrv_qed_map(BlockDriverState *bs, uint64_t 
> guest_offset,
>  return 0;
>  }
>  
> +static int bdrv_qed_copy_header(BlockDriverState *bs, char* filename)
> +{
> +BlockDriverState *backup;
> +uint8_t buffer[512];
> +bdrv_create_file(filename, NULL);
> +bdrv_file_open(&backup, filename, BDRV_O_RDWR);
> +bdrv_read(bs->file, 0, buffer, 1); /*TODO: check return code*/
> +bdrv_write(backup, 0, buffer, 1); /*TODO: check return code*/

Come on, checking return values isn't much more work than adding a TODO
comment.

> +bdrv_close(backup);
> +
> +qed_write_header_sync(bs->opaque);
> +return 0;
> +}

This is probably right, but it makes me wonder if we are sure that QED
never tries to write out the header before bdrv_qed_copy_header() is
called? In that case we would have destroyed the original format before
completing the conversion.

I would feel more comfortable if BDRVQEDState had a field header_offset
that would be set to a temporary location by
bdrv_qed_open_conversion_target and reset to 0 by bdrv_qed_copy_header.

Kevin



Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

2011-09-20 Thread Zhi Yong Wu
On Tue, Sep 20, 2011 at 5:57 PM, Stefan Hajnoczi
 wrote:
> On Tue, Sep 20, 2011 at 05:05:45PM +0800, hkran wrote:
>> On 09/09/2011 05:37 PM, Stefan Hajnoczi wrote:
>> >The 'simple' trace backend uses pthreads and does not work on Windows.  
>> >These
>> >patches switch from pthreads to glib so that the code builds on all 
>> >platforms
>> >supported by glib.
>> >
>> >Only one thing I'm unhappy about: the simpletrace write-out thread used to
>> >block all signals.  I have removed that code and don't expect glib to do it 
>> >for
>> >me.  I'm not sure if there is a problem if signal handlers are invoked in 
>> >the
>> >write-out thread instead of a QEMU thread.  Any thoughts?
>> >
>> >Stefan Hajnoczi (2):
>> >   trace: portable simple trace backend using glib
>> >   trace: use binary file open mode in simpletrace
>> >
>> >  trace/simple.c |   58 
>> > ++-
>> >  1 files changed, 27 insertions(+), 31 deletions(-)
>> >
>> Stefan,
>>
>> I applied the patch and make &install it.
>>
>> After a round of running of the qemu with the patch, a trace file is
>> here, but when I want to open it like this,
>> ./simpletrace.py trace-events trace-29948    //trace-29948 is my tracefile
>>  an error occurs:
>>
>> Traceback (most recent call last):
>>   File "./simpletrace.py", line 151, in 
>>     run(Formatter())
>>   File "./simpletrace.py", line 131, in run
>>     events = parse_events(open(sys.argv[1], 'r'))
>> IOError: [Errno 2] No such file or directory: 'trace-events'
>>
>> Am I using it in a right way?
>
> Looks like your current working directory is scripts/ so simpletrace.py
> will be unable to find the trace-events file which is in the parent
> directory.
>
> Usually I stay in QEMU's root directory and just run:
> $ qemu # ...generate the trace
> $ scripts/simpletrace.py trace-events trace-$PID
I know how to define my own event and play with it now. Very helpful
for me to debug my functions. thanks.

>
>> Additionally, There is something about WIN32 in patch, How can I
>> compile a qemu running on windows? Could you give a reference?
>
> Search for 'mingw' in qemu-doc.texi for instructions.
>
> Stefan
>
>



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] [PATCH 0/2] Make simpletrace work on Windows

2011-09-20 Thread Avi Kivity

On 09/09/2011 12:37 PM, Stefan Hajnoczi wrote:

The 'simple' trace backend uses pthreads and does not work on Windows.  These
patches switch from pthreads to glib so that the code builds on all platforms
supported by glib.

Only one thing I'm unhappy about: the simpletrace write-out thread used to
block all signals.  I have removed that code and don't expect glib to do it for
me.  I'm not sure if there is a problem if signal handlers are invoked in the
write-out thread instead of a QEMU thread.  Any thoughts?



Yes it's a problem when we block a signal completely and only process it 
with sigtimedwait().


--
error compiling committee.c: too many arguments to function




[Qemu-devel] [PATCH] MAINTAINERS: claim maintainership for the OMAP devices

2011-09-20 Thread Peter Maydell
Signed-off-by: Peter Maydell 
---
(1) I've put this in the 'Devices' part of the MAINTAINERS file
because OMAP isn't a specific machine; seemed the best fit
(2) I'll put together a pullreq with this and the other omap
patches later this week or early next.

 MAINTAINERS |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 72b2099..151f393 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -355,6 +355,11 @@ M: Kevin Wolf 
 S: Odd Fixes
 F: hw/ide/
 
+OMAP
+M: Peter Maydell 
+S: Maintained
+F: hw/omap*
+
 PCI
 M: Michael S. Tsirkin 
 S: Supported
-- 
1.7.1




Re: [Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

2011-09-20 Thread Jan Kiszka
On 2011-09-09 11:37, Stefan Hajnoczi wrote:
> Convert the simple trace backend to glib so that it works under Windows.
> We cannot use pthread directly but glib provides portable abstractions.
> Also use glib atomics instead of newish gcc builtins which may not be
> supported on Windows toolchains.

Please avoid restrictive glib thread services. We have qemu_thread
abstractions that allow central tuning (will be needed e.g. to adjust
scheduling parameters).

I'm currently on the way to eliminate remaining pthread users and add
some missing bits to qemu_thread/cond.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] Android ginger bread build up on QEMO x-86

2011-09-20 Thread Singh, Navneet
Hi ,
   How can I port Android -x86 on QEMU. From where I can get the 
gingerbread build and QEMU build to run windows 7 Hp EliteBook 8440p. Please 
also provide the steps for Installation and running the Gingerbread android.

Br
Navneet Singh


Re: [Qemu-devel] [RFC 7/8] cutil: add strocat(), to concat a string to an offset in another

2011-09-20 Thread Paolo Bonzini

On 09/19/2011 04:41 PM, Michael Roth wrote:

+/* concat first 'offset' chars in 'dest' with 'src' */
+char *strocat(char *buf, const char *src, int offset)
+{
+memcpy(&buf[offset], src, strlen(src));
+buf[offset+strlen(src)] = '\0';


This is just strcpy(&buf[offset], src);


+return buf;
+}


Paolo



Re: [Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

2011-09-20 Thread Paolo Bonzini

On 09/20/2011 12:31 PM, Jan Kiszka wrote:

Please avoid restrictive glib thread services. We have qemu_thread
abstractions that allow central tuning (will be needed e.g. to adjust
scheduling parameters).


I think the rationale here was to allow tracing the qemu_thread 
routines.  For your application you can still use ust or systemtap backends.


Paolo




Re: [Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

2011-09-20 Thread Jan Kiszka
On 2011-09-20 12:52, Paolo Bonzini wrote:
> On 09/20/2011 12:31 PM, Jan Kiszka wrote:
>> Please avoid restrictive glib thread services. We have qemu_thread
>> abstractions that allow central tuning (will be needed e.g. to adjust
>> scheduling parameters).
> 
> I think the rationale here was to allow tracing the qemu_thread 
> routines.  For your application you can still use ust or systemtap backends.

OK, if that's a chick-egg thing, this makes sense. Should be documented
then (to avoid someone using this as a general example).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PULL 00/20] Block patches

2011-09-20 Thread Kevin Wolf
The following changes since commit 530889ff95659d8fea81eb556e5706387fdddfa7:

  sun4u: don't set up isa_mem_base (2011-09-18 12:00:19 +)

are available in the git repository at:
  git://repo.or.cz/qemu/kevin.git for-anthony

Alexander Motin (1):
  AHCI Port Interrupt Enable register cleaning on soft reset

Fam Zheng (1):
  VMDK: fix leak of extent_file

Frediano Ziglio (2):
  posix-aio-compat: Removed unused offset variable
  block: avoid SIGUSR2

Kevin Wolf (1):
  raw-posix: Fix bdrv_flush error return values

Paolo Bonzini (11):
  nbd: support feature negotiation
  nbd: sync API definitions with upstream
  nbd: support NBD_SET_FLAGS ioctl
  scsi-generic: do not disable FUA
  dma-helpers: rename is_write to to_dev
  dma-helpers: allow including from target-independent code
  dma-helpers: rewrite completion/cancellation
  scsi-disk: commonize iovec creation between reads and writes
  scsi-disk: lazily allocate bounce buffer
  scsi: fix sign extension problems
  linux-aio: remove process requests callback

Sage Weil (4):
  rbd: ignore failures when reading from default conf location
  rbd: update comment heading
  rbd: call flush, if available
  rbd: allow escaping in config string

 block/nbd.c|4 +-
 block/raw-posix.c  |9 +-
 block/rbd.c|   83 ++
 block/vmdk.c   |   14 ++--
 cpus.c |5 ---
 dma-helpers.c  |   58 +++
 dma.h  |   10 --
 hw/ide/ahci.c  |8 +++--
 hw/ide/core.c  |2 +-
 hw/ide/macio.c |2 +-
 hw/scsi-bus.c  |   22 -
 hw/scsi-disk.c |   84 +++-
 hw/scsi-generic.c  |6 
 linux-aio.c|   11 +--
 nbd.c  |   42 +
 nbd.h  |   20 ++--
 posix-aio-compat.c |   34 +++--
 qemu-nbd.c |   13 
 18 files changed, 254 insertions(+), 173 deletions(-)



[Qemu-devel] [PATCH 02/20] nbd: sync API definitions with upstream

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 nbd.c |2 ++
 nbd.h |   11 ++-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/nbd.c b/nbd.c
index 47ecb22..d32a19e 100644
--- a/nbd.c
+++ b/nbd.c
@@ -67,6 +67,8 @@
 #define NBD_PRINT_DEBUG _IO(0xab, 6)
 #define NBD_SET_SIZE_BLOCKS _IO(0xab, 7)
 #define NBD_DISCONNECT  _IO(0xab, 8)
+#define NBD_SET_TIMEOUT _IO(0xab, 9)
+#define NBD_SET_FLAGS   _IO(0xab, 10)
 
 #define NBD_OPT_EXPORT_NAME (1 << 0)
 
diff --git a/nbd.h b/nbd.h
index b9c3b39..61553f4 100644
--- a/nbd.h
+++ b/nbd.h
@@ -39,11 +39,20 @@ struct nbd_reply {
 
 #define NBD_FLAG_HAS_FLAGS  (1 << 0)/* Flags are there */
 #define NBD_FLAG_READ_ONLY  (1 << 1)/* Device is read-only */
+#define NBD_FLAG_SEND_FLUSH (1 << 2)/* Send FLUSH */
+#define NBD_FLAG_SEND_FUA   (1 << 3)/* Send FUA (Force Unit 
Access) */
+#define NBD_FLAG_ROTATIONAL (1 << 4)/* Use elevator algorithm - 
rotational media */
+#define NBD_FLAG_SEND_TRIM  (1 << 5)/* Send TRIM (discard) */
+
+#define NBD_CMD_MASK_COMMAND   0x
+#define NBD_CMD_FLAG_FUA   (1 << 16)
 
 enum {
 NBD_CMD_READ = 0,
 NBD_CMD_WRITE = 1,
-NBD_CMD_DISC = 2
+NBD_CMD_DISC = 2,
+NBD_CMD_FLUSH = 3,
+NBD_CMD_TRIM = 4
 };
 
 #define NBD_DEFAULT_PORT   10809
-- 
1.7.6.2




[Qemu-devel] [PATCH 03/20] nbd: support NBD_SET_FLAGS ioctl

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

The nbd kernel module cannot enable DISCARD requests unless it is
informed about it.  The flags field in the header is used for this,
and this patch adds support for it.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 nbd.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/nbd.c b/nbd.c
index d32a19e..595f4d8 100644
--- a/nbd.c
+++ b/nbd.c
@@ -378,6 +378,14 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t 
size, size_t blocksize)
 }
 }
 
+if (ioctl(fd, NBD_SET_FLAGS, flags) < 0
+&& errno != ENOTTY) {
+int serrno = errno;
+LOG("Failed setting flags");
+errno = serrno;
+return -1;
+}
+
 TRACE("Clearing NBD socket");
 
 if (ioctl(fd, NBD_CLEAR_SOCK) == -1) {
-- 
1.7.6.2




[Qemu-devel] [PATCH 01/20] nbd: support feature negotiation

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

nbd supports writing flags in bytes 24...27 of the header,
and uses that for the read-only flag.  Add support for it
in qemu-nbd.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 block/nbd.c |4 ++--
 nbd.c   |   32 +---
 nbd.h   |9 ++---
 qemu-nbd.c  |   13 ++---
 4 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 70edd81..76f04d8 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -48,6 +48,7 @@
 
 typedef struct BDRVNBDState {
 int sock;
+uint32_t nbdflags;
 off_t size;
 size_t blocksize;
 char *export_name; /* An NBD server may export several devices */
@@ -111,7 +112,6 @@ static int nbd_establish_connection(BlockDriverState *bs)
 int ret;
 off_t size;
 size_t blocksize;
-uint32_t nbdflags;
 
 if (s->host_spec[0] == '/') {
 sock = unix_socket_outgoing(s->host_spec);
@@ -126,7 +126,7 @@ static int nbd_establish_connection(BlockDriverState *bs)
 }
 
 /* NBD handshake */
-ret = nbd_receive_negotiate(sock, s->export_name, &nbdflags, &size,
+ret = nbd_receive_negotiate(sock, s->export_name, &s->nbdflags, &size,
 &blocksize);
 if (ret == -1) {
 logout("Failed to negotiate with the NBD server\n");
diff --git a/nbd.c b/nbd.c
index 6d81cfb..47ecb22 100644
--- a/nbd.c
+++ b/nbd.c
@@ -30,6 +30,10 @@
 #include 
 #include 
 
+#ifdef __linux__
+#include 
+#endif
+
 #include "qemu_socket.h"
 
 //#define DEBUG_NBD
@@ -172,7 +176,7 @@ int unix_socket_outgoing(const char *path)
   Request (type == 2)
 */
 
-int nbd_negotiate(int csock, off_t size)
+int nbd_negotiate(int csock, off_t size, uint32_t flags)
 {
 char buf[8 + 8 + 8 + 128];
 
@@ -180,14 +184,16 @@ int nbd_negotiate(int csock, off_t size)
 [ 0 ..   7]   passwd   ("NBDMAGIC")
 [ 8 ..  15]   magic(0x00420281861253)
 [16 ..  23]   size
-[24 .. 151]   reserved (0)
+[24 ..  27]   flags
+[28 .. 151]   reserved (0)
  */
 
 TRACE("Beginning negotiation.");
 memcpy(buf, "NBDMAGIC", 8);
 cpu_to_be64w((uint64_t*)(buf + 8), 0x00420281861253LL);
 cpu_to_be64w((uint64_t*)(buf + 16), size);
-memset(buf + 24, 0, 128);
+cpu_to_be32w((uint32_t*)(buf + 24), flags | NBD_FLAG_HAS_FLAGS);
+memset(buf + 28, 0, 124);
 
 if (write_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
 LOG("write failed");
@@ -337,8 +343,8 @@ int nbd_receive_negotiate(int csock, const char *name, 
uint32_t *flags,
 return 0;
 }
 
-#ifndef _WIN32
-int nbd_init(int fd, int csock, off_t size, size_t blocksize)
+#ifdef __linux__
+int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize)
 {
 TRACE("Setting block size to %lu", (unsigned long)blocksize);
 
@@ -358,6 +364,18 @@ int nbd_init(int fd, int csock, off_t size, size_t 
blocksize)
 return -1;
 }
 
+if (flags & NBD_FLAG_READ_ONLY) {
+int read_only = 1;
+TRACE("Setting readonly attribute");
+
+if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) {
+int serrno = errno;
+LOG("Failed setting read-only attribute");
+errno = serrno;
+return -1;
+}
+}
+
 TRACE("Clearing NBD socket");
 
 if (ioctl(fd, NBD_CLEAR_SOCK) == -1) {
@@ -548,7 +566,7 @@ static int nbd_send_reply(int csock, struct nbd_reply 
*reply)
 }
 
 int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
- off_t *offset, bool readonly, uint8_t *data, int data_size)
+ off_t *offset, uint32_t nbdflags, uint8_t *data, int data_size)
 {
 struct nbd_request request;
 struct nbd_reply reply;
@@ -632,7 +650,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size, 
uint64_t dev_offset,
 return -1;
 }
 
-if (readonly) {
+if (nbdflags & NBD_FLAG_READ_ONLY) {
 TRACE("Server is read-only, return error");
 reply.error = 1;
 } else {
diff --git a/nbd.h b/nbd.h
index df7b7af..b9c3b39 100644
--- a/nbd.h
+++ b/nbd.h
@@ -37,6 +37,9 @@ struct nbd_reply {
 uint64_t handle;
 } QEMU_PACKED;
 
+#define NBD_FLAG_HAS_FLAGS  (1 << 0)/* Flags are there */
+#define NBD_FLAG_READ_ONLY  (1 << 1)/* Device is read-only */
+
 enum {
 NBD_CMD_READ = 0,
 NBD_CMD_WRITE = 1,
@@ -53,14 +56,14 @@ int tcp_socket_incoming_spec(const char *address_and_port);
 int unix_socket_outgoing(const char *path);
 int unix_socket_incoming(const char *path);
 
-int nbd_negotiate(int csock, off_t size);
+int nbd_negotiate(int csock, off_t size, uint32_t flags);
 int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
   off_t *size, size_t *blocksize);
-int nbd_init(int fd, int csock, off_t size, size_t blocksize);
+int nbd_init(int fd, int csock, uint32_t flags, of

[Qemu-devel] [PATCH 15/20] rbd: update comment heading

2011-09-20 Thread Kevin Wolf
From: Sage Weil 

Properly document the configuration string syntax and semantics.  Remove
(out of date) details about the librbd implementation.

Signed-off-by: Sage Weil 
Signed-off-by: Kevin Wolf 
---
 block/rbd.c |   28 +---
 1 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index f64b2e0..23d8751 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -13,35 +13,33 @@
 
 #include "qemu-common.h"
 #include "qemu-error.h"
-
 #include "block_int.h"
 
 #include 
 
-
-
 /*
  * When specifying the image filename use:
  *
  * rbd:poolname/devicename[@snapshotname][:option1=value1[:option2=value2...]]
  *
- * poolname must be the name of an existing rados pool
+ * poolname must be the name of an existing rados pool.
  *
- * devicename is the basename for all objects used to
- * emulate the raw device.
+ * devicename is the name of the rbd image.
  *
- * Each option given is used to configure rados, and may be
- * any Ceph option, or "conf". The "conf" option specifies
- * a Ceph configuration file to read.
+ * Each option given is used to configure rados, and may be any valid
+ * Ceph option, "id", or "conf".
  *
- * Metadata information (image size, ...) is stored in an
- * object with the name "devicename.rbd".
+ * The "id" option indicates what user we should authenticate as to
+ * the Ceph cluster.  If it is excluded we will use the Ceph default
+ * (normally 'admin').
  *
- * The raw device is split into 4MB sized objects by default.
- * The sequencenumber is encoded in a 12 byte long hex-string,
- * and is attached to the devicename, separated by a dot.
- * e.g. "devicename.1234567890ab"
+ * The "conf" option specifies a Ceph configuration file to read.  If
+ * it is not specified, we will read from the default Ceph locations
+ * (e.g., /etc/ceph/ceph.conf).  To avoid reading _any_ configuration
+ * file, specify conf=/dev/null.
  *
+ * Configuration values containing :, @, or = can be escaped with a
+ * leading "\".
  */
 
 #define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER)
-- 
1.7.6.2




Re: [Qemu-devel] [Libguestfs] Problem with allocation of big files

2011-09-20 Thread Richard W.M. Jones
On Tue, Sep 20, 2011 at 11:38:15AM +0400, Nikita A Menkovich wrote:
> Hello,
> 
> I found a strange problem with allocationg big files on drive.
> 
> For example in guestfish I allocate large disk image
> for example:
> 
> $ guestfish
> > allocate test.img 20G
> 
> When an image allocating, there is a big slowdown of guest OSes,
> launched on host machine, and on different drives.

I'm guessing this is a problem with the host kernel (Linux?).
Specifically you may be saturating SATA, your Southbridge, or PCI if
it's using that.

Here is the code anyway:

http://git.annexia.org/?p=libguestfs.git;a=blob;f=fish/alloc.c;h=7799e4e1c5f0b34ce079934eea7d7c0a13ecdcd3;hb=refs/heads/stable-1.10#l85

Notice that the 'allocate' command takes the non-sparse code path.

What happens next depends on:

 - does your operating system have the posix_fallocate call?

 - does your filesystem support efficient fallocate? (ext3: no, ext4: yes,
   xfs: yes)

If the answer to either question is 'no' then you end up in a loop
which writes zeroes.  For a 20 GB file this is going to be slow and is
going to saturate SATA.

If the answer to *both* questions is 'yes', then ext4/xfs should be
able to allocate the required extents and lazily initialize it.  This
is typically very fast, almost instantaneous.

> For Linux guests with virtio drivers, there is no so big performance
> penalty, but for FreeBSD guests with ide it seems like the OS locked.
> 
> Manipulating with ionice and niceness do not affect at all.
> 
> Manipulating with cfq, deadline schedulers also do not have any results.
> 
> OS Debian Squeeze
> libvirtd (libvirt) 0.9.2
> QEMU emulator version 0.14.0 (Debian 0.14.0+dfsg-5.1), Copyright (c)
> 2003-2008 Fabrice Bellard
> guestfish 1.10.3 with some local patches, backported from main tree
> 
> Does anyone have ideas how to fix it?

Probably the best idea is to use ext4, or use 'sparse' instead of
'allocate' (but with 'sparse' you pay the same penalty, just you pay
for it later on).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v



[Qemu-devel] [PATCH 17/20] scsi: fix sign extension problems

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

When assigning a 32-bit value to cmd->xfer (which is 64-bits)
it can be erroneously sign extended because the intermediate
32-bit computation is signed.  Fix this by standardizing on
the ld*_be_p functions.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 hw/scsi-bus.c |   22 +++---
 1 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 0248294..aca65a1 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -542,15 +542,15 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice 
*dev, uint8_t *buf)
 break;
 case 1:
 case 2:
-cmd->xfer = buf[8] | (buf[7] << 8);
+cmd->xfer = lduw_be_p(&buf[7]);
 cmd->len = 10;
 break;
 case 4:
-cmd->xfer = buf[13] | (buf[12] << 8) | (buf[11] << 16) | (buf[10] << 
24);
+cmd->xfer = ldl_be_p(&buf[10]);
 cmd->len = 16;
 break;
 case 5:
-cmd->xfer = buf[9] | (buf[8] << 8) | (buf[7] << 16) | (buf[6] << 24);
+cmd->xfer = ldl_be_p(&buf[6]);
 cmd->len = 12;
 break;
 default:
@@ -710,23 +710,15 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
 
 switch (buf[0] >> 5) {
 case 0:
-lba = (uint64_t) buf[3] | ((uint64_t) buf[2] << 8) |
-  (((uint64_t) buf[1] & 0x1f) << 16);
+lba = ldl_be_p(&buf[0]) & 0x1f;
 break;
 case 1:
 case 2:
-lba = (uint64_t) buf[5] | ((uint64_t) buf[4] << 8) |
-  ((uint64_t) buf[3] << 16) | ((uint64_t) buf[2] << 24);
+case 5:
+lba = ldl_be_p(&buf[2]);
 break;
 case 4:
-lba = (uint64_t) buf[9] | ((uint64_t) buf[8] << 8) |
-  ((uint64_t) buf[7] << 16) | ((uint64_t) buf[6] << 24) |
-  ((uint64_t) buf[5] << 32) | ((uint64_t) buf[4] << 40) |
-  ((uint64_t) buf[3] << 48) | ((uint64_t) buf[2] << 56);
-break;
-case 5:
-lba = (uint64_t) buf[5] | ((uint64_t) buf[4] << 8) |
-  ((uint64_t) buf[3] << 16) | ((uint64_t) buf[2] << 24);
+lba = ldq_be_p(&buf[2]);
 break;
 default:
 lba = -1;
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH] pci: implement bridge filtering

2011-09-20 Thread Avi Kivity

On 09/20/2011 11:09 AM, Wen Congyang wrote:

At 09/14/2011 09:48 AM, Wen Congyang Write:
>  At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:
>>  Support bridge filtering on top of the memory
>>  API as suggested by Avi Kivity:
>>
>>  Create a memory region for the bridge's address space.  This region is
>>  not directly added to system_memory or its descendants.  Devices under
>>  the bridge see this region as its pci_address_space().  The region is
>>  as large as the entire address space - it does not take into account
>>  any windows.
>>
>>  For each of the three windows (pref, non-pref, vga), create an alias
>>  with the appropriate start and size.  Map the alias into the bridge's
>>  parent's pci_address_space(), as subregions.
>>
>>  Signed-off-by: Michael S. Tsirkin
>>  ---
>>
>>  The below seems to work fine for me so I applied this.
>>  Still need to test bridge filtering, any help with this
>>  appreciated.
>>
>
>
>  I test bridge filtering, and the BAR still can be visible on guest even if
>  I change the memory region.

Hi Michael S. Tsirkin:
I test pci bridge filtering on real hardware, and I find that I can mmap
the resource after I change the memory base and memory limit(The BAR should
be not visible on OS after changing the memory region).

So I try to write and read to the BAR. Here is my test result:
1. Before changing the pci bridge's memory region, I can read and write to the 
memory, and
I can get the same value that I write.

2. After changing the pci bridge's memory region, I can still read and write to 
the memory,
but it is very slow, and I can not get the same value that I write(The 
value is always 0).

Does this result means that pci bridge filtering works fine?



Yes.  Instead of hitting the BAR, you hit the default mmio handler.

--
error compiling committee.c: too many arguments to function




[Qemu-devel] [PATCH 07/20] dma-helpers: allow including from target-independent code

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

Target-independent code cannot construct sglists, but it can take
them from the outside as a black box.  Allow this.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 dma.h |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/dma.h b/dma.h
index b222346..2bdc236 100644
--- a/dma.h
+++ b/dma.h
@@ -15,10 +15,13 @@
 #include "hw/hw.h"
 #include "block.h"
 
-typedef struct {
+typedef struct ScatterGatherEntry ScatterGatherEntry;
+
+#if defined(TARGET_PHYS_ADDR_BITS)
+struct ScatterGatherEntry {
 target_phys_addr_t base;
 target_phys_addr_t len;
-} ScatterGatherEntry;
+};
 
 struct QEMUSGList {
 ScatterGatherEntry *sg;
@@ -31,6 +34,7 @@ void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint);
 void qemu_sglist_add(QEMUSGList *qsg, target_phys_addr_t base,
  target_phys_addr_t len);
 void qemu_sglist_destroy(QEMUSGList *qsg);
+#endif
 
 typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs, int64_t sector_num,
  QEMUIOVector *iov, int nb_sectors,
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

2011-09-20 Thread Stefan Hajnoczi
On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova 
> ---
>  trace-events |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/trace-events b/trace-events
> index 9d1fbbb..b653d70 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>  # hw/milkymist-softusb.c
>  milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x 
> value %08x"
>  milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x 
> value %08x"
> -milkymist_softusb_mevt(uint8_t m) "m %d"
> -milkymist_softusb_kevt(uint8_t m) "m %d"
> +milkymist_softusb_mevt(uint8_t _m) "m %d"
> +milkymist_softusb_kevt(uint8_t _m) "m %d"

The LTTng community has been very responsive in addressing namespace
issues with libust.  Let's post more details and see if it can be fixed
in libust.

Could you please post your gcc and libust versions?

I have not been able to reproduce the problem on Debian libust-dev
0.15-3.  My gcc version is Debian gcc 4.6.1-4.

Here is the test program:

#include 
#include 

DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
TP_ARGS(m));

int main(int argc, char **argv)
{
return 0;
}

I see no warning or error related to the 'm' argument name.

Stefan



[Qemu-devel] [PATCH 09/20] scsi-disk: commonize iovec creation between reads and writes

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

Also, consistently use qiov.size instead of iov.iov_len.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 hw/scsi-disk.c |   42 ++
 1 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 4a60820..84e8662 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -108,6 +108,13 @@ static void scsi_cancel_io(SCSIRequest *req)
 r->req.aiocb = NULL;
 }
 
+static uint32_t scsi_init_iovec(SCSIDiskReq *r)
+{
+r->iov.iov_len = MIN(r->sector_count * 512, SCSI_DMA_BUF_SIZE);
+qemu_iovec_init_external(&r->qiov, &r->iov, 1);
+return r->qiov.size / 512;
+}
+
 static void scsi_read_complete(void * opaque, int ret)
 {
 SCSIDiskReq *r = (SCSIDiskReq *)opaque;
@@ -125,12 +132,12 @@ static void scsi_read_complete(void * opaque, int ret)
 }
 }
 
-DPRINTF("Data ready tag=0x%x len=%zd\n", r->req.tag, r->iov.iov_len);
+DPRINTF("Data ready tag=0x%x len=%zd\n", r->req.tag, r->qiov.size);
 
-n = r->iov.iov_len / 512;
+n = r->qiov.size / 512;
 r->sector += n;
 r->sector_count -= n;
-scsi_req_data(&r->req, r->iov.iov_len);
+scsi_req_data(&r->req, r->qiov.size);
 }
 
 static void scsi_flush_complete(void * opaque, int ret)
@@ -181,16 +188,10 @@ static void scsi_read_data(SCSIRequest *req)
 return;
 }
 
-n = r->sector_count;
-if (n > SCSI_DMA_BUF_SIZE / 512)
-n = SCSI_DMA_BUF_SIZE / 512;
-
 if (s->tray_open) {
 scsi_read_complete(r, -ENOMEDIUM);
 }
-r->iov.iov_len = n * 512;
-qemu_iovec_init_external(&r->qiov, &r->iov, 1);
-
+n = scsi_init_iovec(r);
 bdrv_acct_start(s->bs, &r->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
 r->req.aiocb = bdrv_aio_readv(s->bs, r->sector, &r->qiov, n,
   scsi_read_complete, r);
@@ -239,7 +240,6 @@ static void scsi_write_complete(void * opaque, int ret)
 {
 SCSIDiskReq *r = (SCSIDiskReq *)opaque;
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-uint32_t len;
 uint32_t n;
 
 if (r->req.aiocb != NULL) {
@@ -253,19 +253,15 @@ static void scsi_write_complete(void * opaque, int ret)
 }
 }
 
-n = r->iov.iov_len / 512;
+n = r->qiov.size / 512;
 r->sector += n;
 r->sector_count -= n;
 if (r->sector_count == 0) {
 scsi_req_complete(&r->req, GOOD);
 } else {
-len = r->sector_count * 512;
-if (len > SCSI_DMA_BUF_SIZE) {
-len = SCSI_DMA_BUF_SIZE;
-}
-r->iov.iov_len = len;
-DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, len);
-scsi_req_data(&r->req, len);
+scsi_init_iovec(r);
+DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, r->qiov.size);
+scsi_req_data(&r->req, r->qiov.size);
 }
 }
 
@@ -284,21 +280,19 @@ static void scsi_write_data(SCSIRequest *req)
 return;
 }
 
-n = r->iov.iov_len / 512;
+n = r->qiov.size / 512;
 if (n) {
 if (s->tray_open) {
 scsi_write_complete(r, -ENOMEDIUM);
 }
-qemu_iovec_init_external(&r->qiov, &r->iov, 1);
-
 bdrv_acct_start(s->bs, &r->acct, n * BDRV_SECTOR_SIZE, 
BDRV_ACCT_WRITE);
 r->req.aiocb = bdrv_aio_writev(s->bs, r->sector, &r->qiov, n,
-   scsi_write_complete, r);
+   scsi_write_complete, r);
 if (r->req.aiocb == NULL) {
 scsi_write_complete(r, -ENOMEM);
 }
 } else {
-/* Invoke completion routine to fetch data from host.  */
+/* Called for the first time.  Ask the driver to send us more data.  */
 scsi_write_complete(r, 0);
 }
 }
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH] pci: implement bridge filtering

2011-09-20 Thread Michael S. Tsirkin
On Tue, Sep 20, 2011 at 04:09:23PM +0800, Wen Congyang wrote:
> At 09/14/2011 09:48 AM, Wen Congyang Write:
> > At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:
> >> Support bridge filtering on top of the memory
> >> API as suggested by Avi Kivity:
> >>
> >> Create a memory region for the bridge's address space.  This region is
> >> not directly added to system_memory or its descendants.  Devices under
> >> the bridge see this region as its pci_address_space().  The region is
> >> as large as the entire address space - it does not take into account
> >> any windows.
> >>
> >> For each of the three windows (pref, non-pref, vga), create an alias
> >> with the appropriate start and size.  Map the alias into the bridge's
> >> parent's pci_address_space(), as subregions.
> >>
> >> Signed-off-by: Michael S. Tsirkin 
> >> ---
> >>
> >> The below seems to work fine for me so I applied this.
> >> Still need to test bridge filtering, any help with this
> >> appreciated.
> >>
> > 
> > 
> > I test bridge filtering, and the BAR still can be visible on guest even if
> > I change the memory region.
> 
> Hi Michael S. Tsirkin:
> I test pci bridge filtering on real hardware, and I find that I can mmap
> the resource after I change the memory base and memory limit(The BAR should
> be not visible on OS after changing the memory region).
> 
> So I try to write and read to the BAR. Here is my test result:
> 1. Before changing the pci bridge's memory region, I can read and write to 
> the memory, and
>I can get the same value that I write.
> 
> 2. After changing the pci bridge's memory region, I can still read and write 
> to the memory,
>but it is very slow, and I can not get the same value that I write(The 
> value is always 0).
> 
> Does this result means that pci bridge filtering works fine?
> 
> Thanks
> Wen Congyang

Sounds more or less right except I expect to get 
and not 0. Avi, any idea?

-- 
MST



[Qemu-devel] [PATCH 06/20] dma-helpers: rename is_write to to_dev

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 dma-helpers.c  |   14 +++---
 dma.h  |2 +-
 hw/ide/core.c  |2 +-
 hw/ide/macio.c |2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/dma-helpers.c b/dma-helpers.c
index 4610ea0..717e384 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -42,7 +42,7 @@ typedef struct {
 BlockDriverAIOCB *acb;
 QEMUSGList *sg;
 uint64_t sector_num;
-int is_write;
+bool to_dev;
 int sg_cur_index;
 target_phys_addr_t sg_cur_byte;
 QEMUIOVector iov;
@@ -75,7 +75,7 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
 
 for (i = 0; i < dbs->iov.niov; ++i) {
 cpu_physical_memory_unmap(dbs->iov.iov[i].iov_base,
-  dbs->iov.iov[i].iov_len, !dbs->is_write,
+  dbs->iov.iov[i].iov_len, !dbs->to_dev,
   dbs->iov.iov[i].iov_len);
 }
 }
@@ -101,7 +101,7 @@ static void dma_bdrv_cb(void *opaque, int ret)
 while (dbs->sg_cur_index < dbs->sg->nsg) {
 cur_addr = dbs->sg->sg[dbs->sg_cur_index].base + dbs->sg_cur_byte;
 cur_len = dbs->sg->sg[dbs->sg_cur_index].len - dbs->sg_cur_byte;
-mem = cpu_physical_memory_map(cur_addr, &cur_len, !dbs->is_write);
+mem = cpu_physical_memory_map(cur_addr, &cur_len, !dbs->to_dev);
 if (!mem)
 break;
 qemu_iovec_add(&dbs->iov, mem, cur_len);
@@ -143,7 +143,7 @@ static AIOPool dma_aio_pool = {
 BlockDriverAIOCB *dma_bdrv_io(
 BlockDriverState *bs, QEMUSGList *sg, uint64_t sector_num,
 DMAIOFunc *io_func, BlockDriverCompletionFunc *cb,
-void *opaque, int is_write)
+void *opaque, bool to_dev)
 {
 DMAAIOCB *dbs = qemu_aio_get(&dma_aio_pool, bs, cb, opaque);
 
@@ -153,7 +153,7 @@ BlockDriverAIOCB *dma_bdrv_io(
 dbs->sector_num = sector_num;
 dbs->sg_cur_index = 0;
 dbs->sg_cur_byte = 0;
-dbs->is_write = is_write;
+dbs->to_dev = to_dev;
 dbs->io_func = io_func;
 dbs->bh = NULL;
 qemu_iovec_init(&dbs->iov, sg->nsg);
@@ -170,12 +170,12 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
 QEMUSGList *sg, uint64_t sector,
 void (*cb)(void *opaque, int ret), void 
*opaque)
 {
-return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, 0);
+return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, false);
 }
 
 BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs,
  QEMUSGList *sg, uint64_t sector,
  void (*cb)(void *opaque, int ret), void 
*opaque)
 {
-return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, 1);
+return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, true);
 }
diff --git a/dma.h b/dma.h
index a6db5ba..b222346 100644
--- a/dma.h
+++ b/dma.h
@@ -39,7 +39,7 @@ typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs, 
int64_t sector_num,
 BlockDriverAIOCB *dma_bdrv_io(BlockDriverState *bs,
   QEMUSGList *sg, uint64_t sector_num,
   DMAIOFunc *io_func, BlockDriverCompletionFunc 
*cb,
-  void *opaque, int is_write);
+  void *opaque, bool to_dev);
 BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
 QEMUSGList *sg, uint64_t sector,
 BlockDriverCompletionFunc *cb, void *opaque);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 9297b9e..f424347 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -603,7 +603,7 @@ handle_rw_error:
 break;
 case IDE_DMA_TRIM:
 s->bus->dma->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
- ide_issue_trim, ide_dma_cb, s, 1);
+ ide_issue_trim, ide_dma_cb, s, true);
 break;
 }
 
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index c1844cb..37b8239 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -156,7 +156,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
 break;
 case IDE_DMA_TRIM:
 m->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
-   ide_issue_trim, pmac_ide_transfer_cb, s, 1);
+   ide_issue_trim, pmac_ide_transfer_cb, s, true);
 break;
 }
 
-- 
1.7.6.2




[Qemu-devel] [PATCH 12/20] posix-aio-compat: Removed unused offset variable

2011-09-20 Thread Kevin Wolf
From: Frediano Ziglio 

Signed-off-by: Frediano Ziglio 
Signed-off-by: Kevin Wolf 
---
 posix-aio-compat.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 3193dbf..63a8fae 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -181,7 +181,6 @@ qemu_pwritev(int fd, const struct iovec *iov, int nr_iov, 
off_t offset)
 
 static ssize_t handle_aiocb_rw_vector(struct qemu_paiocb *aiocb)
 {
-size_t offset = 0;
 ssize_t len;
 
 do {
@@ -189,12 +188,12 @@ static ssize_t handle_aiocb_rw_vector(struct qemu_paiocb 
*aiocb)
 len = qemu_pwritev(aiocb->aio_fildes,
aiocb->aio_iov,
aiocb->aio_niov,
-   aiocb->aio_offset + offset);
+   aiocb->aio_offset);
  else
 len = qemu_preadv(aiocb->aio_fildes,
   aiocb->aio_iov,
   aiocb->aio_niov,
-  aiocb->aio_offset + offset);
+  aiocb->aio_offset);
 } while (len == -1 && errno == EINTR);
 
 if (len == -1)
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

2011-09-20 Thread Stefan Hajnoczi
On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova 
> ---
>  trace-events |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/trace-events b/trace-events
> index 9d1fbbb..b653d70 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>  # hw/milkymist-softusb.c
>  milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x 
> value %08x"
>  milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x 
> value %08x"
> -milkymist_softusb_mevt(uint8_t m) "m %d"
> -milkymist_softusb_kevt(uint8_t m) "m %d"
> +milkymist_softusb_mevt(uint8_t _m) "m %d"
> +milkymist_softusb_kevt(uint8_t _m) "m %d"

Do you also see errors with trace events that have no arguments?

DECLARE_TRACE(ust_qemu_co_queue_next_bh, TP_PROTO(void), TP_ARGS());

In file included from osdep.c:48:0:
trace.h: In function ‘__trace_ust_qemu_co_queue_next_bh’:
trace.h:1030:195: error: ‘void’ must be the only parameter
trace.h:1030:661: error: expected expression before ‘)’ token
trace.h:1030:661: error: too many arguments to function ‘(void (*)(void 
*))__tp_it_func’
trace.h: At top level:
trace.h:1030:661: error: ‘void’ must be the only parameter
trace.h:1030:661: error: ‘void’ must be the only parameter

On my box this can be fixed by changing the DEFINE_TRACE() like this:

DECLARE_TRACE(ust_qemu_co_queue_next_bh, TP_PROTO(), TP_ARGS());

I haven't looked into the details yet but wanted to check.  I suspect that 
different versions of libust might have different quirks here.

Stefan



[Qemu-devel] [PATCH 04/20] raw-posix: Fix bdrv_flush error return values

2011-09-20 Thread Kevin Wolf
bdrv_flush is supposed to use 0/-errno return values

Signed-off-by: Kevin Wolf 
---
 block/raw-posix.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index a624f56..305998d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -839,7 +839,14 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options)
 static int raw_flush(BlockDriverState *bs)
 {
 BDRVRawState *s = bs->opaque;
-return qemu_fdatasync(s->fd);
+int ret;
+
+ret = qemu_fdatasync(s->fd);
+if (ret < 0) {
+return -errno;
+}
+
+return 0;
 }
 
 #ifdef CONFIG_XFS
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH 1/2] trace: portable simple trace backend using glib

2011-09-20 Thread Stefan Hajnoczi
On Tue, Sep 20, 2011 at 11:58 AM, Jan Kiszka  wrote:
> On 2011-09-20 12:52, Paolo Bonzini wrote:
>> On 09/20/2011 12:31 PM, Jan Kiszka wrote:
>>> Please avoid restrictive glib thread services. We have qemu_thread
>>> abstractions that allow central tuning (will be needed e.g. to adjust
>>> scheduling parameters).
>>
>> I think the rationale here was to allow tracing the qemu_thread
>> routines.  For your application you can still use ust or systemtap backends.
>
> OK, if that's a chick-egg thing, this makes sense. Should be documented
> then (to avoid someone using this as a general example).

I'll add a comment.

Stefan



[Qemu-devel] [PATCH 08/20] dma-helpers: rewrite completion/cancellation

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

This fixes various problems with completion/cancellation:

* if the io_func fails to get an AIOCB, the callback wasn't called

* If DMA encounters a bounce buffer conflict, and the DMA operation is
canceled before the bottom half fires, bad things happen.

* memory is not unmapped after cancellation, again causing problems
when doing DMA to I/O areas

* cancellation could leak the iovec

* the callback was missed if the I/O operation failed without returning
an AIOCB

and probably more that I've missed.  The patch fixes them by sharing
the cleanup code between completion and cancellation.  The dma_bdrv_cb
now returns a boolean completed/not completed flag, and the wrapper
dma_continue takes care of tasks to do upon completion.

Most of these are basically impossible in practice, but it is better
to be tidy...

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 dma-helpers.c |   44 +++-
 1 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/dma-helpers.c b/dma-helpers.c
index 717e384..86d2d0a 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -43,6 +43,7 @@ typedef struct {
 QEMUSGList *sg;
 uint64_t sector_num;
 bool to_dev;
+bool in_cancel;
 int sg_cur_index;
 target_phys_addr_t sg_cur_byte;
 QEMUIOVector iov;
@@ -58,7 +59,7 @@ static void reschedule_dma(void *opaque)
 
 qemu_bh_delete(dbs->bh);
 dbs->bh = NULL;
-dma_bdrv_cb(opaque, 0);
+dma_bdrv_cb(dbs, 0);
 }
 
 static void continue_after_map_failure(void *opaque)
@@ -78,6 +79,26 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
   dbs->iov.iov[i].iov_len, !dbs->to_dev,
   dbs->iov.iov[i].iov_len);
 }
+qemu_iovec_reset(&dbs->iov);
+}
+
+static void dma_complete(DMAAIOCB *dbs, int ret)
+{
+dma_bdrv_unmap(dbs);
+if (dbs->common.cb) {
+dbs->common.cb(dbs->common.opaque, ret);
+}
+qemu_iovec_destroy(&dbs->iov);
+if (dbs->bh) {
+qemu_bh_delete(dbs->bh);
+dbs->bh = NULL;
+}
+if (!dbs->in_cancel) {
+/* Requests may complete while dma_aio_cancel is in progress.  In
+ * this case, the AIOCB should not be released because it is still
+ * referenced by dma_aio_cancel.  */
+qemu_aio_release(dbs);
+}
 }
 
 static void dma_bdrv_cb(void *opaque, int ret)
@@ -89,12 +110,9 @@ static void dma_bdrv_cb(void *opaque, int ret)
 dbs->acb = NULL;
 dbs->sector_num += dbs->iov.size / 512;
 dma_bdrv_unmap(dbs);
-qemu_iovec_reset(&dbs->iov);
 
 if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) {
-dbs->common.cb(dbs->common.opaque, ret);
-qemu_iovec_destroy(&dbs->iov);
-qemu_aio_release(dbs);
+dma_complete(dbs, ret);
 return;
 }
 
@@ -120,9 +138,7 @@ static void dma_bdrv_cb(void *opaque, int ret)
 dbs->acb = dbs->io_func(dbs->bs, dbs->sector_num, &dbs->iov,
 dbs->iov.size / 512, dma_bdrv_cb, dbs);
 if (!dbs->acb) {
-dma_bdrv_unmap(dbs);
-qemu_iovec_destroy(&dbs->iov);
-return;
+dma_complete(dbs, -EIO);
 }
 }
 
@@ -131,8 +147,14 @@ static void dma_aio_cancel(BlockDriverAIOCB *acb)
 DMAAIOCB *dbs = container_of(acb, DMAAIOCB, common);
 
 if (dbs->acb) {
-bdrv_aio_cancel(dbs->acb);
+BlockDriverAIOCB *acb = dbs->acb;
+dbs->acb = NULL;
+dbs->in_cancel = true;
+bdrv_aio_cancel(acb);
+dbs->in_cancel = false;
 }
+dbs->common.cb = NULL;
+dma_complete(dbs, 0);
 }
 
 static AIOPool dma_aio_pool = {
@@ -158,10 +180,6 @@ BlockDriverAIOCB *dma_bdrv_io(
 dbs->bh = NULL;
 qemu_iovec_init(&dbs->iov, sg->nsg);
 dma_bdrv_cb(dbs, 0);
-if (!dbs->acb) {
-qemu_aio_release(dbs);
-return NULL;
-}
 return &dbs->common;
 }
 
-- 
1.7.6.2




[Qemu-devel] [PATCH 13/20] AHCI Port Interrupt Enable register cleaning on soft reset

2011-09-20 Thread Kevin Wolf
From: Alexander Motin 

I've found that FreeBSD AHCI driver doesn't work with AHCI hardware
emulation of QEMU 0.15.0. I believe the problem is on QEMU's side. As I
see, it clears port's Interrupt Enable register each time when reset of
any level happens. Is is reasonable for the global controller reset. It
is probably not good, but acceptable for FreeBSD driver for the port
hard reset. But it is IMO wrong for the device soft reset. None of real
hardware I know behaves that way.

Signed-off-by: Alexander Motin 
Signed-off-by: Kevin Wolf 
---
 hw/ide/ahci.c |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index a8659cf..464c28b 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -499,10 +499,7 @@ static void ahci_reset_port(AHCIState *s, int port)
 ide_bus_reset(&d->port);
 ide_state->ncq_queues = AHCI_MAX_CMDS;
 
-pr->irq_stat = 0;
-pr->irq_mask = 0;
 pr->scr_stat = 0;
-pr->scr_ctl = 0;
 pr->scr_err = 0;
 pr->scr_act = 0;
 d->busy_slot = -1;
@@ -1159,12 +1156,17 @@ void ahci_uninit(AHCIState *s)
 void ahci_reset(void *opaque)
 {
 struct AHCIPCIState *d = opaque;
+AHCIPortRegs *pr;
 int i;
 
 d->ahci.control_regs.irqstatus = 0;
 d->ahci.control_regs.ghc = 0;
 
 for (i = 0; i < d->ahci.ports; i++) {
+pr = &d->ahci.dev[i].port_regs;
+pr->irq_stat = 0;
+pr->irq_mask = 0;
+pr->scr_ctl = 0;
 ahci_reset_port(&d->ahci, i);
 }
 }
-- 
1.7.6.2




[Qemu-devel] [PATCH 11/20] VMDK: fix leak of extent_file

2011-09-20 Thread Kevin Wolf
From: Fam Zheng 

Release extent_file on error in vmdk_parse_extents. Added closing files
in freeing extents.

Signed-off-by: Fam Zheng 
Signed-off-by: Kevin Wolf 
---
 block/vmdk.c |   14 ++
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 6c8edfc..5d16ec4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -179,11 +179,16 @@ static void vmdk_free_extents(BlockDriverState *bs)
 {
 int i;
 BDRVVmdkState *s = bs->opaque;
+VmdkExtent *e;
 
 for (i = 0; i < s->num_extents; i++) {
-g_free(s->extents[i].l1_table);
-g_free(s->extents[i].l2_cache);
-g_free(s->extents[i].l1_backup_table);
+e = &s->extents[i];
+g_free(e->l1_table);
+g_free(e->l2_cache);
+g_free(e->l1_backup_table);
+if (e->file != bs->file) {
+bdrv_delete(e->file);
+}
 }
 g_free(s->extents);
 }
@@ -619,12 +624,13 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
flags,
 s->desc_offset = 0;
 ret = vmdk_parse_extents(buf, bs, bs->file->filename);
 if (ret) {
+vmdk_free_extents(bs);
 return ret;
 }
 
 /* try to open parent images, if exist */
 if (vmdk_parent_open(bs)) {
-g_free(s->extents);
+vmdk_free_extents(bs);
 return -EINVAL;
 }
 s->parent_cid = vmdk_read_cid(bs, 1);
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH] pci: implement bridge filtering

2011-09-20 Thread Avi Kivity

On 09/20/2011 02:55 PM, Michael S. Tsirkin wrote:

>
>  Hi Michael S. Tsirkin:
>  I test pci bridge filtering on real hardware, and I find that I can mmap
>  the resource after I change the memory base and memory limit(The BAR should
>  be not visible on OS after changing the memory region).
>
>  So I try to write and read to the BAR. Here is my test result:
>  1. Before changing the pci bridge's memory region, I can read and write to 
the memory, and
> I can get the same value that I write.
>
>  2. After changing the pci bridge's memory region, I can still read and write 
to the memory,
> but it is very slow, and I can not get the same value that I write(The 
value is always 0).
>
>  Does this result means that pci bridge filtering works fine?
>
>  Thanks
>  Wen Congyang

Sounds more or less right except I expect to get 
and not 0. Avi, any idea?



No, what does the default handler do?

--
error compiling committee.c: too many arguments to function




[Qemu-devel] [PATCH 19/20] linux-aio: remove process requests callback

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 linux-aio.c |   11 +--
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/linux-aio.c b/linux-aio.c
index 5265a02..bffa6cd 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -68,15 +68,6 @@ static void qemu_laio_process_completion(struct 
qemu_laio_state *s,
 qemu_aio_release(laiocb);
 }
 
-/*
- * All requests are directly processed when they complete, so there's nothing
- * left to do during qemu_aio_wait().
- */
-static int qemu_laio_process_requests(void *opaque)
-{
-return 0;
-}
-
 static void qemu_laio_completion_cb(void *opaque)
 {
 struct qemu_laio_state *s = opaque;
@@ -215,7 +206,7 @@ void *laio_init(void)
 goto out_close_efd;
 
 qemu_aio_set_fd_handler(s->efd, qemu_laio_completion_cb, NULL,
-qemu_laio_flush_cb, qemu_laio_process_requests, s);
+qemu_laio_flush_cb, NULL, s);
 
 return s;
 
-- 
1.7.6.2




[Qemu-devel] [PATCH v3 1/2] trace: portable simple trace backend using glib

2011-09-20 Thread stefanha
From: Stefan Hajnoczi 

Convert the simple trace backend to glib so that it works under Windows.
We cannot use pthread directly but glib provides portable abstractions.
Also use glib atomics instead of newish gcc builtins which may not be
supported on Windows toolchains.

Signed-off-by: Stefan Hajnoczi 
---
 trace/simple.c |   74 ---
 1 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/trace/simple.c b/trace/simple.c
index a609368..885764a 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -12,8 +12,10 @@
 #include 
 #include 
 #include 
+#ifndef _WIN32
 #include 
 #include 
+#endif
 #include "qemu-timer.h"
 #include "trace.h"
 #include "trace/control.h"
@@ -54,9 +56,9 @@ enum {
  * Trace records are written out by a dedicated thread.  The thread waits for
  * records to become available, writes them out, and then waits again.
  */
-static pthread_mutex_t trace_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t trace_available_cond = PTHREAD_COND_INITIALIZER;
-static pthread_cond_t trace_empty_cond = PTHREAD_COND_INITIALIZER;
+static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
+static GCond *trace_available_cond;
+static GCond *trace_empty_cond;
 static bool trace_available;
 static bool trace_writeout_enabled;
 
@@ -93,29 +95,30 @@ static bool get_trace_record(unsigned int idx, TraceRecord 
*record)
  */
 static void flush_trace_file(bool wait)
 {
-pthread_mutex_lock(&trace_lock);
+g_static_mutex_lock(&trace_lock);
 trace_available = true;
-pthread_cond_signal(&trace_available_cond);
+g_cond_signal(trace_available_cond);
 
 if (wait) {
-pthread_cond_wait(&trace_empty_cond, &trace_lock);
+g_cond_wait(trace_empty_cond, g_static_mutex_get_mutex(&trace_lock));
 }
 
-pthread_mutex_unlock(&trace_lock);
+g_static_mutex_unlock(&trace_lock);
 }
 
 static void wait_for_trace_records_available(void)
 {
-pthread_mutex_lock(&trace_lock);
+g_static_mutex_lock(&trace_lock);
 while (!(trace_available && trace_writeout_enabled)) {
-pthread_cond_signal(&trace_empty_cond);
-pthread_cond_wait(&trace_available_cond, &trace_lock);
+g_cond_signal(trace_empty_cond);
+g_cond_wait(trace_available_cond,
+g_static_mutex_get_mutex(&trace_lock));
 }
 trace_available = false;
-pthread_mutex_unlock(&trace_lock);
+g_static_mutex_unlock(&trace_lock);
 }
 
-static void *writeout_thread(void *opaque)
+static gpointer writeout_thread(gpointer opaque)
 {
 TraceRecord record;
 unsigned int writeout_idx = 0;
@@ -159,7 +162,7 @@ static void trace(TraceEventID event, uint64_t x1, uint64_t 
x2, uint64_t x3,
 
 timestamp = get_clock();
 
-idx = __sync_fetch_and_add(&trace_idx, 1) % TRACE_BUF_LEN;
+idx = g_atomic_int_exchange_and_add((gint *)&trace_idx, 1) % TRACE_BUF_LEN;
 trace_buf[idx] = (TraceRecord){
 .event = event,
 .timestamp_ns = timestamp,
@@ -331,28 +334,47 @@ bool trace_event_set_state(const char *name, bool state)
 return false;
 }
 
-bool trace_backend_init(const char *events, const char *file)
+/* Helper function to create a thread with signals blocked.  Use glib's
+ * portable threads since QEMU abstractions cannot be used due to reentrancy in
+ * the tracer.  Also note the signal masking on POSIX hosts so that the thread
+ * does not steal signals when the rest of the program wants them blocked.
+ */
+static GThread *trace_thread_create(GThreadFunc fn)
 {
-pthread_t thread;
-pthread_attr_t attr;
+GThread *thread;
+#ifndef _WIN32
 sigset_t set, oldset;
-int ret;
-
-pthread_attr_init(&attr);
-pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
 
 sigfillset(&set);
 pthread_sigmask(SIG_SETMASK, &set, &oldset);
-ret = pthread_create(&thread, &attr, writeout_thread, NULL);
+#endif
+thread = g_thread_create(writeout_thread, NULL, FALSE, NULL);
+#ifndef _WIN32
 pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+#endif
 
-if (ret != 0) {
+return thread;
+}
+
+bool trace_backend_init(const char *events, const char *file)
+{
+GThread *thread;
+
+if (!g_thread_supported()) {
+g_thread_init(NULL);
+}
+
+trace_available_cond = g_cond_new();
+trace_empty_cond = g_cond_new();
+
+thread = trace_thread_create(writeout_thread);
+if (!thread) {
 fprintf(stderr, "warning: unable to initialize simple trace 
backend\n");
-} else {
-atexit(st_flush_trace_buffer);
-trace_backend_init_events(events);
-st_set_trace_file(file);
+return false;
 }
 
+atexit(st_flush_trace_buffer);
+trace_backend_init_events(events);
+st_set_trace_file(file);
 return true;
 }
-- 
1.7.5.4




[Qemu-devel] [PATCH v3 2/2] trace: use binary file open mode in simpletrace

2011-09-20 Thread stefanha
From: Stefan Hajnoczi 

For Windows portability the simple trace backend must use the 'b' file
open mode.  This prevents the stdio library from mangling 0x0a/0x0d
newline characters.

Signed-off-by: Stefan Hajnoczi 
---
 trace/simple.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/trace/simple.c b/trace/simple.c
index 885764a..b639dda 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -234,7 +234,7 @@ void st_set_trace_file_enabled(bool enable)
 .x1 = HEADER_VERSION,
 };
 
-trace_fp = fopen(trace_file_name, "w");
+trace_fp = fopen(trace_file_name, "wb");
 if (!trace_fp) {
 return;
 }
-- 
1.7.5.4




[Qemu-devel] [PATCH v3 0/2] Make simpletrace work on Windows

2011-09-20 Thread stefanha
From: Stefan Hajnoczi 

The 'simple' trace backend uses pthreads and does not work on Windows.  These
patches switch from pthreads to glib so that the code builds on all platforms
supported by glib.

v3:
 * Explain that glib threads need to be used directly
 * Explain the signal masking

v2:
 * Block signals in trace write-out thread

Stefan Hajnoczi (2):
  trace: portable simple trace backend using glib
  trace: use binary file open mode in simpletrace

 trace/simple.c |   76 
 1 files changed, 49 insertions(+), 27 deletions(-)

-- 
1.7.5.4




[Qemu-devel] [PATCH 05/20] scsi-generic: do not disable FUA

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

I found no rationale for this in the logs, and it is quite bad because
it will make scsi-generic unsafe WRT power failures.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Kevin Wolf 
---
 hw/scsi-generic.c |6 --
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 5ce01af..8f6b70d 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -244,12 +244,6 @@ static uint8_t *scsi_get_buf(SCSIRequest *req)
 static void scsi_req_fixup(SCSIRequest *req)
 {
 switch(req->cmd.buf[0]) {
-case WRITE_10:
-req->cmd.buf[1] &= ~0x08;  /* disable FUA */
-break;
-case READ_10:
-req->cmd.buf[1] &= ~0x08;  /* disable FUA */
-break;
 case REWIND:
 case START_STOP:
 if (req->dev->type == TYPE_TAPE) {
-- 
1.7.6.2




[Qemu-devel] [PATCH 14/20] rbd: ignore failures when reading from default conf location

2011-09-20 Thread Kevin Wolf
From: Sage Weil 

If we are reading from the default config location, ignore any failures.
It is perfectly legal for the user to specify exactly the options they need
and to not rely on any config file.

Signed-off-by: Sage Weil 
Signed-off-by: Kevin Wolf 
---
 block/rbd.c |   14 --
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 1b78d51..f64b2e0 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -298,11 +298,8 @@ static int qemu_rbd_create(const char *filename, 
QEMUOptionParameter *options)
 }
 
 if (strstr(conf, "conf=") == NULL) {
-if (rados_conf_read_file(cluster, NULL) < 0) {
-error_report("error reading config file");
-rados_shutdown(cluster);
-return -EIO;
-}
+/* try default location, but ignore failure */
+rados_conf_read_file(cluster, NULL);
 }
 
 if (conf[0] != '\0' &&
@@ -441,11 +438,8 @@ static int qemu_rbd_open(BlockDriverState *bs, const char 
*filename, int flags)
 }
 
 if (strstr(conf, "conf=") == NULL) {
-r = rados_conf_read_file(s->cluster, NULL);
-if (r < 0) {
-error_report("error reading config file");
-goto failed_shutdown;
-}
+/* try default location, but ignore failure */
+rados_conf_read_file(s->cluster, NULL);
 }
 
 if (conf[0] != '\0') {
-- 
1.7.6.2




[Qemu-devel] [PATCH] fix error: variable 'pid' set but not used

2011-09-20 Thread Paolo Bonzini
/home/pbonzini/work/upstream/qemu/posix-aio-compat.c: In function 'aio_thread':
/home/pbonzini/work/upstream/qemu/posix-aio-compat.c:314:11: error: variable 
'pid' set but not used [-Werror=unused-but-set-variable]

Signed-off-by: Paolo Bonzini 
---
Kevin, can you add this to your block branch pull for today?

 posix-aio-compat.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 393fa4b..d3c1174 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -311,10 +311,6 @@ static void posix_aio_notify_event(void);
 
 static void *aio_thread(void *unused)
 {
-pid_t pid;
-
-pid = getpid();
-
 mutex_lock(&lock);
 pending_threads--;
 mutex_unlock(&lock);
-- 
1.7.6




Re: [Qemu-devel] [PATCH] pci: implement bridge filtering

2011-09-20 Thread Michael S. Tsirkin
On Tue, Sep 20, 2011 at 02:44:26PM +0300, Avi Kivity wrote:
> On 09/20/2011 11:09 AM, Wen Congyang wrote:
> >At 09/14/2011 09:48 AM, Wen Congyang Write:
> >>  At 09/05/2011 02:13 AM, Michael S. Tsirkin Write:
> >>>  Support bridge filtering on top of the memory
> >>>  API as suggested by Avi Kivity:
> >>>
> >>>  Create a memory region for the bridge's address space.  This region is
> >>>  not directly added to system_memory or its descendants.  Devices under
> >>>  the bridge see this region as its pci_address_space().  The region is
> >>>  as large as the entire address space - it does not take into account
> >>>  any windows.
> >>>
> >>>  For each of the three windows (pref, non-pref, vga), create an alias
> >>>  with the appropriate start and size.  Map the alias into the bridge's
> >>>  parent's pci_address_space(), as subregions.
> >>>
> >>>  Signed-off-by: Michael S. Tsirkin
> >>>  ---
> >>>
> >>>  The below seems to work fine for me so I applied this.
> >>>  Still need to test bridge filtering, any help with this
> >>>  appreciated.
> >>>
> >>
> >>
> >>  I test bridge filtering, and the BAR still can be visible on guest even if
> >>  I change the memory region.
> >
> >Hi Michael S. Tsirkin:
> >I test pci bridge filtering on real hardware, and I find that I can mmap
> >the resource after I change the memory base and memory limit(The BAR should
> >be not visible on OS after changing the memory region).
> >
> >So I try to write and read to the BAR. Here is my test result:
> >1. Before changing the pci bridge's memory region, I can read and write to 
> >the memory, and
> >I can get the same value that I write.
> >
> >2. After changing the pci bridge's memory region, I can still read and write 
> >to the memory,
> >but it is very slow, and I can not get the same value that I write(The 
> > value is always 0).
> >
> >Does this result means that pci bridge filtering works fine?
> >
> 
> Yes.  Instead of hitting the BAR, you hit the default mmio handler.

Hmm, not sure what's right in that case.
But, same if BAR is disabled? Would be nice to make
some handler in bridge to get called, to set
master abort flag etc.

> -- 
> error compiling committee.c: too many arguments to function



[Qemu-devel] [PATCH 10/20] scsi-disk: lazily allocate bounce buffer

2011-09-20 Thread Kevin Wolf
From: Paolo Bonzini 

It will not be needed for reads and writes if the HBA provides a sglist.
In addition, this lets scsi-disk refuse commands with an excessive
allocation length, as well as limit memory on usual well-behaved guests.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 hw/scsi-disk.c |   44 +---
 1 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 84e8662..48abe49 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -55,6 +55,7 @@ typedef struct SCSIDiskReq {
 /* Both sector and sector_count are in terms of qemu 512 byte blocks.  */
 uint64_t sector;
 uint32_t sector_count;
+uint32_t buflen;
 struct iovec iov;
 QEMUIOVector qiov;
 uint32_t status;
@@ -78,13 +79,15 @@ struct SCSIDiskState
 };
 
 static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
-static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
+static int scsi_disk_emulate_command(SCSIDiskReq *r);
 
 static void scsi_free_request(SCSIRequest *req)
 {
 SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 
-qemu_vfree(r->iov.iov_base);
+if (r->iov.iov_base) {
+qemu_vfree(r->iov.iov_base);
+}
 }
 
 /* Helper function for command completion with sense.  */
@@ -110,7 +113,13 @@ static void scsi_cancel_io(SCSIRequest *req)
 
 static uint32_t scsi_init_iovec(SCSIDiskReq *r)
 {
-r->iov.iov_len = MIN(r->sector_count * 512, SCSI_DMA_BUF_SIZE);
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+if (!r->iov.iov_base) {
+r->buflen = SCSI_DMA_BUF_SIZE;
+r->iov.iov_base = qemu_blockalign(s->bs, r->buflen);
+}
+r->iov.iov_len = MIN(r->sector_count * 512, r->buflen);
 qemu_iovec_init_external(&r->qiov, &r->iov, 1);
 return r->qiov.size / 512;
 }
@@ -323,7 +332,7 @@ static void scsi_dma_restart_bh(void *opaque)
 scsi_write_data(&r->req);
 break;
 case SCSI_REQ_STATUS_RETRY_FLUSH:
-ret = scsi_disk_emulate_command(r, r->iov.iov_base);
+ret = scsi_disk_emulate_command(r);
 if (ret == 0) {
 scsi_req_complete(&r->req, GOOD);
 }
@@ -838,13 +847,31 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
 return 0;
 }
 
-static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
+static int scsi_disk_emulate_command(SCSIDiskReq *r)
 {
 SCSIRequest *req = &r->req;
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
 uint64_t nb_sectors;
+uint8_t *outbuf;
 int buflen = 0;
 
+if (!r->iov.iov_base) {
+/*
+ * FIXME: we shouldn't return anything bigger than 4k, but the code
+ * requires the buffer to be as big as req->cmd.xfer in several
+ * places.  So, do not allow CDBs with a very large ALLOCATION
+ * LENGTH.  The real fix would be to modify scsi_read_data and
+ * dma_buf_read, so that they return data beyond the buflen
+ * as all zeros.
+ */
+if (req->cmd.xfer > 65536) {
+goto illegal_request;
+}
+r->buflen = MAX(4096, req->cmd.xfer);
+r->iov.iov_base = qemu_blockalign(s->bs, r->buflen);
+}
+
+outbuf = r->iov.iov_base;
 switch (req->cmd.buf[0]) {
 case TEST_UNIT_READY:
 if (s->tray_open || !bdrv_is_inserted(s->bs))
@@ -995,11 +1022,9 @@ static int32_t scsi_send_command(SCSIRequest *req, 
uint8_t *buf)
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
 int32_t len;
 uint8_t command;
-uint8_t *outbuf;
 int rc;
 
 command = buf[0];
-outbuf = (uint8_t *)r->iov.iov_base;
 DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", req->lun, req->tag, 
buf[0]);
 
 #ifdef DEBUG_SCSI
@@ -1028,7 +1053,7 @@ static int32_t scsi_send_command(SCSIRequest *req, 
uint8_t *buf)
 case GET_CONFIGURATION:
 case SERVICE_ACTION_IN_16:
 case VERIFY_10:
-rc = scsi_disk_emulate_command(r, outbuf);
+rc = scsi_disk_emulate_command(r);
 if (rc < 0) {
 return 0;
 }
@@ -1279,11 +1304,8 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, 
uint32_t tag,
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
 SCSIRequest *req;
-SCSIDiskReq *r;
 
 req = scsi_req_alloc(&scsi_disk_reqops, &s->qdev, tag, lun, hba_private);
-r = DO_UPCAST(SCSIDiskReq, req, req);
-r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
 return req;
 }
 
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH] fix error: variable 'pid' set but not used

2011-09-20 Thread Stefan Hajnoczi
On Tue, Sep 20, 2011 at 1:18 PM, Paolo Bonzini  wrote:
> /home/pbonzini/work/upstream/qemu/posix-aio-compat.c: In function 
> 'aio_thread':
> /home/pbonzini/work/upstream/qemu/posix-aio-compat.c:314:11: error: variable 
> 'pid' set but not used [-Werror=unused-but-set-variable]
>
> Signed-off-by: Paolo Bonzini 
> ---
>        Kevin, can you add this to your block branch pull for today?
>
>  posix-aio-compat.c |    4 
>  1 files changed, 0 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi 



Re: [Qemu-devel] [PATCH] pci: implement bridge filtering

2011-09-20 Thread Avi Kivity

On 09/20/2011 03:22 PM, Michael S. Tsirkin wrote:

>
>  Yes.  Instead of hitting the BAR, you hit the default mmio handler.

Hmm, not sure what's right in that case.
But, same if BAR is disabled? Would be nice to make
some handler in bridge to get called, to set
master abort flag etc.


Put an mmio region spanning the entire pci address range as a child of 
the pci address space.  Make sure it has lower priority than any of the 
BARs (or vga areas).


--
error compiling committee.c: too many arguments to function




[Qemu-devel] [PATCH 16/20] rbd: call flush, if available

2011-09-20 Thread Kevin Wolf
From: Sage Weil 

librbd recently added async writeback and flush support.  If the new
rbd_flush() call is available, call it.

Signed-off-by: Sage Weil 
Signed-off-by: Kevin Wolf 
---
 block/rbd.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 23d8751..98efd2c 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -680,6 +680,17 @@ static BlockDriverAIOCB 
*qemu_rbd_aio_writev(BlockDriverState *bs,
 return rbd_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque, 1);
 }
 
+static int qemu_rbd_flush(BlockDriverState *bs)
+{
+#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 1)
+/* rbd_flush added in 0.1.1 */
+BDRVRBDState *s = bs->opaque;
+return rbd_flush(s->image);
+#else
+return 0;
+#endif
+}
+
 static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
 BDRVRBDState *s = bs->opaque;
@@ -815,6 +826,7 @@ static BlockDriver bdrv_rbd = {
 .bdrv_file_open = qemu_rbd_open,
 .bdrv_close = qemu_rbd_close,
 .bdrv_create= qemu_rbd_create,
+.bdrv_flush = qemu_rbd_flush,
 .bdrv_get_info  = qemu_rbd_getinfo,
 .create_options = qemu_rbd_create_options,
 .bdrv_getlength = qemu_rbd_getlength,
-- 
1.7.6.2




[Qemu-devel] [PATCH 20/20] rbd: allow escaping in config string

2011-09-20 Thread Kevin Wolf
From: Sage Weil 

The config string is variously delimited by =, @, and /, depending on the
field.  Allow these characters to be escaped by preceeding them with \.

Signed-off-by: Sage Weil 
Signed-off-by: Kevin Wolf 
---
 block/rbd.c |   29 +++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 98efd2c..3068c82 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -102,8 +102,15 @@ static int qemu_rbd_next_tok(char *dst, int dst_len,
 *p = NULL;
 
 if (delim != '\0') {
-end = strchr(src, delim);
-if (end) {
+for (end = src; *end; ++end) {
+if (*end == delim) {
+break;
+}
+if (*end == '\\' && end[1] != '\0') {
+end++;
+}
+}
+if (*end == delim) {
 *p = end + 1;
 *end = '\0';
 }
@@ -122,6 +129,19 @@ static int qemu_rbd_next_tok(char *dst, int dst_len,
 return 0;
 }
 
+static void qemu_rbd_unescape(char *src)
+{
+char *p;
+
+for (p = src; *src; ++src, ++p) {
+if (*src == '\\' && src[1] != '\0') {
+src++;
+}
+*p = *src;
+}
+*p = '\0';
+}
+
 static int qemu_rbd_parsename(const char *filename,
   char *pool, int pool_len,
   char *snap, int snap_len,
@@ -146,6 +166,7 @@ static int qemu_rbd_parsename(const char *filename,
 ret = -EINVAL;
 goto done;
 }
+qemu_rbd_unescape(pool);
 
 if (strchr(p, '@')) {
 ret = qemu_rbd_next_tok(name, name_len, p, '@', "object name", &p);
@@ -153,9 +174,11 @@ static int qemu_rbd_parsename(const char *filename,
 goto done;
 }
 ret = qemu_rbd_next_tok(snap, snap_len, p, ':', "snap name", &p);
+qemu_rbd_unescape(snap);
 } else {
 ret = qemu_rbd_next_tok(name, name_len, p, ':', "object name", &p);
 }
+qemu_rbd_unescape(name);
 if (ret < 0 || !p) {
 goto done;
 }
@@ -211,6 +234,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char 
*conf)
 if (ret < 0) {
 break;
 }
+qemu_rbd_unescape(name);
 
 if (!p) {
 error_report("conf option %s has no value", name);
@@ -223,6 +247,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char 
*conf)
 if (ret < 0) {
 break;
 }
+qemu_rbd_unescape(value);
 
 if (strcmp(name, "conf") == 0) {
 ret = rados_conf_read_file(cluster, value);
-- 
1.7.6.2




[Qemu-devel] [PATCH 18/20] block: avoid SIGUSR2

2011-09-20 Thread Kevin Wolf
From: Frediano Ziglio 

Now that iothread is always compiled sending a signal seems only an
additional step. This patch also avoid writing to two pipe (one from signal
and one in qemu_service_io).

Work with kvm enabled or disabled. strace output is more readable (less 
syscalls).

Signed-off-by: Frediano Ziglio 
Reviewed-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 cpus.c |5 -
 posix-aio-compat.c |   29 +
 2 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/cpus.c b/cpus.c
index 54c188c..d0cfe91 100644
--- a/cpus.c
+++ b/cpus.c
@@ -380,11 +380,6 @@ static int qemu_signal_init(void)
 int sigfd;
 sigset_t set;
 
-/* SIGUSR2 used by posix-aio-compat.c */
-sigemptyset(&set);
-sigaddset(&set, SIGUSR2);
-pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-
 /*
  * SIG_IPI must be blocked in the main thread and must not be caught
  * by sigwait() in the signal thread. Otherwise, the cpu thread will
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 63a8fae..393fa4b 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -42,7 +42,6 @@ struct qemu_paiocb {
 int aio_niov;
 size_t aio_nbytes;
 #define aio_ioctl_cmd   aio_nbytes /* for QEMU_AIO_IOCTL */
-int ev_signo;
 off_t aio_offset;
 
 QTAILQ_ENTRY(qemu_paiocb) node;
@@ -308,6 +307,8 @@ static ssize_t handle_aiocb_rw(struct qemu_paiocb *aiocb)
 return nbytes;
 }
 
+static void posix_aio_notify_event(void);
+
 static void *aio_thread(void *unused)
 {
 pid_t pid;
@@ -380,7 +381,7 @@ static void *aio_thread(void *unused)
 aiocb->ret = ret;
 mutex_unlock(&lock);
 
-if (kill(pid, aiocb->ev_signo)) die("kill failed");
+posix_aio_notify_event();
 }
 
 cur_threads--;
@@ -547,18 +548,14 @@ static int posix_aio_flush(void *opaque)
 
 static PosixAioState *posix_aio_state;
 
-static void aio_signal_handler(int signum)
+static void posix_aio_notify_event(void)
 {
-if (posix_aio_state) {
-char byte = 0;
-ssize_t ret;
-
-ret = write(posix_aio_state->wfd, &byte, sizeof(byte));
-if (ret < 0 && errno != EAGAIN)
-die("write()");
-}
+char byte = 0;
+ssize_t ret;
 
-qemu_service_io();
+ret = write(posix_aio_state->wfd, &byte, sizeof(byte));
+if (ret < 0 && errno != EAGAIN)
+die("write()");
 }
 
 static void paio_remove(struct qemu_paiocb *acb)
@@ -622,7 +619,6 @@ BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd,
 return NULL;
 acb->aio_type = type;
 acb->aio_fildes = fd;
-acb->ev_signo = SIGUSR2;
 
 if (qiov) {
 acb->aio_iov = qiov->iov;
@@ -650,7 +646,6 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd,
 return NULL;
 acb->aio_type = QEMU_AIO_IOCTL;
 acb->aio_fildes = fd;
-acb->ev_signo = SIGUSR2;
 acb->aio_offset = 0;
 acb->aio_ioctl_buf = buf;
 acb->aio_ioctl_cmd = req;
@@ -664,7 +659,6 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd,
 
 int paio_init(void)
 {
-struct sigaction act;
 PosixAioState *s;
 int fds[2];
 int ret;
@@ -674,11 +668,6 @@ int paio_init(void)
 
 s = g_malloc(sizeof(PosixAioState));
 
-sigfillset(&act.sa_mask);
-act.sa_flags = 0; /* do not restart syscalls to interrupt select() */
-act.sa_handler = aio_signal_handler;
-sigaction(SIGUSR2, &act, NULL);
-
 s->first_aio = NULL;
 if (qemu_pipe(fds) == -1) {
 fprintf(stderr, "failed to create pipe\n");
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH] fix error: variable 'pid' set but not used

2011-09-20 Thread Kevin Wolf
Am 20.09.2011 14:18, schrieb Paolo Bonzini:
> /home/pbonzini/work/upstream/qemu/posix-aio-compat.c: In function 
> 'aio_thread':
> /home/pbonzini/work/upstream/qemu/posix-aio-compat.c:314:11: error: variable 
> 'pid' set but not used [-Werror=unused-but-set-variable]
> 
> Signed-off-by: Paolo Bonzini 
> ---
>   Kevin, can you add this to your block branch pull for today?

Thanks, merged it into Frediano's patch that caused the warning and
pushed the update to my for-anthony branch.

Kevin



Re: [Qemu-devel] [PATCH v8 3/4] block: add block timer and throttling algorithm

2011-09-20 Thread Marcelo Tosatti
On Mon, Sep 19, 2011 at 05:55:41PM +0800, Zhi Yong Wu wrote:
> On Wed, Sep 14, 2011 at 6:50 PM, Marcelo Tosatti  wrote:
> > On Tue, Sep 13, 2011 at 11:09:46AM +0800, Zhi Yong Wu wrote:
> >> On Fri, Sep 9, 2011 at 10:44 PM, Marcelo Tosatti  
> >> wrote:
> >> > On Thu, Sep 08, 2011 at 06:11:07PM +0800, Zhi Yong Wu wrote:
> >> >> Note:
> >> >>      1.) When bps/iops limits are specified to a small value such as 
> >> >> 511 bytes/s, this VM will hang up. We are considering how to handle 
> >> >> this senario.
> >> >
> >> > You can increase the length of the slice, if the request is larger than
> >> > slice_time * bps_limit.
> >> Yeah, but it is a challenge for how to increase it. Do you have some nice 
> >> idea?
> >
> > If the queue is empty, and the request being processed does not fit the
> > queue, increase the slice so that the request fits.
> Sorry for late reply. actually, do you think that this scenario is
> meaningful for the user?
> Since we implement this, if the user limits the bps below 512
> bytes/second, the VM can also not run every task.
> Can you let us know why we need to make such effort?

It would be good to handle request larger than the slice.

It is not strictly necessary, but in case its not handled, a minimum
should be in place, to reflect maximum request size known. Being able to
specify something which crashes is not acceptable.




Re: [Qemu-devel] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

2011-09-20 Thread Lluís Vilanova
Stefan Hajnoczi writes:

> On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova 
>> ---
>> trace-events |4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/trace-events b/trace-events
>> index 9d1fbbb..b653d70 100644
>> --- a/trace-events
>> +++ b/trace-events
>> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>> # hw/milkymist-softusb.c
>> milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x 
>> value %08x"
>> milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x 
>> value %08x"
>> -milkymist_softusb_mevt(uint8_t m) "m %d"
>> -milkymist_softusb_kevt(uint8_t m) "m %d"
>> +milkymist_softusb_mevt(uint8_t _m) "m %d"
>> +milkymist_softusb_kevt(uint8_t _m) "m %d"

> The LTTng community has been very responsive in addressing namespace
> issues with libust.  Let's post more details and see if it can be fixed
> in libust.

> Could you please post your gcc and libust versions?

> I have not been able to reproduce the problem on Debian libust-dev
> 0.15-3.  My gcc version is Debian gcc 4.6.1-4.

Yup, I was using libust-dev 0.5 (debian stable). After switching to 0.15 (debian
testing) other problems arise.

For example, in my box compiling osdep.c yields lots of "variable ‘__tp_cb_data’
set but not used", but using a minimal example does not raise such problems, so
I'm assuming this is related to some namespace problems related to some other
code in qemu's headers.

Unfortunately, right now I have no time to debug the causes, so I'll just assume
the two patches I sent (this naming issue and the zero-length printf format) are
just no longer necessary with recent versions of libust.


Thanks,
Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth



Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

2011-09-20 Thread P DUMAS
* Stefan Hajnoczi (stefa...@gmail.com) wrote:
> On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> > Signed-off-by: Lluís Vilanova 
> > ---
> >  trace-events |4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/trace-events b/trace-events
> > index 9d1fbbb..b653d70 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
> >  # hw/milkymist-softusb.c
> >  milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x 
> > value %08x"
> >  milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x 
> > value %08x"
> > -milkymist_softusb_mevt(uint8_t m) "m %d"
> > -milkymist_softusb_kevt(uint8_t m) "m %d"
> > +milkymist_softusb_mevt(uint8_t _m) "m %d"
> > +milkymist_softusb_kevt(uint8_t _m) "m %d"
> 
> The LTTng community has been very responsive in addressing namespace
> issues with libust.  Let's post more details and see if it can be fixed
> in libust.
> 
> Could you please post your gcc and libust versions?
> 
> I have not been able to reproduce the problem on Debian libust-dev
> 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
> 
> Here is the test program:
> 
> #include 
> #include 
> 
> DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
> TP_ARGS(m));
> 
> int main(int argc, char **argv)
> {
>   return 0;
> }
> 
> I see no warning or error related to the 'm' argument name.

I remember fixing that for trace_mark() instrumentation in UST 0.x ages
ago (ok, probably last winter). I think simply updating to a new UST
version will fix the issue.

Best regards,

Mathieu

> 
> Stefan
> 
> ___
> ltt-dev mailing list
> ltt-...@lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com



Re: [Qemu-devel] [PATCH 04/14] qdev: take ownership of id pointer

2011-09-20 Thread Anthony Liguori

On 09/20/2011 01:36 AM, Gerd Hoffmann wrote:

On 09/19/11 18:27, Anthony Liguori wrote:

On 09/19/2011 02:34 AM, Gerd Hoffmann wrote:

FYI: Keeping the pointer to the QemuOpts has one more reason: It will
free the
QemuOpts on hot-unplug, which is needed to free the id from QemuOpts
point of
view, which in turn allows to re-use the id when hot-plugging the same
(or
another) device later on.


You mean, tie QemuOpts life cycle to devices life cycle


Yes.


such that you
cannot accidentally create a non-device QemuOpts that conflicts with the
id of a device?


Device QemuOpts have their own id namespace, so this is just about conflicts
within devices. This ...

device_add e1000,id=nic1
device_del nic1
device_add e1000,id=nic1

... will work only if you free the QemuOpts when deleting a device, otherwise
QemuOpts will complain that nic1 is used already.


But we can just verify that the id specified for qdev is unique at creation time 
and fail creation if it isn't, no?


Since not all devices are assigned names via qemuopts, that seems like a safer 
approach anyway.


Regards,

Anthony Liguori



cheers,
Gerd





[Qemu-devel] [PATCH v2 0/3] Remove QEMUFile abuse

2011-09-20 Thread Juan Quintela
Hi

v2: Make malc happy release O;-)
- learn how to use fwrite/fread correctly O:-)
- add errno to all error messages
- make checkpatch happy. Except for
  WARNING: space prohibited between function name and open parenthesis '('

  Please apply.

v1:

QEMUFile is intended to be used only for migration.  Change the other
three users to use FILE * operations directly.  gcc on Fedora 15
complains about fread/write not checking its return value, so I added
checks.  But in several places only print an error message (there is
no error handly that I can hook into).  Notice that this is not worse
than it is today.

Later, Juan.

Juan Quintela (3):
  wavaudio: Use stdio instead of QEMUFile
  wavcapture:  Use stdio instead of QEMUFile
  ds1225y: Use stdio instead of QEMUFile

 audio/wavaudio.c   |   34 --
 audio/wavcapture.c |   48 ++--
 hw/ds1225y.c   |   28 
 3 files changed, 74 insertions(+), 36 deletions(-)

-- 
1.7.6.2




[Qemu-devel] [PATCH 1/3] wavaudio: Use stdio instead of QEMUFile

2011-09-20 Thread Juan Quintela
QEMUFile * is only intended for migration nowadays.  Using it for
anything else just adds pain and a layer of buffers for no good
reason.

Signed-off-by: Juan Quintela 
CC:  malc 

---
 audio/wavaudio.c |   34 --
 1 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/audio/wavaudio.c b/audio/wavaudio.c
index aed1817..7b7c081 100644
--- a/audio/wavaudio.c
+++ b/audio/wavaudio.c
@@ -30,7 +30,7 @@

 typedef struct WAVVoiceOut {
 HWVoiceOut hw;
-QEMUFile *f;
+FILE *f;
 int64_t old_ticks;
 void *pcm_buf;
 int total_samples;
@@ -76,7 +76,9 @@ static int wav_run_out (HWVoiceOut *hw, int live)
 dst = advance (wav->pcm_buf, rpos << hw->info.shift);

 hw->clip (dst, src, convert_samples);
-qemu_put_buffer (wav->f, dst, convert_samples << hw->info.shift);
+if (fwrite (dst, convert_samples << hw->info.shift, 1, wav->f) != 1) {
+dolog ("wav_run_out: fwrite error '%s'\n", strerror(errno));
+}

 rpos = (rpos + convert_samples) % hw->samples;
 samples -= convert_samples;
@@ -152,7 +154,7 @@ static int wav_init_out (HWVoiceOut *hw, struct audsettings 
*as)
 le_store (hdr + 28, hw->info.freq << (bits16 + stereo), 4);
 le_store (hdr + 32, 1 << (bits16 + stereo), 2);

-wav->f = qemu_fopen (conf.wav_path, "wb");
+wav->f = fopen (conf.wav_path, "wb");
 if (!wav->f) {
 dolog ("Failed to open wave file `%s'\nReason: %s\n",
conf.wav_path, strerror (errno));
@@ -161,7 +163,10 @@ static int wav_init_out (HWVoiceOut *hw, struct 
audsettings *as)
 return -1;
 }

-qemu_put_buffer (wav->f, hdr, sizeof (hdr));
+if (fwrite (hdr, sizeof (hdr), 1, wav->f) != 1) {
+dolog ("wav_init_out: fwrite error '%s'\n", strerror(errno));
+return -1;
+}
 return 0;
 }

@@ -180,13 +185,22 @@ static void wav_fini_out (HWVoiceOut *hw)
 le_store (rlen, rifflen, 4);
 le_store (dlen, datalen, 4);

-qemu_fseek (wav->f, 4, SEEK_SET);
-qemu_put_buffer (wav->f, rlen, 4);
-
-qemu_fseek (wav->f, 32, SEEK_CUR);
-qemu_put_buffer (wav->f, dlen, 4);
+if (fseek (wav->f, 4, SEEK_SET) == -1) {
+dolog ("wav_fini_out: fseek error '%s'\n", strerror(errno));
+}
+if (fwrite (rlen, 1, 4, wav->f) != 4) {
+dolog ("wav_fini_out: fwrite error writing rlen '%s'\n",
+   strerror(errno));
+}
+if (fseek (wav->f, 32, SEEK_CUR) == -1) {
+dolog ("wav_fini_out: fseek error '%s'\n", strerror(errno));
+}
+if (fwrite (dlen, 1, 4, wav->f) != 4) {
+dolog ("wav_fini_out: fwrite error writing dlen '%s'\n",
+   strerror(errno));
+}

-qemu_fclose (wav->f);
+fclose (wav->f);
 wav->f = NULL;

 g_free (wav->pcm_buf);
-- 
1.7.6.2




[Qemu-devel] [PATCH 2/3] wavcapture: Use stdio instead of QEMUFile

2011-09-20 Thread Juan Quintela
QEMUFile * is only intended for migration nowadays.  Using it for
anything else just adds pain and a layer of buffers for no good
reason.

Signed-off-by: Juan Quintela 
CC: malc 

---
 audio/wavcapture.c |   48 ++--
 1 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/audio/wavcapture.c b/audio/wavcapture.c
index c64f0ef..9019995 100644
--- a/audio/wavcapture.c
+++ b/audio/wavcapture.c
@@ -3,7 +3,7 @@
 #include "audio.h"

 typedef struct {
-QEMUFile *f;
+FILE *f;
 int bytes;
 char *path;
 int freq;
@@ -40,12 +40,22 @@ static void wav_destroy (void *opaque)
 le_store (rlen, rifflen, 4);
 le_store (dlen, datalen, 4);

-qemu_fseek (wav->f, 4, SEEK_SET);
-qemu_put_buffer (wav->f, rlen, 4);
-
-qemu_fseek (wav->f, 32, SEEK_CUR);
-qemu_put_buffer (wav->f, dlen, 4);
-qemu_fclose (wav->f);
+fseek (wav->f, 4, SEEK_SET);
+if (fseek (wav->f, 4, SEEK_SET) == -1) {
+printf ("wav_destroy: fseek error %s\n", strerror(errno));
+}
+if (fwrite (rlen, 1, 4, wav->f) != 4) {
+printf ("wav_destroy: fwrite error writing rlen '%s'\n",
+strerror(errno));
+}
+if (fseek (wav->f, 32, SEEK_CUR) == -1) {
+printf ("wav_destroy: fseek error '%s'\n", strerror(errno));
+}
+if (fwrite (dlen, 1, 4, wav->f) != 4) {
+printf ("wav_destroy: fwrite error writing dlen '%s'\n",
+strerror(errno));
+}
+fclose (wav->f);
 }

 g_free (wav->path);
@@ -55,7 +65,10 @@ static void wav_capture (void *opaque, void *buf, int size)
 {
 WAVState *wav = opaque;

-qemu_put_buffer (wav->f, buf, size);
+if (fwrite (buf, size, 1, wav->f) != 1) {
+printf ("wav_capture: fwrite error '%s'\n",
+strerror(errno));
+}
 wav->bytes += size;
 }

@@ -130,7 +143,7 @@ int wav_start_capture (CaptureState *s, const char *path, 
int freq,
 le_store (hdr + 28, freq << shift, 4);
 le_store (hdr + 32, 1 << shift, 2);

-wav->f = qemu_fopen (path, "wb");
+wav->f = fopen (path, "wb");
 if (!wav->f) {
 monitor_printf(mon, "Failed to open wave file `%s'\nReason: %s\n",
path, strerror (errno));
@@ -143,19 +156,26 @@ int wav_start_capture (CaptureState *s, const char *path, 
int freq,
 wav->nchannels = nchannels;
 wav->freq = freq;

-qemu_put_buffer (wav->f, hdr, sizeof (hdr));
+if (fwrite (hdr, sizeof (hdr), 1, wav->f) != 1) {
+monitor_printf (mon, "Failed to write header '%s'\n",
+strerror(errno));
+goto error_free;
+}

 cap = AUD_add_capture (&as, &ops, wav);
 if (!cap) {
 monitor_printf(mon, "Failed to add audio capture\n");
-g_free (wav->path);
-qemu_fclose (wav->f);
-g_free (wav);
-return -1;
+goto error_free;
 }

 wav->cap = cap;
 s->opaque = wav;
 s->ops = wav_capture_ops;
 return 0;
+
+error_free:
+g_free (wav->path);
+fclose (wav->f);
+g_free (wav);
+return -1;
 }
-- 
1.7.6.2




[Qemu-devel] [PATCH 3/3] ds1225y: Use stdio instead of QEMUFile

2011-09-20 Thread Juan Quintela
QEMUFile * is only intended for migration nowadays.  Using it for
anything else just adds pain and a layer of buffers for no good
reason.

Signed-off-by: Juan Quintela 
---
 hw/ds1225y.c |   28 
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/ds1225y.c b/hw/ds1225y.c
index 9875c44..6852a61 100644
--- a/hw/ds1225y.c
+++ b/hw/ds1225y.c
@@ -29,7 +29,7 @@ typedef struct {
 DeviceState qdev;
 uint32_t chip_size;
 char *filename;
-QEMUFile *file;
+FILE *file;
 uint8_t *contents;
 } NvRamState;

@@ -70,9 +70,9 @@ static void nvram_writeb (void *opaque, target_phys_addr_t 
addr, uint32_t val)

 s->contents[addr] = val;
 if (s->file) {
-qemu_fseek(s->file, addr, SEEK_SET);
-qemu_put_byte(s->file, (int)val);
-qemu_fflush(s->file);
+fseek(s->file, addr, SEEK_SET);
+fputc(val, s->file);
+fflush(s->file);
 }
 }

@@ -108,15 +108,17 @@ static int nvram_post_load(void *opaque, int version_id)

 /* Close file, as filename may has changed in load/store process */
 if (s->file) {
-qemu_fclose(s->file);
+fclose(s->file);
 }

 /* Write back nvram contents */
-s->file = qemu_fopen(s->filename, "wb");
+s->file = fopen(s->filename, "wb");
 if (s->file) {
 /* Write back contents, as 'wb' mode cleaned the file */
-qemu_put_buffer(s->file, s->contents, s->chip_size);
-qemu_fflush(s->file);
+if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
+printf("nvram_post_load: short write\n");
+}
+fflush(s->file);
 }

 return 0;
@@ -143,7 +145,7 @@ typedef struct {
 static int nvram_sysbus_initfn(SysBusDevice *dev)
 {
 NvRamState *s = &FROM_SYSBUS(SysBusNvRamState, dev)->nvram;
-QEMUFile *file;
+FILE *file;
 int s_io;

 s->contents = g_malloc0(s->chip_size);
@@ -153,11 +155,13 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
 sysbus_init_mmio(dev, s->chip_size, s_io);

 /* Read current file */
-file = qemu_fopen(s->filename, "rb");
+file = fopen(s->filename, "rb");
 if (file) {
 /* Read nvram contents */
-qemu_get_buffer(file, s->contents, s->chip_size);
-qemu_fclose(file);
+if (fread(s->contents, s->chip_size, 1, file) != 1) {
+printf("nvram_sysbus_initfn: short read\n");
+}
+fclose(file);
 }
 nvram_post_load(s, 0);

-- 
1.7.6.2




Re: [Qemu-devel] [PATCH 04/14] qdev: take ownership of id pointer

2011-09-20 Thread Gerd Hoffmann

On 09/20/11 15:04, Anthony Liguori wrote:

On 09/20/2011 01:36 AM, Gerd Hoffmann wrote:

On 09/19/11 18:27, Anthony Liguori wrote:

On 09/19/2011 02:34 AM, Gerd Hoffmann wrote:

FYI: Keeping the pointer to the QemuOpts has one more reason: It will
free the
QemuOpts on hot-unplug, which is needed to free the id from QemuOpts
point of
view, which in turn allows to re-use the id when hot-plugging the same
(or
another) device later on.


You mean, tie QemuOpts life cycle to devices life cycle


Yes.


such that you
cannot accidentally create a non-device QemuOpts that conflicts with the
id of a device?


Device QemuOpts have their own id namespace, so this is just about
conflicts
within devices. This ...

device_add e1000,id=nic1
device_del nic1
device_add e1000,id=nic1

... will work only if you free the QemuOpts when deleting a device,
otherwise
QemuOpts will complain that nic1 is used already.


But we can just verify that the id specified for qdev is unique at
creation time and fail creation if it isn't, no?

Since not all devices are assigned names via qemuopts, that seems like a
safer approach anyway.


I think that happens anyway (didn't check the source though).

Problem is that QemuOpts wants IDs being unique too, so keep the 
QemuOpts hanging around instead of releasing them makes QemuOpts 
complain about nic1 not being unique although there isn't such a device 
in qdev space.  Oh, and not releasing the QemuOpts would also leak 
memory on each hot-unplug.


cheers,
  Gerd




[Qemu-devel] [PATCH 3/7] migration: Check that migration is active before cancel it

2011-09-20 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration.c b/migration.c
index 15d001e..c56d29c 100644
--- a/migration.c
+++ b/migration.c
@@ -132,9 +132,9 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 {
 MigrationState *s = current_migration;

-if (s)
+if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
 s->cancel(s);
-
+}
 return 0;
 }

-- 
1.7.6.2



From arch-general-boun...@archlinux.org Tue Sep 20 06:25:30 2011
Return-path: 
Envelope-to: arch...@mail-archive.com
Delivery-date: Tue, 20 Sep 2011 06:25:30 -0700
Received: from exprod5mx205.postini.com ([64.18.0.64] helo=psmtp.com)
by mail-archive.com with smtp (Exim 4.69)
(envelope-from )
id 1R60KI-fo-0i
for arch...@mail-archive.com; Tue, 20 Sep 2011 06:25:30 -0700
Received: from archlinux.org ([66.211.214.132]) by exprod5mx205.postini.com 
([64.18.4.10]) with SMTP;
Tue, 20 Sep 2011 09:25:26 EDT
Received: from gudrun.archlinux.org (gudrun.archlinux.org [66.211.214.131])
by archlinux.org (Postfix) with ESMTP id 1138092022;
Tue, 20 Sep 2011 09:25:13 -0400 (EDT)
Received: from archlinux.org (gerolde.archlinux.org [66.211.214.132])
 by gudrun.archlinux.org (Postfix) with ESMTP id 53C2B84092
 for ; Tue, 20 Sep 2011 09:25:09 -0400 (EDT)
Received-SPF: pass (gmail.com ... _spf.google.com: 74.125.83.46 is authorized
 to use 'xecy...@gmail.com' in 'mfrom' identity (mechanism 'ip4:74.125.0.0/16'
 matched)) receiver=gerolde.archlinux.org; identity=mailfrom;
 envelope-from="xecy...@gmail.com"; helo=mail-gw0-f46.google.com;
 client-ip=74.125.83.46
Received: from mail-gw0-f46.google.com (mail-gw0-f46.google.com [74.125.83.46])
 by archlinux.org (Postfix) with ESMTPS id 1DF5B92015
 for ; Tue, 20 Sep 2011 09:25:08 -0400 (EDT)
Received: by gwb15 with SMTP id 15so691305gwb.33
 for ; Tue, 20 Sep 2011 06:25:16 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;
 h=from:to:subject:references:date:in-reply-to:message-id:user-agent
 :mime-version:content-type;
 bh=O07F8dYDSDfdkCng/ohskr+1Ysz8wbqPvxhnfiuvMGA=;
 b=sUwUrezTljJ7XZFEd19qkMfGM5iVrZtq+d6GCL8Sma7Dm4gYBe1ZO4clv3YOaEZh8Q
 6K8vHy+p4ejg0JJ34RoNmrnj55OUp2cYoZaq2gzq/hKPQ+imQIgIcE41bwJuvcg//OqR
 d8xx63JC2b2YA1k7QmCCCXpkmB6lNqj6W7zLY=
Received: by 10.68.29.129 with SMTP id k1mr6414660pbh.45.1316525116131;
 Tue, 20 Sep 2011 06:25:16 -0700 (PDT)
Received: from localhost ([2001:da8:8000:e104:ca0a:a9ff:fe72:7a0d])
 by mx.google.com with ESMTPS id i8sm6052325pbp.1.2011.09.20.06.25.12
 (version=TLSv1/SSLv3 cipher=OTHER);
 Tue, 20 Sep 2011 06:25:14 -0700 (PDT)
From: XeCycle 
To: General Discussion about Arch Linux 
References: <87wrd5sw4b@gmail.com> <1316409772-sup-2899@eris>
 <87vcsnsi2h@gmail.com> <20110920091118.GB22919@blizzard>
 <871uvb8rbm@gmail.com> <20110920102258.GB5917@blizzard>
Date: Tue, 20 Sep 2011 21:25:04 +0800
In-Reply-To: <20110920102258.GB5917@blizzard> (Lukas Fleischer's message of
 "Tue, 20 Sep 2011 12:22:58 +0200")
Message-ID: <87wrd373db@gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
 micalg=pgp-sha1; protocol="application/pgp-signature"
Subject: Re: [arch-general] Any suggestions on frequently rebuilding a git
package?
X-BeenThere: arch-gene...@archlinux.org
X-Mailman-Version: 2.1.14
Precedence: list
Reply-To: General Discussion about Arch Linux 
List-Id: General Discussion about Arch Linux 
List-Unsubscribe: , 
 
List-Archive: 
List-Post: 
List-Help: 
List-Subscribe: , 
 
Errors-To: arch-general-boun...@archlinux.org
Sender: arch-general-boun...@archlinux.org
X-pstn-levels: (S:99.9/99.9 CV:99.9000 FC:95.5390 LC:95.5390 
R:95.9108 P:95.9108 M:97.0282 C:98.6951 )
X-pstn-settings: 4 (1.5000:1.5000) s cv gt3 gt2 gt1 r p m c 
X-pstn-addresses: from  [294/10] 

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

Lukas Fleischer  writes:

> On Tue, Sep 20, 2011 at 06:02:21PM +0800, XeCycle wrote:

[...]

>> >> Well, this is my PKGBUILD-git.proto:
>> >>=20
>> >> 43  #
>> >> 44  # BUILD HERE
>> >> 45  #
>> >> 46  ./autogen.sh
>> >> 47  ./configure --prefix=3D/usr
>> >> 48  make
>> >>=20
>> >> Do you mean commenting 46 & 47?
>> >
>> > 
>> > $ sed -n '47,48p' /usr/share/pacman/PKGBUILD-git.proto
>> >   rm -rf "$srcdir/$_gitname-build"
>> >   git clone "$srcdir/$_gitname" "$srcdir/$_gitname-build"
>> > 
>> >

[Qemu-devel] [PATCH 0/7] Handle errors during migration

2011-09-20 Thread Juan Quintela
Hi

This patch series contains error handling for migration.

After this series are applied, migrate_cancel after one error don't
hang.  And we add some error checking left and right.

This is the error handling patches that were on the middle of my
migration-cleanup of some months ago.  migration_cancel fix has been
added.

Later, Juan.

Juan Quintela (5):
  migration: simplify state assignmente
  migration: only flush when there are no errors
  migration: Check that migration is active before cancel it
  migration: If there is one error, it makes no sense to continue
  migration: qemu_savevm_iterate has three return values

Yoshiaki Tamura (2):
  savevm: avoid qemu_savevm_state_iterate() to return 1 when qemu file
has error.
  migration: add error handling to migrate_fd_put_notify().

 buffered_file.c  |   19 +++
 buffered_file.h  |2 +-
 hw/hw.h  |4 ++--
 migration-exec.c |6 +++---
 migration-fd.c   |4 ++--
 migration-tcp.c  |4 ++--
 migration-unix.c |4 ++--
 migration.c  |   33 ++---
 migration.h  |4 ++--
 savevm.c |   27 +++
 10 files changed, 58 insertions(+), 49 deletions(-)

-- 
1.7.6.2




Re: [Qemu-devel] [PATCH] ahci: add port I/O index-data pair

2011-09-20 Thread Kevin Wolf
Am 28.08.2011 20:48, schrieb Alexander Graf:
> 
> On 27.08.2011, at 04:12, Daniel Verkamp wrote:
> 
>> Implement an I/O space index-data register pair as defined by the AHCI
>> spec, including the corresponding SATA PCI capability and BAR.
>>
>> This allows real-mode code to access the AHCI registers; real-mode
>> code cannot address the memory-mapped register space because it is
>> beyond the first megabyte.
> 
> Very nice patch! I'll check and compare with a real ICH-9 when I get back to 
> .de, but I'd assume you also did that already ;). Once I checked that the IO 
> region is set up similarly, I'll give you my ack.

What's the status with review/testing of this patch, Alex? Is it ready
to be merged, should I drop it or do you just need some more time?

Kevin



[Qemu-devel] [PATCH 1/7] migration: simplify state assignmente

2011-09-20 Thread Juan Quintela
Once there, make sure that if we already know that there is one error, just
call migration_fd_cleanup() with the ERROR state.

Signed-off-by: Juan Quintela 
---
 migration.c |   11 +--
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/migration.c b/migration.c
index f5959b4..9a93e3b 100644
--- a/migration.c
+++ b/migration.c
@@ -370,7 +370,6 @@ void migrate_fd_put_ready(void *opaque)

 DPRINTF("iterate\n");
 if (qemu_savevm_state_iterate(s->mon, s->file) == 1) {
-int state;
 int old_vm_running = vm_running;

 DPRINTF("done iterating\n");
@@ -380,17 +379,17 @@ void migrate_fd_put_ready(void *opaque)
 if (old_vm_running) {
 vm_start();
 }
-state = MIG_STATE_ERROR;
-} else {
-state = MIG_STATE_COMPLETED;
+s->state = MIG_STATE_ERROR;
 }
 if (migrate_fd_cleanup(s) < 0) {
 if (old_vm_running) {
 vm_start();
 }
-state = MIG_STATE_ERROR;
+s->state = MIG_STATE_ERROR;
+}
+if (s->state == MIG_STATE_ACTIVE) {
+s->state = MIG_STATE_COMPLETED;
 }
-s->state = state;
 notifier_list_notify(&migration_state_notifiers, NULL);
 }
 }
-- 
1.7.6.2




[Qemu-devel] [PATCH 2/7] migration: only flush when there are no errors

2011-09-20 Thread Juan Quintela
If we have one error while migrating, and then we issuse a
"migrate_cancel" command, guest hang.  Fix it for flushing only when
migration is in MIG_STATE_ACTIVE.  In case of error of cancellation,
don't flush.

We had an infinite loop at buffered_close()

while (!s->has_error && s->buffer_size) {
buffered_flush(s);
if (s->freeze_output)
s->wait_for_unfreeze(s);
}

There was no errors, there were things to send, and connection was
broken.  send() returns -EAGAIN, so we freezed output, but we
unfreeze_output and try again.

Signed-off-by: Juan Quintela 
---
 buffered_file.c  |   17 ++---
 buffered_file.h  |2 +-
 hw/hw.h  |4 ++--
 migration-exec.c |6 +++---
 migration-fd.c   |4 ++--
 migration-tcp.c  |4 ++--
 migration-unix.c |4 ++--
 migration.c  |6 +++---
 migration.h  |4 ++--
 savevm.c |   20 +++-
 10 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 486af57..5ba3d19 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -166,20 +166,23 @@ static int buffered_put_buffer(void *opaque, const 
uint8_t *buf, int64_t pos, in
 return offset;
 }

-static int buffered_close(void *opaque)
+static int buffered_close(void *opaque, bool flush)
 {
 QEMUFileBuffered *s = opaque;
 int ret;

 DPRINTF("closing\n");

-while (!s->has_error && s->buffer_size) {
-buffered_flush(s);
-if (s->freeze_output)
-s->wait_for_unfreeze(s);
+if (flush) {
+while (!s->has_error && s->buffer_size) {
+buffered_flush(s);
+if (s->freeze_output) {
+s->wait_for_unfreeze(s);
+}
+}
 }

-ret = s->close(s->opaque);
+ret = s->close(s->opaque, flush);

 qemu_del_timer(s->timer);
 qemu_free_timer(s->timer);
@@ -233,7 +236,7 @@ static void buffered_rate_tick(void *opaque)
 QEMUFileBuffered *s = opaque;

 if (s->has_error) {
-buffered_close(s);
+buffered_close(s, false);
 return;
 }

diff --git a/buffered_file.h b/buffered_file.h
index 98d358b..16162ec 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -19,7 +19,7 @@
 typedef ssize_t (BufferedPutFunc)(void *opaque, const void *data, size_t size);
 typedef void (BufferedPutReadyFunc)(void *opaque);
 typedef void (BufferedWaitForUnfreezeFunc)(void *opaque);
-typedef int (BufferedCloseFunc)(void *opaque);
+typedef int (BufferedCloseFunc)(void *opaque, bool flush);

 QEMUFile *qemu_fopen_ops_buffered(void *opaque, size_t xfer_limit,
   BufferedPutFunc *put_buffer,
diff --git a/hw/hw.h b/hw/hw.h
index a124da9..129de0e 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -28,7 +28,7 @@ typedef int (QEMUFileGetBufferFunc)(void *opaque, uint8_t 
*buf,
 int64_t pos, int size);

 /* Close a file and return an error code */
-typedef int (QEMUFileCloseFunc)(void *opaque);
+typedef int (QEMUFileCloseFunc)(void *opaque, bool flush);

 /* Called to determine if the file has exceeded it's bandwidth allocation.  The
  * bandwidth capping is a soft limit, not a hard limit.
@@ -55,7 +55,7 @@ QEMUFile *qemu_popen(FILE *popen_file, const char *mode);
 QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
 int qemu_stdio_fd(QEMUFile *f);
 void qemu_fflush(QEMUFile *f);
-int qemu_fclose(QEMUFile *f);
+int qemu_fclose(QEMUFile *f, bool flush);
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
 void qemu_put_byte(QEMUFile *f, int v);

diff --git a/migration-exec.c b/migration-exec.c
index 2cfb6f2..5fe09e3 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -42,12 +42,12 @@ static int file_write(FdMigrationState *s, const void * 
buf, size_t size)
 return write(s->fd, buf, size);
 }

-static int exec_close(FdMigrationState *s)
+static int exec_close(FdMigrationState *s, bool flush)
 {
 int ret = 0;
 DPRINTF("exec_close\n");
 if (s->opaque) {
-ret = qemu_fclose(s->opaque);
+ret = qemu_fclose(s->opaque, flush);
 s->opaque = NULL;
 s->fd = -1;
 if (ret != -1 &&
@@ -123,7 +123,7 @@ static void exec_accept_incoming_migration(void *opaque)

 process_incoming_migration(f);
 qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL);
-qemu_fclose(f);
+qemu_fclose(f, false);
 }

 int exec_start_incoming_migration(const char *command)
diff --git a/migration-fd.c b/migration-fd.c
index aee690a..3908850 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -40,7 +40,7 @@ static int fd_write(FdMigrationState *s, const void * buf, 
size_t size)
 return write(s->fd, buf, size);
 }

-static int fd_close(FdMigrationState *s)
+static int fd_close(FdMigrationState *s, bool flush)
 {
 DPRINTF("fd_close\n");
 if (s->fd != -1) {
@@ -106,7 +106,7 @@ static void fd_accept_incoming_migration(void *opaque)

 process_inco

Re: [Qemu-devel] Memory API code review

2011-09-20 Thread Anthony Liguori

On 09/14/2011 10:07 AM, Avi Kivity wrote:

I would like to carry out an online code review of the memory API so that more
people are familiar with the internals, and perhaps even to catch some bugs or
deficiency. I'd like to use the next kvm conference call slot for this (Tuesday
1400 UTC) since many people already have it reserved in the schedule.

It would be great if people from the wider qemu community be present, rather
than the usual "x86 is everything" crowd (+Jan) that usually participates in the
kvm weekly call.

Juan, Chris, can we dedicate next week's call to this?

We'll also need a way to disseminate a few slides and an editor session for
showing the code. We have an elluminate account that can be used for this, but
usually this has a 50% failure rate on Linux. Anthony, perhaps we can set up a
view-only vnc reflector on qemu.org?



The reflector is at:

qemu.osuosl.org:0

Regards,

Anthony Liguori



[Qemu-devel] [PATCH 7/7] migration: qemu_savevm_iterate has three return values

2011-09-20 Thread Juan Quintela
We were retrying when there was one error, entering a loop.

Signed-off-by: Juan Quintela 
---
 migration.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/migration.c b/migration.c
index 7f8928a..0baed23 100644
--- a/migration.c
+++ b/migration.c
@@ -362,6 +362,7 @@ void migrate_fd_connect(FdMigrationState *s)
 void migrate_fd_put_ready(void *opaque)
 {
 FdMigrationState *s = opaque;
+int ret;

 if (s->state != MIG_STATE_ACTIVE) {
 DPRINTF("put_ready returning because of non-active state\n");
@@ -369,7 +370,10 @@ void migrate_fd_put_ready(void *opaque)
 }

 DPRINTF("iterate\n");
-if (qemu_savevm_state_iterate(s->mon, s->file) == 1) {
+ret = qemu_savevm_state_iterate(s->mon, s->file);
+if (ret == -1) {
+migrate_fd_error(s);
+} else if (ret == 1) {
 int old_vm_running = vm_running;

 DPRINTF("done iterating\n");
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH 04/14] qdev: take ownership of id pointer

2011-09-20 Thread Anthony Liguori

On 09/20/2011 08:21 AM, Gerd Hoffmann wrote:

On 09/20/11 15:04, Anthony Liguori wrote:

On 09/20/2011 01:36 AM, Gerd Hoffmann wrote:

On 09/19/11 18:27, Anthony Liguori wrote:

On 09/19/2011 02:34 AM, Gerd Hoffmann wrote:

FYI: Keeping the pointer to the QemuOpts has one more reason: It will
free the
QemuOpts on hot-unplug, which is needed to free the id from QemuOpts
point of
view, which in turn allows to re-use the id when hot-plugging the same
(or
another) device later on.


You mean, tie QemuOpts life cycle to devices life cycle


Yes.


such that you
cannot accidentally create a non-device QemuOpts that conflicts with the
id of a device?


Device QemuOpts have their own id namespace, so this is just about
conflicts
within devices. This ...

device_add e1000,id=nic1
device_del nic1
device_add e1000,id=nic1

... will work only if you free the QemuOpts when deleting a device,
otherwise
QemuOpts will complain that nic1 is used already.


But we can just verify that the id specified for qdev is unique at
creation time and fail creation if it isn't, no?

Since not all devices are assigned names via qemuopts, that seems like a
safer approach anyway.


I think that happens anyway (didn't check the source though).

Problem is that QemuOpts wants IDs being unique too, so keep the QemuOpts
hanging around instead of releasing them makes QemuOpts complain about nic1 not
being unique although there isn't such a device in qdev space.


I don't think we have a firm requirement that the QemuOpts namespace == device 
namespace.  At any rate, it's not enforced today because devices can be created 
(with an id) outside of device_add.



Oh, and not
releasing the QemuOpts would also leak memory on each hot-unplug.


If you look at my patch, opts is freed at the end of device_add so there is no 
leak.

Regards,

Anthony Liguori



cheers,
Gerd








[Qemu-devel] [PATCH 6/7] migration: If there is one error, it makes no sense to continue

2011-09-20 Thread Juan Quintela

Signed-off-by: Juan Quintela 
---
 buffered_file.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 5ba3d19..10d14f9 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -197,7 +197,7 @@ static int buffered_rate_limit(void *opaque)
 QEMUFileBuffered *s = opaque;

 if (s->has_error)
-return 0;
+return -1;

 if (s->freeze_output)
 return 1;
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH 2/3] wavcapture: Use stdio instead of QEMUFile

2011-09-20 Thread malc
On Tue, 20 Sep 2011, Juan Quintela wrote:

> QEMUFile * is only intended for migration nowadays.  Using it for
> anything else just adds pain and a layer of buffers for no good
> reason.

I've commited two wav patches with some stylistic and changes.

[..snip..]

-- 
mailto:av1...@comtv.ru



Re: [Qemu-devel] [PATCH] ahci: add port I/O index-data pair

2011-09-20 Thread Alexander Graf

On 09/20/2011 03:39 PM, Kevin Wolf wrote:

Am 28.08.2011 20:48, schrieb Alexander Graf:

On 27.08.2011, at 04:12, Daniel Verkamp wrote:


Implement an I/O space index-data register pair as defined by the AHCI
spec, including the corresponding SATA PCI capability and BAR.

This allows real-mode code to access the AHCI registers; real-mode
code cannot address the memory-mapped register space because it is
beyond the first megabyte.

Very nice patch! I'll check and compare with a real ICH-9 when I get back to 
.de, but I'd assume you also did that already ;). Once I checked that the IO 
region is set up similarly, I'll give you my ack.

What's the status with review/testing of this patch, Alex? Is it ready
to be merged, should I drop it or do you just need some more time?


If you don't see regressions with it, I'd say we're good. I still don't 
have a validation program, but considering that it's a reasonably simple 
change and makes us more conforming to real hardware, I don't see why we 
shouldn't have it.



Alex




[Qemu-devel] [PATCH 4/7] savevm: avoid qemu_savevm_state_iterate() to return 1 when qemu file has error.

2011-09-20 Thread Juan Quintela
From: Yoshiaki Tamura 

When qemu on the receiver gets killed during live migration, if debug
is turned on, migrate_fd_put_ready() says,

migration: done iterating

and proceeds.  The reason was qemu_savevm_state_iterate() returning 1
even when qemu file has error.  This patch checks
qemu_file_has_error() before returning 1/0, and avoids
migrate_fd_put_ready() to proceed in case of error.

Signed-off-by: Yoshiaki Tamura 
Signed-off-by: Juan Quintela 
---
 savevm.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/savevm.c b/savevm.c
index a793137..af0d0e7 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1531,14 +1531,15 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
 }
 }

-if (ret)
-return 1;
-
 if (qemu_file_has_error(f)) {
 qemu_savevm_state_cancel(mon, f);
 return -EIO;
 }

+if (ret) {
+return 1;
+}
+
 return 0;
 }

-- 
1.7.6.2




[Qemu-devel] [PATCH 5/7] migration: add error handling to migrate_fd_put_notify().

2011-09-20 Thread Juan Quintela
From: Yoshiaki Tamura 

Although migrate_fd_put_buffer() sets MIG_STATE_ERROR if it failed,
since migrate_fd_put_notify() isn't checking error of underlying
QEMUFile, those resources are kept open.  This patch checks it and
calls migrate_fd_error() in case of error.

Signed-off-by: Yoshiaki Tamura 
Signed-off-by: Juan Quintela 
---
 migration.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration.c b/migration.c
index c56d29c..7f8928a 100644
--- a/migration.c
+++ b/migration.c
@@ -312,6 +312,9 @@ void migrate_fd_put_notify(void *opaque)

 qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
 qemu_file_put_notify(s->file);
+if (qemu_file_has_error(s->file)) {
+migrate_fd_error(s);
+}
 }

 ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
@@ -328,9 +331,6 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void 
*data, size_t size)

 if (ret == -EAGAIN) {
 qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
-} else if (ret < 0) {
-s->state = MIG_STATE_ERROR;
-notifier_list_notify(&migration_state_notifiers, NULL);
 }

 return ret;
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH 04/14] qdev: take ownership of id pointer

2011-09-20 Thread Gerd Hoffmann

  Hi,


Oh, and not
releasing the QemuOpts would also leak memory on each hot-unplug.


If you look at my patch, opts is freed at the end of device_add so there
is no leak.


Ah, ok.  Missed that.

cheers,
  Gerd




[Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code

2011-09-20 Thread Juan Quintela
Hi

this patch applies on top of my previous "migration error"
patches.  All error handling has been moved to that series,
except for "propagate error correctly", without this
refactoring, it is quite complicated to apply it.

Please, review.

Later, Juan.

v3:
- more checkpatch.pl happines
- split error handling in a previous series
- make Anthony happy.  current_migration is still a pointer, but points to
  a static variable.  We can change current_migration when we integrate
  kemari.

v2:
- make Jan^Wcheckpatch.pl happy
- Yoshiaki Tamura suggestions:
  - include its two patches to clean things
  - MAX_THROTTLE define
  - migration_state enum
- I removed spurious differences between migration-{tcp,unix}
- better error propagation, after this patch:
   migrate -d "tcp:name_don_exist:port"
   migrate -d "tcp:name:port_dont_exist"
   migrate -d "exec: prog_dont_exist"
   migrate -d "exec: gzip > /path/dont/exist"
 fail as expected.  Last two used to enter an infinite loop.

The fixes part should be backported to 0.14, waiting for the review to do that.

Later, Juan.

v1:
This series:
- Fold MigrationState into FdMigrationState (and then rename)
- Factorize migration statec creation in a single place
- Make use of MIG_STATE_*, setup through helpers and make them local
- remove relase & cancel callbacks (where used only one in same
  file than defined)
- get_status() is no more, just access directly to .state
- current_migration use cleanup, and make variable static
- max_throotle is gone, now inside current_migration
- change get_migration_status() to migration_has_finished()
  and actualize single user.

Please review.

Later, Juan.


*** BLURB HERE ***

Juan Quintela (23):
  migration: Make *start_outgoing_migration return FdMigrationState
  migration: Use FdMigrationState instead of MigrationState when
possible
  migration: Fold MigrationState into FdMigrationState
  migration: Rename FdMigrationState MigrationState
  migration: Refactor MigrationState creation
  migration: Make all posible migration functions static
  migration: move migrate_create_state to do_migrate
  migration: Introduce MIG_STATE_NONE
  migration: Refactor and simplify error checking in
migrate_fd_put_ready
  migration: Introduce migrate_fd_completed() for consistency
  migration: Our release callback was just free
  migration: Remove get_status() accessor
  migration: Remove migration cancel() callback
  migration: Move exported functions to the end of the file
  migration: use global variable directly
  migration: another case of global variable assigned to local one
  migration: make sure we always have a migration state
  migration: Use bandwidth_limit directly
  migration: Export a function that tells if the migration has finished
correctly
  migration: Make state definitions local
  migration: Don't use callback on file defining it
  migration: propagate error correctly
  migration: make migration-{tcp,unix} consistent

 migration-exec.c |   39 +
 migration-fd.c   |   42 ++-
 migration-tcp.c  |   76 --
 migration-unix.c |  112 ++-
 migration.c  |  400 ++
 migration.h  |   85 ++--
 ui/spice-core.c  |4 +-
 7 files changed, 300 insertions(+), 458 deletions(-)

-- 
1.7.6.2




[Qemu-devel] [PATCH 06/23] migration: Make all posible migration functions static

2011-09-20 Thread Juan Quintela
I have to move two functions postions to avoid forward declarations

Signed-off-by: Juan Quintela 
---
 migration.c |   72 +-
 migration.h |   12 -
 2 files changed, 36 insertions(+), 48 deletions(-)

diff --git a/migration.c b/migration.c
index b62a92a..96f00b7 100644
--- a/migration.c
+++ b/migration.c
@@ -273,15 +273,7 @@ static void migrate_fd_monitor_suspend(MigrationState *s, 
Monitor *mon)
 }
 }

-void migrate_fd_error(MigrationState *s)
-{
-DPRINTF("setting error state\n");
-s->state = MIG_STATE_ERROR;
-notifier_list_notify(&migration_state_notifiers, NULL);
-migrate_fd_cleanup(s);
-}
-
-int migrate_fd_cleanup(MigrationState *s)
+static int migrate_fd_cleanup(MigrationState *s)
 {
 int ret = 0;

@@ -307,7 +299,15 @@ int migrate_fd_cleanup(MigrationState *s)
 return ret;
 }

-void migrate_fd_put_notify(void *opaque)
+void migrate_fd_error(MigrationState *s)
+{
+DPRINTF("setting error state\n");
+s->state = MIG_STATE_ERROR;
+notifier_list_notify(&migration_state_notifiers, NULL);
+migrate_fd_cleanup(s);
+}
+
+static void migrate_fd_put_notify(void *opaque)
 {
 MigrationState *s = opaque;

@@ -318,7 +318,8 @@ void migrate_fd_put_notify(void *opaque)
 }
 }

-ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
+static ssize_t migrate_fd_put_buffer(void *opaque, const void *data,
+ size_t size)
 {
 MigrationState *s = opaque;
 ssize_t ret;
@@ -337,29 +338,7 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void 
*data, size_t size)
 return ret;
 }

-void migrate_fd_connect(MigrationState *s)
-{
-int ret;
-
-s->file = qemu_fopen_ops_buffered(s,
-  s->bandwidth_limit,
-  migrate_fd_put_buffer,
-  migrate_fd_put_ready,
-  migrate_fd_wait_for_unfreeze,
-  migrate_fd_close);
-
-DPRINTF("beginning savevm\n");
-ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
-if (ret < 0) {
-DPRINTF("failed, %d\n", ret);
-migrate_fd_error(s);
-return;
-}
-
-migrate_fd_put_ready(s);
-}
-
-void migrate_fd_put_ready(void *opaque)
+static void migrate_fd_put_ready(void *opaque)
 {
 MigrationState *s = opaque;
 int ret;
@@ -430,7 +409,7 @@ static void migrate_fd_release(MigrationState *s)
 g_free(s);
 }

-void migrate_fd_wait_for_unfreeze(void *opaque)
+static void migrate_fd_wait_for_unfreeze(void *opaque)
 {
 MigrationState *s = opaque;
 int ret;
@@ -449,7 +428,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
 } while (ret == -1 && (s->get_error(s)) == EINTR);
 }

-int migrate_fd_close(void *opaque, bool flush)
+static int migrate_fd_close(void *opaque, bool flush)
 {
 MigrationState *s = opaque;

@@ -479,6 +458,27 @@ int get_migration_state(void)
 }
 }

+void migrate_fd_connect(MigrationState *s)
+{
+int ret;
+
+s->file = qemu_fopen_ops_buffered(s,
+  s->bandwidth_limit,
+  migrate_fd_put_buffer,
+  migrate_fd_put_ready,
+  migrate_fd_wait_for_unfreeze,
+  migrate_fd_close);
+
+DPRINTF("beginning savevm\n");
+ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
+if (ret < 0) {
+DPRINTF("failed, %d\n", ret);
+migrate_fd_error(s);
+return;
+}
+migrate_fd_put_ready(s);
+}
+
 MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
  int detach, int blk, int inc)
 {
diff --git a/migration.h b/migration.h
index bbb2d0b..f5de3da 100644
--- a/migration.h
+++ b/migration.h
@@ -100,20 +100,8 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,

 void migrate_fd_error(MigrationState *s);

-int migrate_fd_cleanup(MigrationState *s);
-
-void migrate_fd_put_notify(void *opaque);
-
-ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);
-
 void migrate_fd_connect(MigrationState *s);

-void migrate_fd_put_ready(void *opaque);
-
-void migrate_fd_wait_for_unfreeze(void *opaque);
-
-int migrate_fd_close(void *opaque, bool flush);
-
 MigrationState *migrate_create_state(Monitor *mon, int64_t bandwidth_limit,
  int detach, int blk, int inc);

-- 
1.7.6.2




[Qemu-devel] [PATCH 15/23] migration: use global variable directly

2011-09-20 Thread Juan Quintela
We are setting a pointer to a local variable in the previous line, just use
the global variable directly.  We remove the ->file test because it is already
done inside qemu_file_set_rate_limit() function.

Signed-off-by: Juan Quintela 
---
 migration.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/migration.c b/migration.c
index 642fee1..1ab26fb 100644
--- a/migration.c
+++ b/migration.c
@@ -459,7 +459,6 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 int64_t d;
-MigrationState *s;

 d = qdict_get_int(qdict, "value");
 if (d < 0) {
@@ -467,9 +466,8 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 }
 max_throttle = d;

-s = current_migration;
-if (s && s->file) {
-qemu_file_set_rate_limit(s->file, max_throttle);
+if (current_migration) {
+qemu_file_set_rate_limit(current_migration->file, max_throttle);
 }

 return 0;
-- 
1.7.6.2




[Qemu-devel] [PATCH 18/23] migration: Use bandwidth_limit directly

2011-09-20 Thread Juan Quintela
Now that current_migration is static, there is no reason for max_throotle
variable.

Signed-off-by: Juan Quintela 
---
 migration.c |   15 +++
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/migration.c b/migration.c
index 57cb1f8..b8632e5 100644
--- a/migration.c
+++ b/migration.c
@@ -31,14 +31,14 @@
 do { } while (0)
 #endif

-/* Migration speed throttling */
-static int64_t max_throttle = (32 << 20);
+#define MAX_THROTTLE  (32 << 20)  /* Migration speed throttling */

 /* When we add fault tolerance, we could have several
migrations at once.  For now we don't need to add
dynamic creation of migration */
 static MigrationState current_migration_static = {
 .state = MIG_STATE_NONE,
+.bandwidth_limit = MAX_THROTTLE,
 };
 static MigrationState *current_migration = ¤t_migration_static;

@@ -375,14 +375,12 @@ void migrate_fd_connect(MigrationState *s)
 migrate_fd_put_ready(s);
 }

-static void migrate_init_state(Monitor *mon, int64_t bandwidth_limit,
-   int detach, int blk, int inc)
+static void migrate_init_state(Monitor *mon, int detach, int blk, int inc)
 {
 memset(current_migration, 0, sizeof(current_migration));
 current_migration->blk = blk;
 current_migration->shared = inc;
 current_migration->mon = NULL;
-current_migration->bandwidth_limit = bandwidth_limit;
 current_migration->state = MIG_STATE_NONE;

 if (!detach) {
@@ -408,7 +406,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 return -1;
 }

-migrate_init_state(mon, max_throttle, detach, blk, inc);
+migrate_init_state(mon, detach, blk, inc);

 if (strstart(uri, "tcp:", &p)) {
 ret = tcp_start_outgoing_migration(current_migration, p);
@@ -448,9 +446,10 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 if (d < 0) {
 d = 0;
 }
-max_throttle = d;
+current_migration->bandwidth_limit = d;

-qemu_file_set_rate_limit(current_migration->file, max_throttle);
+qemu_file_set_rate_limit(current_migration->file,
+ current_migration->bandwidth_limit);
 return 0;
 }

-- 
1.7.6.2




[Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state

2011-09-20 Thread Juan Quintela
This cleans up a lot the code as we don't have to check anymore if
the variable is NULL or not.

We don't make it static, because when we integrate fault tolerance, we
can have several migrations at once.

Signed-off-by: Juan Quintela 
---
 migration.c |  126 +-
 1 files changed, 54 insertions(+), 72 deletions(-)

diff --git a/migration.c b/migration.c
index f405d3c..57cb1f8 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,13 @@
 /* Migration speed throttling */
 static int64_t max_throttle = (32 << 20);

-static MigrationState *current_migration;
+/* When we add fault tolerance, we could have several
+   migrations at once.  For now we don't need to add
+   dynamic creation of migration */
+static MigrationState current_migration_static = {
+.state = MIG_STATE_NONE,
+};
+static MigrationState *current_migration = ¤t_migration_static;

 static NotifierList migration_state_notifiers =
 NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -135,37 +141,34 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
 {
 QDict *qdict;

-if (current_migration) {
-
-switch (current_migration->state) {
-case MIG_STATE_NONE:
-/* no migration has happened ever */
-break;
-case MIG_STATE_ACTIVE:
-qdict = qdict_new();
-qdict_put(qdict, "status", qstring_from_str("active"));
-
-migrate_put_status(qdict, "ram", ram_bytes_transferred(),
-   ram_bytes_remaining(), ram_bytes_total());
-
-if (blk_mig_active()) {
-migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
-   blk_mig_bytes_remaining(),
-   blk_mig_bytes_total());
-}
-
-*ret_data = QOBJECT(qdict);
-break;
-case MIG_STATE_COMPLETED:
-*ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
-break;
-case MIG_STATE_ERROR:
-*ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
-break;
-case MIG_STATE_CANCELLED:
-*ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
-break;
+switch (current_migration->state) {
+case MIG_STATE_NONE:
+/* no migration has happened ever */
+break;
+case MIG_STATE_ACTIVE:
+qdict = qdict_new();
+qdict_put(qdict, "status", qstring_from_str("active"));
+
+migrate_put_status(qdict, "ram", ram_bytes_transferred(),
+   ram_bytes_remaining(), ram_bytes_total());
+
+if (blk_mig_active()) {
+migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
+   blk_mig_bytes_remaining(),
+   blk_mig_bytes_total());
 }
+
+*ret_data = QOBJECT(qdict);
+break;
+case MIG_STATE_COMPLETED:
+*ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
+break;
+case MIG_STATE_ERROR:
+*ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
+break;
+case MIG_STATE_CANCELLED:
+*ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
+break;
 }
 }

@@ -347,11 +350,7 @@ void remove_migration_state_change_notifier(Notifier 
*notify)

 int get_migration_state(void)
 {
-if (current_migration) {
-return current_migration->state;
-} else {
-return MIG_STATE_ERROR;
-}
+return current_migration->state;
 }

 void migrate_fd_connect(MigrationState *s)
@@ -376,28 +375,23 @@ void migrate_fd_connect(MigrationState *s)
 migrate_fd_put_ready(s);
 }

-static MigrationState *migrate_create_state(Monitor *mon,
-int64_t bandwidth_limit,
-int detach, int blk, int inc)
+static void migrate_init_state(Monitor *mon, int64_t bandwidth_limit,
+   int detach, int blk, int inc)
 {
-MigrationState *s = g_malloc0(sizeof(*s));
-
-s->blk = blk;
-s->shared = inc;
-s->mon = NULL;
-s->bandwidth_limit = bandwidth_limit;
-s->state = MIG_STATE_NONE;
+memset(current_migration, 0, sizeof(current_migration));
+current_migration->blk = blk;
+current_migration->shared = inc;
+current_migration->mon = NULL;
+current_migration->bandwidth_limit = bandwidth_limit;
+current_migration->state = MIG_STATE_NONE;

 if (!detach) {
-migrate_fd_monitor_suspend(s, mon);
+migrate_fd_monitor_suspend(current_migration, mon);
 }
-
-return s;
 }

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-MigrationState *s = NULL;
 const char *p;
 int detach = qdict_get_try_bool(qdict, "detach", 0);
 int blk = qdict_get_try_bool(qdict, "blk", 0);
@@ -405,8 +399,7 @@ int do_migrate(Monitor *mon, const

[Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly

2011-09-20 Thread Juan Quintela
This will allows us to hide the status values.

Signed-off-by: Juan Quintela 
---
 migration.c |4 ++--
 migration.h |2 +-
 ui/spice-core.c |4 +---
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/migration.c b/migration.c
index b8632e5..fb95e14 100644
--- a/migration.c
+++ b/migration.c
@@ -348,9 +348,9 @@ void remove_migration_state_change_notifier(Notifier 
*notify)
 notifier_list_remove(&migration_state_notifiers, notify);
 }

-int get_migration_state(void)
+bool migration_has_finished(void)
 {
-return current_migration->state;
+return current_migration->state == MIG_STATE_COMPLETED;
 }

 void migrate_fd_connect(MigrationState *s)
diff --git a/migration.h b/migration.h
index 6949e19..6548977 100644
--- a/migration.h
+++ b/migration.h
@@ -84,7 +84,7 @@ void migrate_fd_connect(MigrationState *s);

 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
-int get_migration_state(void);
+bool migration_has_finished(void);

 uint64_t ram_bytes_remaining(void);
 uint64_t ram_bytes_transferred(void);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 3cbc721..1202993 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -447,9 +447,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data)

 static void migration_state_notifier(Notifier *notifier, void *data)
 {
-int state = get_migration_state();
-
-if (state == MIG_STATE_COMPLETED) {
+if (migration_has_finished()) {
 #if SPICE_SERVER_VERSION >= 0x000701 /* 0.7.1 */
 spice_server_migrate_switch(spice_server);
 #endif
-- 
1.7.6.2




Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

2011-09-20 Thread Mathieu Desnoyers
* Stefan Hajnoczi (stefa...@gmail.com) wrote:
> On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> > Signed-off-by: Lluís Vilanova 
> > ---
> >  trace-events |4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/trace-events b/trace-events
> > index 9d1fbbb..b653d70 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
> >  # hw/milkymist-softusb.c
> >  milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x 
> > value %08x"
> >  milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x 
> > value %08x"
> > -milkymist_softusb_mevt(uint8_t m) "m %d"
> > -milkymist_softusb_kevt(uint8_t m) "m %d"
> > +milkymist_softusb_mevt(uint8_t _m) "m %d"
> > +milkymist_softusb_kevt(uint8_t _m) "m %d"
> 
> The LTTng community has been very responsive in addressing namespace
> issues with libust.  Let's post more details and see if it can be fixed
> in libust.
> 
> Could you please post your gcc and libust versions?
> 
> I have not been able to reproduce the problem on Debian libust-dev
> 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
> 
> Here is the test program:
> 
> #include 
> #include 
> 
> DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
> TP_ARGS(m));
> 
> int main(int argc, char **argv)
> {
>   return 0;
> }
> 
> I see no warning or error related to the 'm' argument name.

(sorry for re-send, I just fixed my SMTP setup)

I remember fixing that for trace_mark() instrumentation in UST 0.x ages
ago (ok, probably last winter). I think simply updating to a new UST
version will fix the issue.

Best regards,

Mathieu

> 
> Stefan
> 
> ___
> ltt-dev mailing list
> ltt-...@lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com



Re: [Qemu-devel] [PATCH 2/7] migration: only flush when there are no errors

2011-09-20 Thread Daniel P. Berrange
On Tue, Sep 20, 2011 at 03:24:41PM +0200, Juan Quintela wrote:
> If we have one error while migrating, and then we issuse a
> "migrate_cancel" command, guest hang.  Fix it for flushing only when
> migration is in MIG_STATE_ACTIVE.  In case of error of cancellation,
> don't flush.
> 
> We had an infinite loop at buffered_close()
> 
> while (!s->has_error && s->buffer_size) {
> buffered_flush(s);
> if (s->freeze_output)
> s->wait_for_unfreeze(s);
> }
> 
> There was no errors, there were things to send, and connection was
> broken.  send() returns -EAGAIN, so we freezed output, but we
> unfreeze_output and try again.

I posted a couple of alternative approaches to fixing this
hang problem

http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg03248.html

My second approach of checking the migration state in migrate_fd_put_buffer()
seems like it would be worthwhile, even with your patch as an additional
safety net against bad code.

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 :|



[Qemu-devel] qxl: pthread_yield on QXL_IO_NOTIFY_OOM

2011-09-20 Thread Jan Kiszka
Hi Gerd,

can you (or anyone familiar with those bits) comment on pthread_yield()
in ioport_write() of hw/qxl.c? Which threads are supposed to run this
way? Can't this relation be expressed explicitly? If not, can we use a
sleep here (how long?)?

The background is that yielding easily breaks into pieces when leaving
the SCHED_OTHER space (what we are working on).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

2011-09-20 Thread Mathieu Desnoyers
* Lluís Vilanova (vilan...@ac.upc.edu) wrote:
> Stefan Hajnoczi writes:
> 
> > On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
> >> Signed-off-by: Lluís Vilanova 
> >> ---
> >> trace-events |4 ++--
> >> 1 files changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/trace-events b/trace-events
> >> index 9d1fbbb..b653d70 100644
> >> --- a/trace-events
> >> +++ b/trace-events
> >> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
> >> # hw/milkymist-softusb.c
> >> milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x 
> >> value %08x"
> >> milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x 
> >> value %08x"
> >> -milkymist_softusb_mevt(uint8_t m) "m %d"
> >> -milkymist_softusb_kevt(uint8_t m) "m %d"
> >> +milkymist_softusb_mevt(uint8_t _m) "m %d"
> >> +milkymist_softusb_kevt(uint8_t _m) "m %d"
> 
> > The LTTng community has been very responsive in addressing namespace
> > issues with libust.  Let's post more details and see if it can be fixed
> > in libust.
> 
> > Could you please post your gcc and libust versions?
> 
> > I have not been able to reproduce the problem on Debian libust-dev
> > 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
> 
> Yup, I was using libust-dev 0.5 (debian stable). After switching to 0.15 
> (debian
> testing) other problems arise.
> 
> For example, in my box compiling osdep.c yields lots of "variable 
> ‘__tp_cb_data’
> set but not used", but using a minimal example does not raise such problems, 
> so
> I'm assuming this is related to some namespace problems related to some other
> code in qemu's headers.

If you can find a few minutes to provide:

1) a link to the git repository/commit ID you are testing
2) the exact configuration you use (detailed way to reproduce the
   problem, as a sequence of commands from a pristine repository)

It will allow me to look into this warning and fix it.

Thanks!

Mathieu

> 
> Unfortunately, right now I have no time to debug the causes, so I'll just 
> assume
> the two patches I sent (this naming issue and the zero-length printf format) 
> are
> just no longer necessary with recent versions of libust.
> 
> 
> Thanks,
> Lluis
> 
> -- 
>  "And it's much the same thing with knowledge, for whenever you learn
>  something new, the whole world becomes that much richer."
>  -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
>  Tollbooth
> 
> ___
> ltt-dev mailing list
> ltt-...@lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com



[Qemu-devel] [PATCH 05/23] migration: Refactor MigrationState creation

2011-09-20 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration-exec.c |   16 +---
 migration-fd.c   |   16 +---
 migration-tcp.c  |   15 +--
 migration-unix.c |   15 +--
 migration.c  |   29 +
 migration.h  |   11 +++
 6 files changed, 32 insertions(+), 70 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index b23f83d..6bddb7b 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -71,7 +71,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
 MigrationState *s;
 FILE *f;

-s = g_malloc0(sizeof(*s));
+s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);

 f = popen(command, "w");
 if (f == NULL) {
@@ -92,20 +92,6 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
 s->close = exec_close;
 s->get_error = file_errno;
 s->write = file_write;
-s->cancel = migrate_fd_cancel;
-s->get_status = migrate_fd_get_status;
-s->release = migrate_fd_release;
-
-s->blk = blk;
-s->shared = inc;
-
-s->state = MIG_STATE_ACTIVE;
-s->mon = NULL;
-s->bandwidth_limit = bandwidth_limit;
-
-if (!detach) {
-migrate_fd_monitor_suspend(s, mon);
-}

 migrate_fd_connect(s);
 return s;
diff --git a/migration-fd.c b/migration-fd.c
index 95a9aa3..040a372 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -59,7 +59,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
 {
 MigrationState *s;

-s = g_malloc0(sizeof(*s));
+s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);

 s->fd = monitor_get_fd(mon, fdname);
 if (s->fd == -1) {
@@ -75,20 +75,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
 s->get_error = fd_errno;
 s->write = fd_write;
 s->close = fd_close;
-s->cancel = migrate_fd_cancel;
-s->get_status = migrate_fd_get_status;
-s->release = migrate_fd_release;
-
-s->blk = blk;
-s->shared = inc;
-
-s->state = MIG_STATE_ACTIVE;
-s->mon = NULL;
-s->bandwidth_limit = bandwidth_limit;
-
-if (!detach) {
-migrate_fd_monitor_suspend(s, mon);
-}

 migrate_fd_connect(s);
 return s;
diff --git a/migration-tcp.c b/migration-tcp.c
index 7c1db92..f93b037 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -89,21 +89,12 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
 if (parse_host_port(&addr, host_port) < 0)
 return NULL;

-s = g_malloc0(sizeof(*s));
+s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);

 s->get_error = socket_errno;
 s->write = socket_write;
 s->close = tcp_close;
-s->cancel = migrate_fd_cancel;
-s->get_status = migrate_fd_get_status;
-s->release = migrate_fd_release;

-s->blk = blk;
-s->shared = inc;
-
-s->state = MIG_STATE_ACTIVE;
-s->mon = NULL;
-s->bandwidth_limit = bandwidth_limit;
 s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
 if (s->fd == -1) {
 g_free(s);
@@ -112,10 +103,6 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,

 socket_set_nonblock(s->fd);

-if (!detach) {
-migrate_fd_monitor_suspend(s, mon);
-}
-
 do {
 ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
 if (ret == -1)
diff --git a/migration-unix.c b/migration-unix.c
index 3148c28..63be155 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -88,21 +88,12 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
 addr.sun_family = AF_UNIX;
 snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);

-s = g_malloc0(sizeof(*s));
+s = migrate_create_state(mon, bandwidth_limit, detach, blk, inc);

 s->get_error = unix_errno;
 s->write = unix_write;
 s->close = unix_close;
-s->cancel = migrate_fd_cancel;
-s->get_status = migrate_fd_get_status;
-s->release = migrate_fd_release;

-s->blk = blk;
-s->shared = inc;
-
-s->state = MIG_STATE_ACTIVE;
-s->mon = NULL;
-s->bandwidth_limit = bandwidth_limit;
 s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
 if (s->fd < 0) {
 DPRINTF("Unable to open socket");
@@ -125,10 +116,6 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
 goto err_after_open;
 }

-if (!detach) {
-migrate_fd_monitor_suspend(s, mon);
-}
-
 if (ret >= 0)
 migrate_fd_connect(s);

diff --git a/migration.c b/migration.c
index d87ad14..b62a92a 100644
--- a/migration.c
+++ b/migration.c
@@ -262,7 +262,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)

 /* shared migration helpers */

-void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
+static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
 {
 s->mon = mon;
 if (monitor_suspend(mon) == 0) {
@@ -398,12 +398,12 @@ void migrate_fd_put_ready(void *opaque)
 }
 }

-int migrate_fd_get_status(MigrationState *s)
+static int migrate_fd

[Qemu-devel] [PATCH 08/23] migration: Introduce MIG_STATE_NONE

2011-09-20 Thread Juan Quintela
Use MIG_STATE_ACTIVE only when migration has really started

Signed-off-by: Juan Quintela 
---
 migration.c |6 +-
 migration.h |   11 +++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/migration.c b/migration.c
index 1d7a488..59e8f06 100644
--- a/migration.c
+++ b/migration.c
@@ -239,6 +239,9 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
 MigrationState *s = current_migration;

 switch (s->get_status(current_migration)) {
+case MIG_STATE_NONE:
+/* no migration has happened ever */
+break;
 case MIG_STATE_ACTIVE:
 qdict = qdict_new();
 qdict_put(qdict, "status", qstring_from_str("active"));
@@ -469,6 +472,7 @@ void migrate_fd_connect(MigrationState *s)
 {
 int ret;

+s->state = MIG_STATE_ACTIVE;
 s->file = qemu_fopen_ops_buffered(s,
   s->bandwidth_limit,
   migrate_fd_put_buffer,
@@ -499,7 +503,7 @@ static MigrationState *migrate_create_state(Monitor *mon,
 s->shared = inc;
 s->mon = NULL;
 s->bandwidth_limit = bandwidth_limit;
-s->state = MIG_STATE_ACTIVE;
+s->state = MIG_STATE_NONE;

 if (!detach) {
 migrate_fd_monitor_suspend(s, mon);
diff --git a/migration.h b/migration.h
index a08b5fa..b9e8a76 100644
--- a/migration.h
+++ b/migration.h
@@ -18,10 +18,13 @@
 #include "qemu-common.h"
 #include "notify.h"

-#define MIG_STATE_ERROR-1
-#define MIG_STATE_COMPLETED0
-#define MIG_STATE_CANCELLED1
-#define MIG_STATE_ACTIVE   2
+enum migration_state {
+MIG_STATE_ERROR,
+MIG_STATE_NONE,
+MIG_STATE_CANCELLED,
+MIG_STATE_ACTIVE,
+MIG_STATE_COMPLETED,
+};

 typedef struct MigrationState MigrationState;

-- 
1.7.6.2




Re: [Qemu-devel] [ltt-dev] [PATCH 2/3] trace: [ust] Do not use 'm' in event argument names (used by ust macros)

2011-09-20 Thread Stefan Hajnoczi
2011/9/20 Mathieu Desnoyers :
> * Lluís Vilanova (vilan...@ac.upc.edu) wrote:
>> Stefan Hajnoczi writes:
>>
>> > On Fri, Sep 16, 2011 at 06:59:38PM +0200, Lluís Vilanova wrote:
>> >> Signed-off-by: Lluís Vilanova 
>> >> ---
>> >> trace-events |    4 ++--
>> >> 1 files changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/trace-events b/trace-events
>> >> index 9d1fbbb..b653d70 100644
>> >> --- a/trace-events
>> >> +++ b/trace-events
>> >> @@ -418,8 +418,8 @@ milkymist_pfpu_pulse_irq(void) "Pulse IRQ"
>> >> # hw/milkymist-softusb.c
>> >> milkymist_softusb_memory_read(uint32_t addr, uint32_t value) "addr %08x 
>> >> value %08x"
>> >> milkymist_softusb_memory_write(uint32_t addr, uint32_t value) "addr %08x 
>> >> value %08x"
>> >> -milkymist_softusb_mevt(uint8_t m) "m %d"
>> >> -milkymist_softusb_kevt(uint8_t m) "m %d"
>> >> +milkymist_softusb_mevt(uint8_t _m) "m %d"
>> >> +milkymist_softusb_kevt(uint8_t _m) "m %d"
>>
>> > The LTTng community has been very responsive in addressing namespace
>> > issues with libust.  Let's post more details and see if it can be fixed
>> > in libust.
>>
>> > Could you please post your gcc and libust versions?
>>
>> > I have not been able to reproduce the problem on Debian libust-dev
>> > 0.15-3.  My gcc version is Debian gcc 4.6.1-4.
>>
>> Yup, I was using libust-dev 0.5 (debian stable). After switching to 0.15 
>> (debian
>> testing) other problems arise.
>>
>> For example, in my box compiling osdep.c yields lots of "variable 
>> ‘__tp_cb_data’
>> set but not used", but using a minimal example does not raise such problems, 
>> so
>> I'm assuming this is related to some namespace problems related to some other
>> code in qemu's headers.
>
> If you can find a few minutes to provide:
>
> 1) a link to the git repository/commit ID you are testing
> 2) the exact configuration you use (detailed way to reproduce the
>   problem, as a sequence of commands from a pristine repository)
>
> It will allow me to look into this warning and fix it.

AFAICT the only problem with libust is the __tp_cb_data set but not
used warning that gcc 4.6 emits, see my test program:

#include 
#include 

DECLARE_TRACE(ust_milkymist_softusb_mevt, TP_PROTO(uint8_t m),
TP_ARGS(m));

int main(int argc, char **argv)
{
   return 0;
}

You will get an error with gcc 4.6 and libust 0.15:

$ gcc -o a -Wall a.c

QEMU itself does have one problem with libust 0.15.  We do
DECLARE_TRACE(name, TP_PROTO(void), TP_ARGS()).  This results in a
compiler error but we can fix this my changing our tracepoints to use
DECLARE_TRACE(name, TP_PROTO(), TP_ARGS()) I think.  Perhaps libust's
behavior here has changed, it shouldn't be hard to update QEMU's
tracetool tracepoint generator though.

If you adapt the example program I used above to use TP_PROTO(void),
TP_ARGS() then you get this error:

a.c: In function ‘__trace_ust_milkymist_softusb_mevt’:
a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
a.c:4:1: error: expected ‘)’ before ‘__tp_it_func’
a.c:4:1: error: expected expression before ‘)’ token
a.c:4:1: warning: variable ‘__tp_it_func’ set but not used
[-Wunused-but-set-variable]
a.c: At top level:
a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
a.c:4:1: error: expected ‘;’, ‘,’ or ‘)’ before ‘void’
a.c:4:1: error: expected declaration specifiers or ‘...’ before ‘)’ token
a.c:4:1: error: expected ‘;’, ‘,’ or ‘)’ before ‘void’
a.c: In function ‘trace_ust_milkymist_softusb_mevt’:
a.c:4:1: warning: variable ‘__tp_cb_data’ set but not used
[-Wunused-but-set-variable]

I can't say for sure whether I ever tested old libust versions with
void tracepoints.  Perhaps we simply never had any and the tracetool
generator has always been broken.  Or perhaps libust changed its
behavior.

Stefan



[Qemu-devel] [PATCH 11/23] migration: Our release callback was just free

2011-09-20 Thread Juan Quintela
We called it from a single place, and always with state !=
MIG_STATE_ACTIVE.  Just remove the whole callback.  For users of the
notifier, notice that this is exactly the case where they don't care,
we are just freeing the state from previous failed migration (it can't
be a sucessful one, otherwise we would not be running on that machine
in the first place).

Signed-off-by: Juan Quintela 
---
 migration.c |   19 +--
 migration.h |1 -
 2 files changed, 1 insertions(+), 19 deletions(-)

diff --git a/migration.c b/migration.c
index 15a363d..6c553a6 100644
--- a/migration.c
+++ b/migration.c
@@ -123,10 +123,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 goto free_migrate_state;
 }

-if (current_migration) {
-current_migration->release(current_migration);
-}
-
+g_free(current_migration);
 current_migration = s;
 notifier_list_notify(&migration_state_notifiers, NULL);
 return 0;
@@ -411,19 +408,6 @@ static void migrate_fd_cancel(MigrationState *s)
 migrate_fd_cleanup(s);
 }

-static void migrate_fd_release(MigrationState *s)
-{
-
-DPRINTF("releasing state\n");
-   
-if (s->state == MIG_STATE_ACTIVE) {
-s->state = MIG_STATE_CANCELLED;
-notifier_list_notify(&migration_state_notifiers, NULL);
-migrate_fd_cleanup(s);
-}
-g_free(s);
-}
-
 static void migrate_fd_wait_for_unfreeze(void *opaque)
 {
 MigrationState *s = opaque;
@@ -503,7 +487,6 @@ static MigrationState *migrate_create_state(Monitor *mon,

 s->cancel = migrate_fd_cancel;
 s->get_status = migrate_fd_get_status;
-s->release = migrate_fd_release;
 s->blk = blk;
 s->shared = inc;
 s->mon = NULL;
diff --git a/migration.h b/migration.h
index b9e8a76..92e3b46 100644
--- a/migration.h
+++ b/migration.h
@@ -40,7 +40,6 @@ struct MigrationState
 int (*write)(MigrationState *s, const void *buff, size_t size);
 void (*cancel)(MigrationState *s);
 int (*get_status)(MigrationState *s);
-void (*release)(MigrationState *s);
 void *opaque;
 int blk;
 int shared;
-- 
1.7.6.2




Re: [Qemu-devel] [PATCH 2/7] migration: only flush when there are no errors

2011-09-20 Thread Juan Quintela
"Daniel P. Berrange"  wrote:
> On Tue, Sep 20, 2011 at 03:24:41PM +0200, Juan Quintela wrote:
>> If we have one error while migrating, and then we issuse a
>> "migrate_cancel" command, guest hang.  Fix it for flushing only when
>> migration is in MIG_STATE_ACTIVE.  In case of error of cancellation,
>> don't flush.
>> 
>> We had an infinite loop at buffered_close()
>> 
>> while (!s->has_error && s->buffer_size) {
>> buffered_flush(s);
>> if (s->freeze_output)
>> s->wait_for_unfreeze(s);
>> }
>> 
>> There was no errors, there were things to send, and connection was
>> broken.  send() returns -EAGAIN, so we freezed output, but we
>> unfreeze_output and try again.
>
> I posted a couple of alternative approaches to fixing this
> hang problem
>
> http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg03248.html
>
> My second approach of checking the migration state in migrate_fd_put_buffer()
> seems like it would be worthwhile, even with your patch as an additional
> safety net against bad code.

We can add that there, but in my tests, the s->write() was returning
correctly an error (or -EAGAIN).  The problem was that we were not
exiting when we didn't needed to.

I agree that we can have *both* tests.  I will add your patch to my
series.

Thanks for the fast review.

Later, Juan.



Re: [Qemu-devel] qxl: pthread_yield on QXL_IO_NOTIFY_OOM

2011-09-20 Thread Gerd Hoffmann

On 09/20/11 16:27, Jan Kiszka wrote:

Hi Gerd,

can you (or anyone familiar with those bits) comment on pthread_yield()
in ioport_write() of hw/qxl.c?  Which threads are supposed to run this
way?


spice server thread.


Can't this relation be expressed explicitly?


The thread is created by libspice-server, so we don't have a handle for it.


If not, can we use a
sleep here (how long?)?


Good question.  I'm tempted to just rip it out and run qxl_spice_oom() 
unconditionally.  The yield thing is best effort only anyway, there is 
no guarantee that the spice server thread actually gets scheduled and 
puts some stuff into the release ring.


cheers,
  Gerd




[Qemu-devel] [PATCH 09/23] migration: Refactor and simplify error checking in migrate_fd_put_ready

2011-09-20 Thread Juan Quintela

Signed-off-by: Juan Quintela 
---
 migration.c |   19 +--
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/migration.c b/migration.c
index 59e8f06..9d877a0 100644
--- a/migration.c
+++ b/migration.c
@@ -368,22 +368,21 @@ static void migrate_fd_put_ready(void *opaque)
 DPRINTF("done iterating\n");
 vm_stop(VMSTOP_MIGRATE);

-if ((qemu_savevm_state_complete(s->mon, s->file)) < 0) {
-if (old_vm_running) {
-vm_start();
+if (qemu_savevm_state_complete(s->mon, s->file) < 0) {
+migrate_fd_error(s);
+} else {
+if (migrate_fd_cleanup(s) < 0) {
+migrate_fd_error(s);
+} else {
+s->state = MIG_STATE_COMPLETED;
+notifier_list_notify(&migration_state_notifiers, NULL);
 }
-s->state = MIG_STATE_ERROR;
 }
-if (migrate_fd_cleanup(s) < 0) {
+if (s->get_status(s) != MIG_STATE_COMPLETED) {
 if (old_vm_running) {
 vm_start();
 }
-s->state = MIG_STATE_ERROR;
 }
-if (s->state == MIG_STATE_ACTIVE) {
-s->state = MIG_STATE_COMPLETED;
-}
-notifier_list_notify(&migration_state_notifiers, NULL);
 }
 }

-- 
1.7.6.2




[Qemu-devel] [PATCH 12/23] migration: Remove get_status() accessor

2011-09-20 Thread Juan Quintela
It is only used inside migration.c, and fields on that struct are
accessed all around the place on that file.

Signed-off-by: Juan Quintela 
---
 migration.c |   16 +---
 migration.h |1 -
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/migration.c b/migration.c
index 6c553a6..d63150e 100644
--- a/migration.c
+++ b/migration.c
@@ -91,7 +91,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 int ret;

 if (current_migration &&
-current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
+current_migration->state == MIG_STATE_ACTIVE) {
 monitor_printf(mon, "migration already in progress\n");
 return -1;
 }
@@ -136,7 +136,7 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 {
 MigrationState *s = current_migration;

-if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
+if (s && s->state == MIG_STATE_ACTIVE) {
 s->cancel(s);
 }
 return 0;
@@ -235,7 +235,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
 if (current_migration) {
 MigrationState *s = current_migration;

-switch (s->get_status(current_migration)) {
+switch (s->state) {
 case MIG_STATE_NONE:
 /* no migration has happened ever */
 break;
@@ -381,7 +381,7 @@ static void migrate_fd_put_ready(void *opaque)
 } else {
 migrate_fd_completed(s);
 }
-if (s->get_status(s) != MIG_STATE_COMPLETED) {
+if (s->state != MIG_STATE_COMPLETED) {
 if (old_vm_running) {
 vm_start();
 }
@@ -389,11 +389,6 @@ static void migrate_fd_put_ready(void *opaque)
 }
 }

-static int migrate_fd_get_status(MigrationState *s)
-{
-return s->state;
-}
-
 static void migrate_fd_cancel(MigrationState *s)
 {
 if (s->state != MIG_STATE_ACTIVE)
@@ -451,7 +446,7 @@ void remove_migration_state_change_notifier(Notifier 
*notify)
 int get_migration_state(void)
 {
 if (current_migration) {
-return migrate_fd_get_status(current_migration);
+return current_migration->state;
 } else {
 return MIG_STATE_ERROR;
 }
@@ -486,7 +481,6 @@ static MigrationState *migrate_create_state(Monitor *mon,
 MigrationState *s = g_malloc0(sizeof(*s));

 s->cancel = migrate_fd_cancel;
-s->get_status = migrate_fd_get_status;
 s->blk = blk;
 s->shared = inc;
 s->mon = NULL;
diff --git a/migration.h b/migration.h
index 92e3b46..6336afe 100644
--- a/migration.h
+++ b/migration.h
@@ -39,7 +39,6 @@ struct MigrationState
 int (*close)(MigrationState *s, bool flush);
 int (*write)(MigrationState *s, const void *buff, size_t size);
 void (*cancel)(MigrationState *s);
-int (*get_status)(MigrationState *s);
 void *opaque;
 int blk;
 int shared;
-- 
1.7.6.2




Re: [Qemu-devel] qxl: pthread_yield on QXL_IO_NOTIFY_OOM

2011-09-20 Thread Jan Kiszka
On 2011-09-20 16:48, Gerd Hoffmann wrote:
> On 09/20/11 16:27, Jan Kiszka wrote:
>> Hi Gerd,
>>
>> can you (or anyone familiar with those bits) comment on pthread_yield()
>> in ioport_write() of hw/qxl.c?  Which threads are supposed to run this
>> way?
> 
> spice server thread.
> 
>> Can't this relation be expressed explicitly?
> 
> The thread is created by libspice-server, so we don't have a handle for it.

And also no communication channel to kick?

> 
>> If not, can we use a
>> sleep here (how long?)?
> 
> Good question.  I'm tempted to just rip it out and run qxl_spice_oom() 
> unconditionally.  The yield thing is best effort only anyway, there is 
> no guarantee that the spice server thread actually gets scheduled and 
> puts some stuff into the release ring.

And the issue with sleeping is that we stall the vcpu. So just reporting
oom is likely better.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH 02/23] migration: Use FdMigrationState instead of MigrationState when possible

2011-09-20 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration.c |   34 --
 migration.h |   16 
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/migration.c b/migration.c
index 1a5e25f..007b162 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,7 @@
 /* Migration speed throttling */
 static int64_t max_throttle = (32 << 20);

-static MigrationState *current_migration;
+static FdMigrationState *current_migration;

 static NotifierList migration_state_notifiers =
 NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -86,7 +86,8 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 const char *uri = qdict_get_str(qdict, "uri");

 if (current_migration &&
-current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
+current_migration->mig_state.get_status(current_migration) ==
+MIG_STATE_ACTIVE) {
 monitor_printf(mon, "migration already in progress\n");
 return -1;
 }
@@ -120,20 +121,20 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 }

 if (current_migration) {
-current_migration->release(current_migration);
+current_migration->mig_state.release(current_migration);
 }

-current_migration = &s->mig_state;
+current_migration = s;
 notifier_list_notify(&migration_state_notifiers, NULL);
 return 0;
 }

 int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-MigrationState *s = current_migration;
+FdMigrationState *s = current_migration;

-if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
-s->cancel(s);
+if (s && s->mig_state.get_status(s) == MIG_STATE_ACTIVE) {
+s->mig_state.cancel(s);
 }
 return 0;
 }
@@ -149,7 +150,7 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 }
 max_throttle = d;

-s = migrate_to_fms(current_migration);
+s = current_migration;
 if (s && s->file) {
 qemu_file_set_rate_limit(s->file, max_throttle);
 }
@@ -227,10 +228,11 @@ static void migrate_put_status(QDict *qdict, const char 
*name,
 void do_info_migrate(Monitor *mon, QObject **ret_data)
 {
 QDict *qdict;
-MigrationState *s = current_migration;

-if (s) {
-switch (s->get_status(s)) {
+if (current_migration) {
+MigrationState *s = ¤t_migration->mig_state;
+
+switch (s->get_status(current_migration)) {
 case MIG_STATE_ACTIVE:
 qdict = qdict_new();
 qdict_put(qdict, "status", qstring_from_str("active"));
@@ -398,16 +400,13 @@ void migrate_fd_put_ready(void *opaque)
 }
 }

-int migrate_fd_get_status(MigrationState *mig_state)
+int migrate_fd_get_status(FdMigrationState *s)
 {
-FdMigrationState *s = migrate_to_fms(mig_state);
 return s->state;
 }

-void migrate_fd_cancel(MigrationState *mig_state)
+void migrate_fd_cancel(FdMigrationState *s)
 {
-FdMigrationState *s = migrate_to_fms(mig_state);
-
 if (s->state != MIG_STATE_ACTIVE)
 return;

@@ -420,9 +419,8 @@ void migrate_fd_cancel(MigrationState *mig_state)
 migrate_fd_cleanup(s);
 }

-void migrate_fd_release(MigrationState *mig_state)
+void migrate_fd_release(FdMigrationState *s)
 {
-FdMigrationState *s = migrate_to_fms(mig_state);

 DPRINTF("releasing state\n");

diff --git a/migration.h b/migration.h
index 0591e82..bbcdefb 100644
--- a/migration.h
+++ b/migration.h
@@ -25,18 +25,18 @@

 typedef struct MigrationState MigrationState;

+typedef struct FdMigrationState FdMigrationState;
+
 struct MigrationState
 {
 /* FIXME: add more accessors to print migration info */
-void (*cancel)(MigrationState *s);
-int (*get_status)(MigrationState *s);
-void (*release)(MigrationState *s);
+void (*cancel)(FdMigrationState *s);
+int (*get_status)(FdMigrationState *s);
+void (*release)(FdMigrationState *s);
 int blk;
 int shared;
 };

-typedef struct FdMigrationState FdMigrationState;
-
 struct FdMigrationState
 {
 MigrationState mig_state;
@@ -120,11 +120,11 @@ void migrate_fd_connect(FdMigrationState *s);

 void migrate_fd_put_ready(void *opaque);

-int migrate_fd_get_status(MigrationState *mig_state);
+int migrate_fd_get_status(FdMigrationState *mig_state);

-void migrate_fd_cancel(MigrationState *mig_state);
+void migrate_fd_cancel(FdMigrationState *mig_state);

-void migrate_fd_release(MigrationState *mig_state);
+void migrate_fd_release(FdMigrationState *mig_state);

 void migrate_fd_wait_for_unfreeze(void *opaque);

-- 
1.7.6.2




[Qemu-devel] [PATCH 13/23] migration: Remove migration cancel() callback

2011-09-20 Thread Juan Quintela
It is used only in one place

Signed-off-by: Juan Quintela 
---
 migration.c |9 -
 migration.h |1 -
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/migration.c b/migration.c
index d63150e..306ac4e 100644
--- a/migration.c
+++ b/migration.c
@@ -132,12 +132,12 @@ free_migrate_state:
 return -1;
 }

+static void migrate_fd_cancel(MigrationState *s);
+
 int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-MigrationState *s = current_migration;
-
-if (s && s->state == MIG_STATE_ACTIVE) {
-s->cancel(s);
+if (current_migration) {
+migrate_fd_cancel(current_migration);
 }
 return 0;
 }
@@ -480,7 +480,6 @@ static MigrationState *migrate_create_state(Monitor *mon,
 {
 MigrationState *s = g_malloc0(sizeof(*s));

-s->cancel = migrate_fd_cancel;
 s->blk = blk;
 s->shared = inc;
 s->mon = NULL;
diff --git a/migration.h b/migration.h
index 6336afe..6949e19 100644
--- a/migration.h
+++ b/migration.h
@@ -38,7 +38,6 @@ struct MigrationState
 int (*get_error)(MigrationState *s);
 int (*close)(MigrationState *s, bool flush);
 int (*write)(MigrationState *s, const void *buff, size_t size);
-void (*cancel)(MigrationState *s);
 void *opaque;
 int blk;
 int shared;
-- 
1.7.6.2




  1   2   >