[Qemu-devel] Announcement: Patchew server is online now

[Fam Zheng]

Hello, everyone

I'm glad to announce the fresh Patchew server deployment:

http://qemu.patchew.org/

The mission of this project is to help identify obvious defects (coding style,
compiling, etc.) of posted patches.

Follow the green "passed" or red "failed" button in the page and you'll see the
testing log. You can also see which series have got reviews or comments.

Q: How will this impact patch submission and merging process?

Nothing is affected (at least for now), use this as an auxiliary.

Q: What exactly are tested for each patch?

* scripts/checkpatch.pl
* git am (on top of qemu.git)
* ./configure --target-list=x86_64-softmmu
* make
* make check

Q: No different architectures, compilers or targets?

Not yet. A simple script does the test in a docker instance. A bigger
coverage is possible though, by adding steps to the script:

[1]: https://github.com/famz/patchew/blob/master/tests/qemu-devel.sh

Q: Where is the said email notification?

The email notification feature is ready. In a few days I will enable
automatic replying to the list, once the web interface and testing are both
running smoothly and up to speed. Currently the only recipient is Fam :)

Q: Where is the source code?

https://github.com/famz/patchew

(Pull requests very welcome!)


Enjoy, and feel free to make comments!

Thank you!


---
Fam Zheng

[Qemu-devel] [Bug 1426593] [NEW] qem-user arm cortex-a8 printf out-of-memory hang

[aaron]

Public bug reported:

using the latest build from git (hash 041ccc922ee474693a2869d4e3b59e920c739bc0 
) and all older versions i have tested.
i am using an amd64 host with an arm chroot using "qemu-user arm cortex-a8" cpu 
emulation to run it

building coreutils hangs on "checking whether printf survives out-of-
memory conditions"

i have not had time to dig into the build system to isolate the test
yet, there were old reports of this bug but i can no longer find them on
google.

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

Title:
  qem-user arm cortex-a8 printf out-of-memory hang

Status in QEMU:
  New

Bug description:
  using the latest build from git (hash 
041ccc922ee474693a2869d4e3b59e920c739bc0 ) and all older versions i have tested.
  i am using an amd64 host with an arm chroot using "qemu-user arm cortex-a8" 
cpu emulation to run it

  building coreutils hangs on "checking whether printf survives out-of-
  memory conditions"

  i have not had time to dig into the build system to isolate the test
  yet, there were old reports of this bug but i can no longer find them
  on google.

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

Re: [Qemu-devel] [PATCH 1/6 v4] target-tilegx: Firstly add to qemu with minimized features

[Chen Gang S]

Firstly, thank you very much for your patient work!


On 2/28/15 01:36, Andreas Färber wrote:
> Hi,
> 
> "target-tilegx: Initial stub" or "...support"? No need to mention QEMU
> (spelling!) in a QEMU commit.
> 

OK, thanks.

> Am 22.02.2015 um 14:33 schrieb Chen Gang S:
>> It almost likes a template for adding an architecture target.
> 
> That's a comment that's not really descriptive of what the patch does.
> Instead, maybe mention that this is the configure and build system
> support etc. and what name to use for enabling it from configure, that
> it's linux-user only for now, ...?
> 

OK, thanks.

>>
>> Signed-off-by: Chen Gang 
>> ---
>>  configure |   7 ++
>>  default-configs/tilegx-linux-user.mak |   1 +
>>  target-tilegx/Makefile.objs   |   1 +
>>  target-tilegx/cpu-qom.h   |  72 +++
>>  target-tilegx/cpu.c   | 162 
>> ++
>>  target-tilegx/cpu.h   |  85 ++
>>  target-tilegx/helper.h|   0
>>  target-tilegx/translate.c |  54 
>>  8 files changed, 382 insertions(+)
>>  create mode 100644 default-configs/tilegx-linux-user.mak
>>  create mode 100644 target-tilegx/Makefile.objs
>>  create mode 100644 target-tilegx/cpu-qom.h
>>  create mode 100644 target-tilegx/cpu.c
>>  create mode 100644 target-tilegx/cpu.h
>>  create mode 100644 target-tilegx/helper.h
>>  create mode 100644 target-tilegx/translate.c
>>
>> diff --git a/configure b/configure
>> index 7ba4bcb..23aa8f6 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5191,6 +5191,9 @@ case "$target_name" in
>>s390x)
>>  gdb_xml_files="s390x-core64.xml s390-acr.xml s390-fpr.xml"
>>;;
>> +  tilegx)
>> +TARGET_ARCH=tilegx
>> +  ;;
>>unicore32)
>>;;
>>xtensa|xtensaeb)
>> @@ -5363,6 +5366,10 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
>>  echo "CONFIG_SPARC_DIS=y"  >> $config_target_mak
>>  echo "CONFIG_SPARC_DIS=y"  >> config-all-disas.mak
>>;;
>> +  tilegx*)
>> +echo "CONFIG_TILEGX_DIS=y"  >> $config_target_mak
>> +echo "CONFIG_TILEGX_DIS=y"  >> config-all-disas.mak
>> +  ;;
> 
> Hadn't you been asked to drop these lines, as you are not yet adding any
> disassembler code that uses it?
> 

NO. And next, I shall drop them.

>>xtensa*)
>>  echo "CONFIG_XTENSA_DIS=y"  >> $config_target_mak
>>  echo "CONFIG_XTENSA_DIS=y"  >> config-all-disas.mak
> [...]
>> diff --git a/target-tilegx/cpu-qom.h b/target-tilegx/cpu-qom.h
>> new file mode 100644
>> index 000..e15a8b8
>> --- /dev/null
>> +++ b/target-tilegx/cpu-qom.h
>> @@ -0,0 +1,72 @@
>> +/*
>> + * QEMU Tilegx CPU
> 
> "TILE-Gx" according to
> http://www.tilera.com/products/?ezchip=585&spage=614 - please fix
> wherever used in textual form.
> 

OK, thanks.

>> + *
>> + * Copyright (c) 2015 Chen Gang
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see
>> + * 
>> + */
>> +#ifndef QEMU_TILEGX_CPU_QOM_H
>> +#define QEMU_TILEGX_CPU_QOM_H
>> +
>> +#include "qom/cpu.h"
>> +
>> +#define TYPE_TILEGX_CPU "tilegx-cpu"
>> +
>> +#define TILEGX_CPU_CLASS(klass) \
>> +OBJECT_CLASS_CHECK(TilegxCPUClass, (klass), TYPE_TILEGX_CPU)
>> +#define TILEGX_CPU(obj) \
>> +OBJECT_CHECK(TilegxCPU, (obj), TYPE_TILEGX_CPU)
>> +#define TILEGX_CPU_GET_CLASS(obj) \
>> +OBJECT_GET_CLASS(TilegxCPUClass, (obj), TYPE_TILEGX_CPU)
>> +
>> +/**
>> + * TilegxCPUClass:
>> + * @parent_realize: The parent class' realize handler.
>> + * @parent_reset: The parent class' reset handler.
>> + *
>> + * A Tilegx CPU model.
>> + */
>> +typedef struct TilegxCPUClass {
> 
> For the benefit of readers, please call this TileGXCPUClass ...
> 

OK, thanks.

>> +/*< private >*/
>> +CPUClass parent_class;
>> +/*< public >*/
>> +
>> +DeviceRealize parent_realize;
>> +void (*parent_reset)(CPUState *cpu);
>> +} TilegxCPUClass;
>> +
>> +/**
>> + * TilegxCPU:
>> + * @env: #CPUTLState
>> + *
>> + * A Tilegx CPU.
>> + */
>> +typedef struct TilegxCPU {
> 
> ... and TileGXCPU. (or TileGx...)
> 

OK, thanks. I shall call it TileGXCPU.

>> +/*< private >*/
>> +CPUState parent_obj;
>> +uint64_t base_vectors;
> 
> This should not be in here, the private section serves to hide the
> parent field from documentation.
> 
> base_vectors shoul

Re: [Qemu-devel] [RFC PATCH 0/7] hw/arm/virt: Add cpu-add way cpu hotplug support

[Wei Huang]

On 02/26/2015 01:32 AM, Shannon Zhao wrote:
> On 2015/2/19 1:19, Wei Huang wrote:
>> Nice work. I will help review the patches.
>>
> Thanks for your help :-)
> 
>> Other than the CPU hotplug support, we are also seeking the guest VM
>> powerdown support via ACPI. This feature is important for management
>> tool to control guest VMs. In fact I had a similar GPIO patch for
>> powerdown recently. Given that you already posted a more complete
>> version, I wonder if you can add this feature to your patchset? FYI, I
>> attached my powerdown patch in this email. Here is a brief list of items:
>>
>> * DSDT description of power button (to-be-done)
>> * Hook up with qemu_qemu_register_powerdown_notifier() with the GPIO IRQ
>>
>> Feel free to add my name as signed-off-by in this feature if you decide
>> to add it.
> 
> Yes, I think the powerdown support is very useful, but maybe this feature can 
> be
> added by a new patchset as this patchset cares about CPU hotplug.
> 
> We can add powerdown, memory hotplug, cpu remove and memory remove support 
> step
> by step if the approach of this patchset is accepted. I can make a patch to 
> add
> powerdown support based on this patchset and your attached patch if you want 
> to
> test this feature. How do you think about this?
That sounds good to me. It does make sense to separate them. Keep me in
loop.

Thanks,
-Wei
> 

Re: [Qemu-devel] [PATCH 1/6 v4] target-tilegx: Firstly add to qemu with minimized features

[Chen Gang S]

On 2/28/15 10:09, Chris Metcalf wrote:
> On 2/27/2015 9:10 PM, Chen Gang S wrote:
>> By the way, does Gx8000 mean 8000 core count?
> 
> It's marketing, so it doesn't have to mean anything.  The TILE-Gx 8036 is the 
> 36-core part, the 8072 is the 72-core part, 8009 is the 9-core part, etc.  
> There is no "8000" chip, it's just a way of describing the TILE-Gx chip 
> series.
> 

OK, thanks.

-- 
Chen Gang

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

Re: [Qemu-devel] [PATCH V2 06/11] virtio-s390: switch to bus specific queue limit

[Jason Wang]

On Fri, Feb 27, 2015 at 5:49 PM, Cornelia Huck 
 wrote:

On Fri, 27 Feb 2015 06:42:57 +0008
Jason Wang  wrote:

 On Thu, Feb 26, 2015 at 9:05 PM, Cornelia Huck 
  wrote:

 > On Thu, 26 Feb 2015 15:04:41 +0800
 > Jason Wang  wrote:
 > 



 >>   typedef struct AdapterRoutes {
 >>   AdapterInfo adapter;
 >>   int num_routes;
 >>  -int gsi[VIRTIO_PCI_QUEUE_MAX];
 >>  +int gsi[VIRTIO_S390_QUEUE_MAX];
 > 
 > Adapter routes are only applicable for the ccw transport, not for 
the

 > old s390 transport.
 
 Sure, will fix this.
 
 > 
 > 
 > (I'm also wondering whether this should be the generic limit 
instead.)
 
 As you pointed out in V1, there will be more issues if we just 
increase 
 the generic limit. So I switch to use per transport limit. Since 
the 
 limit was not changed for both s390 and ccw, it should be ok.


I'm just wondering how many gsis we want to support for adapter 
routes.

They were introduced for virtio-ccw, but recently s390 pci has started
to use them as well, so a virtio limit seems silly here. I'll switch
them to some kind of generic limit instead, I think.


Get your point. My understanding is you can do this on top of this 
series.


Thanks

Re: [Qemu-devel] [PATCH V2 04/11] virtio-ccw: introduce ccw specific queue limit

[Jason Wang]

On Fri, Feb 27, 2015 at 5:41 PM, Cornelia Huck 
 wrote:

On Fri, 27 Feb 2015 03:46:25 +0008
Jason Wang  wrote:

 On Thu, Feb 26, 2015 at 9:02 PM, Cornelia Huck 
  wrote:

 > On Thu, 26 Feb 2015 15:04:39 +0800
 > Jason Wang  wrote:
 > 
 >>  Instead of depending on marco, using a bus specific limit.
 >>  
 >>  Cc: Alexander Graf 

 >>  Cc: Cornelia Huck 
 >>  Cc: Christian Borntraeger 
 >>  Cc: Richard Henderson 
 >>  Signed-off-by: Jason Wang 
 >>  ---
 >>   hw/s390x/s390-virtio-ccw.c |  7 +--
 >>   hw/s390x/virtio-ccw.c  | 13 +++--
 >>   include/hw/virtio/virtio.h |  1 +
 >>   3 files changed, 13 insertions(+), 8 deletions(-)
 >>  
 >>  diff --git a/hw/s390x/s390-virtio-ccw.c 
b/hw/s390x/s390-virtio-ccw.c

 >>  index 71bafe0..6aeb815 100644
 >>  --- a/hw/s390x/s390-virtio-ccw.c
 >>  +++ b/hw/s390x/s390-virtio-ccw.c
 >>  @@ -43,6 +43,7 @@ void io_subsystem_reset(void)
 >>  
 >>   static int virtio_ccw_hcall_notify(const uint64_t *args)

 >>   {
 >>  +VirtIODevice *vdev;
 >>   uint64_t subch_id = args[0];
 >>   uint64_t queue = args[1];
 >>   SubchDev *sch;
 >>  @@ -55,10 +56,12 @@ static int virtio_ccw_hcall_notify(const 
 >> uint64_t *args)

 >>   if (!sch || !css_subch_visible(sch)) {
 >>   return -EINVAL;
 >>   }
 >>  -if (queue >= VIRTIO_PCI_QUEUE_MAX) {
 >>  +
 >>  +vdev = virtio_ccw_get_vdev(sch);
 >>  +if (queue >= virtio_get_queue_max(vdev)) {
 > 
 > But we already know we have a virtio_ccw device, right? So why 
not 
 > just

 > check against VIRTIO_CCW_QUEUE_MAX?
 
 The problem is whether or not you want to increase the max queue 
for 
 ccw. If yes, for migration compatibility, you could not just use a 
 marco here since legacy machine type will also get increased. 


Confused. With "legacy machine type", do you mean s390-virtio? It does
not register this hcall.


Ok, I thought s390 may have something like pc had to keep the migration 
compatibility for migration. But seems not.





 Something 
 dynamically like this is need so the machine initialization code 
can 
 change this limit. 


I fear I don't understand how the machine code comes into play here. 
Is

the virtio-ccw device code supposed to inherit the limit from the
machine?


As I said in the previous reply. The machine code is used to keep 
migration compatibility. We don't want to break the migration from 2.3 
to 2.2. If the limit of ccw will be increased in the future and we want 
to keep migration compatibility. The machine codes may do something 
similar to virtio pci.





Anyway, if we move the queue limit to the VirtIODevice, checking the
limit there instead of using a #define should work out fine.

Re: [Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit

[Jason Wang]

On Fri, Feb 27, 2015 at 5:34 PM, Cornelia Huck 
 wrote:

On Fri, 27 Feb 2015 03:42:00 +0008
Jason Wang  wrote:

 On Thu, Feb 26, 2015 at 8:57 PM, Cornelia Huck 
  wrote:

 > On Thu, 26 Feb 2015 15:04:38 +0800
 > Jason Wang  wrote:
 > 
 >>  This patch introduces a bus specific queue limitation. It will 
be
 >>  useful for increasing the limit for one of the bus without 
 >> disturbing

 >>  other buses.



 >>  diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
 >>  index ffc22e8..5a806b5 100644
 >>  --- a/hw/virtio/virtio.c
 >>  +++ b/hw/virtio/virtio.c
 >>  @@ -541,6 +541,14 @@ void virtio_update_irq(VirtIODevice *vdev)
 >>   virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
 >>   }
 >>  
 >>  +int virtio_get_queue_max(VirtIODevice *vdev)

 >>  +{
 >>  +BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
 >>  +VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 >>  +
 >>  +return k->queue_max;
 >>  +}
 >>  +
 > 
 > Are all callers of this in the slow path? So we don't introduce

 > processing overhead.
 
 Looks not. For overhead, do you mean one introduced by 
 VIRTIO_BUS_GET_CLASS()? Not sure how much it will affact but we've 
 already used something like this in the datapath, e.g 
 virtio_notify_vector(). 


I may have misremembered how much overhead those types of operation
introduce.

But it made me think: This function is basically introducing a
per-VirtIODevice queue limit. We set it once in the VirtioBusClass
during initialization, but don't expect it to change. Why don't we 
just

propagate it to a new member of VirtIODevice during initialization
instead?


The limit may be changed. Consider that patch 9 increases the pci limit 
from 64 to 513. But we want to keep the migration compatibility for 
legacy machine types e.g machines earlier than pc-q35-2.3 or 
pc-i440fx-2.3, so the limit was changed back to 64 in pc_compat_2_2(). 
More work will be done if we want to propagate it to VirtIODevice, and 
it looks unnecessary.

Re: [Qemu-devel] [PATCH v2] migration: Convert 'status' of MigrationInfo to use an enum type

[zhanghailiang]

On 2015/2/28 1:42, Eric Blake wrote:

On 02/27/2015 10:07 AM, Dr. David Alan Gilbert wrote:


Rather than pollute the user-exposed enum with a state that we will
never report, can we come up with some internal-only method for tracking
cancelling separate from the enum?


Well I guess we could just report it; but would that break any external tools?


It might - I seem to recall in the past that when we added a new state
string, that at least libvirt choked when encountering the unknown
string (but I don't recall if it was migration or something else).  At
least libvirt already has an enum tracking:

enum {
 QEMU_MONITOR_MIGRATION_STATUS_INACTIVE,
 QEMU_MONITOR_MIGRATION_STATUS_ACTIVE,
 QEMU_MONITOR_MIGRATION_STATUS_COMPLETED,
 QEMU_MONITOR_MIGRATION_STATUS_ERROR,
 QEMU_MONITOR_MIGRATION_STATUS_CANCELLED,
 QEMU_MONITOR_MIGRATION_STATUS_SETUP,

 QEMU_MONITOR_MIGRATION_STATUS_LAST
};

and a string mapping of just the following states:

VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
   QEMU_MONITOR_MIGRATION_STATUS_LAST,
   "inactive", "active", "completed", "failed", "cancelled",
"setup")

Furthermore, the function qemuMigrationUpdateJobStatus() is doing
various things depending on the observed state, where the current trick
of treating 'cancelling' like 'active' would mean that changing
'cancelling' to be an independent state WOULD have observable behavior
change in libvirt.  But I don't know if the change would break things,


Er, i have tested with returning 'cancelling' to users, and
only when we try to cancel a migration, libvirt sometimes will report :
Migration: [ 69 %]^Cerror: internal error: unexpected migration status in 
cancelling.
But the cancelling process is still completed.

And, yes, it is very rare, depend on the time window. (In my test, i add a 
sleep of 5s
to extend the time between cancelling and cancelled.)


or if it would still end up resolving nicely (after all, cancelling only
occurs for a short window before the migration aborts anyway, so it
might just sort itself out when it finally gets to cancelled).




On the other hand, we can argue that clients that are unprepared to
handle new enum states gracefully are broken, and we also have the
argument that it is okay for a new qemu to require a new libvirt release
(the other direction is not okay - a new libvirt must not require


Agreed, upgrade libvirt is reasonable.
So, should i send v3 with exposing 'cancelling'to user, and CC libvirt ?

Thanks.


upgrading to a new qemu).  So exposing 'cancelling' may make this patch
easier.

Re: [Qemu-devel] [PATCH v4 05/11] block: Move BDS close notifiers into BB

[Fam Zheng]

On Fri, 02/27 11:43, Max Reitz wrote:
>  static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState 
> *dev,
>  Error **errp)
>  {
> @@ -763,12 +794,26 @@ static void virtio_scsi_hotplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>  SCSIDevice *sd = SCSI_DEVICE(dev);
>  
>  if (s->ctx && !s->dataplane_disabled) {
> +VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier;
> +
> +insert_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1);
> +insert_notifier->n.notify = virtio_scsi_blk_insert_notifier;
> +insert_notifier->s = s;
> +insert_notifier->sd = sd;
> +blk_add_insert_bs_notifier(sd->conf.blk, &insert_notifier->n);
> +QTAILQ_INSERT_TAIL(&s->insert_notifiers, insert_notifier, next);

Could you instead embed a Notifier into SCSIDevice, similarly? That way there
is no need to maintain a list in VirtIOSCSI.

> +
> +remove_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1);
> +remove_notifier->n.notify = virtio_scsi_blk_remove_notifier;
> +remove_notifier->s = s;
> +remove_notifier->sd = sd;
> +blk_add_remove_bs_notifier(sd->conf.blk, &remove_notifier->n);
> +QTAILQ_INSERT_TAIL(&s->remove_notifiers, remove_notifier, next);
> +
>  if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
>  return;
>  }
> -assert(!s->blocker);
> -error_setg(&s->blocker, "block device is in use by data plane");
> -blk_op_block_all(sd->conf.blk, s->blocker);
> +virtio_scsi_set_up_op_blockers(s, sd);
>  }

Fam

Re: [Qemu-devel] [PATCH v2] migration: Convert 'status' of MigrationInfo to use an enum type

[zhanghailiang]

On 2015/2/28 0:48, Eric Blake wrote:

On 02/26/2015 11:19 PM, zhanghailiang wrote:

The original 'status' is an open-coded 'str' type, convert it to use an
enum type.
This conversion is backwards compatible, better documented and
more convenient for future extensibility.

In addition, Fix a typo for qapi-schema.json: comppleted -> completed

Signed-off-by: zhanghailiang 
---
v2:
- Remove '(since xyz)' strings. (Eric Blake)
---
  hmp.c |  7 ---
  migration/migration.c | 37 +++--
  qapi-schema.json  | 37 -
  3 files changed, 51 insertions(+), 30 deletions(-)



  case MIG_STATE_ACTIVE:
  case MIG_STATE_CANCELLING:
  info->has_status = true;
-info->status = g_strdup("active");
+/* Note: when the real state of migration is 'cancelling',
+ we still return 'active' status to user, it makes no difference
+ for user. */


Rather than pollute the user-exposed enum with a state that we will
never report, can we come up with some internal-only method for tracking
cancelling separate from the enum?



+++ b/qapi-schema.json
@@ -411,18 +411,45 @@
 'overflow': 'int' } }

  ##
+# @MigState:


Do we have to abbreviate?  I guess leaving it like this makes the rest
of the existing code base have less churn (since it matches the spelling
of the enum that was previous interanl only), but it might look nicer as


Yes, this is the reason ..., agreed, i don't like the abbreviate,
But there is already a 'MigrationState' type defined:

typedef struct MigrationState MigrationState;

struct MigrationState
{
int64_t bandwidth_limit;
size_t bytes_xfer;
size_t xfer_limit;
QemuThread thread;
QEMUBH *cleanup_bh;
QEMUFile *file;

...

So, what about MigrationStatus ? ;)


MigrationState.  If you do decide to go with a longer name, it might be
nice to split this into a series, one patch that does only renames to
migration.c (but no code additions outside of that file), and the other
that moves the (now-correctly-named) enum into the public qapi file.


+#
+# An enumeration of migration status.
+#
+# @failed: some error occurred during migration process.
+#
+# @none: no migration has ever happened.
+#
+# @setup: migration process has been initiated.
+#
+# @cancelling: in the process of cancelling migration.
+#


If the user can never see this state, I'd rather we think about a
solution that avoids advertising the state in the public api...


+# @cancelled: cancelling migration is finished.
+#
+# @active: in the process of doing migration.
+#
+# @completed: migration is finished.
+#
+# Since: 2.3
+#
+# Notes: @cancelling is only used internally, and return @active to user
+#instead of @cancelling, it make no difference for users.


...rather than needing this note to air our dirty laundry.

Re: [Qemu-devel] [PATCH 1/6 v4] target-tilegx: Firstly add to qemu with minimized features

[Chris Metcalf]

On 2/27/2015 9:10 PM, Chen Gang S wrote:

By the way, does Gx8000 mean 8000 core count?


It's marketing, so it doesn't have to mean anything.  The TILE-Gx 8036 is the 36-core 
part, the 8072 is the 72-core part, 8009 is the 9-core part, etc.  There is no 
"8000" chip, it's just a way of describing the TILE-Gx chip series.

--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

Re: [Qemu-devel] [PATCH 1/6 v4] target-tilegx: Firstly add to qemu with minimized features

[Chen Gang S]

On 2/28/15 05:49, Chris Metcalf wrote:
> On 2/27/2015 12:36 PM, Andreas Färber wrote:
>> "TILE-Gx" according to
>> http://www.tilera.com/products/?ezchip=585&spage=614  - please fix
>> wherever used in textual form.
> 
> Yes, really only "tilegx" or "TILE-Gx" should be used.  In type names I agree 
> that TileGX is probably clearer than any alternatives.
> 

OK, thanks. I shall use TILE-Gx for it.

And next, for the type names, I shall use TileGX.

>> What about CPU models? Do the Gx72, Gx36, etc. differ only in core count
>> or also in instruction set features?
> 
> From a userspace perspective, it's mostly the core count.  They also have a 
> different set of on-chip hardware accelerators, etc., but that's out of scope 
> for this project.
> 

OK, thanks. I guess your meaning is "For qemu, we needn't consider about
Gx72, Gx36 ..., we can treat all of them as TILE-Gx".

By the way, does Gx8000 mean 8000 core count?

Thanks.
-- 
Chen Gang

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

Re: [Qemu-devel] [PATCH] bootdevice: fix segment fault when booting guest with '-kernel' and '-initrd'

[Gonglei]

On 2015/2/27 22:15, Paolo Bonzini wrote:
> Looks good, I am applying it locally so that it can get into qemu.git
> very soon.
> 
OK, thanks.

Regards,
-Gonglei
> Paolo
> 
> On 27/02/2015 02:49, arei.gong...@huawei.com wrote:
>> From: Gonglei 
>>
>> Reproducer:
>>
>>  $./qemu-system-x86_64 --enable-kvm -kernel 
>> /home/vmlinuz-2.6.32.12-0.7-default \
>>   -initrd /home/initrd-2.6.32.12-0.7-default -append \
>>  "root=/dev/ram rw console=ttyS0,115200" -dtb guest.dtb -vnc :10 --monitor 
>> stdio -smp 2
>> QEMU 2.2.50 monitor - type 'help' for more information
>> (qemu) Segmentation fault (core dumped)
>>
>> Reported-by: Edivaldo de Araujo Pereira 
>> Signed-off-by: Gonglei 
>> ---
>>  bootdevice.c | 13 +
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/bootdevice.c b/bootdevice.c
>> index df9ab0e..673bfef 100644
>> --- a/bootdevice.c
>> +++ b/bootdevice.c
>> @@ -223,10 +223,15 @@ char *get_boot_devices_list(size_t *size, bool 
>> ignore_suffixes)
>>  }
>>  
>>  if (!ignore_suffixes) {
>> -d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus, 
>> i->dev);
>> -if (d) {
>> -assert(!i->suffix);
>> -suffix = d;
>> +if (i->dev) {
>> +d = 
>> qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus,
>> +  i->dev);
>> +if (d) {
>> +assert(!i->suffix);
>> +suffix = d;
>> +} else {
>> +suffix = g_strdup(i->suffix);
>> +}
>>  } else {
>>  suffix = g_strdup(i->suffix);
>>  }
>>

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Fam Zheng]

On Fri, 02/27 19:40, Paolo Bonzini wrote:
> 
> 
> On 19/02/2015 08:48, Michael S. Tsirkin wrote:
> > V=1 should show what's going on, it's not nice
> > to silence things unconditionally.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  Makefile | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 6817c6f..84ca8be 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -110,7 +110,7 @@ endif
> >  
> >  %/config-devices.mak: default-configs/%.mak
> > $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh 
> > $@ $<, "  GEN   $@")
> > -   @if test -f $@; then \
> > +   $(call quiet-command, if test -f $@; then \
> >   if cmp -s $@.old $@; then \
> > mv $@.tmp $@; \
> > cp -p $@ $@.old; \
> > @@ -126,7 +126,7 @@ endif
> >  else \
> >   mv $@.tmp $@; \
> >   cp -p $@ $@.old; \
> > -fi
> > +fi, "  TEST $@");
> >  
> >  defconfig:
> > rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> > 
> 
> Squashing this to make the non-verbose messages clearer, ok?
> 
> diff --git a/Makefile b/Makefile
> index 5604209..d92d4cd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -109,7 +109,7 @@ endif
>  -include $(SUBDIR_DEVICES_MAK_DEP)
>  
>  %/config-devices.mak: default-configs/%.mak
> - $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh 
> $@ $<, "  GEN   $@")
> + $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh 
> $@.tmp $<, "  GEN   $@.tmp")
>   $(call quiet-command, if test -f $@; then \
> if cmp -s $@.old $@; then \
>   mv $@.tmp $@; \
> @@ -126,7 +126,7 @@ endif
>else \
> mv $@.tmp $@; \
> cp -p $@ $@.old; \
> -  fi, "  TEST $@");
> +  fi, "  GEN  $@");
>  
>  defconfig:
>   rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh
> index 7242707..7958086 100644
> --- a/scripts/make_device_config.sh
> +++ b/scripts/make_device_config.sh
> @@ -2,7 +2,7 @@
>  # Construct a target device config file from a default, pulling in any
>  # files from include directives.
>  
> -dest=$1.tmp
> +dest=$1
>  dep=`dirname $1`-`basename $1`.d
>  src=$2
>  src_dir=`dirname $src`

Looks good to me.

Fam

[Qemu-devel] [PATCH RESEND 17/17] iotests: add incremental backup failure recovery test

[John Snow]

