[Qemu-devel] Re: Qemu-KVM Livate Migration 0.12.2 -> 0.12.3/4 broken?

[Avi Kivity]

On 05/16/2010 11:04 PM, Juan Quintela wrote:



So, to make the story short: I know what is happening, and I know how to
fix it, just that fix is not trivial.  I just need time.

   

Meanwhile, we have a broken 0.12.4.  Is there a quick'n'dirty
workaround that will be forward compatible with the real fix that we
can push out?
 

revert the patch.  It almost never happen (being in the middle of one
IO) while migrating.
   


If the guest does any work, it will always happen.

   

We've regressed from failing some migrations to failing all migrations.
 

Humm, 0.12.4 ->  0.12.4 should work.  My advise is just revert the patch
and live with it for another week, what do you think?
   


A week is fine.  I'm not going to release 0.12.4.1 for the week until 
you fix it, and just changing things in git doesn't help users.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Jan Kiszka]

Avi Kivity wrote:
> On 05/17/2010 03:12 AM, Anthony Liguori wrote:
>> On Sun, May 16, 2010 at 12:38 PM, Jamie Lokier 
>> wrote:
>>   
>>> Anthony Liguori wrote:
>>> 
 Instead of encoding just as a string, it would be a good idea to encode
 it as something like:

 {'__class__': 'base64', 'data': ...}

>>> Is there a benefit to the class indirection, over simply a keyword?:
>>>
>>> {'__base64__': ...}
>>>
>>> __class__ seems to suggest much more than it's being used for here.
>>>  
>> Yes.  The problem with JSON is that it's based on JavaScript and
>> JavaScript is goofy :-)
>>
>>
> 
> I suggest completely ignoring JavaScript.  JSON is simply an encoding
> for numbers, strings, arrays, and key-value stores.  Where's the goofiness?
> 
>> JavaScript's object mechanism doesn't map well to most other languages
>> since it's prototype based.  What we're calling QDict's are really
>> objects in JavaScript and they carry mostly no type information.  With
>> JS, it's very simple to treat a generic object as a specialized class
>> after instantiation which means objects don't need type information.
>>
>> For non-prototype languages, which is the vast majority of clients,
>> it's necessary to have type information at instantiation time since
>> monkey patching is awkward at best.  That's why we need a special,
>> reserved, object member to carry type information.  The remainder of
>> the object members represent the serialized state of the object.
>>
> 
> The alternative is to have a schema.  Sun RPC/XDR doesn't carry any type
> information (you can't even distinguish between a number and text) yet C
> clients have to problem extracting typed information from it.
> 
> Having __class__ everywhere means we're carrying the schema in every
> message instead of just once.

The device_show command is already an example where you don't have a
predefined schema. It is derived from the data stream the encodes the
vmstate fields. So far we have no collision between base64-encoded
buffers and real strings, but this may actually change when we start
annotating the fields with symbolic constants.

I really don't see the problem with __class__. Being a text protocol,
JSON is already fairly verbose.

Jan

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

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Avi Kivity]

On 05/17/2010 10:40 AM, Jan Kiszka wrote:



The alternative is to have a schema.  Sun RPC/XDR doesn't carry any type
information (you can't even distinguish between a number and text) yet C
clients have to problem extracting typed information from it.

Having __class__ everywhere means we're carrying the schema in every
message instead of just once.
 

The device_show command is already an example where you don't have a
predefined schema. It is derived from the data stream the encodes the
vmstate fields. So far we have no collision between base64-encoded
buffers and real strings, but this may actually change when we start
annotating the fields with symbolic constants.
   


What is the receiver to do with it?

If it doesn't know the schema (and there is no schema), then all it can 
do is display the key/values.  If it does know the schema, then 
__class__ is unnecessary.


My worry is that __class__ will make the protocol more ad-hoc.


I really don't see the problem with __class__. Being a text protocol,
JSON is already fairly verbose.
   


The problem is not the verbosity, it's that information is carried too 
late.  Many clients want to know this information at compile time or 
initialization time, and we are providing it at object instantiating time.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Jan Kiszka]

Avi Kivity wrote:
> On 05/17/2010 10:40 AM, Jan Kiszka wrote:
>>> The alternative is to have a schema.  Sun RPC/XDR doesn't carry any type
>>> information (you can't even distinguish between a number and text) yet C
>>> clients have to problem extracting typed information from it.
>>>
>>> Having __class__ everywhere means we're carrying the schema in every
>>> message instead of just once.
>>>  
>> The device_show command is already an example where you don't have a
>> predefined schema. It is derived from the data stream the encodes the
>> vmstate fields. So far we have no collision between base64-encoded
>> buffers and real strings, but this may actually change when we start
>> annotating the fields with symbolic constants.
>>
> 
> What is the receiver to do with it?
> 
> If it doesn't know the schema (and there is no schema), then all it can 
> do is display the key/values.  If it does know the schema, then 
> __class__ is unnecessary.

There is a schema describing the fields (name, size, number of
elements), but their types (int, buffer, sub-field, array of X) are
derived from the JSON objects (ie. the JSON parser does this job).

> 
> My worry is that __class__ will make the protocol more ad-hoc.
> 
>> I really don't see the problem with __class__. Being a text protocol,
>> JSON is already fairly verbose.
>>
> 
> The problem is not the verbosity, it's that information is carried too 
> late.  Many clients want to know this information at compile time or 
> initialization time, and we are providing it at object instantiating time.

What clients do you have in mind?

Jan

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

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Avi Kivity]

On 05/17/2010 10:57 AM, Jan Kiszka wrote:

Avi Kivity wrote:
   

On 05/17/2010 10:40 AM, Jan Kiszka wrote:
 

The alternative is to have a schema.  Sun RPC/XDR doesn't carry any type
information (you can't even distinguish between a number and text) yet C
clients have to problem extracting typed information from it.

Having __class__ everywhere means we're carrying the schema in every
message instead of just once.

 

The device_show command is already an example where you don't have a
predefined schema. It is derived from the data stream the encodes the
vmstate fields. So far we have no collision between base64-encoded
buffers and real strings, but this may actually change when we start
annotating the fields with symbolic constants.

   

What is the receiver to do with it?

If it doesn't know the schema (and there is no schema), then all it can
do is display the key/values.  If it does know the schema, then
__class__ is unnecessary.
 

There is a schema describing the fields (name, size, number of
elements),


Surely the schema has to describe the type as well?  If it does, you can 
use the schema to generate a classes at compile time.



  but their types (int, buffer, sub-field, array of X) are
derived from the JSON objects (ie. the JSON parser does this job).
   


The names of fields are also type information.



I really don't see the problem with __class__. Being a text protocol,
JSON is already fairly verbose.

   

The problem is not the verbosity, it's that information is carried too
late.  Many clients want to know this information at compile time or
initialization time, and we are providing it at object instantiating time.
 

What clients do you have in mind?

   


Any client that doesn't allow object types to be created dynamically; C, 
C++, Java, and the like could all benefit from a schema and wouldn't be 
able to do much with __class__ unless all classes were predefined.  
Python, JavaScript, and the like wouldn't care.


Another way of looking at it: if the client sees { __class__: foo, f1: 
10, f2: 9 }, it cannot derive any information from __class__ unless it 
was aware of foo beforehand.  If that's the case, let's make it part of 
the schema so it is available at compile time instead of runtime.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Avi Kivity]

On 05/17/2010 11:10 AM, Avi Kivity wrote:


Another way of looking at it: if the client sees { __class__: foo, f1: 
10, f2: 9 }, it cannot derive any information from __class__ unless it 
was aware of foo beforehand.  If that's the case, let's make it part 
of the schema so it is available at compile time instead of runtime.




Or both.  That's similar to a non-object field's type being described 
both in the schema at during runtime.  A static parser can verify 
__class__ matches the expectations, a dynamic parser can look up 
__class__ and call the appropriate constructor.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

[Qemu-devel] Re: Qemu-KVM Livate Migration 0.12.2 -> 0.12.3/4 broken?

[Michael Tokarev]

17.05.2010 11:00, Avi Kivity wrote:

On 05/16/2010 11:04 PM, Juan Quintela wrote:

[]

We've regressed from failing some migrations to failing all migrations.

Humm, 0.12.4 -> 0.12.4 should work. My advise is just revert the patch
and live with it for another week, what do you think?


A week is fine. I'm not going to release 0.12.4.1 for the week until you
fix it, and just changing things in git doesn't help users.


There are at least 1 other problem in the migration code that's
worth looking at (IMHO anyway): namely, migration on 32 bit host
is completely (as far as i can see) broken (instant kvm process
crash) in 0.12, but it works on 64bits:

http://www.mail-archive.com/k...@vger.kernel.org/msg34051.html

I wonder why it is not noticed before -- it's broken since 0.12...

Thanks!

/mjt

Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Markus Armbruster]

Avi Kivity  writes:

> On 05/14/2010 08:03 PM, Markus Armbruster wrote:
>> Avi Kivity  writes:
>>
>>
>>> On 05/14/2010 11:50 AM, Markus Armbruster wrote:
[...]
 We have a list of buses, each containing a list of device functions.
 Not sure the additional level of nesting you propose buys us anything.


>>> A slot is the hotpluggable entity.  Open your computer and you can
>>> actually see them.
>>>  
>> QEMU doesn't really know that.
>>
>
> How can that be?  Do we signal hotplug notifications to a function or
> to a slot?
>
> Can we hotplug a single function in an already occupied slot?

What I meant to say: we have no concept of "slot" in the higher level
interfaces, we have only bus and device.

If a PCI device has multiple functions, we have a separate qdev device
for each function.  You can't unplug a "slot" (concept doesn't exist in
qdev), only a qdev device.  Naturally, when you unplug a qdev device,
all functions in the same PCI slot need to go.  This happens deep down
in the bowels of ACPI, in piix4_device_hotplug().  qdev is not aware of
this magic relation between the qdev devices for functions in the same
slot.

[...]

Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Markus Armbruster]

Avi Kivity  writes:

> On 05/15/2010 01:54 AM, Luiz Capitulino wrote:
>> On Fri, 14 May 2010 19:03:36 +0200
>> Markus Armbruster  wrote:
>>
>>
 What about PCI domains?

>>> Good point.  Better to provide for them neatly now, instead of kludging
>>> them in later.
>>>  
>>   When I did this conversion I asked Micheal for help with that and he said
>> QEMU doesn't support PCI domains.
>>
>
> That's very different from "will never support pci domains".
>
> The protocol must be forward looking, or we will need endless fixes for it.

Precisely.

Re: [Qemu-devel] [PATCH 0/3] mingw32 compile fixes

[Markus Armbruster]

Would you mind sending patch series with proper References: headers, so
that the parts of the series are properly threaded?

You can ask either git-format-patch or git-send-email to add them.

[Qemu-devel] [PATCH] Fix error handling in qemu_read_config_file

[Kevin Wolf]

We need to close the file even in error case. While at it, make the callers
catch all kind of errors. ENOENT is allowed for default config files, they
are optional.

Reported-by: Luiz Capitulino 
Signed-off-by: Kevin Wolf 
---
 qemu-config.c |   12 
 vl.c  |4 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index bf3d493..b2a4128 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -521,14 +521,18 @@ out:
 int qemu_read_config_file(const char *filename)
 {
 FILE *f = fopen(filename, "r");
+int ret;
+
 if (f == NULL) {
 return -errno;
 }
 
-if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
-return -EINVAL;
-}
+ret = qemu_config_parse(f, vm_config_groups, filename);
 fclose(f);
 
-return 0;
+if (ret == 0) {
+return 0;
+} else {
+return -EINVAL;
+}
 }
diff --git a/vl.c b/vl.c
index c8abce6..4ca1bee 100644
--- a/vl.c
+++ b/vl.c
@@ -2683,12 +2683,12 @@ int main(int argc, char **argv, char **envp)
 int ret;
 
 ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
-if (ret == -EINVAL) {
+if (ret < 0 && ret != -ENOENT) {
 exit(1);
 }
 
 ret = qemu_read_config_file(arch_config_name);
-if (ret == -EINVAL) {
+if (ret < 0 && ret != -ENOENT) {
 exit(1);
 }
 }
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH 0/3] mingw32 compile fixes

[Markus Armbruster]

Blue Swirl  writes:

> On 5/16/10, Stefan Weil  wrote:
>> Am 15.05.2010 22:49, schrieb Blue Swirl:
>>
>>
>> > Hi,
>> >
>> > With this mingw32 compiler:
>> >
>> > $ i586-mingw32msvc-gcc -v
>> > Using built-in specs.
>> > Target: i586-mingw32msvc
>> > Configured with:
[...]
>> > build will not succeed because formats %zd, %zu, %hh, %lld, %llx and
>> > %llu are not known by the compiler.
>> >
>> > Any %ll* use is clearly a bug, we have PRI*64 macros just for this
>> purpose.
>> >
>> > For %hh and %z there may be better ways than these patches.
>> >
>> > With the patches I can build working Win32 binaries and there are no
>> warnings.
[...]
>>  It's a compiler bug that the compiler does not know these format strings.
>>  The code works nevertheless (at least with mingw libraries which are
>>  not too old) because the format strings are interpreted by the C runtime
>>  library.
>>
>>  Is it worth changing a lot of files when we can expect a newer mingw
>>  compiler version which works correctly for standard format strings?
>
> When and if that version becomes popular, PRIz* and the %hh hack could
> be removed or a compiler check could be added. But I don't think it's
> worth it, the macros are easy to use.

They're also ugly as sin.

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Jan Kiszka]

Avi Kivity wrote:
> On 05/17/2010 10:57 AM, Jan Kiszka wrote:
>> Avi Kivity wrote:
>>
>>> On 05/17/2010 10:40 AM, Jan Kiszka wrote:
>>>  
> The alternative is to have a schema.  Sun RPC/XDR doesn't carry any type
> information (you can't even distinguish between a number and text) yet C
> clients have to problem extracting typed information from it.
>
> Having __class__ everywhere means we're carrying the schema in every
> message instead of just once.
>
>  
 The device_show command is already an example where you don't have a
 predefined schema. It is derived from the data stream the encodes the
 vmstate fields. So far we have no collision between base64-encoded
 buffers and real strings, but this may actually change when we start
 annotating the fields with symbolic constants.


>>> What is the receiver to do with it?
>>>
>>> If it doesn't know the schema (and there is no schema), then all it can
>>> do is display the key/values.  If it does know the schema, then
>>> __class__ is unnecessary.
>>>  
>> There is a schema describing the fields (name, size, number of
>> elements),
> 
> Surely the schema has to describe the type as well?  If it does, you can 
> use the schema to generate a classes at compile time.
> 
>>   but their types (int, buffer, sub-field, array of X) are
>> derived from the JSON objects (ie. the JSON parser does this job).
>>
> 
> The names of fields are also type information.

Not in the case of device_show. The clients have no idea of the vmstate
structures before they were transfered. Granted, that will likely remain
a special case in the QMP command set.

> 
 I really don't see the problem with __class__. Being a text protocol,
 JSON is already fairly verbose.


>>> The problem is not the verbosity, it's that information is carried too
>>> late.  Many clients want to know this information at compile time or
>>> initialization time, and we are providing it at object instantiating time.
>>>  
>> What clients do you have in mind?
>>
>>
> 
> Any client that doesn't allow object types to be created dynamically; C, 
> C++, Java, and the like could all benefit from a schema and wouldn't be 
> able to do much with __class__ unless all classes were predefined.  
> Python, JavaScript, and the like wouldn't care.
> 
> Another way of looking at it: if the client sees { __class__: foo, f1: 
> 10, f2: 9 }, it cannot derive any information from __class__ unless it 
> was aware of foo beforehand.  If that's the case, let's make it part of 
> the schema so it is available at compile time instead of runtime.
> 

Maybe a misunderstanding on my side: I'm not arguing against predefining
what __class__ values exists and how dicts that carry such keys are encoded.

Jan

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

[Qemu-devel] Re: [PATCH] net: Remove dead code from net/socket.c

[Jan Kiszka]

Miguel Di Ciurcio Filho wrote:
> On Fri, May 14, 2010 at 2:03 PM, Jan Kiszka  wrote:
>>> In both cases the info_str string is written inside
>>> net_socket_fd_init_(stream|dgram), and after that, it is
>>> overwritten on a subsequent snprintf() in net_socket_accept().
>>>
>> There is non-zero time window between registration and acceptance. And
>> you have the path taken for fd sockets.
>>
> 
> It is non-zero, yes, but what is the point of writing to info_str with
> an fd number for less them 5ms, when that value will be overwritten
> anyway?

You are right regarding listen sockets, I forgot that they block, thus
there is no chance to request any information about them before the link
has been established.

But my point about the sockets created by file descriptor remains valid.

> 
> I'm an student, just trying to understand what is going on :-D

Always welcome! People who look from a different angle at this can often
find cruft that others already stopped to realize.

Jan

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

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Avi Kivity]

On 05/17/2010 11:55 AM, Jan Kiszka wrote:



The names of fields are also type information.
 

Not in the case of device_show. The clients have no idea of the vmstate
structures before they were transfered. Granted, that will likely remain
a special case in the QMP command set.
   


For that use case, I agree.  Maybe we should send both the parsed and 
unparsed information.


But if the client isn't going to interpret the object and only display 
it, then there is no need for __class__?


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Avi Kivity]

On 05/17/2010 11:27 AM, Markus Armbruster wrote:


   

A slot is the hotpluggable entity.  Open your computer and you can
actually see them.

 

QEMU doesn't really know that.

   

How can that be?  Do we signal hotplug notifications to a function or
to a slot?

Can we hotplug a single function in an already occupied slot?
 

What I meant to say: we have no concept of "slot" in the higher level
interfaces, we have only bus and device.

If a PCI device has multiple functions, we have a separate qdev device
for each function.  You can't unplug a "slot" (concept doesn't exist in
qdev), only a qdev device.  Naturally, when you unplug a qdev device,
all functions in the same PCI slot need to go.  This happens deep down
in the bowels of ACPI, in piix4_device_hotplug().  qdev is not aware of
this magic relation between the qdev devices for functions in the same
slot.
   


IMO, that's a serious bug.  A slot is a user visible entity, both in 
that devices can only be hotplugged only as slots, not functions, and to 
determine the maximum number of devices you can add.  If the user knows 
about it, qemu should too.


We can easily represent a slot/device as a qbus with each of the 
functions as devices attached to it.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

[Qemu-devel] Re: [PATCH] block: fix aio_flush segfaults for read-only protocols (e.g. curl)

[Kevin Wolf]

Am 16.05.2010 13:59, schrieb Avi Kivity:
> Not all block format drivers expose an io_flush method (reasonable for
> read-only protocols), so calling io_flush there will immediately segfault.
> 
> Fix by checking for the method's existence before calling it.
> 
> Signed-off-by: Avi Kivity 

Thanks, applied to the block branch.

Kevin

[Qemu-devel] Re: Qemu-KVM Livate Migration 0.12.2 -> 0.12.3/4 broken?

[Juan Quintela]

Michael Tokarev  wrote:
> 17.05.2010 11:00, Avi Kivity wrote:
>> On 05/16/2010 11:04 PM, Juan Quintela wrote:
> []
 We've regressed from failing some migrations to failing all migrations.
