Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

[Isaku Yamahata]

On Thu, Aug 26, 2010 at 08:02:38AM -0500, Anthony Liguori wrote:
> BTW, if you could transfer some of this discussion to a wiki page on  
> qemu.org, I think that would be extremely valuable.

http://wiki.qemu.org/Features/ResetAPI


-- 
yamahata

Re: [Qemu-devel] Template for developing a Qemu device with?PCIe?and MSI-X

[Isaku Yamahata]

On Thu, Aug 26, 2010 at 01:17:38PM -0500, Adnan Khaleel wrote:
> You also want to catch up pci api clean up.
> pci_{set, get}_{byte, word, long, quad}(),
> pci_config_set_vendor() ...
> 
> Are you referring to the setting up of the config registers where we pass on
> the vendor id and device id etc? Would you elaborate a little more.

Yes. Now there are helper functions to address pci endian.
So there is no need to ugly bit operation in  pcie_msix_initfn().


> Also, I've got a bunch of questions but let me state my assumptions first so
> that you have a better idea of what I'm referring to.
> - The device template is a pcie endpoint
> - I want to be able to setup 64bit bar addresses, with large apertures

Use PCI_BASE_ADDRESS_MEM_TYPE_64.


> - For now, I'd like to be able to do basic MMIO and regular IO reads and
> writes.
> 
> 
> PCIe questions.
> 1. What does the topology of the bridge with respect to the root look like? Is
> it
> 
> Root <---> PCIe Bridge

lspci -t would help.
Roughly the bus topology looks like

  root port - upstream port --- downstream port -> end node device
||
... ...


> 2. If so, where is the slot where I can insert the PCIe device? Is it off the
> Bridge or would it be better for it to be off the root?
> 
> Root <---> PCIe Bridge <---> PCIe/MSI-X device
> 
> Or
> 
> Root <---> PCIe Bridge
>  <---> PCIe/MSI-X Device

On downstream port switch. See above.


> And hence my confusion about how to do the following:
> static void pcie_msix_register(void)
> {  
> pci_bridge_qdev_register(&pcie_msix_info);  // Is this what I should be
> doing?
> OR
> pci_qdev_register(&pcie_msix_info); // Or this
> }

pci_qdev_register()


> 3. I wasn't sure how to register the device how to do the initializing. Please
> see the following section of code:
> 
> void pcie_msix_init(PCIBus *bus)
> {
> // Is this how we should be doing this?
> pci_create_simple(bus, -1, "pcie_msix_device");
> OR
> pci_bridge_create(...);
> }
> Or if should I use pci_bridge_create(...) in place of the pci_create_simple
> (...)

pci_create_simple()


> Also, this confusion led me to being unsure what the following device struct
> should look like
> 
> typedef struct PCIE_MSIX_DEVState_St {
> PCIDevice dev;
> int mmio_index;
> } PCIE_MSIX_DEVState;
> 
> For the simple device function that I've described above, what is the purpose
> of this struct? What other data should be captured?
> 
> Which include the initializing of the following static structs. Btw, can you
> tell me what VMStateDescrption is used for by Qemu? Also, what should the
> "fields" member contain? I couldn't quite make out.
> 
> static const VMStateDescription vmstate_pcie_msix = {
> .name = "pcie-msix-device",
> .version_id = 1,
> .minimum_version_id = 1,
> .minimum_version_id_old = 1,
> .fields = (VMStateField[]) {
> VMSTATE_PCIE_DEVICE(dev, PCIE_MSIX_DEVState),
> VMSTATE_STRUCT(dev.aer_log, PCIE_MSIX_DEVState, 0,
> vmstate_pcie_aer_log, struct pcie_aer_log),
> VMSTATE_END_OF_LIST()
> }
> };
>
> 4. What is the qdev.props field used for?
> static PCIDeviceInfo pcie_msix_info = {
> .qdev.name = PCIE_MSIX_DEVICE,
> .qdev.desc = "PCIE MSIX device template",
> .qdev.size = sizeof(PCIE_MSIX_DEVState),
> .qdev.reset = pcie_msix_reset,
> .qdev.vmsd = &vmstate_pcie_msix,
> .is_express = 1,
> .config_write = pcie_msix_write_config,
> .init = pcie_msix_initfn,
> .exit = pcie_msix_exitfn,
> .qdev.props = (Property[]) {   
> DEFINE_PROP_END_OF_LIST(),
> }
> };

Perhaps at first it would be better to get a idea of what qdev device model is.
The following slide would be a good start point.
http://www.linux-kvm.org/wiki/images/f/fe/2010-forum-armbru-qdev.pdf


> 5. Device instantiation
> I init the device in pc_q35_bridge_init() in pc_q35.c
> 
> pcie_msix_init(root_port_bus);
> 
> I know I'm doing this incorrectly since I'm not specifying several things.
> Again, is this the correct place to init the device?

Yes if you really wanted the end node device to be on root port.
Otherwise downstream port is the place on which the end node device is.


> MSI/MSIX questions
> 1. How is an interrupt notification passed on to Qemu? In the regular case I'd
> use qemu_set_irq(..) to do so but what is the correct way of doing it in the
> MSIX paradigm? For example in the case of a DMA transfer.

Use msix_notify()/msi_notify().
virtio_pci_notify() in virtio-pci.c is an example.
pcie_notify() in pcie.c is also another example.
-- 
yamahata

Re: [Qemu-devel] [PATCH 3/5] HACKING: add memory management rules

[Kevin Wolf]

Am 26.08.2010 20:38, schrieb Blue Swirl:
> Add memory management rules, somewhat like libvirt HACKING.
> 
> Signed-off-by: Blue Swirl 
> ---
>  HACKING |   11 +++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/HACKING b/HACKING
> index 19fc874..554009e 100644
> --- a/HACKING
> +++ b/HACKING
> @@ -72,3 +72,14 @@ Typedefs are used to eliminate the redundant
> 'struct' keyword.
>  2.4. Reserved namespaces in C
>  Underscore capital, double underscore, and underscore 't' suffixes should be
>  avoided.
> +
> +3. Low level memory management
> +
> +Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
> +APIs is not allowed in the QEMU codebase. Instead of these routines,
> +use the replacement qemu_malloc/qemu_mallocz/qemu_realloc/qemu_free or
> +qemu_vmalloc/qemu_memalign/qemu_vfree APIs.
> +
> +Memory allocated by qemu_vmalloc or qemu_memalign must be freed with
> +qemu_vfree, since breaking this will cause problems on Win32 and user
> +emulators.

Maybe add that a NULL check for the qemu_malloc result is redundant.

And a warning about the stupid qemu_malloc(0) behaviour would be
appropriate, too. Because if you don't know it, you'll write buggy code.

Kevin

qemu-devel@nongnu.org

[Markus Armbruster]

"Edgar E. Iglesias"  writes:

> On Sat, Aug 21, 2010 at 09:42:51AM +, Blue Swirl wrote:
>> Combining bitwise AND and logical NOT is suspicious.
>> 
>> Fixed by this Coccinelle script:
>> // From http://article.gmane.org/gmane.linux.kernel/646367
>> @@ expression E1,E2; @@
>> (
>>   !E1 & !E2
>> |
>> - !E1 & E2
>> + !(E1 & E2)
>> )
>> 
>> Signed-off-by: Blue Swirl 
>> ---
>> 
>> Maybe the middle hunk should be fixed this way instead:
>> -} else if ((rw == 1) & !matching->d) {
>> +} else if ((rw == 1) && !matching->d) {
>> 
>> ---
>>  hw/etraxfs_eth.c|2 +-
>>  target-sh4/helper.c |4 ++--
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/hw/etraxfs_eth.c b/hw/etraxfs_eth.c
>> index b897c9c..ade96f1 100644
>> --- a/hw/etraxfs_eth.c
>> +++ b/hw/etraxfs_eth.c
>> @@ -464,7 +464,7 @@ static int eth_match_groupaddr(struct fs_eth *eth,
>> const unsigned char *sa)
>> 
>>  /* First bit on the wire of a MAC address signals multicast or
>> physical address.  */
>> -if (!m_individual && !sa[0] & 1)
>> +if (!m_individual && !(sa[0] & 1))
>>  return 0;
>
>
> Yep, the etrax part is a bug and your patch looks OK to me.
>
> Thanks
>
> Acked-by: Edgar E. Iglesias 

Such review is exactly what we need when static analysis spots fishy
code.  Thanks!

Volunteers to look at the other two hunks?

Re: [Qemu-devel] [PATCH 4/5] Add generic drive hotplugging

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Tue, Aug 24, 2010 at 03:46:14PM +0200, Alexander Graf wrote:
>> Daniel P. Berrange wrote:
>> > On Tue, Aug 24, 2010 at 03:40:25PM +0200, Alexander Graf wrote:
>> >   
>> >> Daniel P. Berrange wrote:
>> >> 
>> >>> On Tue, Aug 24, 2010 at 12:45:19PM +0200, Alexander Graf wrote:
>> >>>   
>> > The key is that you should use  if=none for all cases. Here are two
>> > examples of how libvirt does it currently:
>> >
>> > VirtIO:
>> >
>> >   drive_add dummy 
>> > file=/var/lib/libvirt/images/data.img,if=none,id=drive-virtio-disk1,format=raw
>> >   device_add 
>> > virtio-blk-pci,bus=pci.0,addr=0x0,drive=drive-virtio-disk1,id=virtio-disk1'
>> >
>> > SCSI:
>> >
>> >   drive_add dummy 
>> > file=/var/lib/libvirt/images/data.img,if=none,id=drive-scsi0-0-1,format=raw'
>> >   device_add 
>> > scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1
>> >
>> > The 'dummy' value there can be absolutely anything you want.
>> > It is totaly ignored when QEMU sees if=none in 2nd arg.
>> >   
>> >   
>> >   
>>  I'd be all for removing the pci-hotplug.c version of drive_add then. But
>>  I think the IF_SCSI option there is to append a drive to an existing
>>  SCSI bus, no?
>>  
>>  
>> >>> Actually this SCSI example I give above is appending a drive to an 
>> >>> existing
>> >>> bus (scsi0), in slot 1 (scsi-id=1).  To best of my knowledge there is no
>> >>> remaining use case that requires use of IF_SCSI, IF_IDE, etc. The IF_NONE
>> >>> approach can cope with all, modulo bugs that appear periodically with 
>> >>> code
>> >>> that mistakenly checks for a particular IF_XXX constant.
>> >>>
>> >>> If you wanted to also create a new SCSI bus, before creating the drive on
>> >>> it, you'd need to run three commands in total:
>> >>>
>> >>>   device_add lsi,id=scsi0,bus=pci.0,addr=0x7
>> >>>   drive_add dummy 
>> >>> file=/var/lib/libvirt/images/data.img,if=none,id=drive-scsi0-0-1,format=raw
>> >>>   device_add 
>> >>> scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1
>> >>>   
>> >>>   
>> >> Nice - so we can just deprecate if=!none?
>> >> 
>> >
>> > In theory yes, but its not nice to tell users to switch everything over to
>> > use if=none, if we're going to deprecate that too in the next release when
>> > blockdev appears. Might as well just deprecate entire of drive_add/-drive
>> > at once.
>> >   
>> 
>> I guess I still fail to see the reason for blockdev when we force
>> drive_add to if=none...
>
> Markus can no doubt explain better than me, but off the top of my head
>  
>  - 'drive' has guest properties that should be against the device
>not the drive which is for host properties (eg serial=, if=)
>  - 'file' is mangled to include protocol/format which means that 
>it can't be unambigously parsed. (eg filenames containing :)
>
> Fixing those, particularly the latter, would breaks back-compat so we
> really need a new arg with sensible definition. This is what blockdev
> is intended todo (as well as a internal code cleanup)

Yes, -drive and drive_add are a tangled mess.  I decided to start with a
clean slate, because getting it right is difficult enough without the
historical baggage.

Perhaps the end result can be hammered into the drive_add mold somehow,
by adding a few new options and deprecating a few old ones.  Let's
decide when we get there.

The goal is clean separation of host and guest properties.  device_add
is for guest properties, blockdev_add is for host properties.  Just like
netdev_add, only for block instead of network.

We also want sufficient flexibility (or at least extensibility) to be
able to specify any useful host block driver configuration.  And all
that without the syntactic embarrassments that plague drive_add, such as
':' in filenames.

[Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Daniel P. Berrange]

On Fri, Aug 27, 2010 at 10:57:10AM +0530, Amit Shah wrote:
> This error message denotes some command was not successful in completing
> as the guest was unresponsive.
> 
> Use it in the virtio-balloon code when showing older, cached data.
> 
> Signed-off-by: Amit Shah 
> ---
>  hw/virtio-balloon.c |1 +
>  qerror.c|4 
>  qerror.h|3 +++
>  3 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> index d6c66cf..309c343 100644
> --- a/hw/virtio-balloon.c
> +++ b/hw/virtio-balloon.c
> @@ -140,6 +140,7 @@ static void complete_stats_request(VirtIOBalloon *vb)
>  
>  static void show_old_stats(void *opaque)
>  {
> +qerror_report(QERR_MACHINE_STOPPED);
>  complete_stats_request(opaque);
>  }


NACK. It has always been allowed & valid to call query-balloon
to get the current balloon level. We must not throw an error
just because the recently added mem stats can't be refreshed.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 4/5] Add generic drive hotplugging

[Markus Armbruster]

Alexander Graf  writes:

> The monitor command for hotplugging is in i386 specific code. This is just
> plain wrong, as S390 just learned how to do hotplugging too and needs to
> get drives for that.
>
> So let's add a generic copy to generic code that handles drive_add in a
> way that doesn't have pci dependencies.
>
> I'm not fully happy with the patch as is. IMHO there should only be a
> single target agnostic drive_hot_add function available. How we could
> potentially fit IF_SCSI in there I don't know though.
>
> Signed-off-by: Alexander Graf 

Yes, we need a target-agnostic command to add host block devices.  All
we have now is x86's drive_add, which isn't target-agnostic, because its
normal function is to add both host and guest part, and the latter is
entangled with PCI.

Your patch creates a copy of x86's drive_add with the guest bits
stripped from the code, so it's usable for non-PCI targets.  Guest bits
remain in the syntax (first argument, useless baggage with your
command).  Ugly.

I'm working on a clean solution for this problem.  If you must have a
solution right away, and ugly is fine, then your patch should do for
now.

In my opinion, pci_add, pci_del and drive_add all need to go.  They're
insufficiently general, and they fail to separate host and guest parts.

Re: [Qemu-devel] [PATCH 4/5] Add generic drive hotplugging

[Alexander Graf]

On 27.08.2010, at 11:53, Markus Armbruster wrote:

> Alexander Graf  writes:
> 
>> The monitor command for hotplugging is in i386 specific code. This is just
>> plain wrong, as S390 just learned how to do hotplugging too and needs to
>> get drives for that.
>> 
>> So let's add a generic copy to generic code that handles drive_add in a
>> way that doesn't have pci dependencies.
>> 
>> I'm not fully happy with the patch as is. IMHO there should only be a
>> single target agnostic drive_hot_add function available. How we could
>> potentially fit IF_SCSI in there I don't know though.
>> 
>> Signed-off-by: Alexander Graf 
> 
> Yes, we need a target-agnostic command to add host block devices.  All
> we have now is x86's drive_add, which isn't target-agnostic, because its
> normal function is to add both host and guest part, and the latter is
> entangled with PCI.
> 
> Your patch creates a copy of x86's drive_add with the guest bits
> stripped from the code, so it's usable for non-PCI targets.  Guest bits
> remain in the syntax (first argument, useless baggage with your
> command).  Ugly.
> 
> I'm working on a clean solution for this problem.  If you must have a
> solution right away, and ugly is fine, then your patch should do for
> now.

Yes. IMHO it'd be useful to just broaden the scope of drive_add for now and 
remove it later in favor of your improved blockdev_add framework. So I guess 
the code duplication doesn't really hurt either, since this will all go away 
anyways.

> In my opinion, pci_add, pci_del and drive_add all need to go.  They're
> insufficiently general, and they fail to separate host and guest parts.

Sure :). Please make sure to always keep non-PCI capably systems in mind on 
your redesign!


Alex

[Qemu-devel] [PATCH v4 05/10] add spice into the configure file

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 configure |   35 +++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 0639b33..b391165 100755
--- a/configure
+++ b/configure
@@ -320,6 +320,7 @@ pkgversion=""
 check_utests="no"
 user_pie="no"
 zero_malloc=""
+spice=""
 
 # OS specific
 if check_define __linux__ ; then
@@ -621,6 +622,10 @@ for opt do
   ;;
   --enable-kvm) kvm="yes"
   ;;
+  --disable-spice) spice="no"
+  ;;
+  --enable-spice) spice="yes"
+  ;;
   --enable-profiler) profiler="yes"
   ;;
   --enable-cocoa)
@@ -900,6 +905,8 @@ echo "  --enable-docsenable documentation build"
 echo "  --disable-docs   disable documentation build"
 echo "  --disable-vhost-net  disable vhost-net acceleration support"
 echo "  --enable-vhost-net   enable vhost-net acceleration support"
+echo "  --disable-spice  disable spice"
+echo "  --enable-spice   enable spice"
 echo ""
 echo "NOTE: The object files are built at the place where configure is 
launched"
 exit 1
@@ -2050,6 +2057,29 @@ if compile_prog "" ""; then
 gcc_attribute_warn_unused_result=yes
 fi
 
+# spice probe
+if test "$spice" != "no" ; then
+  cat > $TMPC << EOF
+#include 
+int main(void) { spice_server_new(); return 0; }
+EOF
+  spice_cflags=$($pkgconfig --cflags spice-protocol spice-server 2>/dev/null)
+  spice_libs=$($pkgconfig --libs spice-protocol spice-server 2>/dev/null)
+  if $pkgconfig --atleast-version=0.5.3 spice-server &&\
+ compile_prog "$spice_cflags" "$spice_libs" ; then
+spice="yes"
+libs_softmmu="$libs_softmmu $spice_libs"
+QEMU_CFLAGS="$QEMU_CFLAGS $spice_cflags"
+  else
+if test "$spice" = "yes" ; then
+  feature_not_found "spice"
+fi
+spice="no"
+  fi
+fi
+
+##
+
 ##
 # check if we have fdatasync
 
@@ -2192,6 +,7 @@ echo "preadv support$preadv"
 echo "fdatasync $fdatasync"
 echo "uuid support  $uuid"
 echo "vhost-net support $vhost_net"
+echo "spice support $spice"
 
 if test $sdl_too_old = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -2429,6 +2460,10 @@ if test "$fdatasync" = "yes" ; then
   echo "CONFIG_FDATASYNC=y" >> $config_host_mak
 fi
 