To test the failure case, we modify iotests.py to allow us to specify
that we'd like to allow failures when we wait for block job events.

Reviewed-by: Max Reitz 
Signed-off-by: John Snow 
---
 tests/qemu-iotests/124| 57 ++-
 tests/qemu-iotests/124.out|  4 +--
 tests/qemu-iotests/iotests.py |  6 +++--
 3 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index da8f0e0..1c07387 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -135,7 +135,11 @@ class TestIncrementalBackup(iotests.QMPTestCase):
  mode='existing')
 self.assert_qmp(result, 'return', {})
 
-event = self.wait_until_completed(bitmap.node, check_offset=validate)
+event = self.wait_until_completed(bitmap.node, check_offset=validate,
+  allow_failures=(not validate))
+if 'error' in event['data']:
+bitmap.del_target()
+return False
 if validate:
 return self.check_incremental(target)
 
@@ -180,6 +184,57 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 return True
 
 
+def test_incremental_failure(self):
+'''Test: Verify backups made after a failure are correct.
+
+Simulate a failure during an incremental backup block job,
+emulate additional writes, then create another incremental backup
+afterwards and verify that the backup created is correct.
+'''
+
+# Create a blkdebug interface to this img as 'drive1'
+result = self.vm.qmp('blockdev-add', options={
+'id': 'drive1',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'blkdebug',
+'image': {
+'driver': 'file',
+'filename': self.test_img
+},
+'set-state': [{
+'event': 'flush_to_disk',
+'state': 1,
+'new_state': 2
+}],
+'inject-error': [{
+'event': 'read_aio',
+'errno': 5,
+'state': 2,
+'immediately': False,
+'once': True
+}],
+}
+})
+self.assert_qmp(result, 'return', {})
+
+self.create_full_backup()
+self.add_bitmap('bitmap0', 'drive1')
+# Note: at this point, during a normal execution,
+# Assume that the VM resumes and begins issuing IO requests here.
+
+self.hmp_io_writes('drive1', (('0xab', 0, 512),
+  ('0xfe', '16M', '256k'),
+  ('0x64', '32736k', '64k')))
+
+result = self.create_incremental(validate=False)
+self.assertFalse(result)
+self.hmp_io_writes('drive1', (('0x9a', 0, 512),
+  ('0x55', '8M', '352k'),
+  ('0x78', '15872k', '1M')))
+self.create_incremental()
+
+
 def test_sync_dirty_bitmap_missing(self):
 self.assert_no_active_block_jobs()
 self.files.append(self.foo_img)
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 8d7e996..89968f3 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-...
+
 --
-Ran 3 tests
+Ran 4 tests
 
 OK
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1402854..0522501 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -273,14 +273,16 @@ class QMPTestCase(unittest.TestCase):
 self.assert_no_active_block_jobs()
 return result
 
-def wait_until_completed(self, drive='drive0', check_offset=True):
+def wait_until_completed(self, drive='drive0', check_offset=True,
+ allow_failures=False):
 '''Wait for a block job to finish, returning the event'''
 completed = False
 while not completed:
 for event in self.vm.get_qmp_events(wait=True):
 if event['event'] == 'BLOCK_JOB_COMPLETED':
 self.assert_qmp(event, 'data/device', drive)
-self.assert_qmp_absent(event, 'data/error')
+if not allow_failures:
+self.assert_qmp_absent(event, 'data/error')
 if check_offset:
 self.assert_qmp(event, 'data/offset', 
event['data']['len'])
 completed = True
-- 
1.9.3

[Qemu-devel] [PATCH RESEND 13/17] block: Ensure consistent bitmap function prototypes

[John Snow]

We often don't need the BlockDriverState for functions
that operate on bitmaps. Remove it.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
---
 block.c   | 13 ++---
 block/backup.c|  2 +-
 block/mirror.c| 26 ++
 blockdev.c|  2 +-
 include/block/block.h | 11 +--
 migration/block.c |  7 +++
 6 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/block.c b/block.c
index 55243f9..e6b2696 100644
--- a/block.c
+++ b/block.c
@@ -5429,7 +5429,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState 
*bs, const char *name)
 return NULL;
 }
 
-void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
 {
 assert(!bdrv_dirty_bitmap_frozen(bitmap));
 g_free(bitmap->name);
@@ -5598,7 +5598,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
 BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
 BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
-info->count = bdrv_get_dirty_count(bs, bm);
+info->count = bdrv_get_dirty_count(bm);
 info->granularity = bdrv_dirty_bitmap_granularity(bm);
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
@@ -5646,20 +5646,19 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap 
*bitmap)
 return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
 }
 
-void bdrv_dirty_iter_init(BlockDriverState *bs,
-  BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
+void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
 {
 hbitmap_iter_init(hbi, bitmap->bitmap, 0);
 }
 
-void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
 {
 assert(bdrv_dirty_bitmap_enabled(bitmap));
 hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
-void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
  int64_t cur_sector, int nr_sectors)
 {
 assert(bdrv_dirty_bitmap_enabled(bitmap));
@@ -5705,7 +5704,7 @@ void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
 hbitmap_iter_init(hbi, hbi->hb, offset);
 }
 
-int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
 {
 return hbitmap_count(bitmap->bitmap);
 }
diff --git a/block/backup.c b/block/backup.c
index d46c0a3..41bd9af 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -313,7 +313,7 @@ static void coroutine_fn backup_run(void *opaque)
 int64_t last_cluster = -1;
 bool polyrhythmic;
 
-bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
+bdrv_dirty_iter_init(job->sync_bitmap, &hbi);
 /* Does the granularity happen to match our backup cluster size? */
 polyrhythmic = (bdrv_dirty_bitmap_granularity(job->sync_bitmap) !=
 BACKUP_CLUSTER_SIZE);
diff --git a/block/mirror.c b/block/mirror.c
index f89eccf..dcd6f65 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -125,11 +125,9 @@ static void mirror_write_complete(void *opaque, int ret)
 MirrorOp *op = opaque;
 MirrorBlockJob *s = op->s;
 if (ret < 0) {
-BlockDriverState *source = s->common.bs;
 BlockErrorAction action;
 
-bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
-  op->nb_sectors);
+bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sectors);
 action = mirror_error_action(s, false, -ret);
 if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
 s->ret = ret;
@@ -143,11 +141,9 @@ static void mirror_read_complete(void *opaque, int ret)
 MirrorOp *op = opaque;
 MirrorBlockJob *s = op->s;
 if (ret < 0) {
-BlockDriverState *source = s->common.bs;
 BlockErrorAction action;
 
-bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
-  op->nb_sectors);
+bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sectors);
 action = mirror_error_action(s, true, -ret);
 if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
 s->ret = ret;
@@ -170,10 +166,9 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
 
 s->sector_num = hbitmap_iter_next(&s->hbi);
 if (s->sector_num < 0) {
-bdrv_dirty_iter_init(source, s->dirty_bitmap, &s->hbi);
+bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
 s->sector_num = hbitmap_iter_next(&s->hbi);
-trace_mirror_restart_iter(s,
-  bdrv_get_dirty_count(source, 
s->dirty_bitmap));
+  

[Qemu-devel] [PATCH RESEND 16/17] iotests: add simple incremental backup case

[John Snow]

Reviewed-by: Max Reitz 
Signed-off-by: John Snow 
---
 tests/qemu-iotests/124 | 122 +
 tests/qemu-iotests/124.out |   4 +-
 2 files changed, 124 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 7985cd1..da8f0e0 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -28,6 +28,36 @@ def io_write_patterns(img, patterns):
 for pattern in patterns:
 iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
 
+class Bitmap:
+def __init__(self, name, node):
+self.name = name
+self.node = node
+self.pattern = os.path.join(iotests.test_dir.replace('%', '%%'),
+'%s.backup.%%i.img' % name)
+self.num = 0
+self.backups = list()
+
+def new_target(self, num=None):
+if num is None:
+num = self.num
+self.num = num + 1
+target = self.pattern % num
+self.backups.append(target)
+return target
+
+def last_target(self):
+return self.backups[-1]
+
+def del_target(self):
+os.remove(self.backups.pop())
+self.num -= 1
+
+def __del__(self):
+for backup in self.backups:
+try:
+os.remove(backup)
+except OSError:
+pass
 
 class TestIncrementalBackup(iotests.QMPTestCase):
 def setUp(self):
@@ -58,6 +88,98 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 iotests.qemu_img('create', '-f', fmt, img, size)
 self.files.append(img)
 
+
+def create_full_backup(self, drive='drive0'):
+res = self.vm.qmp('drive-backup', device=drive,
+  sync='full', format=iotests.imgfmt,
+  target=self.full_bak)
+self.assert_qmp(res, 'return', {})
+self.wait_until_completed(drive)
+self.check_full_backup()
+self.files.append(self.full_bak)
+
+
+def check_full_backup(self):
+self.assertTrue(iotests.compare_images(self.test_img, self.full_bak))
+
+
+def add_bitmap(self, name, node='drive0'):
+bitmap = Bitmap(name, node)
+self.bitmaps.append(bitmap)
+result = self.vm.qmp('block-dirty-bitmap-add', node=bitmap.node,
+ name=bitmap.name)
+self.assert_qmp(result, 'return', {})
+return bitmap
+
+
+def create_incremental(self, bitmap=None, num=None,
+   parent=None, parentFormat=None, validate=True):
+if bitmap is None:
+bitmap = self.bitmaps[-1]
+
+# If this is the first incremental backup for a bitmap,
+# use the full backup as a backing image. Otherwise, use
+# the last incremental backup.
+if parent is None:
+if bitmap.num == 0:
+parent = self.full_bak
+else:
+parent = bitmap.last_target()
+
+target = bitmap.new_target(num)
+self.img_create(target, iotests.imgfmt, parent=parent)
+
+result = self.vm.qmp('drive-backup', device=bitmap.node,
+ sync='dirty-bitmap', bitmap=bitmap.name,
+ format=iotests.imgfmt, target=target,
+ mode='existing')
+self.assert_qmp(result, 'return', {})
+
+event = self.wait_until_completed(bitmap.node, check_offset=validate)
+if validate:
+return self.check_incremental(target)
+
+
+def check_incremental(self, target=None):
+if target is None:
+target = self.bitmaps[-1].last_target()
+self.assertTrue(iotests.compare_images(self.test_img, target))
+return True
+
+
+def hmp_io_writes(self, drive, patterns):
+for pattern in patterns:
+self.vm.hmp_qemu_io(drive, 'write -P%s %s %s' % pattern)
+self.vm.hmp_qemu_io(drive, 'flush')
+
+
+def test_incremental_simple(self):
+'''
+Test: Create and verify three incremental backups.
+
+Create a bitmap and a full backup before VM execution begins,
+then create a series of three incremental backups "during execution,"
+i.e.; after IO requests begin modifying the drive.
+'''
+self.create_full_backup()
+self.add_bitmap('bitmap0', 'drive0')
+
+# Sanity: Create a "hollow" incremental backup
+self.create_incremental()
+# Three writes: One complete overwrite, one new segment,
+# and one partial overlap.
+self.hmp_io_writes('drive0', (('0xab', 0, 512),
+  ('0xfe', '16M', '256k'),
+  ('0x64', '32736k', '64k')))
+self.create_incremental()
+# Three more writes, one of each kind, like above
+self.hmp_io_writes('drive0', (('0x9a', 0, 512),
+  ('0x55', '8M', '352k'),
+  

[Qemu-devel] [PATCH RESEND 09/17] qmp: Add support of "dirty-bitmap" sync mode for drive-backup

[John Snow]

For "dirty-bitmap" sync mode, the block job will iterate through the
given dirty bitmap to decide if a sector needs backup (backup all the
dirty clusters and skip clean ones), just as allocation conditions of
"top" sync mode.

Signed-off-by: Fam Zheng 
Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
---
 block.c   |   9 +++
 block/backup.c| 149 +++---
 block/mirror.c|   4 ++
 blockdev.c|  18 +-
 hmp.c |   3 +-
 include/block/block.h |   1 +
 include/block/block_int.h |   2 +
 qapi/block-core.json  |  13 ++--
 qmp-commands.hx   |   7 ++-
 9 files changed, 172 insertions(+), 34 deletions(-)

diff --git a/block.c b/block.c
index e8f66a6..3ecc2dd 100644
--- a/block.c
+++ b/block.c
@@ -5686,6 +5686,15 @@ static void bdrv_reset_dirty(BlockDriverState *bs, 
int64_t cur_sector,
 }
 }
 
+/**
+ * Advance an HBitmapIter to an arbitrary offset.
+ */
+void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
+{
+assert(hbi->hb);
+hbitmap_iter_init(hbi, hbi->hb, offset);
+}
+
 int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
 return hbitmap_count(bitmap->bitmap);
diff --git a/block/backup.c b/block/backup.c
index 1c535b1..d46c0a3 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,6 +37,8 @@ typedef struct CowRequest {
 typedef struct BackupBlockJob {
 BlockJob common;
 BlockDriverState *target;
+/* bitmap for sync=dirty-bitmap */
+BdrvDirtyBitmap *sync_bitmap;
 MirrorSyncMode sync_mode;
 RateLimit limit;
 BlockdevOnError on_source_error;
@@ -242,6 +244,31 @@ static void backup_complete(BlockJob *job, void *opaque)
 g_free(data);
 }
 
+static bool coroutine_fn yield_and_check(BackupBlockJob *job)
+{
+if (block_job_is_cancelled(&job->common)) {
+return true;
+}
+
+/* we need to yield so that qemu_aio_flush() returns.
+ * (without, VM does not reboot)
+ */
+if (job->common.speed) {
+uint64_t delay_ns = ratelimit_calculate_delay(&job->limit,
+  job->sectors_read);
+job->sectors_read = 0;
+block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
+} else {
+block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0);
+}
+
+if (block_job_is_cancelled(&job->common)) {
+return true;
+}
+
+return false;
+}
+
 static void coroutine_fn backup_run(void *opaque)
 {
 BackupBlockJob *job = opaque;
@@ -254,13 +281,13 @@ static void coroutine_fn backup_run(void *opaque)
 };
 int64_t start, end;
 int ret = 0;
+bool error_is_read;
 
 QLIST_INIT(&job->inflight_reqs);
 qemu_co_rwlock_init(&job->flush_rwlock);
 
 start = 0;
-end = DIV_ROUND_UP(job->common.len / BDRV_SECTOR_SIZE,
-   BACKUP_SECTORS_PER_CLUSTER);
+end = DIV_ROUND_UP(job->common.len, BACKUP_CLUSTER_SIZE);
 
 job->bitmap = hbitmap_alloc(end, 0);
 
@@ -278,28 +305,62 @@ static void coroutine_fn backup_run(void *opaque)
 qemu_coroutine_yield();
 job->common.busy = true;
 }
+} else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+/* Dirty Bitmap sync has a slightly different iteration method */
+HBitmapIter hbi;
+int64_t sector;
+int64_t cluster;
+int64_t last_cluster = -1;
+bool polyrhythmic;
+
+bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
+/* Does the granularity happen to match our backup cluster size? */
+polyrhythmic = (bdrv_dirty_bitmap_granularity(job->sync_bitmap) !=
+BACKUP_CLUSTER_SIZE);
+
+/* Find the next dirty /sector/ and copy that /cluster/ */
+while ((sector = hbitmap_iter_next(&hbi)) != -1) {
+cluster = sector / BACKUP_SECTORS_PER_CLUSTER;
+
+/* Fake progress updates for any clusters we skipped,
+ * excluding this current cluster. */
+if (cluster != last_cluster + 1) {
+job->common.offset += ((cluster - last_cluster - 1) *
+   BACKUP_CLUSTER_SIZE);
+}
+
+if (yield_and_check(job)) {
+goto leave;
+}
+
+do {
+ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER,
+BACKUP_SECTORS_PER_CLUSTER, 
&error_is_read);
+if ((ret < 0) &&
+backup_error_action(job, error_is_read, -ret) ==
+BLOCK_ERROR_ACTION_REPORT) {
+goto leave;
+}
+} while (ret < 0);
+
+/* Advance (or rewind) our iterator if we need to. */
+if (polyrhythmic) {
+bdrv_set_dirty_iter(&hbi,
+(cluster + 1) * 
BACKUP_SECTORS_PER_CLUSTER);

[Qemu-devel] [PATCH RESEND 12/17] block: add BdrvDirtyBitmap documentation

[John Snow]

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
---
 block.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 06b4264..55243f9 100644
--- a/block.c
+++ b/block.c
@@ -60,11 +60,11 @@
  * or enabled. A frozen bitmap can only abdicate() or reclaim().
  */
 struct BdrvDirtyBitmap {
-HBitmap *bitmap;
-BdrvDirtyBitmap *successor;
-int64_t size;
-char *name;
-bool disabled;
+HBitmap *bitmap;/* Dirty sector bitmap implementation */
+BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
+char *name; /* Optional non-empty unique ID */
+int64_t size;   /* Size of the bitmap (Number of sectors) */
+bool disabled;  /* Bitmap is read-only */
 QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
-- 
1.9.3

[Qemu-devel] [PATCH RESEND 10/17] qmp: add block-dirty-bitmap-clear

[John Snow]

Add bdrv_clear_dirty_bitmap and a matching QMP command,
qmp_block_dirty_bitmap_clear that enables a user to reset
the bitmap attached to a drive.

This allows us to reset a bitmap in the event of a full
drive backup.

Reviewed-by: Max Reitz 
Signed-off-by: John Snow 
---
 block.c   |  8 
 blockdev.c| 37 +
 include/block/block.h |  1 +
 qapi/block-core.json  | 14 ++
 qmp-commands.hx   | 29 +
 5 files changed, 89 insertions(+)

diff --git a/block.c b/block.c
index 3ecc2dd..d969b24 100644
--- a/block.c
+++ b/block.c
@@ -62,6 +62,7 @@
 struct BdrvDirtyBitmap {
 HBitmap *bitmap;
 BdrvDirtyBitmap *successor;
+int64_t size;
 char *name;
 bool disabled;
 QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -5460,6 +5461,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 }
 bitmap = g_new0(BdrvDirtyBitmap, 1);
 bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
+bitmap->size = bitmap_size;
 bitmap->name = g_strdup(name);
 bitmap->disabled = false;
 QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
@@ -5662,6 +5664,12 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap,
 hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+assert(bdrv_dirty_bitmap_enabled(bitmap));
+hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
+}
+
 static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
int nr_sectors)
 {
diff --git a/blockdev.c b/blockdev.c
index b432736..eaca97d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2050,6 +2050,43 @@ void qmp_block_dirty_bitmap_remove(const char *node, 
const char *name,
 aio_context_release(aio_context);
 }
 
+/**
+ * Completely clear a bitmap, for the purposes of synchronizing a bitmap
+ * immediately after a full backup operation.
+ */
+void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
+  Error **errp)
+{
+AioContext *aio_context;
+BdrvDirtyBitmap *bitmap;
+BlockDriverState *bs;
+
+bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
+if (!bitmap) {
+return;
+}
+
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently frozen and cannot be modified",
+   name);
+goto out;
+} else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently disabled and cannot be cleared",
+   name);
+goto out;
+}
+
+bdrv_clear_dirty_bitmap(bitmap);
+
+ out:
+aio_context_release(aio_context);
+}
+
 int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index e2d8331..ca8e91a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -479,6 +479,7 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
 void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
  int64_t cur_sector, int nr_sectors);
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_iter_init(BlockDriverState *bs,
   BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
 void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f012de2..50970c4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1023,6 +1023,20 @@
   'data': 'BlockDirtyBitmap' }
 
 ##
+# @block-dirty-bitmap-clear
+#
+# Clear (reset) a dirty bitmap on the device
+#
+# Returns: nothing on success
+#  If @node is not a valid block device, DeviceNotFound
+#  If @name is not found, GenericError with an explanation
+#
+# Since 2.4
+##
+{ 'command': 'block-dirty-bitmap-clear',
+  'data': 'BlockDirtyBitmap' }
+
+##
 # @block_set_io_throttle:
 #
 # Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a605d88..3389fff 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1301,6 +1301,35 @@ Example:
 EQMP
 
 {
+.name   = "block-dirty-bitmap-clear",
+.args_type  = "node:B,name:s",
+.mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_clear,
+},
+
+SQMP
+
+block-dirty-bitmap-clear
+
+Since 2.4
+
+Reset the dirty bitmap associated with a node so that an incremental backup
+from this point in time forward will only backup clusters modified after this
+clear operation.
+
+Arguments:
+
+- "node": device/

[Qemu-devel] [PATCH RESEND 01/17] docs: incremental backup documentation

[John Snow]

Signed-off-by: John Snow 
---
 docs/bitmaps.md | 303 
 1 file changed, 303 insertions(+)
 create mode 100644 docs/bitmaps.md

diff --git a/docs/bitmaps.md b/docs/bitmaps.md
new file mode 100644
index 000..ebb6ae8
--- /dev/null
+++ b/docs/bitmaps.md
@@ -0,0 +1,303 @@
+# Dirty Bitmaps
+
+* Dirty bitmaps can be created at any time and attached to any node
+(not just complete drives.)
+
+## Dirty Bitmap Names
+
+* A dirty bitmap's name is unique to the node, but bitmaps attached to 
different
+nodes can share the same name.
+
+## Bitmap Modes
+
+* A Bitmap can be "enabled" (tracking writes, the default) or "disabled"
+(read-only, I/O is ignored.) This state is currently only changed internally
+for the purposes of migration, and otherwise remains enabled.
+
+* A Bitmap can be "frozen," which means that it is currently in-use by a backup
+operation and cannot be deleted, enabled, disabled, renamed, written to, reset,
+etc.
+
+## Basic QMP Usage
+
+### Supported Commands ###
+
+* block-dirty-bitmap-add
+* block-dirty-bitmap-remove
+* block-dirty-bitmap-clear
+
+### Creation
+
+* To create a new bitmap, enabled, on the drive with id=drive0:
+
+```json
+{ "execute": "block-dirty-bitmap-add",
+  "arguments": {
+"node": "drive0",
+"name": "bitmap0"
+  }
+}
+```
+
+* This bitmap will have a default granularity that matches the cluster size of
+its associated drive, if available, clamped to between [4KiB, 64KiB].
+The current default for qcow2 is 64KiB.
+
+* To create a new bitmap that tracks changes in 32KiB segments:
+
+```json
+{ "execute": "block-dirty-bitmap-add",
+  "arguments": {
+"node": "drive0",
+"name": "bitmap0",
+"granularity": 32768
+  }
+}
+```
+
+### Deletion
+
+* Can be performed on a disabled bitmap, but not a frozen one.
+
+* Because bitmaps are only unique to the node to which they are attached,
+you must specify the node/drive name here, too.
+
+```json
+{ "execute": "block-dirty-bitmap-remove",
+  "arguments": {
+"node": "drive0",
+"name": "bitmap0"
+  }
+}
+```
+
+### Resetting
+
+* Resetting a bitmap will clear all information it holds.
+* An incremental backup created from an empty bitmap will copy no data,
+as if nothing has changed.
+
+```json
+{ "execute": "block-dirty-bitmap-clear",
+  "arguments": {
+"node": "drive0",
+"name": "bitmap0"
+  }
+}
+```
+
+## Transactions (Not yet implemented)
+
+* Transactional commands are forthcoming in a future version,
+  and are not yet available for use. This section serves as
+  documentation of intent for their design and usage.
+
+### Justification
+Bitmaps can be safely modified when the VM is paused or halted by using
+the basic QMP commands. For instance, you might perform the following actions:
+
+1. Boot the VM in a paused state.
+2. Create a full drive backup of drive0.
+3. Create a new bitmap attached to drive0.
+4. Resume execution of the VM.
+5. Incremental backups are ready to be created.
+
+At this point, the bitmap and drive backup would be correctly in sync,
+and incremental backups made from this point forward would be correctly aligned
+to the full drive backup.
+
+This is not particularly useful if we decide we want to start incremental
+backups after the VM has been running for a while, for which we will need to
+perform actions such as the following:
+
+1. Boot the VM and begin execution.
+2. Using a single transaction, perform the following operations:
+* Create bitmap0.
+* Create a full drive backup of drive0.
+3. Incremental backups are now ready to be created.
+
+### Supported Bitmap Transactions
+
+* block-dirty-bitmap-add
+* block-dirty-bitmap-clear
+
+The usages are identical to their respective QMP commands, but see below
+for examples.
+
+### Example: New Incremental Backup
+
+As outlined in the justification, perhaps we want to create a new incremental
+backup chain attached to a drive.
+
+```json
+{ "execute": "transaction",
+  "arguments": {
+"actions": [
+  {"type": "block-dirty-bitmap-add",
+   "data": {"node": "drive0", "name": "bitmap0"} },
+  {"type": "drive-backup",
+   "data": {"device": "drive0", "target": "/path/to/full_backup.img",
+"sync": "full", "format": "qcow2"} }
+]
+  }
+}
+```
+
+### Example: New Incremental Backup Anchor Point
+
+Maybe we just want to create a new full backup with an existing bitmap and
+want to reset the bitmap to track the new chain.
+
+```json
+{ "execute": "transaction",
+  "arguments": {
+"actions": [
+  {"type": "block-dirty-bitmap-clear",
+   "data": {"node": "drive0", "name": "bitmap0"} },
+  {"type": "drive-backup",
+   "data": {"device": "drive0", "target": "/path/to/new_full_backup.img",
+"sync": "full", "format": "qcow2"} }
+]
+  }
+}
+```
+
+## Incremental Backups
+
+The star of the show.
+
+**Nota Bene!** Only incremental backups of entire drives are supported for now.
+So despite the fact 

[Qemu-devel] [PATCH RESEND 14/17] block: Resize bitmaps on bdrv_truncate

[John Snow]

Signed-off-by: John Snow 
---
 block.c| 22 
 include/block/block.h  |  1 +
 include/qemu/hbitmap.h | 10 ++
 util/hbitmap.c | 54 ++
 4 files changed, 87 insertions(+)

diff --git a/block.c b/block.c
index e6b2696..5eaa874 100644
--- a/block.c
+++ b/block.c
@@ -3543,6 +3543,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
 ret = drv->bdrv_truncate(bs, offset);
 if (ret == 0) {
 ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+bdrv_dirty_bitmap_truncate(bs);
 if (bs->blk) {
 blk_dev_resize_cb(bs->blk);
 }
@@ -5562,6 +5563,27 @@ BdrvDirtyBitmap 
*bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
 return parent;
 }
 
+static void dirty_bitmap_truncate(BdrvDirtyBitmap *bitmap, uint64_t size)
+{
+/* Should only be frozen during a block backup job, which should have
+ * blocked any resize actions. */
+assert(!bdrv_dirty_bitmap_frozen(bitmap));
+hbitmap_truncate(bitmap->bitmap, size);
+}
+
+void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
+{
+BdrvDirtyBitmap *bitmap;
+uint64_t size = bdrv_nb_sectors(bs);
+
+QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+continue;
+}
+dirty_bitmap_truncate(bitmap, size);
+}
+}
+
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
 BdrvDirtyBitmap *bm, *next;
diff --git a/include/block/block.h b/include/block/block.h
index a8c6369..3a85690 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -454,6 +454,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState 
*bs,
   uint32_t granularity,
   const char *name,
   Error **errp);