>>> Humm, 0.12.4 -> 0.12.4 should work. My advise is just revert the patch
>>> and live with it for another week, what do you think?
>>
>> A week is fine. I'm not going to release 0.12.4.1 for the week until you
>> fix it, and just changing things in git doesn't help users.
>
> There are at least 1 other problem in the migration code that's
> worth looking at (IMHO anyway): namely, migration on 32 bit host
> is completely (as far as i can see) broken (instant kvm process
> crash) in 0.12, but it works on 64bits:
>
> http://www.mail-archive.com/k...@vger.kernel.org/msg34051.html
>
> I wonder why it is not noticed before -- it's broken since 0.12...

People has become rich and everybody has a 64 bit hardware :-)

Will take a look at the end of the week.

Thank for the report, Juan.

[Qemu-devel] Re: Qemu-KVM Livate Migration 0.12.2 -> 0.12.3/4 broken?

[Michael Tokarev]

17.05.2010 13:07, Juan Quintela wrote:

Michael Tokarev  wrote:

[]

http://www.mail-archive.com/k...@vger.kernel.org/msg34051.html
I wonder why it is not noticed before -- it's broken since 0.12...


People has become rich and everybody has a 64 bit hardware :-)


Actually I don't think there's a CPU that's 32bits and has
necessary VT instructions.  At least not the ones to care
about - maybe there were some models but they'e not common
and are gone long time ago.

But apparently there are quite some people who use 32bit
OS on 64bit-capable hardware, for one reason or another.
I'm doing it due to historical reasons, but at least I'm
running 64bit kernel... ;)


Will take a look at the end of the week.


Thank you!

/mjt

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Jan Kiszka]

Avi Kivity wrote:
> On 05/17/2010 11:55 AM, Jan Kiszka wrote:
>>> The names of fields are also type information.
>>>  
>> Not in the case of device_show. The clients have no idea of the vmstate
>> structures before they were transfered. Granted, that will likely remain
>> a special case in the QMP command set.
>>
> 
> For that use case, I agree.  Maybe we should send both the parsed and 
> unparsed information.

Now I can't parse what you mean.

> 
> But if the client isn't going to interpret the object and only display 
> it, then there is no need for __class__?

For that uncommon case, yes. But the common one is to perform a bit more
than raw JSON dictionary printing.

Jan

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

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Avi Kivity]

On 05/17/2010 12:17 PM, Jan Kiszka wrote:

Avi Kivity wrote:
   

On 05/17/2010 11:55 AM, Jan Kiszka wrote:
 

The names of fields are also type information.

 

Not in the case of device_show. The clients have no idea of the vmstate
structures before they were transfered. Granted, that will likely remain
a special case in the QMP command set.

   

For that use case, I agree.  Maybe we should send both the parsed and
unparsed information.
 

Now I can't parse what you mean.
   



Sorry.  I meant that if we have a raw buffer that we can decode (like 
pci config space and its fields) maybe it makes sense to send both 
formats.  The raw buffer is something likely to be stable, and the 
decoded fields are more readable.  But I realize now that doesn't make 
sense in the context of base64 encoding which started all this.




But if the client isn't going to interpret the object and only display
it, then there is no need for __class__?
 

For that uncommon case, yes. But the common one is to perform a bit more
than raw JSON dictionary printing.
   


But the common one is likely to know the type of the object beforehand 
(or it can't do much beyond printing, since it has no idea what fields 
to expect).



--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

Re: [Qemu-devel] Re: Qemu-KVM Livate Migration 0.12.2 -> 0.12.3/4 broken?

[Alexander Graf]

On 17.05.2010, at 11:11, Michael Tokarev wrote:

> 17.05.2010 13:07, Juan Quintela wrote:
>> Michael Tokarev  wrote:
> []
>>> http://www.mail-archive.com/k...@vger.kernel.org/msg34051.html
>>> I wonder why it is not noticed before -- it's broken since 0.12...
>> 
>> People has become rich and everybody has a 64 bit hardware :-)
> 
> Actually I don't think there's a CPU that's 32bits and has
> necessary VT instructions.  At least not the ones to care
> about - maybe there were some models but they'e not common
> and are gone long time ago.

Intel Core Solo / Core Duo (old)
Intel Atom z5xx (new)

In fact, my mobile KVM development machine is a z530 based notebook. That one's 
32-bit only. But then again I have no idea why anyone would need migration 
there.


Alex

[Qemu-devel] [RFC PATCH v3 2/3] block: call the snapshot handlers of the protocol drivers

[MORITA Kazutaka]

When snapshot handlers are not defined in the format driver, it is
better to call the ones of the protocol driver.  This enables us to
implement snapshot support in the protocol driver.

We need to call bdrv_close() and bdrv_open() handlers of the format
driver before and after bdrv_snapshot_goto() call of the protocol.  It is
because the contents of the block driver state may need to be changed
after loading vmstate.

Signed-off-by: MORITA Kazutaka 
---
 block.c |   61 +++--
 1 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/block.c b/block.c
index f3bf3f2..c987e57 100644
--- a/block.c
+++ b/block.c
@@ -1683,9 +1683,11 @@ int bdrv_save_vmstate(BlockDriverState *bs, const 
uint8_t *buf,
 BlockDriver *drv = bs->drv;
 if (!drv)
 return -ENOMEDIUM;
-if (!drv->bdrv_save_vmstate)
-return -ENOTSUP;
-return drv->bdrv_save_vmstate(bs, buf, pos, size);
+if (drv->bdrv_save_vmstate)
+return drv->bdrv_save_vmstate(bs, buf, pos, size);
+if (bs->file)
+return bdrv_save_vmstate(bs->file, buf, pos, size);
+return -ENOTSUP;
 }
 
 int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
@@ -1694,9 +1696,11 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
 BlockDriver *drv = bs->drv;
 if (!drv)
 return -ENOMEDIUM;
-if (!drv->bdrv_load_vmstate)
-return -ENOTSUP;
-return drv->bdrv_load_vmstate(bs, buf, pos, size);
+if (drv->bdrv_load_vmstate)
+return drv->bdrv_load_vmstate(bs, buf, pos, size);
+if (bs->file)
+return bdrv_load_vmstate(bs->file, buf, pos, size);
+return -ENOTSUP;
 }
 
 void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event)
@@ -1720,20 +1724,37 @@ int bdrv_snapshot_create(BlockDriverState *bs,
 BlockDriver *drv = bs->drv;
 if (!drv)
 return -ENOMEDIUM;
-if (!drv->bdrv_snapshot_create)
-return -ENOTSUP;
-return drv->bdrv_snapshot_create(bs, sn_info);
+if (drv->bdrv_snapshot_create)
+return drv->bdrv_snapshot_create(bs, sn_info);
+if (bs->file)
+return bdrv_snapshot_create(bs->file, sn_info);
+return -ENOTSUP;
 }
 
 int bdrv_snapshot_goto(BlockDriverState *bs,
const char *snapshot_id)
 {
 BlockDriver *drv = bs->drv;
+int ret, open_ret;
+
 if (!drv)
 return -ENOMEDIUM;
-if (!drv->bdrv_snapshot_goto)
-return -ENOTSUP;
-return drv->bdrv_snapshot_goto(bs, snapshot_id);
+if (drv->bdrv_snapshot_goto)
+return drv->bdrv_snapshot_goto(bs, snapshot_id);
+
+if (bs->file) {
+drv->bdrv_close(bs);
+ret = bdrv_snapshot_goto(bs->file, snapshot_id);
+open_ret = drv->bdrv_open(bs, bs->open_flags);
+if (open_ret < 0) {
+bdrv_delete(bs);
+bs->drv = NULL;
+return open_ret;
+}
+return ret;
+}
+
+return -ENOTSUP;
 }
 
 int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
@@ -1741,9 +1762,11 @@ int bdrv_snapshot_delete(BlockDriverState *bs, const 
char *snapshot_id)
 BlockDriver *drv = bs->drv;
 if (!drv)
 return -ENOMEDIUM;
-if (!drv->bdrv_snapshot_delete)
-return -ENOTSUP;
-return drv->bdrv_snapshot_delete(bs, snapshot_id);
+if (drv->bdrv_snapshot_delete)
+return drv->bdrv_snapshot_delete(bs, snapshot_id);
+if (bs->file)
+return bdrv_snapshot_delete(bs->file, snapshot_id);
+return -ENOTSUP;
 }
 
 int bdrv_snapshot_list(BlockDriverState *bs,
@@ -1752,9 +1775,11 @@ int bdrv_snapshot_list(BlockDriverState *bs,
 BlockDriver *drv = bs->drv;
 if (!drv)
 return -ENOMEDIUM;
-if (!drv->bdrv_snapshot_list)
-return -ENOTSUP;
-return drv->bdrv_snapshot_list(bs, psn_info);
+if (drv->bdrv_snapshot_list)
+return drv->bdrv_snapshot_list(bs, psn_info);
+if (bs->file)
+return bdrv_snapshot_list(bs->file, psn_info);
+return -ENOTSUP;
 }
 
 #define NB_SUFFIXES 4
-- 
1.5.6.5

[Qemu-devel] [RFC PATCH v3 1/3] close all the block drivers before the qemu process exits

[MORITA Kazutaka]

This patch calls the close handler of the block driver before the qemu
process exits.

This is necessary because the sheepdog block driver releases the lock
of VM images in the close handler.

Signed-off-by: MORITA Kazutaka 
---
 block.c |9 +
 block.h |1 +
 vl.c|1 +
 3 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index bfe46e3..f3bf3f2 100644
--- a/block.c
+++ b/block.c
@@ -636,6 +636,15 @@ void bdrv_close(BlockDriverState *bs)
 }
 }
 
+void bdrv_close_all(void)
+{
+BlockDriverState *bs;
+
+QTAILQ_FOREACH(bs, &bdrv_states, list) {
+bdrv_close(bs);
+}
+}
+
 void bdrv_delete(BlockDriverState *bs)
 {
 /* remove from list, if necessary */
diff --git a/block.h b/block.h
index 278259c..531e802 100644
--- a/block.h
+++ b/block.h
@@ -121,6 +121,7 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
 /* Ensure contents are flushed to disk.  */
 void bdrv_flush(BlockDriverState *bs);
 void bdrv_flush_all(void);
+void bdrv_close_all(void);
 
 int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
diff --git a/vl.c b/vl.c
index 85bcc84..5ce7807 100644
--- a/vl.c
+++ b/vl.c
@@ -2007,6 +2007,7 @@ static void main_loop(void)
 exit(0);
 }
 }
+bdrv_close_all();
 pause_all_vcpus();
 }
 
-- 
1.5.6.5

[Qemu-devel] [RFC PATCH v3 0/3] Sheepdog: distributed storage system for QEMU

[MORITA Kazutaka]

Hi all,

This patch adds a block driver for Sheepdog distributed storage
system.

Changes from v2 to v3 are:

 - add drv->bdrv_close() and drv->bdrv_open() before and after
   bdrv_snapshot_goto() call of the protocol.
 - address the review comments on the sheepdog driver code.
   I'll send the details in the reply of the review mail.

Changes from v1 to v2 are:

 - rebase onto git://repo.or.cz/qemu/kevin.git block
 - modify the sheepdog driver as a protocol driver
 - add new patch to call the snapshot handler of the protocol

If this patchset is okay, I'll work on the image creation problem of the
protocol driver.

Thanks,

Kazutaka


MORITA Kazutaka (3):
  close all the block drivers before the qemu process exits
  block: call the snapshot handlers of the protocol drivers
  block: add sheepdog driver for distributed storage support

 Makefile.objs|2 +-
 block.c  |   70 ++-
 block.h  |1 +
 block/sheepdog.c | 1845 ++
 vl.c |1 +
 5 files changed, 1900 insertions(+), 19 deletions(-)
 create mode 100644 block/sheepdog.c

[Qemu-devel] [RFC PATCH v3 3/3] block: add sheepdog driver for distributed storage support

[MORITA Kazutaka]

Sheepdog is a distributed storage system for QEMU. It provides highly
available block level storage volumes to VMs like Amazon EBS.  This
patch adds a qemu block driver for Sheepdog.

Sheepdog features are:
- No node in the cluster is special (no metadata node, no control
  node, etc)
- Linear scalability in performance and capacity
- No single point of failure
- Autonomous management (zero configuration)
- Useful volume management support such as snapshot and cloning
- Thin provisioning
- Autonomous load balancing

The more details are available at the project site:
http://www.osrg.net/sheepdog/

Signed-off-by: MORITA Kazutaka 
---
 Makefile.objs|2 +-
 block/sheepdog.c | 1845 ++
 2 files changed, 1846 insertions(+), 1 deletions(-)
 create mode 100644 block/sheepdog.c

diff --git a/Makefile.objs b/Makefile.objs
index ecdd53e..6edbc57 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
 block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o 
vvfat.o
 block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
-block-nested-y += parallels.o nbd.o blkdebug.o
+block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
 block-nested-$(CONFIG_CURL) += curl.o
diff --git a/block/sheepdog.c b/block/sheepdog.c
new file mode 100644
index 000..4672f00
--- /dev/null
+++ b/block/sheepdog.c
@@ -0,0 +1,1845 @@
+/*
+ * Copyright (C) 2009-2010 Nippon Telegraph and Telephone Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+#include 
+#include 
+
+#include "qemu-common.h"
+#include "qemu-error.h"
+#include "block_int.h"
+
+#define SD_PROTO_VER 0x01
+
+#define SD_DEFAULT_ADDR "localhost:7000"
+
+#define SD_OP_CREATE_AND_WRITE_OBJ  0x01
+#define SD_OP_READ_OBJ   0x02
+#define SD_OP_WRITE_OBJ  0x03
+
+#define SD_OP_NEW_VDI0x11
+#define SD_OP_LOCK_VDI   0x12
+#define SD_OP_RELEASE_VDI0x13
+#define SD_OP_GET_VDI_INFO   0x14
+#define SD_OP_READ_VDIS  0x15
+
+#define SD_FLAG_CMD_WRITE0x01
+#define SD_FLAG_CMD_COW  0x02
+
+#define SD_RES_SUCCESS   0x00 /* Success */
+#define SD_RES_UNKNOWN   0x01 /* Unknown error */
+#define SD_RES_NO_OBJ0x02 /* No object found */
+#define SD_RES_EIO   0x03 /* I/O error */
+#define SD_RES_VDI_EXIST 0x04 /* Vdi exists already */
+#define SD_RES_INVALID_PARMS 0x05 /* Invalid parameters */
+#define SD_RES_SYSTEM_ERROR  0x06 /* System error */
+#define SD_RES_VDI_LOCKED0x07 /* Vdi is locked */
+#define SD_RES_NO_VDI0x08 /* No vdi found */
+#define SD_RES_NO_BASE_VDI   0x09 /* No base vdi found */
+#define SD_RES_VDI_READ  0x0A /* Cannot read requested vdi */
+#define SD_RES_VDI_WRITE 0x0B /* Cannot write requested vdi */
+#define SD_RES_BASE_VDI_READ 0x0C /* Cannot read base vdi */
+#define SD_RES_BASE_VDI_WRITE   0x0D /* Cannot write base vdi */
+#define SD_RES_NO_TAG0x0E /* Requested tag is not found */
+#define SD_RES_STARTUP   0x0F /* Sheepdog is on starting up */
+#define SD_RES_VDI_NOT_LOCKED   0x10 /* Vdi is not locked */
+#define SD_RES_SHUTDOWN  0x11 /* Sheepdog is shutting down */
+#define SD_RES_NO_MEM0x12 /* Cannot allocate memory */
+#define SD_RES_FULL_VDI  0x13 /* we already have the maximum vdis */
+#define SD_RES_VER_MISMATCH  0x14 /* Protocol version mismatch */
+#define SD_RES_NO_SPACE  0x15 /* Server has no room for new objects */
+#define SD_RES_WAIT_FOR_FORMAT  0x16 /* Sheepdog is waiting for a format 
operation */
+#define SD_RES_WAIT_FOR_JOIN0x17 /* Sheepdog is waiting for other nodes 
joining */
+#define SD_RES_JOIN_FAILED   0x18 /* Target node had failed to join sheepdog */
+
+/*
+ * Object ID rules
+ *
+ *  0 - 19 (20 bits): data object space
+ * 20 - 31 (12 bits): reserved data object space
+ * 32 - 55 (24 bits): vdi object space
+ * 56 - 59 ( 4 bits): reserved vdi object space
+ * 60 - 63 ( 4 bits): object type indentifier space
+ */
+
+#define VDI_SPACE_SHIFT   32
+#define VDI_BIT (UINT64_C(1) << 63)
+#define VMSTATE_BIT (UINT64_C(1) << 62)
+#define MAX_DATA_OBJS (1ULL << 20)
+#define MAX_CHILDREN 1024
+#define SD_MAX_VDI_LEN 256
+#define SD_NR_VDIS   (1U << 24)
+#define SD_DATA_OBJ_SIZE (UINT64_C(1) << 22)
+
+#define SD_INODE_SIZE (sizeof(SheepdogInode))
+#define CURRENT_VDI_ID 0
+
+typedef struct SheepdogReq {
+   uint8_t proto_ver;
+   uint8_t opcode;
+   uint16_tflags;
+   uint32_tepoch;
+   uint32_tid;
+   uint32_tdata_length

[Qemu-devel] [PATCH] Add cache=volatile parameter to -drive

[Alexander Graf]

Usually the guest can tell the host to flush data to disk. In some cases we
don't want to flush though, but try to keep everything in cache.

So let's add a new cache value to -drive that allows us to set the cache
policy to most aggressive, disabling flushes. We call this mode "volatile",
as guest data is not guaranteed to survive host crashes anymore.

This patch also adds a noop function for aio, so we can do nothing in AIO
fashion.

Signed-off-by: Alexander Graf 

---

v2 -> v3:

  - Add description of cache=volatile
  - Squash aio noop noop patch into this one
---
 block.c |   28 
 block.h |1 +
 qemu-config.c   |2 +-
 qemu-options.hx |   13 ++---
 vl.c|3 +++
 5 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 48305b7..b742965 100644
--- a/block.c
+++ b/block.c
@@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState 
*bs,
 BlockDriverCompletionFunc *cb, void *opaque);
 static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
 BlockDriverCompletionFunc *cb, void *opaque);
+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque);
 static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
 uint8_t *buf, int nb_sectors);
 static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
@@ -1306,6 +1308,10 @@ const char *bdrv_get_device_name(BlockDriverState *bs)
 
 void bdrv_flush(BlockDriverState *bs)
 {
+if (bs->open_flags & BDRV_O_NO_FLUSH) {
+return;
+}
+
 if (bs->drv && bs->drv->bdrv_flush)
 bs->drv->bdrv_flush(bs);
 }