+if test "$spice" = "yes" ; then
+  echo "CONFIG_SPICE=y" >> $config_host_mak
+fi
+
 # XXX: suppress that
 if [ "$bsd" = "yes" ] ; then
   echo "CONFIG_BSD=y" >> $config_host_mak
-- 
1.7.1

[Qemu-devel] [PATCH v4 00/10] initial spice support.

[Gerd Hoffmann]

  Hi,

Here comes v4 of the iniial spice support patch series, hopefully the
final version.  It brings just the very basic bits:

 * Detect spice in configure, Makefile windup.
 * Support for keyboard, mouse and tablet.
 * Support for simple display output (works as DisplayChangeListener,
   plays with any gfx card, sends simple draw commands to update
   dirty regions).

Note that this patch series does *not* yet contain the qxl paravirtual
gfx card.  That will come as part of a additional patch series after
sorting the vgabios support.

The patches are also available in the git repository at:
  git://anongit.freedesktop.org/spice/qemu submit.4

Changes since v3:
  * Drop global spice_server variable, provide a thin wrapper function
instead so spice interfaces can be registered without needing
spice_server.
  * Update locking comments.

Changes since v2:
  * Add copyright headers to the files.
  * Add dprint for debug logging.
  * Add mapping for buttons and leds.
  * Add comments for locking+threads.
  * Drop includes which qemu-common.h brings in.
  * Compile -spice switch unconditionally.
  * Hook up spice init using module.h, drop #ifdefs.
  * Misc minor tweaks.

Gerd Hoffmann (10):
  Use display types for local display only.
  Use machine_init() to register virtfs config options.
  add pflib: PixelFormat conversion library.
  configure: add logging
  add spice into the configure file
  spice: core bits
  spice: add keyboard
  spice: add mouse
  spice: simple display
  spice: add tablet support

 Makefile.objs  |3 +
 configure  |   42 +-
 fsdev/qemu-fsdev.c |9 +
 pflib.c|  213 +++
 pflib.h|   20 +++
 qemu-config.c  |   18 +++
 qemu-config.h  |1 +
 qemu-options.hx|   21 +++
 sysemu.h   |1 -
 ui/qemu-spice.h|   41 +
 ui/spice-core.c|  183 +++
 ui/spice-display.c |  410 
 ui/spice-display.h |   69 +
 ui/spice-input.c   |  208 ++
 vl.c   |   49 --
 15 files changed, 1269 insertions(+), 19 deletions(-)
 create mode 100644 pflib.c
 create mode 100644 pflib.h
 create mode 100644 ui/qemu-spice.h
 create mode 100644 ui/spice-core.c
 create mode 100644 ui/spice-display.c
 create mode 100644 ui/spice-display.h
 create mode 100644 ui/spice-input.c

[Qemu-devel] [PATCH v4 03/10] add pflib: PixelFormat conversion library.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 Makefile.objs |1 +
 pflib.c   |  213 +
 pflib.h   |   20 ++
 3 files changed, 234 insertions(+), 0 deletions(-)
 create mode 100644 pflib.c
 create mode 100644 pflib.h

diff --git a/Makefile.objs b/Makefile.objs
index 4a1eaa1..edfca87 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -83,6 +83,7 @@ common-obj-y += qemu-char.o savevm.o #aio.o
 common-obj-y += msmouse.o ps2.o
 common-obj-y += qdev.o qdev-properties.o
 common-obj-y += block-migration.o
+common-obj-y += pflib.o
 
 common-obj-$(CONFIG_BRLAPI) += baum.o
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/pflib.c b/pflib.c
new file mode 100644
index 000..1154d0c
--- /dev/null
+++ b/pflib.c
@@ -0,0 +1,213 @@
+/*
+ * PixelFormat conversion library.
+ *
+ * Author: Gerd Hoffmann 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#include "qemu-common.h"
+#include "console.h"
+#include "pflib.h"
+
+typedef struct QemuPixel QemuPixel;
+
+typedef void (*pf_convert)(QemuPfConv *conv,
+   void *dst, void *src, uint32_t cnt);
+typedef void (*pf_convert_from)(PixelFormat *pf,
+QemuPixel *dst, void *src, uint32_t cnt);
+typedef void (*pf_convert_to)(PixelFormat *pf,
+  void *dst, QemuPixel *src, uint32_t cnt);
+
+struct QemuPfConv {
+pf_convertconvert;
+PixelFormat   src;
+PixelFormat   dst;
+
+/* for copy_generic() */
+pf_convert_from   conv_from;
+pf_convert_to conv_to;
+QemuPixel *conv_buf;
+uint32_t  conv_cnt;
+};
+
+struct QemuPixel {
+uint8_t red;
+uint8_t green;
+uint8_t blue;
+uint8_t alpha;
+};
+
+/* --- */
+/* PixelFormat -> QemuPixel conversions*/
+
+static void conv_16_to_pixel(PixelFormat *pf,
+ QemuPixel *dst, void *src, uint32_t cnt)
+{
+uint16_t *src16 = src;
+
+while (cnt > 0) {
+dst->red   = ((*src16 & pf->rmask) >> pf->rshift) << (8 - pf->rbits);
+dst->green = ((*src16 & pf->gmask) >> pf->gshift) << (8 - pf->gbits);
+dst->blue  = ((*src16 & pf->bmask) >> pf->bshift) << (8 - pf->bbits);
+dst->alpha = ((*src16 & pf->amask) >> pf->ashift) << (8 - pf->abits);
+dst++, src16++, cnt--;
+}
+}
+
+/* assumes pf->{r,g,b,a}bits == 8 */
+static void conv_32_to_pixel_fast(PixelFormat *pf,
+  QemuPixel *dst, void *src, uint32_t cnt)
+{
+uint32_t *src32 = src;
+
+while (cnt > 0) {
+dst->red   = (*src32 & pf->rmask) >> pf->rshift;
+dst->green = (*src32 & pf->gmask) >> pf->gshift;
+dst->blue  = (*src32 & pf->bmask) >> pf->bshift;
+dst->alpha = (*src32 & pf->amask) >> pf->ashift;
+dst++, src32++, cnt--;
+}
+}
+
+static void conv_32_to_pixel_generic(PixelFormat *pf,
+ QemuPixel *dst, void *src, uint32_t cnt)
+{
+uint32_t *src32 = src;
+
+while (cnt > 0) {
+if (pf->rbits < 8) {
+dst->red   = ((*src32 & pf->rmask) >> pf->rshift) << (8 - 
pf->rbits);
+} else {
+dst->red   = ((*src32 & pf->rmask) >> pf->rshift) >> (pf->rbits - 
8);
+}
+if (pf->gbits < 8) {
+dst->green = ((*src32 & pf->gmask) >> pf->gshift) << (8 - 
pf->gbits);
+} else {
+dst->green = ((*src32 & pf->gmask) >> pf->gshift) >> (pf->gbits - 
8);
+}
+if (pf->bbits < 8) {
+dst->blue  = ((*src32 & pf->bmask) >> pf->bshift) << (8 - 
pf->bbits);
+} else {
+dst->blue  = ((*src32 & pf->bmask) >> pf->bshift) >> (pf->bbits - 
8);
+}
+if (pf->abits < 8) {
+dst->alpha = ((*src32 & pf->amask) >> pf->ashift) << (8 - 
pf->abits);
+} else {
+dst->alpha = ((*src32 & pf->amask) >> pf->ashift) >> (pf->abits - 
8);
+}
+dst++, src32++, cnt--;
+}
+}
+
+/* --- */
+/* QemuPixel -> PixelFormat conversions*/
+
+static void conv_pixel_to_16(PixelFormat *pf,
+ void *dst, QemuPixel *src, uint32_t cnt)
+{
+uint16_t *dst16 = dst;
+
+while (cnt > 0) {
+*dst16  = ((uint16_t)src->red   >> (8 - pf->rbits)) << pf->rshift;
+*dst16 |= ((uint16_t)src->green >> (8 - pf->gbits)) << pf->gshift;
+*dst16 |= ((uint16_t)src->blue  >> (8 - pf->bbits)) << pf->bshift;
+*dst16 |= ((uint16_t)src->alpha >> (8 - pf->abits)) << pf->ashift;
+dst16++, src++, cnt--;
+}
+}
+
+static void conv_pixel_to_32(PixelFormat *pf,
+ void *dst, QemuPixe

[Qemu-devel] [PATCH v4 02/10] Use machine_init() to register virtfs config options.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 fsdev/qemu-fsdev.c |9 +
 vl.c   |5 -
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index ad69b0e..280b8f5 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -16,6 +16,7 @@
 #include "qemu-queue.h"
 #include "osdep.h"
 #include "qemu-common.h"
+#include "qemu-config.h"
 
 static QTAILQ_HEAD(FsTypeEntry_head, FsTypeListEntry) fstype_entries =
 QTAILQ_HEAD_INITIALIZER(fstype_entries);
@@ -75,3 +76,11 @@ FsTypeEntry *get_fsdev_fsentry(char *id)
 }
 return NULL;
 }
+
+static void fsdev_register_config(void)
+{
+qemu_add_opts(&qemu_fsdev_opts);
+qemu_add_opts(&qemu_virtfs_opts);
+}
+machine_init(fsdev_register_config);
+
diff --git a/vl.c b/vl.c
index d90b275..40bf30b 100644
--- a/vl.c
+++ b/vl.c
@@ -1856,11 +1856,6 @@ int main(int argc, char **argv, char **envp)
 tb_size = 0;
 autostart= 1;
 
-#ifdef CONFIG_VIRTFS
-qemu_add_opts(&qemu_fsdev_opts);
-qemu_add_opts(&qemu_virtfs_opts);
-#endif
-
 /* first pass of option parsing */
 optind = 1;
 while (optind < argc) {
-- 
1.7.1

[Qemu-devel] [PATCH v4 07/10] spice: add keyboard

[Gerd Hoffmann]

Open keyboard channel.  Now you can type into the spice client and the
keyboard events are sent to your guest.  You'll need some other display
like vnc to actually see the guest responding to them though.

Signed-off-by: Gerd Hoffmann 
---
 Makefile.objs|2 +-
 ui/qemu-spice.h  |1 +
 ui/spice-core.c  |2 +
 ui/spice-input.c |   82 ++
 4 files changed, 86 insertions(+), 1 deletions(-)
 create mode 100644 ui/spice-input.c

diff --git a/Makefile.objs b/Makefile.objs
index 1a53027..20e6ad7 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -88,7 +88,7 @@ common-obj-y += pflib.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
 
-common-obj-$(CONFIG_SPICE) += ui/spice-core.o
+common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o
 
 audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
 audio-obj-$(CONFIG_SDL) += sdlaudio.o
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index 50faefb..175c961 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -28,6 +28,7 @@
 extern int using_spice;
 
 void qemu_spice_init(void);
+void qemu_spice_input_init(void);
 int qemu_spice_add_interface(SpiceBaseInstance *sin);
 
 #else  /* CONFIG_SPICE */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 178d740..07d6a0c 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -161,6 +161,8 @@ void qemu_spice_init(void)
 
 spice_server_init(spice_server, &core_interface);
 using_spice = 1;
+
+qemu_spice_input_init();
 }
 
 int qemu_spice_add_interface(SpiceBaseInstance *sin)
diff --git a/ui/spice-input.c b/ui/spice-input.c
new file mode 100644
index 000..efecc22
--- /dev/null
+++ b/ui/spice-input.c
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "qemu-common.h"
+#include "qemu-spice.h"
+#include "console.h"
+
+/* keyboard bits */
+
+typedef struct QemuSpiceKbd {
+SpiceKbdInstance sin;
+int ledstate;
+} QemuSpiceKbd;
+
+static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag);
+static uint8_t kbd_get_leds(SpiceKbdInstance *sin);
+static void kbd_leds(void *opaque, int l);
+
+static const SpiceKbdInterface kbd_interface = {
+.base.type  = SPICE_INTERFACE_KEYBOARD,
+.base.description   = "qemu keyboard",
+.base.major_version = SPICE_INTERFACE_KEYBOARD_MAJOR,
+.base.minor_version = SPICE_INTERFACE_KEYBOARD_MINOR,
+.push_scan_freg = kbd_push_key,
+.get_leds   = kbd_get_leds,
+};
+
+static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag)
+{
+kbd_put_keycode(frag);
+}
+
+static uint8_t kbd_get_leds(SpiceKbdInstance *sin)
+{
+QemuSpiceKbd *kbd = container_of(sin, QemuSpiceKbd, sin);
+return kbd->ledstate;
+}
+
+static void kbd_leds(void *opaque, int ledstate)
+{
+QemuSpiceKbd *kbd = opaque;
+
+kbd->ledstate = 0;
+if (ledstate & QEMU_SCROLL_LOCK_LED)
+kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_SCROLL_LOCK;
+if (ledstate & QEMU_NUM_LOCK_LED)
+kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_NUM_LOCK;
+if (ledstate & QEMU_CAPS_LOCK_LED)
+kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_CAPS_LOCK;
+spice_server_kbd_leds(&kbd->sin, ledstate);
+}
+
+void qemu_spice_input_init(void)
+{
+QemuSpiceKbd *kbd;
+
+kbd = qemu_mallocz(sizeof(*kbd));
+kbd->sin.base.sif = &kbd_interface.base;
+qemu_spice_add_interface(&kbd->sin.base);
+qemu_add_led_event_handler(kbd_leds, kbd);
+}
-- 
1.7.1

[Qemu-devel] [PATCH v4 01/10] Use display types for local display only.

[Gerd Hoffmann]

This patch drops DT_VNC.  The display types are only used to select
select the local display (i.e. curses, sdl, coca, ...).  Remote
displays (for now only vnc, spice will follow) can be enabled
independently.

Signed-off-by: Gerd Hoffmann 
---
 sysemu.h |1 -
 vl.c |   24 +---
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/sysemu.h b/sysemu.h
index a1f6466..b81a70e 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -94,7 +94,6 @@ typedef enum DisplayType
 DT_DEFAULT,
 DT_CURSES,
 DT_SDL,
-DT_VNC,
 DT_NOGRAPHIC,
 } DisplayType;
 
diff --git a/vl.c b/vl.c
index 91d1684..d90b275 100644
--- a/vl.c
+++ b/vl.c
@@ -172,6 +172,7 @@ static const char *data_dir;
 const char *bios_name = NULL;
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
 DisplayType display_type = DT_DEFAULT;
+int display_remote = 0;
 const char* keyboard_layout = NULL;
 ram_addr_t ram_size;
 const char *mem_path = NULL;
@@ -2468,7 +2469,7 @@ int main(int argc, char **argv, char **envp)
 }
 break;
case QEMU_OPTION_vnc:
-display_type = DT_VNC;
+display_remote++;
vnc_display = optarg;
break;
 case QEMU_OPTION_no_acpi:
@@ -2898,17 +2899,17 @@ int main(int argc, char **argv, char **envp)
 /* just use the first displaystate for the moment */
 ds = get_displaystate();
 