+void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
 int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index c19c1cb..a75157e 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,16 @@ struct HBitmapIter {
 HBitmap *hbitmap_alloc(uint64_t size, int granularity);
 
 /**
+ * hbitmap_truncate:
+ * @hb: The bitmap to change the size of.
+ * @size: The number of elements to change the bitmap to accommodate.
+ *
+ * truncate or grow an existing bitmap to accommodate a new number of elements.
+ * This may invalidate existing HBitmapIterators.
+ */
+void hbitmap_truncate(HBitmap *hb, uint64_t size);
+
+/**
  * hbitmap_merge:
  * @a: The bitmap to store the result in.
  * @b: The bitmap to merge into @a.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 962ff29..0934a61 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -90,6 +90,9 @@ struct HBitmap {
  * bitmap will still allocate HBITMAP_LEVELS arrays.
  */
 unsigned long *levels[HBITMAP_LEVELS];
+
+/* The length of each levels[] array. */
+uint64_t sizes[HBITMAP_LEVELS];
 };
 
 /* Advance hbi to the next nonzero word and return it.  hbi->pos
@@ -384,6 +387,7 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
 hb->granularity = granularity;
 for (i = HBITMAP_LEVELS; i-- > 0; ) {
 size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+hb->sizes[i] = size;
 hb->levels[i] = g_new0(unsigned long, size);
 }
 
@@ -396,6 +400,56 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
 return hb;
 }
 
+void hbitmap_truncate(HBitmap *hb, uint64_t size)
+{
+bool truncate;
+unsigned i;
+uint64_t num_elements = size;
+uint64_t old;
+
+/* Size comes in as logical elements, adjust for granularity. */
+size = (size + (1ULL << hb->granularity) - 1) >> hb->granularity;
+assert(size <= ((uint64_t)1 << HBITMAP_LOG_MAX_SIZE));
+truncate = size < hb->size;
+
+if (size == hb->size) {
+/* A hard day's work */
+return;
+}
+
+hb->size = size;
+for (i = HBITMAP_LEVELS; i-- > 0; ) {
+size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+if (hb->sizes[i] == size) {
+continue;
+}
+old = hb->sizes[i];
+hb->sizes[i] = size;
+hb->levels[i] = g_realloc_n(hb->levels[i], size, sizeof(unsigned 
long));
+if (!truncate) {
+memset(&hb->levels[i][old], 0x00, size - old);
+}
+}
+assert(size == 1);
+
+/* Clear out any "extra space" we may have that the user didn't request:
+ * It may have garbage data in it, now. */
+if (truncate) {
+/* Due to granularity fuzziness, we may accidentally reset some of
+ * the last bits that are actually valid. So, record the current value,
+ * reset the "dead range," then

[Qemu-devel] [PATCH RESEND 11/17] qmp: Add dirty bitmap status fields in query-block

[John Snow]

Adds the "disabled" and "frozen" status booleans.

Signed-off-by: Fam Zheng 
Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
---
 block.c  | 2 ++
 qapi/block-core.json | 7 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index d969b24..06b4264 100644
--- a/block.c
+++ b/block.c
@@ -5602,6 +5602,8 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 info->granularity = bdrv_dirty_bitmap_granularity(bm);
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
+info->disabled = bm->disabled;
+info->frozen = bdrv_dirty_bitmap_frozen(bm);
 entry->value = info;
 *plist = entry;
 plist = &entry->next;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 50970c4..7e4e14b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -335,10 +335,15 @@
 #
 # @granularity: granularity of the dirty bitmap in bytes (since 1.4)
 #
+# @disabled: whether the dirty bitmap is disabled (Since 2.4)
+#
+# @frozen: whether the dirty bitmap is frozen (Since 2.4)
+#
 # Since: 1.3
 ##
 { 'type': 'BlockDirtyInfo',
-  'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32'} }
+  'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
+   'disabled': 'bool', 'frozen': 'bool'} }
 
 ##
 # @BlockInfo:
-- 
1.9.3

[Qemu-devel] [PATCH RESEND 15/17] iotests: add invalid input incremental backup tests

[John Snow]

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/124 | 89 ++
 tests/qemu-iotests/124.out |  5 +++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 95 insertions(+)
 create mode 100644 tests/qemu-iotests/124
 create mode 100644 tests/qemu-iotests/124.out

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
new file mode 100644
index 000..7985cd1
--- /dev/null
+++ b/tests/qemu-iotests/124
@@ -0,0 +1,89 @@
+#!/usr/bin/env python
+#
+# Tests for incremental drive-backup
+#
+# Copyright (C) 2015 John Snow for Red Hat, Inc.
+#
+# Based on 056.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import iotests
+
+
+def io_write_patterns(img, patterns):
+for pattern in patterns:
+iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
+
+
+class TestIncrementalBackup(iotests.QMPTestCase):
+def setUp(self):
+self.bitmaps = list()
+self.files = list()
+self.vm = iotests.VM()
+self.test_img = os.path.join(iotests.test_dir, 'base.img')
+self.full_bak = os.path.join(iotests.test_dir, 'backup.img')
+self.foo_img = os.path.join(iotests.test_dir, 'foo.bar')
+self.img_create(self.test_img, iotests.imgfmt)
+self.vm.add_drive(self.test_img)
+# Create a base image with a distinctive patterning
+io_write_patterns(self.test_img, (('0x41', 0, 512),
+  ('0xd5', '1M', '32k'),
+  ('0xdc', '32M', '124k')))
+self.vm.launch()
+
+
+def img_create(self, img, fmt=iotests.imgfmt, size='64M',
+   parent=None, parentFormat=None):
+plist = list()
+if parent:
+if parentFormat is None:
+parentFormat = fmt
+iotests.qemu_img('create', '-f', fmt, img, size,
+ '-b', parent, '-F', parentFormat)
+else:
+iotests.qemu_img('create', '-f', fmt, img, size)
+self.files.append(img)
+
+def test_sync_dirty_bitmap_missing(self):
+self.assert_no_active_block_jobs()
+self.files.append(self.foo_img)
+result = self.vm.qmp('drive-backup', device='drive0',
+ sync='dirty-bitmap', format=iotests.imgfmt,
+ target=self.foo_img)
+self.assert_qmp(result, 'error/class', 'GenericError')
+
+
+def test_sync_dirty_bitmap_not_found(self):
+self.assert_no_active_block_jobs()
+self.files.append(self.foo_img)
+result = self.vm.qmp('drive-backup', device='drive0',
+ sync='dirty-bitmap', bitmap='unknown',
+ format=iotests.imgfmt, target=self.foo_img)
+self.assert_qmp(result, 'error/class', 'GenericError')
+
+
+def tearDown(self):
+self.vm.shutdown()
+for filename in self.files:
+try:
+os.remove(filename)
+except OSError:
+pass
+
+
+if __name__ == '__main__':
+iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
new file mode 100644
index 000..fbc63e6
--- /dev/null
+++ b/tests/qemu-iotests/124.out
@@ -0,0 +1,5 @@
+..
+--
+Ran 2 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 93a0250..df79c1a 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -121,3 +121,4 @@
 114 rw auto quick
 116 rw auto quick
 123 rw auto quick
+124 rw auto backing
-- 
1.9.3

[Qemu-devel] [PATCH RESEND 07/17] block: Add bitmap disabled status

[John Snow]

Add a status indicating the enabled/disabled state of the bitmap.
A bitmap is by default enabled, but you can lock the bitmap into
a read-only state by setting disabled = true.

A previous version of this patch added a QMP interface for changing
the state of the bitmap, but it has since been removed for now until
a use case emerges where this state must be revealed to the user.

The disabled state WILL be used internally for bitmap migration and
bitmap persistence.

Signed-off-by: Fam Zheng 
Signed-off-by: John Snow 
---
 block.c   | 25 +
 include/block/block.h |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/block.c b/block.c
index 9a82826..7e71aba 100644
--- a/block.c
+++ b/block.c
@@ -54,6 +54,7 @@
 struct BdrvDirtyBitmap {
 HBitmap *bitmap;
 char *name;
+bool disabled;
 QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -5450,10 +5451,16 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 bitmap = g_new0(BdrvDirtyBitmap, 1);
 bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
 bitmap->name = g_strdup(name);
+bitmap->disabled = false;
 QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
 return bitmap;
 }
 
+bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
+{
+return !bitmap->disabled;
+}
+
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
 BdrvDirtyBitmap *bm, *next;
@@ -5468,6 +5475,16 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
 }
 }
 
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+bitmap->disabled = true;
+}
+
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+bitmap->disabled = false;
+}
+
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 {
 BdrvDirtyBitmap *bm;
@@ -5532,12 +5549,14 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
 void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
 {
+assert(bdrv_dirty_bitmap_enabled(bitmap));
 hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
 void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
  int64_t cur_sector, int nr_sectors)
 {
+assert(bdrv_dirty_bitmap_enabled(bitmap));
 hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
@@ -5546,6 +5565,9 @@ static void bdrv_set_dirty(BlockDriverState *bs, int64_t 
cur_sector,
 {
 BdrvDirtyBitmap *bitmap;
 QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+continue;
+}
 hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
 }
 }
@@ -,6 +5577,9 @@ static void bdrv_reset_dirty(BlockDriverState *bs, 
int64_t cur_sector,
 {
 BdrvDirtyBitmap *bitmap;
 QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+continue;
+}
 hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
 }
 }
diff --git a/include/block/block.h b/include/block/block.h
index 2a11fbc..879a3af 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -458,9 +458,12 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState 
*bs,
 const char *name);
 void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
 uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t 
sector);
 void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
-- 
1.9.3

[Qemu-devel] [PATCH RESEND 06/17] hbitmap: add hbitmap_merge

[John Snow]

We add a bitmap merge operation to assist in error cases
where we wish to combine two bitmaps together.

This is algorithmically O(bits) provided HBITMAP_LEVELS remains
constant. For a full bitmap on a 64bit machine:
sum(bits/64^k, k, 0, HBITMAP_LEVELS) ~= 1.01587 * bits

We may be able to improve running speed for particularly sparse
bitmaps by using iterators, but the running time for dense maps
will be worse.

We present the simpler solution first, and we can refine it later
if needed.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 include/qemu/hbitmap.h | 11 +++
 util/hbitmap.c | 32 
 2 files changed, 43 insertions(+)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 550d7ce..c19c1cb 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,17 @@ struct HBitmapIter {
 HBitmap *hbitmap_alloc(uint64_t size, int granularity);
 
 /**
+ * hbitmap_merge:
+ * @a: The bitmap to store the result in.
+ * @b: The bitmap to merge into @a.
+ *
+ * Merge two bitmaps together.
+ * A := A (BITOR) B.
+ * B is left unmodified.
+ */
+bool hbitmap_merge(HBitmap *a, const HBitmap *b);
+
+/**
  * hbitmap_empty:
  * @hb: HBitmap to operate on.
  *
diff --git a/util/hbitmap.c b/util/hbitmap.c
index ab13971..962ff29 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -395,3 +395,35 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
 hb->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
 return hb;
 }
+
+/**
+ * Given HBitmaps A and B, let A := A (BITOR) B.
+ * Bitmap B will not be modified.
+ */
+bool hbitmap_merge(HBitmap *a, const HBitmap *b)
+{
+int i, j;
+uint64_t size;
+
+if ((a->size != b->size) || (a->granularity != b->granularity)) {
+return false;
+}
+
+if (hbitmap_count(b) == 0) {
+return true;
+}
+
+/* This merge is O(size), as BITS_PER_LONG and HBITMAP_LEVELS are constant.
+ * It may be possible to improve running times for sparsely populated maps
+ * by using hbitmap_iter_next, but this is suboptimal for dense maps.
+ */
+size = a->size;
+for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
+size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+for (j = 0; j < size; j++) {
+a->levels[i][j] |= b->levels[i][j];
+}
+}
+
+return true;
+}
-- 
1.9.3

[Qemu-devel] [PATCH RESEND 03/17] qmp: Ensure consistent granularity type

[John Snow]

We treat this field with a variety of different types everywhere
in the code. Now it's just uint32_t.

Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Signed-off-by: John Snow 
---
 block.c   | 11 ++-
 block/mirror.c|  4 ++--
 include/block/block.h |  2 +-
 include/block/block_int.h |  2 +-
 qapi/block-core.json  |  2 +-
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index 99c6e00..2c3cadc 100644
--- a/block.c
+++ b/block.c
@@ -5425,12 +5425,13 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
 }
 
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
-  int granularity,
+  uint32_t granularity,
   const char *name,
   Error **errp)
 {
 int64_t bitmap_size;
 BdrvDirtyBitmap *bitmap;
+uint32_t sector_granularity;
 
 assert((granularity & (granularity - 1)) == 0);
 
@@ -5438,8 +5439,8 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 error_setg(errp, "Bitmap already exists: %s", name);
 return NULL;
 }
-granularity >>= BDRV_SECTOR_BITS;
-assert(granularity);
+sector_granularity = granularity >> BDRV_SECTOR_BITS;
+assert(sector_granularity);
 bitmap_size = bdrv_nb_sectors(bs);
 if (bitmap_size < 0) {
 error_setg_errno(errp, -bitmap_size, "could not get length of device");
@@ -5447,7 +5448,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 return NULL;
 }
 bitmap = g_new0(BdrvDirtyBitmap, 1);
-bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
 bitmap->name = g_strdup(name);
 QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
 return bitmap;
@@ -5478,7 +5479,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
 info->count = bdrv_get_dirty_count(bs, bm);
 info->granularity =
-((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+((uint32_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
 entry->value = info;
diff --git a/block/mirror.c b/block/mirror.c
index f073ad7..f8aac33 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -656,7 +656,7 @@ static const BlockJobDriver commit_active_job_driver = {
 
 static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
  const char *replaces,
- int64_t speed, int64_t granularity,
+ int64_t speed, uint32_t granularity,
  int64_t buf_size,
  BlockdevOnError on_source_error,
  BlockdevOnError on_target_error,
@@ -717,7 +717,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 
 void mirror_start(BlockDriverState *bs, BlockDriverState *target,
   const char *replaces,
-  int64_t speed, int64_t granularity, int64_t buf_size,
+  int64_t speed, uint32_t granularity, int64_t buf_size,
   MirrorSyncMode mode, BlockdevOnError on_source_error,
   BlockdevOnError on_target_error,
   BlockCompletionFunc *cb,
diff --git a/include/block/block.h b/include/block/block.h
index ece4270..21d70e6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -451,7 +451,7 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
QEMUIOVector *qiov);
 struct HBitmapIter;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
-  int granularity,
+  uint32_t granularity,
   const char *name,
   Error **errp);
 BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index dccb092..fb9e100 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -590,7 +590,7 @@ void commit_active_start(BlockDriverState *bs, 
BlockDriverState *base,
  */
 void mirror_start(BlockDriverState *bs, BlockDriverState *target,
   const char *replaces,
-  int64_t speed, int64_t granularity, int64_t buf_size,
+  int64_t speed, uint32_t granularity, int64_t buf_size,
   MirrorSyncMode mode, BlockdevOnError on_source_error,
   BlockdevOnError on_target_error,
   BlockCompletionF

[Qemu-devel] [PATCH RESEND 08/17] block: Add bitmap successors

[John Snow]

A bitmap successor is an anonymous BdrvDirtyBitmap that is intended to
be created just prior to a sensitive operation (e.g. Incremental Backup)
that can either succeed or fail, but during the course of which we still
want a bitmap tracking writes.

On creating a successor, we "freeze" the parent bitmap which prevents
its deletion, enabling, anonymization, or creating a bitmap with the
same name.

On success, the parent bitmap can "abdicate" responsibility to the
successor, which will inherit its name. The successor will have been
tracking writes during the course of the backup operation. The parent
will be safely deleted.

On failure, we can "reclaim" the successor from the parent, unifying
them such that the resulting bitmap describes all writes occurring since
the last successful backup, for instance. Reclamation will thaw the
parent, but not explicitly re-enable it.

BdrvDirtyBitmap operations that target a single bitmap are protected
by assertions that the bitmap is not frozen and/or disabled.

BdrvDirtyBitmap operations that target a group of bitmaps, such as
bdrv_{set,reset}_dirty will ignore frozen/disabled drives with a
conditional instead.

QMP transactions that enable/disable bitmaps have extra error checking
surrounding them that prevent modifying bitmaps that are frozen.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
---
 block.c   | 104 +-
 blockdev.c|   7 
 include/block/block.h |  10 +
 qapi/block-core.json  |   1 +
 4 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 7e71aba..e8f66a6 100644
--- a/block.c
+++ b/block.c
@@ -51,8 +51,17 @@
 #include 
 #endif
 
+/**
+ * A BdrvDirtyBitmap can be in three possible states:
+ * (1) successor is false and disabled is false: full r/w mode
+ * (2) successor is false and disabled is true: read only mode ("disabled")
+ * (3) successor is set: frozen mode.
+ * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
+ * or enabled. A frozen bitmap can only abdicate() or reclaim().
+ */
 struct BdrvDirtyBitmap {
 HBitmap *bitmap;
+BdrvDirtyBitmap *successor;
 char *name;
 bool disabled;
 QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -5421,6 +5430,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState 
*bs, const char *name)
 
 void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
+assert(!bdrv_dirty_bitmap_frozen(bitmap));
 g_free(bitmap->name);
 bitmap->name = NULL;
 }
@@ -5456,9 +5466,98 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 return bitmap;
 }
 
+bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
+{
+return bitmap->successor;
+}
+
 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
 {
-return !bitmap->disabled;
+return !(bitmap->disabled || bitmap->successor);
+}
+
+/**
+ * Create a successor bitmap destined to replace this bitmap after an 
operation.
+ * Requires that the bitmap is not frozen and has no successor.
+ */
+int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
+   BdrvDirtyBitmap *bitmap, Error **errp)
+{
+uint64_t granularity;
+BdrvDirtyBitmap *child;
+
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+error_setg(errp, "Cannot create a successor for a bitmap that is "
+   "currently frozen");
+return -1;
+}
+assert(!bitmap->successor);
+
+/* Create an anonymous successor */
+granularity = bdrv_dirty_bitmap_granularity(bitmap);
+child = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
+if (!child) {
+return -1;
+}
+
+/* Successor will be on or off based on our current state. */
+child->disabled = bitmap->disabled;
+
+/* Install the successor and freeze the parent */
+bitmap->successor = child;
+return 0;
+}
+
+/**
+ * For a bitmap with a successor, yield our name to the successor,
+ * Delete the old bitmap, and return a handle to the new bitmap.
+ */
+BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
+BdrvDirtyBitmap *bitmap,
+Error **errp)
+{
+char *name;
+BdrvDirtyBitmap *successor = bitmap->successor;
+
+if (successor == NULL) {
+error_setg(errp, "Cannot relinquish control if "
+   "there's no successor present");
+return NULL;
+}
+
+name = bitmap->name;
+bitmap->name = NULL;
+successor->name = name;
+bitmap->successor = NULL;
+bdrv_release_dirty_bitmap(bs, bitmap);
+
+return successor;
+}
+
+/**
+ * In cases of failure where we can no longer safely delete the parent,
+ * We may wish to re-join the parent and child/successor.
+ * The merged parent will be un-frozen, but not explicitly re-enabled.
+ */
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
+   

[Qemu-devel] [PATCH RESEND 04/17] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove

[John Snow]

The new command pair is added to manage a user created dirty bitmap. The
dirty bitmap's name is mandatory and must be unique for the same device,
but different devices can have bitmaps with the same names.

The granularity is an optional field. If it is not specified, we will
choose a default granularity based on the cluster size if available,
clamped to between 4K and 64K to mirror how the 'mirror' code was
already choosing granularity. If we do not have cluster size info
available, we choose 64K. This code has been factored out into a helper
shared with block/mirror.

This patch also introduces the 'block_dirty_bitmap_lookup' helper,
which takes a device name and a dirty bitmap name and validates the
lookup, returning NULL and setting errp if there is a problem with
either field. This helper will be re-used in future patches in this
series.

The types added to block-core.json will be re-used in future patches
in this series, see:
'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}'

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block.c   |  20 ++
 block/mirror.c|  10 +
 blockdev.c| 100 ++
 include/block/block.h |   1 +
 qapi/block-core.json  |  55 +++
 qmp-commands.hx   |  56 
 6 files changed, 233 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 2c3cadc..c627448 100644
--- a/block.c
+++ b/block.c
@@ -5499,6 +5499,26 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap, int64_t sector
 }
 }
 
+/**
+ * Chooses a default granularity based on the existing cluster size,
+ * but clamped between [4K, 64K]. Defaults to 64K in the case that there
+ * is no cluster size information available.
+ */
+uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
+{
+BlockDriverInfo bdi;
+uint32_t granularity;
+
+if (bdrv_get_info(bs, &bdi) >= 0 && bdi.cluster_size > 0) {
+granularity = MAX(4096, bdi.cluster_size);
+granularity = MIN(65536, granularity);
+} else {
+granularity = 65536;
+}
+
+return granularity;
+}
+
 void bdrv_dirty_iter_init(BlockDriverState *bs,
   BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
 {
diff --git a/block/mirror.c b/block/mirror.c
index f8aac33..1cb700e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -668,15 +668,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 MirrorBlockJob *s;
 
 if (granularity == 0) {
-/* Choose the default granularity based on the target file's cluster
- * size, clamped between 4k and 64k.  */
-BlockDriverInfo bdi;
-if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
-granularity = MAX(4096, bdi.cluster_size);
-granularity = MIN(65536, granularity);
-} else {
-granularity = 65536;
-}
+granularity = bdrv_get_default_bitmap_granularity(target);
 }
 
 assert ((granularity & (granularity - 1)) == 0);
diff --git a/blockdev.c b/blockdev.c
index ae73539..c40c08c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1162,6 +1162,48 @@ out_aio_context:
 return NULL;
 }
 
+/**
+ * Return a dirty bitmap (if present), after validating
+ * the node reference and bitmap names. Returns NULL on error,
+ * including when the BDS and/or bitmap is not found.
+ */
+static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+  const char *name,
+  BlockDriverState **pbs,
+  Error **errp)
+{
+BlockDriverState *bs;
+BdrvDirtyBitmap *bitmap;
+
+if (!node) {
+error_setg(errp, "Node cannot be NULL");
+return NULL;
+}
+if (!name) {
+error_setg(errp, "Bitmap name cannot be NULL");
+return NULL;
+}
+
+bs = bdrv_lookup_bs(node, node, NULL);
+if (!bs) {
+error_setg(errp, "Node '%s' not found", node);
+return NULL;
+}
+
+/* If caller provided a BDS*, provide the result of that lookup, too. */
+if (pbs) {
+*pbs = bs;
+}
+
+bitmap = bdrv_find_dirty_bitmap(bs, name);
+if (!bitmap) {
+error_setg(errp, "Dirty bitmap '%s' not found", name);
+return NULL;
+}
+
+return bitmap;
+}
+
 /* New and old BlockDriverState structs for atomic group operations */
 
 typedef struct BlkTransactionState BlkTransactionState;
@@ -1942,6 +1984,64 @@ void qmp_block_set_io_throttle(const char *device, 
int64_t bps, int64_t bps_rd,
 aio_context_release(aio_context);
 }
 
+void qmp_block_dirty_bitmap_add(const char *node, const char *name,
+bool has_granularity, uint32_t granularity,
+Error **errp)
+{
+AioC

[Qemu-devel] [PATCH RESEND 05/17] block: Introduce bdrv_dirty_bitmap_granularity()

[John Snow]

This returns the granularity (in bytes) of dirty bitmap,
which matches the QMP interface and the existing query
interface.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block.c   | 8 ++--
 include/block/block.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index c627448..9a82826 100644
--- a/block.c
+++ b/block.c
@@ -5478,8 +5478,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
 BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
 info->count = bdrv_get_dirty_count(bs, bm);
-info->granularity =
-((uint32_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+info->granularity = bdrv_dirty_bitmap_granularity(bm);
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
 entry->value = info;
@@ -5519,6 +5518,11 @@ uint32_t 
bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
 return granularity;
 }
 
+uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
+{
+return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
+}
+
 void bdrv_dirty_iter_init(BlockDriverState *bs,
   BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
 {
diff --git a/include/block/block.h b/include/block/block.h
index b2d019b..2a11fbc 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -460,6 +460,7 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
+uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t 
sector);
 void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
-- 
1.9.3

[Qemu-devel] [PATCH RESEND 00/17] block: transactionless incremental backup series

[John Snow]

New topic, new version, same great incremental backup taste.

This series relies on [Qemu-devel] [PULL 30/69] blkdebug: fix "once" rule,
part of Stefan's 69 patch pull request submitted 2015-02-27.

This patchset enables the in-memory part of the incremental backup
feature, without transactional support.

Support for transactions will come at a later date, but getting this
portion upstream will help stabilize work on bitmap persistence and
bitmap migration.

Transactional support is being postponed to allow more development
to create a transactional callback system that will allow objects used
by the drive backup routines to perform cleanup actions conditionally
based on the outcome of all backups in the transaction group.

This series was once said to have been written by a man known only
to Qemu-Devel as "Fam Zheng", A programming maverick hellbent on never
again wasting space by making full backups when he did not have to. The
true origins of this series are now lost to the sands of time.

(11+ revisions later: Thanks, Fam!)

[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/17:[0210] [FC] 'docs: incremental backup documentation'
002/17:[0002] [FC] 'qapi: Add optional field "name" to block dirty bitmap'
003/17:[] [--] 'qmp: Ensure consistent granularity type'
004/17:[0027] [FC] 'qmp: Add block-dirty-bitmap-add and 
block-dirty-bitmap-remove'
005/17:[] [--] 'block: Introduce bdrv_dirty_bitmap_granularity()'
006/17:[] [--] 'hbitmap: add hbitmap_merge'
007/17:[down] 'block: Add bitmap disabled status'
008/17:[0017] [FC] 'block: Add bitmap successors'
009/17:[0004] [FC] 'qmp: Add support of "dirty-bitmap" sync mode for 
drive-backup'
010/17:[0011] [FC] 'qmp: add block-dirty-bitmap-clear'
011/17:[0004] [FC] 'qmp: Add dirty bitmap status fields in query-block'
012/17:[] [--] 'block: add BdrvDirtyBitmap documentation'
013/17:[] [--] 'block: Ensure consistent bitmap function prototypes'
014/17:[0019] [FC] 'block: Resize bitmaps on bdrv_truncate'
015/17:[0002] [FC] 'iotests: add invalid input incremental backup tests'
016/17:[] [--] 'iotests: add simple incremental backup case'
017/17:[0008] [FC] 'iotests: add incremental backup failure recovery test'

Deletions:
 - Removed Transactions, to be added later.
 - Removed Transaction tests, as above.

Changes:
01: Indentation fixes.
Removed enable/disable documentation.
Added a note that transactions aren't implemented yet.
Removed my needless commas
Added error case documentation.

07: QMP enable/disable commands are deleted.

14: Some comments concerning assertions.
Scrub re-alloc memory if we expand the array.
Do not attempt to scrub memory if fix_count is 0

Changes made with Reviews kept:

02: Since 2.4
04: Since 2.4
Demingled the QMP command documentation.
08: Additions to what was qmp_block_dirty_enable/disable
are no longer present as those function no longer exist.
09: Since 2.4
10: Since 2.4
Demingled QMP command documentation.
11: Since 2.4
15: Test 112 --> 124
17: Number of tests altered. (Only 4, now.)

Fam Zheng (1):
  qapi: Add optional field "name" to block dirty bitmap

John Snow (16):
  docs: incremental backup documentation
  qmp: Ensure consistent granularity type
  qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
  block: Introduce bdrv_dirty_bitmap_granularity()
  hbitmap: add hbitmap_merge
  block: Add bitmap disabled status
  block: Add bitmap successors
  qmp: Add support of "dirty-bitmap" sync mode for drive-backup
  qmp: add block-dirty-bitmap-clear
  qmp: Add dirty bitmap status fields in query-block
  block: add BdrvDirtyBitmap documentation
  block: Ensure consistent bitmap function prototypes
  block: Resize bitmaps on bdrv_truncate
  iotests: add invalid input incremental backup tests
  iotests: add simple incremental backup case
  iotests: add incremental backup failure recovery test

 block.c   | 248 --
 block/backup.c| 149 +
 block/mirror.c|  46 +++
 blockdev.c| 162 +-
 docs/bitmaps.md   | 303 ++
 hmp.c |   3 +-
 include/block/block.h |  34 -
 include/block/block_int.h |   4 +-
 include/qemu/hbitmap.h|  21 +++
 migration/block.c |   9 +-
 qapi/block-core.json  |  92 -
 qmp-commands.hx   |  92 -
 tests/qemu-iotests/124| 266 
 tests/qemu-iotests/124.out|   5 +
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/iotests.py |   6 +-
 util/hbitmap.c|  86 
 17 files changed, 1437 insertions(+), 90 deletions(-)
 cre

[Qemu-devel] [PATCH RESEND 02/17] qapi: Add optional field "name" to block dirty bitmap

[John Snow]

From: Fam Zheng 

This field will be set for user created dirty bitmap. Also pass in an
error pointer to bdrv_create_dirty_bitmap, so when a name is already
taken on this BDS, it can report an error message. This is not global
check, two BDSes can have dirty bitmap with a common name.

Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will
be used later when other QMP commands want to reference dirty bitmap by
name.

Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap.

Signed-off-by: Fam Zheng 
Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block.c   | 32 +++-
 block/mirror.c|  2 +-
 include/block/block.h |  7 ++-
 migration/block.c |  2 +-
 qapi/block-core.json  |  4 +++-
 5 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index e2408f2..99c6e00 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,7 @@
 
 struct BdrvDirtyBitmap {
 HBitmap *bitmap;
+char *name;
 QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -5404,7 +5405,28 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
QEMUIOVector *qiov)
 return true;
 }
 
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int 
granularity,
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
+{
+BdrvDirtyBitmap *bm;
+
+assert(name);
+QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
+if (bm->name && !strcmp(name, bm->name)) {
+return bm;
+}
+}
+return NULL;
+}
+
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+g_free(bitmap->name);
+bitmap->name = NULL;
+}
+
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+  int granularity,
+  const char *name,
   Error **errp)
 {
 int64_t bitmap_size;
@@ -5412,6 +5434,10 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
 
 assert((granularity & (granularity - 1)) == 0);
 
+if (name && bdrv_find_dirty_bitmap(bs, name)) {
+error_setg(errp, "Bitmap already exists: %s", name);
+return NULL;
+}
 granularity >>= BDRV_SECTOR_BITS;
 assert(granularity);
 bitmap_size = bdrv_nb_sectors(bs);
@@ -5422,6 +5448,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
 }
 bitmap = g_new0(BdrvDirtyBitmap, 1);
 bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+bitmap->name = g_strdup(name);
 QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
 return bitmap;
 }
@@ -5433,6 +5460,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
 if (bm == bitmap) {
 QLIST_REMOVE(bitmap, list);
 hbitmap_free(bitmap->bitmap);
+g_free(bitmap->name);
 g_free(bitmap);
 return;
 }
@@ -5451,6 +5479,8 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 info->count = bdrv_get_dirty_count(bs, bm);
 info->granularity =
 ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+info->has_name = !!bm->name;
+info->name = g_strdup(bm->name);
 entry->value = info;
 *plist = entry;
 plist = &entry->next;
diff --git a/block/mirror.c b/block/mirror.c
index 4056164..f073ad7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -703,7 +703,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 s->granularity = granularity;
 s->buf_size = MAX(buf_size, granularity);
 
-s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, errp);
+s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
 if (!s->dirty_bitmap) {
 return;
 }
diff --git a/include/block/block.h b/include/block/block.h
index cea8a2a..ece4270 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -450,8 +450,13 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
QEMUIOVector *qiov);
 
 struct HBitmapIter;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int 
