Re: [Qemu-block] [PATCH for-2.7 1/1] ide: fix halted IO segfault at reset
On 27/07/2016 00:07, John Snow wrote:
> If one attempts to perform a system_reset after a failed IO request
> that causes the VM to enter a paused state, QEMU will segfault trying
> to free up the pending IO requests.
>
> These requests have already been completed and freed, though, so all
> we need to do is free them before we enter the paused state.
>
> Existing AHCI tests verify that halted requests are still resumed
> successfully after a STOP event.
>
> Signed-off-by: John Snow
> ---
> hw/ide/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 081c9eb..d117b7c 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -823,6 +823,7 @@ static void ide_dma_cb(void *opaque, int ret)
> }
> if (ret < 0) {
> if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
> +s->bus->dma->aiocb = NULL;
> return;
> }
> }
>
The patch is (was, since it's committed :)) okay, but I think there is
another bug in the REPORT case, where ide_rw_error and
ide_atapi_io_error are not calling ide_set_inactive and thus are leaving
s->bus->dma->aiocb non-NULL.
Paolo
Re: [Qemu-block] [PATCH 2/4] nbd: Limit nbdflags to 16 bits
On 21/07/2016 21:34, Eric Blake wrote:
> Furthermore, upstream NBD has never passed the global flags to
> the kernel via ioctl(NBD_SET_FLAGS) (the ioctl was first
> introduced in NBD 2.9.22; then a latent bug in NBD 3.1 actually
> tried to OR the global flags with the transmission flags, with
> the disaster that the addition of NBD_FLAG_NO_ZEROES in 3.9
> caused all earlier NBD 3.x clients to treat every export as
> read-only; NBD 3.10 and later intentionally clip things to 16
> bits to pass only transmission flags). Qemu should follow suit,
> since the current two global flags (NBD_FLAG_FIXED_NEWSTYLE
> and NBD_FLAG_NO_ZEROES) have no impact on the kernel's behavior
> during transmission.
Should squash this in too:
diff --git a/nbd/server.c b/nbd/server.c
index 80fbb4d..6fa2f9c 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -575,7 +575,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData
*data)
oldStyle = client->exp != NULL && !client->tlscreds;
if (oldStyle) {
-TRACE("advertising size %" PRIu64 " and flags %x",
+TRACE("advertising size %" PRIu64 " and flags %" PRIx16,
client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
stq_be_p(buf + 16, client->exp->size);
@@ -605,7 +605,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData
*data)
goto fail;
}
-TRACE("advertising size %" PRIu64 " and flags %x",
+TRACE("advertising size %" PRIu64 " and flags %" PRIx16,
client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 18, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
Re: [Qemu-block] [PATCH 3/4] osdep: Document differences in rounding macros
On 21/07/2016 21:34, Eric Blake wrote: > Make it obvious which macros are safe in which situations. > > Useful since QEMU_ALIGN_UP and ROUND_UP both purport to do > the same thing, but differ on whether the alignment must be > a power of 2. > > Signed-off-by: Eric Blake > --- > include/qemu/osdep.h | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index fbb8759..9991fb0 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -158,7 +158,8 @@ extern int daemon(int, int); > /* Round number down to multiple */ > #define QEMU_ALIGN_DOWN(n, m) ((n) / (m) * (m)) > > -/* Round number up to multiple */ > +/* Round number up to multiple. Safe when m is not a power of 2 (see > + * ROUND_UP for a faster version when a power of 2 is guaranteed) */ > #define QEMU_ALIGN_UP(n, m) QEMU_ALIGN_DOWN((n) + (m) - 1, (m)) > > /* Check if n is a multiple of m */ > @@ -175,6 +176,9 @@ extern int daemon(int, int); > /* Check if pointer p is n-bytes aligned */ > #define QEMU_PTR_IS_ALIGNED(p, n) QEMU_IS_ALIGNED((uintptr_t)(p), (n)) > > +/* Round number up to multiple. Requires that d be a power of 2 (see > + * QEMU_ALIGN_UP for a safer but slower version on arbitrary > + * numbers) */ > #ifndef ROUND_UP > #define ROUND_UP(n,d) (((n) + (d) - 1) & -(d)) > #endif Ouch, this is ugly, especially since DIV_ROUND_UP does not require alignment! Not your fault of course, and the patch is arguably an improvement. Paolo
Re: [Qemu-block] [PATCH 4/4] block: Cater to iscsi with non-power-of-2 discard
On 28/07/2016 04:39, Eric Blake wrote: > On 07/27/2016 01:25 AM, Fam Zheng wrote: >> On Thu, 07/21 13:34, Eric Blake wrote: >>> +max_write_zeroes = max_write_zeroes / alignment * alignment; >> >> Not using QEMU_ALIGN_DOWN despite patch 3? > > Looks like I missed that on the rebase. Can fix if there is a reason for > a respin. Since Stefan acked this, I'm applying the patch and fixing it to use QEMU_ALIGN_DOWN. Paolo
Re: [Qemu-block] [PATCH for-2.8] qtail: clean up direct access to tqe_prev field
On 25/07/2016 14:47, Igor Mammedov wrote:
> instead of accessing tqe_prev field dircetly outside
> of queue.h use macros to check if element is in list
> and make sure that afer element is removed from list
> tqe_prev field could be used to do the same check.
>
> Signed-off-by: Igor Mammedov
> ---
> The patch is split from as cleanup is not urgent
> [PATCH 0/6] Fix migration issues with arbitrary cpu-hot(un)plug
> and made on top of
> [PATCH v2 0/6] Fix migration issues with arbitrary cpu-hot(un)plug
>
> posting it to list so that it won't be forgotten or lost
> and affected people could review it at there leisure time.
>
> ---
> include/qemu/queue.h | 2 ++
> blockdev.c | 2 +-
> exec.c | 3 +--
> net/filter.c | 2 +-
> 4 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/qemu/queue.h b/include/qemu/queue.h
> index c2b6c81..342073f 100644
> --- a/include/qemu/queue.h
> +++ b/include/qemu/queue.h
> @@ -407,6 +407,7 @@ struct {
> \
> else\
> (head)->tqh_last = (elm)->field.tqe_prev; \
> *(elm)->field.tqe_prev = (elm)->field.tqe_next; \
> +(elm)->field.tqe_prev = NULL; \
> } while (/*CONSTCOND*/0)
>
> #define QTAILQ_FOREACH(var, head, field)\
> @@ -430,6 +431,7 @@ struct {
> \
> #define QTAILQ_EMPTY(head) ((head)->tqh_first == NULL)
> #define QTAILQ_FIRST(head) ((head)->tqh_first)
> #define QTAILQ_NEXT(elm, field) ((elm)->field.tqe_next)
> +#define QTAILQ_IN_USE(elm, field)((elm)->field.tqe_prev != NULL)
>
> #define QTAILQ_LAST(head, headname) \
> (*(((struct headname *)((head)->tqh_last))->tqh_last))
> diff --git a/blockdev.c b/blockdev.c
> index eafeba9..0b73158 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -4031,7 +4031,7 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
> goto out;
> }
>
> -if (!blk && !bs->monitor_list.tqe_prev) {
> +if (!blk && !QTAILQ_IN_USE(bs, monitor_list)) {
> error_setg(errp, "Node %s is not owned by the monitor",
> bs->node_name);
> goto out;
> diff --git a/exec.c b/exec.c
> index 50e3ee2..8e8416b 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -614,14 +614,13 @@ void cpu_exec_exit(CPUState *cpu)
> CPUClass *cc = CPU_GET_CLASS(cpu);
>
> cpu_list_lock();
> -if (cpu->node.tqe_prev == NULL) {
> +if (!QTAILQ_IN_USE(cpu, node)) {
> /* there is nothing to undo since cpu_exec_init() hasn't been called
> */
> cpu_list_unlock();
> return;
> }
>
> QTAILQ_REMOVE(&cpus, cpu, node);
> -cpu->node.tqe_prev = NULL;
> cpu->cpu_index = UNASSIGNED_CPU_INDEX;
> cpu_list_unlock();
>
> diff --git a/net/filter.c b/net/filter.c
> index 888fe6d..1dfd2ca 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -239,7 +239,7 @@ static void netfilter_finalize(Object *obj)
> }
>
> if (nf->netdev && !QTAILQ_EMPTY(&nf->netdev->filters) &&
> -nf->next.tqe_prev) {
> +QTAILQ_IN_USE(nf, next)) {
> QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
> }
> g_free(nf->netdev_id);
>
Looks good, thanks!
Paolo
Re: [Qemu-block] [PATCH 2/4] nbd: Limit nbdflags to 16 bits
On 08/01/2016 03:17 AM, Paolo Bonzini wrote:
>
>
> On 21/07/2016 21:34, Eric Blake wrote:
>> Furthermore, upstream NBD has never passed the global flags to
>> the kernel via ioctl(NBD_SET_FLAGS) (the ioctl was first
>> introduced in NBD 2.9.22; then a latent bug in NBD 3.1 actually
>> tried to OR the global flags with the transmission flags, with
>> the disaster that the addition of NBD_FLAG_NO_ZEROES in 3.9
>> caused all earlier NBD 3.x clients to treat every export as
>> read-only; NBD 3.10 and later intentionally clip things to 16
>> bits to pass only transmission flags). Qemu should follow suit,
>> since the current two global flags (NBD_FLAG_FIXED_NEWSTYLE
>> and NBD_FLAG_NO_ZEROES) have no impact on the kernel's behavior
>> during transmission.
>
> Should squash this in too:
>
> diff --git a/nbd/server.c b/nbd/server.c
> index 80fbb4d..6fa2f9c 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -575,7 +575,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData
> *data)
>
> oldStyle = client->exp != NULL && !client->tlscreds;
> if (oldStyle) {
> -TRACE("advertising size %" PRIu64 " and flags %x",
> +TRACE("advertising size %" PRIu64 " and flags %" PRIx16,
>client->exp->size, client->exp->nbdflags | myflags);
No, we shouldn't. Last time I tried that, we tickled a clang bug where
%hx gripes when presented an 'int' argument, in spite of the int
argument being computed as 'short | short'. See commit 2cb34749, and
your discussion leading up to it:
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg04663.html
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH 2/4] nbd: Limit nbdflags to 16 bits
On 01/08/2016 13:43, Eric Blake wrote:
> On 08/01/2016 03:17 AM, Paolo Bonzini wrote:
>>
>>
>> On 21/07/2016 21:34, Eric Blake wrote:
>>> Furthermore, upstream NBD has never passed the global flags to
>>> the kernel via ioctl(NBD_SET_FLAGS) (the ioctl was first
>>> introduced in NBD 2.9.22; then a latent bug in NBD 3.1 actually
>>> tried to OR the global flags with the transmission flags, with
>>> the disaster that the addition of NBD_FLAG_NO_ZEROES in 3.9
>>> caused all earlier NBD 3.x clients to treat every export as
>>> read-only; NBD 3.10 and later intentionally clip things to 16
>>> bits to pass only transmission flags). Qemu should follow suit,
>>> since the current two global flags (NBD_FLAG_FIXED_NEWSTYLE
>>> and NBD_FLAG_NO_ZEROES) have no impact on the kernel's behavior
>>> during transmission.
>>
>> Should squash this in too:
>>
>> diff --git a/nbd/server.c b/nbd/server.c
>> index 80fbb4d..6fa2f9c 100644
>> --- a/nbd/server.c
>> +++ b/nbd/server.c
>> @@ -575,7 +575,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData
>> *data)
>>
>> oldStyle = client->exp != NULL && !client->tlscreds;
>> if (oldStyle) {
>> -TRACE("advertising size %" PRIu64 " and flags %x",
>> +TRACE("advertising size %" PRIu64 " and flags %" PRIx16,
>>client->exp->size, client->exp->nbdflags | myflags);
>
> No, we shouldn't. Last time I tried that, we tickled a clang bug where
> %hx gripes when presented an 'int' argument, in spite of the int
> argument being computed as 'short | short'. See commit 2cb34749, and
> your discussion leading up to it:
> https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg04663.html
Uff, you're right. :( I remembered the discussion, but not the outcome.
Paolo
Re: [Qemu-block] [PATCH v4 01/11] block: Accept node-name for block-stream
On Thu 14 Jul 2016 03:28:04 PM CEST, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > block-stream to accept a node-name without lifting the restriction that > we're operating at a root node. > > In case of an invalid device name, the command returns the GenericError > error class now instead of DeviceNotFound, because this is what > qmp_get_root_bs() returns. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto
Re: [Qemu-block] [PATCH v4 04/11] block: Accept node-name for blockdev-mirror
On Thu 14 Jul 2016 03:28:07 PM CEST, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > blockdev-mirror to accept a node-name without lifting the restriction > that we're operating at a root node. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto
Re: [Qemu-block] [PATCH v4 02/11] block: Accept node-name for block-commit
On Thu 14 Jul 2016 03:28:05 PM CEST, Kevin Wolf wrote:
> -blk = blk_by_name(device);
> -if (!blk) {
> -error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> - "Device '%s' not found", device);
> +bs = qmp_get_root_bs(device, &local_err);
> +if (!bs) {
> +bs = bdrv_lookup_bs(device, device, NULL);
> +if (!bs) {
> +error_free(local_err);
> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> + "Device '%s' not found", device);
> +} else {
> +error_propagate(errp, local_err);
> +}
> return;
It seems that you're calling bdrv_lookup_bs() here twice, once in
qmp_get_root_bs() and then again directly. If the purpose is to see
whether the error is "device not found" or "device is not a root node" I
think the code would be clearer if you do everything here.
On a related note, you're keeping the DeviceNotFound error here, but not
in block-stream. Wouldn't it be better to keep both APIs consistent?
Berto
Re: [Qemu-block] [PATCH v4 05/11] block: Accept node-name for blockdev-snapshot-delete-internal-sync
On Thu 14 Jul 2016 03:28:08 PM CEST, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > blockdev-snapshot-delete-internal-sync to accept a node-name without > lifting the restriction that we're operating at a root node. > > In case of an invalid device name, the command returns the GenericError > error class now instead of DeviceNotFound, because this is what > qmp_get_root_bs() returns. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto
Re: [Qemu-block] [PATCH v4 08/11] block: Accept node-name for drive-backup
On Thu 14 Jul 2016 03:28:11 PM CEST, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > drive-backup and the corresponding transaction action to accept a > node-name without lifting the restriction that we're operating at a root > node. > > In case of an invalid device name, the command returns the GenericError > error class now instead of DeviceNotFound, because this is what > qmp_get_root_bs() returns. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto
Re: [Qemu-block] [PATCH v4 07/11] block: Accept node-name for change-backing-file
On Thu 14 Jul 2016 03:28:10 PM CEST, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > change-backing-file to accept a node-name without lifting the > restriction that we're operating at a root node. > > In case of an invalid device name, the command returns the GenericError > error class now instead of DeviceNotFound, because this is what > qmp_get_root_bs() returns. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto
Re: [Qemu-block] [PATCH v4 06/11] block: Accept node-name for blockdev-snapshot-internal-sync
On Thu 14 Jul 2016 03:28:09 PM CEST, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > blockdev-snapshot-internal-sync to accept a node-name without lifting > the restriction that we're operating at a root node. > > In case of an invalid device name, the command returns the GenericError > error class now instead of DeviceNotFound, because this is what > qmp_get_root_bs() returns. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto
Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] ide: ide-cd without drive property for empty drive
Am 14.07.2016 um 21:48 hat Eric Blake geschrieben:
> On 07/14/2016 07:49 AM, Kevin Wolf wrote:
> > This allows to create an empty ide-cd device without manually creating a
> > BlockBackend.
> >
> > Signed-off-by: Kevin Wolf
> > ---
> > hw/ide/qdev.c | 20 +++-
> > 1 file changed, 15 insertions(+), 5 deletions(-)
>
> > @@ -158,6 +154,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind
> > kind)
> > IDEState *s = bus->ifs + dev->unit;
> > Error *err = NULL;
> >
> > +if (!dev->conf.blk) {
> > +if (kind != IDE_CD) {
> > +error_report("No drive specified");
> > +return -1;
> > +} else {
> > +/* Anonymous BlockBackend for an empty drive */
> > +dev->conf.blk = blk_new();
>
> So we either fail or dev->conf.blk is set...
>
> > +}
> > +}
> > +
> > if (dev->conf.discard_granularity == -1) {
> > dev->conf.discard_granularity = 512;
> > } else if (dev->conf.discard_granularity &&
> > @@ -257,7 +263,11 @@ static int ide_cd_initfn(IDEDevice *dev)
> >
> > static int ide_drive_initfn(IDEDevice *dev)
> > {
> > -DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
> > +DriveInfo *dinfo = NULL;
> > +
> > +if (dev->conf.blk) {
> > +dinfo = blk_legacy_dinfo(dev->conf.blk);
> > +}
> >
> > return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
>
> ...yet, this claims dev->conf.blk can be NULL. What am I missing?
That ide_drive_initfn() is the outer function and runs first, before we
handle the case in ide_dev_initfn()?
Kevin
pgpDwc34m0THv.pgp
Description: PGP signature
Re: [Qemu-block] [PATCH] backup: block-job error BUG
On 07/18/2016 05:22 PM, Vladimir Sementsov-Ogievskiy wrote:
Hi all!
This is a variant of existing test case which produces test failure.
It looks like the reason is:
one block job is in backup_complete, in synchronous bdrv_flush (success job)
other (job with injected io err) tries to synchronously cancel "success job"
It looks like some kind of dead-lock
-..
+EEE
+==
+ERROR: test_transaction_failure (__main__.TestIncrementalBackup)
+Test: Verify backups made from a transaction that partially fails.
+--
+Traceback (most recent call last):
+ File "124", line 478, in test_transaction_failure
+self.wait_qmp_backup_cancelled(drive0['id'])
+ File "124", line 173, in wait_qmp_backup_cancelled
+match={'data': {'device': device}})
+ File "/work/src/qemu/tests/qemu-iotests/iotests.py", line 308, in event_wait
+event = self._qmp.pull_event(wait=timeout)
+ File "/work/src/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 194,
in pull_event
+self.__get_events(wait)
+ File "/work/src/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 109,
in __get_events
+raise QMPTimeoutError("Timeout waiting for event")
+QMPTimeoutError: Timeout waiting for event
+
+==
+ERROR: test_transaction_failure (__main__.TestIncrementalBackup)
+Test: Verify backups made from a transaction that partially fails.
+--
+Traceback (most recent call last):
+ File "124", line 272, in tearDown
+self.vm.shutdown()
+ File "/work/src/qemu/tests/qemu-iotests/iotests.py", line 260, in shutdown
+self._qmp.cmd('quit')
+ File "/work/src/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 172,
in cmd
+return self.cmd_obj(qmp_cmd)
+ File "/work/src/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 157,
in cmd_obj
+return self.__json_read()
+ File "/work/src/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 66,
in __json_read
+data = self.__sockfile.readline()
+ File "/usr/lib64/python2.7/socket.py", line 447, in readline
+data = self._sock.recv(self._rbufsize)
+timeout: timed out
+
+==
+ERROR: test_incremental_failure (__main__.TestIncrementalBackupBlkdebug)
+Test: Verify backups made after a failure are correct.
+--
+Traceback (most recent call last):
+ File "124", line 549, in setUp
+self.vm.launch()
+ File "/work/src/qemu/tests/qemu-iotests/iotests.py", line 246, in launch
+self._qmp = qmp.QEMUMonitorProtocol(self._monitor_path, server=True)
+ File "/work/src/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 44,
in __init__
+self.__sock.bind(self.__address)
+ File "/usr/lib64/python2.7/socket.py", line 224, in meth
+return getattr(self._sock,name)(*args)
+error: [Errno 98] Address already in use
+
I guess this is a residual failure based off of our inability to clean
up properly after the real failure.
--
Ran 10 tests
-OK
+FAILED (errors=3)
Failures: 124
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/124 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index de7cdbe..74de117 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -448,9 +448,9 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.assertFalse(self.vm.get_qmp_events(wait=False))
# Emulate some writes
-self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
- ('0xfe', '16M', '256k'),
- ('0x64', '32736k', '64k')))
+#self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
+# ('0xfe', '16M', '256k'),
+# ('0x64', '32736k', '64k')))
self.hmp_io_writes(drive1['id'], (('0xba', 0, 512),
('0xef', '16M', '256k'),
('0x46', '32736k', '64k')))
Python diffs are awful.
For the sake of the list: You modified the test_transaction_failure test
case, which tests an injected failure on one of two drives, both with
writes that need to be committed.
You induce the failure by removing the pending writes from drive0 such
that drive0 has nothing it needs to commit at all.
We then timeout waiting for notification that drive0's job was
canceled... Hm, perhaps this is a race condition where drive0 finishes
too quickly to cancel?
Re: [Qemu-block] [PATCH 1/3] blockjob: fix dead pointer in txn list
On 07/27/2016 06:49 AM, Vladimir Sementsov-Ogievskiy wrote:
Job may be freed in block_job_unref and in this case this would break
transaction QLIST.
Fix this by removing job from this list before unref.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
blockjob.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/blockjob.c b/blockjob.c
index a5ba3be..e045091 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -216,6 +216,7 @@ static void block_job_completed_single(BlockJob *job)
}
job->cb(job->opaque, job->ret);
if (job->txn) {
+QLIST_REMOVE(job, txn_list);
block_job_txn_unref(job->txn);
}
block_job_unref(job);
Has this caused actual problems for you?
This function is only ever called in a transactional context if the
transaction is over -- so we're not likely to use the pointers ever
again anyway.
Still, it's good practice, and the caller uses a safe iteration of the
list, so I think this should be safe.
But I don't think this SHOULD fix an actual bug. If it does, I think
something else is wrong.
Tested-by: John Snow
Reviewed-by: John Snow
[Qemu-block] [PATCH v8 00/10] Dirty bitmap changes for migration/persistence work
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/10:[] [-C] 'block: Hide HBitmap in block dirty bitmap interface' 002/10:[] [--] 'HBitmap: Introduce "meta" bitmap to track bit changes' 003/10:[] [--] 'tests: Add test code for meta bitmap' 004/10:[0004] [FC] 'block: Support meta dirty bitmap' 005/10:[] [--] 'block: Add two dirty bitmap getters' 006/10:[] [--] 'block: Assert that bdrv_release_dirty_bitmap succeeded' 007/10:[] [--] 'hbitmap: serialization' 008/10:[] [-C] 'block: BdrvDirtyBitmap serialization interface' 009/10:[] [--] 'tests: Add test code for hbitmap serialization' 010/10:[] [--] 'block: More operations for meta dirty bitmap' === v8: Hello, is it v8 you're looking for? === 01: Rebase conflict over int64_t header for bdrv_reset_dirty_bitmap. 04: Revised sector math to make literally any sense. 08: Rebase conflict over int64_t header for bdrv_reset_dirty_bitmap. === v7: === 02: Fix rebase mishap. 04: Slight loop adjustment. 09: Fix constant on 32bit machines. === v6: Rebase. Stole series from Fam. === 02: Added documentation changes as suggested by Max. === v5: Rebase: first 5 patches from last revision are already merged. === Addressed Max's comments: 01: - "block.c" -> "block/dirty-bitmap.c" in commit message. - "an BdrvDirtyBitmapIter" -> "an BdrvDirtyBitmapIter" in code comment. - hbitmap_next => next_dirty as variable name. - bdrv_dirty_iter_free()/bdrv_dirty_iter_new() pairs => bdrv_set_dirty_iter. 02: Move the assert fix into 01. 04: Truncate the meta bitmap (done by hbitmap_truncate). 06: Add Max's r-b. 07: I left the memcpy vs cpu_to_le32/64w as is to pick up Max's r-b. That could be improved on top if wanted. 10: Add Max's r-b. For convenience, this branch is available at: https://github.com/jnsnow/qemu.git branch meta-bitmap https://github.com/jnsnow/qemu/tree/meta-bitmap This version is tagged meta-bitmap-v8: https://github.com/jnsnow/qemu/releases/tag/meta-bitmap-v8 Fam Zheng (8): block: Hide HBitmap in block dirty bitmap interface HBitmap: Introduce "meta" bitmap to track bit changes tests: Add test code for meta bitmap block: Support meta dirty bitmap block: Add two dirty bitmap getters block: Assert that bdrv_release_dirty_bitmap succeeded tests: Add test code for hbitmap serialization block: More operations for meta dirty bitmap Vladimir Sementsov-Ogievskiy (2): hbitmap: serialization block: BdrvDirtyBitmap serialization interface block/backup.c | 14 ++- block/dirty-bitmap.c | 160 ++- block/mirror.c | 24 ++-- include/block/dirty-bitmap.h | 35 +- include/qemu/hbitmap.h | 100 + include/qemu/typedefs.h | 1 + tests/test-hbitmap.c | 255 +++ util/hbitmap.c | 206 +++--- 8 files changed, 755 insertions(+), 40 deletions(-) -- 2.7.4
[Qemu-block] [PATCH v8 05/10] block: Add two dirty bitmap getters
From: Fam Zheng
For dirty bitmap users to get the size and the name of a
BdrvDirtyBitmap.
Signed-off-by: Fam Zheng
Reviewed-by: John Snow
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
block/dirty-bitmap.c | 10 ++
include/block/dirty-bitmap.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 9c6febb..860acc9 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -154,6 +154,16 @@ void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
hbitmap_reset(bitmap->meta, sector, nb_sectors);
}
+int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
+{
+return bitmap->size;
+}
+
+const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
+{
+return bitmap->name;
+}
+
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
{
return bitmap->successor;
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 69c500b..c4e7858 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -32,6 +32,8 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState
*bs);
uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
+const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
+int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap);
DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t sector);
--
2.7.4
[Qemu-block] [PATCH v8 06/10] block: Assert that bdrv_release_dirty_bitmap succeeded
From: Fam Zheng
We use a loop over bs->dirty_bitmaps to make sure the caller is
only releasing a bitmap owned by bs. Let's also assert that in this case
the caller is releasing a bitmap that does exist.
Signed-off-by: Fam Zheng
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
block/dirty-bitmap.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 860acc9..31d5296 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -305,6 +305,9 @@ static void
bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
}
}
}
+if (bitmap) {
+abort();
+}
}
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
--
2.7.4
[Qemu-block] [PATCH v8 07/10] hbitmap: serialization
From: Vladimir Sementsov-Ogievskiy
Functions to serialize / deserialize(restore) HBitmap. HBitmap should be
saved to linear sequence of bits independently of endianness and bitmap
array element (unsigned long) size. Therefore Little Endian is chosen.
These functions are appropriate for dirty bitmap migration, restoring
the bitmap in several steps is available. To save performance, every
step writes only the last level of the bitmap. All other levels are
restored by hbitmap_deserialize_finish() as a last step of restoring.
So, HBitmap is inconsistent while restoring.
Signed-off-by: Vladimir Sementsov-Ogievskiy
[Fix left shift operand to 1UL; add "finish" parameter. - Fam]
Signed-off-by: Fam Zheng
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
include/qemu/hbitmap.h | 79
util/hbitmap.c | 137 +
2 files changed, 216 insertions(+)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 1725919..eb46475 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -146,6 +146,85 @@ void hbitmap_reset_all(HBitmap *hb);
bool hbitmap_get(const HBitmap *hb, uint64_t item);
/**
+ * hbitmap_serialization_granularity:
+ * @hb: HBitmap to operate on.
+ *
+ * Granularity of serialization chunks, used by other serialization functions.
+ * For every chunk:
+ * 1. Chunk start should be aligned to this granularity.
+ * 2. Chunk size should be aligned too, except for last chunk (for which
+ * start + count == hb->size)
+ */
+uint64_t hbitmap_serialization_granularity(const HBitmap *hb);
+
+/**
+ * hbitmap_serialization_size:
+ * @hb: HBitmap to operate on.
+ * @start: Starting bit
+ * @count: Number of bits
+ *
+ * Return number of bytes hbitmap_(de)serialize_part needs
+ */
+uint64_t hbitmap_serialization_size(const HBitmap *hb,
+uint64_t start, uint64_t count);
+
+/**
+ * hbitmap_serialize_part
+ * @hb: HBitmap to operate on.
+ * @buf: Buffer to store serialized bitmap.
+ * @start: First bit to store.
+ * @count: Number of bits to store.
+ *
+ * Stores HBitmap data corresponding to given region. The format of saved data
+ * is linear sequence of bits, so it can be used by hbitmap_deserialize_part
+ * independently of endianness and size of HBitmap level array elements
+ */
+void hbitmap_serialize_part(const HBitmap *hb, uint8_t *buf,
+uint64_t start, uint64_t count);
+
+/**
+ * hbitmap_deserialize_part
+ * @hb: HBitmap to operate on.
+ * @buf: Buffer to restore bitmap data from.
+ * @start: First bit to restore.
+ * @count: Number of bits to restore.
+ * @finish: Whether to call hbitmap_deserialize_finish automatically.
+ *
+ * Restores HBitmap data corresponding to given region. The format is the same
+ * as for hbitmap_serialize_part.
+ *
+ * If @finish is false, caller must call hbitmap_serialize_finish before using
+ * the bitmap.
+ */
+void hbitmap_deserialize_part(HBitmap *hb, uint8_t *buf,
+ uint64_t start, uint64_t count,
+ bool finish);
+
+/**
+ * hbitmap_deserialize_zeroes
+ * @hb: HBitmap to operate on.
+ * @start: First bit to restore.
+ * @count: Number of bits to restore.
+ * @finish: Whether to call hbitmap_deserialize_finish automatically.
+ *
+ * Fills the bitmap with zeroes.
+ *
+ * If @finish is false, caller must call hbitmap_serialize_finish before using
+ * the bitmap.
+ */
+void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t start, uint64_t count,
+bool finish);
+
+/**
+ * hbitmap_deserialize_finish
+ * @hb: HBitmap to operate on.
+ *
+ * Repair HBitmap after calling hbitmap_deserialize_data. Actually, all HBitmap
+ * layers are restored here.
+ */
+void hbitmap_deserialize_finish(HBitmap *hb);
+
+/**
* hbitmap_free:
* @hb: HBitmap to operate on.
*
diff --git a/util/hbitmap.c b/util/hbitmap.c
index f303975..6a13c12 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -397,6 +397,143 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item)
return (hb->levels[HBITMAP_LEVELS - 1][pos >> BITS_PER_LEVEL] & bit) != 0;
}
+uint64_t hbitmap_serialization_granularity(const HBitmap *hb)
+{
+/* Require at least 64 bit granularity to be safe on both 64 bit and 32 bit
+ * hosts. */
+return 64 << hb->granularity;
+}
+
+/* Start should be aligned to serialization granularity, chunk size should be
+ * aligned to serialization granularity too, except for last chunk.
+ */
+static void serialization_chunk(const HBitmap *hb,
+uint64_t start, uint64_t count,
+unsigned long **first_el, size_t *el_count)
+{
+uint64_t last = start + count - 1;
+uint64_t gran = hbitmap_serialization_granularity(hb);
+
+assert((start & (gran - 1)) == 0);
+assert((last >> hb->granularity) < hb->size);
+if ((last >> hb->granularity) != hb->size - 1) {
+a
[Qemu-block] [PATCH v8 02/10] HBitmap: Introduce "meta" bitmap to track bit changes
From: Fam Zheng
Upon each bit toggle, the corresponding bit in the meta bitmap will be
set.
Signed-off-by: Fam Zheng
[Amended text inline. --js]
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
include/qemu/hbitmap.h | 21 +++
util/hbitmap.c | 69 +++---
2 files changed, 75 insertions(+), 15 deletions(-)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 8ab721e..1725919 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -178,6 +178,27 @@ void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap
*hb, uint64_t first);
*/
unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi);
+/* hbitmap_create_meta:
+ * Create a "meta" hbitmap to track dirtiness of the bits in this HBitmap.
+ * The caller owns the created bitmap and must call hbitmap_free_meta(hb) to
+ * free it.
+ *
+ * Currently, we only guarantee that if a bit in the hbitmap is changed it
+ * will be reflected in the meta bitmap, but we do not yet guarantee the
+ * opposite.
+ *
+ * @hb: The HBitmap to operate on.
+ * @chunk_size: How many bits in @hb does one bit in the meta track.
+ */
+HBitmap *hbitmap_create_meta(HBitmap *hb, int chunk_size);
+
+/* hbitmap_free_meta:
+ * Free the meta bitmap of @hb.
+ *
+ * @hb: The HBitmap whose meta bitmap should be freed.
+ */
+void hbitmap_free_meta(HBitmap *hb);
+
/**
* hbitmap_iter_next:
* @hbi: HBitmapIter to operate on.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 99fd2ba..f303975 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -78,6 +78,9 @@ struct HBitmap {
*/
int granularity;
+/* A meta dirty bitmap to track the dirtiness of bits in this HBitmap. */
+HBitmap *meta;
+
/* A number of progressively less coarse bitmaps (i.e. level 0 is the
* coarsest). Each bit in level N represents a word in level N+1 that
* has a set bit, except the last level where each bit represents the
@@ -209,25 +212,27 @@ static uint64_t hb_count_between(HBitmap *hb, uint64_t
start, uint64_t last)
}
/* Setting starts at the last layer and propagates up if an element
- * changes from zero to non-zero.
+ * changes.
*/
static inline bool hb_set_elem(unsigned long *elem, uint64_t start, uint64_t
last)
{
unsigned long mask;
-bool changed;
+unsigned long old;
assert((last >> BITS_PER_LEVEL) == (start >> BITS_PER_LEVEL));
assert(start <= last);
mask = 2UL << (last & (BITS_PER_LONG - 1));
mask -= 1UL << (start & (BITS_PER_LONG - 1));
-changed = (*elem == 0);
+old = *elem;
*elem |= mask;
-return changed;
+return old != *elem;
}
-/* The recursive workhorse (the depth is limited to HBITMAP_LEVELS)... */
-static void hb_set_between(HBitmap *hb, int level, uint64_t start, uint64_t
last)
+/* The recursive workhorse (the depth is limited to HBITMAP_LEVELS)...
+ * Returns true if at least one bit is changed. */
+static bool hb_set_between(HBitmap *hb, int level, uint64_t start,
+ uint64_t last)
{
size_t pos = start >> BITS_PER_LEVEL;
size_t lastpos = last >> BITS_PER_LEVEL;
@@ -256,23 +261,28 @@ static void hb_set_between(HBitmap *hb, int level,
uint64_t start, uint64_t last
if (level > 0 && changed) {
hb_set_between(hb, level - 1, pos, lastpos);
}
+return changed;
}
void hbitmap_set(HBitmap *hb, uint64_t start, uint64_t count)
{
/* Compute range in the last layer. */
+uint64_t first, n;
uint64_t last = start + count - 1;
trace_hbitmap_set(hb, start, count,
start >> hb->granularity, last >> hb->granularity);
-start >>= hb->granularity;
+first = start >> hb->granularity;
last >>= hb->granularity;
-count = last - start + 1;
assert(last < hb->size);
+n = last - first + 1;
-hb->count += count - hb_count_between(hb, start, last);
-hb_set_between(hb, HBITMAP_LEVELS - 1, start, last);
+hb->count += n - hb_count_between(hb, first, last);
+if (hb_set_between(hb, HBITMAP_LEVELS - 1, first, last) &&
+hb->meta) {
+hbitmap_set(hb->meta, start, count);
+}
}
/* Resetting works the other way round: propagate up if the new
@@ -293,8 +303,10 @@ static inline bool hb_reset_elem(unsigned long *elem,
uint64_t start, uint64_t l
return blanked;
}
-/* The recursive workhorse (the depth is limited to HBITMAP_LEVELS)... */
-static void hb_reset_between(HBitmap *hb, int level, uint64_t start, uint64_t
last)
+/* The recursive workhorse (the depth is limited to HBITMAP_LEVELS)...
+ * Returns true if at least one bit is changed. */
+static bool hb_reset_between(HBitmap *hb, int level, uint64_t start,
+ uint64_t last)
{
size_t pos = start >> BITS_PER_LEVEL;
size_t lastpos = last >> BITS_PER_LEVEL;
@@ -337,22 +349,29 @@ static void hb_reset_between(HBitmap *hb, int level,
uint64_t start, uint64_t la
if (level > 0 &&
[Qemu-block] [PATCH v8 09/10] tests: Add test code for hbitmap serialization
From: Fam Zheng
Signed-off-by: Fam Zheng
[Fixed minor constant issue. --js]
Signed-off-by: John Snow
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
tests/test-hbitmap.c | 139 +++
1 file changed, 139 insertions(+)
diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index e3abde1..0ed918c 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -11,6 +11,7 @@
#include "qemu/osdep.h"
#include "qemu/hbitmap.h"
+#include "qemu/bitmap.h"
#include "block/block.h"
#define LOG_BITS_PER_LONG (BITS_PER_LONG == 32 ? 5 : 6)
@@ -737,6 +738,16 @@ static void test_hbitmap_meta_one(TestHBitmapData *data,
const void *unused)
}
}
+static void test_hbitmap_serialize_granularity(TestHBitmapData *data,
+ const void *unused)
+{
+int r;
+
+hbitmap_test_init(data, L3 * 2, 3);
+r = hbitmap_serialization_granularity(data->hb);
+g_assert_cmpint(r, ==, 64 << 3);
+}
+
static void test_hbitmap_meta_zero(TestHBitmapData *data, const void *unused)
{
hbitmap_test_init_meta(data, 0, 0, 1);
@@ -744,6 +755,125 @@ static void test_hbitmap_meta_zero(TestHBitmapData *data,
const void *unused)
hbitmap_check_meta(data, 0, 0);
}
+static void hbitmap_test_serialize_range(TestHBitmapData *data,
+ uint8_t *buf, size_t buf_size,
+ uint64_t pos, uint64_t count)
+{
+size_t i;
+
+assert(hbitmap_granularity(data->hb) == 0);
+hbitmap_reset_all(data->hb);
+memset(buf, 0, buf_size);
+if (count) {
+hbitmap_set(data->hb, pos, count);
+}
+hbitmap_serialize_part(data->hb, buf, 0, data->size);
+for (i = 0; i < data->size; i++) {
+int is_set = test_bit(i, (unsigned long *)buf);
+if (i >= pos && i < pos + count) {
+g_assert(is_set);
+} else {
+g_assert(!is_set);
+}
+}
+hbitmap_reset_all(data->hb);
+hbitmap_deserialize_part(data->hb, buf, 0, data->size, true);
+
+for (i = 0; i < data->size; i++) {
+int is_set = hbitmap_get(data->hb, i);
+if (i >= pos && i < pos + count) {
+g_assert(is_set);
+} else {
+g_assert(!is_set);
+}
+}
+}
+
+static void test_hbitmap_serialize_basic(TestHBitmapData *data,
+ const void *unused)
+{
+int i, j;
+size_t buf_size;
+uint8_t *buf;
+uint64_t positions[] = { 0, 1, L1 - 1, L1, L2 - 1, L2, L2 + 1, L3 - 1 };
+int num_positions = sizeof(positions) / sizeof(positions[0]);
+
+hbitmap_test_init(data, L3, 0);
+buf_size = hbitmap_serialization_size(data->hb, 0, data->size);
+buf = g_malloc0(buf_size);
+
+for (i = 0; i < num_positions; i++) {
+for (j = 0; j < num_positions; j++) {
+hbitmap_test_serialize_range(data, buf, buf_size,
+ positions[i],
+ MIN(positions[j], L3 - positions[i]));
+}
+}
+
+g_free(buf);
+}
+
+static void test_hbitmap_serialize_part(TestHBitmapData *data,
+const void *unused)
+{
+int i, j, k;
+size_t buf_size;
+uint8_t *buf;
+uint64_t positions[] = { 0, 1, L1 - 1, L1, L2 - 1, L2, L2 + 1, L3 - 1 };
+int num_positions = sizeof(positions) / sizeof(positions[0]);
+
+hbitmap_test_init(data, L3, 0);
+buf_size = L2;
+buf = g_malloc0(buf_size);
+
+for (i = 0; i < num_positions; i++) {
+hbitmap_set(data->hb, positions[i], 1);
+}
+
+for (i = 0; i < data->size; i += buf_size) {
+hbitmap_serialize_part(data->hb, buf, i, buf_size);
+for (j = 0; j < buf_size; j++) {
+bool should_set = false;
+for (k = 0; k < num_positions; k++) {
+if (positions[k] == j + i) {
+should_set = true;
+break;
+}
+}
+g_assert_cmpint(should_set, ==, test_bit(j, (unsigned long *)buf));
+}
+}
+
+g_free(buf);
+}
+
+static void test_hbitmap_serialize_zeroes(TestHBitmapData *data,
+ const void *unused)
+{
+int i;
+HBitmapIter iter;
+int64_t next;
+uint64_t positions[] = { 0, L1, L2, L3 - L1};
+int num_positions = sizeof(positions) / sizeof(positions[0]);
+
+hbitmap_test_init(data, L3, 0);
+
+for (i = 0; i < num_positions; i++) {
+hbitmap_set(data->hb, positions[i], L1);
+}
+
+for (i = 0; i < num_positions; i++) {
+hbitmap_deserialize_zeroes(data->hb, positions[i], L1, true);
+hbitmap_iter_init(&iter, data->hb, 0);
+next = hbitmap_iter_next(&iter);
+if (i == num_positions - 1) {
+g_assert_cmpint(next, ==, -1);
+} else {
+g_assert_cmpint(next, ==, positions[i + 1]);
+
[Qemu-block] [PATCH v8 04/10] block: Support meta dirty bitmap
From: Fam Zheng
The added group of operations enables tracking of the changed bits in
the dirty bitmap.
Signed-off-by: Fam Zheng
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
block/dirty-bitmap.c | 52
include/block/dirty-bitmap.h | 9
2 files changed, 61 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index c572dfa..9c6febb 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -38,6 +38,7 @@
*/
struct BdrvDirtyBitmap {
HBitmap *bitmap;/* Dirty sector bitmap implementation */
+HBitmap *meta; /* Meta dirty bitmap */
BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap (Number of sectors) */
@@ -103,6 +104,56 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState
*bs,
return bitmap;
}
+/* bdrv_create_meta_dirty_bitmap
+ *
+ * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. I.e.
+ * when a dirty status bit in @bitmap is changed (either from reset to set or
+ * the other way around), its respective meta dirty bitmap bit will be marked
+ * dirty as well.
+ *
+ * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
+ * @chunk_size: how many bytes of bitmap data does each bit in the meta bitmap
+ * track.
+ */
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+ int chunk_size)
+{
+assert(!bitmap->meta);
+bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
+ chunk_size * BITS_PER_BYTE);
+}
+
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+assert(bitmap->meta);
+hbitmap_free_meta(bitmap->bitmap);
+bitmap->meta = NULL;
+}
+
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors)
+{
+uint64_t i;
+int sectors_per_bit = 1 << hbitmap_granularity(bitmap->meta);
+
+/* To optimize: we can make hbitmap to internally check the range in a
+ * coarse level, or at least do it word by word. */
+for (i = sector; i < sector + nb_sectors; i += sectors_per_bit) {
+if (hbitmap_get(bitmap->meta, i)) {
+return true;
+}
+}
+return false;
+}
+
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors)
+{
+hbitmap_reset(bitmap->meta, sector, nb_sectors);
+}
+
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
{
return bitmap->successor;
@@ -233,6 +284,7 @@ static void
bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
if ((!bitmap || bm == bitmap) && (!only_named || bm->name)) {
assert(!bm->active_iterators);
assert(!bdrv_dirty_bitmap_frozen(bm));
+assert(!bm->meta);
QLIST_REMOVE(bm, list);
hbitmap_free(bm->bitmap);
g_free(bm->name);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 0ef927d..69c500b 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -8,6 +8,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
uint32_t granularity,
const char *name,
Error **errp);
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+ int chunk_size);
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
@@ -36,6 +39,12 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int64_t nr_sectors);
void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int64_t nr_sectors);
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors);
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors);
BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
uint64_t first_sector);
void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
--
2.7.4
[Qemu-block] [PATCH v8 10/10] block: More operations for meta dirty bitmap
From: Fam Zheng
Callers can create an iterator of meta bitmap with
bdrv_dirty_meta_iter_new(), then use the bdrv_dirty_iter_* operations on
it. Meta iterators are also counted by bitmap->active_iterators.
Also add a couple of functions to retrieve granularity and count.
Signed-off-by: Fam Zheng
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
block/dirty-bitmap.c | 19 +++
include/block/dirty-bitmap.h | 3 +++
2 files changed, 22 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 384146b..519737c 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -393,6 +393,11 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap
*bitmap)
return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
}
+uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap)
+{
+return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->meta);
+}
+
BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
uint64_t first_sector)
{
@@ -403,6 +408,15 @@ BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap
*bitmap,
return iter;
}
+BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap)
+{
+BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1);
+hbitmap_iter_init(&iter->hbi, bitmap->meta, 0);
+iter->bitmap = bitmap;
+bitmap->active_iterators++;
+return iter;
+}
+
void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter)
{
if (!iter) {
@@ -514,3 +528,8 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
{
return hbitmap_count(bitmap->bitmap);
}
+
+int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
+{
+return hbitmap_count(bitmap->meta);
+}
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index efc2965..9dea14b 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -30,6 +30,7 @@ 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);
+uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
@@ -47,12 +48,14 @@ int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, int64_t sector,
int nb_sectors);
+BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
uint64_t first_sector);
void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
+int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitmap,
--
2.7.4
[Qemu-block] [PATCH v8 08/10] block: BdrvDirtyBitmap serialization interface
From: Vladimir Sementsov-Ogievskiy
Several functions to provide necessary access to BdrvDirtyBitmap for
block-migration.c
Signed-off-by: Vladimir Sementsov-Ogievskiy
[Add the "finish" parameters. - Fam]
Signed-off-by: Fam Zheng
Reviewed-by: John Snow
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
block/dirty-bitmap.c | 37 +
include/block/dirty-bitmap.h | 14 ++
2 files changed, 51 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 31d5296..384146b 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -453,6 +453,43 @@ void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap,
HBitmap *in)
hbitmap_free(tmp);
}
+uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitmap,
+ uint64_t start, uint64_t count)
+{
+return hbitmap_serialization_size(bitmap->bitmap, start, count);
+}
+
+uint64_t bdrv_dirty_bitmap_serialization_align(const BdrvDirtyBitmap *bitmap)
+{
+return hbitmap_serialization_granularity(bitmap->bitmap);
+}
+
+void bdrv_dirty_bitmap_serialize_part(const BdrvDirtyBitmap *bitmap,
+ uint8_t *buf, uint64_t start,
+ uint64_t count)
+{
+hbitmap_serialize_part(bitmap->bitmap, buf, start, count);
+}
+
+void bdrv_dirty_bitmap_deserialize_part(BdrvDirtyBitmap *bitmap,
+uint8_t *buf, uint64_t start,
+uint64_t count, bool finish)
+{
+hbitmap_deserialize_part(bitmap->bitmap, buf, start, count, finish);
+}
+
+void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitmap *bitmap,
+ uint64_t start, uint64_t count,
+ bool finish)
+{
+hbitmap_deserialize_zeroes(bitmap->bitmap, start, count, finish);
+}
+
+void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap)
+{
+hbitmap_deserialize_finish(bitmap->bitmap);
+}
+
void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
int64_t nr_sectors)
{
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index c4e7858..efc2965 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -55,4 +55,18 @@ void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t
sector_num);
int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
+uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitmap,
+ uint64_t start, uint64_t count);
+uint64_t bdrv_dirty_bitmap_serialization_align(const BdrvDirtyBitmap *bitmap);
+void bdrv_dirty_bitmap_serialize_part(const BdrvDirtyBitmap *bitmap,
+ uint8_t *buf, uint64_t start,
+ uint64_t count);
+void bdrv_dirty_bitmap_deserialize_part(BdrvDirtyBitmap *bitmap,
+uint8_t *buf, uint64_t start,
+uint64_t count, bool finish);
+void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitmap *bitmap,
+ uint64_t start, uint64_t count,
+ bool finish);
+void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
+
#endif
--
2.7.4
[Qemu-block] [PATCH v8 01/10] block: Hide HBitmap in block dirty bitmap interface
From: Fam Zheng
HBitmap is an implementation detail of block dirty bitmap that should be hidden
from users. Introduce a BdrvDirtyBitmapIter to encapsulate the underlying
HBitmapIter.
A small difference in the interface is, before, an HBitmapIter is initialized
in place, now the new BdrvDirtyBitmapIter must be dynamically allocated because
the structure definition is in block/dirty-bitmap.c.
Two current users are converted too.
Signed-off-by: Fam Zheng
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
block/backup.c | 14 --
block/dirty-bitmap.c | 39 +--
block/mirror.c | 24 +---
include/block/dirty-bitmap.h | 7 +--
include/qemu/typedefs.h | 1 +
5 files changed, 60 insertions(+), 25 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 2c05323..fc0fccf 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -325,14 +325,14 @@ static int coroutine_fn
backup_run_incremental(BackupBlockJob *job)
int64_t end;
int64_t last_cluster = -1;
int64_t sectors_per_cluster = cluster_size_sectors(job);
-HBitmapIter hbi;
+BdrvDirtyBitmapIter *dbi;
granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
clusters_per_iter = MAX((granularity / job->cluster_size), 1);
-bdrv_dirty_iter_init(job->sync_bitmap, &hbi);
+dbi = bdrv_dirty_iter_new(job->sync_bitmap, 0);
/* Find the next dirty sector(s) */
-while ((sector = hbitmap_iter_next(&hbi)) != -1) {
+while ((sector = bdrv_dirty_iter_next(dbi)) != -1) {
cluster = sector / sectors_per_cluster;
/* Fake progress updates for any clusters we skipped */
@@ -344,7 +344,7 @@ static int coroutine_fn
backup_run_incremental(BackupBlockJob *job)
for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
do {
if (yield_and_check(job)) {
-return ret;
+goto out;
}
ret = backup_do_cow(job, cluster * sectors_per_cluster,
sectors_per_cluster, &error_is_read,
@@ -352,7 +352,7 @@ static int coroutine_fn
backup_run_incremental(BackupBlockJob *job)
if ((ret < 0) &&
backup_error_action(job, error_is_read, -ret) ==
BLOCK_ERROR_ACTION_REPORT) {
-return ret;
+goto out;
}
} while (ret < 0);
}
@@ -360,7 +360,7 @@ static int coroutine_fn
backup_run_incremental(BackupBlockJob *job)
/* If the bitmap granularity is smaller than the backup granularity,
* we need to advance the iterator pointer to the next cluster. */
if (granularity < job->cluster_size) {
-bdrv_set_dirty_iter(&hbi, cluster * sectors_per_cluster);
+bdrv_set_dirty_iter(dbi, cluster * sectors_per_cluster);
}
last_cluster = cluster - 1;
@@ -372,6 +372,8 @@ static int coroutine_fn
backup_run_incremental(BackupBlockJob *job)
job->common.offset += ((end - last_cluster - 1) * job->cluster_size);
}
+out:
+bdrv_dirty_iter_free(dbi);
return ret;
}
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index f2bfdcf..c572dfa 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -42,9 +42,15 @@ struct BdrvDirtyBitmap {
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap (Number of sectors) */
bool disabled; /* Bitmap is read-only */
+int active_iterators; /* How many iterators are active */
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
+struct BdrvDirtyBitmapIter {
+HBitmapIter hbi;
+BdrvDirtyBitmap *bitmap;
+};
+
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
{
BdrvDirtyBitmap *bm;
@@ -212,6 +218,7 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
assert(!bdrv_dirty_bitmap_frozen(bitmap));
+assert(!bitmap->active_iterators);
hbitmap_truncate(bitmap->bitmap, size);
bitmap->size = size;
}
@@ -224,6 +231,7 @@ static void
bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *bm, *next;
QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
if ((!bitmap || bm == bitmap) && (!only_named || bm->name)) {
+assert(!bm->active_iterators);
assert(!bdrv_dirty_bitmap_frozen(bm));
QLIST_REMOVE(bm, list);
hbitmap_free(bm->bitmap);
@@ -320,9 +328,29 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap
*bitmap)
return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
}
-void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
+BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBi
[Qemu-block] [PATCH v8 03/10] tests: Add test code for meta bitmap
From: Fam Zheng
Signed-off-by: Fam Zheng
Reviewed-by: John Snow
Reviewed-by: Max Reitz
Signed-off-by: John Snow
---
tests/test-hbitmap.c | 116 +++
1 file changed, 116 insertions(+)
diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index c0e9895..e3abde1 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -11,6 +11,7 @@
#include "qemu/osdep.h"
#include "qemu/hbitmap.h"
+#include "block/block.h"
#define LOG_BITS_PER_LONG (BITS_PER_LONG == 32 ? 5 : 6)
@@ -20,6 +21,7 @@
typedef struct TestHBitmapData {
HBitmap *hb;
+HBitmap *meta;
unsigned long *bits;
size_t size;
size_t old_size;
@@ -91,6 +93,14 @@ static void hbitmap_test_init(TestHBitmapData *data,
}
}
+static void hbitmap_test_init_meta(TestHBitmapData *data,
+ uint64_t size, int granularity,
+ int meta_chunk)
+{
+hbitmap_test_init(data, size, granularity);
+data->meta = hbitmap_create_meta(data->hb, meta_chunk);
+}
+
static inline size_t hbitmap_test_array_size(size_t bits)
{
size_t n = DIV_ROUND_UP(bits, BITS_PER_LONG);
@@ -133,6 +143,9 @@ static void hbitmap_test_teardown(TestHBitmapData *data,
const void *unused)
{
if (data->hb) {
+if (data->meta) {
+hbitmap_free_meta(data->hb);
+}
hbitmap_free(data->hb);
data->hb = NULL;
}
@@ -634,6 +647,103 @@ static void
test_hbitmap_truncate_shrink_large(TestHBitmapData *data,
hbitmap_test_truncate(data, size, -diff, 0);
}
+static void hbitmap_check_meta(TestHBitmapData *data,
+ int64_t start, int count)
+{
+int64_t i;
+
+for (i = 0; i < data->size; i++) {
+if (i >= start && i < start + count) {
+g_assert(hbitmap_get(data->meta, i));
+} else {
+g_assert(!hbitmap_get(data->meta, i));
+}
+}
+}
+
+static void hbitmap_test_meta(TestHBitmapData *data,
+ int64_t start, int count,
+ int64_t check_start, int check_count)
+{
+hbitmap_reset_all(data->hb);
+hbitmap_reset_all(data->meta);
+
+/* Test "unset" -> "unset" will not update meta. */
+hbitmap_reset(data->hb, start, count);
+hbitmap_check_meta(data, 0, 0);
+
+/* Test "unset" -> "set" will update meta */
+hbitmap_set(data->hb, start, count);
+hbitmap_check_meta(data, check_start, check_count);
+
+/* Test "set" -> "set" will not update meta */
+hbitmap_reset_all(data->meta);
+hbitmap_set(data->hb, start, count);
+hbitmap_check_meta(data, 0, 0);
+
+/* Test "set" -> "unset" will update meta */
+hbitmap_reset_all(data->meta);
+hbitmap_reset(data->hb, start, count);
+hbitmap_check_meta(data, check_start, check_count);
+}
+
+static void hbitmap_test_meta_do(TestHBitmapData *data, int chunk_size)
+{
+uint64_t size = chunk_size * 100;
+hbitmap_test_init_meta(data, size, 0, chunk_size);
+
+hbitmap_test_meta(data, 0, 1, 0, chunk_size);
+hbitmap_test_meta(data, 0, chunk_size, 0, chunk_size);
+hbitmap_test_meta(data, chunk_size - 1, 1, 0, chunk_size);
+hbitmap_test_meta(data, chunk_size - 1, 2, 0, chunk_size * 2);
+hbitmap_test_meta(data, chunk_size - 1, chunk_size + 1, 0, chunk_size * 2);
+hbitmap_test_meta(data, chunk_size - 1, chunk_size + 2, 0, chunk_size * 3);
+hbitmap_test_meta(data, 7 * chunk_size - 1, chunk_size + 2,
+ 6 * chunk_size, chunk_size * 3);
+hbitmap_test_meta(data, size - 1, 1, size - chunk_size, chunk_size);
+hbitmap_test_meta(data, 0, size, 0, size);
+}
+
+static void test_hbitmap_meta_byte(TestHBitmapData *data, const void *unused)
+{
+hbitmap_test_meta_do(data, BITS_PER_BYTE);
+}
+
+static void test_hbitmap_meta_word(TestHBitmapData *data, const void *unused)
+{
+hbitmap_test_meta_do(data, BITS_PER_LONG);
+}
+
+static void test_hbitmap_meta_sector(TestHBitmapData *data, const void *unused)
+{
+hbitmap_test_meta_do(data, BDRV_SECTOR_SIZE * BITS_PER_BYTE);
+}
+
+/**
+ * Create an HBitmap and test set/unset.
+ */
+static void test_hbitmap_meta_one(TestHBitmapData *data, const void *unused)
+{
+int i;
+int64_t offsets[] = {
+0, 1, L1 - 1, L1, L1 + 1, L2 - 1, L2, L2 + 1, L3 - 1, L3, L3 + 1
+};
+
+hbitmap_test_init_meta(data, L3 * 2, 0, 1);
+for (i = 0; i < ARRAY_SIZE(offsets); i++) {
+hbitmap_test_meta(data, offsets[i], 1, offsets[i], 1);
+hbitmap_test_meta(data, offsets[i], L1, offsets[i], L1);
+hbitmap_test_meta(data, offsets[i], L2, offsets[i], L2);
+}
+}
+
+static void test_hbitmap_meta_zero(TestHBitmapData *data, const void *unused)
+{
+hbitmap_test_init_meta(data, 0, 0, 1);
+
+hbitmap_check_meta(data, 0, 0);
+}
+
static void hbitmap_test_add(const char *testpath,
Re: [Qemu-block] [PATCH for-2.7 1/1] ide: fix halted IO segfault at reset
On 08/01/2016 04:52 AM, Paolo Bonzini wrote:
On 27/07/2016 00:07, John Snow wrote:
If one attempts to perform a system_reset after a failed IO request
that causes the VM to enter a paused state, QEMU will segfault trying
to free up the pending IO requests.
These requests have already been completed and freed, though, so all
we need to do is free them before we enter the paused state.
Existing AHCI tests verify that halted requests are still resumed
successfully after a STOP event.
Signed-off-by: John Snow
---
hw/ide/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 081c9eb..d117b7c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -823,6 +823,7 @@ static void ide_dma_cb(void *opaque, int ret)
}
if (ret < 0) {
if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
+s->bus->dma->aiocb = NULL;
return;
}
}
The patch is (was, since it's committed :)) okay, but I think there is
another bug in the REPORT case, where ide_rw_error and
ide_atapi_io_error are not calling ide_set_inactive and thus are leaving
s->bus->dma->aiocb non-NULL.
Paolo
...Aha. Good eye.
I can probably just shift the aiocb nulling up a bit, but leave it in
ide_dma_cb.
Re: [Qemu-block] [PATCH for-2.7 1/1] ide: fix halted IO segfault at reset
On 08/01/2016 04:52 AM, Paolo Bonzini wrote:
On 27/07/2016 00:07, John Snow wrote:
If one attempts to perform a system_reset after a failed IO request
that causes the VM to enter a paused state, QEMU will segfault trying
to free up the pending IO requests.
These requests have already been completed and freed, though, so all
we need to do is free them before we enter the paused state.
Existing AHCI tests verify that halted requests are still resumed
successfully after a STOP event.
Signed-off-by: John Snow
---
hw/ide/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 081c9eb..d117b7c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -823,6 +823,7 @@ static void ide_dma_cb(void *opaque, int ret)
}
if (ret < 0) {
if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
+s->bus->dma->aiocb = NULL;
return;
}
}
The patch is (was, since it's committed :)) okay, but I think there is
another bug in the REPORT case, where ide_rw_error and
ide_atapi_io_error are not calling ide_set_inactive and thus are leaving
s->bus->dma->aiocb non-NULL.
Paolo
Actually, won't we hit ide_dma_error on REPORT which calls
ide_set_inactive? I think this might be OK, but I have to audit a little
more carefully -- I will do so tomorrow.
I think the ide_rw_error case is likely OK, but I always manage to
forget exactly how the ATAPI DMA looks.