-if (display_type == DT_DEFAULT) {
+if (display_type == DT_DEFAULT && !display_remote) {
 #if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
 display_type = DT_SDL;
 #else
-display_type = DT_VNC;
 vnc_display = "localhost:0,to=99";
 show_vnc_port = 1;
 #endif
 }
 
 
+/* init local displays */
 switch (display_type) {
 case DT_NOGRAPHIC:
 break;
@@ -2926,7 +2927,12 @@ int main(int argc, char **argv, char **envp)
 cocoa_display_init(ds, full_screen);
 break;
 #endif
-case DT_VNC:
+default:
+break;
+}
+
+/* init remote displays */
+if (vnc_display) {
 vnc_display_init(ds);
 if (vnc_display_open(ds, vnc_display) < 0)
 exit(1);
@@ -2934,12 +2940,10 @@ int main(int argc, char **argv, char **envp)
 if (show_vnc_port) {
 printf("VNC server running on `%s'\n", vnc_display_local_addr(ds));
 }
-break;
-default:
-break;
 }
-dpy_resize(ds);
 
+/* display setup */
+dpy_resize(ds);
 dcl = ds->listeners;
 while (dcl != NULL) {
 if (dcl->dpy_refresh != NULL) {
@@ -2949,12 +2953,10 @@ int main(int argc, char **argv, char **envp)
 }
 dcl = dcl->next;
 }
-
-if (display_type == DT_NOGRAPHIC || display_type == DT_VNC) {
+if (ds->gui_timer == NULL) {
 nographic_timer = qemu_new_timer(rt_clock, nographic_update, NULL);
 qemu_mod_timer(nographic_timer, qemu_get_clock(rt_clock));
 }
-
 text_consoles_set_display(ds);
 
 if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
-- 
1.7.1

[Qemu-devel] [PATCH v4 06/10] spice: core bits

[Gerd Hoffmann]

Add -spice command line switch.  Has support setting passwd and port for
now.  With this patch applied the spice client can successfully connect
to qemu.  You can't do anything useful yet though.

Signed-off-by: Gerd Hoffmann 
---
 Makefile.objs   |2 +
 qemu-config.c   |   18 ++
 qemu-config.h   |1 +
 qemu-options.hx |   21 +++
 ui/qemu-spice.h |   39 
 ui/spice-core.c |  181 +++
 vl.c|   15 +
 7 files changed, 277 insertions(+), 0 deletions(-)
 create mode 100644 ui/qemu-spice.h
 create mode 100644 ui/spice-core.c

diff --git a/Makefile.objs b/Makefile.objs
index edfca87..1a53027 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -88,6 +88,8 @@ common-obj-y += pflib.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
 
+common-obj-$(CONFIG_SPICE) += ui/spice-core.o
+
 audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
 audio-obj-$(CONFIG_SDL) += sdlaudio.o
 audio-obj-$(CONFIG_OSS) += ossaudio.o
diff --git a/qemu-config.c b/qemu-config.c
index 3abe655..4c47cd0 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -336,6 +336,24 @@ static QemuOptsList qemu_cpudef_opts = {
 },
 };
 
+QemuOptsList qemu_spice_opts = {
+.name = "spice",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_spice_opts.head),
+.desc = {
+{
+.name = "port",
+.type = QEMU_OPT_NUMBER,
+},{
+.name = "password",
+.type = QEMU_OPT_STRING,
+},{
+.name = "disable-ticketing",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end if list */ }
+},
+};
+
 static QemuOptsList *vm_config_groups[32] = {
 &qemu_drive_opts,
 &qemu_chardev_opts,
diff --git a/qemu-config.h b/qemu-config.h
index 533a049..20d707f 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -3,6 +3,7 @@
 
 extern QemuOptsList qemu_fsdev_opts;
 extern QemuOptsList qemu_virtfs_opts;
+extern QemuOptsList qemu_spice_opts;
 
 QemuOptsList *qemu_find_opts(const char *group);
 void qemu_add_opts(QemuOptsList *list);
diff --git a/qemu-options.hx b/qemu-options.hx
index 453f129..87bd451 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -670,6 +670,27 @@ STEXI
 Enable SDL.
 ETEXI
 
+DEF("spice", HAS_ARG, QEMU_OPTION_spice,
+"-spiceenable spice\n", QEMU_ARCH_ALL)
+STEXI
+...@item -spice @var{option}[,@var{option}[,...]]
+...@findex -spice
+Enable the spice remote desktop protocol. Valid options are
+
+...@table @option
+
+...@item port=
+Set the TCP port spice is listening on.
+
+...@item password=
+Set the password you need to authenticate.
+
+...@item disable-ticketing
+Allow client connects without authentication.
+
+...@end table
+ETEXI
+
 DEF("portrait", 0, QEMU_OPTION_portrait,
 "-portrait   rotate graphical output 90 deg left (only PXA LCD)\n",
 QEMU_ARCH_ALL)
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
new file mode 100644
index 000..50faefb
--- /dev/null
+++ b/ui/qemu-spice.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#ifndef QEMU_SPICE_H
+#define QEMU_SPICE_H
+
+#ifdef CONFIG_SPICE
+
+#include 
+
+#include "qemu-option.h"
+#include "qemu-config.h"
+
+extern int using_spice;
+
+void qemu_spice_init(void);
+int qemu_spice_add_interface(SpiceBaseInstance *sin);
+
+#else  /* CONFIG_SPICE */
+
+#define using_spice 0
+
+#endif /* CONFIG_SPICE */
+
+#endif /* QEMU_SPICE_H */
diff --git a/ui/spice-core.c b/ui/spice-core.c
new file mode 100644
index 000..178d740
--- /dev/null
+++ b/ui/spice-core.c
@@ -0,0 +1,181 @@
+/*
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include 
+#include 
+
+#i

[Qemu-devel] [PATCH v4 08/10] spice: add mouse

[Gerd Hoffmann]

Open mouse channel.  Now you can move the guests mouse pointer.
No tablet / absolute positioning (yet) though.

Signed-off-by: Gerd Hoffmann 
---
 ui/spice-input.c |   49 +
 1 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/ui/spice-input.c b/ui/spice-input.c
index efecc22..afb4d61 100644
--- a/ui/spice-input.c
+++ b/ui/spice-input.c
@@ -71,12 +71,61 @@ static void kbd_leds(void *opaque, int ledstate)
 spice_server_kbd_leds(&kbd->sin, ledstate);
 }
 
+/* mouse bits */
+
+typedef struct QemuSpiceMouse {
+SpiceMouseInstance sin;
+} QemuSpiceMouse;
+
+static int map_buttons(int spice_buttons)
+{
+int qemu_buttons = 0;
+
+/*
+ * Note: SPICE_MOUSE_BUTTON_* specifies the wire protocol but this
+ * isn't what we get passed in via interface callbacks for the
+ * middle and right button ...
+ */
+if (spice_buttons & SPICE_MOUSE_BUTTON_MASK_LEFT)
+qemu_buttons |= MOUSE_EVENT_LBUTTON;
+if (spice_buttons & 0x04 /* SPICE_MOUSE_BUTTON_MASK_MIDDLE */)
+qemu_buttons |= MOUSE_EVENT_MBUTTON;
+if (spice_buttons & 0x02 /* SPICE_MOUSE_BUTTON_MASK_RIGHT */)
+qemu_buttons |= MOUSE_EVENT_RBUTTON;
+return qemu_buttons;
+}
+
+static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int dz,
+ uint32_t buttons_state)
+{
+kbd_mouse_event(dx, dy, dz, map_buttons(buttons_state));
+}
+
+static void mouse_buttons(SpiceMouseInstance *sin, uint32_t buttons_state)
+{
+kbd_mouse_event(0, 0, 0, map_buttons(buttons_state));
+}
+
+static const SpiceMouseInterface mouse_interface = {
+.base.type  = SPICE_INTERFACE_MOUSE,
+.base.description   = "mouse",
+.base.major_version = SPICE_INTERFACE_MOUSE_MAJOR,
+.base.minor_version = SPICE_INTERFACE_MOUSE_MINOR,
+.motion = mouse_motion,
+.buttons= mouse_buttons,
+};
+
 void qemu_spice_input_init(void)
 {
 QemuSpiceKbd *kbd;
+QemuSpiceMouse *mouse;
 
 kbd = qemu_mallocz(sizeof(*kbd));
 kbd->sin.base.sif = &kbd_interface.base;
 qemu_spice_add_interface(&kbd->sin.base);
 qemu_add_led_event_handler(kbd_leds, kbd);
+
+mouse = qemu_mallocz(sizeof(*mouse));
+mouse->sin.base.sif = &mouse_interface.base;
+qemu_spice_add_interface(&mouse->sin.base);
 }
-- 
1.7.1

[Qemu-devel] [PATCH v4 10/10] spice: add tablet support

[Gerd Hoffmann]

Add support for the spice tablet interface.  The tablet interface will
be registered (and then used by the spice client) as soon as a absolute
pointing device is available and used by the guest, i.e. you'll have to
configure your guest with '-usbdevice tablet'.

Signed-off-by: Gerd Hoffmann 
---
 ui/spice-display.c |2 +-
 ui/spice-input.c   |   91 
 2 files changed, 85 insertions(+), 8 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 9e11fb3..30dfb9f 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -177,7 +177,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
 surface.width  = ds_get_width(ssd->ds);
 surface.height = ds_get_height(ssd->ds);
 surface.stride = -surface.width * 4;
-surface.mouse_mode = 0;
+surface.mouse_mode = true;
 surface.flags  = 0;
 surface.type   = 0;
 surface.mem= (intptr_t)ssd->buf;
diff --git a/ui/spice-input.c b/ui/spice-input.c
index afb4d61..c6ac632 100644
--- a/ui/spice-input.c
+++ b/ui/spice-input.c
@@ -17,6 +17,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -73,9 +74,13 @@ static void kbd_leds(void *opaque, int ledstate)
 
 /* mouse bits */
 
-typedef struct QemuSpiceMouse {
-SpiceMouseInstance sin;
-} QemuSpiceMouse;
+typedef struct QemuSpicePointer {
+SpiceMouseInstance  mouse;
+SpiceTabletInstance tablet;
+int width, height, x, y;
+Notifier mouse_mode;
+bool absolute;
+} QemuSpicePointer;
 
 static int map_buttons(int spice_buttons)
 {
@@ -115,17 +120,89 @@ static const SpiceMouseInterface mouse_interface = {
 .buttons= mouse_buttons,
 };
 
+static void tablet_set_logical_size(SpiceTabletInstance* sin, int width, int 
height)
+{
+QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
+
+if (height < 16)
+height = 16;
+if (width < 16)
+width = 16;
+pointer->width  = width;
+pointer->height = height;
+}
+
+static void tablet_position(SpiceTabletInstance* sin, int x, int y,
+uint32_t buttons_state)
+{
+QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
+
+pointer->x = x * 0x7FFF / (pointer->width - 1);
+pointer->y = y * 0x7FFF / (pointer->height - 1);
+kbd_mouse_event(pointer->x, pointer->y, 0, map_buttons(buttons_state));
+}
+
+
+static void tablet_wheel(SpiceTabletInstance* sin, int wheel,
+ uint32_t buttons_state)
+{
+QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
+
+kbd_mouse_event(pointer->x, pointer->y, wheel, map_buttons(buttons_state));
+}
+
+static void tablet_buttons(SpiceTabletInstance *sin,
+   uint32_t buttons_state)
+{
+QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
+
+kbd_mouse_event(pointer->x, pointer->y, 0, map_buttons(buttons_state));
+}
+
+static const SpiceTabletInterface tablet_interface = {
+.base.type  = SPICE_INTERFACE_TABLET,
+.base.description   = "tablet",
+.base.major_version = SPICE_INTERFACE_TABLET_MAJOR,
+.base.minor_version = SPICE_INTERFACE_TABLET_MINOR,
+.set_logical_size   = tablet_set_logical_size,
+.position   = tablet_position,
+.wheel  = tablet_wheel,
+.buttons= tablet_buttons,
+};
+
+static void mouse_mode_notifier(Notifier *notifier)
+{
+QemuSpicePointer *pointer = container_of(notifier, QemuSpicePointer, 
mouse_mode);
+bool is_absolute  = kbd_mouse_is_absolute();
+
+if (pointer->absolute == is_absolute)
+return;
+
+if (is_absolute) {
+qemu_spice_add_interface(&pointer->tablet.base);
+} else {
+spice_server_remove_interface(&pointer->tablet.base);
+}
+pointer->absolute = is_absolute;
+}
+
 void qemu_spice_input_init(void)
 {
 QemuSpiceKbd *kbd;
-QemuSpiceMouse *mouse;
+QemuSpicePointer *pointer;
 
 kbd = qemu_mallocz(sizeof(*kbd));
 kbd->sin.base.sif = &kbd_interface.base;
 qemu_spice_add_interface(&kbd->sin.base);
 qemu_add_led_event_handler(kbd_leds, kbd);
 
-mouse = qemu_mallocz(sizeof(*mouse));
-mouse->sin.base.sif = &mouse_interface.base;
-qemu_spice_add_interface(&mouse->sin.base);
+pointer = qemu_mallocz(sizeof(*pointer));
+pointer->mouse.base.sif  = &mouse_interface.base;
+pointer->tablet.base.sif = &tablet_interface.base;
+qemu_spice_add_interface(&pointer->mouse.base);
+
+pointer->absolute = false;
+pointer->mouse_mode.notify = mouse_mode_notifier;
+qemu_add_mouse_mode_change_notifier(&pointer->mouse_mode);
+mouse_mode_notifier(&pointer->mouse_mode);
 }
-- 
1.7.1

[Qemu-devel] [PATCH v4 09/10] spice: simple display

[Gerd Hoffmann]

With that patch applied you'll actually see the guests screen in the
spice client.  This does *not* bring qxl and full spice support though.
This is basically the qxl vga mode made more generic, so it plays
together with any qemu-emulated gfx card.  You can display stdvga or
cirrus via spice client.  You can have both vnc and spice enabled and
clients connected at the same time.

Signed-off-by: Gerd Hoffmann 
---
 Makefile.objs  |2 +-
 ui/qemu-spice.h|1 +
 ui/spice-display.c |  410 
 ui/spice-display.h |   69 +
 vl.c   |5 +
 5 files changed, 486 insertions(+), 1 deletions(-)
 create mode 100644 ui/spice-display.c
 create mode 100644 ui/spice-display.h

diff --git a/Makefile.objs b/Makefile.objs
index 20e6ad7..d09a1e4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -88,7 +88,7 @@ common-obj-y += pflib.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
 
-common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o
+common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o 
ui/spice-display.o
 
 audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
 audio-obj-$(CONFIG_SDL) += sdlaudio.o
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index 175c961..063c7dc 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -29,6 +29,7 @@ extern int using_spice;
 
 void qemu_spice_init(void);
 void qemu_spice_input_init(void);
+void qemu_spice_display_init(DisplayState *ds);
 int qemu_spice_add_interface(SpiceBaseInstance *sin);
 
 #else  /* CONFIG_SPICE */
diff --git a/ui/spice-display.c b/ui/spice-display.c
new file mode 100644
index 000..9e11fb3
--- /dev/null
+++ b/ui/spice-display.c
@@ -0,0 +1,410 @@
+/*
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include 
+
+#include "qemu-common.h"
+#include "qemu-spice.h"
+#include "qemu-timer.h"
+#include "qemu-queue.h"
+#include "monitor.h"
+#include "console.h"
+#include "sysemu.h"
+
+#include "spice-display.h"
+
+static int debug = 0;
+
+static void __attribute__((format(printf,2,3)))
+dprint(int level, const char *fmt, ...)
+{
+va_list args;
+
+if (level <= debug) {
+va_start(args, fmt);
+vfprintf(stderr, fmt, args);
+va_end(args);
+}
+}
+
+int qemu_spice_rect_is_empty(const QXLRect* r)
+{
+return r->top == r->bottom || r->left == r->right;
+}
+
+void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
+{
+if (qemu_spice_rect_is_empty(r)) {
+return;
+}
+
+if (qemu_spice_rect_is_empty(dest)) {
+*dest = *r;
+return;
+}
+
+dest->top = MIN(dest->top, r->top);
+dest->left = MIN(dest->left, r->left);
+dest->bottom = MAX(dest->bottom, r->bottom);
+dest->right = MAX(dest->right, r->right);
+}
+
+/*
+ * Called from spice server thread context (via interface_get_command).
+ * We do *not* hold the global qemu mutex here, so extra care is needed
+ * when calling qemu functions.  Qemu interfaces used:
+ *- pflib (is re-entrant).
+ *- qemu_malloc (underlying glibc malloc is re-entrant).
+ */
+SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+{
+SimpleSpiceUpdate *update;
+QXLDrawable *drawable;
+QXLImage *image;
+QXLCommand *cmd;
+uint8_t *src, *dst;
+int by, bw, bh;
+
+if (qemu_spice_rect_is_empty(&ssd->dirty)) {
+return NULL;
+};
+
+pthread_mutex_lock(&ssd->lock);
+dprint(2, "%s: lr %d -> %d,  tb -> %d -> %d\n", __FUNCTION__,
+   ssd->dirty.left, ssd->dirty.right,
+   ssd->dirty.top, ssd->dirty.bottom);
+
+update   = qemu_mallocz(sizeof(*update));
+drawable = &update->drawable;
+image= &update->image;
+cmd  = &update->ext.cmd;
+
+bw   = ssd->dirty.right - ssd->dirty.left;
+bh   = ssd->dirty.bottom - ssd->dirty.top;
+update->bitmap = qemu_malloc(bw * bh * 4);
+
+drawable->bbox= ssd->dirty;
+drawable->clip.type   = SPICE_CLIP_TYPE_NONE;
+drawable->effect  = QXL_EFFECT_OPAQUE;
+drawable->release_info.id = (intptr_t)update;
+drawable->type= QXL_DRAW_COPY;
+
+drawable->u.copy.rop_descriptor  = SPICE_ROPD_OP_PUT;
+drawable->u.copy.src_bitmap  = (intptr_t)image;
+drawable->u.copy.src_a

[Qemu-devel] [PATCH v4 04/10] configure: add logging

[Gerd Hoffmann]

Write compile commands and messages to config.log.
Useful for debugging configure.

Signed-off-by: Gerd Hoffmann 
---
 configure |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 146dac0..0639b33 100755
--- a/configure
+++ b/configure
@@ -16,15 +16,18 @@ TMPO="${TMPDIR1}/qemu-conf-${RANDOM}-$$-${RANDOM}.o"
 TMPE="${TMPDIR1}/qemu-conf-${RANDOM}-$$-${RANDOM}.exe"
 
 trap "rm -f $TMPC $TMPO $TMPE ; exit" EXIT INT QUIT TERM
+rm -f config.log
 
 compile_object() {
-  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC > /dev/null 2> /dev/null
+  echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log
+  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1
 }
 
 compile_prog() {
   local_cflags="$1"
   local_ldflags="$2"
-  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > 
/dev/null 2> /dev/null
+  echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags 
>> config.log
+  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> 
config.log 2>&1
 }
 
 # check whether a command is available to this shell (may be either an
-- 
1.7.1

[Qemu-devel] [PATCH][Tracing v2] Process -trace using QemuOptsList

[Prerna Saxena]

[PATCH] Add -trace file FILENAME switch to qemu startup command.
 This processes the argument using QemuOptsList


Signed-off-by: Prerna Saxena 
---
 qemu-config.c |   18 ++
 qemu-config.h |3 +++
 vl.c  |5 -
 3 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 95abe61..9106511 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -294,6 +294,21 @@ QemuOptsList qemu_mon_opts = {
 },
 };
 
+#ifdef CONFIG_SIMPLE_TRACE
+QemuOptsList qemu_trace_opts = {
+.name = "trace",
+.implied_opt_name = "trace",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_trace_opts.head),
+.desc = {
+{
+.name = "file",
+.type = QEMU_OPT_STRING,
+},
+{ /* end if list */ }
+},
+};
+#endif
+
 QemuOptsList qemu_cpudef_opts = {
 .name = "cpudef",
 .head = QTAILQ_HEAD_INITIALIZER(qemu_cpudef_opts.head),
@@ -352,6 +367,9 @@ static QemuOptsList *vm_config_groups[] = {
 &qemu_global_opts,
 &qemu_mon_opts,
 &qemu_cpudef_opts,
+#ifdef CONFIG_SIMPLE_TRACE
+&qemu_trace_opts,
+#endif
 NULL,
 };
 
diff --git a/qemu-config.h b/qemu-config.h
index dca69d4..4db2fb5 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -14,6 +14,9 @@ extern QemuOptsList qemu_rtc_opts;
 extern QemuOptsList qemu_global_opts;
 extern QemuOptsList qemu_mon_opts;
 extern QemuOptsList qemu_cpudef_opts;
+#ifdef CONFIG_SIMPLE_TRACE
+extern QemuOptsList qemu_trace_opts;
+#endif
 
 QemuOptsList *qemu_find_opts(const char *group);
 int qemu_set_option(const char *str);
diff --git a/vl.c b/vl.c
index 99664e9..0ff04e9 100644
--- a/vl.c
+++ b/vl.c
@@ -2599,7 +2599,10 @@ int main(int argc, char **argv, char **envp)
 break;
 #ifdef CONFIG_SIMPLE_TRACE
 case QEMU_OPTION_trace:
-trace_file = optarg;
+opts = qemu_opts_parse(&qemu_trace_opts, optarg, 0);
+if (opts) {
+trace_file = qemu_opt_get(opts, "file");
+}
 break;
 #endif
 case QEMU_OPTION_readconfig:
-- 
1.7.2.1



-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

[Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Anthony Liguori]

On 08/27/2010 04:29 AM, Daniel P. Berrange wrote:

On Fri, Aug 27, 2010 at 10:57:10AM +0530, Amit Shah wrote:
   

This error message denotes some command was not successful in completing
as the guest was unresponsive.

Use it in the virtio-balloon code when showing older, cached data.

Signed-off-by: Amit Shah
---
  hw/virtio-balloon.c |1 +
  qerror.c|4 
  qerror.h|3 +++
  3 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index d6c66cf..309c343 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -140,6 +140,7 @@ static void complete_stats_request(VirtIOBalloon *vb)

  static void show_old_stats(void *opaque)
  {
+qerror_report(QERR_MACHINE_STOPPED);
  complete_stats_request(opaque);
  }
 


NACK. It has always been allowed&  valid to call query-balloon
to get the current balloon level. We must not throw an error
just because the recently added mem stats can't be refreshed.
   


I think that's a fair comment but why even bother fixing the command.  
Let's introduce a new command that just gets a single piece of 
information instead of having a command return lots of information.


Regards,

Anthony Liguori


Regards,
Daniel
   

Re: [Qemu-devel] [PATCH v4 09/10] spice: simple display

[Anthony Liguori]

On 08/27/2010 04:59 AM, Gerd Hoffmann wrote:

With that patch applied you'll actually see the guests screen in the
spice client.  This does *not* bring qxl and full spice support though.
This is basically the qxl vga mode made more generic, so it plays
together with any qemu-emulated gfx card.  You can display stdvga or
cirrus via spice client.  You can have both vnc and spice enabled and
clients connected at the same time.

Signed-off-by: Gerd Hoffmann
   

[snip]

+/*
+ * Called from spice server thread context (via interface_get_command).
+ * We do *not* hold the global qemu mutex here, so extra care is needed
+ * when calling qemu functions.  Qemu interfaces used:
+ *- pflib (is re-entrant).
+ *- qemu_malloc (underlying glibc malloc is re-entrant).
+ */
   


Thanks, that's exactly what I was looking for.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Daniel P. Berrange]

On Fri, Aug 27, 2010 at 07:39:37AM -0500, Anthony Liguori wrote:
> On 08/27/2010 04:29 AM, Daniel P. Berrange wrote:
> >On Fri, Aug 27, 2010 at 10:57:10AM +0530, Amit Shah wrote:
> >   
> >>This error message denotes some command was not successful in completing
> >>as the guest was unresponsive.
> >>
> >>Use it in the virtio-balloon code when showing older, cached data.
> >>
> >>Signed-off-by: Amit Shah
> >>---
> >>  hw/virtio-balloon.c |1 +
> >>  qerror.c|4 
> >>  qerror.h|3 +++
> >>  3 files changed, 8 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> >>index d6c66cf..309c343 100644
> >>--- a/hw/virtio-balloon.c
> >>+++ b/hw/virtio-balloon.c
> >>@@ -140,6 +140,7 @@ static void complete_stats_request(VirtIOBalloon *vb)
> >>
> >>  static void show_old_stats(void *opaque)
> >>  {
> >>+qerror_report(QERR_MACHINE_STOPPED);
> >>  complete_stats_request(opaque);
> >>  }
> >> 
> >
> >NACK. It has always been allowed&  valid to call query-balloon
> >to get the current balloon level. We must not throw an error
> >just because the recently added mem stats can't be refreshed.
> 
> I think that's a fair comment but why even bother fixing the command.  
> Let's introduce a new command that just gets a single piece of 
> information instead of having a command return lots of information.

The existing query-balloon command that has been around for years &
is used by all current apps has a significant regression since we added
the memstats code to it: a guest can now trivially inflict a DOS on the
mgmt app if it crashes or is malicious. IMHO we need to fix that regression
for 0.13 so that existing apps don't suffer[1]. Adding a timeout to silently
skip the stats refresh if the guest doesn't respond, but without raising
an error seems the best tradeoff we can do here.

Beyond fixing that regression, I agree that this command is terminally
flawed & we need to deprecate it & provide better specified new
replacement(s). This seems like 0.14 work to me though.

Regards,
Daniel

[1] I know that they could already suffer if there was a bug in qemu
that prevented it responding, even if the guest was not being
malicious/crashed.
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Fri, Aug 27, 2010 at 07:39:37AM -0500, Anthony Liguori wrote:
>> On 08/27/2010 04:29 AM, Daniel P. Berrange wrote:
>> >On Fri, Aug 27, 2010 at 10:57:10AM +0530, Amit Shah wrote:
>> >   
>> >>This error message denotes some command was not successful in completing
>> >>as the guest was unresponsive.
>> >>
>> >>Use it in the virtio-balloon code when showing older, cached data.
>> >>
>> >>Signed-off-by: Amit Shah
>> >>---
>> >>  hw/virtio-balloon.c |1 +
>> >>  qerror.c|4 
>> >>  qerror.h|3 +++
>> >>  3 files changed, 8 insertions(+), 0 deletions(-)
>> >>
>> >>diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
>> >>index d6c66cf..309c343 100644
>> >>--- a/hw/virtio-balloon.c
>> >>+++ b/hw/virtio-balloon.c
>> >>@@ -140,6 +140,7 @@ static void complete_stats_request(VirtIOBalloon *vb)
>> >>
>> >>  static void show_old_stats(void *opaque)
>> >>  {
>> >>+qerror_report(QERR_MACHINE_STOPPED);
>> >>  complete_stats_request(opaque);
>> >>  }
>> >> 
>> >
>> >NACK. It has always been allowed&  valid to call query-balloon
>> >to get the current balloon level. We must not throw an error
>> >just because the recently added mem stats can't be refreshed.
>> 
>> I think that's a fair comment but why even bother fixing the command.  
>> Let's introduce a new command that just gets a single piece of 
>> information instead of having a command return lots of information.
>
> The existing query-balloon command that has been around for years &
> is used by all current apps has a significant regression since we added
> the memstats code to it: a guest can now trivially inflict a DOS on the
> mgmt app if it crashes or is malicious. IMHO we need to fix that regression
> for 0.13 so that existing apps don't suffer[1]. Adding a timeout to silently
> skip the stats refresh if the guest doesn't respond, but without raising
> an error seems the best tradeoff we can do here.

I agree.

Adding a roundtrip through the guest to an existing command was a
mistake.

> Beyond fixing that regression, I agree that this command is terminally
> flawed & we need to deprecate it & provide better specified new
> replacement(s). This seems like 0.14 work to me though.

Yup.

> Regards,
> Daniel
>
> [1] I know that they could already suffer if there was a bug in qemu
> that prevented it responding, even if the guest was not being
> malicious/crashed.

[Qemu-devel] Re: [PATCH][Tracing v2] Process -trace using QemuOptsList

[Stefan Hajnoczi]

On Fri, Aug 27, 2010 at 04:53:15PM +0530, Prerna Saxena wrote:
> [PATCH] Add -trace file FILENAME switch to qemu startup command.
>  This processes the argument using QemuOptsList
> 
> 
> Signed-off-by: Prerna Saxena 

Thanks, I will merge this for tracing v2.

Stefan

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Luiz Capitulino]

