Re: [Qemu-devel] [PATCH 6/6] xics-kvm: Support for in-kernel XICS interrupt controller

[Alexey Kardashevskiy]

On 08/06/2013 08:12 PM, Andreas Färber wrote:
>> +/* Call emulated XICS implementation for consistency */
>> +assert(xics_info && xics_info->cpu_setup);
>> +xics_info->cpu_setup(icp, cpu);
>> +}
>> +
>> +void xics_kvm_set_nr_irqs(XICSState *icp, uint32_t nr_irqs)
>> +{
>> +icp->ics = ICS(object_new(TYPE_ICS_KVM));
>> +object_property_add_child(OBJECT(icp), "ics", OBJECT(icp->ics), NULL);
>> +icp->ics->icp = icp;
> 
> instance_init + object_initialize()?

Create ICS_KVM in instance_init of XICS_KVM and initialize it here? Why split?

> 
>> +icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
>> +}
>> +
>> +void xics_kvm_set_nr_servers(XICSState *icp, uint32_t nr_servers)
>> +{
>> +int i;
>> +
>> +icp->nr_servers = nr_servers;
>> +
>> +icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
>> +for (i = 0; i < icp->nr_servers; i++) {
>> +char buffer[32];
>> +object_initialize(&icp->ss[i], TYPE_ICP_KVM);
>> +snprintf(buffer, sizeof(buffer), "icp[%d]", i);
>> +object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]), 
>> NULL);
>> +}
>> +}
>> +
>> +static void rtas_dummy(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>> +   uint32_t token,
>> +   uint32_t nargs, target_ulong args,
>> +   uint32_t nret, target_ulong rets)
>> +{
>> +error_report("pseries: %s must never be called for in-kernel XICS\n",
>> +__func__);
>> +}
>> +
>> +static void xics_kvm_realize(DeviceState *dev, Error **errp)
>> +{
>> +KVMXICSState *icpkvm = KVM_XICS(dev);
>> +XICSState *icp = XICS_COMMON(dev);
>> +int i, rc;
>> +Error *error = NULL;
>> +struct kvm_create_device xics_create_device = {
>> +.type = KVM_DEV_TYPE_XICS,
>> +.flags = 0,
>> +};
>> +
>> +assert(kvm_enabled());
>> +assert(kvm_check_extension(kvm_state, KVM_CAP_IRQ_XICS));
> 
> error_setg() - if device can be created without accel=kvm (which it
> looks as if it can) then you should just error out the nice way here.


I check kvm_enabled() (I thought I check both but was wrong, will fix it)
where I try to create XICS_KVM so we should not be here if that check failed.


-- 
Alexey

Re: [Qemu-devel] [PATCH 5/6] xics: split to xics and xics-common

[Andreas Färber]

Am 07.08.2013 08:06, schrieb Alexey Kardashevskiy:
> On 08/06/2013 07:53 PM, Andreas Färber wrote:
>> Am 06.08.2013 10:27, schrieb Alexey Kardashevskiy:
>>> +}
>>> +}
>>> +
>>> +void xics_prop_set_nr_irqs(Object *obj, struct Visitor *v,
>>> +   void *opaque, const char *name, struct Error 
>>> **errp)
>>
>> You should be able to drop both "struct"s.
> 
> Yeah, fixed. This is just how the ObjectPropertyAccessor type is defined.

Yeah, reason is some circular header dependencies. ;)

>>> +static void xics_common_initfn(Object *obj)
>>> +{
>>> +object_property_add(obj, "nr_irqs", "int",
>>> +NULL, xics_prop_set_nr_irqs, NULL, NULL, NULL);
>>> +object_property_add(obj, "nr_servers", "int",
>>> +NULL, xics_prop_set_nr_servers, NULL, NULL, NULL);
>>
>> Since this property is visible in the QOM tree, it would be nice to
>> implement trivial getters returns the value from the state fields.
> 
> Added. How do I test it?

./QMP/qom-list to find the path, if not fixed in code yet, and
./QMP/qom-get path.nr_servers to retrieve the value,
./QMP/qom-set path.nr_servers to verify it doesn't kill the process.

-qmp unix:./qmp-sock,server,nowait is what I use on the QEMU side.

> "info qtree" prints only
> DEVICE_CLASS(class)->props which is null in this case.

True, info qtree is from qdev times and deprecated. I recently attached
a "qom-tree" script doing the equivalent that you could try.

I'll try to address -device xics,? showing them for 1.7. (Anthony
indicated on a KVM call that the expected way to discover properties was
to instantiate the type rather than looking at its class. Requires that
types are instantiatable without asserting KVM accelerator, which gets
initialized later.)

>>> @@ -651,47 +765,32 @@ static void rtas_int_on(PowerPCCPU *cpu, 
>>> sPAPREnvironment *spapr,
>>>  /*
>>>   * XICS
>>>   */
>>> +void xics_set_nr_irqs(XICSState *icp, uint32_t nr_irqs)
>>> +{
>>> +icp->ics = ICS(object_new(TYPE_ICS));
>>> +object_property_add_child(OBJECT(icp), "ics", OBJECT(icp->ics), NULL);
>>
>> Why create this single object in the setter? Can't you just create this
>> in the common type's instance_init similar to before but using
>> object_initialize() instead of object_new() and
>> object_property_set_bool() in the realizefn?
> 
> I cannot create in the common code as there are TYPE_ICS and TYPE_ICS_KVM
> (oops, KVM is at the end, will fix it) types.
> 
> And I would really want not to create it in instance_init() as I want to
> have the object created and all its properties initialized in one place.

Doing it in instance_init facilitates improving the setter to allow
multiple uses as a follow-up patch, since it must only be created once.
If you don't, then the child will not be present in the composition tree
before setting this seemingly unrelated property. That seems worse to me
than accessing a field in two places - instance_init will be run before
any property setter.

BTW these dynamic setters do not check automatically whether the object
has already been realized. If you want the setter to return an Error in
that case, you will need to check for DeviceState *dev = DEVICE(icp);
dev->realized yourself. Not sure if that's the semantics you desire, but
I guess so?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 6/6] xics-kvm: Support for in-kernel XICS interrupt controller

[Andreas Färber]

Am 07.08.2013 09:03, schrieb Alexey Kardashevskiy:
> On 08/06/2013 08:12 PM, Andreas Färber wrote:
>>> +/* Call emulated XICS implementation for consistency */
>>> +assert(xics_info && xics_info->cpu_setup);
>>> +xics_info->cpu_setup(icp, cpu);
>>> +}
>>> +
>>> +void xics_kvm_set_nr_irqs(XICSState *icp, uint32_t nr_irqs)
>>> +{
>>> +icp->ics = ICS(object_new(TYPE_ICS_KVM));
>>> +object_property_add_child(OBJECT(icp), "ics", OBJECT(icp->ics), NULL);
>>> +icp->ics->icp = icp;
>>
>> instance_init + object_initialize()?
> 
> Create ICS_KVM in instance_init of XICS_KVM and initialize it here? Why split?

Answered on 5/6 (mail bounced at first due to kernel.crashing.org).

>>> +icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
>>> +}
>>> +
>>> +void xics_kvm_set_nr_servers(XICSState *icp, uint32_t nr_servers)
>>> +{
>>> +int i;
>>> +
>>> +icp->nr_servers = nr_servers;
>>> +
>>> +icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
>>> +for (i = 0; i < icp->nr_servers; i++) {
>>> +char buffer[32];
>>> +object_initialize(&icp->ss[i], TYPE_ICP_KVM);
>>> +snprintf(buffer, sizeof(buffer), "icp[%d]", i);
>>> +object_property_add_child(OBJECT(icp), buffer, 
>>> OBJECT(&icp->ss[i]), NULL);
>>> +}
>>> +}
[...]
>>> +static void xics_kvm_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +KVMXICSState *icpkvm = KVM_XICS(dev);
>>> +XICSState *icp = XICS_COMMON(dev);
>>> +int i, rc;
>>> +Error *error = NULL;
>>> +struct kvm_create_device xics_create_device = {
>>> +.type = KVM_DEV_TYPE_XICS,
>>> +.flags = 0,
>>> +};
>>> +
>>> +assert(kvm_enabled());
>>> +assert(kvm_check_extension(kvm_state, KVM_CAP_IRQ_XICS));
>>
>> error_setg() - if device can be created without accel=kvm (which it
>> looks as if it can) then you should just error out the nice way here.
> 
> 
> I check kvm_enabled() (I thought I check both but was wrong, will fix it)
> where I try to create XICS_KVM so we should not be here if that check failed.

-device, -object and patch Anthony's patch QMP are other ways to
instantiate the same type. :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC] [PATCHv5 06/16] aio / timers: Untangle include files

[Stefan Hajnoczi]

On Wed, Aug 07, 2013 at 12:52:57AM +0100, Alex Bligh wrote:
> --On 6 August 2013 12:10:17 +0200 Stefan Hajnoczi
>  wrote:
> 
> >Fails to build ui/vnc-auth-sasl.c
> >
> >Please make sure that your ./configure output shows most optional
> >dependencies are available (you need to install development packages for
> >these libraries).  Otherwise you will not see build breakage.
> 
> I think I have caught most of these I can test on Ubuntu Precise in
> my v7 patch. I enabled all options I could, and installed all the
> dev options I could for linux. Some things would not build, despite
> installing the development libraries (by 'not build' I mostly mean
> configure would not pick them up when passed --enable-foo with
> the obvious dev libraries installed). These are:

Some tips on which packages to install:

> host big endian   no
> gprof enabled no
> sparse enabledno
> strip binariesno
> profiler  no
> static build  no

All the ones above are okay, fine to ignore.

> SDL support   no

Do you have libsdl1.2-dev installed?

> mingw32 support   no

Ok.

> vde support   no

You need libvde-dev and/or libvdeplug2-dev.

> RDMA support  no

Ok.

> spice support no (/)

libspice-protocol-dev and/or libspice-server-dev (can't remember).

> usb net redir no
> libiscsi support  no
> seccomp support   no
> GlusterFS support no

Ok.

> libssh2 support   no

libssh2-1-dev

Re: [Qemu-devel] [RFC] [PATCHv6 01/16] aio / timers: add qemu-timer.c utility functions

[Stefan Hajnoczi]

On Tue, Aug 06, 2013 at 03:18:13PM +0100, Alex Bligh wrote:
> Stefan,
> 
> --On 6 August 2013 15:59:11 +0200 Stefan Hajnoczi
>  wrote:
> 
> >>--On 6 August 2013 14:02:18 +0200 Stefan Hajnoczi
> >> wrote:
> >>My preference would be to move these to qemu_clock_deadline_ns (without
> >>the INT32_MAX check) and delete the old qemu_clock_deadline routine
> >>entirely, but I don't really understand the full set of circumstances
> >>in which the qtest routines are meant to work.
> >
> >Okay, that's excellent.  It would be great to move to a single function.
> >
> >The way qtest works is that it executes QEMU in a mode that does not run
> >guest code.  Instead of running guest code it listens for commands over
> >a socket.  The wire protocol can peek/poke memory, notify of interrupts,
> >and warp the clock.
> >
> >There are test cases that use qtest to test emulated devices.
> >
> >When qtest either steps the clock or sets it to a completely new value
> >using qtest_clock_warp() it runs all vm_clock timers that should expire
> >before the new time.
> >
> >Does this help?
> 
> Nearly :-)
> 
> How do I actually run the code (i.e. how do I test whether I've broken
> it)? I take it that's something different from just 'make check'?

make check includes qtest test cases like rtc-test, i440fx-test,
fdc-test, and others.  As long as they continue to pass all is good.

Stefan

Re: [Qemu-devel] [RFC] [PATCHv6 05/16] aio / timers: Split QEMUClock into QEMUClock and QEMUTimerList

[Stefan Hajnoczi]

On Tue, Aug 06, 2013 at 03:52:37PM +0100, Alex Bligh wrote:
> Stefan,
> 
> >>I think I disagree here.
> >>
> >>At the very least we should put the conversion to use the new API
> >>into a separate patch (possibly a separate patch set). It's fantastically
> >>intrusive.
> >
> >Yes, it should be a separate patch.
> ...
> >>Even if the period is just a month (i.e. the old API goes before 1.7),
> >>why break things unnecessarily?
> >
> >Nothing upstream breaks.  Only out-of-tree code breaks but that's life.
> >
> >What's important is that upstream will be clean and easy to understand
> >or debug.  Given how undocumented the QEMU codebase is, leaving legacy
> >API layers around just does more to confuse new contributors.
> >
> >That's why I'd really like to transition now instead of leaving things
> >in a more complex state than before.
> 
> OK. I'm just concerned about the potential fall out. If that's
> what everyone wants, I will do a monster patch to fix this up. Need
> that be part of this series? I can't help thinking it would be
> better to have the series applied first.
> 
> >We end up with:
> >
> >AioContext->tlg and default_timerlistgroup.
> >
> >Regarding naming, I think default_timerlistgroup should be called
> >main_loop_timerlistgroup instead.  The meaning of "default" is not
> >obvious.
> 
> I agree. I will change this.
> 
> >Now let's think about how callers will create QEMUTimers:
> >
> >1. AioContext
> >
> >   timer_new(ctx->tlg[QEMU_CLOCK_RT], SCALE_NS, cb, opaque);
> >
> >   Or with a wrapper:
> >
> >   QEMUTimer *aio_new_timer(AioContext *ctx, QEMUClockType type, int
> >scale, QEMUTimerCB *cb, void *opaque)
> >   {
> >   return timer_new(ctx->tlg[type], scale, cb, opaque);
> >   }
> >
> >   aio_new_timer(ctx, QEMU_CLOCK_RT, SCALE_NS, cb, opaque);
> 
> I was actually thinking about adding that wrapper anyway.
> 
> Do you think we need to wrap timer_mod, timer_del, timer_free
> etc. to make aio_timer_mod and so forth, even though they are
> a straight pass through?

Those wrappers are not necessary.  Once the caller has their QEMUTimer
pointer they should use the QEMUTimer APIs directly.

> >2. main-loop
> >
> >   /* without legacy qemu_timer_new() */
> >   timer_new(main_loop_tlg[QEMU_CLOCK_RT], SCALE_NS, cb, opaque);
> >
> >   Or with a wrapper:
> >
> >   QEMUTimer *qemu_new_timer(QEMUClockType type, int scale,
> > QEMUTimerCB *cb, void *opaque)
> >   {
> >   return timer_new(main_loop_tlg[type], scale, cb, opaque);
> >   }
> >
> >   qemu_new_timer(QEMU_CLOCK_RT, SCALE_NS, cb, opaque);
> >
> >Is this what you have in mind too?
> 
> Yes.
> 
> Certainly qemu_timer_new (as opposed to qemu_new_timer)
> would be a good addition.

Okay, great.  This makes the conversion from legacy QEMUClock functions
pretty straightforward.  It can be done mechanically in a single big
patch that converts the tree.

> >But this doesn't allow for the array indexing that you do in
> >TimerListGroup later.  I didn't know that at this point in the patch
> >series.
> 
> Yep. I'll leave that if that's OK.

Yes, I'm convinced now that the it's worth having.

>  struct QEMUTimer {
>  int64_t expire_time; /* in nanoseconds */
>  -QEMUClock *clock;
>  +QEMUTimerList *tl;
> >>>
> >>> 'timer_list' is easier to read than just 'tl'.
> >>
> >>It caused a pile of line wrap issues which made the patch harder
> >>to read, so I shortened it. I can put it back if you like.
> >
> >Are you sure it's the QEMUTimer->tl field that causes line wraps?
> >
> >I took a quick look and it seemed like only the QEMUTimerList *tl
> >function argument to the deadline functions could cause line wrap.  The
> >argument variable is unrelated and can stay short since it has a very
> >narrow context - the reader can be expected to remember the tl argument
> >while reading the code for the function.
> 
> >From memory it was lots of ->tl expanding within the code the issue
> rather than the arguments to functions. I'll try again. To be honest
> it's probably easier to change tl to timer_list throughout.

If you convert both that's good too.

> >
>  +bool qemu_clock_use_for_deadline(QEMUClock *clock)
>  +{
>  +return !(use_icount && (clock->type = QEMU_CLOCK_VIRTUAL));
>  +}
> >>>
> >>> Please use doc comments (see include/object/qom.h for example doc
> >>> comment syntax).  No idea why this function is needed or what it will
> >>> be used for.
> >>
> >>I will comment it, but it mostly does what it says in the tin. Per
> >>Paolo's comment, the vm_clock should not be used for calculation of
> >>deadlines to ppoll etc. if use_icount is true, because it's not actually
> >>in nanoseconds; rather qemu_notify() or aio_notify() get called by the
> >>vm cpu thread when the relevant instruction counter is exceeded.
> >
> >I didn't know that but the explanation makes sense.  Definitely
> >something that could be in a comment.  Perhaps its best to introduce

Re: [Qemu-devel] [PATCH 5/6] xics: split to xics and xics-common

[Alexey Kardashevskiy]

On 08/07/2013 05:03 PM, Andreas Färber wrote:
> Am 07.08.2013 08:06, schrieb Alexey Kardashevskiy:
>> On 08/06/2013 07:53 PM, Andreas Färber wrote:
>>> Am 06.08.2013 10:27, schrieb Alexey Kardashevskiy:
 +}
 +}
 +
 +void xics_prop_set_nr_irqs(Object *obj, struct Visitor *v,
 +   void *opaque, const char *name, struct Error 
 **errp)
>>>
>>> You should be able to drop both "struct"s.
>>
>> Yeah, fixed. This is just how the ObjectPropertyAccessor type is defined.
> 
> Yeah, reason is some circular header dependencies. ;)
> 
 +static void xics_common_initfn(Object *obj)
 +{
 +object_property_add(obj, "nr_irqs", "int",
 +NULL, xics_prop_set_nr_irqs, NULL, NULL, NULL);
 +object_property_add(obj, "nr_servers", "int",
 +NULL, xics_prop_set_nr_servers, NULL, NULL, NULL);
>>>
>>> Since this property is visible in the QOM tree, it would be nice to
>>> implement trivial getters returns the value from the state fields.
>>
>> Added. How do I test it?
> 
> ./QMP/qom-list to find the path, if not fixed in code yet, and

Something is wrong. XICS does not have an id but it should not be a
problem, right (btw how to set it from the code?)?

[aik@dyn232 ~]$ ./qemu-impreza/QMP/qom-list -s ./qmp-sock
/
[aik@dyn232 ~]$

This is qtree:

(qemu) info qtree
bus: main-system-bus
  type System
  dev: spapr-pci-host-bridge, id ""
index = 0
buid = 0x8002000
liobn = 0x8000
mem_win_addr = 0x100a000
mem_win_size = 0x2000
io_win_addr = 0x1008000
io_win_size = 0x1
msi_win_addr = 0x1009000
irq 0
bus: pci
  type PCI
  dev: spapr-vio-bridge, id ""
irq 0
bus: spapr-vio
  type spapr-vio-bus
  dev: spapr-vty, id "ser1"
reg = 1895825409
chardev = char1
irq = 4102
  dev: spapr-nvram, id "nvram@7100"
reg = 1895825408
drive = 
irq = 4097
  dev: xics-kvm, id ""
irq 0


> ./QMP/qom-get path.nr_servers to retrieve the value,
> ./QMP/qom-set path.nr_servers to verify it doesn't kill the process.
> 
> -qmp unix:./qmp-sock,server,nowait is what I use on the QEMU side.




>> "info qtree" prints only
>> DEVICE_CLASS(class)->props which is null in this case.
> 
> True, info qtree is from qdev times and deprecated. I recently attached
> a "qom-tree" script doing the equivalent that you could try.
> 
> I'll try to address -device xics,? showing them for 1.7. (Anthony
> indicated on a KVM call that the expected way to discover properties was
> to instantiate the type rather than looking at its class. Requires that
> types are instantiatable without asserting KVM accelerator, which gets
> initialized later.)
> 
 @@ -651,47 +765,32 @@ static void rtas_int_on(PowerPCCPU *cpu, 
 sPAPREnvironment *spapr,
  /*
   * XICS
   */
 +void xics_set_nr_irqs(XICSState *icp, uint32_t nr_irqs)
 +{
 +icp->ics = ICS(object_new(TYPE_ICS));
 +object_property_add_child(OBJECT(icp), "ics", OBJECT(icp->ics), NULL);
>>>
>>> Why create this single object in the setter? Can't you just create this
>>> in the common type's instance_init similar to before but using
>>> object_initialize() instead of object_new() and
>>> object_property_set_bool() in the realizefn?
>>
>> I cannot create in the common code as there are TYPE_ICS and TYPE_ICS_KVM
>> (oops, KVM is at the end, will fix it) types.
>>
>> And I would really want not to create it in instance_init() as I want to
>> have the object created and all its properties initialized in one place.
> 
> Doing it in instance_init facilitates improving the setter to allow
> multiple uses as a follow-up patch, since it must only be created once.
> If you don't, then the child will not be present in the composition tree
> before setting this seemingly unrelated property.

It will still be happening to ICP's and cannot be avoided. Why to make it
different for ICS and ICP? No IRQs number effectively means "no IRQ source".


> That seems worse to me
> than accessing a field in two places - instance_init will be run before
> any property setter.


> BTW these dynamic setters do not check automatically whether the object
> has already been realized. If you want the setter to return an Error in
> that case, you will need to check for DeviceState *dev = DEVICE(icp);
> dev->realized yourself. Not sure if that's the semantics you desire, but
> I guess so?


realize() will fail if a property is not set. Setters will fail if a
property is already set. By fail I mean that Error** thingy, not abort().
Do not really see why would we need something more here.



-- 
Alexey

Re: [Qemu-devel] [RFC] [PATCHv6 08/16] aio / timers: Add QEMUTimerListGroup to AioContext

[Stefan Hajnoczi]

On Tue, Aug 06, 2013 at 03:58:40PM +0100, Alex Bligh wrote:
> --On 6 August 2013 16:45:12 +0200 Stefan Hajnoczi
>  wrote:
> 
> >>Because otherwise make check SEGVs after the patch.
> >
> >It wasn't clear from the patch why there would be a crash.  I looked
> >deeper and timerlistgroup_init() calls qemu_get_clock() indirectly, so
> >we need to make sure that qemu_clocks[] is initialized to avoid a NULL
> >pointer dereference.
> 
> Actually now I recall v4 had:
> 
> @@ -215,6 +216,12 @@ AioContext *aio_context_new(void)
> aio_set_event_notifier(ctx, &ctx->notifier,
>(EventNotifierHandler *)
>event_notifier_test_and_clear, NULL);
> +/* Assert if we don't have rt_clock yet. If you see this assertion
> + * it means you are using AioContext without having first called
> + * init_clocks() in main().
> + */
> +assert(rt_clock);
> +ctx->tl = qemu_new_timerlist(rt_clock);
> 
> The equivalent in v7 would be an assert in timerlist_new_from_clock
> to check 'clock' is non-NULL. I shall put that in as the reason for
> this SEGV is non-obvious.

Nice, the comment makes the SEGV clear.

Re: [Qemu-devel] [PATCH 6/6] xics-kvm: Support for in-kernel XICS interrupt controller

[Alexey Kardashevskiy]

On 08/07/2013 05:08 PM, Andreas Färber wrote:
 +icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
 +}
 +
 +void xics_kvm_set_nr_servers(XICSState *icp, uint32_t nr_servers)
 +{
 +int i;
 +
 +icp->nr_servers = nr_servers;
 +
 +icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
 +for (i = 0; i < icp->nr_servers; i++) {
 +char buffer[32];
 +object_initialize(&icp->ss[i], TYPE_ICP_KVM);
 +snprintf(buffer, sizeof(buffer), "icp[%d]", i);
 +object_property_add_child(OBJECT(icp), buffer, 
 OBJECT(&icp->ss[i]), NULL);
 +}
 +}
> [...]
 +static void xics_kvm_realize(DeviceState *dev, Error **errp)
 +{
 +KVMXICSState *icpkvm = KVM_XICS(dev);
 +XICSState *icp = XICS_COMMON(dev);
 +int i, rc;
 +Error *error = NULL;
 +struct kvm_create_device xics_create_device = {
 +.type = KVM_DEV_TYPE_XICS,
 +.flags = 0,
 +};
 +
 +assert(kvm_enabled());
 +assert(kvm_check_extension(kvm_state, KVM_CAP_IRQ_XICS));
>>>
>>> error_setg() - if device can be created without accel=kvm (which it
>>> looks as if it can) then you should just error out the nice way here.
>>
>>
>> I check kvm_enabled() (I thought I check both but was wrong, will fix it)
>> where I try to create XICS_KVM so we should not be here if that check failed.
> 
> -device, -object and patch Anthony's patch QMP are other ways to
> instantiate the same type. :)

Hmm. Nice. I thought "-device xics,nr_irqs=1,nr_servers=2" won't work but
it does (well, fails, but lot further, in realize()) :)



-- 
Alexey

Re: [Qemu-devel] [PATCH v6 01/18] block: ensure bdrv_drain_all() works during bdrv_delete()

[Stefan Hajnoczi]

On Wed, Aug 07, 2013 at 10:42:26AM +0800, Wenchao Xia wrote:
> >On Fri, Jul 26, 2013 at 02:43:44PM +0800, Wenchao Xia wrote:
> >>Reviewed-by: Wenchao Xia 
> >>
> >>One question: old code missed itself in bdrv_drain_all(), is that a bug?
> >
> >Sorry, I don't understand the question.  Can you rephrase it?
> >
>   Before this patch, in the code path: bdrv_close()->bdrv_drain_all(),
> the *bs does not exist in bdrv_states, so the code missed the chance to
> drain the request on *bs. That is a bug, and this patch is actually a
> bugfix?

Yes, exactly.  It's a bug fix and therefore I CCed
qemu-sta...@nongnu.org.

Stefan

Re: [Qemu-devel] [PATCH 0/3] rdma: validate remote provided RDMAControlHeader::len

[Paolo Bonzini]

> RDMAControlHeader::len is remote-provided. So validate the value before use.
> 
> Isaku Yamahata (3):
>   rdma: use resp.len after validation in qemu_rdma_registration_stop
>   rdma: validate RDMAControlHeader::len
>   rdma: check if RDMAControlHeader::len match transferred byte
> 
>  migration-rdma.c |   44 ++--
>  1 file changed, 30 insertions(+), 14 deletions(-)

Looks good, thanks!

We will have a mix of assertions and errors after this patch.
Asserting on the destination is probably okay, but as a follow-up
perhaps we can change the assertion(s) to errors.

Paolo

Re: [Qemu-devel] [PATCH v6 01/18] block: ensure bdrv_drain_all() works during bdrv_delete()

[Stefan Hajnoczi]

On Thu, Jul 25, 2013 at 05:18:08PM +0200, Stefan Hajnoczi wrote:
> In bdrv_delete() make sure to call bdrv_make_anon() *after* bdrv_close()
> so that the device is still seen by bdrv_drain_all() when iterating
> bdrv_states.
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Stefan Hajnoczi 
> ---
>  block.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 6cd39fa..9d68270 100644
> --- a/block.c
> +++ b/block.c
> @@ -1600,11 +1600,11 @@ void bdrv_delete(BlockDriverState *bs)
>  assert(!bs->job);
>  assert(!bs->in_use);
>  
> +bdrv_close(bs);
> +
>  /* remove from list, if necessary */
>  bdrv_make_anon(bs);
>  
> -bdrv_close(bs);
> -
>  g_free(bs);
>  }

Kevin: Although I haven't seen reports of this bug, it might be a good
idea to merge this single patch for QEMU 1.6.

Stefan

Re: [Qemu-devel] [PATCH for-1.6] Bugfix for loading multiboot kernels

[Paolo Bonzini]

- Original Message -
> From: "Martijn van den Broek" 
> To: qemu-devel@nongnu.org
> Cc: chout...@adacore.com, pbonz...@redhat.com, ag...@suse.de
> Sent: Tuesday, August 6, 2013 8:45:39 PM
> Subject: [PATCH] Bugfix for loading multiboot kernels
> 
> This patch fixes a bug in rom_copy introduced by
> commit d60fa42e8bae39440f997ebfe8fe328269a57d16.
> 
> rom_copy failed to load roms with a "datasize" of 0.
> As a result, multiboot kernels were not loaded correctly
> when they contain a segment with a "file size" of 0.
> 
> https://bugs.launchpad.net/qemu/+bug/1208944
> 
> Signed-off-by: Martijn van den Broek 
> ---
>  hw/core/loader.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index c3c28cf..6875b7e 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -814,9 +814,6 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
>  if (rom->addr > end) {
>  break;
>  }
> -if (!rom->data) {
> -continue;
> -}
> 
>  d = dest + (rom->addr - addr);
>  s = rom->data;
> @@ -826,7 +823,9 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
>  l = dest - d;
>  }
> 
> -memcpy(d, s, l);
> +if (l > 0) {
> +memcpy(d, s, l);
> +}
> 
>  if (rom->romsize > rom->datasize) {
>  /* If datasize is less than romsize, it means that we didn't
> --
> 1.8.1.msysgit.1
> 

Reviewed-by: Paolo Bonzini 

and marking the patch for 1.5 and 1.6:

Cc: qemu-sta...@nongnu.org

Paolo

Re: [Qemu-devel] [PATCH for-1.6] qemu-timer: fix get_clock() gettimeofday() fallback #ifdef

[Stefan Hajnoczi]

On Tue, Aug 06, 2013 at 08:26:16PM -0400, Brad Smith wrote:
> On 06/08/13 7:48 AM, Stefan Hajnoczi wrote:
> >If CLOCK_MONOTONIC is not defined by system headers we use
> >gettimeofday(2).  Apparently this is not used very often since no one
> >noticed the #ifdef was actually broken and left the function definition
> >unterminated.
> 
> Can you show what this supposedly fixes? This code built just fine on
> OpenBSD before d05ef160453e98546a4197496dc8a3cb2defac53.

Hi Brad, thanks for pointing this out.  I misread the code:

  static inline int64_t get_clock(void)
  {
  #ifdef CLOCK_MONOTONIC
  if (use_rt_clock) {
  struct timespec ts;
  clock_gettime(CLOCK_MONOTONIC, &ts);
  return ts.tv_sec * 10LL + ts.tv_nsec;
  } else
  #endif  <--- thought this was #else
  {
  /* XXX: using gettimeofday leads to problems if the date
 changes, so it should be avoided. */
  return get_clock_realtime();
  }
  }
  #endif  <--- this is actually for #ifdef _WIN32

So I thought the function definition would not be terminated properly.  But
after you mentioned it, my change makes no sense.

Dropping this patch.

Thanks,
Stefan

Re: [Qemu-devel] [PATCH] Moving alarm_timer assignment before atexit()

[Stefan Hajnoczi]