granularity,
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+  int granularity,
+  const char *name,
   Error **errp);
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
+const char *name);
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 int bdrv_get_dirty(BlockDriverState *

Re: [Qemu-devel] [PATCH 4/6 v4] linux-user: Support tilegx architecture in syscall

[Chen Gang S]

On 02/28/2015 01:40 AM, Andreas Färber wrote:
> Am 22.02.2015 um 14:36 schrieb Chen Gang S:
>> Add tilegx architecture in "syscall_defs.h", all related features (ioctrl,
>> and stat) are based on Linux kernel tilegx 64-bit implementation.
>>
>> Signed-off-by: Chen Gang 
>> ---
>>  linux-user/syscall_defs.h | 38 ++
>>  1 file changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index 9ed6de8..a0d9d77 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
> [...]
>> @@ -2312,7 +2340,9 @@ struct target_flock {
>>  struct target_flock64 {
>>  short  l_type;
>>  short  l_whence;
>> -#if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) 
>> || defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined 
>> (TARGET_MICROBLAZE)
>> +#if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) 
>> || \
>> +defined(TARGET_SPARC) || defined(TARGET_HPPA) \
>> +|| defined(TARGET_MICROBLAZE) || defined(TARGET_TILEGX)
> 
> Nit: You're inconsistent as to where you're placing ||.
> 

OK.

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

Re: [Qemu-devel] [PATCH 6/6 v4] linux-user/syscall.c: Switch all macros which are not defined in tilegx

[Chen Gang S]

On 02/28/2015 02:24 AM, Andreas Färber wrote:
> Am 22.02.2015 um 14:37 schrieb Chen Gang S:
>> For tilegx, several syscall macros are not supported, so switch them to
>> avoid building break.
>>
>> Signed-off-by: Chen Gang 
>> ---
>>  linux-user/syscall.c | 50 +-
>>  1 file changed, 49 insertions(+), 1 deletion(-)
> 
> LGTM,
> 
> Reviewed-by: Andreas Färber 
> 
> However I suggest "conditionalize syscalls" or "disable syscalls" over
> "switch ... macros/them".
> 

OK, thanks. What you said above sounds reasonable to me.

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

Re: [Qemu-devel] [PATCH 0/2] block: driver should override flags in bdrv_open()

[Max Reitz]

Ping

On 2015-02-02 at 12:08, Max Reitz wrote:

BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
override by explicitly specifying a block driver. This series implements
this and adds two iotests (one for NBD, one for file) to test it.

For the NBD test to succeed, this series relies on
"iotests: Specify format for qemu-nbd".

Also, the second patch's "group" file modifying hunk relies on
"qemu-iotests: add 116 invalid QED input file tests".


Max Reitz (2):
   block: driver should override flags in bdrv_open()
   iotests: Add tests for overriding BDRV_O_PROTOCOL

  block.c| 49 +-
  tests/qemu-iotests/051 |  1 -
  tests/qemu-iotests/051.out |  3 ---
  tests/qemu-iotests/119 | 60 ++
  tests/qemu-iotests/119.out | 11 
  tests/qemu-iotests/120 | 65 ++
  tests/qemu-iotests/120.out | 15 +++
  tests/qemu-iotests/group   |  2 ++
  8 files changed, 184 insertions(+), 22 deletions(-)
  create mode 100755 tests/qemu-iotests/119
  create mode 100644 tests/qemu-iotests/119.out
  create mode 100755 tests/qemu-iotests/120
  create mode 100644 tests/qemu-iotests/120.out

Re: [Qemu-devel] [PATCH v3 00/12] qcow2: Add new overlap check functions

[Max Reitz]

Ping

On 2015-02-09 at 14:25, Max Reitz wrote:

As has been requested, this series adds new overlap check functions to
the qcow2 code. My local branch is called "qcow2-improved-overlap-v1",
but I am not so sure whether it is actually an improvement; that is left
for you to decide, dear reviewers.

See patch 1 for an explanation of why this series exists and what it
does. Patch 1 is basically the core of this series, the rest just
employs the functions introduced there.

In a later patch, we may want to change the meaning of the "constant"
overlap checking option to mean the same as "cached", which is
everything except for inactive L2 tables. This series does make
checking for overlaps with inactive L2 tables at runtime just as cheap
as everything else (constant time plus caching), but using these checks
means qemu has to read all the snapshot L1 tables when opening a qcow2
file. This does not take long, of course, but it does result in a bit of
overhead so I did not want to enable it by default.

I think just enabling all overlap checks by default after this series
should be fine and useful, though.


For benchmarks, please see my cover letter for v2:
http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg03430.html

I'll just quote the (fixed) tl;dr here:

* CPU usage at runtime decreased by 15000 to 27500 percent on
   overlap-check-heavy tasks
* No additional performance problems at loading time (in theory has the
   same runtime complexity as a single overlap check right now; in
   practice I could not find any problems)
* Decent RAM usage (170 kB for a 1 TB image with 64 kB clusters; 170 MB
   for a 1 PB image etc. pp.)


v3:
- Patch 1:
   - s/few memory/little memory/ [Eric]
   - s/2014/2015/ [Eric]
   - s/of a or a/of or a/ [Eric]
   - Use nb_clusters_minus_one instead of nb_clusters, because
 nb_clusters == 0 is invalid [Eric]
   - Drop cached_windows_index [Stefan]
   - Reset current_nb_clusters to 1 when a new range starts in the
 algorithm which rebuilds the fragment list from the bitmap
 representation
- Patch 3: Added a TODO comment that the user should be able to override
   the default cache size limit
- Patch 8: As the patch "qcow2: Buffer L1 table in snapshot refcount
   update" has been dropped, this patch was changed to no longer rely on
   it
- Patch 11: Replaced the size limit for a snapshot's L1 table by
   QCOW_MAX_L1_SIZE (was: INT_MAX / sizeof(uint64_t)) [Eric]


git-backport-diff against v2:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/12:[0026] [FC] 'qcow2: Add new overlap check functions'
002/12:[] [--] 'qcow2: Pull up overlap check option evaluation'
003/12:[0001] [FC] 'qcow2: Create metadata list'
004/12:[] [--] 'qcow2/overlaps: Protect image header'
005/12:[] [--] 'qcow2/overlaps: Protect refcount table'
006/12:[] [--] 'qcow2/overlaps: Protect refcount blocks'
007/12:[] [--] 'qcow2/overlaps: Protect active L1 table'
008/12:[0002] [FC] 'qcow2/overlaps: Protect active L2 tables'
009/12:[] [--] 'qcow2/overlaps: Protect snapshot table'
010/12:[] [--] 'qcow2/overlaps: Protect inactive L1 tables'
011/12:[0002] [FC] 'qcow2/overlaps: Protect inactive L2 tables'
012/12:[] [--] 'qcow2: Use new metadata overlap check function'


Max Reitz (12):
   qcow2: Add new overlap check functions
   qcow2: Pull up overlap check option evaluation
   qcow2: Create metadata list
   qcow2/overlaps: Protect image header
   qcow2/overlaps: Protect refcount table
   qcow2/overlaps: Protect refcount blocks
   qcow2/overlaps: Protect active L1 table
   qcow2/overlaps: Protect active L2 tables
   qcow2/overlaps: Protect snapshot table
   qcow2/overlaps: Protect inactive L1 tables
   qcow2/overlaps: Protect inactive L2 tables
   qcow2: Use new metadata overlap check function

  block/Makefile.objs|   3 +-
  block/qcow2-cluster.c  |  13 ++
  block/qcow2-overlap.c  | 396 +
  block/qcow2-refcount.c | 202 ++---
  block/qcow2-snapshot.c | 105 -
  block/qcow2.c  | 131 ++--
  block/qcow2.h  |  13 ++
  7 files changed, 691 insertions(+), 172 deletions(-)
  create mode 100644 block/qcow2-overlap.c

Re: [Qemu-devel] [PATCH 1/6 v4] target-tilegx: Firstly add to qemu with minimized features

[Chris Metcalf]

On 2/27/2015 12:36 PM, Andreas Färber wrote:

"TILE-Gx" according to
http://www.tilera.com/products/?ezchip=585&spage=614  - please fix
wherever used in textual form.


Yes, really only "tilegx" or "TILE-Gx" should be used.  In type names I agree 
that TileGX is probably clearer than any alternatives.


What about CPU models? Do the Gx72, Gx36, etc. differ only in core count
or also in instruction set features?


From a userspace perspective, it's mostly the core count.  They also have a 
different set of on-chip hardware accelerators, etc., but that's out of scope 
for this project.

--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests

[Max Reitz]

On 2015-02-27 at 16:44, Stefan Weil wrote:

Am 27.02.2015 um 18:42 schrieb Paolo Bonzini:


An optimized fix could be to use a CoRwLock, then:



Note related to our previous discussion: why does the code use 
CoRwlock instead of CoRwLock? Should it be renamed before more code 
uses it?


Well, I'm simply not using it (in v3 at least...), so I hope I'll get 
away without touching that. :-)


Max

Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests

[Stefan Weil]

Am 27.02.2015 um 18:42 schrieb Paolo Bonzini:


An optimized fix could be to use a CoRwLock, then:



Note related to our previous discussion: why does the code use CoRwlock 
instead of CoRwLock? Should it be renamed before more code uses it?


Stefan

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] PPC: Introduce the Virtual Time Base (VTB) SPR register

[Alexander Graf]

On 27.02.15 16:28, Alexander Graf wrote:
> 
> 
> On 27.02.15 00:19, Cyril Bur wrote:
>> This patch adds basic support for the VTB.
>>
>> PowerISA:
>> The Virtual Time Base (VTB) is a 64-bit incrementing counter.
>> Virtual Time Base increments at the same rate as the Time Base until its 
>> value
>> becomes 0x___ (2 64 - 1); at the next increment its value
>> becomes 0x___. There is no interrupt or other indication when
>> this occurs.
>>
>> The operation of the Virtual Time Base has the following additional
>> properties.
>> 1. Loading a GPR from the Virtual Time Base has no effect on the accuracy of
>> the Virtual Time Base.
>> 2. Copying the contents of a GPR to the Virtual Time Base replaces the
>> contents of the Virtual Time Base with the contents of the GPR.
>>
>> Signed-off-by: Cyril Bur 
> 
> Thanks, applied to ppc-next.

Sorry, the patch breaks compilation for ppc-softmmu:

  target-ppc/translate_init.c:877:13: error: ‘gen_spr_vtb’ defined but
not used [-Werror=unused-function]

Please move the definition of gen_spr_vtb to the other p8 related spr
registration functions so it only gets added when we compile for 64bit.

I've removed the patch from my queue again.


Alex

Re: [Qemu-devel] [PATCH 1/2] Rolling statistics utilities

[Eric Blake]

On 02/27/2015 12:06 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> There are various places where it's useful to hold a series
> of values that change over time and get summaries about them.
> 
> This provides:
> 
>- a count of the number of items
>- min/max
>- mean
>- a weighted mean (where you can set the weight to determine
>   whether it changes quickly or slowly)
>- the last 'n' values
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  include/qemu/rolling-stats.h | 101 +++

> +
> +/**
> + * Return a string representing the RStats data, intended for JSON parsing
> + *
> + * Returns: An allocated string the caller must free
> + *  or NULL on error
> + *
> + * e.g.
> + *{ "min": -3.57, "max": 126.3, "mean": 7.83, "weighted_mean": 8.56,
> + *  "count": 5678,
> + *  "values": [ 4.3, 5.8, 1.2, 7.9, 10.3 ],
> + *  "tags": [ 458, 783, 950, 951, 952 ] }

Looks useful at first glance.  Maybe s/weighted_mean/weighted-mean/
since we favor - in new QMP.


> +
> +qemu_mutex_lock(&r->mutex);
> +space  = 60 /* for text */ +
> + /* 4 double values (min/max/mean/weighted) + the stored
> +  * values, plus a normal guess for the size of them printed
> +  * with %g and some padding.  I'm not sure of the worst case.
> +  */
> + (4 + r->allocated) * 13 +
> + /* and the count and tags as 64bit ints and some padding */
> + (1 + r->allocated) * 23;
> +space_left = space - 1;
> +
> +result = g_try_malloc(space);
> +
> +if (!result) {
> +qemu_mutex_unlock(&r->mutex);
> +return NULL;
> +}
> +
> +current = result;
> +tmp = snprintf(current, space_left, "Min/Max: %.8g, %.8g Mean: %.8g "
> +"(Weighted: %.8g) Count: %" PRIu64
> +" Values: ",

Eww. Why pre-compute things for a possibly not-wide-enough snprintf,
when you can instead use glib's g_string_printf that allocates the
perfect size as it goes?  For that matter, your cover letter comment
about putting the struct in QAPI and letting the generated visitor
automatically produce the JSON might make this simpler than building it
by hand.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v14 04/19] block: Introduce bdrv_dirty_bitmap_granularity()

[Eric Blake]

On 02/20/2015 04:07 PM, John Snow wrote:
> This returns the granularity (in bytes) of dirty bitmap,
> which matches the QMP interface and the existing query
> interface.
> 
> Signed-off-by: John Snow 
> Reviewed-by: Max Reitz 
> ---
>  block.c   | 8 ++--
>  include/block/block.h | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests

[Stefan Weil]

Am 27.02.2015 um 21:23 schrieb Max Reitz:

On 2015-02-27 at 15:21, Stefan Weil wrote:

Am 27.02.2015 um 19:55 schrieb Max Reitz:

On 2015-02-27 at 13:15, Max Reitz wrote:

On 2015-02-27 at 13:12, Stefan Weil wrote:

Am 27.02.2015 um 19:09 schrieb Max Reitz:

It always fails for me. Do you have an SSD?

On the real machine: yes. On the virtual machine: maybe.


I don't, so maybe that's the issue. But running it in tmpfs doesn't 
change anything for me, still fails (every time).


And now it doesn't fail any more on my HDD (but still in tmpfs). Great.

How about this test case (which is in line with what Paolo 
identified is the potential problem)? :-)


Max



No failure with the new one, too.


Hm, have you tried it multiple times?

Max


I tried it only a few times. While running it in a loop I now get failures.

Stefan

Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests

[Stefan Weil]

Am 27.02.2015 um 19:55 schrieb Max Reitz:

On 2015-02-27 at 13:15, Max Reitz wrote:

On 2015-02-27 at 13:12, Stefan Weil wrote:

Am 27.02.2015 um 19:09 schrieb Max Reitz:

It always fails for me. Do you have an SSD?

On the real machine: yes. On the virtual machine: maybe.


I don't, so maybe that's the issue. But running it in tmpfs doesn't 
change anything for me, still fails (every time).


And now it doesn't fail any more on my HDD (but still in tmpfs). Great.

How about this test case (which is in line with what Paolo identified 
is the potential problem)? :-)


Max



No failure with the new one, too.

Stefan

Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests

[Max Reitz]

On 2015-02-27 at 15:21, Stefan Weil wrote:

Am 27.02.2015 um 19:55 schrieb Max Reitz:

On 2015-02-27 at 13:15, Max Reitz wrote:

On 2015-02-27 at 13:12, Stefan Weil wrote:

Am 27.02.2015 um 19:09 schrieb Max Reitz:

It always fails for me. Do you have an SSD?

On the real machine: yes. On the virtual machine: maybe.


I don't, so maybe that's the issue. But running it in tmpfs doesn't 
change anything for me, still fails (every time).


And now it doesn't fail any more on my HDD (but still in tmpfs). Great.

How about this test case (which is in line with what Paolo identified 
is the potential problem)? :-)


Max



No failure with the new one, too.


Hm, have you tried it multiple times?

Max

Re: [Qemu-devel] [PATCH RFC v4 00/13] Dirty bitmaps migration

[John Snow]

On 02/27/2015 12:24 PM, Vladimir Sementsov-Ogievskiy wrote:

These patches provide dirty bitmap migration feature. Only named dirty
bitmaps are to be migrated. Migration may be enabled using migration
capabilities.

v4 significant changes:
  0001: tiny bugfix: out[i] -> out[i-start], same for 'in'
  0007: set chunk size to 1kb, disable live iteration for
migrating data < 1mb size.

  tests: only one with md5 sum is here. used function event_wait
 by John Snow. (I hope, you don't mind me just adding this
 function with your 'Signed-off-by')

  rfc: This patch set is based on v13 of
   "block: incremental backup series" by John Snow, which are
   not pushed yet.

v3:
  based on v13 of "block: incremental backup series" by John Snow.

  changes from v2:
  removed patch for adding dirty parameter (migration capablities used
  instead).

  0001: printf's dropped, qapi used
  0002: part0 -> zeroes
  0003: part0 -> zeroes
  0005: dirty_dirty -> meta
add comments about meta bitmap

  0006: the format is changed, nodes used instead of devices.

  other patches are new.

  rfc: there are two tests. They are the same but using different
  interfaces: md5 checksum of the bitmap last layer in query-block or
  separate query-block-dirty-bitmap with dirty bitmap regions.
  The second form is more appropriate for debugging, the first is more
  appropriate for simple regression control. Which should go to
  upstream?

v2:
  1. bug-fixes, that are already in upstream, and renaming of function
  bdrv_reset_dirty_bitmap (which is already in Snow's series) are
  dropped
  2. bitmap store/restore: the concept renamed to serialization, added
  function hbitmap_deserialize_part0, to not transfer zero blocks
  3. migration dirty parameter: added description comment
  4. Other patches are new.

v2.rfc:
Actually, in this version of the series I'm trying not use
migration/block.c at all. Instead a separate migration unit is added
in the new file migration/dirty-bitmap.c. Now bitmaps are migrated
like blocks in block migration, they have their "dirty-dirty" bitmaps,
for tracking set/unset changes during migration.

The advantages are:
   - no complications of migration/block.c
   - separate dirty-dirty bitmaps provide handling of "unset's"
   - more effective meta-data/data ratio - no tiny bitmap-blocks.



v1:
These patches provide dirty bitmap migration feature. Only named dirty
bitmaps are to be migrated. Migration is made as a part of block
migration in block-migration.c.

Dirty bitmap migration may be enabled by "dirty" parameter for qmp migrate
command. If "blk" and "inc" parameters are false when "dirty" is true
block migration is actually skipped: no allocatoions, no bdrv_read's,
no bdrv_write's, only bitmaps are migrated.

The patch set includes two my previous bug fixes, which are necessary
for it. The patch set is based on Incremental backup series by John
Snow.

*** BLURB HERE ***

Vladimir Sementsov-Ogievskiy (13):
   hbitmap: serialization
   block: BdrvDirtyBitmap serialization interface
   block: tiny refactoring: minimize hbitmap_(set/reset) usage
   block: add meta bitmaps
   block: add bdrv_next_dirty_bitmap()
   qapi: add dirty-bitmaps migration capability
   migration: add migration/block-dirty-bitmap.c
   iotests: maintain several vms in test
   iotests: add add_incoming_migration to VM class
   iotests: add event_wait to VM class
   qapi: add md5 checksum of last dirty bitmap level to query-block
   iotests: add dirty bitmap migration test
   migration/qemu-file: make functions qemu_(get/put)_string public

  block.c|  98 +-
  include/block/block.h  |  22 ++
  include/migration/block.h  |   1 +
  include/migration/migration.h  |   1 +
  include/migration/qemu-file.h  |  17 +
  include/qemu/hbitmap.h |  67 
  migration/Makefile.objs|   2 +-
  migration/block-dirty-bitmap.c | 693 +
  migration/migration.c  |   9 +
  migration/qemu-file.c  |  18 ++
  qapi-schema.json   |   5 +-
  qapi/block-core.json   |   4 +-
  tests/qemu-iotests/117 |  84 +
  tests/qemu-iotests/117.out |   5 +
  tests/qemu-iotests/group   |   1 +
  tests/qemu-iotests/iotests.py  |  19 +-
  util/hbitmap.c | 106 +++
  vl.c   |   1 +
  18 files changed, 1144 insertions(+), 9 deletions(-)
  create mode 100644 migration/block-dirty-bitmap.c
  create mode 100755 tests/qemu-iotests/117
  create mode 100644 tests/qemu-iotests/117.out



I think you forgot to CC me on the series.

Unless -- you're not trying to get rid of me, are you? :>

Re: [Qemu-devel] [PATCH v4 05/11] block: Move BDS close notifiers into BB

[Max Reitz]

On 2015-02-27 at 11:43, Max Reitz wrote:

The only remaining user of the BDS close notifiers is NBD which uses
them to determine when a BDS tree is being ejected. This patch removes
the BDS-level close notifiers and adds a notifier list to the
BlockBackend structure that is invoked whenever a BDS is removed.

Symmetrically to that, another notifier list is added that is invoked
whenever a BDS is inserted. The dataplane implementations for virtio-blk
and virtio-scsi use both notifier types for setting up and removing op
blockers. This is not only important for setting up the op blockers on
insertion, but also for removing them on ejection since bdrv_delete()
asserts that there are no op blockers set up.

Signed-off-by: Max Reitz 
---
  block.c |  7 
  block/block-backend.c   | 19 +++--
  blockdev-nbd.c  | 36 +
  hw/block/dataplane/virtio-blk.c | 77 +++-
  hw/scsi/virtio-scsi.c   | 87 ++---
  include/block/block.h   |  1 -
  include/block/block_int.h   |  2 -
  include/hw/virtio/virtio-scsi.h | 10 +
  include/sysemu/block-backend.h  |  3 +-
  nbd.c   | 13 ++
  10 files changed, 182 insertions(+), 73 deletions(-)


[snip]


diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 5469bad..570f2fd 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -755,6 +755,37 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice 
*dev, SCSISense sense)
  }
  }
  
+static void virtio_scsi_set_up_op_blockers(VirtIOSCSI *s, SCSIDevice *sd)

+{
+if (!s->blocker) {
+error_setg(&s->blocker, "block device is in use by data plane");


This will no longer be necessary with v2 of my virtio-scsi op blocker patch.


+}
+blk_op_block_all(sd->conf.blk, s->blocker);
+}
+
+static void virtio_scsi_remove_op_blockers(VirtIOSCSI *s, SCSIDevice *sd)
+{
+if (s->blocker) {
+blk_op_unblock_all(sd->conf.blk, s->blocker);
+}
+}
+
+static void virtio_scsi_blk_insert_notifier(Notifier *n, void *data)
+{
+VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier,
+n, n);
+assert(cn->sd->conf.blk == data);
+virtio_scsi_set_up_op_blockers(cn->s, cn->sd);
+}
+
+static void virtio_scsi_blk_remove_notifier(Notifier *n, void *data)
+{
+VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier,
+n, n);
+assert(cn->sd->conf.blk == data);
+virtio_scsi_remove_op_blockers(cn->s, cn->sd);
+}
+
  static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
  Error **errp)
  {
@@ -763,12 +794,26 @@ static void virtio_scsi_hotplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
  SCSIDevice *sd = SCSI_DEVICE(dev);
  
  if (s->ctx && !s->dataplane_disabled) {

+VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier;
+
+insert_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1);
+insert_notifier->n.notify = virtio_scsi_blk_insert_notifier;
+insert_notifier->s = s;
+insert_notifier->sd = sd;
+blk_add_insert_bs_notifier(sd->conf.blk, &insert_notifier->n);
+QTAILQ_INSERT_TAIL(&s->insert_notifiers, insert_notifier, next);
+
+remove_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1);
+remove_notifier->n.notify = virtio_scsi_blk_remove_notifier;
+remove_notifier->s = s;
+remove_notifier->sd = sd;
+blk_add_remove_bs_notifier(sd->conf.blk, &remove_notifier->n);
+QTAILQ_INSERT_TAIL(&s->remove_notifiers, remove_notifier, next);
+
  if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
  return;
  }
-assert(!s->blocker);
-error_setg(&s->blocker, "block device is in use by data plane");


Same for these two lines removed (they just don't exist any more).


-blk_op_block_all(sd->conf.blk, s->blocker);
+virtio_scsi_set_up_op_blockers(s, sd);
  }
  
  if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {

@@ -784,6 +829,7 @@ static void virtio_scsi_hotunplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
  VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
  VirtIOSCSI *s = VIRTIO_SCSI(vdev);
  SCSIDevice *sd = SCSI_DEVICE(dev);
+VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier;
  
  if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {

  virtio_scsi_push_event(s, sd,
@@ -791,11 +837,39 @@ static void virtio_scsi_hotunplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 VIRTIO_SCSI_EVT_RESET_REMOVED);
  }
  
-if (s->ctx && s->blocker) {

-blk_op_unblock_all(sd->conf.blk, s->blocker);
+if (s->ctx) {
+virtio_scsi_remove_op_blo

Re: [Qemu-devel] [PATCH RFC v4 11/13] qapi: add md5 checksum of last dirty bitmap level to query-block

[John Snow]

On 02/27/2015 01:32 PM, Eric Blake wrote:

On 02/27/2015 10:24 AM, Vladimir Sementsov-Ogievskiy wrote:

Reviewed-by: John Snow 
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block.c| 1 +
  include/qemu/hbitmap.h | 8 
  qapi/block-core.json   | 4 +++-
  util/hbitmap.c | 8 
  4 files changed, 20 insertions(+), 1 deletion(-)




+++ b/qapi/block-core.json
@@ -336,11 +336,13 @@
  #
  # @frozen: whether the dirty bitmap is frozen (Since 2.3)
  #
+# @md5: md5 checksum of the last bitmap level (since 2.3)
+#
  # Since: 1.3
  ##
  { 'type': 'BlockDirtyInfo',
'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
-   'disabled': 'bool', 'frozen': 'bool'} }
+   'disabled': 'bool', 'frozen': 'bool', 'md5': 'str'} }


How long does it take to compute the md5 sum?  Is enabling this
information unconditionally going to significantly slow down the call,
when the information is useful primarily only for debugging?

That said, it looks okay code-wise, so as long as I am not uncovering a
design flaw:
Reviewed-by: Eric Blake 



A dirty bitmap that contains 1MiB of raw data using the default 
granularity of 64KiB describes 512GiB of disk space.


Allocating 1MiB, filling each byte with a pattern (255 % offset), 
computing the MD5 checksum and printing the hash takes, on my computer, 
.006 seconds.


The same procedure with 8MiB (4TiB) and timing exclusively the hash 
computation, it's 0.014033 seconds.


The time starts to get appreciable at around 64MiB of data, which would 
imply 32TiB of disk, which takes about a tenth of a second.

[Qemu-devel] [PATCH v3] block/vdi: Add locking for parallel requests

[Max Reitz]

When allocating a new cluster, the first write to it must be the one
doing the allocation, because that one pads its write request to the
cluster size; if another write to that cluster is executed before it,
that write will be overwritten due to the padding.

See https://bugs.launchpad.net/qemu/+bug/1422307 for what can go wrong
without this patch.

Cc: qemu-stable 
Signed-off-by: Max Reitz 
---
v3: Hopefully finally found the real issue which causes the problems
described in the bug report; at least it sounds very reasonable and
I can no longer reproduce any of the issues described there.
Thank you, Paolo and Stefan!
---
 block/vdi.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/block/vdi.c b/block/vdi.c
index 74030c6..53bd02f 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -53,6 +53,7 @@
 #include "block/block_int.h"
 #include "qemu/module.h"
 #include "migration/migration.h"
+#include "block/coroutine.h"
 
 #if defined(CONFIG_UUID)
 #include 
@@ -196,6 +197,8 @@ typedef struct {
 /* VDI header (converted to host endianness). */
 VdiHeader header;
 
+CoMutex write_lock;
+
 Error *migration_blocker;
 } BDRVVdiState;
 
@@ -504,6 +507,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options, 
int flags,
   "vdi", bdrv_get_device_name(bs), "live migration");
 migrate_add_blocker(s->migration_blocker);
 
+qemu_co_mutex_init(&s->write_lock);
+
 return 0;
 
  fail_free_bmap:
@@ -639,11 +644,31 @@ static int vdi_co_write(BlockDriverState *bs,
buf, n_sectors * SECTOR_SIZE);
 memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
(s->block_sectors - n_sectors - sector_in_block) * 
SECTOR_SIZE);
+
+/* Note that this coroutine does not yield anywhere from reading 
the
+ * bmap entry until here, so in regards to all the coroutines 
trying
+ * to write to this cluster, the one doing the allocation will
+ * always be the first to try to acquire the lock.
+ * Therefore, it is also the first that will actually be able to
+ * acquire the lock and thus the padded cluster is written before
+ * the other coroutines can write to the affected area. */
+qemu_co_mutex_lock(&s->write_lock);
 ret = bdrv_write(bs->file, offset, block, s->block_sectors);
+qemu_co_mutex_unlock(&s->write_lock);
 } else {
 uint64_t offset = s->header.offset_data / SECTOR_SIZE +
   (uint64_t)bmap_entry * s->block_sectors +
   sector_in_block;
+qemu_co_mutex_lock(&s->write_lock);
+/* This lock is only used to make sure the following write 
operation
+ * is executed after the write issued by the coroutine allocating
+ * this cluster, therefore we do not need to keep it locked.
+ * As stated above, the allocating coroutine will always try to 
lock
+ * the mutex before all the other concurrent accesses to that
+ * cluster, therefore at this point we can be absolutely certain
+ * that that write operation has returned (there may be other 
writes
+ * in flight, but they do not concern this very operation). */
+qemu_co_mutex_unlock(&s->write_lock);
 ret = bdrv_write(bs->file, offset, buf, n_sectors);
 }
 
-- 
2.1.0

[Qemu-devel] [Bug 1426472] Re: Recent regression: segfault on startup with -snapshot

[Paolo Bonzini]

** Changed in: qemu
   Status: New => In Progress

** Changed in: qemu
   Importance: Undecided => Critical

** Changed in: qemu
 Assignee: (unassigned) => Paolo Bonzini (bonzini)

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