@@ -2082,6 +2088,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
 {
 BlockDriver *drv = bs->drv;
 
+if (bs->open_flags & BDRV_O_NO_FLUSH) {
+return bdrv_aio_noop_em(bs, cb, opaque);
+}
+
 if (!drv)
 return NULL;
 return drv->bdrv_aio_flush(bs, cb, opaque);
@@ -2196,6 +2206,24 @@ static BlockDriverAIOCB 
*bdrv_aio_flush_em(BlockDriverState *bs,
 return &acb->common;
 }
 
+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BlockDriverAIOCBSync *acb;
+
+acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque);
+acb->is_write = 1; /* don't bounce in the completion hadler */
+acb->qiov = NULL;
+acb->bounce = NULL;
+acb->ret = 0;
+
+if (!acb->bh)
+acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
+
+qemu_bh_schedule(acb->bh);
+return &acb->common;
+}
+
 /**/
 /* sync block device emulation */
 
diff --git a/block.h b/block.h
index f87d24e..8032b6b 100644
--- a/block.h
+++ b/block.h
@@ -33,6 +33,7 @@ typedef struct QEMUSnapshotInfo {
 #define BDRV_O_CACHE_WB0x0040 /* use write-back caching */
 #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool 
*/
 #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
+#define BDRV_O_NO_FLUSH0x0200 /* disable flushing on this disk */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)
 
diff --git a/qemu-config.c b/qemu-config.c
index d500885..bf3d493 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -54,7 +54,7 @@ QemuOptsList qemu_drive_opts = {
 },{
 .name = "cache",
 .type = QEMU_OPT_STRING,
-.help = "host cache usage (none, writeback, writethrough)",
+.help = "host cache usage (none, writeback, writethrough, 
volatile)",
 },{
 .name = "aio",
 .type = QEMU_OPT_STRING,
diff --git a/qemu-options.hx b/qemu-options.hx
index 12f6b51..6dedb4a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,8 +118,9 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
 "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
 "   [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-"   [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
-"   [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
+"   [,cache=writethrough|writeback|volatile|none][,format=f]\n"
+"   [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
+"   [,readonly=on|off]\n"
 "use 'file' as a drive image\n", QEMU_ARCH_ALL)
 STEXI
 @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
@@ -148,7 +149,7 @@ These options have the same definition as they have in 
@option{-hdachs}.
 @item snapsh...@var{snapshot}
 @var{snapshot} is "on" or "off" and allows to enable snapshot for given drive 
(see @option{-snapshot}).
 @item cac...@var{cache}
-...@var{cache} is "none", "writeback", or "writethrough" and controls how the 
host cache is used to access block data.
+...@var{cache} is "none", "writeback", "volatile", or "writethrough" and 
controls how the host

[Qemu-devel] Re: [RFC PATCH v2 3/3] block: add sheepdog driver for distributed storage support

[MORITA Kazutaka]

Hi,

Thank you very much for the reviewing!

At Fri, 14 May 2010 13:08:06 +0200,
Kevin Wolf wrote:

> > +
> > +struct sd_req {
> > +   uint8_t proto_ver;
> > +   uint8_t opcode;
> > +   uint16_tflags;
> > +   uint32_tepoch;
> > +   uint32_tid;
> > +   uint32_tdata_length;
> > +   uint32_topcode_specific[8];
> > +};
> 
> CODING_STYLE says that structs should be typedefed and their names
> should be in CamelCase. So something like this:
> 
> typedef struct SheepdogReq {
> ...
> } SheepdogReq;
> 
> (Or, if your prefer, SDReq; but with things like SDAIOCB I think it
> becomes hard to read)
> 

I see. I use Sheepdog as a prefix, like SheepdogReq.


> > +/*
> > +
> > +#undef eprintf
> > +#define eprintf(fmt, args...)  
> > \
> > +do {   
> > \
> > +   fprintf(stderr, "%s %d: " fmt, __func__, __LINE__, ##args); \
> > +} while (0)
> 
> What about using error_report() instead of fprintf? Though it should be
> the same currently.
> 

Yes, using common helper functions is better.  I replaced all the
printf.


> > +
> > +   for (i = 0; i < ARRAY_SIZE(errors); ++i)
> > +   if (errors[i].err == err)
> > +   return errors[i].desc;
> 
> CODING_STYLE requires braces here.
> 

I fixed all the missing braces.


> > +
> > +   return "Invalid error code";
> > +}
> > +
> > +static inline int before(uint32_t seq1, uint32_t seq2)
> > +{
> > +return (int32_t)(seq1 - seq2) < 0;
> > +}
> > +
> > +static inline int after(uint32_t seq1, uint32_t seq2)
> > +{
> > +   return (int32_t)(seq2 - seq1) < 0;
> > +}
> 
> These functions look strange... Is the difference to seq1 < seq2 that
> the cast introduces intentional? (after(0x0, 0xabcdefff) == 1)
> 
> If yes, why is this useful? This needs a comment. If no, why even bother
> to have this function instead of directly using < or > ?
> 

These functions are used to compare sequential numbers which can be
wrap-around. For example, linux/net/tcp.h in the linux kernel.

Anyway, sheepdog doesn't use these functions, so I removed them.


> > +   if (snapid)
> > +   dprintf("%" PRIx32 " non current inode was open.\n", vid);
> > +   else
> > +   s->is_current = 1;
> > +
> > +   fd = connect_to_sdog(s->addr);
> 
> I wonder why you need to open another connection here instead of using
> s->fd. This pattern repeats at least in the snapshot functions, so I'm
> sure it's there for a reason. Maybe add a comment?
> 

We can use s->fd only for aio read/write operations.  It is because
the block driver may be during waiting response from the server, so we
cannot send other requests to the discriptor to avoid receiving wrong
data.

I added the comment to get_sheep_fd().


> > +
> > +   iov.iov_base = &s->inode;
> > +   iov.iov_len = sizeof(s->inode);
> > +   aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
> > +   data_len, offset, 0, 0, offset);
> > +   if (!aio_req) {
> > +   eprintf("too many requests\n");
> > +   acb->ret = -EIO;
> > +   goto out;
> > +   }
> 
> Randomly failing requests is probably not a good idea. The guest might
> decide that the disk/file system is broken and stop using it. Can't you
> use a list like in AIOPool, so you can dynamically add new requests as
> needed?
> 

I agree.  In the v3 patch, AIO requests are allocated dynamically, and
all the requests are linked to the outstanding_aio_head in the
BDRVSheepdogState.


> > +
> > +static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
> > +{
> > +   struct bdrv_sd_state *s = bs->opaque;
> > +   struct bdrv_sd_state *old_s;
> > +   char vdi[SD_MAX_VDI_LEN];
> > +   char *buf = NULL;
> > +   uint32_t vid;
> > +   uint32_t snapid = 0;
> > +   int ret = -ENOENT, fd;
> > +
> > +   old_s = qemu_malloc(sizeof(struct bdrv_sd_state));
> > +   if (!old_s) {
> 
> qemu_malloc never returns NULL.
> 

I removed all the NULL checks.


> > +
> > +BlockDriver bdrv_sheepdog = {
> > +   .format_name = "sheepdog",
> > +   .protocol_name = "sheepdog",
> > +   .instance_size = sizeof(struct bdrv_sd_state),
> > +   .bdrv_file_open = sd_open,
> > +   .bdrv_close = sd_close,
> > +   .bdrv_create = sd_create,
> > +
> > +   .bdrv_aio_readv = sd_aio_readv,
> > +   .bdrv_aio_writev = sd_aio_writev,
> > +
> > +   .bdrv_snapshot_create = sd_snapshot_create,
> > +   .bdrv_snapshot_goto = sd_snapshot_goto,
> > +   .bdrv_snapshot_delete = sd_snapshot_delete,
> > +   .bdrv_snapshot_list = sd_snapshot_list,
> > +
> > +   .bdrv_save_vmstate = sd_save_vmstate,
> > +   .bdrv_load_vmstate = sd_load_vmstate,
> > +
> > +   .create_options = sd_create_options,
> > +};
> 
> Please align the = to the same column, at least in each block.
> 

I have aligned in the v3 patch.


Thanks,

Kazutaka

[Qemu-devel] Re: Qemu-KVM Livate Migration 0.12.2 -> 0.12.3/4 broken?

[Jan Kiszka]

Juan Quintela wrote:
> Jan Kiszka  wrote:
>> Juan Quintela wrote:
>>> Lack of "proper" subsections.  IDE is something like:
>>>
>>> const VMStateDescription vmstate_ide_drive = {
>>> .version_id = 4,
>>> 
>>> }
>>>
>>> static const VMStateDescription vmstate_bmdma = {
>>> .name = "ide bmdma",
>>> .version_id = 4,
>>> ...
>>> }
>>>
>>> const VMStateDescription vmstate_ide_pci = {
>>> .name = "ide",
>>> .version_id = 4,
>>> 
>>> VMSTATE_STRUCT_ARRAY(bmdma, PCIIDEState, 2, 0,
>>>  vmstate_bmdma, BMDMAState),
>>> VMSTATE_IDE_DRIVES(bus[0].ifs, PCIIDEState),
>>> VMSTATE_IDE_DRIVES(bus[1].ifs, PCIIDEState),
>>> 
>>> }
>>>
>>>
>>> Notice that everything is at version 4.  It used to be everything at
>>> version 3.  Now the problem is that when migrating from v3 -> v4.  We
>>> put in one place v3, But we only have a version number at the toplevel,
>>> rest of "subsections" don't sent a version number.  There is no way to
>>> fix it in the general case.  We can hack something around for ide, but
>>> that will just be a hack, or we can backport marcelo change and port it
>>> as a proper subsection (that is my plan).  I expect to have time at the
>>> end of next time to work on this.
>> BTW, the IDE subsystem is yet lacking a proper vmstate section split-up
>> along qdev boundaries (ie. vmstate_ide_pci should not contain drive
>> structures). Do you plan to address this as well?
> 
> Not for Friday, and not for 0.12.

For sure. I missed that this was only a 0.12 issue.

> 
> That is 0.13 material, and have to get one agreement on how to go.
> We can go for:
> - good structure
> - backward compatibility
> 
> I can't see any good way to get both at this stage :(  But I am open to
> sugestions.

Based on recent experiments with vmstate to enhance the hpet, I'm fairly
optimistic that we can have both (just the code complexity suffers a
bit): Split up the drive sections for new versions, but keep the legacy
fields with attached .field_exists() filters for reading of old
versions. But I may also underestimate issues of this particular case.

> 
> Later, Juan.
> 
> PD. BTW, very good work with printing the vmstate, that was one of the goals
> when we added it, that was the next step after porting everything to
> vmstate :)
> 

I'm sorry for stealing you the pleasure to add it. :)

Jan



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Kevin Wolf]

Am 17.05.2010 12:14, schrieb Alexander Graf:
> Usually the guest can tell the host to flush data to disk. In some cases we
> don't want to flush though, but try to keep everything in cache.
> 
> So let's add a new cache value to -drive that allows us to set the cache
> policy to most aggressive, disabling flushes. We call this mode "volatile",
> as guest data is not guaranteed to survive host crashes anymore.
> 
> This patch also adds a noop function for aio, so we can do nothing in AIO
> fashion.
> 
> Signed-off-by: Alexander Graf 

Thanks, looks good to me now. Applied it to the block branch, but
depending on Anthony's opinion I might drop it again.

Kevin

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Gerd Hoffmann]

+directfb="no"


Should be ="" (aka autodetect).


+if test "$directfb" = "yes" ; then
+  directfb_libs=`directfb-config --libs`
+  directfb_cflags=`directfb-config --cflags`
+  libs_softmmu="$directfb_libs $libs_softmmu"
+fi


use pkgconfig here.  directfb-config most likely is just a pkgconfig 
wrapper anyway.


cheeers,
  Gerd

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Gerd Hoffmann]

  Hi,


Can you provide some performance data to justify this since SDL provides
the same ability?


IMHO no performance data is needed to justify this because SDL running 
on top of the linux framebuffer is simply unusable IMHO.


cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v3 2/3] block: call the snapshot handlers of the protocol drivers

[Kevin Wolf]

Am 17.05.2010 12:19, schrieb MORITA Kazutaka:
> When snapshot handlers are not defined in the format driver, it is
> better to call the ones of the protocol driver.  This enables us to
> implement snapshot support in the protocol driver.
> 
> We need to call bdrv_close() and bdrv_open() handlers of the format
> driver before and after bdrv_snapshot_goto() call of the protocol.  It is
> because the contents of the block driver state may need to be changed
> after loading vmstate.
> 
> Signed-off-by: MORITA Kazutaka 
> ---
>  block.c |   61 +++--
>  1 files changed, 43 insertions(+), 18 deletions(-)
> 
> diff --git a/block.c b/block.c
> index f3bf3f2..c987e57 100644
> --- a/block.c
> +++ b/block.c
> @@ -1683,9 +1683,11 @@ int bdrv_save_vmstate(BlockDriverState *bs, const 
> uint8_t *buf,
>  BlockDriver *drv = bs->drv;
>  if (!drv)
>  return -ENOMEDIUM;
> -if (!drv->bdrv_save_vmstate)
> -return -ENOTSUP;
> -return drv->bdrv_save_vmstate(bs, buf, pos, size);
> +if (drv->bdrv_save_vmstate)
> +return drv->bdrv_save_vmstate(bs, buf, pos, size);
> +if (bs->file)
> +return bdrv_save_vmstate(bs->file, buf, pos, size);
> +return -ENOTSUP;
>  }
>  
>  int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
> @@ -1694,9 +1696,11 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t 
> *buf,
>  BlockDriver *drv = bs->drv;
>  if (!drv)
>  return -ENOMEDIUM;
> -if (!drv->bdrv_load_vmstate)
> -return -ENOTSUP;
> -return drv->bdrv_load_vmstate(bs, buf, pos, size);
> +if (drv->bdrv_load_vmstate)
> +return drv->bdrv_load_vmstate(bs, buf, pos, size);
> +if (bs->file)
> +return bdrv_load_vmstate(bs->file, buf, pos, size);
> +return -ENOTSUP;
>  }
>  
>  void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event)
> @@ -1720,20 +1724,37 @@ int bdrv_snapshot_create(BlockDriverState *bs,
>  BlockDriver *drv = bs->drv;
>  if (!drv)
>  return -ENOMEDIUM;
> -if (!drv->bdrv_snapshot_create)
> -return -ENOTSUP;
> -return drv->bdrv_snapshot_create(bs, sn_info);
> +if (drv->bdrv_snapshot_create)
> +return drv->bdrv_snapshot_create(bs, sn_info);
> +if (bs->file)
> +return bdrv_snapshot_create(bs->file, sn_info);
> +return -ENOTSUP;
>  }
>  
>  int bdrv_snapshot_goto(BlockDriverState *bs,
> const char *snapshot_id)
>  {
>  BlockDriver *drv = bs->drv;
> +int ret, open_ret;
> +
>  if (!drv)
>  return -ENOMEDIUM;
> -if (!drv->bdrv_snapshot_goto)
> -return -ENOTSUP;
> -return drv->bdrv_snapshot_goto(bs, snapshot_id);
> +if (drv->bdrv_snapshot_goto)
> +return drv->bdrv_snapshot_goto(bs, snapshot_id);
> +
> +if (bs->file) {
> +drv->bdrv_close(bs);
> +ret = bdrv_snapshot_goto(bs->file, snapshot_id);
> +open_ret = drv->bdrv_open(bs, bs->open_flags);
> +if (open_ret < 0) {
> +bdrv_delete(bs);

I think you mean bs->file here.

Kevin

> +bs->drv = NULL;
> +return open_ret;
> +}
> +return ret;
> +}
> +
> +return -ENOTSUP;
>  }
>  
>  int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)

Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Markus Armbruster]

Avi Kivity  writes:

> On 05/17/2010 11:27 AM, Markus Armbruster wrote:
>>
>>
> A slot is the hotpluggable entity.  Open your computer and you can
> actually see them.
>
>  
 QEMU doesn't really know that.


>>> How can that be?  Do we signal hotplug notifications to a function or
>>> to a slot?
>>>
>>> Can we hotplug a single function in an already occupied slot?
>>>  
>> What I meant to say: we have no concept of "slot" in the higher level
>> interfaces, we have only bus and device.
>>
>> If a PCI device has multiple functions, we have a separate qdev device
>> for each function.  You can't unplug a "slot" (concept doesn't exist in
>> qdev), only a qdev device.  Naturally, when you unplug a qdev device,
>> all functions in the same PCI slot need to go.  This happens deep down
>> in the bowels of ACPI, in piix4_device_hotplug().  qdev is not aware of
>> this magic relation between the qdev devices for functions in the same
>> slot.
>>
>
> IMO, that's a serious bug.  A slot is a user visible entity, both in
> that devices can only be hotplugged only as slots, not functions, and
> to determine the maximum number of devices you can add.  If the user
> knows about it, qemu should too.
>
> We can easily represent a slot/device as a qbus with each of the
> functions as devices attached to it.

Dunno.  Gerd, what do you think?

Re: [Qemu-devel] Re: [PATCH 00/26] split out piix specific part from pc emulator and some clean ups

[Markus Armbruster]

Blue Swirl  writes:

> Thanks, applied all.

Thanks indeed!  13 versions and I-don't-remember-how-many months, must
have taxed your patience.

[Qemu-devel] Re: [PATCH 0/2] pckbd improvements

[Paolo Bonzini]

> > > In 2/2, A20 logic changes a bit but I doubt any guest would be broken
> > > if A20 line written through I/O port 92 couldn't be read via i8042.
> > > The reverse (write using i8042 and read port 92) will work.
> > >
> >
> >  Why take the risk?
> 
> The alternative is to route a signal from port 92 to i8042. Or maybe
> port 92 should belong to i8042, that could make things simpler but
> then the port would appear on non-PC architectures as well.
>
> But I doubt any OS would depend on such details, because the details
> seem to be murky:
> http://www.win.tue.nl/~aeb/linux/kbd/A20.html

True, but I don't see why we should introduce a possible regression.
I would at the very least test it on real hardware before doing the
change.

Paolo

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Christoph Hellwig]

On Fri, May 14, 2010 at 05:58:50PM +0100, Julian Pidancet wrote:
> This patch implements a DirectFB driver for QEMU. It allows Qemu to
> draw a VM graphic output directly in the framebuffer of the host,
> without having to rely on X11.
> DirectFB also provides with a generic interface take advantage of graphic
> hardware acceleration for a bunch of different supported cards.

Doesn't DirectFB still require utterly broken out of tree kernel
patches?

[Qemu-devel] Re: [RFC PATCH v3 2/3] block: call the snapshot handlers of the protocol drivers

[MORITA Kazutaka]

At Mon, 17 May 2010 13:08:08 +0200,
Kevin Wolf wrote:
> 
> Am 17.05.2010 12:19, schrieb MORITA Kazutaka:
> >  
> >  int bdrv_snapshot_goto(BlockDriverState *bs,
> > const char *snapshot_id)
> >  {
> >  BlockDriver *drv = bs->drv;
> > +int ret, open_ret;
> > +
> >  if (!drv)
> >  return -ENOMEDIUM;
> > -if (!drv->bdrv_snapshot_goto)
> > -return -ENOTSUP;
> > -return drv->bdrv_snapshot_goto(bs, snapshot_id);
> > +if (drv->bdrv_snapshot_goto)
> > +return drv->bdrv_snapshot_goto(bs, snapshot_id);
> > +
> > +if (bs->file) {
> > +drv->bdrv_close(bs);
> > +ret = bdrv_snapshot_goto(bs->file, snapshot_id);
> > +open_ret = drv->bdrv_open(bs, bs->open_flags);
> > +if (open_ret < 0) {
> > +bdrv_delete(bs);
> 
> I think you mean bs->file here.
> 
> Kevin

This is an error of re-opening the format driver, so what we should
delete here is not bs->file but bs, isn't it?  If we failed to open bs
here, the drive doesn't seem to work anymore.

Regards,

Kazutaka

> > +bs->drv = NULL;
> > +return open_ret;
> > +}
> > +return ret;
> > +}
> > +
> > +return -ENOTSUP;
> >  }

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Julian Pidancet]

On 05/17/2010 11:53 AM, Gerd Hoffmann wrote:
>> +directfb="no"
> 
> Should be ="" (aka autodetect).
>



>> +if test "$directfb" = "yes" ; then
>> +  directfb_libs=`directfb-config --libs`
>> +  directfb_cflags=`directfb-config --cflags`
>> +  libs_softmmu="$directfb_libs $libs_softmmu"
>> +fi
> 
> use pkgconfig here.  directfb-config most likely is just a pkgconfig 
> wrapper anyway.
> 

Unfortunately, directfb-config is not a pkgconfig wrapper, it is a standalone 
shell script.
Thus, I don't know if there is an easier way of "autodetecting" directfb than 
probing the directfb-config script.

-- 
Julian

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Julian Pidancet]

On 05/17/2010 12:44 PM, Christoph Hellwig wrote:
> On Fri, May 14, 2010 at 05:58:50PM +0100, Julian Pidancet wrote:
>> This patch implements a DirectFB driver for QEMU. It allows Qemu to
>> draw a VM graphic output directly in the framebuffer of the host,
>> without having to rely on X11.
>> DirectFB also provides with a generic interface take advantage of graphic
>> hardware acceleration for a bunch of different supported cards.
> 
> Doesn't DirectFB still require utterly broken out of tree kernel
> patches?
> 

I have not heard about that, sorry.
My test configuration has an i915-like Intel graphic card, and a nearly 
standard 2.6.32 linux kernel. No specific patching was required.

-- 
Julian

Re: [Qemu-devel] Re: [RFC PATCH v3 2/3] block: call the snapshot handlers of the protocol drivers

[Kevin Wolf]

Am 17.05.2010 14:19, schrieb MORITA Kazutaka:
> At Mon, 17 May 2010 13:08:08 +0200,
> Kevin Wolf wrote:
>>
>> Am 17.05.2010 12:19, schrieb MORITA Kazutaka:
>>>  
>>>  int bdrv_snapshot_goto(BlockDriverState *bs,
>>> const char *snapshot_id)
>>>  {
>>>  BlockDriver *drv = bs->drv;
>>> +int ret, open_ret;
>>> +
>>>  if (!drv)
>>>  return -ENOMEDIUM;
>>> -if (!drv->bdrv_snapshot_goto)
>>> -return -ENOTSUP;
>>> -return drv->bdrv_snapshot_goto(bs, snapshot_id);
>>> +if (drv->bdrv_snapshot_goto)
>>> +return drv->bdrv_snapshot_goto(bs, snapshot_id);
>>> +
>>> +if (bs->file) {
>>> +drv->bdrv_close(bs);
>>> +ret = bdrv_snapshot_goto(bs->file, snapshot_id);
>>> +open_ret = drv->bdrv_open(bs, bs->open_flags);
>>> +if (open_ret < 0) {
>>> +bdrv_delete(bs);
>>
>> I think you mean bs->file here.
>>
>> Kevin
> 
> This is an error of re-opening the format driver, so what we should
> delete here is not bs->file but bs, isn't it?  If we failed to open bs
> here, the drive doesn't seem to work anymore.

But bdrv_delete means basically free it. This is almost guaranteed to
lead to crashes because that BlockDriverState is still in use in other
places.

One additional case of use after free is in the very next line:

>>> +bs->drv = NULL;

You can't do that when bs is freed, obviously. But I think just setting
bs->drv to NULL without bdrv_deleting it before is the better way. It
will fail any requests (with -ENOMEDIUM), but can't produce crashes.
This is also what bdrv_commit does in such situations.

In this state, we don't access the underlying file any more, so we could
delete bs->file - this is why I thought you actually meant to do that.

Kevin

[Qemu-devel] [PATCH] MIPS DINSU

[Dmitry Antipov]

Hello,

shouldn't it be in that way?

Dmitry

--- qemu-0.12.4/target-mips/translate.c 2010-05-17 16:12:58.048661610 +0400
+++ qemu-0.12.4/target-mips/translate.c 2010-05-17 16:13:12.281656754 +0400
@@ -2761,7 +2761,7 @@
 case OPC_DINSU:
 if (lsb > msb)
 goto fail;
-mask = ((1ULL << (msb - lsb + 1)) - 1) << lsb;
+mask = ((1ULL << (msb - lsb + 1)) - 1) << (lsb + 32);
 gen_load_gpr(t0, rt);
 tcg_gen_andi_tl(t0, t0, ~mask);
 tcg_gen_shli_tl(t1, t1, lsb + 32);

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Christoph Hellwig]

On Mon, May 17, 2010 at 01:14:05PM +0100, Julian Pidancet wrote:
> I have not heard about that, sorry.
> My test configuration has an i915-like Intel graphic card, and a nearly 
> standard 2.6.32 linux kernel. No specific patching was required.

You're right.  I've checked it and the horrible kernel code is entirely
optional.  Sorry for the noise.

Re: [Qemu-devel] Re: [PATCH 2/2] Add flush=off parameter to -drive

[Christoph Hellwig]

On Tue, May 11, 2010 at 11:11:12PM +0100, Paul Brook wrote:
> .. though it may be a kernel bug rather that a qemu bug, depending on the 
> exact details.  Either way, I consider any mode that inhibits host filesystem 
> write cache but not volatile drive cache to be pretty worthless.  Either we 
> guaranteed data integrity on completion or we don't.

O_DIRECT just means bypassing the pagecache, it does not mean flushing
the disk cache on every access, which for certain workloads can be very
painful.  It also doesn't require a synchronous writeout of metadata
required to reach the data, e.g. in case when we have to allocate blocks
for a sparse image file.

To get the behaviour you want you need O_DIRECT|O_DSYNC, which is
something that is not exposed by qemu's current cache= suboption.

If we want to implement this properly we need to split the cache option,
as I already mentioned.  This would also have benefits in other areas,
but again refer to my previous mail for that.

Re: [Qemu-devel] Re: [PATCH 2/2] Add flush=off parameter to -drive

[Christoph Hellwig]

On Fri, May 14, 2010 at 11:16:47AM +0200, Markus Armbruster wrote:
> If you change -drive, please keep the coming separation in mind, and
> avoid parameters that mix up guest and host.

This new option seems to be exactly that, unfortunately..

Re: [Qemu-devel] Re: [PATCH 2/2] Add flush=off parameter to -drive

[Alexander Graf]

On 17.05.2010, at 14:41, Christoph Hellwig wrote:

> On Fri, May 14, 2010 at 11:16:47AM +0200, Markus Armbruster wrote:
>> If you change -drive, please keep the coming separation in mind, and
>> avoid parameters that mix up guest and host.
> 
> This new option seems to be exactly that, unfortunately..
> 

How so? It only affects the host. The guest never gets to see that flushing is 
disabled.


Alex

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Anthony Liguori]

On 05/17/2010 05:14 AM, Alexander Graf wrote:

Usually the guest can tell the host to flush data to disk. In some cases we
don't want to flush though, but try to keep everything in cache.

So let's add a new cache value to -drive that allows us to set the cache
policy to most aggressive, disabling flushes. We call this mode "volatile",
as guest data is not guaranteed to survive host crashes anymore.

This patch also adds a noop function for aio, so we can do nothing in AIO
fashion.

Signed-off-by: Alexander Graf
   


I'd like to see some performance data with at least an ext3 host file 
system and an ext4 file system.


My concern is that ext3 exaggerates the cost of fsync() which will 
result in diminishing value over time for this feature as people move to 
ext4/btrfs.


Regards,

Anthony Liguori


---

v2 ->  v3:

   - Add description of cache=volatile
   - Squash aio noop noop patch into this one
---
  block.c |   28 
  block.h |1 +
  qemu-config.c   |2 +-
  qemu-options.hx |   13 ++---
  vl.c|3 +++
  5 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 48305b7..b742965 100644
--- a/block.c
+++ b/block.c
@@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState 
*bs,
  BlockDriverCompletionFunc *cb, void *opaque);
  static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
  BlockDriverCompletionFunc *cb, void *opaque);
+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque);
  static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
  uint8_t *buf, int nb_sectors);
  static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
@@ -1306,6 +1308,10 @@ const char *bdrv_get_device_name(BlockDriverState *bs)

  void bdrv_flush(BlockDriverState *bs)
  {
+if (bs->open_flags&  BDRV_O_NO_FLUSH) {
+return;
+}
+
  if (bs->drv&&  bs->drv->bdrv_flush)
  bs->drv->bdrv_flush(bs);
  }
@@ -2082,6 +2088,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
  {
  BlockDriver *drv = bs->drv;

+if (bs->open_flags&  BDRV_O_NO_FLUSH) {
+return bdrv_aio_noop_em(bs, cb, opaque);
+}
+
  if (!drv)
  return NULL;
  return drv->bdrv_aio_flush(bs, cb, opaque);
@@ -2196,6 +2206,24 @@ static BlockDriverAIOCB 
*bdrv_aio_flush_em(BlockDriverState *bs,
  return&acb->common;
  }

+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BlockDriverAIOCBSync *acb;
+
+acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque);
+acb->is_write = 1; /* don't bounce in the completion hadler */
+acb->qiov = NULL;
+acb->bounce = NULL;
+acb->ret = 0;
+
+if (!acb->bh)
+acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
+
+qemu_bh_schedule(acb->bh);
+return&acb->common;
+}
+
  /**/
  /* sync block device emulation */