On Fri, 27 Aug 2010 15:59:21 +0200
Markus Armbruster  wrote:

> "Daniel P. Berrange"  writes:
> 
> > On Fri, Aug 27, 2010 at 07:39:37AM -0500, Anthony Liguori wrote:
> >> On 08/27/2010 04:29 AM, Daniel P. Berrange wrote:
> >> >On Fri, Aug 27, 2010 at 10:57:10AM +0530, Amit Shah wrote:
> >> >   
> >> >>This error message denotes some command was not successful in completing
> >> >>as the guest was unresponsive.
> >> >>
> >> >>Use it in the virtio-balloon code when showing older, cached data.
> >> >>
> >> >>Signed-off-by: Amit Shah
> >> >>---
> >> >>  hw/virtio-balloon.c |1 +
> >> >>  qerror.c|4 
> >> >>  qerror.h|3 +++
> >> >>  3 files changed, 8 insertions(+), 0 deletions(-)
> >> >>
> >> >>diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> >> >>index d6c66cf..309c343 100644
> >> >>--- a/hw/virtio-balloon.c
> >> >>+++ b/hw/virtio-balloon.c
> >> >>@@ -140,6 +140,7 @@ static void complete_stats_request(VirtIOBalloon *vb)
> >> >>
> >> >>  static void show_old_stats(void *opaque)
> >> >>  {
> >> >>+qerror_report(QERR_MACHINE_STOPPED);
> >> >>  complete_stats_request(opaque);
> >> >>  }
> >> >> 
> >> >
> >> >NACK. It has always been allowed&  valid to call query-balloon
> >> >to get the current balloon level. We must not throw an error
> >> >just because the recently added mem stats can't be refreshed.
> >> 
> >> I think that's a fair comment but why even bother fixing the command.  
> >> Let's introduce a new command that just gets a single piece of 
> >> information instead of having a command return lots of information.
> >
> > The existing query-balloon command that has been around for years &
> > is used by all current apps has a significant regression since we added
> > the memstats code to it: a guest can now trivially inflict a DOS on the
> > mgmt app if it crashes or is malicious. IMHO we need to fix that regression
> > for 0.13 so that existing apps don't suffer[1]. Adding a timeout to silently
> > skip the stats refresh if the guest doesn't respond, but without raising
> > an error seems the best tradeoff we can do here.
> 
> I agree.
> 
> Adding a roundtrip through the guest to an existing command was a
> mistake.

I wondered if we could drop it for now to make it right in 0.14, but I
believe it's already part of the user monitor for some time and libvirt
uses the stats, right?

I think we need testing/unstable namespace in QMP, where commands can be
tested for while so that we reduce the risk of nasty surprises like this one.

> 
> > Beyond fixing that regression, I agree that this command is terminally
> > flawed & we need to deprecate it & provide better specified new
> > replacement(s). This seems like 0.14 work to me though.
> 
> Yup.
> 
> > Regards,
> > Daniel
> >
> > [1] I know that they could already suffer if there was a bug in qemu
> > that prevented it responding, even if the guest was not being
> > malicious/crashed.
> 

[Qemu-devel] [PATCH 1 of 2] Introduce a new 'connected' xendev op called when Connected.

[Stefano Stabellini]

From: John Haxby 

Introduce a new 'connected' xendev op called when Connected.

Rename the existing xendev 'connect' op to 'initialised' and introduce
a new 'connected' op.  This new op, if defined, is called when the
backend is connected.  Note that since there is no state transition this
may be called more than once.

Signed-off-by: John Haxby 
Signed-off-by: Stefano Stabellini 


diff --git a/hw/xen_backend.c b/hw/xen_backend.c
index a2e408f..b99055a 100644
--- a/hw/xen_backend.c
+++ b/hw/xen_backend.c
@@ -400,13 +400,13 @@ static int xen_be_try_init(struct XenDevice *xendev)
 }
 
 /*
- * Try to connect xendev.  Depends on the frontend being ready
+ * Try to initialise xendev.  Depends on the frontend being ready
  * for it (shared ring and evtchn info in xenstore, state being
  * Initialised or Connected).
  *
  * Goes to Connected on success.
  */
-static int xen_be_try_connect(struct XenDevice *xendev)
+static int xen_be_try_initialise(struct XenDevice *xendev)
 {
 int rc = 0;
 
@@ -420,10 +420,10 @@ static int xen_be_try_connect(struct XenDevice *xendev)
}
 }
 
-if (xendev->ops->connect)
-   rc = xendev->ops->connect(xendev);
+if (xendev->ops->initialise)
+   rc = xendev->ops->initialise(xendev);
 if (rc != 0) {
-   xen_be_printf(xendev, 0, "connect() failed\n");
+   xen_be_printf(xendev, 0, "initialise() failed\n");
return rc;
 }
 
@@ -432,6 +432,28 @@ static int xen_be_try_connect(struct XenDevice *xendev)
 }
 
 /*
+ * Try to let xendev know that it is connected.  Depends on the
+ * frontend being Connected.  Note that this may be called more
+ * than once since the backend state is not modified.
+ */
+static void xen_be_try_connected(struct XenDevice *xendev)
+{
+if (!xendev->ops->connected)
+   return;
+
+if (xendev->fe_state != XenbusStateConnected) {
+   if (xendev->ops->flags & DEVOPS_FLAG_IGNORE_STATE) {
+   xen_be_printf(xendev, 2, "frontend not ready, ignoring\n");
+   } else {
+   xen_be_printf(xendev, 2, "frontend not ready (yet)\n");
+   return;
+   }
+}
+
+xendev->ops->connected(xendev);
+}
+
+/*
  * Teardown connection.
  *
  * Goes to Closed when done.
@@ -483,7 +505,12 @@ void xen_be_check_state(struct XenDevice *xendev)
rc = xen_be_try_init(xendev);
break;
case XenbusStateInitWait:
-   rc = xen_be_try_connect(xendev);
+   rc = xen_be_try_initialise(xendev);
+   break;
+   case XenbusStateConnected:
+   /* xendev->be_state doesn't change */
+   xen_be_try_connected(xendev);
+   rc = -1;
break;
 case XenbusStateClosed:
 rc = xen_be_try_reset(xendev);
diff --git a/hw/xen_backend.h b/hw/xen_backend.h
index cc25f9d..154922a 100644
--- a/hw/xen_backend.h
+++ b/hw/xen_backend.h
@@ -23,7 +23,8 @@ struct XenDevOps {
 uint32_t  flags;
 void  (*alloc)(struct XenDevice *xendev);
 int   (*init)(struct XenDevice *xendev);
-int   (*connect)(struct XenDevice *xendev);
+int   (*initialise)(struct XenDevice *xendev);
+void  (*connected)(struct XenDevice *xendev);
 void  (*event)(struct XenDevice *xendev);
 void  (*disconnect)(struct XenDevice *xendev);
 int   (*free)(struct XenDevice *xendev);
diff --git a/hw/xen_console.c b/hw/xen_console.c
index d2261f4..258c003 100644
--- a/hw/xen_console.c
+++ b/hw/xen_console.c
@@ -202,7 +202,7 @@ static int con_init(struct XenDevice *xendev)
 return 0;
 }
 
-static int con_connect(struct XenDevice *xendev)
+static int con_initialise(struct XenDevice *xendev)
 {
 struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
 int limit;
@@ -263,7 +263,7 @@ struct XenDevOps xen_console_ops = {
 .size   = sizeof(struct XenConsole),
 .flags  = DEVOPS_FLAG_IGNORE_STATE,
 .init   = con_init,
-.connect= con_connect,
+.initialise = con_initialise,
 .event  = con_event,
 .disconnect = con_disconnect,
 };
diff --git a/hw/xenfb.c b/hw/xenfb.c
index da5297b..b535d8c 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -359,7 +359,7 @@ static int input_init(struct XenDevice *xendev)
 return 0;
 }
 
-static int input_connect(struct XenDevice *xendev)
+static int input_initialise(struct XenDevice *xendev)
 {
 struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
 int rc;
@@ -861,7 +861,7 @@ static int fb_init(struct XenDevice *xendev)
 return 0;
 }
 
-static int fb_connect(struct XenDevice *xendev)
+static int fb_initialise(struct XenDevice *xendev)
 {
 struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev);
 struct xenfb_page *fb_page;
@@ -955,7 +955,7 @@ static void fb_event(struct XenDevice *xendev)
 struct XenDevOps xen_kbdmouse_ops = {
 .size   = sizeof(struct XenInput),
 .init   = input_init,
-.connect= input_connect,
+.initialise = input_i

[Qemu-devel] [PATCH 2 of 2] Move the xenfb pointer handler to the connected method

[Stefano Stabellini]

From: John Haxby 

Move the xenfb pointer handler to the connected method

Ensure that we read "request-abs-pointer" after the frontend has written
it.  This means that we will correctly set up an ansolute or relative
pointer handler correctly.

Signed-off-by: John Haxby 
Signed-off-by: Stefano Stabellini 


diff --git a/hw/xenfb.c b/hw/xenfb.c
index b535d8c..72e5277 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -364,19 +364,27 @@ static int input_initialise(struct XenDevice *xendev)
 struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
 int rc;
 
-if (xenstore_read_fe_int(xendev, "request-abs-pointer",
- &in->abs_pointer_wanted) == -1)
-   in->abs_pointer_wanted = 0;
-
 rc = common_bind(&in->c);
 if (rc != 0)
return rc;
 
 qemu_add_kbd_event_handler(xenfb_key_event, in);
+return 0;
+}
+
+static void input_connected(struct XenDevice *xendev)
+{
+struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
+
+if (xenstore_read_fe_int(xendev, "request-abs-pointer",
+ &in->abs_pointer_wanted) == -1)
+   in->abs_pointer_wanted = 0;
+
+if (in->qmouse)
+   qemu_remove_mouse_event_handler(in->qmouse);
 in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in,
  in->abs_pointer_wanted,
  "Xen PVFB Mouse");
-return 0;
 }
 
 static void input_disconnect(struct XenDevice *xendev)
@@ -956,6 +964,7 @@ struct XenDevOps xen_kbdmouse_ops = {
 .size   = sizeof(struct XenInput),
 .init   = input_init,
 .initialise = input_initialise,
+.connected  = input_connected,
 .disconnect = input_disconnect,
 .event  = input_event,
 };

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Anthony Liguori]

On 08/27/2010 09:15 AM, Luiz Capitulino wrote:


I wondered if we could drop it for now to make it right in 0.14, but I
believe it's already part of the user monitor for some time and libvirt
uses the stats, right?

I think we need testing/unstable namespace in QMP, where commands can be
tested for while so that we reduce the risk of nasty surprises like this one.
   


It's trying to plug a sieve with a band-aid.  It's certainly an 
"improvement" but it's of question utility looking at the bigger picture.


Balloon is a perfect example of where what we really need to do is build 
interface interfaces that make sense, and then expose them in QMP.


What's a reasonable C-level API to query statistics that potentially may 
never return?  Building in a timeout is something of a crappy API 
because it puts policy deep in the API that is trivial to implement 
elsewhere.  What you'd probably do is something like:


BalloonStatsRequest *query_guest_balloon_stats(CompletionCallback *cb, 
void *opaque);

int cancel_guest_balloon_stats(BalloonStatsRequest *req);
void release_guest_balloon_stats(BalloonStatsRequest *req);

Regards,

Anthony Liguori

 

Beyond fixing that regression, I agree that this command is terminally
flawed&  we need to deprecate it&  provide better specified new
replacement(s). This seems like 0.14 work to me though.
   

Yup.

 

Regards,
Daniel

[1] I know that they could already suffer if there was a bug in qemu
 that prevented it responding, even if the guest was not being
 malicious/crashed.
   
 
   

Re: [Qemu-devel] Re: Introduce a new 'connected' xendev op called when Connected.

[Stefano Stabellini]

On Mon, 23 Aug 2010, Anthony Liguori wrote:
> On 08/23/2010 05:21 AM, John Haxby wrote:
> >  Any reason why this (and its sister patch) were never picked up?
> >
> > jch
> 
> It was likely missed originally because there wasn't a [PATCH] in the 
> subject.  Can you resubmit?  It's not obvious to me what it's sister 
> patch is so I'd suggest resubmitting that too.
> 

done, thanks