On Wed, Aug 07, 2013 at 08:39:19AM +0200, Laszlo Ersek wrote:
> On 08/07/13 01:29, Amos Kong wrote:
> > We register exit clean function by atexit(),
> > but alarm_timer is NULL here. If exit is caused
> > between atexit() and alarm_timer assignment,
> > real timer can't be cleaned.
> 
> That's correct in general, but I don't see how it could happen in the
> code being patched. pthread_atfork() won't call exit().

Agreed.  I can remember thinking about this when reading the code and
deciding not to bother changing it.

Since the patch is on the list though, we might as well apply it.

The only thing I suggest changing is to note that this is currently not
a bug, just a clean-up.

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH v2] block: Produce zeros when protocols reading beyond end of file

[Stefan Hajnoczi]

On Tue, Aug 06, 2013 at 10:38:32AM +0800, Asias He wrote:
> On Tue, Aug 06, 2013 at 10:02:22AM +0800, Fam Zheng wrote:
> > On Tue, 08/06 09:53, Asias He wrote:
> > > From: MORITA Kazutaka 
> > > 
> > > While Asias is debugging an issue creating qcow2 images on top of
> > > non-file protocols.  It boils down to this example using NBD:
> > > 
> > > $ qemu-io -c 'open -g nbd+unix:///?socket=/tmp/nbd.sock' -c 'read -v 0 
> > > 512'
> > > 
> > > Notice the open -g option to set bs->growable.  This means you can
> > > read/write beyond end of file.  Reading beyond end of file is supposed
> > > to produce zeroes.
> > > 
> > > We rely on this behavior in qcow2_create2() during qcow2 image
> > > creation.  We create a new file and then write the qcow2 header
> > > structure using bdrv_pwrite().  Since QCowHeader is not a multiple of
> > > sector size, block.c first uses bdrv_read() on the empty file to fetch
> > > the first sector (should be all zeroes).
> > > 
> > > Here is the output from the qemu-io NBD example above:
> > > 
> > > $ qemu-io -c 'open -g nbd+unix:///?socket=/tmp/nbd.sock' -c 'read -v 0 
> > > 512'
> > > :  ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab  
> > > 
> > > 0010:  ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab  
> > > 
> > > 0020:  ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab  
> > > 
> > > ...
> > > 
> > > We are not zeroing the buffer!  As a result qcow2 image creation on top
> > > of protocols is not guaranteed to work even when file creation is
> > > supported by the protocol.
> > 
> > It seems to me that "read beyond EOF" is more protocol defined than to
> > be forced in block layer. Is this behavior common to all protocols, is
> > it reasonable if some protocol wants other values than zero?
> 
> The reason to do this in block layer is that we do not want to duplicate
> the memset in all protocols.
> 
> Do we actually have protocols that really want values other than zero?

I think we rely on zeroes when bs->growable is true, because
bdrv_pwrite() handles sub-sector I/O using read-modify-write.  So it
reads beyond EOF first (expects to get zeroes), then copies the
sub-sector data from the caller, then writes it back.  If we don't zero
beyond-EOF data then we would write unexpected values to the image.

Stefan

[Qemu-devel] [PATCH] powerpc iommu: enable multiple TCE requests

[Alexey Kardashevskiy]

Currently only single TCE entry per requiest is supported (H_PUT_TCE).
However PAPR+ specification allows multiple entry requests such as
H_PUT_TCE_INDIRECT and H_STUFFF_TCE. Having less transitions to the host
kernel via ioctls, support of these calls can accelerate IOMMU operations.

This also removes some leftovers in debug output of the H_PUT_TCE handler.

Signed-off-by: Alexey Kardashevskiy 
Signed-off-by: David Gibson 
---

This patch requires a KVM_CAP_SPAPR_MULTITCE capability from kernel headers
which is not there yet.
However it still would be nice to have "Reviewed-by" from someone when
the capability will make it to the upstream. Thanks.

---
 hw/ppc/spapr.c   | 16 ++--
 hw/ppc/spapr_iommu.c | 74 +---
 target-ppc/kvm.c |  7 +
 target-ppc/kvm_ppc.h |  7 +
 4 files changed, 99 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9494915..a6b1f54 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -301,6 +301,8 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
 CPUState *cs;
 uint32_t start_prop = cpu_to_be32(initrd_base);
 uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
+char hypertas_propm[] = 
"hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
+"\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk\0hcall-multi-tce";
 char hypertas_prop[] = "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
 "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk";
 char qemu_hypertas_prop[] = "hcall-memop1";
@@ -480,8 +482,18 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
 /* RTAS */
 _FDT((fdt_begin_node(fdt, "rtas")));
 