diff --git a/block.h b/block.h
index f87d24e..8032b6b 100644
--- a/block.h
+++ b/block.h
@@ -33,6 +33,7 @@ typedef struct QEMUSnapshotInfo {
  #define BDRV_O_CACHE_WB0x0040 /* use write-back caching */
  #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread 
pool */
  #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
+#define BDRV_O_NO_FLUSH0x0200 /* disable flushing on this disk */

  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)

diff --git a/qemu-config.c b/qemu-config.c
index d500885..bf3d493 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -54,7 +54,7 @@ QemuOptsList qemu_drive_opts = {
  },{
  .name = "cache",
  .type = QEMU_OPT_STRING,
-.help = "host cache usage (none, writeback, writethrough)",
+.help = "host cache usage (none, writeback, writethrough, 
volatile)",
  },{
  .name = "aio",
  .type = QEMU_OPT_STRING,
diff --git a/qemu-options.hx b/qemu-options.hx
index 12f6b51..6dedb4a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,8 +118,9 @@ ETEXI
  DEF("drive", HAS_ARG, QEMU_OPTION_drive,
  "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
  "   [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-"   [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
-"   [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
+"   [,cache=writethrough|writeback|volatile|none][,format=f]\n"
+"   [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
+"   [,readonly=on|off]\n"
  "use 'file' as a drive image\n", QEMU_ARCH_ALL)
  STEXI
  @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
@@ -148,7 +149,7 @@ These options have the same definition as they have in 
@option{-hdachs}.
 

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Alexander Graf]

On 17.05.2010, at 14:58, Anthony Liguori wrote:

> On 05/17/2010 05:14 AM, Alexander Graf wrote:
>> Usually the guest can tell the host to flush data to disk. In some cases we
>> don't want to flush though, but try to keep everything in cache.
>> 
>> So let's add a new cache value to -drive that allows us to set the cache
>> policy to most aggressive, disabling flushes. We call this mode "volatile",
>> as guest data is not guaranteed to survive host crashes anymore.
>> 
>> This patch also adds a noop function for aio, so we can do nothing in AIO
>> fashion.
>> 
>> Signed-off-by: Alexander Graf
>>   
> 
> I'd like to see some performance data with at least an ext3 host file system 
> and an ext4 file system.

For ext3 data, please see my cover-letter from v2:

http://www.mail-archive.com/qemu-devel@nongnu.org/msg31714.html

> My concern is that ext3 exaggerates the cost of fsync() which will result in 
> diminishing value over time for this feature as people move to ext4/btrfs.

There will be ext3 file systems for years out. Just because people can use 
better and faster file systems doesn't mean they do. And I'm sure they can't 
always choose. If anything, I can try and see what the numbers look like for 
xfs.


Alex

Re: [Qemu-devel] Re: [RFC PATCH v3 2/3] block: call the snapshot handlers of the protocol drivers

[MORITA Kazutaka]

On Mon, May 17, 2010 at 9:20 PM, Kevin Wolf  wrote:
> Am 17.05.2010 14:19, schrieb MORITA Kazutaka:
>> At Mon, 17 May 2010 13:08:08 +0200,
>> Kevin Wolf wrote:
>>>
>>> Am 17.05.2010 12:19, schrieb MORITA Kazutaka:

  int bdrv_snapshot_goto(BlockDriverState *bs,
                         const char *snapshot_id)
  {
      BlockDriver *drv = bs->drv;
 +    int ret, open_ret;
 +
      if (!drv)
          return -ENOMEDIUM;
 -    if (!drv->bdrv_snapshot_goto)
 -        return -ENOTSUP;
 -    return drv->bdrv_snapshot_goto(bs, snapshot_id);
 +    if (drv->bdrv_snapshot_goto)
 +        return drv->bdrv_snapshot_goto(bs, snapshot_id);
 +
 +    if (bs->file) {
 +        drv->bdrv_close(bs);
 +        ret = bdrv_snapshot_goto(bs->file, snapshot_id);
 +        open_ret = drv->bdrv_open(bs, bs->open_flags);
 +        if (open_ret < 0) {
 +            bdrv_delete(bs);
>>>
>>> I think you mean bs->file here.
>>>
>>> Kevin
>>
>> This is an error of re-opening the format driver, so what we should
>> delete here is not bs->file but bs, isn't it?  If we failed to open bs
>> here, the drive doesn't seem to work anymore.
>
> But bdrv_delete means basically free it. This is almost guaranteed to
> lead to crashes because that BlockDriverState is still in use in other
> places.
>
> One additional case of use after free is in the very next line:
>
 +            bs->drv = NULL;
>
> You can't do that when bs is freed, obviously. But I think just setting
> bs->drv to NULL without bdrv_deleting it before is the better way. It
> will fail any requests (with -ENOMEDIUM), but can't produce crashes.
> This is also what bdrv_commit does in such situations.
>
> In this state, we don't access the underlying file any more, so we could
> delete bs->file - this is why I thought you actually meant to do that.
>

I'm sorry for the confusion.  I understand what we should do here.
I'll fix it for the next post.

Thanks,

Kazutaka

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Anthony Liguori]

On 05/17/2010 02:45 AM, Avi Kivity wrote:

On 05/17/2010 10:40 AM, Jan Kiszka wrote:


The alternative is to have a schema.  Sun RPC/XDR doesn't carry any 
type
information (you can't even distinguish between a number and text) 
yet C

clients have to problem extracting typed information from it.

Having __class__ everywhere means we're carrying the schema in every
message instead of just once.

The device_show command is already an example where you don't have a
predefined schema. It is derived from the data stream the encodes the
vmstate fields. So far we have no collision between base64-encoded
buffers and real strings, but this may actually change when we start
annotating the fields with symbolic constants.


What is the receiver to do with it?

If it doesn't know the schema (and there is no schema), then all it 
can do is display the key/values.  If it does know the schema, then 
__class__ is unnecessary.


My worry is that __class__ will make the protocol more ad-hoc.


I really don't see the problem with __class__. Being a text protocol,
JSON is already fairly verbose.


The problem is not the verbosity, it's that information is carried too 
late.  Many clients want to know this information at compile time or 
initialization time, and we are providing it at object instantiating 
time.


Whether a protocol is self-describing is orthogonal to whether it's well 
defined (ala a schema).  A self-describing protocol is very convenient 
for dynamic languages like Python.  We should also provide a formal 
schema though for languages that require IDL to generate bindings (like C).


Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Anthony Liguori]

On 05/17/2010 08:02 AM, Alexander Graf wrote:

My concern is that ext3 exaggerates the cost of fsync() which will result in 
diminishing value over time for this feature as people move to ext4/btrfs.
 

There will be ext3 file systems for years out. Just because people can use 
better and faster file systems doesn't mean they do. And I'm sure they can't 
always choose. If anything, I can try and see what the numbers look like for 
xfs.
   


But ext3 with barrier=1 is pretty uncommon in practice.  Another data 
point would be an ext3 host file system with barrier=0.


Regards,

Anthony Liguori



Alex

   

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Alexander Graf]

On 17.05.2010, at 15:09, Anthony Liguori wrote:

> On 05/17/2010 08:02 AM, Alexander Graf wrote:
>>> My concern is that ext3 exaggerates the cost of fsync() which will result 
>>> in diminishing value over time for this feature as people move to 
>>> ext4/btrfs.
>>> 
>> There will be ext3 file systems for years out. Just because people can use 
>> better and faster file systems doesn't mean they do. And I'm sure they can't 
>> always choose. If anything, I can try and see what the numbers look like for 
>> xfs.
>>   
> 
> But ext3 with barrier=1 is pretty uncommon in practice.  Another data point 
> would be an ext3 host file system with barrier=0.

Who defines what is common and what not? To me, the SLES11 default is common. 
In fact, the numbers in the referred mail were done on an 11.1 system.


Alex

[Qemu-devel] Re: [PATCH 1/2] QMP: Introduce commands doc

[Luiz Capitulino]

On Sat, 15 May 2010 10:42:44 +0200
Jan Kiszka  wrote:

> Luiz Capitulino wrote:
> > On Fri, 14 May 2010 19:08:07 +0200
> > Jan Kiszka  wrote:
> > 
> >> Avi Kivity wrote:
> >>> On 05/14/2010 08:01 PM, Avi Kivity wrote:
>  On 05/14/2010 07:52 PM, Jan Kiszka wrote:
> >> In order not to compromise QMP adoption and make users' life easier,
> >> this commit adds a simple text documentation which fully describes
> >> all QMP supported commands.
> >>
> >> This is not ideal for a number of reasons (harder to maintain,
> >> text-only, etc) but does improve the current situation.
> > Even if it's temporary - maintaining it in a separate file looks rather
> > unhandy.
> >
> > Can't we generate the per-command documentation snippets also from
> > qemu-monitor.hx and merge them with a header/footer into some text file?
> > That .hx file is the one anyone adding/converting commands has to touch
> > anyway.
>  If we do, then the generated documentation should be included in the 
>  patch changelog for review.
> 
> >>> I mean, a patch introducing or modifying a monitor command.
> >> The snippets should be readable by themselves. I'm only proposing to
> >> keep them in the central file, at the same location where the others
> >> are. There is no difference compared to existing monitor commands, we
> >> just add the third documentation snippet, this time for QMP.
> > 
> >  It's not only the snippets. We add a json type for each parameter, a
> > more descriptive info and notes. Only QMP commands should be shown
> > this way.
> 
> Whatever its semantic is, technically it's a text block of a few lines
> that has to be added somewhere.
> 
> > 
> >  I'm sure there's a way to hack the qemu's doc script to do all this,
> > but the right solution is to move _everything_ to json and generate good,
> > well formatted documentation from there (along with self-description).
> 
> I'm not talking about The Right solution, I'm talking about a more handy
> temporary setup. When do you plan to refactor the command documentation
> that way? And how many commands will be touched in the meantime?

 It's something we would like to do ASAP, but there are a number of things
we'll have to do first: bug fixes and some commands libvirt wants to use.

> >  Also, adapting things will take time and this will delay even more this
> > doc to be merged, which is what I'm trying to avoid.
> > 
> 
> I can provide you the patch for hxtool and Makefile (a few lines), and
> I'm willing to split up the existing doc, just drop me a note. So
> nothing needs to be delayed any further.

 I'd love that.

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Anthony Liguori]

On 05/17/2010 08:17 AM, Alexander Graf wrote:

On 17.05.2010, at 15:09, Anthony Liguori wrote:

   

On 05/17/2010 08:02 AM, Alexander Graf wrote:
 

My concern is that ext3 exaggerates the cost of fsync() which will result in 
diminishing value over time for this feature as people move to ext4/btrfs.

 

There will be ext3 file systems for years out. Just because people can use 
better and faster file systems doesn't mean they do. And I'm sure they can't 
always choose. If anything, I can try and see what the numbers look like for 
xfs.

   

But ext3 with barrier=1 is pretty uncommon in practice.  Another data point 
would be an ext3 host file system with barrier=0.
 

Who defines what is common and what not? To me, the SLES11 default is common. 
In fact, the numbers in the referred mail were done on an 11.1 system.
   


But it wasn't the SLES10 default so there's a smaller window of systems 
that are going to be configured this way.  But this is orthogonal to the 
main point.  Let's quantify how important this detail is before we 
discuss the affected user base.


Regards,

Anthony Liguori


Alex

   

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Anthony Liguori]