Re: [Qemu-devel] [PATCH 2/4] scsi-disk: fix the mode data header returned by the MODE SENSE(10) command

[Bernhard Kohl]

Am 16.08.2010 19:02, schrieb ext Kevin Wolf:


> +if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM ||
> +bdrv_is_read_only(s->bs)) {

This looks like a mismerge. The check for CDROMs was removed when they
became read-only by definition. Please don't reintroduce it.


OK, I will remove that check in v2.


> +if (req->cmd.buf[0] == MODE_SENSE)
> +outbuf[3] = 8; /* Block descriptor length  */
> +else /* MODE_SENSE_10 */
> +outbuf[7] = 8; /* Block descriptor length  */

Please add curly braces here (see CODING_STYLE).


OK, I will add curly braces in v2.

Bernhard

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Anthony Liguori]

On 08/27/2010 10:33 AM, Daniel P. Berrange wrote:

On Fri, Aug 27, 2010 at 09:59:55AM -0500, Anthony Liguori wrote:
   

On 08/27/2010 09:15 AM, Luiz Capitulino wrote:
 

I wondered if we could drop it for now to make it right in 0.14, but I
believe it's already part of the user monitor for some time and libvirt
uses the stats, right?

I think we need testing/unstable namespace in QMP, where commands can be
tested for while so that we reduce the risk of nasty surprises like this
one.

   

It's trying to plug a sieve with a band-aid.  It's certainly an
"improvement" but it's of question utility looking at the bigger picture.

Balloon is a perfect example of where what we really need to do is build
interface interfaces that make sense, and then expose them in QMP.

What's a reasonable C-level API to query statistics that potentially may
never return?  Building in a timeout is something of a crappy API
because it puts policy deep in the API that is trivial to implement
elsewhere.  What you'd probably do is something like:

BalloonStatsRequest *query_guest_balloon_stats(CompletionCallback *cb,
void *opaque);
int cancel_guest_balloon_stats(BalloonStatsRequest *req);
void release_guest_balloon_stats(BalloonStatsRequest *req);
 

IMHO this is flawed because it does not allow you to fetch the
balloon stats independantly of asking the guest OS to refresh
them. So if the guest dies, it is not possible to ask QEMU
what the last known stats were.
   


The "last known" stats could be totally meaningless as they could be 
from 3 hours ago and there may have been a full reboot into a different 
OS since then.


Whether a "last known" stat is useful is a policy decision and belongs 
in the management tooling.



Although the current guest driver implementation requires the
host to request an update before the guest will refresh stats,
in theory someone could provide a guest driver impl thta does
a periodic push to the host without requiring a request.

Thus for a fully generalized access mechanism you need two
APIs and one event

  - API to request guest balloon stats update
  - API to get current known balloon stats
   


Current known balloon stats is not meaningful within qemu.


  - Event to notify client of a balloon stats update

And for the non-stats related balloon level we need

  - Event to notify client of balloon level change
   


An alternative API would avoid a 1-1 relationship between requests and 
responses.   That would mean:


void balloon_set_target(BalloonInterface *b, uint64_t value);
uint64_t balloon_get_STAT_NAME(BalloonInterface *b);

void balloon_request_update(BalloonInterface *b);

void balloon_add_stat_change_notifier(BalloonInterface *b, StatNotifer 
*notifer, uint64_t mask);
void balloon_del_stat_change_notifier(BalloonInterface *b, StatNotifier 
*notifier);


This API allows you to set and get individual stats.  Because there are 
individual getters/setters, it provides a better backwards/future 
compatibility and is pretty discoverable.  It offers no real indication 
of the age of the stat.  It's unclear to me if that's a good or bad thing.


balloon_request_update() requests that a guest update it's status but 
there's no specific guarantees in the interface.  A mask might prove 
valuable here.


Then there's an interface to register notifications of when the 
statistics are updated.  I think this probably would cover most use cases.


A main difference between this API is that the API guarantees absolutely 
no "freshness" to the statistics.  It provides an interface to freshen 
them but provides no guarantees about when that will happen and even if 
it will happen.


Regards,

Anthony Liguori


Even if the API to request a guest balloon stats update is fully async
and cancellable, you still need the event to notify client of the
balloon stats update, because in a multiple monitor scenario the
client wanting notification may be different to the client requesting
the refresh.

Regards,
Daniel
   

Re: [Qemu-devel] [PATCH 4/4] scsi-disk: fix the block descriptor returned by the MODE SENSE command

[Bernhard Kohl]

Am 16.08.2010 19:34, schrieb ext Kevin Wolf:


The patch itself looks okay. However, it made me wonder what this line
wants to tell us:

if ((~dbd) & nb_sectors) {

Is it just me or doesn't this make any sense at all? dbd is a single
bit, 0x8 if set or 0x0 otherwise. nb_sectors is the number of sectors.
Can this operation have any meaningful result? I suppose it was meant to
be something like:

if (!dbd && nb_sectors) {

Can you please check this and add a patch 5/4 if necessary?


For my opinion it is nonsense too. And it does not work. I tested it:

r...@grml ~ # sg_modes /dev/sdb -6 -v -H -H -p 0x3f
inquiry cdb: 12 00 00 00 24 00
QEMU  QEMU HARDDISK 0.13   peripheral_type: disk [0x0]
mode sense (6) cdb: 1a 00 3f 00 fc 00
mode sense (6): requested 252 bytes but got 220 bytes
Mode parameter header from MODE SENSE(6):
 00 1f 00 00 08
  Mode data length=32, medium type=0x00, WP=0, DpoFua=0, longlba=0
  Block descriptor length=8
> Direct access device block descriptors:
   Density code=0x0
 00 00 a0 00 00 00 00 02 00

>> page_code=0x8, page_control=0
 00 08 12 00 00 00 00 00 00  00 00 00 00 00 00 00 00
 10 00 00 00 00
r...@grml ~ # sg_modes /dev/sdb -6 -v -H -H -p 0x3f -d
inquiry cdb: 12 00 00 00 24 00
QEMU  QEMU HARDDISK 0.13   peripheral_type: disk [0x0]
mode sense (6) cdb: 1a 08 3f 00 fc 00
mode sense (6): requested 252 bytes but got 220 bytes
Mode parameter header from MODE SENSE(6):
 00 1f 00 00 08
  Mode data length=32, medium type=0x00, WP=0, DpoFua=0, longlba=0
  Block descriptor length=8
> Direct access device block descriptors:
   Density code=0x0
 00 00 a0 00 00 00 00 02 00

>> page_code=0x8, page_control=0
 00 08 12 00 00 00 00 00 00  00 00 00 00 00 00 00 00
 10 00 00 00 00
r...@grml ~ #

I will add a patch 5/5 in v2 as you proposed.

Bernhard

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Daniel P. Berrange]

On Fri, Aug 27, 2010 at 09:59:55AM -0500, Anthony Liguori wrote:
> On 08/27/2010 09:15 AM, Luiz Capitulino wrote:
> >
> >I wondered if we could drop it for now to make it right in 0.14, but I
> >believe it's already part of the user monitor for some time and libvirt
> >uses the stats, right?
> >
> >I think we need testing/unstable namespace in QMP, where commands can be
> >tested for while so that we reduce the risk of nasty surprises like this 
> >one.
> >   
> 
> It's trying to plug a sieve with a band-aid.  It's certainly an 
> "improvement" but it's of question utility looking at the bigger picture.
> 
> Balloon is a perfect example of where what we really need to do is build 
> interface interfaces that make sense, and then expose them in QMP.
> 
> What's a reasonable C-level API to query statistics that potentially may 
> never return?  Building in a timeout is something of a crappy API 
> because it puts policy deep in the API that is trivial to implement 
> elsewhere.  What you'd probably do is something like:
> 
> BalloonStatsRequest *query_guest_balloon_stats(CompletionCallback *cb, 
> void *opaque);
> int cancel_guest_balloon_stats(BalloonStatsRequest *req);
> void release_guest_balloon_stats(BalloonStatsRequest *req);

IMHO this is flawed because it does not allow you to fetch the
balloon stats independantly of asking the guest OS to refresh
them. So if the guest dies, it is not possible to ask QEMU
what the last known stats were.

Although the current guest driver implementation requires the
host to request an update before the guest will refresh stats,
in theory someone could provide a guest driver impl thta does
a periodic push to the host without requiring a request.

Thus for a fully generalized access mechanism you need two
APIs and one event

 - API to request guest balloon stats update
 - API to get current known balloon stats
 - Event to notify client of a balloon stats update

And for the non-stats related balloon level we need

 - Event to notify client of balloon level change


Even if the API to request a guest balloon stats update is fully async
and cancellable, you still need the event to notify client of the
balloon stats update, because in a multiple monitor scenario the
client wanting notification may be different to the client requesting
the refresh.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 3/4] scsi-disk: fix changeable values returned by the MODE SENSE command

[Bernhard Kohl]

Am 16.08.2010 19:12, schrieb ext Kevin Wolf:


> @@ -654,7 +656,8 @@ static int 
scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf)

>  p += 8;
>  }
>
> -switch (page) {
> +/* Don't return pages if Changeable Values (1) are requested. */
> +if (page_control != 1) switch (page) {

Coding style (missing braces, and switch belongs on its own line).


I restored that line to its original version in v2.


>  case 0x04:
>  case 0x05:
>  case 0x08:

I don't think this is enough. Wouldn't this still let the command return
successfully? In fact we need it to fail:

"If the logical unit does not implement changeable parameters mode pages
and the device server receives a MODE SENSE command with 01b in the PC
field, then the command shall be terminated with CHECK CONDITION status,
with the sense key set to ILLEGAL REQUEST, and the additional sense code
set to INVALID FIELD IN CDB."


This clause was not yet included in early SCSI-2 spec versions. For highest
compatibility I will implement the following in v2. I found this in all spec
versions:

"A PC field value of 1h requests that the target return a mask denoting
those mode parameters that are changeable. In the mask, the fields of
the mode parameters that are changeable shall be set to all one bits and
the fields of the mode parameters that are non-changeable (i.e. defined
by the target) shall be set to all zero bits."

By the way, my legacy OS fails, if MODE_SENSE returns non GOOD for PC=1.
I assume that the variant to return CHECK CONDITION for PC=1 is not
widely implemented by real devices.


This should do it if I'm not mistaken:

if (page_control == 0x01) {
return -1;
}


The same applies for Saved Values (PC=3) and unsupported page codes.
This is described in all spec versions too. I will implement this in v2.

Bernhard

[Qemu-devel] Re: [Xen-devel] Virtualization project idea

[Stefano Stabellini]

On Fri, 27 Aug 2010, Konrad Rzeszutek Wilk wrote:
> > > I'm an engineering student and is searching for a feasible project in
> > > virtualization. I'd like to know if its possible to share USB devices
> > > (flash drive, hard disk, mouse, keyboards etc) across guests and host
> > > (VMs). Also, do you have any idea to extend it? Or any innovative idea
> > > so that we can implement it.
> > 
> > Yes see comments wrt USB pass-through. What about implementing USB over
> > IP? Possibly via a VNC extension so that remote USB devices can be
> > attached to a VM. Since VNC provides a terminal with keyboard/mouse/gfx,
> > why not USB too? I believe vbox has this functionality except using the
> > RDP protocol.
> 
> That sounds pretty awesome. I think it might make sense to ask the QEMU
> folks about this - they have some VNC experts on their mailing list.
> 

actually SPICE supports USB AFAIK and Gerd is adding SPICE support to
qemu as we speak

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Luiz Capitulino]

On Fri, 27 Aug 2010 09:59:55 -0500
Anthony Liguori  wrote:

> On 08/27/2010 09:15 AM, Luiz Capitulino wrote:
> >
> > I wondered if we could drop it for now to make it right in 0.14, but I
> > believe it's already part of the user monitor for some time and libvirt
> > uses the stats, right?
> >
> > I think we need testing/unstable namespace in QMP, where commands can be
> > tested for while so that we reduce the risk of nasty surprises like this 
> > one.
> >
> 
> It's trying to plug a sieve with a band-aid.  It's certainly an 
> "improvement" but it's of question utility looking at the bigger picture.

Are you talking about the testing namespace idea? It doesn't have anything
to do with balloon or how our interfaces are built. It would be just a way
to play with commands that has been converted to QMP but are not available
because they're not stable yet (eg. Jan's device_show).

> Balloon is a perfect example of where what we really need to do is build 
> interface interfaces that make sense, and then expose them in QMP.

Main question is: can we drop the stats the way they are today to do the
Right Thing for 0.14 or not?

I think we have agreed on the internal interfaces approach. My only
concern is whether this will conflict when extending the wire protocol
(eg. adding new arguments to existing commands). Not a problem if the
C API is not stable, of course.

> What's a reasonable C-level API to query statistics that potentially may 
> never return?  Building in a timeout is something of a crappy API 
> because it puts policy deep in the API that is trivial to implement 
> elsewhere.  What you'd probably do is something like:
> 
> BalloonStatsRequest *query_guest_balloon_stats(CompletionCallback *cb, 
> void *opaque);
> int cancel_guest_balloon_stats(BalloonStatsRequest *req);

Shouldn't the API provide a general cancel method? All functions that
talk to the guest will need one.

> void release_guest_balloon_stats(BalloonStatsRequest *req);
> 
> Regards,
> 
> Anthony Liguori
> 
> >>  
> >>> Beyond fixing that regression, I agree that this command is terminally
> >>> flawed&  we need to deprecate it&  provide better specified new
> >>> replacement(s). This seems like 0.14 work to me though.
> >>>
> >> Yup.
> >>
> >>  
> >>> Regards,
> >>> Daniel
> >>>
> >>> [1] I know that they could already suffer if there was a bug in qemu
> >>>  that prevented it responding, even if the guest was not being
> >>>  malicious/crashed.
> >>>
> >>  
> >
> 

[Qemu-devel] [Bug 618533] Re: OpenSolaris guest fails to see the Solaris partitions of a physical disk in qemu-kvm-9999 (GIT)

[devsk]

what's the list address? All the lists at the kvm main page are for kvm
only.

-- 
OpenSolaris guest fails to see the Solaris partitions of a physical disk in 
qemu-kvm- (GIT)
https://bugs.launchpad.net/bugs/618533
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
# qemu-kvm --version
QEMU emulator version 0.13.50 (qemu-kvm-devel), Copyright (c) 2003-2008 Fabrice 
Bellard

The following disk is presented to guest as IDE disk with /dev/sdd as path.

# fdisk -l /dev/sdd

Disk /dev/sdd: 750.2 GB, 750156374016 bytes
255 heads, 63 sectors/track, 91201 cylinders, total 1465149168 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x7f3a03aa

   Device Boot  Start End  Blocks   Id  System
/dev/sdd1  63 7887914 3943926   1b  Hidden W95 FAT32
/dev/sdd2 7887915   980736119   486424102+  83  Linux
/dev/sdd3   *   980736120  108315049451207187+  bf  Solaris
/dev/sdd4  1083150495  1465144064   1909967855  Extended
/dev/sdd5  1083150558  110774600912297726   83  Linux
/dev/sdd6  1107746073  1465144064   1786989967  HPFS/NTFS

The prtvtoc output is attached from both VirtualBox and Qemu-KVM. As can be 
seen in the screenshots, a valid Solaris partition table (which is inside the 
/dev/sdd3, marked 'bf' in Linux fdisk output) is not seen in Qemu but seen in 
Virtualbox.

This makes the guest unbootable in Qemu because the root FS is not found but 
its bootable in Virtualbox.

Re: [Qemu-devel] Template for developing a Qemu device with PCIe and MSI-X

[Cam Macdonell]

On Wed, Aug 25, 2010 at 4:39 PM, Adnan Khaleel  wrote:
> Hi Isaku,
>
> I've made some progress in coding the device template but its no where near
> complete.
>
> I've created some files and am attaching it to this note. Based on what I
> could gather from the pcie source files I've made a stab at creating a
> simple model. I've also attached a file for a simple pci device that works
> under regular Qemu. I would like to duplicate its functionality in your pcie
> environment for starters.
>
> Could you please take a look at the files I've created and tell me if I've
> understood your pcie model correctly. Any help will be truly appreciated.
>
> Adnan

Hi Adnan,

There is a fairly simple device I've created called "ivshmem" that is
in the qemu git tree.  It is a regular PCI device that exports a
shared memory object via a BAR and supports a few registers and
optional MSI-X interrupts (I had to pick through the virtio code to
get MSI-X working, so looking at ivshmem might save you some effort).
My device is somewhat similar to a graphics card actually which I
recall is your goal.   The purpose of ivshmem is to support sharing
memory between multiple guests running on the same host.  It follows
the qdev model which you will need to do.

Cam

>
> The five files I've modified from your git repository are as follows
>
> hw/pci_ids.h    // Added vendor id defines
> hw/pc_q35.c    // Device instantiation
> hw/pcie_msix_template.h  // Device header file
> hw/pcie_msix_template.c  // Device file
> Makefile.objs   // Added pcie_msix_template.o to list of
> objects being built
>
> Everything should compile without any warnings or errors.
>
> The last file:
> sc_link_pci.c
> Is the original PCI device that I'm trying to convert into being PCIe and
> MSI-X and is included merely for reference to help you understand what I'd
> like to achieve in your environment.
>
>
> 
> From: Isaku Yamahata [mailto:yamah...@valinux.co.jp]
> To: Adnan Khaleel [mailto:ad...@khaleel.us]
> Cc: qemu-devel@nongnu.org
> Sent: Wed, 18 Aug 2010 22:19:04 -0500
> Subject: Re: [Qemu-devel] Template for developing a Qemu device with PCIe
> and MSI-X
>
> On Wed, Aug 18, 2010 at 02:10:10PM -0500, Adnan Khaleel wrote:
>> Hello Qemu developers,
>>
>> I'm interested in developing a device model that plugs into Qemu that is
>> based
>> on a PCIe interface and uses MSI-X. My goal is to ultimately attach a GPU
>> simulator to this PCIe interface and use the entire platfom (Qemu + GPU
>> simulator) for studying cpu, gpu interactions.
>>
>> I'm not terribly familiar with the Qemu device model and I'm looking for
>> some
>> assistance, perhaps a starting template for pcie and msi-x that would
>> offer the
>> basic functionality that I could then build upon.
>>
>> I have looked at the various devices that already modelled that are
>> included
>> with Qemu (v0.12.5 at least) and I've noticed several a few pci devices,
>> eg;
>> ne2k and cirrus-pci etc, however only one device truly seems to utilize
>> both
>> the technologies that I'm interested in and that is the virtio-pci.c
>>
>> I'm not sure what virtio-pci does so I'm not sure if that is a suitable
>> starting point for me.
>>
>> Any help, suggestions etc would be extremely helpful and much appreciated.
>
> Qemu doesn't support pcie at the moment.
> Only partial patches have been merged, still more patches have to
> be merged for pcie to fully work. The following repo is available.
>
> git clone http://people.valinux.co.jp/~yamahata/qemu/q35/qemu
> git clone http://people.valinux.co.jp/~yamahata/qemu/q35/seabios
> git clone http://people.valinux.co.jp/~yamahata/qemu/q35/vgabios
>
> Note: patched seabios and vgabios are needed, you have to pass ACPI DSDT
> for q35.
> example:
> qemu-system-x86_64 -M pc_q35 -acpitable
> load_header,data=roms/seabios/src/q35-acpi-dsdt.aml
>
> This repo is for those who want to try/develop pcie support,
> not for upstream merge. So they include patches unsuitable for upstream.
> The repo includes pcie port switch emulator which utilize pcie and
> MSI(not MSI-X).
>
> The difference between PCI device and PCIe device is configuration
> space size.
> By setting PCIDeviceInfo::is_express = 1, you'll get 4K configuration
> space. Helper functions for pcie are found in qemu/hw/pcie.c
> For msi-x, see qemu/hw/msix.c.
>
> Thanks,
> --
> yamahata
>