Title:
  Recent regression: segfault on startup with -snapshot

Status in QEMU:
  In Progress

Bug description:
  As of git revision 041ccc922ee474693a2869d4e3b59e920c739bc0, qemu
  segfaults on startup when I try to boot a hard disk image with the
  -snapshot option.

  To reproduce:

wget http://wiki.qemu.org/download/linux-0.2.img.bz2
bunzip2 linux-0.2.img.bz2 
qemu-system-i386 -hda linux-0.2.img -snapshot

  When I run this, qemu-system-i386 crashes with a segmentation fault.
  This is on a Debian 7 amd64 host.

  git bisect implicates the following commit:

  commit a464982499b2f637f6699e3d03e0a9d2e0b5288b
  Author: Paolo Bonzini 
  Date:   Wed Feb 11 17:15:18 2015 +0100

  rcu: run RCU callbacks under the BQL

  This needs to go away sooner or later, but one complication is the
  complex VFIO data structures that are modified in instance_finalize.
  Take a shortcut for now.

  Reviewed-by: Michael Roth 
  Tested-by: Michael Roth 
  Signed-off-by: Paolo Bonzini 

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

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Paolo Bonzini]

On 19/02/2015 08:48, Michael S. Tsirkin wrote:
> V=1 should show what's going on, it's not nice
> to silence things unconditionally.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 6817c6f..84ca8be 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -110,7 +110,7 @@ endif
>  
>  %/config-devices.mak: default-configs/%.mak
>   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh 
> $@ $<, "  GEN   $@")
> - @if test -f $@; then \
> + $(call quiet-command, if test -f $@; then \
> if cmp -s $@.old $@; then \
>   mv $@.tmp $@; \
>   cp -p $@ $@.old; \
> @@ -126,7 +126,7 @@ endif
>else \
> mv $@.tmp $@; \
> cp -p $@ $@.old; \
> -  fi
> +  fi, "  TEST $@");
>  
>  defconfig:
>   rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> 

Squashing this to make the non-verbose messages clearer, ok?

diff --git a/Makefile b/Makefile
index 5604209..d92d4cd 100644
--- a/Makefile
+++ b/Makefile
@@ -109,7 +109,7 @@ endif
 -include $(SUBDIR_DEVICES_MAK_DEP)
 
 %/config-devices.mak: default-configs/%.mak
-   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh 
$@ $<, "  GEN   $@")
+   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh 
$@.tmp $<, "  GEN   $@.tmp")
$(call quiet-command, if test -f $@; then \
  if cmp -s $@.old $@; then \
mv $@.tmp $@; \
@@ -126,7 +126,7 @@ endif
 else \
  mv $@.tmp $@; \
  cp -p $@ $@.old; \
-fi, "  TEST $@");
+fi, "  GEN  $@");
 
 defconfig:
rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh
index 7242707..7958086 100644
--- a/scripts/make_device_config.sh
+++ b/scripts/make_device_config.sh
@@ -2,7 +2,7 @@
 # Construct a target device config file from a default, pulling in any
 # files from include directives.
 
-dest=$1.tmp
+dest=$1
 dep=`dirname $1`-`basename $1`.d
 src=$2
 src_dir=`dirname $src`

[Qemu-devel] [PATCH 0/2] RFC: Rolling statistics

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Hi,
  This is an attempt at a generic rolling statistics utility to
allow data (e.g. bandwidth usage, times etc) to be collected
easily.  They hold some basic values (min/max/mean/weighted mean)
and the last 'n' raw values.I'd like to use this
maybe in fault-tolerance code.

  This is a first cut, and I think I probably need to rework it
as a qapi type somehow, but I'm interested in thoughts.

Dave


Dr. David Alan Gilbert (2):
  Rolling statistics utilities
  Tests for rolling statistics code

 include/qemu/rolling-stats.h | 101 +++
 include/qemu/typedefs.h  |   1 +
 tests/Makefile   |   3 +
 tests/test-rolling-stats.c   | 161 ++
 util/Makefile.objs   |   1 +
 util/rolling-stats.c | 393 +++
 6 files changed, 660 insertions(+)
 create mode 100644 include/qemu/rolling-stats.h
 create mode 100644 tests/test-rolling-stats.c
 create mode 100644 util/rolling-stats.c

-- 
2.1.0

[Qemu-devel] [PATCH 1/2] Rolling statistics utilities

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

There are various places where it's useful to hold a series
of values that change over time and get summaries about them.

This provides:

   - a count of the number of items
   - min/max
   - mean
   - a weighted mean (where you can set the weight to determine
  whether it changes quickly or slowly)
   - the last 'n' values

Signed-off-by: Dr. David Alan Gilbert 
---
 include/qemu/rolling-stats.h | 101 +++
 include/qemu/typedefs.h  |   1 +
 util/Makefile.objs   |   1 +
 util/rolling-stats.c | 393 +++
 4 files changed, 496 insertions(+)
 create mode 100644 include/qemu/rolling-stats.h
 create mode 100644 util/rolling-stats.c

diff --git a/include/qemu/rolling-stats.h b/include/qemu/rolling-stats.h
new file mode 100644
index 000..d64ce78
--- /dev/null
+++ b/include/qemu/rolling-stats.h
@@ -0,0 +1,101 @@
+/*
+ * A container for statistics that are measured repeatedly.
+ *
+ * Copyright 2015 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ *   Dr. David Alan Gilbert 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef __QEMU_ROLLING_STATS_H
+#define __QEMU_ROLLING_STATS_H 1
+
+#include "qemu/typedefs.h"
+/**
+ * Allocate and initialise an 'RStats' structure.
+ *
+ * @len:Number of values to hold
+ * @weight: Weight for the weighted average, between 0..1
+ *  smaller values cause the average to update more quickly.
+ *
+ * Returns: A pointer to the RStats structure.
+ */
+RStats *rstats_init(size_t len, double weight);
+
+/**
+ * Deallocate the RStats structure
+ */
+void rstats_destroy(RStats *r);
+
+/**
+ * Add a new value into the RStats record.
+ *
+ * @r: The RStats to update
+ * @value: The new value to be recorded
+ * @tag: A tag to be associated with the value, not used by the calculations
+ *   (e.g. a time or iteration count)
+ */
+void rstats_add_value(RStats *r, double value, uint64_t tag);
+
+/**
+ * Reset the RStats structure
+ *
+ */
+void rstats_reset(RStats *r);
+
+/**
+ * Return a string representing the RStats data, intended for human consumption
+ *
+ * Returns: An allocated string the caller must free
+ *  or NULL on error
+ *
+ * e.g. (Without the line break!)
+ *   Min/Max: -3.57, 126.3  Mean: 7.83 (weighted: 8.56)  Values: 4.3@458,
+ *   5.8@783, 1.2@950, 7.9@951, 10.3@952
+ */
+char *rstats_as_pretty_string(RStats *r);
+
+/**
+ * Return a string representing the RStats data, intended for JSON parsing
+ *
+ * Returns: An allocated string the caller must free
+ *  or NULL on error
+ *
+ * e.g.
+ *{ "min": -3.57, "max": 126.3, "mean": 7.83, "weighted_mean": 8.56,
+ *  "count": 5678,
+ *  "values": [ 4.3, 5.8, 1.2, 7.9, 10.3 ],
+ *  "tags": [ 458, 783, 950, 951, 952 ] }
+ */
+char *rstats_as_json_string(RStats *r);
+
+/**
+ * Return the mean
+ * @r: The RStats to examine
+ *
+ * Returns: The mean value
+ */
+double rstats_get_mean(const RStats *r);
+
+/**
+ * Return the weighted mean
+ * @r: The RStats to examine
+ *
+ * Returns: The weighted mean
+ */
+double rstats_get_weighted_mean(const RStats *r);
+
+/**
+ * Return the minimum and maximum values
+ * @r: The RStats to examine
+ * @min: Pointer to return the minimum in
+ * @max: Pointer to return the maximum in
+ *
+ */
+void rstats_get_min_max(const RStats *r, double *min, double *max);
+
+#endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index cde3314..3488dfd 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -69,6 +69,7 @@ typedef struct QEMUSizedBuffer QEMUSizedBuffer;
 typedef struct QEMUTimerListGroup QEMUTimerListGroup;
 typedef struct QEMUTimer QEMUTimer;
 typedef struct Range Range;
+typedef struct RStats RStats;
 typedef struct SerialState SerialState;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct SMBusDevice SMBusDevice;
diff --git a/util/Makefile.objs b/util/Makefile.objs
index ceaba30..b45af66 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -17,4 +17,5 @@ util-obj-y += throttle.o
 util-obj-y += getauxval.o
 util-obj-y += readline.o
 util-obj-y += rfifolock.o
+util-obj-y += rolling-stats.o
 util-obj-y += rcu.o
diff --git a/util/rolling-stats.c b/util/rolling-stats.c
new file mode 100644
index 000..f2f1984
--- /dev/null
+++ b/util/rolling-stats.c
@@ -0,0 +1,393 @@
+/*
+ * A container for statistics that are measured repeatedly.
+ *
+ * Copyright 2015 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ *   Dr. David Alan Gilbert 
+ *
+ * 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 
+
+#include "qemu-common.h"
+#include "qemu/rolling-stats.h"
+#include "qemu/thread.h"
+#include "qemu/typedefs.h"
+
+struct RStats {
+size_t allocated;  /* Amount of space for 

[Qemu-devel] [PATCH 0/2] cpus: fix deadlock and segfault

[Paolo Bonzini]

Reported by Leon Alrae on the mailing list, and by
Andreas Gustafsson as Launchpad bug 1426472.

Paolo Bonzini (2):
  cpus: fix deadlock and segfault in qemu_mutex_lock_iothread
  cpus: be more paranoid in avoiding deadlocks

 cpus.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.3.0

Re: [Qemu-devel] [PATCH v14 03/19] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove

[Eric Blake]

On 02/20/2015 04:07 PM, John Snow wrote:
> The new command pair is added to manage a user created dirty bitmap. The
> dirty bitmap's name is mandatory and must be unique for the same device,
> but different devices can have bitmaps with the same names.
> 
> The granularity is an optional field. If it is not specified, we will
> choose a default granularity based on the cluster size if available,
> clamped to between 4K and 64K to mirror how the 'mirror' code was
> already choosing granularity. If we do not have cluster size info
> available, we choose 64K. This code has been factored out into a helper
> shared with block/mirror.
> 
> This patch also introduces the 'block_dirty_bitmap_lookup' helper,
> which takes a device name and a dirty bitmap name and validates the
> lookup, returning NULL and setting errp if there is a problem with
> either field. This helper will be re-used in future patches in this
> series.
> 
> The types added to block-core.json will be re-used in future patches
> in this series, see:
> 'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}'
> 
> Signed-off-by: John Snow 
> Reviewed-by: Max Reitz 
> ---
>  block.c   |  20 ++
>  block/mirror.c|  10 +
>  blockdev.c| 100 
> ++
>  include/block/block.h |   1 +
>  qapi/block-core.json  |  55 +++
>  qmp-commands.hx   |  51 +
>  6 files changed, 228 insertions(+), 9 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

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

[Stefan Hajnoczi]

On Fri, Feb 27, 2015 at 6:17 PM, Stefan Hajnoczi  wrote:
> Ekaterina Tumanova (5):
>   block: add bdrv functions for geometry and blocksize
>   raw-posix: Factor block size detection out of raw_probe_alignment()
>   block: Add driver methods to probe blocksizes and geometry
>   block-backend: Add wrappers for blocksizes and geometry probing
>   BlockConf: Call backend functions to detect geometry and blocksizes

Max Reitz found an issue with this patch.

Peter: Please squash the following trivial fix into "BlockConf: Call
backend functions to detect geometry and blocksizes":
https://lists.nongnu.org/archive/html/qemu-devel/2015-02/msg05512.html

I want to avoid spamming the list with another 60 patches.

Stefan

[Qemu-devel] [PATCH 1/2] cpus: fix deadlock and segfault in qemu_mutex_lock_iothread

[Paolo Bonzini]

When two threads (other than the low-priority TCG VCPU thread)
are competing for the iothread lock, a deadlock can happen.  This
is because iothread_requesting_mutex is set to false by the first
thread that gets the mutex, and then the VCPU thread might never
yield from the execution loop.  If iothread_requesting_mutex is
changed from a bool to a counter, the deadlock is fixed.

However, there is another bug in qemu_mutex_lock_iothread that
can be triggered by the new call_rcu thread.  The bug happens
if qemu_mutex_lock_iothread is called before the CPUs are
created.  In that case, first_cpu is NULL and the caller
segfaults in qemu_mutex_lock_iothread.  To fix this, just
do not do the kick if first_cpu is NULL.

Reported-by: Leon Alrae 
Reported-by: Andreas Gustafsson 
Signed-off-by: Paolo Bonzini 
---
 cpus.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/cpus.c b/cpus.c
index 1cd9867..83c078e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -778,7 +778,7 @@ static void qemu_tcg_init_cpu_signals(void)
 
 static QemuMutex qemu_global_mutex;
 static QemuCond qemu_io_proceeded_cond;
-static bool iothread_requesting_mutex;
+static unsigned iothread_requesting_mutex;
 
 static QemuThread io_thread;
 
@@ -1115,15 +1115,15 @@ bool qemu_in_vcpu_thread(void)
 
 void qemu_mutex_lock_iothread(void)
 {
-if (!tcg_enabled()) {
+if (!tcg_enabled() || !first_cpu) {
 qemu_mutex_lock(&qemu_global_mutex);
 } else {
-iothread_requesting_mutex = true;
+atomic_inc(&iothread_requesting_mutex);
 if (qemu_mutex_trylock(&qemu_global_mutex)) {
 qemu_cpu_kick_thread(first_cpu);
 qemu_mutex_lock(&qemu_global_mutex);
 }
-iothread_requesting_mutex = false;
+atomic_dec(&iothread_requesting_mutex);
 qemu_cond_broadcast(&qemu_io_proceeded_cond);
 }
 }
-- 
2.3.0

Re: [Qemu-devel] [PULL 04/11] target-i386: Rename cpu_x86_init() to cpu_x86_init_user()

[Eduardo Habkost]

On Fri, Feb 27, 2015 at 03:10:27PM +0100, Andreas Färber wrote:
> Am 26.02.2015 um 16:59 schrieb Eduardo Habkost:
> > On Wed, Feb 25, 2015 at 11:06:55PM +0100, Andreas Färber wrote:
> >> Am 25.02.2015 um 20:58 schrieb Eduardo Habkost:
> >>> The function is used only for CONFIG_USER, so make its purpose clear.
> >>>
> >>> Reviewed-by: Paolo Bonzini 
> >>> Signed-off-by: Eduardo Habkost 
> >>> ---
> >>>  target-i386/cpu.c | 2 +-
> >>>  target-i386/cpu.h | 4 ++--
> >>>  2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> Please don't. I happily got all architectures aligned that it's at least
> >> cpu_something_init, and it only happens to be user-only for x86. It is
> >> rather the legacy function that was used in both system and user.
> > 
> > If that's a legacy function, what are the steps you plan to follow to
> > eliminate it? I would be glad to help eliminating legacy code.
> > 
> > Initialization of CPUs in *-user and *-softmmu is different in i386, so
> > we are going to have different code for both. How do you think I should
> > name the *-user-specific CPU init function in target-i386, then?
> 
> I would prefer to leave its name as it is (unless we are renaming all,
> which would probably be a waste of effort giving the next steps) and
> simply not use it in PC code. If you want to enforce this, you could
> #ifdef CONFIG_USER_ONLY it.
> 
> For some targets - as can be seen in your uc32 patch - there is already
> a cpu_generic_init() that calls into the CPUClass hooks of the given CPU
> type. I would like to call that from linux-user directly (or from a
> lightweight wrapper to be shared between linux-user and bsd-user, I
> assume we're going need some target-specific #ifdefs) and drop
> cpu_init() in its current form. In particular I want to somehow move the
> realized=true part out of it, which means either inlining it into dozens
> of machines or finishing the recursive realization work so that we only
> need one central realized=true for /machine.

Moving realized=true outside cpu_generic_init() would make lots of sense
for PC, as we already need to perform additional steps in the PC init
code before realizing the CPU (and I expect the list of PC-specific CPU
initialization steps to grow). And when we do this, cpu_x86_init()
(used by CONFIG_USER) and cpu_x86_create() (used by PC) can become the
same function.

But I don't see how to implement this without requiring changing machine
code case-by-case, as existing machine code may expect realized=true to
be set very early and break if you don't set it immediately after
cpu_*_init(). In other words, even if we implement recursive
realization, we may need to inline realize=true on all machines as an
intermediate (and automated) step, and then change machines to just rely
on recursive realization, one by one.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] Makefile.target: binary depends on config-devices

[Paolo Bonzini]

On 19/02/2015 08:48, Michael S. Tsirkin wrote:
> relink binary whenever config-devices.mak changes:
> this makes sense as we are adding/removing devices,
> so binary has to be relinked to be up to date.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  Makefile.target | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.target b/Makefile.target
> index 58c6ae1..2262d89 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,9 +175,11 @@ all-obj-y += $(common-obj-y)
>  all-obj-y += $(target-obj-y)
>  all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
>  
> +$(QEMU_PROG_BUILD): config-devices.mak
> +
>  # build either PROG or PROGW
>  $(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
> - $(call LINK,$^)
> + $(call LINK, $(filter-out %.mak, $^))
>  
>  gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh
>   $(call quiet-command,rm -f $@ && $(SHELL) 
> $(SRC_PATH)/scripts/feature_to_c.sh $@ $(TARGET_XML_FILES),"  GEN   
> $(TARGET_DIR)$@")
> 

Not exactly beautiful, but I don't have any better idea.

Applied, thanks.

Paolo

Re: [Qemu-devel] [PATCH] Makefile.objs: add dummy rule for .dsl files

[Paolo Bonzini]

On 26/02/2015 17:33, Michael S. Tsirkin wrote:
> .hex files are created from .dsl files, so
> cpp adds the dependency %.hex: %.dsl automatically,
> but fails to add a dummy rule so if .dsl
> file is missing, make will fail.
> 
> This happened to us in the past when we
> removed some dsl files.
> 
> Create an extra .d file with a dummy dependency.
> 
> Cc: Peter Maydell 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/i386/Makefile.objs | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 45b90a8..7a47914 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -19,6 +19,7 @@ iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > 
> /dev/null`" \
>  ifdef IASL
>  #IASL Present. Generate hex files from .dsl
>  hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl 
> $(SRC_PATH)/scripts/acpi_extract_preprocess.py 
> $(SRC_PATH)/scripts/acpi_extract.py
> + $(call quiet-command, echo "$<:" > "$@"-extra.d, "  DEP $@")
>   $(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) 
> $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
>   $(call quiet-command, $(PYTHON) 
> $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  
> ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
>   $(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l 
> -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL 
> $(TARGET_DIR)$*.dsl.i")
> 

Applied, thanks.

Paolo

Re: [Qemu-devel] [PULL 04/11] target-i386: Rename cpu_x86_init() to cpu_x86_init_user()

[Andreas Färber]

Am 27.02.2015 um 20:01 schrieb Eduardo Habkost:
> On Fri, Feb 27, 2015 at 03:10:27PM +0100, Andreas Färber wrote:
>> Am 26.02.2015 um 16:59 schrieb Eduardo Habkost:
>>> On Wed, Feb 25, 2015 at 11:06:55PM +0100, Andreas Färber wrote:
 Am 25.02.2015 um 20:58 schrieb Eduardo Habkost:
> The function is used only for CONFIG_USER, so make its purpose clear.
>
> Reviewed-by: Paolo Bonzini 
> Signed-off-by: Eduardo Habkost 
> ---
>  target-i386/cpu.c | 2 +-
>  target-i386/cpu.h | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

 Please don't. I happily got all architectures aligned that it's at least
 cpu_something_init, and it only happens to be user-only for x86. It is
 rather the legacy function that was used in both system and user.
>>>
>>> If that's a legacy function, what are the steps you plan to follow to
>>> eliminate it? I would be glad to help eliminating legacy code.
>>>
>>> Initialization of CPUs in *-user and *-softmmu is different in i386, so
>>> we are going to have different code for both. How do you think I should
>>> name the *-user-specific CPU init function in target-i386, then?
>>
>> I would prefer to leave its name as it is (unless we are renaming all,
>> which would probably be a waste of effort giving the next steps) and
>> simply not use it in PC code. If you want to enforce this, you could
>> #ifdef CONFIG_USER_ONLY it.
>>
>> For some targets - as can be seen in your uc32 patch - there is already
>> a cpu_generic_init() that calls into the CPUClass hooks of the given CPU
>> type. I would like to call that from linux-user directly (or from a
>> lightweight wrapper to be shared between linux-user and bsd-user, I
>> assume we're going need some target-specific #ifdefs) and drop
>> cpu_init() in its current form. In particular I want to somehow move the
>> realized=true part out of it, which means either inlining it into dozens
>> of machines or finishing the recursive realization work so that we only
>> need one central realized=true for /machine.
> 
> Moving realized=true outside cpu_generic_init() would make lots of sense
> for PC, as we already need to perform additional steps in the PC init
> code before realizing the CPU (and I expect the list of PC-specific CPU
> initialization steps to grow). And when we do this, cpu_x86_init()
> (used by CONFIG_USER) and cpu_x86_create() (used by PC) can become the
> same function.
> 
> But I don't see how to implement this without requiring changing machine
> code case-by-case, as existing machine code may expect realized=true to
> be set very early and break if you don't set it immediately after
> cpu_*_init(). In other words, even if we implement recursive
> realization, we may need to inline realize=true on all machines as an
> intermediate (and automated) step, and then change machines to just rely
> on recursive realization, one by one.

Yes, that's why it's not done yet, it takes a lot of review and testing.
It is not going to be a pure Coccinelle patch. :) Doing that for the
inlining is an idea I had not yet considered, thanks.

On my qom-cpu-x86 branch I have already moved realized=true for the PC,
in order to make a CPU core object its parent.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Stefan Hajnoczi]

On Fri, Feb 27, 2015 at 6:29 PM, Max Reitz  wrote:
> On 2015-02-27 at 13:26, Ekaterina Tumanova wrote:
>>
>> check conf.blk before calling blkconf_blocksizes
>>
>> Signed-off-by: Ekaterina Tumanova 
>> ---
>>   hw/scsi/scsi-disk.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>
>
> Reviewed-by: Max Reitz 

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH v6 02/15] spapr_drc: initial implementation of sPAPRDRConnector device

[Michael Roth]

Quoting Bharata B Rao (2015-02-27 03:52:09)
> On Fri, Feb 27, 2015 at 8:41 AM, Michael Roth  
> wrote:
> > +static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
> > +   int fdt_start_offset, bool coldplug, Error **errp)
> > +{
> > +DPRINTFN("drc: %x, attach", get_index(drc));
> > +
> > +if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) {
> > +error_setg(errp, "an attached device is still awaiting release");
> > +return;
> > +}
> > +g_assert(drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE);
> > +g_assert(fdt || coldplug);
> > +
> > +/* NOTE: setting initial isolation state to UNISOLATED means we can't
> > + * detach unless guest has a userspace/kernel that moves this state
> > + * back to ISOLATED in response to an unplug event, or this is done
> > + * manually by the admin prior. if we force things while the guest
> > + * may be accessing the device, we can easily crash the guest, so we
> > + * we defer completion of removal in such cases to the reset() hook.
> > + */
> > +drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
> > +drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
> 
> BTW shouldn't isolation_state and allocation_state be set to
> UNISOLATED and USABLE in response to guest's rtas-set-indicator calls
> ? At least, that is how it is done/expected for CPU hotplug case. Ref:

Well, for PCI we always start in USABLE (state diagram in PAPR+ 13.4), and
stay there. So set_allocation_state() should actually be a no-op for PCI;
there's no valid transition to UNUSABLE (or EXCHANGE/RECOVER).

For non-PCI, it looks like we should start in UNUSABLE, and let RTAS take it
from there. It also looks like the resource removal shouldn't be finalized
until there's a transition from ISOLATED/USABLE to ISOLATED/UNUSABLE.

I just pushed a patch here that I'm hoping will fix the non-PCI cases. Can
you give it a spin and let me know?

https://github.com/mdroth/qemu/commit/d91b0c6d6f794292e384cf129368aaae90294f5b

> kernel code arch/powerpc/platforms/pseries/dlpar.c:
> dlpar_acquire[release]_drc().
> 
> Same question for detach().
> 
> Regards,
> Bharata.

[Qemu-devel] [PATCH 2/2] Tests for rolling statistics code

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Signed-off-by: Dr. David Alan Gilbert 
---
 tests/Makefile |   3 +
 tests/test-rolling-stats.c | 161 +
 2 files changed, 164 insertions(+)
 create mode 100644 tests/test-rolling-stats.c

diff --git a/tests/Makefile b/tests/Makefile
index 307035c..2d42eb2 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -72,6 +72,8 @@ check-unit-y += tests/test-qemu-opts$(EXESUF)
 gcov-files-test-qemu-opts-y = qom/test-qemu-opts.c
 check-unit-y += tests/test-write-threshold$(EXESUF)
 gcov-files-test-write-threshold-y = block/write-threshold.c
+check-unit-y += tests/test-rolling-stats$(EXESUF)
+gcov-files-test-rolling-stats = util/rolling-stats.c
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -367,6 +369,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o 
qemu-char.o qemu-timer.o
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a 
libqemustub.a
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o 
$(block-obj-y) libqemuutil.a libqemustub.a
+tests/test-rolling-stats$(EXESUF): tests/test-rolling-stats.o libqemuutil.a 
libqemustub.a
 
 ifeq ($(CONFIG_POSIX),y)
 LIBS += -lutil
