[Qemu-devel] Passing NFS volume on host to win7 guest

[liebrecht]

Hope I dont have to use samba for this.

From reading online searches, it seems the following  could work where 
backup is the nfs exported volume to be shared.


-virtfs 
local,path=/backup,mount_tag=host0,security_model=passthrough,id=host0


qemu boots into win7 with this string added so it was accepted but I 
could not find a way to access it from within win7

Anyone has good advice

[Qemu-devel] Patchew down?

[Philippe Mathieu-Daudé]

Hi,

I think patchew is having some trouble since at least 1 week:
new series aren't added.

Regards,

Phil.

Re: [Qemu-devel] [PATCH v4 5/5] migration: Use strnlen() for fixed-size string

[Richard Henderson]

On 12/29/18 4:33 AM, Philippe Mathieu-Daudé wrote:
> GCC 8 introduced the -Wstringop-overflow, which detect buffer overflow
> by string-modifying functions declared in , such strncpy(),
> used in global_state_store_running().
> 
> GCC indeed found an incorrect use of strlen(), because this array
> is loaded by VMSTATE_BUFFER(runstate, GlobalState) then parsed
> using qapi_enum_parse which does not get the buffer length.
> 
> Use strnlen() which returns sizeof(s->runstate) if the array is not
> NUL-terminated, assert the size is within range, and enforce the array
> to be NUL-terminated to avoid an overflow in qapi_enum_parse().
> 
> This fixes:
> 
> CC  migration/global_state.o
>   qemu/migration/global_state.c: In function 'global_state_pre_save':
>   qemu/migration/global_state.c:109:15: error: 'strlen' argument 1 declared 
> attribute 'nonstring' [-Werror=stringop-overflow=]
>s->size = strlen((char *)s->runstate) + 1;
>  ^~~
>   qemu/migration/global_state.c:24:13: note: argument 'runstate' declared here
>uint8_t runstate[100] QEMU_NONSTRING;
>^~~~
>   cc1: all warnings being treated as errors
>   make: *** [qemu/rules.mak:69: migration/global_state.o] Error 1
> 
> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  migration/global_state.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v4 1/5] qemu/compiler: Define QEMU_NONSTRING

[Richard Henderson]

On 12/29/18 4:33 AM, Philippe Mathieu-Daudé wrote:
> GCC 8 introduced the -Wstringop-truncation checker to detect truncation by
> the strncat and strncpy functions (closely related to -Wstringop-overflow,
> which detect buffer overflow by string-modifying functions declared in
> ).
> 
> In tandem of -Wstringop-truncation, the "nonstring" attribute was added:
> 
>   The nonstring variable attribute specifies that an object or member
>   declaration with type array of char, signed char, or unsigned char,
>   or pointer to such a type is intended to store character arrays that
>   do not necessarily contain a terminating NUL. This is useful in detecting
>   uses of such arrays or pointers with functions that expect NUL-terminated
>   strings, and to avoid warnings when such an array or pointer is used as
>   an argument to a bounded string manipulation function such as strncpy.
> 
>   From the GCC manual: 
> https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-nonstring-variable-attribute
> 
> Add the QEMU_NONSTRING macro which checks if the compiler supports this
> attribute.
> 
> Suggested-by: Michael S. Tsirkin 
> Reviewed-by: Eric Blake 
> Reviewed-by: Michael S. Tsirkin 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 4/5 v2] RISC-V: Add debug support for accessing CSRs.

[Richard Henderson]

On 12/29/18 9:10 AM, Jim Wilson wrote:
> Adds a debugger parameter to csr_read_helper and csr_write_helper.  When
> this is true, we disable illegal instruction checks.
> 
> Signed-off-by: Jim Wilson 
> ---
>  linux-user/riscv/signal.c |  5 ++-
>  target/riscv/cpu.h|  7 +++-
>  target/riscv/cpu_helper.c |  4 +-
>  target/riscv/gdbstub.c|  4 +-
>  target/riscv/op_helper.c  | 93 
> ---
>  5 files changed, 76 insertions(+), 37 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 1/5 v2] RISC-V: Add 32-bit gdb xml files.

[Richard Henderson]

On 12/29/18 9:07 AM, Jim Wilson wrote:
> Signed-off-by: Jim Wilson 
> ---
>  configure   |   1 +
>  gdb-xml/riscv-32bit-cpu.xml |  43 
>  gdb-xml/riscv-32bit-csr.xml | 250 
> 
>  gdb-xml/riscv-32bit-fpu.xml |  46 
>  4 files changed, 340 insertions(+)
>  create mode 100644 gdb-xml/riscv-32bit-cpu.xml
>  create mode 100644 gdb-xml/riscv-32bit-csr.xml
>  create mode 100644 gdb-xml/riscv-32bit-fpu.xml

Don't the csr's vary between priv-1.9.1 and priv-1.10?

I wonder if it would be better to auto-generate these, via the
gdb_get_dynamic_xml hook.  That way you don't need near identical copies for
32-bit vs 64-bit.


r~

Re: [Qemu-devel] [PATCH 3/5 v2] RISC-V: Map gdb CSR reg numbers to hw reg numbers.

[Richard Henderson]

On 12/29/18 9:09 AM, Jim Wilson wrote:
> +++ b/target/riscv/csr-map.h
> @@ -0,0 +1,248 @@
> +/*
> + * The GDB CSR xml files list them in documentation order, not numerical 
> order,
> + * and are missing entries for unnamed CSRs.  So we need to map the gdb 
> numbers
> + * to the hardware numbers.
> + */
> +
> +int csr_register_map[] = {

static const?

Putting an initialized variable in a header file doesn't seem right.  Is this
supposed to be a declaration that is shared between c files?


r~

Re: [Qemu-devel] [PATCH v2 8/8] target/ppc: remove various HOST_WORDS_BIGENDIAN hacks in int_helper.c

[Richard Henderson]

On 12/29/18 12:52 AM, Mark Cave-Ayland wrote:
> Following on from the previous work, there are numerous endian-related hacks
> in int_helper.c that can now be replaced with Vsr* macros.
> 
> There are also a few places where the VECTOR_FOR_INORDER_I macro can be
> replaced with a normal iterator since the processing order is irrelevant.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  target/ppc/int_helper.c | 155 
> ++--
>  1 file changed, 45 insertions(+), 110 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 7/8] target/ppc: remove ROTRu32 and ROTRu64 macros from int_helper.c

[Richard Henderson]

On 12/29/18 12:52 AM, Mark Cave-Ayland wrote:
> Richard points out that these macros suffer from a -fsanitize=shift bug in 
> that
> they improperly handle n == 0 turning it into a shift by 32/64 respectively.
> Replace them with QEMU's existing ror32() and ror64() functions instead.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  target/ppc/int_helper.c | 48 
>  1 file changed, 20 insertions(+), 28 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 6/8] target/ppc: simplify VEXT_SIGNED macro in int_helper.c

[Richard Henderson]

On 12/29/18 12:52 AM, Mark Cave-Ayland wrote:
> As pointed out by Richard: it does not need the mask argument, nor does it 
> need
> the recast argument. The masking is implied by the cast argument, and the
> recast is implied by the assignment.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  target/ppc/int_helper.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] AVX support for TCG

[Richard Henderson]

On 12/29/18 12:43 AM, Nick Renieris wrote:
>>> Do you think this could work as a GSoC project? I'm potentially
>>> interested in working on it this summer.
> 
>> Could be.  My first guess is something like 4 months work for this.
> 
> Four months full-time? If so I would say it's not viable for a GSoC
> project (it's 3 months), I've done the 12-hours-a-day-crunch thing for
> a week or so in GSoC 2017 and it was _not_ fun.
> Also, I hope you meant four months for me, not for you - I'm
> completely new to the QEMU codebase. I expect it will take me weeks
> just to understand x86's 'translate.c' (who thought it'd be a good
> idea to put all this stuff in _one_ file?).

I did have a beginner in mind when guessing 4 months.  Don't take that as a
fully speced out answer, but it may well be that full avx2 support cannot be
done within the 3 months of gsoc.  I would certainly expect avx512 to take even
longer.

