From: John Snow <jsnow@redhat.com>

in blockdev_backup_prepare, we check to make sure that the target is

associated with a compatible aio context. However, do_blockdev_backup is

called later and has some logic to move the target to a compatible

aio_context. The transaction version will fail certain commands

needlessly early as a result.

Allow blockdev_backup_prepare to simply call do_blockdev_backup, which

will ultimately decide if the contexts are compatible or not.

Note: the transaction version has always disallowed this operation since

its initial commit bd8baecd (2014), whereas the version of

qmp_blockdev_backup at the time, from commit c29c1dd312f, tried to

enforce the aio_context switch instead. It's not clear, and I can't see

from the mailing list archives at the time, why the two functions take a

different approach. It wasn't until later in efd7556708b (2016) that the

standalone version tried to determine if it could set the context or

not.

Reported-by: aihua liang <aliang@redhat.com>

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

Signed-off-by: John Snow <jsnow@redhat.com>

Message-id: 20190523170643.20794-2-jsnow@redhat.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

blockdev.c | 4 ----

1 file changed, 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c

index XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)

}

aio_context = bdrv_get_aio_context(bs);

- if (aio_context != bdrv_get_aio_context(target)) {

- error_setg(errp, "Backup between two IO threads is not implemented");

- return;

- }

aio_context_acquire(aio_context);

state->bs = bs;

--

2.21.0

From: John Snow <jsnow@redhat.com>

Cap waits to 60 seconds so that iotests can fail gracefully if something

goes wrong.

Signed-off-by: John Snow <jsnow@redhat.com>

Message-id: 20190523170643.20794-3-jsnow@redhat.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 14 +++++++-------

1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

output_list += [key + '=' + obj[key]]

return ','.join(output_list)

- def get_qmp_events_filtered(self, wait=True):

+ def get_qmp_events_filtered(self, wait=60.0):

result = []

for ev in self.get_qmp_events(wait=wait):

result.append(filter_qmp_event(ev))

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

# Returns None on success, and an error string on failure

def run_job(self, job, auto_finalize=True, auto_dismiss=False,

- pre_finalize=None):

+ pre_finalize=None, wait=60.0):

error = None

while True:

- for ev in self.get_qmp_events_filtered(wait=True):

+ for ev in self.get_qmp_events_filtered(wait=wait):

if ev['event'] == 'JOB_STATUS_CHANGE':

status = ev['data']['status']

if status == 'aborting':

@@ -XXX,XX +XXX,XX @@ 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, wait=60.0):