[Qemu-devel] Re: [Xen-devel] Virtualization project idea

[David Markey]

Wow, good news.

When are we porting our Xen stuff upstream? :)

On 27 August 2010 17:51, Stefano Stabellini <
stefano.stabell...@eu.citrix.com> wrote:

> On Fri, 27 Aug 2010, Konrad Rzeszutek Wilk wrote:
> > > > I'm an engineering student and is searching for a feasible project in
> > > > virtualization. I'd like to know if its possible to share USB devices
> > > > (flash drive, hard disk, mouse, keyboards etc) across guests and host
> > > > (VMs). Also, do you have any idea to extend it? Or any innovative
> idea
> > > > so that we can implement it.
> > >
> > > Yes see comments wrt USB pass-through. What about implementing USB over
> > > IP? Possibly via a VNC extension so that remote USB devices can be
> > > attached to a VM. Since VNC provides a terminal with
> keyboard/mouse/gfx,
> > > why not USB too? I believe vbox has this functionality except using the
> > > RDP protocol.
> >
> > That sounds pretty awesome. I think it might make sense to ask the QEMU
> > folks about this - they have some VNC experts on their mailing list.
> >
>
> actually SPICE supports USB AFAIK and Gerd is adding SPICE support to
> qemu as we speak
>
> ___
> Xen-devel mailing list
> xen-de...@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

[Qemu-devel] Re: [Xen-devel] Virtualization project idea

[Pasi Kärkkäinen]

On Fri, Aug 27, 2010 at 06:01:49PM +0100, David Markey wrote:
>Wow, good news.
>When are we porting our Xen stuff upstream? :)
>

I think that's in progress right now.. two rounds of patches already sent.

-- Pasi

Re: [Qemu-devel] [PATCH] qdev: Reset hotplugged devices

[Wei Xu]

Isaku and Anthony:

This is excellent discussion! Thanks for forwarding.

Wei Xu

we...@cisco.com


On 8/26/10 8:52 PM, "Isaku Yamahata"  wrote:

> I added CC for those who might be interested in this discussion.
> 
> On Thu, Aug 26, 2010 at 08:02:38AM -0500, Anthony Liguori wrote:
>> On 08/26/2010 03:38 AM, Isaku Yamahata wrote:
>>> 
 I think that starts by understanding exactly what's guaranteed and
 understanding what the use cases are for it.
  
>>> Fair enough. How about the followings?
>>>
>> 
>> Thanks for enumerating.
>> 
>>> This is just a starting point. I borrowed terminology pci/pcie spec.
>>> 
>>> 
>>> reset
>>> Bring the state of hardware state to consistent state.
>>> (some state might be left unknown.)
>>> 
>>> 
>>> system reset
>>> a hardware mechanism for setting or returning all hardware states
>>> to the initial conditions.
>>> Use case:
>>> In qemu, system_system_reset().
>>> 
>>> 
>>> cold reset(power on reset)
>>> system reset following the application of power.
>>> Use case:
>>> In qemu, system_reset() in main().
>>> We might want to use this as a power cycle.
>>> When a device is hot plugged, the device should be cold reset too.
>>> This is your motivation.
>>> QEMU_RESET_COLD
>>> Guarantee:
>>> The internal status must be same to qdev_init() + qdev_reset()
>>>
>> 
>> This is what we do today in QEMU and from a functional perspective it
>> covers the type of function we need today.
>> 
>>> 
>>> warm reset
>>> system reset without cycling the supplied power.
>>> Use case:
>>> In qemu, system_reset() in main_loop(). There are many places
>>> which calls qemu_system_reset_request().
>>> Some state are retained across warm reset. Like PCIe AER, error
>>> reporting registers need to keep its contents across warm reset
>>> as OS would examine them and report it when hardware errors caused
>>> warm reset.
>>> QEMU_RESET_WARM
>>>
>> 
>> With AER, I can't imagine that this matters that much unless we're doing
>> PCI passthrough, right?
> 
> Even without PCI passthrough, AER errors can be injected into emulated
> pci/pcie
> devices in a virtual pcie bus hierarchy from qemu command line.
> This is useful to test guest OS AER handler.
> 
> 
>> So maybe the way we should frame this discussion is, what's the type of
>> reset semantics that we need to support for PCI passthrough?  The next
>> question after that is how do we achieve the different types of reset
>> for passthrough devices?
> 
> What I want is hot reset in pcie terminology on a virtual bus as
> PCI_BRIDGE_CTL_BUS_RESET emulation and propagated reset on devices/child
> buses which might be a directly assigned.
> In direct assigned device case, device-assignment code would be notified the
> reset.
> 
> As hot reset has same effect to warm reset in functionality sense
> (the difference is the way to signal it in physical/signal layer which
> qemu doesn't care),
> I'd like to implement pci bus reset as triggering warm reset on a
> (virtual) bus by utilizing qdev frame work.
> This would be applicable to ata, scsi, I suppose.
> 
> 
> It's another story how to virtualize hot reset on a given directly assigned
> pci function or a pcie bus hierarchy. For example, as PCI device assignment
> is done per function basis, co-existing functions in the same card shouldn't
> be reset.
> Another example is, virtual pci bus hierarchy might be reset, but it would
> be difficult problem how to map the virtual bus topology to host bus topology.
> 
> thanks,
> 
>> BTW, if you could transfer some of this discussion to a wiki page on
>> qemu.org, I think that would be extremely valuable.
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>>> bus reset
>>> Reset bus and devices on the bus.
>>> Bus reset is usually triggered when cold reset, warm reset and
>>> commanding the bus controller to reset the child bus.
>>> When bus reset is triggered as command to bus controller,
>>> the effect is usually same to warm reset on devices on the bus.
>>> 
>>> Typically on parallel bus, bus reset is started by asserting
>>> a designated signal.
>>> Example: PCI RST#, ATA RESET-, SCSI RST
>>> 
>>> Use case:
>>> bus reset as result of programming bus controller.
>>> Qemu is currently missing it which I'd like to fill for pci bus.
>>> ATA and SCSI could benefit from this.
>>> QEMU_RESET_WARM with bus.
>>> Guarantee:
>>> device state under the bus is same as warm reset.
>>> 
>>> 
>>> device/function reset:
>>> Reset triggered by sending reset command to a device.
>>> This is bus/device specific.
>>> There might be many reset commands whose effects are different.
>>> Example: PCI FLR, ATA DEVICE RESET command,
>>>   scsi bus device reset message.
>>> 
>>> This reset is bus specific, so it wouldn't be suitable for qdev
>>> frame work and could be handled by each bus level.
>>> 
>>> 
>>> hot reset:
>>> I just put it here for completeness because pcie defines hot reset.
>>> A reset propagated in-band across a Link using a Physical Layer
>>>

[Qemu-devel] vnc reverse connection segfault

[David Weinstein]

Possible bug in qemu-0.12.4 on Linux, and I think applicable to qemu-0.12.5

The VNC reverse connection option appears to be parsed correctly,
however, the handling of the VncDisplay structure leads to a segfault:

Command line:
./i386-softmmu/qemu -vnc :9990,reverse -usb -monitor stdio
~/vmimg/linux-0.2.img
I took the liberty of hiding the ip address to protect the innocent ;-)

Error:
Switching to Thread 0x7fd99ceb06e0 (LWP 15174)]
0x004fa5df in vnc_refresh_server_surface (vd=0xd40e50) at vnc.c:2262
2262guest_row  = vd->guest.ds->data;

Backtrace:
(gdb) bt
#0  0x004fa5df in vnc_refresh_server_surface (vd=0xd40e50) at vnc.c:2262
#1  0x004fa872 in vnc_refresh (opaque=0xd40e50) at vnc.c:2303
#2  0x004fa9aa in vnc_init_timer (vd=0xd40e50) at vnc.c:2334
#3  0x004fab2d in vnc_connect (vd=0xd40e50, csock=0xc) at vnc.c:2377
#4  0x004fb226 in vnc_display_open (ds=0xc97b20,
display=0x7fffd871 ":9990,reverse") at vnc.c:2674
#5  0x0040fd1d in main (argc=0x9, argv=0x7fffb9c8,
envp=0x7fffba18) at /home/d/qemu/vl.c:6127


(gdb) p *vd
$2 = {
  timer = 0xcabbc0,
  timer_interval = 0x1e,
  lsock = 0x,
  ds = 0xc97b20,
  clients = 0xd82a10,
  kbd_layout = 0xcbf010,
  guest = {
dirty = {{0x0, 0x0, 0x0, 0x0} },
ds = 0x0
  },
  server = 0x0,
  display = 0xcb5f90 ":9990,reverse",
  password = 0x0,
  auth = 0x1
}

I'm going through the code to make sure I'm using the appropriate
option for a reverse VNC, but assuming I got that right I will then
look to see if there's a patch I might submit to fix this. However, if
someone else has already seen it...

Cheers,

David

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Anthony Liguori]

On 08/27/2010 11:08 AM, Luiz Capitulino wrote:

It's trying to plug a sieve with a band-aid.  It's certainly an
"improvement" but it's of question utility looking at the bigger picture.
 

Are you talking about the testing namespace idea? It doesn't have anything
to do with balloon or how our interfaces are built. It would be just a way
to play with commands that has been converted to QMP but are not available
because they're not stable yet (eg. Jan's device_show).
   


My point is that we shouldn't build any QMP APIs and we definitely 
shouldn't try to QMP-ize monitor commands.


Instead, we should design logical C APIs that we could consume within 
QEMU that we think we can support long term and then expose those over QMP.


Having a sandbox doesn't really solve the fundamental problem of making 
sure the interface is consumable.



Balloon is a perfect example of where what we really need to do is build
interface interfaces that make sense, and then expose them in QMP.
 

Main question is: can we drop the stats the way they are today to do the
Right Thing for 0.14 or not?
   


I don't see how 0.13.0 is going to get releases with anything but the 
current behavior.  It's unfortunate but we're too delayed and can't 
afford a change like this this late in the game.


In terms of the stable branch, the least disruptive thing would be a 
timeout.



I think we have agreed on the internal interfaces approach. My only
concern is whether this will conflict when extending the wire protocol
(eg. adding new arguments to existing commands). Not a problem if the
C API is not stable, of course.
   


We don't do that.  It's a recipe for disaster.  QEMU isn't written in 
Python and if we try to module our interfaces are if we were a Python 
library, we're destined to fail.



What's a reasonable C-level API to query statistics that potentially may
never return?  Building in a timeout is something of a crappy API
because it puts policy deep in the API that is trivial to implement
elsewhere.  What you'd probably do is something like:

BalloonStatsRequest *query_guest_balloon_stats(CompletionCallback *cb,
void *opaque);
int cancel_guest_balloon_stats(BalloonStatsRequest *req);
 

Shouldn't the API provide a general cancel method? All functions that
talk to the guest will need one.
   


See next proposal.  There's no cancel but I'd argue it's not needed.  
You don't care if the request succeeds or fails so there's no point in 
cancelling it.  Cancellation only works best when a request has a 
discrete life cycle but in the case of requesting a guest to update 
stats, there is not really a well define dstart and end.


Regards,

Anthony Liguori


void release_guest_balloon_stats(BalloonStatsRequest *req);

Regards,

Anthony Liguori

 


 

Beyond fixing that regression, I agree that this command is terminally
flawed&   we need to deprecate it&   provide better specified new
replacement(s). This seems like 0.14 work to me though.

   

Yup.


 

Regards,
Daniel

[1] I know that they could already suffer if there was a bug in qemu
  that prevented it responding, even if the guest was not being
  malicious/crashed.

   


 


   
 
   

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Luiz Capitulino]

On Fri, 27 Aug 2010 14:02:45 -0500
Anthony Liguori  wrote:

> On 08/27/2010 11:08 AM, Luiz Capitulino wrote:
> >> It's trying to plug a sieve with a band-aid.  It's certainly an
> >> "improvement" but it's of question utility looking at the bigger picture.
> >>  
> > Are you talking about the testing namespace idea? It doesn't have anything
> > to do with balloon or how our interfaces are built. It would be just a way
> > to play with commands that has been converted to QMP but are not available
> > because they're not stable yet (eg. Jan's device_show).
> >
> 
> My point is that we shouldn't build any QMP APIs and we definitely 
> shouldn't try to QMP-ize monitor commands.
> 
> Instead, we should design logical C APIs that we could consume within 
> QEMU that we think we can support long term and then expose those over QMP.

Comments on the Python comment below.

> Having a sandbox doesn't really solve the fundamental problem of making 
> sure the interface is consumable.

It doesn't and it shouldn't. It's just a way to test commands that might
not be stable yet. A very minor feature, anyway.

> >> Balloon is a perfect example of where what we really need to do is build
> >> interface interfaces that make sense, and then expose them in QMP.
> >>  
> > Main question is: can we drop the stats the way they are today to do the
> > Right Thing for 0.14 or not?
> >
> 
> I don't see how 0.13.0 is going to get releases with anything but the 
> current behavior.  It's unfortunate but we're too delayed and can't 
> afford a change like this this late in the game.
> 
> In terms of the stable branch, the least disruptive thing would be a 
> timeout.

Okay.

> > I think we have agreed on the internal interfaces approach. My only
> > concern is whether this will conflict when extending the wire protocol
> > (eg. adding new arguments to existing commands). Not a problem if the
> > C API is not stable, of course.
> >
> 
> We don't do that.  It's a recipe for disaster.  QEMU isn't written in 
> Python and if we try to module our interfaces are if we were a Python 
> library, we're destined to fail.

You mean we don't do simple protocol extensions?

So, if we need an new argument to an existing command, we add a new
command instead? Just because QEMU is not written in Python?

> >> What's a reasonable C-level API to query statistics that potentially may
> >> never return?  Building in a timeout is something of a crappy API
> >> because it puts policy deep in the API that is trivial to implement
> >> elsewhere.  What you'd probably do is something like:
> >>
> >> BalloonStatsRequest *query_guest_balloon_stats(CompletionCallback *cb,
> >> void *opaque);
> >> int cancel_guest_balloon_stats(BalloonStatsRequest *req);
> >>  
> > Shouldn't the API provide a general cancel method? All functions that
> > talk to the guest will need one.
> >
> 
> See next proposal.  There's no cancel but I'd argue it's not needed.  
> You don't care if the request succeeds or fails so there's no point in 
> cancelling it.  Cancellation only works best when a request has a 
> discrete life cycle but in the case of requesting a guest to update 
> stats, there is not really a well define dstart and end.
> 
> Regards,
> 
> Anthony Liguori
> 
> >> void release_guest_balloon_stats(BalloonStatsRequest *req);
> >>
> >> Regards,
> >>
> >> Anthony Liguori
> >>
> >>  
> 
>   
> > Beyond fixing that regression, I agree that this command is terminally
> > flawed&   we need to deprecate it&   provide better specified new
> > replacement(s). This seems like 0.14 work to me though.
> >
> >
>  Yup.
> 
> 
>   
> > Regards,
> > Daniel
> >
> > [1] I know that they could already suffer if there was a bug in qemu
> >   that prevented it responding, even if the guest was not being
> >   malicious/crashed.
> >
> >
> 
>   
> >>>
> >>>
> >>  
> >
> 

Re: [Qemu-devel] Re: Unusual physical address when using 64-bit BAR

[Cam Macdonell]

