Re: [Qemu-devel] [PATCH 1/4] machine: conversion of QEMUMachineInitArgs to MachineState

[Marcel Apfelbaum]

On Thu, 2014-05-15 at 17:04 +0200, Markus Armbruster wrote:
> Marcel Apfelbaum  writes:
> 
> > Total removal of QEMUMachineInitArgs struct. QEMUMachineInitArgs's fields
> > are copied into MachineState. Removed duplicated fields from MachineState.
> >
> > All the other changes are only mechanical refactoring, no semantic changes.
> >
> > Signed-off-by: Marcel Apfelbaum 
> > ---
> >- I am perfectly aware that patches touching a lot of files
> >  are not desirable, but this one is a very simple replacement
> >  patch:
> >QEMUMachineInitArgs -> MachineState
> >args -> ms
> >- This is the simplest way to get rid of QEMUMachineInitArgs fast.
> 
> Snipping all patch hunks that only replace QEMUMachineInitArgs *args by
> MachineState *ms leaves just the hunks quoted below.  Did I miss any?
Not that I am aware of.
Thanks for going over the patch!
Marcel

> 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 28f0047..eba0574 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -7,17 +7,10 @@
> >  #include "hw/qdev.h"
> >  #include "qom/object.h"
> >  
> > -typedef struct QEMUMachineInitArgs {
> > -const MachineClass *machine;
> > -ram_addr_t ram_size;
> > -const char *boot_order;
> > -const char *kernel_filename;
> > -const char *kernel_cmdline;
> > -const char *initrd_filename;
> > -const char *cpu_model;
> > -} QEMUMachineInitArgs;
> >  
> > -typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
> > +typedef struct MachineState MachineState;
> > +
> > +typedef void QEMUMachineInitFunc(MachineState *ms);
> >  
> >  typedef void QEMUMachineResetFunc(void);
> >  
> > @@ -61,8 +54,6 @@ int qemu_register_machine(QEMUMachine *m);
> >  #define MACHINE_CLASS(klass) \
> >  OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
> >  
> > -typedef struct MachineState MachineState;
> > -
> >  MachineClass *find_default_machine(void);
> >  extern MachineState *current_machine;
> >  
> > @@ -79,7 +70,7 @@ struct MachineClass {
> >  const char *alias;
> >  const char *desc;
> >  
> > -void (*init)(QEMUMachineInitArgs *args);
> > +void (*init)(MachineState *state);
> >  void (*reset)(void);
> >  void (*hot_add_cpu)(const int64_t id, Error **errp);
> >  int (*kvm_type)(const char *arg);
> > @@ -111,9 +102,6 @@ struct MachineState {
> >  char *accel;
> >  bool kernel_irqchip;
> >  int kvm_shadow_mem;
> > -char *kernel;
> > -char *initrd;
> > -char *append;
> >  char *dtb;
> >  char *dumpdtb;
> >  int phandle_start;
> > @@ -123,7 +111,13 @@ struct MachineState {
> >  bool usb;
> >  char *firmware;
> >  
> > -QEMUMachineInitArgs init_args;
> > +const MachineClass *machine;
> > +ram_addr_t ram_size;
> > +const char *boot_order;
> > +const char *kernel_filename;
> > +const char *kernel_cmdline;
> > +const char *initrd_filename;
> > +const char *cpu_model;
> >  };
> >  
> >  #endif
> > diff --git a/vl.c b/vl.c
> > index c4505dc..58673bd 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4422,16 +4422,15 @@ int main(int argc, char **argv, char **envp)
> >  
> >  qdev_machine_init();
> >  
> > -current_machine->init_args = (QEMUMachineInitArgs) {
> > -.machine = machine_class,
> > -.ram_size = ram_size,
> > -.boot_order = boot_order,
> > -.kernel_filename = kernel_filename,
> > -.kernel_cmdline = kernel_cmdline,
> > -.initrd_filename = initrd_filename,
> > -.cpu_model = cpu_model };
> > -
> > -machine_class->init(¤t_machine->init_args);
> > +current_machine->machine = machine_class;
> > +current_machine->ram_size = ram_size;
> > +current_machine->boot_order = boot_order;
> > +current_machine->kernel_filename = kernel_filename;
> > +current_machine->kernel_cmdline = kernel_cmdline;
> > +current_machine->initrd_filename = initrd_filename;
> > +current_machine->cpu_model = cpu_model;
> > +
> > +machine_class->init(current_machine);
> >  
> >  audio_init();
> 
> This can't lose any implicit zero initialization, because
> current_machine has been created by object_new(), which zeroes the whole
> struct.  Good.

Re: [Qemu-devel] [PATCH 2/4] qapi: output visitor crashes qemu if it encounters a NULL value

[Marcel Apfelbaum]

On Wed, 2014-05-14 at 15:38 -0500, Michael Roth wrote:
> Quoting Luiz Capitulino (2014-05-14 13:25:16)
> > On Wed, 14 May 2014 20:29:37 +0300
> > Marcel Apfelbaum  wrote:
> > 
> > > On Wed, 2014-05-14 at 19:00 +0200, Andreas Färber wrote:
> > > > Am 13.05.2014 21:08, schrieb Eric Blake:
> > > > > On 05/13/2014 11:36 AM, Andreas Färber wrote:
> > > > >> Am 07.05.2014 16:42, schrieb Marcel Apfelbaum:
> > > > >>> A NULL value is not added to visitor's stack, but there is no
> > > > >>> check for that when the visitor tries to return that value,
> > > > >>> leading to Qemu crash.
> > > > >>> 
> > > > >>> Reviewed-by: Eric Blake  Signed-off-by:
> > > > >>> Marcel Apfelbaum 
> > > > >> 
> > > > >> Where does the Rb come from on this v1? Is it in any tree
> > > > >> already?
> > > > >> 
> > > > > 
> > > > > The (weak) R-b was here: 
> > > > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02861.html
> > > > 
> > > > Thanks.
> > > > > 
> > > > So Luiz was okay with it too, but his last message seems to be
> > > > indicating this needs to be fixed somewhere else, too:
> > > > 
> > > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05228.html
> > > > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00217.html
> > > > 
> > > > Can/should that be addressed as a follow-up? Or is there a test case
> > > > that breaks?
> > > Simple and "popular" test case: the user does not use the -kernel-cmdline 
> > > parameter.
> > > The patch is needed because otherwise the main function will fail
> > > if no value is passed by the user to string parameters. 
> > > 
> > > Regarding Luiz's concern, it can be a follow-up as I am not aware of
> > > any problem with that.
> > 
> > My concern was that I wasn't sure if this is the right fix for the issue
> > or if it's papering over the real bug. I quickly checked the code and it
> > seemed to make sense, but I didn't have time to study it deeper.
> 
> Not sure the fix is bad or not, but the cause might be a little more subtle
> than NULL string values as mentioned in the other thread. QmpOutputVisitor
> encodes NULL strings as "" via qmp_output_type_str(), so the problem doesn't
> seem to lie there: it shouldn't generate NULL values on the stack.
> 
> I think the real issue is that object_property_get_str() actually calls an
> accessor via property_get_str to get the string, then explicitly *skips*
> the call to visit_type_str() if it is NULL (as it would be in the case of,
> say, kernel_cmdline option being NULL). So I wonder if maybe the real issue
> we're fixing is a corner case where you call qmp_output_get_qobject() on
> an "empty" QmpOutputVisitor.
> 
> Surprised that's not covered by tests, but didn't see any coverage doing
> a cursory glance. Actually, might as well just add one..
> 
> diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
> index e073d83..f190eaa 100644
> --- a/tests/test-qmp-output-visitor.c
> +++ b/tests/test-qmp-output-visitor.c
> @@ -434,6 +434,17 @@ static void test_visitor_out_union(TestOutputVisitorData 
> *data,
>  QDECREF(qdict);
>  }
>  
> +static void test_visitor_out_empty(TestOutputVisitorData *data,
> +   const void *unused)
> +{
> +QObject *arg;
> +QDict *qdict;
> +
> +arg = qmp_output_get_qobject(data->qov);
> +qdict = qobject_to_qdict(arg);
> +QDECREF(qdict);
> +}
> +
>  static void init_native_list(UserDefNativeListUnion *cvalue)
>  {
>  int i;
> @@ -782,6 +793,8 @@ int main(int argc, char **argv)
>  &out_visitor_data, 
> test_visitor_out_list_qapi_free);
>  output_visitor_test_add("/visitor/output/union",
>  &out_visitor_data, test_visitor_out_union);
> +output_visitor_test_add("/visitor/output/empty",
> +&out_visitor_data, test_visitor_out_empty);
>  output_visitor_test_add("/visitor/output/native_list/int",
>  &out_visitor_data, 
> test_visitor_out_native_list_int);
>  output_visitor_test_add("/visitor/output/native_list/int8",
> 
> mdroth@loki:~/w/qemu-build$ tests/test-qmp-output-visitor 
> /visitor/output/int: OK
> /visitor/output/bool: OK
> /visitor/output/number: OK
> /visitor/output/string: OK
> /visitor/output/no-string: OK
> /visitor/output/enum: OK
> /visitor/output/enum-errors: OK
> /visitor/output/struct: OK
> /visitor/output/struct-nested: OK
> /visitor/output/struct-errors: OK
> /visitor/output/list: OK
> /visitor/output/list-qapi-free: OK
> /visitor/output/union: OK
> /visitor/output/empty: Segmentation fault (core dumped)
> 
> So I guess the question is whether we should support converting an empty
> QmpOutputVisitor to a QObject. I would say yes, and that a NULL value is
> probably the most reasonable value.
> 
> I would ask that commit/code is a little more explicit about what corner case
> is being handled though, and that something like the above unit test be
> included with the series.

Re: [Qemu-devel] [PATCH 1/4] machine: conversion of QEMUMachineInitArgs to MachineState

[Marcel Apfelbaum]

On Fri, 2014-05-16 at 20:38 +0200, Andreas Färber wrote:
> Am 16.05.2014 18:20, schrieb Igor Mammedov:
> > On Wed,  7 May 2014 17:42:57 +0300
> > Marcel Apfelbaum  wrote:
> > 
> >> Total removal of QEMUMachineInitArgs struct. QEMUMachineInitArgs's fields
> >> are copied into MachineState. Removed duplicated fields from MachineState.
> >>
> >> All the other changes are only mechanical refactoring, no semantic changes.
> >>
> >> Signed-off-by: Marcel Apfelbaum 
> >> ---
> >>- I am perfectly aware that patches touching a lot of files
> >>  are not desirable, but this one is a very simple replacement
> >>  patch:
> >>QEMUMachineInitArgs -> MachineState
> >>args -> ms
> >>- This is the simplest way to get rid of QEMUMachineInitArgs fast.
> >>
> > One more thing:
> > pc_q35_init() and pc_init1() both use qdev_get_machine() for adding
> > properties. Please replace it with passed in machine.
> 
> For the previous series we had clarified that qdev_get_machine() can
> safely be used. The benefit of passing "machine" would be efficiency of
> machine init only, so let's just do that as a follow-up cleanup please.

Fine by me.
Thanks,
Marcel
> 
> Regards,
> Andreas
> 

Re: [Qemu-devel] OS X compile fix

[Andreas Färber]

Hi,

Am 18.05.2014 01:14, schrieb Peter Bartoli:
> 
> At the recommendation of Mark Cave-Ayland, I'm sending this patch in to
> remedy a long-time Mac OS X compile issue.

Define "long-time"? 2.0 compiled just fine for me on v10.5.8 with
gcc-4.2. I see a number of deprecation and possibly-used-uninitialized
warnings but no breakage. What OSX version and what compiler are you
using, and what error or warning are you seeing without your patch?

Right now on qemu.git I have a different build issue: In scripts/qapi.py
"except IOError as e" needs to be "except IOError, e". But
block/raw-posix.c still builds fine here.

>  The offsets are a way off in
> this diff, but it still works.

Offsets are not the only issue with this "patch", see
http://wiki.qemu-project.org/Contribute/SubmitAPatch

* HTML format
* No Signed-off-by
* No [PATCH]
* Block maintainers not CC'ed (not so important here)
* Insufficient description of what is being fixed

Regards,
Andreas

> --- block/raw-posix.c.orig  2012-12-03 11:37:05.0 -0800
> +++ block/raw-posix.c   2012-12-03 18:24:47.0 -0800
> @@ -914,11 +914,11 @@
>  size = 0;
>  }
>  if (size == 0)
>  #endif
>  #if defined(__APPLE__) && defined(__MACH__)
> -size = LONG_LONG_MAX;
> +size = LLONG_MAX;
>  #else
>  size = lseek(fd, 0LL, SEEK_END);
>  #endif
>  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
>  switch(s->type) {

Re: [Qemu-devel] [PATCH 1/4] machine: conversion of QEMUMachineInitArgs to MachineState

[Marcel Apfelbaum]

On Fri, 2014-05-16 at 20:33 +0200, Andreas Färber wrote:
> Am 16.05.2014 16:39, schrieb Igor Mammedov:
> > On Wed,  7 May 2014 17:42:57 +0300
> > Marcel Apfelbaum  wrote:
> > 
> >> Total removal of QEMUMachineInitArgs struct. QEMUMachineInitArgs's fields
> >> are copied into MachineState. Removed duplicated fields from MachineState.
> >>
> >> All the other changes are only mechanical refactoring, no semantic changes.
> >>
> >> Signed-off-by: Marcel Apfelbaum 
> >> ---
> >>- I am perfectly aware that patches touching a lot of files
> >>  are not desirable, but this one is a very simple replacement
> >>  patch:
> >>QEMUMachineInitArgs -> MachineState
> >>args -> ms
> >>- This is the simplest way to get rid of QEMUMachineInitArgs fast.
> >>
> > Patch doesn't apply cleanly on current master
> 
> patch -p1 applied all hunks ...
> 
> > 
> > [...]
> > 
> >>  
> >>  #define PC_Q35_MACHINE_OPTIONS \
> >> diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c
> >> index 9adb57f..fb7a817 100644
> >> --- a/hw/i386/xen_machine_pv.c
> >> +++ b/hw/i386/xen_machine_pv.c
> >> @@ -28,11 +28,11 @@
> > ^^^ file was moved
> 
> ... and prompted for the hw/xenpv/xen_machine_pv.c path.
> 
> > I've reviewed all targets, it looks good.
> > +1 to afaerber's comment s/ms/machine/ 
> 
> 4 "args->" were missed in exynos4_boards.c debug output.
> 
> for f in hw/*/*.c; do
>   sed -i 's/*ms\\)/*machine\\)/g' $f
>   sed -i 's/*ms,/*machine,/g' $f
>   sed -i 's/ ms->/ machine->/g' $f
>   sed -i 's/(ms->/(machine->/g' $f
>   sed -i 's/!ms->/!machine->/g' $f
>   sed -i 's/(ms)/(machine)/g' $f
>   sed -i 's/(ms,/(machine,/g' $f
>   sed -i 's/ ms,/ machine,/g' $f
>   sed -i 's/, ms)/, machine)/g' $f
> done
> git checkout -- hw/ppc/ppc4xx_pci.c # false "(ms," positive
> 
> highbank.c had a conflicting "machine" argument -> machine_id.
> 
> e500.c needed tweaking due to passing ms in a struct. And it was
> shallow-copying *ms, which must not be done for QOM. Passing the pointer
> instead.

Thanks for the help, Andreas!
Marcel

> 
> Andreas
> 

[Qemu-devel] [PATCH v4] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename

[Jun Li]

This patch fixed bdrv_get_full_backing_filename can not calculate the correct 
full path name for backing_file via path_combine.

Such as:
create a snapshot chain:
$BASE_IMG<-sn1<-sn2
backing file is : /home/wookpecker/img.qcow2
sn1 : /home/woodpecker/tmp/sn1
sn2 : /home/woodpecker/tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.

In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.

Signed-off-by: Jun Li 
---
This is v4 of the patch. This version is the same with v3. Only difference is 
the commit description.
What does this patch has fixed, please ref following bug(it gives the detailed 
description):
https://bugzilla.redhat.com/show_bug.cgi?id=1084302
---
 block.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index c90c71a..d163a8c 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
 
 void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t 
sz)
 {
+struct stat sb;
+char *linkname;
+
 if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
 pstrcpy(dest, sz, bs->backing_file);
 } else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}
 }
 }
 
-- 
1.9.0

Re: [Qemu-devel] [PATCH] virtio-net: announce self by guest

[Michael S. Tsirkin]

On Fri, May 16, 2014 at 01:02:51PM +0800, Jason Wang wrote:
> On 05/15/2014 05:45 PM, Michael S. Tsirkin wrote:
> > On Thu, May 15, 2014 at 05:22:28PM +0800, Jason Wang wrote:
> >> On 05/15/2014 04:28 PM, Michael S. Tsirkin wrote:
> >>> Thanks, looks good.
> >>> Some minor comments below,
> >>>
> >>> On Thu, May 15, 2014 at 03:16:47PM +0800, Jason Wang wrote:
>  It's hard to track all mac addresses and their configurations (e.g
>  vlan or ipv6) in qemu. Without those informations, it's impossible to
> >>> s/those informations/this information/
> >>>
>  build proper garp packet after migration. The only possible solution
>  to this is let guest (who knew all configurations) to do this.
> >>> s/knew/knows/
> >>>
>  So, this patch introduces a new readonly config status bit of virtio-net,
>  VIRTIO_NET_S_ANNOUNCE which is used to notify guest to announce
>  presence of its link through config update interrupt.When guest has
>  done the announcement, it should ack the notification through
>  VIRTIO_NET_CTRL_ANNOUNCE_ACK cmd. This feature is negotiated by a new
>  feature bit VIRTIO_NET_F_ANNOUNCE (which has already been supported by
>  Linux guest).
> 
>  During load, a counter of announcing rounds were set so that the after
> >>> s/were/is/
> >>> s/the after/after/
> >> Will correct those typos.
>  the vm is running it can trigger rounds of config interrupts to notify
>  the guest to build and send the correct garps.
> 
>  Tested with ping to guest with vlan during migration. Without the
>  patch, lots of the packets were lost after migration. With the patch,
>  could not notice packet loss after migration.
> >>> below changelog should go after ---, until the ack list.
> >>>
> >> Ok.
>  Reference:
>  RFC v2: 
>  https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg01750.html
>  RFC v1: 
>  https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg02648.html
>  V7: 
>  https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg01127.html
> 
>  Changes from RFC v2:
>  - use QEMU_CLOCK_VIRTUAL instead of QEMU_CLOCK_REALTIME
>  - compat self announce for 2.0 machine type
> 
>  Changes from RFC v1:
>  - clean VIRTIO_NET_S_ANNOUNCE bit during reset
>  - free announce timer during clean
>  - make announce work for non-vhost case
> 
>  Changes from V7:
>  - Instead of introducing a global method for each kind of nic, this
>    version limits the changes to virtio-net itself.
> 
>  Cc: Liuyongan 
>  Cc: Amos Kong 
>  Signed-off-by: Jason Wang 
>  ---
>   hw/net/virtio-net.c|   48 
>  
>   include/hw/i386/pc.h   |5 
>   include/hw/virtio/virtio-net.h |   16 +
>   3 files changed, 69 insertions(+), 0 deletions(-)
> 
>  diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>  index 940a7cf..98d59e9 100644
>  --- a/hw/net/virtio-net.c
>  +++ b/hw/net/virtio-net.c
>  @@ -25,6 +25,7 @@
>   #include "monitor/monitor.h"
>   
>   #define VIRTIO_NET_VM_VERSION11
>  +#define VIRTIO_NET_ANNOUNCE_ROUNDS3
>   
>   #define MAC_TABLE_ENTRIES64
>   #define MAX_VLAN(1 << 12)   /* Per 802.1Q definition */
> >>> I would make it  5 to be consistent with SELF_ANNOUNCE_ROUNDS
> >>> in savevm.c
> >>>
> >>> in fact, let's move SELF_ANNOUNCE_ROUNDS to include/migration/vmstate.h
> >>> and reuse it.
> >> Ok.
>  @@ -99,6 +100,25 @@ static bool virtio_net_started(VirtIONet *n, uint8_t 
>  status)
>   (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running;
>   }
>   
>  +static void virtio_net_announce_timer(void *opaque)
>  +{
>  +VirtIONet *n = opaque;
>  +VirtIODevice *vdev = VIRTIO_DEVICE(n);
>  +
>  +if (n->announce &&
> >>> I would make it > 0 here, just in case it becomes negative as a result
> >>> of some bug.
> >> Sure.
>  +virtio_net_started(n, vdev->status) &&
>  +vdev->guest_features & (0x1 << VIRTIO_NET_F_GUEST_ANNOUNCE) &&
>  +vdev->guest_features & (0x1 << VIRTIO_NET_F_CTRL_VQ)) {
>  +
>  +n->announce--;
>  +n->status |= VIRTIO_NET_S_ANNOUNCE;
>  +virtio_notify_config(vdev);
>  +} else {
>  +timer_del(n->announce_timer);
> >>> why do this here?
> >>>
>  +n->announce = 0;
> >>> why is this here?
> >>>
> >> If guest shuts down the device, there's no need to do the announcing.
> > It's still weird.
> > Either announce is 0 and then timer was not running
> > or it's > 0 and then this won't trigger.
> 
> Right, the logic here is for QEMU_CLOCK_REALTIME. But there's another
> question, we use QEMU_CLOCK_VIRTUAL while qemu is using
> QEMU_CLOCK_REALTIME for its announcing. This looks fine but not sure
> whether this is safe.

M

Re: [Qemu-devel] [PATCH v4] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename

[Jun Li]

please ignore this one.

On 05/18/2014 05:01 PM, Jun Li wrote:

This patch fixed bdrv_get_full_backing_filename can not calculate the correct 
full path name for backing_file via path_combine.

Such as:
create a snapshot chain:
$BASE_IMG<-sn1<-sn2
backing file is : /home/wookpecker/img.qcow2
sn1 : /home/woodpecker/tmp/sn1
sn2 : /home/woodpecker/tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.

In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.

Signed-off-by: Jun Li 
---
This is v4 of the patch. This version is the same with v3. Only difference is 
the commit description.
What does this patch has fixed, please ref following bug(it gives the detailed 
description):
https://bugzilla.redhat.com/show_bug.cgi?id=1084302
---
  block.c | 18 +-
  1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index c90c71a..d163a8c 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
  
  void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz)

  {
+struct stat sb;
+char *linkname;
+
  if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
  pstrcpy(dest, sz, bs->backing_file);
  } else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}
  }
  }
  

[Qemu-devel] [PATCH v4] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename

[Jun Li]

This patch fixed bdrv_get_full_backing_filename can not calculate the correct 
full path name for backing_file via path_combine.

Such as:
create a snapshot chain:
$BASE_IMG<-sn1<-sn2
backing file is : ./img.qcow2
sn1 : ./tmp/sn1
sn2 : ./tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.

In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.

Signed-off-by: Jun Li 
---
This is v4 of the patch. This version is the same with v3. Only difference is 
the commit description.
What does this patch has fixed, please ref following bug(it gives the detailed 
description):
https://bugzilla.redhat.com/show_bug.cgi?id=1084302
---
 block.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index c90c71a..d163a8c 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
 
 void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t 
sz)
 {
+struct stat sb;
+char *linkname;
+
 if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
 pstrcpy(dest, sz, bs->backing_file);
 } else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}
 }
 }
 