'''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', {})

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

cancelled = False

result = None

while not cancelled:

- for event in self.vm.get_qmp_events(wait=True):

+ for event in self.vm.get_qmp_events(wait=wait):

if event['event'] == 'BLOCK_JOB_COMPLETED' or \

event['event'] == 'BLOCK_JOB_CANCELLED':

self.assert_qmp(event, 'data/device', drive)

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

self.assert_no_active_block_jobs()

return result

- def wait_until_completed(self, drive='drive0', check_offset=True):

+ def wait_until_completed(self, drive='drive0', check_offset=True, wait=60.0):

'''Wait for a block job to finish, returning the event'''

while True:

- for event in self.vm.get_qmp_events(wait=True):

+ for event in self.vm.get_qmp_events(wait=wait):

if event['event'] == 'BLOCK_JOB_COMPLETED':

self.assert_qmp(event, 'data/device', drive)

self.assert_qmp_absent(event, 'data/error')

--

2.21.0

From: John Snow <jsnow@redhat.com>

Instead of event_wait which looks for a single event, add an events_wait

which can look for any number of events simultaneously. However, it

will still only return one at a time, whichever happens first.

Signed-off-by: John Snow <jsnow@redhat.com>

Message-id: 20190523170643.20794-4-jsnow@redhat.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

python/qemu/__init__.py | 69 +++++++++++++++++++++++++++++------------

1 file changed, 49 insertions(+), 20 deletions(-)

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py

index XXXXXXX..XXXXXXX 100644

--- a/python/qemu/__init__.py

+++ b/python/qemu/__init__.py

@@ -XXX,XX +XXX,XX @@ class QEMUMachine(object):

self._qmp.clear_events()

return events

- def event_wait(self, name, timeout=60.0, match=None):

+ @staticmethod

+ def event_match(event, match=None):

"""

- Wait for specified timeout on named event in QMP; optionally filter

- results by match.

+ Check if an event matches optional match criteria.

- The 'match' is checked to be a recursive subset of the 'event'; skips

- branch processing on match's value None

- {"foo": {"bar": 1}} matches {"foo": None}

- {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}

+ The match criteria takes the form of a matching subdict. The event is

+ checked to be a superset of the subdict, recursively, with matching

+ values whenever those values are not None.

+

+ Examples, with the subdict queries on the left:

+ - None matches any object.

+ - {"foo": None} matches {"foo": {"bar": 1}}

+ - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}

+ - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}

"""

- def event_match(event, match=None):

- if match is None:

- return True

+ if match is None:

+ return True

- for key in match:

- if key in event:

- if isinstance(event[key], dict):

- if not event_match(event[key], match[key]):

- return False

- elif event[key] != match[key]:

+ for key in match:

+ if key in event:

+ if isinstance(event[key], dict):

+ if not QEMUMachine.event_match(event[key], match[key]):

return False

- else:

+ elif event[key] != match[key]:

return False

+ else:

+ return False

+ return True

- return True

+ def event_wait(self, name, timeout=60.0, match=None):

+ """

+ event_wait waits for and returns a named event from QMP with a timeout.

+

+ name: The event to wait for.

+ timeout: QEMUMonitorProtocol.pull_event timeout parameter.

+ match: Optional match criteria. See event_match for details.

+ """

+ return self.events_wait([(name, match)], timeout)

+

+ def events_wait(self, events, timeout=60.0):

+ """

+ events_wait waits for and returns a named event from QMP with a timeout.

+

+ events: a sequence of (name, match_criteria) tuples.

+ The match criteria are optional and may be None.

+ See event_match for details.

+ timeout: QEMUMonitorProtocol.pull_event timeout parameter.

+ """

+ def _match(event):

+ for name, match in events:

+ if (event['event'] == name and

+ self.event_match(event, match)):

+ return True

+ return False

# Search cached events

for event in self._events:

- if (event['event'] == name) and event_match(event, match):

+ if _match(event):

self._events.remove(event)

return event

# Poll for new events

while True:

event = self._qmp.pull_event(wait=timeout)

- if (event['event'] == name) and event_match(event, match):

+ if _match(event):

return event

self._events.append(event)

--

2.21.0

From: John Snow <jsnow@redhat.com>

Don't pull events out of the queue that don't belong to us;

be choosier so that we can use this method to drive jobs that

were launched by transactions that may have more jobs.

Signed-off-by: John Snow <jsnow@redhat.com>

Message-id: 20190523170643.20794-5-jsnow@redhat.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 48 +++++++++++++++++++++--------------

1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

# Returns None on success, and an error string on failure

def run_job(self, job, auto_finalize=True, auto_dismiss=False,

pre_finalize=None, wait=60.0):

+ match_device = {'data': {'device': job}}

+ match_id = {'data': {'id': job}}

+ events = [

+ ('BLOCK_JOB_COMPLETED', match_device),

+ ('BLOCK_JOB_CANCELLED', match_device),

+ ('BLOCK_JOB_ERROR', match_device),

+ ('BLOCK_JOB_READY', match_device),

+ ('BLOCK_JOB_PENDING', match_id),

+ ('JOB_STATUS_CHANGE', match_id)

+ ]

error = None

while True:

- for ev in self.get_qmp_events_filtered(wait=wait):

- if ev['event'] == 'JOB_STATUS_CHANGE':

- status = ev['data']['status']

- if status == 'aborting':

- result = self.qmp('query-jobs')

- for j in result['return']:

- if j['id'] == job:

- error = j['error']

- log('Job failed: %s' % (j['error']))

- elif status == 'pending' and not auto_finalize:

- if pre_finalize:

- pre_finalize()

- self.qmp_log('job-finalize', id=job)

- elif status == 'concluded' and not auto_dismiss:

- self.qmp_log('job-dismiss', id=job)

- elif status == 'null':

- return error

- else:

- log(ev)

+ ev = filter_qmp_event(self.events_wait(events))

+ if ev['event'] != 'JOB_STATUS_CHANGE':

+ log(ev)

+ continue

+ status = ev['data']['status']

+ if status == 'aborting':

+ result = self.qmp('query-jobs')

+ for j in result['return']:

+ if j['id'] == job:

+ error = j['error']

+ log('Job failed: %s' % (j['error']))

+ elif status == 'pending' and not auto_finalize:

+ if pre_finalize:

+ pre_finalize()

+ self.qmp_log('job-finalize', id=job)

+ elif status == 'concluded' and not auto_dismiss:

+ self.qmp_log('job-dismiss', id=job)

+ elif status == 'null':

+ return error

def node_info(self, node_name):

nodes = self.qmp('query-named-block-nodes')

--

2.21.0

From: John Snow <jsnow@redhat.com>

Signed-off-by: John Snow <jsnow@redhat.com>

Message-id: 20190523170643.20794-6-jsnow@redhat.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

[mreitz: Moved from 250 to 256]

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/256 | 122 +++++++++++++++++++++++++++++++++++++

tests/qemu-iotests/256.out | 119 ++++++++++++++++++++++++++++++++++++

tests/qemu-iotests/group | 1 +

3 files changed, 242 insertions(+)

create mode 100755 tests/qemu-iotests/256

create mode 100644 tests/qemu-iotests/256.out

diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256

new file mode 100755

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/256

@@ -XXX,XX +XXX,XX @@

+#!/usr/bin/env python

+#

+# Test incremental/backup across iothread contexts

+#

+# Copyright (c) 2019 John Snow for Red Hat, Inc.

+#

+# 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 <http://www.gnu.org/licenses/>.

+#

+# owner=jsnow@redhat.com

+

+import os

+import iotests

+from iotests import log

+

+iotests.verify_image_format(supported_fmts=['qcow2'])

+size = 64 * 1024 * 1024

+

+with iotests.FilePath('img0') as img0_path, \

+ iotests.FilePath('img1') as img1_path, \

+ iotests.FilePath('img0-full') as img0_full_path, \

+ iotests.FilePath('img1-full') as img1_full_path, \

+ iotests.FilePath('img0-incr') as img0_incr_path, \

+ iotests.FilePath('img1-incr') as img1_incr_path, \

+ iotests.VM() as vm:

+

+ def create_target(filepath, name, size):

+ basename = os.path.basename(filepath)

+ nodename = "file_{}".format(basename)

+ log(vm.command('blockdev-create', job_id='job1',

+ options={

+ 'driver': 'file',

+ 'filename': filepath,

+ 'size': 0,

+ }))

+ vm.run_job('job1')

+ log(vm.command('blockdev-add', driver='file',

+ node_name=nodename, filename=filepath))

+ log(vm.command('blockdev-create', job_id='job2',

+ options={

+ 'driver': iotests.imgfmt,

+ 'file': nodename,

+ 'size': size,

+ }))

+ vm.run_job('job2')

+ log(vm.command('blockdev-add', driver=iotests.imgfmt,

+ node_name=name,

+ file=nodename))

+

+ log('--- Preparing images & VM ---\n')

+ vm.add_object('iothread,id=iothread0')

+ vm.add_object('iothread,id=iothread1')

+ vm.add_device('virtio-scsi-pci,id=scsi0,iothread=iothread0')

+ vm.add_device('virtio-scsi-pci,id=scsi1,iothread=iothread1')

+ iotests.qemu_img_create('-f', iotests.imgfmt, img0_path, str(size))

+ iotests.qemu_img_create('-f', iotests.imgfmt, img1_path, str(size))

+ vm.add_drive(img0_path, interface='none')

+ vm.add_device('scsi-hd,id=device0,drive=drive0,bus=scsi0.0')

+ vm.add_drive(img1_path, interface='none')

+ vm.add_device('scsi-hd,id=device1,drive=drive1,bus=scsi1.0')

+

+ log('--- Starting VM ---\n')

+ vm.launch()

+

+ log('--- Create Targets & Full Backups ---\n')

+ create_target(img0_full_path, 'img0-full', size)

+ create_target(img1_full_path, 'img1-full', size)

+ ret = vm.qmp_log('transaction', indent=2, actions=[

+ { 'type': 'block-dirty-bitmap-add',

+ 'data': { 'node': 'drive0', 'name': 'bitmap0' }},

+ { 'type': 'block-dirty-bitmap-add',

+ 'data': { 'node': 'drive1', 'name': 'bitmap1' }},

+ { 'type': 'blockdev-backup',

+ 'data': { 'device': 'drive0',

+ 'target': 'img0-full',

+ 'sync': 'full',

+ 'job-id': 'j0' }},

+ { 'type': 'blockdev-backup',

+ 'data': { 'device': 'drive1',

+ 'target': 'img1-full',

+ 'sync': 'full',

+ 'job-id': 'j1' }}

+ ])

+ if "error" in ret:

+ raise Exception(ret['error']['desc'])

+ vm.run_job('j0', auto_dismiss=True)

+ vm.run_job('j1', auto_dismiss=True)

+

+ log('\n--- Create Targets & Incremental Backups ---\n')

+ create_target(img0_incr_path, 'img0-incr', size)

+ create_target(img1_incr_path, 'img1-incr', size)

+ ret = vm.qmp_log('transaction', indent=2, actions=[

+ { 'type': 'blockdev-backup',

+ 'data': { 'device': 'drive0',

+ 'target': 'img0-incr',

+ 'sync': 'incremental',

+ 'bitmap': 'bitmap0',

+ 'job-id': 'j2' }},

+ { 'type': 'blockdev-backup',

+ 'data': { 'device': 'drive1',

+ 'target': 'img1-incr',

+ 'sync': 'incremental',

+ 'bitmap': 'bitmap1',

+ 'job-id': 'j3' }}

+ ])

+ if "error" in ret:

+ raise Exception(ret['error']['desc'])

+ vm.run_job('j2', auto_dismiss=True)

+ vm.run_job('j3', auto_dismiss=True)

+

+ log('\n--- Done ---')

+ vm.shutdown()

diff --git a/tests/qemu-iotests/256.out b/tests/qemu-iotests/256.out

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/256.out

@@ -XXX,XX +XXX,XX @@

+--- Preparing images & VM ---

+

+--- Starting VM ---

+

+--- Create Targets & Full Backups ---

+

+{}

+{"execute": "job-dismiss", "arguments": {"id": "job1"}}

+{"return": {}}

+{}

+{}

+{"execute": "job-dismiss", "arguments": {"id": "job2"}}

+{"return": {}}

+{}

+{}

+{"execute": "job-dismiss", "arguments": {"id": "job1"}}

+{"return": {}}

+{}

+{}

+{"execute": "job-dismiss", "arguments": {"id": "job2"}}

+{"return": {}}

+{}

+{

+ "execute": "transaction",

+ "arguments": {

+ "actions": [

+ {

+ "data": {

+ "name": "bitmap0",

+ "node": "drive0"

+ },

+ "type": "block-dirty-bitmap-add"

+ },

+ {

+ "data": {

+ "name": "bitmap1",

+ "node": "drive1"

+ },

+ "type": "block-dirty-bitmap-add"

+ },

+ {

+ "data": {

+ "device": "drive0",

+ "job-id": "j0",

+ "sync": "full",

+ "target": "img0-full"

+ },

+ "type": "blockdev-backup"

+ },

+ {

+ "data": {

+ "device": "drive1",

+ "job-id": "j1",

+ "sync": "full",

+ "target": "img1-full"

+ },

+ "type": "blockdev-backup"

+ }

+ ]

+ }

+}

+{

+ "return": {}

+}

+{"data": {"device": "j0", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+{"data": {"device": "j1", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+

+--- Create Targets & Incremental Backups ---

+

+{}

+{"execute": "job-dismiss", "arguments": {"id": "job1"}}

+{"return": {}}

+{}

+{}

+{"execute": "job-dismiss", "arguments": {"id": "job2"}}

+{"return": {}}

+{}

+{}

+{"execute": "job-dismiss", "arguments": {"id": "job1"}}

+{"return": {}}

+{}

+{}

+{"execute": "job-dismiss", "arguments": {"id": "job2"}}

+{"return": {}}

+{}

+{

+ "execute": "transaction",

+ "arguments": {

+ "actions": [

+ {

+ "data": {

+ "bitmap": "bitmap0",

+ "device": "drive0",

+ "job-id": "j2",

+ "sync": "incremental",

+ "target": "img0-incr"

+ },

+ "type": "blockdev-backup"

+ },

+ {

+ "data": {

+ "bitmap": "bitmap1",

+ "device": "drive1",

+ "job-id": "j3",

+ "sync": "incremental",

+ "target": "img1-incr"

+ },

+ "type": "blockdev-backup"

+ }

+ ]

+ }

+}

+{

+ "return": {}

+}

+{"data": {"device": "j2", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+{"data": {"device": "j3", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+

+--- Done ---

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/group

+++ b/tests/qemu-iotests/group

@@ -XXX,XX +XXX,XX @@

253 rw auto quick

254 rw auto backing quick

255 rw auto quick

+256 rw auto quick

--

2.21.0

From: John Snow <jsnow@redhat.com>

Before, event_match didn't always recurse if the event value was not a

dictionary, and would instead check for equality immediately.

By delaying equality checking to post-recursion, we can allow leaf

values like "5" to match "None" and take advantage of the generic

None-returns-True clause.

This makes the matching a little more obviously consistent at the

expense of being able to check for explicit None values, which is

probably not that important given what this function is used for.

Signed-off-by: John Snow <jsnow@redhat.com>

Message-id: 20190528183857.26167-1-jsnow@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

python/qemu/__init__.py | 24 ++++++++++++++----------

1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py

index XXXXXXX..XXXXXXX 100644

--- a/python/qemu/__init__.py

+++ b/python/qemu/__init__.py

@@ -XXX,XX +XXX,XX @@ class QEMUMachine(object):

The match criteria takes the form of a matching subdict. The event is

checked to be a superset of the subdict, recursively, with matching

- values whenever those values are not None.

+ values whenever the subdict values are not None.

+

+ This has a limitation that you cannot explicitly check for None values.

Examples, with the subdict queries on the left:

- None matches any object.

- {"foo": None} matches {"foo": {"bar": 1}}

- - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}

- - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}

+ - {"foo": None} matches {"foo": 5}

+ - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}

+ - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}

"""

if match is None:

return True

- for key in match:

- if key in event:

- if isinstance(event[key], dict):

+ try:

+ for key in match:

+ if key in event:

if not QEMUMachine.event_match(event[key], match[key]):

return False

- elif event[key] != match[key]:

+ else:

return False

- else:

- return False

- return True

+ return True

+ except TypeError:

+ # either match or event wasn't iterable (not a dict)

+ return match == event

def event_wait(self, name, timeout=60.0, match=None):

"""

--

2.21.0

It is possible for an empty file to take up blocks on a filesystem, for

example:

$ qemu-img create -f raw test.img 1G

Formatting 'test.img', fmt=raw size=1073741824

$ mkfs.ext4 -I 128 -q test.img

$ mkdir test-mount

$ sudo mount -o loop test.img test-mount

$ sudo touch test-mount/test-file

$ stat -c 'blocks=%b' test-mount/test-file

blocks=8

These extra blocks (one cluster) are apparently used for metadata,

because they are always there, on top of blocks used for data:

$ sudo dd if=/dev/zero of=test-mount/test-file bs=1M count=1

1+0 records in

1+0 records out

1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.00135339 s, 775 MB/s

$ stat -c 'blocks=%b' test-mount/test-file

blocks=2056

Make iotest 175 take this into account.

Reported-by: Thomas Huth <thuth@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Nir Soffer <nsoffer@redhat.com>

Message-id: 20190516144319.12570-1-mreitz@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/175 | 26 ++++++++++++++++++++++----

tests/qemu-iotests/175.out | 8 ++++----

2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/175

+++ b/tests/qemu-iotests/175

@@ -XXX,XX +XXX,XX @@ status=1    # failure is the default!

_cleanup()

{

-    _cleanup_test_img

+ _cleanup_test_img

+ rm -f "$TEST_DIR/empty"

}

trap "_cleanup; exit \$status" 0 1 2 3 15

+# Some file systems sometimes allocate extra blocks independently of

+# the file size. This function hides the resulting difference in the

+# stat -c '%b' output.

+# Parameter 1: Number of blocks an empty file occupies

+# Parameter 2: Image size in bytes

+_filter_blocks()

+{

+ extra_blocks=$1

+ img_size=$2

+

+ sed -e "s/blocks=$extra_blocks\\(\$\\|[^0-9]\\)/nothing allocated/" \

+ -e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/everything allocated/"

+}

+

# get standard environment, filters and checks

. ./common.rc

. ./common.filter

@@ -XXX,XX +XXX,XX @@ _supported_fmt raw

_supported_proto file

_supported_os Linux

-size=1m

+size=$((1 * 1024 * 1024))

+

+touch "$TEST_DIR/empty"

+extra_blocks=$(stat -c '%b' "$TEST_DIR/empty")

echo

echo "== creating image with default preallocation =="

_make_test_img $size | _filter_imgfmt

-stat -c "size=%s, blocks=%b" $TEST_IMG

+stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size

for mode in off full falloc; do

echo

echo "== creating image with preallocation $mode =="

IMGOPTS=preallocation=$mode _make_test_img $size | _filter_imgfmt

- stat -c "size=%s, blocks=%b" $TEST_IMG

+ stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size

done

# success, all done

diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/175.out

+++ b/tests/qemu-iotests/175.out

@@ -XXX,XX +XXX,XX @@ QA output created by 175

== creating image with default preallocation ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576

-size=1048576, blocks=0

+size=1048576, nothing allocated

== creating image with preallocation off ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off

-size=1048576, blocks=0

+size=1048576, nothing allocated

== creating image with preallocation full ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full

-size=1048576, blocks=2048

+size=1048576, everything allocated

== creating image with preallocation falloc ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc

-size=1048576, blocks=2048

+size=1048576, everything allocated

*** done

--

2.21.0

In 219, we wait for the job to make progress before we emit its status.

This makes the output reliable. We do not wait for any more progress if

the job's current-progress already matches its total-progress.

Unfortunately, there is a bug: Right after the job has been started,

it's possible that total-progress is still 0. In that case, we may skip

the first progress-making step and keep ending up 64 kB short.

To fix that bug, we can simply wait for total-progress to reach 4 MB

(the image size) after starting the job.

Reported-by: Karen Mezick <kmezick@redhat.com>

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

Signed-off-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190516161114.27596-1-mreitz@redhat.com

Reviewed-by: John Snow <jsnow@redhat.com>

[mreitz: Adjusted commit message as per John's proposal]

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/219 | 13 ++++++++++---

1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/219 b/tests/qemu-iotests/219

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/219

+++ b/tests/qemu-iotests/219

@@ -XXX,XX +XXX,XX @@ import iotests

iotests.verify_image_format(supported_fmts=['qcow2'])

+img_size = 4 * 1024 * 1024

+

def pause_wait(vm, job_id):

with iotests.Timeout(3, "Timeout waiting for job to pause"):

while True:

@@ -XXX,XX +XXX,XX @@ def test_pause_resume(vm):

iotests.log(vm.qmp('query-jobs'))

def test_job_lifecycle(vm, job, job_args, has_ready=False):

+ global img_size

+

iotests.log('')

iotests.log('')

iotests.log('Starting block job: %s (auto-finalize: %s; auto-dismiss: %s)' %

@@ -XXX,XX +XXX,XX @@ def test_job_lifecycle(vm, job, job_args, has_ready=False):

iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))

iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))

+ # Wait for total-progress to stabilize

+ while vm.qmp('query-jobs')['return'][0]['total-progress'] < img_size:

+ pass

+

# RUNNING state:

# pause/resume should work, complete/finalize/dismiss should error out

iotests.log('')

@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('disk.img') as disk_path, \

iotests.FilePath('copy.img') as copy_path, \

iotests.VM() as vm:

- img_size = '4M'

- iotests.qemu_img_create('-f', iotests.imgfmt, disk_path, img_size)

- iotests.qemu_io('-c', 'write 0 %s' % (img_size),

+ iotests.qemu_img_create('-f', iotests.imgfmt, disk_path, str(img_size))

+ iotests.qemu_io('-c', 'write 0 %i' % (img_size),

'-f', iotests.imgfmt, disk_path)

iotests.log('Launching VM...')

--

2.21.0

From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

The uninitialized memory allocated for the command FIFO of the

floppy controller during the VM hardware initialization incurs

many unwanted reports by Valgrind when VM state is being saved.

That verbosity hardens a search for the real memory issues when

the iotests run. Particularly, the patch eliminates 20 unnecessary

reports of the Valgrind tool in the iotest #169.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Message-id: 1559154027-282547-1-git-send-email-andrey.shinkevich@virtuozzo.com

Reviewed-by: John Snow <jsnow@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

hw/block/fdc.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/fdc.c

+++ b/hw/block/fdc.c

@@ -XXX,XX +XXX,XX @@ static void fdctrl_realize_common(DeviceState *dev, FDCtrl *fdctrl,

FLOPPY_DPRINTF("init controller\n");

fdctrl->fifo = qemu_memalign(512, FD_SECTOR_LEN);

+ memset(fdctrl->fifo, 0, FD_SECTOR_LEN);

fdctrl->fifo_size = 512;

fdctrl->result_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,

fdctrl_result_timer, fdctrl);

--

2.21.0

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Test fails at least for qcow, because of different cluster sizes in

base and top (and therefore different granularities of bitmaps we are

trying to merge).

The test aim is to check block-dirty-bitmap-merge between different

nodes functionality, no needs to check all formats. So, let's just drop

support for anything except qcow2.

Reported-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Message-id: 20190605155405.104384-1-vsementsov@virtuozzo.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/254 | 2 ++

1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/254

+++ b/tests/qemu-iotests/254

@@ -XXX,XX +XXX,XX @@

import iotests

from iotests import qemu_img_create, file_path, log

+iotests.verify_image_format(supported_fmts=['qcow2'])

+

disk, top = file_path('disk', 'top')

size = 1024 * 1024

--

2.21.0

img_rebase() can leak a QDict in two occasions. Fix it.

Coverity: CID 1401416

Fixes: d16699b64671466b42079c45b89127aeea1ca565

Fixes: 330c72957196e0ae382abcaa97ebf4eb9bc8574f

Signed-off-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190528195338.12376-1-mreitz@redhat.com

Reviewed-by: John Snow <jsnow@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

qemu-img.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)

out_baseimg,

&local_err);

if (local_err) {

+ qobject_unref(options);

error_reportf_err(local_err,

"Could not resolve backing filename: ");

ret = -1;

@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)

*/

prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);

if (prefix_chain_bs) {

+ qobject_unref(options);

g_free(out_real_path);

+

blk_new_backing = blk_new(qemu_get_aio_context(),

BLK_PERM_CONSISTENT_READ,

BLK_PERM_ALL);

--

2.21.0

A snapshot is something that reflects the state of something at a

certain point in time. It does not change.

The file our snapshot commands create (or the node they install) is not

a snapshot, as it does change over time. It is an overlay. We cannot

do anything about the parameter names, but we can at least adjust the

descriptions to reflect that fact.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Message-id: 20190603202236.1342-2-mreitz@redhat.com

Reviewed-by: John Snow <jsnow@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

qapi/block-core.json | 20 ++++++++++----------

1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json

index XXXXXXX..XXXXXXX 100644

--- a/qapi/block-core.json

+++ b/qapi/block-core.json

@@ -XXX,XX +XXX,XX @@

#

# Either @device or @node-name must be set but not both.

#

-# @device: the name of the device to generate the snapshot from.

+# @device: the name of the device to take a snapshot of.

#

# @node-name: graph node name to generate the snapshot from (Since 2.0)

#

-# @snapshot-file: the target of the new image. If the file exists, or

-# if it is a device, the snapshot will be created in the existing

-# file/device. Otherwise, a new file will be created.

+# @snapshot-file: the target of the new overlay image. If the file

+# exists, or if it is a device, the overlay will be created in the

+# existing file/device. Otherwise, a new file will be created.

#

# @snapshot-node-name: the graph node name of the new image (Since 2.0)

#

-# @format: the format of the snapshot image, default is 'qcow2'.

+# @format: the format of the overlay image, default is 'qcow2'.

#

# @mode: whether and how QEMU should create a new image, default is

# 'absolute-paths'.

@@ -XXX,XX +XXX,XX @@

##

# @BlockdevSnapshot:

#

-# @node: device or node name that will have a snapshot created.

+# @node: device or node name that will have a snapshot taken.

#

# @overlay: reference to the existing block device that will become

-# the overlay of @node, as part of creating the snapshot.

+# the overlay of @node, as part of taking the snapshot.

# It must not have a current backing file (this can be

# achieved by passing "backing": null to blockdev-add).

#

@@ -XXX,XX +XXX,XX @@

##

# @blockdev-snapshot-sync:

#

-# Generates a synchronous snapshot of a block device.

+# Takes a synchronous snapshot of a block device.

#

# For the arguments, see the documentation of BlockdevSnapshotSync.

#

@@ -XXX,XX +XXX,XX @@

##

# @blockdev-snapshot:

#

-# Generates a snapshot of a block device.

+# Takes a snapshot of a block device.

#

-# Create a snapshot, by installing 'node' as the backing image of

+# Take a snapshot, by installing 'node' as the backing image of

# 'overlay'. Additionally, if 'node' is associated with a block

# device, the block device changes to using 'overlay' as its new active

# image.

--

2.21.0

There are error messages which refer to an overlay node as the snapshot.

That is wrong, those are two different things.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Message-id: 20190603202236.1342-3-mreitz@redhat.com

Reviewed-by: John Snow <jsnow@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

blockdev.c | 10 +++++-----

tests/qemu-iotests/085.out | 10 +++++-----

2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/blockdev.c b/blockdev.c

index XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

s->has_snapshot_node_name ? s->snapshot_node_name : NULL;

if (node_name && !snapshot_node_name) {

- error_setg(errp, "New snapshot node name missing");

+ error_setg(errp, "New overlay node name missing");

goto out;

}

if (snapshot_node_name &&

bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {

- error_setg(errp, "New snapshot node name already in use");

+ error_setg(errp, "New overlay node name already in use");

goto out;

}

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

}

if (bdrv_has_blk(state->new_bs)) {

- error_setg(errp, "The snapshot is already in use");

+ error_setg(errp, "The overlay is already in use");

goto out;

}

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

}

if (state->new_bs->backing != NULL) {

- error_setg(errp, "The snapshot already has a backing image");

+ error_setg(errp, "The overlay already has a backing image");

goto out;

}

if (!state->new_bs->drv->supports_backing) {

- error_setg(errp, "The snapshot does not support backing images");

+ error_setg(errp, "The overlay does not support backing images");

goto out;

}

diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/085.out

+++ b/tests/qemu-iotests/085.out

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/

=== Invalid command - cannot create a snapshot using a file BDS ===

-{"error": {"class": "GenericError", "desc": "The snapshot does not support backing images"}}

+{"error": {"class": "GenericError", "desc": "The overlay does not support backing images"}}

=== Invalid command - snapshot node used as active layer ===

-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}

-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}

-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}

+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}

+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}

+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}

=== Invalid command - snapshot node used as backing hd ===

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/

Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base

{"return": {}}

-{"error": {"class": "GenericError", "desc": "The snapshot already has a backing image"}}

+{"error": {"class": "GenericError", "desc": "The overlay already has a backing image"}}

=== Invalid command - The node does not exist ===

--

2.21.0

Move img_convert()'s quiet flag into the ImgConvertState so it is

accessible by nested functions. -q dictates that it suppresses anything

but errors, so if those functions want to emit warnings, they need to

query this flag first. (There currently are no such warnings, but there

will be as of the next patch.)

Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Message-id: 20190507203508.18026-2-mreitz@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

qemu-img.c | 13 +++++++------

1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ typedef struct ImgConvertState {

int64_t target_backing_sectors; /* negative if unknown */

bool wr_in_order;

bool copy_range;

+ bool quiet;

int min_sparse;

int alignment;

size_t cluster_sectors;

@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)

QDict *open_opts = NULL;

char *options = NULL;

Error *local_err = NULL;

- bool writethrough, src_writethrough, quiet = false, image_opts = false,

+ bool writethrough, src_writethrough, image_opts = false,

skip_create = false, progress = false, tgt_image_opts = false;

int64_t ret = -EINVAL;

bool force_share = false;

@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)

src_cache = optarg;

break;

case 'q':

- quiet = true;

+ s.quiet = true;

break;

case 'n':

skip_create = true;

@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)

}

/* Initialize before goto out */

- if (quiet) {

+ if (s.quiet) {

progress = false;

}

qemu_progress_init(progress, 1.0);

@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)

for (bs_i = 0; bs_i < s.src_num; bs_i++) {

s.src[bs_i] = img_open(image_opts, argv[optind + bs_i],

- fmt, src_flags, src_writethrough, quiet,

+ fmt, src_flags, src_writethrough, s.quiet,

force_share);

if (!s.src[bs_i]) {

ret = -1;