On 05/15/2010 08:10 PM, Paul Brook wrote:

The other solution would be to use the DirectFB driver for SDL which
would allow to do slightly the same as this patch. But that would mean
having to deal with an additional layer in the graphical stack, which is
not exactly what one wants from a performance or a complexity point of
view.
 

I don't buy your complexity argument.  Doesn't DirectFB-via-SDL already work?
If not why not? I'm pretty sure fixing that would be way simpler than adding a
whole new output backend.
   


Yeah, I don't buy it either.  I think performance data is probably the 
only way to justify this and I'm sceptical that if there is a 
performance advantage that it wouldn't be possible to just fix SDL's 
DirectFB support.


Regards,

Anthony Liguori


Paul

   

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Alexander Graf]

Anthony Liguori wrote:
> On 05/17/2010 08:17 AM, Alexander Graf wrote:
>> On 17.05.2010, at 15:09, Anthony Liguori wrote:
>>
>>   
>>> On 05/17/2010 08:02 AM, Alexander Graf wrote:
>>> 
> My concern is that ext3 exaggerates the cost of fsync() which will
> result in diminishing value over time for this feature as people
> move to ext4/btrfs.
>
>  
 There will be ext3 file systems for years out. Just because people
 can use better and faster file systems doesn't mean they do. And
 I'm sure they can't always choose. If anything, I can try and see
 what the numbers look like for xfs.


>>> But ext3 with barrier=1 is pretty uncommon in practice.  Another
>>> data point would be an ext3 host file system with barrier=0.
>>>  
>> Who defines what is common and what not? To me, the SLES11 default is
>> common. In fact, the numbers in the referred mail were done on an
>> 11.1 system.
>>
>
> But it wasn't the SLES10 default so there's a smaller window of
> systems that are going to be configured this way.  But this is
> orthogonal to the main point.  Let's quantify how important this
> detail is before we discuss the affected user base.

Alright. I took my Netbook (2GB of RAM) and a USB hard disk, so I can
easily remount the data fs the vmdk image is on. Here are the results:

# mkfs.ext3 /dev/sdc1
# mount /dev/sdc1 /mnt -obarrier=1

cache=writeback

real0m52.801s
user0m16.065s
sys 0m6.688s

cache=volatile

real0m47.876s
user0m15.921s
sys 0m6.548s

# mount /dev/sdc1 /mnt -obarrier=0

cache=writeback

real0m53.588s
user0m15.901s
sys 0m6.576s

cache=volatile

real0m48.715s
user0m16.581s
sys 0m5.856s

I don't see a difference between the results. Apparently the barrier
option doesn't change a thing.


Alex

Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Alexander Graf]

Alexander Graf wrote:
> Anthony Liguori wrote:
>   
>> On 05/17/2010 08:17 AM, Alexander Graf wrote:
>> 
>>> On 17.05.2010, at 15:09, Anthony Liguori wrote:
>>>
>>>   
>>>   
 On 05/17/2010 08:02 AM, Alexander Graf wrote:
 
 
>> My concern is that ext3 exaggerates the cost of fsync() which will
>> result in diminishing value over time for this feature as people
>> move to ext4/btrfs.
>>
>>  
>> 
> There will be ext3 file systems for years out. Just because people
> can use better and faster file systems doesn't mean they do. And
> I'm sure they can't always choose. If anything, I can try and see
> what the numbers look like for xfs.
>
>
>   
 But ext3 with barrier=1 is pretty uncommon in practice.  Another
 data point would be an ext3 host file system with barrier=0.
  
 
>>> Who defines what is common and what not? To me, the SLES11 default is
>>> common. In fact, the numbers in the referred mail were done on an
>>> 11.1 system.
>>>
>>>   
>> But it wasn't the SLES10 default so there's a smaller window of
>> systems that are going to be configured this way.  But this is
>> orthogonal to the main point.  Let's quantify how important this
>> detail is before we discuss the affected user base.
>> 
>
> Alright. I took my Netbook (2GB of RAM) and a USB hard disk, so I can
> easily remount the data fs the vmdk image is on. Here are the results:
>
> # mkfs.ext3 /dev/sdc1
> # mount /dev/sdc1 /mnt -obarrier=1
>
> cache=writeback
>
> real0m52.801s
> user0m16.065s
> sys 0m6.688s
>
> cache=volatile
>
> real0m47.876s
> user0m15.921s
> sys 0m6.548s
>
> # mount /dev/sdc1 /mnt -obarrier=0
>
> cache=writeback
>
> real0m53.588s
> user0m15.901s
> sys 0m6.576s
>
> cache=volatile
>
> real0m48.715s
> user0m16.581s
> sys 0m5.856s
>
> I don't see a difference between the results. Apparently the barrier
> option doesn't change a thing.
>   

The same test case for XFS:

cache=writeback

real0m50.868s
user0m11.133s
sys0m12.733s

cache=volatile

real0m43.680s
user0m16.089s
sys0m7.812s

Though I did have numbers here going as far down as 25 seconds for a run!


Alex

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Julian Pidancet]

On 05/17/2010 02:30 PM, Anthony Liguori wrote:
> On 05/15/2010 08:10 PM, Paul Brook wrote:
>>> The other solution would be to use the DirectFB driver for SDL which
>>> would allow to do slightly the same as this patch. But that would mean
>>> having to deal with an additional layer in the graphical stack, which is
>>> not exactly what one wants from a performance or a complexity point of
>>> view.
>>>  
>> I don't buy your complexity argument.  Doesn't DirectFB-via-SDL already work?
>> If not why not? I'm pretty sure fixing that would be way simpler than adding 
>> a
>> whole new output backend.
>>
> 
> Yeah, I don't buy it either.  I think performance data is probably the 
> only way to justify this and I'm sceptical that if there is a 
> performance advantage that it wouldn't be possible to just fix SDL's 
> DirectFB support.
> 

I don't think wether fixing or not SDL is the debate here, the question would 
be more wether or not we want to add a lightweight display driver to qemu. 
Also, I think a DirectFB driver is fairly easy to maintain.

I will get some performance data as soon as I have some time.

By the way, sorry for the patch reposts, it seems that my smtp server had quite 
some trouble to relay messages last week.

-- 
Julian

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Anthony Liguori]

On 05/17/2010 05:14 AM, Alexander Graf wrote:

Usually the guest can tell the host to flush data to disk. In some cases we
don't want to flush though, but try to keep everything in cache.

So let's add a new cache value to -drive that allows us to set the cache
policy to most aggressive, disabling flushes. We call this mode "volatile",
as guest data is not guaranteed to survive host crashes anymore.

This patch also adds a noop function for aio, so we can do nothing in AIO
fashion.

Signed-off-by: Alexander Graf

---

v2 ->  v3:

   - Add description of cache=volatile
   - Squash aio noop noop patch into this one
---
  block.c |   28 
  block.h |1 +
  qemu-config.c   |2 +-
  qemu-options.hx |   13 ++---
  vl.c|3 +++
  5 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 48305b7..b742965 100644
--- a/block.c
+++ b/block.c
@@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState 
*bs,
  BlockDriverCompletionFunc *cb, void *opaque);
  static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
  BlockDriverCompletionFunc *cb, void *opaque);
+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque);
  static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
  uint8_t *buf, int nb_sectors);
  static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
@@ -1306,6 +1308,10 @@ const char *bdrv_get_device_name(BlockDriverState *bs)

  void bdrv_flush(BlockDriverState *bs)
  {
+if (bs->open_flags&  BDRV_O_NO_FLUSH) {
+return;
+}
+
  if (bs->drv&&  bs->drv->bdrv_flush)
  bs->drv->bdrv_flush(bs);
  }
@@ -2082,6 +2088,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
  {
  BlockDriver *drv = bs->drv;

+if (bs->open_flags&  BDRV_O_NO_FLUSH) {
+return bdrv_aio_noop_em(bs, cb, opaque);
+}
+
  if (!drv)
  return NULL;
  return drv->bdrv_aio_flush(bs, cb, opaque);
@@ -2196,6 +2206,24 @@ static BlockDriverAIOCB 
*bdrv_aio_flush_em(BlockDriverState *bs,
  return&acb->common;
  }

+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BlockDriverAIOCBSync *acb;
+
+acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque);
+acb->is_write = 1; /* don't bounce in the completion hadler */
+acb->qiov = NULL;
+acb->bounce = NULL;
+acb->ret = 0;
+
+if (!acb->bh)
+acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
+
+qemu_bh_schedule(acb->bh);
+return&acb->common;
+}
+
  /**/
  /* sync block device emulation */

diff --git a/block.h b/block.h
index f87d24e..8032b6b 100644
--- a/block.h
+++ b/block.h
@@ -33,6 +33,7 @@ typedef struct QEMUSnapshotInfo {
  #define BDRV_O_CACHE_WB0x0040 /* use write-back caching */
  #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread 
pool */
  #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
+#define BDRV_O_NO_FLUSH0x0200 /* disable flushing on this disk */

  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)

diff --git a/qemu-config.c b/qemu-config.c
index d500885..bf3d493 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -54,7 +54,7 @@ QemuOptsList qemu_drive_opts = {
  },{
  .name = "cache",
  .type = QEMU_OPT_STRING,
-.help = "host cache usage (none, writeback, writethrough)",
+.help = "host cache usage (none, writeback, writethrough, 
volatile)",
  },{
  .name = "aio",
  .type = QEMU_OPT_STRING,
diff --git a/qemu-options.hx b/qemu-options.hx
index 12f6b51..6dedb4a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,8 +118,9 @@ ETEXI
  DEF("drive", HAS_ARG, QEMU_OPTION_drive,
  "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
  "   [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-"   [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
-"   [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
+"   [,cache=writethrough|writeback|volatile|none][,format=f]\n"
+"   [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
+"   [,readonly=on|off]\n"
  "use 'file' as a drive image\n", QEMU_ARCH_ALL)
  STEXI
  @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
@@ -148,7 +149,7 @@ These options have the same definition as they have in 
@option{-hdachs}.
  @item snapsh...@var{snapshot}
  @var{snapshot} is "on" or "off" and allows to enable snapshot for given drive 
(see @option{-snapshot}).
  @item cac...@var{cache}
-...@var{cache} is "none", "writeback", or "writethrough" and controls how the 
host cache is used to access block data.
+...@var{

[Qemu-devel] Qemu-KVM 0.12.4 Guest entered Paused State

[Peter Lieven]

Hi,

i have a VM where I just did a dist-upgrade in Ubuntu and the VM entered
paused state without any obvious reason.

Is there a way to debug why this happened? I have not touched the VM by
now to leave the opportunity to debug.

Peter

[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Anthony Liguori]

On 05/17/2010 09:04 AM, Alexander Graf wrote:

Anthony Liguori wrote:
   

On 05/17/2010 08:17 AM, Alexander Graf wrote:
 

On 17.05.2010, at 15:09, Anthony Liguori wrote:


   

On 05/17/2010 08:02 AM, Alexander Graf wrote:

 

My concern is that ext3 exaggerates the cost of fsync() which will
result in diminishing value over time for this feature as people
move to ext4/btrfs.


 

There will be ext3 file systems for years out. Just because people
can use better and faster file systems doesn't mean they do. And
I'm sure they can't always choose. If anything, I can try and see
what the numbers look like for xfs.


   

But ext3 with barrier=1 is pretty uncommon in practice.  Another
data point would be an ext3 host file system with barrier=0.

 

Who defines what is common and what not? To me, the SLES11 default is
common. In fact, the numbers in the referred mail were done on an
11.1 system.

   

But it wasn't the SLES10 default so there's a smaller window of
systems that are going to be configured this way.  But this is
orthogonal to the main point.  Let's quantify how important this
detail is before we discuss the affected user base.
 

Alright. I took my Netbook (2GB of RAM) and a USB hard disk, so I can
easily remount the data fs the vmdk image is on. Here are the results:

# mkfs.ext3 /dev/sdc1
# mount /dev/sdc1 /mnt -obarrier=1

cache=writeback

real0m52.801s
user0m16.065s
sys 0m6.688s

cache=volatile

real0m47.876s
user0m15.921s
sys 0m6.548s

# mount /dev/sdc1 /mnt -obarrier=0

cache=writeback

real0m53.588s
user0m15.901s
sys 0m6.576s

cache=volatile

real0m48.715s
user0m16.581s
sys 0m5.856s

I don't see a difference between the results. Apparently the barrier
option doesn't change a thing.
   


Ok.  I don't like it, but I can see how it's compelling.  I'd like to 
see the documentation improved though.  I also think a warning printed 
on stdio about the safety of the option would be appropriate.


Regards,

Anthony Liguori



Alex

   

Re: [Qemu-devel] [PATCH 0/3] mingw32 compile fixes

[Blue Swirl]

On 5/17/10, Markus Armbruster  wrote:
> Blue Swirl  writes:
>
>  > On 5/16/10, Stefan Weil  wrote:
>  >> Am 15.05.2010 22:49, schrieb Blue Swirl:
>  >>
>  >>
>  >> > Hi,
>  >> >
>  >> > With this mingw32 compiler:
>  >> >
>  >> > $ i586-mingw32msvc-gcc -v
>  >> > Using built-in specs.
>  >> > Target: i586-mingw32msvc
>  >> > Configured with:
>
> [...]
>
> >> > build will not succeed because formats %zd, %zu, %hh, %lld, %llx and
>  >> > %llu are not known by the compiler.
>  >> >
>  >> > Any %ll* use is clearly a bug, we have PRI*64 macros just for this
>  >> purpose.
>  >> >
>  >> > For %hh and %z there may be better ways than these patches.
>  >> >
>  >> > With the patches I can build working Win32 binaries and there are no
>  >> warnings.
>
> [...]
>
> >>  It's a compiler bug that the compiler does not know these format strings.
>  >>  The code works nevertheless (at least with mingw libraries which are
>  >>  not too old) because the format strings are interpreted by the C runtime
>  >>  library.
>  >>
>  >>  Is it worth changing a lot of files when we can expect a newer mingw
>  >>  compiler version which works correctly for standard format strings?
>  >
>  > When and if that version becomes popular, PRIz* and the %hh hack could
>  > be removed or a compiler check could be added. But I don't think it's
>  > worth it, the macros are easy to use.
>
>
> They're also ugly as sin.

Avi's signature tells the reason, the macros are products of standards
committees.

But on second thought, perhaps it's not OK standard-wise to invent
PRIz* macros, at least they should be prefixed with Q to avoid
conflict. That probably does not improve the beauty of the macros. ;-)

I also noticed the SCN* macros now. We are incorrectly using PRI*
macros where SCN* should be used. Too bad this doesn't solve the %hh
problem, SCNx8 is not defined on mingw32.

Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Paul Brook]

> > I don't see a difference between the results. Apparently the barrier
> > option doesn't change a thing.
> 
> Ok.  I don't like it, but I can see how it's compelling.  I'd like to
> see the documentation improved though.  I also think a warning printed
> on stdio about the safety of the option would be appropriate.

I disagree with this last bit.

Errors should be issued if the user did something wrong.
Warnings should be issued if qemu did (or will soon do) something other than 
what the user requested, or otherwise made questionable decisions on the 
user's behalf.

In this case we're doing exactly what the user requested. The only plausible 
failure case is where a user is blindly trying options that they clearly don't 
understand or read the documentation for. I have zero sympathy for complaints 
like "Someone on the Internet told me to use --breakme, and broke thinks".

Paul

Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Anthony Liguori]

On 05/17/2010 11:23 AM, Paul Brook wrote:

I don't see a difference between the results. Apparently the barrier
option doesn't change a thing.
   

Ok.  I don't like it, but I can see how it's compelling.  I'd like to
see the documentation improved though.  I also think a warning printed
on stdio about the safety of the option would be appropriate.
 

I disagree with this last bit.

Errors should be issued if the user did something wrong.
Warnings should be issued if qemu did (or will soon do) something other than
what the user requested, or otherwise made questionable decisions on the
user's behalf.

In this case we're doing exactly what the user requested. The only plausible
failure case is where a user is blindly trying options that they clearly don't
understand or read the documentation for. I have zero sympathy for complaints
like "Someone on the Internet told me to use --breakme, and broke thinks".
   


I see it as the equivalent to the Taint bit in Linux.  I want to make it 
clear to users up front that if you use this option, and you have data 
loss issues, don't complain.


Just putting something in qemu-doc.texi is not enough IMHO.  Few people 
actually read it.


Regards,

Anthony Liguori


Paul

   

Re: [Qemu-devel] Qemu-KVM 0.12.4 Guest entered Paused State

[Anthony Liguori]

On 05/17/2010 10:16 AM, Peter Lieven wrote:

Hi,

i have a VM where I just did a dist-upgrade in Ubuntu and the VM entered
paused state without any obvious reason.

Is there a way to debug why this happened? I have not touched the VM by
now to leave the opportunity to debug.
   


You have run out of disk space on the host.

Regards,

Anthony Liguori


Peter


   

Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Alexander Graf]

On 17.05.2010, at 18:26, Anthony Liguori wrote:

> On 05/17/2010 11:23 AM, Paul Brook wrote:
 I don't see a difference between the results. Apparently the barrier
 option doesn't change a thing.
   
>>> Ok.  I don't like it, but I can see how it's compelling.  I'd like to
>>> see the documentation improved though.  I also think a warning printed
>>> on stdio about the safety of the option would be appropriate.
>>> 
>> I disagree with this last bit.
>> 
>> Errors should be issued if the user did something wrong.
>> Warnings should be issued if qemu did (or will soon do) something other than
>> what the user requested, or otherwise made questionable decisions on the
>> user's behalf.
>> 
>> In this case we're doing exactly what the user requested. The only plausible
>> failure case is where a user is blindly trying options that they clearly 
>> don't
>> understand or read the documentation for. I have zero sympathy for complaints
>> like "Someone on the Internet told me to use --breakme, and broke thinks".
>>   
> 
> I see it as the equivalent to the Taint bit in Linux.  I want to make it 
> clear to users up front that if you use this option, and you have data loss 
> issues, don't complain.
> 
> Just putting something in qemu-doc.texi is not enough IMHO.  Few people 
> actually read it.

But that's why it's no default and also called "volatile". If you prefer, we 
can call it cache=destroys_your_image.


Alex

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Anthony Liguori]

On 05/17/2010 10:09 AM, Julian Pidancet wrote:

On 05/17/2010 02:30 PM, Anthony Liguori wrote:
   

On 05/15/2010 08:10 PM, Paul Brook wrote:
 

The other solution would be to use the DirectFB driver for SDL which
would allow to do slightly the same as this patch. But that would mean
having to deal with an additional layer in the graphical stack, which is
not exactly what one wants from a performance or a complexity point of
view.

 

I don't buy your complexity argument.  Doesn't DirectFB-via-SDL already work?
If not why not? I'm pretty sure fixing that would be way simpler than adding a
whole new output backend.

   

Yeah, I don't buy it either.  I think performance data is probably the
only way to justify this and I'm sceptical that if there is a
performance advantage that it wouldn't be possible to just fix SDL's
DirectFB support.

 

I don't think wether fixing or not SDL is the debate here, the question would 
be more wether or not we want to add a lightweight display driver to qemu. 
Also, I think a DirectFB driver is fairly easy to maintain.
   


Generally speaking, adding DirectFB doesn't seem like a net win to me.  
We still have to maintain SDL so now there's just more code to 
maintain.  I'd rather there be one way of doing things that everybody 
focused on making work best than have two ways to do the same thing (if 
at all possible).


Regards,

Anthony Liguori


I will get some performance data as soon as I have some time.

By the way, sorry for the patch reposts, it seems that my smtp server had quite 
some trouble to relay messages last week.

   

[Qemu-devel] Re: [PATCH 0/2] pckbd improvements

[Blue Swirl]

On 5/17/10, Paolo Bonzini  wrote:
> > > > In 2/2, A20 logic changes a bit but I doubt any guest would be broken
>  > > > if A20 line written through I/O port 92 couldn't be read via i8042.
>  > > > The reverse (write using i8042 and read port 92) will work.
>  > > >
>  > >
>  > >  Why take the risk?
>  >
>  > The alternative is to route a signal from port 92 to i8042. Or maybe
>  > port 92 should belong to i8042, that could make things simpler but
>  > then the port would appear on non-PC architectures as well.
>  >
>  > But I doubt any OS would depend on such details, because the details
>  > seem to be murky:
>  > http://www.win.tue.nl/~aeb/linux/kbd/A20.html
>
>
> True, but I don't see why we should introduce a possible regression.
>  I would at the very least test it on real hardware before doing the
>  change.

I found one description on how i8042 and port 92 A20 lines work
together, chapter 6.11 in
http://www.smsc.com/media/Downloads_Public/Data_Sheets/47s45x.pdf

In that chip, the signals are OR'ed together. As I explained in my
reply to Jamie, QEMU does not implement this.

So I think the correct way is to move the port 92 to pckbd.c and maybe
add OR'ing (worth a separate patch).

Re: [Qemu-devel] [PATCH 0/3] mingw32 compile fixes

[Markus Armbruster]

Blue Swirl  writes:

> On 5/17/10, Markus Armbruster  wrote:
>> Blue Swirl  writes:
>>
>>  > On 5/16/10, Stefan Weil  wrote:
>>  >> Am 15.05.2010 22:49, schrieb Blue Swirl:
>>  >>
>>  >>
>>  >> > Hi,
>>  >> >
>>  >> > With this mingw32 compiler:
>>  >> >
>>  >> > $ i586-mingw32msvc-gcc -v
>>  >> > Using built-in specs.
>>  >> > Target: i586-mingw32msvc
>>  >> > Configured with:
>>
>> [...]
>>
>> >> > build will not succeed because formats %zd, %zu, %hh, %lld, %llx and
>>  >> > %llu are not known by the compiler.
>>  >> >
>>  >> > Any %ll* use is clearly a bug, we have PRI*64 macros just for this
>>  >> purpose.
>>  >> >
>>  >> > For %hh and %z there may be better ways than these patches.
>>  >> >
>>  >> > With the patches I can build working Win32 binaries and there are no
>>  >> warnings.
>>
>> [...]
>>
>> >>  It's a compiler bug that the compiler does not know these format strings.
>>  >>  The code works nevertheless (at least with mingw libraries which are
>>  >>  not too old) because the format strings are interpreted by the C runtime
>>  >>  library.
>>  >>
>>  >>  Is it worth changing a lot of files when we can expect a newer mingw
>>  >>  compiler version which works correctly for standard format strings?
>>  >
>>  > When and if that version becomes popular, PRIz* and the %hh hack could
>>  > be removed or a compiler check could be added. But I don't think it's
>>  > worth it, the macros are easy to use.
>>
>>
>> They're also ugly as sin.
>
> Avi's signature tells the reason, the macros are products of standards
> committees.

Committees have much ugliness to answer for, but they're not to blame
for ugly printing of size_t, signed char, unsigned char, long long and
unsigned long long.  In fact, the committee came up with a non-ugly
solution for them: conversion specification length modifiers z, hh, ll.

> But on second thought, perhaps it's not OK standard-wise to invent
> PRIz* macros, at least they should be prefixed with Q to avoid
> conflict.

Correct.

>   That probably does not improve the beauty of the macros. ;-)

And correct again.

All this ugliness just to silence a single compiler's broken warnings?
I'd switch off the warning and be done with it.

[...]

Re: [Qemu-devel] [PATCH 1/3] Fix %lld or %llx printf format use

[Markus Armbruster]

Blue Swirl  writes:

> Signed-off-by: Blue Swirl 
> ---
>  audio/audio_template.h  |2 +-
>  block/curl.c|9 ---
>  block/parallels.c   |7 -
>  block/qcow2.c   |8 --
>  darwin-user/commpage.c  |2 +-
>  darwin-user/syscall.c   |2 +-
>  hw/vga.c|2 +-
>  hw/vhost_net.c  |2 +-
>  ia64-dis.c  |9 +--
>  nbd.c   |4 +-
>  qemu-img.c  |   10 
>  qemu-io.c   |   57 
> ---
>  target-cris/translate.c |4 +-
>  target-ppc/translate.c  |7 +++--
>  target-sparc/helper.c   |2 +-
>  15 files changed, 69 insertions(+), 58 deletions(-)
>
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index 6b19848..2f5224b 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -541,7 +541,7 @@ uint64_t glue (AUD_get_elapsed_usec_, TYPE) (SW
> *sw, QEMUAudioTimeStamp *ts)
>
>  cur_ts = sw->hw->ts_helper;
>  old_ts = ts->old_ts;
> -/* dolog ("cur %lld old %lld\n", cur_ts, old_ts); */
> +/* dolog ("cur %" PRId64 " old %" PRId64 "\n", cur_ts, old_ts); */
>
>  if (cur_ts >= old_ts) {
>  delta = cur_ts - old_ts;
> diff --git a/block/curl.c b/block/curl.c
> index b944740..94b451c 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -118,7 +118,7 @@ static size_t curl_read_cb(void *ptr, size_t size,
> size_t nmemb, void *opaque)
>  size_t realsize = size * nmemb;
>  int i;
>
> -DPRINTF("CURL: Just reading %lld bytes\n", (unsigned long long)realsize);
> +DPRINTF("CURL: Just reading %" PRId64 " bytes\n", (uint64_t)realsize);
>
>  if (!s || !s->orig_buf)
>  goto read_end;

realsize is size_t.  Why don't you print it as such?  Same issue
elsewhere.

[...]

[Qemu-devel] [PATCH 0/2] pckbd improvements v2

[Blue Swirl]

This version should not change the i8042 vs. port 92 logic.

Blue Swirl (2):
  Compile pckbd only once
  pckbd: improve debugging

 Makefile.objs|1 +
 Makefile.target  |8 +-
 default-configs/i386-softmmu.mak |1 +
 default-configs/mips-softmmu.mak |1 +
 default-configs/mips64-softmmu.mak   |1 +
 default-configs/mips64el-softmmu.mak |1 +
 default-configs/mipsel-softmmu.mak   |1 +
 default-configs/ppc-softmmu.mak  |1 +
 default-configs/ppc64-softmmu.mak|1 +
 default-configs/ppcemb-softmmu.mak   |1 +
 default-configs/sparc64-softmmu.mak  |1 +
 default-configs/x86_64-softmmu.mak   |1 +
 hw/pc.c  |   33 +++---
 hw/pc.h  |5 +-
 hw/pckbd.c   |  112 ++
 hw/vmmouse.c |2 +-
 16 files changed, 102 insertions(+), 69 deletions(-)

[Qemu-devel] [PATCH 1/2] Compile pckbd only once

[Blue Swirl]

Use a qemu_irq to indicate A20 line changes. Move I/O port 92
to pckbd.c.

Signed-off-by: Blue Swirl 
---
 Makefile.objs|1 +
 Makefile.target  |8 ++--
 default-configs/i386-softmmu.mak |1 +
 default-configs/mips-softmmu.mak |1 +
 default-configs/mips64-softmmu.mak   |1 +
 default-configs/mips64el-softmmu.mak |1 +
 default-configs/mipsel-softmmu.mak   |1 +
 default-configs/ppc-softmmu.mak  |1 +
 default-configs/ppc64-softmmu.mak|1 +
 default-configs/ppcemb-softmmu.mak   |1 +
 default-configs/sparc64-softmmu.mak  |1 +
 default-configs/x86_64-softmmu.mak   |1 +
 hw/pc.c  |   33 -
 hw/pc.h  |5 +-
 hw/pckbd.c   |   86 +++---
 hw/vmmouse.c |2 +-
 16 files changed, 88 insertions(+), 57 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 110f8fd..814b9e1 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -151,6 +151,7 @@ hw-obj-$(CONFIG_SERIAL) += serial.o
 hw-obj-$(CONFIG_PARALLEL) += parallel.o
 hw-obj-$(CONFIG_I8254) += i8254.o
 hw-obj-$(CONFIG_PCSPK) += pcspk.o
+hw-obj-$(CONFIG_PCKBD) += pckbd.o
 hw-obj-$(CONFIG_USB_UHCI) += usb-uhci.o
 hw-obj-$(CONFIG_FDC) += fdc.o
 hw-obj-$(CONFIG_ACPI) += acpi.o
diff --git a/Makefile.target b/Makefile.target
index f2cd4ff..b1e4289 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -188,7 +188,7 @@ obj-y += rtl8139.o
 obj-y += e1000.o

 # Hardware support
-obj-i386-y = pckbd.o dma.o
+obj-i386-y = dma.o
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o i8259.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
@@ -201,7 +201,7 @@ obj-i386-y += pm_smbus.o apm.o acpi_piix4.o pc_piix.o
 obj-ppc-y = ppc.o
 obj-ppc-y += vga.o dma.o
 # PREP target
-obj-ppc-y += pckbd.o i8259.o mc146818rtc.o
+obj-ppc-y += i8259.o mc146818rtc.o
 obj-ppc-y += ppc_prep.o
 # OldWorld PowerMac
 obj-ppc-y += ppc_oldworld.o
@@ -219,7 +219,7 @@ obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o
mips_mipssim.o
 obj-mips-y += mips_addr.o mips_timer.o mips_int.o
 obj-mips-y += dma.o vga.o i8259.o
 obj-mips-y += g364fb.o jazz_led.o
-obj-mips-y += gt64xxx.o pckbd.o mc146818rtc.o
+obj-mips-y += gt64xxx.o mc146818rtc.o
 obj-mips-y += piix4.o cirrus_vga.o
 obj-mips-y += pm_smbus.o apm.o acpi_piix4.o

@@ -244,7 +244,7 @@ obj-cris-y += etraxfs_timer.o
 obj-cris-y += etraxfs_ser.o

 ifeq ($(TARGET_ARCH), sparc64)
-obj-sparc-y = sun4u.o pckbd.o apb_pci.o
+obj-sparc-y = sun4u.o apb_pci.o
 obj-sparc-y += vga.o
 obj-sparc-y += mc146818rtc.o
 obj-sparc-y += cirrus_vga.o
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 4c1495f..92ef535 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -8,6 +8,7 @@ CONFIG_SERIAL=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
+CONFIG_PCKBD=y
 CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
index 7793dbc..15015c3 100644
--- a/default-configs/mips-softmmu.mak
+++ b/default-configs/mips-softmmu.mak
@@ -10,6 +10,7 @@ CONFIG_SERIAL=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
+CONFIG_PCKBD=y
 CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
diff --git a/default-configs/mips64-softmmu.mak
b/default-configs/mips64-softmmu.mak
index aa65d92..982462a 100644
--- a/default-configs/mips64-softmmu.mak
+++ b/default-configs/mips64-softmmu.mak
@@ -10,6 +10,7 @@ CONFIG_SERIAL=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
+CONFIG_PCKBD=y
 CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
diff --git a/default-configs/mips64el-softmmu.mak
b/default-configs/mips64el-softmmu.mak
index b9b8c71..c23d186 100644
--- a/default-configs/mips64el-softmmu.mak
+++ b/default-configs/mips64el-softmmu.mak
@@ -10,6 +10,7 @@ CONFIG_SERIAL=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
+CONFIG_PCKBD=y
 CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
diff --git a/default-configs/mipsel-softmmu.mak
b/default-configs/mipsel-softmmu.mak
index e14831e..cd76389 100644
--- a/default-configs/mipsel-softmmu.mak
+++ b/default-configs/mipsel-softmmu.mak
@@ -10,6 +10,7 @@ CONFIG_SERIAL=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
+CONFIG_PCKBD=y
 CONFIG_USB_UHCI=y
 CONFIG_FDC=y
 CONFIG_ACPI=y
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 958bf75..03cc20f 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -8,6 +8,7 @@ CONFIG_M48T59=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
 CONFIG_I8254=y
+CONFIG_PCKBD=y
 CONFIG_FDC=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
diff --git a/default-configs/ppc64-softmmu.mak
b/default-configs/ppc64-softmmu.mak
index 9896662..8c77133 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -8,6 +8,7 @@ CONFIG_M48T59=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
 CONFIG_

Re: [Qemu-devel] [PULL] virtio, pci fixes

[Anthony Liguori]

On 05/12/2010 05:37 PM, Michael S. Tsirkin wrote:

The following changes since commit 54d7cf136f040713095cbc064f62d753bff6f9d2:

   doc: Clean up monitor command function index (2010-05-10 11:36:04 -0500)
   


Pulled.  Thanks.

Regards,

Anthony Liguori


are available in the git repository at:
   git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony

Alex Williamson (1):
   pci: cleanly backout of pci_qdev_init()

Michael S. Tsirkin (3):
   pci: irq_state vmstate breakage
   virtio: invoke set_features on load
   virtio-net: return with value in void function

  hw/pci.c|   20 +---
  hw/virtio-net.c |2 +-
  hw/virtio.c |2 ++
  3 files changed, 16 insertions(+), 8 deletions(-)


   

Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Anthony Liguori]

On 05/17/2010 06:19 AM, Markus Armbruster wrote:

Avi Kivity  writes:

   

On 05/17/2010 11:27 AM, Markus Armbruster wrote:
 


   

A slot is the hotpluggable entity.  Open your computer and you can
actually see them.


 

QEMU doesn't really know that.


   

How can that be?  Do we signal hotplug notifications to a function or
to a slot?

Can we hotplug a single function in an already occupied slot?

 

What I meant to say: we have no concept of "slot" in the higher level
interfaces, we have only bus and device.

If a PCI device has multiple functions, we have a separate qdev device
for each function.  You can't unplug a "slot" (concept doesn't exist in
qdev), only a qdev device.  Naturally, when you unplug a qdev device,
all functions in the same PCI slot need to go.  This happens deep down
in the bowels of ACPI, in piix4_device_hotplug().  qdev is not aware of
this magic relation between the qdev devices for functions in the same
slot.

   

IMO, that's a serious bug.  A slot is a user visible entity, both in
that devices can only be hotplugged only as slots, not functions, and
to determine the maximum number of devices you can add.  If the user
knows about it, qemu should too.

We can easily represent a slot/device as a qbus with each of the
functions as devices attached to it.
 

Dunno.  Gerd, what do you think?
   


Personally, I would think slots should be a property of the PCI bus (ala 
qdev).  It's something that you probably want to be configurable since 
it's not very common to see machines with 32 slots.


That said, since it affects the ACPI tables, it may be difficult to do 
in practice.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Anthony Liguori]

On 05/15/2010 01:19 AM, Avi Kivity wrote:

On 05/15/2010 01:54 AM, Luiz Capitulino wrote:

On Fri, 14 May 2010 19:03:36 +0200
Markus Armbruster  wrote:


What about PCI domains?

Good point.  Better to provide for them neatly now, instead of kludging
them in later.
  When I did this conversion I asked Micheal for help with that and 
he said

QEMU doesn't support PCI domains.


That's very different from "will never support pci domains".

The protocol must be forward looking, or we will need endless fixes 
for it.


But we can always add a domain property to extend the address (with a 
default domain of 0).


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 00/12] [PULL] qemu-kvm.git uq/master queue

[Anthony Liguori]

On 05/12/2010 04:24 PM, Marcelo Tosatti wrote:

The following changes since commit 54d7cf136f040713095cbc064f62d753bff6f9d2:
   Markus Armbruster (1):
 doc: Clean up monitor command function index
   


Pulled.  Thanks.

Regards,

Anthony Liguori


are available in the git repository at:

   git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Gleb Natapov (2):
   Do not stop VM if emulation failed in userspace.
   kvm: fix 8001.EDX supported bit filtering

Jan Kiszka (2):
   kvm: synchronize state from cpu context
   kvm: validate context for kvm cpu get/put operations