-- 
1.9.0

Re: [Qemu-devel] [PATCH v3 03/16] s390: Convert debug printfs to QEMU_DPRINTF

[Peter Crosthwaite]

On Sun, May 18, 2014 at 9:03 AM, Marc Marí  wrote:
> Modify debug macros to have the same format through the codebase and use 
> regular
> ifs instead of ifdef.
>
> Signed-off-by: Marc Marí 
> ---
>  hw/s390x/s390-virtio-bus.c |9 +
>  hw/s390x/s390-virtio.c |9 +
>  target-s390x/helper.c  |   23 +++
>  target-s390x/kvm.c |9 +
>  4 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 9c71afa..2a1799e 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -38,13 +38,14 @@
>  /* #define DEBUG_S390 */
>
>  #ifdef DEBUG_S390
> -#define DPRINTF(fmt, ...) \
> -do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 1
>  #else
> -#define DPRINTF(fmt, ...) \
> -do { } while (0)
> +#define DEBUG_S390_ENABLED 0
>  #endif
>
> +#define DPRINTF(fmt, ...) \
> +QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio bus", fmt, ## __VA_ARGS__)
> +
>  #define VIRTIO_EXT_CODE   0x2603
>
>  static void virtio_s390_bus_new(VirtioBusState *bus, size_t bus_size,
> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
> index aef2003..b69afb4 100644
> --- a/hw/s390x/s390-virtio.c
> +++ b/hw/s390x/s390-virtio.c
> @@ -42,13 +42,14 @@
>  //#define DEBUG_S390
>
>  #ifdef DEBUG_S390
> -#define DPRINTF(fmt, ...) \
> -do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 1
>  #else
> -#define DPRINTF(fmt, ...) \
> -do { } while (0)
> +#define DEBUG_S390_ENABLED 0
>  #endif
>
> +#define DPRINTF(fmt, ...) \
> +QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio", fmt, ## __VA_ARGS__)
> +
>  #define MAX_BLK_DEVS10
>  #define ZIPL_FILENAME   "s390-zipl.rom"
>
> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> index 7c76fc1..c2aa568 100644
> --- a/target-s390x/helper.c
> +++ b/target-s390x/helper.c
> @@ -30,19 +30,26 @@
>  //#define DEBUG_S390_STDOUT
>
>  #ifdef DEBUG_S390
> -#ifdef DEBUG_S390_STDOUT
> -#define DPRINTF(fmt, ...) \
> -do { fprintf(stderr, fmt, ## __VA_ARGS__); \
> - qemu_log(fmt, ##__VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 1
>  #else
> -#define DPRINTF(fmt, ...) \
> -do { qemu_log(fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 0
>  #endif
> +
> +#ifdef DEBUG_S390_STDOUT

This STDOUT vs _log() choice is a bit irregular, and I think you might
be better off abandoning it completely. Richard, Alex, do we really
need to optionally route printfery to log or stderr? (considering _log
is NOW stderr by default now, and -D option gives you some flexibility
there). Can we have just log and drop STDOUT mode?

Regards,
Peter

> +#define DEBUG_S390_STDOUT_ENABLED 1
>  #else
> -#define DPRINTF(fmt, ...) \
> -do { } while (0)
> +#define DEBUG_S390_STDOUT_ENABLED 0
>  #endif
>
> +#define DPRINTF(fmt, ...) \
> +do { \
> +if(DEBUG_S390_ENABLED) { \
> +qemu_log(fmt, ##__VA_ARGS__); \
> +QEMU_DPRINTF(DEBUG_S390_STDOUT_ENABLED, "s390x helper", \
> +fmt, ## __VA_ARGS__); \
> +} \
> +} while (0)
> +
>  #ifdef DEBUG_S390_PTE
>  #define PTE_DPRINTF DPRINTF
>  #else
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 56179af..63c46c4 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -41,13 +41,14 @@
>  /* #define DEBUG_KVM */
>
>  #ifdef DEBUG_KVM
> -#define DPRINTF(fmt, ...) \
> -do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_KVM_ENABLED 1
>  #else
> -#define DPRINTF(fmt, ...) \
> -do { } while (0)
> +#define DEBUG_KVM_ENABLED 0
>  #endif
>
> +#define DPRINTF(fmt, ...) \
> +QEMU_DPRINTF(DEBUG_KVM_ENABLED, "s390 kvm", fmt, ## __VA_ARGS__)
> +
>  #define IPA0_DIAG   0x8300
>  #define IPA0_SIGP   0xae00
>  #define IPA0_B2 0xb200
> --
> 1.7.10.4
>
>

Re: [Qemu-devel] [PATCH v1 2/3] memory: Add sysbus memory device

[Peter Crosthwaite]

On Wed, Apr 16, 2014 at 10:24 PM, Alexander Graf  wrote:
>
> On 15.04.14 04:21, Peter Crosthwaite wrote:
>>
>> Add a sysbus device consisting of a single ram. This allows for
>> instantiation of RAM just like any other device. There are a number
>> of good reasons to want to do this this:
>>
>> 1: Consistency. RAM is not that special where board level files should
>> have to instantiate it with a completely different API. This reduces
>> complexity of board level development by hiding the memory API
>> completely and handling everything via the sysbus API.
>>
>> 2: Device tree completeness. Ram Now shows up in info-qtree and
>> friends. E.g. Info qtree gives meaningful information under the
>> root system bus:
>>
>>dev: sysbus-memory, id "zynq.ocm_ram"
>>  size = 262144 (0x4)
>>  read-only = false
>>  irq 0
>>  mmio fffc/0004
>>dev: sysbus-memory, id "zynq.ext_ram"
>>  size = 134217728 (0x800)
>>  read-only = false
>>  irq 0
>>  mmio /0800
>>
>> 3: Remove dependence of global state. Board files don't have to
>> explicity request the global singleton (and much unloved)
>> address_space_memory() and go hacking on it. address_space_memory()
>> is still ultimately used, but the ugliness is hidden in one place - the
>> sysbus core (we can fix that another day).
>>
>> 4: Data driven machine creation. There is list discussion on being able
>> to create or append-to sysbus machines in a data-driven way (whether
>> thats from command-line, monitor or scripts or whatever). This patch
>> removes the memory special case from that problem and allows RAM
>> instantiation to come via whatever solutions we come up with sysbus
>> device instantiation.
>>
>> Signed-off-by: Peter Crosthwaite 
>
>
> Could you please show that this approach works for more complicated
> machines, like x86's pc machine and its PCI holes?
>

Hi Alex,

Do you mean attaching it within a PCI address space? Im not sure
that's valid. That would require some sort of generalisation that
applied to both sysbus and PCI. I actually had a go at something like
this a while back. Basically, the setup was for PCI devices to inherit
from sysbus. This allowed solving the reverse problem. Attaching a PCI
device to sysbus. I guess with a few changes it could be made to work
both ways, but it seems the sysbus and PC world are completely
separate the way the tree stands today.

Regards,
Peter

>
> Alex
>
>

Re: [Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename

[Jun Li]

On 05/12/2014 11:15 PM, Eric Blake wrote:

On 05/10/2014 10:35 AM, Jun Li wrote:

From: Jun Li 

I see three different "[PATCH v3]" mails with this subject.  To make
sure we are reviewing the right version, it might help to bump to version 4.

Yes, I have send v4 of this patch and changed the commit description.



This patch fixed the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1084302 .

path_combine can not calculate the correct full path name for backing_file.
Such as:
create a snapshot chain:
sn2->sn1->$BASE_IMG
backing file is : /home/wookpecker/img.qcow2
sn1 : /home/woodpecker/tmp/sn1
sn2 : /home/woodpecker/tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.

I'm having a hard time parsing that.  We usually represent backing
chains with the base image on the left:

base <- sn1

Can you show the output of 'qemu-img info --backing-chain
/home/woodpecker/tmp/sn2' to make it a bit more obvious what your setup
is?  What command are you issuing that triggers a path_combine that is
getting the wrong result?
Sorry, this is the wrong commit description, please see it on v4. In 
version 4, I describe it just as followings:

create a snapshot chain:
$BASE_IMG<-sn1<-sn2
backing file is : ./img.qcow2
sn1 : ./tmp/sn1
sn2 : ./tmp/sn2

So,
# /opt/qemu-git-arm/bin/qemu-img info --backing-chain ./tmp/sn2
image: ./tmp/sn2
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
backing file: ./tmp/sn1 (actual path: /home/lijun/Work/tmp/sn1)
Format specific information:
compat: 1.1
lazy refcounts: false

image: /home/lijun/Work/tmp/sn1
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
backing file: ./img.qcow2 (actual path: /home/lijun/Work/img.qcow2)
Format specific information:
compat: 1.1
lazy refcounts: false

image: /home/lijun/Work/img.qcow2
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
--
Before this patch,
# qemu-img create -f qcow2 -b ./img.qcow2  ./tmp/sn1
# qemu-img create -f qcow2 -b ./tmp/sn1 ./tmp/sn2
qemu-img: Could not open './tmp/sn1': No such file or directory




In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.

This feels fishy to me, and liable to do the wrong thing.  I need more
context on how to reproduce the issue you are attempting to fix before I
can even decide if your fix is the right approach.
This patch fixed the following bug(*Bug 1084302* 
 -Should improve 
error info when can't find backing file for snapshot):

https://bugzilla.redhat.com/show_bug.cgi?id=1084302



Signed-off-by: Jun Li 
---
  block.c | 18 +-
  1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index b749d31..6674719 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
  
  void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz)

  {
+struct stat sb;
+char *linkname;
+
  if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
  pstrcpy(dest, sz, bs->backing_file);
  } else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}

Why are you tweaking just this caller, instead of path_combine() to
affect all other callers?

I will check. Thx.

Best Regards,
Jun Li

[Qemu-devel] [PATCH 1/2] nbd: Don't export a block device with no medium.

[Hani Benhabiles]

The device is exported with erroneous values and can't be read.

Before the patch:
$ sudo nbd-client localhost -p 10809 /dev/nbd0 -name floppy0
Negotiation: ..size = 17592186044415MB
bs=1024, sz=18446744073709547520 bytes

$ sudo mount /dev/nbd0 /mnt/tmp/
mount: block device /dev/nbd0 is write-protected, mounting read-only
mount: /dev/nbd0: can't read superblock

After the patch:
(qemu) nbd_server_add ide0-hd0
(qemu) nbd_server_add floppy0
Device 'floppy0' has no medium

Signed-off-by: Hani Benhabiles 
---
 blockdev-nbd.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 922cf56..a700d52 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -91,6 +91,10 @@ void qmp_nbd_server_add(const char *device, bool 
has_writable, bool writable,
 error_set(errp, QERR_DEVICE_NOT_FOUND, device);
 return;
 }
+if (!bdrv_is_inserted(bs)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+return;
+}
 
 if (!has_writable) {
 writable = false;
-- 
1.8.3.2

[Qemu-devel] [PATCH 2/2] nbd: Don't validate from and len in NBD_CMD_DISC.

[Hani Benhabiles]

These values aren't used in this case.

Currently, the from field in the request sent by the nbd kernel module leading
to a false error message when ending the connection with the client.

$ qemu-nbd some.img -v
// After nbd-client -d /dev/nbd0
nbd.c:nbd_trip():L1031: From: 18446744073709551104, Len: 0, Size: 20971520,
Offset: 0
nbd.c:nbd_trip():L1032: requested operation past EOF--bad client?
nbd.c:nbd_receive_request():L638: read failed

Signed-off-by: Hani Benhabiles 
---
 nbd.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/nbd.c b/nbd.c
index e5084b6..dc076d7 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1001,6 +1001,7 @@ static void nbd_trip(void *opaque)
 struct nbd_request request;
 struct nbd_reply reply;
 ssize_t ret;
+uint32_t type;
 
 TRACE("Reading request.");
 if (client->closing) {
@@ -1023,8 +1024,8 @@ static void nbd_trip(void *opaque)
 reply.error = -ret;
 goto error_reply;
 }
-
-if ((request.from + request.len) > exp->size) {
+type = request.type & NBD_CMD_MASK_COMMAND;
+if (type != NBD_CMD_DISC && (request.from + request.len) > exp->size) {
 LOG("From: %" PRIu64 ", Len: %u, Size: %" PRIu64
 ", Offset: %" PRIu64 "\n",
 request.from, request.len,
@@ -1033,7 +1034,7 @@ static void nbd_trip(void *opaque)
 goto invalid_request;
 }
 
-switch (request.type & NBD_CMD_MASK_COMMAND) {
+switch (type) {
 case NBD_CMD_READ:
 TRACE("Request type is READ");
 
-- 
1.8.3.2

Re: [Qemu-devel] [Qemu-trivial] [PATCH] arch_init: Simplify code for load_xbzrle()

[Chen Gang]

On 05/17/2014 03:54 PM, Michael Tokarev wrote:
> 10.05.2014 16:51, Chen Gang wrote:
>> For xbzrle_decode_buffer(), when decoding contents will exceed writing
>> buffer, it will return -1, so need not check the return value whether
>> large than writing buffer.
>>
>> And when failure occurs within load_xbzrle(), it always return -1
>> without any resources which need release.
>>
>> So can remove the related checking statements, and also can remove 'rc'
>> and 'ret' local variables,
> 
> Just one comment below.
> 
>> @@ -933,18 +932,13 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, 
>> void *host)
>>  qemu_get_buffer(f, xbzrle_decoded_buf, xh_len);
>>  
>>  /* decode RLE */
>> -ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
>> -   TARGET_PAGE_SIZE);
>> -if (ret == -1) {
>> +if (xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
>> + TARGET_PAGE_SIZE) == -1) {
> 
> Can we compare like '< 0' here, not like '== -1' ?

That's fine to me.

> Are there any other possible return values from xbzrle_decode_buffer()
> which are less than zero but non-error?
> 

Although, at present, when it fails, it will only return -1.


> To me, anything less than zero is always error (unless it is one of the
> possible non-error values, like offset for example which can be negative).
> 

That sounds reasonable to me, too.

> Especially having in mind that in the future, some function may extend
> its error return to include the actual error code (like -errno), in which
> case code which compares with -1 will not work anymore.
> 

Yeah, in the future, it may do.

> Is it okay to me to apply this with s/== -1/< 0/ ?
> 

At least, it is OK to me.


BTW: the related test code for xbzrle_decode_buffer() may also need
improved (although, after read through, I still don't known what it
really want to do).

diff --git a/tests/test-xbzrle.c b/tests/test-xbzrle.c
index db93b0a..c8b4e58 100644
--- a/tests/test-xbzrle.c
+++ b/tests/test-xbzrle.c
@@ -162,7 +162,7 @@ static void encode_decode_range(void)
 PAGE_SIZE);
 
 rc = xbzrle_decode_buffer(compressed, dlen, test, PAGE_SIZE);
-g_assert(rc < PAGE_SIZE);
+g_assert(rc < PAGE_SIZE && rc >= 0);
 g_assert(memcmp(test, buffer, PAGE_SIZE) == 0);
 
 g_free(buffer);

Please help check when you have time. If necessary, I shall send related
patch for it. (this fix may be still incorrect, if so, please help send
related patch for it, and welcome to mark me as Reported-by for it).


Thanks.
-- 
Chen Gang

Open, share and attitude like air, water and life which God blessed

Re: [Qemu-devel] OS X compile fix

[Peter Maydell]

On 18 May 2014 09:50, Andreas Färber  wrote:
> Am 18.05.2014 01:14, schrieb Peter Bartoli:
>> At the recommendation of Mark Cave-Ayland, I'm sending this patch in to
>> remedy a long-time Mac OS X compile issue.
>
> Define "long-time"? 2.0 compiled just fine for me on v10.5.8 with
> gcc-4.2. I see a number of deprecation and possibly-used-uninitialized
> warnings but no breakage. What OSX version and what compiler are you
> using, and what error or warning are you seeing without your patch?

Yeah, this must be only a problem on certain OSX setups -- it's
never been an issue for me on 10.8/x86_64.

That said, it looks like LONG_LONG_MAX is a pre-C99 version
and LLONG_MAX the standard macro name. We use LLONG_MAX
elsewhere (though admittedly only in a test case) so I think it makes
sense to make this change, especially if it fixes compilation on some
setups.

(This use of LONG_LONG_MAX has been in the codebase since
2006, so presumably any configs which somehow fail to provide
this macro have always failed to build without this fix...)

> Right now on qemu.git I have a different build issue: In scripts/qapi.py
> "except IOError as e" needs to be "except IOError, e". But
> block/raw-posix.c still builds fine here.

"except .. as" came in with python 2.6; we need to use the older
syntax since we must work with 2.4 still. (Compare commit
21e0043bada1 where we fixed a previous round of this.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v12 4/4] qapi: Add a primitive to include other files from a QAPI schema file

[Peter Maydell]

On 7 May 2014 19:46, Lluís Vilanova  wrote:
> The primitive uses JSON syntax, and include paths are relative to the file 
> using the directive:
>
>   { 'include': 'path/to/file.json' }
>
> Signed-off-by: Lluís Vilanova 
> Reviewed-by: Eric Blake 
> Reviewed-by: Markus Armbruster 

> @@ -74,10 +91,33 @@ class QAPISchema:
>  self.accept()
>
>  while self.tok != None:
> -expr_info = {'fp': fp, 'line': self.line}
> -expr_elem = {'expr': self.get_expr(False),
> - 'info': expr_info}
> -self.exprs.append(expr_elem)
> +expr_info = {'file': input_relname, 'line': self.line, 'parent': 
> self.parent_info}
> +expr = self.get_expr(False)
> +if isinstance(expr, dict) and "include" in expr:
> +if len(expr) != 1:
> +raise QAPIExprError(expr_info, "Invalid 'include' 
> directive")
> +include = expr["include"]
> +if not isinstance(include, str):
> +raise QAPIExprError(expr_info,
> +'Expected a file name (string), got: 
> %s'
> +% include)
> +include_path = os.path.join(self.input_dir, include)
> +if any(include_path == elem[1]
> +   for elem in self.include_hist):
> +raise QAPIExprError(expr_info, "Inclusion loop for %s"
> +% include)
> +try:
> +fobj = open(include_path, 'r')
> +except IOError as e:

Hi. I'm afraid "except ... as" is only supported in Python 2.6,
and we have to support Python 2.4 still (as Andreas mentioned
in another thread this breaks compilation on MacOSX 10.5).
Please can you fix this to use the older syntax (cf also commit
21e0043bad which fixed an earlier instance of this problem)?

thanks
-- PMM

[Qemu-devel] [PATCH] macio: handle non-block ATAPI DMA transfers

[Mark Cave-Ayland]

Currently the macio DMA routines assume that all DMA requests are for read/write
block transfers. This is not always the case for ATAPI, for example when
requesting a TOC where the response is generated directly in the IDE buffer.

Detect these non-block ATAPI DMA transfers (where no lba is specified in the
command) and copy the results directly into RAM as indicated by the DBDMA
descriptor. This fixes CDROM access under MorphOS.

Signed-off-by: Mark Cave-Ayland 
---
 hw/ide/macio.c |   21 +
 1 file changed, 21 insertions(+)

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 1c20616..af57168 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -337,6 +337,27 @@ static void pmac_ide_transfer(DBDMA_io *io)
 
 s->io_buffer_size = 0;
 if (s->drive_kind == IDE_CD) {
+
+/* Handle non-block ATAPI DMA transfers */
+if (s->lba == -1) {
+s->io_buffer_size = MIN(io->len, s->packet_transfer_size);
+bdrv_acct_start(s->bs, &s->acct, s->io_buffer_size,
+BDRV_ACCT_READ);
+MACIO_DPRINTF("non-block ATAPI DMA transfer size: %d\n",
+  s->io_buffer_size);
+
+/* Copy ATAPI buffer directly to RAM and finish */
+cpu_physical_memory_write(io->addr, s->io_buffer,
+  s->io_buffer_size);
+ide_atapi_cmd_ok(s);
+m->dma_active = false;
+
+MACIO_DPRINTF("end of non-block ATAPI DMA transfer\n");
+bdrv_acct_done(s->bs, &s->acct);
+io->dma_end(io);
+return;
+}
+
 bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ);
 pmac_ide_atapi_transfer_cb(io, 0);
 return;
-- 
1.7.10.4

[Qemu-devel] target-sparc has inverse cwp logic for SAVE/RESTORE?

[Mark Cave-Ayland]

Hi all,

I've been working on debugging a window-related OpenBIOS issue and 
noticed that the cwp register logic in QEMU appears to be backwards 
according to the SPARCv9 specification. From sections 6.3.6.1 and 6.3.6.2:


"The SAVE instruction allocates a new register window and saves the 
caller’s register window by incrementing the CWP register."


"The RESTORE instruction restores the previous register window by 
decrementing the CWP register."


In target-sparc/win_helper.c the logic in helper_save() and 
helper_restore() is inverted, i.e. executing SAVE decrements cwp while 
executing RESTORE increments cwp.


The surprise here was that executing SAVE when cwp == 0 changed cwp to 7 
rather than 1. AFAICT there should be no functional difference, but it 
would make things less confusing when debugging window traps if the 
logic from the specification was followed. Does anyone know why this is 
currently done this way?



ATB,

Mark.

Re: [Qemu-devel] [PATCH v2] kvmclock: Ensure time in migration never goes backward

[Marcelo Tosatti]

On Fri, May 16, 2014 at 05:15:21PM +0200, Alexander Graf wrote:
> When we migrate we ask the kernel about its current belief on what the guest
> time would be. However, I've seen cases where the kvmclock guest structure
> indicates a time more recent than the kvm returned time.
> 
> To make sure we never go backwards, calculate what the guest would have seen
> as time at the point of migration and use that value instead of the kernel
> returned one when it's more recent.
> 
> While the underlying bug is supposedly fixed on newer KVM versions, it doesn't
> hurt to base the view of the kvmclock after migration on the same foundation
> in host as well as guest.

Remove this last phrase from the changelog please, the underlying bug is
not fixed on newer KVM versions.

Otherwise

Reviewed-by: Marcelo Tosatti 

Re: [Qemu-devel] target-sparc has inverse cwp logic for SAVE/RESTORE?

[Olivier Danet]

On 18/05/2014 14:48, Mark Cave-Ayland wrote:
> Hi all,
> 
> I've been working on debugging a window-related OpenBIOS issue and noticed 
> that the cwp register logic in QEMU appears to be backwards according to the 
> SPARCv9 specification. From sections 6.3.6.1 and 6.3.6.2:
> 
> "The SAVE instruction allocates a new register window and saves the caller’s 
> register window by incrementing the CWP register."
> 
> "The RESTORE instruction restores the previous register window by 
> decrementing the CWP register."
> 
> In target-sparc/win_helper.c the logic in helper_save() and helper_restore() 
> is inverted, i.e. executing SAVE decrements cwp while executing RESTORE 
> increments cwp.
> 
> The surprise here was that executing SAVE when cwp == 0 changed cwp to 7 
> rather than 1. AFAICT there should be no functional difference, but it would 
> make things less confusing when debugging window traps if the logic from the 
> specification was followed. Does anyone know why this is currently done this 
> way?
> 
> 
> ATB,
> 
> Mark.
> 
The problem may be related to the fact that the 32bits SPARCv8 and 64bits 
SPARCv9 work in opposite directions !

SparcV9 standard, page 360/399 :
The SPARC-V9 CWP register is incremented during a SAVE instruction and 
decremented during
a RESTORE instruction. Although this is the opposite of PSR.CWP’s behavior in 
SPARC-V8, the
only software it should affect is a few trap handlers that operate in 
privileged mode, and that must
be rewritten for SPARC-V9 anyway. This change will have no effect on 
nonprivileged software.


Olivier

Re: [Qemu-devel] [PULL 00/10] input layer rework continued

[Olivier Danet]

On 16/05/2014 08:47, Gerd Hoffmann wrote:
>   Hi,
> 
> Update for the input layer.  Add keycode mapping helpers,
> start switching over devices to the new input api, fixes.
> 
> please pull,
>   Gerd
> 
> The following changes since commit 1a381811b495651ddfc2b50d7c8cfaaf375816b0:
> 
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-s390-20140515' into 
> staging (2014-05-15 18:56:08 +0100)
> 
> are available in the git repository at:
> 
> 
>   git://git.kraxel.org/qemu tags/pull-input-8
> 
> for you to fetch changes up to 59e7a130054b55fe15cdfdebf284332b04d990ef:
> 
>   input: sparc32 kbd: claim en-us layout (2014-05-16 08:30:12 +0200)
> 
> 
> Input code update:
>  - add keycode mapping helpers to core.
>  - start switching devices to new input api.
>  - misc bugfixes.
> 
> 
> Gerd Hoffmann (9):
>   input: key mapping helpers
>   input: add qemu_input_handler_deactivate
>   input: use KeyValue directly in sendkey monitor command
>   input: switch ps/2 kbd to new input api
>   input: switch ps/2 mouse to new input api
>   input: switch sparc32 kbd to new input api
>   input: remove sparc keymap hack
>   input: sparc32 kbd: fix some key mappings
>   input: sparc32 kbd: claim en-us layout
> 
> Gonglei (1):
>   ps2: set ps/2 output buffer size as the same as kernel
> 
>  hw/char/escc.c | 233 
> +++--
>  hw/input/ps2.c | 166 +-
>  include/ui/input.h |   5 ++
>  trace-events   |   4 +-
>  ui/Makefile.objs   |   3 +-
>  ui/input-keymap.c  | 191 +++
>  ui/input-legacy.c  | 226 +++
>  ui/input.c |   7 ++
>  8 files changed, 553 insertions(+), 282 deletions(-)
>  create mode 100644 ui/input-keymap.c
> 

Sorry for that very late answer !

This patchset works fine for me with the Sparc keyboard emulation.

Thank you for merging the new mappings.

Regards
Olivier

Re: [Qemu-devel] [Bug 1320360] [NEW] usb passthrough not working anymore

[Martin Röh]

Hi,

as far as I can see from the rpm specs of the opensuse rpm package the 
--enable-libusb is set .


Regards

Martin

Am 18.05.2014 06:52, schrieb Gonglei (Arei):

Hi,

 From qemu-1.7 release version, the old usb-host(host-linux.c) had been removed,
re-implemented by libusbx. So you should build qemu with --enable-libusb, then
you can use usb pass-through capacity.

BTW, Gerd, should we enable libusb by default now? Thanks.


Best regards,
-Gonglei


-Original Message-
From: qemu-devel-bounces+arei.gonglei=huawei@nongnu.org
[mailto:qemu-devel-bounces+arei.gonglei=huawei@nongnu.org] On
Behalf Of Martin R?h
Sent: Saturday, May 17, 2014 3:35 AM
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] [Bug 1320360] [NEW] usb passthrough not working
anymore

Public bug reported:

Hi,

I'm using qemu 2.0.0 with opensuse 13.1 x84_64 bit as host and window7
as guest. Til qemu version 1.6.2 USB passthrough works perfectly, but
starting with qemu 2.0.0 passthrough stop working. I can still add the
usb device but when I start the guest following message appears:

"unable to execute QEMU command 'device_add': 'usb-host' is not a valid
device model name"

Then the guest will not start.

I try it with different usb devices (iphone, stick, hdd), always the
same error.

Are there any news / hints about this ?

Regards

Martin

** Affects: qemu
  Importance: Undecided
  Status: New

--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1320360

Title:
   usb passthrough not working anymore

Status in QEMU:
   New

Bug description:
   Hi,

   I'm using qemu 2.0.0 with opensuse 13.1 x84_64 bit as host and window7
   as guest. Til qemu version 1.6.2 USB passthrough works perfectly, but
   starting with qemu 2.0.0 passthrough stop working. I can still add the
   usb device but when I start the guest following message appears:

   "unable to execute QEMU command 'device_add': 'usb-host' is not a
   valid device model name"

   Then the guest will not start.

   I try it with different usb devices (iphone, stick, hdd), always the
   same error.

   Are there any news / hints about this ?

   Regards

   Martin

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1320360/+subscriptions






smime.p7s
Description: S/MIME Cryptographic Signature

Re: [Qemu-devel] [Bug 1320360] [NEW] usb passthrough not working anymore

[Martin Röh]

Hi,

if I try to start the vm by virt-manager I get this detailed error log:

Fehler beim Starten der Domain: internal error: early end of file from 
monitor: possible problem:
qemu-system-x86_64: -device usb-host,hostbus=1,hostaddr=3,id=hostdev0: 
'usb-host' is not a valid device model name



Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/asyncjob.py", line 91, in 
cb_wrapper

callback(asyncjob, *args, **kwargs)
  File "/usr/share/virt-manager/virtManager/asyncjob.py", line 127, in 
tmpcb

callback(*args, **kwargs)
  File "/usr/share/virt-manager/virtManager/domain.py", line 1363, in 
startup

self._backend.create()
  File "/usr/lib64/python2.7/site-packages/libvirt.py", line 917, in create
if ret == -1: raise libvirtError ('virDomainCreate() failed', dom=self)
libvirtError: internal error: early end of file from monitor: possible 
problem:
qemu-system-x86_64: -device usb-host,hostbus=1,hostaddr=3,id=hostdev0: 
'usb-host' is not a valid device model name


Regards

Martin

Am 18.05.2014 06:52, schrieb Gonglei (Arei):

Hi,

 From qemu-1.7 release version, the old usb-host(host-linux.c) had been removed,
re-implemented by libusbx. So you should build qemu with --enable-libusb, then
you can use usb pass-through capacity.

BTW, Gerd, should we enable libusb by default now? Thanks.


Best regards,
-Gonglei


-Original Message-
From: qemu-devel-bounces+arei.gonglei=huawei@nongnu.org
[mailto:qemu-devel-bounces+arei.gonglei=huawei@nongnu.org] On
Behalf Of Martin R?h
Sent: Saturday, May 17, 2014 3:35 AM
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] [Bug 1320360] [NEW] usb passthrough not working
anymore

Public bug reported:

Hi,

I'm using qemu 2.0.0 with opensuse 13.1 x84_64 bit as host and window7
as guest. Til qemu version 1.6.2 USB passthrough works perfectly, but
starting with qemu 2.0.0 passthrough stop working. I can still add the
usb device but when I start the guest following message appears:

"unable to execute QEMU command 'device_add': 'usb-host' is not a valid
device model name"

Then the guest will not start.

I try it with different usb devices (iphone, stick, hdd), always the
same error.

Are there any news / hints about this ?

Regards

Martin

** Affects: qemu
  Importance: Undecided
  Status: New

--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1320360

Title:
   usb passthrough not working anymore

Status in QEMU:
   New

Bug description:
   Hi,

   I'm using qemu 2.0.0 with opensuse 13.1 x84_64 bit as host and window7
   as guest. Til qemu version 1.6.2 USB passthrough works perfectly, but
   starting with qemu 2.0.0 passthrough stop working. I can still add the
   usb device but when I start the guest following message appears:

   "unable to execute QEMU command 'device_add': 'usb-host' is not a
   valid device model name"

   Then the guest will not start.

   I try it with different usb devices (iphone, stick, hdd), always the
   same error.

   Are there any news / hints about this ?

   Regards

   Martin

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1320360/+subscriptions






smime.p7s
Description: S/MIME Cryptographic Signature

Re: [Qemu-devel] [Bug 1320360] Re: usb passthrough not working anymore

[Martin Röh]

The command line is

/usr/bin/qemu-system-x86_64 -machine accel=kvm -name win8 -S -machine 
pc-i440fx-2.0,accel=kvm,usb=off -cpu Nehalem -m 2048 -realtime mlock=off 
-smp 2,sockets=2,cores=1,threads=1 -uuid 
424ca5ec-2fb4-4d1c-84c4-25b92d468b8e -no-user-config -nodefaults 
-chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/win8.monitor,server,nowait 
-mon chardev=charmonitor,id=monitor,mode=control -rtc 
base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=discard 
-no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global 
PIIX4_PM.disable_s4=1 -boot strict=on -device 
ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device 
ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 
-device 
ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 
-device 
ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 
-device ahci,id=ahci0,bus=pci.0,addr=0x7 -drive 
file=/opt/emu/win8.raw,if=none,id=drive-virtio-disk0,format=raw -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=2
 
-netdev tap,fd=22,id=hostnet0,vhost=on,vhostfd=23 -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:44:d1:dc,bus=pci.0,addr=0x3 
-chardev pty,id=charserial0 -device 
isa-serial,chardev=charserial0,id=serial0 -device usb-tablet,id=input0 
-vnc 127.0.0.1:0 -device VGA,id=video0,bus=pci.0,addr=0x2 -device 
intel-hda,id=sound0,bus=pci.0,addr=0x8 -device 
hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6

Am 17.05.2014 01:15, schrieb Lekensteyn:
> Be sure to add the -usb option. What is your command line?
>
> See also
> http://git.qemu.org/?p=qemu.git;a=blob;f=docs/usb2.txt;h=c7a445afcd55fe1f12033d529d668a1306d5a9f4;hb=HEAD#l111
>

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1320360

Title:
  usb passthrough not working anymore

Status in QEMU:
  New

Bug description:
  Hi,

  I'm using qemu 2.0.0 with opensuse 13.1 x84_64 bit as host and window7
  as guest. Til qemu version 1.6.2 USB passthrough works perfectly, but
  starting with qemu 2.0.0 passthrough stop working. I can still add the
  usb device but when I start the guest following message appears:

  "unable to execute QEMU command 'device_add': 'usb-host' is not a
  valid device model name"

  Then the guest will not start.

  I try it with different usb devices (iphone, stick, hdd), always the
  same error.

  Are there any news / hints about this ?

  Regards

  Martin

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1320360/+subscriptions

Re: [Qemu-devel] OS X compile fix

[Peter Bartoli]

On May 18, 2014, at 4:40 AM, Peter Maydell  wrote:
> On 18 May 2014 09:50, Andreas Färber  wrote:
>> Am 18.05.2014 01:14, schrieb Peter Bartoli:
>>> At the recommendation of Mark Cave-Ayland, I'm sending this patch in to
>>> remedy a long-time Mac OS X compile issue.
>> 
>> Define "long-time"? 2.0 compiled just fine for me on v10.5.8 with
>> gcc-4.2. I see a number of deprecation and possibly-used-uninitialized
>> warnings but no breakage. What OSX version and what compiler are you
>> using, and what error or warning are you seeing without your patch?
> 
> Yeah, this must be only a problem on certain OSX setups -- it's
> never been an issue for me on 10.8/x86_64.
> 
> That said, it looks like LONG_LONG_MAX is a pre-C99 version
> and LLONG_MAX the standard macro name. We use LLONG_MAX
> elsewhere (though admittedly only in a test case) so I think it makes
> sense to make this change, especially if it fixes compilation on some
> setups.
> 
> (This use of LONG_LONG_MAX has been in the codebase since
> 2006, so presumably any configs which somehow fail to provide
> this macro have always failed to build without this fix...)
> 
>> Right now on qemu.git I have a different build issue: In scripts/qapi.py
>> "except IOError as e" needs to be "except IOError, e". But
>> block/raw-posix.c still builds fine here.
> 
> "except .. as" came in with python 2.6; we need to use the older
> syntax since we must work with 2.4 still. (Compare commit
> 21e0043bada1 where we fixed a previous round of this.)

So, just to add, in case it's helpful, "long time" means since 2012-ish, when 
QEMU first came to my attention.  I'm almost always running the current version 
of Mac OS X on the only system I'm running QEMU on, and have always had to use 
that patch to compile it.  This makes me wonder what I'm missing that gives you 
that macro.

I'm currently running Mac OS X 10.9.3 (current), I use the following configure 
options ...

--sysconfdir=/usr/local/share --mandir=/usr/local/man --enable-system 
--enable-user --enable-cocoa --disable-sdl --disable-gtk --enable-vhost-net 
--enable-guest-agent --enable-vhdx

... and whether I compile with Apple's built-in compiler, Apple LLVM version 
5.1 (clang-503.0.40) ...

  CCblock/raw-posix.o
block/raw-posix.c:1189:16: error: use of undeclared identifier 'LONG_LONG_MAX'
size = LONG_LONG_MAX;

... or apple-gcc-4.2 from MacPorts, which used to be the only option for me to 
successfully compile QEMU ...

  CCblock/raw-posix.o
block/raw-posix.c: In function 'raw_getlength':
block/raw-posix.c:1189: error: 'LONG_LONG_MAX' undeclared (first use in this 
function)
block/raw-posix.c:1189: error: (Each undeclared identifier is reported only once
block/raw-posix.c:1189: error: for each function it appears in.)
make: *** [block/raw-posix.o] Error 1

... or gcc 4.8, also wfrom MacPorts, I still get the same error.

  CCblock/raw-posix.o
block/raw-posix.c: In function 'raw_getlength':
block/raw-posix.c:1189:16: error: 'LONG_LONG_MAX' undeclared (first use in this 
function)
 size = LONG_LONG_MAX;
^
block/raw-posix.c:1189:16: note: each undeclared identifier is reported only 
once for each function it appears in
block/raw-posix.c: In function 'hdev_open':
block/raw-posix.c:1583:23: warning: variable 'kernResult' set but not used 
[-Wunused-but-set-variable]
 kern_return_t kernResult;
   ^
make: *** [block/raw-posix.o] Error 1

-peter

Re: [Qemu-devel] OS X compile fix

[Peter Maydell]

On 18 May 2014 23:45, Peter Bartoli  wrote:
> So, just to add, in case it's helpful, "long time" means since 2012-ish, when 
> QEMU first came to my attention.  I'm almost always running the current 
> version of Mac OS X on the only system I'm running QEMU on, and have always 
> had to use that patch to compile it.  This makes me wonder what I'm missing 
> that gives you that macro.

For me this simple test program builds:

manooth$ cat /tmp/zz9.c
#include 
#include 

int main(void) {
printf("LONG_LONG_MAX = %llx\n", LONG_LONG_MAX);
return 0;
}
manooth$ clang -o /tmp/zz9  /tmp/zz9.c
manooth$ /tmp/zz9
LONG_LONG_MAX = 7fff
manooth$ clang --version
Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn)
Target: x86_64-apple-darwin12.5.0
Thread model: posix

and looking at the preprocessor output it's defined in
/usr/lib/clang/5.0/include/limits.h

(with some include guards, but we're not compiling with
flags that force strict ANSI mode).

However this is just for interest's sake -- we should use the
standard-defined macro rather than the GNU extension
where the former exists...

thanks
-- PMM

Re: [Qemu-devel] OS X compile fix

[Peter Bartoli]

On May 18, 2014, at 4:09 PM, Peter Maydell  wrote:
> and looking at the preprocessor output it's defined in
> /usr/lib/clang/5.0/include/limits.h

Thanks, Peter ... I don't have a /usr/lib/clang ... where are you getting it 
from?

-peter

Re: [Qemu-devel] [Xen-devel] [v2][PATCH 6/8] xen, gfx passthrough: support Intel IGD passthrough with VT-D

[Zhang, Yang Z]

Konrad Rzeszutek Wilk wrote on 2014-05-16:
> On Fri, May 16, 2014 at 06:53:42PM +0800, Tiejun Chen wrote:
> > Some registers of Intel IGD are mapped in host bridge, so it needs to
> > passthrough these registers of physical host bridge to guest because
> > emulated host bridge in guest doesn't have these mappings.

Thanks for your review for the whole series patch.

> 
> When you say mapped - you mean they are aliased - so if I change the value in
> the IGD they will change in the host bridge as well, right?

I guess you mean the physical host bridge. For it, only PAVPC will write to 
physical host bridge directly, see:

+switch (config_addr) {
+case 0x58:  /* PAVPC Offset */
+break;
+default:
+goto write_default;
+}


> 
> >
> > The original patch is from Weidong Han < weidong.han @ intel.com >
> >
> > Signed-off-by: Yang Zhang 
> > Signed-off-by: Tiejun Chen  Cc:Weidong Han
> > 
> > ---
> > v2:
> >
> > * To introduce is_igd_passthrough() to make sure we touch physical host
> bridge
> >   only in IGD case.
> >
> >  hw/xen/xen_pt.h  |   4 ++
> >  hw/xen/xen_pt_graphics.c | 140
> > +++
> >  2 files changed, 144 insertions(+)
> >
> > diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index 4d3a18d..507165c
> > 100644
> > --- a/hw/xen/xen_pt.h
> > +++ b/hw/xen/xen_pt.h
> > @@ -302,5 +302,9 @@ extern int xen_has_gfx_passthru;  int
> > xen_pt_register_vga_regions(XenHostPCIDevice *dev);  int
> > xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);  int
> > xen_pt_setup_vga(XenHostPCIDevice *dev);
> > +int pci_create_pch(PCIBus *bus);
> > +void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
> > +   uint32_t val, int len); uint32_t
> > +igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
> >
> >  #endif /* !XEN_PT_H */
> > diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c index
> > 6b86293..066bc4d 100644
> > --- a/hw/xen/xen_pt_graphics.c
> > +++ b/hw/xen/xen_pt_graphics.c
> > @@ -4,6 +4,7 @@
> >  #include "xen_pt.h"
> >  #include "xen-host-pci-device.h"
> >  #include "hw/xen/xen_backend.h"
> > +#include "hw/pci/pci_bus.h"
> >
> >  static int is_vga_passthrough(XenHostPCIDevice *dev)  { @@ -246,3
> > +247,142 @@ static int create_pch_isa_bridge(PCIBus *bus,
> XenHostPCIDevice *hdev)
> >  XEN_PT_LOG(dev, "Intel PCH ISA bridge is created.\n");
> >  return 0;
> >  }
> > +
> > +int pci_create_pch(PCIBus *bus)
> > +{
> > +XenHostPCIDevice hdev;
> > +int r = 0;
> > +
> > +if (!xen_has_gfx_passthru) {
> > +return -1;
> > +}
> > +
> > +r = xen_host_pci_device_get(&hdev, 0, 0, 0x1f, 0);
> > +if (r) {
> > +XEN_PT_ERR(NULL, "Fail to find intel PCH in host\n");
> 
> Failed to find Intel PCH on host!
> 
> > +goto err;
> > +}
> > +
> > +if (hdev.vendor_id == PCI_VENDOR_ID_INTEL) {
> > +r = create_pch_isa_bridge(bus, &hdev);
> > +if (r) {
> > +XEN_PT_ERR(NULL, "Fail to create PCH ISA bridge.\n");
> 
> Failed
> > +goto err;
> > +}
> > +}
> > +
> > +xen_host_pci_device_put(&hdev);
> > +
> > +return  r;
> 
> Remove this return and let it go through.
> > +
> > +err:
> > +return r;
> > +}
> > +
> > +/*
> > + * Currently we just pass this physical host bridge for IGD, 00:02.0.
> > + */
> > +static int is_igd_passthrough(PCIDevice *pci_dev) {
> > +PCIDevice *f = pci_dev->bus->devices[PCI_DEVFN(2, 0)];
> > +if (pci_dev->bus->devices[PCI_DEVFN(2, 0)]) {
> > +XenPCIPassthroughState *s =
> DO_UPCAST(XenPCIPassthroughState, dev, f);
> > +return (is_vga_passthrough(&s->real_device)
> > +&& (s->real_device.vendor_id ==
> PCI_VENDOR_ID_INTEL));
> > +} else {
> > +return 0;
> > +}
> > +}
> > +
> > +void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
> > +   uint32_t val, int len) {
> > +XenHostPCIDevice dev;
> > +int r;
> > +
> > +assert(pci_dev->devfn == 0x00);
> 
> Now I am confused. That would mean the host bridge, while earlier
> (pci_create_pch) you were using the ISA bridge.

IGD read/write is through the host bridge.
ISA bridge is only for detect purpose. In i915 driver it will probe ISA bridge 
to discover the IGD, see comment in i915_drv.c:
intel_detect_pch():
 * The reason to probe ISA bridge instead of Dev31:Fun0 is to
 * make graphics device passthrough work easy for VMM, that only
 * need to expose ISA bridge to let driver know the real hardware
 * underneath. This is a requirement from virtualization team.



> 
> Could you add a comment please?

Sure. 

> > +
> > +if (!is_igd_passthrough(pci_dev)) {
> > +goto write_default;
> > +}
> > +
> > +switch (config_addr) {
> > +case 0x58:  /* PAVPC Offset */
> > +break;
> > +default:
> > +goto write_default;
> 
> As I understand this function will be called when the g

Re: [Qemu-devel] [Bug 1320360] [NEW] usb passthrough not working anymore

[Gonglei (Arei)]

Hi,

> -Original Message-
> From: Martin Röh [mailto:mar...@roeh.org]
> Sent: Monday, May 19, 2014 4:40 AM
> To: Gonglei (Arei); Bug 1320360; qemu-devel@nongnu.org
> Cc: Gerd Hoffmann
> Subject: Re: [Qemu-devel] [Bug 1320360] [NEW] usb passthrough not working
> anymore
> 
> Hi,
> 
> if I try to start the vm by virt-manager I get this detailed error log:
> 
> Fehler beim Starten der Domain: internal error: early end of file from
> monitor: possible problem:
> qemu-system-x86_64: -device usb-host,hostbus=1,hostaddr=3,id=hostdev0:
> 'usb-host' is not a valid device model name
> 
> 
> Traceback (most recent call last):
>File "/usr/share/virt-manager/virtManager/asyncjob.py", line 91, in
> cb_wrapper
>  callback(asyncjob, *args, **kwargs)
>File "/usr/share/virt-manager/virtManager/asyncjob.py", line 127, in
> tmpcb
>  callback(*args, **kwargs)
>File "/usr/share/virt-manager/virtManager/domain.py", line 1363, in
> startup
>  self._backend.create()
>File "/usr/lib64/python2.7/site-packages/libvirt.py", line 917, in create
>  if ret == -1: raise libvirtError ('virDomainCreate() failed', dom=self)
> libvirtError: internal error: early end of file from monitor: possible
> problem:
> qemu-system-x86_64: -device usb-host,hostbus=1,hostaddr=3,id=hostdev0:
> 'usb-host' is not a valid device model name
> 
The above error information shows "usb-host" didn't been built in 
qemu-system-x86_64
binary file. You can get the qemu-2.0.0 source files from 
http://wiki.qemu.org/Download
and rebuild it with '--enable-libusb' during configure.


Best regards,
-Gonglei

Re: [Qemu-devel] [RFC v1 11/25] irq: Slim conversion of qemu_irq to QOM [WIP]

[Peter Crosthwaite]

On Fri, May 16, 2014 at 11:56 AM, Peter Crosthwaite
 wrote:
> From: Andreas Färber 
>
> As a prequel to any big Pin refactoring plans, do an in-place conversion
> of qemu_irq to an Object, so that we can reference it in link<> properties.
>
> Signed-off-by: Andreas Färber 
> ---
>
>  hw/core/irq.c| 44 +---
>  include/hw/irq.h |  2 ++
>  2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/irq.c b/hw/core/irq.c
> index 03c8cb3..0bcd27b 100644
> --- a/hw/core/irq.c
> +++ b/hw/core/irq.c
> @@ -23,8 +23,13 @@
>   */
>  #include "qemu-common.h"
>  #include "hw/irq.h"
> +#include "qom/object.h"
> +
> +#define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ)
>
>  struct IRQState {
> +Object parent_obj;
> +
>  qemu_irq_handler handler;
>  void *opaque;
>  int n;
> @@ -38,6 +43,14 @@ void qemu_set_irq(qemu_irq irq, int level)
>  irq->handler(irq->opaque, irq->n, level);
>  }
>
> +static void irq_nonfirst_free(void *obj)
> +{
> +struct IRQState *s = obj;
> +
> +/* Unreference the first IRQ in this array */
> +object_unref(OBJECT(s - s->n));
> +}
> +
>  qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler 
> handler,
> void *opaque, int n)
>  {
> @@ -51,11 +64,23 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, 
> qemu_irq_handler handler,
>  s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
>  p = old ? g_renew(struct IRQState, s[0], n + n_old) :
>  g_new(struct IRQState, n);

So using g_renew on the actual object storage is very fragile, as it
means you cannot reliably take pointers to the objects. I think
however this whole issue could be simplified by de-arrayifying the
whole thing, and treating IRQs individually (interdiff):

 qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
void *opaque, int n)
 {
 qemu_irq *s;
-struct IRQState *p;
 int i;

 if (!old) {
 n_old = 0;
 }
 s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
-p = old ? g_renew(struct IRQState, s[0], n + n_old) :
-g_new(struct IRQState, n);
-memset(p + n_old, 0, n * sizeof(*p));
 for (i = 0; i < n + n_old; i++) {
 if (i >= n_old) {
-Object *obj;
-
-object_initialize(p, sizeof(*p), TYPE_IRQ);
-p->handler = handler;
-p->opaque = opaque;
-p->n = i;
-obj = OBJECT(p);
-/* Let the first IRQ clean them all up */
-if (unlikely(i == 0)) {
-obj->free = g_free;
-} else {
-object_ref(OBJECT(s[0]));
-obj->free = irq_nonfirst_free;
-}
+s[i] = qemu_allocate_irq(handler, opaque, i);
 }
-s[i] = p;
-p++;
 }
 return s;

The system for freeing may need some thought, and I wonder if their
are API clients out there assuming the IRQ storage elements are
contiguous (if there are they have too much internal knowledge and
need to be fixed).

But with this change these objects are now usable with links and canon
paths etc.

Will roll into V2 of this patch.

Regards,
Peter

Re: [Qemu-devel] [RFC v1 17/25] sysbus: Use TYPE_DEVICE GPIO functionality

[Peter Crosthwaite]

On Fri, May 16, 2014 at 11:59 AM, Peter Crosthwaite
 wrote:
> Re-implement the Sysbus GPIOs to use the existing TYPE_DEVICE
> GPIO named framework. A constant string name is chosen to avoid
> conflicts with existing unnamed GPIOs.
>
> This unifies GPIOs are IRQs for sysbus devices and allows removal
> of all Sysbus state for GPIOs.
>
> Any existing and future-added functionality for GPIOs is now
> also available for sysbus IRQs.
>
> For the anti-sysbus campaigners, this patch brings us one step
> closer to deleting the abstraction completely.
>
> Signed-off-by: Peter Crosthwaite 
> ---
>
>  hw/core/sysbus.c| 20 +++-
>  include/hw/sysbus.h |  7 +++
>  2 files changed, 6 insertions(+), 21 deletions(-)
>
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index f4e760d..2fae2bd 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -41,11 +41,7 @@ static const TypeInfo system_bus_info = {
>
>  void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
>  {
> -assert(n >= 0 && n < dev->num_irq);
> -dev->irqs[n] = NULL;
> -if (dev->irqp[n]) {
> -*dev->irqp[n] = irq;
> -}
> +qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);
>  }
>
>  static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
> @@ -89,22 +85,13 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, 
> hwaddr addr,
>  /* Request an IRQ source.  The actual IRQ object may be populated later.  */
>  void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
>  {
> -int n;
> -
> -assert(dev->num_irq < QDEV_MAX_IRQ);
> -n = dev->num_irq++;
> -dev->irqp[n] = p;
> +qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1);

This is not repeatable as qemu_init_gpio_out() doesnt allow repeated
calls for the same GPIO output set (doh!) - that feature only exists
for GPIO inputs. Rather than investing further into unnamed GPIOs on
the DEVICE level though, I think it's better to just name each sysbus
IRQ individually. If you really really want arrayified IRQs then you
can still use the DEVICE GPIO API directly. To fix in V2:

 /* Request an IRQ source.  The actual IRQ object may be populated later.  */
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
 {
-qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1);
+char *irq_name = g_strdup_printf(SYSBUS_DEVICE_GPIO_IRQ "-%d",
+ dev->num_irq++);
+qdev_init_gpio_out_named(DEVICE(dev), p, irq_name, 1);
+g_free(irq_name);
 }

Regards,
Peter

Re: [Qemu-devel] [PATCH V26 00/32] replace QEMUOptionParameter with QemuOpts

[Chun Yan Liu]

>>> On 5/6/2014 at 09:26 PM, in message
<20140506132615.gv15...@stefanha-thinkpad.redhat.com>, Stefan Hajnoczi
 wrote: 
> On Tue, Apr 29, 2014 at 05:10:24PM +0800, Chunyan Liu wrote: 
> > This patch series is to replace QEMUOptionParameter with QemuOpts, so that  
> only 
> > one Qemu Option structure is kept in QEMU code. 
> >  
> > --- 
> > Changes to v25: 
> >   * split v25 2/31 (add def_value_str to QemuOptDesc) into two patches: 
> > 1st patch adds def_value_str, 2nd patch repurpose qemu_opts_print. 
> >   * update 4/32 qapi command line description. 
> >   * update 12/32 (change block layer to support both) according to 
> > Eric's comments. 
> >   * small update to gluster.c 
> >   * rebase to latest code 
> >  
> > All patches are also available from: 
> > https://github.com/chunyanliu/qemu/commits/QemuOpts 
>  
> I looked through the comments from Leandro and Eric.  A respin is 
> necessary to address the test failures that Leandro found.  Please also 
> address the comments Eric made. 
>  
> We're almost there now! 
>  
>  

Hi, Stefan, Leandro & Eric,

Could you help to have a look at the new update? I've sent it about 10
days ago.  It addressed all Eric's comments in last version and two
memory free fix to address iotest issue. Hope it's the final work and
won't take you more time :-)

The thread has been cut into two, since the git-send-email died in the
middle at the 1st time sending. Please refer to:

[0~25]
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg01098.html
[26~33]
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg01135.html

Thanks!

-Chunyan

Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexey Kardashevskiy]

On 05/17/2014 11:45 AM, Alexey Kardashevskiy wrote:
> On 05/17/2014 06:46 AM, Alexander Graf wrote:
>>
>> On 16.05.14 17:57, Alexey Kardashevskiy wrote:
>>> On 05/17/2014 12:16 AM, Alexander Graf wrote:
 On 15.05.14 13:28, Alexey Kardashevskiy wrote:
> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
> list. However if the kernel is quite old, it may be missing a definition
> of the actual CPU. To provide an ability for old kernels to work on modern
> hardware, a Processor Compatibility Mode has been introduced
> by the PowerISA specification.
>
>   From the hardware prospective, it is supported by the Processor
> Compatibility Register (PCR) which is defined in PowerISA. The register
> enables one of the compatibility modes (2.05/2.06/2.07).
> Since PCR is a hypervisor privileged register and cannot be
> accessed from the guest, the mode selection is done via
> ibm,client-architecture-support (CAS) RTAS call using which the guest
> specifies what "raw" and "architected" CPU versions it supports.
> QEMU works out the best match, changes a "cpu-version" property of
> every CPU and notifies the guest about the change by setting these
> properties in the buffer passed as a response on a custom H_CAS hypercall.
>
> Signed-off-by: Alexey Kardashevskiy 
> ---
>hw/ppc/spapr.c   | 40 +
>hw/ppc/spapr_hcall.c | 83
> 
>trace-events |  5 
>3 files changed, 128 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a2c9106..a0882a1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -35,6 +35,7 @@
>#include "kvm_ppc.h"
>#include "mmu-hash64.h"
>#include "cpu-models.h"
> +#include "qom/cpu.h"
>  #include "hw/boards.h"
>#include "hw/ppc/ppc.h"
> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr,
> target_ulong size)
>{
>void *fdt;
>sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> +CPUState *cs;
> +int smt = kvmppc_smt_threads();
>  size -= sizeof(hdr);
>  fdt = g_malloc0(size);
>_FDT((fdt_create(fdt, size)));
> +_FDT((fdt_begin_node(fdt, "cpus")));
> +
> +CPU_FOREACH(cs) {
> +PowerPCCPU *cpu = POWERPC_CPU(cs);
> +DeviceClass *dc = DEVICE_GET_CLASS(cpu);
> +int smpt = spapr_get_compat_smp_threads(cpu);
> +int i, index = ppc_get_vcpu_dt_id(cpu);
> +uint32_t servers_prop[smpt];
> +uint32_t gservers_prop[smpt * 2];
> +char tmp[32];
> +
> +if ((index % smt) != 0) {
> +continue;
> +}
> +
> +snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
> +trace_spapr_cas_add_subnode(tmp);
> +
> +_FDT((fdt_begin_node(fdt, tmp)));
> +_FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
> +
> +/* Build interrupt servers and gservers properties */
> +for (i = 0; i < smpt; i++) {
> +servers_prop[i] = cpu_to_be32(index + i);
> +/* Hack, direct the group queues back to cpu 0 */
> +gservers_prop[i*2] = cpu_to_be32(index + i);
> +gservers_prop[i*2 + 1] = 0;
> +}
> +_FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
> +   servers_prop, sizeof(servers_prop;
> +_FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
> +   gservers_prop, sizeof(gservers_prop;
> +
> +_FDT((fdt_end_node(fdt)));
> +}
> +
> +_FDT((fdt_end_node(fdt)));
 Why is this so much code? Can we only replace full nodes?
>>> It is a diff tree, in only has properties to change.
>>
>> So why do we change so much then? :)
> 
> 
> 3 (three) properties are "much" now? :)
> 
 Then please
 extract the original CPU node creation into a function and just call it
 from the original generation and from here.

 If we can also replace single properties I'd say we only need to override
 cpu-version.
>>>
>>> Mmm. The user could run qemu with threads=8 on POWER8 with SLES11 which
>>> must not see 8 threads but it can update itself and reboot with the kernel
>>> which is aware of POWER8. You are saying we do not want to support that?
>>
>> First off, I think that practically speaking people will want to use
>> 2-thread granularity on POWER8 because that gives them the best load
>> exploitation on the system.
> 
> Practically speaking, people will want to use kernels which can work on
> POWER8 in raw mode, no? Here we are trying

Re: [Qemu-devel] [Qemu-ppc] [PATCH 6/9] spapr: Add ibm, client-architecture-support call

[Nikunj A Dadhania]

Alexey Kardashevskiy  writes:

> The PAPR+ specification defines a ibm,client-architecture-support (CAS)
> RTAS call which purpose is to provide a negotiation mechanism for
> the guest and the hypervisor to work out the best compatibility parameters.
> During the negotiation process, the guest provides an array of various
> options and capabilities which it supports, the hypervisor adjusts
> the device tree and (optionally) reboots the guest.
>
> At the moment the Linux guest calls CAS method at early boot so SLOF
> gets called. SLOF allocates a memory buffer for the device tree changes
> and calls a custom KVMPPC_H_CAS hypercall. QEMU parses the options,
> composes a diff for the device tree, copies it to the buffer provided
> by SLOF and returns to SLOF. SLOF updates the device tree and returns
> control to the guest kernel. Only then the Linux guest parses the device
> tree so it is possible to avoid unnecessary reboot in most cases.
>
> The device tree diff is a header with the update format version
> (defined as 0 in this patch) 

That is 1 in the code.

> @@ -562,6 +563,31 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>  return fdt;
>  }
>
> +int spapr_h_cas_compose_response(target_ulong addr, target_ulong size)
> +{
> +void *fdt;
> +sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> +

Regards
Nikunj

Re: [Qemu-devel] [Bug 1320360] [NEW] usb passthrough not working anymore

[Gerd Hoffmann]

On So, 2014-05-18 at 22:36 +0200, Martin Röh wrote:
> Hi,
> 
> as far as I can see from the rpm specs of the opensuse rpm package the 
> --enable-libusb is set .

> > BTW, Gerd, should we enable libusb by default now? Thanks.

By default libusb will be used when found on the system.
When it isn't there qemu will be built without usb-host support.

If you explicitly ask for libusb support (via --enable-libusb) and
libusbx isn't found configure should fail.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] configure: Automatically select GTK+ 3.0 if GTK+ 2.0 is unavailable

[Gerd Hoffmann]

  Hi,

> +if test "$gtkabi" = ""; then
> +# The GTK ABI was not specified explicitly, so try whether 2.0 is 
> available.
> +# Use 3.0 as a fallback if that is available.
> +if $pkg_config --exists "gtk+-2.0 >= 2.18.0"; then
> +gtkabi=2.0
> +elif $pkg_config --exists "gtk+-3.0 >= 3.0.0"; then
> +gtkabi=3.0
> +else
> +gtkabi=2.0
> +fi
> +fi

Shouldn't we probe for gtk3 first?

cheers,
  Gerd

[Qemu-devel] [PATCH] virtio-balloon: return empty data when no stats are available

[Ján Tomko]

If the guest hasn't updated the stats yet, instead of returning
an error, return '-1' for the stats and '0' as 'last-update'.

This lets applications ignore this without parsing the error message.

Related libvirt patch and discussion:
https://www.redhat.com/archives/libvir-list/2014-May/msg00460.html

Signed-off-by: Ján Tomko 
---
 hw/virtio/virtio-balloon.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 971a921..6661c53 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -111,11 +111,6 @@ static void balloon_stats_get_all(Object *obj, struct 
Visitor *v,
 VirtIOBalloon *s = opaque;
 int i;
 
-if (!s->stats_last_update) {
-error_setg(errp, "guest hasn't updated any stats yet");
-return;
-}
-
 visit_start_struct(v, NULL, "guest-stats", name, 0, errp);
 visit_type_int(v, &s->stats_last_update, "last-update", errp);
 
@@ -361,6 +356,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, 
Error **errp)
 s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
 s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
 
+reset_stats(s);
+
 register_savevm(dev, "virtio-balloon", -1, 1,
 virtio_balloon_save, virtio_balloon_load, s);
 
-- 
1.8.3.2

Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD

[Gerd Hoffmann]

  Hi,

> +/*
> + * Some video bioses and gfx drivers will assume the bdf of IGD is 
> 00:02.0.
> + * So user need to set it to 00:02.0 in Xen configure file explicitly,
> + * otherwise IGD will fail to work.
> + */
> +pci_reserve_pci_devfn(b, PCI_DEVFN(2, 0));

That is asking for trouble.  Slot 2 is used by the qemu vga cards by
default, and for quite a while (before memory api was merged) it even
was impossible to change it.  libvirt still places the vga card at slot
2 for that reason -> boom.  I wouldn't be surprised if you find that
assumption in other management libs / apps too.

Why do you need that patch in the first place?  It should be possible to
configure qemu to not occupy slot 2 if you need it that way.  Just pass
'-vga none' to qemu.  Which you probably want anyway if you pass-through
a vga to the guest.  And explicitly configure a slot (via addr=
property) for all your pci devices.  Doing it only for the IGD works too
if you list the device before any other pci device on the qemu command
line.

cheers,
  Gerd

Re: [Qemu-devel] [libvirt] [PATCHv2] Don't log an internal error when the guest hasn't updated balloon stats

[Ján Tomko]

On 05/16/2014 03:13 PM, Luiz Capitulino wrote:
> On Fri, 16 May 2014 00:11:24 -0600
> Eric Blake  wrote:
> 
>>> Is "no stats yet" really an error? 
> 
> This is a special case where the guest hasn't ever filled QEMU with balloon
> stats. There are two possible cases. Either the guest hasn't done it yet, but
> will do in the future or the guest will never do it (eg. the guest doesn't
> support balloon, the guest crashed, etc).
> 
>>> Libvirt has done nothing wrong, and
>>> I'd argue the guest hasn't done anything wrong, either.  Should we
>>> simply return an empty result?  Like "cat" on a file that hasn't gotten
>>> its data, yet.
>>
>> Yes, that would be reasonable.
> 
> I'm fine with the two possible solutions here: adding a new TryAgain error
> class or returning an "empty" result.
> 
> I say "empty" because those fields are not optionals, so we'll have to fill
> them with some value. Shouldn't be a problem for most fields, as the spec
> (docs/virtio-balloon-stats.txt) already defines that stats that the guest
> doesn't report are returned as -1. The only exception here is the last-update
> field, which can't hold a negative iirc. The only choice is to return 0 there.
> I guess that this shouldn't be a problem either.
> 
> Who volunteers to fix this?
> 

I've tried:

http://marc.info/?l=qemu-devel&m=140048179520115&w=2

Jan



signature.asc
Description: OpenPGP digital signature