On Tue, Aug 24, 2010 at 8:21 PM, Isaku Yamahata  wrote:
> On Tue, Aug 24, 2010 at 10:52:36AM -0600, Cam Macdonell wrote:
>> Hi, 64-bit BARs still do not seem to be working.
>>
>> When using the latest seabios the guest does not hit a "BUG:"
>> statement, but booting still fails
>>
>> HPET: 1 timers in total, 0 timers will be used for per-cpu timer
>> divide error:  [#1] SMP
>> last sysfs file:
>> CPU 0
>> Modules linked in:
>>
>> Pid: 1, comm: swapper Not tainted 2.6.35+ #299 /Bochs
>> RIP: 0010:[]  [] hpet_alloc+0x12c/0x35b
>> RSP: 0018:88007d7b3d80  EFLAGS: 00010246
>> RAX: 00038d7ea4c68000 RBX: 88007d062cc0 RCX: 
>> RDX:  RSI:  RDI: 817bb9b0
>> RBP: 88007d7b3dc0 R08: 80d0 R09: c900
>> R10: 88007d72b5a0 R11:  R12: 88007d7b3dd0
>> R13: c900 R14:  R15: 817a41c3
>> FS:  () GS:88000200() knlGS:
>> CS:  0010 DS:  ES:  CR0: 8005003b
>> CR2:  CR3: 01a42000 CR4: 06f0
>> DR0:  DR1:  DR2: 
>> DR3:  DR6: 0ff0 DR7: 0400
>> Process swapper (pid: 1, threadinfo 88007d7b2000, task 88007d7b8000)
>> Stack:
>>  88007f43ab90 88007f43ab90 81ca6174 81b1f5e1
>> <0>  0100 0100 
>> <0> 88007d7b3e80 810294ea fed0 c900
>> Call Trace:
>>  [] ? hpet_late_init+0x0/0xea
>>  [] hpet_reserve_platform_timers+0x10b/0x115
>>  [] ? hpet_late_init+0x0/0xea
>>  [] hpet_late_init+0x6b/0xea
>>  [] ? hpet_late_init+0x0/0xea
>>  [] do_one_initcall+0x5e/0x159
>>  [] kernel_init+0x19a/0x228
>>  [] kernel_thread_helper+0x4/0x10
>>  [] ? kernel_init+0x0/0x228
>>  [] ? kernel_thread_helper+0x0/0x10
>> Code: 89 1d ca b2 b3 00 48 c1 ea 21 8b 73 34 49 c7 c7 c3 41 7a 81 48
>> 8d 04 02 4c 89 f2 48 c7 c7 b0 b9 7b 81 48 c1 ea 20 48 89 d1 31 d2 <48>
>> f7 f1 83 7b 30 01 48 c7 c1 86 1c 7d 81 49 0f 46 cf 48 89 43
>> RIP  [] hpet_alloc+0x12c/0x35b
>>  RSP 
>> ---[ end trace a7919e7f17c0a725 ]---
>> Kernel panic - not syncing: Attempted to kill init!
>> Pid: 1, comm: swapper Tainted: G      D     2.6.35+ #299
>> Call Trace:
>>  [] panic+0x8b/0x10b
>>  [] ? exit_ptrace+0x38/0x121
>>  [] do_exit+0x7a/0x722
>>  [] ? spin_unlock_irqrestore+0xe/0x10
>>  [] ? kmsg_dump+0x12b/0x145
>>  [] oops_end+0xbf/0xc7
>>  [] die+0x5a/0x63
>>  [] do_trap+0x121/0x130
>>  [] do_divide_error+0x96/0x9f
>>  [] ? hpet_alloc+0x12c/0x35b
>>  [] ? radix_tree_preload+0x34/0x88
>>  [] divide_error+0x1b/0x20
>>  [] ? hpet_alloc+0x12c/0x35b
>>  [] ? hpet_late_init+0x0/0xea
>>  [] hpet_reserve_platform_timers+0x10b/0x115
>>  [] ? hpet_late_init+0x0/0xea
>>  [] hpet_late_init+0x6b/0xea
>>  [] ? hpet_late_init+0x0/0xea
>>  [] do_one_initcall+0x5e/0x159
>>  [] kernel_init+0x19a/0x228
>>  [] kernel_thread_helper+0x4/0x10
>>  [] ? kernel_init+0x0/0x228
>>  [] ? kernel_thread_helper+0x0/0x10
>>
>> seabios output for the device:
>>
>> PCI: bus=0 devfn=0x20: vendor_id=0x1af4 device_id=0x1110
>> region 0: 0xf102
>> region 2: 0x
>> init smm
>>
>> Running the latest seabios, the debug output only remaps the BAR
>> twice, once with a potentially correct address of e
>>
>> pci_read_config: (val) 0xe004 <- 0x18 (addr)
>
> The upstream seabios lacks overflow check at the moment.
> I haven't found time to address PMM yet.
>
>
>> pci_default_write_config: (val) 0x0 -> 0x18 (addr)
>> IVSHMEM: guest pci addr = e000, guest h/w addr = 2164588544, size = 
>> 2000
>> pci_read_config: (val) 0xe004 <- 0x18 (addr)
>> pci_default_write_config: (val) 0x0 -> 0x18 (addr)
>> pci_read_config: (val) 0x0 <- 0x1c (addr)
>> pci_default_write_config: (val) 0x0 -> 0x1c (addr)
>> IVSHMEM: guest pci addr = , guest h/w addr =
>> 2164588544, size = 2000
>> pci_read_config: (val) 0x <- 0x1c (addr)
>> pci_default_write_config: (val) 0x0 -> 0x1c (addr)
>> pci_read_config: (val) 0x0 <- 0x20 (addr)
>>
>> the pci writes are all still 0, I can't see how my debug statements
>> are incorrect though.  Below is my trivial pci config debugging patch.
>
> The debug out put should be before the for-loop.
>
>    for (i = 0; i < l && addr + i < config_size; val >>= 8, ++i) {
>                                                 ^
>                                                 Here val becomes 0
>        uint8_t wmask = d->wmask[addr + i];
>        d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask);
>    }
>
> --
> yamahata
>
>

Ah, thanks.  I moved the debug statement and now the writes are the
proper values.

In seabios-kvm, it seems the guest is writing the address of c040 to
0x1c which results to the 48-bit address c040.

pci_read_config: (val) 0x1af4 <- 0x0 (addr)
pci_read_config: (val) 0x0 <- 

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Anthony Liguori]

On 08/27/2010 02:24 PM, Luiz Capitulino wrote:



I don't see how 0.13.0 is going to get releases with anything but the
current behavior.  It's unfortunate but we're too delayed and can't
afford a change like this this late in the game.

In terms of the stable branch, the least disruptive thing would be a
timeout.
 

Okay.

   

I think we have agreed on the internal interfaces approach. My only
concern is whether this will conflict when extending the wire protocol
(eg. adding new arguments to existing commands). Not a problem if the
C API is not stable, of course.

   

We don't do that.  It's a recipe for disaster.  QEMU isn't written in
Python and if we try to module our interfaces are if we were a Python
library, we're destined to fail.
 

You mean we don't do simple protocol extensions?

So, if we need an new argument to an existing command, we add a new
command instead? Just because QEMU is not written in Python?
   


Because it's too easy to get it wrong in QEMU.  Here's the rationale.

If I can't trivially call a QMP function in C, then I'm not going to use 
QMP functions within QEMU.  I'm not going to create an embedded JSON 
string just to call a function with three integer arguments.


Yes, if we need to do that, we can create a C API that both the QMP 
interface uses and we also use internally but why?  All that does is 
introduce the chance that the C API will have more features than the QMP 
interface.


If we don't use these functions in QEMU, then how do we know that these 
functions have reasonable semantics?  This is exactly the problem we 
suffer today.  We have internal APIs that do reasonable things but 
everything that deals with QMP is a special case.  That creates too many 
opportunities to get things wrong.


I think it's a vitally important requirement that all future QMP 
functions have direct mappings to a C interface.  The long term goal 
should be for that interface to be used by all of the command line 
arguments, SDL, and the human monitor.  If those things only relied on a 
single API and we exposed that API via QMP, than we would have an 
extremely useful interface.


Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Luiz Capitulino]

On Fri, 27 Aug 2010 14:37:54 -0500
Anthony Liguori  wrote:

> On 08/27/2010 02:24 PM, Luiz Capitulino wrote:
> >
> >> I don't see how 0.13.0 is going to get releases with anything but the
> >> current behavior.  It's unfortunate but we're too delayed and can't
> >> afford a change like this this late in the game.
> >>
> >> In terms of the stable branch, the least disruptive thing would be a
> >> timeout.
> >>  
> > Okay.
> >
> >
> >>> I think we have agreed on the internal interfaces approach. My only
> >>> concern is whether this will conflict when extending the wire protocol
> >>> (eg. adding new arguments to existing commands). Not a problem if the
> >>> C API is not stable, of course.
> >>>
> >>>
> >> We don't do that.  It's a recipe for disaster.  QEMU isn't written in
> >> Python and if we try to module our interfaces are if we were a Python
> >> library, we're destined to fail.
> >>  
> > You mean we don't do simple protocol extensions?
> >
> > So, if we need an new argument to an existing command, we add a new
> > command instead? Just because QEMU is not written in Python?
> >
> 
> Because it's too easy to get it wrong in QEMU.  Here's the rationale.
> 
> If I can't trivially call a QMP function in C, then I'm not going to use 
> QMP functions within QEMU.  I'm not going to create an embedded JSON 
> string just to call a function with three integer arguments.

... And as a QMP client I don't see the point in having to use a new
command just because a new argument was needed. The language QEMU is
written is also unimportant. The wire format is all I see and it's
language independent.

> Yes, if we need to do that, we can create a C API that both the QMP 
> interface uses and we also use internally but why?  All that does is 
> introduce the chance that the C API will have more features than the QMP 
> interface.

Why? I mean, what would stop us on extending QMP when the C interface is
also extended? Examples?

At first, this seems a good middle ground to me: QMP is implemented on
top of the C API and the JSON stuff is limited to QMP.

Of course that C API functions returning structured data will have to
create and return qobjects. But that's needed even with the direct
mapping.

> If we don't use these functions in QEMU, then how do we know that these 
> functions have reasonable semantics?  This is exactly the problem we 
> suffer today.  We have internal APIs that do reasonable things but 
> everything that deals with QMP is a special case.  That creates too many 
> opportunities to get things wrong.
> 
> I think it's a vitally important requirement that all future QMP 
> functions have direct mappings to a C interface.  The long term goal 
> should be for that interface to be used by all of the command line 
> arguments, SDL, and the human monitor.  If those things only relied on a 
> single API and we exposed that API via QMP, than we would have an 
> extremely useful interface.

I agree, I just think an indirect mapping would be more beneficial to
QMP clients.

[Qemu-devel] [PATCH 0/5] virtio-net: More configurability and bh handling for tx

[Alex Williamson]

Add the ability to configure the tx_timer timeout and add a bottom
half tx handler that typically shows a nice perf boost over the
time based approach.  See last patch for perf details.  Make this
the new default when the iothread is enabled.  Thanks,

Alex

---

Alex Williamson (5):
  virtio-net: Switch default to new bottom half TX handler for iothread
  virtio-net: Introduce a new bottom half packet TX
  virtio-net: Rename tx_timer_active to tx_waiting
  virtio-net: Limit number of packets sent per TX flush
  virtio-net: Make tx_timer timeout configurable


 hw/s390-virtio-bus.c |6 ++
 hw/s390-virtio-bus.h |4 ++
 hw/syborg_virtio.c   |   10 +++-
 hw/virtio-net.c  |  129 --
 hw/virtio-net.h  |2 -
 hw/virtio-pci.c  |   10 +++-
 hw/virtio.h  |9 +++
 7 files changed, 137 insertions(+), 33 deletions(-)

[Qemu-devel] [PATCH 2/5] virtio-net: Limit number of packets sent per TX flush

[Alex Williamson]

If virtio_net_flush_tx is called with notification disabled, we can
race with the guest, processing packets at the same rate as they
get produced.  The trouble is that this means we have no guaranteed
exit condition from the function and can spend minutes in there.
Currently flush_tx is only called with notification on, which seems
to limit us to one pass through the queue per call.  An upcoming
patch changes this.

One pass through the queue (256) seems to be a good default value
for this, balancing latency with throughput.  We use a signed int
for txburst because 2^31 packets in a burst would take many, many
minutes to process and it allows us to easily return a negative
value value from virtio_net_flush_tx() to indicate a back-off
or error condition.

Signed-off-by: Alex Williamson 
---

 hw/s390-virtio-bus.c |4 +++-
 hw/s390-virtio-bus.h |2 ++
 hw/syborg_virtio.c   |6 +-
 hw/virtio-net.c  |   23 ---
 hw/virtio-pci.c  |6 +-
 hw/virtio.h  |2 +-
 6 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 4da0b40..1483362 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -110,7 +110,8 @@ static int s390_virtio_net_init(VirtIOS390Device *dev)
 {
 VirtIODevice *vdev;
 
-vdev = virtio_net_init((DeviceState *)dev, &dev->nic, dev->txtimer);
+vdev = virtio_net_init((DeviceState *)dev, &dev->nic,
+   dev->txtimer, dev->txburst);
 if (!vdev) {
 return -1;
 }
@@ -328,6 +329,7 @@ static VirtIOS390DeviceInfo s390_virtio_net = {
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic),
 DEFINE_PROP_UINT32("txtimer", VirtIOS390Device, txtimer, 1),
+DEFINE_PROP_INT32("txburst", VirtIOS390Device, txburst, 256),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index 922daf2..808aea0 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -45,6 +45,8 @@ typedef struct VirtIOS390Device {
 uint32_t max_virtserial_ports;
 /* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */
 uint32_t txtimer;
+/* Max tx packets for virtio-net to burst at a time */
+int32_t txburst;
 } VirtIOS390Device;
 
 typedef struct VirtIOS390Bus {
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index c8d731a..7b76972 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -70,6 +70,8 @@ typedef struct {
 uint32_t host_features;
 /* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */
 uint32_t txtimer;
+/* Max tx packets for virtio-net to burst at a time */
+int32_t txburst;
 } SyborgVirtIOProxy;
 
 static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
@@ -286,7 +288,8 @@ static int syborg_virtio_net_init(SysBusDevice *dev)
 VirtIODevice *vdev;
 SyborgVirtIOProxy *proxy = FROM_SYSBUS(SyborgVirtIOProxy, dev);
 
-vdev = virtio_net_init(&dev->qdev, &proxy->nic, proxy->txtimer);
+vdev = virtio_net_init(&dev->qdev, &proxy->nic,
+   proxy->txtimer, proxy->txburst);
 return syborg_virtio_init(proxy, vdev);
 }
 
@@ -298,6 +301,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = {
 DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic),
 DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features),
 DEFINE_PROP_UINT32("txtimer", SyborgVirtIOProxy, txtimer, 1),
+DEFINE_PROP_INT32("txburst", SyborgVirtIOProxy, txburst, 256),
 DEFINE_PROP_END_OF_LIST(),
 }
 };
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 9ef29f0..ac4aa8f 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -37,6 +37,7 @@ typedef struct VirtIONet
 NICState *nic;
 QEMUTimer *tx_timer;
 uint32_t tx_timeout;
+int32_t tx_burst;
 int tx_timer_active;
 uint32_t has_vnet_hdr;
 uint8_t has_ufo;
@@ -620,7 +621,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc, 
const uint8_t *buf, size_
 return size;
 }
 
-static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq);
+static int32_t virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq);
 
 static void virtio_net_tx_complete(VLANClientState *nc, ssize_t len)
 {
@@ -636,16 +637,18 @@ static void virtio_net_tx_complete(VLANClientState *nc, 
ssize_t len)
 }
 
 /* TX */
-static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq)
+static int32_t virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq)
 {
 VirtQueueElement elem;
+int32_t num_packets = 0;
 
-if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
-return;
+if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+return num_packets;
+}
 
 if (n->async_tx.elem.out_num) {
 virtio_queue_set_notification(n->tx_vq, 0);
-return;
+return num_packets;
 }
 
 while (virtqueue_pop(vq, &elem)) {
@@ -682,14 +685,19 @@ static void virti

[Qemu-devel] [PATCH 1/5] virtio-net: Make tx_timer timeout configurable

[Alex Williamson]

The tx_timer used for TX mitigation in virtio-net has a hard coded
timeout.  Make an option for this to be configurable using a
txtimer= device config option.  Note that we reserve a value of
"1" to simply mean use the default, we'll later use the value "0"
to disable the timer.  Everything else is the timeout in ns.  We
limit the timeout to a uint32_t, because anything over a 4s timeout
probably doens't make any kind of practical sense.

Signed-off-by: Alex Williamson 
---

 hw/s390-virtio-bus.c |3 ++-
 hw/s390-virtio-bus.h |2 ++
 hw/syborg_virtio.c   |5 -
 hw/virtio-net.c  |   17 ++---
 hw/virtio-net.h  |2 --
 hw/virtio-pci.c  |5 -
 hw/virtio.h  |3 ++-
 7 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index fe6884d..4da0b40 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -110,7 +110,7 @@ static int s390_virtio_net_init(VirtIOS390Device *dev)
 {
 VirtIODevice *vdev;
 
-vdev = virtio_net_init((DeviceState *)dev, &dev->nic);
+vdev = virtio_net_init((DeviceState *)dev, &dev->nic, dev->txtimer);
 if (!vdev) {
 return -1;
 }
@@ -327,6 +327,7 @@ static VirtIOS390DeviceInfo s390_virtio_net = {
 .qdev.size = sizeof(VirtIOS390Device),
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic),
+DEFINE_PROP_UINT32("txtimer", VirtIOS390Device, txtimer, 1),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index 333fea8..922daf2 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -43,6 +43,8 @@ typedef struct VirtIOS390Device {
 uint32_t host_features;
 /* Max. number of ports we can have for a the virtio-serial device */
 uint32_t max_virtserial_ports;
+/* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */
+uint32_t txtimer;
 } VirtIOS390Device;
 
 typedef struct VirtIOS390Bus {
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index abf0370..c8d731a 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -68,6 +68,8 @@ typedef struct {
 uint32_t id;
 NICConf nic;
 uint32_t host_features;
+/* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */
+uint32_t txtimer;
 } SyborgVirtIOProxy;
 
 static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
@@ -284,7 +286,7 @@ static int syborg_virtio_net_init(SysBusDevice *dev)
 VirtIODevice *vdev;
 SyborgVirtIOProxy *proxy = FROM_SYSBUS(SyborgVirtIOProxy, dev);
 
-vdev = virtio_net_init(&dev->qdev, &proxy->nic);
+vdev = virtio_net_init(&dev->qdev, &proxy->nic, proxy->txtimer);
 return syborg_virtio_init(proxy, vdev);
 }
 
@@ -295,6 +297,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = {
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic),
 DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features),
+DEFINE_PROP_UINT32("txtimer", SyborgVirtIOProxy, txtimer, 1),
 DEFINE_PROP_END_OF_LIST(),
 }
 };
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 075f72d..9ef29f0 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -36,6 +36,7 @@ typedef struct VirtIONet
 VirtQueue *ctrl_vq;
 NICState *nic;
 QEMUTimer *tx_timer;
+uint32_t tx_timeout;
 int tx_timer_active;
 uint32_t has_vnet_hdr;
 uint8_t has_ufo;
@@ -702,7 +703,7 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, 
VirtQueue *vq)
 virtio_net_flush_tx(n, vq);
 } else {
 qemu_mod_timer(n->tx_timer,
-   qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
+   qemu_get_clock(vm_clock) + n->tx_timeout);
 n->tx_timer_active = 1;
 virtio_queue_set_notification(vq, 0);
 }
@@ -842,7 +843,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int 
version_id)
 
 if (n->tx_timer_active) {
 qemu_mod_timer(n->tx_timer,
-   qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
+   qemu_get_clock(vm_clock) + n->tx_timeout);
 }
 return 0;
 }
@@ -903,7 +904,8 @@ static void virtio_net_vmstate_change(void *opaque, int 
running, int reason)
 virtio_net_set_status(&n->vdev, n->vdev.status & status);
 }
 
-VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf)
+VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
+  uint32_t txtimer)
 {
 VirtIONet *n;
 
@@ -931,6 +933,15 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
*conf)
 
 n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n);
 n->tx_timer_active = 0;