Marcelo Tosatti (8):
   Fix -mem-path with hugetlbfs
   kvm: set cpu_single_env around KVM_RUN ioctl
   make SIG_IPI to tcg vcpu thread reliable
   standardize on qemu_cpu_kick for signalling cpu thread(s)
   port qemu-kvm's on_vcpu code
   add cpu_is_stopped helper
   move stop/stopped CPU_COMMON fields after area zeroed by reset
   kvm: enable smp>  1

  cpu-all.h  |2 +
  cpu-defs.h |6 ++-
  cpu-exec.c |7 
  cpus.c |   88 ---
  exec-all.h |3 ++
  exec.c |8 +++-
  kvm-all.c  |   24 ++---
  kvm.h  |4 ++
  qemu-common.h  |8 +
  target-i386/kvm.c  |   29 -
  target-ppc/kvm.c   |   10 ++
  target-s390x/kvm.c |   10 ++
  12 files changed, 169 insertions(+), 30 deletions(-)


   

Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Avi Kivity]

On 05/17/2010 09:10 PM, Anthony Liguori wrote:


The protocol must be forward looking, or we will need endless fixes 
for it.



But we can always add a domain property to extend the address (with a 
default domain of 0).




That's what I meant by endless fixes.  Each one is reasonable by itself, 
but if you do enough of them, people go blech when they see the 
documentation, or get it wrong if they don't.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

[Qemu-devel] Re: [PULL 00/21] Block patches

[Anthony Liguori]

On 05/14/2010 12:10 PM, Kevin Wolf wrote:

The following changes since commit 14ac15d3ac8e0ef1c91204e2ac772b6412a6b99e:
   Anthony Liguori (1):
 Update SeaBIOS

are available in the git repository at:

   git://repo.or.cz/qemu/kevin.git block
   


Pulled, thanks.

Regards,

Anthony Liguori


Bruce Rogers (1):
   use qemu_free() instead of free()

Christoph Hellwig (9):
   cloop: use pread
   cloop: use qemu block API
   bochs: use pread
   bochs: use qemu block API
   parallels: use pread
   parallels: use qemu block API
   dmg: fix reading of uncompressed chunks
   dmg: use pread
   dmg: use qemu block API

Daniel P. Berrange (1):
   Fix docs for block stats monitor command

Kevin Wolf (5):
   ide: Fix ide_dma_cancel
   block: Avoid unchecked casts for AIOCBs
   block: Fix protocol detection for Windows devices
   block: Fix bdrv_commit
   block: Remove special case for vvfat

Ryota Ozaki (1):
   qemu-nbd: Improve error reporting

Stefan Hajnoczi (1):
   block: Remove semicolon in BDRV_SECTOR_MASK macro

Stefan Weil (3):
   block/vdi: Allow disk images of size 0
   block/vpc: Fix conversion from size to disk geometry
   block/vdi: Fix image opening and creation for odd disk sizes

  block.c   |   49 +++-
  block.h   |2 +-
  block/blkdebug.c  |4 +-
  block/bochs.c |   81 +++
  block/cloop.c |   48 
  block/dmg.c   |  109 +
  block/parallels.c |   51 +++--
  block/qcow.c  |2 +-
  block/qcow2.c |2 +-
  block/vdi.c   |   34 -
  block/vpc.c   |   21 ++
  hw/ide/core.c |8 ++--
  qemu-nbd.c|   34 -
  13 files changed, 213 insertions(+), 232 deletions(-)
   

Re: [Qemu-devel] [PATCH 00/22] tcg-i386 cleanup and improvement, v2

[Richard Henderson]

Ping?

On 04/28/2010 11:24 AM, Richard Henderson wrote:
> Changes v1->v2:
>   * Dropped controversial bswap changes; bswap16 continues to use rolw.
>   * Tidy data16 as the last of the hard-coded constants.
> 
> 
> r~
> 
> 
> Richard Henderson (22):
>   tcg-i386: Allocate call-saved registers first.
>   tcg-i386: Tidy initialization of tcg_target_call_clobber_regs.
>   tcg-i386: Tidy ext8u and ext16u operations.
>   tcg-i386: Tidy ext8s and ext16s operations.
>   tcg-i386: Tidy bswap operations.
>   tcg-i386: Tidy shift operations.
>   tcg-i386: Tidy move operations.
>   tcg-i386: Eliminate extra move from qemu_ld64.
>   tcg-i386: Tidy jumps.
>   tcg-i386: Tidy immediate arithmetic operations.
>   tcg-i386: Tidy non-immediate arithmetic operations.
>   tcg-i386: Tidy movi.
>   tcg-i386: Tidy push/pop.
>   tcg-i386: Tidy calls.
>   tcg-i386: Tidy ret.
>   tcg-i386: Tidy setcc.
>   tcg-i386: Tidy unary arithmetic.
>   tcg-i386: Tidy multiply.
>   tcg-i386: Tidy xchg.
>   tcg-i386: Tidy lea.
>   tcg-i386: Use lea for three-operand add.
>   tcg-i386: Tidy data16 prefixes.
> 
>  tcg/i386/tcg-target.c |  718 
> +
>  1 files changed, 433 insertions(+), 285 deletions(-)
> 
> 
> 

Re: [Qemu-devel] [PATCH] Fix --enable-user-pie compilation.

[Richard Henderson]

Ping?  This options really doesn't work atm...

r~

On 05/03/2010 03:54 PM, Richard Henderson wrote:
> We forgot to propagate -fpie to the libdis-user directory.
> 
> Signed-off-by: Richard Henderson 
> ---
>  configure |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/configure b/configure
> index 87942f4..dc11b65 100755
> --- a/configure
> +++ b/configure
> @@ -2344,6 +2344,9 @@ for d in libdis libdis-user; do
>  ln -s $source_path/Makefile.dis $d/Makefile
>  echo > $d/config.mak
>  done
> +if test "$static" = "no" -a "$user_pie" = "yes" ; then
> +  echo "QEMU_CFLAGS+=-fpie" > libdis-user/config.mak
> +fi
>  
>  for target in $target_list; do
>  target_dir="$target"

Re: [Qemu-devel] [PATCH 0/2] two tcg improvements

[Richard Henderson]

Ping?

Update: The TYPE parameter to tcg_out_mov would be helpful
for the S390 port as well.  The 32-bit LR is 2 bytes, while
the 64-bit LGR is 4 bytes.


r~


On 05/03/2010 04:30 PM, Richard Henderson wrote:
> The first patch allows the x86-64 port to avoid the REX.W prefix
> on moves, by allowing reg-reg moves to be typed just as reg-imm
> moves already are.  This does require trivial changes to each port.
> 
> The second patch fixes an oversight in commit
>   86feb1c860dc38e9c89e787c5210e8191800385e
> whereby I only modified the 32-bit host versions of the inline
> functions and not the 64-bit host versions.  This is visible on
> x86-64 host with arm guest in that we unnecessarily emit some
> MOVSLQ insns instead of plain 32-bit MOV insns.
> 
> 
> r~
> 
> 
> 
> Richard Henderson (2):
>   tcg: Add TYPE parameter to tcg_out_mov.
>   tcg: Use INDEX_op_qemu_ld32 for 32-bit results.
> 
>  tcg/arm/tcg-target.c|2 +-
>  tcg/hppa/tcg-target.c   |   38 ++--
>  tcg/i386/tcg-target.c   |   49 
> ---
>  tcg/ia64/tcg-target.c   |3 +-
>  tcg/mips/tcg-target.c   |   28 +-
>  tcg/ppc/tcg-target.c|   48 +++---
>  tcg/ppc64/tcg-target.c  |   10 
>  tcg/s390/tcg-target.c   |2 +-
>  tcg/sparc/tcg-target.c  |   10 
>  tcg/tcg-op.h|8 +++
>  tcg/tcg.c   |   12 +-
>  tcg/x86_64/tcg-target.c |   20 ++
>  12 files changed, 121 insertions(+), 109 deletions(-)
> 
> 
> 

[Qemu-devel] [PATCH 2/2] pckbd: improve debugging

[Blue Swirl]

Signed-off-by: Blue Swirl 
---
 hw/pckbd.c |   28 +++-
 1 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/pckbd.c b/hw/pckbd.c
index 5d268b4..3812284 100644
--- a/hw/pckbd.c
+++ b/hw/pckbd.c
@@ -29,6 +29,12 @@

 /* debug PC keyboard */
 //#define DEBUG_KBD
+#ifdef DEBUG_KBD
+#define DPRINTF(fmt, ...)   \
+do { printf("KBD: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...)
+#endif

 /* Keyboard Controller Commands */
 #define KBD_CCMD_READ_MODE 0x20/* Read mode bits */
@@ -191,9 +197,7 @@ static uint32_t kbd_read_status(void *opaque, uint32_t addr)
 KBDState *s = opaque;
 int val;
 val = s->status;
-#if defined(DEBUG_KBD)
-printf("kbd: read status=0x%02x\n", val);
-#endif
+DPRINTF("kbd: read status=0x%02x\n", val);
 return val;
 }

@@ -209,6 +213,7 @@ static void ioport92_write(void *opaque, uint32_t
addr, uint32_t val)
 {
 KBDState *s = opaque;

+DPRINTF("kbd: write outport=0x%02x\n", val);
 s->outport = val;
 if (s->a20_out) {
 qemu_set_irq(*s->a20_out, (val >> 1) & 1);
@@ -221,17 +226,18 @@ static void ioport92_write(void *opaque,
uint32_t addr, uint32_t val)
 static uint32_t ioport92_read(void *opaque, uint32_t addr)
 {
 KBDState *s = opaque;
+uint32_t ret;

-return s->outport;
+ret = s->outport;
+DPRINTF("kbd: read outport=0x%02x\n", ret);
+return ret;
 }

 static void kbd_write_command(void *opaque, uint32_t addr, uint32_t val)
 {
 KBDState *s = opaque;

-#ifdef DEBUG_KBD
-printf("kbd: write cmd=0x%02x\n", val);
-#endif
+DPRINTF("kbd: write cmd=0x%02x\n", val);
 switch(val) {
 case KBD_CCMD_READ_MODE:
 kbd_queue(s, s->mode, 0);
@@ -307,9 +313,7 @@ static uint32_t kbd_read_data(void *opaque, uint32_t addr)
 else
 val = ps2_read_data(s->kbd);

-#if defined(DEBUG_KBD)
-printf("kbd: read data=0x%02x\n", val);
-#endif
+DPRINTF("kbd: read data=0x%02x\n", val);
 return val;
 }

@@ -317,9 +321,7 @@ static void kbd_write_data(void *opaque, uint32_t
addr, uint32_t val)
 {
 KBDState *s = opaque;

-#ifdef DEBUG_KBD
-printf("kbd: write data=0x%02x\n", val);
-#endif
+DPRINTF("kbd: write data=0x%02x\n", val);

 switch(s->write_cmd) {
 case 0:
-- 
1.6.2.4

Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Gerd Hoffmann]

  Hi,


IMO, that's a serious bug. A slot is a user visible entity, both in
that devices can only be hotplugged only as slots, not functions, and
to determine the maximum number of devices you can add. If the user
knows about it, qemu should too.

We can easily represent a slot/device as a qbus with each of the
functions as devices attached to it.


Point being?


Dunno. Gerd, what do you think?


There is a PCIDevice for each function.  PCIDevice->devfn (aka addr 
property) contains slot+function.  So hw/pci.c can figure which device 
functions belong to the same slot.  The pci hotplug code might need some 
fixes to handle multi-function devices correctly though (I guess this is 
the original issue?).  unplug is probably easy, plug might be harder. 
You have to plug-in all functions belonging to the slot first, then 
signal the guest that the slot has been hotplugged, which might need 
changes in the monitor protocol.


cheers,
  Gerd

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Gerd Hoffmann]

On 05/17/10 14:04, Julian Pidancet wrote:

On 05/17/2010 11:53 AM, Gerd Hoffmann wrote:

+directfb="no"


Should be ="" (aka autodetect).






+if test "$directfb" = "yes" ; then
+  directfb_libs=`directfb-config --libs`
+  directfb_cflags=`directfb-config --cflags`
+  libs_softmmu="$directfb_libs $libs_softmmu"
+fi


use pkgconfig here.  directfb-config most likely is just a pkgconfig
wrapper anyway.



Unfortunately, directfb-config is not a pkgconfig wrapper, it is a standalone 
shell script.
Thus, I don't know if there is an easier way of "autodetecting" directfb than 
probing the directfb-config script.



'pkg-config directfb'
exit code 0 => success
exit code 1 => failure

pkg-config --cflags directfb
finds cflags

Check 'pkg-config --help' && man page for more info.

Oh, and btw: the patch doesn't build when applied to latest master bits.

HTH,
  Gerd

Re: [Qemu-devel] [PATCH 00/22] tcg-i386 cleanup and improvement, v2

[Aurelien Jarno]

Pong.

Sorry -ENOTIME for QEMU

On Mon, May 17, 2010 at 11:26:37AM -0700, Richard Henderson wrote:
> Ping?
> 
> On 04/28/2010 11:24 AM, Richard Henderson wrote:
> > Changes v1->v2:
> >   * Dropped controversial bswap changes; bswap16 continues to use rolw.
> >   * Tidy data16 as the last of the hard-coded constants.
> > 
> > 
> > r~
> > 
> > 
> > Richard Henderson (22):
> >   tcg-i386: Allocate call-saved registers first.
> >   tcg-i386: Tidy initialization of tcg_target_call_clobber_regs.
> >   tcg-i386: Tidy ext8u and ext16u operations.
> >   tcg-i386: Tidy ext8s and ext16s operations.
> >   tcg-i386: Tidy bswap operations.
> >   tcg-i386: Tidy shift operations.
> >   tcg-i386: Tidy move operations.
> >   tcg-i386: Eliminate extra move from qemu_ld64.
> >   tcg-i386: Tidy jumps.
> >   tcg-i386: Tidy immediate arithmetic operations.
> >   tcg-i386: Tidy non-immediate arithmetic operations.
> >   tcg-i386: Tidy movi.
> >   tcg-i386: Tidy push/pop.
> >   tcg-i386: Tidy calls.
> >   tcg-i386: Tidy ret.
> >   tcg-i386: Tidy setcc.
> >   tcg-i386: Tidy unary arithmetic.
> >   tcg-i386: Tidy multiply.
> >   tcg-i386: Tidy xchg.
> >   tcg-i386: Tidy lea.
> >   tcg-i386: Use lea for three-operand add.
> >   tcg-i386: Tidy data16 prefixes.
> > 
> >  tcg/i386/tcg-target.c |  718 
> > +
> >  1 files changed, 433 insertions(+), 285 deletions(-)
> > 
> > 
> > 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] A20 line control (was Re: [PATCH 0/2] pckbd improvements)

[Jamie Lokier]

Blue Swirl wrote:
> On 5/16/10, Jamie Lokier  wrote:
> > Blue Swirl wrote:
> >  > On 5/16/10, Paolo Bonzini  wrote:
> >  > > On 05/15/2010 11:49 AM, Blue Swirl wrote:
> >  > >
> >  > > > In 2/2, A20 logic changes a bit but I doubt any guest would be broken
> >  > > > if A20 line written through I/O port 92 couldn't be read via i8042.
> >  > > > The reverse (write using i8042 and read port 92) will work.
> >  > > >
> >  > >
> >  > >  Why take the risk?
> >  >
> >  > The alternative is to route a signal from port 92 to i8042. Or maybe
> >  > port 92 should belong to i8042, that could make things simpler but
> >  > then the port would appear on non-PC architectures as well.
> >  >
> >  > But I doubt any OS would depend on such details, because the details
> >  > seem to be murky:
> >  > http://www.win.tue.nl/~aeb/linux/kbd/A20.html
> >
> >
> > It's not hard to imagine some DOS memory driver or 286/386 DOS
> >  extender expecting to read the bit, if that's normal on PCs.
> >
> >  The earlier PCs didn't have port 92h, so presumably older DOS software
> >  uses the keyboard controller exclusively.
> >
> >  The details are murky, but on the other hand, I remember back in day,
> >  A20 line was common knowledge amongst DOS hackers on 286s and 386s,
> >  and the time I remember it from, port 92h was not available, so it
> >  can't have been too murky to use the i8042.
> 
> Right, but with this patch, writing to and reading from i8042 would
> still work, likewise for writing to and reading from port 92. Even
> writing via i8042, but reading via port 92 would work. What would not
> work reliably (even then, 50% probability of being correct) is when
> port 92 is written, but reading happens with i8042.
> 
> >  i8042 emulation isn't the same on PC on a non-PC because of the
> >  PC-specific wiring (outside the chip), such as its ability to reset
> >  the motherboard.  I don't see that it makes sense for qemu to pretend
> >  there are no differences at all.  Or, perhaps it makes sense to imply
> >  different GPIO wiring, separate from the i8042 itself.
> >
> >  On the other hand, something which makes sense to me:
> >
> >  In a PC, are port 92h and i8042's outputs OR'd together or AND'd
> >  together to control A20 proper?  Then they'd be two independent
> >  signals, and shouldn't mirror each other.
> 
> That's exactly what I meant, how could also random OS designer trust
> that the signals are combined the same way on every PC? With logic
> circuits, i8042 would still see its own output only, not the combined
> signal. If instead the signals were wired together, with some
> combination of inputs the output would not match what QEMU generates.
> Currently QEMU does not implement any logic for A20 line, which
> obviously can't match real hardware (or maybe some kind of dual port
> memory).

http://www.openwatcom.org/index.php/A20_Line

According to that page, MS-DOS's HIMEM.SYS tries 17 different methods
to control the A20 line! :-) Meanwhile, DOS/4GW, a DOS extender (there
are lots of those) allows the method to be set manually.

But there are only two common ones that are still implemented in
modern PC hardware: The keyboard commands to read, modify and write
the output port, and port 92h.

The random DOS-extender designers had to try each method, by checking
if the address space was actually wrapped.

With port 92h being known as the "fast A20 gate", I'm pretty sure any
program which includes that method will try that one first.

According to the wiki page (actually, my interpretation of it), the
output signal from port 92h is usually OR'd with the output signal
from the keyboard controller port.  That is, they are independent signals.

-- Jamie

Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive

[Jamie Lokier]

Alexander Graf wrote:
> 
> On 17.05.2010, at 18:26, Anthony Liguori wrote:
> 
> > On 05/17/2010 11:23 AM, Paul Brook wrote:
>  I don't see a difference between the results. Apparently the barrier
>  option doesn't change a thing.
>    
> >>> Ok.  I don't like it, but I can see how it's compelling.  I'd like to
> >>> see the documentation improved though.  I also think a warning printed
> >>> on stdio about the safety of the option would be appropriate.
> >>> 
> >> I disagree with this last bit.
> >> 
> >> Errors should be issued if the user did something wrong.
> >> Warnings should be issued if qemu did (or will soon do) something other 
> >> than
> >> what the user requested, or otherwise made questionable decisions on the
> >> user's behalf.
> >> 
> >> In this case we're doing exactly what the user requested. The only 
> >> plausible
> >> failure case is where a user is blindly trying options that they clearly 
> >> don't
> >> understand or read the documentation for. I have zero sympathy for 
> >> complaints
> >> like "Someone on the Internet told me to use --breakme, and broke thinks".
> >>   
> > 
> > I see it as the equivalent to the Taint bit in Linux.  I want to make it 
> > clear to users up front that if you use this option, and you have data loss 
> > issues, don't complain.
> > 
> > Just putting something in qemu-doc.texi is not enough IMHO.  Few people 
> > actually read it.
> 
> But that's why it's no default and also called "volatile". If you prefer, we 
> can call it cache=destroys_your_image.

With that semantic, a future iteration of cache=volatile could even
avoid writing to the backing file at all, if that's yet faster.  I
wonder if that would be faster.  Anyone fancy doing a hack with the
whole guest image as a big malloc inside qemu?  I don't have enough RAM :-)

-- Jamie

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Gerd Hoffmann]