diff --git a/tests/test-rolling-stats.c b/tests/test-rolling-stats.c
new file mode 100644
index 000..85dc949
--- /dev/null
+++ b/tests/test-rolling-stats.c
@@ -0,0 +1,161 @@
+/*
+ * Test rolling statistics framework
+ *
+ * Copyright 2015 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ *   Dr. David Alan Gilbert 
+ *
+ * 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.
+ *
+ */
+
+#include "qemu-common.h"
+#include "qemu/rolling-stats.h"
+
+static void test_1(void)
+{
+RStats *r = rstats_init(5, 0.2);
+char *str;
+
+str = rstats_as_pretty_string(r);
+g_assert_cmpstr(str, ==, "Min/Max: 0, 0 Mean: 0 (Weighted: 0)"
+ " Count: 0 Values: ");
+g_free(str);
+str = rstats_as_json_string(r);
+g_assert_cmpstr(str, ==, "{ \"min\": 0, \"max\": 0, \"mean\": 0,"
+ " \"weighted_mean\": 0, \"count\": 0,"
+ " \"values\": [ ] }");
+g_free(str);
+
+rstats_add_value(r, 1.0, 11);
+str = rstats_as_pretty_string(r);
+g_assert_cmpstr(str, ==, "Min/Max: 1, 1 Mean: 1 (Weighted: 1)"
+ " Count: 1 Values: 1@11");
+g_free(str);
+str = rstats_as_json_string(r);
+g_assert_cmpstr(str, ==, "{ \"min\": 1, \"max\": 1, \"mean\": 1,"
+ " \"weighted_mean\": 1, \"count\": 1,"
+ " \"values\": ["
+ " { \"value\": 1, \"tag\": 11 }"
+ " ] }");
+g_free(str);
+
+rstats_add_value(r, 2.0, 22);
+str = rstats_as_pretty_string(r);
+g_assert_cmpstr(str, ==, "Min/Max: 1, 2 Mean: 1.5 (Weighted: 1.8)"
+ " Count: 2 Values: 1@11, 2@22");
+g_free(str);
+str = rstats_as_json_string(r);
+g_assert_cmpstr(str, ==, "{ \"min\": 1, \"max\": 2, \"mean\": 1.5,"
+ " \"weighted_mean\": 1.8, \"count\": 2,"
+ " \"values\": ["
+ " { \"value\": 1, \"tag\": 11 },"
+ " { \"value\": 2, \"tag\": 22 }"
+ " ] }");
+g_free(str);
+
+rstats_add_value(r, -1.5, 33);
+str = rstats_as_pretty_string(r);
+g_assert_cmpstr(str, ==, "Min/Max: -1.5, 2 Mean: 0.5 (Weighted: -0.84)"
+ " Count: 3 Values: 1@11, 2@22, -1.5@33");
+g_free(str);
+str = rstats_as_json_string(r);
+g_assert_cmpstr(str, ==, "{ \"min\": -1.5, \"max\": 2, \"mean\": 0.5,"
+ " \"weighted_mean\": -0.84, \"count\": 3,"
+ " \"values\": ["
+ " { \"value\": 1, \"tag\": 11 },"
+ " { \"value\": 2, \"tag\": 22 },"
+ " { \"value\": -1.5, \"tag\": 33 }"
+ " ] }");
+g_free(str);
+
+rstats_add_value(r, 0.5, 44);
+str = rstats_as_pretty_string(r);
+g_assert_cmpstr(str, ==, "Min/Max: -1.5, 2 Mean: 0.5 (Weighted: 0.232)"
+ " Count: 4 Values: 1@11, 2@22, -1.5@33, 0.5@44");
+g_free(str);
+str = rstats_as_json_string(r);
+g_assert_cmpstr(str, ==, "{ \"min\": -1.5, \"max\": 2,"
+ " \"mean\": 0.5,"
+ " \"weighted_mean\": 0.232, \"count\": 4,"
+ " \"values\": ["
+ " { \"value\": 1, \"tag\": 11 },"
+ " { \"value\": 2, \"tag\": 22 },"
+ " { \"value\": -1.5, \"tag

Re: [Qemu-devel] [PATCH v2] virtio-scsi: Allocate op blocker reason before blocking

[Paolo Bonzini]

On 27/02/2015 18:11, Max Reitz wrote:
> s->blocker is really only used in hw/scsi/virtio-scsi.c; the only places
> where it is used in hw/scsi/virtio-scsi-dataplane.c is when it is
> allocated and when it is freed. That does not make a whole lot of sense
> (and is actually wrong because this leads to s->blocker potentially
> being NULL when blk_op_block_all() is called in virtio-scsi.c), so move
> the allocation and destruction of s->blocker to the device realization
> and unrealization in virtio-scsi.c, respectively.
> 
> Case in point:
> 
> $ echo -e 'eject drv\nquit' | \
> x86_64-softmmu/qemu-system-x86_64 \
> -monitor stdio -machine accel=qtest -display none \
> -object iothread,id=thr -device virtio-scsi-pci,iothread=thr \
> -drive if=none,file=test.qcow2,format=qcow2,id=drv \
> -device scsi-cd,drive=drv
> 
> Without this patch:
> 
> (qemu) eject drv
> [1]10102 done
>10103 segmentation fault (core dumped)
> 
> With this patch:
> 
> (qemu) eject drv
> Device 'drv' is busy: block device is in use by data plane
> (qemu) quit
> 
> Signed-off-by: Max Reitz 
> ---
> v2:
> - Put the reproducer into the commit message [Markus] and modified its
>   wording to be more fitting of a commit message ("Case in point"
>   instead of the imperative "Try").
> - As noted by Fam on my bdrv_close_all() series, there can be multiple
>   block devices per virtio-scsi bus; therefore, it's wrong to delete the
>   blocker if one of these is unplugged. Instead, just allocate the
>   blocker with the virtio-scsi device itself and free it when the device
>   is unrealized.
> ---
>  hw/scsi/virtio-scsi-dataplane.c | 4 
>  hw/scsi/virtio-scsi.c   | 4 
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> index 03a1e8c..9b775d4 100644
> --- a/hw/scsi/virtio-scsi-dataplane.c
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -211,8 +211,6 @@ void virtio_scsi_dataplane_start(VirtIOSCSI *s)
>  
>  s->dataplane_starting = true;
>  
> -assert(!s->blocker);
> -error_setg(&s->blocker, "block device is in use by data plane");
>  /* Set up guest notifier (irq) */
>  rc = k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, true);
>  if (rc != 0) {
> @@ -279,8 +277,6 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
>  if (!s->dataplane_started || s->dataplane_stopping) {
>  return;
>  }
> -error_free(s->blocker);
> -s->blocker = NULL;
>  s->dataplane_stopping = true;
>  assert(s->ctx == iothread_get_aio_context(vs->conf.iothread));
>  
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 9e2c718..9c2a272 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -904,6 +904,8 @@ static void virtio_scsi_device_realize(DeviceState *dev, 
> Error **errp)
>  virtio_scsi_save, virtio_scsi_load, s);
>  s->migration_state_notifier.notify = virtio_scsi_migration_state_changed;
>  add_migration_state_change_notifier(&s->migration_state_notifier);
> +
> +error_setg(&s->blocker, "block device is in use by data plane");
>  }
>  
>  static void virtio_scsi_instance_init(Object *obj)
> @@ -929,6 +931,8 @@ static void virtio_scsi_device_unrealize(DeviceState 
> *dev, Error **errp)
>  {
>  VirtIOSCSI *s = VIRTIO_SCSI(dev);
>  
> +error_free(s->blocker);
> +
>  unregister_savevm(dev, "virtio-scsi", s);
>  remove_migration_state_change_notifier(&s->migration_state_notifier);
>  
> 

Applying, thanks.

Paolo

Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests

[Max Reitz]

On 2015-02-27 at 13:15, Max Reitz wrote:

On 2015-02-27 at 13:12, Stefan Weil wrote:

Am 27.02.2015 um 19:09 schrieb Max Reitz:

It always fails for me. Do you have an SSD?

On the real machine: yes. On the virtual machine: maybe.


I don't, so maybe that's the issue. But running it in tmpfs doesn't 
change anything for me, still fails (every time).


And now it doesn't fail any more on my HDD (but still in tmpfs). Great.

How about this test case (which is in line with what Paolo identified is 
the potential problem)? :-)


Max


test.rb
Description: application/ruby

Re: [Qemu-devel] [PATCH RFC v4 06/13] qapi: add dirty-bitmaps migration capability

[Eric Blake]

On 02/27/2015 10:24 AM, Vladimir Sementsov-Ogievskiy wrote:
> Reviewed-by: John Snow 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  include/migration/migration.h | 1 +
>  migration/migration.c | 9 +
>  qapi-schema.json  | 5 -
>  3 files changed, 14 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 


>  #
> +# @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps. (since 
> 2.3)

Just to make sure, this only affects the source side, and does not have
to be set on the destination (that is, the destination will
automatically handle incoming dirty bitmaps correctly without having to
tweak the knob first)?  Of course, libvirt will check that the
destination advertises the feature before enabling the knob on the
source (to avoid the case of the source sending something the
destination won't understand).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 08/21] rcu: run RCU callbacks under the BQL

[Paolo Bonzini]

On 27/02/2015 15:24, Leon Alrae wrote:
> On 27/02/2015 13:17, Paolo Bonzini wrote:
>> Can you test this patch?  (On top of the previous one).
> 
> With this change the system doesn't boot at all I'm afraid.

Hmm, it works for me and fixes the deadlock with Aurelien's images.

Just to be clear, this is the patch I'm testing on top of
origin/master:

diff --git a/cpus.c b/cpus.c
index 1cd9867..83c078e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -778,7 +778,7 @@ static void qemu_tcg_init_cpu_signals(void)

 static QemuMutex qemu_global_mutex;
 static QemuCond qemu_io_proceeded_cond;
-static bool iothread_requesting_mutex;
+static unsigned iothread_requesting_mutex;

 static QemuThread io_thread;

@@ -1115,15 +1115,15 @@ bool qemu_in_vcpu_thread(void)

 void qemu_mutex_lock_iothread(void)
 {
-if (!tcg_enabled()) {
+if (!tcg_enabled() || !first_cpu) {
 qemu_mutex_lock(&qemu_global_mutex);
 } else {
-iothread_requesting_mutex = true;
+atomic_inc(&iothread_requesting_mutex);
 if (qemu_mutex_trylock(&qemu_global_mutex)) {
 qemu_cpu_kick_thread(first_cpu);
 qemu_mutex_lock(&qemu_global_mutex);
 }
-iothread_requesting_mutex = false;
+atomic_dec(&iothread_requesting_mutex);
 qemu_cond_broadcast(&qemu_io_proceeded_cond);
 }
 }


I couldn't reproduce it with stress, but a reboot loop finally triggered
it on Aurelien's images.

Interestingly, the bug has been there forever but was only triggered in
relatively weird cases such as TCG+migration.  My patch just made it
much more visible.

Paolo

> 
> BTW I managed to reproduce the original problem using Aurel's Debian
> images. Could you try and see if you can reproduce the problem as well?
> 
> Images:
> $ wget https://people.debian.org/~aurel32/qemu/mips/vmlinux-3.2.0-4-4kc-malta
> $ wget 
> https://people.debian.org/~aurel32/qemu/mips/debian_squeeze_mips_standard.qcow2
> 
> Run:
> $ qemu-system-mips -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
> debian_squeeze_mips_standard.qcow2 -append "root=/dev/sda1
> console=ttyS0" -snapshot -nographic
> 
> debian-mips login: root
> password: root
> root@debian-mips:~# apt-get install stress
> 
> (and now try to stress a few times, usually after 2 or 3 times QEMU freezes)
> 
> root@debian-mips:~# stress --timeout 15s --cpu 4 --io 2 --vm 2 --vm-bytes 1M

Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests

[Max Reitz]

On 2015-02-27 at 13:09, Max Reitz wrote:

On 2015-02-27 at 12:42, Paolo Bonzini wrote:


On 27/02/2015 15:05, Max Reitz wrote:
Concurrently modifying the bmap does not seem to be a good idea; 
this patch adds
a lock for it. See https://bugs.launchpad.net/qemu/+bug/1422307 for 
what

can go wrong without.

Cc: qemu-stable 
Signed-off-by: Max Reitz 
---
v2:
- Make the mutex cover vdi_co_write() completely [Kevin]
- Add a TODO comment [Kevin]

I think I know what the bug is.  Suppose you have two concurrent writes
to a non-allocated block, one at 16K...32K (in bytes) and one at
32K...48K.  The first write is enlarged to contain zeros, the second is
not.  Then you have two writes in flight:

   0   zeros
   ... zeros
   16K data1
   ... data1
   32K zeros  data2
   ... zeros  data2
   48K zeros
   ... zeros
   64K

And the contents of 32K...48K are undefined.  If the above diagnosis is
correct, I'm not even sure why Max's v1 patch worked...


Maybe that's an issue, too; but the test case I sent out does 1 MB 
requests (and it fails), so this shouldn't matter there.


Considering that test case didn't work for Stefan (Weil), and it fails 
in a pretty strange way for me (no output from the qemu-io command at 
all, and while most reads from raw were successful, all reads from vdi 
failed (the pattern verification failed), maybe that's something 
completely different.


Indeed, when I do sub-MB writes, I get sporadic errors which seem much 
more related to the original bug report, so it's probably the issue you 
found that's the real problem.


Also, my test case suddenly stopped reproducing the issue on my HDD and 
only does it on tmpfs. Weird.


Max


An optimized fix could be to use a CoRwLock, then:


Yes, I'm actually already working on that.

Max


- take it shared (read) around the write in the
"VDI_IS_ALLOCATED(bmap_entry)" path

- take it exclusive (write) around the write in the
"!VDI_IS_ALLOCATED(bmap_entry)" path

Paolo

[Qemu-devel] [PATCH 2/2] cpus: be more paranoid in avoiding deadlocks

[Paolo Bonzini]

For good measure, ensure that the following sequence:

   thread 1 calls qemu_mutex_lock_iothread
   thread 2 calls qemu_mutex_lock_iothread
   VCPU thread are created
   VCPU thread enters execution loop

results in the VCPU threads letting the other two threads run
and obeying iothread_requesting_mutex even if the VCPUs are
not halted.  To do this, check iothread_requesting_mutex
before execution starts.

I could not reproduce this problem; it was only found by
inspection.

Signed-off-by: Paolo Bonzini 
---
 cpus.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 83c078e..0fac143 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1025,6 +1025,9 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 }
 }
 
+/* process any pending work */
+exit_request = 1;
+
 while (1) {
 tcg_exec_all();
 
@@ -1115,10 +1118,11 @@ bool qemu_in_vcpu_thread(void)
 
 void qemu_mutex_lock_iothread(void)
 {
+atomic_inc(&iothread_requesting_mutex);
 if (!tcg_enabled() || !first_cpu) {
 qemu_mutex_lock(&qemu_global_mutex);
+atomic_dec(&iothread_requesting_mutex);
 } else {
-atomic_inc(&iothread_requesting_mutex);
 if (qemu_mutex_trylock(&qemu_global_mutex)) {
 qemu_cpu_kick_thread(first_cpu);
 qemu_mutex_lock(&qemu_global_mutex);
-- 
2.3.0

Re: [Qemu-devel] [PATCH RFC v4 11/13] qapi: add md5 checksum of last dirty bitmap level to query-block

[Eric Blake]

On 02/27/2015 10:24 AM, Vladimir Sementsov-Ogievskiy wrote:
> Reviewed-by: John Snow 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block.c| 1 +
>  include/qemu/hbitmap.h | 8 
>  qapi/block-core.json   | 4 +++-
>  util/hbitmap.c | 8 
>  4 files changed, 20 insertions(+), 1 deletion(-)
> 

> +++ b/qapi/block-core.json
> @@ -336,11 +336,13 @@
>  #
>  # @frozen: whether the dirty bitmap is frozen (Since 2.3)
>  #
> +# @md5: md5 checksum of the last bitmap level (since 2.3)
> +#
>  # Since: 1.3
>  ##
>  { 'type': 'BlockDirtyInfo',
>'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
> -   'disabled': 'bool', 'frozen': 'bool'} }
> +   'disabled': 'bool', 'frozen': 'bool', 'md5': 'str'} }

How long does it take to compute the md5 sum?  Is enabling this
information unconditionally going to significantly slow down the call,
when the information is useful primarily only for debugging?

That said, it looks okay code-wise, so as long as I am not uncovering a
design flaw:
Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Ekaterina Tumanova]

check conf.blk before calling blkconf_blocksizes

Signed-off-by: Ekaterina Tumanova 
---
 hw/scsi/scsi-disk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 2921728..df5140e 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-blkconf_blocksizes(&s->qdev.conf);
+if (s->qdev.conf.blk) {
+blkconf_blocksizes(&s->qdev.conf);
+}
 s->qdev.blocksize = s->qdev.conf.logical_block_size;
 s->qdev.type = TYPE_DISK;
 if (!s->product) {
-- 
2.1.4

[Qemu-devel] [PULL 66/69] qtest/ahci: add flush retry test

[Stefan Hajnoczi]

From: John Snow 

Signed-off-by: John Snow 
Message-id: 1424738729-17082-7-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ahci-test.c | 44 
 1 file changed, 44 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 6c99f19..64dff8d 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -46,6 +46,7 @@
 
 /*** Globals ***/
 static char tmp_path[] = "/tmp/qtest.XX";
+static char debug_path[] = "/tmp/qtest-blkdebug.XX";
 static bool ahci_pedantic;
 
 /*** Function Declarations ***/
@@ -966,6 +967,41 @@ static void test_flush(void)
 ahci_shutdown(ahci);
 }
 
+static void test_flush_retry(void)
+{
+AHCIQState *ahci;
+AHCICommand *cmd;
+uint8_t port;
+const char *s;
+
+prepare_blkdebug_script(debug_path, "flush_to_disk");
+ahci = ahci_boot_and_enable("-drive file=blkdebug:%s:%s,if=none,id=drive0,"
+"format=qcow2,cache=writeback,"
+"rerror=stop,werror=stop "
+"-M q35 "
+"-device ide-hd,drive=drive0 ",
+debug_path,
+tmp_path);
+
+/* Issue Flush Command */
+port = ahci_port_select(ahci);
+ahci_port_clear(ahci, port);
+cmd = ahci_command_create(CMD_FLUSH_CACHE);
+ahci_command_commit(ahci, cmd, port);
+ahci_command_issue_async(ahci, cmd);
+qmp_eventwait("STOP");
+
+/* Complete the command */
+s = "{'execute':'cont' }";
+qmp_async(s);
+qmp_eventwait("RESUME");
+ahci_command_wait(ahci, cmd);
+ahci_command_verify(ahci, cmd);
+
+ahci_command_free(cmd);
+ahci_shutdown(ahci);
+}
+
 
/**/
 /* AHCI I/O Test Matrix Definitions   
*/
 
@@ -1147,6 +1183,7 @@ int main(int argc, char **argv)
 {
 const char *arch;
 int ret;
+int fd;
 int c;
 int i, j, k, m;
 
@@ -1186,6 +1223,11 @@ int main(int argc, char **argv)
 close(mkstemp(tmp_path));
 mkqcow2(tmp_path, TEST_IMAGE_SIZE_MB);
 
+/* Create temporary blkdebug instructions */
+fd = mkstemp(debug_path);
+g_assert(fd >= 0);
+close(fd);
+
 /* Run the tests */
 qtest_add_func("/ahci/sanity", test_sanity);
 qtest_add_func("/ahci/pci_spec",   test_pci_spec);
@@ -1207,11 +1249,13 @@ int main(int argc, char **argv)
 qtest_add_func("/ahci/io/dma/lba28/fragmented", test_dma_fragmented);
 
 qtest_add_func("/ahci/flush/simple", test_flush);
+qtest_add_func("/ahci/flush/retry", test_flush_retry);
 
 ret = g_test_run();
 
 /* Cleanup */
 unlink(tmp_path);
+unlink(debug_path);
 
 return ret;
 }
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2] hmp: info spice: Show string channel name

[Eric Blake]

On 02/27/2015 08:36 AM, Cole Robinson wrote:
> Useful for debugging.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=822418
> Signed-off-by: Cole Robinson 
> ---
> v2:
> Explicitly list spice channel mappings
> Use ARRAY_SIZE macro
> 
>  hmp.c | 26 ++
>  1 file changed, 26 insertions(+)
> 

> +const char *channel_name;
> +const char * const channel_names[] = {
> +[ SPICE_CHANNEL_MAIN ] = "main",
> +[ SPICE_CHANNEL_DISPLAY] = "display",
> +[ SPICE_CHANNEL_INPUTS ] = "input",

Why 'input' instead of 'inputs'?

> +[ SPICE_CHANNEL_CURSOR ] = "cursor",
> +[ SPICE_CHANNEL_PLAYBACK ] = "playback",
> +[ SPICE_CHANNEL_RECORD ] = "record",
> +[ SPICE_CHANNEL_TUNNEL ] = "tunnel",
> +[ SPICE_CHANNEL_SMARTCARD ] = "smartcard",
> +[ SPICE_CHANNEL_USBREDIR ] = "usbredir",
> +[ SPICE_CHANNEL_PORT ] = "port",
> +[ SPICE_CHANNEL_WEBDAV ] = "webdav",
> +};

Are we guaranteed that this array is never sparse?

> +channel_name = "unknown";
> +if (chan->value->channel_type > 0 &&
> +chan->value->channel_type < ARRAY_SIZE(channel_names)) {
> +channel_name = channel_names[chan->value->channel_type];

If it could be sparse, you might need an additional '&&
channel_names[chan->value->channel_type]' in the conditional.  Otherwise,

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 6/6 v4] linux-user/syscall.c: Switch all macros which are not defined in tilegx

[Andreas Färber]

Am 22.02.2015 um 14:37 schrieb Chen Gang S:
> For tilegx, several syscall macros are not supported, so switch them to
> avoid building break.
> 
> Signed-off-by: Chen Gang 
> ---
>  linux-user/syscall.c | 50 +-
>  1 file changed, 49 insertions(+), 1 deletion(-)

LGTM,

Reviewed-by: Andreas Färber 

However I suggest "conditionalize syscalls" or "disable syscalls" over
"switch ... macros/them".

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] [PULL 68/69] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c

[Stefan Hajnoczi]

From: Marc Marí 

The MSIX interrupt was always acked without checking its value, which caused a
race condition. If the ISR was raised between the read and the acking, the ISR
was never detected and it timed out.

Signed-off-by: Marc Marí 
Reviewed-by: John Snow 
Tested-by: John Snow 
Message-id: 1424795655-16952-1-git-send-email-marc.mari.barc...@gmail.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/virtio-pci.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 046a316..f9fb924 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -142,8 +142,12 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice 
*d, QVirtQueue *vq)
 return qpci_msix_pending(dev->pdev, vqpci->msix_entry);
 } else {
 data = readl(vqpci->msix_addr);
-writel(vqpci->msix_addr, 0);
-return data == vqpci->msix_data;
+if (data == vqpci->msix_data) {
+writel(vqpci->msix_addr, 0);
+return true;
+} else {
+return false;
+}
 }
 } else {
 return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_ISR_STATUS) & 
1;
@@ -162,8 +166,12 @@ static bool 
qvirtio_pci_get_config_isr_status(QVirtioDevice *d)
 return qpci_msix_pending(dev->pdev, dev->config_msix_entry);
 } else {
 data = readl(dev->config_msix_addr);
-writel(dev->config_msix_addr, 0);
-return data == dev->config_msix_data;
+if (data == dev->config_msix_data) {
+writel(dev->config_msix_addr, 0);
+return true;
+} else {
+return false;
+}
 }
 } else {
 return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_ISR_STATUS) & 
2;
-- 
2.1.0

[Qemu-devel] [PATCH 0/1]

[Ekaterina Tumanova]

for Max Reitz:

Can you please apply this patch and re-test?

Thanks!
Kate

p.s. This is supposed to be merged with patch 5/5 of
"Geometry and blocksize detection for backing devices"

Ekaterina Tumanova (1):
  scsi-hd: fix property unset case

 hw/scsi/scsi-disk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.1.4

[Qemu-devel] [PULL 65/69] libqos: add blkdebug_prepare_script

[Stefan Hajnoczi]

From: John Snow 

Pull this helper out of ide-test and into libqos,
to be shared with ahci-test.

Signed-off-by: John Snow 
Message-id: 1424738729-17082-6-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ide-test.c  | 23 +--
 tests/libqos/libqos.c | 22 ++
 tests/libqos/libqos.h |  1 +
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 1dae84f..78382e9 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -29,6 +29,7 @@
 #include 
 
 #include "libqtest.h"
+#include "libqos/libqos.h"
 #include "libqos/pci-pc.h"
 #include "libqos/malloc-pc.h"
 
@@ -494,28 +495,6 @@ static void test_flush(void)
 ide_test_quit();
 }
 
-static void prepare_blkdebug_script(const char *debug_fn, const char *event)
-{
-FILE *debug_file = fopen(debug_fn, "w");
-int ret;
-
-fprintf(debug_file, "[inject-error]\n");
-fprintf(debug_file, "event = \"%s\"\n", event);
-fprintf(debug_file, "errno = \"5\"\n");
-fprintf(debug_file, "state = \"1\"\n");
-fprintf(debug_file, "immediately = \"off\"\n");
-fprintf(debug_file, "once = \"on\"\n");
-
-fprintf(debug_file, "[set-state]\n");
-fprintf(debug_file, "event = \"%s\"\n", event);
-fprintf(debug_file, "new_state = \"2\"\n");
-fflush(debug_file);
-g_assert(!ferror(debug_file));
-
-ret = fclose(debug_file);
-g_assert(ret == 0);
-}
-
 static void test_retry_flush(const char *machine)
 {
 uint8_t data;
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index c825486..4bb047c 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -98,3 +98,25 @@ void mkqcow2(const char *file, unsigned size_mb)
 {
 return mkimg(file, "qcow2", size_mb);
 }
+
+void prepare_blkdebug_script(const char *debug_fn, const char *event)
+{
+FILE *debug_file = fopen(debug_fn, "w");
+int ret;
+
+fprintf(debug_file, "[inject-error]\n");
+fprintf(debug_file, "event = \"%s\"\n", event);
+fprintf(debug_file, "errno = \"5\"\n");
+fprintf(debug_file, "state = \"1\"\n");
+fprintf(debug_file, "immediately = \"off\"\n");
+fprintf(debug_file, "once = \"on\"\n");
+
+fprintf(debug_file, "[set-state]\n");
+fprintf(debug_file, "event = \"%s\"\n", event);
+fprintf(debug_file, "new_state = \"2\"\n");
+fflush(debug_file);
+g_assert(!ferror(debug_file));
+
+ret = fclose(debug_file);
+g_assert(ret == 0);
+}
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index 8cb2987..f57362b 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -21,6 +21,7 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, 
...);
 void qtest_shutdown(QOSState *qs);
 void mkimg(const char *file, const char *fmt, unsigned size_mb);
 void mkqcow2(const char *file, unsigned size_mb);
+void prepare_blkdebug_script(const char *debug_fn, const char *event);
 
 static inline uint64_t qmalloc(QOSState *q, size_t bytes)
 {
-- 
2.1.0

[Qemu-devel] [PULL 69/69] tests: Check QVIRTIO_F_ANY_LAYOUT flag in virtio-blk test

[Stefan Hajnoczi]

From: Marc Marí 

Check the QVIRTIO_F_ANY_LAYOUT flag before performing operations with 2
descriptor layout. This is to follow the specification strictly.

This patch depends on:
[PATCH v5 0/5] libqos: Virtio MMIO driver

Signed-off-by: Marc Marí 
Message-id: 1424815154-27243-1-git-send-email-marc.mari.barc...@gmail.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/virtio-blk-test.c | 118 +---
 1 file changed, 62 insertions(+), 56 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 3b943fc..4078321 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -191,59 +191,11 @@ static void test_basic(const QVirtioBus *bus, 
QVirtioDevice *dev,
 
 qvirtio_set_driver_ok(bus, dev);
 
-/* Write and read with 2 descriptor layout */
-/* Write request */
-req.type = QVIRTIO_BLK_T_OUT;
-req.ioprio = 1;
-req.sector = 0;
-req.data = g_malloc0(512);
-strcpy(req.data, "TEST");
-
-req_addr = virtio_blk_request(alloc, &req, 512);
-
-g_free(req.data);
-
-free_head = qvirtqueue_add(vq, req_addr, 528, false, true);
-qvirtqueue_add(vq, req_addr + 528, 1, true, false);
-qvirtqueue_kick(bus, dev, vq, free_head);
-
-qvirtio_wait_queue_isr(bus, dev, vq, QVIRTIO_BLK_TIMEOUT_US);
-status = readb(req_addr + 528);
-g_assert_cmpint(status, ==, 0);
-
-guest_free(alloc, req_addr);
-
-/* Read request */
-req.type = QVIRTIO_BLK_T_IN;
-req.ioprio = 1;
-req.sector = 0;
-req.data = g_malloc0(512);
-
-req_addr = virtio_blk_request(alloc, &req, 512);
-
-g_free(req.data);
-
-free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
-qvirtqueue_add(vq, req_addr + 16, 513, true, false);
-
-qvirtqueue_kick(bus, dev, vq, free_head);
-
-qvirtio_wait_queue_isr(bus, dev, vq, QVIRTIO_BLK_TIMEOUT_US);
-status = readb(req_addr + 528);
-g_assert_cmpint(status, ==, 0);
-
-data = g_malloc0(512);
-memread(req_addr + 16, data, 512);
-g_assert_cmpstr(data, ==, "TEST");
-g_free(data);
-
-guest_free(alloc, req_addr);
-
 /* Write and read with 3 descriptor layout */
 /* Write request */
 req.type = QVIRTIO_BLK_T_OUT;
 req.ioprio = 1;
-req.sector = 1;
+req.sector = 0;
 req.data = g_malloc0(512);
 strcpy(req.data, "TEST");
 
@@ -266,7 +218,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice 
*dev,
 /* Read request */
 req.type = QVIRTIO_BLK_T_IN;
 req.ioprio = 1;
-req.sector = 1;
+req.sector = 0;
 req.data = g_malloc0(512);
 
 req_addr = virtio_blk_request(alloc, &req, 512);
@@ -289,6 +241,56 @@ static void test_basic(const QVirtioBus *bus, 
QVirtioDevice *dev,
 g_free(data);
 
 guest_free(alloc, req_addr);
+
+if (features & QVIRTIO_F_ANY_LAYOUT) {
+/* Write and read with 2 descriptor layout */
+/* Write request */
+req.type = QVIRTIO_BLK_T_OUT;
+req.ioprio = 1;
+req.sector = 1;
+req.data = g_malloc0(512);
+strcpy(req.data, "TEST");
+
+req_addr = virtio_blk_request(alloc, &req, 512);
+
+g_free(req.data);
+
+free_head = qvirtqueue_add(vq, req_addr, 528, false, true);
+qvirtqueue_add(vq, req_addr + 528, 1, true, false);
+qvirtqueue_kick(bus, dev, vq, free_head);
+
+qvirtio_wait_queue_isr(bus, dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+status = readb(req_addr + 528);
+g_assert_cmpint(status, ==, 0);
+
+guest_free(alloc, req_addr);
+
+/* Read request */
+req.type = QVIRTIO_BLK_T_IN;
+req.ioprio = 1;
+req.sector = 1;
+req.data = g_malloc0(512);
+
+req_addr = virtio_blk_request(alloc, &req, 512);
+
+g_free(req.data);
+
+free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
+qvirtqueue_add(vq, req_addr + 16, 513, true, false);
+
+qvirtqueue_kick(bus, dev, vq, free_head);
+
+qvirtio_wait_queue_isr(bus, dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+status = readb(req_addr + 528);
+g_assert_cmpint(status, ==, 0);
+
+data = g_malloc0(512);
+memread(req_addr + 16, data, 512);
+g_assert_cmpstr(data, ==, "TEST");
+g_free(data);
+
+guest_free(alloc, req_addr);
+}
 }
 
 static void pci_basic(void)
@@ -521,7 +523,8 @@ static void pci_msix(void)
 
 g_free(req.data);
 
-free_head = qvirtqueue_add(&vqpci->vq, req_addr, 528, false, true);
+free_head = qvirtqueue_add(&vqpci->vq, req_addr, 16, false, true);
+qvirtqueue_add(&vqpci->vq, req_addr + 16, 512, false, true);
 qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
 qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);
 
@@ -544,7 +547,8 @@ static void pci_msix(void)
 g_free(req.data);
 
 free_head = qvirtqueue_add(&vqpci->vq, req_addr, 16, false, true);
-qvirtqueue_add(&vqpci->vq, req_addr + 16, 513, true, false);
+  

[Qemu-devel] [PULL 67/69] sheepdog: fix confused return values

[Stefan Hajnoczi]

From: Liu Yuan 

These functions mix up -1 and -errno in return values and would might cause
trouble error handling in the call chain.

This patch let them return -errno and add some comments.

Cc: qemu-devel@nongnu.org
Cc: Markus Armbruster 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Reported-by: Markus Armbruster 
Signed-off-by: Liu Yuan 
Message-id: 1424231875-7131-1-git-send-email-namei.u...@gmail.com
Signed-off-by: Stefan Hajnoczi 
---
 block/sheepdog.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index a2679c2..60a4853 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -545,6 +545,7 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, 
QEMUIOVector *qiov,
 return acb;
 }
 
+/* Return -EIO in case of error, file descriptor on success */
 static int connect_to_sdog(BDRVSheepdogState *s, Error **errp)
 {
 int fd;
@@ -564,11 +565,14 @@ static int connect_to_sdog(BDRVSheepdogState *s, Error 
**errp)
 
 if (fd >= 0) {
 qemu_set_nonblock(fd);
+} else {
+fd = -EIO;
 }
 
 return fd;
 }
 
+/* Return 0 on success and -errno in case of error */
 static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
 unsigned int *wlen)
 {
@@ -577,11 +581,13 @@ static coroutine_fn int send_co_req(int sockfd, 
SheepdogReq *hdr, void *data,
 ret = qemu_co_send(sockfd, hdr, sizeof(*hdr));
 if (ret != sizeof(*hdr)) {
 error_report("failed to send a req, %s", strerror(errno));
+ret = -socket_error();
 return ret;
 }
 
 ret = qemu_co_send(sockfd, data, *wlen);
 if (ret != *wlen) {
+ret = -socket_error();
 error_report("failed to send a req, %s", strerror(errno));
 }
 
@@ -656,6 +662,11 @@ out:
 srco->finished = true;
 }
 
+/*
+ * Send the request to the sheep in a synchronous manner.
+ *
+ * Return 0 on success, -errno in case of error.
+ */
 static int do_req(int sockfd, AioContext *aio_context, SheepdogReq *hdr,
   void *data, unsigned int *wlen, unsigned int *rlen)
 {
-- 
2.1.0

[Qemu-devel] [PULL 64/69] libqtest: add qmp_async

[Stefan Hajnoczi]

From: John Snow 

Add qmp_async, which lets us send QMP commands asynchronously.
This is useful when we want to send commands that will trigger
event responses, but we don't know in what order to expect them.

Sometimes the event responses may arrive even before the command
confirmation will show up, so it is convenient to leave the responses
in the stream.

Signed-off-by: John Snow 
Message-id: 1424738729-17082-5-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqtest.c | 30 +-
 tests/libqtest.h | 27 +++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index a3ee8ae..70a172e 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -388,7 +388,12 @@ QDict *qtest_qmp_receive(QTestState *s)
 return qmp.response;
 }
 
-QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
+/**
+ * Allow users to send a message without waiting for the reply,
+ * in the case that they choose to discard all replies up until
+ * a particular EVENT is received.
+ */
+void qtest_async_qmpv(QTestState *s, const char *fmt, va_list ap)
 {
 va_list ap_copy;
 QObject *qobj;
@@ -417,6 +422,11 @@ QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list 
ap)
 QDECREF(qstr);
 qobject_decref(qobj);
 }
+}
+
+QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
+{
+qtest_async_qmpv(s, fmt, ap);
 
 /* Receive reply */
 return qtest_qmp_receive(s);
@@ -433,6 +443,15 @@ QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
 return response;
 }
 