> Another question, are there existing discussions about this
> refactoring effort or specifically AVX? I asked a similar question on
> IRC a few days ago and got no answers.

Not that I recall.  I have some code at

https://github.com/rth7680/qemu/commits/i386-avx

that attempts to remove the sse_op_table(s).  However, it also splits up the
sse operations into units of uint64_t.  Which seemed sort of reasonable at the
time, considering that a lot of sse is 2*mmx.

But in the intervening 2.5 years since I worked on that branch, we have learned
that calls to helpers dominate.  It's better to have a single call that does 4x
the work than 4 separate calls.

The tcg-op-gvec.h infrastructure allows for the different modes that avx+mmx
allows:

(1) 64-bit operations,
(2) 128-bit operations, modifying only the low 128 bits,
(3) 128-bit operations, zeroing bits beyond the first 128,
(4) N*128-bit operations, zeroing bits beyond the first N*128.

so we should not need a great proliferation of helper functions, merely a
re-organization of what we have now.


r~

[Qemu-devel] [Bug 1810000] Re: qemu system emulator crashed with the attachment of usb-bt-dongle device

[PH]

** Description changed:

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

Title:
  qemu system emulator crashed with the attachment of usb-bt-dongle
  device

Status in QEMU:
  New

Bug description:
  I am testing usb-bt-dongle device on xchi host controller, and found
  that the qemu crashed directly with an assertion failer.

  Here is the information to reproduce the crash:

  Qemu git revision: 9b2e891ec5ccdb4a7d583b77988848282606fdea
  System emulator: qemu-x86_64
  VM image: 
https://people.debian.org/~aurel32/qemu/amd64/debian_squeeze_amd64_desktop.qcow2
  CommandLine: qemu-system-x86_64 -M q35 -device qemu-xhci,id=xhci -enable-kvm 