On 05/16/10 03:10, Paul Brook wrote:

The other solution would be to use the DirectFB driver for SDL which
would allow to do slightly the same as this patch. But that would mean
having to deal with an additional layer in the graphical stack, which is
not exactly what one wants from a performance or a complexity point of
view.


I don't buy your complexity argument.  Doesn't DirectFB-via-SDL already work?


Run a guest.  Switch to another (host) virtual terminal.  Watch qemu 
continue drawing on the framebuffer.  Doesn't count as "working" in my book.



If not why not? I'm pretty sure fixing that would be way simpler than adding a
whole new output backend.


Didn't investigate where the bug is and how hard it would be to fix it.

cheers,
  Gerd

[Qemu-devel] Re: [PATCH 3/8] Add QBuffer

[Jamie Lokier]

Jan Kiszka wrote:
> Jamie Lokier wrote:
> > Anthony Liguori wrote:
> >> Instead of encoding just as a string, it would be a good idea to encode 
> >> it as something like:
> >>
> >> {'__class__': 'base64', 'data': ...}
> > 
> > Is there a benefit to the class indirection, over simply a keyword?:
> > 
> > {'__base64__': ...}
> > 
> > __class__ seems to suggest much more than it's being used for here.
> > 
> 
> Depending on how sophisticated your parser is, you could directly push
> the result into an object of the proper type. And we can add more
> complex objects in the future that do not only consists of a single data
> key. Note that this extension is not just about encoding, it is about
> typecasting (dict -> custom type).

Sure, if that's the plan.

Does it make sense to combine encoding and custom types in this way?
It looks like mixing syntax and semantics, which has consequences for
code using generic parsers with separate semantic layer, but I realise
there's no "correct" answer.

Back to the syntax: I'm under the impression from earlier discussion
that the '__*__' keyspace reserved, so even types could use the
compact syntax?

Or is there something Javascript-ish (and not merely JSON-ish) about
'__class__' in particular which makes it appropriate?

-- Jamie

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Anthony Liguori]

On 05/17/2010 03:20 PM, Gerd Hoffmann wrote:

On 05/16/10 03:10, Paul Brook wrote:

The other solution would be to use the DirectFB driver for SDL which
would allow to do slightly the same as this patch. But that would mean
having to deal with an additional layer in the graphical stack, 
which is

not exactly what one wants from a performance or a complexity point of
view.


I don't buy your complexity argument.  Doesn't DirectFB-via-SDL 
already work?


Run a guest.  Switch to another (host) virtual terminal.  Watch qemu 
continue drawing on the framebuffer.  Doesn't count as "working" in my 
book.


A common theme with SDL seems to be that it does lots of things poorly.  
I wonder if we'd be better suited just dropping SDL and focusing on 
doing a few things well.


The fact that we have cocoa support in the tree is basically an 
admission of failure with SDL.


Regards,

Anthony Liguori

If not why not? I'm pretty sure fixing that would be way simpler than 
adding a

whole new output backend.


Didn't investigate where the bug is and how hard it would be to fix it.

cheers,
  Gerd

[Qemu-devel] Re: [QEMU-KVM]: Megasas + TCM_Loop + SG_IO into Windows XP guests

[Nicholas A. Bellinger]

On Fri, 2010-05-14 at 02:42 -0700, Nicholas A. Bellinger wrote:
> On Fri, 2010-05-14 at 09:22 +0200, Hannes Reinecke wrote:
> > Nicholas A. Bellinger wrote:
> > > Greetings Hannes and co,
> > > 
> 
> > Let's see if I can find some time working on the megasas emulation.
> > Maybe I find something.
> > Last time I checked it was with a Windows7 build, but I didn't do
> > any real tests there. Basically just checking if the system boots up :-)
> > 
> 
> Nothing fancy just yet.  This is involving a normal NTFS filesystem
> format on a small TCM/FILEIO LUN using scsi-generic and a userspace
> FILEIO with scsi-disk.
> 
> This involves the XP guest waiting until the very last READ_10 once the
> format has completed (eg: all WRITE and VERIFY CDBs complete with GOOD
> status AFAICT) before announcing that mkfs.ntfs failed without any
> helpful exception message (due to missing metadata of some sort I would
> assume..?)
> 
> So perhaps dumping QEMU and TCM_Loop SCSI payloads to determine if any
> correct blocks from megasas_handle_io() are actually making it out to
> KVM host is going to be my next option.  ;)
> 

Greetings Hannes,

So I spent some more time with XP guests this weekend, and I noticed two
things immediately when using hw/lsi53c895a.c instead of hw/megasas.c
with the same two TCM_Loop SAS LUNs via SG_IO from last week:

1) With lsi53c895a, XP guests are able to boot successfully w/ out the
synchronous SG_IO hack that is currently required to get past the first
36-byte INQUIRY for megasas + XP SP2

2) With lsi53c895a, XP is able to successfully create and mount a NTFS
filesystem, reboot, and read blocks appear to be functioning properly.
FYI I have not run any 'write known pattern then read-back and compare
blocks' data integrity tests from with in the XP guests just yet, but I
am confident that TCM scatterlist -> se_mem_t mapping is working as
expected on the KVM Host.

Futhermore, after formatting a 5 GB TCM/FILEIO LUN with lsi53c895a, and
then rebooting with megasas with the same two configured TCM_Loop SG_IO
devices, it appears to be able to mount and read blocks successfully.
Attempting to write new blocks on the mounted filesystem also appears to
work to some degree, but throughput slows down to a crawl during XP
guest buffer cache flush, which is likely attributed to the use of my
quick SYNC SG_IO hack.

So it appears that there are two seperate issues here, and AFAICT they
both look to be XP and megasas specific.  For #2, it may be something
about the format of the incoming scatterlists generated during XP's
mkfs.ntfs that is causing some issues.  While watching output during fs
creation, I noticed the following WRITE_10s with a starting 4088 byte
scatterlist and a trailing 8 byte scatterlist:

megasas: writel mmio 40: 2b0b003
megasas: Found mapped frame 2 context 82b0b000 pa 2b0b000
megasas: Enqueue frame context 82b0b000 tail 493 busy 1
megasas: LD SCSI dev 2 lun 0 sdev 0xdc0230 xfer 16384
scsi-generic: Using cur_addr: 0x0ff6c008 cur_len: 0x0ff8
scsi-generic: Adding iovec for mem: 0x7f1783b96008 len: 0x0ff8
scsi-generic: Using cur_addr: 0x0fd6e000 cur_len: 0x1000
scsi-generic: Adding iovec for mem: 0x7f1783998000 len: 0x1000
scsi-generic: Using cur_addr: 0x0fe2f000 cur_len: 0x1000
scsi-generic: Adding iovec for mem: 0x7f1783a59000 len: 0x1000
scsi-generic: Using cur_addr: 0x0fdf cur_len: 0x1000
scsi-generic: Adding iovec for mem: 0x7f1783a1a000 len: 0x1000
scsi-generic: Using cur_addr: 0x0fded000 cur_len: 0x0008
scsi-generic: Adding iovec for mem: 0x7f1783a17000 len: 0x0008
scsi-generic: execute IOV: iovec_count: 5, dxferp: 0xd92420, dxfer_len: 16384
scsi-generic: ---> Issuing SG_IO CDB len 10: 0x2a 00 00 00 
fa be 00 00 20 00 
scsi-generic: scsi_write_complete() ret = 0
scsi-generic: Command complete 0x0xd922c0 tag=0x82b0b000 status=0
megasas: LD SCSI req 0xd922c0 cmd 0xda92c0 lun 0xdc0230 finished with status 0 
len 16384
megasas: Complete frame context 82b0b000 tail 493 busy 0 doorbell 0

Also, the final READ_10 that produces the 'could not create filesystem'
exception is for LBA 63 and XP looking for the first FS blocks after
GPT.

Could there be some breakage in megasas with a length < PAGE_SIZE for
the scatterlist..?As lsi53c895a seems to work OK for this case, is
there something about the logic of parsing the incoming struct
scatterlists that is different between the two HBA drivers..?  AFAICT
both are using Gerd's common code in hw/scsi-bus.c, unless there is
something about megasas_map_sgl() that is causing issues with the
above..?

Best,

--nab

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[malc]

On Mon, 17 May 2010, Anthony Liguori wrote:

> On 05/17/2010 03:20 PM, Gerd Hoffmann wrote:
> > On 05/16/10 03:10, Paul Brook wrote:
> > > > The other solution would be to use the DirectFB driver for SDL which
> > > > would allow to do slightly the same as this patch. But that would mean
> > > > having to deal with an additional layer in the graphical stack, which is
> > > > not exactly what one wants from a performance or a complexity point of
> > > > view.
> > > 
> > > I don't buy your complexity argument.  Doesn't DirectFB-via-SDL already
> > > work?
> > 
> > Run a guest.  Switch to another (host) virtual terminal.  Watch qemu
> > continue drawing on the framebuffer.  Doesn't count as "working" in my book.
> 
> A common theme with SDL seems to be that it does lots of things poorly.  I
> wonder if we'd be better suited just dropping SDL and focusing on doing a few
> things well.

There's one thing that SDL does marvelously well - it's just one fairly
small and self contained library that doesn't unleash dependency hell on
the user.

> The fact that we have cocoa support in the tree is basically an admission of
> failure with SDL.

I don't think so, the way i see it: someone had an itch (i.e. an
application that does not integrate well with his windowing environment)
and he scratched it.

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

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Anthony Liguori]

On 05/17/2010 04:35 PM, malc wrote:

There's one thing that SDL does marvelously well - it's just one fairly
small and self contained library that doesn't unleash dependency hell on
the user.
   

The fact that we have cocoa support in the tree is basically an admission of
failure with SDL.
 

I don't think so, the way i see it: someone had an itch (i.e. an
application that does not integrate well with his windowing environment)
and he scratched it.
   


SDL doesn't integrate well into a modern Gnome desktop either.  I don't 
see why we have Cocoa and not Gtk.  If the answer is, someone needs to 
send patches, expect patches soon :-)


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[malc]

On Mon, 17 May 2010, Anthony Liguori wrote:

> On 05/17/2010 04:35 PM, malc wrote:
> > There's one thing that SDL does marvelously well - it's just one fairly
> > small and self contained library that doesn't unleash dependency hell on
> > the user.
> >
> > > The fact that we have cocoa support in the tree is basically an admission
> > > of
> > > failure with SDL.
> > >  
> > I don't think so, the way i see it: someone had an itch (i.e. an
> > application that does not integrate well with his windowing environment)
> > and he scratched it.
> >
> 
> SDL doesn't integrate well into a modern Gnome desktop either.  I don't see
> why we have Cocoa and not Gtk.  If the answer is, someone needs to send
> patches, expect patches soon :-)
> 

If those patches don't try to force Gnome on me (by removing SDL that is
and being optional) let them come.

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

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Alexander Graf]

On 17.05.2010, at 23:45, malc wrote:

> On Mon, 17 May 2010, Anthony Liguori wrote:
> 
>> On 05/17/2010 04:35 PM, malc wrote:
>>> There's one thing that SDL does marvelously well - it's just one fairly
>>> small and self contained library that doesn't unleash dependency hell on
>>> the user.
>>> 
 The fact that we have cocoa support in the tree is basically an admission
 of
 failure with SDL.
 
>>> I don't think so, the way i see it: someone had an itch (i.e. an
>>> application that does not integrate well with his windowing environment)
>>> and he scratched it.
>>> 
>> 
>> SDL doesn't integrate well into a modern Gnome desktop either.  I don't see
>> why we have Cocoa and not Gtk.  If the answer is, someone needs to send
>> patches, expect patches soon :-)
>> 
> 
> If those patches don't try to force Gnome on me (by removing SDL that is
> and being optional) let them come.

I'm trying to think of a project where the clean separation between multiple 
video outputs implemented in the backend and a separate frontend worked out. So 
far the only case that has a strikingly similar architecture coming to my mind 
is mplayer. And I wouldn't call mplayer's GUI story a huge success.

In fact, couldn't we rather keep all graphic output out of qemu and just expose 
VNC, possibly with self-made additions to the protocol to speed up local 
rendering (thinking an SHM extension here)? Then we could still offer a 
separate SDL based viewer that could do the same things it does now. But we'd 
also open up the gate for a whole new integration level with possible GUIs.


Alex

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[malc]

On Tue, 18 May 2010, Alexander Graf wrote:

> 
> On 17.05.2010, at 23:45, malc wrote:
> 
> > On Mon, 17 May 2010, Anthony Liguori wrote:
> > 
> >> On 05/17/2010 04:35 PM, malc wrote:
> >>> There's one thing that SDL does marvelously well - it's just one fairly
> >>> small and self contained library that doesn't unleash dependency hell on
> >>> the user.
> >>> 
>  The fact that we have cocoa support in the tree is basically an admission
>  of
>  failure with SDL.
>  
> >>> I don't think so, the way i see it: someone had an itch (i.e. an
> >>> application that does not integrate well with his windowing environment)
> >>> and he scratched it.
> >>> 
> >> 
> >> SDL doesn't integrate well into a modern Gnome desktop either.  I don't see
> >> why we have Cocoa and not Gtk.  If the answer is, someone needs to send
> >> patches, expect patches soon :-)
> >> 
> > 
> > If those patches don't try to force Gnome on me (by removing SDL that is
> > and being optional) let them come.
> 
> I'm trying to think of a project where the clean separation between
> multiple video outputs implemented in the backend and a separate
> frontend worked out. So far the only case that has a strikingly
> similar architecture coming to my mind is mplayer. And I wouldn't
> call mplayer's GUI story a huge success.

Xine, VLC do have something resembling this separation too. As for
mplayer's GUI, never used it, what i did (and still) use is my own
video output device for mplayer, which is a lot faster than Xv, or
anything else for that matter, on my hardware.

> 
> In fact, couldn't we rather keep all graphic output out of qemu and
> just expose VNC, possibly with self-made additions to the protocol
> to speed up local rendering (thinking an SHM extension here)? Then
> we could still offer a separate SDL based viewer that could do the
> same things it does now. But we'd also open up the gate for a whole
> new integration level with possible GUIs.
> 

This idea is not new, nothing has come out of it till this day, so the
answer to your question (couldn't we...) is probably: no, we couldn't.

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

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Anthony Liguori]

On 05/17/2010 05:26 PM, Alexander Graf wrote:

I'm trying to think of a project where the clean separation between multiple 
video outputs implemented in the backend and a separate frontend worked out. So 
far the only case that has a strikingly similar architecture coming to my mind 
is mplayer. And I wouldn't call mplayer's GUI story a huge success.

In fact, couldn't we rather keep all graphic output out of qemu and just expose 
VNC, possibly with self-made additions to the protocol to speed up local 
rendering (thinking an SHM extension here)?


I think the whole reason this has failed is that if the GUI is a 
separate project, the path of least resistance is to use existing 
interfaces instead of inventing new ones.  That means these GUIs tend to 
be restricted by whatever management interface exists which isn't 
actually good enough.


You really need to have the GUI as part of the main project such that 
when it needs a new interface, it can be added very easily.



  Then we could still offer a separate SDL based viewer that could do the same 
things it does now. But we'd also open up the gate for a whole new integration 
level with possible GUIs.
   


You could, but I think it introduces more complexity which just is going 
to get in the way of building a good GUI.


Regards,

Anthony Liguori


Alex

   

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Anthony Liguori]

On 05/17/2010 05:42 PM, malc wrote:

In fact, couldn't we rather keep all graphic output out of qemu and
just expose VNC, possibly with self-made additions to the protocol
to speed up local rendering (thinking an SHM extension here)? Then
we could still offer a separate SDL based viewer that could do the
same things it does now. But we'd also open up the gate for a whole
new integration level with possible GUIs.

 

This idea is not new, nothing has come out of it till this day, so the
answer to your question (couldn't we...) is probably: no, we couldn't.
   


Because shared memory is just a graphics optimization and for most 
users, the difference between gtk-vnc and native SDL isn't noticable.  
So if a shared memory transport was the key missing piece, we'd have an 
awesome GUI based on gtk-vnc that just had slower graphics.


IMHO, the problem with an external GUI is that the interaction just gets 
too complicated.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Alexander Graf]

On 18.05.2010, at 00:46, Anthony Liguori wrote:

> On 05/17/2010 05:26 PM, Alexander Graf wrote:
>> I'm trying to think of a project where the clean separation between multiple 
>> video outputs implemented in the backend and a separate frontend worked out. 
>> So far the only case that has a strikingly similar architecture coming to my 
>> mind is mplayer. And I wouldn't call mplayer's GUI story a huge success.
>> 
>> In fact, couldn't we rather keep all graphic output out of qemu and just 
>> expose VNC, possibly with self-made additions to the protocol to speed up 
>> local rendering (thinking an SHM extension here)?
> 
> I think the whole reason this has failed is that if the GUI is a separate 
> project, the path of least resistance is to use existing interfaces instead 
> of inventing new ones.  That means these GUIs tend to be restricted by 
> whatever management interface exists which isn't actually good enough.
> 
> You really need to have the GUI as part of the main project such that when it 
> needs a new interface, it can be added very easily.

I agree on that part.

> 
>>  Then we could still offer a separate SDL based viewer that could do the 
>> same things it does now. But we'd also open up the gate for a whole new 
>> integration level with possible GUIs.
>>   
> 
> You could, but I think it introduces more complexity which just is going to 
> get in the way of building a good GUI.

The main benefit I see by taking an always-vnc approach would be that 
everything becomes 100% networkable. There's nothing getting in your way 
because you're doing things on a remote machine. And you even get the same 
look&feel you got from the local connection because it's the same tool 
connecting you.

But yeah, maybe it does add too much complexity. I don't know.


Alex

Re: [Qemu-devel] [PATCH] Add QEMU DirectFB display driver

[Anthony Liguori]

On 05/17/2010 05:49 PM, Alexander Graf wrote:

  Then we could still offer a separate SDL based viewer that could do the same 
things it does now. But we'd also open up the gate for a whole new integration 
level with possible GUIs.

   

You could, but I think it introduces more complexity which just is going to get 
in the way of building a good GUI.
 

The main benefit I see by taking an always-vnc approach would be that everything 
becomes 100% networkable. There's nothing getting in your way because you're doing 
things on a remote machine. And you even get the same look&feel you got from 
the local connection because it's the same tool connecting you.
   


My current thinking with GUIs is that network transparency is more 
trouble than it's worth.


The problem is, simple things end up being overly complicated.  You 
can't just pop up a file dialog box to select a new CD-ROM ISO because 
you can't browse files on a remote machine.  Instead, you need to have 
something like a "pool" concept or something like that.


Regards,

Anthony Liguori


But yeah, maybe it does add too much complexity. I don't know.


Alex