+void qtest_async_qmp(QTestState *s, const char *fmt, ...)
+{
+va_list ap;
+
+va_start(ap, fmt);
+qtest_async_qmpv(s, fmt, ap);
+va_end(ap);
+}
+
 void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap)
 {
 QDict *response = qtest_qmpv(s, fmt, ap);
@@ -704,6 +723,15 @@ QDict *qmp(const char *fmt, ...)
 return response;
 }
 
+void qmp_async(const char *fmt, ...)
+{
+va_list ap;
+
+va_start(ap, fmt);
+qtest_async_qmpv(global_qtest, fmt, ap);
+va_end(ap);
+}
+
 void qmp_discard_response(const char *fmt, ...)
 {
 va_list ap;
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 2aa2e6f..cc57124 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -64,6 +64,15 @@ void qtest_qmp_discard_response(QTestState *s, const char 
*fmt, ...);
 QDict *qtest_qmp(QTestState *s, const char *fmt, ...);
 
 /**
+ * qtest_async_qmp:
+ * @s: #QTestState instance to operate on.
+ * @fmt...: QMP message to send to qemu
+ *
+ * Sends a QMP message to QEMU and leaves the response in the stream.
+ */
+void qtest_async_qmp(QTestState *s, const char *fmt, ...);
+
+/**
  * qtest_qmpv_discard_response:
  * @s: #QTestState instance to operate on.
  * @fmt: QMP message to send to QEMU
@@ -84,6 +93,16 @@ void qtest_qmpv_discard_response(QTestState *s, const char 
*fmt, va_list ap);
 QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap);
 
 /**
+ * qtest_async_qmpv:
+ * @s: #QTestState instance to operate on.
+ * @fmt: QMP message to send to QEMU
+ * @ap: QMP message arguments
+ *
+ * Sends a QMP message to QEMU and leaves the response in the stream.
+ */
+void qtest_async_qmpv(QTestState *s, const char *fmt, va_list ap);
+
+/**
  * qtest_receive:
  * @s: #QTestState instance to operate on.
  *
@@ -388,6 +407,14 @@ static inline void qtest_end(void)
 QDict *qmp(const char *fmt, ...);
 
 /**
+ * qmp_async:
+ * @fmt...: QMP message to send to qemu
+ *
+ * Sends a QMP message to QEMU and leaves the response in the stream.
+ */
+void qmp_async(const char *fmt, ...);
+
+/**
  * qmp_discard_response:
  * @fmt...: QMP message to send to qemu
  *
-- 
2.1.0

[Qemu-devel] [PULL 59/69] qtest/ahci: add qcow2 support to ahci-test

[Stefan Hajnoczi]

From: John Snow 

This will enable the testing of high offsets without
wasting a lot of disk space, and does not impact the
previous tests.

mkimg and mkqcow2 are added to libqos for other tests.

Signed-off-by: John Snow 
Message-id: 1424905602-24715-8-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/Makefile|  1 +
 tests/ahci-test.c | 16 ++--
 tests/libqos/libqos.c | 37 +
 tests/libqos/libqos.h |  2 ++
 4 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index fed8096..c525a1e 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -415,6 +415,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
 $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
+   QTEST_QEMU_IMG=qemu-img$(EXESUF) \
MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
gtester $(GTESTER_OPTIONS) -m=$(SPEED) 
$(check-qtest-$*-y),"GTESTER $@")
$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index cf0b98b..3f93c15 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -39,8 +39,8 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/pci_regs.h"
 
-/* Test-specific defines. */
-#define TEST_IMAGE_SIZE(64 * 1024 * 1024)
+/* Test-specific defines -- in MiB */
+#define TEST_IMAGE_SIZE_MB (200 * 1024)
 
 /*** Globals ***/
 static char tmp_path[] = "/tmp/qtest.XX";
@@ -81,7 +81,7 @@ static AHCIQState *ahci_boot(void)
 s = g_malloc0(sizeof(AHCIQState));
 
 cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
-",format=raw"
+",format=qcow2"
 " -M q35 "
 "-device ide-hd,drive=drive0 "
 "-global ide-hd.ver=%s";
@@ -1051,7 +1051,6 @@ static void create_ahci_io_test(enum IOMode type, enum 
AddrMode addr,
 int main(int argc, char **argv)
 {
 const char *arch;
-int fd;
 int ret;
 int c;
 int i, j, k;
@@ -1088,12 +1087,9 @@ int main(int argc, char **argv)
 return 0;
 }
 
-/* Create a temporary raw image */
-fd = mkstemp(tmp_path);
-g_assert(fd >= 0);
-ret = ftruncate(fd, TEST_IMAGE_SIZE);
-g_assert(ret == 0);
-close(fd);
+/* Create a temporary qcow2 image */
+close(mkstemp(tmp_path));
+mkqcow2(tmp_path, TEST_IMAGE_SIZE_MB);
 
 /* Run the tests */
 qtest_add_func("/ahci/sanity", test_sanity);
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index bc8beb2..c825486 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -61,3 +61,40 @@ void qtest_shutdown(QOSState *qs)
 qtest_quit(qs->qts);
 g_free(qs);
 }
+
+void mkimg(const char *file, const char *fmt, unsigned size_mb)
+{
+gchar *cli;
+bool ret;
+int rc;
+GError *err = NULL;
+char *qemu_img_path;
+gchar *out, *out2;
+
+qemu_img_path = getenv("QTEST_QEMU_IMG");
+assert(qemu_img_path);
+
+cli = g_strdup_printf("./%s create -f %s %s %uM", qemu_img_path,
+  fmt, file, size_mb);
+ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
+if (err) {
+fprintf(stderr, "%s\n", err->message);
+g_error_free(err);
+}
+g_assert(ret && !err);
+
+ret = g_spawn_check_exit_status(rc, &err);
+if (err) {
+fprintf(stderr, "%s\n", err->message);
+}
+g_assert(ret && !err);
+
+g_free(out);
+g_free(out2);
+g_free(cli);
+}
+
+void mkqcow2(const char *file, unsigned size_mb)
+{
+return mkimg(file, "qcow2", size_mb);
+}
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index 612d41e..8cb2987 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -19,6 +19,8 @@ typedef struct QOSState {
 QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap);
 QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...);
 void qtest_shutdown(QOSState *qs);
+void mkimg(const char *file, const char *fmt, unsigned size_mb);
+void mkqcow2(const char *file, unsigned size_mb);
 
 static inline uint64_t qmalloc(QOSState *q, size_t bytes)
 {
-- 
2.1.0

[Qemu-devel] [PULL 55/69] libqos/ahci: add ahci command helpers

[Stefan Hajnoczi]

From: John Snow 

ahci_command_set_flags:  Set additional flags in the command header.
ahci_command_clr_flags:  Clear flags from the command header.
ahci_command_set_offset: Change the IO sector from 0.
ahci_command_adjust: Adjust many values simultaneously.

To be used to adjust the command header if the default values/guesses
were incorrect or undesirable.

Signed-off-by: John Snow 
Message-id: 1424905602-24715-4-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/ahci.c | 42 ++
 tests/libqos/ahci.h |  5 +
 2 files changed, 47 insertions(+)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 9dc505c..14651ff 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -713,6 +713,40 @@ void ahci_command_free(AHCICommand *cmd)
 g_free(cmd);
 }
 
+void ahci_command_set_flags(AHCICommand *cmd, uint16_t cmdh_flags)
+{
+cmd->header.flags |= cmdh_flags;
+}
+
+void ahci_command_clr_flags(AHCICommand *cmd, uint16_t cmdh_flags)
+{
+cmd->header.flags &= ~cmdh_flags;
+}
+
+void ahci_command_set_offset(AHCICommand *cmd, uint64_t lba_sect)
+{
+RegH2DFIS *fis = &(cmd->fis);
+if (cmd->props->lba28) {
+g_assert_cmphex(lba_sect, <=, 0xFFF);
+} else if (cmd->props->lba48) {
+g_assert_cmphex(lba_sect, <=, 0x);
+} else {
+/* Can't set offset if we don't know the format. */
+g_assert_not_reached();
+}
+
+/* LBA28 uses the low nibble of the device/control register for LBA24:27 */
+fis->lba_lo[0] = (lba_sect & 0xFF);
+fis->lba_lo[1] = (lba_sect >> 8) & 0xFF;
+fis->lba_lo[2] = (lba_sect >> 16) & 0xFF;
+if (cmd->props->lba28) {
+fis->device = (fis->device & 0xF0) || (lba_sect >> 24) & 0x0F;
+}
+fis->lba_hi[0] = (lba_sect >> 24) & 0xFF;
+fis->lba_hi[1] = (lba_sect >> 32) & 0xFF;
+fis->lba_hi[2] = (lba_sect >> 40) & 0xFF;
+}
+
 void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer)
 {
 cmd->buffer = buffer;
@@ -740,6 +774,14 @@ void ahci_command_set_prd_size(AHCICommand *cmd, unsigned 
prd_size)
 ahci_command_set_sizes(cmd, cmd->xbytes, prd_size);
 }
 
+void ahci_command_adjust(AHCICommand *cmd, uint64_t offset, uint64_t buffer,
+ size_t xbytes, unsigned prd_size)
+{
+ahci_command_set_sizes(cmd, xbytes, prd_size);
+ahci_command_set_buffer(cmd, buffer);
+ahci_command_set_offset(cmd, offset);
+}
+
 void ahci_command_commit(AHCIQState *ahci, AHCICommand *cmd, uint8_t port)
 {
 uint16_t i, prdtl;
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 39b99d3..888545d 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -537,11 +537,16 @@ void ahci_command_verify(AHCIQState *ahci, AHCICommand 
*cmd);
 void ahci_command_free(AHCICommand *cmd);
 
 /* Command adjustments */
+void ahci_command_set_flags(AHCICommand *cmd, uint16_t cmdh_flags);
+void ahci_command_clr_flags(AHCICommand *cmd, uint16_t cmdh_flags);
+void ahci_command_set_offset(AHCICommand *cmd, uint64_t lba_sect);
 void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer);
 void ahci_command_set_size(AHCICommand *cmd, uint64_t xbytes);
 void ahci_command_set_prd_size(AHCICommand *cmd, unsigned prd_size);
 void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes,
 unsigned prd_size);
+void ahci_command_adjust(AHCICommand *cmd, uint64_t lba_sect, uint64_t gbuffer,
+ uint64_t xbytes, unsigned prd_size);
 
 /* Command Misc */
 uint8_t ahci_command_slot(AHCICommand *cmd);
-- 
2.1.0

[Qemu-devel] [PULL 53/69] libqos/ahci: Zero-fill AHCI headers

[Stefan Hajnoczi]

From: John Snow 

Even though it's just the reserved space, make sure they're zeroes.

Signed-off-by: John Snow 
Message-id: 1424905602-24715-2-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/ahci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index a6105c7..9dc505c 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -487,7 +487,7 @@ void ahci_get_command_header(AHCIQState *ahci, uint8_t port,
 void ahci_set_command_header(AHCIQState *ahci, uint8_t port,
  uint8_t slot, AHCICommandHeader *cmd)
 {
-AHCICommandHeader tmp;
+AHCICommandHeader tmp = { .flags = 0 };
 uint64_t ba = ahci->port[port].clb;
 ba += slot * sizeof(AHCICommandHeader);
 
-- 
2.1.0

[Qemu-devel] [PULL 58/69] qtest/ahci: add fragmented dma test

[Stefan Hajnoczi]

From: John Snow 

Test what happens when we try to use extremely short PRDTs
to accomplish a small data transfer.

Signed-off-by: John Snow 
Message-id: 1424905602-24715-7-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ahci-test.c | 59 +++
 1 file changed, 59 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 21f20f7..cf0b98b 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -851,6 +851,63 @@ static void test_identify(void)
 ahci_shutdown(ahci);
 }
 
+/**
+ * Fragmented DMA test: Perform a standard 4K DMA read/write
+ * test, but make sure the physical regions are fragmented to
+ * be very small, each just 32 bytes, to see how AHCI performs
+ * with chunks defined to be much less than a sector.
+ */
+static void test_dma_fragmented(void)
+{
+AHCIQState *ahci;
+AHCICommand *cmd;
+uint8_t px;
+size_t bufsize = 4096;
+unsigned char *tx = g_malloc(bufsize);
+unsigned char *rx = g_malloc0(bufsize);
+unsigned i;
+uint64_t ptr;
+
+ahci = ahci_boot_and_enable();
+px = ahci_port_select(ahci);
+ahci_port_clear(ahci, px);
+
+/* create pattern */
+for (i = 0; i < bufsize; i++) {
+tx[i] = (bufsize - i);
+}
+
+/* Create a DMA buffer in guest memory, and write our pattern to it. */
+ptr = guest_alloc(ahci->parent->alloc, bufsize);
+g_assert(ptr);
+memwrite(ptr, tx, bufsize);
+
+cmd = ahci_command_create(CMD_WRITE_DMA);
+ahci_command_adjust(cmd, 0, ptr, bufsize, 32);
+ahci_command_commit(ahci, cmd, px);
+ahci_command_issue(ahci, cmd);
+ahci_command_verify(ahci, cmd);
+g_free(cmd);
+
+cmd = ahci_command_create(CMD_READ_DMA);
+ahci_command_adjust(cmd, 0, ptr, bufsize, 32);
+ahci_command_commit(ahci, cmd, px);
+ahci_command_issue(ahci, cmd);
+ahci_command_verify(ahci, cmd);
+g_free(cmd);
+
+/* Read back the guest's receive buffer into local memory */
+memread(ptr, rx, bufsize);
+guest_free(ahci->parent->alloc, ptr);
+
+g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0);
+
+ahci_shutdown(ahci);
+
+g_free(rx);
+g_free(tx);
+}
+
 
/**/
 /* AHCI I/O Test Matrix Definitions   
*/
 
@@ -1054,6 +,8 @@ int main(int argc, char **argv)
 }
 }
 
+qtest_add_func("/ahci/io/dma/lba28/fragmented", test_dma_fragmented);
+
 ret = g_test_run();
 
 /* Cleanup */
-- 
2.1.0

[Qemu-devel] [PULL 60/69] qtest/ahci: test different disk sectors

[Stefan Hajnoczi]

From: John Snow 

Test sector offset 0, 1, and the last sector(s)
in LBA28 and LBA48 modes.

Signed-off-by: John Snow 
Message-id: 1424905602-24715-9-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ahci-test.c   | 68 +++--
 tests/libqos/ahci.c | 10 
 tests/libqos/ahci.h |  4 ++--
 3 files changed, 63 insertions(+), 19 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 3f93c15..fb4739f 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -41,6 +41,8 @@
 
 /* Test-specific defines -- in MiB */
 #define TEST_IMAGE_SIZE_MB (200 * 1024)
+#define TEST_IMAGE_SECTORS ((TEST_IMAGE_SIZE_MB / AHCI_SECTOR_SIZE) \
+* 1024 * 1024)
 
 /*** Globals ***/
 static char tmp_path[] = "/tmp/qtest.XX";
@@ -712,7 +714,7 @@ static void ahci_test_identify(AHCIQState *ahci)
 ahci_port_clear(ahci, px);
 
 /* "Read" 512 bytes using CMD_IDENTIFY into the host buffer. */
-ahci_io(ahci, px, CMD_IDENTIFY, &buff, buffsize);
+ahci_io(ahci, px, CMD_IDENTIFY, &buff, buffsize, 0);
 
 /* Check serial number/version in the buffer */
 /* NB: IDENTIFY strings are packed in 16bit little endian chunks.
@@ -728,11 +730,12 @@ static void ahci_test_identify(AHCIQState *ahci)
 g_assert_cmphex(rc, ==, 0);
 
 sect_size = le16_to_cpu(*((uint16_t *)(&buff[5])));
-g_assert_cmphex(sect_size, ==, 0x200);
+g_assert_cmphex(sect_size, ==, AHCI_SECTOR_SIZE);
 }
 
 static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize,
-   uint8_t read_cmd, uint8_t write_cmd)
+   uint64_t sector, uint8_t read_cmd,
+   uint8_t write_cmd)
 {
 uint64_t ptr;
 uint8_t port;
@@ -758,9 +761,9 @@ static void ahci_test_io_rw_simple(AHCIQState *ahci, 
unsigned bufsize,
 memwrite(ptr, tx, bufsize);
 
 /* Write this buffer to disk, then read it back to the DMA buffer. */
-ahci_guest_io(ahci, port, write_cmd, ptr, bufsize);
+ahci_guest_io(ahci, port, write_cmd, ptr, bufsize, sector);
 qmemset(ptr, 0x00, bufsize);
-ahci_guest_io(ahci, port, read_cmd, ptr, bufsize);
+ahci_guest_io(ahci, port, read_cmd, ptr, bufsize, sector);
 
 /*** Read back the Data ***/
 memread(ptr, rx, bufsize);
@@ -948,12 +951,45 @@ enum IOOps {
 NUM_IO_OPS
 };
 
+enum OffsetType {
+OFFSET_BEGIN = 0,
+OFFSET_ZERO = OFFSET_BEGIN,
+OFFSET_LOW,
+OFFSET_HIGH,
+NUM_OFFSETS
+};
+
+static const char *offset_str[NUM_OFFSETS] = { "zero", "low", "high" };
+
 typedef struct AHCIIOTestOptions {
 enum BuffLen length;
 enum AddrMode address_type;
 enum IOMode io_type;
+enum OffsetType offset;
 } AHCIIOTestOptions;
 
+static uint64_t offset_sector(enum OffsetType ofst,
+  enum AddrMode addr_type,
+  uint64_t buffsize)
+{
+uint64_t ceil;
+uint64_t nsectors;
+
+switch (ofst) {
+case OFFSET_ZERO:
+return 0;
+case OFFSET_LOW:
+return 1;
+case OFFSET_HIGH:
+ceil = (addr_type == ADDR_MODE_LBA28) ? 0xfff : 0x;
+ceil = MIN(ceil, TEST_IMAGE_SECTORS - 1);
+nsectors = buffsize / AHCI_SECTOR_SIZE;
+return ceil - nsectors + 1;
+default:
+g_assert_not_reached();
+}
+}
+
 /**
  * Table of possible I/O ATA commands given a set of enumerations.
  */
@@ -981,12 +1017,12 @@ static const uint8_t 
io_cmds[NUM_MODES][NUM_ADDR_MODES][NUM_IO_OPS] = {
  * transfer modes, and buffer sizes.
  */
 static void test_io_rw_interface(enum AddrMode lba48, enum IOMode dma,
- unsigned bufsize)
+ unsigned bufsize, uint64_t sector)
 {
 AHCIQState *ahci;
 
 ahci = ahci_boot_and_enable();
-ahci_test_io_rw_simple(ahci, bufsize,
+ahci_test_io_rw_simple(ahci, bufsize, sector,
io_cmds[dma][lba48][IO_READ],
io_cmds[dma][lba48][IO_WRITE]);
 ahci_shutdown(ahci);
@@ -999,6 +1035,7 @@ static void test_io_interface(gconstpointer opaque)
 {
 AHCIIOTestOptions *opts = (AHCIIOTestOptions *)opaque;
 unsigned bufsize;
+uint64_t sector;
 
 switch (opts->length) {
 case LEN_SIMPLE:
@@ -1017,13 +1054,14 @@ static void test_io_interface(gconstpointer opaque)
 g_assert_not_reached();
 }
 
-test_io_rw_interface(opts->address_type, opts->io_type, bufsize);
+sector = offset_sector(opts->offset, opts->address_type, bufsize);
+test_io_rw_interface(opts->address_type, opts->io_type, bufsize, sector);
 g_free(opts);
 return;
 }
 
 static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
-enum BuffLen len)
+enum BuffLen len, enum OffsetType offset)
 {
 static const char *arch;
 char *name;
@@ -1032,15 +

[Qemu-devel] [PULL 54/69] qtest/ahci: Add a macro bootup routine

[Stefan Hajnoczi]

From: John Snow 

Add a routine that can be used to engage the AHCI
device at a not-granular level so that bringing up
the functionality of the HBA is easy in future tests
that are not concerned with testing the bring-up process.

Signed-off-by: John Snow 
Message-id: 1424905602-24715-3-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ahci-test.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 53fd068..9fe9fb5 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -107,6 +107,21 @@ static void ahci_shutdown(AHCIQState *ahci)
 qtest_shutdown(qs);
 }
 
+/**
+ * Boot and fully enable the HBA device.
+ * @see ahci_boot, ahci_pci_enable and ahci_hba_enable.
+ */
+static AHCIQState *ahci_boot_and_enable(void)
+{
+AHCIQState *ahci;
+ahci = ahci_boot();
+
+ahci_pci_enable(ahci);
+ahci_hba_enable(ahci);
+
+return ahci;
+}
+
 /*** Specification Adherence Tests ***/
 
 /**
@@ -831,9 +846,7 @@ static void test_identify(void)
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot();
-ahci_pci_enable(ahci);
-ahci_hba_enable(ahci);
+ahci = ahci_boot_and_enable();
 ahci_test_identify(ahci);
 ahci_shutdown(ahci);
 }
@@ -845,9 +858,7 @@ static void test_dma_rw_simple(void)
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot();
-ahci_pci_enable(ahci);
-ahci_hba_enable(ahci);
+ahci = ahci_boot_and_enable();
 ahci_test_dma_rw_simple(ahci);
 ahci_shutdown(ahci);
 }
-- 
2.1.0

[Qemu-devel] [PULL 62/69] qtest/ahci: Allow override of default CLI options

[Stefan Hajnoczi]

From: John Snow 

Signed-off-by: John Snow 
Message-id: 1424738729-17082-3-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ahci-test.c| 67 
 tests/libqos/libqos-pc.c |  5 
 tests/libqos/libqos-pc.h |  1 +
 3 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index b344121..6c99f19 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -75,19 +75,12 @@ static void string_bswap16(uint16_t *s, size_t bytes)
 /**
  * Start a Q35 machine and bookmark a handle to the AHCI device.
  */
-static AHCIQState *ahci_boot(void)
+static AHCIQState *ahci_vboot(const char *cli, va_list ap)
 {
 AHCIQState *s;
-const char *cli;
 
 s = g_malloc0(sizeof(AHCIQState));
-
-cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
-",format=qcow2"
-" -M q35 "
-"-device ide-hd,drive=drive0 "
-"-global ide-hd.ver=%s";
-s->parent = qtest_pc_boot(cli, tmp_path, "testdisk", "version");
+s->parent = qtest_pc_vboot(cli, ap);
 alloc_set_flags(s->parent->alloc, ALLOC_LEAK_ASSERT);
 
 /* Verify that we have an AHCI device present. */
@@ -97,12 +90,35 @@ static AHCIQState *ahci_boot(void)
 }
 
 /**
+ * Start a Q35 machine and bookmark a handle to the AHCI device.
+ */
+static AHCIQState *ahci_boot(const char *cli, ...)
+{
+AHCIQState *s;
+va_list ap;
+
+if (cli) {
+va_start(ap, cli);
+s = ahci_vboot(cli, ap);
+va_end(ap);
+} else {
+cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
+",format=qcow2"
+" -M q35 "
+"-device ide-hd,drive=drive0 "
+"-global ide-hd.ver=%s";
+s = ahci_boot(cli, tmp_path, "testdisk", "version");
+}
+
+return s;
+}
+
+/**
  * Clean up the PCI device, then terminate the QEMU instance.
  */
 static void ahci_shutdown(AHCIQState *ahci)
 {
 QOSState *qs = ahci->parent;
-
 ahci_clean_mem(ahci);
 free_ahci_device(ahci->dev);
 g_free(ahci);
@@ -113,10 +129,18 @@ static void ahci_shutdown(AHCIQState *ahci)
  * Boot and fully enable the HBA device.
  * @see ahci_boot, ahci_pci_enable and ahci_hba_enable.
  */
-static AHCIQState *ahci_boot_and_enable(void)
+static AHCIQState *ahci_boot_and_enable(const char *cli, ...)
 {
 AHCIQState *ahci;
-ahci = ahci_boot();
+va_list ap;
+
+if (cli) {
+va_start(ap, cli);
+ahci = ahci_vboot(cli, ap);
+va_end(ap);
+} else {
+ahci = ahci_boot(NULL);
+}
 
 ahci_pci_enable(ahci);
 ahci_hba_enable(ahci);
@@ -807,7 +831,7 @@ static void ahci_test_flush(AHCIQState *ahci)
 static void test_sanity(void)
 {
 AHCIQState *ahci;
-ahci = ahci_boot();
+ahci = ahci_boot(NULL);
 ahci_shutdown(ahci);
 }
 
@@ -818,7 +842,7 @@ static void test_sanity(void)
 static void test_pci_spec(void)
 {
 AHCIQState *ahci;
-ahci = ahci_boot();
+ahci = ahci_boot(NULL);
 ahci_test_pci_spec(ahci);
 ahci_shutdown(ahci);
 }
@@ -830,8 +854,7 @@ static void test_pci_spec(void)
 static void test_pci_enable(void)
 {
 AHCIQState *ahci;
-
-ahci = ahci_boot();
+ahci = ahci_boot(NULL);
 ahci_pci_enable(ahci);
 ahci_shutdown(ahci);
 }
@@ -844,7 +867,7 @@ static void test_hba_spec(void)
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot();
+ahci = ahci_boot(NULL);
 ahci_pci_enable(ahci);
 ahci_test_hba_spec(ahci);
 ahci_shutdown(ahci);
@@ -858,7 +881,7 @@ static void test_hba_enable(void)
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot();
+ahci = ahci_boot(NULL);
 ahci_pci_enable(ahci);
 ahci_hba_enable(ahci);
 ahci_shutdown(ahci);
@@ -872,7 +895,7 @@ static void test_identify(void)
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot_and_enable();
+ahci = ahci_boot_and_enable(NULL);
 ahci_test_identify(ahci);
 ahci_shutdown(ahci);
 }
@@ -894,7 +917,7 @@ static void test_dma_fragmented(void)
 unsigned i;
 uint64_t ptr;
 
-ahci = ahci_boot_and_enable();
+ahci = ahci_boot_and_enable(NULL);
 px = ahci_port_select(ahci);
 ahci_port_clear(ahci, px);
 
@@ -938,7 +961,7 @@ static void test_flush(void)
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot_and_enable();
+ahci = ahci_boot_and_enable(NULL);
 ahci_test_flush(ahci);
 ahci_shutdown(ahci);
 }