-_FDT((fdt_property(fdt, "ibm,hypertas-functions", hypertas_prop,
-   sizeof(hypertas_prop;
+/* In TCG mode, the multitce functions, which we implement are a
+ * win.  With KVM, we could fall back to the qemu implementation
+ * when KVM doesn't support them, but that would be much slower
+ * than just using the KVM implementations of the single TCE
+ * hypercalls. */
+if (kvmppc_spapr_use_multitce()) {
+_FDT((fdt_property(fdt, "ibm,hypertas-functions", hypertas_propm,
+   sizeof(hypertas_propm;
+} else {
+_FDT((fdt_property(fdt, "ibm,hypertas-functions", hypertas_prop,
+   sizeof(hypertas_prop;
+}
 _FDT((fdt_property(fdt, "qemu,hypertas-functions", qemu_hypertas_prop,
sizeof(qemu_hypertas_prop;
 
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 3d4a1fc..22b09be 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -244,6 +244,71 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, 
target_ulong ioba,
 return H_SUCCESS;
 }
 
+static target_ulong h_put_tce_indirect(PowerPCCPU *cpu,
+   sPAPREnvironment *spapr,
+   target_ulong opcode, target_ulong *args)
+{
+int i;
+target_ulong liobn = args[0];
+target_ulong ioba = args[1];
+target_ulong tce_list = args[2];
+target_ulong npages = args[3];
+target_ulong ret = 0;
+sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
+
+if (tcet) {
+for (i = 0; i < npages; ++i, ioba += SPAPR_TCE_PAGE_SIZE) {
+target_ulong tce = ldq_phys((tce_list & ~SPAPR_TCE_PAGE_MASK) +
+i * sizeof(target_ulong));
+ret = put_tce_emu(tcet, ioba, tce);
+if (ret) {
+break;
+}
+}
+return ret;
+}
+#ifdef DEBUG_TCE
+fprintf(stderr, "%s on liobn=" TARGET_FMT_lx
+"  ioba 0x" TARGET_FMT_lx "  TCE 0x" TARGET_FMT_lx
+" ret = " TARGET_FMT_ld "\n",
+__func__, liobn, ioba, tce_list, ret);
+#endif
+
+return H_PARAMETER;
+}
+
+static target_ulong h_stuff_tce(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+  target_ulong opcode, target_ulong *args)
+{
+int i;
+target_ulong liobn = args[0];
+target_ulong ioba = args[1];
+target_ulong tce_value = args[2];
+target_ulong npages = args[3];
+target_ulong ret = 0;
+sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
+
+ioba &= ~(SPAPR_TCE_PAGE_SIZE - 1);
+
+if (tcet) {
+for (i = 0; i < npages; ++i, ioba += SPAPR_TCE_PAGE_SIZE) {
+ret = put_tce_emu(tcet, ioba, tce_value);
+if (ret) {
+break;
+}
+}
+return ret;
+}
+#ifdef DEBUG_TCE
+fprintf(stderr, "%s on liobn=" TARGET_FMT_lx
+"  ioba 0x" TARGET_FMT_lx "  TCE 0x" TARGET_FMT_lx
+" ret = " TARGET_FMT_ld "\n",
+__func__, liobn, ioba, tce_value, ret);
+#endif
+
+return H_PARAMETER;
+}
+
 static target_ulong h_put_tce(PowerPCCPU *cpu, sPAPREnvironment *spapr,
   

Re: [Qemu-devel] [PATCH] Moving alarm_timer assignment before atexit()

[Amos Kong]

On Wed, Aug 07, 2013 at 09:57:19AM +0200, Stefan Hajnoczi wrote:
> On Wed, Aug 07, 2013 at 08:39:19AM +0200, Laszlo Ersek wrote:
> > On 08/07/13 01:29, Amos Kong wrote:
> > > We register exit clean function by atexit(),
> > > but alarm_timer is NULL here. If exit is caused
> > > between atexit() and alarm_timer assignment,
> > > real timer can't be cleaned.
> > 
> > That's correct in general, but I don't see how it could happen in the
> > code being patched. pthread_atfork() won't call exit().

I try to sleep 10 seconds after atexit(), but no crash occurred when I
killed qemu process.

atexit(quit_timer);
sleep(10);// kill qemu by 'pkill qemu', no crash
pthread_atfork();
alarm_timer = t;

> Agreed.  I can remember thinking about this when reading the code and
> deciding not to bother changing it.
> 
> Since the patch is on the list though, we might as well apply it.
> 
> The only thing I suggest changing is to note that this is currently not
> a bug, just a clean-up.

It seems just a cleanup.
 
> Reviewed-by: Stefan Hajnoczi 


BTW, can we add a check in quit_timers() to avoid one kind of crash
(try to quit the uninited timers, or quit timer repeatedly) ?


diff --git a/qemu-timer.c b/qemu-timer.c
index b2d95e2..023e4ae 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -728,8 +728,10 @@ static void win32_rearm_timer(struct
qemu_alarm_timer *t,
 static void quit_timers(void)
 {
 struct qemu_alarm_timer *t = alarm_timer;
-alarm_timer = NULL;
-t->stop(t);
+if (t) {
+alarm_timer = NULL;
+t->stop(t);
+}
 }
 
 #ifdef CONFIG_POSIX

-- 
Amos.

Re: [Qemu-devel] [SeaBIOS] [PATCH] don't expose pvpanic device in the UI

[Stefan Hajnoczi]

On Tue, Aug 06, 2013 at 02:45:48PM +0200, Andreas Färber wrote:
> Am 06.08.2013 14:09, schrieb Michael S. Tsirkin:
> > On Tue, Aug 06, 2013 at 01:03:34PM +0200, Andreas Färber wrote:
> >> FWIW -M q35 does not create all Q35 devices, there's -readconfig
> >> docs/q35-chipset.cfg for the rest. The criteria certainly is not
> >> migratability, since ICH9 AHCI (part of -M q35) is unmigratable,
> >> unfortunately.
> > 
> > Wasn't this fixed recently?
> 
> Not in qemu.git at least, that's why I couldn't test PCIe migration with
> q35 recently and resorted to picking random devices into i440fx.
> 
> Jason had added a VMStateDescription with fields extending my initial
> work, but it is .unmigratable = 1 /* Still buggy under I/O load */.

I haven't read the whole discussion but the recent AHCI fix I'm aware of
is making FLUSH commands work correctly.  That wasn't specifically aimed
at fixing migration.

Stefan

[Qemu-devel] [PATCH 2/8] memory: Sanity check that no listeners remain on a destroyed AddressSpace

[Alexey Kardashevskiy]

From: David Gibson 

At the moment, most AddressSpace objects last as long as the guest system
in practice, but that could well change in future.  In addition, for VFIO
we will be introducing some private per-AdressSpace information, which must
be disposed of before the AddressSpace itself is destroyed.

To reduce the chances of subtle bugs in this area, this patch adds
asssertions to ensure that when an AddressSpace is destroyed, there are no
remaining MemoryListeners using that AS as a filter.

Signed-off-by: David Gibson 
---
 memory.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/memory.c b/memory.c
index 886f838..ffedde7 100644
--- a/memory.c
+++ b/memory.c
@@ -1726,11 +1726,18 @@ void address_space_init(AddressSpace *as, MemoryRegion 
*root, const char *name)
 
 void address_space_destroy(AddressSpace *as)
 {
+MemoryListener *listener;
+
 /* Flush out anything from MemoryListeners listening in on this */
 memory_region_transaction_begin();
 as->root = NULL;
 memory_region_transaction_commit();
 QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
+
+QTAILQ_FOREACH(listener, &memory_listeners, link) {
+assert(listener->address_space_filter != as);
+}
+
 address_space_destroy_dispatch(as);
 flatview_unref(as->current_map);
 g_free(as->name);
-- 
1.8.3.2

[Qemu-devel] [PATCH 0/8 v3] vfio on power: preparations for VFIO, guest IOMMUs and VFIO itself

[Alexey Kardashevskiy]

This patch series represents a third attempt at better integration of
the vfio code with qemu's handling of guest IOMMUs. David posted two previous
series, I am posting now his rework after last posting.

This also contains a working VFIO driver for QEMU which depends on
MSIX rework and IRQFD patches (which are not in upstream yet) and
won't work as isbut I am pretty sure there is still enough to discuss :)

Thanks!


Alexey Kardashevskiy (4):
  vfio: Add guest side IOMMU support
  spapr vfio: add vfio_container_spapr_get_info()
  spapr vfio: add spapr-pci-vfio-host-bridge to support vfio
  spapr vfio: enable for spapr

David Gibson (4):
  pci: Introduce helper to retrieve a PCI device's DMA address space
  memory: Sanity check that no listeners remain on a destroyed
AddressSpace
  vfio: Introduce VFIO address spaces
  vfio: Create VFIOAddressSpace objects as needed

 hw/misc/vfio.c  | 300 +++-
 hw/pci/pci.c|   9 +-
 hw/ppc/spapr_iommu.c| 176 ++
 hw/ppc/spapr_pci.c  | 209 +++---
 include/hw/misc/vfio.h  |  11 ++
 include/hw/pci-host/spapr.h |  12 ++
 include/hw/pci/pci.h|   1 +
 include/hw/ppc/spapr.h  |  19 +++
 memory.c|   7 ++
 target-ppc/kvm.c|  33 +
 target-ppc/kvm_ppc.h|  12 ++
 trace-events|   4 +
 12 files changed, 724 insertions(+), 69 deletions(-)
 create mode 100644 include/hw/misc/vfio.h

-- 
1.8.3.2

[Qemu-devel] [PATCH 1/8] pci: Introduce helper to retrieve a PCI device's DMA address space

[Alexey Kardashevskiy]

From: David Gibson 

A PCI device's DMA address space (possibly an IOMMU) is returned by a
method on the PCIBus.  At the moment that only has one caller, so the
method is simply open coded.  We'll need another caller for VFIO, so
this patch introduces a helper/wrapper function.

Signed-off-by: David Gibson 
---
 hw/pci/pci.c | 9 -
 include/hw/pci/pci.h | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 4bce3e7..3cea25f 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -813,7 +813,7 @@ static PCIDevice *do_pci_register_device(PCIDevice 
*pci_dev, PCIBus *bus,
 
 pci_dev->bus = bus;
 if (bus->iommu_fn) {
-dma_as = bus->iommu_fn(bus, bus->iommu_opaque, devfn);
+dma_as = pci_iommu_as(pci_dev);
 } else {
 /* FIXME: inherit memory region from bus creator */
 dma_as = &address_space_memory;
@@ -2248,6 +2248,13 @@ static void pci_device_class_init(ObjectClass *klass, 
void *data)
 k->props = pci_props;
 }
 
+AddressSpace *pci_iommu_as(PCIDevice *dev)
+{
+PCIBus *bus = PCI_BUS(dev->bus);
+
+return bus->iommu_fn(bus, bus->iommu_opaque, dev->devfn);
+}
+
 void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque)
 {
 bus->iommu_fn = fn;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index b6ad9e4..614f809 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -407,6 +407,7 @@ void pci_device_deassert_intx(PCIDevice *dev);
 
 typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
 
+AddressSpace *pci_iommu_as(PCIDevice *dev);
 void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque);
 
 static inline void
-- 
1.8.3.2

[Qemu-devel] [PATCH 6/8] spapr vfio: add vfio_container_spapr_get_info()

[Alexey Kardashevskiy]

As sPAPR platform supports DMA windows on a PCI bus, the information
about their location and size should be passed into the guest via
the device tree.

The patch adds a helper to read this info from the container fd.

Signed-off-by: Alexey Kardashevskiy 
---
 hw/misc/vfio.c | 37 +
 include/hw/misc/vfio.h | 11 +++
 2 files changed, 48 insertions(+)
 create mode 100644 include/hw/misc/vfio.h

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 3855efe..4571f64 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -39,6 +39,7 @@
 #include "qemu/range.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
+#include "hw/misc/vfio.h"
 
 /* #define DEBUG_VFIO */
 #ifdef DEBUG_VFIO
@@ -3488,3 +3489,39 @@ static void register_vfio_pci_dev_type(void)
 }
 
 type_init(register_vfio_pci_dev_type)
+
+int vfio_container_spapr_get_info(AddressSpace *as, int32_t groupid,
+  struct vfio_iommu_spapr_tce_info *info,
+  int *group_fd)
+{
+VFIOAddressSpace *space;
+VFIOGroup *group;
+VFIOContainer *container;
+int ret, fd;
+
+space = vfio_get_address_space(as);
+if (!space) {
+return -1;
+}
+group = vfio_get_group(groupid, space);
+if (!group) {
+return -1;
+}
+container = group->container;
+if (!group->container) {
+return -1;
+}
+fd = container->fd;
+if (!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
+return -1;
+}
+ret = ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, info);
+if (ret) {
+error_report("vfio: failed to get iommu info for container: %s",
+ strerror(errno));
+return -1;
+}
+*group_fd = group->fd;
+
+return 0;
+}
diff --git a/include/hw/misc/vfio.h b/include/hw/misc/vfio.h
new file mode 100644
index 000..ac9a971
--- /dev/null
+++ b/include/hw/misc/vfio.h
@@ -0,0 +1,11 @@
+#ifndef VFIO_API_H
+#define VFIO_API_H
+
+#include "qemu/typedefs.h"
+#include 
+
+extern int vfio_container_spapr_get_info(AddressSpace *as, int32_t groupid,
+ struct vfio_iommu_spapr_tce_info 
*info,
+ int *group_fd);
+
+#endif
-- 
1.8.3.2

[Qemu-devel] [PATCH 4/8] vfio: Create VFIOAddressSpace objects as needed

[Alexey Kardashevskiy]

From: David Gibson 

So far, VFIO has a notion of different logical DMA address spaces, but
only ever uses one (system memory).  This patch extends this, creating
new VFIOAddressSpace objects as necessary, according to the AddressSpace
reported by the PCI subsystem for this device's DMAs.

This isn't enough yet to support guest side IOMMUs with VFIO, but it does
mean we could now support VFIO devices on, for example, a guest side PCI
host bridge which maps system memory at somewhere other than 0 in PCI
space.

Signed-off-by: David Gibson 
---
 hw/misc/vfio.c | 42 ++
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index bcd67a0..e1ee56e 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -133,9 +133,10 @@ enum {
 typedef struct VFIOAddressSpace {
 AddressSpace *as;
 QLIST_HEAD(, VFIOContainer) containers;
+QLIST_ENTRY(VFIOAddressSpace) list;
 } VFIOAddressSpace;
 
-static VFIOAddressSpace vfio_address_space_memory;
+QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces;
 
 struct VFIOGroup;
 
@@ -2609,10 +2610,33 @@ static int vfio_load_rom(VFIODevice *vdev)
 return 0;
 }
 
-static void vfio_address_space_init(VFIOAddressSpace *space, AddressSpace *as)
+static VFIOAddressSpace *vfio_get_address_space(AddressSpace *as)
 {
+VFIOAddressSpace *space;
+
+QLIST_FOREACH(space, &vfio_address_spaces, list) {
+if (space->as == as)
+return space;
+}
+
+/* No suitable VFIOAddressSpace, create a new one */
+space = g_malloc0(sizeof(*space));
 space->as = as;
 QLIST_INIT(&space->containers);
+
+QLIST_INSERT_HEAD(&vfio_address_spaces, space, list);
+
+return space;
+}
+
+static void vfio_put_address_space(VFIOAddressSpace *space)
+{
+if (!QLIST_EMPTY(&space->containers)) {
+return;
+}
+
+QLIST_REMOVE(space, list);
+g_free(space);
 }
 
 static int vfio_connect_container(VFIOGroup *group, VFIOAddressSpace *space)
@@ -2707,6 +2731,8 @@ static void vfio_disconnect_container(VFIOGroup *group)
 group->container = NULL;
 
 if (QLIST_EMPTY(&container->group_list)) {
+VFIOAddressSpace *space = container->space;
+
 if (container->iommu_data.release) {
 container->iommu_data.release(container);
 }
@@ -2714,6 +2740,8 @@ static void vfio_disconnect_container(VFIOGroup *group)
 DPRINTF("vfio_disconnect_container: close container->fd\n");
 close(container->fd);
 g_free(container);
+
+vfio_put_address_space(space);
 }
 }
 
@@ -3085,6 +3113,7 @@ static int vfio_initfn(PCIDevice *pdev)
 {
 VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
 VFIOGroup *group;
+VFIOAddressSpace *space;
 char path[PATH_MAX], iommu_group_path[PATH_MAX], *group_name;
 ssize_t len;
 struct stat st;
@@ -3120,14 +3149,12 @@ static int vfio_initfn(PCIDevice *pdev)
 DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__, vdev->host.domain,
 vdev->host.bus, vdev->host.slot, vdev->host.function, groupid);
 
-if (pci_iommu_as(pdev) != &address_space_memory) {
-error_report("vfio: DMA address space must be system memory");
-return -ENXIO;
-}
+space = vfio_get_address_space(pci_iommu_as(pdev));
 
-group = vfio_get_group(groupid, &vfio_address_space_memory);
+group = vfio_get_group(groupid, space);
 if (!group) {
 error_report("vfio: failed to get group %d", groupid);
+vfio_put_address_space(space);
 return -ENOENT;
 }
 
@@ -3348,7 +3375,6 @@ static const TypeInfo vfio_pci_dev_info = {
 
 static void register_vfio_pci_dev_type(void)
 {
-vfio_address_space_init(&vfio_address_space_memory, &address_space_memory);
 type_register_static(&vfio_pci_dev_info);
 }
 
-- 
1.8.3.2

[Qemu-devel] [PATCH 3/8] vfio: Introduce VFIO address spaces

[Alexey Kardashevskiy]

From: David Gibson 

The only model so far supported for VFIO passthrough devices is the model
usually used on x86, where all of the guest's RAM is mapped into the
(host) IOMMU and there is no IOMMU visible in the guest.

This patch begins to relax this model, introducing the notion of a
VFIOAddressSpace.  This represents a logical DMA address space which will
be visible to one or more VFIO devices by appropriate mapping in the (host)
IOMMU.  Thus the currently global list of containers becomes local to
a VFIOAddressSpace, and we verify that we don't attempt to add a VFIO
group to multiple address spaces.

For now, only one VFIOAddressSpace is created and used, corresponding to
main system memory, that will change in future patches.

Signed-off-by: David Gibson 
---
 hw/misc/vfio.c | 57 +
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index adcd23d..bcd67a0 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -130,9 +130,17 @@ enum {
 VFIO_INT_MSIX = 3,
 };
 
+typedef struct VFIOAddressSpace {
+AddressSpace *as;
+QLIST_HEAD(, VFIOContainer) containers;
+} VFIOAddressSpace;
+
+static VFIOAddressSpace vfio_address_space_memory;
+
 struct VFIOGroup;
 
 typedef struct VFIOContainer {
+VFIOAddressSpace *space;
 int fd; /* /dev/vfio/vfio, empowered by the attached groups */
 struct {
 /* enable abstraction to support various iommu backends */
@@ -197,9 +205,6 @@ typedef struct VFIOGroup {
 
 #define MSIX_CAP_LENGTH 12
 
-static QLIST_HEAD(, VFIOContainer)
-container_list = QLIST_HEAD_INITIALIZER(container_list);
-
 static QLIST_HEAD(, VFIOGroup)
 group_list = QLIST_HEAD_INITIALIZER(group_list);
 
@@ -2604,16 +2609,28 @@ static int vfio_load_rom(VFIODevice *vdev)
 return 0;
 }
 
-static int vfio_connect_container(VFIOGroup *group)
+static void vfio_address_space_init(VFIOAddressSpace *space, AddressSpace *as)
+{
+space->as = as;
+QLIST_INIT(&space->containers);
+}
+
+static int vfio_connect_container(VFIOGroup *group, VFIOAddressSpace *space)
 {
 VFIOContainer *container;
 int ret, fd;
 
 if (group->container) {
-return 0;
+if (group->container->space == space) {
+return 0;
+} else {
+error_report("vfio: group %d used in multiple address spaces",
+ group->groupid);
+return -EBUSY;
+}
 }
 
-QLIST_FOREACH(container, &container_list, next) {
+QLIST_FOREACH(container, &space->containers, next) {
 if (!ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container->fd)) {
 group->container = container;
 QLIST_INSERT_HEAD(&container->group_list, group, container_next);
@@ -2636,6 +2653,7 @@ static int vfio_connect_container(VFIOGroup *group)
 }
 
 container = g_malloc0(sizeof(*container));
+container->space = space;
 container->fd = fd;
 
 if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU)) {
@@ -2658,7 +2676,8 @@ static int vfio_connect_container(VFIOGroup *group)
 container->iommu_data.listener = vfio_memory_listener;
 container->iommu_data.release = vfio_listener_release;
 
-memory_listener_register(&container->iommu_data.listener, 
&address_space_memory);
+memory_listener_register(&container->iommu_data.listener,
+ container->space->as);
 } else {
 error_report("vfio: No available IOMMU models");
 g_free(container);
@@ -2667,7 +2686,7 @@ static int vfio_connect_container(VFIOGroup *group)
 }
 
 QLIST_INIT(&container->group_list);
-QLIST_INSERT_HEAD(&container_list, container, next);
+QLIST_INSERT_HEAD(&space->containers, container, next);
 
 group->container = container;
 QLIST_INSERT_HEAD(&container->group_list, group, container_next);
@@ -2698,7 +2717,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
 }
 }
 
-static VFIOGroup *vfio_get_group(int groupid)
+static VFIOGroup *vfio_get_group(int groupid, VFIOAddressSpace *space)
 {
 VFIOGroup *group;
 char path[32];
@@ -2706,7 +2725,15 @@ static VFIOGroup *vfio_get_group(int groupid)
 
 QLIST_FOREACH(group, &group_list, next) {
 if (group->groupid == groupid) {
-return group;
+/* Found it.  Now is it already in the right context? */
+assert(group->container);
+if (group->container->space == space) {
+return group;
+} else {
+error_report("vfio: group %d used in multiple address spaces",
+ group->groupid);
+return NULL;
+}
 }
 }
 
@@ -2739,7 +2766,7 @@ static VFIOGroup *vfio_get_group(int groupid)
 group->groupid = groupid;
 QLIST_INIT(&group->device_list);
 
-if (vfio_connect_container(group)) {
+if (vfio_connect_container(group, space)) {
 

[Qemu-devel] [PATCH 5/8] vfio: Add guest side IOMMU support

[Alexey Kardashevskiy]

This patch uses the new IOMMU notifiers to allow VFIO pass through devices
to work with guest side IOMMUs, as long as the host-side VFIO iommu has
sufficient capability and granularity to match the guest side. This works
by tracking all map and unmap operations on the guest IOMMU using the
notifiers, and mirroring them into VFIO.

There are a number of FIXMEs, and the scheme involves rather more notifier
structures than I'd like, but it shuold make for a reasonable proof of
concept.

Signed-off-by: David Gibson 
---
 hw/misc/vfio.c | 149 +
 1 file changed, 129 insertions(+), 20 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index e1ee56e..3855efe 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -150,10 +150,18 @@ typedef struct VFIOContainer {
 };
 void (*release)(struct VFIOContainer *);
 } iommu_data;
+QLIST_HEAD(, VFIOGuestIOMMU) guest_iommus;
 QLIST_HEAD(, VFIOGroup) group_list;
 QLIST_ENTRY(VFIOContainer) next;
 } VFIOContainer;
 
+typedef struct VFIOGuestIOMMU {
+VFIOContainer *container;
+MemoryRegion *iommu;
+Notifier n;
+QLIST_ENTRY(VFIOGuestIOMMU) list;
+} VFIOGuestIOMMU;
+
 /* Cache of MSI-X setup plus extra mmap and memory region for split BAR map */
 typedef struct VFIOMSIXInfo {
 uint8_t table_bar;
@@ -1917,19 +1925,70 @@ static int vfio_dma_map(VFIOContainer *container, 
hwaddr iova,
 
 static bool vfio_listener_skipped_section(MemoryRegionSection *section)
 {
-return !memory_region_is_ram(section->mr);
+return !memory_region_is_ram(section->mr) &&
+!memory_region_is_iommu(section->mr);
 }
 
-static void vfio_listener_region_add(MemoryListener *listener,
- MemoryRegionSection *section)
+static void vfio_iommu_map_notify(Notifier *n, void *data)
 {
-VFIOContainer *container = container_of(listener, VFIOContainer,
-iommu_data.listener);
-hwaddr iova, end;
+VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
+VFIOContainer *container = giommu->container;
+IOMMUTLBEntry *iotlb = data;
+MemoryRegion *mr;
+hwaddr xlat;
+hwaddr len = iotlb->addr_mask + 1;
 void *vaddr;
 int ret;
 
-assert(!memory_region_is_iommu(section->mr));
+DPRINTF("iommu map @ %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
+iotlb->iova, iotlb->iova + iotlb->address_mask);
+
+/* The IOMMU TLB entry we have just covers translation through
+ * this IOMMU to its immediate target.  We need to translate
+ * it the rest of the way through to memory. */
+mr = address_space_translate(&address_space_memory,
+ iotlb->translated_addr,
+ &xlat, &len, iotlb->perm & IOMMU_WO);
+if (!memory_region_is_ram(mr)) {
+DPRINTF("iommu map to non memory area %"HWADDR_PRIx"\n",
+xlat);
+return;
+}
+if (len & iotlb->addr_mask) {
+DPRINTF("iommu has granularity incompatible with target AS\n");
+return;
+}
+
+vaddr = memory_region_get_ram_ptr(mr) + xlat;
+
+if (iotlb->perm != IOMMU_NONE) {
+ret = vfio_dma_map(container, iotlb->iova,
+   iotlb->addr_mask + 1, vaddr,
+   !(iotlb->perm & IOMMU_WO) || mr->readonly);
+if (ret) {
+error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
+ "0x%"HWADDR_PRIx", %p) = %d (%m)",
+ container, iotlb->iova,
+ iotlb->addr_mask + 1, vaddr, ret);
+}
+} else {
+ret = vfio_dma_unmap(container, iotlb->iova, iotlb->addr_mask + 1);
+if (ret) {
+error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
+ "0x%"HWADDR_PRIx") = %d (%m)",
+ container, iotlb->iova,
+ iotlb->addr_mask + 1, ret);
+}
+}
+}
+
+static void vfio_listener_region_add(MemoryListener *listener,
+ MemoryRegionSection *section)
+{
+VFIOContainer *container = container_of(listener, VFIOContainer,
+iommu_data.listener);
+hwaddr iova, end;
+int ret;
 
 if (vfio_listener_skipped_section(section)) {
 DPRINTF("SKIPPING region_add %"HWADDR_PRIx" - %"PRIx64"\n",
@@ -1952,19 +2011,51 @@ static void vfio_listener_region_add(MemoryListener 
*listener,
 return;
 }
 
-vaddr = memory_region_get_ram_ptr(section->mr) +
-section->offset_within_region +
-(iova - section->offset_within_address_space);
-
-DPRINTF("region_add %"HWADDR_PRIx" - %"HWADDR_PRIx" [%p]\n",
-iova, end - 1, vaddr);
-
-memory_region_ref(section->mr);
-ret = vfio_dma_map(container, iova, end - iova, vaddr, section->readonly);
-if (ret) {
-error_report("vfio

[Qemu-devel] [PATCH 7/8] spapr vfio: add spapr-pci-vfio-host-bridge to support vfio

[Alexey Kardashevskiy]

The patch adds a spapr-pci-vfio-host-bridge device type
which is a PCI Host Bridge with VFIO support. The new device
inherits from the spapr-pci-host-bridge device and adds
the following properties:
iommu - IOMMU group ID which represents a Partitionable
Endpoint, QEMU/ppc64 uses a separate PHB for
an IOMMU group so the guest kernel has to have
PCI Domain support enabled.
forceaddr (optional, 0 by default) - forces QEMU to copy
device:function from the host address as
certain guest drivers expect devices to appear in
particular locations;
mf (optional, 0 by default) - forces multifunction bit for
the function #0 of a found device, only makes sense
for multifunction devices and only with the forceaddr
property set. It would not be required if there
was a way to know in advance whether a device is
multifunctional or not.
scan (optional, 1 by default) - if non-zero, the new PHB walks
through all non-bridge devices in the group and tries
adding them to the PHB; if zero, all devices in the group
have to be configured manually via the QEMU command line.

The patch also adds a VFIO IOMMU type support to the existing
sPAPR TCE list in spapr_iommu.c.

The patch also uses the host kernel support of a new KVM_CAP_SPAPR_TCE_IOMMU
capability and KVM_CREATE_SPAPR_TCE_IOMMU ioctl which let QEMU tell
the host what LIOBN is used for an IOMMU group. This ioctl turns real mode TCE
requests handling on which accelerates actual throughput in 2.5-5 times.

Examples:
1) Scan and add all devices from IOMMU group with ID=1 to QEMU's PHB #6:
-device spapr-pci-vfio-host-bridge,id=DEVICENAME,iommu=1,index=6

2) Configure and Add 3 functions of a multifunctional device to QEMU:
(the NEC PCI USB card is used as an example here):
-device spapr-pci-vfio-host-bridge,id=USB,iommu=4,scan=0,index=7 \
-device vfio-pci,host=4:0:1.0,addr=1.0,bus=USB,multifunction=true
-device vfio-pci,host=4:0:1.1,addr=1.1,bus=USB
-device vfio-pci,host=4:0:1.2,addr=1.2,bus=USB

Cc: David Gibson 
Signed-off-by: Alexey Kardashevskiy 
---
 hw/ppc/spapr_iommu.c| 176 -
 hw/ppc/spapr_pci.c  | 209 +---
 include/hw/pci-host/spapr.h |  12 +++
 include/hw/ppc/spapr.h  |  19 
 target-ppc/kvm.c|  33 +++
 target-ppc/kvm_ppc.h|  12 +++
 trace-events|   4 +
 7 files changed, 429 insertions(+), 36 deletions(-)

diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 22b09be..096b6a9 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -16,12 +16,14 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
+
 #include "hw/hw.h"
 #include "sysemu/kvm.h"
 #include "hw/qdev.h"
 #include "kvm_ppc.h"
 #include "sysemu/dma.h"
 #include "exec/address-spaces.h"
+#include "trace.h"
 
 #include "hw/ppc/spapr.h"
 
@@ -244,6 +246,74 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, 
target_ulong ioba,
 return H_SUCCESS;
 }
 
+static IOMMUTLBEntry spapr_vfio_translate_iommu(MemoryRegion *iommu, hwaddr 
addr)
+{
+IOMMUTLBEntry entry;
+/* Must never be called */
+assert(0);
+return entry;
+}
+
+static MemoryRegionIOMMUOps spapr_vfio_iommu_ops = {
+.translate = spapr_vfio_translate_iommu,
+};
+
+static int spapr_tce_table_vfio_realize(DeviceState *dev)
+{
+sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
+
+memory_region_init_iommu(&tcet->iommu, NULL, &spapr_vfio_iommu_ops,
+ "iommu-vfio-spapr", (uint64_t)INT64_MAX+1);
+
+QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
+
+return 0;
+}
+
+sPAPRTCETable *spapr_vfio_new_table(DeviceState *owner, uint32_t liobn,
+int group_fd)
+{
+sPAPRTCETable *tcet;
+int fd;
+
+if (spapr_tce_find_by_liobn(liobn)) {
+fprintf(stderr, "Attempted to create TCE table with duplicate"
+" LIOBN 0x%x\n", liobn);
+return NULL;
+}
+
+fd = kvmppc_create_spapr_tce_iommu(liobn, group_fd);
+
+tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE_VFIO));
+tcet->liobn = liobn;
+tcet->fd = fd;
+object_property_add_child(OBJECT(owner), "tce-table", OBJECT(tcet), NULL);
+
+qdev_init_nofail(DEVICE(tcet));
+
+return tcet;
+}
+
+static target_ulong put_tce_vfio(sPAPRTCETable *tcet, target_ulong ioba,
+ target_ulong tce)
+{
+IOMMUTLBEntry entry;
+
+entry.iova = ioba & ~SPAPR_TCE_PAGE_MASK;
+entry.translated_addr = tce & ~SPAPR_TCE_PAGE_MASK;
+entry.addr_mask = SPAPR_TCE_PAGE_MASK;
+entry.perm = 0;
+if ((tce

[Qemu-devel] [PATCH 8/8] spapr vfio: enable for spapr

[Alexey Kardashevskiy]

This turns the sPAPR support on and enables VFIO container use
in the kernel.

Signed-off-by: Alexey Kardashevskiy 
---
 hw/misc/vfio.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 4571f64..4484826 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -2812,6 +2812,37 @@ static int vfio_connect_container(VFIOGroup *group, 
VFIOAddressSpace *space)
 
 memory_listener_register(&container->iommu_data.listener,
  container->space->as);
+} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
+ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
+if (ret) {
+error_report("vfio: failed to set group container: %m");
+g_free(container);
+close(fd);
+return -errno;
+}
+
+ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
+if (ret) {
+error_report("vfio: failed to set iommu for container: %m");
+g_free(container);
+close(fd);
+return -errno;
+}
+
+ret = ioctl(fd, VFIO_IOMMU_ENABLE);
+if (ret) {
+error_report("vfio: failed to enable container: %s",
+strerror(errno));
+g_free(container);
+close(fd);
+return -errno;
+}
+
+container->iommu_data.listener = vfio_memory_listener;
+container->iommu_data.release = vfio_listener_release;
+
+memory_listener_register(&container->iommu_data.listener,
+ container->space->as);
 } else {
 error_report("vfio: No available IOMMU models");
 g_free(container);
-- 
1.8.3.2

Re: [Qemu-devel] [RFC V3 0/2] continuous leaky bucket throttling

[Stefan Hajnoczi]

On Fri, Aug 02, 2013 at 05:53:00PM +0200, Benoît Canet wrote:
> This patchset implement continous leaky bucket throttling.
> 
> It works mostly on the general case.
> The exception is where the load is composed of both reads and writes and two
> limits iops_rd and iops_wr are set.
> The resulting iops are a little above half of the given limits.
> I tried various strategies to avoid this: two timer, two throttled request
> queues or even a different algorithm using a priority queue.
> The problem is still the same in every version of the code: reads and writes
> operation seems entangled.
> 
> Benoît Canet (2):
>   throttle: Add a new throttling API implementing continuus leaky
> bucket.
>   block: Enable the new throttling code in the block layer.
> 
>  block.c   |  316 
>  block/qapi.c  |   21 +--
>  blockdev.c|  115 ++--
>  include/block/block.h |1 -
>  include/block/block_int.h |   33 +---
>  include/qemu/throttle.h   |  111 
>  util/Makefile.objs|1 +
>  util/throttle.c   |  436 
> +
>  8 files changed, 698 insertions(+), 336 deletions(-)
>  create mode 100644 include/qemu/throttle.h
>  create mode 100644 util/throttle.c

I saw more discussion on IRC.  Does this mean you will send another
revision to address outstanding issues?

Just wanted to check if you are waiting for code review or if you are
still developing the next patch revision.

Stefan

Re: [Qemu-devel] [PATCH v3 For-1.6 1/7] rdma: bugfix: make IPv6 support work

[Orit Wasserman]

On 08/04/2013 05:54 AM, mrhi...@linux.vnet.ibm.com wrote:
> From: "Michael R. Hines" 
> 
> RDMA does not use sockets, so we cannot use many of the socket
> helper functions, but we *do* use inet_parse() which gives
> RDMA all the necessary details of the connection parameters.
> 
> However, when testing with libvirt, a simple IPv6 migration test failed
> because we were not using getaddrinfo() properly.
> 
> This makes IPv6 migration over RDMA work.
> 
> Signed-off-by: Michael R. Hines 
> ---
>  migration-rdma.c |   33 +
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/migration-rdma.c b/migration-rdma.c
> index d044830..9cf73e3 100644
> --- a/migration-rdma.c
> +++ b/migration-rdma.c
> @@ -392,6 +392,7 @@ typedef struct RDMAContext {
>  uint64_t unregistrations[RDMA_SIGNALED_SEND_MAX];
>  
>  GHashTable *blockmap;
> +bool ipv6;
>  } RDMAContext;
>  
>  /*
> @@ -744,6 +745,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>  char port_str[16];
>  struct rdma_cm_event *cm_event;
>  char ip[40] = "unknown";
> +int af = rdma->ipv6 ? PF_INET6 : PF_INET;
>  
>  if (rdma->host == NULL || !strcmp(rdma->host, "")) {
>  ERROR(errp, "RDMA hostname has not been set\n");
> @@ -773,7 +775,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>  goto err_resolve_get_addr;
>  }
>  
> -inet_ntop(AF_INET, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
> +inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
>  ip, sizeof ip);
>  DPRINTF("%s => %s\n", rdma->host, ip);
>  
> @@ -2236,9 +2238,12 @@ err_rdma_source_connect:
>  static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
>  {
>  int ret = -EINVAL, idx;
> +int af = rdma->ipv6 ? PF_INET6 : PF_INET;
>  struct sockaddr_in sin;
>  struct rdma_cm_id *listen_id;
>  char ip[40] = "unknown";
> +struct addrinfo *res;
> +char port_str[16];
>  
>  for (idx = 0; idx <= RDMA_WRID_MAX; idx++) {
>  rdma->wr_data[idx].control_len = 0;
> @@ -2266,27 +2271,30 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, 
> Error **errp)
>  }
>  
>  memset(&sin, 0, sizeof(sin));
> -sin.sin_family = AF_INET;
> +sin.sin_family = af;
>  sin.sin_port = htons(rdma->port);
> +snprintf(port_str, 16, "%d", rdma->port);
> +port_str[15] = '\0';
>  
>  if (rdma->host && strcmp("", rdma->host)) {
> -struct hostent *dest_addr;
> -dest_addr = gethostbyname(rdma->host);
> -if (!dest_addr) {
> -ERROR(errp, "migration could not gethostbyname!\n");
> -ret = -EINVAL;
> +ret = getaddrinfo(rdma->host, port_str, NULL, &res);

Hi Michael,

getaddrinfo can return a list of addresses, you need to handle it.
Look at qemu-sockets.c for example.

Regards,
Orit
> +if (ret < 0) {
> +ERROR(errp, "could not getaddrinfo address %s\n", rdma->host);
>  goto err_dest_init_bind_addr;
>  }
> -memcpy(&sin.sin_addr.s_addr, dest_addr->h_addr,
> -dest_addr->h_length);
> -inet_ntop(AF_INET, dest_addr->h_addr, ip, sizeof ip);
> +
> +
> +inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
> +ip, sizeof ip);
>  } else {
> -sin.sin_addr.s_addr = INADDR_ANY;
> +ERROR(errp, "migration host and port not specified!\n");
> +ret = -EINVAL;
> +goto err_dest_init_bind_addr;
>  }
>  
>  DPRINTF("%s => %s\n", rdma->host, ip);
>  
> -ret = rdma_bind_addr(listen_id, (struct sockaddr *)&sin);
> +ret = rdma_bind_addr(listen_id, res->ai_addr);
>  if (ret) {
>  ERROR(errp, "Error: could not rdma_bind_addr!\n");
>  goto err_dest_init_bind_addr;
> @@ -2321,6 +2329,7 @@ static void *qemu_rdma_data_init(const char *host_port, 
> Error **errp)
>  if (addr != NULL) {
>  rdma->port = atoi(addr->port);
>  rdma->host = g_strdup(addr->host);
> +rdma->ipv6 = addr->ipv6;
>  } else {
>  ERROR(errp, "bad RDMA migration address '%s'", host_port);
>  g_free(rdma);
> 

Re: [Qemu-devel] [PATCH v3 For-1.6 2/7] rdma: forgot to turn off the debugging flag

[Orit Wasserman]

On 08/04/2013 05:54 AM, mrhi...@linux.vnet.ibm.com wrote:
> From: "Michael R. Hines" 
> 
> Ooops. We forgot to turn off the flag.
> 
> Signed-off-by: Michael R. Hines 
> ---
>  migration-rdma.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration-rdma.c b/migration-rdma.c
> index 9cf73e3..fe6118d 100644
> --- a/migration-rdma.c
> +++ b/migration-rdma.c
> @@ -27,7 +27,7 @@
>  #include 
>  #include 
>  
> -#define DEBUG_RDMA
> +//#define DEBUG_RDMA
>  //#define DEBUG_RDMA_VERBOSE
>  //#define DEBUG_RDMA_REALLY_VERBOSE
>  
> 

Reviewed-by: Orit Wasserman 

Re: [Qemu-devel] [PATCH v3 For-1.6 3/7] rdma: correct newlines in error statements

[Orit Wasserman]

On 08/04/2013 05:54 AM, mrhi...@linux.vnet.ibm.com wrote:
> From: "Michael R. Hines" 
> 
> Don't print newlines on the error_setg() function,
> but still allow newlines on fprintf().
> 
> Signed-off-by: Michael R. Hines 
> ---
>  migration-rdma.c |   68 
> +++---
>  1 file changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/migration-rdma.c b/migration-rdma.c
> index fe6118d..c958e5f 100644
> --- a/migration-rdma.c
> +++ b/migration-rdma.c
> @@ -60,7 +60,7 @@
>   */
>  #define ERROR(errp, fmt, ...) \
>  do { \
> -fprintf(stderr, "RDMA ERROR: " fmt, ## __VA_ARGS__); \
> +fprintf(stderr, "RDMA ERROR: " fmt "\n", ## __VA_ARGS__); \
>  if (errp && (*(errp) == NULL)) { \
>  error_setg(errp, "RDMA ERROR: " fmt, ## __VA_ARGS__); \
>  } \
> @@ -748,21 +748,21 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>  int af = rdma->ipv6 ? PF_INET6 : PF_INET;
>  
>  if (rdma->host == NULL || !strcmp(rdma->host, "")) {
> -ERROR(errp, "RDMA hostname has not been set\n");
> +ERROR(errp, "RDMA hostname has not been set");
>  return -1;
>  }
>  
>  /* create CM channel */
>  rdma->channel = rdma_create_event_channel();
>  if (!rdma->channel) {
> -ERROR(errp, "could not create CM channel\n");
> +ERROR(errp, "could not create CM channel");
>  return -1;
>  }
>  
>  /* create CM id */
>  ret = rdma_create_id(rdma->channel, &rdma->cm_id, NULL, RDMA_PS_TCP);
>  if (ret) {
> -ERROR(errp, "could not create channel id\n");
> +ERROR(errp, "could not create channel id");
>  goto err_resolve_create_id;
>  }
>  
> @@ -771,7 +771,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>  
>  ret = getaddrinfo(rdma->host, port_str, NULL, &res);
>  if (ret < 0) {
> -ERROR(errp, "could not getaddrinfo address %s\n", rdma->host);
> +ERROR(errp, "could not getaddrinfo address %s", rdma->host);
>  goto err_resolve_get_addr;
>  }
>  
> @@ -783,7 +783,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>  ret = rdma_resolve_addr(rdma->cm_id, NULL, res->ai_addr,
>  RDMA_RESOLVE_TIMEOUT_MS);
>  if (ret) {
> -ERROR(errp, "could not resolve address %s\n", rdma->host);
> +ERROR(errp, "could not resolve address %s", rdma->host);
>  goto err_resolve_get_addr;
>  }
>  
> @@ -791,12 +791,12 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>  
>  ret = rdma_get_cm_event(rdma->channel, &cm_event);
>  if (ret) {
> -ERROR(errp, "could not perform event_addr_resolved\n");
> +ERROR(errp, "could not perform event_addr_resolved");
>  goto err_resolve_get_addr;
>  }
>  
>  if (cm_event->event != RDMA_CM_EVENT_ADDR_RESOLVED) {
> -ERROR(errp, "result not equal to event_addr_resolved %s\n",
> +ERROR(errp, "result not equal to event_addr_resolved %s",
>  rdma_event_str(cm_event->event));
>  perror("rdma_resolve_addr");
>  goto err_resolve_get_addr;
> @@ -806,17 +806,17 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>  /* resolve route */
>  ret = rdma_resolve_route(rdma->cm_id, RDMA_RESOLVE_TIMEOUT_MS);
>  if (ret) {
> -ERROR(errp, "could not resolve rdma route\n");
> +ERROR(errp, "could not resolve rdma route");
>  goto err_resolve_get_addr;
>  }
>  
>  ret = rdma_get_cm_event(rdma->channel, &cm_event);
>  if (ret) {
> -ERROR(errp, "could not perform event_route_resolved\n");
> +ERROR(errp, "could not perform event_route_resolved");
>  goto err_resolve_get_addr;
>  }
>  if (cm_event->event != RDMA_CM_EVENT_ROUTE_RESOLVED) {
> -ERROR(errp, "result not equal to event_route_resolved: %s\n",
> +ERROR(errp, "result not equal to event_route_resolved: %s",
>  rdma_event_str(cm_event->event));
>  rdma_ack_cm_event(cm_event);
>  goto err_resolve_get_addr;
> @@ -2117,26 +2117,26 @@ static int qemu_rdma_source_init(RDMAContext *rdma, 
> Error **errp, bool pin_all)
>  if (ret) {
>  ERROR(temp, "rdma migration: error allocating pd and cq! Your 
> mlock()"
>  " limits may be too low. Please check $ ulimit -a # and "
> -"search for 'ulimit -l' in the output\n");
> +"search for 'ulimit -l' in the output");
>  goto err_rdma_source_init;
>  }
>  
>  ret = qemu_rdma_alloc_qp(rdma);
>  if (ret) {
> -ERROR(temp, "rdma migration: error allocating qp!\n");
> +ERROR(temp, "rdma migration: error allocating qp!");
>  goto err_rdma_source_init;
>  }
>  
>  ret = qemu_rdma_init_ram_blocks(rdma);
>  if (ret) {
> -ERROR(temp, "rdma

[Qemu-devel] [PATCH 2/2] pseries: enable irqfd for pci

[Alexey Kardashevskiy]

Signed-off-by: Alexey Kardashevskiy 
---
 hw/intc/xics_kvm.c | 5 +
 hw/ppc/spapr_pci.c | 6 ++
 2 files changed, 11 insertions(+)

diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index 1f078f3..9978a47 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -448,6 +448,11 @@ static void xics_kvm_realize(DeviceState *dev, Error 
**errp)
 goto fail;
 }
 }
+
+kvm_kernel_irqchip = true;
+kvm_irqfds_allowed = true;
+kvm_msi_via_irqfd_allowed = true;
+
 return;
 
 fail:
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 6e14020..869ca43 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -457,6 +457,11 @@ static void spapr_msi_write(void *opaque, hwaddr addr,
 qemu_irq_pulse(xics_get_qirq(spapr->icp, irq));
 }
 
+static int spapr_pci_map_msi(KVMState *s, PCIBus *bus, MSIMessage msg)
+{
+return msg.data;
+}
+
 static const MemoryRegionOps spapr_msi_ops = {
 /* There is no .read as the read result is undefined by PCI spec */
 .read = NULL,
@@ -631,6 +636,7 @@ static int spapr_phb_init(SysBusDevice *s)
 
 sphb->lsi_table[i].irq = irq;
 }
+bus->map_msi = spapr_pci_map_msi;
 
 return 0;
 }
-- 
1.8.3.2

[Qemu-devel] [PATCH 0/2 v4] kvm irqfd: support msimessage to irq translation in PHB

[Alexey Kardashevskiy]

Yet another try to push IRQFD support for sPAPR.
This includes rework for QEMU and enablement for sPAPR PCI.

Alexey Kardashevskiy (2):
  kvm irqfd: support msimessage to irq translation in PHB
  pseries: enable irqfd for pci

 hw/i386/kvm/pci-assign.c | 4 ++--
 hw/intc/xics_kvm.c   | 5 +
 hw/misc/vfio.c   | 4 ++--
 hw/pci/pci.c | 9 +
 hw/ppc/spapr_pci.c   | 6 ++
 hw/virtio/virtio-pci.c   | 2 +-
 include/hw/pci/pci.h | 2 ++
 include/hw/pci/pci_bus.h | 1 +
 kvm-all.c| 3 +++
 9 files changed, 31 insertions(+), 5 deletions(-)

-- 
1.8.3.2

[Qemu-devel] [PATCH 1/2] kvm irqfd: support msimessage to irq translation in PHB

[Alexey Kardashevskiy]

On PPC64 systems MSI Messages are translated to system IRQ in a PCI
host bridge. This is already supported for emulated MSI/MSIX but
not for irqfd where the current QEMU allocates IRQ numbers from
irqchip and maps MSIMessages to those IRQ in the host kernel.

The patch extends irqfd support in order to avoid unnecessary
mapping and reuse the one which already exists in a PCI host bridge.

Specifically, a map_msi callback is added to PCIBus and pci_bus_map_msi()
to PCI API. The latter tries a bus specific map_msi and falls back to
the default kvm_irqchip_add_msi_route() if none set.

Signed-off-by: Alexey Kardashevskiy 

---
Changes:
2013/08/07 v5:
* pci_bus_map_msi now has default behaviour which is to call
kvm_irqchip_add_msi_route
* kvm_irqchip_release_virq fixed not crash when there is no routes
---
 hw/i386/kvm/pci-assign.c | 4 ++--
 hw/misc/vfio.c   | 4 ++--
 hw/pci/pci.c | 9 +
 hw/virtio/virtio-pci.c   | 2 +-
 include/hw/pci/pci.h | 2 ++
 include/hw/pci/pci_bus.h | 1 +
 kvm-all.c| 3 +++
 7 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 5618173..fb59982 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1008,9 +1008,9 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
 MSIMessage msg = msi_get_message(pci_dev, 0);
 int virq;
 
-virq = kvm_irqchip_add_msi_route(kvm_state, msg);
+virq = pci_bus_map_msi(kvm_state, pci_dev->bus, msg);
 if (virq < 0) {
-perror("assigned_dev_update_msi: kvm_irqchip_add_msi_route");
+perror("assigned_dev_update_msi: pci_bus_map_msi");
 return;
 }
 
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 017e693..adcd23d 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -643,7 +643,7 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, 
unsigned int nr,
  * Attempt to enable route through KVM irqchip,
  * default to userspace handling if unavailable.
  */
-vector->virq = msg ? kvm_irqchip_add_msi_route(kvm_state, *msg) : -1;
+vector->virq = msg ? pci_bus_map_msi(kvm_state, vdev->pdev.bus, *msg) : -1;
 if (vector->virq < 0 ||
 kvm_irqchip_add_irqfd_notifier(kvm_state, &vector->interrupt,
vector->virq) < 0) {
@@ -811,7 +811,7 @@ retry:
  * Attempt to enable route through KVM irqchip,
  * default to userspace handling if unavailable.
  */
-vector->virq = kvm_irqchip_add_msi_route(kvm_state, msg);
+vector->virq = pci_bus_map_msi(kvm_state, vdev->pdev.bus, msg);
 if (vector->virq < 0 ||
 kvm_irqchip_add_irqfd_notifier(kvm_state, &vector->interrupt,
vector->virq) < 0) {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 4c004f5..4bce3e7 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1249,6 +1249,15 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
 dev->intx_routing_notifier = notifier;
 }
 
+int pci_bus_map_msi(KVMState *s, PCIBus *bus, MSIMessage msg)
+{
+if (bus->map_msi) {
+return bus->map_msi(s, bus, msg);
+}
+
+return kvm_irqchip_add_msi_route(s, msg);
+}
+
 /*
  * PCI-to-PCI bridge specification
  * 9.1: Interrupt routing. Table 9-1
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index d37037e..21180d2 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -481,7 +481,7 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy 
*proxy,
 int ret;
 
 if (irqfd->users == 0) {
-ret = kvm_irqchip_add_msi_route(kvm_state, msg);
+ret = pci_bus_map_msi(kvm_state, proxy->pci_dev.bus, msg);
 if (ret < 0) {
 return ret;
 }
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index ccec2ba..b6ad9e4 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -332,6 +332,7 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev);
 typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
+typedef int (*pci_map_msi_fn)(KVMState *s, PCIBus *bus, MSIMessage msg);
 
 typedef enum {
 PCI_HOTPLUG_DISABLED,
@@ -375,6 +376,7 @@ bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute 
*new);
 void pci_bus_fire_intx_routing_notifier(PCIBus *bus);
 void pci_device_set_intx_routing_notifier(PCIDevice *dev,
   PCIINTxRoutingNotifier notifier);
+int pci_bus_map_msi(KVMState *s, PCIBus *bus, MSIMessage msg);
 void pci_device_reset(PCIDevice *dev);
 void pci_bus_reset(PCIBus *bus);
 
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 9df1788..5bf7994 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -16,6 +16,7 @@ struct PCIBus {
 pci_set_irq_fn set_irq;
 

Re: [Qemu-devel] [PATCH 0/3] rdma: validate remote provided RDMAControlHeader::len

[Orit Wasserman]

On 08/07/2013 05:26 AM, Isaku Yamahata wrote:
> RDMAControlHeader::len is remote-provided. So validate the value before use.
> 
> Isaku Yamahata (3):
>   rdma: use resp.len after validation in qemu_rdma_registration_stop
>   rdma: validate RDMAControlHeader::len
>   rdma: check if RDMAControlHeader::len match transferred byte
> 
>  migration-rdma.c |   44 ++--
>  1 file changed, 30 insertions(+), 14 deletions(-)
> 

Series 
Reviewed-by: Orit Wasserman 

[Qemu-devel] [PATCH] sheepdog: implement .bdrv_get_allocated_file_size

[Liu Yuan]

With this patch, qemu-img info sheepdog:image will show disk size for sheepdog
images.

Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Cc: MORITA Kazutaka 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c |   19 +++
 1 file changed, 19 insertions(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index afe0533..7699aad 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2321,6 +2321,22 @@ sd_co_is_allocated(BlockDriverState *bs, int64_t 
sector_num, int nb_sectors,
 return ret;
 }
 
+static int64_t sd_get_allocated_file_size(BlockDriverState *bs)
+{
+BDRVSheepdogState *s = bs->opaque;
+SheepdogInode *inode = &s->inode;
+unsigned long i, last = DIV_ROUND_UP(inode->vdi_size, SD_DATA_OBJ_SIZE);
+uint64_t size = 0;
+
+for (i = 0; i < last; i++) {
+if (inode->data_vdi_id[i] == 0) {
+continue;
+}
+size += SD_DATA_OBJ_SIZE;
+}
+return size;
+}
+
 static QEMUOptionParameter sd_create_options[] = {
 {
 .name = BLOCK_OPT_SIZE,
@@ -2349,6 +2365,7 @@ static BlockDriver bdrv_sheepdog = {
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_getlength = sd_getlength,
+.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
 .bdrv_truncate  = sd_truncate,
 
 .bdrv_co_readv  = sd_co_readv,
@@ -2377,6 +2394,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_getlength = sd_getlength,
+.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
 .bdrv_truncate  = sd_truncate,
 
 .bdrv_co_readv  = sd_co_readv,
@@ -2405,6 +2423,7 @@ static BlockDriver bdrv_sheepdog_unix = {
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_getlength = sd_getlength,
+.bdrv_get_allocated_file_size = sd_get_allocated_file_size,
 .bdrv_truncate  = sd_truncate,
 
 .bdrv_co_readv  = sd_co_readv,
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH for-1.6? v2 01/21] mips_mipssim: Silence BIOS loading warning for qtest

[Aurelien Jarno]

On Mon, Aug 05, 2013 at 03:27:07PM +0200, Andreas Färber wrote:
> Signed-off-by: Andreas Färber 
> ---
>  hw/mips/mips_mipssim.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index fea1a15..d8c4347 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -37,6 +37,7 @@
>  #include "elf.h"
>  #include "hw/sysbus.h"
>  #include "exec/address-spaces.h"
> +#include "sysemu/qtest.h"
>  
>  static struct _loaderparams {
>  int ram_size;
> @@ -189,7 +190,8 @@ mips_mipssim_init(QEMUMachineInitArgs *args)
>  } else {
>  bios_size = -1;
>  }
> -if ((bios_size < 0 || bios_size > BIOS_SIZE) && !kernel_filename) {
> +if ((bios_size < 0 || bios_size > BIOS_SIZE) &&
> +!kernel_filename && !qtest_enabled()) {
>  /* Bail out if we have neither a kernel image nor boot vector code. 
> */
>  fprintf(stderr,
>  "qemu: Warning, could not load MIPS bios '%s', and no 
> -kernel argument was specified\n",

Reviewed-by: Aurelien Jarno 

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

Re: [Qemu-devel] [PATCH for-1.6? v2 18/21] shix: Drop debug output

[Aurelien Jarno]

On Mon, Aug 05, 2013 at 03:27:24PM +0200, Andreas Färber wrote:
> Signed-off-by: Andreas Färber 
> ---
>  hw/sh4/shix.c | 7 ---
>  1 file changed, 7 deletions(-)
> 
> diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
> index 84dd666..66cbea4 100644
> --- a/hw/sh4/shix.c
> +++ b/hw/sh4/shix.c
> @@ -50,7 +50,6 @@ static void shix_init(QEMUMachineInitArgs *args)
>  if (!cpu_model)
>  cpu_model = "any";
>  
> -printf("Initializing CPU\n");
>  cpu = cpu_sh4_init(cpu_model);
>  if (cpu == NULL) {
>  fprintf(stderr, "Unable to find CPU definition\n");
> @@ -58,16 +57,13 @@ static void shix_init(QEMUMachineInitArgs *args)
>  }
>  
>  /* Allocate memory space */
> -printf("Allocating ROM\n");
>  memory_region_init_ram(rom, NULL, "shix.rom", 0x4000);
>  vmstate_register_ram_global(rom);
>  memory_region_set_readonly(rom, true);
>  memory_region_add_subregion(sysmem, 0x, rom);
> -printf("Allocating SDRAM 1\n");
>  memory_region_init_ram(&sdram[0], NULL, "shix.sdram1", 0x0100);
>  vmstate_register_ram_global(&sdram[0]);
>  memory_region_add_subregion(sysmem, 0x0800, &sdram[0]);
> -printf("Allocating SDRAM 2\n");
>  memory_region_init_ram(&sdram[1], NULL, "shix.sdram2", 0x0100);
>  vmstate_register_ram_global(&sdram[1]);
>  memory_region_add_subregion(sysmem, 0x0c00, &sdram[1]);
> @@ -75,10 +71,8 @@ static void shix_init(QEMUMachineInitArgs *args)
>  /* Load BIOS in 0 (and access it through P2, 0xA000) */
>  if (bios_name == NULL)
>  bios_name = BIOS_FILENAME;
> -printf("%s: load BIOS '%s'\n", __func__, bios_name);
>  ret = load_image_targphys(bios_name, 0, 0x4000);
>  if (ret < 0) {   /* Check bios size */
> - fprintf(stderr, "ret=%d\n", ret);
>   fprintf(stderr, "qemu: could not load SHIX bios '%s'\n",
>   bios_name);
>   exit(1);
> @@ -88,7 +82,6 @@ static void shix_init(QEMUMachineInitArgs *args)
>  s = sh7750_init(cpu, sysmem);
>  /* X Check success */
>  tc58128_init(s, "shix_linux_nand.bin", NULL);
> -fprintf(stderr, "initialization terminated\n");
>  }
>  
>  static QEMUMachine shix_machine = {

Reviewed-by: Aurelien Jarno 

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

Re: [Qemu-devel] [PATCH for-1.6? v2 19/21] shix: Don't require firmware presence for qtest

[Aurelien Jarno]

On Mon, Aug 05, 2013 at 03:27:25PM +0200, Andreas Färber wrote:
> Adopt error_report() while at it.
> 
> Signed-off-by: Andreas Färber 
> ---
>  hw/block/tc58128.c | 10 ++
>  hw/sh4/shix.c  |  9 +
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c
> index a3929d4..728f1c3 100644
> --- a/hw/block/tc58128.c
> +++ b/hw/block/tc58128.c
> @@ -1,6 +1,8 @@
>  #include "hw/hw.h"
>  #include "hw/sh4/sh.h"
>  #include "hw/loader.h"
> +#include "sysemu/qtest.h"
> +#include "qemu/error-report.h"
>  
>  #define CE1  0x0100
>  #define CE2  0x0200
> @@ -36,10 +38,10 @@ static void init_dev(tc58128_dev * dev, const char 
> *filename)
>   /* Load flash image skipping the first block */
>   ret = load_image(filename, dev->flash_contents + 528 * 32);
>   if (ret < 0) {
> - fprintf(stderr, "ret=%d\n", ret);
> - fprintf(stderr, "qemu: could not load flash image %s\n",
> - filename);
> - exit(1);
> +if (!qtest_enabled()) {
> +error_report("Could not load flash image %s", filename);
> +exit(1);
> +}
>   } else {
>   /* Build first block with number of blocks */
>   blocks = (ret + 528 * 32 - 1) / (528 * 32);
> diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
> index 66cbea4..5ee7af6 100644
> --- a/hw/sh4/shix.c
> +++ b/hw/sh4/shix.c
> @@ -30,9 +30,11 @@
>  #include "hw/hw.h"
>  #include "hw/sh4/sh.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/qtest.h"
>  #include "hw/boards.h"
>  #include "hw/loader.h"
>  #include "exec/address-spaces.h"
> +#include "qemu/error-report.h"
>  
>  #define BIOS_FILENAME "shix_bios.bin"
>  #define BIOS_ADDRESS 0xA000
> @@ -72,10 +74,9 @@ static void shix_init(QEMUMachineInitArgs *args)
>  if (bios_name == NULL)
>  bios_name = BIOS_FILENAME;
>  ret = load_image_targphys(bios_name, 0, 0x4000);
> -if (ret < 0) {   /* Check bios size */
> - fprintf(stderr, "qemu: could not load SHIX bios '%s'\n",
> - bios_name);
> - exit(1);
> +if (ret < 0 && !qtest_enabled()) {
> +error_report("Could not load SHIX bios '%s'", bios_name);
> +exit(1);
>  }
>  
>  /* Register peripherals */

Reviewed-by: Aurelien Jarno 

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

Re: [Qemu-devel] [RFC] [PATCHv5 06/16] aio / timers: Untangle include files

[Alex Bligh]

Stefan,


Some tips on which packages to install:


host big endian   no
gprof enabled no
sparse enabledno
strip binariesno
profiler  no
static build  no


All the ones above are okay, fine to ignore.


SDL support   no


Do you have libsdl1.2-dev installed?


ii  libsdl1.2-dev 1.2.14-6.4ubuntu3

but still doesn't work


vde support   no


You need libvde-dev and/or libvdeplug2-dev.


ii  libvde-dev2.2.3-3build2

but still doesn't work


spice support no (/)


libspice-protocol-dev and/or libspice-server-dev (can't remember).


ii  gir1.2-spice-client-glib-2.0  0.9-0ubuntu1 
GObject for communicating with Spice servers (GObject-Introspection)
ii  gir1.2-spice-client-gtk-2.0   0.9-0ubuntu1 
GTK2 widget for SPICE clients (GObject-Introspection)
ii  gir1.2-spice-client-gtk-3.0   0.9-0ubuntu1 
GTK3 widget for SPICE clients (GObject-Introspection)
ii  libspice-client-glib-2.0-10.9-0ubuntu1 
GObject for communicating with Spice servers (runtime library)
ii  libspice-client-glib-2.0-dev  0.9-0ubuntu1 
GObject for communicating with Spice servers (development files)
ii  libspice-client-gtk-2.0-1 0.9-0ubuntu1 
GTK2 widget for SPICE clients (runtime library)
ii  libspice-client-gtk-2.0-dev   0.9-0ubuntu1 
GTK2 widget for SPICE clients (development files)
ii  libspice-client-gtk-3.0-1 0.9-0ubuntu1 
GTK3 widget for SPICE clients (runtime library)
ii  libspice-client-gtk-3.0-dev   0.9-0ubuntu1 
GTK3 widget for SPICE clients (development files)
ii  libspice-protocol-dev 0.10.1-1 
SPICE protocol headers
ii  libspice-server-dev   0.10.0-1 
Header files and development documentation for spice-server
ii  libspice-server1  0.10.0-1 
Implements the server side of the SPICE protocol


but still doesn't work


usb net redir no
libiscsi support  no
seccomp support   no
GlusterFS support no


Ok.


libssh2 support   no


libssh2-1-dev


I thought I'd installed that but I hadn't. I have now and it builds
fine with v7.

--
Alex Bligh

Re: [Qemu-devel] [RFC] [PATCHv6 01/16] aio / timers: add qemu-timer.c utility functions

[Alex Bligh]

--On 7 August 2013 09:21:17 +0200 Stefan Hajnoczi  
wrote:



How do I actually run the code (i.e. how do I test whether I've broken
it)? I take it that's something different from just 'make check'?


make check includes qtest test cases like rtc-test, i440fx-test,
fdc-test, and others.  As long as they continue to pass all is good.


Oh great. We're fine then.

--
Alex Bligh

Re: [Qemu-devel] [RFC] [PATCHv6 05/16] aio / timers: Split QEMUClock into QEMUClock and QEMUTimerList

[Alex Bligh]

--On 7 August 2013 09:27:36 +0200 Stefan Hajnoczi  
wrote:



Do you think we need to wrap timer_mod, timer_del, timer_free
etc. to make aio_timer_mod and so forth, even though they are
a straight pass through?


Those wrappers are not necessary.  Once the caller has their QEMUTimer
pointer they should use the QEMUTimer APIs directly.


Good. That's the approach I took in v7. I'm progressing well with a perl
program to do automatic conversion.

--
Alex Bligh

Re: [Qemu-devel] [PATCH 1/8] pci: Introduce helper to retrieve a PCI device's DMA address space

[Michael S. Tsirkin]

On Wed, Aug 07, 2013 at 06:21:25PM +1000, Alexey Kardashevskiy wrote:
> From: David Gibson 
> 
> A PCI device's DMA address space (possibly an IOMMU) is returned by a
> method on the PCIBus.  At the moment that only has one caller, so the
> method is simply open coded.  We'll need another caller for VFIO, so
> this patch introduces a helper/wrapper function.
> 
> Signed-off-by: David Gibson 
> ---
>  hw/pci/pci.c | 9 -
>  include/hw/pci/pci.h | 1 +
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 4bce3e7..3cea25f 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -813,7 +813,7 @@ static PCIDevice *do_pci_register_device(PCIDevice 
> *pci_dev, PCIBus *bus,
>  
>  pci_dev->bus = bus;
>  if (bus->iommu_fn) {
> -dma_as = bus->iommu_fn(bus, bus->iommu_opaque, devfn);
> +dma_as = pci_iommu_as(pci_dev);
>  } else {
>  /* FIXME: inherit memory region from bus creator */
>  dma_as = &address_space_memory;
> @@ -2248,6 +2248,13 @@ static void pci_device_class_init(ObjectClass *klass, 
> void *data)
>  k->props = pci_props;
>  }
>  
> +AddressSpace *pci_iommu_as(PCIDevice *dev)
> +{
> +PCIBus *bus = PCI_BUS(dev->bus);
> +
> +return bus->iommu_fn(bus, bus->iommu_opaque, dev->devfn);
> +}
> +
>  void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque)
>  {
>  bus->iommu_fn = fn;
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index b6ad9e4..614f809 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -407,6 +407,7 @@ void pci_device_deassert_intx(PCIDevice *dev);
>  
>  typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
>  
> +AddressSpace *pci_iommu_as(PCIDevice *dev);

I prefer full names, and it needs to mention "device". How about:
AddressSpace *pci_device_iommu_address_space(PCIDevice *dev);

>  void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque);
>  
>  static inline void
> -- 
> 1.8.3.2

Re: [Qemu-devel] [RFC] [PATCHv7 01/22] aio / timers: Add qemu_clock_free and expose qemu_clock_new and clock types

[Stefan Hajnoczi]

On Wed, Aug 07, 2013 at 12:48:55AM +0100, Alex Bligh wrote:
I forgot to mention that there's no need to change the doc comments
you've already written.  I just wanted to share that to save you work in
the future.

Re: [Qemu-devel] [RFC] [PATCHv7 01/22] aio / timers: Add qemu_clock_free and expose qemu_clock_new and clock types

[Stefan Hajnoczi]

On Wed, Aug 07, 2013 at 12:48:55AM +0100, Alex Bligh wrote:
> Add qemu_clock_free and expose qemu_new_clock as qemu_clock_new.
> 
> Expose clock types.
> 
> Signed-off-by: Alex Bligh 
> ---
>  include/qemu/timer.h |   22 ++
>  qemu-timer.c |   11 ++-
>  2 files changed, 28 insertions(+), 5 deletions(-)

Regarding doc comments: if the function is self-documenting then the doc
comment might not add much.  I think in those cases it's okay to omit
details about arguments or return values if they are clear from the
function prototype.

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [RFC] [PATCHv7 01/22] aio / timers: Add qemu_clock_free and expose qemu_clock_new and clock types

[Alex Bligh]

--On 7 August 2013 13:31:07 +0200 Stefan Hajnoczi  
wrote:



Regarding doc comments: if the function is self-documenting then the doc
comment might not add much.  I think in those cases it's okay to omit
details about arguments or return values if they are clear from the
function prototype.


Given I've done them for every function, I hope they don't cause any
active harm. I was concerned that with an API change some things that
one might have thought were obvious are in fact not obvious (e.g.
timer_del vs. timer_free).

--
Alex Bligh

Re: [Qemu-devel] [PATCH 2/5] target-ppc: USE LPCR_ILE to control exception endian on POWER7

[Anthony Liguori]

Andreas Färber  writes:

> Am 07.08.2013 02:47, schrieb Anton Blanchard:
>> On POWER7, LPCR_ILE is used to control what endian guests take
>> their exceptions in so use it instead of MSR_ILE.
>> 
>> Signed-off-by: Anton Blanchard 
>> ---
>>  target-ppc/cpu.h |  2 ++
>>  target-ppc/excp_helper.c | 10 ++
>>  2 files changed, 12 insertions(+)
>> 
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index 711db08..422a6bb 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -453,6 +453,8 @@ struct ppc_slb_t {
>>  #define MSR_RI   1  /* Recoverable interrupt1   
>>  */
>>  #define MSR_LE   0  /* Little-endian mode   1 
>> hflags */
>>  
>> +#define LPCR_ILE (1 << (63-38))
>> +
>>  #define msr_sf   ((env->msr >> MSR_SF)   & 1)
>>  #define msr_isf  ((env->msr >> MSR_ISF)  & 1)
>>  #define msr_shv  ((env->msr >> MSR_SHV)  & 1)
>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>> index e9fcad8..e957761 100644
>> --- a/target-ppc/excp_helper.c
>> +++ b/target-ppc/excp_helper.c
>> @@ -611,9 +611,19 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>> excp_model, int excp)
>>  tlb_flush(env, 1);
>>  }
>>  
>> +#ifdef TARGET_PPC64
>> +if (excp_model == POWERPC_EXCP_POWER7) {
>> +if (env->spr[SPR_LPCR] & LPCR_ILE) {
>> +new_msr |= (target_ulong)1 << MSR_LE;
>> +}
>> +} else if (msr_ile) {
>> +new_msr |= (target_ulong)1 << MSR_LE;
>> +}
>> +#else
>
> You could just do
>
> ...
> } else
> #endif
>
> to spare duplicating the msr_ile path.

If the condition was more complicated, I'd agree, but I really dislike
splitting statements up via an #ifdef.

Regards,

Anthony Liguori

>
> Andreas
>
>>  if (msr_ile) {
>>  new_msr |= (target_ulong)1 << MSR_LE;
>>  }
>> +#endif
>>  
>>  /* Jump to handler */
>>  vector = env->excp_vectors[excp];
>> 
>
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] Using virtio-mmio

[Erlon Cruz]

On Sat, Aug 3, 2013 at 7:03 AM, Peter Maydell wrote:

> On 2 August 2013 18:48, Erlon Cruz  wrote:
> > kernel:  Linux 3.10-rc6 -> vexpress_defconfig
>
> > The kernel boots, but it cant mount the SD:
> >
> > mmc0: host doesn't support card's voltages
> > mmc0: error -22 whilst initialising SD card
> > SD: Unknown CMD1
>
> The mainline kernel's vexpress_defconfig is known to
> be broken. In particular you need to make sure you
> define at least CONFIG_REGULATOR and CONFIG_REGULATOR_VEXPRESS,
> otherwise the MMC controller driver thinks it supports no
> voltages at all and fails like this.
>

Hmm, adding this option fixed the MMC problem. I mean, I can see the MMC
booting vexpress-a15. vexpress-a9 still doesnt shows up. I have tried
several kernels/configs combinations.

v3.9 - vexpress_defconfig + VEXPRESS
v3.9 - with configs from linaro sources as in this tutorial (
https://wiki.linaro.org/Resources/HowTo/KernelDeploy)
v3.9 - with a config from (3.2.0-1800-linaro-lt-vexpress-a9) that I can
sucessfully boot

Can you guys suggest any other config I could try? Or kernel version or
repo?

Kind Regards,
Erlon





>
> -- PMM
>

Re: [Qemu-devel] [PATCH 00/16 RFC v6] ACPI memory hotplug

[Erlon Cruz]

On Fri, Aug 2, 2013 at 9:35 AM, Anthony Liguori  wrote:

> Applied.  Thanks.
>
> Where can I find this branch?


> Regards,
>
> Anthony Liguori
>
>
>

[Qemu-devel] [Bug 1209180] [NEW] hw/usb/core.c:415: usb_handle_packet: Assertion `p->ep->type != 3 || (dev->flags & (1 << USB_DEV_FLAG_IS_HOST))' failed

[Lekensteyn]

Public bug reported:

After the patch at http://lists.nongnu.org/archive/html/qemu-
devel/2013-05/msg01567.html, I cannot get my Logitech Unifying USB
receiver passed to the guest anymore.

Minimal reproduction command:

qemu-system-x86_64 -usbdevice host:046d:c52b

Debug patch:

diff --git a/hw/usb/core.c b/hw/usb/core.c
index 05948ca..20753cc 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -409,6 +409,8 @@ void usb_handle_packet(USBDevice *dev, USBPacket *p)
 /* hcd drivers cannot handle async for isoc */
 assert(p->ep->type != USB_ENDPOINT_XFER_ISOC);
 /* using async for interrupt packets breaks migration */
+   printf("type=%x %x %x %p %p\n", p->ep->type,
+   dev->flags, p->ep->dev->flags, dev, p->ep->dev);
 assert(p->ep->type != USB_ENDPOINT_XFER_INT ||
(dev->flags & (1 << USB_DEV_FLAG_IS_HOST)));
 usb_packet_set_state(p, USB_PACKET_ASYNC);

yields:

type=0 1 1 0x565de4c0 0x565de4c0
type=0 1 1 0x565de4c0 0x565de4c0
type=0 1 1 0x565de4c0 0x565de4c0
type=0 1 1 0x565de4c0 0x565de4c0
type=0 1 1 0x565de4c0 0x565de4c0
type=3 1 1 0x565de4c0 0x565de4c0
qemu-system-x86_64: hw/usb/core.c:415: usb_handle_packet: Assertion 
`p->ep->type != 3 || (dev->flags & (1 << USB_DEV_FLAG_IS_HOST))' failed.

Program received signal SIGABRT, Aborted.
0x710c61c9 in raise () from /usr/lib/libc.so.6
(gdb) bt
#0  0x710c61c9 in raise () from /usr/lib/libc.so.6
#1  0x710c75c8 in abort () from /usr/lib/libc.so.6
#2  0x710bf356 in __assert_fail_base () from /usr/lib/libc.so.6
#3  0x710bf402 in __assert_fail () from /usr/lib/libc.so.6
#4  0x556e4b38 in usb_handle_packet (dev=, 
p=p@entry=0x565da610) at hw/usb/core.c:414
#5  0x556f93ef in uhci_handle_td (s=s@entry=0x565ce5d0, 
q=0x566a9dc0, q@entry=0x0, qh_addr=qh_addr@entry=980834, 
td=td@entry=0x7fffdd30, td_addr=, 
int_mask=int_mask@entry=0x7fffdd1c)
at hw/usb/hcd-uhci.c:904
#6  0x556f9949 in uhci_process_frame (s=s@entry=0x565ce5d0) at 
hw/usb/hcd-uhci.c:1084
#7  0x556f9c35 in uhci_frame_timer (opaque=0x565ce5d0) at 
hw/usb/hcd-uhci.c:1183
#8  0x5573e086 in qemu_run_timers (clock=0x5654e200) at 
qemu-timer.c:394
#9  0x5573e325 in qemu_run_timers (clock=) at 
qemu-timer.c:459
#10 qemu_run_all_timers () at qemu-timer.c:452
#11 0x55711bee in main_loop_wait (nonblocking=) at 
main-loop.c:470
#12 0x555e0161 in main_loop () at vl.c:2029
#13 main (argc=, argv=, envp=) at 
vl.c:4419

Looking around, I see that host-bsd.c and host-linux.c both set
USB_DEV_FLAG_IS_HOST, but host-libusbx.c doesn't do that.

Affected QEMU versions: 1.5.1, 1.5.2
Not affected: 1.5.0
libusb version: 1.0.16

** 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/1209180

Title:
  hw/usb/core.c:415: usb_handle_packet: Assertion `p->ep->type != 3 ||
  (dev->flags & (1 << USB_DEV_FLAG_IS_HOST))' failed

Status in QEMU:
  New

Bug description:
  After the patch at http://lists.nongnu.org/archive/html/qemu-
  devel/2013-05/msg01567.html, I cannot get my Logitech Unifying USB
  receiver passed to the guest anymore.

  Minimal reproduction command:

  qemu-system-x86_64 -usbdevice host:046d:c52b

  Debug patch:

  diff --git a/hw/usb/core.c b/hw/usb/core.c
  index 05948ca..20753cc 100644
  --- a/hw/usb/core.c
  +++ b/hw/usb/core.c
  @@ -409,6 +409,8 @@ void usb_handle_packet(USBDevice *dev, USBPacket *p)
   /* hcd drivers cannot handle async for isoc */
   assert(p->ep->type != USB_ENDPOINT_XFER_ISOC);
   /* using async for interrupt packets breaks migration */
  + printf("type=%x %x %x %p %p\n", p->ep->type,
  + dev->flags, p->ep->dev->flags, dev, p->ep->dev);
   assert(p->ep->type != USB_ENDPOINT_XFER_INT ||
  (dev->flags & (1 << USB_DEV_FLAG_IS_HOST)));
   usb_packet_set_state(p, USB_PACKET_ASYNC);

  yields:

  type=0 1 1 0x565de4c0 0x565de4c0
  type=0 1 1 0x565de4c0 0x565de4c0
  type=0 1 1 0x565de4c0 0x565de4c0
  type=0 1 1 0x565de4c0 0x565de4c0
  type=0 1 1 0x565de4c0 0x565de4c0
  type=3 1 1 0x565de4c0 0x565de4c0
  qemu-system-x86_64: hw/usb/core.c:415: usb_handle_packet: Assertion 
`p->ep->type != 3 || (dev->flags & (1 << USB_DEV_FLAG_IS_HOST))' failed.

  Program received signal SIGABRT, Aborted.
  0x710c61c9 in raise () from /usr/lib/libc.so.6
  (gdb) bt
  #0  0x710c61c9 in raise () from /usr/lib/libc.so.6
  #1  0x710c75c8 in abort () from /usr/lib/libc.so.6
  #2  0x710bf356 in __assert_fail_base () from /usr/lib/libc.so.6
  #3  0x710bf402 in __assert_fail () from /usr/lib/libc.so.6

Re: [Qemu-devel] [PATCH 5/6] xics: split to xics and xics-common

[Andreas Färber]

Am 07.08.2013 09:26, schrieb Alexey Kardashevskiy:
> On 08/07/2013 05:03 PM, Andreas Färber wrote:
>> Am 07.08.2013 08:06, schrieb Alexey Kardashevskiy:
>>> [...] How do I test it?
>>
>> ./QMP/qom-list to find the path, if not fixed in code yet, and
> 
> Something is wrong. XICS does not have an id but it should not be a
> problem, right (btw how to set it from the code?)?
> 
> [aik@dyn232 ~]$ ./qemu-impreza/QMP/qom-list -s ./qmp-sock
> /

That's expected for lack of argument.

$ ./QMP/qom-list -s ./qmp-sock /machine
unattached/
peripheral/
peripheral-anon/
type

This confirms "path ... not fixed in code yet" (otherwise there would've
been an "spapr/" entry or anything else telling.

So you need to look through /machine/unassigned for the device[n] with
qom-get /machine/unassigned/device[n].type matching your type, possibly
device[0] as in my case. From there on you see your icp[0]/ and ics/
children without searching the unassigned list again.

Or simply use my qom-tree script:
http://lists.nongnu.org/archive/html/qemu-devel/2013-07/txte1WIbWzu5Z.txt

Setting a canonical path is done via object_property_add_child() after
instantiating and before realizing a device.
For x86 the northbridge is used as name, i.e. "i440fx" for pc and "q35"
for q35. Suggest to discuss with Anthony how to structure the
composition tree for sPAPR.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [Bug 1209180] Re: hw/usb/core.c:415: usb_handle_packet: Assertion `p->ep->type != 3 || (dev->flags & (1 << USB_DEV_FLAG_IS_HOST))' failed

[Cole Robinson]

Your observation is correct, but this is already fixed upstream:

commit 628e54857a82a3cb65ef96c12640c30d6307a064
Author: Gerd Hoffmann 
Date:   Wed Jun 12 13:17:02 2013 +0200

usb-host-libusb: set USB_DEV_FLAG_IS_HOST

** Changed in: qemu
   Status: New => Fix Committed

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

Title:
  hw/usb/core.c:415: usb_handle_packet: Assertion `p->ep->type != 3 ||
  (dev->flags & (1 << USB_DEV_FLAG_IS_HOST))' failed

Status in QEMU:
  Fix Committed

Bug description:
  After the patch at http://lists.nongnu.org/archive/html/qemu-
  devel/2013-05/msg01567.html, I cannot get my Logitech Unifying USB
  receiver passed to the guest anymore.

  Minimal reproduction command:

  qemu-system-x86_64 -usbdevice host:046d:c52b

  Debug patch:

  diff --git a/hw/usb/core.c b/hw/usb/core.c
  index 05948ca..20753cc 100644
  --- a/hw/usb/core.c
  +++ b/hw/usb/core.c
  @@ -409,6 +409,8 @@ void usb_handle_packet(USBDevice *dev, USBPacket *p)
   /* hcd drivers cannot handle async for isoc */
   assert(p->ep->type != USB_ENDPOINT_XFER_ISOC);
   /* using async for interrupt packets breaks migration */
  + printf("type=%x %x %x %p %p\n", p->ep->type,
  + dev->flags, p->ep->dev->flags, dev, p->ep->dev);
   assert(p->ep->type != USB_ENDPOINT_XFER_INT ||
  (dev->flags & (1 << USB_DEV_FLAG_IS_HOST)));
   usb_packet_set_state(p, USB_PACKET_ASYNC);

  yields:

  type=0 1 1 0x565de4c0 0x565de4c0
  type=0 1 1 0x565de4c0 0x565de4c0
  type=0 1 1 0x565de4c0 0x565de4c0
  type=0 1 1 0x565de4c0 0x565de4c0
  type=0 1 1 0x565de4c0 0x565de4c0
  type=3 1 1 0x565de4c0 0x565de4c0
  qemu-system-x86_64: hw/usb/core.c:415: usb_handle_packet: Assertion 
`p->ep->type != 3 || (dev->flags & (1 << USB_DEV_FLAG_IS_HOST))' failed.

  Program received signal SIGABRT, Aborted.
  0x710c61c9 in raise () from /usr/lib/libc.so.6
  (gdb) bt
  #0  0x710c61c9 in raise () from /usr/lib/libc.so.6
  #1  0x710c75c8 in abort () from /usr/lib/libc.so.6
  #2  0x710bf356 in __assert_fail_base () from /usr/lib/libc.so.6
  #3  0x710bf402 in __assert_fail () from /usr/lib/libc.so.6
  #4  0x556e4b38 in usb_handle_packet (dev=, 
p=p@entry=0x565da610) at hw/usb/core.c:414
  #5  0x556f93ef in uhci_handle_td (s=s@entry=0x565ce5d0, 
q=0x566a9dc0, q@entry=0x0, qh_addr=qh_addr@entry=980834, 
td=td@entry=0x7fffdd30, td_addr=, 
int_mask=int_mask@entry=0x7fffdd1c)
  at hw/usb/hcd-uhci.c:904
  #6  0x556f9949 in uhci_process_frame (s=s@entry=0x565ce5d0) at 
hw/usb/hcd-uhci.c:1084
  #7  0x556f9c35 in uhci_frame_timer (opaque=0x565ce5d0) at 
hw/usb/hcd-uhci.c:1183
  #8  0x5573e086 in qemu_run_timers (clock=0x5654e200) at 
qemu-timer.c:394
  #9  0x5573e325 in qemu_run_timers (clock=) at 
qemu-timer.c:459
  #10 qemu_run_all_timers () at qemu-timer.c:452
  #11 0x55711bee in main_loop_wait (nonblocking=) at 
main-loop.c:470
  #12 0x555e0161 in main_loop () at vl.c:2029
  #13 main (argc=, argv=, envp=) 
at vl.c:4419

  Looking around, I see that host-bsd.c and host-linux.c both set
  USB_DEV_FLAG_IS_HOST, but host-libusbx.c doesn't do that.

  Affected QEMU versions: 1.5.1, 1.5.2
  Not affected: 1.5.0
  libusb version: 1.0.16

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

Re: [Qemu-devel] [PATCH v2 2/9] block: vhdx - add header update capability.

[Jeff Cody]

On Thu, Aug 01, 2013 at 03:44:41PM +0200, Stefan Hajnoczi wrote:
> On Wed, Jul 31, 2013 at 11:23:47PM -0400, Jeff Cody wrote:
> > @@ -212,6 +242,24 @@ bool vhdx_checksum_is_valid(uint8_t *buf, size_t size, 
> > int crc_offset)
> >  
> >  
> >  /*
> > + * This generates a UUID that is compliant with the MS GUIDs used
> > + * in the VHDX spec (and elsewhere).
> > + *
> > + * We can do this with uuid_generate if uuid.h is present,
> > + * however not all systems have uuid and the generation is
> > + * pretty straightforward for the DCE + random usage case
> 
> The talk of uuid.h not being available confuses me.  The code has no
> #ifdef and we do not define uuid_generate() ourselves.
> 
> Is this an outdated comment?
> 
> > +/* Update the VHDX headers
> > + *
> > + * This follows the VHDX spec procedures for header updates.
> > + *
> > + *  - non-current header is updated with largest sequence number
> > + */
> > +static int vhdx_update_header(BlockDriverState *bs, BDRVVHDXState *s, bool 
> > rw)
> 
> "rw" should be named "generate_file_write_guid".  If you call
> vhdx_update_header() again later you do not need to update FileWriteGuid
> again according to the spec.  There's probably no harm in doing so but
> the spec explicitly says "If this is the first header update within the
> session, use a new value for the FileWriteGuid".
> 
> This means that future calls to vhdx_update_header() in the same session
> should set "generate_file_write_guid" to false.  Therefore "rw" is not
> the right name.
> 

Reading through your comments again made me realize there is a
misunderstanding (a good sign in itself that 'rw' should be renamed).
The 'rw' is not to generate the FileWriteGuid, but rather the
DataWriteGuid.  FileWriteGuid is copied from our session_guid, so it
is only generated once and used through the entire open session.  The
s->session_guid field is generated upon vhdx_open(), for usage if the
file is opened BDRV_O_RDWR.

The DataWriteGuid is generated before any user-visible writes
(anything observable via a virtual disk read).  And that DataWriteGuid
is what the 'rw' field is used for, and the function
vhdx_user_visible_write() handles calling vhdx_update_headers() with
that field set to true.

I'll rename 'rw' to 'generate_data_write_guid'.


> > +{
> > +int ret = 0;
> > +int hdr_idx = 0;
> > +uint64_t header_offset = VHDX_HEADER1_OFFSET;
> > +
> > +VHDXHeader *active_header;
> > +VHDXHeader *inactive_header;
> > +VHDXHeader header_le;
> > +uint8_t *buffer;
> > +
> > +/* operate on the non-current header */
> > +if (s->curr_header == 0) {
> > +hdr_idx = 1;
> > +header_offset = VHDX_HEADER2_OFFSET;
> > +}
> > +
> > +active_header   = s->headers[s->curr_header];
> > +inactive_header = s->headers[hdr_idx];
> > +
> > +inactive_header->sequence_number = active_header->sequence_number + 1;
> > +
> > +/* a new file guid must be generate before any file write, including
> 
> s/generate/generated/
> 
> > + * headers */
> > +memcpy(&inactive_header->file_write_guid, &s->session_guid,
> > +   sizeof(MSGUID));
> > +
> > +/* a new data guid only needs to be generate before any guest-visible
> > + * writes, so update it if the image is opened r/w. */
> > +if (rw) {
> > +vhdx_guid_generate(&inactive_header->data_write_guid);
> > +}
> > +
> > +/* the header checksum is not over just the packed size of VHDXHeader,
> > + * but rather over the entire 'reserved' range for the header, which is
> > + * 4KB (VHDX_HEADER_SIZE). */
> > +
> > +buffer = qemu_blockalign(bs, VHDX_HEADER_SIZE);
> > +/* we can't assume the extra reserved bytes are 0 */
> > +ret = bdrv_pread(bs->file, header_offset, buffer, VHDX_HEADER_SIZE);
> > +if (ret < 0) {
> > +goto exit;
> > +}
> > +/* overwrite the actual VHDXHeader portion */
> > +memcpy(buffer, inactive_header, sizeof(VHDXHeader));
> > +inactive_header->checksum = vhdx_update_checksum(buffer,
> > + VHDX_HEADER_SIZE, 4);
> > +vhdx_header_le_export(inactive_header, &header_le);
> > +bdrv_pwrite_sync(bs->file, header_offset, &header_le, 
> > sizeof(VHDXHeader));
> 
> This function can fail and it's a good indicator that future I/Os will
> also be in trouble.  Please propagate the error return.
> 
> > @@ -801,8 +955,7 @@ static int vhdx_open(BlockDriverState *bs, QDict 
> > *options, int flags)
> >  }
> >  
> >  if (flags & BDRV_O_RDWR) {
> > -ret = -ENOTSUP;
> > -goto fail;
> > +vhdx_update_headers(bs, s, false);
> 
> Error handling is missing.  At this point it looks like we cannot write
> to the file.  Propagating the error seems reasonable.

Re: [Qemu-devel] [PATCH v3 For-1.6 1/7] rdma: bugfix: make IPv6 support work

[Michael R. Hines]

On 08/07/2013 04:39 AM, Orit Wasserman wrote:

On 08/04/2013 05:54 AM, mrhi...@linux.vnet.ibm.com wrote:

From: "Michael R. Hines" 

RDMA does not use sockets, so we cannot use many of the socket
helper functions, but we *do* use inet_parse() which gives
RDMA all the necessary details of the connection parameters.

However, when testing with libvirt, a simple IPv6 migration test failed
because we were not using getaddrinfo() properly.

This makes IPv6 migration over RDMA work.

Signed-off-by: Michael R. Hines 
---
  migration-rdma.c |   33 +
  1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/migration-rdma.c b/migration-rdma.c
index d044830..9cf73e3 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -392,6 +392,7 @@ typedef struct RDMAContext {
  uint64_t unregistrations[RDMA_SIGNALED_SEND_MAX];
  
  GHashTable *blockmap;

+bool ipv6;
  } RDMAContext;
  
  /*

@@ -744,6 +745,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error 
**errp)
  char port_str[16];
  struct rdma_cm_event *cm_event;
  char ip[40] = "unknown";
+int af = rdma->ipv6 ? PF_INET6 : PF_INET;
  
  if (rdma->host == NULL || !strcmp(rdma->host, "")) {

  ERROR(errp, "RDMA hostname has not been set\n");
@@ -773,7 +775,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error 
**errp)
  goto err_resolve_get_addr;
  }
  
-inet_ntop(AF_INET, &((struct sockaddr_in *) res->ai_addr)->sin_addr,

+inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
  ip, sizeof ip);
  DPRINTF("%s => %s\n", rdma->host, ip);
  
@@ -2236,9 +2238,12 @@ err_rdma_source_connect:

  static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
  {
  int ret = -EINVAL, idx;
+int af = rdma->ipv6 ? PF_INET6 : PF_INET;
  struct sockaddr_in sin;
  struct rdma_cm_id *listen_id;
  char ip[40] = "unknown";
+struct addrinfo *res;
+char port_str[16];
  
  for (idx = 0; idx <= RDMA_WRID_MAX; idx++) {

  rdma->wr_data[idx].control_len = 0;
@@ -2266,27 +2271,30 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error 
**errp)
  }
  
  memset(&sin, 0, sizeof(sin));

-sin.sin_family = AF_INET;
+sin.sin_family = af;
  sin.sin_port = htons(rdma->port);
+snprintf(port_str, 16, "%d", rdma->port);
+port_str[15] = '\0';
  
  if (rdma->host && strcmp("", rdma->host)) {

-struct hostent *dest_addr;
-dest_addr = gethostbyname(rdma->host);
-if (!dest_addr) {
-ERROR(errp, "migration could not gethostbyname!\n");
-ret = -EINVAL;
+ret = getaddrinfo(rdma->host, port_str, NULL, &res);

Hi Michael,

getaddrinfo can return a list of addresses, you need to handle it.
Look at qemu-sockets.c for example.

Regards,
Orit

+if (ret < 0) {
+ERROR(errp, "could not getaddrinfo address %s\n", rdma->host);
  goto err_dest_init_bind_addr;
  }
-memcpy(&sin.sin_addr.s_addr, dest_addr->h_addr,
-dest_addr->h_length);
-inet_ntop(AF_INET, dest_addr->h_addr, ip, sizeof ip);
+
+
+inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
+ip, sizeof ip);
  } else {
-sin.sin_addr.s_addr = INADDR_ANY;
+ERROR(errp, "migration host and port not specified!\n");
+ret = -EINVAL;
+goto err_dest_init_bind_addr;
  }
  
  DPRINTF("%s => %s\n", rdma->host, ip);
  
-ret = rdma_bind_addr(listen_id, (struct sockaddr *)&sin);

+ret = rdma_bind_addr(listen_id, res->ai_addr);
  if (ret) {
  ERROR(errp, "Error: could not rdma_bind_addr!\n");
  goto err_dest_init_bind_addr;
@@ -2321,6 +2329,7 @@ static void *qemu_rdma_data_init(const char *host_port, 
Error **errp)
  if (addr != NULL) {
  rdma->port = atoi(addr->port);
  rdma->host = g_strdup(addr->host);
+rdma->ipv6 = addr->ipv6;
  } else {
  ERROR(errp, "bad RDMA migration address '%s'", host_port);
  g_free(rdma);





Acknowledged.

- Michael

Re: [Qemu-devel] [PATCH 0/3] rdma: validate remote provided RDMAControlHeader::len

[Michael R. Hines]

On 08/06/2013 10:26 PM, Isaku Yamahata wrote:

RDMAControlHeader::len is remote-provided. So validate the value before use.

Isaku Yamahata (3):
   rdma: use resp.len after validation in qemu_rdma_registration_stop
   rdma: validate RDMAControlHeader::len
   rdma: check if RDMAControlHeader::len match transferred byte

  migration-rdma.c |   44 ++--
  1 file changed, 30 insertions(+), 14 deletions(-)



Thank you. I will apply to my tree and re-send.

- Michael

Re: [Qemu-devel] [PATCH v2 4/9] block: vhdx - log support struct and defines

[Kevin Wolf]

Am 01.08.2013 um 05:23 hat Jeff Cody geschrieben:
> This adds some magic number defines, and internal structure
> definitions for VHDX log replay support.
> 
> Signed-off-by: Jeff Cody 
> ---
>  block/vhdx.h | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/block/vhdx.h b/block/vhdx.h
> index c8d8593..2db6615 100644
> --- a/block/vhdx.h
> +++ b/block/vhdx.h
> @@ -151,7 +151,10 @@ typedef struct QEMU_PACKED VHDXRegionTableEntry {
>  
>  
>  /*  LOG ENTRY STRUCTURES  */
> +#define VHDX_LOG_MIN_SIZE (1024*1024)

There should be spaces around the operator.

Also, VHDX_LOG_ENTRY_MIN_SIZE? I thought at first that this was the
minimum size of the whole log (or does that happen to be the same and I
just missed it in the spec?)

Kevin

Re: [Qemu-devel] [PATCH v2 5/9] block: vhdx - break endian translation functions out

[Kevin Wolf]

Am 01.08.2013 um 05:23 hat Jeff Cody geschrieben:
> This moves the endian translation functions out from the vhdx.c source,
> into a separate source file. In addition to the previously defined
> endian functions, new endian translation functions for log support are
> added as well.
> 
> Signed-off-by: Jeff Cody 

> +void vhdx_log_entry_hdr_le_import(VHDXLogEntryHeader *hdr)
> +{
> +assert(hdr != NULL);
> +
> +le32_to_cpus(&hdr->signature);
> +le32_to_cpus(&hdr->checksum);
> +le32_to_cpus(&hdr->entry_length);
> +le32_to_cpus(&hdr->tail);
> +le64_to_cpus(&hdr->sequence_number);
> +le32_to_cpus(&hdr->descriptor_count);
> +leguid_to_cpus(&hdr->log_guid);
> +le64_to_cpus(&hdr->flushed_file_offset);
> +le64_to_cpus(&hdr->last_file_offset);
> +}
> +
> +void vhdx_log_entry_hdr_le_export(VHDXLogEntryHeader *hdr)
> +{
> +assert(hdr != NULL);
> +
> +cpu_to_le32s(&hdr->signature);
> +cpu_to_le32s(&hdr->checksum);
> +cpu_to_le32s(&hdr->entry_length);
> +cpu_to_le32s(&hdr->tail);
> +cpu_to_le64s(&hdr->sequence_number);
> +cpu_to_le32s(&hdr->descriptor_count);
> +cpu_to_le64s(&hdr->flushed_file_offset);
> +cpu_to_le64s(&hdr->last_file_offset);
> +cpu_to_leguids(&hdr->log_guid);
> +}

Almost as critical as Stefan's comment: Can you keep the order of fields
consistent between import and export?

Kevin

[Qemu-devel] [PATCH v10 01/10] configure: Support configuring C++ compiler

[Tomoki Sekiyama]

Add configuration for C++ compiler in configure and Makefiles.
The C++ compiler is choosed as following:
 - ${CXX}, if it is specified.
 - ${cross_prefix}g++, if ${cross_prefix} is specified.
 - Otherwise, c++ is used.

Currently, usage of C++ language is only for access to Windows VSS
using COM+ services in qemu-guest-agent for Windows.

Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Micael Roth 
---
 configure |   13 +
 rules.mak |9 -
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 18fa608..eabd8e5 100755
--- a/configure
+++ b/configure
@@ -252,6 +252,8 @@ for opt do
   ;;
   --cc=*) CC="$optarg"
   ;;
+  --cxx=*) CXX="$optarg"
+  ;;
   --source-path=*) source_path="$optarg"
   ;;
   --cpu=*) cpu="$optarg"
@@ -282,6 +284,12 @@ else
   cc="${CC-${cross_prefix}gcc}"
 fi
 
+if test -z "${CXX}${cross_prefix}"; then
+  cxx="c++"
+else
+  cxx="${CXX-${cross_prefix}g++}"
+fi
+
 ar="${AR-${cross_prefix}ar}"
 as="${AS-${cross_prefix}as}"
 cpp="${CPP-$cc -E}"
@@ -622,6 +630,8 @@ for opt do
   ;;
   --host-cc=*) host_cc="$optarg"
   ;;
+  --cxx=*)
+  ;;
   --objcc=*) objcc="$optarg"
   ;;
   --make=*) make="$optarg"
@@ -1023,6 +1033,7 @@ echo "  --cross-prefix=PREFIXuse PREFIX for compile 
tools [$cross_prefix]"
 echo "  --cc=CC  use C compiler CC [$cc]"
 echo "  --host-cc=CC use C compiler CC [$host_cc] for code run at"
 echo "   build time"
+echo "  --cxx=CXXuse C++ compiler CXX [$cxx]"
 echo "  --objcc=OBJCCuse Objective-C compiler OBJCC [$objcc]"
 echo "  --extra-cflags=CFLAGSappend extra C compiler flags QEMU_CFLAGS"
 echo "  --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS"
@@ -3522,6 +3533,7 @@ fi
 echo "Source path   $source_path"
 echo "C compiler$cc"
 echo "Host C compiler   $host_cc"
+echo "C++ compiler  $cxx"
 echo "Objective-C compiler $objcc"
 echo "CFLAGS$CFLAGS"
 echo "QEMU_CFLAGS   $QEMU_CFLAGS"
@@ -4103,6 +4115,7 @@ echo "PYTHON=$python" >> $config_host_mak
 echo "CC=$cc" >> $config_host_mak
 echo "CC_I386=$cc_i386" >> $config_host_mak
 echo "HOST_CC=$host_cc" >> $config_host_mak
+echo "CXX=$cxx" >> $config_host_mak
 echo "OBJCC=$objcc" >> $config_host_mak
 echo "AR=$ar" >> $config_host_mak
 echo "AS=$as" >> $config_host_mak
diff --git a/rules.mak b/rules.mak
index 4499745..abc2e84 100644
--- a/rules.mak
+++ b/rules.mak
@@ -8,9 +8,13 @@ MAKEFLAGS += -rR
 %.d:
 %.h:
 %.c:
+%.cpp:
 %.m:
 %.mak:
 
+# Flags for C++ compilation
+QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes 
-Wmissing-prototypes -Wnested-externs -Wold-style-declaration 
-Wold-style-definition -Wredundant-decls, $(QEMU_CFLAGS))
+
 # Flags for dependency generation
 QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
 
@@ -50,6 +54,9 @@ endif
 %.o: %.asm
$(call quiet-command,$(AS) $(ASFLAGS) -o $@ $<,"  AS
$(TARGET_DIR)$@")
 
+%.o: %.cpp
+   $(call quiet-command,$(CXX) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) 
$(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CXX   $(TARGET_DIR)$@")
+
 %.o: %.m
$(call quiet-command,$(OBJCC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) 
$(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  OBJC  $(TARGET_DIR)$@")
 
@@ -70,7 +77,7 @@ quiet-command = $(if $(V),$1,$(if $(2),@echo $2 && $1, @$1))
 cc-option = $(if $(shell $(CC) $1 $2 -S -o /dev/null -xc /dev/null \
   >/dev/null 2>&1 && echo OK), $2, $3)
 
-VPATH_SUFFIXES = %.c %.h %.S %.m %.mak %.texi %.sh %.rc
+VPATH_SUFFIXES = %.c %.h %.S %.cpp %.m %.mak %.texi %.sh %.rc
 set-vpath = $(if $1,$(foreach PATTERN,$(VPATH_SUFFIXES),$(eval vpath 
$(PATTERN) $1)))
 
 # find-in-path

[Qemu-devel] [PATCH v10 03/10] checkpatch.pl: Check .cpp files

[Tomoki Sekiyama]

Enable checkpatch.pl to apply the same checks as C source files for
C++ files with .cpp extensions. It also adds some exceptions for C++
sources to suppress errors for:
  - <> used in C++ template arguments (e.g. template )
  - :: used to represent namespaces   (e.g. SomeClass::method())
  - : used in class declaration   (e.g. class T : public Super)
  - ~ used in destructor method name  (e.g. T::~T())
  - spacing around 'catch'(e.g. catch (...))

Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Michael Roth 
---
 scripts/checkpatch.pl |   34 --
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ec0aa4c..9d46e5a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1363,7 +1363,7 @@ sub process {
 # Check for incorrect file permissions
if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
my $permhere = $here . "FILE: $realfile\n";
-   if ($realfile =~ 
/(Makefile|Kconfig|\.c|\.h|\.S|\.tmpl)$/) {
+   if ($realfile =~ 
/(Makefile|Kconfig|\.c|\.cpp|\.h|\.S|\.tmpl)$/) {
ERROR("do not set execute permissions for 
source files\n" . $permhere);
}
}
@@ -1460,7 +1460,7 @@ sub process {
}
 
 # check we are in a valid source file if not then ignore this hunk
-   next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
+   next if ($realfile !~ /\.(h|c|cpp|s|S|pl|sh)$/);
 
 #80 column limit
if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ &&
@@ -1495,7 +1495,7 @@ sub process {
}
 
 # check we are in a valid source file C or perl if not then ignore this hunk
-   next if ($realfile !~ /\.(h|c|pl)$/);
+   next if ($realfile !~ /\.(h|c|cpp|pl)$/);
 
 # in QEMU, no tabs are allowed
if ($rawline =~ /^\+.*\t/) {
@@ -1505,7 +1505,7 @@ sub process {
}
 
 # check we are in a valid C source file if not then ignore this hunk
-   next if ($realfile !~ /\.(h|c)$/);
+   next if ($realfile !~ /\.(h|c|cpp)$/);
 
 # check for RCS/CVS revision markers
if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
@@ -1969,6 +1969,9 @@ sub process {
asm|__asm__)$/x)
{
 
+   # Ignore 'catch (...)' in C++
+   } elsif ($name =~ /^catch$/ && $realfile =~ 
/(\.cpp|\.h)$/) {
+
# cpp #define statements have non-optional spaces, ie
# if there is a space between the name and the open
# parenthesis it is simply not a parameter group.
@@ -1992,7 +1995,7 @@ sub process {
\+=|-=|\*=|\/=|%=|\^=|\|=|&=|
=>|->|<<|>>|<|>|=|!|~|
&&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%|
-   \?|:
+   \?|::|:
}x;
my @elements = split(/($ops|;)/, $opline);
my $off = 0;
@@ -2062,6 +2065,10 @@ sub process {
# // is a comment
} elsif ($op eq '//') {
 
+   # Ignore : used in class declaration in C++
+   } elsif ($opv eq ':B' && $ctx =~ /Wx[WE]/ &&
+$line =~ /class/ && $realfile 
=~ /(\.cpp|\.h)$/) {
+
# No spaces for:
#   ->
#   :   when part of a bitfield
@@ -2088,7 +2095,10 @@ sub process {
} elsif ($op eq '!' || $op eq '~' ||
 $opv eq '*U' || $opv eq '-U' ||
 $opv eq '&U' || $opv eq '&&U') {
-   if ($ctx !~ /[WEBC]x./ && $ca !~ 
/(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
+   if ($op eq '~' && $ca =~ /::$/ && 
$realfile =~ /(\.cpp|\.h)$/) {
+   # '~' used as a name of 
Destructor
+
+   } elsif ($ctx !~ /[WEBC]x./ && $ca !~ 
/(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
ERROR("space required before 
that '$op' $at\n" . $hereptr);
}
if ($op eq '*' && $cc 
=~/\s*$Modifier\b/) {
@@ -2135,6 +2145,18 @@ sub process {
} elsif ($ctx !~ /[EWC]x[CWE]/) {
my $ok = 0;
 
+   if ($realfile =~ /\.cpp|\.h$/) {
+   #

[Qemu-devel] [PATCH v10 08/10] qemu-ga: Call Windows VSS requester in fsfreeze command handler

[Tomoki Sekiyama]

Support guest-fsfreeze-freeze and guest-fsfreeze-thaw commands for Windows
guests. When fsfreeze command is issued, it calls the VSS requester to
freeze filesystems and applications. On thaw command, it again tells the VSS
requester to thaw them.

This also adds calling of initialize functions for the VSS requester.

Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Michael Roth 
---
 qga/Makefile.objs|1 
 qga/commands-win32.c |   82 ++---
 qga/vss-win32.c  |  141 ++
 qga/vss-win32.h  |   24 +
 4 files changed, 240 insertions(+), 8 deletions(-)
 create mode 100644 qga/vss-win32.c
 create mode 100644 qga/vss-win32.h

diff --git a/qga/Makefile.objs b/qga/Makefile.objs
index c4bd151..1c5986c 100644
--- a/qga/Makefile.objs
+++ b/qga/Makefile.objs
@@ -1,6 +1,7 @@
 qga-obj-y = commands.o guest-agent-command-state.o main.o
 qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
 qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o
+qga-obj-$(CONFIG_WIN32) += vss-win32.o
 qga-obj-y += qapi-generated/qga-qapi-types.o qapi-generated/qga-qapi-visit.o
 qga-obj-y += qapi-generated/qga-qmp-marshal.o
 
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 24e4ad0..7a37f5c 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include "qga/guest-agent-core.h"
+#include "qga/vss-win32.h"
 #include "qga-qmp-commands.h"
 #include "qapi/qmp/qerror.h"
 
@@ -156,27 +157,89 @@ void qmp_guest_file_flush(int64_t handle, Error **err)
  */
 GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
 {
-error_set(err, QERR_UNSUPPORTED);
-return 0;
+if (!vss_initialized()) {
+error_set(err, QERR_UNSUPPORTED);
+return 0;
+}
+
+if (ga_is_frozen(ga_state)) {
+return GUEST_FSFREEZE_STATUS_FROZEN;
+}
+
+return GUEST_FSFREEZE_STATUS_THAWED;
 }
 
 /*
- * Walk list of mounted file systems in the guest, and freeze the ones which
- * are real local file systems.
+ * Freeze local file systems using Volume Shadow-copy Service.
+ * The frozen state is limited for up to 10 seconds by VSS.
  */
 int64_t qmp_guest_fsfreeze_freeze(Error **err)
 {
-error_set(err, QERR_UNSUPPORTED);
+int i;
+Error *local_err = NULL;
+
+if (!vss_initialized()) {
+error_set(err, QERR_UNSUPPORTED);
+return 0;
+}
+
+slog("guest-fsfreeze called");
+
+/* cannot risk guest agent blocking itself on a write in this state */
+ga_set_frozen(ga_state);
+
+qga_vss_fsfreeze(&i, err, true);
+if (error_is_set(err)) {
+goto error;
+}
+
+return i;
+
+error:
+qmp_guest_fsfreeze_thaw(&local_err);
+if (error_is_set(&local_err)) {
+g_debug("cleanup thaw: %s", error_get_pretty(local_err));
+error_free(local_err);
+}
 return 0;
 }
 
 /*
- * Walk list of frozen file systems in the guest, and thaw them.
+ * Thaw local file systems using Volume Shadow-copy Service.
  */
 int64_t qmp_guest_fsfreeze_thaw(Error **err)
 {
-error_set(err, QERR_UNSUPPORTED);
-return 0;
+int i;
+
+if (!vss_initialized()) {
+error_set(err, QERR_UNSUPPORTED);
+return 0;
+}
+
+qga_vss_fsfreeze(&i, err, false);
+
+ga_unset_frozen(ga_state);
+return i;
+}
+
+static void guest_fsfreeze_cleanup(void)
+{
+Error *err = NULL;
+
+if (!vss_initialized()) {
+return;
+}
+
+if (ga_is_frozen(ga_state) == GUEST_FSFREEZE_STATUS_FROZEN) {
+qmp_guest_fsfreeze_thaw(&err);
+if (err) {
+slog("failed to clean up frozen filesystems: %s",
+ error_get_pretty(err));
+error_free(err);
+}
+}
+
+vss_deinit(true);
 }
 
 /*
@@ -354,4 +417,7 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList 
*vcpus, Error **errp)
 /* register init/cleanup routines for stateful command groups */
 void ga_command_state_init(GAState *s, GACommandState *cs)
 {
+if (vss_init(true)) {
+ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
+}
 }
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
new file mode 100644
index 000..89c0f3b
--- /dev/null
+++ b/qga/vss-win32.c
@@ -0,0 +1,141 @@
+/*
+ * QEMU Guest Agent VSS utility functions
+ *
+ * Copyright Hitachi Data Systems Corp. 2013
+ *
+ * Authors:
+ *  Tomoki Sekiyama   
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include 
+#include "qga/guest-agent-core.h"
+#include "qga/vss-win32.h"
+#include "qga/vss-win32/requester.h"
+
+#define QGA_VSS_DLL "qga-vss.dll"
+
+static HMODULE provider_lib;
+
+/* Call a function in qga-vss.dll with the specified name */
+static HRESULT call_vss_provider_func(const char *func_name)
+{
+FARPROC WINAPI func;
+
+g_assert(provider_lib);
+
+func = GetProcAddress(provider_lib, func_name)

[Qemu-devel] [PATCH v10 05/10] qemu-ga: Add configure options to specify path to Windows/VSS SDK

[Tomoki Sekiyama]

To enable VSS support in qemu-ga for Windows, header files included in
VSS SDK are required.
The VSS support is enabled by the configure option like below:
  ./configure --with-vss-sdk="/path/to/VSS SDK"

If the path is omitted, it tries to search the headers from default paths
and VSS support is enabled only if the SDK is found.
VSS support is disabled if --without-vss-sdk or --with-vss-sdk=no is
specified.

VSS SDK is available from:
  http://www.microsoft.com/en-us/download/details.aspx?id=23490

To cross-compile using mingw, you need to setup the SDK on Windows
environments to extract headers. You can also extract the SDK headers on
POSIX environments using scripts/extract-vss-headers and msitools.

In addition, --with-win-sdk="/path/to/Windows SDK" option is also added to
specify path to Windows SDK, which may be used for native-compile of .tlb
file of qemu-ga VSS provider. However, this is usually unnecessary because
pre-compiled .tlb file is included.

Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Michael Roth 
---
 .gitignore |1 +
 Makefile   |1 +
 configure  |   78 
 3 files changed, 80 insertions(+)

diff --git a/.gitignore b/.gitignore
index 0fe114d..02d15f0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -79,6 +79,7 @@ fsdev/virtfs-proxy-helper.pod
 *.la
 *.pc
 .libs
+.sdk
 *.swp
 *.orig
 .pc
diff --git a/Makefile b/Makefile
index 29f1043..0caf4fe 100644
--- a/Makefile
+++ b/Makefile
@@ -273,6 +273,7 @@ distclean: clean
for d in $(TARGET_DIRS); do \
rm -rf $$d || exit 1 ; \
 done
+   rm -Rf .sdk
if test -f pixman/config.log; then make -C pixman distclean; fi
if test -f dtc/version_gen.h; then make $(DTC_MAKE_ARGS) clean; fi
 
diff --git a/configure b/configure
index eabd8e5..137cfee 100755
--- a/configure
+++ b/configure
@@ -232,6 +232,9 @@ usb_redir=""
 glx=""
 zlib="yes"
 guest_agent=""
+guest_agent_with_vss="no"
+vss_win32_sdk=""
+win_sdk="no"
 want_tools="yes"
 libiscsi=""
 coroutine=""
@@ -923,6 +926,18 @@ for opt do
   ;;
   --disable-guest-agent) guest_agent="no"
   ;;
+  --with-vss-sdk) vss_win32_sdk=""
+  ;;
+  --with-vss-sdk=*) vss_win32_sdk="$optarg"
+  ;;
+  --without-vss-sdk) vss_win32_sdk="no"
+  ;;
+  --with-win-sdk) win_sdk=""
+  ;;
+  --with-win-sdk=*) win_sdk="$optarg"
+  ;;
+  --without-win-sdk) win_sdk="no"
+  ;;
   --enable-tools) want_tools="yes"
   ;;
   --disable-tools) want_tools="no"
@@ -1159,6 +1174,8 @@ echo "  --disable-usb-redir  disable usb network 
redirection support"
 echo "  --enable-usb-redir   enable usb network redirection support"
 echo "  --disable-guest-agentdisable building of the QEMU Guest Agent"
 echo "  --enable-guest-agent enable building of the QEMU Guest Agent"
+echo "  --with-vss-sdk=SDK-path  enable Windows VSS support in QEMU Guest 
Agent"
+echo "  --with-win-sdk=SDK-path  path to Windows Platform SDK (to build VSS 
.tlb)"
 echo "  --disable-seccompdisable seccomp support"
 echo "  --enable-seccomp enables seccomp support"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
@@ -3093,6 +3110,61 @@ if test "$usb_redir" != "no" ; then
 fi
 
 ##
+# check if we have VSS SDK headers for win
+
+if test "$mingw32" = "yes" -a "$guest_agent" != "no" -a "$vss_win32_sdk" != 
"no" ; then
+  case "$vss_win32_sdk" in
+"")   vss_win32_include="-I$source_path" ;;
+*\ *) # The SDK is installed in "Program Files" by default, but we cannot
+  # handle path with spaces. So we symlink the headers into ".sdk/vss".
+  vss_win32_include="-I$source_path/.sdk/vss"
+ symlink "$vss_win32_sdk/inc" "$source_path/.sdk/vss/inc"
+ ;;
+*)vss_win32_include="-I$vss_win32_sdk"
+  esac
+  cat > $TMPC << EOF
+#define __MIDL_user_allocate_free_DEFINED__
+#include 
+int main(void) { return VSS_CTX_BACKUP; }
+EOF
+  if compile_prog "$vss_win32_include" "" ; then
+guest_agent_with_vss="yes"
+QEMU_CFLAGS="$QEMU_CFLAGS $vss_win32_include"
+libs_qga="-lole32 -loleaut32 -lshlwapi -luuid -lstdc++ 
-Wl,--enable-stdcall-fixup $libs_qga"
+  else
+if test "$vss_win32_sdk" != "" ; then
+  echo "ERROR: Please download and install Microsoft VSS SDK:"
+  echo "ERROR:   
http://www.microsoft.com/en-us/download/details.aspx?id=23490";
+  echo "ERROR: On POSIX-systems, you can extract the SDK headers by:"
+  echo "ERROR:   scripts/extract-vsssdk-headers setup.exe"
+  echo "ERROR: The headers are extracted in the directory \`inc'."
+  feature_not_found "VSS support"
+fi
+guest_agent_with_vss="no"
+  fi
+fi
+
+##
+# lookup Windows platform SDK (if not specified)
+# The SDK is needed only to build .tlb (type library) file of guest agent
+# VSS provider from the source. It is usually unnecessary because the
+# pre-compiled .tlb file is included.
+
+i

[Qemu-devel] [PATCH v10 02/10] Add c++ keywords to QAPI helper script

[Tomoki Sekiyama]

Add c++ keywords to avoid errors in compiling with c++ compiler.
This also renames class member of PciDeviceInfo to q_class.

Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Michael Roth 
---
 hmp.c   |2 +-
 hw/pci/pci.c|2 +-
 scripts/qapi.py |   12 +++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hmp.c b/hmp.c
index c45514b..79931d2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -528,7 +528,7 @@ static void hmp_info_pci_device(Monitor *mon, const 
PciDeviceInfo *dev)
 if (dev->class_info.has_desc) {
 monitor_printf(mon, "%s", dev->class_info.desc);
 } else {
-monitor_printf(mon, "Class %04" PRId64, dev->class_info.class);
+monitor_printf(mon, "Class %04" PRId64, dev->class_info.q_class);
 }
 
 monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 4c004f5..25d08eb 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1466,7 +1466,7 @@ static PciDeviceInfo *qmp_query_pci_device(PCIDevice 
*dev, PCIBus *bus,
 info->function = PCI_FUNC(dev->devfn);
 
 class = pci_get_word(dev->config + PCI_CLASS_DEVICE);
-info->class_info.class = class;
+info->class_info.q_class = class;
 desc = get_class_desc(class);
 if (desc->desc) {
 info->class_info.has_desc = true;
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0ebea94..d0a92df 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -236,9 +236,19 @@ def c_var(name, protect=True):
 # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
 # excluding _.*
 gcc_words = set(['asm', 'typeof'])
+# C++ ISO/IEC 14882:2003 2.11
+cpp_words = set(['bool', 'catch', 'class', 'const_cast', 'delete',
+ 'dynamic_cast', 'explicit', 'false', 'friend', 'mutable',
+ 'namespace', 'new', 'operator', 'private', 'protected',
+ 'public', 'reinterpret_cast', 'static_cast', 'template',
+ 'this', 'throw', 'true', 'try', 'typeid', 'typename',
+ 'using', 'virtual', 'wchar_t',
+ # alternative representations
+ 'and', 'and_eq', 'bitand', 'bitor', 'compl', 'not',
+ 'not_eq', 'or', 'or_eq', 'xor', 'xor_eq'])
 # namespace pollution:
 polluted_words = set(['unix'])
-if protect and (name in c89_words | c99_words | c11_words | gcc_words | 
polluted_words):
+if protect and (name in c89_words | c99_words | c11_words | gcc_words | 
cpp_words | polluted_words):
 return "q_" + name
 return name.replace('-', '_').lstrip("*")
 

[Qemu-devel] [PATCH v10 00/10] qemu-ga: fsfreeze on Windows using VSS

[Tomoki Sekiyama]

Hi,

I rebased the patch series to add fsfreeze for Windows qemu-ga.

changes from v9:
  - Fix conflict with commit e8ef31a3518c "qemu-ga: build it even if !system"

changes from v8:
 - Add hEventTimeout to improve timeout error message (patch 07, see below)
 - Build qga-vss.tlb if configure'd --with-win-sdk (patch 05, 07)
 - Use "qga-vss-dll-obj-$(CONFIG_QGA_VSS)" in Makefile.objs (patch 07)
 - Fix typo in QGAVSSRequesterFunc (patch 07, 08)

v9: http://lists.nongnu.org/archive/html/qemu-devel/2013-07/msg06071.html

* Description
  In Windows, VSS (Volume Shadow Copy Service) provides a facility to
  quiesce filesystems and applications before disk snapshots are taken.
  This patch series implements "fsfreeze" command of qemu-ga using VSS.


* How to build & run qemu-ga with VSS support

 - Download Microsoft VSS SDK from:
   http://www.microsoft.com/en-us/download/details.aspx?id=23490

 - Setup the SDK
   scripts/extract-vsssdk-headers setup.exe (on POSIX-systems)

 - Specify installed SDK directory to configure option as:
   ./configure --cross-prefix=i686-w64-mingw32- \
   --with-vss-sdk="path/to/VSS SDK" \
   --without-systems --without-tools

 - make qemu-ga.exe qga/vss-win32/qga-vss.{dll,tlb}

 - Install qemu-ga.exe, qga/vss-win32/qga-vss.{dll,tlb}, and
   the other required mingw libraries into the same directory in guests

 - Run `qemu-ga.exe -s install' and `net start qemu-ga' in the guests

Any feedback are appreciated.

---
Tomoki Sekiyama (10):
  configure: Support configuring C++ compiler
  Add c++ keywords to QAPI helper script
  checkpatch.pl: Check .cpp files
  Add a script to extract VSS SDK headers on POSIX system
  qemu-ga: Add configure options to specify path to Windows/VSS SDK
  error: Add error_set_win32 and error_setg_win32
  qemu-ga: Add Windows VSS provider and requester as DLL
  qemu-ga: Call Windows VSS requester in fsfreeze command handler
  qemu-ga: Install Windows VSS provider on `qemu-ga -s install'
  QMP/qemu-ga-client: Make timeout longer for guest-fsfreeze-freeze command


 .gitignore |1 
 Makefile   |3 
 Makefile.objs  |2 
 QMP/qemu-ga-client |4 
 configure  |   96 +++
 hmp.c  |2 
 hw/pci/pci.c   |2 
 include/qapi/error.h   |   13 +
 qga/Makefile.objs  |3 
 qga/commands-win32.c   |   82 ++
 qga/main.c |   10 +
 qga/vss-win32.c|  166 +
 qga/vss-win32.h|   27 ++
 qga/vss-win32/Makefile.objs|   23 ++
 qga/vss-win32/install.cpp  |  458 +++
 qga/vss-win32/provider.cpp |  523 
 qga/vss-win32/qga-vss.def  |   13 +
 qga/vss-win32/qga-vss.idl  |   20 ++
 qga/vss-win32/qga-vss.tlb  |  Bin
 qga/vss-win32/requester.cpp|  507 +++
 qga/vss-win32/requester.h  |   42 +++
 qga/vss-win32/vss-common.h |  129 ++
 rules.mak  |9 +
 scripts/checkpatch.pl  |   34 ++-
 scripts/extract-vsssdk-headers |   35 +++
 scripts/qapi.py|   12 +
 util/error.c   |   35 +++
 27 files changed, 2229 insertions(+), 22 deletions(-)
 create mode 100644 qga/vss-win32.c
 create mode 100644 qga/vss-win32.h
 create mode 100644 qga/vss-win32/Makefile.objs
 create mode 100644 qga/vss-win32/install.cpp
 create mode 100644 qga/vss-win32/provider.cpp
 create mode 100644 qga/vss-win32/qga-vss.def
 create mode 100644 qga/vss-win32/qga-vss.idl
 create mode 100644 qga/vss-win32/qga-vss.tlb
 create mode 100644 qga/vss-win32/requester.cpp
 create mode 100644 qga/vss-win32/requester.h
 create mode 100644 qga/vss-win32/vss-common.h
 create mode 100755 scripts/extract-vsssdk-headers

[Qemu-devel] [PATCH v10 06/10] error: Add error_set_win32 and error_setg_win32

[Tomoki Sekiyama]

These functions help maintaining homogeneous formatting of error messages
with Windows error code and description (generated by
g_win32_error_message()).

Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Michael Roth 
---
 include/qapi/error.h |   13 +
 util/error.c |   35 +++
 2 files changed, 48 insertions(+)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index ffd1cea..7d4c696 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -36,6 +36,15 @@ void error_set(Error **err, ErrorClass err_class, const char 
*fmt, ...) GCC_FMT_
  */
 void error_set_errno(Error **err, int os_error, ErrorClass err_class, const 
char *fmt, ...) GCC_FMT_ATTR(4, 5);
 
+#ifdef _WIN32
+/**
+ * Set an indirect pointer to an error given a ErrorClass value and a
+ * printf-style human message, followed by a g_win32_error_message() string if
+ * @win32_err is not zero.
+ */
+void error_set_win32(Error **err, int win32_err, ErrorClass err_class, const 
char *fmt, ...) GCC_FMT_ATTR(4, 5);
+#endif
+
 /**
  * Same as error_set(), but sets a generic error
  */
@@ -43,6 +52,10 @@ void error_set_errno(Error **err, int os_error, ErrorClass 
err_class, const char
 error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
 #define error_setg_errno(err, os_error, fmt, ...) \
 error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## 
__VA_ARGS__)
+#ifdef _WIN32
+#define error_setg_win32(err, win32_err, fmt, ...) \
+error_set_win32(err, win32_err, ERROR_CLASS_GENERIC_ERROR, fmt, ## 
__VA_ARGS__)
+#endif
 
 /**
  * Helper for open() errors
diff --git a/util/error.c b/util/error.c
index 53b0435..ec0faa6 100644
--- a/util/error.c
+++ b/util/error.c
@@ -76,6 +76,41 @@ void error_setg_file_open(Error **errp, int os_errno, const 
char *filename)
 error_setg_errno(errp, os_errno, "Could not open '%s'", filename);
 }
 
+#ifdef _WIN32
+
+void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
+ const char *fmt, ...)
+{
+Error *err;
+char *msg1;
+va_list ap;
+
+if (errp == NULL) {
+return;
+}
+assert(*errp == NULL);
+
+err = g_malloc0(sizeof(*err));
+
+va_start(ap, fmt);
+msg1 = g_strdup_vprintf(fmt, ap);
+if (win32_err != 0) {
+char *msg2 = g_win32_error_message(win32_err);
+err->msg = g_strdup_printf("%s: %s (error: %x)", msg1, msg2,
+   (unsigned)win32_err);
+g_free(msg2);
+g_free(msg1);
+} else {
+err->msg = msg1;
+}
+va_end(ap);
+err->err_class = err_class;
+
+*errp = err;
+}
+
+#endif
+
 Error *error_copy(const Error *err)
 {
 Error *err_new;

[Qemu-devel] [PATCH v10 04/10] Add a script to extract VSS SDK headers on POSIX system

[Tomoki Sekiyama]

VSS SDK(*) setup.exe is only runnable on Windows. This adds a script
to extract VSS SDK headers on POSIX-systems using msitools.

  * http://www.microsoft.com/en-us/download/details.aspx?id=23490

From: Paolo Bonzini 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Michael Roth 
---
 scripts/extract-vsssdk-headers |   35 +++
 1 file changed, 35 insertions(+)
 create mode 100755 scripts/extract-vsssdk-headers

diff --git a/scripts/extract-vsssdk-headers b/scripts/extract-vsssdk-headers
new file mode 100755
index 000..9e38510
--- /dev/null
+++ b/scripts/extract-vsssdk-headers
@@ -0,0 +1,35 @@
+#! /bin/bash
+
+# extract-vsssdk-headers
+# Author: Paolo Bonzini 
+
+set -e
+if test $# != 1 || ! test -f "$1"; then
+  echo 'Usage: extract-vsssdk-headers /path/to/setup.exe' >&2
+  exit 1
+fi
+
+if ! command -v msiextract > /dev/null; then
+  echo 'msiextract not found. Please install msitools.' >&2
+  exit 1
+fi
+
+if test -e inc; then
+  echo '"inc" already exists.' >&2
+  exit 1
+fi
+
+# Extract .MSI file in the .exe, looking for the OLE compound
+# document signature.  Extra data at the end does not matter.
+export LC_ALL=C
+MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1'
+offset=$(grep -abom1 "$MAGIC" "$1" | sed -n 's/:/\n/; P')
+tmpdir=$(mktemp -d)
+trap 'rm -fr -- "$tmpdir" vsssdk.msi' EXIT HUP INT QUIT ALRM TERM
+tail -c +$(($offset+1)) -- "$1" > vsssdk.msi
+
+# Now extract the files.
+msiextract -C $tmpdir vsssdk.msi
+mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc
+echo 'Extracted SDK headers into "inc" directory.'
+exit 0

[Qemu-devel] [PATCH v10 10/10] QMP/qemu-ga-client: Make timeout longer for guest-fsfreeze-freeze command

[Tomoki Sekiyama]

guest-fsfreeze-freeze command can take longer than 3 seconds when heavy
disk I/O is running. To avoid unexpected timeout, this changes the timeout
to 60 seconds (timeout of pre-commit phase of VSS).

Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Michael Roth 
---
 QMP/qemu-ga-client |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/QMP/qemu-ga-client b/QMP/qemu-ga-client
index 46676c3..b5f7e7c 100755
--- a/QMP/qemu-ga-client
+++ b/QMP/qemu-ga-client
@@ -267,7 +267,9 @@ def main(address, cmd, args):
 print('Hint: qemu is not running?')
 sys.exit(1)
 
-if cmd != 'ping':
+if cmd == 'fsfreeze' and args[0] == 'freeze':
+client.sync(60)
+elif cmd != 'ping':
 client.sync()
 
 globals()['_cmd_' + cmd](client, args)

[Qemu-devel] [PATCH v10 09/10] qemu-ga: Install Windows VSS provider on `qemu-ga -s install'

[Tomoki Sekiyama]

Register QGA VSS provider library into Windows when qemu-ga is installed as
Windows service ('-s install' option). It is deregistered when the service
is uninstalled ('-s uninstall' option).

Signed-off-by: Tomoki Sekiyama 
Reviewed-by: Michael Roth 
---
 qga/main.c  |   10 +-
 qga/vss-win32.c |   25 +
 qga/vss-win32.h |3 +++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index 0e04e73..6c746c8 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -34,6 +34,7 @@
 #include "qemu/bswap.h"
 #ifdef _WIN32
 #include "qga/service-win32.h"
+#include "qga/vss-win32.h"
 #include 
 #endif
 #ifdef __linux__
@@ -1031,8 +1032,15 @@ int main(int argc, char **argv)
 fixed_state_dir = (state_dir == dfl_pathnames.state_dir) ?
   NULL :
   state_dir;
-return ga_install_service(path, log_filepath, fixed_state_dir);
+if (ga_install_vss_provider()) {
+return EXIT_FAILURE;
+}
+if (ga_install_service(path, log_filepath, fixed_state_dir)) {
+return EXIT_FAILURE;
+}
+return 0;
 } else if (strcmp(service, "uninstall") == 0) {
+ga_uninstall_vss_provider();
 return ga_uninstall_service();
 } else {
 printf("Unknown service command.\n");
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index 89c0f3b..24c4288 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -119,6 +119,31 @@ bool vss_initialized(void)
 return !!provider_lib;
 }
 
+int ga_install_vss_provider(void)
+{
+HRESULT hr;
+
+if (!vss_init(false)) {
+fprintf(stderr, "Installation of VSS provider is skipped. "
+"fsfreeze will be disabled.\n");
+return 0;
+}
+hr = call_vss_provider_func("COMRegister");
+vss_deinit(false);
+
+return SUCCEEDED(hr) ? 0 : EXIT_FAILURE;
+}
+
+void ga_uninstall_vss_provider(void)
+{
+if (!vss_init(false)) {
+fprintf(stderr, "Removal of VSS provider is skipped.\n");
+return;
+}
+call_vss_provider_func("COMUnregister");
+vss_deinit(false);
+}
+
 /* Call VSS requester and freeze/thaw filesystems and applications */
 void qga_vss_fsfreeze(int *nr_volume, Error **err, bool freeze)
 {
diff --git a/qga/vss-win32.h b/qga/vss-win32.h
index eac669c..db8fbe5 100644
--- a/qga/vss-win32.h
+++ b/qga/vss-win32.h
@@ -19,6 +19,9 @@ bool vss_init(bool init_requester);
 void vss_deinit(bool deinit_requester);
 bool vss_initialized(void);
 
+int ga_install_vss_provider(void);
+void ga_uninstall_vss_provider(void);
+
 void qga_vss_fsfreeze(int *nr_volume, Error **err, bool freeze);
 
 #endif

Re: [Qemu-devel] [RFC] [PATCHv6 00/16] aio / timers: Add AioContext timers and use ppoll

[Paolo Bonzini]

> For anyone wishing to review this series, here's a diagram showing the
> new relationships and a summary of this series:
> 
> http://vmsplice.net/~stefan/timerlist.jpg
> 
> TimerList is the list of QEMUTimers which are pending on a QEMUClock
> clock source.

I just started reviewing this and the diagram may be different in
v7, anyway: why do you need a default TimerListGroup?  I would have
thought it is enough to use the default AioContext's own TLG.

Paolo

> Previously QEMUClock contained this list inline but the goal is to let
> AioContexts have their own independent timer lists.  This makes timers
> much more thread-friendly since we don't work on global lists and
> callbacks are dispatched in the same thread's AioContext where they were
> created.
> 
> TimerListGroup holds a timerlist for each clock source ("rt", "vm", and
> "host").  The main loop has the default TimerListGroup.  Each AioContext
> has its own TimerListGroup.
> 
> The other major change is that QEMU now uses the event loop's poll()
> timeout value instead of a separate OS-specific timer mechanism (aka the
> "alarm timer").
> 
> Stefan
> 

Re: [Qemu-devel] [RFC] [PATCHv7 01/22] aio / timers: Add qemu_clock_free and expose qemu_clock_new and clock types

[Paolo Bonzini]

On Aug 07 2013, Alex Bligh wrote:
> 
> --On 7 August 2013 13:31:07 +0200 Stefan Hajnoczi
>  wrote:
> 
> > Regarding doc comments: if the function is self-documenting then the doc
> > comment might not add much.  I think in those cases it's okay to omit
> > details about arguments or return values if they are clear from the
> > function prototype.
> 
> Given I've done them for every function, I hope they don't cause any
> active harm. I was concerned that with an API change some things that
> one might have thought were obvious are in fact not obvious (e.g.
> timer_del vs. timer_free).

No, they definitely do not harm.

Paolo

[Qemu-devel] [PATCH for-1.6 3/4] rdma: check if RDMAControlHeader::len match transferred byte

[mrhines]

From: Isaku Yamahata 

RDMAControlHeader::len is provided from remote, so check if the value
match the actual transferred byte_len.

Reviewed-by: Orit Wasserman 
Reviewed-by: Michael R. Hines 
Signed-off-by: Isaku Yamahata 
Signed-off-by: Michael R. Hines 
---
 migration-rdma.c |   32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/migration-rdma.c b/migration-rdma.c
index ebe1f55..30e08cd 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -1214,7 +1214,8 @@ static void qemu_rdma_signal_unregister(RDMAContext 
*rdma, uint64_t index,
  * (of any kind) has completed.
  * Return the work request ID that completed.
  */
-static uint64_t qemu_rdma_poll(RDMAContext *rdma, uint64_t *wr_id_out)
+static uint64_t qemu_rdma_poll(RDMAContext *rdma, uint64_t *wr_id_out,
+   uint32_t *byte_len)
 {
 int ret;
 struct ibv_wc wc;
@@ -1285,6 +1286,9 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma, 
uint64_t *wr_id_out)
 }
 
 *wr_id_out = wc.wr_id;
+if (byte_len) {
+*byte_len = wc.byte_len;
+}
 
 return  0;
 }
@@ -1302,7 +1306,8 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma, 
uint64_t *wr_id_out)
  * completions only need to be recorded, but do not actually
  * need further processing.
  */
-static int qemu_rdma_block_for_wrid(RDMAContext *rdma, int wrid_requested)
+static int qemu_rdma_block_for_wrid(RDMAContext *rdma, int wrid_requested,
+uint32_t *byte_len)
 {
 int num_cq_events = 0, ret = 0;
 struct ibv_cq *cq;
@@ -1314,7 +1319,7 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma, 
int wrid_requested)
 }
 /* poll cq first */
 while (wr_id != wrid_requested) {
-ret = qemu_rdma_poll(rdma, &wr_id_in);
+ret = qemu_rdma_poll(rdma, &wr_id_in, byte_len);
 if (ret < 0) {
 return ret;
 }
@@ -1356,7 +1361,7 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma, 
int wrid_requested)
 }
 
 while (wr_id != wrid_requested) {
-ret = qemu_rdma_poll(rdma, &wr_id_in);
+ret = qemu_rdma_poll(rdma, &wr_id_in, byte_len);
 if (ret < 0) {
 goto err_block_for_wrid;
 }
@@ -1442,7 +1447,7 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, 
uint8_t *buf,
 return ret;
 }
 
-ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL);
+ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL);
 if (ret < 0) {
 fprintf(stderr, "rdma migration: send polling control error!\n");
 }
@@ -1483,7 +1488,9 @@ static int qemu_rdma_post_recv_control(RDMAContext *rdma, 
int idx)
 static int qemu_rdma_exchange_get_response(RDMAContext *rdma,
 RDMAControlHeader *head, int expecting, int idx)
 {
-int ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RECV_CONTROL + idx);
+uint32_t byte_len;
+int ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RECV_CONTROL + idx,
+   &byte_len);
 
 if (ret < 0) {
 fprintf(stderr, "rdma migration: recv polling control error!\n");
@@ -1509,6 +1516,11 @@ static int qemu_rdma_exchange_get_response(RDMAContext 
*rdma,
 fprintf(stderr, "too long length: %d\n", head->len);
 return -EINVAL;
 }
+if (sizeof(*head) + head->len != byte_len) {
+fprintf(stderr, "Malformed length: %d byte_len %d\n",
+head->len, byte_len);
+return -EINVAL;
+}
 
 return 0;
 }
@@ -1738,7 +1750,7 @@ retry:
 count++, current_index, chunk,
 sge.addr, length, rdma->nb_sent, block->nb_chunks);
 
-ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE);
+ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
 
 if (ret < 0) {
 fprintf(stderr, "Failed to Wait for previous write to complete "
@@ -1882,7 +1894,7 @@ retry:
 
 if (ret == ENOMEM) {
 DDPRINTF("send queue is full. wait a little\n");
-ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE);
+ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
 if (ret < 0) {
 fprintf(stderr, "rdma migration: failed to make "
 "room in full send queue! %d\n", ret);
@@ -2471,7 +2483,7 @@ static int qemu_rdma_drain_cq(QEMUFile *f, RDMAContext 
*rdma)
 }
 
 while (rdma->nb_sent) {
-ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE);
+ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
 if (ret < 0) {
 fprintf(stderr, "rdma migration: complete polling error!\n");
 return -EIO;
@@ -2607,7 +2619,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f, void 
*opaque,
  */
 while (1) {
 uint64_t wr_id, wr_id_in;
-int ret = qemu_rdma_poll(rdma, &wr_id_in);
+int ret 

[Qemu-devel] [PATCH for-1.6 2/4] rdma: validate RDMAControlHeader::len

[mrhines]

From: Isaku Yamahata 

RMDAControlHeader::len is provided from remote, so validate it.

Reviewed-by: Orit Wasserman 
Reviewed-by: Michael R. Hines 
Signed-off-by: Isaku Yamahata 
Signed-off-by: Michael R. Hines 
---
 migration-rdma.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/migration-rdma.c b/migration-rdma.c
index 6721266..ebe1f55 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -1424,6 +1424,7 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, 
uint8_t *buf,
  * The copy makes the RDMAControlHeader simpler to manipulate
  * for the time being.
  */
+assert(head->len <= RDMA_CONTROL_MAX_BUFFER - sizeof(*head));
 memcpy(wr->control, head, sizeof(RDMAControlHeader));
 control_to_network((void *) wr->control);
 
@@ -1504,6 +1505,10 @@ static int qemu_rdma_exchange_get_response(RDMAContext 
*rdma,
 control_desc[head->type], head->type, head->len);
 return -EIO;
 }
+if (head->len > RDMA_CONTROL_MAX_BUFFER - sizeof(*head)) {
+fprintf(stderr, "too long length: %d\n", head->len);
+return -EINVAL;
+}
 
 return 0;
 }
-- 
1.7.10.4

[Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, proper getaddrinfo() handling

[mrhines]

From: "Michael R. Hines" 

Some nice buffer-overrun checks and fixing incorrect usage of getaddrinfo()

Isaku Yamahata (3):
  rdma: use resp.len after validation in qemu_rdma_registration_stop
  rdma: validate RDMAControlHeader::len
  rdma: check if RDMAControlHeader::len match transferred byte

Michael R. Hines (1):
  rdma: proper getaddrinfo() handling

 migration-rdma.c |  100 --
 1 file changed, 59 insertions(+), 41 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH for-1.6 1/4] rdma: use resp.len after validation in qemu_rdma_registration_stop

[mrhines]

From: Isaku Yamahata 

resp.len is given from remote host. So should be validated before use.
Otherwise memcpy can access beyond the buffer.

Cc: Michael R. Hines 
Reviewed-by: Orit Wasserman 
Reviewed-by: Michael R. Hines 
Signed-off-by: Isaku Yamahata 
Signed-off-by: Michael R. Hines 
---
 migration-rdma.c |7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/migration-rdma.c b/migration-rdma.c
index 3a380d4..6721266 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -3045,10 +3045,6 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void 
*opaque,
 return ret;
 }
 
-qemu_rdma_move_header(rdma, reg_result_idx, &resp);
-memcpy(rdma->block,
-rdma->wr_data[reg_result_idx].control_curr, resp.len);
-
 nb_remote_blocks = resp.len / sizeof(RDMARemoteBlock);
 
 /*
@@ -3070,6 +3066,9 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void 
*opaque,
 return -EINVAL;
 }
 
+qemu_rdma_move_header(rdma, reg_result_idx, &resp);
+memcpy(rdma->block,
+rdma->wr_data[reg_result_idx].control_curr, resp.len);
 for (i = 0; i < nb_remote_blocks; i++) {
 network_to_remote_block(&rdma->block[i]);
 
-- 
1.7.10.4

[Qemu-devel] [PATCH for-1.6 4/4] rdma: proper getaddrinfo() handling

[mrhines]

From: "Michael R. Hines" 

getaddrinfo() already knows what it's doing,
wqand can potentially return multiple addresses.

Signed-off-by: Michael R. Hines 
---
 migration-rdma.c |   56 --
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/migration-rdma.c b/migration-rdma.c
index 30e08cd..d71cca5 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -392,7 +392,6 @@ typedef struct RDMAContext {
 uint64_t unregistrations[RDMA_SIGNALED_SEND_MAX];
 
 GHashTable *blockmap;
-bool ipv6;
 } RDMAContext;
 
 /*
@@ -745,7 +744,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error 
**errp)
 char port_str[16];
 struct rdma_cm_event *cm_event;
 char ip[40] = "unknown";
-int af = rdma->ipv6 ? PF_INET6 : PF_INET;
+struct addrinfo *e;
 
 if (rdma->host == NULL || !strcmp(rdma->host, "")) {
 ERROR(errp, "RDMA hostname has not been set");
@@ -775,18 +774,23 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
Error **errp)
 goto err_resolve_get_addr;
 }
 
-inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
-ip, sizeof ip);
-DPRINTF("%s => %s\n", rdma->host, ip);
+for (e = res; e != NULL; e = e->ai_next) {
+inet_ntop(e->ai_family,
+&((struct sockaddr_in *) e->ai_addr)->sin_addr, ip, sizeof ip);
+DPRINTF("Trying %s => %s\n", rdma->host, ip);
 
-/* resolve the first address */
-ret = rdma_resolve_addr(rdma->cm_id, NULL, res->ai_addr,
-RDMA_RESOLVE_TIMEOUT_MS);
-if (ret) {
-ERROR(errp, "could not resolve address %s", rdma->host);
-goto err_resolve_get_addr;
+/* resolve the first address */
+ret = rdma_resolve_addr(rdma->cm_id, NULL, e->ai_addr,
+RDMA_RESOLVE_TIMEOUT_MS);
+if (!ret) {
+goto route;
+}
 }
 
+ERROR(errp, "could not resolve address %s", rdma->host);
+goto err_resolve_get_addr;
+
+route:
 qemu_rdma_dump_gid("source_resolve_addr", rdma->cm_id);
 
 ret = rdma_get_cm_event(rdma->channel, &cm_event);
@@ -2260,8 +2264,6 @@ err_rdma_source_connect:
 static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
 {
 int ret = -EINVAL, idx;
-int af = rdma->ipv6 ? PF_INET6 : PF_INET;
-struct sockaddr_in sin;
 struct rdma_cm_id *listen_id;
 char ip[40] = "unknown";
 struct addrinfo *res;
@@ -2292,35 +2294,36 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error 
**errp)
 goto err_dest_init_create_listen_id;
 }
 
-memset(&sin, 0, sizeof(sin));
-sin.sin_family = af;
-sin.sin_port = htons(rdma->port);
 snprintf(port_str, 16, "%d", rdma->port);
 port_str[15] = '\0';
 
 if (rdma->host && strcmp("", rdma->host)) {
+struct addrinfo *e;
+
 ret = getaddrinfo(rdma->host, port_str, NULL, &res);
 if (ret < 0) {
 ERROR(errp, "could not getaddrinfo address %s", rdma->host);
 goto err_dest_init_bind_addr;
 }
 
+for (e = res; e != NULL; e = e->ai_next) {
+inet_ntop(e->ai_family,
+&((struct sockaddr_in *) e->ai_addr)->sin_addr, ip, sizeof ip);
+DPRINTF("Trying %s => %s\n", rdma->host, ip);
+ret = rdma_bind_addr(listen_id, e->ai_addr);
+if (!ret) {
+goto listen;
+}
+}
 
-inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
-ip, sizeof ip);
+ERROR(errp, "Error: could not rdma_bind_addr!");
+goto err_dest_init_bind_addr;
 } else {
 ERROR(errp, "migration host and port not specified!");
 ret = -EINVAL;
 goto err_dest_init_bind_addr;
 }
-
-DPRINTF("%s => %s\n", rdma->host, ip);
-
-ret = rdma_bind_addr(listen_id, res->ai_addr);
-if (ret) {
-ERROR(errp, "Error: could not rdma_bind_addr!");
-goto err_dest_init_bind_addr;
-}
+listen:
 
 rdma->listen_id = listen_id;
 qemu_rdma_dump_gid("dest_init", listen_id);
@@ -2351,7 +2354,6 @@ static void *qemu_rdma_data_init(const char *host_port, 
Error **errp)
 if (addr != NULL) {
 rdma->port = atoi(addr->port);
 rdma->host = g_strdup(addr->host);
-rdma->ipv6 = addr->ipv6;
 } else {
 ERROR(errp, "bad RDMA migration address '%s'", host_port);
 g_free(rdma);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2 4/9] block: vhdx - log support struct and defines

[Jeff Cody]

On Wed, Aug 07, 2013 at 05:22:45PM +0200, Kevin Wolf wrote:
> Am 01.08.2013 um 05:23 hat Jeff Cody geschrieben:
> > This adds some magic number defines, and internal structure
> > definitions for VHDX log replay support.
> > 
> > Signed-off-by: Jeff Cody 
> > ---
> >  block/vhdx.h | 21 -
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block/vhdx.h b/block/vhdx.h
> > index c8d8593..2db6615 100644
> > --- a/block/vhdx.h
> > +++ b/block/vhdx.h
> > @@ -151,7 +151,10 @@ typedef struct QEMU_PACKED VHDXRegionTableEntry {
> >  
> >  
> >  /*  LOG ENTRY STRUCTURES  */
> > +#define VHDX_LOG_MIN_SIZE (1024*1024)
> 
> There should be spaces around the operator.
> 

OK.  Checkpatch.pl missed this, I guess it doesn't check macros?

> Also, VHDX_LOG_ENTRY_MIN_SIZE? I thought at first that this was the
> minimum size of the whole log (or does that happen to be the same and I
> just missed it in the spec?)
>

This is indeed the minimum size for the entire log - once you get to
patch 7, you'll see it used.  But an interesting thing about this, is
the original 0.95 spec said that 1MB was also the minimum log *entry*
size as well.  But that was incorrect, and the 1.00 spec notes that a
log entry size min is 4KB (same as the log sector size).

Jeff

Re: [Qemu-devel] [RFC] [PATCHv6 00/16] aio / timers: Add AioContext timers and use ppoll

[Alex Bligh]

Paolo,

--On 7 August 2013 11:43:58 -0400 Paolo Bonzini  wrote:


For anyone wishing to review this series, here's a diagram showing the
new relationships and a summary of this series:

http://vmsplice.net/~stefan/timerlist.jpg

TimerList is the list of QEMUTimers which are pending on a QEMUClock
clock source.


I just started reviewing this and the diagram may be different in
v7,


No the data structures are the same, just bits renamed.


anyway: why do you need a default TimerListGroup?  I would have
thought it is enough to use the default AioContext's own TLG.


The default TimerListGroup has been renamed main_loop_tlg (or similar)
in order to make this clearer.

Right now, we have a peculiar double set of polling and waiting,
an inner one in AioContext and an outer one in main_loop. Existing
timer users may not be safe to be called within the inner AioContext
and may expect to be run only from main_loop. Also, I believe there
are some binaries that don't even have an AioContext at the moment,
or may not have them at all times when timers are needed.

I understand this may be simplified in the future, in which case
if we always (in every binary) have a default AioContext AND it
exists early enough, we can remove the main_loop_tlg and make
the qemu_ functions uses the default AioContext. That's far from
a huge change but I'd rather not do it is part of this series
as I suspect there is the risk of breakage.

--
Alex Bligh

Re: [Qemu-devel] [PATCH v2 5/9] block: vhdx - break endian translation functions out

[Jeff Cody]

On Wed, Aug 07, 2013 at 05:29:04PM +0200, Kevin Wolf wrote:
> Am 01.08.2013 um 05:23 hat Jeff Cody geschrieben:
> > This moves the endian translation functions out from the vhdx.c source,
> > into a separate source file. In addition to the previously defined
> > endian functions, new endian translation functions for log support are
> > added as well.
> > 
> > Signed-off-by: Jeff Cody 
> 
> > +void vhdx_log_entry_hdr_le_import(VHDXLogEntryHeader *hdr)
> > +{
> > +assert(hdr != NULL);
> > +
> > +le32_to_cpus(&hdr->signature);
> > +le32_to_cpus(&hdr->checksum);
> > +le32_to_cpus(&hdr->entry_length);
> > +le32_to_cpus(&hdr->tail);
> > +le64_to_cpus(&hdr->sequence_number);
> > +le32_to_cpus(&hdr->descriptor_count);
> > +leguid_to_cpus(&hdr->log_guid);
> > +le64_to_cpus(&hdr->flushed_file_offset);
> > +le64_to_cpus(&hdr->last_file_offset);
> > +}
> > +
> > +void vhdx_log_entry_hdr_le_export(VHDXLogEntryHeader *hdr)
> > +{
> > +assert(hdr != NULL);
> > +
> > +cpu_to_le32s(&hdr->signature);
> > +cpu_to_le32s(&hdr->checksum);
> > +cpu_to_le32s(&hdr->entry_length);
> > +cpu_to_le32s(&hdr->tail);
> > +cpu_to_le64s(&hdr->sequence_number);
> > +cpu_to_le32s(&hdr->descriptor_count);
> > +cpu_to_le64s(&hdr->flushed_file_offset);
> > +cpu_to_le64s(&hdr->last_file_offset);
> > +cpu_to_leguids(&hdr->log_guid);
> > +}
> 
> Almost as critical as Stefan's comment: Can you keep the order of fields
> consistent between import and export?
> 

Absolutely! :)

Re: [Qemu-devel] [PATCH v2] Convert stderr message calling error_get_pretty() to error_report()

[Laszlo Ersek]

On 08/05/13 21:40, Seiji Aguchi wrote:
> Convert stderr messages calling error_get_pretty()
> to error_report().
> 
> Timestamp is prepended by -msg timstamp option with it.
> 
> Per Markus's comment below, A conversion from fprintf() to
> error_report() is always an improvement, regardless of 
> error_get_pretty().
> 
> http://marc.info/?l=qemu-devel&m=137513283408601&w=2
> 
> But, it is not reasonable to convert them at one time
> because fprintf() is used everwhere in qemu.
> 
> So, it should be done step by step with avoiding regression.
> 
> Signed-off-by: Seiji Aguchi 
> ---
> Change from v2
>  - Remove "\n" from strings because error_report() appends it.
> 
>  Patch v1
>  http://marc.info/?l=qemu-devel&m=137452706024842&w=2
> ---
>  arch_init.c |4 ++--
>  hw/char/serial.c|5 +++--
>  hw/i386/pc.c|6 +++---
>  qemu-char.c |2 +-
>  target-i386/cpu.c   |2 +-
>  target-ppc/translate_init.c |3 ++-
>  vl.c|9 +
>  7 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 68a7ab7..94d45e1 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -1125,8 +1125,8 @@ void do_acpitable_option(const QemuOpts *opts)
>  
>  acpi_table_add(opts, &err);
>  if (err) {
> -fprintf(stderr, "Wrong acpi table provided: %s\n",
> -error_get_pretty(err));
> +error_report("Wrong acpi table provided: %s",
> + error_get_pretty(err));
>  error_free(err);
>  exit(1);
>  }
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 6025592..a31eb57 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -27,6 +27,7 @@
>  #include "sysemu/char.h"
>  #include "qemu/timer.h"
>  #include "exec/address-spaces.h"
> +#include "qemu/error-report.h"
>  
>  //#define DEBUG_SERIAL
>  
> @@ -696,7 +697,7 @@ SerialState *serial_init(int base, qemu_irq irq, int 
> baudbase,
>  s->chr = chr;
>  serial_realize_core(s, &err);
>  if (err != NULL) {
> -fprintf(stderr, "%s\n", error_get_pretty(err));
> +error_report("%s", error_get_pretty(err));
>  error_free(err);
>  exit(1);
>  }
> @@ -760,7 +761,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>  
>  serial_realize_core(s, &err);
>  if (err != NULL) {
> -fprintf(stderr, "%s\n", error_get_pretty(err));
> +error_report("%s", error_get_pretty(err));
>  error_free(err);
>  exit(1);
>  }
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a2b9d88..5bce10f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -979,7 +979,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
> *icc_bridge)
>  cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
>   icc_bridge, &error);
>  if (error) {
> -fprintf(stderr, "%s\n", error_get_pretty(error));
> +error_report("%s", error_get_pretty(error));
>  error_free(error);
>  exit(1);
>  }
> @@ -1097,8 +1097,8 @@ void pc_acpi_init(const char *default_dsdt)
>  
>  acpi_table_add(opts, &err);
>  if (err) {
> -fprintf(stderr, "WARNING: failed to load %s: %s\n", filename,
> -error_get_pretty(err));
> +error_report("WARNING: failed to load %s: %s", filename,
> + error_get_pretty(err));
>  error_free(err);
>  }
>  g_free(arg);
> diff --git a/qemu-char.c b/qemu-char.c
> index 3f606c9..2e9a11d 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3299,7 +3299,7 @@ CharDriverState *qemu_chr_new(const char *label, const 
> char *filename, void (*in
>  
>  chr = qemu_chr_new_from_opts(opts, init, &err);
>  if (error_is_set(&err)) {
> -fprintf(stderr, "%s\n", error_get_pretty(err));
> +error_report("%s", error_get_pretty(err));
>  error_free(err);
>  }
>  if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 71ab915..43061e2 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1843,7 +1843,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>  
>  out:
>  if (error) {
> -fprintf(stderr, "%s\n", error_get_pretty(error));
> +error_report("%s", error_get_pretty(error));
>  error_free(error);
>  if (cpu != NULL) {
>  object_unref(OBJECT(cpu));
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 0724226..3cf74a2 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -27,6 +27,7 @@
>  #include "cpu-models.h"
>  #include "mmu-hash32.h"
>  #include "mmu-hash64.h"
> +#include "qemu/error-report.h"
>  
>  //#define PPC_DUMP_CPU
>  //#define PPC_DEBUG_SPR
> @@ -8177,7 +8178,7 @@ PowerPCCPU *cpu_ppc_init(const char *cpu_model)
>

Re: [Qemu-devel] [PATCH 2/5] qemu_coroutine_self should not be marked coroutine_fn as it cannot yield

[Stefan Hajnoczi]

On Mon, Aug 05, 2013 at 08:44:04PM +0200, Charlie Shepherd wrote:
> From: Charlie Shepherd 
> 
> While it only really makes sense to call qemu_coroutine_self() in a coroutine
> context, it cannot actually yield execution itself, so remove the coroutine_fn
> annotation.
> ---
>  include/block/coroutine.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

By removing coroutine_fn the rules have changed:

It's now legal to call qemu_coroutine_self() outside a coroutine.
Previously only callers that knew the internals of the coroutine
implementation did that.

coroutine_fn gives coroutine backend implementors more flexibility in
how they choose to implement qemu_coroutine_self().  From an API
perspective I prefer to keep qemu_coroutine_self() marked as a
coroutine_fn.

I guess the practical problem is that CPC will get
upset that it's being called by the coroutine implementation from
non-coroutine contexts.  This can be solved:

1. Keep the public qemu_coroutine_self() marked coroutine_fn.

2. Have it call an internal coroutine_self() function that is not
   coroutine_fn.

This way the API stays strict and the internal implementation doesn't
violate the coroutine_fn calling rule.

Stefan

Re: [Qemu-devel] [PATCH 3/5] Convert BlockDriver to explicit coroutine annotations

[Stefan Hajnoczi]

On Mon, Aug 05, 2013 at 08:33:10PM +0100, Charlie Shepherd wrote:
> On 05/08/2013 20:23, Gabriel Kerneis wrote:
> >On Mon, Aug 05, 2013 at 08:44:05PM +0200, Charlie Shepherd wrote:
> >>diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h
> >>index f133d65..d0ab27d 100644
> >>--- a/include/block/coroutine_int.h
> >>+++ b/include/block/coroutine_int.h
> >>@@ -48,6 +48,6 @@ Coroutine *qemu_coroutine_new(void);
> >>  void qemu_coroutine_delete(Coroutine *co);
> >>  CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to,
> >>CoroutineAction action);
> >>-void coroutine_fn qemu_co_queue_run_restart(Coroutine *co);
> >>+void qemu_co_queue_run_restart(Coroutine *co);
> >>  #endif
> >Adding coroutine_fn where it is necessary seems straightforward to me: just
> >follow the "callers of coroutine_fn should be coroutine_fn" rule (assuming 
> >you
> >got it right and did not over-annotate). On the other hand, you
> >should probably explain in the commit message why you *remove* those three
> >coroutine_fn annotations.
> 
> Yes that does merit some explanation.
> 
> Building the tree with cps inference warned that these functions
> were annotated spuriously. I initially thought this was because they
> called a coroutine function that hadn't been annotated, but it seems
> the *_handler functions called qemu_aio_set_fd_handler which, from
> my investigation, it seems does not need annotating. Therefore they
> were indeed spuriously annotated and so I removed the annotation.
> 
> qemu_co_queue_run_restart is a bit different. It is only called from
> coroutine_swap in qemu-coroutine.c, and it enters coroutines that
> were waiting but have now moved to the runnable state by the actions
> of the most recent coroutine (I believe). I think we discussed this
> on IRC on Thursday? It only calls qemu_coroutine_enter, and cannot
> yield, so again I removed the annotation. I'll add these
> explanations to the commit message.

I have mixed feelings about removing coroutine_fn annotations from a
function when it does not yield or call other coroutine_fn functions.

These functions were probably written as part of a coroutine code path.
The coroutine_fn annotation tells me I'm in coroutine context.

By removing this information those modifying the code now need to
convert it back to coroutine_fn after auditing callers before they can
use coroutine context.

The thing is, these leaf functions are typically only called from
coroutine context anyway.  I think they should be left marked
coroutine_fn.

I'd compare this to a comment that says "lock foo is held across this
function" but the function doesn't use anything that lock foo protects.
Removing the comment isn't really helpful, you are throwing away
information that can be useful when modifying the function.

Stefan

[Qemu-devel] [ANNOUNCE] QEMU 1.6.0-rc2 is now available

[Anthony Liguori]

Hi,

On behalf of the QEMU Team, I'd like to announce the availability of the
third release candidate for the QEMU 1.6 release.  This release is meant
for testing purposes and should not be used in a production environment.

http://wiki.qemu.org/download/qemu-1.6.0-rc2.tar.bz2

You can help improve the quality of the QEMU 1.6 release by testing this
release and reporting bugs on Launchpad:

https://bugs.launchpad.net/qemu/

The release plan for the 1.6 release is available at:

http://wiki.qemu.org/Planning/1.6

Please add entries to the ChangeLog for the 1.6 release below:

http://wiki.qemu.org/ChangeLog/Next

The following changes have been made since v1.6.0-rc1:

 - fw_cfg: the I/O port variant expects little-endian (Paolo Bonzini)
 - po: Update all *.po files (Stefan Weil)
 - target-ppc: Add POWER5+ v2.1 CPU model (Andreas Färber)
 - target-ppc: Prepare POWER5P CPU family (Andreas Färber)
 - target-ppc: Turn POWER5gr CPU into alias for POWER5 (Andreas Färber)
 - target-ppc: Turn POWER5gs CPU into alias for POWER5+ (Andreas Färber)
 - target-ppc: Fix POWER7+ model (Andreas Färber)
 - Bugfix for loading multiboot kernels (Martijn van den Broek)
 - target-i386: Fix X86CPU error handling (Andreas Färber)
 - vmdk: rename num_gtes_per_gte to num_gtes_per_gt (Fam Zheng)
 - vmdk: use heap allocation for whole_grain (Fam Zheng)
 - vmdk: check l1 size before opening image (Fam Zheng)
 - vmdk: check l2 table size when opening (Fam Zheng)
 - vmdk: check granularity field in opening (Fam Zheng)
 - qemu-iotests: add empty test case for vmdk (Fam Zheng)
 - qemu-iotests: add poke_file utility function (Stefan Hajnoczi)
 - vmdk: use unsigned values for on disk header fields (Fam Zheng)
 - vmdk: Make VMDK3Header and VmdkGrainMarker QEMU_PACKED (Fam Zheng)
 - target-mips: fix decoding of microMIPS POOL32Axf instructions (Leon Alrae)
 - sheepdog: add missing .bdrv_has_zero_init (Liu Yuan)
 - qemu-iotests: filter QEMU version in monitor banner (Stefan Hajnoczi)
 - iov: handle EOF in iov_send_recv (MORITA Kazutaka)
 - ignore SIGPIPE in qemu-img and qemu-io (MORITA Kazutaka)
 - qemu-img: Error out for excess arguments (Kevin Wolf)
 - semaphore: fix a hangup problem under load on NetBSD hosts. (Izumi Tsutsui)
 - rdma: memory leak RDMAContext::host (Isaku Yamahata)
 - rdma: use RDMA_WRID_READY (Isaku Yamahata)
 - rdma: qemu_rdma_post_send_control uses wrongly RDMA_WRID_MAX (Isaku Yamahata)
 - rdma: don't use negative index to array (Isaku Yamahata)
 - rdma: correct newlines in error statements (Michael R. Hines)
 - rdma: forgot to turn off the debugging flag (Michael R. Hines)
 - rdma: bugfix: make IPv6 support work (Michael R. Hines)
 - pxa2xx: Avoid object_get_link_property() assertion for "parent_bus" (Andreas 
Färber)
 - target-ppc: Add POWER7+ CPU model (Alexey Kardashevskiy)
 - pcnet: Flush queued packets on end of STOP state (Jan Kiszka)
 - target-mips: fix 34Kf configuration for DSP ASE (Yongbok Kim)
 - block: Disable driver-specific options for 1.6 (Kevin Wolf)
 - vmdk: fix comment for vmdk_co_write_zeroes (Fam Zheng)
 - memory.c: drop kvm.h dependency (Michael S. Tsirkin)
 - block/iscsi.c: Fix printf format error. (Richard W.M. Jones)
 - qemu-ga: build it even if !system (Michael Tokarev)
 - usb-redir: fix use-after-free (Gerd Hoffmann)
 - xhci: fix segfault (Gerd Hoffmann)
 - i82378: Cleanup implementation (Hervé Poussineau)
 - pci-host/prep: Set isa_mem_base in the PCI host bridge (Hervé Poussineau)
 - cpu: Fix VMSTATE_CPU() semantics (Andreas Färber)
 - Update OpenBIOS images (Mark Cave-Ayland)
 - target-xtensa: check register window inline (Max Filippov)
 - target-xtensa: don't generate dead code to access invalid SRs (Max Filippov)
 - tests/tcg/xtensa: Fix out-of-tree build (Andreas Färber)
 - target-xtensa: avoid double-stopping at breakpoints (Max Filippov)
 - target-xtensa: add fallthrough markers (Max Filippov)
 - target-xtensa: add extui unit test (Max Filippov)

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] seabios: update to 1.7.3

[Alex Williamson]

Don't we really want this too?

commit 2a9aeabdfb34374ecac25e7a8d21c9e368618cd4
Author: Kevin O'Connor 
Date:   Sun Jul 14 13:55:52 2013 -0400

Fix USB EHCI detection that was broken in hlist conversion of 
PCIDevices.

Make sure the PCI device list is ordered in bus order.

Don't iterate past the end of the list when detecting EHCI devices.

Signed-off-by: Kevin O'Connor 

seabios 1.7.3 is actually a regression for me when trying to assign a
secondary passthrough graphics card because the PCI devices are in the
wrong order and seabios tries to run the passthrough VBIOS rather than
the emulated VBIOS.  Based on the comment, something is broken with EHCI
too.  Thanks,

Alex

On Wed, 2013-07-24 at 15:46 +0200, Gerd Hoffmann wrote:
> Changes summary (git shortlog rel-1.7.2.2..rel-1.7.3):
> 
> Alex Williamson (4):
>   seabios q35: Enable all PIRQn IRQs at startup
>   seabios q35: Add new PCI slot to irq routing function
>   seabios: Add a dummy PCI slot to irq mapping function
>   pciinit: Enable default VGA device
> 
> Asias He (2):
>   virtio-scsi: Set _DRIVER_OK flag before scsi target scanning
>   virtio-scsi: Pack struct virtio_scsi_{req_cmd,resp_cmd}
> 
> Avik Sil (1):
>   USB-EHCI: Fix null pointer assignment
> 
> Christian Gmeiner (5):
>   geodevga: fix errors in geode_fp_* functions
>   geodevga: move framebuffer setup
>   geodevga: move output setup to own function
>   geodevga: add debug to msr functions
>   geodevga: fix wrong define name
> 
> David Woodhouse (26):
>   Add macros for pushing and popping struct bregs
>   Clean up #if in pirtable.c. CONFIG_PIRTABLE can't be set if 
> CONFIG_COREBOOT is
>   post: Export functions which will be used individually by CSM
>   Export callrom() for CSM to use
>   Export copy_smbios() from biostables.c
>   Import LegacyBios.h from OVMF
>   Complete and checksum EFI_COMPATIBILITY16_TABLE at build time
>   Add pic_save_mask() and pic_restore_mask() functions
>   Add CSM support
>   Add README.CSM
>   Add find_pmtimer() function
>   Enable PMTIMER for CSM build
>   Fix rom_reserve()/rom_confirm() for CSM oprom dispatch
>   Don't calibrate TSC if PMTIMER is already set up
>   Move find_pmtimer() to ACPI table setup where it logically belongs
>   Use find_pmtimer() after copying Xen ACPI tables
>   Use find_pmtimer() after copying coreboot ACPI tables
>   Unify return path for CSM to go via csm_return()
>   Make CONFIG_OPTIONROMS_DEPLOYED depend on CONFIG_QEMU
>   Implement !CONFIG_OPTIONROMS support for CSM
>   Implement !CONFIG_BOOT for CSM
>   Enable VGA output when settings bochs-specific mode
>   Disable CONFIG_THREAD_OPTIONROMS for CSM build
>   Fix return type of le64_to_cpu() and be64_to_cpu()
>   Rename find_pmtimer() to find_acpi_features()
>   Add acpi_reboot() reset method using RESET_REG
> 
> Gerd Hoffmann (3):
>   config: allow DEBUG_IO for !QEMU
>   coreboot: add qemu detection
>   tweak coreboot qemu detection
> 
> Hu Tao (1):
>   Add pvpanic device driver
> 
> Kevin O'Connor (99):
>   pmm: Use 'struct segoff_s' in pmm header.
>   Minor: Update README - variable changes are now reset on soft-reboots.
>   Normalize POST initialization function name suffixes.
>   POST: Reorganize post init functions for better grouping and 
> reusability.
>   Fix rebase error in commit 8a0a972f that broke LOWMEM variables.
>   Support calling a function other than maininit() from reloc_preinit().
>   Ensure exported symbols are visible in the final link
>   POST: Move QEMU specific ramsize and BIOS table setup to paravirt.c.
>   POST: Reorganize post entry and "preinit" functions.
>   POST: Move cpu caching and dma setup to platform_hardware_setup().
>   Undo incorrect assumptions about Xen in commit 6ca0460f.
>   Determine century during init and store in VARLOW mem during runtime.
>   No need to check both CONFIG_THREADS and CONFIG_THREAD_OPTIONROMS.
>   Add runningOnQEMU() and runningOnXen() for runtime platform detection.
>   Consistently use CONFIG_COREBOOT, CONFIG_QEMU, and runningOnXen().
>   Convert kvm_para_available() to runningOnKVM().
>   Minor - move definitions to paravirt.c from paravirt.h.
>   Only perform SMP setup on QEMU.
>   Start device_hardware_setup in mainint even with 
> CONFIG_THREAD_OPTIONROMS.
>   The mathcp setup touches the PIC and thus move to the "setup" phase.
>   Update tools/acpi_extract.py to handle iasl 20130117 release.
>   Support skipping content when reading from QEMU fw_cfg romfile entries.
>   Convert fw_cfg ACPI entries into romfile entries.
>   Convert fw_cfg SMBIOS entries into romfile entries.
>   Convert basic integer fw_cfg entries into romf

[Qemu-devel] [PATCH for 1.6] w32: Add missing version.o to all executables (fix regression)

[Stefan Weil]

QEMU executables for w32, w64 had included meta information built from
version.rc. These rules were changed several times some months ago.

The latest version added version.o to the tools, but not to the system
emulations.

This patch adds the meta information to all system emulations again.

It builds a version.o for each target (which allows different information
for each target in the future).

I removed the libtool part with version-lobj-y (why was it added?).

Signed-off-by: Stefan Weil 
---
 Makefile|6 ++
 Makefile.target |6 ++
 rules.mak   |5 ++---
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 29f1043..59788c4 100644
--- a/Makefile
+++ b/Makefile
@@ -167,13 +167,11 @@ recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
 
 bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
 
-version.o: $(SRC_PATH)/version.rc config-host.h | version.lo
-version.lo: $(SRC_PATH)/version.rc config-host.h
+version.o: $(SRC_PATH)/version.rc config-host.h
 
 version-obj-$(CONFIG_WIN32) += version.o
-version-lobj-$(CONFIG_WIN32) += version.lo
 
-Makefile: $(version-obj-y) $(version-lobj-y)
+Makefile: $(version-obj-y)
 
 ##
 # Build libraries
diff --git a/Makefile.target b/Makefile.target
index 9a49852..01037fe 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -13,6 +13,12 @@ QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target-$(TARGET_BASE_ARCH) 
-DNEED_CPU_H
 
 QEMU_CFLAGS+=-I$(SRC_PATH)/include
 
+version.o: $(SRC_PATH)/version.rc ../config-host.h
+
+version-obj-$(CONFIG_WIN32) += version.o
+
+Makefile: $(version-obj-y)
+
 ifdef CONFIG_USER_ONLY
 # user emulator name
 QEMU_PROG=qemu-$(TARGET_NAME)
diff --git a/rules.mak b/rules.mak
index 4499745..0bcd876 100644
--- a/rules.mak
+++ b/rules.mak
@@ -20,7 +20,7 @@ QEMU_INCLUDES += -I$(

Re: [Qemu-devel] [RFC V3 0/2] continuous leaky bucket throttling

[Benoît Canet]

> I saw more discussion on IRC.  Does this mean you will send another
> revision to address outstanding issues?
> 
> Just wanted to check if you are waiting for code review or if you are
> still developing the next patch revision.

I am currently finishing to write unit tests for the next patch revision.
Once unit tests are done I will rebase the previous QMP throttling patches on
top of the new code then post the new series.

I think that the result will be a net improvement.

Thanks for asking

Best regards

Benoît

Re: [Qemu-devel] [PATCH 2/5] qemu_coroutine_self should not be marked coroutine_fn as it cannot yield

[Gabriel Kerneis]

On Wed, Aug 07, 2013 at 09:18:05PM +0200, Stefan Hajnoczi wrote:
> I guess the practical problem is that CPC will get
> upset that it's being called by the coroutine implementation from
> non-coroutine contexts.

But is it really the case? If Charlie added an annotation, it probably means
that in practice it was only called from coroutine context anyway.

The only downside from CPC's point of view is that there is a cost to making a
coroutine_fn, and it's a pity to pay it when it's useless (ie. when the function
never yields anyway).

-- 
Gabriel

Re: [Qemu-devel] [PATCH v4 3/7] target-i386: enable PCLMULQDQ on Westmere CPU

[Eduardo Habkost]

On Sun, Mar 31, 2013 at 01:02:22PM +0200, Aurelien Jarno wrote:
> The PCLMULQDQ instruction has been introduced on the Westmere CPU.
> 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Edgar E. Iglesias 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-i386/cpu.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 41382c5..5941d40 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -687,7 +687,7 @@ static x86_def_t builtin_x86_defs[] = {
>   CPUID_DE | CPUID_FP87,
>  .ext_features = CPUID_EXT_AES | CPUID_EXT_POPCNT | CPUID_EXT_SSE42 |
>   CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
> - CPUID_EXT_SSE3,
> + CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,

I have just noticed that this patch entered v1.5.0 without any code to
keep pc-1.4 and older machine-types compatible. I will try to send a
patch in time to get this fixed on QEMU v1.6.0.


>  .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
>  .ext3_features = CPUID_EXT3_LAHF_LM,
>  .xlevel = 0x800A,
> -- 
> 1.7.10.4
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [sheepdog] [PATCH] sheepdog: implement .bdrv_get_allocated_file_size

[MORITA Kazutaka]

At Wed,  7 Aug 2013 16:59:53 +0800,
Liu Yuan wrote:
> 
> With this patch, qemu-img info sheepdog:image will show disk size for sheepdog
> images.
> 
> Cc: Kevin Wolf 
> Cc: Stefan Hajnoczi 
> Cc: MORITA Kazutaka 
> Signed-off-by: Liu Yuan 
> ---
>  block/sheepdog.c |   19 +++
>  1 file changed, 19 insertions(+)

Looks good to me.

Reviewed-by: MORITA Kazutaka 

Re: [Qemu-devel] [PATCH 3/5] Convert BlockDriver to explicit coroutine annotations

[Charlie Shepherd]

On 06/08/2013 10:24, Kevin Wolf wrote:

Am 05.08.2013 um 20:44 hat Charlie Shepherd geschrieben:

This patch converts the .bdrv_open, .bdrv_file_open and .bdrv_create members of 
struct BlockDriver
to be explicitly annotated as coroutine_fn, rather than yielding dynamically 
depending on whether
they are executed in a coroutine context or not.

I wonder whether this patch should be split into one for each converted
BlockDriver function. It would probably make review easier.

For those function where you actually just correct the coroutine_fn
annotation, but whose behaviour doesn't change, a single patch for all
might be enough.


  block.c   | 16 +++
  block/blkdebug.c  | 10 -
  block/blkverify.c |  4 ++--
  block/bochs.c |  8 
  block/cloop.c |  6 +++---
  block/cow.c   | 12 +--
  block/curl.c  | 12 +--
  block/dmg.c   |  6 +++---
  block/nbd.c   | 28 -
  block/parallels.c |  6 +++---
  block/qcow.c  |  8 
  block/qcow2-cluster.c |  8 
  block/qcow2.c | 48 +--
  block/qcow2.h |  4 ++--
  block/qed.c   |  8 
  block/raw-posix.c | 34 +++---
  block/raw.c   |  8 
  block/sheepdog.c  | 24 +++---
  block/snapshot.c  | 32 -
  block/ssh.c   | 14 ++---
  block/vdi.c   | 12 +--
  block/vhdx.c  |  4 ++--
  block/vmdk.c  | 12 +--
  block/vpc.c   | 12 +--
  block/vvfat.c | 12 +--
  include/block/block_int.h | 10 -
  include/block/coroutine.h |  4 ++--
  include/block/coroutine_int.h |  2 +-
  qemu-coroutine-lock.c |  4 ++--
  30 files changed, 218 insertions(+), 152 deletions(-)

diff --git a/block.c b/block.c
index 6c493ad..aaa122c 100644
--- a/block.c
+++ b/block.c
@@ -374,7 +374,7 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
  CreateCo *cco = opaque;
  assert(cco->drv);
  
-cco->ret = cco->drv->bdrv_create(cco->filename, cco->options);

+cco->ret = cco->drv->bdrv_co_create(cco->filename, cco->options);
  }
  
  int bdrv_create(BlockDriver *drv, const char* filename,

@@ -390,7 +390,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
  .ret = NOT_DONE,
  };
  
-if (!drv->bdrv_create) {

+if (!drv->bdrv_co_create) {
  ret = -ENOTSUP;
  goto out;
  }
@@ -697,7 +697,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
  /* bdrv_open() with directly using a protocol as drv. This layer is 
already
   * opened, so assign it to bs (while file becomes a closed 
BlockDriverState)
   * and return immediately. */
-if (file != NULL && drv->bdrv_file_open) {
+if (file != NULL && drv->bdrv_co_file_open) {
  bdrv_swap(file, bs);
  return 0;
  }
@@ -728,10 +728,10 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
  bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB);
  
  /* Open the image, either directly or using a protocol */

-if (drv->bdrv_file_open) {
+if (drv->bdrv_co_file_open) {
  assert(file == NULL);
  assert(drv->bdrv_parse_filename || filename != NULL);
-ret = drv->bdrv_file_open(bs, options, open_flags);
+ret = drv->bdrv_co_file_open(bs, options, open_flags);

bdrv_open_common() isn't coroutine_fn, though?

Ah well, I see that you change it in a later patch. Please make sure
that the code is in a consistent state after each single commit.

For this series, I think this suggests that you indeed split by converted
function, but put everything related to this function in one patch. For
example, the bdrv_open_common() conversion would be in the same patch as
this hunk.


Yeah, it's in this kind of scenario that I struggled to split up the 
patch series properly. I'll try and implement your approach in the next 
version.



  } else {
  if (file == NULL) {
  qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
@@ -742,7 +742,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
  }
  assert(file != NULL);
  bs->file = file;
-ret = drv->bdrv_open(bs, options, open_flags);
+ret = drv->bdrv_co_open(bs, options, open_flags);
  }
  
  if (ret < 0) {

diff --git a/block/qcow2.c b/block/qcow2.c
index 0eceefe..2ed0bb6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -58,6 +58,10 @@ typedef struct {
  #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
  #define  QCOW2

Re: [Qemu-devel] [PATCH 4/5] Convert block functions to coroutine versions

[Charlie Shepherd]

On 06/08/2013 10:36, Kevin Wolf wrote:

Am 05.08.2013 um 20:44 hat Charlie Shepherd geschrieben:

This patch follows on from the previous one and converts some block layer 
functions to be
explicitly annotated with coroutine_fn instead of yielding depending upon 
calling context.
---
  block.c   | 235 ++
  block/mirror.c|   4 +-
  include/block/block.h |  37 
  3 files changed, 148 insertions(+), 128 deletions(-)

diff --git a/block.c b/block.c
index aaa122c..e7011f9 100644
--- a/block.c
+++ b/block.c
@@ -164,7 +164,7 @@ bool bdrv_io_limits_enabled(BlockDriverState *bs)
   || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
  }
  
-static void bdrv_io_limits_intercept(BlockDriverState *bs,

+static void coroutine_fn bdrv_io_limits_intercept(BlockDriverState *bs,
   bool is_write, int nb_sectors)
  {
  int64_t wait_time = -1;
@@ -364,7 +364,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char 
*format_name,
  
  typedef struct CreateCo {

  BlockDriver *drv;
-char *filename;
+const char *filename;
  QEMUOptionParameter *options;
  int ret;
  } CreateCo;

Like Gabriel said, this should be a separate patch. Keeping it in this
series is probably easiest.


@@ -372,48 +372,48 @@ typedef struct CreateCo {
  static void coroutine_fn bdrv_create_co_entry(void *opaque)
  {
  CreateCo *cco = opaque;
-assert(cco->drv);
-
-cco->ret = cco->drv->bdrv_co_create(cco->filename, cco->options);
+cco->ret = bdrv_create(cco->drv, cco->filename, cco->options);
  }
  
-int bdrv_create(BlockDriver *drv, const char* filename,

+int coroutine_fn bdrv_create(BlockDriver *drv, const char* filename,
  QEMUOptionParameter *options)
  {
  int ret;
+char *dup_fn;
+
+assert(drv);
+if (!drv->bdrv_co_create) {
+return -ENOTSUP;
+}
  
+dup_fn = g_strdup(filename);

+ret = drv->bdrv_co_create(dup_fn, options);
+g_free(dup_fn);
+return ret;
+}
+
+
+int bdrv_create_sync(BlockDriver *drv, const char* filename,
+QEMUOptionParameter *options)

bdrv_foo_sync is an unfortunate naming convention, because
bdrv_pwrite_sync already exists and means something totally different:
It's a write directly followed by a disk flush.


Yes it's not a great naming convention. I could rename bdrv_pwrite_sync 
to something like bdrv_pwrite_and_sync? Or is that too horrible?
An alternative would be bdrv_foo and bdrv_sync_foo? But bdrv_sync_pwrite 
and bdrv_pwrite_sync would be quite confusing in a hurry.



diff --git a/include/block/block.h b/include/block/block.h
index dd8eca1..57d8ba2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -125,9 +125,9 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState 
*bs_top);
  void bdrv_delete(BlockDriverState *bs);
  int bdrv_parse_cache_flags(const char *mode, int *flags);
  int bdrv_parse_discard_flags(const char *mode, int *flags);
-int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+int coroutine_fn bdrv_file_open(BlockDriverState **pbs, const char *filename,
 QDict *options, int flags);
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
+int coroutine_fn bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
int flags, BlockDriver *drv);
  BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
@@ -150,18 +150,24 @@ void bdrv_dev_eject_request(BlockDriverState *bs, bool 
force);
  bool bdrv_dev_has_removable_media(BlockDriverState *bs);
  bool bdrv_dev_is_tray_open(BlockDriverState *bs);
  bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
-int bdrv_read(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
-int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
+int bdrv_read_sync(BlockDriverState *bs, int64_t sector_num,
+  uint8_t *buf, int nb_sectors);
+int coroutine_fn bdrv_read_unthrottled(BlockDriverState *bs, int64_t 
sector_num,
uint8_t *buf, int nb_sectors);
-int bdrv_write(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_write(BlockDriverState *bs, int64_t sector_num,
+   const uint8_t *buf, int nb_sectors);
+int bdrv_write_sync(BlockDriverState *bs, int64_t sector_num,
 const uint8_t *buf, int nb_sectors);
-int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
-int bdrv_pread(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_writev(BlockDriverState *bs, int64_t sector_num, 
QEMUIOVector *qiov);
+int coroutine_fn bdrv_pread(BlockDriverState *bs, int64_t offset,
+   void *buf, int count);
+int bdrv_pread_sync(BlockDriverState *bs, int64_t offset,
 void *buf, int cou

Re: [Qemu-devel] [PATCH 5/5] Convert block layer callers' annotations

[Charlie Shepherd]

On 05/08/2013 21:15, Gabriel Kerneis wrote:

On Mon, Aug 05, 2013 at 08:44:07PM +0200, Charlie Shepherd wrote:

This patch updates the callers of block layer functions converted to explicit
coroutine_fn annotation in the previous patch.

It looks like this patch is made of three parts:
- updating the annotations, following the rule "caller of coroutine_fn must be
   coroutine_fn",
- introducing a run_handler function in qemu-img.c,
- replacing bdrv_* by bdrv_*_sync in qemu-io-cmds.c.

The first one is purely mechanical and boring. The second one is moderately
inventive (note that I did not review the code in detail). The third one is
simple but definitely deserves some explanation. Again, I think this could be
split in three different patches, with an expanded commit message.


Ok thanks this seems like a logical way to split them up.


Charlie

Re: [Qemu-devel] [PATCH 1/5] Add an explanation of when a function should be marked coroutine_fn

[Charlie Shepherd]

On 06/08/2013 09:39, Stefan Hajnoczi wrote:

On Mon, Aug 05, 2013 at 08:44:03PM +0200, Charlie Shepherd wrote:

From: Charlie Shepherd 

Coroutine functions that can yield directly or indirectly should be annotated
with a coroutine_fn annotation. Add an explanation to that effect in
include/block/coroutine.h.
---
  include/block/coroutine.h | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 377805a..3b94b6d 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -37,6 +37,9 @@
   * static checker support for catching such errors.  This annotation might 
make
   * it possible and in the meantime it serves as documentation.
   *
+ * A function must be marked with coroutine_fn if it can yield execution, 
either
+ * directly or indirectly.
+ *

This is correct except for the case of dynamic functions that do:

   if (qemu_in_coroutine()) {
   } else {
   Coroutine *co = qemu_coroutine_new(...);
   ...
   }

Here the function is coroutine_fn only if the caller is in coroutine
context.

I think your comment update should include a note about this.  When
you've split all dynamic functions into coroutine/non-coroutine versions
then the note can be removed.


Hmm ok, I can add a note to that effect.


Charlie

Re: [Qemu-devel] RFC: [PATCH 0/5] Explicitly annotating coroutine_fn functions

[Charlie Shepherd]

On 06/08/2013 10:37, Kevin Wolf wrote:

Am 05.08.2013 um 20:44 hat Charlie Shepherd geschrieben:

This patch series is a follow up to a previous RFC about converting functions
that dynamically yield execution depending on whether they are in executing in
a coroutine context or not to be explicitly statically annotated. This change
is necessary for the GSoC CPC project, but was also agreed in an IRC
conversation on #qemu to be benefical overall if it can be upstream before the
end of the project. This is an update to see if the approach I'm taking to
implementing this conversion is correct.

In order to statically check the tree to ensure the annotations are correct,
I've been using CPC to compile the QEMU tree. This does a source to source
translation to convert coroutine code to continuation-passing style (the
purpose of the GSoC project), but as a side benefit statically checks
annotations (any functions annotated with coroutine_fn are transformed into
CPS, so have a different "calling style" to the standard C convention).

In order to compile the tree with CPC:
  $ git clone git://github.com/kerneis/cpc.git
  $ cd cpc
  $ make
  $ ./configure
  $ make
  $ cd ..
  $ export CPC=$(pwd)/cpc/bin/cpc
  $ cd qemu
  $ mkdir -p bin/cpc
  $ cd bin/cpc
  $ ../../configure --enable-debug --disable-werror  --target-list=x86_64-softmmu 
--cc="$CPC" --extra-cflags="--noMakeStaticGlobal --useLogicalOperators 
--useCaseRange --save-temps -U__SSE2__ -w -Dcoroutine_fn='__attribute__((__cps__))' 
--docpsInference  --warnall "
  $ make

Against which tree is this? It didn't apply on top of qemu-git master,
nor on my block branch.


Sorry, just to clarify, this isn't based on QEMU but is a repository 
containing the CPC tool, which can then be used to statically check 
QEMU; you'll need to clone it into its own repo. In this example it 
should be a sibling directory to whichever QEMU checkout you're testing 
it on.



Charlie

Re: [Qemu-devel] [PATCH 2/5] qemu_coroutine_self should not be marked coroutine_fn as it cannot yield

[Charlie Shepherd]

On 07/08/2013 20:18, Stefan Hajnoczi wrote:

On Mon, Aug 05, 2013 at 08:44:04PM +0200, Charlie Shepherd wrote:

From: Charlie Shepherd 

While it only really makes sense to call qemu_coroutine_self() in a coroutine
context, it cannot actually yield execution itself, so remove the coroutine_fn
annotation.
---
  include/block/coroutine.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

By removing coroutine_fn the rules have changed:

It's now legal to call qemu_coroutine_self() outside a coroutine.
Previously only callers that knew the internals of the coroutine
implementation did that.


Yes, I agree that it's probably not helpful to loosen the restrictions 
on when its possible to call qemu_coroutine_self().



coroutine_fn gives coroutine backend implementors more flexibility in
how they choose to implement qemu_coroutine_self().  From an API
perspective I prefer to keep qemu_coroutine_self() marked as a
coroutine_fn.

I guess the practical problem is that CPC will get
upset that it's being called by the coroutine implementation from
non-coroutine contexts.  This can be solved:

1. Keep the public qemu_coroutine_self() marked coroutine_fn.

2. Have it call an internal coroutine_self() function that is not
coroutine_fn.

This way the API stays strict and the internal implementation doesn't
violate the coroutine_fn calling rule.


This is a good solution, I'll add a patch implementing it to the series.


Charlie

Re: [Qemu-devel] [PATCH 2/5] qemu_coroutine_self should not be marked coroutine_fn as it cannot yield

[Charlie Shepherd]

On 07/08/2013 23:13, Gabriel Kerneis wrote:

On Wed, Aug 07, 2013 at 09:18:05PM +0200, Stefan Hajnoczi wrote:

I guess the practical problem is that CPC will get
upset that it's being called by the coroutine implementation from
non-coroutine contexts.

But is it really the case? If Charlie added an annotation, it probably means
that in practice it was only called from coroutine context anyway.


It was also called from coroutine implementation in functions that 
weren't annotated coroutine_fn (qemu_coroutine_switch() and friends).



The only downside from CPC's point of view is that there is a cost to making a
coroutine_fn, and it's a pity to pay it when it's useless (ie. when the function
never yields anyway).


Yes although in this case it won't be too high as the function will 
simply be


Coroutine *internal_qemu_coroutine_self(void);

coroutine_fn Coroutine *qemu_coroutine_self(void)
{
return internal_qemu_coroutine_self();
}

and the original qemu_coroutine_self couldn't be inlined, so I don't 
think it is too prohibitive. Besides, I doubt qemu_coroutine_self() is 
called in many fast-paths.



Charlie

Re: [Qemu-devel] [PATCH 3/5] Convert BlockDriver to explicit coroutine annotations

[Charlie Shepherd]

On 07/08/2013 20:30, Stefan Hajnoczi wrote:

On Mon, Aug 05, 2013 at 08:33:10PM +0100, Charlie Shepherd wrote:

On 05/08/2013 20:23, Gabriel Kerneis wrote:

On Mon, Aug 05, 2013 at 08:44:05PM +0200, Charlie Shepherd wrote:

diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h
index f133d65..d0ab27d 100644
--- a/include/block/coroutine_int.h
+++ b/include/block/coroutine_int.h
@@ -48,6 +48,6 @@ Coroutine *qemu_coroutine_new(void);
  void qemu_coroutine_delete(Coroutine *co);
  CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to,
CoroutineAction action);
-void coroutine_fn qemu_co_queue_run_restart(Coroutine *co);
+void qemu_co_queue_run_restart(Coroutine *co);
  #endif

Adding coroutine_fn where it is necessary seems straightforward to me: just
follow the "callers of coroutine_fn should be coroutine_fn" rule (assuming you
got it right and did not over-annotate). On the other hand, you
should probably explain in the commit message why you *remove* those three
coroutine_fn annotations.

Yes that does merit some explanation.

Building the tree with cps inference warned that these functions
were annotated spuriously. I initially thought this was because they
called a coroutine function that hadn't been annotated, but it seems
the *_handler functions called qemu_aio_set_fd_handler which, from
my investigation, it seems does not need annotating. Therefore they
were indeed spuriously annotated and so I removed the annotation.

qemu_co_queue_run_restart is a bit different. It is only called from
coroutine_swap in qemu-coroutine.c, and it enters coroutines that
were waiting but have now moved to the runnable state by the actions
of the most recent coroutine (I believe). I think we discussed this
on IRC on Thursday? It only calls qemu_coroutine_enter, and cannot
yield, so again I removed the annotation. I'll add these
explanations to the commit message.

I have mixed feelings about removing coroutine_fn annotations from a
function when it does not yield or call other coroutine_fn functions.

These functions were probably written as part of a coroutine code path.
The coroutine_fn annotation tells me I'm in coroutine context.

By removing this information those modifying the code now need to
convert it back to coroutine_fn after auditing callers before they can
use coroutine context.

The thing is, these leaf functions are typically only called from
coroutine context anyway.  I think they should be left marked
coroutine_fn.

I'd compare this to a comment that says "lock foo is held across this
function" but the function doesn't use anything that lock foo protects.
Removing the comment isn't really helpful, you are throwing away
information that can be useful when modifying the function.


I see, so in that case coroutine_fn is more a contract saying "this 
function can only be, and is only, called from a coroutine context"? 
That feels fair enough, but I'll amend the comment in coroutine.h to 
that effect.



Charlie

[Qemu-devel] [PATCH v3 0/9] VHDX log replay and write support

[Jeff Cody]

This patch series contains the initial VHDX log parsing, replay,
and write support.

=== v3 changes ===  

Thank you Kevin & Stefan for the feedback; incoporated in v3:

Patch 1: --- nil ---

Patch 2: * use sizeof(crc) instead of 4
 * remove outdated comment
 * use sizeof(MSGUID) instead of 16
 * direct assignment of guid structs rather than memcpy
 * rename 'rw' to 'generate_data_write_guid'
 * use offsetof() instead of 4
 * comment typos
 * add missing error checking
 * MSGUID is now QEMU_PACKED
 * configure enable for vhdx is now correct and not braindead

Patch 3: --- nil ---

Patch 4: * code style fixes
 * removed unused struct (VHDXLogEntryInfo)

Patch 5: * more direct assignment of guid rather than memcpy
 * order of operation in export/import the same now
 * became less generous with newlines (bah-humbug!)

Patch 6: * more direct assignment of guid rather than memcpy 
 * add error check in vhdx_user_visible_write(), now returns int

Patch 7: * check error return now of vhdx_user_visible_write()

Patch 8: * check error return now of vhdx_user_visible_write()
 * vhdx_log_write_sectors() uses bdrv_pwrite() vs bdrv_pwrite_sync()
 * more direct assignment of guid rather than memcpy 
 * use offsetof() instead of 4

Patch 9: --- nil ---

=== end v3 changelog ===

v2 changes:  Incorporated Fam's review feedback - Thank you Fam for the feedback

=== end v2 changelog ===

This will allow an existing log in a VHDX image to be replayed (e.g., a VHDX
image from a Hyper-V host that crashed).  In addition, metadata writes are
enabled through the log.  This allows write support to be enabled for VHDX,
as the BAT can be updated safely via the log journal.

These exact patches are available from github, for testing:
https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v2-upstream

The latest vhdx work (including anything beyond these patches, such as
backing/parent file support) can be found at:
https://github.com/codyprime/qemu-kvm-jtc/tree/jtc-vhdx-latest

Jeff Cody (9):
  block: vhdx - minor comments and typo correction.
  block: vhdx - add header update capability.
  block: vhdx code movement - VHDXMetadataEntries and BDRVVHDXState to
header.
  block: vhdx - log support struct and defines
  block: vhdx - break endian translation functions out
  block: vhdx - update log guid in header, and first write tracker
  block: vhdx - log parsing, replay, and flush support
  block: vhdx - add log write support
  block: vhdx write support

 block/Makefile.objs |2 +-
 block/vhdx-endian.c |  141 +++
 block/vhdx-log.c| 1011 +++
 block/vhdx.c|  399 ++--
 block/vhdx.h|  135 +--
 configure   |   24 ++
 6 files changed, 1585 insertions(+), 127 deletions(-)
 create mode 100644 block/vhdx-endian.c
 create mode 100644 block/vhdx-log.c

-- 
1.8.1.4

[Qemu-devel] [PATCH v3 1/9] block: vhdx - minor comments and typo correction.

[Jeff Cody]

Just a couple of minor comments to help note where allocated
buffers are freed, and a typo fix.

Signed-off-by: Jeff Cody 
---
 block/vhdx.c | 6 --
 block/vhdx.h | 6 +++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index e9704b1..56bc88e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -6,9 +6,9 @@
  * Authors:
  *  Jeff Cody 
  *
- *  This is based on the "VHDX Format Specification v0.95", published 4/12/2012
+ *  This is based on the "VHDX Format Specification v1.00", published 8/25/2012
  *  by Microsoft:
- *  https://www.microsoft.com/en-us/download/details.aspx?id=29681
+ *  https://www.microsoft.com/en-us/download/details.aspx?id=34750
  *
  * This work is licensed under the terms of the GNU LGPL, version 2 or later.
  * See the COPYING.LIB file in the top-level directory.
@@ -262,6 +262,7 @@ static int vhdx_parse_header(BlockDriverState *bs, 
BDRVVHDXState *s)
 uint64_t h2_seq = 0;
 uint8_t *buffer;
 
+/* header1 & header2 are freed in vhdx_close() */
 header1 = qemu_blockalign(bs, sizeof(VHDXHeader));
 header2 = qemu_blockalign(bs, sizeof(VHDXHeader));
 
@@ -787,6 +788,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, 
int flags)
 goto fail;
 }
 
+/* s->bat is freed in vhdx_close() */
 s->bat = qemu_blockalign(bs, s->bat_rt.length);
 
 ret = bdrv_pread(bs->file, s->bat_offset, s->bat, s->bat_rt.length);
diff --git a/block/vhdx.h b/block/vhdx.h
index fb687ed..9eb6b97 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -6,9 +6,9 @@
  * Authors:
  *  Jeff Cody 
  *
- *  This is based on the "VHDX Format Specification v0.95", published 4/12/2012
+ *  This is based on the "VHDX Format Specification v1.00", published 8/25/2012
  *  by Microsoft:
- *  https://www.microsoft.com/en-us/download/details.aspx?id=29681
+ *  https://www.microsoft.com/en-us/download/details.aspx?id=34750
  *
  * This work is licensed under the terms of the GNU LGPL, version 2 or later.
  * See the COPYING.LIB file in the top-level directory.
@@ -116,7 +116,7 @@ typedef struct QEMU_PACKED VHDXHeader {
valid. */
 uint16_tlog_version;/* version of the log format. Mustn't 
be
zero, unless log_guid is also zero 
*/
-uint16_tversion;/* version of th evhdx file.  
Currently,
+uint16_tversion;/* version of the vhdx file.  
Currently,
only supported version is "1" */
 uint32_tlog_length; /* length of the log.  Must be multiple
of 1MB */
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 2/9] block: vhdx - add header update capability.

[Jeff Cody]

This adds the ability to update the headers in a VHDX image, including
generating a new MS-compatible GUID.

As VHDX depends on uuid.h, VHDX is now a configurable build option.  If
VHDX support is enabled, that will also enable uuid as well.  The
default is to have VHDX enabled.

To enable/disable VHDX:  --enable-vhdx, --disable-vhdx

Signed-off-by: Jeff Cody 
---
 block/Makefile.objs |   2 +-
 block/vhdx.c| 161 +++-
 block/vhdx.h|  14 -
 configure   |  24 
 4 files changed, 196 insertions(+), 5 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 4cf9aa4..e5e54e6 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -2,7 +2,7 @@ block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o 
bochs.o vpc.o vvfat
 block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o 
qcow2-cache.o
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
-block-obj-y += vhdx.o
+block-obj-$(CONFIG_VHDX) += vhdx.o
 block-obj-y += parallels.o blkdebug.o blkverify.o
 block-obj-y += snapshot.o qapi.o
 block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
diff --git a/block/vhdx.c b/block/vhdx.c
index 56bc88e..7bd7c12 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -21,6 +21,7 @@
 #include "qemu/crc32c.h"
 #include "block/vhdx.h"
 
+#include 
 
 /* Several metadata and region table data entries are identified by
  * guids in  a MS-specific GUID format. */
@@ -156,11 +157,40 @@ typedef struct BDRVVHDXState {
 VHDXBatEntry *bat;
 uint64_t bat_offset;
 
+MSGUID session_guid;
+
+
 VHDXParentLocatorHeader parent_header;
 VHDXParentLocatorEntry *parent_entries;
 
 } BDRVVHDXState;
 
+/* Calculates new checksum.
+ *
+ * Zero is substituted during crc calculation for the original crc field
+ * crc_offset: byte offset in buf of the buffer crc
+ * buf: buffer pointer
+ * size: size of buffer (must be > crc_offset+4)
+ *
+ * Note: The resulting checksum is in the CPU endianness, not necessarily
+ *   in the file format endianness (LE).  Any header export to disk should
+ *   make sure that vhdx_header_le_export() is used to convert to the
+ *   correct endianness
+ */
+uint32_t vhdx_update_checksum(uint8_t *buf, size_t size, int crc_offset)
+{
+uint32_t crc;
+
+assert(buf != NULL);
+assert(size > (crc_offset + sizeof(crc)));
+
+memset(buf + crc_offset, 0, sizeof(crc));
+crc =  crc32c(0x, buf, size);
+memcpy(buf + crc_offset, &crc, sizeof(crc));
+
+return crc;
+}
+
 uint32_t vhdx_checksum_calc(uint32_t crc, uint8_t *buf, size_t size,
 int crc_offset)
 {
@@ -212,6 +242,19 @@ bool vhdx_checksum_is_valid(uint8_t *buf, size_t size, int 
crc_offset)
 
 
 /*
+ * This generates a UUID that is compliant with the MS GUIDs used
+ * in the VHDX spec (and elsewhere).
+ */
+void vhdx_guid_generate(MSGUID *guid)
+{
+uuid_t uuid;
+assert(guid != NULL);
+
+uuid_generate(uuid);
+memcpy(guid, uuid, sizeof(MSGUID));
+}
+
+/*
  * Per the MS VHDX Specification, for every VHDX file:
  *  - The header section is fixed size - 1 MB
  *  - The header section is always the first "object"
@@ -249,6 +292,113 @@ static void vhdx_header_le_import(VHDXHeader *h)
 le64_to_cpus(&h->log_offset);
 }
 
+/* All VHDX structures on disk are little endian */
+static void vhdx_header_le_export(VHDXHeader *orig_h, VHDXHeader *new_h)
+{
+assert(orig_h != NULL);
+assert(new_h != NULL);
+
+new_h->signature   = cpu_to_le32(orig_h->signature);
+new_h->checksum= cpu_to_le32(orig_h->checksum);
+new_h->sequence_number = cpu_to_le64(orig_h->sequence_number);
+
+new_h->file_write_guid = orig_h->file_write_guid;
+new_h->data_write_guid = orig_h->data_write_guid;
+new_h->log_guid= orig_h->log_guid;
+
+cpu_to_leguids(&new_h->file_write_guid);
+cpu_to_leguids(&new_h->data_write_guid);
+cpu_to_leguids(&new_h->log_guid);
+
+new_h->log_version = cpu_to_le16(orig_h->log_version);
+new_h->version = cpu_to_le16(orig_h->version);
+new_h->log_length  = cpu_to_le32(orig_h->log_length);
+new_h->log_offset  = cpu_to_le64(orig_h->log_offset);
+}
+
+/* Update the VHDX headers
+ *
+ * This follows the VHDX spec procedures for header updates.
+ *
+ *  - non-current header is updated with largest sequence number
+ */
+static int vhdx_update_header(BlockDriverState *bs, BDRVVHDXState *s,
+  bool generate_data_write_guid)
+{
+int ret = 0;
+int hdr_idx = 0;
+uint64_t header_offset = VHDX_HEADER1_OFFSET;
+
+VHDXHeader *active_header;
+VHDXHeader *inactive_header;
+VHDXHeader header_le;
+uint8_t *buffer;
+
+/* operate on the non-current header */
+if (s->curr_header == 0) {
+hdr_idx = 1;
+header_offset = VHDX_HEADER2_OFFSET;
+}
+
+active_h

[Qemu-devel] [PATCH v3 3/9] block: vhdx code movement - VHDXMetadataEntries and BDRVVHDXState to header.

[Jeff Cody]

In preparation for VHDX log support, move these structures to the
header.

Signed-off-by: Jeff Cody 
---
 block/vhdx.c | 51 ---
 block/vhdx.h | 47 +++
 2 files changed, 47 insertions(+), 51 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 7bd7c12..68648e1 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -104,16 +104,6 @@ static const MSGUID parent_vhdx_guid = { .data1 = 
0xb04aefb7,
  META_PAGE_83_PRESENT | META_LOGICAL_SECTOR_SIZE_PRESENT | \
  META_PHYS_SECTOR_SIZE_PRESENT)
 
-typedef struct VHDXMetadataEntries {
-VHDXMetadataTableEntry file_parameters_entry;
-VHDXMetadataTableEntry virtual_disk_size_entry;
-VHDXMetadataTableEntry page83_data_entry;
-VHDXMetadataTableEntry logical_sector_size_entry;
-VHDXMetadataTableEntry phys_sector_size_entry;
-VHDXMetadataTableEntry parent_locator_entry;
-uint16_t present;
-} VHDXMetadataEntries;
-
 
 typedef struct VHDXSectorInfo {
 uint32_t bat_idx;   /* BAT entry index */
@@ -124,47 +114,6 @@ typedef struct VHDXSectorInfo {
 uint64_t block_offset;  /* block offset, in bytes */
 } VHDXSectorInfo;
 
-
-
-typedef struct BDRVVHDXState {
-CoMutex lock;
-
-int curr_header;
-VHDXHeader *headers[2];
-
-VHDXRegionTableHeader rt;
-VHDXRegionTableEntry bat_rt; /* region table for the BAT */
-VHDXRegionTableEntry metadata_rt;/* region table for the metadata */
-
-VHDXMetadataTableHeader metadata_hdr;
-VHDXMetadataEntries metadata_entries;
-
-VHDXFileParameters params;
-uint32_t block_size;
-uint32_t block_size_bits;
-uint32_t sectors_per_block;
-uint32_t sectors_per_block_bits;
-
-uint64_t virtual_disk_size;
-uint32_t logical_sector_size;
-uint32_t physical_sector_size;
-
-uint64_t chunk_ratio;
-uint32_t chunk_ratio_bits;
-uint32_t logical_sector_size_bits;
-
-uint32_t bat_entries;
-VHDXBatEntry *bat;
-uint64_t bat_offset;
-
-MSGUID session_guid;
-
-
-VHDXParentLocatorHeader parent_header;
-VHDXParentLocatorEntry *parent_entries;
-
-} BDRVVHDXState;
-
 /* Calculates new checksum.
  *
  * Zero is substituted during crc calculation for the original crc field
diff --git a/block/vhdx.h b/block/vhdx.h
index 403f766..74b2d5d 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -308,6 +308,53 @@ typedef struct QEMU_PACKED VHDXParentLocatorEntry {
 
 /* - END VHDX SPECIFICATION STRUCTURES  */
 
+typedef struct VHDXMetadataEntries {
+VHDXMetadataTableEntry file_parameters_entry;
+VHDXMetadataTableEntry virtual_disk_size_entry;
+VHDXMetadataTableEntry page83_data_entry;
+VHDXMetadataTableEntry logical_sector_size_entry;
+VHDXMetadataTableEntry phys_sector_size_entry;
+VHDXMetadataTableEntry parent_locator_entry;
+uint16_t present;
+} VHDXMetadataEntries;
+
+typedef struct BDRVVHDXState {
+CoMutex lock;
+
+int curr_header;
+VHDXHeader *headers[2];
+
+VHDXRegionTableHeader rt;
+VHDXRegionTableEntry bat_rt; /* region table for the BAT */
+VHDXRegionTableEntry metadata_rt;/* region table for the metadata */
+
+VHDXMetadataTableHeader metadata_hdr;
+VHDXMetadataEntries metadata_entries;
+
+VHDXFileParameters params;
+uint32_t block_size;
+uint32_t block_size_bits;
+uint32_t sectors_per_block;
+uint32_t sectors_per_block_bits;
+
+uint64_t virtual_disk_size;
+uint32_t logical_sector_size;
+uint32_t physical_sector_size;
+
+uint64_t chunk_ratio;
+uint32_t chunk_ratio_bits;
+uint32_t logical_sector_size_bits;
+
+uint32_t bat_entries;
+VHDXBatEntry *bat;
+uint64_t bat_offset;
+
+MSGUID session_guid;
+
+VHDXParentLocatorHeader parent_header;
+VHDXParentLocatorEntry *parent_entries;
+
+} BDRVVHDXState;
 
 void vhdx_guid_generate(MSGUID *guid);
 
-- 
1.8.1.4