-device usb-bt-dongle  -hda ./debian_wheezy_amd64_standard.qcow2

  Error message:

  qemu-system-x86_64: /build/qemu-
  Eap4uc/qemu-2.11+dfsg/hw/usb/core.c:592: usb_packet_copy: Assertion
  `p->actual_length + bytes <= iov->size' failed.

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

Re: [Qemu-devel] d_off field in struct dirent and 32-on-64 emulation

[Andy Lutomirski]

> On Dec 28, 2018, at 6:54 PM, Matthew Wilcox  wrote:
>
>> On Sat, Dec 29, 2018 at 12:12:27AM +, Peter Maydell wrote:
>> On Fri, 28 Dec 2018 at 23:16, Andreas Dilger  wrot
>>> On Dec 28, 2018, at 4:18 AM, Peter Maydell  wrote:
 The problem is that there is no 32-bit API in some cases
 (unless I have misunderstood the kernel code) -- not all
 host architectures implement compat syscalls or allow them
 to be called from 64-bit processes or implement all the older
 syscall variants that had smaller offets. If there was a guaranteed
 "this syscall always exists and always gives me 32-bit offsets"
 we could use it.
>>>
>>> The "32bitapi" mount option would use 32-bit hash for seekdir
>>> and telldir, regardless of what kernel API was used.  That would
>>> just set the FMODE_32BITHASH flag in the file->f_mode for all files.
>>
>> A mount option wouldn't be much use to QEMU -- we can't tell
>> our users how to mount their filesystems, which they're
>> often doing lots of other things with besides running QEMU.
>> (Otherwise we could just tell them "don't use ext4", which
>> would also solve the problem :-)) We need something we can
>> use at the individual-syscall level.
>
> Could you use a prctl to set whether you were running in 32 or 64 bit
> mode?  Or do you change which kind of task you're emulating too often
> to make this a good idea?


How would this work?  We already have the separate
COMPAT_DEFINE_SYSCALL entries *and* in_compat_syscall(). Now we’d have
a third degree of freedom.

Either the arches people care about should add reasonable ways to
issue 32-bit syscalls from 64-bit mode or there should be an explicit
way to ask for the 32-bit directory offsets.

Re: [Qemu-devel] [PATCH 2/4] Add CET SHSTK and IBT CPUID feature-word definitions.

[Yang Weijiang]

On Fri, Dec 28, 2018 at 03:25:10PM +0100, Paolo Bonzini wrote:
Thanks a lot Paolo for the comments!

I'll fix the issue in next version.

> On 26/12/18 09:25, Yang Weijiang wrote:
> > @@ -1233,6 +1252,14 @@ static const ExtSaveArea x86_ext_save_areas[] = {
> >{ .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
> >  .offset = offsetof(X86XSaveArea, pkru_state),
> >  .size = sizeof(XSavePKRU) },
> > +[XSTATE_CET_U_BIT] = {
> > +.feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_CET_SHSTK,
> > +.offset = offsetof(X86XSaveArea, cet_u),
> 
> These offsets are incorrect, since supervisor states are only stored in
> the compacted format.  In fact, in patch 4, supervisor states should
> return 0 in CPUID(EAX=0Dh,ECX=n).EBX.
> 
> You can use offset == 0 to distinguish supervisor and user states, so
> that supervisor states are skipped in xsave_area_size and x86_cpu_reset.
> 
> Thanks,
> 
> Paolo
> 
> > +.size = sizeof(XSaveCETU) },
> > +[XSTATE_CET_S_BIT] = {
> > +.feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_CET_SHSTK,
> > +.offset = offsetof(X86XSaveArea, cet_s),
> > +.size = sizeof(XSaveCETS) },
> >  };
> >  
> >  static uint32_t xsave_area_size(uint64_t mask)

[Qemu-devel] [PATCH v5 01/11] block/backup: simplify backup_incremental_init_copy_bitmap

[Vladimir Sementsov-Ogievskiy]

Simplify backup_incremental_init_copy_bitmap using the function
bdrv_dirty_bitmap_next_dirty_area.

Note: move to job->len instead of bitmap size: it should not matter but
less code.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/backup.c | 40 
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 435414e964..fbe7ce19e1 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -406,43 +406,27 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
 /* init copy_bitmap from sync_bitmap */
 static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
 {
-BdrvDirtyBitmapIter *dbi;
-int64_t offset;
-int64_t end = DIV_ROUND_UP(bdrv_dirty_bitmap_size(job->sync_bitmap),
-   job->cluster_size);
-
-dbi = bdrv_dirty_iter_new(job->sync_bitmap);
-while ((offset = bdrv_dirty_iter_next(dbi)) != -1) {
-int64_t cluster = offset / job->cluster_size;
-int64_t next_cluster;
-
-offset += bdrv_dirty_bitmap_granularity(job->sync_bitmap);
-if (offset >= bdrv_dirty_bitmap_size(job->sync_bitmap)) {
-hbitmap_set(job->copy_bitmap, cluster, end - cluster);
-break;
-}
+uint64_t offset = 0;
+uint64_t bytes = job->len;
 
-offset = bdrv_dirty_bitmap_next_zero(job->sync_bitmap, offset,
- UINT64_MAX);
-if (offset == -1) {
-hbitmap_set(job->copy_bitmap, cluster, end - cluster);
-break;
-}
+while (bdrv_dirty_bitmap_next_dirty_area(job->sync_bitmap,
+ &offset, &bytes))
+{
+uint64_t cluster = offset / job->cluster_size;
+uint64_t last_cluster = (offset + bytes) / job->cluster_size;
 
-next_cluster = DIV_ROUND_UP(offset, job->cluster_size);
-hbitmap_set(job->copy_bitmap, cluster, next_cluster - cluster);
-if (next_cluster >= end) {
+hbitmap_set(job->copy_bitmap, cluster, last_cluster - cluster + 1);
+
+offset = (last_cluster + 1) * job->cluster_size;
+if (offset >= job->len) {
 break;
 }
-
-bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
+bytes = job->len - offset;
 }
 
 /* TODO job_progress_set_remaining() would make more sense */
 job_progress_update(&job->common.job,
 job->len - hbitmap_count(job->copy_bitmap) * job->cluster_size);
-
-bdrv_dirty_iter_free(dbi);
 }
 
 static int coroutine_fn backup_run(Job *job, Error **errp)
-- 
2.18.0

[Qemu-devel] [PATCH v5 04/11] iotests: handle -f argument correctly for qemu_io_silent

[Vladimir Sementsov-Ogievskiy]

Correctly rewrite default argument. After the patch, the function can
be used for other (not only default test-chosen) image format.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/iotests.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 9595429fea..76877ad584 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -139,7 +139,12 @@ def qemu_io(*args):
 
 def qemu_io_silent(*args):
 '''Run qemu-io and return the exit code, suppressing stdout'''
-args = qemu_io_args + list(args)
+if '-f' in qemu_io_args and '-f' in args:
+ind = qemu_io_args.index('-f')
+args = qemu_io_args[:ind] + qemu_io_args[ind+2:] + list(args)
+else:
+args = qemu_io_args + list(args)
+
 exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'))
 if exitcode < 0:
 sys.stderr.write('qemu-io received signal %i: %s\n' %
-- 
2.18.0

[Qemu-devel] [PATCH v5 03/11] block: improve should_update_child

[Vladimir Sementsov-Ogievskiy]

As it already said in the comment, we don't want to create loops in
parent->child relations. So, when we try to append @to to @c, we should
check that @c is not in @to children subtree, and we should check it
recursively, not only the first level. The patch provides BFS-based
search, to check the relations.

This is needed for further fleecing-hook filter usage: we need to
append it to source, when the hook is already a parent of target, and
source may be in a backing chain of target (fleecing-scheme). So, on
appending, the hook should not became a child (direct or through
children subtree) of the target.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block.c | 33 -
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 4f5ff2cc12..8f04f293da 100644
--- a/block.c
+++ b/block.c
@@ -3536,7 +3536,7 @@ void bdrv_close_all(void)
 
 static bool should_update_child(BdrvChild *c, BlockDriverState *to)
 {
-BdrvChild *to_c;
+GList *queue = NULL, *pos;
 
 if (c->role->stay_at_node) {
 return false;
@@ -3572,13 +3572,36 @@ static bool should_update_child(BdrvChild *c, 
BlockDriverState *to)
  * if A is a child of B, that means we cannot replace A by B there
  * because that would create a loop.  Silently detaching A from B
  * is also not really an option.  So overall just leaving A in
- * place there is the most sensible choice. */
-QLIST_FOREACH(to_c, &to->children, next) {
-if (to_c == c) {
-return false;
+ * place there is the most sensible choice.
+ *
+ * We would also create a loop in any cases where @c is only
+ * indirectly referenced by @to. Prevent this by returning false
+ * if @c is found (by breadth-first search) anywhere in the whole
+ * subtree of @to.
+ */
+
+pos = queue = g_list_append(queue, to);
+while (pos) {
+BlockDriverState *v = pos->data;
+BdrvChild *c2;
+
+QLIST_FOREACH(c2, &v->children, next) {
+if (c2 == c) {
+g_list_free(queue);
+return false;
+}
+
+if (g_list_find(queue, c2->bs)) {
+continue;
+}
+
+queue = g_list_append(queue, c2->bs);
 }
+
+pos = pos->next;
 }
 
+g_list_free(queue);
 return true;
 }
 
-- 
2.18.0

[Qemu-devel] [PATCH v5 05/11] iotests: allow resume_drive by node name

[Vladimir Sementsov-Ogievskiy]

After node graph changes, we may not be able to resume_drive by device
name (backing files are not recursively searched). So, lets allow to
resume by node-name. Set constant name for breakpoints, to avoid
introducing extra parameters.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/iotests.py | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 76877ad584..c9779f432f 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -415,11 +415,11 @@ class VM(qtest.QEMUQtestMachine):
 self.pause_drive(drive, "write_aio")
 return
 self.qmp('human-monitor-command',
-command_line='qemu-io %s "break %s bp_%s"' % (drive, 
event, drive))
+command_line='qemu-io %s "break %s bp_0"' % (drive, event))
 
 def resume_drive(self, drive):
 self.qmp('human-monitor-command',
-command_line='qemu-io %s "remove_break bp_%s"' % (drive, 
drive))
+command_line='qemu-io %s "remove_break bp_0"' % (drive))
 
 def hmp_qemu_io(self, drive, cmd):
 '''Write to a given drive using an HMP command'''
@@ -543,13 +543,14 @@ class QMPTestCase(unittest.TestCase):
 
self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),
  self.vm.flatten_qmp_object(reference))
 
-def cancel_and_wait(self, drive='drive0', force=False, resume=False):
+def cancel_and_wait(self, drive='drive0', force=False, resume=False,
+resume_node=None):
 '''Cancel a block job and wait for it to finish, returning the event'''
 result = self.vm.qmp('block-job-cancel', device=drive, force=force)
 self.assert_qmp(result, 'return', {})
 
 if resume:
-self.vm.resume_drive(drive)
+self.vm.resume_drive(resume_node or drive)
 
 cancelled = False
 result = None
-- 
2.18.0

[Qemu-devel] [PATCH v5 08/11] block/io: refactor wait_serialising_requests

[Vladimir Sementsov-Ogievskiy]

Split out do_wait_serialising_requests with additional possibility to
not actually wait but just check, that there is something to wait for.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/io.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/block/io.c b/block/io.c
index bd9d688f8b..b87c11a6ec 100644
--- a/block/io.c
+++ b/block/io.c
@@ -720,12 +720,13 @@ void bdrv_dec_in_flight(BlockDriverState *bs)
 bdrv_wakeup(bs);
 }
 
-static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
+static bool coroutine_fn do_wait_serialising_requests(BdrvTrackedRequest *self,
+  bool wait)
 {
 BlockDriverState *bs = self->bs;
 BdrvTrackedRequest *req;
 bool retry;
-bool waited = false;
+bool found = false;
 
 if (!atomic_read(&bs->serialising_in_flight)) {
 return false;
@@ -751,11 +752,13 @@ static bool coroutine_fn 
wait_serialising_requests(BdrvTrackedRequest *self)
  * will wait for us as soon as it wakes up, then just go on
  * (instead of producing a deadlock in the former case). */
 if (!req->waiting_for) {
-self->waiting_for = req;
-qemu_co_queue_wait(&req->wait_queue, &bs->reqs_lock);
-self->waiting_for = NULL;
-retry = true;
-waited = true;
+found = true;
+if (wait) {
+self->waiting_for = req;
+qemu_co_queue_wait(&req->wait_queue, &bs->reqs_lock);
+self->waiting_for = NULL;
+retry = true;
+}
 break;
 }
 }
@@ -763,7 +766,12 @@ static bool coroutine_fn 
wait_serialising_requests(BdrvTrackedRequest *self)
 qemu_co_mutex_unlock(&bs->reqs_lock);
 } while (retry);
 
-return waited;
+return found;
+}
+
+static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
+{
+return do_wait_serialising_requests(self, true);
 }
 
 static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
-- 
2.18.0

[Qemu-devel] [PATCH v5 09/11] block: add lock/unlock range functions

[Vladimir Sementsov-Ogievskiy]

From: Vladimir Sementsov-Ogievskiy 

Introduce lock/unlock range functionality, based on serialized
requests. This is needed to refactor backup, dropping local
tracked-request-like synchronization.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/block_int.h |  4 
 block/io.c| 45 ++-
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index f605622216..3475ee5360 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -64,6 +64,7 @@ enum BdrvTrackedRequestType {
 BDRV_TRACKED_WRITE,
 BDRV_TRACKED_DISCARD,
 BDRV_TRACKED_TRUNCATE,
+BDRV_TRACKED_LOCK,
 };
 
 typedef struct BdrvTrackedRequest {
@@ -856,6 +857,9 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
 int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
 int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags);
+void *coroutine_fn bdrv_co_try_lock(BdrvChild *child,
+int64_t offset, unsigned int bytes);
+void coroutine_fn bdrv_co_unlock(void *opaque);
 
 extern unsigned int bdrv_drain_all_count;
 void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_parent);
diff --git a/block/io.c b/block/io.c
index b87c11a6ec..38a63d16f1 100644
--- a/block/io.c
+++ b/block/io.c
@@ -720,6 +720,15 @@ void bdrv_dec_in_flight(BlockDriverState *bs)
 bdrv_wakeup(bs);
 }
 
+static bool ignore_intersection(BdrvTrackedRequest *a, BdrvTrackedRequest *b)
+{
+return a == b || (!a->serialising && !b->serialising) ||
+(a->type == BDRV_TRACKED_LOCK && b->type == BDRV_TRACKED_READ &&
+ !b->serialising) ||
+(b->type == BDRV_TRACKED_LOCK && a->type == BDRV_TRACKED_READ &&
+ !a->serialising);
+}
+
 static bool coroutine_fn do_wait_serialising_requests(BdrvTrackedRequest *self,
   bool wait)
 {
@@ -736,7 +745,7 @@ static bool coroutine_fn 
do_wait_serialising_requests(BdrvTrackedRequest *self,
 retry = false;
 qemu_co_mutex_lock(&bs->reqs_lock);
 QLIST_FOREACH(req, &bs->tracked_requests, list) {
-if (req == self || (!req->serialising && !self->serialising)) {
+if (ignore_intersection(self, req)) {
 continue;
 }
 if (tracked_request_overlaps(req, self->overlap_offset,
@@ -774,6 +783,12 @@ static bool coroutine_fn 
wait_serialising_requests(BdrvTrackedRequest *self)
 return do_wait_serialising_requests(self, true);
 }
 
+static bool coroutine_fn should_wait_serialising_requests(
+BdrvTrackedRequest *self)
+{
+return do_wait_serialising_requests(self, false);
+}
+
 static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
size_t size)
 {
@@ -3250,3 +3265,31 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
PreallocMode prealloc,
 
 return tco.ret;
 }
+
+void *coroutine_fn bdrv_co_try_lock(BdrvChild *child,
+int64_t offset, unsigned int bytes)
+{
+BlockDriverState *bs = child->bs;
+BdrvTrackedRequest *req = g_new(BdrvTrackedRequest, 1);
+
+tracked_request_begin(req, bs, offset, bytes, BDRV_TRACKED_LOCK);
+mark_request_serialising(req, bdrv_get_cluster_size(bs));
+
+if (should_wait_serialising_requests(req)) {
+tracked_request_end(req);
+g_free(req);
+return NULL;
+}
+
+return req;
+}
+
+void coroutine_fn bdrv_co_unlock(void *opaque)
+{
+BdrvTrackedRequest *req = opaque;
+
+assert(req->type == BDRV_TRACKED_LOCK);
+
+tracked_request_end(req);
+g_free(req);
+}
-- 
2.18.0

[Qemu-devel] [PATCH v5 11/11] block/backup: use backup-top instead of write notifiers

[Vladimir Sementsov-Ogievskiy]

Drop write notifiers and use filter node instead. Changes:

1. copy-before-writes now handled by filter node, so, drop all
   is_write_notifier arguments.

2. we don't have intersecting requests, so their handling is dropped.
Instead, synchronization works as follows:
when backup or backup-top starts copying of some area it firstly
clears copy-bitmap bits, and nobody touches areas, not marked with
dirty bits in copy-bitmap, so there is no intersection. Also, backup
job copy operations are surrounded by bdrv region lock, which is
actually serializing request, to not interfer with guest writes and
not read changed data from source (before reading we clear
corresponding bit in copy-bitmap, so, this area is not more handled by
backup-top).

3. To sync with in-flight requests we now just drain hook node, we
don't need rw-lock.

4. After the whole backup loop (top, full, incremental modes), we need
to check for not copied clusters, which were under backup-top operation
and we skipped them, but backup-top operation failed, error returned to
the guest and dirty bits set back.

5. Don't create additional blk, use backup-top children for copy
operations.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/backup.c | 285 ++---
 1 file changed, 149 insertions(+), 136 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 88c0242b4e..e332909fb7 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -26,105 +26,61 @@
 #include "qemu/bitmap.h"
 #include "qemu/error-report.h"
 
-#define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16)
+#include "block/backup-top.h"
 
-typedef struct CowRequest {
-int64_t start_byte;
-int64_t end_byte;
-QLIST_ENTRY(CowRequest) list;
-CoQueue wait_queue; /* coroutines blocked on this request */
-} CowRequest;
+#define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16)
 
 typedef struct BackupBlockJob {
 BlockJob common;
-BlockBackend *target;
+BdrvChild *source;
+BdrvChild *target;
 /* bitmap for sync=incremental */
 BdrvDirtyBitmap *sync_bitmap;
 MirrorSyncMode sync_mode;
 BlockdevOnError on_source_error;
 BlockdevOnError on_target_error;
-CoRwlock flush_rwlock;
 uint64_t len;
 uint64_t bytes_read;
 int64_t cluster_size;
 bool compress;
-NotifierWithReturn before_write;
-QLIST_HEAD(, CowRequest) inflight_reqs;
 
 HBitmap *copy_bitmap;
 bool use_copy_range;
 int64_t copy_range_size;
 
 bool serialize_target_writes;
+
+BlockDriverState *backup_top;
+uint64_t backup_top_progress;
 } BackupBlockJob;
 
 static const BlockJobDriver backup_job_driver;
 
-/* See if in-flight requests overlap and wait for them to complete */
-static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job,
-   int64_t start,
-   int64_t end)
-{
-CowRequest *req;
-bool retry;
-
-do {
-retry = false;
-QLIST_FOREACH(req, &job->inflight_reqs, list) {
-if (end > req->start_byte && start < req->end_byte) {
-qemu_co_queue_wait(&req->wait_queue, NULL);
-retry = true;
-break;
-}
-}
-} while (retry);
-}
-
-/* Keep track of an in-flight request */
-static void cow_request_begin(CowRequest *req, BackupBlockJob *job,
-  int64_t start, int64_t end)
-{
-req->start_byte = start;
-req->end_byte = end;
-qemu_co_queue_init(&req->wait_queue);
-QLIST_INSERT_HEAD(&job->inflight_reqs, req, list);
-}
-
-/* Forget about a completed request */
-static void cow_request_end(CowRequest *req)
-{
-QLIST_REMOVE(req, list);
-qemu_co_queue_restart_all(&req->wait_queue);
-}
-
 /* Copy range to target with a bounce buffer and return the bytes copied. If
  * error occurred, return a negative error number */
 static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
   int64_t start,
   int64_t end,
-  bool is_write_notifier,
   bool *error_is_read,
   void **bounce_buffer)
 {
 int ret;
 struct iovec iov;
 QEMUIOVector qiov;
-BlockBackend *blk = job->common.blk;
 int nbytes;
-int read_flags = is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0;
 int write_flags = job->serialize_target_writes ? BDRV_REQ_SERIALISING : 0;
 
 assert(QEMU_IS_ALIGNED(start, job->cluster_size));
 hbitmap_reset(job->copy_bitmap, start, job->cluster_size);
 nbytes = MIN(job->cluster_size, job->len - start);
 if (!*bounce_buffer) {
-*bounce_buffer = blk_blockalign(blk, job->cluster_size);
+*bounce_buffer = qemu_blockalign(job->source->bs, job->c

[Qemu-devel] [PATCH v5 06/11] iotests: prepare 055 to graph changes during backup job

[Vladimir Sementsov-Ogievskiy]

Backup will append fleecing-hook node above source node, so, we can't
resume by device name (because resume don't search recursively through
backing chain).

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/055 | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index 3437c11507..be5451e1c5 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -48,7 +48,8 @@ class TestSingleDrive(iotests.QMPTestCase):
 def setUp(self):
 qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(image_len))
 
-self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
+self.vm = iotests.VM().add_drive('blkdebug::' + test_img,
+ 'node-name=source')
 self.vm.add_drive(blockdev_target_img, interface="none")
 if iotests.qemu_default_machine == 'pc':
 self.vm.add_drive(None, 'media=cdrom', 'ide')
@@ -69,7 +70,7 @@ class TestSingleDrive(iotests.QMPTestCase):
 result = self.vm.qmp(cmd, device='drive0', target=target, sync='full')
 self.assert_qmp(result, 'return', {})
 
-event = self.cancel_and_wait(resume=True)
+event = self.cancel_and_wait(resume=True, resume_node='source')
 self.assert_qmp(event, 'data/type', 'backup')
 
 def test_cancel_drive_backup(self):
@@ -87,7 +88,7 @@ class TestSingleDrive(iotests.QMPTestCase):
 self.assert_qmp(result, 'return', {})
 
 self.pause_job('drive0', wait=False)
-self.vm.resume_drive('drive0')
+self.vm.resume_drive('source')
 self.pause_wait('drive0')
 
 result = self.vm.qmp('query-block-jobs')
@@ -165,7 +166,8 @@ class TestSetSpeed(iotests.QMPTestCase):
 def setUp(self):
 qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(image_len))
 
-self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
+self.vm = iotests.VM().add_drive('blkdebug::' + test_img,
+ 'node-name=source')
 self.vm.add_drive(blockdev_target_img, interface="none")
 self.vm.launch()
 
@@ -197,7 +199,7 @@ class TestSetSpeed(iotests.QMPTestCase):
 self.assert_qmp(result, 'return[0]/device', 'drive0')
 self.assert_qmp(result, 'return[0]/speed', 8 * 1024 * 1024)
 
-event = self.cancel_and_wait(resume=True)
+event = self.cancel_and_wait(resume=True, resume_node='source')
 self.assert_qmp(event, 'data/type', 'backup')
 
 # Check setting speed option works
@@ -210,7 +212,7 @@ class TestSetSpeed(iotests.QMPTestCase):
 self.assert_qmp(result, 'return[0]/device', 'drive0')
 self.assert_qmp(result, 'return[0]/speed', 4 * 1024 * 1024)
 
-event = self.cancel_and_wait(resume=True)
+event = self.cancel_and_wait(resume=True, resume_node='source')
 self.assert_qmp(event, 'data/type', 'backup')
 
 def test_set_speed_drive_backup(self):
@@ -236,7 +238,7 @@ class TestSetSpeed(iotests.QMPTestCase):
 result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
 self.assert_qmp(result, 'error/class', 'GenericError')
 
-event = self.cancel_and_wait(resume=True)
+event = self.cancel_and_wait(resume=True, resume_node='source')
 self.assert_qmp(event, 'data/type', 'backup')
 
 def test_set_speed_invalid_drive_backup(self):
@@ -464,7 +466,8 @@ class TestDriveCompression(iotests.QMPTestCase):
 pass
 
 def do_prepare_drives(self, fmt, args, attach_target):
-self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
+self.vm = iotests.VM().add_drive('blkdebug::' + test_img,
+ 'node-name=source')
 
 qemu_img('create', '-f', fmt, blockdev_target_img,
  str(TestDriveCompression.image_len), *args)
@@ -507,7 +510,7 @@ class TestDriveCompression(iotests.QMPTestCase):
 result = self.vm.qmp(cmd, device='drive0', sync='full', compress=True, 
**args)
 self.assert_qmp(result, 'return', {})
 
-event = self.cancel_and_wait(resume=True)
+event = self.cancel_and_wait(resume=True, resume_node='source')
 self.assert_qmp(event, 'data/type', 'backup')
 
 self.vm.shutdown()
@@ -532,7 +535,7 @@ class TestDriveCompression(iotests.QMPTestCase):
 self.assert_qmp(result, 'return', {})
 
 self.pause_job('drive0', wait=False)
-self.vm.resume_drive('drive0')
+self.vm.resume_drive('source')
 self.pause_wait('drive0')
 
 result = self.vm.qmp('query-block-jobs')
-- 
2.18.0

[Qemu-devel] [PATCH v5 02/11] block/backup: move to copy_bitmap with granularity

[Vladimir Sementsov-Ogievskiy]

We are going to share this bitmap between backup and backup-top filter
driver, so let's share something more meaningful. It also simplifies
some calculations.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/backup.c | 48 +++-
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index fbe7ce19e1..0b3fddeb6c 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -114,7 +114,8 @@ static int coroutine_fn 
backup_cow_with_bounce_buffer(BackupBlockJob *job,
 int read_flags = is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0;
 int write_flags = job->serialize_target_writes ? BDRV_REQ_SERIALISING : 0;
 
-hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
+assert(QEMU_IS_ALIGNED(start, job->cluster_size));
+hbitmap_reset(job->copy_bitmap, start, job->cluster_size);
 nbytes = MIN(job->cluster_size, job->len - start);
 if (!*bounce_buffer) {
 *bounce_buffer = blk_blockalign(blk, job->cluster_size);
@@ -150,7 +151,7 @@ static int coroutine_fn 
backup_cow_with_bounce_buffer(BackupBlockJob *job,
 
 return nbytes;
 fail:
-hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
+hbitmap_set(job->copy_bitmap, start, job->cluster_size);
 return ret;
 
 }
@@ -170,16 +171,15 @@ static int coroutine_fn 
backup_cow_with_offload(BackupBlockJob *job,
 int write_flags = job->serialize_target_writes ? BDRV_REQ_SERIALISING : 0;
 
 assert(QEMU_IS_ALIGNED(job->copy_range_size, job->cluster_size));
+assert(QEMU_IS_ALIGNED(start, job->cluster_size));
 nbytes = MIN(job->copy_range_size, end - start);
 nr_clusters = DIV_ROUND_UP(nbytes, job->cluster_size);
-hbitmap_reset(job->copy_bitmap, start / job->cluster_size,
-  nr_clusters);
+hbitmap_reset(job->copy_bitmap, start, job->cluster_size * nr_clusters);
 ret = blk_co_copy_range(blk, start, job->target, start, nbytes,
 read_flags, write_flags);
 if (ret < 0) {
 trace_backup_do_cow_copy_range_fail(job, start, ret);
-hbitmap_set(job->copy_bitmap, start / job->cluster_size,
-nr_clusters);
+hbitmap_set(job->copy_bitmap, start, job->cluster_size * nr_clusters);
 return ret;
 }
 
@@ -207,7 +207,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
 cow_request_begin(&cow_request, job, start, end);
 
 while (start < end) {
-if (!hbitmap_get(job->copy_bitmap, start / job->cluster_size)) {
+if (!hbitmap_get(job->copy_bitmap, start)) {
 trace_backup_do_cow_skip(job, start);
 start += job->cluster_size;
 continue; /* already copied */
@@ -303,6 +303,11 @@ static void backup_clean(Job *job)
 assert(s->target);
 blk_unref(s->target);
 s->target = NULL;
+
+if (s->copy_bitmap) {
+hbitmap_free(s->copy_bitmap);
+s->copy_bitmap = NULL;
+}
 }
 
 static void backup_attached_aio_context(BlockJob *job, AioContext *aio_context)
@@ -315,7 +320,6 @@ static void backup_attached_aio_context(BlockJob *job, 
AioContext *aio_context)
 void backup_do_checkpoint(BlockJob *job, Error **errp)
 {
 BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
-int64_t len;
 
 assert(block_job_driver(job) == &backup_job_driver);
 
@@ -325,8 +329,7 @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
 return;
 }
 
-len = DIV_ROUND_UP(backup_job->len, backup_job->cluster_size);
-hbitmap_set(backup_job->copy_bitmap, 0, len);
+hbitmap_set(backup_job->copy_bitmap, 0, backup_job->len);
 }
 
 static void backup_drain(BlockJob *job)
@@ -381,16 +384,16 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
 {
 int ret;
 bool error_is_read;
-int64_t cluster;
+int64_t offset;
 HBitmapIter hbi;
 
 hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
-while ((cluster = hbitmap_iter_next(&hbi)) != -1) {
+while ((offset = hbitmap_iter_next(&hbi)) != -1) {
 do {
 if (yield_and_check(job)) {
 return 0;
 }
-ret = backup_do_cow(job, cluster * job->cluster_size,
+ret = backup_do_cow(job, offset,
 job->cluster_size, &error_is_read, false);
 if (ret < 0 && backup_error_action(job, error_is_read, -ret) ==
BLOCK_ERROR_ACTION_REPORT)
@@ -412,12 +415,9 @@ static void 
backup_incremental_init_copy_bitmap(BackupBlockJob *job)
 while (bdrv_dirty_bitmap_next_dirty_area(job->sync_bitmap,
  &offset, &bytes))
 {
-uint64_t cluster = offset / job->cluster_size;
-uint64_t last_cluster = (offset + bytes) / job->cluster_size;
+hbitmap_set(job->copy_bitmap, offset, bytes);
 
-hbitmap_set(job->copy_bitmap, cluster, last_cluster - cluster + 1);
-
-   

[Qemu-devel] [PATCH v5 10/11] block/backup: tiny refactor backup_job_create

[Vladimir Sementsov-Ogievskiy]

Move copy-bitmap find/create code. It's needed for the following
commit, as we'll need copy_bitmap before actual block job creation. Do
it in a separate commit to simplify review.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/backup.c | 69 +-
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 0b3fddeb6c..88c0242b4e 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -561,6 +561,8 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 BlockDriverInfo bdi;
 BackupBlockJob *job = NULL;
 int ret;
+int64_t cluster_size;
+HBitmap *copy_bitmap = NULL;
 
 assert(bs);
 assert(target);
@@ -615,6 +617,35 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 return NULL;
 }
 
+/*
+ * If there is no backing file on the target, we cannot rely on COW if our
+ * backup cluster size is smaller than the target cluster size. Even for
+ * targets with a backing file, try to avoid COW if possible.
+ */
+ret = bdrv_get_info(target, &bdi);
+if (ret == -ENOTSUP && !target->backing) {
+/* Cluster size is not defined */
+warn_report("The target block device doesn't provide "
+"information about the block size and it doesn't have a "
+"backing file. The default block size of %u bytes is "
+"used. If the actual block size of the target exceeds "
+"this default, the backup may be unusable",
+BACKUP_CLUSTER_SIZE_DEFAULT);
+cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
+} else if (ret < 0 && !target->backing) {
+error_setg_errno(errp, -ret,
+"Couldn't determine the cluster size of the target image, "
+"which has no backing file");
+error_append_hint(errp,
+"Aborting, since this may create an unusable destination image\n");
+return NULL;
+} else if (ret < 0 && target->backing) {
+/* Not fatal; just trudge on ahead. */
+cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
+} else {
+cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+}
+
 len = bdrv_getlength(bs);
 if (len < 0) {
 error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -622,6 +653,8 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 goto error;
 }
 
+copy_bitmap = hbitmap_alloc(len, ctz32(cluster_size));
+
 /* job->len is fixed, so we can't allow resize */
 job = block_job_create(job_id, &backup_job_driver, txn, bs,
BLK_PERM_CONSISTENT_READ,
@@ -650,35 +683,9 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 
 /* Detect image-fleecing (and similar) schemes */
 job->serialize_target_writes = bdrv_chain_contains(target, bs);
-
-/* If there is no backing file on the target, we cannot rely on COW if our
- * backup cluster size is smaller than the target cluster size. Even for
- * targets with a backing file, try to avoid COW if possible. */
-ret = bdrv_get_info(target, &bdi);
-if (ret == -ENOTSUP && !target->backing) {
-/* Cluster size is not defined */
-warn_report("The target block device doesn't provide "
-"information about the block size and it doesn't have a "
-"backing file. The default block size of %u bytes is "
-"used. If the actual block size of the target exceeds "
-"this default, the backup may be unusable",
-BACKUP_CLUSTER_SIZE_DEFAULT);
-job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
-} else if (ret < 0 && !target->backing) {
-error_setg_errno(errp, -ret,
-"Couldn't determine the cluster size of the target image, "
-"which has no backing file");
-error_append_hint(errp,
-"Aborting, since this may create an unusable destination image\n");
-goto error;
-} else if (ret < 0 && target->backing) {
-/* Not fatal; just trudge on ahead. */
-job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
-} else {
-job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
-}
-
-job->copy_bitmap = hbitmap_alloc(len, ctz32(job->cluster_size));
+job->cluster_size = cluster_size;
+job->copy_bitmap = copy_bitmap;
+copy_bitmap = NULL;
 job->use_copy_range = true;
 job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
 blk_get_max_transfer(job->target));
@@ -694,6 +701,10 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 return &job->common;
 
  error:
+if (copy_bitmap) {
+assert(!job || !job->copy_bitmap);
+hbitmap_free(copy_bitmap);

[Qemu-devel] [PATCH v5 00/11] backup-top filter driver for backup

[Vladimir Sementsov-Ogievskiy]

Hi all!

These series introduce backup-top driver. It's a filter-node, which
do copy-before-write operation. Mirror uses filter-node for handling
guest writes, let's move to filter-node (from write-notifiers) for
backup too

v5:

Now, based on John's bitmaps branch

patch "block: allow serialized reads to intersect" removed
03: reword comment (Kevin)
07: fix comments style
08: new
09: mostly rewritten
10: fix comment style
11: - fix comments style
- fix uninitialized backup_top
- fix return -> goto error if failed to
  append filter

v4:
fixes, rewrite driver to be implicit, drop new interfaces and
don't move to BdrvDirtyBitmap for now, as it's not obvious will
it be really needed and don't relate to these series more.

v3 was "[PATCH v3 00/18] fleecing-hook driver for backup"

v2 was "[RFC v2] new, node-graph-based fleecing and backup"



These series (v5) are based on John's bitmaps branch

Based-on: https://github.com/jnsnow/qemu bitmaps

Vladimir Sementsov-Ogievskiy (11):
  block/backup: simplify backup_incremental_init_copy_bitmap
  block/backup: move to copy_bitmap with granularity
  block: improve should_update_child
  iotests: handle -f argument correctly for qemu_io_silent
  iotests: allow resume_drive by node name
  iotests: prepare 055 to graph changes during backup job
  block: introduce backup-top filter driver
  block/io: refactor wait_serialising_requests
  block: add lock/unlock range functions
  block/backup: tiny refactor backup_job_create
  block/backup: use backup-top instead of write notifiers

 block/backup-top.h|  43 
 include/block/block_int.h |   4 +
 block.c   |  33 ++-
 block/backup-top.c| 306 
 block/backup.c| 424 +-
 block/io.c|  69 +-
 block/Makefile.objs   |   2 +
 tests/qemu-iotests/055|  23 +-
 tests/qemu-iotests/iotests.py |  16 +-
 9 files changed, 682 insertions(+), 238 deletions(-)
 create mode 100644 block/backup-top.h
 create mode 100644 block/backup-top.c

-- 
2.18.0

[Qemu-devel] [PATCH v5 07/11] block: introduce backup-top filter driver

[Vladimir Sementsov-Ogievskiy]

Backup-top filter does copy-before-write operation. It should be
inserted above active disk and has a target node for CBW, like the
following:

+---+
| Guest |
+---+---+
|r,w
v
+---+---+  target   +---+
| backup_top|-->| target(qcow2) |
+---+---+   CBW +---+---+
|
backing |r,w
v
+---+-+
| Active disk |
+-+

The driver will be used in backup instead of write-notifiers.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/backup-top.h  |  43 +++
 block/backup-top.c  | 306 
 block/Makefile.objs |   2 +
 3 files changed, 351 insertions(+)
 create mode 100644 block/backup-top.h
 create mode 100644 block/backup-top.c

diff --git a/block/backup-top.h b/block/backup-top.h
new file mode 100644
index 00..abe4dda574
--- /dev/null
+++ b/block/backup-top.h
@@ -0,0 +1,43 @@
+/*
+ * backup-top filter driver
+ *
+ * The driver performs Copy-Before-Write (CBW) operation: it is injected above
+ * some node, and before each write it copies _old_ data to the target node.
+ *
+ * Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved.
+ *
+ * Author:
+ *  Sementsov-Ogievskiy Vladimir 
+ *
+ * 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 .
+ */
+
+#include "qemu/osdep.h"
+
+#include "block/block_int.h"
+
+typedef struct BDRVBackupTopState {
+HBitmap *copy_bitmap; /* what should be copied to @target on guest write. 
*/
+BdrvChild *target;
+
+uint64_t bytes_copied;
+} BDRVBackupTopState;
+
+void bdrv_backup_top_drop(BlockDriverState *bs);
+uint64_t bdrv_backup_top_progress(BlockDriverState *bs);
+
+BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+ BlockDriverState *target,
+ HBitmap *copy_bitmap,
+ Error **errp);
diff --git a/block/backup-top.c b/block/backup-top.c
new file mode 100644
index 00..0e7b3e3142
--- /dev/null
+++ b/block/backup-top.c
@@ -0,0 +1,306 @@
+/*
+ * backup-top filter driver
+ *
+ * The driver performs Copy-Before-Write (CBW) operation: it is injected above
+ * some node, and before each write it copies _old_ data to the target node.
+ *
+ * Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved.
+ *
+ * Author:
+ *  Sementsov-Ogievskiy Vladimir 
+ *
+ * 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 .
+ */
+
+#include "qemu/osdep.h"
+
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "block/block_int.h"
+#include "block/qdict.h"
+
+#include "block/backup-top.h"
+
+static coroutine_fn int backup_top_co_preadv(
+BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, int flags)
+{
+/*
+ * Features to be implemented:
+ * F1. COR. save read data to fleecing target for fast access
+ * (to reduce reads). This possibly may be done with use of 
copy-on-read
+ * filter, but we need an ability to make COR requests optional: for
+ * example, if target is a ram-cache, and if it is full now, we should
+ * skip doing COR request, as it is actually not necessary.
+ *
+ * F2. Feature for guest: read from fleecing target if data is in ram-cache
+ * and is unchanged
+ */
+
+return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
+}
+
+static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
+  uint64_t bytes)
+{
+int ret = 0;
+BDRVBackupTopState *s = bs->opaque;
+uint64_t gran = 1UL << hbitmap_granularity(s->copy_bitmap);
+uint64_t end = QEMU_ALIGN_UP(offset + 

[Qemu-devel] ping3 Re: [PATCH v4 00/10] NBD reconnect

[Vladimir Sementsov-Ogievskiy]

ping

31.07.2018 20:30, Vladimir Sementsov-Ogievskiy wrote:
> Hi all.
> 
> Here is NBD reconnect. Previously, if connection failed all current
> and future requests will fail. After the series, nbd-client driver
> will try to reconnect unlimited times. During first @reconnect-delay
> seconds of reconnecting all requests will wait for the connection,
> and if it is established requests will be resent. After
> @reconnect-delay period all requests will be failed (until successful
> reconnect).
> 
> v4: - add Eric's r-b to 01.
>  - drop CONNECTING_INIT mode, don't reconnect on _open.
>  - new api: only one parameter @reconnect-delay
>  - new interval scheme between reconnect attempts
>  (1 - 2 - 4 - 8 - 16 - 16 ... seconds)
>  - fixes and refactorings in main patch (09), including merge with
>old 08 patch
>  
> 
> v3:
> 06: fix build error in function 'nbd_co_send_request':
>   error: 'i' may be used uninitialized in this function
> 
> v2 notes:
> Here is v2 of NBD reconnect, but it is very very different from v1, so,
> forget about v1.
> The series includes my "NBD reconnect: preliminary refactoring", with
> changes in 05: leave asserts (Eric).
> 
> Vladimir Sementsov-Ogievskiy (10):
>block/nbd-client: split channel errors from export errors
>block/nbd: move connection code from block/nbd to block/nbd-client
>block/nbd-client: split connection from initialization
>block/nbd-client: fix nbd_reply_chunk_iter_receive
>block/nbd-client: don't check ioc
>block/nbd-client: move from quit to state
>block/nbd-client: rename read_reply_co to connection_co
>block/nbd: add cmdline and qapi parameter reconnect-delay
>block/nbd-client: nbd reconnect
>iotests: test nbd reconnect
> 
>   qapi/block-core.json  |  12 +-
>   block/nbd-client.h|  20 +-
>   block/nbd-client.c| 515 
> +++---
>   block/nbd.c   |  56 ++---
>   tests/qemu-iotests/220|  67 ++
>   tests/qemu-iotests/220.out|   7 +
>   tests/qemu-iotests/group  |   1 +
>   tests/qemu-iotests/iotests.py |   4 +
>   8 files changed, 512 insertions(+), 170 deletions(-)
>   create mode 100755 tests/qemu-iotests/220
>   create mode 100644 tests/qemu-iotests/220.out
> 


-- 
Best regards,
Vladimir

[Qemu-devel] [PATCH] chardev: Allow for pty path passing.

[Paulo Neves]

If a user requires a virtual serial device like provided
by the pty char device, the user needs to accept the
returned device name. This makes the program need to
have smarts to parse or communicate with qemu to get the
pty device.
With this patch the program can pass the path where
a symlink to the pty device will be, removing the
need for 2 way communication or smarts.
---
 chardev/char-pty.c | 38 --
 chardev/char.c |  6 +-
 qapi/char.json |  4 ++--
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 761ae6d..b465263 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -38,6 +38,7 @@
 typedef struct {
 Chardev parent;
 QIOChannel *ioc;
+char *link_name;
 int read_bytes;
 
 /* Protected by the Chardev chr_write_lock.  */
@@ -231,6 +232,11 @@ static void char_pty_finalize(Object *obj)
 qemu_mutex_lock(&chr->chr_write_lock);
 pty_chr_state(chr, 0);
 object_unref(OBJECT(s->ioc));
+
+if (s->link_name) {
+unlink(s->link_name);
+}
+
 if (s->timer_tag) {
 g_source_remove(s->timer_tag);
 s->timer_tag = 0;
@@ -244,8 +250,9 @@ static void char_pty_open(Chardev *chr,
   bool *be_opened,
   Error **errp)
 {
+ChardevHostdev *opts = backend->u.pty.data;
 PtyChardev *s;
-int master_fd, slave_fd;
+int master_fd, slave_fd, symlink_ret;
 char pty_name[PATH_MAX];
 char *name;
 
@@ -256,13 +263,23 @@ static void char_pty_open(Chardev *chr,
 }
 
 close(slave_fd);
+
+s = PTY_CHARDEV(chr);
+s->link_name = opts->device;
+symlink_ret = symlink(pty_name, s->link_name);
+
+if (symlink_ret < 0) {
+close(master_fd);
+error_setg_errno(errp, errno, "Failed to create symlink to PTY");
+return;
+}
+
 qemu_set_nonblock(master_fd);
 
 chr->filename = g_strdup_printf("pty:%s", pty_name);
 error_report("char device redirected to %s (label %s)",
  pty_name, chr->label);
 
-s = PTY_CHARDEV(chr);
 s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));
 name = g_strdup_printf("chardev-pty-%s", chr->label);
 qio_channel_set_name(QIO_CHANNEL(s->ioc), name);
@@ -271,6 +288,22 @@ static void char_pty_open(Chardev *chr,
 *be_opened = false;
 }
 
+static void char_pty_parse(QemuOpts *opts, ChardevBackend *backend,
+Error **errp)
+{
+const char *symlink_path = qemu_opt_get(opts, "path");
+if(symlink_path == NULL) {
+error_setg(errp, "chardev: pty symlink: no device path given");
+return;
+
+}
+ChardevHostdev *dev;
+
+backend->type = CHARDEV_BACKEND_KIND_PTY;
+dev = backend->u.pipe.data = g_new0(ChardevHostdev, 1);
+qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(dev));
+dev->device = g_strdup(symlink_path);
+}
 static void char_pty_class_init(ObjectClass *oc, void *data)
 {
 ChardevClass *cc = CHARDEV_CLASS(oc);
@@ -279,6 +312,7 @@ static void char_pty_class_init(ObjectClass *oc, void *data)
 cc->chr_write = char_pty_chr_write;
 cc->chr_update_read_handler = pty_chr_update_read_handler;
 cc->chr_add_watch = pty_chr_add_watch;
+cc->parse = char_pty_parse;
 }
 
 static const TypeInfo char_pty_type_info = {
diff --git a/chardev/char.c b/chardev/char.c
index 5d52cd5..e4c5371 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -357,7 +357,6 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const 
char *filename)
 }
 
 if (strcmp(filename, "null")== 0 ||
-strcmp(filename, "pty") == 0 ||
 strcmp(filename, "msmouse") == 0 ||
 strcmp(filename, "wctablet") == 0 ||
 strcmp(filename, "braille") == 0 ||
@@ -402,6 +401,11 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const 
char *filename)
 qemu_opt_set(opts, "path", p, &error_abort);
 return opts;
 }
+if (strstart(filename, "pty:", &p)) {
+qemu_opt_set(opts, "backend", "pty", &error_abort);
+qemu_opt_set(opts, "path", p, &error_abort);
+return opts;
+}
 if (strstart(filename, "tcp:", &p) ||
 strstart(filename, "telnet:", &p) ||
 strstart(filename, "tn3270:", &p)) {
diff --git a/qapi/char.json b/qapi/char.json
index 6de0f29..dae4231 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -224,7 +224,7 @@
 ##
 # @ChardevHostdev:
 #
-# Configuration info for device and pipe chardevs.
+# Configuration info for device, pty and pipe chardevs.
 #
 # @device: The name of the special file for the device,
 #  i.e. /dev/ttyS0 on Unix or COM1: on Windows
@@ -380,7 +380,7 @@
'pipe'   : 'ChardevHostdev',
'socket' : 'ChardevSocket',
'udp': 'ChardevUdp',
-   'pty': 'ChardevCommon',
+

[Qemu-devel] [PATCH] chardev: Allow for pty path passing.

[Paulo Neves]

Hello all.
I am trying to get qemu to spawn a pty terminal in a location
which I control without needing to retrieve information of the PTY device
a-posteriori from qemu. To this end i inspired myself in the functionality
of socat, which allows PTY terminals to be spawned in a location
specified by a command line argument. The way socat does is a
bit cheating: It requests the PTY terminal the same way that qemu
does, through openpty, and creates a symlink between the path
specified by the user and the kernel given virtual terminal device.

In the following patch i was able to implement this functionality
successfully and it works as intended. The problem is that for some
reason when I send Ctrl-a-x through the pty the finalize method of
the pty char dev does not get called, thus not removing the symlink.
The finalize only is called when there is an error in the creation of
the symlink like for example when the symlink already exists.

This leads to a dangling symlink when qemu closes and the PTY is released.
Is there a better way to guarantee that my unlink of the symlink is
called when qemu closes? Am i not familiar with qemu code development
so if i am doing something terrible please let me know.

Re: [Qemu-devel] [PATCH v4] hw/arm: Add arm SBSA reference machine

[Hongbo Zhang]

well, checked it again, creating an generic xhci may take much efforts
and it is far beyond than my main task of creating an arm server
platform, so I'll create a generic system ehci, it satisfies our
requirements very well, will send it out separately.

On Wed, 21 Nov 2018 at 16:35, Hongbo Zhang  wrote:
>
> Well, considering all the comments/suggestions I received, I'd like to
> try to create a generic sysbus xhci.
> This would take some time for me, so my plan is to send out a new v5
> iteration of this sbsa-ref patch with updates according to Peter's
> comments, but without any xhci feature this time, and later I'll
> create the sysbus xhci with another patch set, including adding this
> new xhci to the sbsa-ref platform.
> Advantage of doing so: this won't make the sbsa-ref patch too
> complicated to review, and the xhci work won't block the process of
> sbsa-ref work.
>
> On Tue, 20 Nov 2018 at 17:40, Ard Biesheuvel  
> wrote:
> >
> > On Tue, 20 Nov 2018 at 09:49, Hongbo Zhang  wrote:
> > >
> > > On Tue, 20 Nov 2018 at 16:16, Gerd Hoffmann  wrote:
> > > >
> > > >   Hi,
> > > >
> > > > > > But I assume you specifically want things that do a lot of DMA, so I
> > > > > > couldn't get away with asking for keeping it to the SBSA UART(s)? :)
> > > > >
> > > > > Ideally, it would be something that supports platform MSIs as well,
> > > > > but that is unlikely to be a priority for anyone (and i would be very
> > > > > surprised if any of QEMU's sysbus host controllers implement that at
> > > > > the moment)
> > > > >
> > > > > I can live with just sysbus AHCI, though, if EHCI/XHCI are too 
> > > > > cumbersome.
> > > >
> > > > Generic sysbus ehci should not be difficuilt, 10 lines patch or so.
> > > >
> > > > Gives you only usb2 though.  usb1 devices are handled either via ohci
> > > > companion controller (should be doable), or via integrated usb2 hub
> > > > (more common on recent hardware, but qemu usb-hub is usb1 only).
> > > >
> > > > Generic sysbus xhci would be more work.  But you'll get usb 1+2+3
> > > > support.  And wiring up platform MSIs should be possible too.
> > > >
> > > In kernel, these drivers are enabled by default:
> > > CONFIG_USB_XHCI_PLATFORM=y
> > > CONFIG_USB_EHCI_HCD_PLATFORM=y
> > > CONFIG_USB_OHCI_HCD_PLATFORM=y
> > >
> > > If we do generic EHCI or XHCI in qemu, can the driver in kernel work
> > > with it? (I guess yes?)
> > > My thought is if the available kernel driver will work without any
> > > change, then we can create it in qemu.
> > >
> >
> > Yes that should work. Our MacchiatoBin DSDT has the following for its
> > XHCI controllers:
> >
> > Device (XHC0)
> > {
> > Name (_HID, "PNP0D10")  // _HID: Hardware ID
> > Name (_UID, 0x00)   // _UID: Unique ID
> > Name (_CCA, 0x01)   // _CCA: Cache Coherency Attribute
> >
> > Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
> > {
> > Memory32Fixed (ReadWrite,
> > 0xF250, // Address Base (MMIO)
> > 0x4000, // Address Length
> > )
> > Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
> > {
> >   CP_GIC_SPI_CP0_USB_H0
> > }
> > })
> > }
> >
> > and this gets picked up by the kernel without problems.
> >
> > Wiring up platform MSIs would involve modifying the IORT as well,
> > which is one of the reasons I'd prefer to make these fixed devices: we
> > cannot rely on qemu_fw_cfg to generate it for us, and adding a lot of
> > new code to generate this on the fly is simply not worth the effort.

[Qemu-devel] [PATCH] hw/usb: Add generic sys-bus EHCI controller

[Hongbo Zhang]

This patch introduces a new system bus generic EHCI controller.
For the system bus EHCI controller, we've already had "xlnx",
"exynos4210", "tegra2", "ppc4xx" and "fusbh200", they are specific and
only suitable for their own platforms, platforms such as an Arm server,
may need a generic system bus EHCI controller, this patch creates it,
and the kernel driver ehci_platform.c works well on it.

Signed-off-by: Hongbo Zhang 
---
 hw/usb/hcd-ehci-sysbus.c | 17 +
 hw/usb/hcd-ehci.h|  1 +
 2 files changed, 18 insertions(+)

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 3b83beb..331faf8 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -94,6 +94,22 @@ static const TypeInfo ehci_type_info = {
 .class_size= sizeof(SysBusEHCIClass),
 };
 
+static void ehci_platform_class_init(ObjectClass *oc, void *data)
+{
+SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
+DeviceClass *dc = DEVICE_CLASS(oc);
+
+sec->capsbase = 0x0;
+sec->opregbase = 0x20;
+set_bit(DEVICE_CATEGORY_USB, dc->categories);
+}
+
+static const TypeInfo ehci_platform_type_info = {
+.name  = TYPE_PLATFORM_EHCI,
+.parent= TYPE_SYS_BUS_EHCI,
+.class_init= ehci_platform_class_init,
+};
+
 static void ehci_xlnx_class_init(ObjectClass *oc, void *data)
 {
 SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
@@ -245,6 +261,7 @@ static const TypeInfo ehci_fusbh200_type_info = {
 static void ehci_sysbus_register_types(void)
 {
 type_register_static(&ehci_type_info);
+type_register_static(&ehci_platform_type_info);
 type_register_static(&ehci_xlnx_type_info);
 type_register_static(&ehci_exynos4210_type_info);
 type_register_static(&ehci_tegra2_type_info);
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 0bc364b..cd30b5d 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -342,6 +342,7 @@ typedef struct EHCIPCIState {
 
 
 #define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
+#define TYPE_PLATFORM_EHCI "platform-ehci-usb"
 #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
 #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
 #define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
-- 
2.7.4