@@ -1053,7 +1076,7 @@ static void test_io_rw_interface(enum AddrMode lba48, 
enum IOMode dma,
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot_and_enable();
+ahci = ahci_boot_and_enable(NULL);
 ahci_test_io_rw_simple(ahci, bufsize, sector,
io_cmds[dma][lba48][IO_READ],
io_cmds[dma][lba48][IO_WRITE]);
diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c
index bbace89..1403699 100644
--- a/tests/libqos/libqos-pc.c
+++ b/tests/libqos/libqos-pc.c
@@ -6,6 +6,11 @@ static QOSOps qos_ops = 

[Qemu-devel] [PULL 50/69] ahci: add support for restarting non-queued commands

[Stefan Hajnoczi]

From: Paolo Bonzini 

This is easy, since start_dma already restarts processing from the
beginning of the PRDT.

Migration is also easy to cover; the comment about busy_slot is
wrong, busy_slot will only be set if there is an error.  In this
case we have nothing to do really.  The core IDE code will restart
the operation and command list processing will proceed after the
erroring command has been completed.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Message-id: 1424708286-16483-16-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 3f4fc77..56a4867 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1160,6 +1160,11 @@ static void ahci_start_dma(IDEDMA *dma, IDEState *s,
 dma_cb(s, 0);
 }
 
+static void ahci_restart_dma(IDEDMA *dma)
+{
+/* Nothing to do, ahci_start_dma already resets s->io_buffer_offset.  */
+}
+
 /**
  * Called in DMA R/W chains to read the PRDT, utilizing ahci_populate_sglist.
  * Not currently invoked by PIO R/W chains,
@@ -1248,6 +1253,7 @@ static void ahci_irq_set(void *opaque, int n, int level)
 
 static const IDEDMAOps ahci_dma_ops = {
 .start_dma = ahci_start_dma,
+.restart_dma = ahci_restart_dma,
 .start_transfer = ahci_start_transfer,
 .prepare_buf = ahci_dma_prepare_buf,
 .commit_buf = ahci_commit_buf,
@@ -1282,6 +1288,7 @@ void ahci_init(AHCIState *s, DeviceState *qdev, 
AddressSpace *as, int ports)
 ad->port_no = i;
 ad->port.dma = &ad->dma;
 ad->port.dma->ops = &ahci_dma_ops;
+ide_register_restart_cb(&ad->port);
 }
 }
 
@@ -1360,16 +1367,16 @@ static int ahci_state_post_load(void *opaque, int 
version_id)
 map_page(s->as, &ad->res_fis,
  ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256);
 /*
- * All pending i/o should be flushed out on a migrate. However,
- * we might not have cleared the busy_slot since this is done
- * in a bh. Also, issue i/o against any slots that are pending.
+ * If an error is present, ad->busy_slot will be valid and not -1.
+ * In this case, an operation is waiting to resume and will re-check
+ * for additional AHCI commands to execute upon completion.
+ *
+ * In the case where no error was present, busy_slot will be -1,
+ * and we should check to see if there are additional commands waiting.
  */
-if ((ad->busy_slot != -1) &&
-!(ad->port.ifs[0].status & (BUSY_STAT|DRQ_STAT))) {
-pr->cmd_issue &= ~(1 << ad->busy_slot);
-ad->busy_slot = -1;
+if (ad->busy_slot == -1) {
+check_cmd(s, i);
 }
-check_cmd(s, i);
 }
 
 return 0;
-- 
2.1.0

[Qemu-devel] [PULL 61/69] qtest/ahci: Add simple flush test

[Stefan Hajnoczi]

From: John Snow 

Signed-off-by: John Snow 
Message-id: 1424738729-17082-2-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ahci-test.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index fb4739f..b344121 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -774,6 +774,29 @@ static void ahci_test_io_rw_simple(AHCIQState *ahci, 
unsigned bufsize,
 g_free(rx);
 }
 
+static void ahci_test_nondata(AHCIQState *ahci, uint8_t ide_cmd)
+{
+uint8_t px;
+AHCICommand *cmd;
+
+/* Sanitize */
+px = ahci_port_select(ahci);
+ahci_port_clear(ahci, px);
+
+/* Issue Command */
+cmd = ahci_command_create(ide_cmd);
+ahci_command_commit(ahci, cmd, px);
+ahci_command_issue(ahci, cmd);
+ahci_command_verify(ahci, cmd);
+ahci_command_free(cmd);
+}
+
+static void ahci_test_flush(AHCIQState *ahci)
+{
+ahci_test_nondata(ahci, CMD_FLUSH_CACHE);
+}
+
+
 
/**/
 /* Test Interfaces
*/
 
/**/
@@ -911,6 +934,15 @@ static void test_dma_fragmented(void)
 g_free(tx);
 }
 
+static void test_flush(void)
+{
+AHCIQState *ahci;
+
+ahci = ahci_boot_and_enable();
+ahci_test_flush(ahci);
+ahci_shutdown(ahci);
+}
+
 
/**/
 /* AHCI I/O Test Matrix Definitions   
*/
 
@@ -1151,6 +1183,8 @@ int main(int argc, char **argv)
 
 qtest_add_func("/ahci/io/dma/lba28/fragmented", test_dma_fragmented);
 
+qtest_add_func("/ahci/flush/simple", test_flush);
+
 ret = g_test_run();
 
 /* Cleanup */
-- 
2.1.0

[Qemu-devel] [PULL 49/69] ahci: Migrate IDEStatus

[Stefan Hajnoczi]

From: John Snow 

Amazingly, we weren't doing this before.

Make sure we migrate the IDEState structure that belongs to
the AHCIDevice.IDEBus structure during migrations.

No version numbering changes because AHCI is not officially
migratable (and we can all see with good reason why) so we
do not impact any official builds by altering the stream and
leaving it at version 1.

This fixes the rerror=stop/werror=stop test case where we wish
to migrate a halted job. Previously, the error code would not
migrate, so even if the job completed successfully, AHCI would
report an error because it would still have the placeholder
error code from initialization time.

Reviewed-by: Paolo Bonzini 
Signed-off-by: John Snow 
Message-id: 1424708286-16483-15-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c | 1 +
 hw/ide/internal.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index bc6d5ce..3f4fc77 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1321,6 +1321,7 @@ static const VMStateDescription vmstate_ahci_device = {
 .version_id = 1,
 .fields = (VMStateField[]) {
 VMSTATE_IDE_BUS(port, AHCIDevice),
+VMSTATE_IDE_DRIVE(port.ifs[0], AHCIDevice),
 VMSTATE_UINT32(port_state, AHCIDevice),
 VMSTATE_UINT32(finished, AHCIDevice),
 VMSTATE_UINT32(port_regs.lst_addr, AHCIDevice),
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 0d5f881..965cc55 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -526,6 +526,9 @@ extern const VMStateDescription vmstate_ide_drive;
 #define VMSTATE_IDE_DRIVES(_field, _state) \
 VMSTATE_STRUCT_ARRAY(_field, _state, 2, 3, vmstate_ide_drive, IDEState)
 
+#define VMSTATE_IDE_DRIVE(_field, _state) \
+VMSTATE_STRUCT(_field, _state, 1, vmstate_ide_drive, IDEState)
+
 void ide_bus_reset(IDEBus *bus);
 int64_t ide_get_sector(IDEState *s);
 void ide_set_sector(IDEState *s, int64_t sector_num);
-- 
2.1.0

[Qemu-devel] [PULL 47/69] ide: make more functions static

[Stefan Hajnoczi]

From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Message-id: 1424708286-16483-13-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c | 12 
 hw/ide/internal.h |  4 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index ff28db0..ef52f35 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -561,6 +561,8 @@ static bool ide_sect_range_ok(IDEState *s,
 return true;
 }
 
+static void ide_sector_read(IDEState *s);
+
 static void ide_sector_read_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
@@ -595,7 +597,7 @@ static void ide_sector_read_cb(void *opaque, int ret)
 s->io_buffer_offset += 512 * n;
 }
 
-void ide_sector_read(IDEState *s)
+static void ide_sector_read(IDEState *s)
 {
 int64_t sector_num;
 int n;
@@ -682,7 +684,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int 
op)
 return action != BLOCK_ERROR_ACTION_IGNORE;
 }
 
-void ide_dma_cb(void *opaque, int ret)
+static void ide_dma_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
 int n;
@@ -810,6 +812,8 @@ void ide_start_dma(IDEState *s, BlockCompletionFunc *cb)
 }
 }
 
+static void ide_sector_write(IDEState *s);
+
 static void ide_sector_write_timer_cb(void *opaque)
 {
 IDEState *s = opaque;
@@ -869,7 +873,7 @@ static void ide_sector_write_cb(void *opaque, int ret)
 }
 }
 
-void ide_sector_write(IDEState *s)
+static void ide_sector_write(IDEState *s)
 {
 int64_t sector_num;
 int n;
@@ -923,7 +927,7 @@ static void ide_flush_cb(void *opaque, int ret)
 ide_set_irq(s->bus);
 }
 
-void ide_flush_cache(IDEState *s)
+static void ide_flush_cache(IDEState *s)
 {
 if (s->blk == NULL) {
 ide_flush_cb(s, 0);
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 02206a9..0d5f881 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -557,10 +557,6 @@ void ide_init_ioport(IDEBus *bus, ISADevice *isa, int 
iobase, int iobase2);
 void ide_register_restart_cb(IDEBus *bus);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
-void ide_dma_cb(void *opaque, int ret);
-void ide_sector_write(IDEState *s);
-void ide_sector_read(IDEState *s);
-void ide_flush_cache(IDEState *s);
 
 void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
 EndTransferFunc *end_transfer_func);
-- 
2.1.0

[Qemu-devel] [PULL 36/69] ide: start extracting ide_restart_dma out of bmdma_restart_dma

[Stefan Hajnoczi]

From: Paolo Bonzini 

This patch begins refactoring the restart dma functions
out of bmdma to be shared with AHCI and other future
IDE HBA implementations.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Message-id: 1424708286-16483-2-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/pci.c | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index e3f2054..da3e392 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -184,18 +184,24 @@ static void bmdma_set_inactive(IDEDMA *dma, bool more)
 }
 }
 
-static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd)
+static void bmdma_restart_dma(BMDMAState *bm)
 {
 IDEState *s = bmdma_active_if(bm);
 
 ide_set_sector(s, bm->sector_num);
-s->io_buffer_index = 0;
-s->io_buffer_size = 0;
 s->nsector = bm->nsector;
-s->dma_cmd = dma_cmd;
 bm->cur_addr = bm->addr;
-bm->dma_cb = ide_dma_cb;
-bmdma_start_dma(&bm->dma, s, bm->dma_cb);
+}
+
+static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
+{
+BMDMAState *bm = DO_UPCAST(BMDMAState, dma, s->bus->dma);
+
+bmdma_restart_dma(bm);
+s->io_buffer_index = 0;
+s->io_buffer_size = 0;
+s->dma_cmd = dma_cmd;
+bmdma_start_dma(&bm->dma, s, ide_dma_cb);
 }
 
 /* TODO This should be common IDE code */
@@ -203,6 +209,7 @@ static void bmdma_restart_bh(void *opaque)
 {
 BMDMAState *bm = opaque;
 IDEBus *bus = bm->bus;
+IDEState *s;
 bool is_read;
 int error_status;
 
@@ -213,6 +220,7 @@ static void bmdma_restart_bh(void *opaque)
 return;
 }
 
+s = bmdma_active_if(bm);
 is_read = (bus->error_status & IDE_RETRY_READ) != 0;
 
 /* The error status must be cleared before resubmitting the request: The
@@ -223,18 +231,18 @@ static void bmdma_restart_bh(void *opaque)
 
 if (error_status & IDE_RETRY_DMA) {
 if (error_status & IDE_RETRY_TRIM) {
-bmdma_restart_dma(bm, IDE_DMA_TRIM);
+ide_restart_dma(s, IDE_DMA_TRIM);
 } else {
-bmdma_restart_dma(bm, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
+ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
 }
 } else if (error_status & IDE_RETRY_PIO) {
 if (is_read) {
-ide_sector_read(bmdma_active_if(bm));
+ide_sector_read(s);
 } else {
-ide_sector_write(bmdma_active_if(bm));
+ide_sector_write(s);
 }
 } else if (error_status & IDE_RETRY_FLUSH) {
-ide_flush_cache(bmdma_active_if(bm));
+ide_flush_cache(s);
 } else {
 IDEState *s = bmdma_active_if(bm);
 
-- 
2.1.0

[Qemu-devel] [PULL 56/69] qtest/ahci: Add DMA test variants

[Stefan Hajnoczi]

From: John Snow 

These test a few different pathways in the AHCI code.

short:  Test the minimum transfer size, exactly one sector.
simple: Test a transfer using a single PRD, in this case, 4K.
double: Test transferring 8K, which we will split up as two PRDs.
long:   Test transferring a lot of data using many PRDs, 256K.
Signed-off-by: John Snow 
Message-id: 1424905602-24715-5-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ahci-test.c | 38 ++
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 9fe9fb5..9394d85 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -731,12 +731,11 @@ static void ahci_test_identify(AHCIQState *ahci)
 g_assert_cmphex(sect_size, ==, 0x200);
 }
 
-static void ahci_test_dma_rw_simple(AHCIQState *ahci)
+static void ahci_test_dma_rw_simple(AHCIQState *ahci, unsigned bufsize)
 {
 uint64_t ptr;
 uint8_t port;
 unsigned i;
-const unsigned bufsize = 4096;
 unsigned char *tx = g_malloc(bufsize);
 unsigned char *rx = g_malloc0(bufsize);
 
@@ -751,7 +750,7 @@ static void ahci_test_dma_rw_simple(AHCIQState *ahci)
 ptr = ahci_alloc(ahci, bufsize);
 g_assert(ptr);
 
-/* Write some indicative pattern to our 4K buffer. */
+/* Write some indicative pattern to our buffer. */
 for (i = 0; i < bufsize; i++) {
 tx[i] = (bufsize - i);
 }
@@ -852,15 +851,35 @@ static void test_identify(void)
 }
 
 /**
- * Perform a simple DMA R/W test, using a single PRD and non-NCQ commands.
+ * Perform a simple DMA R/W test using non-NCQ commands.
  */
+static void test_dma_rw_interface(unsigned bufsize)
+{
+AHCIQState *ahci;
+
+ahci = ahci_boot_and_enable();
+ahci_test_dma_rw_simple(ahci, bufsize);
+ahci_shutdown(ahci);
+}
+
 static void test_dma_rw_simple(void)
 {
-AHCIQState *ahci;
+test_dma_rw_interface(4096);
+}
 
-ahci = ahci_boot_and_enable();
-ahci_test_dma_rw_simple(ahci);
-ahci_shutdown(ahci);
+static void test_dma_rw_double(void)
+{
+test_dma_rw_interface(8192);
+}
+
+static void test_dma_rw_long(void)
+{
+test_dma_rw_interface(4096 * 64);
+}
+
+static void test_dma_rw_short(void)
+{
+test_dma_rw_interface(512);
 }
 
 
/**/
@@ -919,6 +938,9 @@ int main(int argc, char **argv)
 qtest_add_func("/ahci/hba_enable", test_hba_enable);
 qtest_add_func("/ahci/identify",   test_identify);
 qtest_add_func("/ahci/dma/simple", test_dma_rw_simple);
+qtest_add_func("/ahci/dma/double", test_dma_rw_double);
+qtest_add_func("/ahci/dma/long",   test_dma_rw_long);
+qtest_add_func("/ahci/dma/short",  test_dma_rw_short);
 
 ret = g_test_run();
 
-- 
2.1.0

[Qemu-devel] [PULL 41/69] ide: move restart callback to common code

[Stefan Hajnoczi]

From: Paolo Bonzini 

With BMDMA specific excised from the restart functions,
create a HBA-agnostic restart callback to be shared
between the different HBAs.

Change the callback registered with the vmstate_change
handler to always point to ide_restart_cb instead of
relying on the IDEDMAOps.restart_cb() member.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Message-id: 1424708286-16483-7-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c | 80 ++-
 hw/ide/internal.h |  2 ++
 hw/ide/pci.c  | 79 --
 hw/ide/pci.h  |  1 -
 4 files changed, 81 insertions(+), 81 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 9d510b1..a4250f6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2314,6 +2314,10 @@ static int ide_nop_int(IDEDMA *dma, int x)
 return 0;
 }
 
+static void ide_nop(IDEDMA *dma)
+{
+}
+
 static int32_t ide_nop_int32(IDEDMA *dma, int x)
 {
 return 0;
@@ -2325,14 +2329,88 @@ static void ide_nop_restart(void *opaque, int x, 
RunState y)
 
 static const IDEDMAOps ide_dma_nop_ops = {
 .prepare_buf= ide_nop_int32,
+.restart_dma= ide_nop,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
 .restart_cb = ide_nop_restart,
 };
 
+static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
+{
+s->bus->dma->ops->restart_dma(s->bus->dma);
+s->io_buffer_index = 0;
+s->io_buffer_size = 0;
+s->dma_cmd = dma_cmd;
+ide_start_dma(s, ide_dma_cb);
+}
+
+static void ide_restart_bh(void *opaque)
+{
+IDEBus *bus = opaque;
+IDEState *s;
+bool is_read;
+int error_status;
+
+qemu_bh_delete(bus->bh);
+bus->bh = NULL;
+
+error_status = bus->error_status;
+if (bus->error_status == 0) {
+return;
+}
+
+s = idebus_active_if(bus);
+is_read = (bus->error_status & IDE_RETRY_READ) != 0;
+
+/* The error status must be cleared before resubmitting the request: The
+ * request may fail again, and this case can only be distinguished if the
+ * called function can set a new error status. */
+bus->error_status = 0;
+
+if (error_status & IDE_RETRY_DMA) {
+if (error_status & IDE_RETRY_TRIM) {
+ide_restart_dma(s, IDE_DMA_TRIM);
+} else {
+ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
+}
+} else if (error_status & IDE_RETRY_PIO) {
+if (is_read) {
+ide_sector_read(s);
+} else {
+ide_sector_write(s);
+}
+} else if (error_status & IDE_RETRY_FLUSH) {
+ide_flush_cache(s);
+} else {
+/*
+ * We've not got any bits to tell us about ATAPI - but
+ * we do have the end_transfer_func that tells us what
+ * we're trying to do.
+ */
+if (s->end_transfer_func == ide_atapi_cmd) {
+ide_atapi_dma_restart(s);
+}
+}
+}
+
+static void ide_restart_cb(void *opaque, int running, RunState state)
+{
+IDEBus *bus = opaque;
+
+if (!running)
+return;
+
+if (!bus->bh) {
+bus->bh = qemu_bh_new(ide_restart_bh, bus);
+qemu_bh_schedule(bus->bh);
+}
+}
+
 void ide_register_restart_cb(IDEBus *bus)
 {
-qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus);
+if (bus->dma->ops->restart_dma) {
+qemu_add_vm_change_state_handler(ide_restart_cb, bus);
+}
 }
 
 static IDEDMA ide_dma_nop = {
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 57ef5ca..9a11c3c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -456,6 +456,8 @@ struct IDEBus {
 IDEDevice *master;
 IDEDevice *slave;
 IDEState ifs[2];
+QEMUBH *bh;
+
 int bus_id;
 int max_units;
 IDEDMA *dma;
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 45254d4..cd1bbb0 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -194,84 +194,6 @@ static void bmdma_restart_dma(IDEDMA *dma)
 bm->cur_addr = bm->addr;
 }
 
-static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
-{
-if (s->bus->dma->ops->restart_dma) {
-s->bus->dma->ops->restart_dma(s->bus->dma);
-}
-s->io_buffer_index = 0;
-s->io_buffer_size = 0;
-s->dma_cmd = dma_cmd;
-ide_start_dma(s, ide_dma_cb);
-}
-
-/* TODO This should be common IDE code */
-static void bmdma_restart_bh(void *opaque)
-{
-IDEBus *bus = opaque;
-BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus->dma);
-IDEState *s;
-bool is_read;
-int error_status;
-
-qemu_bh_delete(bm->bh);
-bm->bh = NULL;
-
-error_status = bus->error_status;
-if (bus->error_status == 0) {
-return;
-}
-
-s = idebus_active_if(bus);
-is_read = (bus->error_status & IDE_RETRY_READ) != 0;
-
-/* The error status must be cleared before resubmitting the request: The
- * request may fail again, and this case can only be distinguished if

[Qemu-devel] [PULL 45/69] ide: migrate initial request state via IDEBus

[Stefan Hajnoczi]

From: Paolo Bonzini 

This only breaks backwards migration compatibility if the bus is in
an error state.  It is in principle possible to avoid this by making
two subsections (one for version 1, and one for version 2, but with
the same name) with different "_needed" callbacks.  The v1 callback would
return true if error_status != 0 and the bus is PATA; the v2 callback
would return true if error_status != 0 and the bus is AHCI.

Forward migration keeps working.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Message-id: 1424708286-16483-11-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 71ec1e7..b62a94a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2643,10 +2643,13 @@ const VMStateDescription vmstate_ide_drive = {
 
 static const VMStateDescription vmstate_ide_error_status = {
 .name ="ide_bus/error",
-.version_id = 1,
+.version_id = 2,
 .minimum_version_id = 1,
 .fields = (VMStateField[]) {
 VMSTATE_INT32(error_status, IDEBus),
+VMSTATE_INT64_V(retry_sector_num, IDEBus, 2),
+VMSTATE_UINT32_V(retry_nsector, IDEBus, 2),
+VMSTATE_UINT8_V(retry_unit, IDEBus, 2),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
2.1.0

[Qemu-devel] [PULL 52/69] qtest/ide: Test flush / retry for ISA and PCI

[Stefan Hajnoczi]

From: John Snow 

This patch adds tests for werror and rerror functionality
for the PCI and ISA ide buses.

Tests for the AHCI device are to be included at a later
date after requisite patches have been merged upstream
to support needed functionality by the tests.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Reviewed-by: Paolo Bonzini 
Message-id: 1424708286-16483-18-git-send-email-js...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/ide-test.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 29f4039..b28a302 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -118,7 +118,6 @@ static void ide_test_start(const char *cmdline_fmt, ...)
 va_end(ap);
 
 qtest_start(cmdline);
-qtest_irq_intercept_in(global_qtest, "ioapic");
 guest_malloc = pc_alloc_init();
 
 g_free(cmdline);
@@ -388,6 +387,7 @@ static void test_bmdma_setup(void)
 "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
 "-global ide-hd.ver=%s",
 tmp_path, "testdisk", "version");
+qtest_irq_intercept_in(global_qtest, "ioapic");
 }
 
 static void test_bmdma_teardown(void)
@@ -516,7 +516,7 @@ static void prepare_blkdebug_script(const char *debug_fn, 
const char *event)
 g_assert(ret == 0);
 }
 
-static void test_retry_flush(void)
+static void test_retry_flush(const char *machine)
 {
 uint8_t data;
 const char *s;
@@ -580,6 +580,16 @@ static void test_flush_nodev(void)
 ide_test_quit();
 }
 
+static void test_pci_retry_flush(const char *machine)
+{
+test_retry_flush("pc");
+}
+
+static void test_isa_retry_flush(const char *machine)
+{
+test_retry_flush("isapc");
+}
+
 int main(int argc, char **argv)
 {
 const char *arch = qtest_get_arch();
@@ -617,9 +627,9 @@ int main(int argc, char **argv)
 qtest_add_func("/ide/bmdma/teardown", test_bmdma_teardown);
 
 qtest_add_func("/ide/flush", test_flush);
-qtest_add_func("/ide/flush_nodev", test_flush_nodev);
-
-qtest_add_func("/ide/retry/flush", test_retry_flush);
+qtest_add_func("/ide/flush/nodev", test_flush_nodev);
+qtest_add_func("/ide/flush/retry_pci", test_pci_retry_flush);
+qtest_add_func("/ide/flush/retry_isa", test_isa_retry_flush);
 
 ret = g_test_run();
 
-- 
2.1.0

[Qemu-devel] [PULL 32/69] raw-posix: Factor block size detection out of raw_probe_alignment()

[Stefan Hajnoczi]

From: Ekaterina Tumanova 

Put it in new probe_logical_blocksize().

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
Message-id: 1424087278-49393-3-git-send-email-tuman...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi 
---
 block/raw-posix.c | 51 +++
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c0b46ca..34d403d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -218,39 +218,58 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
-static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+/*
+ * Get logical block size via ioctl. On success store it in @sector_size_p.
+ */
+static int probe_logical_blocksize(int fd, unsigned int *sector_size_p)
 {
-BDRVRawState *s = bs->opaque;
-char *buf;
 unsigned int sector_size;
+bool success = false;
 
-/* For /dev/sg devices the alignment is not really used.
-   With buffered I/O, we don't have any restrictions. */
-if (bs->sg || !s->needs_alignment) {
-bs->request_alignment = 1;
-s->buf_align = 1;
-return;
-}
+errno = ENOTSUP;
 
 /* Try a few ioctls to get the right size */
-bs->request_alignment = 0;
-s->buf_align = 0;
-
 #ifdef BLKSSZGET
 if (ioctl(fd, BLKSSZGET, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
 #ifdef DKIOCGETBLOCKSIZE
 if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
 #ifdef DIOCGSECTORSIZE
 if (ioctl(fd, DIOCGSECTORSIZE, §or_size) >= 0) {
-bs->request_alignment = sector_size;
+*sector_size_p = sector_size;
+success = true;
 }
 #endif
+
+return success ? 0 : -errno;
+}
+
+static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !s->needs_alignment) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+bs->request_alignment = 0;
+s->buf_align = 0;
+/* Let's try to use the logical blocksize for the alignment. */
+if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) {
+bs->request_alignment = 0;
+}
 #ifdef CONFIG_XFS
 if (s->is_xfs) {
 struct dioattr da;
-- 
2.1.0

[Qemu-devel] [PULL 33/69] block: Add driver methods to probe blocksizes and geometry

[Stefan Hajnoczi]

From: Ekaterina Tumanova 

Introduce driver methods of defining disk blocksizes (physical and
logical) and hard drive geometry.
Methods are only implemented for "host_device". For "raw" devices
driver calls child's method.

For now geometry detection will only work for DASD devices. To check
that a local check_for_dasd function was introduced. It calls BIODASDINFO2
ioctl and returns its rc.

Blocksizes detection function will probe sizes for DASD devices.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
Message-id: 1424087278-49393-4-git-send-email-tuman...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi 
---
 block/raw-posix.c | 103 ++
 block/raw_bsd.c   |  12 +++
 2 files changed, 115 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 34d403d..3263d2b 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -56,6 +56,10 @@
 #include 
 #include 
 #include 
+#include 
+#ifdef __s390__
+#include 
+#endif
 #ifndef FS_NOCOW_FL
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
@@ -251,6 +255,23 @@ static int probe_logical_blocksize(int fd, unsigned int 
*sector_size_p)
 return success ? 0 : -errno;
 }
 
+/**
+ * Get physical block size of @fd.
+ * On success, store it in @blk_size and return 0.
+ * On failure, return -errno.
+ */
+static int probe_physical_blocksize(int fd, unsigned int *blk_size)
+{
+#ifdef BLKPBSZGET
+if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
+return -errno;
+}
+return 0;
+#else
+return -ENOTSUP;
+#endif
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
@@ -674,6 +695,86 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 bs->bl.opt_mem_alignment = s->buf_align;
 }
 
+static int check_for_dasd(int fd)
+{
+#ifdef BIODASDINFO2
+struct dasd_information2_t info = {0};
+
+return ioctl(fd, BIODASDINFO2, &info);
+#else
+return -1;
+#endif
+}
+
+/**
+ * Try to get @bs's logical and physical block size.
+ * On success, store them in @bsz and return zero.
+ * On failure, return negative errno.
+ */
+static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+BDRVRawState *s = bs->opaque;
+int ret;
+
+/* If DASD, get blocksizes */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+ret = probe_logical_blocksize(s->fd, &bsz->log);
+if (ret < 0) {
+return ret;
+}
+return probe_physical_blocksize(s->fd, &bsz->phys);
+}
+
+/**
+ * Try to get @bs's geometry: cyls, heads, sectors.
+ * On success, store them in @geo and return 0.
+ * On failure return -errno.
+ * (Allows block driver to assign default geometry values that guest sees)
+ */
+#ifdef __linux__
+static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+BDRVRawState *s = bs->opaque;
+struct hd_geometry ioctl_geo = {0};
+uint32_t blksize;
+
+/* If DASD, get its geometry */
+if (check_for_dasd(s->fd) < 0) {
+return -ENOTSUP;
+}
+if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
+return -errno;
+}
+/* HDIO_GETGEO may return success even though geo contains zeros
+   (e.g. certain multipath setups) */
+if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
+return -ENOTSUP;
+}
+/* Do not return a geometry for partition */
+if (ioctl_geo.start != 0) {
+return -ENOTSUP;
+}
+geo->heads = ioctl_geo.heads;
+geo->sectors = ioctl_geo.sectors;
+if (!probe_physical_blocksize(s->fd, &blksize)) {
+/* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */
+geo->cylinders = bdrv_nb_sectors(bs) / (blksize / BDRV_SECTOR_SIZE)
+ / (geo->heads * geo->sectors);
+return 0;
+}
+geo->cylinders = ioctl_geo.cylinders;
+
+return 0;
+}
+#else /* __linux__ */
+static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
+{
+return -ENOTSUP;
+}
+#endif
+
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
 int ret;
@@ -2215,6 +2316,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_get_info = raw_get_info,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
+.bdrv_probe_blocksizes = hdev_probe_blocksizes,
+.bdrv_probe_geometry = hdev_probe_geometry,
 
 .bdrv_detach_aio_context = raw_detach_aio_context,
 .bdrv_attach_aio_context = raw_attach_aio_context,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 05b02c7..e3d2d04 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -235,6 +235,16 @@ static int raw_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 1;
 }
 
+static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+return bdrv_probe_blocksizes(bs->file, bsz);