+if (txtimer) {
+if (txtimer == 1) {
+/* For convenience, 1 = "on" = predefined default, anything else
+ * specifies and actual timeout value */
+n->tx_timeout = 15; /* 150 us */
+} e

[Qemu-devel] [PATCH 3/5] virtio-net: Rename tx_timer_active to tx_waiting

[Alex Williamson]

De-couple this from the timer since we might want to use
different backends to send the packet.

Signed-off-by: Alex Williamson 
---

 hw/virtio-net.c |   18 +-
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ac4aa8f..8b652f2 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -38,7 +38,7 @@ typedef struct VirtIONet
 QEMUTimer *tx_timer;
 uint32_t tx_timeout;
 int32_t tx_burst;
-int tx_timer_active;
+int tx_waiting;
 uint32_t has_vnet_hdr;
 uint8_t has_ufo;
 struct {
@@ -704,15 +704,15 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, 
VirtQueue *vq)
 {
 VirtIONet *n = to_virtio_net(vdev);
 
-if (n->tx_timer_active) {
+if (n->tx_waiting) {
 virtio_queue_set_notification(vq, 1);
 qemu_del_timer(n->tx_timer);
-n->tx_timer_active = 0;
+n->tx_waiting = 0;
 virtio_net_flush_tx(n, vq);
 } else {
 qemu_mod_timer(n->tx_timer,
qemu_get_clock(vm_clock) + n->tx_timeout);
-n->tx_timer_active = 1;
+n->tx_waiting = 1;
 virtio_queue_set_notification(vq, 0);
 }
 }
@@ -721,7 +721,7 @@ static void virtio_net_tx_timer(void *opaque)
 {
 VirtIONet *n = opaque;
 
-n->tx_timer_active = 0;
+n->tx_waiting = 0;
 
 /* Just in case the driver is not ready on more */
 if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
@@ -744,7 +744,7 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
 virtio_save(&n->vdev, f);
 
 qemu_put_buffer(f, n->mac, ETH_ALEN);
-qemu_put_be32(f, n->tx_timer_active);
+qemu_put_be32(f, n->tx_waiting);
 qemu_put_be32(f, n->mergeable_rx_bufs);
 qemu_put_be16(f, n->status);
 qemu_put_byte(f, n->promisc);
@@ -773,7 +773,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int 
version_id)
 virtio_load(&n->vdev, f);
 
 qemu_get_buffer(f, n->mac, ETH_ALEN);
-n->tx_timer_active = qemu_get_be32(f);
+n->tx_waiting = qemu_get_be32(f);
 n->mergeable_rx_bufs = qemu_get_be32(f);
 
 if (version_id >= 3)
@@ -849,7 +849,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int 
version_id)
 }
 n->mac_table.first_multi = i;
 
-if (n->tx_timer_active) {
+if (n->tx_waiting) {
 qemu_mod_timer(n->tx_timer,
qemu_get_clock(vm_clock) + n->tx_timeout);
 }
@@ -940,7 +940,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
*conf,
 qemu_format_nic_info_str(&n->nic->nc, conf->macaddr.a);
 
 n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n);
-n->tx_timer_active = 0;
+n->tx_waiting = 0;
 if (txtimer) {
 if (txtimer == 1) {
 /* For convenience, 1 = "on" = predefined default, anything else

[Qemu-devel] [PATCH 5/5] virtio-net: Switch default to new bottom half TX handler for iothread

[Alex Williamson]

The bottom half handler shows big improvements over the timer
with few downsides, default to it when the iothread is enabled.

Using the following tests, with the guest and host connected
via tap+bridge:

guest> netperf -t TCP_STREAM -H $HOST
host> netperf -t TCP_STREAM -H $GUEST
guest> netperf -t UDP_STREAM -H $HOST
host> netperf -t UDP_STREAM -H $GUEST
guest> netperf -t TCP_RR -H $HOST

Results: base throughput, exits/throughput ->
   patched throughput, exits/throughput

--enable-io-thread
TCP guest->host 2737.77, 47.82  -> 6767.09, 29.15 = 247%, 61%
TCP host->guest 2231.33, 74.00  -> 4125.80, 67.61 = 185%, 91%
UDP guest->host 6281.68, 14.66  -> 12569.27, 1.98 = 200%, 14%
UDP host->guest 275.91,  289.22 -> 264.80, 293.53 = 96%, 101%
interations/s   1949.65, 82.97  -> 7417.56, 84.31 = 380%, 102%

No --enable-io-thread
TCP guest->host 3041.57, 55.11 -> 1038.93, 517.57 = 34%, 939%
TCP host->guest 2416.03, 76.67 -> 5655.92, 55.52  = 234%, 72%
UDP guest->host 12255.82, 6.11 -> 7775.87, 31.32  = 63%, 513%
UDP host->guest 587.92, 245.95 -> 611.88, 239.92  = 104%, 98%
interations/s   1975.59, 83.21 -> 8935.50, 88.18  = 452%, 106%

Signed-off-by: Alex Williamson 
---

 hw/s390-virtio-bus.c |3 ++-
 hw/syborg_virtio.c   |3 ++-
 hw/virtio-pci.c  |3 ++-
 hw/virtio.h  |6 ++
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 1483362..985f99a 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -328,7 +328,8 @@ static VirtIOS390DeviceInfo s390_virtio_net = {
 .qdev.size = sizeof(VirtIOS390Device),
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic),
-DEFINE_PROP_UINT32("txtimer", VirtIOS390Device, txtimer, 1),
+DEFINE_PROP_UINT32("txtimer", VirtIOS390Device, txtimer,
+   TXTIMER_DEFAULT),
 DEFINE_PROP_INT32("txburst", VirtIOS390Device, txburst, 256),
 DEFINE_PROP_END_OF_LIST(),
 },
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index 7b76972..ee5746d 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -300,7 +300,8 @@ static SysBusDeviceInfo syborg_virtio_net_info = {
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic),
 DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features),
-DEFINE_PROP_UINT32("txtimer", SyborgVirtIOProxy, txtimer, 1),
+DEFINE_PROP_UINT32("txtimer", SyborgVirtIOProxy, txtimer,
+   TXTIMER_DEFAULT),
 DEFINE_PROP_INT32("txburst", SyborgVirtIOProxy, txburst, 256),
 DEFINE_PROP_END_OF_LIST(),
 }
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index e025c09..9740f57 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -695,7 +695,8 @@ static PCIDeviceInfo virtio_info[] = {
 DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
 DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
 DEFINE_NIC_PROPERTIES(VirtIOPCIProxy, nic),
-DEFINE_PROP_UINT32("txtimer", VirtIOPCIProxy, txtimer, 1),
+DEFINE_PROP_UINT32("txtimer", VirtIOPCIProxy, txtimer,
+   TXTIMER_DEFAULT),
 DEFINE_PROP_INT32("txburst", VirtIOPCIProxy, txburst, 256),
 DEFINE_PROP_END_OF_LIST(),
 },
diff --git a/hw/virtio.h b/hw/virtio.h
index 4051889..a1a17a2 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -183,6 +183,12 @@ void virtio_update_irq(VirtIODevice *vdev);
 void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
 void *opaque);
 
+#ifdef CONFIG_IOTHREAD
+ #define TXTIMER_DEFAULT 0
+#else
+ #define TXTIMER_DEFAULT 1
+#endif
+
 /* Base devices.  */
 VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf);
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,

[Qemu-devel] [PATCH 4/5] virtio-net: Introduce a new bottom half packet TX

[Alex Williamson]

Based on a patch from Mark McLoughlin, this patch introduces a new
bottom half packet transmitter that avoids the latency imposed by
the tx_timer approach.  Rather than scheduling a timer when a TX
packet comes in, schedule a bottom half to be run from the iothread.
The bottom half handler first attempts to flush the queue with
notification disabled (this is where we could race with a guest
without txburst).  If we flush a full burst, reschedule immediately.
If we send short of a full burst, try to re-enable notification.
To avoid a race with TXs that may have occurred, we must then
flush again.  If we find some packets to send, the guest it probably
active, so we can reschedule again.

tx_timer and tx_bh are mutually exclusive, so we can re-use the
tx_waiting flag to indicate one or the other needs to be setup.
This allows us to seamlessly migrate between timer and bh TX
handling.

Signed-off-by: Alex Williamson 
---

 hw/virtio-net.c |   81 ++-
 1 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 8b652f2..3288c77 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -36,6 +36,7 @@ typedef struct VirtIONet
 VirtQueue *ctrl_vq;
 NICState *nic;
 QEMUTimer *tx_timer;
+QEMUBH *tx_bh;
 uint32_t tx_timeout;
 int32_t tx_burst;
 int tx_waiting;
@@ -704,16 +705,25 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, 
VirtQueue *vq)
 {
 VirtIONet *n = to_virtio_net(vdev);
 
-if (n->tx_waiting) {
-virtio_queue_set_notification(vq, 1);
-qemu_del_timer(n->tx_timer);
-n->tx_waiting = 0;
-virtio_net_flush_tx(n, vq);
+if (n->tx_timer) {
+if (n->tx_waiting) {
+virtio_queue_set_notification(vq, 1);
+qemu_del_timer(n->tx_timer);
+n->tx_waiting = 0;
+virtio_net_flush_tx(n, vq);
+} else {
+qemu_mod_timer(n->tx_timer,
+   qemu_get_clock(vm_clock) + n->tx_timeout);
+n->tx_waiting = 1;
+virtio_queue_set_notification(vq, 0);
+}
 } else {
-qemu_mod_timer(n->tx_timer,
-   qemu_get_clock(vm_clock) + n->tx_timeout);
+if (unlikely(n->tx_waiting)) {
+return;
+}
+virtio_queue_set_notification(n->tx_vq, 0);
+qemu_bh_schedule(n->tx_bh);
 n->tx_waiting = 1;
-virtio_queue_set_notification(vq, 0);
 }
 }
 
@@ -731,6 +741,41 @@ static void virtio_net_tx_timer(void *opaque)
 virtio_net_flush_tx(n, n->tx_vq);
 }
 
+static void virtio_net_tx_bh(void *opaque)
+{
+VirtIONet *n = opaque;
+int32_t ret;
+
+n->tx_waiting = 0;
+
+/* Just in case the driver is not ready on more */
+if (unlikely(!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)))
+return;
+
+ret = virtio_net_flush_tx(n, n->tx_vq);
+if (ret == -EBUSY) {
+return; /* Notification re-enable handled by tx_complete */
+}
+
+/* If we flush a full burst of packets, assume there are
+ * more coming and immediately reschedule */
+if (ret >= n->tx_burst) {
+qemu_bh_schedule(n->tx_bh);
+n->tx_waiting = 1;
+return;
+}
+
+/* If less than a full burst, re-enable notification and flush
+ * anything that may have come in while we weren't looking.  If
+ * we find something, assume the guest is still active and reschedule */
+virtio_queue_set_notification(n->tx_vq, 1);
+if (virtio_net_flush_tx(n, n->tx_vq) > 0) {
+virtio_queue_set_notification(n->tx_vq, 0);
+qemu_bh_schedule(n->tx_bh);
+n->tx_waiting = 1;
+}
+}
+
 static void virtio_net_save(QEMUFile *f, void *opaque)
 {
 VirtIONet *n = opaque;
@@ -850,8 +895,12 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int 
version_id)
 n->mac_table.first_multi = i;
 
 if (n->tx_waiting) {
-qemu_mod_timer(n->tx_timer,
-   qemu_get_clock(vm_clock) + n->tx_timeout);
+if (n->tx_timer) {
+qemu_mod_timer(n->tx_timer,
+   qemu_get_clock(vm_clock) + n->tx_timeout);
+} else {
+qemu_bh_schedule(n->tx_bh);
+}
 }
 return 0;
 }
@@ -939,9 +988,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
*conf,
 
 qemu_format_nic_info_str(&n->nic->nc, conf->macaddr.a);
 
-n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n);
 n->tx_waiting = 0;
 if (txtimer) {
+n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n);
 if (txtimer == 1) {
 /* For convenience, 1 = "on" = predefined default, anything else
  * specifies and actual timeout value */
@@ -949,6 +998,8 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
*conf,
 } else {
 n->tx_timeout = txtimer;
 }
+} else {
+n->tx_bh = qemu_bh_new(virtio_net_

[Qemu-devel] webcam under windows xp guest

[Frans de Boer]

I have searched the Internet, but could not find conclusive answers. I
did find a lot of questions, but that's about it.

I run Linux, QEMU/KVM 0.12.5 and have loaded windows XP.
My webcam is being recognized by Windows XP, but all I get is a black
screen when I try to use it. I use the -usbdevice host:vendor:id
directive. The light goes on (and never off again until I reboot the
physical machine).

Since this problem is not new, I just wonder what might be wrong?

Regards,
Frans.

[Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message

[Amit Shah]

On (Fri) Aug 27 2010 [07:39:37], Anthony Liguori wrote:
> >
> >NACK. It has always been allowed&  valid to call query-balloon
> >to get the current balloon level. We must not throw an error
> >just because the recently added mem stats can't be refreshed.
> 
> I think that's a fair comment but why even bother fixing the
> command.  Let's introduce a new command that just gets a single
> piece of information instead of having a command return lots of
> information.

Instead, let's have query-balloon do the same thing as it did in 0.12
and add a new command for 0.13 (query-balloon-stats) that does the stats
with timeout?

That way we won't have regressions from 0.12 and have the new behaviour
only for 0.13, which we can say is 'experimental'.

Amit

Re: [Qemu-devel] webcam under windows xp guest

[Mulyadi Santosa]

On Sat, Aug 28, 2010 at 05:07, Frans de Boer  wrote:
> I run Linux, QEMU/KVM 0.12.5 and have loaded windows XP.
> My webcam is being recognized by Windows XP, but all I get is a black
> screen when I try to use it. I use the -usbdevice host:vendor:id
> directive. The light goes on (and never off again until I reboot the
> physical machine).

Perhaps just lending idea...how about running usb snooper like
http://usbsnoop.sourceforge.net/ and put the log somewhere so some
bright people here could figure it out more precisely?

-- 
regards,

Mulyadi Santosa
Freelance Linux trainer and consultant

blog: the-hydra.blogspot.com
training: mulyaditraining.blogspot.com

Re: [Qemu-devel] [PATCH v4 00/10] initial spice support.

[Blue Swirl]

On Fri, Aug 27, 2010 at 9:59 AM, Gerd Hoffmann  wrote:
>  Hi,
>
> Here comes v4 of the iniial spice support patch series, hopefully the
> final version.  It brings just the very basic bits:
>
>  * Detect spice in configure, Makefile windup.
>  * Support for keyboard, mouse and tablet.
>  * Support for simple display output (works as DisplayChangeListener,
>   plays with any gfx card, sends simple draw commands to update
>   dirty regions).
>
> Note that this patch series does *not* yet contain the qxl paravirtual
> gfx card.  That will come as part of a additional patch series after
> sorting the vgabios support.
>
> The patches are also available in the git repository at:
>  git://anongit.freedesktop.org/spice/qemu submit.4

Please read CODING_STYLE, especially the braces rule.

Re: [Qemu-devel] [Bug 618533] Re: OpenSolaris guest fails to see the Solaris partitions of a physical disk in qemu-kvm-9999 (GIT)

[blueswirl]

On Fri, Aug 27, 2010 at 3:57 PM, devsk <618...@bugs.launchpad.net> wrote:
> what's the list address? All the lists at the kvm main page are for kvm
> only.
>
> --
> OpenSolaris guest fails to see the Solaris partitions of a physical disk in 
> qemu-kvm- (GIT)
> https://bugs.launchpad.net/bugs/618533
> You received this bug notification because you are a member of qemu-
> devel-ml, which is subscribed to QEMU.
>
> Status in QEMU: New
>
> Bug description:
> # qemu-kvm --version
> QEMU emulator version 0.13.50 (qemu-kvm-devel), Copyright (c) 2003-2008 
> Fabrice Bellard
>
> The following disk is presented to guest as IDE disk with /dev/sdd as path.
>
> # fdisk -l /dev/sdd
>
> Disk /dev/sdd: 750.2 GB, 750156374016 bytes
> 255 heads, 63 sectors/track, 91201 cylinders, total 1465149168 sectors
> Units = sectors of 1 * 512 = 512 bytes
> Sector size (logical/physical): 512 bytes / 512 bytes
> I/O size (minimum/optimal): 512 bytes / 512 bytes
> Disk identifier: 0x7f3a03aa
>
>   Device Boot      Start         End      Blocks   Id  System
> /dev/sdd1              63     7887914     3943926   1b  Hidden W95 FAT32
> /dev/sdd2         7887915   980736119   486424102+  83  Linux
> /dev/sdd3   *   980736120  1083150494    51207187+  bf  Solaris
> /dev/sdd4      1083150495  1465144064   190996785    5  Extended
> /dev/sdd5      1083150558  1107746009    12297726   83  Linux
> /dev/sdd6      1107746073  1465144064   178698996    7  HPFS/NTFS
>
> The prtvtoc output is attached from both VirtualBox and Qemu-KVM. As can be 
> seen in the screenshots, a valid Solaris partition table (which is inside the 
> /dev/sdd3, marked 'bf' in Linux fdisk output) is not seen in Qemu but seen in 
> Virtualbox.
>
> This makes the guest unbootable in Qemu because the root FS is not found but 
> its bootable in Virtualbox.

Sorry, I missed that this was for qemu-kvm. Anyway, the patch is
probably applicable to QEMU as well and you actually filed the bug to
Qemu.

qemu-devel list information can be found in
http://lists.nongnu.org/mailman/listinfo/qemu-devel

-- 
OpenSolaris guest fails to see the Solaris partitions of a physical disk in 
qemu-kvm- (GIT)
https://bugs.launchpad.net/bugs/618533
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
# qemu-kvm --version
QEMU emulator version 0.13.50 (qemu-kvm-devel), Copyright (c) 2003-2008 Fabrice 
Bellard

The following disk is presented to guest as IDE disk with /dev/sdd as path.

# fdisk -l /dev/sdd

Disk /dev/sdd: 750.2 GB, 750156374016 bytes
255 heads, 63 sectors/track, 91201 cylinders, total 1465149168 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x7f3a03aa

   Device Boot  Start End  Blocks   Id  System
/dev/sdd1  63 7887914 3943926   1b  Hidden W95 FAT32
/dev/sdd2 7887915   980736119   486424102+  83  Linux
/dev/sdd3   *   980736120  108315049451207187+  bf  Solaris
/dev/sdd4  1083150495  1465144064   1909967855  Extended
/dev/sdd5  1083150558  110774600912297726   83  Linux
/dev/sdd6  1107746073  1465144064   1786989967  HPFS/NTFS

The prtvtoc output is attached from both VirtualBox and Qemu-KVM. As can be 
seen in the screenshots, a valid Solaris partition table (which is inside the 
/dev/sdd3, marked 'bf' in Linux fdisk output) is not seen in Qemu but seen in 
Virtualbox.

This makes the guest unbootable in Qemu because the root FS is not found but 
its bootable in Virtualbox.