Re: [Qemu-block] [PATCH v2 00/16] qapi: Allow blockdev-add for NBD

[Max Reitz]

On 01.03.2016 00:24, Eric Blake wrote:
> On 02/29/2016 04:19 PM, Max Reitz wrote:
>> Turns out NBD is not so simple to do if you do it right. Anyway, this
>> series adds blockdev-add support for NBD clients.
>>
>> Patches 1 and 2 add one less and one more complicated QDict function,
>> respectively, which I needed in later NBD patches: Patch 1 for handling
>> legacy options (move "host" to "address.data.host" etc.) and patch 2
>> because I'd like to use the input visitor for transforming the NBD
>> options into a SocketAddress. Unfortunately, the block layer uses
>> flattened QDicts everywhere, so we'll have to unflatten (erect?) them
>> before we can use that visitor.
> 
> Dan had a patch proposal that called the operation "crumple"; I need to
> review both proposals and see which one I like.
> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg04618.html

Well, here I go again, not looking at patches on the list...

Looking at the design, I like his idea of having an escape sequence;
also, his qdict_crumple() can return boths lists and dicts where my
qdict_unflatten() only returns dicts (then again, this is what
qdict_flatten() always works on). And his patch doesn't suffer from as
much indentation as mine does.

What I like more about my patch, however, is that I'm reusing
qdict_array_split() and qdict_array_entries(). That is mostly because my
function modifies the given QDict, where Dan's does not.

>>
>> Patch 3 adds a test for qdict_unflatten().
>>
>> Patches 4, 5, 6, and 7 are minor patches with no functional relation to
>> this series, other than later patches will touch the code they touch,
>> too.
>>
>> Patches 8 and 9 prepare the code for the addition of a new option
>> prefix, which is "address.".
>>
>> Patch 10 makes the NBD client accept a SocketAddress under the "address"
>> option (or rather, a flattened SocketAddress QDict with its keys
>> prefixed by "address."). The old options "host", "port", and "path" are
>> supported as legacy options and translated to the respective
>> SocketAddress representation.
>>
>> Patch 11 drops usage of "host", "port", and "path" outside of
>> nbd_has_filename_options_conflict(),
>> nbd_process_legacy_socket_options(), and nbd_refresh_filename(), making
>> those options nothing but legacy.
>>
>> Patch 12, the goal of this series, is again not very complicated.
>>
>> Patches 13, 14, and 15 are required for the iotest added in patch 16. It
>> will invoke qemu-nbd, so patch 13 is required. Besides qemu-nbd, it will
>> launch an NBD server VM concurrently to the client VM, which is why
>> patch 14 is required. And finally, it will test whether we can add an
>> NBD BDS by passing it a file descriptor, which patch 15 is needed for
>> (so we use the socket_scm_helper to pass sockets to qemu).
>>
>> Patch 16 then adds the iotest for NBD's blockdev-add interface.
>>
>>
>> Note on the relation to v1: As you can see, most of this series is
>> completely new. Patch 5 was patch 1 in v1, and the only thing that has
>> changed is that I removed the full stop at the end of the error message;
>> consequently I kept Eric's R-b.
> 
> Looks like my review queue is getting longer because I (like several
> other people) are trying to post last-minute series before soft freeze.

So is mine. :-)

Yes, during the last week I had to prioritize patches over reviewing.

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [PATCH v2 16/16] iotests: Add test for NBD's blockdev-add interface

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/147 | 194 +
 tests/qemu-iotests/147.out |   5 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 200 insertions(+)
 create mode 100755 tests/qemu-iotests/147
 create mode 100644 tests/qemu-iotests/147.out

diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
new file mode 100755
index 000..f31de69
--- /dev/null
+++ b/tests/qemu-iotests/147
@@ -0,0 +1,194 @@
+#!/usr/bin/env python
+#
+# Test case for the QMP 'change' command and all other associated
+# commands
+#
+# Copyright (C) 2016 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 .
+#
+
+import os
+import socket
+import stat
+import time
+import iotests
+from iotests import cachemode, imgfmt, qemu_img, qemu_nbd
+
+NBD_PORT = 10811
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+
+class NBDBlockdevAddBase(iotests.QMPTestCase):
+def blockdev_add_options(self, address, export=None):
+options = { 'id': 'drive0',
+'driver': 'raw',
+'file': {
+'driver': 'nbd',
+'address': address
+} }
+if export is not None:
+options['file']['export'] = export
+return options
+
+def client_test(self, filename, address, export=None):
+bao = self.blockdev_add_options(address, export)
+result = self.vm.qmp('blockdev-add', options=bao)
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/inserted/image/filename', filename)
+
+result = self.vm.qmp('x-blockdev-del', id='drive0')
+self.assert_qmp(result, 'return', {})
+
+
+class QemuNBD(NBDBlockdevAddBase):
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
+self.vm = iotests.VM()
+self.vm.launch()
+self.qemu_nbd = None
+
+def tearDown(self):
+self.vm.shutdown()
+if self.qemu_nbd is not None:
+self.qemu_nbd.wait()
+os.remove(test_img)
+
+def _server_up(self, *args):
+self.qemu_nbd = qemu_nbd('-f', imgfmt, test_img, *args)
+time.sleep(1)
+
+def test_inet(self):
+self._server_up('-p', str(NBD_PORT))
+address = { 'type': 'inet',
+'data': {
+'host': 'localhost',
+'port': str(NBD_PORT)
+} }
+self.client_test('nbd://localhost:%i' % NBD_PORT, address)
+
+def test_unix(self):
+socket = os.path.join(iotests.test_dir, 'qemu-nbd.socket')
+self._server_up('-k', socket)
+address = { 'type': 'unix',
+'data': { 'path': socket } }
+self.client_test('nbd+unix://?socket=' + socket, address)
+os.remove(socket)
+
+
+class BuiltinNBD(NBDBlockdevAddBase):
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
+self.vm = iotests.VM()
+self.vm.launch()
+self.server = iotests.VM('.server')
+self.server.add_drive_raw('if=none,id=nbd-export,' +
+  'file=%s,' % test_img +
+  'format=%s,' % imgfmt +
+  'cache=%s' % cachemode)
+self.server.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+self.server.shutdown()
+os.remove(test_img)
+
+def _server_up(self, address):
+result = self.server.qmp('nbd-server-start', addr=address)
+self.assert_qmp(result, 'return', {})
+
+result = self.server.qmp('nbd-server-add', device='nbd-export')
+self.assert_qmp(result, 'return', {})
+
+def _server_down(self):
+result = self.server.qmp('nbd-server-stop')
+self.assert_qmp(result, 'return', {})
+
+def test_inet(self):
+address = { 'type': 'inet',
+'data': {
+'host': 'localhost',
+'port': str(NBD_PORT)
+} }
+self._server_up(address)
+self.client_test('nbd://localhost:%i/nbd-export' % NBD_PORT,
+ address, 'nbd-export')
+self._server_down()
+
+def test_inet6(self):
+address = { 'type': 'inet'

Re: [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields()

[John Snow]

On 02/29/2016 09:36 AM, Kevin Wolf wrote:
> Hi all,
> 
> I'm currently trying to get rid of bdrv_move_feature_fields(), so we can
> finally have more than one BB per BDS. Generally the way to do this is
> to move features from BDS and block.c to BB and block-backend.c.
> However, for two of the features I'm not sure about this:
> 
> * Copy on Read:
> 
>   When Jeff introduced bdrv_append() in commit 8802d1fd, the CoR flag
>   was already moved to the new top level when taking a snapshot. Does
>   anyone remember why it works like that? It doesn't seem to make a lot
>   of sense to me.
> 
>   The use case for manually enabled CoR is to avoid reading data twice
>   from a slow remote image, so we want to save it to a local overlay,
>   say an ISO image accessed via HTTP to a local qcow2 overlay. When
>   taking a snapshot, we end up with a backing chain like this:
> 
>   http <- local.qcow2 <- snap_overlay.qcow2
> 
>   There is no point in performing copy on read from local.qcow2 into
>   snap_overlay.qcow2, we just want to keep copying data from the remote
>   source into local.qcow2.
> 
>   Possible caveat: We would be writing to a backing file, but that's
>   similar to what some block jobs do, so if we design our op blockers to
>   cover this case, it should be fine.
> 
>   I'm actually pretty sure that simply removing COR from the list, and
>   therefore changing the behaviour to not move it to the top any more,
>   is the right thing to do and could be considered a bug fix.
> 
> * Dirty bitmaps:
> 
>   We're currently trying, and if I'm not mistaken failing, to move dirty
>   bitmaps to the top. The (back then one) bitmap was first added to the
>   list in Paolo's commit a9fc4408, with the following commit message:
> 
> While these should not be in use at the time a transaction is
> started, a command in the prepare phase of a transaction might have
> added them, so they need to be brought over.
> 
>   At that point, there was no transactionable command that did this in
>   the prepare phase. Today we have mirror and backup, but op blockers
>   should prevent them from being mixed with snapshots in a single
>   transaction, so I can't see how this change had any effect.
> 
>   The reason why I think we're failing to move dirty bitmaps to the top
>   today is that we're moving the head of the list to a different object
>   without updating the prev link in the first element, so in any case
>   it's buggy today.
> 
>   I really would like to keep bitmaps on the BDS where they are, but
>   unfortunately, we also have user-defined bitmaps by now, and if we
>   change whether they stick with the top level, that's a change that is
>   visible on the QMP interface.
> 
>   On the other hand, the QMP interface clearly describes bitmaps as
>   belonging to a node rather than a BB (you can use node-name, even with
>   no BB attached), so moving them could be considered a bug, even if
>   it is the existing behaviour.
> 

Rule of thumb: follow the node such that the (node, name) pair given at
creation time continues to work to identify the bitmap.

For bitmaps created using device names this means floating to the top so
that e.g. (drive0, bitmap0) continues to work and address what you think
it does.

For intermediary nodes or nodes explicitly referenced by their node name
instead of their device name, this means it should stick to the node.

All of the BlockDirtyBitmap commands use block_dirty_bitmap_lookup for
the resolution, which identifies the BDS with bdrv_lookup_bs(node, node,
NULL) and then tries to find the bitmap on that singular BDS with
bdrv_find_dirty_bitmap(bs, name).

For bitmaps created with device names, this means that if the bitmaps
don't float up, we actually lose addressability to the bitmap.

This may imply a new `bool float` property to be filled in at name
resolution time.

>   I can imagine use cases for both ways, so the interface that would
>   make the most sense to me is to generally keep BDSes at their node,
>   and to provide a QMP command to move them to a different one.
> 
>   With compatibility in mind, this seems to be a reall tough one,
>   though.
> 

I am sure that exceptions, as always, exist. If this breaks any behavior
I am more than willing to say that this is simply a necessary bugfix and
document accordingly. We do not even have any libvirt support yet, and
half of the feature is still being actively concocted right now.

I think we have some right to change behavior for this feature still.

> Any comments or ideas how to proceed with those two?
> 
> Kevin
> 

I can draft something up if you'd like.
--js

[Qemu-block] [PATCH v2 12/16] qapi: Allow blockdev-add for NBD

[Max Reitz]

Signed-off-by: Max Reitz 
---
 qapi/block-core.json | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9bf1b22..21760e0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1633,13 +1633,14 @@
 # Drivers that are supported in block device operations.
 #
 # @host_device, @host_cdrom: Since 2.1
+# @nbd: Since 2.6
 #
 # Since: 2.0
 ##
 { 'enum': 'BlockdevDriver',
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
-'http', 'https', 'null-aio', 'null-co', 'parallels',
+'http', 'https', 'nbd', 'null-aio', 'null-co', 'parallels',
 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
 'vmdk', 'vpc', 'vvfat' ] }
 
@@ -2068,6 +2069,24 @@
 '*read-pattern': 'QuorumReadPattern' } }
 
 ##
+# @BlockdevOptionsNbd
+#
+# Driver specific block device options for NBD.
+#
+# @address: NBD server address
+#
+# @export:  #optional export name
+#
+# @tls-creds:   #optional TLS credentials ID
+#
+# Since: 2.6
+##
+{ 'struct': 'BlockdevOptionsNbd',
+  'data': { 'address': 'SocketAddress',
+'*export': 'str',
+'*tls-creds': 'str' } }
+
+##
 # @BlockdevOptions
 #
 # Options for creating a block device.
@@ -2093,7 +2112,7 @@
   'http':   'BlockdevOptionsFile',
   'https':  'BlockdevOptionsFile',
 # TODO iscsi: Wait for structured options
-# TODO nbd: Should take InetSocketAddress for 'host'?
+  'nbd':'BlockdevOptionsNbd',
 # TODO nfs: Wait for structured options
   'null-aio':   'BlockdevOptionsNull',
   'null-co':'BlockdevOptionsNull',
-- 
2.7.1

Re: [Qemu-block] [PATCH v2 00/16] qapi: Allow blockdev-add for NBD

[Eric Blake]

On 02/29/2016 04:19 PM, Max Reitz wrote:
> Turns out NBD is not so simple to do if you do it right. Anyway, this
> series adds blockdev-add support for NBD clients.
> 
> Patches 1 and 2 add one less and one more complicated QDict function,
> respectively, which I needed in later NBD patches: Patch 1 for handling
> legacy options (move "host" to "address.data.host" etc.) and patch 2
> because I'd like to use the input visitor for transforming the NBD
> options into a SocketAddress. Unfortunately, the block layer uses
> flattened QDicts everywhere, so we'll have to unflatten (erect?) them
> before we can use that visitor.

Dan had a patch proposal that called the operation "crumple"; I need to
review both proposals and see which one I like.
https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg04618.html

> 
> Patch 3 adds a test for qdict_unflatten().
> 
> Patches 4, 5, 6, and 7 are minor patches with no functional relation to
> this series, other than later patches will touch the code they touch,
> too.
> 
> Patches 8 and 9 prepare the code for the addition of a new option
> prefix, which is "address.".
> 
> Patch 10 makes the NBD client accept a SocketAddress under the "address"
> option (or rather, a flattened SocketAddress QDict with its keys
> prefixed by "address."). The old options "host", "port", and "path" are
> supported as legacy options and translated to the respective
> SocketAddress representation.
> 
> Patch 11 drops usage of "host", "port", and "path" outside of
> nbd_has_filename_options_conflict(),
> nbd_process_legacy_socket_options(), and nbd_refresh_filename(), making
> those options nothing but legacy.
> 
> Patch 12, the goal of this series, is again not very complicated.
> 
> Patches 13, 14, and 15 are required for the iotest added in patch 16. It
> will invoke qemu-nbd, so patch 13 is required. Besides qemu-nbd, it will
> launch an NBD server VM concurrently to the client VM, which is why
> patch 14 is required. And finally, it will test whether we can add an
> NBD BDS by passing it a file descriptor, which patch 15 is needed for
> (so we use the socket_scm_helper to pass sockets to qemu).
> 
> Patch 16 then adds the iotest for NBD's blockdev-add interface.
> 
> 
> Note on the relation to v1: As you can see, most of this series is
> completely new. Patch 5 was patch 1 in v1, and the only thing that has
> changed is that I removed the full stop at the end of the error message;
> consequently I kept Eric's R-b.

Looks like my review queue is getting longer because I (like several
other people) are trying to post last-minute series before soft freeze.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [PATCH v2 11/16] block/nbd: Use SocketAddress options

[Max Reitz]

Drop the use of legacy options in favor of the SocketAddress
representation, even for internal use (i.e. for storing the result of
the filename parsing).

Signed-off-by: Max Reitz 
---
 block/nbd.c | 34 +-
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 213ba70..471395a 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -88,9 +88,13 @@ static int nbd_parse_uri(const char *filename, QDict 
*options)
 ret = -EINVAL;
 goto out;
 }
-qdict_put(options, "path", qstring_from_str(qp->p[0].value));
+qdict_put(options, "address.type", qstring_from_str("unix"));
+qdict_put(options, "address.data.path",
+  qstring_from_str(qp->p[0].value));
 } else {
 QString *host;
+char *port_str;
+
 /* nbd[+tcp]://host[:port]/export */
 if (!uri->server) {
 ret = -EINVAL;
@@ -105,12 +109,12 @@ static int nbd_parse_uri(const char *filename, QDict 
*options)
 host = qstring_from_str(uri->server);
 }
 
-qdict_put(options, "host", host);
-if (uri->port) {
-char* port_str = g_strdup_printf("%d", uri->port);
-qdict_put(options, "port", qstring_from_str(port_str));
-g_free(port_str);
-}
+qdict_put(options, "address.type", qstring_from_str("inet"));
+qdict_put(options, "address.data.host", host);
+
+port_str = g_strdup_printf("%d", uri->port ?: NBD_DEFAULT_PORT);
+qdict_put(options, "address.data.port", qstring_from_str(port_str));
+g_free(port_str);
 }
 
 out:
@@ -187,7 +191,8 @@ static void nbd_parse_filename(const char *filename, QDict 
*options,
 
 /* are we a UNIX or TCP socket? */
 if (strstart(host_spec, "unix:", &unixpath)) {
-qdict_put(options, "path", qstring_from_str(unixpath));
+qdict_put(options, "address.type", qstring_from_str("unix"));
+qdict_put(options, "address.data.path", qstring_from_str(unixpath));
 } else {
 InetSocketAddress *addr = NULL;
 
@@ -196,8 +201,9 @@ static void nbd_parse_filename(const char *filename, QDict 
*options,
 goto out;
 }
 
-qdict_put(options, "host", qstring_from_str(addr->host));
-qdict_put(options, "port", qstring_from_str(addr->port));
+qdict_put(options, "address.type", qstring_from_str("inet"));
+qdict_put(options, "address.data.host", qstring_from_str(addr->host));
+qdict_put(options, "address.data.port", qstring_from_str(addr->port));
 qapi_free_InetSocketAddress(addr);
 }
 
@@ -506,10 +512,12 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 }
 } else {
 if (path) {
-qdict_put(opts, "path", qstring_from_str(path));
+qdict_put(opts, "address.type", qstring_from_str("unix"));
+qdict_put(opts, "address.data.path", qstring_from_str(path));
 } else {
-qdict_put(opts, "host", qstring_from_str(host));
-qdict_put(opts, "port", qstring_from_str(port));
+qdict_put(opts, "address.type", qstring_from_str("inet"));
+qdict_put(opts, "address.data.host", qstring_from_str(host));
+qdict_put(opts, "address.data.port", qstring_from_str(port));
 }
 }
 if (export) {
-- 
2.7.1

[Qemu-block] [PATCH v2 13/16] iotests.py: Add qemu_nbd function

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 0a238ec..dd8805a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -28,7 +28,7 @@ import qmp
 import qtest
 import struct
 
-__all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io',
+__all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io', 'qemu_nbd',
'VM', 'QMPTestCase', 'notrun', 'main']
 
 # This will not work if arguments contain spaces but is necessary if we
@@ -41,6 +41,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
 if os.environ.get('QEMU_IO_OPTIONS'):
 qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
 
+qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
+if os.environ.get('QEMU_NBD_OPTIONS'):
+qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
+
 qemu_args = [os.environ.get('QEMU_PROG', 'qemu')]
 if os.environ.get('QEMU_OPTIONS'):
 qemu_args += os.environ['QEMU_OPTIONS'].strip().split(' ')
@@ -86,6 +90,11 @@ def qemu_io(*args):
 sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' 
'.join(args)))
 return subp.communicate()[0]
 
+def qemu_nbd(*args):
+'''Run qemu-nbd in the background'''
+subp = subprocess.Popen(qemu_nbd_args + list(args))
+return subp
+
 def compare_images(img1, img2):
 '''Return True if two image files are identical'''
 return qemu_img('compare', '-f', imgfmt,
-- 
2.7.1

[Qemu-block] [PATCH v2 14/16] iotests.py: Allow concurrent qemu instances

[Max Reitz]

By adding an optional suffix to the files used for communication with a
VM, we can launch multiple VM instances concurrently.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index dd8805a..fed5301 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -130,10 +130,13 @@ def event_match(event, match=None):
 class VM(object):
 '''A QEMU VM'''
 
-def __init__(self):
-self._monitor_path = os.path.join(test_dir, 'qemu-mon.%d' % 
os.getpid())
-self._qemu_log_path = os.path.join(test_dir, 'qemu-log.%d' % 
os.getpid())
-self._qtest_path = os.path.join(test_dir, 'qemu-qtest.%d' % 
os.getpid())
+def __init__(self, path_suffix=''):
+self._monitor_path = os.path.join(test_dir, 'qemu-mon%s.%d' %
+(path_suffix, os.getpid()))
+self._qemu_log_path = os.path.join(test_dir, 'qemu-log%s.%d' %
+ (path_suffix, 
os.getpid()))
+self._qtest_path = os.path.join(test_dir, 'qemu-qtest%s.%d' %
+  (path_suffix, os.getpid()))
 self._args = qemu_args + ['-chardev',
  'socket,id=mon,path=' + self._monitor_path,
  '-mon', 'chardev=mon,mode=control',
-- 
2.7.1

[Qemu-block] [PATCH v2 04/16] block/nbd: Drop trailing "." in error messages

[Max Reitz]

Signed-off-by: Max Reitz 
---
 block/nbd.c   | 4 ++--
 tests/qemu-iotests/051.out| 4 ++--
 tests/qemu-iotests/051.pc.out | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index db57b49..ce31119 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -194,9 +194,9 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict 
*options, char **export,
 
 if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
 if (qdict_haskey(options, "path")) {
-error_setg(errp, "path and host may not be used at the same 
time.");
+error_setg(errp, "path and host may not be used at the same time");
 } else {
-error_setg(errp, "one of path and host must be specified.");
+error_setg(errp, "one of path and host must be specified");
 }
 return NULL;
 }
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 0f8a8d3..5166bea 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -222,7 +222,7 @@ Testing: -drive driver=file
 QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name
 
 Testing: -drive driver=nbd
-QEMU_PROG: -drive driver=nbd: one of path and host must be specified.
+QEMU_PROG: -drive driver=nbd: one of path and host must be specified
 
 Testing: -drive driver=raw
 QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the 
protocol level
@@ -231,7 +231,7 @@ Testing: -drive file.driver=file
 QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file 
name
 
 Testing: -drive file.driver=nbd
-QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified.
+QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified
 
 Testing: -drive file.driver=raw
 QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the 
protocol level
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 85fc05d..d0bbfcb 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -316,7 +316,7 @@ Testing: -drive driver=file
 QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name
 
 Testing: -drive driver=nbd
-QEMU_PROG: -drive driver=nbd: one of path and host must be specified.
+QEMU_PROG: -drive driver=nbd: one of path and host must be specified
 
 Testing: -drive driver=raw
 QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the 
protocol level
@@ -325,7 +325,7 @@ Testing: -drive file.driver=file
 QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file 
name
 
 Testing: -drive file.driver=nbd
-QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified.
+QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified
 
 Testing: -drive file.driver=raw
 QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the 
protocol level
-- 
2.7.1

[Qemu-block] [PATCH v2 09/16] block/nbd: "address" in nbd_refresh_filename()

[Max Reitz]

As of a future patch, the NBD block driver will accept a SocketAddress
structure for a new "address" option. In order to support this,
nbd_refresh_filename() needs some changes.

The two TODOs introduced by this patch will be removed in the very next
one. They exist to explain that it is currently impossible for
nbd_refresh_filename() to emit an "address.*" option (which the NBD
block driver does not handle yet). The next patch will arm these code
paths, but it will also enable handling of these options.

Signed-off-by: Max Reitz 
---
 block/nbd.c | 80 +
 1 file changed, 59 insertions(+), 21 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 2d96dd1..86907bc 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -419,37 +419,75 @@ static void nbd_attach_aio_context(BlockDriverState *bs,
 static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
 {
 QDict *opts = qdict_new();
-const char *path   = qdict_get_try_str(options, "path");
-const char *host   = qdict_get_try_str(options, "host");
-const char *port   = qdict_get_try_str(options, "port");
+bool can_generate_filename = true;
+const char *path = NULL, *host = NULL, *port = NULL;
 const char *export = qdict_get_try_str(options, "export");
 const char *tlscreds = qdict_get_try_str(options, "tls-creds");
 
-if (host && !port) {
-port = stringify(NBD_DEFAULT_PORT);
+if (qdict_get_try_str(options, "address.type")) {
+/* This path will only be possible as of a future patch;
+ * TODO: Remove this note once it is */
+
+const char *type = qdict_get_str(options, "address.type");
+
+if (!strcmp(type, "unix")) {
+path = qdict_get_str(options, "address.data.path");
+} else if (!strcmp(type, "inet")) {
+host = qdict_get_str(options, "address.data.host");
+port = qdict_get_str(options, "address.data.port");
+
+can_generate_filename = !qdict_haskey(options, "address.data.to")
+ && !qdict_haskey(options, "address.data.ipv4")
+ && !qdict_haskey(options, 
"address.data.ipv6");
+} else {
+can_generate_filename = false;
+}
+} else {
+path = qdict_get_try_str(options, "path");
+host = qdict_get_try_str(options, "host");
+port = qdict_get_try_str(options, "port");
+
+if (host && !port) {
+port = stringify(NBD_DEFAULT_PORT);
+}
 }
 
 qdict_put(opts, "driver", qstring_from_str("nbd"));
 
-if (path && export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd+unix:///%s?socket=%s", export, path);
-} else if (path && !export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd+unix://?socket=%s", path);
-} else if (!path && export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s:%s/%s", host, port, export);
-} else if (!path && !export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s:%s", host, port);
+if (can_generate_filename) {
+if (path && export) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd+unix:///%s?socket=%s", export, path);
+} else if (path && !export) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd+unix://?socket=%s", path);
+} else if (!path && export) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd://%s:%s/%s", host, port, export);
+} else if (!path && !export) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd://%s:%s", host, port);
+}
 }
 
-if (path) {
-qdict_put(opts, "path", qstring_from_str(path));
+if (qdict_get_try_str(options, "address.type")) {
+/* This path will only be possible as of a future patch;
+ * TODO: Remove this note once it is */
+
+const QDictEntry *e;
+for (e = qdict_first(options); e; e = qdict_next(options, e)) {
+if (!strncmp(e->key, "address.", 8)) {
+qobject_incref(e->value);
+qdict_put_obj(opts, e->key, e->value);
+}
+}
 } else {
-qdict_put(opts, "host", qstring_from_str(host));
-qdict_put(opts, "port", qstring_from_str(port));
+if (path) {
+qdict_put(opts, "path", qstring_from_str(path));
+} else {
+qdict_put(opts, "host", qstring_from_str(host));
+qdict_put(opts, "port", qstring_from_str(port));
+}
 }
 if (export) {
 qdict_put(opts, "export", qstring_from_str(export));
-- 
2.7.1

[Qemu-block] [PATCH v2 05/16] block/nbd: Reject port parameter without host

[Max Reitz]

This is better than the generic block layer finding out later that the
port parameter has not been used.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block/nbd.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index ce31119..6a2fc27 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -200,6 +200,10 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict 
*options, char **export,
 }
 return NULL;
 }
+if (qdict_haskey(options, "port") && !qdict_haskey(options, "host")) {
+error_setg(errp, "port may not be used without host");
+return NULL;
+}
 
 saddr = g_new0(SocketAddress, 1);
 
-- 
2.7.1

[Qemu-block] [PATCH v2 06/16] block/nbd: Default port in nbd_refresh_filename()

[Max Reitz]

Instead of not emitting the port in nbd_refresh_filename(), just set it
to the default if the user did not specify it. This makes the logic a
bit simpler.

Signed-off-by: Max Reitz 
---
 block/nbd.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 6a2fc27..8d9a217 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -411,6 +411,10 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 const char *export = qdict_get_try_str(options, "export");
 const char *tlscreds = qdict_get_try_str(options, "tls-creds");
 
+if (host && !port) {
+port = stringify(NBD_DEFAULT_PORT);
+}
+
 qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));
 
 if (path && export) {
@@ -419,27 +423,19 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 } else if (path && !export) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
  "nbd+unix://?socket=%s", path);
-} else if (!path && export && port) {
+} else if (!path && export) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
  "nbd://%s:%s/%s", host, port, export);
-} else if (!path && export && !port) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s/%s", host, export);
-} else if (!path && !export && port) {
+} else if (!path && !export) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
  "nbd://%s:%s", host, port);
-} else if (!path && !export && !port) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s", host);
 }
 
 if (path) {
 qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path)));
-} else if (port) {
-qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
-qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
 } else {
 qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
+qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
 }
 if (export) {
 qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export)));
-- 
2.7.1

[Qemu-block] [PATCH v2 08/16] block/nbd: Add nbd_has_filename_options_conflict()

[Max Reitz]

Right now, we have four possible options that conflict with specifying
an NBD filename, and a future patch will add another one ("address").
This future option is a nested QDict that is flattened at this point,
requiring as to test each option whether its key has an "address."
prefix. Therefore, we will then need to iterate through all options.

Adding this iteration logic now will simplify adding the new option
later. A nice side effect is that the user will not receive a long list
of five options which are not supposed to be specified with a filename,
but we can actually print the problematic option.

Signed-off-by: Max Reitz 
---
 block/nbd.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 145db39..2d96dd1 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -119,6 +119,25 @@ out:
 return ret;
 }
 
+static bool nbd_has_filename_options_conflict(QDict *options, Error **errp)
+{
+const QDictEntry *e;
+
+for (e = qdict_first(options); e; e = qdict_next(options, e)) {
+if (!strcmp(e->key, "host")
+|| !strcmp(e->key, "port")
+|| !strcmp(e->key, "path")
+|| !strcmp(e->key, "export"))
+{
+error_setg(errp, "Option '%s' cannot be used with a file name",
+   e->key);
+return true;
+}
+}
+
+return false;
+}
+
 static void nbd_parse_filename(const char *filename, QDict *options,
Error **errp)
 {
@@ -127,12 +146,7 @@ static void nbd_parse_filename(const char *filename, QDict 
*options,
 const char *host_spec;
 const char *unixpath;
 
-if (qdict_haskey(options, "host")
-|| qdict_haskey(options, "port")
-|| qdict_haskey(options, "path"))
-{
-error_setg(errp, "host/port/path and a file name may not be specified "
- "at the same time");
+if (nbd_has_filename_options_conflict(options, errp)) {
 return;
 }
 
-- 
2.7.1

[Qemu-block] [PATCH v2 02/16] qdict: Add qdict_unflatten()

[Max Reitz]

The QMP input visitor is rather unhappy with flattened QDicts, which is
how they are generally used in the block layer. This function allows
unflattening a QDict so we can use an input visitor on it.

Signed-off-by: Max Reitz 
---
 include/qapi/qmp/qdict.h |   1 +
 qobject/qdict.c  | 189 +++
 2 files changed, 190 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 223f746..0ec7477 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -70,6 +70,7 @@ void qdict_set_default_str(QDict *dst, const char *key, const 
char *val);
 
 QDict *qdict_clone_shallow(const QDict *src);
 void qdict_flatten(QDict *qdict);
+bool qdict_unflatten(QDict *qdict, Error **errp);
 
 void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start);
 void qdict_array_split(QDict *src, QList **dst);
diff --git a/qobject/qdict.c b/qobject/qdict.c
index bbfe39f..800af38 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -771,6 +771,195 @@ int qdict_array_entries(QDict *src, const char *subqdict)
 }
 
 /**
+ * qlist_unflatten(): Recursive helper function for qdict_unflatten(). Invokes
+ * qdict_unflatten() and qlist_unflatten() on all its QDict and QList members,
+ * respectively.
+ */
+static bool qlist_unflatten(QList *qlist, Error **errp)
+{
+const QListEntry *entry;
+
+for (entry = qlist_first(qlist); entry; entry = qlist_next(entry)) {
+switch (qobject_type(entry->value)) {
+case QTYPE_QDICT:
+if (!qdict_unflatten(qobject_to_qdict(entry->value), errp)) {
+return false;
+}
+break;
+
+case QTYPE_QLIST:
+if (!qlist_unflatten(qobject_to_qlist(entry->value), errp)) {
+return false;
+}
+break;
+
+default:
+break;
+}
+}
+
+return true;
+}
+
+/**
+ * qdict_unflatten(): The opposite of qdict_flatten().
+ *
+ * Every entry whose key is of the form "${prefix}.${index}" is moved to index
+ * "${index}" in a QList whose key in @qdict is "${prefix}", if
+ * qdict_array_entries(qdict, "${prefix}.") yields a positive value.
+ *
+ * Every entry whose key is of the form "${prefix}.${index}.${trailing}" is
+ * moved into a QDict at index "${index}" in a QList whose key in @qdict is
+ * "${prefix}". The moved object's key in the nested QDict is "${trailing}".
+ * This is only done if qdict_array_entries(qdict, "${prefix}.") yields a
+ * positive value.
+ *
+ * Every remaining entry whose key is of the form "${prefix}.${trailing}" is
+ * moved into a QDict whose key in @qdict is "${prefix}". The moved object's 
key
+ * in the nested QDict is "${trailing}".
+ *
+ * This algorithm then recurses on all QDict members (including indirect ones
+ * in QLists) of this QDict.
+ *
+ * This function will never overwrite existing members. For instance:
+ *   qdict_unflatten({ "x": 42, "x.y": 23 })
+ * is an error because there already is an "x" element which is not a QDict.
+ * However,
+ *   qdict_unflatten({ "x": { "a": 0 }, "x.y": 23 })
+ *   => { "x": { "a": 0, "y": 23 } }
+ * because the flattened "x.y" can be merged into the existing "x" QDict 
without
+ * overwriting any of its members. In contrast to that,
+ *   qdict_unflatten({ "x": { "y": 0 }, "x.y": 23 })
+ * is an error because "y" nested in "x" would need to be overwritten.
+ *
+ * This function returns true on success and false on error (in which case 
*errp
+ * is set). On error, the contents of @qdict are undefined.
+ */
+bool qdict_unflatten(QDict *qdict, Error **errp)
+{
+const QDictEntry *entry;
+
+/* First pass: Unflatten this level */
+entry = qdict_first(qdict);
+while (entry) {
+const char *prefix_end = strchr(entry->key, '.');
+
+if (prefix_end) {
+size_t prefix_length = prefix_end - entry->key;
+char *prefix, *prefix_dot;
+
+prefix = g_malloc(prefix_length + 1);
+strncpy(prefix, entry->key, prefix_length);
+prefix[prefix_length] = 0;
+
+prefix_dot = g_strdup_printf("%s.", prefix);
+
+if (qdict_array_entries(qdict, prefix_dot) > 0) {
+/* Move all entries with this prefix into a nested QList */
+QDict *array_qdict;
+QList *target_qlist;
+
+/* We cannot merge two non-empty lists without one overwriting
+ * members of the other */
+if (qdict_haskey(qdict, prefix)) {
+if (qobject_type(qdict_get(qdict, prefix)) != QTYPE_QLIST 
||
+!qlist_empty(qdict_get_qlist(qdict, prefix)))
+{
+error_setg(errp, "Cannot unflatten list '%s': Overlaps 
"
+   "with existing member", prefix);
+g_free(prefix);
+g_free(prefix_dot);
+return 

[Qemu-block] [PATCH v2 10/16] block/nbd: Accept SocketAddress

[Max Reitz]

Add a new option "address" to the NBD block driver which accepts a
SocketAddress.

"path", "host" and "port" are still supported as legacy options and are
mapped to their corresponding SocketAddress representation.

Signed-off-by: Max Reitz 
---
 block/nbd.c   | 93 +++
 tests/qemu-iotests/051.out|  4 +-
 tests/qemu-iotests/051.pc.out |  4 +-
 3 files changed, 62 insertions(+), 39 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 86907bc..213ba70 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -31,6 +31,8 @@
 #include "qemu/uri.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-input-visitor.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qint.h"
@@ -127,7 +129,9 @@ static bool nbd_has_filename_options_conflict(QDict 
*options, Error **errp)
 if (!strcmp(e->key, "host")
 || !strcmp(e->key, "port")
 || !strcmp(e->key, "path")
-|| !strcmp(e->key, "export"))
+|| !strcmp(e->key, "export")
+|| !strcmp(e->key, "address")
+|| !strncmp(e->key, "address.", 8))
 {
 error_setg(errp, "Option '%s' cannot be used with a file name",
e->key);
@@ -201,44 +205,64 @@ out:
 g_free(file);
 }
 
+static bool nbd_process_legacy_socket_options(QDict *options, Error **errp)
+{
+if (qdict_haskey(options, "path") && qdict_haskey(options, "host")) {
+error_setg(errp, "path and host may not be used at the same time");
+return false;
+} else if (qdict_haskey(options, "path")) {
+if (qdict_haskey(options, "port")) {
+error_setg(errp, "port may not be used without host");
+return false;
+}
+
+qdict_put(options, "address.type", qstring_from_str("unix"));
+qdict_change_key(options, "path", "address.data.path");
+} else if (qdict_haskey(options, "host")) {
+qdict_put(options, "address.type", qstring_from_str("inet"));
+qdict_change_key(options, "host", "address.data.host");
+if (!qdict_change_key(options, "port", "address.data.port")) {
+qdict_put(options, "address.data.port",
+  qstring_from_str(stringify(NBD_DEFAULT_PORT)));
+}
+}
+
+return true;
+}
+
 static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char 
**export,
  Error **errp)
 {
-SocketAddress *saddr;
+SocketAddress *saddr = NULL;
+QDict *addr = NULL;
+QmpInputVisitor *iv;
+Error *local_err = NULL;
 
-if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
-if (qdict_haskey(options, "path")) {
-error_setg(errp, "path and host may not be used at the same time");
-} else {
-error_setg(errp, "one of path and host must be specified");
-}
-return NULL;
+if (!nbd_process_legacy_socket_options(options, errp)) {
+goto fail;
 }
-if (qdict_haskey(options, "port") && !qdict_haskey(options, "host")) {
-error_setg(errp, "port may not be used without host");
-return NULL;
+
+qdict_extract_subqdict(options, &addr, "address.");
+if (!qdict_unflatten(addr, errp)) {
+goto fail;
 }
 
-saddr = g_new0(SocketAddress, 1);
+if (!qdict_size(addr)) {
+error_setg(errp, "NBD server address missing");
+goto fail;
+}
 
-if (qdict_haskey(options, "path")) {
-saddr->type = SOCKET_ADDRESS_KIND_UNIX;
-saddr->u.q_unix = g_new0(UnixSocketAddress, 1);
-saddr->u.q_unix->path = g_strdup(qdict_get_str(options, "path"));
-qdict_del(options, "path");
-} else {
-saddr->type = SOCKET_ADDRESS_KIND_INET;
-saddr->u.inet = g_new0(InetSocketAddress, 1);
-saddr->u.inet->host = g_strdup(qdict_get_str(options, "host"));
-if (!qdict_get_try_str(options, "port")) {
-saddr->u.inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
-} else {
-saddr->u.inet->port = g_strdup(qdict_get_str(options, "port"));
-}
-qdict_del(options, "host");
-qdict_del(options, "port");
+iv = qmp_input_visitor_new(QOBJECT(addr));
+visit_type_SocketAddress(qmp_input_get_visitor(iv), NULL, &saddr,
+ &local_err);
+qmp_input_visitor_cleanup(iv);
+if (local_err) {
+error_propagate(errp, local_err);
+goto fail;
 }
 
+/* TODO: Detect superfluous (unused) options under in addr */
+
 s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
 
 *export = g_strdup(qdict_get_try_str(options, "export"));
@@ -246,7 +270,12 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict 
*options, char **export,
 qdict_del(options, "export");
 }
 
+QDECREF(addr);
 return saddr;
+
+fail:
+QDECREF(addr);
+ 

[Qemu-block] [PATCH v2 15/16] socket_scm_helper: Accept fd directly

[Max Reitz]

This gives us more freedom about the fd that is passed to qemu, allowing
us to e.g. pass sockets.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/socket_scm_helper.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tests/qemu-iotests/socket_scm_helper.c 
b/tests/qemu-iotests/socket_scm_helper.c
index 80cadf4..eb76d31 100644
--- a/tests/qemu-iotests/socket_scm_helper.c
+++ b/tests/qemu-iotests/socket_scm_helper.c
@@ -60,7 +60,7 @@ static int send_fd(int fd, int fd_to_send)
 }
 
 /* Convert string to fd number. */
-static int get_fd_num(const char *fd_str)
+static int get_fd_num(const char *fd_str, bool silent)
 {
 int sock;
 char *err;
@@ -68,12 +68,16 @@ static int get_fd_num(const char *fd_str)
 errno = 0;
 sock = strtol(fd_str, &err, 10);
 if (errno) {
-fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
-strerror(errno));
+if (!silent) {
+fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
+strerror(errno));
+}
 return -1;
 }
 if (!*fd_str || *err || sock < 0) {
-fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str);
+if (!silent) {
+fprintf(stderr, "bad numerical value for socket fd '%s'\n", 
fd_str);
+}
 return -1;
 }
 
@@ -104,18 +108,21 @@ int main(int argc, char **argv, char **envp)
 }
 
 
-sock = get_fd_num(argv[1]);
+sock = get_fd_num(argv[1], false);
 if (sock < 0) {
 return EXIT_FAILURE;
 }
 
-/* Now only open a file in readonly mode for test purpose. If more precise
-   control is needed, use python script in file operation, which is
-   supposed to fork and exec this program. */
-fd = open(argv[2], O_RDONLY);
+fd = get_fd_num(argv[2], true);
 if (fd < 0) {
-fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
-return EXIT_FAILURE;
+/* Now only open a file in readonly mode for test purpose. If more
+   precise control is needed, use python script in file operation, 
which
+   is supposed to fork and exec this program. */
+fd = open(argv[2], O_RDONLY);
+if (fd < 0) {
+fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
+return EXIT_FAILURE;
+}
 }
 
 ret = send_fd(sock, fd);
-- 
2.7.1

[Qemu-block] [PATCH v2 07/16] block/nbd: Use qdict_put()

[Max Reitz]

Instead of inlining this nice macro (i.e. resorting to
qdict_put_obj(..., QOBJECT(...))), use it.

Signed-off-by: Max Reitz 
---
 block/nbd.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 8d9a217..145db39 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -415,7 +415,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 port = stringify(NBD_DEFAULT_PORT);
 }
 
-qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));
+qdict_put(opts, "driver", qstring_from_str("nbd"));
 
 if (path && export) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
@@ -432,16 +432,16 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 }
 
 if (path) {
-qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path)));
+qdict_put(opts, "path", qstring_from_str(path));
 } else {
-qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
-qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
+qdict_put(opts, "host", qstring_from_str(host));
+qdict_put(opts, "port", qstring_from_str(port));
 }
 if (export) {
-qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export)));
+qdict_put(opts, "export", qstring_from_str(export));
 }
 if (tlscreds) {
-qdict_put_obj(opts, "tls-creds", QOBJECT(qstring_from_str(tlscreds)));
+qdict_put(opts, "tls-creds", qstring_from_str(tlscreds));
 }
 
 bs->full_open_options = opts;
-- 
2.7.1

[Qemu-block] [PATCH v2 01/16] qdict: Add qdict_change_key()

[Max Reitz]

This is a shorthand function for changing a QDict's entry's key.

Signed-off-by: Max Reitz 
---
 include/qapi/qmp/qdict.h |  1 +
 qobject/qdict.c  | 23 +++
 2 files changed, 24 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 71b8eb0..223f746 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -39,6 +39,7 @@ size_t qdict_size(const QDict *qdict);
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value);
 void qdict_del(QDict *qdict, const char *key);
 int qdict_haskey(const QDict *qdict, const char *key);
+bool qdict_change_key(QDict *qdict, const char *old_key, const char *new_key);
 QObject *qdict_get(const QDict *qdict, const char *key);
 QDict *qobject_to_qdict(const QObject *obj);
 void qdict_iter(const QDict *qdict,
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 9833bd0..bbfe39f 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -168,6 +168,29 @@ int qdict_haskey(const QDict *qdict, const char *key)
 }
 
 /**
+ * qdict_change_key(): Changes an entry's key
+ *
+ * Removes the entry with the key 'old_key' and inserts its associated value as
+ * a new entry with the key 'new_key'.
+ *
+ * Returns false if 'old_key' does not exist, true otherwise.
+ */
+bool qdict_change_key(QDict *qdict, const char *old_key, const char *new_key)
+{
+QObject *value = qdict_get(qdict, old_key);
+
+if (!value) {
+return false;
+}
+
+qobject_incref(value);
+qdict_del(qdict, old_key);
+qdict_put_obj(qdict, new_key, value);
+
+return true;
+}
+
+/**
  * qdict_size(): Return the size of the dictionary
  */
 size_t qdict_size(const QDict *qdict)
-- 
2.7.1

[Qemu-block] [PATCH v2 00/16] qapi: Allow blockdev-add for NBD

[Max Reitz]

Turns out NBD is not so simple to do if you do it right. Anyway, this
series adds blockdev-add support for NBD clients.

Patches 1 and 2 add one less and one more complicated QDict function,
respectively, which I needed in later NBD patches: Patch 1 for handling
legacy options (move "host" to "address.data.host" etc.) and patch 2
because I'd like to use the input visitor for transforming the NBD
options into a SocketAddress. Unfortunately, the block layer uses
flattened QDicts everywhere, so we'll have to unflatten (erect?) them
before we can use that visitor.

Patch 3 adds a test for qdict_unflatten().

Patches 4, 5, 6, and 7 are minor patches with no functional relation to
this series, other than later patches will touch the code they touch,
too.

Patches 8 and 9 prepare the code for the addition of a new option
prefix, which is "address.".

Patch 10 makes the NBD client accept a SocketAddress under the "address"
option (or rather, a flattened SocketAddress QDict with its keys
prefixed by "address."). The old options "host", "port", and "path" are
supported as legacy options and translated to the respective
SocketAddress representation.

Patch 11 drops usage of "host", "port", and "path" outside of
nbd_has_filename_options_conflict(),
nbd_process_legacy_socket_options(), and nbd_refresh_filename(), making
those options nothing but legacy.

Patch 12, the goal of this series, is again not very complicated.

Patches 13, 14, and 15 are required for the iotest added in patch 16. It
will invoke qemu-nbd, so patch 13 is required. Besides qemu-nbd, it will
launch an NBD server VM concurrently to the client VM, which is why
patch 14 is required. And finally, it will test whether we can add an
NBD BDS by passing it a file descriptor, which patch 15 is needed for
(so we use the socket_scm_helper to pass sockets to qemu).

Patch 16 then adds the iotest for NBD's blockdev-add interface.


Note on the relation to v1: As you can see, most of this series is
completely new. Patch 5 was patch 1 in v1, and the only thing that has
changed is that I removed the full stop at the end of the error message;
consequently I kept Eric's R-b.


Max Reitz (16):
  qdict: Add qdict_change_key()
  qdict: Add qdict_unflatten()
  check-qdict: Add a test for qdict_unflatten()
  block/nbd: Drop trailing "." in error messages
  block/nbd: Reject port parameter without host
  block/nbd: Default port in nbd_refresh_filename()
  block/nbd: Use qdict_put()
  block/nbd: Add nbd_has_filename_options_conflict()
  block/nbd: "address" in nbd_refresh_filename()
  block/nbd: Accept SocketAddress
  block/nbd: Use SocketAddress options
  qapi: Allow blockdev-add for NBD
  iotests.py: Add qemu_nbd function
  iotests.py: Allow concurrent qemu instances
  socket_scm_helper: Accept fd directly
  iotests: Add test for NBD's blockdev-add interface

 block/nbd.c| 231 +++-
 include/qapi/qmp/qdict.h   |   2 +
 qapi/block-core.json   |  23 ++-
 qobject/qdict.c| 212 ++
 tests/check-qdict.c| 267 +
 tests/qemu-iotests/051.out |   4 +-
 tests/qemu-iotests/051.pc.out  |   4 +-
 tests/qemu-iotests/147 | 194 
 tests/qemu-iotests/147.out |   5 +
 tests/qemu-iotests/group   |   1 +
 tests/qemu-iotests/iotests.py  |  22 ++-
 tests/qemu-iotests/socket_scm_helper.c |  29 ++--
 12 files changed, 898 insertions(+), 96 deletions(-)
 create mode 100755 tests/qemu-iotests/147
 create mode 100644 tests/qemu-iotests/147.out

-- 
2.7.1

[Qemu-block] [PATCH v2 03/16] check-qdict: Add a test for qdict_unflatten()

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/check-qdict.c | 267 
 1 file changed, 267 insertions(+)

diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index a43056c..f6a5cda 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -325,6 +325,272 @@ static void qdict_flatten_test(void)
 QDECREF(dict3);
 }
 
+static void qdict_unflatten_test(void)
+{
+QDict *dict;
+QList *list_a, *list_o;
+QDict *dict_a1, *dict_d, *dict_df, *dict_dh, *dict_i, *dict_l;
+const QListEntry *le;
+
+/*
+ * Test the unflattening of
+ *
+ * {
+ * "a.0": 0,
+ * "a.1.b": 1,
+ * "a.1.c": 2,
+ * "a.2": 3,
+ * "d.e": 4,
+ * "d.f.g": 5,
+ * "d.h.0": 6,
+ * "d.h.2": 7,
+ * "i.0": 8,
+ * "i.j": 9,
+ * "k": 10,
+ * "l": {
+ * "m": 11
+ * },
+ * "l.n": 12,
+ * "o": [],
+ * "o.0": 13
+ * }
+ *
+ * to
+ *
+ * {
+ * "a": [
+ * 0,
+ * {
+ * "b": 1,
+ * "c": 2
+ * },
+ * 3
+ * ],
+ * "d": {
+ * "e": 4,
+ * "f": {
+ * "g": 5
+ * },
+ * "h": {
+ * "0": 6,
+ * "2": 7
+ * },
+ * },
+ * "i": {
+ * "0": 8,
+ * "j": 9
+ * },
+ * "k": 10,
+ * "l": {
+ * "m": 11,
+ * "n": 12
+ * },
+ * "o": [
+ * 13
+ * ]
+ * }
+ *
+ * This tests:
+ * - Unflattening in general
+ * - Conversion of "x.0", "x.2" into a dict instead of a list
+ * - Conversion of "x.0", "x.y" into a dict instead of a list
+ * - Merging of previously existing and new unflattened dicts
+ *   ({ "x": { "y": 0 }, "x.z": 1 } => { "x": { "y": 0, "z": 1 } })
+ * - Merging of previously existing and new unflattened lists; only works
+ *   if the previous list was empty
+ *   ({ "x": [], "x.0": 0 } => { "x": [ 0 ] })
+ */
+
+dict = qdict_new();
+
+qdict_put(dict, "a.0",   qint_from_int( 0));
+qdict_put(dict, "a.1.b", qint_from_int( 1));
+qdict_put(dict, "a.1.c", qint_from_int( 2));
+qdict_put(dict, "a.2",   qint_from_int( 3));
+qdict_put(dict, "d.e",   qint_from_int( 4));
+qdict_put(dict, "d.f.g", qint_from_int( 5));
+qdict_put(dict, "d.h.0", qint_from_int( 6));
+qdict_put(dict, "d.h.2", qint_from_int( 7));
+qdict_put(dict, "i.0",   qint_from_int( 8));
+qdict_put(dict, "i.j",   qint_from_int( 9));
+qdict_put(dict, "k", qint_from_int(10));
+qdict_put(dict, "l", qdict_new());
+qdict_put(qdict_get_qdict(dict, "l"), "m", qint_from_int(11));
+qdict_put(dict, "l.n",   qint_from_int(12));
+qdict_put(dict, "o", qlist_new());
+qdict_put(dict, "o.0",   qint_from_int(13));
+
+qdict_unflatten(dict, &error_abort);
+
+list_a = qdict_get_qlist(dict, "a");
+g_assert(list_a);
+
+/* a.0 */
+le = qlist_first(list_a);
+g_assert(le);
+g_assert(qint_get_int(qobject_to_qint(le->value)) == 0);
+/* a.1 */
+le = qlist_next(le);
+g_assert(le);
+dict_a1 = qobject_to_qdict(le->value);
+g_assert(dict_a1);
+g_assert(qdict_get_int(dict_a1, "b") == 1);
+g_assert(qdict_get_int(dict_a1, "c") == 2);
+g_assert(qdict_size(dict_a1) == 2);
+/* a.2 */
+le = qlist_next(le);
+g_assert(le);
+g_assert(qint_get_int(qobject_to_qint(le->value)) == 3);
+
+g_assert(!qlist_next(le));
+
+dict_d = qdict_get_qdict(dict, "d");
+g_assert(dict_d);
+g_assert(qdict_get_int(dict_d, "e") == 4);
+
+dict_df = qdict_get_qdict(dict_d, "f");
+g_assert(dict_df);
+g_assert(qdict_get_int(dict_df, "g") == 5);
+g_assert(qdict_size(dict_df) == 1);
+
+dict_dh = qdict_get_qdict(dict_d, "h");
+g_assert(dict_dh);
+g_assert(qdict_get_int(dict_dh, "0") == 6);
+g_assert(qdict_get_int(dict_dh, "2") == 7);
+g_assert(qdict_size(dict_dh) == 2);
+
+g_assert(qdict_size(dict_d) == 3);
+
+dict_i = qdict_get_qdict(dict, "i");
+g_assert(dict_i);
+g_assert(qdict_get_int(dict_i, "0") == 8);
+g_assert(qdict_get_int(dict_i, "j") == 9);
+g_assert(qdict_size(dict_i) == 2);
+
+g_assert(qdict_get_int(dict, "k") == 10);
+
+dict_l = qdict_get_qdict(dict, "l");
+g_assert(dict_l);
+g_assert(qdict_get_int(dict_l, "m") == 11);
+g_assert(qdict_get_int(dict_l, "n") == 12);
+g_assert(qdict_size(dict_l) == 2);
+
+list_o = qdict_get_qlist(dict, "o");
+g_assert(list_o);
+
+/* o.0 */
+le = qlist_first(list_o);
+g_assert(le);
+g_assert(qint_get_int(qobject_to_qint(le->value)) == 13);
+
+g_assert(!qlist_next(le));
+
+g_assert(qdict_size(dict) == 6);
+
+QDECREF(dict);
+
+
+/*
+ * Test that unflattening
+

Re: [Qemu-block] [PATCH v3 11/15] block: Assert that bdrv_release_dirty_bitmap succeeded

[John Snow]

On 02/27/2016 04:20 AM, Fam Zheng wrote:
> 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 
> ---
>  block/dirty-bitmap.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index a0c5acb..27d33e7 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -304,6 +304,9 @@ static void 
> bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>  }
>  }
>  }
> +if (bitmap) {
> +abort();
> +}
>  }
>  
>  void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
> 

Bad context, but I assume what happens is if we have a bitmap, we have
an early return above, so this should be unreachable.

Reviewed-by: John Snow 

Re: [Qemu-block] [PATCH v3 09/15] block: Support meta dirty bitmap

[John Snow]

On 02/27/2016 04:20 AM, Fam Zheng wrote:
> The added group of operations enables tracking of the changed bits in
> the dirty bitmap.
> 
> Signed-off-by: Fam Zheng 
> ---
>  block/dirty-bitmap.c | 51 
> 
>  include/block/dirty-bitmap.h |  9 
>  2 files changed, 60 insertions(+)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 16f73b2..5f19320 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 gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
> +
> +/* 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 += gran) {
> +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;
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index e1dbd8e..3b27742 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -9,6 +9,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);
> @@ -37,6 +40,12 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> int64_t cur_sector, int nr_sectors);
>  void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>   int64_t cur_sector, int 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);
> 

Do we need to amend the teardown in bdrv_do_release_matching_dirty_bitmap?

Re: [Qemu-block] [PATCH v3 08/15] tests: Add test code for meta bitmap

[John Snow]

On 02/27/2016 04:20 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> Reviewed-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 abe1427..c00c2b5 100644
> --- a/tests/test-hbitmap.c
> +++ b/tests/test-hbitmap.c
> @@ -12,6 +12,7 @@
>  #include "qemu/osdep.h"
>  #include 
>  #include "qemu/hbitmap.h"
> +#include "block/block.h"
>  
>  #define LOG_BITS_PER_LONG  (BITS_PER_LONG == 32 ? 5 : 6)
>  
> @@ -21,6 +22,7 @@
>  
>  typedef struct TestHBitmapData {
>  HBitmap   *hb;
> +HBitmap   *meta;
>  unsigned long *bits;
>  size_t size;
>  size_t old_size;
> @@ -92,6 +94,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 = (bits + BITS_PER_LONG - 1) / BITS_PER_LONG;
> @@ -134,6 +144,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;
>  }
> @@ -635,6 +648,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_

[Qemu-block] [PULL 02/12] sheepdog: allow to delete snapshot

[Jeff Cody]

From: Vasiliy Tolstov 

This patch implements a blockdriver function bdrv_snapshot_delete() in
the sheepdog driver. With the new function, snapshots of sheepdog can
be deleted from libvirt.

Cc: Jeff Cody 
Signed-off-by: Hitoshi Mitake 
Signed-off-by: Vasiliy Tolstov 
Message-id: 1450873346-22334-1-git-send-email-mitake.hito...@lab.ntt.co.jp
Signed-off-by: Jeff Cody 
---
 block/sheepdog.c | 125 ++-
 1 file changed, 123 insertions(+), 2 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index a0098c1..8739acc 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -284,6 +284,12 @@ static inline bool is_snapshot(struct SheepdogInode *inode)
 return !!inode->snap_ctime;
 }
 
+static inline size_t count_data_objs(const struct SheepdogInode *inode)
+{
+return DIV_ROUND_UP(inode->vdi_size,
+(1UL << inode->block_size_shift));
+}
+
 #undef DPRINTF
 #ifdef DEBUG_SDOG
 #define DPRINTF(fmt, args...)   \
@@ -2478,13 +2484,128 @@ out:
 return ret;
 }
 
+#define NR_BATCHED_DISCARD 128
+
+static bool remove_objects(BDRVSheepdogState *s)
+{
+int fd, i = 0, nr_objs = 0;
+Error *local_err = NULL;
+int ret = 0;
+bool result = true;
+SheepdogInode *inode = &s->inode;
+
+fd = connect_to_sdog(s, &local_err);
+if (fd < 0) {
+error_report_err(local_err);
+return false;
+}
+
+nr_objs = count_data_objs(inode);
+while (i < nr_objs) {
+int start_idx, nr_filled_idx;
+
+while (i < nr_objs && !inode->data_vdi_id[i]) {
+i++;
+}
+start_idx = i;
+
+nr_filled_idx = 0;
+while (i < nr_objs && nr_filled_idx < NR_BATCHED_DISCARD) {
+if (inode->data_vdi_id[i]) {
+inode->data_vdi_id[i] = 0;
+nr_filled_idx++;
+}
+
+i++;
+}
+
+ret = write_object(fd, s->aio_context,
+   (char *)&inode->data_vdi_id[start_idx],
+   vid_to_vdi_oid(s->inode.vdi_id), inode->nr_copies,
+   (i - start_idx) * sizeof(uint32_t),
+   offsetof(struct SheepdogInode,
+data_vdi_id[start_idx]),
+   false, s->cache_flags);
+if (ret < 0) {
+error_report("failed to discard snapshot inode.");
+result = false;
+goto out;
+}
+}
+
+out:
+closesocket(fd);
+return result;
+}
+
 static int sd_snapshot_delete(BlockDriverState *bs,
   const char *snapshot_id,
   const char *name,
   Error **errp)
 {
-/* FIXME: Delete specified snapshot id.  */
-return 0;
+uint32_t snap_id = 0;
+char snap_tag[SD_MAX_VDI_TAG_LEN];
+Error *local_err = NULL;
+int fd, ret;
+char buf[SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN];
+BDRVSheepdogState *s = bs->opaque;
+unsigned int wlen = SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN, rlen = 0;
+uint32_t vid;
+SheepdogVdiReq hdr = {
+.opcode = SD_OP_DEL_VDI,
+.data_length = wlen,
+.flags = SD_FLAG_CMD_WRITE,
+};
+SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
+
+if (!remove_objects(s)) {
+return -1;
+}
+
+memset(buf, 0, sizeof(buf));
+memset(snap_tag, 0, sizeof(snap_tag));
+pstrcpy(buf, SD_MAX_VDI_LEN, s->name);
+if (qemu_strtoul(snapshot_id, NULL, 10, (unsigned long *)&snap_id)) {
+return -1;
+}
+
+if (snap_id) {
+hdr.snapid = snap_id;
+} else {
+pstrcpy(snap_tag, sizeof(snap_tag), snapshot_id);
+pstrcpy(buf + SD_MAX_VDI_LEN, SD_MAX_VDI_TAG_LEN, snap_tag);
+}
+
+ret = find_vdi_name(s, s->name, snap_id, snap_tag, &vid, true,
+&local_err);
+if (ret) {
+return ret;
+}
+
+fd = connect_to_sdog(s, &local_err);
+if (fd < 0) {
+error_report_err(local_err);
+return -1;
+}
+
+ret = do_req(fd, s->aio_context, (SheepdogReq *)&hdr,
+ buf, &wlen, &rlen);
+closesocket(fd);
+if (ret) {
+return ret;
+}
+
+switch (rsp->result) {
+case SD_RES_NO_VDI:
+error_report("%s was already deleted", s->name);
+case SD_RES_SUCCESS:
+break;
+default:
+error_report("%s, %s", sd_strerror(rsp->result), s->name);
+return -1;
+}
+
+return ret;
 }
 
 static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
-- 
1.9.3

[Qemu-block] [PULL 04/12] curl: add support for HTTP authentication parameters

[Jeff Cody]

From: "Daniel P. Berrange" 

If connecting to a web server which has authentication
turned on, QEMU gets a 401 as curl has not been configured
with any authentication credentials.

This adds 4 new parameters to the curl block driver
options 'username', 'password-secret', 'proxy-username'
and 'proxy-password-secret'. Passwords are provided using
the recently added 'secret' object type

 $QEMU \
 -object secret,id=sec0,filename=/home/berrange/example.pw \
 -object secret,id=sec1,filename=/home/berrange/proxy.pw \
 -drive driver=http,url=http://example.com/some.img,\
username=dan,password-secret=sec0,\
proxy-username=dan,proxy-password-secret=sec1

Of course it is possible to use the same secret for both the
proxy & server passwords if desired, or omit the proxy auth
details, or the server auth details as required.

Signed-off-by: Daniel P. Berrange 
Message-id: 1453385961-10718-3-git-send-email-berra...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/curl.c | 66 
 1 file changed, 66 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index 1507e0a..c70bfb4 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -27,6 +27,7 @@
 #include "block/block_int.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
+#include "crypto/secret.h"
 #include 
 
 // #define DEBUG_CURL
@@ -78,6 +79,10 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
 #define CURL_BLOCK_OPT_TIMEOUT "timeout"
 #define CURL_BLOCK_OPT_COOKIE"cookie"
+#define CURL_BLOCK_OPT_USERNAME "username"
+#define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
+#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
+#define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
 
 struct BDRVCURLState;
 
@@ -120,6 +125,10 @@ typedef struct BDRVCURLState {
 char *cookie;
 bool accept_range;
 AioContext *aio_context;
+char *username;
+char *password;
+char *proxyusername;
+char *proxypassword;
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
@@ -419,6 +428,21 @@ static CURLState *curl_init_state(BlockDriverState *bs, 
BDRVCURLState *s)
 curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg);
 curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1);
 
+if (s->username) {
+curl_easy_setopt(state->curl, CURLOPT_USERNAME, s->username);
+}
+if (s->password) {
+curl_easy_setopt(state->curl, CURLOPT_PASSWORD, s->password);
+}
+if (s->proxyusername) {
+curl_easy_setopt(state->curl,
+ CURLOPT_PROXYUSERNAME, s->proxyusername);
+}
+if (s->proxypassword) {
+curl_easy_setopt(state->curl,
+ CURLOPT_PROXYPASSWORD, s->proxypassword);
+}
+
 /* Restrict supported protocols to avoid security issues in the more
  * obscure protocols.  For example, do not allow POP3/SMTP/IMAP see
  * CVE-2013-0249.
@@ -525,10 +549,31 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Pass the cookie or list of cookies with each request"
 },
+{
+.name = CURL_BLOCK_OPT_USERNAME,
+.type = QEMU_OPT_STRING,
+.help = "Username for HTTP auth"
+},
+{
+.name = CURL_BLOCK_OPT_PASSWORD_SECRET,
+.type = QEMU_OPT_STRING,
+.help = "ID of secret used as password for HTTP auth",
+},
+{
+.name = CURL_BLOCK_OPT_PROXY_USERNAME,
+.type = QEMU_OPT_STRING,
+.help = "Username for HTTP proxy auth"
+},
+{
+.name = CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET,
+.type = QEMU_OPT_STRING,
+.help = "ID of secret used as password for HTTP proxy auth",
+},
 { /* end of list */ }
 },
 };
 
+
 static int curl_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
@@ -539,6 +584,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 const char *file;
 const char *cookie;
 double d;
+const char *secretid;
 
 static int inited = 0;
 
@@ -580,6 +626,26 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 goto out_noclean;
 }
 
+s->username = g_strdup(qemu_opt_get(opts, CURL_BLOCK_OPT_USERNAME));
+secretid = qemu_opt_get(opts, CURL_BLOCK_OPT_PASSWORD_SECRET);
+
+if (secretid) {
+s->password = qcrypto_secret_lookup_as_utf8(secretid, errp);
+if (!s->password) {
+goto out_noclean;
+}
+}
+
+s->proxyusername = g_strdup(
+qemu_opt_get(opts, CURL_BLOCK_OPT_PROXY_USERNAME));
+secretid = qemu_opt_get(opts, CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET);
+if (secretid) {
+ 

[Qemu-block] [PULL 10/12] block/backup: make backup cluster size configurable

[Jeff Cody]

From: John Snow 

64K might not always be appropriate, make this a runtime value.

Signed-off-by: John Snow 
Reviewed-by: Fam Zheng 
Message-id: 1456433911-24718-2-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/backup.c | 64 +-
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 00cafdb..76addef 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -21,10 +21,7 @@
 #include "qemu/ratelimit.h"
 #include "sysemu/block-backend.h"
 
-#define BACKUP_CLUSTER_BITS 16
-#define BACKUP_CLUSTER_SIZE (1 << BACKUP_CLUSTER_BITS)
-#define BACKUP_SECTORS_PER_CLUSTER (BACKUP_CLUSTER_SIZE / BDRV_SECTOR_SIZE)
-
+#define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16)
 #define SLICE_TIME 1ULL /* ns */
 
 typedef struct CowRequest {
@@ -46,9 +43,16 @@ typedef struct BackupBlockJob {
 CoRwlock flush_rwlock;
 uint64_t sectors_read;
 HBitmap *bitmap;
+int64_t cluster_size;
 QLIST_HEAD(, CowRequest) inflight_reqs;
 } BackupBlockJob;
 
+/* Size of a cluster in sectors, instead of bytes. */
+static inline int64_t cluster_size_sectors(BackupBlockJob *job)
+{
+  return job->cluster_size / BDRV_SECTOR_SIZE;
+}
+
 /* See if in-flight requests overlap and wait for them to complete */
 static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job,
int64_t start,
@@ -97,13 +101,14 @@ static int coroutine_fn backup_do_cow(BlockDriverState *bs,
 QEMUIOVector bounce_qiov;
 void *bounce_buffer = NULL;
 int ret = 0;
+int64_t sectors_per_cluster = cluster_size_sectors(job);
 int64_t start, end;
 int n;
 
 qemu_co_rwlock_rdlock(&job->flush_rwlock);
 
-start = sector_num / BACKUP_SECTORS_PER_CLUSTER;
-end = DIV_ROUND_UP(sector_num + nb_sectors, BACKUP_SECTORS_PER_CLUSTER);
+start = sector_num / sectors_per_cluster;
+end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster);
 
 trace_backup_do_cow_enter(job, start, sector_num, nb_sectors);
 
@@ -118,12 +123,12 @@ static int coroutine_fn backup_do_cow(BlockDriverState 
*bs,
 
 trace_backup_do_cow_process(job, start);
 
-n = MIN(BACKUP_SECTORS_PER_CLUSTER,
+n = MIN(sectors_per_cluster,
 job->common.len / BDRV_SECTOR_SIZE -
-start * BACKUP_SECTORS_PER_CLUSTER);
+start * sectors_per_cluster);
 
 if (!bounce_buffer) {
-bounce_buffer = qemu_blockalign(bs, BACKUP_CLUSTER_SIZE);
+bounce_buffer = qemu_blockalign(bs, job->cluster_size);
 }
 iov.iov_base = bounce_buffer;
 iov.iov_len = n * BDRV_SECTOR_SIZE;
@@ -131,10 +136,10 @@ static int coroutine_fn backup_do_cow(BlockDriverState 
*bs,
 
 if (is_write_notifier) {
 ret = bdrv_co_readv_no_serialising(bs,
-   start * BACKUP_SECTORS_PER_CLUSTER,
+   start * sectors_per_cluster,
n, &bounce_qiov);
 } else {
-ret = bdrv_co_readv(bs, start * BACKUP_SECTORS_PER_CLUSTER, n,
+ret = bdrv_co_readv(bs, start * sectors_per_cluster, n,
 &bounce_qiov);
 }
 if (ret < 0) {
@@ -147,11 +152,11 @@ static int coroutine_fn backup_do_cow(BlockDriverState 
*bs,
 
 if (buffer_is_zero(iov.iov_base, iov.iov_len)) {
 ret = bdrv_co_write_zeroes(job->target,
-   start * BACKUP_SECTORS_PER_CLUSTER,
+   start * sectors_per_cluster,
n, BDRV_REQ_MAY_UNMAP);
 } else {
 ret = bdrv_co_writev(job->target,
- start * BACKUP_SECTORS_PER_CLUSTER, n,
+ start * sectors_per_cluster, n,
  &bounce_qiov);
 }
 if (ret < 0) {
@@ -322,21 +327,22 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
 int64_t cluster;
 int64_t end;
 int64_t last_cluster = -1;
+int64_t sectors_per_cluster = cluster_size_sectors(job);
 BlockDriverState *bs = job->common.bs;
 HBitmapIter hbi;
 
 granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
-clusters_per_iter = MAX((granularity / BACKUP_CLUSTER_SIZE), 1);
+clusters_per_iter = MAX((granularity / job->cluster_size), 1);
 bdrv_dirty_iter_init(job->sync_bitmap, &hbi);
 
 /* Find the next dirty sector(s) */
 while ((sector = hbitmap_iter_next(&hbi)) != -1) {
-cluster = sector / BACKUP_SECTORS_PER_CLUSTER;
+cluster = sector / sectors_per_cluster;
 
 /* Fake progress updates for any clusters we skipped */
 if (cluster != last_cluster + 1) {
 job->common.offset += ((cluster - last_cluster - 1) *

[Qemu-block] [PULL 11/12] block/backup: avoid copying less than full target clusters

[Jeff Cody]

From: John Snow 

During incremental backups, if the target has a cluster size that is
larger than the backup cluster size and we are backing up to a target
that cannot (for whichever reason) pull clusters up from a backing image,
we may inadvertantly create unusable incremental backup images.

For example:

If the bitmap tracks changes at a 64KB granularity and we transmit 64KB
of data at a time but the target uses a 128KB cluster size, it is
possible that only half of a target cluster will be recognized as dirty
by the backup block job. When the cluster is allocated on the target
image but only half populated with data, we lose the ability to
distinguish between zero padding and uninitialized data.

This does not happen if the target image has a backing file that points
to the last known good backup.

Even if we have a backing file, though, it's likely going to be faster
to just buffer the redundant data ourselves from the live image than
fetching it from the backing file, so let's just always round up to the
target granularity.

The same logic applies to backup modes top, none, and full. Copying
fractional clusters without the guarantee of COW is dangerous, but even
if we can rely on COW, it's likely better to just re-copy the data.

Reported-by: Fam Zheng 
Signed-off-by: John Snow 
Reviewed-by: Fam Zheng 
Message-id: 1456433911-24718-3-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/backup.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 76addef..0f1b1bc 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -501,6 +501,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
   BlockJobTxn *txn, Error **errp)
 {
 int64_t len;
+BlockDriverInfo bdi;
+int ret;
 
 assert(bs);
 assert(target);
@@ -570,15 +572,32 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 goto error;
 }
 
-bdrv_op_block_all(target, job->common.blocker);
-
 job->on_source_error = on_source_error;
 job->on_target_error = on_target_error;
 job->target = target;
 job->sync_mode = sync_mode;
 job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ?
sync_bitmap : NULL;
-job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
+
+/* If there is no backing file on the target, we cannot rely on COW if our
+ * backup cluster size is smaller than the target cluster size. Even for
+ * targets with a backing file, try to avoid COW if possible. */
+ret = bdrv_get_info(job->target, &bdi);
+if (ret < 0 && !target->backing) {
+error_setg_errno(errp, -ret,
+"Couldn't determine the cluster size of the target image, "
+"which has no backing file");
+error_append_hint(errp,
+"Aborting, since this may create an unusable destination image\n");
+goto error;
+} else if (ret < 0 && target->backing) {
+/* Not fatal; just trudge on ahead. */
+job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
+} else {
+job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+}
+
+bdrv_op_block_all(target, job->common.blocker);
 job->common.len = len;
 job->common.co = qemu_coroutine_create(backup_run);
 block_job_txn_add_job(txn, &job->common);
-- 
1.9.3

[Qemu-block] [PULL 09/12] mirror: Add mirror_wait_for_io

[Jeff Cody]

From: Fam Zheng 

The three lines are duplicated a number of times now, refactor a
function.

Signed-off-by: Fam Zheng 
Reviewed-by: Max Reitz 
Message-id: 1454637630-10585-3-git-send-email-f...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/mirror.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 48cd0b3..9635fa8 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -196,6 +196,14 @@ static int mirror_cow_align(MirrorBlockJob *s,
 return ret;
 }
 
+static inline void mirror_wait_for_io(MirrorBlockJob *s)
+{
+assert(!s->waiting_for_io);
+s->waiting_for_io = true;
+qemu_coroutine_yield();
+s->waiting_for_io = false;
+}
+
 /* Submit async read while handling COW.
  * Returns: nb_sectors if no alignment is necessary, or
  *  (new_end - sector_num) if tail is rounded up or down due to
@@ -228,9 +236,7 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t 
sector_num,
 
 while (s->buf_free_count < nb_chunks) {
 trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
-s->waiting_for_io = true;
-qemu_coroutine_yield();
-s->waiting_for_io = false;
+mirror_wait_for_io(s);
 }
 
 /* Allocate a MirrorOp that is used as an AIO callback.  */
@@ -321,9 +327,7 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
 break;
 }
 trace_mirror_yield_in_flight(s, next_sector, s->in_flight);
-s->waiting_for_io = true;
-qemu_coroutine_yield();
-s->waiting_for_io = false;
+mirror_wait_for_io(s);
 /* Now retry.  */
 } else {
 hbitmap_next = hbitmap_iter_next(&s->hbi);
@@ -414,9 +418,7 @@ static void mirror_free_init(MirrorBlockJob *s)
 static void mirror_drain(MirrorBlockJob *s)
 {
 while (s->in_flight > 0) {
-s->waiting_for_io = true;
-qemu_coroutine_yield();
-s->waiting_for_io = false;
+mirror_wait_for_io(s);
 }
 }
 
@@ -604,9 +606,7 @@ static void coroutine_fn mirror_run(void *opaque)
 if (s->in_flight == MAX_IN_FLIGHT || s->buf_free_count == 0 ||
 (cnt == 0 && s->in_flight > 0)) {
 trace_mirror_yield(s, s->in_flight, s->buf_free_count, cnt);
-s->waiting_for_io = true;
-qemu_coroutine_yield();
-s->waiting_for_io = false;
+mirror_wait_for_io(s);
 continue;
 } else if (cnt != 0) {
 delay_ns = mirror_iteration(s);
-- 
1.9.3

[Qemu-block] [PULL 03/12] rbd: add support for getting password from QCryptoSecret object

[Jeff Cody]

From: "Daniel P. Berrange" 

Currently RBD passwords must be provided on the command line
via

  $QEMU -drive file=rbd:pool/image:id=myname:\
   key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\
   auth_supported=cephx

This is insecure because the key is visible in the OS process
listing.

This adds support for an 'password-secret' parameter in the RBD
parameters that can be used with the QCryptoSecret object to
provide the password via a file:

  echo "QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=" > poolkey.b64
  $QEMU -object secret,id=secret0,file=poolkey.b64,format=base64 \
-drive driver=rbd,filename=rbd:pool/image:id=myname:\
   auth_supported=cephx,password-secret=secret0

Reviewed-by: Josh Durgin 
Signed-off-by: Daniel P. Berrange 
Message-id: 1453385961-10718-2-git-send-email-berra...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/block/rbd.c b/block/rbd.c
index 51b64f3..abfea61 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -16,6 +16,7 @@
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "block/block_int.h"
+#include "crypto/secret.h"
 
 #include 
 
@@ -228,6 +229,27 @@ static char *qemu_rbd_parse_clientname(const char *conf, 
char *clientname)
 return NULL;
 }
 
+
+static int qemu_rbd_set_auth(rados_t cluster, const char *secretid,
+ Error **errp)
+{
+if (secretid == 0) {
+return 0;
+}
+
+gchar *secret = qcrypto_secret_lookup_as_base64(secretid,
+errp);
+if (!secret) {
+return -1;
+}
+
+rados_conf_set(cluster, "key", secret);
+g_free(secret);
+
+return 0;
+}
+
+
 static int qemu_rbd_set_conf(rados_t cluster, const char *conf,
  bool only_read_conf_file,
  Error **errp)
@@ -299,10 +321,13 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
 char conf[RBD_MAX_CONF_SIZE];
 char clientname_buf[RBD_MAX_CONF_SIZE];
 char *clientname;
+const char *secretid;
 rados_t cluster;
 rados_ioctx_t io_ctx;
 int ret;
 
+secretid = qemu_opt_get(opts, "password-secret");
+
 if (qemu_rbd_parsename(filename, pool, sizeof(pool),
snap_buf, sizeof(snap_buf),
name, sizeof(name),
@@ -350,6 +375,11 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
 return -EIO;
 }
 
+if (qemu_rbd_set_auth(cluster, secretid, errp) < 0) {
+rados_shutdown(cluster);
+return -EIO;
+}
+
 if (rados_connect(cluster) < 0) {
 error_setg(errp, "error connecting");
 rados_shutdown(cluster);
@@ -423,6 +453,11 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Specification of the rbd image",
 },
+{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
+.help = "ID of secret providing the password",
+},
 { /* end of list */ }
 },
 };
@@ -436,6 +471,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 char conf[RBD_MAX_CONF_SIZE];
 char clientname_buf[RBD_MAX_CONF_SIZE];
 char *clientname;
+const char *secretid;
 QemuOpts *opts;
 Error *local_err = NULL;
 const char *filename;
@@ -450,6 +486,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 
 filename = qemu_opt_get(opts, "filename");
+secretid = qemu_opt_get(opts, "password-secret");
 
 if (qemu_rbd_parsename(filename, pool, sizeof(pool),
snap_buf, sizeof(snap_buf),
@@ -488,6 +525,11 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 }
 
+if (qemu_rbd_set_auth(s->cluster, secretid, errp) < 0) {
+r = -EIO;
+goto failed_shutdown;
+}
+
 /*
  * Fallback to more conservative semantics if setting cache
  * options fails. Ignore errors from setting rbd_cache because the
@@ -919,6 +961,11 @@ static QemuOptsList qemu_rbd_create_opts = {
 .type = QEMU_OPT_SIZE,
 .help = "RBD object size"
 },
+{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
+.help = "ID of secret providing the password",
+},
 { /* end of list */ }
 }
 };
-- 
1.9.3

[Qemu-block] [PULL 07/12] vhdx: Simplify vhdx_set_shift_bits()

[Jeff Cody]

From: Max Reitz 

For values which are powers of two (and we do assume all of these to
be), sizeof(x) * 8 - 1 - clz(x) == ctz(x). Therefore, use ctz().

Signed-off-by: Max Reitz 
Message-id: 1450451066-13335-3-git-send-email-mre...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/vhdx.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 1e7e03e..9a51428 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -264,10 +264,10 @@ static void vhdx_region_unregister_all(BDRVVHDXState *s)
 
 static void vhdx_set_shift_bits(BDRVVHDXState *s)
 {
-s->logical_sector_size_bits = 31 - clz32(s->logical_sector_size);
-s->sectors_per_block_bits =   31 - clz32(s->sectors_per_block);
-s->chunk_ratio_bits = 63 - clz64(s->chunk_ratio);
-s->block_size_bits =  31 - clz32(s->block_size);
+s->logical_sector_size_bits = ctz32(s->logical_sector_size);
+s->sectors_per_block_bits =   ctz32(s->sectors_per_block);
+s->chunk_ratio_bits = ctz64(s->chunk_ratio);
+s->block_size_bits =  ctz32(s->block_size);
 }
 
 /*
-- 
1.9.3

[Qemu-block] [PULL 08/12] mirror: Rewrite mirror_iteration

[Jeff Cody]

From: Fam Zheng 

The "pnum < nb_sectors" condition in deciding whether to actually copy
data is unnecessarily strict, and the qiov initialization is
unnecessarily for bdrv_aio_write_zeroes and bdrv_aio_discard.

Rewrite mirror_iteration to fix both flaws.

The output of iotests 109 is updated because we now report the offset
and len slightly differently in mirroring progress.

Signed-off-by: Fam Zheng 
Reviewed-by: Max Reitz 
Message-id: 1454637630-10585-2-git-send-email-f...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/mirror.c | 335 +++--
 tests/qemu-iotests/109.out |  80 +--
 trace-events   |   1 -
 3 files changed, 243 insertions(+), 173 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 2c0edfa..48cd0b3 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -47,7 +47,6 @@ typedef struct MirrorBlockJob {
 BlockdevOnError on_source_error, on_target_error;
 bool synced;
 bool should_complete;
-int64_t sector_num;
 int64_t granularity;
 size_t buf_size;
 int64_t bdev_length;
@@ -64,6 +63,8 @@ typedef struct MirrorBlockJob {
 int ret;
 bool unmap;
 bool waiting_for_io;
+int target_cluster_sectors;
+int max_iov;
 } MirrorBlockJob;
 
 typedef struct MirrorOp {
@@ -159,116 +160,79 @@ static void mirror_read_complete(void *opaque, int ret)
 mirror_write_complete, op);
 }
 
-static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
+/* Round sector_num and/or nb_sectors to target cluster if COW is needed, and
+ * return the offset of the adjusted tail sector against original. */
+static int mirror_cow_align(MirrorBlockJob *s,
+int64_t *sector_num,
+int *nb_sectors)
+{
+bool need_cow;
+int ret = 0;
+int chunk_sectors = s->granularity >> BDRV_SECTOR_BITS;
+int64_t align_sector_num = *sector_num;
+int align_nb_sectors = *nb_sectors;
+int max_sectors = chunk_sectors * s->max_iov;
+
+need_cow = !test_bit(*sector_num / chunk_sectors, s->cow_bitmap);
+need_cow |= !test_bit((*sector_num + *nb_sectors - 1) / chunk_sectors,
+  s->cow_bitmap);
+if (need_cow) {
+bdrv_round_to_clusters(s->target, *sector_num, *nb_sectors,
+   &align_sector_num, &align_nb_sectors);
+}
+
+if (align_nb_sectors > max_sectors) {
+align_nb_sectors = max_sectors;
+if (need_cow) {
+align_nb_sectors = QEMU_ALIGN_DOWN(align_nb_sectors,
+   s->target_cluster_sectors);
+}
+}
+
+ret = align_sector_num + align_nb_sectors - (*sector_num + *nb_sectors);
+*sector_num = align_sector_num;
+*nb_sectors = align_nb_sectors;
+assert(ret >= 0);
+return ret;
+}
+
+/* Submit async read while handling COW.
+ * Returns: nb_sectors if no alignment is necessary, or
+ *  (new_end - sector_num) if tail is rounded up or down due to
+ *  alignment or buffer limit.
+ */
+static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
+  int nb_sectors)
 {
 BlockDriverState *source = s->common.bs;
-int nb_sectors, sectors_per_chunk, nb_chunks, max_iov;
-int64_t end, sector_num, next_chunk, next_sector, hbitmap_next_sector;
-uint64_t delay_ns = 0;
+int sectors_per_chunk, nb_chunks;
+int ret = nb_sectors;
 MirrorOp *op;
-int pnum;
-int64_t ret;
-BlockDriverState *file;
 
-max_iov = MIN(source->bl.max_iov, s->target->bl.max_iov);
-
-s->sector_num = hbitmap_iter_next(&s->hbi);
-if (s->sector_num < 0) {
-bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
-s->sector_num = hbitmap_iter_next(&s->hbi);
-trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
-assert(s->sector_num >= 0);
-}
-
-hbitmap_next_sector = s->sector_num;
-sector_num = s->sector_num;
 sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
-end = s->bdev_length / BDRV_SECTOR_SIZE;
 
-/* Extend the QEMUIOVector to include all adjacent blocks that will
- * be copied in this operation.
- *
- * We have to do this if we have no backing file yet in the destination,
- * and the cluster size is very large.  Then we need to do COW ourselves.
- * The first time a cluster is copied, copy it entirely.  Note that,
- * because both the granularity and the cluster size are powers of two,
- * the number of sectors to copy cannot exceed one cluster.
- *
- * We also want to extend the QEMUIOVector to include more adjacent
- * dirty blocks if possible, to limit the number of I/O operations and
- * run efficiently even with a small granularity.
- */
-nb_chunks = 0;
-nb_sectors = 0;
-next_sector = sector_num;
-next_chunk = sector_num / sectors_per_chunk;
+/* We can only handle as much as buf

[Qemu-block] [PULL 12/12] iotests/124: Add cluster_size mismatch test

[Jeff Cody]

From: John Snow 

If a backing file isn't specified in the target image and the
cluster_size is larger than the bitmap granularity, we run the risk of
creating bitmaps with allocated clusters but empty/no data which will
prevent the proper reading of the backup in the future.

Signed-off-by: John Snow 
Reviewed-by: Fam Zheng 
Message-id: 1456433911-24718-4-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody 
---
 tests/qemu-iotests/124 | 58 ++
 tests/qemu-iotests/124.out |  4 ++--
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 7d33422..de7cdbe 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -132,14 +132,16 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
 
 
 def img_create(self, img, fmt=iotests.imgfmt, size='64M',
-   parent=None, parentFormat=None):
+   parent=None, parentFormat=None, **kwargs):
+optargs = []
+for k,v in kwargs.iteritems():
+optargs = optargs + ['-o', '%s=%s' % (k,v)]
+args = ['create', '-f', fmt] + optargs + [img, size]
 if parent:
 if parentFormat is None:
 parentFormat = fmt
-iotests.qemu_img('create', '-f', fmt, img, size,
- '-b', parent, '-F', parentFormat)
-else:
-iotests.qemu_img('create', '-f', fmt, img, size)
+args = args + ['-b', parent, '-F', parentFormat]
+iotests.qemu_img(*args)
 self.files.append(img)
 
 
@@ -307,6 +309,52 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
 return self.do_incremental_simple(granularity=131072)
 
 
+def test_larger_cluster_target(self):
+'''
+Test: Create and verify backups made to a larger cluster size target.
+
+With a default granularity of 64KiB, verify that backups made to a
+larger cluster size target of 128KiB without a backing file works.
+'''
+drive0 = self.drives[0]
+
+# Create a cluster_size=128k full backup / "anchor" backup
+self.img_create(drive0['backup'], cluster_size='128k')
+self.assertTrue(self.do_qmp_backup(device=drive0['id'], sync='full',
+   format=drive0['fmt'],
+   target=drive0['backup'],
+   mode='existing'))
+
+# Create bitmap and dirty it with some new writes.
+# overwrite [32736, 32799] which will dirty bitmap clusters at
+# 32M-64K and 32M. 32M+64K will be left undirtied.
+bitmap0 = self.add_bitmap('bitmap0', drive0)
+self.hmp_io_writes(drive0['id'],
+   (('0xab', 0, 512),
+('0xfe', '16M', '256k'),
+('0x64', '32736k', '64k')))
+
+
+# Prepare a cluster_size=128k backup target without a backing file.
+(target, _) = bitmap0.new_target()
+self.img_create(target, bitmap0.drive['fmt'], cluster_size='128k')
+
+# Perform Incremental Backup
+self.assertTrue(self.do_qmp_backup(device=bitmap0.drive['id'],
+   sync='incremental',
+   bitmap=bitmap0.name,
+   format=bitmap0.drive['fmt'],
+   target=target,
+   mode='existing'))
+self.make_reference_backup(bitmap0)
+
+# Add the backing file, then compare and exit.
+iotests.qemu_img('rebase', '-f', drive0['fmt'], '-u', '-b',
+ drive0['backup'], '-F', drive0['fmt'], target)
+self.vm.shutdown()
+self.check_backups()
+
+
 def test_incremental_transaction(self):
 '''Test: Verify backups made from transactionally created bitmaps.
 
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index dae404e..36376be 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-.
+..
 --
-Ran 9 tests
+Ran 10 tests
 
 OK
-- 
1.9.3

[Qemu-block] [PULL 06/12] vhdx: DIV_ROUND_UP() in vhdx_calc_bat_entries()

[Jeff Cody]

From: Max Reitz 

We have DIV_ROUND_UP(), so we can use it to produce more easily readable
code. It may be slower than the bit shifting currently performed
(because it actually performs a division), but since
vhdx_calc_bat_entries() is never used in a hot path, this is completely
fine.

Signed-off-by: Max Reitz 
Message-id: 1450451066-13335-2-git-send-email-mre...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/vhdx.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 72042e9..1e7e03e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -857,14 +857,8 @@ static void vhdx_calc_bat_entries(BDRVVHDXState *s)
 {
 uint32_t data_blocks_cnt, bitmap_blocks_cnt;
 
-data_blocks_cnt = s->virtual_disk_size >> s->block_size_bits;
-if (s->virtual_disk_size - (data_blocks_cnt << s->block_size_bits)) {
-data_blocks_cnt++;
-}
-bitmap_blocks_cnt = data_blocks_cnt >> s->chunk_ratio_bits;
-if (data_blocks_cnt - (bitmap_blocks_cnt << s->chunk_ratio_bits)) {
-bitmap_blocks_cnt++;
-}
+data_blocks_cnt = DIV_ROUND_UP(s->virtual_disk_size, s->block_size);
+bitmap_blocks_cnt = DIV_ROUND_UP(data_blocks_cnt, s->chunk_ratio);
 
 if (s->parent_entries) {
 s->bat_entries = bitmap_blocks_cnt * (s->chunk_ratio + 1);
-- 
1.9.3

[Qemu-block] [PULL 05/12] iscsi: add support for getting CHAP password via QCryptoSecret API

[Jeff Cody]

From: "Daniel P. Berrange" 

The iSCSI driver currently accepts the CHAP password in plain text
as a block driver property. This change adds a new "password-secret"
property that accepts the ID of a QCryptoSecret instance.

  $QEMU \
 -object secret,id=sec0,filename=/home/berrange/example.pw \
 -drive driver=iscsi,url=iscsi://example.com/target-foo/lun1,\
user=dan,password-secret=sec0

Signed-off-by: Daniel P. Berrange 
Message-id: 1453385961-10718-4-git-send-email-berra...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/iscsi.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 9fe76f4..128ea79 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -39,6 +39,7 @@
 #include "sysemu/sysemu.h"
 #include "qmp-commands.h"
 #include "qapi/qmp/qstring.h"
+#include "crypto/secret.h"
 
 #include 
 #include 
@@ -1080,6 +1081,8 @@ static void parse_chap(struct iscsi_context *iscsi, const 
char *target,
 QemuOpts *opts;
 const char *user = NULL;
 const char *password = NULL;
+const char *secretid;
+char *secret = NULL;
 
 list = qemu_find_opts("iscsi");
 if (!list) {
@@ -1099,8 +1102,20 @@ static void parse_chap(struct iscsi_context *iscsi, 
const char *target,
 return;
 }
 
+secretid = qemu_opt_get(opts, "password-secret");
 password = qemu_opt_get(opts, "password");
-if (!password) {
+if (secretid && password) {
+error_setg(errp, "'password' and 'password-secret' properties are "
+   "mutually exclusive");
+return;
+}
+if (secretid) {
+secret = qcrypto_secret_lookup_as_utf8(secretid, errp);
+if (!secret) {
+return;
+}
+password = secret;
+} else if (!password) {
 error_setg(errp, "CHAP username specified but no password was given");
 return;
 }
@@ -1108,6 +1123,8 @@ static void parse_chap(struct iscsi_context *iscsi, const 
char *target,
 if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
 error_setg(errp, "Failed to set initiator username and password");
 }
+
+g_free(secret);
 }
 
 static void parse_header_digest(struct iscsi_context *iscsi, const char 
*target,
@@ -1858,6 +1875,11 @@ static QemuOptsList qemu_iscsi_opts = {
 .type = QEMU_OPT_STRING,
 .help = "password for CHAP authentication to target",
 },{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
+.help = "ID of the secret providing password for CHAP "
+"authentication to target",
+},{
 .name = "header-digest",
 .type = QEMU_OPT_STRING,
 .help = "HeaderDigest setting. "
-- 
1.9.3

[Qemu-block] [PULL 00/12] Block patches

[Jeff Cody]

The following changes since commit 071608b519adf62bc29c914343a21c5407ab1ac9:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-20160229-1' into 
staging (2016-02-29 12:24:26 +)

are available in the git repository at:


  g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to cc199b16cf4cb9279aca73f5f5dce2cc337b9079:

  iotests/124: Add cluster_size mismatch test (2016-02-29 14:55:14 -0500)


Block patches


Daniel P. Berrange (3):
  rbd: add support for getting password from QCryptoSecret object
  curl: add support for HTTP authentication parameters
  iscsi: add support for getting CHAP password via QCryptoSecret API

Fam Zheng (2):
  mirror: Rewrite mirror_iteration
  mirror: Add mirror_wait_for_io

John Snow (3):
  block/backup: make backup cluster size configurable
  block/backup: avoid copying less than full target clusters
  iotests/124: Add cluster_size mismatch test

Max Reitz (2):
  vhdx: DIV_ROUND_UP() in vhdx_calc_bat_entries()
  vhdx: Simplify vhdx_set_shift_bits()

Peter Lieven (1):
  block/nfs: add support for setting debug level

Vasiliy Tolstov (1):
  sheepdog: allow to delete snapshot

 block/backup.c |  87 +++
 block/curl.c   |  66 +
 block/iscsi.c  |  24 ++-
 block/mirror.c | 353 +++--
 block/nfs.c|  12 ++
 block/rbd.c|  47 ++
 block/sheepdog.c   | 125 +++-
 block/vhdx.c   |  18 +--
 tests/qemu-iotests/109.out |  80 +-
 tests/qemu-iotests/124 |  58 +++-
 tests/qemu-iotests/124.out |   4 +-
 trace-events   |   1 -
 12 files changed, 641 insertions(+), 234 deletions(-)

-- 
1.9.3

[Qemu-block] [PULL 01/12] block/nfs: add support for setting debug level

[Jeff Cody]

From: Peter Lieven 

recent libnfs versions support logging debug messages. Add
support for it in qemu through an URL parameter.

Example:
 qemu -cdrom nfs://127.0.0.1/iso/my.iso?debug=2

Signed-off-by: Peter Lieven 
Reviewed-by: Fam Zheng 
Message-id: 1447052973-14513-1-git-send-email...@kamp.de
Signed-off-by: Jeff Cody 
---
 block/nfs.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index 5eb8c13..7220e89 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -36,6 +36,7 @@
 #include 
 
 #define QEMU_NFS_MAX_READAHEAD_SIZE 1048576
+#define QEMU_NFS_MAX_DEBUG_LEVEL 2
 
 typedef struct NFSClient {
 struct nfs_context *context;
@@ -334,6 +335,17 @@ static int64_t nfs_client_open(NFSClient *client, const 
char *filename,
 }
 nfs_set_readahead(client->context, val);
 #endif
+#ifdef LIBNFS_FEATURE_DEBUG
+} else if (!strcmp(qp->p[i].name, "debug")) {
+/* limit the maximum debug level to avoid potential flooding
+ * of our log files. */
+if (val > QEMU_NFS_MAX_DEBUG_LEVEL) {
+error_report("NFS Warning: Limiting NFS debug level"
+ " to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
+val = QEMU_NFS_MAX_DEBUG_LEVEL;
+}
+nfs_set_debug(client->context, val);
+#endif
 } else {
 error_setg(errp, "Unknown NFS parameter name: %s",
qp->p[i].name);
-- 
1.9.3

Re: [Qemu-block] [PATCH v4 0/3] blockjob: correct backup cluster size for backups

[Jeff Cody]

On Thu, Feb 25, 2016 at 03:58:28PM -0500, John Snow wrote:
> Backups sometimes need a non-64KiB transfer cluster size.
> See patch #2 for the detailed justificaton.
> 
> ===
> v4:
> ===
> 
> 02: Polished the error message.
> 
> ===
> v3:
> ===
> 
> 01: +R-B
> 02: Added failure mode for bdrv_get_info, including critical failure for when
> we don't have a backing file but couldn't retrieve the cluster_size info.
> 
> ===
> v2:
> ===
> 
> 01: Removed "sectors_per_cluster" as a cached property of the Backup Block 
> Job,
> In favor of recomputing it with a small function where needed.
> (I like v1 more. Thoughts?)
> 02: Expand correction to all backup modes instead of just incremental.
> Added credit: Thanks to Fam Z for noticing this problem!
> 03: Minor phrasing change in a comment.
> Added r-b.
> 
> 
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch incremental-granularity-fix
> https://github.com/jnsnow/qemu/tree/incremental-granularity-fix
> 
> This version is tagged incremental-granularity-fix-v4:
> https://github.com/jnsnow/qemu/releases/tag/incremental-granularity-fix-v4
> 
> John Snow (3):
>   block/backup: make backup cluster size configurable
>   block/backup: avoid copying less than full target clusters
>   iotests/124: Add cluster_size mismatch test
> 
>  block/backup.c | 87 
> ++
>  tests/qemu-iotests/124 | 58 ---
>  tests/qemu-iotests/124.out |  4 +--
>  3 files changed, 112 insertions(+), 37 deletions(-)
> 
> -- 
> 2.4.3
> 

Thanks, applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc.git block


Jeff

Re: [Qemu-block] [PATCH] qcow2: Clarify that compressed cluster offset requires shift

[Max Reitz]

On 29.02.2016 17:06, Eric Blake wrote:
> On 02/29/2016 08:46 AM, Max Reitz wrote:
> 
  Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):

 -Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to 
 a
 -cluster boundary!
 +Bit  0 -  x:Bits 9-(x+9) of host cluster offset. This is
>>>
>>> I'd rather use one of "9..(x+9)", "[9, x+9]", "9 - (x+9)", "9—(x+9)" in
>>> order to clarify that this is not a minus but a range.
>>
>> ...unless, of course, this is actually a byte offset, which it seems to
>> be, judging from qcow2_get_cluster_offset() (and
>> qcow2_decompress_cluster(), as Kevin said on IRC).
> 
> Hmm, then I may be totally wrong. (Serves me right for trying to write a
> spec patch without testing actual behavior).
> 
> At the least, we may want to say that things are usually not aligned to
> even a _sector_ boundary (not just cluster boundary), and also clarify
> whether the sector length is truncated or rounded up (zlib doesn't care
> about trailing garbage during decompression, but you DO need to know how
> many sectors to read to ensure that you are supplying zlib with trailing
> garbage, rather than a truncated stream).

Yes, I agree.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH] qcow2: Clarify that compressed cluster offset requires shift

[Eric Blake]

On 02/29/2016 08:46 AM, Max Reitz wrote:

>>>  Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):
>>>
>>> -Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a
>>> -cluster boundary!
>>> +Bit  0 -  x:Bits 9-(x+9) of host cluster offset. This is
>>
>> I'd rather use one of "9..(x+9)", "[9, x+9]", "9 - (x+9)", "9—(x+9)" in
>> order to clarify that this is not a minus but a range.
> 
> ...unless, of course, this is actually a byte offset, which it seems to
> be, judging from qcow2_get_cluster_offset() (and
> qcow2_decompress_cluster(), as Kevin said on IRC).

Hmm, then I may be totally wrong. (Serves me right for trying to write a
spec patch without testing actual behavior).

At the least, we may want to say that things are usually not aligned to
even a _sector_ boundary (not just cluster boundary), and also clarify
whether the sector length is truncated or rounded up (zlib doesn't care
about trailing garbage during decompression, but you DO need to know how
many sectors to read to ensure that you are supplying zlib with trailing
garbage, rather than a truncated stream).

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] QCow2 compression

[Eric Blake]

On 02/29/2016 07:01 AM, Kevin Wolf wrote:
>> I have for example a compressed cluster with an L2 entry value of 4A
>> C0 00 00 00 3D 97 50. This would lead me to believe the cluster starts
>> at offset 0x3D9750 and has a length of 0x2B 512-byte sectors (or 0x2B
>> times 0x200 = 0x5600). Added to the offset this would give an end for
>> the cluster at offset 0x3DED50. However, it is clear from looking at
>> the image that the compressed cluster extends further, the data ending
>> at 0x3DEDD5 and being followed by some zero padding until 0x3DEDF0
>> where the file ends. How can I know the data extends beyond the length
>> I calculated? Did I misunderstand the documentation somewhere? Why
>> does the file end here versus a cluster aligned offset?
> 
> This zero padding happens in the very last cluster in the image in order
> to ensure that the image file is aligned to a multiple of the cluster
> size (qcow2 images are defined to consist of "units of constant size",
> i.e. only full clusters).

The qcow2 spec is just fine with a host file that is not a multiple of a
cluster size (the bytes not present must behave as if they read as 0).
Whether we write explicit padding bytes or just truncate the file, on
the last cluster, shouldn't matter.  Although if we are only padding
part of the way, that's a bit odd.

> qemu-devel is alright for this kind of questions. I'm also copying
> qemu-block now, which makes the email thread more visible for the
> relevant people (as qemu-devel is relatively high traffic these days),
> but qemu-devel should always be included anyway.

Good idea, and sorry I wrote my first reply before seeing your message,
where I didn't include qemu-block.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] QCow2 compression

[Eric Blake]

On 02/29/2016 07:59 AM, Eric Blake wrote:

>> an L2 entry value of 4A C0 00 00 00 3D 97 50.
> 
> So with default 64k clusters, x = 62 - (16 - 8) = 54.  Bits 0-54 are the
> host cluster offset, or 0x003d9750, but that is in terms of host
> sectors.  The comment in block/qcow2.c is telling, and perhaps we should
> improve the qcow2 spec to make it obvious:
> 
>   - Size of compressed clusters is stored in sectors to reduce bit usage
> in the cluster offsets.
> 
> Thus, in your image, the guest compressed data starts at sector
> 0x003d9750, or host file offset 0x7b2ea000.  This value is NOT aligned
> to a cluster, but IS aligned to a sector (since a sector is the smallest
> unit we write to), and makes more sense than something ending in 0x50
> (which is not sector aligned).

Disclaimer - I did not test things, so I may be misreading the spec myself.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] [PATCH v4 RFC 00/17] qcow2: persistent dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

On 27.02.2016 00:41, John Snow wrote:

Do you have this mirrored in a git repo so I can browse it more
effectively? I can't figure out what all the prerequisite patches are,
so it will be convenient to just have a repo in that case for the RFC.


done: https://src.openvz.org/users/vsementsov/repos/qemu/browse



On 02/17/2016 10:28 AM, Vladimir Sementsov-Ogievskiy wrote:

This series add persistent dirty bitmaps feature to qcow2.
Specification is in docs/spec/qcow2.txt (not merged yet, see
[PATCH v10] spec: add qcow2 bitmaps extension specification)

This series are based on Fam's
[PATCH v2 00/13] Dirty bitmap changes for migration/persistence work
(meta bitmaps not used, and only hbitmap_deserialize_finish from
serialization)

This also needs some preparation patches, most of them are in my
bitmap-migration series. I've not listed them here to keep things
simpler, this is RFC any way.

v4:

Previous version was posted more than five months ago, so I will not
carefully list all changes.

What should be noted:
  - some changes are made to sutisfy last version of specification
- removed staff, related to possibility of saving bitmaps for one
  disk in the other qcow2.
  - to make bitmap store/load zero-copy, I've moved load/store code to
HBitmap - this is new patch 01.
so, bdrv_dirty_bitmap_serialize_part and friends are not needed,
only hbitmap_deserialize_finish, to repair bitmap consistency after
loading its last level.
  - two patches added about AUTO and EXTRA_DATA_COMPATIBLE flags
  - some fixes made after John's comments on v3

Vladimir Sementsov-Ogievskiy (17):
   hbitmap: load/store
   qcow2: Bitmaps extension: structs and consts
   qcow2-dirty-bitmap: read dirty bitmap directory
   qcow2-dirty-bitmap: add qcow2_bitmap_load()
   qcow2-dirty-bitmap: add qcow2_bitmap_store()
   qcow2: add dirty bitmaps extension
   qcow2-dirty-bitmap: add qcow2_bitmap_load_check()
   block: store persistent dirty bitmaps
   block: add bdrv_load_dirty_bitmap()
   qcow2-dirty-bitmap: add autoclear bit
   qemu: command line option for dirty bitmaps
   qcow2-dirty-bitmap: add IN_USE flag
   qcow2-dirty-bitmaps: disallow stroing bitmap to other bs
   iotests: add VM.test_launcn()
   iotests: test internal persistent dirty bitmap
   qcow2-dirty-bitmap: add AUTO flag
   qcow2-dirty-bitmap: add EXTRA_DATA_COMPATIBLE flag

  block.c   |   3 +
  block/Makefile.objs   |   2 +-
  block/dirty-bitmap.c  | 101 +
  block/qcow2-dirty-bitmap.c| 839 ++
  block/qcow2.c | 105 +-
  block/qcow2.h |  59 +++
  blockdev.c|  36 ++
  include/block/block_int.h |   9 +
  include/block/dirty-bitmap.h  |  21 ++
  include/qemu/hbitmap.h|  12 +
  include/sysemu/blockdev.h |   1 +
  include/sysemu/sysemu.h   |   1 +
  qemu-options.hx   |  35 ++
  tests/qemu-iotests/160| 112 ++
  tests/qemu-iotests/160.out|  21 ++
  tests/qemu-iotests/group  |   1 +
  tests/qemu-iotests/iotests.py |  25 ++
  util/hbitmap.c| 182 +
  vl.c  |  78 
  19 files changed, 1640 insertions(+), 3 deletions(-)
  create mode 100644 block/qcow2-dirty-bitmap.c
  create mode 100755 tests/qemu-iotests/160
  create mode 100644 tests/qemu-iotests/160.out




--
Best regards,
Vladimir

Re: [Qemu-block] [PATCH] qcow2: Clarify that compressed cluster offset requires shift

[Max Reitz]

On 29.02.2016 16:42, Max Reitz wrote:
> On 29.02.2016 16:11, Eric Blake wrote:
>> The specs for the host cluster offset of a compressed cluster
>> were not clear that the offset is in terms of sectors, and requires
>> a shift by 9 to be a byte offset.  Add some more text to make the
>> interpretation obvious.
>>
>> CC: mgre...@cinci.rr.com
>> Signed-off-by: Eric Blake 
>> ---
>>  docs/specs/qcow2.txt | 9 +++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
>> index 80cdfd0..7049415 100644
>> --- a/docs/specs/qcow2.txt
>> +++ b/docs/specs/qcow2.txt
>> @@ -323,11 +323,16 @@ Standard Cluster Descriptor:
>>
>>  Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):
>>
>> -Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a
>> -cluster boundary!
>> +Bit  0 -  x:Bits 9-(x+9) of host cluster offset. This is
> 
> I'd rather use one of "9..(x+9)", "[9, x+9]", "9 - (x+9)", "9—(x+9)" in
> order to clarify that this is not a minus but a range.

...unless, of course, this is actually a byte offset, which it seems to
be, judging from qcow2_get_cluster_offset() (and
qcow2_decompress_cluster(), as Kevin said on IRC).

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH] qcow2: Clarify that compressed cluster offset requires shift

[Max Reitz]

On 29.02.2016 16:11, Eric Blake wrote:
> The specs for the host cluster offset of a compressed cluster
> were not clear that the offset is in terms of sectors, and requires
> a shift by 9 to be a byte offset.  Add some more text to make the
> interpretation obvious.
> 
> CC: mgre...@cinci.rr.com
> Signed-off-by: Eric Blake 
> ---
>  docs/specs/qcow2.txt | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 80cdfd0..7049415 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -323,11 +323,16 @@ Standard Cluster Descriptor:
> 
>  Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):
> 
> -Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a
> -cluster boundary!
> +Bit  0 -  x:Bits 9-(x+9) of host cluster offset. This is

I'd rather use one of "9..(x+9)", "[9, x+9]", "9 - (x+9)", "9—(x+9)" in
order to clarify that this is not a minus but a range.

Max

> +usually _not_ aligned to a cluster boundary!
> 
> x+1 - 61:Compressed size of the images in sectors of 512 bytes
> 
> +The bits of the host cluster offset not specified in the cluster descriptor
> +are 0 (bits 0-8 are obvious because a 512-byte sector is the smallest
> +addressable unit, while bits 56-63 implies that a qcow2 file cannot exceed
> +2^56 bytes in size).
> +
>  If a cluster is unallocated, read requests shall read the data from the 
> backing
>  file (except if bit 0 in the Standard Cluster Descriptor is set). If there is
>  no backing file or the backing file is smaller than the image, they shall 
> read
> 




signature.asc
Description: OpenPGP digital signature

[Qemu-block] ping [PATCH v14] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Programmingkid]

I do think this patch is ready to be added to QEMU. I have listened to what you 
said and implemented your changes. 

https://patchwork.ozlabs.org/patch/579325/

Mac OS X can be picky when it comes to allowing the user
to use physical devices in QEMU. Most mounted volumes
appear to be off limits to QEMU. If an issue is detected,
a message is displayed showing the user how to unmount a
volume. Now QEMU uses both CD and DVD media.

Signed-off-by: John Arbuckle 

---
Changed filename variable to const char * type.
Removed snprintf call for filename variable.
filename is set to bsd_path if using a physical device that isn't a DVD or CD.


 block/raw-posix.c |  167 -
 1 files changed, 127 insertions(+), 40 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6df3067..48dc5a8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -44,6 +44,7 @@
 #include 
 #include 
 //#include 
+#include 
 #include 
 #endif
 
@@ -1963,33 +1964,47 @@ BlockDriver bdrv_file = {
 /* host device */
 
 #if defined(__APPLE__) && defined(__MACH__)
-static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator );
 static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
 CFIndex maxPathSize, int flags);
-kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator )
+static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator)
 {
-kern_return_t   kernResult;
+kern_return_t kernResult = KERN_FAILURE;
 mach_port_t masterPort;
 CFMutableDictionaryRef  classesToMatch;
+const char *matching_array[] = {kIODVDMediaClass, kIOCDMediaClass};
+char *mediaType = NULL;
 
 kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort );
 if ( KERN_SUCCESS != kernResult ) {
 printf( "IOMasterPort returned %d\n", kernResult );
 }
 
-classesToMatch = IOServiceMatching( kIOCDMediaClass );
-if ( classesToMatch == NULL ) {
-printf( "IOServiceMatching returned a NULL dictionary.\n" );
-} else {
-CFDictionarySetValue( classesToMatch, CFSTR( kIOMediaEjectableKey ), 
kCFBooleanTrue );
-}
-kernResult = IOServiceGetMatchingServices( masterPort, classesToMatch, 
mediaIterator );
-if ( KERN_SUCCESS != kernResult )
-{
-printf( "IOServiceGetMatchingServices returned %d\n", kernResult );
-}
+int index;
+for (index = 0; index < ARRAY_SIZE(matching_array); index++) {
+classesToMatch = IOServiceMatching(matching_array[index]);
+if (classesToMatch == NULL) {
+error_report("IOServiceMatching returned NULL for %s",
+ matching_array[index]);
+continue;
+}
+CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey),
+ kCFBooleanTrue);
+kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch,
+  mediaIterator);
+if (kernResult != KERN_SUCCESS) {
+error_report("Note: IOServiceGetMatchingServices returned %d",
+ kernResult);
+continue;
+}
 
-return kernResult;
+/* If a match was found, leave the loop */
+if (*mediaIterator != 0) {
+DPRINTF("Matching using %s\n", matching_array[index]);
+mediaType = g_strdup(matching_array[index]);
+break;
+}
+}
+return mediaType;
 }
 
 kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
@@ -2021,7 +2036,46 @@ kern_return_t GetBSDPath(io_iterator_t mediaIterator, 
char *bsdPath,
 return kernResult;
 }
 
-#endif
+/* Sets up a real cdrom for use in QEMU */
+static bool setup_cdrom(char *bsd_path, Error **errp)
+{
+int index, num_of_test_partitions = 2, fd;
+char test_partition[MAXPATHLEN];
+bool partition_found = false;
+
+/* look for a working partition */
+for (index = 0; index < num_of_test_partitions; index++) {
+snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path,
+ index);
+fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE);
+if (fd >= 0) {
+partition_found = true;
+qemu_close(fd);
+break;
+}
+}
+
+/* if a working partition on the device was not found */
+if (partition_found == false) {
+error_setg(errp, "Failed to find a working partition on disc");
+} else {
+DPRINTF("Using %s as optical disc\n", test_partition);
+pstrcpy(bsd_path, MAXPATHLEN, test_partition);
+}
+return partition_found;
+}
+
+/* Prints directions on mounting and unmounting a device */
+static void print_unmounting_directions(const char *file_name)
+{
+error_report("If device %s is mounted on the desktop, unmount"
+ " it first before using it in QEMU", file_name);
+error_report("Command to u

[Qemu-block] [PATCH] qcow2: Clarify that compressed cluster offset requires shift

[Eric Blake]

The specs for the host cluster offset of a compressed cluster
were not clear that the offset is in terms of sectors, and requires
a shift by 9 to be a byte offset.  Add some more text to make the
interpretation obvious.

CC: mgre...@cinci.rr.com
Signed-off-by: Eric Blake 
---
 docs/specs/qcow2.txt | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 80cdfd0..7049415 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -323,11 +323,16 @@ Standard Cluster Descriptor:

 Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):

-Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a
-cluster boundary!
+Bit  0 -  x:Bits 9-(x+9) of host cluster offset. This is
+usually _not_ aligned to a cluster boundary!

x+1 - 61:Compressed size of the images in sectors of 512 bytes

+The bits of the host cluster offset not specified in the cluster descriptor
+are 0 (bits 0-8 are obvious because a 512-byte sector is the smallest
+addressable unit, while bits 56-63 implies that a qcow2 file cannot exceed
+2^56 bytes in size).
+
 If a cluster is unallocated, read requests shall read the data from the backing
 file (except if bit 0 in the Standard Cluster Descriptor is set). If there is
 no backing file or the backing file is smaller than the image, they shall read
-- 
2.5.0

Re: [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields()

[Paolo Bonzini]

On 29/02/2016 15:36, Kevin Wolf wrote:
> Hi all,
> 
> I'm currently trying to get rid of bdrv_move_feature_fields(), so we can
> finally have more than one BB per BDS. Generally the way to do this is
> to move features from BDS and block.c to BB and block-backend.c.
> However, for two of the features I'm not sure about this:
> 
> * Copy on Read:
> 
>   When Jeff introduced bdrv_append() in commit 8802d1fd, the CoR flag
>   was already moved to the new top level when taking a snapshot. Does
>   anyone remember why it works like that? It doesn't seem to make a lot
>   of sense to me.
> 
>   The use case for manually enabled CoR is to avoid reading data twice
>   from a slow remote image, so we want to save it to a local overlay,
>   say an ISO image accessed via HTTP to a local qcow2 overlay. When
>   taking a snapshot, we end up with a backing chain like this:
> 
>   http <- local.qcow2 <- snap_overlay.qcow2
> 
>   There is no point in performing copy on read from local.qcow2 into
>   snap_overlay.qcow2, we just want to keep copying data from the remote
>   source into local.qcow2.
> 
>   Possible caveat: We would be writing to a backing file, but that's
>   similar to what some block jobs do, so if we design our op blockers to
>   cover this case, it should be fine.

Yes, considering that bdrv_append() didn't reopen read-only it probably
was a mistake.

>   On the other hand, the QMP interface clearly describes bitmaps as
>   belonging to a node rather than a BB (you can use node-name, even with
>   no BB attached), so moving them could be considered a bug, even if
>   it is the existing behaviour.
> 
>   I can imagine use cases for both ways, so the interface that would
>   make the most sense to me is to generally keep BDSes at their node,
>   and to provide a QMP command to move them to a different one.

I'm not sure if anyone has actually ever used dirty bitmaps except the
Parallel folks.  I think it makes sense to keep them in the node.

At the time I added the code to bdrv_move_feature_fields(), the
reasoning was probably that no one expected dirty bitmaps except on the
top BDS, and there were no blockers, so anything else would have caused
bugs.  Nowadays, with blockers _and_ user bitmaps, it's probably best
not to move the dirty bitmaps.

Paolo

[Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields()

[Kevin Wolf]

Hi all,

I'm currently trying to get rid of bdrv_move_feature_fields(), so we can
finally have more than one BB per BDS. Generally the way to do this is
to move features from BDS and block.c to BB and block-backend.c.
However, for two of the features I'm not sure about this:

* Copy on Read:

  When Jeff introduced bdrv_append() in commit 8802d1fd, the CoR flag
  was already moved to the new top level when taking a snapshot. Does
  anyone remember why it works like that? It doesn't seem to make a lot
  of sense to me.

  The use case for manually enabled CoR is to avoid reading data twice
  from a slow remote image, so we want to save it to a local overlay,
  say an ISO image accessed via HTTP to a local qcow2 overlay. When
  taking a snapshot, we end up with a backing chain like this:

  http <- local.qcow2 <- snap_overlay.qcow2

  There is no point in performing copy on read from local.qcow2 into
  snap_overlay.qcow2, we just want to keep copying data from the remote
  source into local.qcow2.

  Possible caveat: We would be writing to a backing file, but that's
  similar to what some block jobs do, so if we design our op blockers to
  cover this case, it should be fine.

  I'm actually pretty sure that simply removing COR from the list, and
  therefore changing the behaviour to not move it to the top any more,
  is the right thing to do and could be considered a bug fix.

* Dirty bitmaps:

  We're currently trying, and if I'm not mistaken failing, to move dirty
  bitmaps to the top. The (back then one) bitmap was first added to the
  list in Paolo's commit a9fc4408, with the following commit message:

While these should not be in use at the time a transaction is
started, a command in the prepare phase of a transaction might have
added them, so they need to be brought over.

  At that point, there was no transactionable command that did this in
  the prepare phase. Today we have mirror and backup, but op blockers
  should prevent them from being mixed with snapshots in a single
  transaction, so I can't see how this change had any effect.

  The reason why I think we're failing to move dirty bitmaps to the top
  today is that we're moving the head of the list to a different object
  without updating the prev link in the first element, so in any case
  it's buggy today.

  I really would like to keep bitmaps on the BDS where they are, but
  unfortunately, we also have user-defined bitmaps by now, and if we
  change whether they stick with the top level, that's a change that is
  visible on the QMP interface.

  On the other hand, the QMP interface clearly describes bitmaps as
  belonging to a node rather than a BB (you can use node-name, even with
  no BB attached), so moving them could be considered a bug, even if
  it is the existing behaviour.

  I can imagine use cases for both ways, so the interface that would
  make the most sense to me is to generally keep BDSes at their node,
  and to provide a QMP command to move them to a different one.

  With compatibility in mind, this seems to be a reall tough one,
  though.

Any comments or ideas how to proceed with those two?

Kevin

Re: [Qemu-block] [Qemu-devel] QCow2 compression

[Kevin Wolf]

[ Cc: qemu-block ]

Am 27.02.2016 um 06:00 hat mgre...@cinci.rr.com geschrieben:
> Hello, I am hoping someone here can help me. I am implementing QCow2
> support for a PC emulator project and have a couple questions
> regarding compression I haven't been able to figure out on my own.
> 
> First some background: I am using the information I found at
> https://people.gnome.org/~markmc/qcow-image-format.html and I have
> implemented working support for QCow2 images as described there except
> for snapshots, encryption, and compression. Of these, only compression
> is of immediate use to me.

First of all, the preferable source is the qcow2 specification from the
QEMU git repository:

http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/qcow2.txt

The description you were using is good, but rather old. Not a problem
for the basics of compression because these things haven't ever changed,
but if you want to make sense of everything in a current image, you'll
need something more recent.

> I have some QCow2 images all using 16-bit clusters created using
> qemu-img 2.1.2 (the version bundled with Debian stable). According to
> the documentation I linked, 8 bits of an L2 table entry following the
> copy flag should say how many 512 byte sectors a compressed cluster
> takes up and the remaining bits are the actual offset of the cluster
> within the file.

The spec says this (which is essentially the same):

L2 table entry:

Bit  0 -  61:   Cluster descriptor

  62:   0 for standard clusters
1 for compressed clusters

  63:   0 for a cluster that is unused or requires COW, 1 if its
refcount is exactly one. This information is only accurate
in L2 tables that are reachable from the active L1
table.

Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):

Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a
cluster boundary!

   x+1 - 61:Compressed size of the images in sectors of 512 bytes

> I have for example a compressed cluster with an L2 entry value of 4A
> C0 00 00 00 3D 97 50. This would lead me to believe the cluster starts
> at offset 0x3D9750 and has a length of 0x2B 512-byte sectors (or 0x2B
> times 0x200 = 0x5600). Added to the offset this would give an end for
> the cluster at offset 0x3DED50. However, it is clear from looking at
> the image that the compressed cluster extends further, the data ending
> at 0x3DEDD5 and being followed by some zero padding until 0x3DEDF0
> where the file ends. How can I know the data extends beyond the length
> I calculated? Did I misunderstand the documentation somewhere? Why
> does the file end here versus a cluster aligned offset?

This zero padding happens in the very last cluster in the image in order
to ensure that the image file is aligned to a multiple of the cluster
size (qcow2 images are defined to consist of "units of constant size",
i.e. only full clusters).

The zeros are not part of the compressed data, though, that's why the
Compressed Cluster Descriptor indicates a shorter size. Had another
compressed cluster been written to the same image, it might have ended
up where you are seeing the zero padding now. (The trick with
compression is that multiple guest clusters can end up in a single host
cluster.)

> A final question: I noticed that compressed clusters typically have a
> reference count higher than one, yet there are no snapshots present in
> the image. I suspect the count is incremented for each compressed
> cluster that exists even partially within a normal sized cluster
> region of the file, but I can find no documentation to this effect and
> am merely speculating. Am I correct?

Yes. You have multiple L2 entries referring to the same cluster, so it
needs to have a refcount that represents this.

Once you overwrite a compressed cluster, a copy-on-write operation is
performed and the refcount is decreased. You want to free the (host)
cluster holding the compressed data only after the last L2 entry using
it has gone.

> If it is the wrong place to ask these questions, I would appreciate it
> if anyone could direct me to a more appropriate venue. Thank you for
> taking the time to read this and tanks in advance for any assistance.

qemu-devel is alright for this kind of questions. I'm also copying
qemu-block now, which makes the email thread more visible for the
relevant people (as qemu-devel is relatively high traffic these days),
but qemu-devel should always be included anyway.

Kevin

[Qemu-block] [PATCH v4 26/26] block: remove support for legecy AES qcow/qcow2 encryption

[Daniel P. Berrange]

Refuse to use images with the legacy AES-CBC encryption
format in the system emulators. They are still fully
supported in the qemu-img, qemu-io & qemu-nbd tools in
order to allow data to be liberated and for compatibility
with older QEMU versions. Continued support in these tools
is not a notable burden with the new FDE framework.

Signed-off-by: Daniel P. Berrange 
---
 block.c| 12 +---
 block/qcow.c   |  9 +
 block/qcow2.c  |  8 
 include/block/block.h  |  1 +
 tests/qemu-iotests/049.out |  3 ---
 tests/qemu-iotests/087.out | 12 
 tests/qemu-iotests/134.out | 12 
 7 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/block.c b/block.c
index 861cb76..442cf69 100644
--- a/block.c
+++ b/block.c
@@ -313,6 +313,11 @@ static int bdrv_is_whitelisted(BlockDriver *drv, bool 
read_only)
 return 0;
 }
 
+bool bdrv_uses_whitelist(void)
+{
+return use_bdrv_whitelist;
+}
+
 typedef struct CreateCo {
 BlockDriver *drv;
 char *filename;
@@ -1023,13 +1028,6 @@ static int bdrv_open_common(BlockDriverState *bs, 
BdrvChild *file,
 goto free_and_fail;
 }
 
-if (bs->encrypted) {
-error_report("Encrypted images are deprecated");
-error_printf("Support for them will be removed in a future release.\n"
- "You can use 'qemu-img convert' to convert your image"
- " to an unencrypted one.\n");
-}
-
 ret = refresh_total_sectors(bs, bs->total_sectors);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "Could not refresh total sector count");
diff --git a/block/qcow.c b/block/qcow.c
index 003d6d8..f8af9e9 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -182,6 +182,15 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 if (s->crypt_method_header) {
 if (s->crypt_method_header == QCOW_CRYPT_AES) {
 QCryptoBlockOptionsQCow *tmp;
+
+if (bdrv_uses_whitelist()) {
+error_setg(errp,
+   "Use of AES-CBC encrypted qcow images is no longer "
+   "supported. Please use the qcow2 LUKS format 
instead.");
+ret = -ENOSYS;
+goto fail;
+}
+
 ov = opts_visitor_new(opts);
 
 crypto_opts = g_new0(QCryptoBlockOpenOptions, 1);
diff --git a/block/qcow2.c b/block/qcow2.c
index daae6ec..7391b39 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1223,6 +1223,14 @@ static int qcow2_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 s->crypt_method_header = header.crypt_method;
 if (s->crypt_method_header) {
+if (bdrv_uses_whitelist() &&
+s->crypt_method_header == QCOW_CRYPT_AES) {
+error_setg(errp,
+   "Use of AES-CBC encrypted qcow2 images is no longer "
+   "supported. Please use the qcow2 LUKS format instead.");
+ret = -ENOSYS;
+goto fail;
+}
 bs->encrypted = 1;
 }
 
diff --git a/include/block/block.h b/include/block/block.h
index 7d7f126..46950b8 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -191,6 +191,7 @@ void bdrv_io_limits_update_group(BlockDriverState *bs, 
const char *group);
 
 void bdrv_init(void);
 void bdrv_init_with_whitelist(void);
+bool bdrv_uses_whitelist(void);
 BlockDriver *bdrv_find_protocol(const char *filename,
 bool allow_protocol_prefix,
 Error **errp);
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index c9f0bc5..e0bedc0 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -187,9 +187,6 @@ qemu-img create -f qcow2 -o encryption=off TEST_DIR/t.qcow2 
64M
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 qemu-img create -f qcow2 --object secret,id=sec0,data=123456 -o 
encryption=on,key-secret=sec0 TEST_DIR/t.qcow2 64M
-qemu-img: Encrypted images are deprecated
-Support for them will be removed in a future release.
-You can use 'qemu-img convert' to convert your image to an unencrypted one.
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=on 
cluster_size=65536 lazy_refcounts=off refcount_bits=16 key-secret=sec0
 
 == Check lazy_refcounts option (only with v3) ==
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 6582dda..b8842d5 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -38,17 +38,11 @@ QMP_VERSION
 
 === Encrypted image ===
 
-qemu-img: Encrypted images are deprecated
-Support for them will be removed in a future release.
-You can use 'qemu-img convert' to convert your image to an unencrypted one.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on 
key-secret=sec0
 Testing: -S
 QMP_VERSION
 {"return": {}}
 {"retur

[Qemu-block] [PATCH v4 25/26] block: remove all encryption handling APIs

[Daniel P. Berrange]

Now that all encryption keys must be provided upfront via
the QCryptoSecret API and associated block driver properties
there is no need for any explicit encryption handling APIs
in the block layer. Encryption can be handled transparently
within the block driver. We only retain an API for querying
whether an image is encrypted or not, since that is a
potentially useful piece of metadata to report to the user.

Signed-off-by: Daniel P. Berrange 
---
 block.c   | 82 ++-
 block/crypto.c|  1 -
 block/qapi.c  |  2 +-
 block/qcow.c  |  1 -
 block/qcow2.c |  1 -
 blockdev.c| 41 ++--
 include/block/block.h |  4 ---
 include/block/block_int.h |  1 -
 8 files changed, 5 insertions(+), 128 deletions(-)

diff --git a/block.c b/block.c
index 1fd4a14..861cb76 100644
--- a/block.c
+++ b/block.c
@@ -1691,17 +1691,8 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 goto close_and_fail;
 }
 
-if (!bdrv_key_required(bs)) {
-if (bs->blk) {
-blk_dev_change_media_cb(bs->blk, true);
-}
-} else if (!runstate_check(RUN_STATE_PRELAUNCH)
-   && !runstate_check(RUN_STATE_INMIGRATE)
-   && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */
-error_setg(errp,
-   "Guest must be stopped for opening of encrypted image");
-ret = -EBUSY;
-goto close_and_fail;
+if (bs->blk) {
+blk_dev_change_media_cb(bs->blk, true);
 }
 
 QDECREF(options);
@@ -2187,7 +2178,6 @@ static void bdrv_close(BlockDriverState *bs)
 bs->backing_format[0] = '\0';
 bs->total_sectors = 0;
 bs->encrypted = 0;
-bs->valid_key = 0;
 bs->sg = 0;
 bs->zero_beyond_eof = false;
 QDECREF(bs->options);
@@ -2796,74 +2786,6 @@ int bdrv_is_encrypted(BlockDriverState *bs)
 return bs->encrypted;
 }
 
-int bdrv_key_required(BlockDriverState *bs)
-{
-BdrvChild *backing = bs->backing;
-
-if (backing && backing->bs->encrypted && !backing->bs->valid_key) {
-return 1;
-}
-return (bs->encrypted && !bs->valid_key);
-}
-
-int bdrv_set_key(BlockDriverState *bs, const char *key)
-{
-int ret;
-if (bs->backing && bs->backing->bs->encrypted) {
-ret = bdrv_set_key(bs->backing->bs, key);
-if (ret < 0)
-return ret;
-if (!bs->encrypted)
-return 0;
-}
-if (!bs->encrypted) {
-return -EINVAL;
-} else if (!bs->drv || !bs->drv->bdrv_set_key) {
-return -ENOMEDIUM;
-}
-ret = bs->drv->bdrv_set_key(bs, key);
-if (ret < 0) {
-bs->valid_key = 0;
-} else if (!bs->valid_key) {
-bs->valid_key = 1;
-if (bs->blk) {
-/* call the change callback now, we skipped it on open */
-blk_dev_change_media_cb(bs->blk, true);
-}
-}
-return ret;
-}
-
-/*
- * Provide an encryption key for @bs.
- * If @key is non-null:
- * If @bs is not encrypted, fail.
- * Else if the key is invalid, fail.
- * Else set @bs's key to @key, replacing the existing key, if any.
- * If @key is null:
- * If @bs is encrypted and still lacks a key, fail.
- * Else do nothing.
- * On failure, store an error object through @errp if non-null.
- */
-void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
-{
-if (key) {
-if (!bdrv_is_encrypted(bs)) {
-error_setg(errp, "Node '%s' is not encrypted",
-  bdrv_get_device_or_node_name(bs));
-} else if (bdrv_set_key(bs, key) < 0) {
-error_setg(errp, QERR_INVALID_PASSWORD);
-}
-} else {
-if (bdrv_key_required(bs)) {
-error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
-  "'%s' (%s) is encrypted",
-  bdrv_get_device_or_node_name(bs),
-  bdrv_get_encrypted_filename(bs));
-}
-}
-}
-
 const char *bdrv_get_format_name(BlockDriverState *bs)
 {
 return bs->drv ? bs->drv->format_name : NULL;
diff --git a/block/crypto.c b/block/crypto.c
index 72780f6..cf7823c 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -313,7 +313,6 @@ static int block_crypto_open_generic(QCryptoBlockFormat 
format,
 }
 
 bs->encrypted = 1;
-bs->valid_key = 1;
 
 qemu_co_mutex_init(&crypto->lock);
 
diff --git a/block/qapi.c b/block/qapi.c
index db2d3fb..400229a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -43,7 +43,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, 
Error **errp)
 info->ro = bs->read_only;
 info->drv= g_strdup(bs->drv->format_name);
 info->encrypted  = bs->encrypted;
-info->encryption_key_missing = bdrv_key_required(bs);
+info->encryption_key_missing = false;
 
 info->cache = g_new(Blockdev

[Qemu-block] [PATCH v4 20/26] qcow2: make qcow2_encrypt_sectors encrypt in place

[Daniel P. Berrange]

Instead of requiring separate input/output buffers for
encrypting data, change qcow2_encrypt_sectors() to assume
use of a single buffer, encrypting in place. The current
callers all used the same buffer for input/output already.

Reviewed-by: Eric Blake 
Reviewed-by: Fam Zheng 
Signed-off-by: Daniel P. Berrange 
---
 block/qcow2-cluster.c | 17 +
 block/qcow2.c |  5 ++---
 block/qcow2.h |  3 +--
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 3e887e9..3802d37 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -342,12 +342,8 @@ static int count_contiguous_clusters_by_type(int 
nb_clusters,
 return i;
 }
 
-/* The crypt function is compatible with the linux cryptoloop
-   algorithm for < 4 GB images. NOTE: out_buf == in_buf is
-   supported */
 int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
-  uint8_t *out_buf, const uint8_t *in_buf,
-  int nb_sectors, bool enc,
+  uint8_t *buf, int nb_sectors, bool enc,
   Error **errp)
 {
 union {
@@ -367,14 +363,12 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
sector_num,
 }
 if (enc) {
 ret = qcrypto_cipher_encrypt(s->cipher,
- in_buf,
- out_buf,
+ buf, buf,
  512,
  errp);
 } else {
 ret = qcrypto_cipher_decrypt(s->cipher,
- in_buf,
- out_buf,
+ buf, buf,
  512,
  errp);
 }
@@ -382,8 +376,7 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
sector_num,
 return -1;
 }
 sector_num++;
-in_buf += 512;
-out_buf += 512;
+buf += 512;
 }
 return 0;
 }
@@ -431,7 +424,7 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
 Error *err = NULL;
 assert(s->cipher);
 if (qcow2_encrypt_sectors(s, start_sect + n_start,
-  iov.iov_base, iov.iov_base, n,
+  iov.iov_base, n,
   true, &err) < 0) {
 ret = -EIO;
 error_free(err);
diff --git a/block/qcow2.c b/block/qcow2.c
index 8babecd..0264df7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1506,7 +1506,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 assert(s->cipher);
 Error *err = NULL;
 if (qcow2_encrypt_sectors(s, sector_num,  cluster_data,
-  cluster_data, cur_nr_sectors, false,
+  cur_nr_sectors, false,
   &err) < 0) {
 error_free(err);
 ret = -EIO;
@@ -1606,8 +1606,7 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState 
*bs,
 qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size);
 
 if (qcow2_encrypt_sectors(s, sector_num, cluster_data,
-  cluster_data, cur_nr_sectors,
-  true, &err) < 0) {
+  cur_nr_sectors, true, &err) < 0) {
 error_free(err);
 ret = -EIO;
 goto fail;
diff --git a/block/qcow2.h b/block/qcow2.h
index a063a3c..ae04285 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -540,8 +540,7 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int 
l1_index);
 void qcow2_l2_cache_reset(BlockDriverState *bs);
 int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
 int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
-  uint8_t *out_buf, const uint8_t *in_buf,
-  int nb_sectors, bool enc, Error **errp);
+  uint8_t *buf, int nb_sectors, bool enc, Error 
**errp);
 
 int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
 int *num, uint64_t *cluster_offset);
-- 
2.5.0

[Qemu-block] [PATCH v4 22/26] qcow: make encrypt_sectors encrypt in place

[Daniel P. Berrange]

Instead of requiring separate input/output buffers for
encrypting data, change encrypt_sectors() to assume
use of a single buffer, encrypting in place. One current
caller all uses the same buffer for input/output already
and the other two callers are easily converted todo so.

Signed-off-by: Daniel P. Berrange 
---
 block/qcow.c | 36 ++--
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 251910c..096980b 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -294,12 +294,9 @@ static int qcow_set_key(BlockDriverState *bs, const char 
*key)
 return 0;
 }
 
-/* The crypt function is compatible with the linux cryptoloop
-   algorithm for < 4 GB images. NOTE: out_buf == in_buf is
-   supported */
 static int encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
-   uint8_t *out_buf, const uint8_t *in_buf,
-   int nb_sectors, bool enc, Error **errp)
+   uint8_t *buf, int nb_sectors, bool enc,
+   Error **errp)
 {
 union {
 uint64_t ll[2];
@@ -318,14 +315,12 @@ static int encrypt_sectors(BDRVQcowState *s, int64_t 
sector_num,
 }
 if (enc) {
 ret = qcrypto_cipher_encrypt(s->cipher,
- in_buf,
- out_buf,
+ buf, buf,
  512,
  errp);
 } else {
 ret = qcrypto_cipher_decrypt(s->cipher,
- in_buf,
- out_buf,
+ buf, buf,
  512,
  errp);
 }
@@ -333,8 +328,7 @@ static int encrypt_sectors(BDRVQcowState *s, int64_t 
sector_num,
 return -1;
 }
 sector_num++;
-in_buf += 512;
-out_buf += 512;
+buf += 512;
 }
 return 0;
 }
@@ -454,13 +448,12 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 uint64_t start_sect;
 assert(s->cipher);
 start_sect = (offset & ~(s->cluster_size - 1)) >> 9;
-memset(s->cluster_data + 512, 0x00, 512);
+memset(s->cluster_data, 0x00, 512);
 for(i = 0; i < s->cluster_sectors; i++) {
 if (i < n_start || i >= n_end) {
 Error *err = NULL;
 if (encrypt_sectors(s, start_sect + i,
-s->cluster_data,
-s->cluster_data + 512, 1,
+s->cluster_data, 1,
 true, &err) < 0) {
 error_free(err);
 errno = EIO;
@@ -639,7 +632,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
 }
 if (bs->encrypted) {
 assert(s->cipher);
-if (encrypt_sectors(s, sector_num, buf, buf,
+if (encrypt_sectors(s, sector_num, buf,
 n, false, &err) < 0) {
 goto fail;
 }
@@ -674,9 +667,7 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
 BDRVQcowState *s = bs->opaque;
 int index_in_cluster;
 uint64_t cluster_offset;
-const uint8_t *src_buf;
 int ret = 0, n;
-uint8_t *cluster_data = NULL;
 struct iovec hd_iov;
 QEMUIOVector hd_qiov;
 uint8_t *buf;
@@ -714,21 +705,15 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
 if (bs->encrypted) {
 Error *err = NULL;
 assert(s->cipher);
-if (!cluster_data) {
-cluster_data = g_malloc0(s->cluster_size);
-}
-if (encrypt_sectors(s, sector_num, cluster_data, buf,
+if (encrypt_sectors(s, sector_num, buf,
 n, true, &err) < 0) {
 error_free(err);
 ret = -EIO;
 break;
 }
-src_buf = cluster_data;
-} else {
-src_buf = buf;
 }
 
-hd_iov.iov_base = (void *)src_buf;
+hd_iov.iov_base = (void *)buf;
 hd_iov.iov_len = n * 512;
 qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
 qemu_co_mutex_unlock(&s->lock);
@@ -750,7 +735,6 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
 if (qiov->niov > 1) {
 qemu_vfree(orig_buf);
 }
-g_free(cluster_data);
 
 return ret;
 }
-- 
2.5.0

[Qemu-block] [PATCH v4 17/26] tests: refactor python I/O tests helper main method

[Daniel P. Berrange]

The iotests.py helper provides a main() method for running
tests via the python unit test framework. Not all tests
will want to use this, so refactor it to split the testing
of compatible formats and platforms into separate helper
methods

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/iotests.py | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 5f82bbe..51e53bb 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -29,7 +29,8 @@ import qtest
 import struct
 
 __all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io',
-   'VM', 'QMPTestCase', 'notrun', 'main']
+   'VM', 'QMPTestCase', 'notrun', 'main', 'verify_image_format',
+   'verify_platform']
 
 # This will not work if arguments contain spaces but is necessary if we
 # want to support the override options that ./check supports.
@@ -394,17 +395,22 @@ def notrun(reason):
 print '%s not run: %s' % (seq, reason)
 sys.exit(0)
 
-def main(supported_fmts=[], supported_oses=['linux']):
-'''Run tests'''
-
-debug = '-d' in sys.argv
-verbosity = 1
+def verify_image_format(supported_fmts=[]):
 if supported_fmts and (imgfmt not in supported_fmts):
 notrun('not suitable for this image format: %s' % imgfmt)
 
+def verify_platform(supported_oses=['linux']):
 if True not in [sys.platform.startswith(x) for x in supported_oses]:
 notrun('not suitable for this OS: %s' % sys.platform)
 
+def main(supported_fmts=[], supported_oses=['linux']):
+'''Run tests'''
+
+debug = '-d' in sys.argv
+verbosity = 1
+verify_image_format(supported_fmts)
+verify_platform(supported_oses)
+
 # We need to filter out the time taken from the output so that qemu-iotest
 # can reliably diff the results against master output.
 import StringIO
-- 
2.5.0

[Qemu-block] [PATCH v4 23/26] qcow: convert QCow to use QCryptoBlock for encryption

[Daniel P. Berrange]

This converts the qcow2 driver to make use of the QCryptoBlock
APIs for encrypting image content. This is only wired up to
permit use of the legacy QCow encryption format. Users who wish
to have the strong LUKS format should switch to qcow2 instead.

With this change it is now required to use the QCryptoSecret
object for providing passwords, instead of the current block
password APIs / interactive prompting.

  $QEMU \
-object secret,id=sec0,filename=/home/berrange/encrypted.pw \
-drive file=/home/berrange/encrypted.qcow,key-secret=sec0

Signed-off-by: Daniel P. Berrange 
---
 block/qcow.c | 176 +++
 qapi/block-core.json |  17 -
 2 files changed, 96 insertions(+), 97 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 096980b..5cb5297 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -27,7 +27,9 @@
 #include "qemu/module.h"
 #include 
 #include "qapi/qmp/qerror.h"
-#include "crypto/cipher.h"
+#include "crypto/block.h"
+#include "qapi/opts-visitor.h"
+#include "qapi-visit.h"
 #include "migration/migration.h"
 
 /**/
@@ -41,6 +43,8 @@
 
 #define QCOW_OFLAG_COMPRESSED (1LL << 63)
 
+#define QCOW_OPT_KEY_SECRET "key-secret"
+
 typedef struct QCowHeader {
 uint32_t magic;
 uint32_t version;
@@ -73,7 +77,7 @@ typedef struct BDRVQcowState {
 uint8_t *cluster_cache;
 uint8_t *cluster_data;
 uint64_t cluster_cache_offset;
-QCryptoCipher *cipher; /* NULL if no key yet */
+QCryptoBlock *crypto; /* Disk encryption format driver */
 uint32_t crypt_method_header;
 CoMutex lock;
 Error *migration_blocker;
@@ -93,6 +97,19 @@ static int qcow_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 0;
 }
 
+static QemuOptsList qcow_runtime_opts = {
+.name = "qcow",
+.head = QTAILQ_HEAD_INITIALIZER(qcow_runtime_opts.head),
+.desc = {
+{
+.name = QCOW_OPT_KEY_SECRET,
+.type = QEMU_OPT_STRING,
+.help = "ID of the secret that provides the encryption key",
+},
+{ /* end of list */ }
+},
+};
+
 static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
@@ -100,6 +117,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 unsigned int len, i, shift;
 int ret;
 QCowHeader header;
+QemuOpts *opts = NULL;
+Error *local_err = NULL;
+QCryptoBlockOpenOptions *crypto_opts = NULL;
+unsigned int cflags = 0;
+OptsVisitor *ov;
 
 ret = bdrv_pread(bs->file->bs, 0, &header, sizeof(header));
 if (ret < 0) {
@@ -148,19 +170,53 @@ static int qcow_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }
 
-if (header.crypt_method > QCOW_CRYPT_AES) {
-error_setg(errp, "invalid encryption method in qcow header");
-ret = -EINVAL;
-goto fail;
-}
-if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) {
-error_setg(errp, "AES cipher not available");
+opts = qemu_opts_create(&qcow_runtime_opts, NULL, 0, &error_abort);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
 ret = -EINVAL;
 goto fail;
 }
+
 s->crypt_method_header = header.crypt_method;
 if (s->crypt_method_header) {
+if (s->crypt_method_header == QCOW_CRYPT_AES) {
+QCryptoBlockOptionsQCow *tmp;
+ov = opts_visitor_new(opts);
+
+crypto_opts = g_new0(QCryptoBlockOpenOptions, 1);
+crypto_opts->format = Q_CRYPTO_BLOCK_FORMAT_QCOW;
+visit_type_QCryptoBlockOptionsQCow(opts_get_visitor(ov),
+   "qcow", &tmp,
+   &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+opts_visitor_cleanup(ov);
+ret = -EINVAL;
+goto fail;
+} else {
+memcpy(&crypto_opts->u.qcow, tmp, sizeof(*tmp));
+g_free(tmp);
+}
+opts_visitor_cleanup(ov);
+
+if (flags & BDRV_O_NO_IO) {
+cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
+}
+s->crypto = qcrypto_block_open(crypto_opts, NULL, NULL,
+   cflags, errp);
+if (!s->crypto) {
+error_setg(errp, "Could not setup encryption layer");
+ret = -EINVAL;
+goto fail;
+}
+} else {
+error_setg(errp, "invalid encryption method in qcow header");
+ret = -EINVAL;
+goto fail;
+}
 bs->encrypted = 1;
+bs->valid_key = 1;
 }
 s->cluster_bits = header.cluster_bits;
 s->cluster_size = 1 << s->cluster_bits;
@@ -239,6 +295

[Qemu-block] [PATCH v4 21/26] qcow2: convert QCow2 to use QCryptoBlock for encryption

[Daniel P. Berrange]

This converts the qcow2 driver to make use of the QCryptoBlock
APIs for encrypting image content. As well as continued support
for the legacy QCow2 encryption format, the appealing benefit
is that it enables support for the LUKS format inside qcow2.

With the LUKS format it is necessary to store the LUKS
partition header and key material in the QCow2 file. This
data can be many MB in size, so cannot go into the QCow2
header region directly. Thus the spec defines a FDE
(Full Disk Encryption) header extension that specifies
the offset of a set of clusters to hold the FDE headers,
as well as the length of that region. The LUKS header is
thus stored in these extra allocated clusters before the
main image payload.

With this change it is now required to use the QCryptoSecret
object for providing passwords, instead of the current block
password APIs / interactive prompting.

  $QEMU \
-object secret,id=sec0,filename=/home/berrange/encrypted.pw \
-drive file=/home/berrange/encrypted.qcow2,key-secret=sec0

The new LUKS format is set as the new default format when
creating encrypted images. ie

  # qemu-img create --object secret,data=123456,id=sec0 \
   -f qcow2 -o encryption,key-secret=sec0 \
   test.qcow2 10G

results in the creation of an image using the LUKS format.

For compatibility the old qcow2 AES format can still be used
via the 'encryption-format' parameter which accepts the
values 'luks' or 'qcow'.

  # qemu-img create --object secret,data=123456,id=sec0 \
   -f qcow2 -o encryption,key-secret=sec0,encryption-format=qcow \
   test.qcow2 10G

Signed-off-by: Daniel P. Berrange 
---
 block/qcow2-cluster.c  |  46 +---
 block/qcow2-refcount.c |  10 +
 block/qcow2.c  | 511 ++---
 block/qcow2.h  |  21 +-
 docs/specs/qcow2.txt   |  76 +++
 qapi/block-core.json   |   7 +-
 tests/qemu-iotests/049 |   2 +-
 tests/qemu-iotests/049.out |   7 +-
 tests/qemu-iotests/082.out | 189 +
 tests/qemu-iotests/087 |  28 ++-
 tests/qemu-iotests/087.out |  16 +-
 tests/qemu-iotests/134 |  18 +-
 tests/qemu-iotests/134.out |  21 +-
 tests/qemu-iotests/common  |   1 +
 14 files changed, 788 insertions(+), 165 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 3802d37..456a83e 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -342,45 +342,6 @@ static int count_contiguous_clusters_by_type(int 
nb_clusters,
 return i;
 }
 
-int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
-  uint8_t *buf, int nb_sectors, bool enc,
-  Error **errp)
-{
-union {
-uint64_t ll[2];
-uint8_t b[16];
-} ivec;
-int i;
-int ret;
-
-for(i = 0; i < nb_sectors; i++) {
-ivec.ll[0] = cpu_to_le64(sector_num);
-ivec.ll[1] = 0;
-if (qcrypto_cipher_setiv(s->cipher,
- ivec.b, G_N_ELEMENTS(ivec.b),
- errp) < 0) {
-return -1;
-}
-if (enc) {
-ret = qcrypto_cipher_encrypt(s->cipher,
- buf, buf,
- 512,
- errp);
-} else {
-ret = qcrypto_cipher_decrypt(s->cipher,
- buf, buf,
- 512,
- errp);
-}
-if (ret < 0) {
-return -1;
-}
-sector_num++;
-buf += 512;
-}
-return 0;
-}
-
 static int coroutine_fn copy_sectors(BlockDriverState *bs,
  uint64_t start_sect,
  uint64_t cluster_offset,
@@ -422,10 +383,9 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
 
 if (bs->encrypted) {
 Error *err = NULL;
-assert(s->cipher);
-if (qcow2_encrypt_sectors(s, start_sect + n_start,
-  iov.iov_base, n,
-  true, &err) < 0) {
+assert(s->crypto);
+if (qcrypto_block_encrypt(s->crypto, start_sect + n_start,
+  iov.iov_base, n, &err) < 0) {
 ret = -EIO;
 error_free(err);
 goto out;
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 52a0a9f..71d7590 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1848,6 +1848,16 @@ static int calculate_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
 return ret;
 }
 
+/* encryption */
+if (s->crypto_header.length) {
+ret = inc_refcounts(bs, res, refcount_table, nb_clusters,
+s->crypto_header.offset,
+s->crypto_header.length);
+if (ret < 0) {
+return ret;
+  

[Qemu-block] [PATCH v4 14/26] block: add flag to indicate that no I/O will be performed

[Daniel P. Berrange]

When opening an image it is useful to know whether the caller
intends to perform I/O on the image or not. In the case of
encrypted images this will allow the block driver to avoid
having to prompt for decryption keys when we merely want to
query header metadata about the image. eg qemu-img info

This flag is enforced at the top level only, since even if
we don't want todo I/O on the 'qcow2' file payload, the
underlying 'file' driver will still need todo I/O to read
the qcow2 header, for example.

Reviewed-by: Eric Blake 
Signed-off-by: Daniel P. Berrange 
---
 block.c   |  5 +++--
 block/io.c|  2 ++
 include/block/block.h |  1 +
 qemu-img.c| 44 ++--
 4 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/block.c b/block.c
index ba24b8e..1fd4a14 100644
--- a/block.c
+++ b/block.c
@@ -720,7 +720,8 @@ static void bdrv_inherited_options(int *child_flags, QDict 
*child_options,
 flags |= BDRV_O_UNMAP;
 
 /* Clear flags that only apply to the top layer */
-flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
+flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ |
+   BDRV_O_NO_IO);
 
 *child_flags = flags;
 }
@@ -740,7 +741,7 @@ static void bdrv_inherited_fmt_options(int *child_flags, 
QDict *child_options,
 child_file.inherit_options(child_flags, child_options,
parent_flags, parent_options);
 
-*child_flags &= ~BDRV_O_PROTOCOL;
+*child_flags &= ~(BDRV_O_PROTOCOL | BDRV_O_NO_IO);
 }
 
 const BdrvChildRole child_format = {
diff --git a/block/io.c b/block/io.c
index a69bfc4..010e25a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -862,6 +862,7 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert(!qiov || bytes == qiov->size);
+assert((bs->open_flags & BDRV_O_NO_IO) == 0);
 
 /* Handle Copy on Read and associated serialisation */
 if (flags & BDRV_REQ_COPY_ON_READ) {
@@ -1148,6 +1149,7 @@ static int coroutine_fn 
bdrv_aligned_pwritev(BlockDriverState *bs,
 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert(!qiov || bytes == qiov->size);
+assert((bs->open_flags & BDRV_O_NO_IO) == 0);
 
 waited = wait_serialising_requests(req);
 assert(!waited || !req->serialising);
diff --git a/include/block/block.h b/include/block/block.h
index 1c4f4d8..4e2653d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -91,6 +91,7 @@ typedef struct HDGeometry {
 #define BDRV_O_PROTOCOL0x8000  /* if no block driver is explicitly given:
   select an appropriate protocol driver,
   ignoring the format layer */
+#define BDRV_O_NO_IO   0x1 /* don't initialize for I/O */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 
diff --git a/qemu-img.c b/qemu-img.c
index 2edb139..e91100e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -224,13 +224,13 @@ static int print_block_option_help(const char *filename, 
const char *fmt)
 
 
 static int img_open_password(BlockBackend *blk, const char *filename,
- bool require_io, bool quiet)
+ int flags, bool quiet)
 {
 BlockDriverState *bs;
 char password[256];
 
 bs = blk_bs(blk);
-if (bdrv_is_encrypted(bs) && require_io) {
+if (bdrv_is_encrypted(bs) && !(flags & BDRV_O_NO_IO)) {
 qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
 if (qemu_read_password(password, sizeof(password)) < 0) {
 error_report("No password given");
@@ -248,7 +248,7 @@ static int img_open_password(BlockBackend *blk, const char 
*filename,
 static BlockBackend *img_open_opts(const char *id,
const char *optstr,
QemuOpts *opts, int flags,
-   bool require_io, bool quiet)
+   bool quiet)
 {
 QDict *options;
 Error *local_err = NULL;
@@ -260,7 +260,7 @@ static BlockBackend *img_open_opts(const char *id,
 return NULL;
 }
 
-if (img_open_password(blk, optstr, require_io, quiet) < 0) {
+if (img_open_password(blk, optstr, flags, quiet) < 0) {
 blk_unref(blk);
 return NULL;
 }
@@ -269,7 +269,7 @@ static BlockBackend *img_open_opts(const char *id,
 
 static BlockBackend *img_open_file(const char *id, const char *filename,
const char *fmt, int flags,
-   bool require_io, bool quiet)
+   bool quiet)
 {
 BlockBackend *blk;
 Error *local_err = NULL;
@@ -286,7 +286,7 @@ static BlockBackend *img_open_file(const char *id, c

[Qemu-block] [PATCH v4 24/26] block: rip out all traces of password prompting

[Daniel P. Berrange]

Now that qcow & qcow2 are wired up to get encryption keys
via the QCryptoSecret object, nothing is relying on the
interactive prompting for passwords. All the code related
to password prompting can thus be ripped out.

Signed-off-by: Daniel P. Berrange 
---
 hmp.c | 31 -
 hw/usb/dev-storage.c  | 34 
 include/monitor/monitor.h |  7 -
 include/qemu/osdep.h  |  2 --
 monitor.c | 68 ---
 qemu-img.c| 31 -
 qemu-io.c | 21 ---
 qmp.c | 10 +--
 tests/qemu-iotests/087|  2 ++
 util/oslib-posix.c| 66 -
 util/oslib-win32.c| 24 -
 11 files changed, 3 insertions(+), 293 deletions(-)

diff --git a/hmp.c b/hmp.c
index 5b6084a..b24d367 100644
--- a/hmp.c
+++ b/hmp.c
@@ -969,37 +969,12 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
 g_free(data);
 }
 
-static void hmp_cont_cb(void *opaque, int err)
-{
-if (!err) {
-qmp_cont(NULL);
-}
-}
-
-static bool key_is_missing(const BlockInfo *bdev)
-{
-return (bdev->inserted && bdev->inserted->encryption_key_missing);
-}
-
 void hmp_cont(Monitor *mon, const QDict *qdict)
 {
-BlockInfoList *bdev_list, *bdev;
 Error *err = NULL;
 
-bdev_list = qmp_query_block(NULL);
-for (bdev = bdev_list; bdev; bdev = bdev->next) {
-if (key_is_missing(bdev->value)) {
-monitor_read_block_device_key(mon, bdev->value->device,
-  hmp_cont_cb, NULL);
-goto out;
-}
-}
-
 qmp_cont(&err);
 hmp_handle_error(mon, &err);
-
-out:
-qapi_free_BlockInfoList(bdev_list);
 }
 
 void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
@@ -1380,12 +1355,6 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 
 qmp_blockdev_change_medium(device, target, !!arg, arg,
!!read_only, read_only_mode, &err);
-if (err &&
-error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
-error_free(err);
-monitor_read_block_device_key(mon, device, NULL, NULL);
-return;
-}
 }
 
 hmp_handle_error(mon, &err);
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 5ae0424..de318cf 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -554,21 +554,6 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket 
*p)
 }
 }
 
-static void usb_msd_password_cb(void *opaque, int err)
-{
-MSDState *s = opaque;
-Error *local_err = NULL;
-
-if (!err) {
-usb_device_attach(&s->dev, &local_err);
-}
-
-if (local_err) {
-error_report_err(local_err);
-qdev_unplug(&s->dev.qdev, NULL);
-}
-}
-
 static void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req)
 {
 MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
@@ -614,25 +599,6 @@ static void usb_msd_realize_storage(USBDevice *dev, Error 
**errp)
 return;
 }
 
-if (blk_bs(blk)) {
-bdrv_add_key(blk_bs(blk), NULL, &err);
-if (err) {
-if (monitor_cur_is_qmp()) {
-error_propagate(errp, err);
-return;
-}
-error_free(err);
-err = NULL;
-if (cur_mon) {
-monitor_read_bdrv_key_start(cur_mon, blk_bs(blk),
-usb_msd_password_cb, s);
-s->dev.auto_attach = 0;
-} else {
-autostart = 0;
-}
-}
-}
-
 blkconf_serial(&s->conf, &dev->serial);
 blkconf_blocksizes(&s->conf);
 
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index aa0f373..cd38020 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -21,13 +21,6 @@ void monitor_init(CharDriverState *chr, int flags);
 int monitor_suspend(Monitor *mon);
 void monitor_resume(Monitor *mon);
 
-int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
-BlockCompletionFunc *completion_cb,
-void *opaque);
-int monitor_read_block_device_key(Monitor *mon, const char *device,
-  BlockCompletionFunc *completion_cb,
-  void *opaque);
-
 int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
 int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp);
 
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 4538fdc..0f99327 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -322,8 +322,6 @@ void qemu_set_tty_echo(int fd, bool echo);
 
 void os_mem_prealloc(int fd, char *area, size_t sz);
 
-int qemu_read_password(char *buf, int buf_size);
-
 /**
  * qemu_fork:
  *
diff --git a/m

[Qemu-block] [PATCH v4 13/26] crypto: implement the LUKS block encryption format

[Daniel P. Berrange]

Provide a block encryption implementation that follows the
LUKS/dm-crypt specification.

This supports all combinations of hash, cipher algorithm,
cipher mode and iv generator that are implemented by the
current crypto layer.

The notable missing feature is support for the 'xts'
cipher mode, which is commonly used for disk encryption
instead of 'cbc'. This is because it is not provided by
either nettle or libgcrypt. A suitable implementation
will be identified & integrated later.

There is support for opening existing volumes formatted
by dm-crypt, and for formatting new volumes. In the latter
case it will only use key slot 0.

Signed-off-by: Daniel P. Berrange 
---
 crypto/Makefile.objs  |1 +
 crypto/block-luks.c   | 1313 +
 crypto/block-luks.h   |   28 +
 crypto/block.c|2 +
 qapi/crypto.json  |   51 +-
 tests/test-crypto-block.c |   95 
 6 files changed, 1486 insertions(+), 4 deletions(-)
 create mode 100644 crypto/block-luks.c
 create mode 100644 crypto/block-luks.h

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 4f5510b..efec4e9 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -21,6 +21,7 @@ crypto-obj-y += afsplit.o
 crypto-obj-y += xts.o
 crypto-obj-y += block.o
 crypto-obj-y += block-qcow.o
+crypto-obj-y += block-luks.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
new file mode 100644
index 000..a611c5d
--- /dev/null
+++ b/crypto/block-luks.c
@@ -0,0 +1,1313 @@
+/*
+ * QEMU Crypto block device encryption LUKS format
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "crypto/block-luks.h"
+
+#include "crypto/hash.h"
+#include "crypto/afsplit.h"
+#include "crypto/pbkdf.h"
+#include "crypto/secret.h"
+#include "crypto/random.h"
+
+#ifdef CONFIG_UUID
+#include 
+#endif
+
+#include "qemu/coroutine.h"
+
+/*
+ * Reference for the LUKS format implemented here is
+ *
+ *   docs/on-disk-format.pdf
+ *
+ * in 'cryptsetup' package source code
+ *
+ * This file implements the 1.2.1 specification, dated
+ * Oct 16, 2011.
+ */
+
+typedef struct QCryptoBlockLUKS QCryptoBlockLUKS;
+typedef struct QCryptoBlockLUKSHeader QCryptoBlockLUKSHeader;
+typedef struct QCryptoBlockLUKSKeySlot QCryptoBlockLUKSKeySlot;
+
+
+/* The following constants are all defined by the LUKS spec */
+#define QCRYPTO_BLOCK_LUKS_VERSION 1
+
+#define QCRYPTO_BLOCK_LUKS_MAGIC_LEN 6
+#define QCRYPTO_BLOCK_LUKS_CIPHER_NAME_LEN 32
+#define QCRYPTO_BLOCK_LUKS_CIPHER_MODE_LEN 32
+#define QCRYPTO_BLOCK_LUKS_HASH_SPEC_LEN 32
+#define QCRYPTO_BLOCK_LUKS_DIGEST_LEN 20
+#define QCRYPTO_BLOCK_LUKS_SALT_LEN 32
+#define QCRYPTO_BLOCK_LUKS_UUID_LEN 40
+#define QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS 8
+#define QCRYPTO_BLOCK_LUKS_STRIPES 4000
+#define QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS 1000
+#define QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS 1000
+#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET 4096
+
+#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED 0xDEAD
+#define QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED 0x00AC71F3
+
+#define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL
+
+static const char qcrypto_block_luks_magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN] = {
+'L', 'U', 'K', 'S', 0xBA, 0xBE
+};
+
+typedef struct QCryptoBlockLUKSNameMap QCryptoBlockLUKSNameMap;
+struct QCryptoBlockLUKSNameMap {
+const char *name;
+int id;
+};
+
+typedef struct QCryptoBlockLUKSCipherSizeMap QCryptoBlockLUKSCipherSizeMap;
+struct QCryptoBlockLUKSCipherSizeMap {
+uint32_t key_bytes;
+int id;
+};
+typedef struct QCryptoBlockLUKSCipherNameMap QCryptoBlockLUKSCipherNameMap;
+struct QCryptoBlockLUKSCipherNameMap {
+const char *name;
+const QCryptoBlockLUKSCipherSizeMap *sizes;
+};
+
+
+static const QCryptoBlockLUKSCipherSizeMap
+qcrypto_block_luks_cipher_size_map_aes[] = {
+{ 16, QCRYPTO_CIPHER_ALG_AES_128 },
+{ 24, QCRYPTO_CIPHER_ALG_AES_192 },
+{ 32, QCRYPTO_CIPHER_ALG_AES_256 },
+{ 0, 0 },
+};
+
+static const QCryptoBlockLUKSCipherSizeMap
+qcrypto_block_luks_cipher_size_map_cast5[] = {
+{ 16, QCRYPTO_CIPHER_ALG_CAST5_128 },
+{ 0, 0 },
+};
+
+static const QCryptoBlockLUKSCipherSizeMap
+qcrypto_block_luks_cipher_size_map_serpent[] = {
+{ 1

[Qemu-block] [PATCH v4 16/26] tests: redirect stderr to stdout for iotests

[Daniel P. Berrange]

The python I/O tests helper for running qemu-img/qemu-io
setup stdout to be captured to a pipe, but left stderr
untouched. As a result, if something failed in qemu-img/
qemu-io, data written to stderr would get output directly
and not line up with data on the test stdout due to
buffering.  If we explicitly redirect stderr to the same
pipe as stdout, things are much clearer when they go
wrong.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/iotests.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 0a238ec..5f82bbe 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -71,7 +71,9 @@ def qemu_img_verbose(*args):
 
 def qemu_img_pipe(*args):
 '''Run qemu-img and return its output'''
-subp = subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE)
+subp = subprocess.Popen(qemu_img_args + list(args),
+stdout=subprocess.PIPE,
+stderr=subprocess.STDOUT)
 exitcode = subp.wait()
 if exitcode < 0:
 sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' 
'.join(qemu_img_args + list(args
@@ -80,7 +82,8 @@ def qemu_img_pipe(*args):
 def qemu_io(*args):
 '''Run qemu-io and return the stdout data'''
 args = qemu_io_args + list(args)
-subp = subprocess.Popen(args, stdout=subprocess.PIPE)
+subp = subprocess.Popen(args, stdout=subprocess.PIPE,
+stderr=subprocess.STDOUT)
 exitcode = subp.wait()
 if exitcode < 0:
 sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' 
'.join(args)))
-- 
2.5.0

[Qemu-block] [PATCH v4 09/26] crypto: import an implementation of the XTS cipher mode

[Daniel P. Berrange]

The XTS (XEX with tweaked-codebook and ciphertext stealing)
cipher mode is commonly used in full disk encryption. There
is unfortunately no implementation of it in either libgcrypt
or nettle, so we need to provide our own.

The libtomcrypt project provides a repository of crypto
algorithms under a choice of either "public domain" or
the "what the fuck public license".

So this impl is taken from the libtomcrypt GIT repo and
adapted to be compatible with the way we need to call
ciphers provided by nettle/gcrypt.

Signed-off-by: Daniel P. Berrange 
---
 crypto/Makefile.objs|   1 +
 crypto/xts.c| 256 +
 include/crypto/xts.h|  86 ++
 tests/.gitignore|   1 +
 tests/Makefile  |   2 +
 tests/test-crypto-xts.c | 423 
 6 files changed, 769 insertions(+)
 create mode 100644 crypto/xts.c
 create mode 100644 include/crypto/xts.h
 create mode 100644 tests/test-crypto-xts.c

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 5136737..85a9edb 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -18,6 +18,7 @@ crypto-obj-y += ivgen-essiv.o
 crypto-obj-y += ivgen-plain.o
 crypto-obj-y += ivgen-plain64.o
 crypto-obj-y += afsplit.o
+crypto-obj-y += xts.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/xts.c b/crypto/xts.c
new file mode 100644
index 000..b5a9b23
--- /dev/null
+++ b/crypto/xts.c
@@ -0,0 +1,256 @@
+/*
+ * QEMU Crypto XTS cipher mode
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ * This code is originally derived from public domain / WTFPL code in
+ * LibTomCrypt crytographic library http://libtom.org. The XTS code
+ * was donated by Elliptic Semiconductor Inc (www.ellipticsemi.com)
+ * to the LibTom Projects
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/xts.h"
+
+static void xts_mult_x(uint8_t *I)
+{
+int x;
+uint8_t t, tt;
+
+for (x = t = 0; x < 16; x++) {
+tt = I[x] >> 7;
+I[x] = ((I[x] << 1) | t) & 0xFF;
+t = tt;
+}
+if (tt) {
+I[0] ^= 0x87;
+}
+}
+
+
+/**
+ * xts_tweak_uncrypt:
+ * @param ctxt: the cipher context
+ * @param func: the cipher function
+ * @src: buffer providing the cipher text of XTS_BLOCK_SIZE bytes
+ * @dst: buffer to output the plain text of XTS_BLOCK_SIZE bytes
+ * @iv: the initialization vector tweak of XTS_BLOCK_SIZE bytes
+ *
+ * Decrypt data with a tweak
+ */
+static void xts_tweak_decrypt(const void *ctx,
+  xts_cipher_func *func,
+  const uint8_t *src,
+  uint8_t *dst,
+  uint8_t *iv)
+{
+unsigned long x;
+
+/* tweak encrypt block i */
+#ifdef LTC_FAST
+for (x = 0; x < XTS_BLOCK_SIZE; x += sizeof(LTC_FAST_TYPE)) {
+*((LTC_FAST_TYPE *)&dst[x]) =
+*((LTC_FAST_TYPE *)&src[x]) ^ *((LTC_FAST_TYPE *)&iv[x]);
+}
+#else
+for (x = 0; x < XTS_BLOCK_SIZE; x++) {
+dst[x] = src[x] ^ iv[x];
+}
+#endif
+
+func(ctx, XTS_BLOCK_SIZE, dst, dst);
+
+#ifdef LTC_FAST
+for (x = 0; x < XTS_BLOCK_SIZE; x += sizeof(LTC_FAST_TYPE)) {
+*((LTC_FAST_TYPE *)&dst[x]) ^= *((LTC_FAST_TYPE *)&iv[x]);
+}
+#else
+for (x = 0; x < XTS_BLOCK_SIZE; x++) {
+dst[x] = dst[x] ^ iv[x];
+}
+#endif
+
+/* LFSR the tweak */
+xts_mult_x(iv);
+}
+
+
+void xts_decrypt(const void *datactx,
+ const void *tweakctx,
+ xts_cipher_func *encfunc,
+ xts_cipher_func *decfunc,
+ uint8_t *iv,
+ size_t length,
+ uint8_t *dst,
+ const uint8_t *src)
+{
+uint8_t PP[XTS_BLOCK_SIZE], CC[XTS_BLOCK_SIZE], T[XTS_BLOCK_SIZE];
+unsigned long i, m, mo, lim;
+
+/* get number of blocks */
+m = length >> 4;
+mo = length & 15;
+
+/* must have at least one full block */
+g_assert(m != 0);
+
+if (mo == 0) {
+lim = m;
+} else {
+lim = m - 1;
+}
+
+/* encrypt the iv */
+encfunc(tweakctx, XTS_BLOCK_SIZE, T, iv);
+
+for (i = 0; i < lim; i++) {
+xts_tweak_decrypt(datactx, decfunc, src, dst, T);
+
+src += XTS_BLOCK_SIZE;
+dst += XTS_B

[Qemu-block] [PATCH v4 03/26] crypto: add support for generating initialization vectors

[Daniel P. Berrange]

There are a number of different algorithms that can be used
to generate initialization vectors for disk encryption. This
introduces a simple internal QCryptoBlockIV object to provide
a consistent internal API to the different algorithms. The
initially implemented algorithms are 'plain', 'plain64' and
'essiv', each matching the same named algorithm provided
by the Linux kernel dm-crypt driver.

Signed-off-by: Daniel P. Berrange 
---
 crypto/Makefile.objs  |   4 +
 crypto/ivgen-essiv.c  | 118 ++
 crypto/ivgen-essiv.h  |  28 +++
 crypto/ivgen-plain.c  |  59 +
 crypto/ivgen-plain.h  |  28 +++
 crypto/ivgen-plain64.c|  59 +
 crypto/ivgen-plain64.h|  28 +++
 crypto/ivgen.c|  99 ++
 crypto/ivgenpriv.h|  49 +++
 include/crypto/ivgen.h| 206 ++
 qapi/crypto.json  |  19 +
 tests/.gitignore  |   1 +
 tests/Makefile|   2 +
 tests/test-crypto-ivgen.c | 169 +
 14 files changed, 869 insertions(+)
 create mode 100644 crypto/ivgen-essiv.c
 create mode 100644 crypto/ivgen-essiv.h
 create mode 100644 crypto/ivgen-plain.c
 create mode 100644 crypto/ivgen-plain.h
 create mode 100644 crypto/ivgen-plain64.c
 create mode 100644 crypto/ivgen-plain64.h
 create mode 100644 crypto/ivgen.c
 create mode 100644 crypto/ivgenpriv.h
 create mode 100644 include/crypto/ivgen.h
 create mode 100644 tests/test-crypto-ivgen.c

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 4d2cd3e..3040989 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -13,6 +13,10 @@ crypto-obj-$(CONFIG_GNUTLS) += random-gnutls.o
 crypto-obj-y += pbkdf.o
 crypto-obj-$(CONFIG_NETTLE) += pbkdf-nettle.o
 crypto-obj-$(if $(CONFIG_NETTLE),n,$(CONFIG_GCRYPT)) += pbkdf-gcrypt.o
+crypto-obj-y += ivgen.o
+crypto-obj-y += ivgen-essiv.o
+crypto-obj-y += ivgen-plain.o
+crypto-obj-y += ivgen-plain64.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/ivgen-essiv.c b/crypto/ivgen-essiv.c
new file mode 100644
index 000..d3dc4b6
--- /dev/null
+++ b/crypto/ivgen-essiv.c
@@ -0,0 +1,118 @@
+/*
+ * QEMU Crypto block IV generator - essiv
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/ivgen-essiv.h"
+
+typedef struct QCryptoIVGenESSIV QCryptoIVGenESSIV;
+struct QCryptoIVGenESSIV {
+QCryptoCipher *cipher;
+};
+
+static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen,
+const uint8_t *key, size_t nkey,
+Error **errp)
+{
+uint8_t *salt;
+size_t nhash;
+size_t nsalt;
+QCryptoIVGenESSIV *essiv = g_new0(QCryptoIVGenESSIV, 1);
+
+/* Not neccessarily the same as nkey */
+nsalt = qcrypto_cipher_get_key_len(ivgen->cipher);
+
+nhash = qcrypto_hash_digest_len(ivgen->hash);
+/* Salt must be larger of hash size or key size */
+salt = g_new0(uint8_t, MAX(nhash, nsalt));
+
+if (qcrypto_hash_bytes(ivgen->hash, (const gchar *)key, nkey,
+   &salt, &nhash,
+   errp) < 0) {
+g_free(essiv);
+return -1;
+}
+
+/* Now potentially truncate salt to match cipher key len */
+essiv->cipher = qcrypto_cipher_new(ivgen->cipher,
+   QCRYPTO_CIPHER_MODE_ECB,
+   salt, MIN(nhash, nsalt),
+   errp);
+if (!essiv->cipher) {
+g_free(essiv);
+g_free(salt);
+return -1;
+}
+
+g_free(salt);
+ivgen->private = essiv;
+
+return 0;
+}
+
+static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen,
+ uint64_t sector,
+ uint8_t *iv, size_t niv,
+ Error **errp)
+{
+QCryptoIVGenESSIV *essiv = ivgen->private;
+size_t ndata = qcrypto_cipher_get_block_len(ivgen->cipher);
+uint8_t *data = g_new(uint8_t, ndata);
+
+sector = cpu_to_le64(sector);
+memcpy(data, (uint8_t *)§or, ndata);
+if (sizeof(sector) < ndata) {
+ 

[Qemu-block] [PATCH v4 08/26] crypto: add support for the twofish cipher algorithm

[Daniel P. Berrange]

New cipher algorithms 'twofish-128', 'twofish-192' and
'twofish-256' are defined for the Twofish algorithm.
The gcrypt backend does not support 'twofish-192'.

The nettle and gcrypt cipher backends are updated to
support the new cipher and a test vector added to the
cipher test suite. The new algorithm is enabled in the
LUKS block encryption driver.

Signed-off-by: Daniel P. Berrange 
---
 crypto/cipher-gcrypt.c | 12 
 crypto/cipher-nettle.c | 30 ++
 crypto/cipher.c|  6 ++
 qapi/crypto.json   |  6 +-
 tests/test-crypto-cipher.c | 29 +
 5 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index a667d59..ce26456 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -33,6 +33,8 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg)
 case QCRYPTO_CIPHER_ALG_SERPENT_128:
 case QCRYPTO_CIPHER_ALG_SERPENT_192:
 case QCRYPTO_CIPHER_ALG_SERPENT_256:
+case QCRYPTO_CIPHER_ALG_TWOFISH_128:
+case QCRYPTO_CIPHER_ALG_TWOFISH_256:
 return true;
 default:
 return false;
@@ -104,6 +106,14 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
 gcryalg = GCRY_CIPHER_SERPENT256;
 break;
 
+case QCRYPTO_CIPHER_ALG_TWOFISH_128:
+gcryalg = GCRY_CIPHER_TWOFISH128;
+break;
+
+case QCRYPTO_CIPHER_ALG_TWOFISH_256:
+gcryalg = GCRY_CIPHER_TWOFISH;
+break;
+
 default:
 error_setg(errp, "Unsupported cipher algorithm %d", alg);
 return NULL;
@@ -140,6 +150,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
 case QCRYPTO_CIPHER_ALG_SERPENT_128:
 case QCRYPTO_CIPHER_ALG_SERPENT_192:
 case QCRYPTO_CIPHER_ALG_SERPENT_256:
+case QCRYPTO_CIPHER_ALG_TWOFISH_128:
+case QCRYPTO_CIPHER_ALG_TWOFISH_256:
 ctx->blocksize = 16;
 break;
 case QCRYPTO_CIPHER_ALG_CAST5_128:
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index 74b55ab..9b057fd 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #if CONFIG_NETTLE_VERSION_MAJOR < 3
 typedef nettle_crypt_func nettle_cipher_func;
@@ -89,6 +90,18 @@ static void serpent_decrypt_wrapper(cipher_ctx_t ctx, 
cipher_length_t length,
 serpent_decrypt(ctx, length, dst, src);
 }
 
+static void twofish_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+uint8_t *dst, const uint8_t *src)
+{
+twofish_encrypt(ctx, length, dst, src);
+}
+
+static void twofish_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+uint8_t *dst, const uint8_t *src)
+{
+twofish_decrypt(ctx, length, dst, src);
+}
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
 void *ctx_encrypt;
@@ -110,6 +123,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg)
 case QCRYPTO_CIPHER_ALG_SERPENT_128:
 case QCRYPTO_CIPHER_ALG_SERPENT_192:
 case QCRYPTO_CIPHER_ALG_SERPENT_256:
+case QCRYPTO_CIPHER_ALG_TWOFISH_128:
+case QCRYPTO_CIPHER_ALG_TWOFISH_192:
+case QCRYPTO_CIPHER_ALG_TWOFISH_256:
 return true;
 default:
 return false;
@@ -200,6 +216,20 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
 ctx->blocksize = SERPENT_BLOCK_SIZE;
 break;
 
+case QCRYPTO_CIPHER_ALG_TWOFISH_128:
+case QCRYPTO_CIPHER_ALG_TWOFISH_192:
+case QCRYPTO_CIPHER_ALG_TWOFISH_256:
+ctx->ctx_encrypt = g_new0(struct twofish_ctx, 1);
+ctx->ctx_decrypt = NULL; /* 1 ctx can do both */
+
+twofish_set_key(ctx->ctx_encrypt, nkey, key);
+
+ctx->alg_encrypt = twofish_encrypt_wrapper;
+ctx->alg_decrypt = twofish_decrypt_wrapper;
+
+ctx->blocksize = TWOFISH_BLOCK_SIZE;
+break;
+
 default:
 error_setg(errp, "Unsupported cipher algorithm %d", alg);
 goto error;
diff --git a/crypto/cipher.c b/crypto/cipher.c
index 0f6fe98..89fa5a2 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -31,6 +31,9 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
 [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
 [QCRYPTO_CIPHER_ALG_SERPENT_192] = 24,
 [QCRYPTO_CIPHER_ALG_SERPENT_256] = 32,
+[QCRYPTO_CIPHER_ALG_TWOFISH_128] = 16,
+[QCRYPTO_CIPHER_ALG_TWOFISH_192] = 24,
+[QCRYPTO_CIPHER_ALG_TWOFISH_256] = 32,
 };
 
 static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
@@ -42,6 +45,9 @@ static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
 [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
 [QCRYPTO_CIPHER_ALG_SERPENT_192] = 16,
 [QCRYPTO_CIPHER_ALG_SERPENT_256] = 16,
+[QCRYPTO_CIPHER_ALG_TWOFISH_128] = 16,
+[QCRYPTO_CIPHER_ALG_TWOFISH_192] = 16,
+[QCRYPTO_CIPHER_ALG_TWOFISH_256] = 16,
 };
 
 static

[Qemu-block] [PATCH v4 11/26] crypto: wire up XTS mode for cipher APIs

[Daniel P. Berrange]

Introduce 'XTS' as a permitted mode for the cipher APIs.
With XTS the key provided must be twice the size of the
key normally required for any given algorithm. This is
because the key will be split into two pieces for use
in XTS mode.

Signed-off-by: Daniel P. Berrange 
---
 crypto/cipher-builtin.c|  85 +---
 crypto/cipher-gcrypt.c | 123 -
 crypto/cipher-nettle.c |  79 --
 crypto/cipher.c|  27 +++--
 qapi/crypto.json   |   3 +-
 tests/test-crypto-cipher.c | 134 -
 6 files changed, 405 insertions(+), 46 deletions(-)

diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c
index 836ed1a..88963f6 100644
--- a/crypto/cipher-builtin.c
+++ b/crypto/cipher-builtin.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "crypto/aes.h"
 #include "crypto/desrfb.h"
+#include "crypto/xts.h"
 
 typedef struct QCryptoCipherBuiltinAESContext QCryptoCipherBuiltinAESContext;
 struct QCryptoCipherBuiltinAESContext {
@@ -30,6 +31,7 @@ struct QCryptoCipherBuiltinAESContext {
 typedef struct QCryptoCipherBuiltinAES QCryptoCipherBuiltinAES;
 struct QCryptoCipherBuiltinAES {
 QCryptoCipherBuiltinAESContext key;
+QCryptoCipherBuiltinAESContext key_tweak;
 uint8_t iv[AES_BLOCK_SIZE];
 };
 typedef struct QCryptoCipherBuiltinDESRFB QCryptoCipherBuiltinDESRFB;
@@ -123,6 +125,30 @@ static void qcrypto_cipher_aes_ecb_decrypt(AES_KEY *key,
 }
 
 
+static void qcrypto_cipher_aes_xts_encrypt(const void *ctx,
+   size_t length,
+   uint8_t *dst,
+   const uint8_t *src)
+{
+const QCryptoCipherBuiltinAESContext *aesctx = ctx;
+
+qcrypto_cipher_aes_ecb_encrypt((AES_KEY *)&aesctx->enc,
+   src, dst, length);
+}
+
+
+static void qcrypto_cipher_aes_xts_decrypt(const void *ctx,
+   size_t length,
+   uint8_t *dst,
+   const uint8_t *src)
+{
+const QCryptoCipherBuiltinAESContext *aesctx = ctx;
+
+qcrypto_cipher_aes_ecb_decrypt((AES_KEY *)&aesctx->dec,
+   src, dst, length);
+}
+
+
 static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
   const void *in,
   void *out,
@@ -141,6 +167,14 @@ static int qcrypto_cipher_encrypt_aes(QCryptoCipher 
*cipher,
 &ctxt->state.aes.key.enc,
 ctxt->state.aes.iv, 1);
 break;
+case QCRYPTO_CIPHER_MODE_XTS:
+xts_encrypt(&ctxt->state.aes.key,
+&ctxt->state.aes.key_tweak,
+qcrypto_cipher_aes_xts_encrypt,
+qcrypto_cipher_aes_xts_decrypt,
+ctxt->state.aes.iv,
+len, out, in);
+break;
 default:
 g_assert_not_reached();
 }
@@ -167,6 +201,14 @@ static int qcrypto_cipher_decrypt_aes(QCryptoCipher 
*cipher,
 &ctxt->state.aes.key.dec,
 ctxt->state.aes.iv, 0);
 break;
+case QCRYPTO_CIPHER_MODE_XTS:
+xts_decrypt(&ctxt->state.aes.key,
+&ctxt->state.aes.key_tweak,
+qcrypto_cipher_aes_xts_encrypt,
+qcrypto_cipher_aes_xts_decrypt,
+ctxt->state.aes.iv,
+len, out, in);
+break;
 default:
 g_assert_not_reached();
 }
@@ -200,21 +242,46 @@ static int qcrypto_cipher_init_aes(QCryptoCipher *cipher,
 QCryptoCipherBuiltin *ctxt;
 
 if (cipher->mode != QCRYPTO_CIPHER_MODE_CBC &&
-cipher->mode != QCRYPTO_CIPHER_MODE_ECB) {
+cipher->mode != QCRYPTO_CIPHER_MODE_ECB &&
+cipher->mode != QCRYPTO_CIPHER_MODE_XTS) {
 error_setg(errp, "Unsupported cipher mode %d", cipher->mode);
 return -1;
 }
 
 ctxt = g_new0(QCryptoCipherBuiltin, 1);
 
-if (AES_set_encrypt_key(key, nkey * 8, &ctxt->state.aes.key.enc) != 0) {
-error_setg(errp, "Failed to set encryption key");
-goto error;
-}
+if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
+if (AES_set_encrypt_key(key, nkey * 4, &ctxt->state.aes.key.enc) != 0) 
{
+error_setg(errp, "Failed to set encryption key");
+goto error;
+}
 
-if (AES_set_decrypt_key(key, nkey * 8, &ctxt->state.aes.key.dec) != 0) {
-error_setg(errp, "Failed to set decryption key");
-goto error;
+if (AES_set_decrypt_key(key, nkey * 4, &ctxt->state.aes.key.dec) != 0) 
{
+error_setg(errp, "Failed to set decryption key");
+goto error;
+}
+
+if (AES_set_encrypt_key(key + (nkey / 2), nkey * 4,
+ 

[Qemu-block] [PATCH v4 15/26] qemu-img/qemu-io: don't prompt for passwords if not required

[Daniel P. Berrange]

The qemu-img/qemu-io tools prompt for disk encryption passwords
regardless of whether any are actually required. Adding a check
on bdrv_key_required() avoids this prompt for disk formats which
have been converted to the QCryptoSecret APIs.

This is just a temporary hack to ensure the block I/O tests
continue to work after each patch, since the last patch will
completely delete all the password prompting code.

Reviewed-by: Eric Blake 
Signed-off-by: Daniel P. Berrange 
---
 qemu-img.c | 3 ++-
 qemu-io.c  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index e91100e..b9a501c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -230,7 +230,8 @@ static int img_open_password(BlockBackend *blk, const char 
*filename,
 char password[256];
 
 bs = blk_bs(blk);
-if (bdrv_is_encrypted(bs) && !(flags & BDRV_O_NO_IO)) {
+if (bdrv_is_encrypted(bs) && bdrv_key_required(bs) &&
+!(flags & BDRV_O_NO_IO)) {
 qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
 if (qemu_read_password(password, sizeof(password)) < 0) {
 error_report("No password given");
diff --git a/qemu-io.c b/qemu-io.c
index 8c31257..d825723 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -69,7 +69,7 @@ static int openfile(char *name, int flags, QDict *opts)
 }
 
 bs = blk_bs(qemuio_blk);
-if (bdrv_is_encrypted(bs)) {
+if (bdrv_is_encrypted(bs) && bdrv_key_required(bs)) {
 char password[256];
 printf("Disk image '%s' is encrypted.\n", name);
 if (qemu_read_password(password, sizeof(password)) < 0) {
-- 
2.5.0

[Qemu-block] [PATCH v4 18/26] tests: add output filter to python I/O tests helper

[Daniel P. Berrange]

Add a 'log' method to iotests.py which prints messages to
stdout, with optional filtering of data. Port over some
standard filters for present in the shell common.filter
code.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/iotests.py | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 51e53bb..8499e1b 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -30,7 +30,8 @@ import struct
 
 __all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io',
'VM', 'QMPTestCase', 'notrun', 'main', 'verify_image_format',
-   'verify_platform']
+   'verify_platform', 'filter_test_dir', 'filter_win32',
+   'filter_qemu_io', 'filter_chown', 'log']
 
 # This will not work if arguments contain spaces but is necessary if we
 # want to support the override options that ./check supports.
@@ -105,6 +106,28 @@ def create_image(name, size):
 i = i + 512
 file.close()
 
+test_dir_re = re.compile(r"%s" % test_dir)
+def filter_test_dir(msg):
+return test_dir_re.sub("TEST_DIR", msg)
+
+win32_re = re.compile(r"\r")
+def filter_win32(msg):
+return win32_re.sub("", msg)
+
+qemu_io_re = re.compile(r"[0-9]* ops; [0-9\/:. sec]* \([0-9\/.inf]* 
[EPTGMKiBbytes]*\/sec and [0-9\/.inf]* ops\/sec\)")
+def filter_qemu_io(msg):
+msg = filter_win32(msg)
+return qemu_io_re.sub("X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)", 
msg)
+
+chown_re = re.compile(r"chown [0-9]+:[0-9]+")
+def filter_chown(msg):
+return chown_re.sub("chown UID:GID", msg)
+
+def log(msg, filters=[]):
+for flt in filters:
+msg = flt(msg)
+print msg
+
 # Test if 'match' is a recursive subset of 'event'
 def event_match(event, match=None):
 if match is None:
-- 
2.5.0

[Qemu-block] [PATCH v4 10/26] crypto: refactor code for dealing with AES cipher

[Daniel P. Berrange]

The built-in and nettle cipher backends for AES maintain
two separate AES contexts, one for encryption and one for
decryption. This is going to be inconvenient for the future
code dealing with XTS, so wrap them up in a single struct
so there is just one pointer to pass around for both
encryptin and decryption.

Signed-off-by: Daniel P. Berrange 
---
 crypto/cipher-builtin.c | 126 ++--
 crypto/cipher-nettle.c  |  57 +++---
 2 files changed, 109 insertions(+), 74 deletions(-)

diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c
index 138b7a0..836ed1a 100644
--- a/crypto/cipher-builtin.c
+++ b/crypto/cipher-builtin.c
@@ -22,10 +22,14 @@
 #include "crypto/aes.h"
 #include "crypto/desrfb.h"
 
+typedef struct QCryptoCipherBuiltinAESContext QCryptoCipherBuiltinAESContext;
+struct QCryptoCipherBuiltinAESContext {
+AES_KEY enc;
+AES_KEY dec;
+};
 typedef struct QCryptoCipherBuiltinAES QCryptoCipherBuiltinAES;
 struct QCryptoCipherBuiltinAES {
-AES_KEY encrypt_key;
-AES_KEY decrypt_key;
+QCryptoCipherBuiltinAESContext key;
 uint8_t iv[AES_BLOCK_SIZE];
 };
 typedef struct QCryptoCipherBuiltinDESRFB QCryptoCipherBuiltinDESRFB;
@@ -67,6 +71,58 @@ static void qcrypto_cipher_free_aes(QCryptoCipher *cipher)
 }
 
 
+static void qcrypto_cipher_aes_ecb_encrypt(AES_KEY *key,
+   const void *in,
+   void *out,
+   size_t len)
+{
+const uint8_t *inptr = in;
+uint8_t *outptr = out;
+while (len) {
+if (len > AES_BLOCK_SIZE) {
+AES_encrypt(inptr, outptr, key);
+inptr += AES_BLOCK_SIZE;
+outptr += AES_BLOCK_SIZE;
+len -= AES_BLOCK_SIZE;
+} else {
+uint8_t tmp1[AES_BLOCK_SIZE], tmp2[AES_BLOCK_SIZE];
+memcpy(tmp1, inptr, len);
+/* Fill with 0 to avoid valgrind uninitialized reads */
+memset(tmp1 + len, 0, sizeof(tmp1) - len);
+AES_encrypt(tmp1, tmp2, key);
+memcpy(outptr, tmp2, len);
+len = 0;
+}
+}
+}
+
+
+static void qcrypto_cipher_aes_ecb_decrypt(AES_KEY *key,
+   const void *in,
+   void *out,
+   size_t len)
+{
+const uint8_t *inptr = in;
+uint8_t *outptr = out;
+while (len) {
+if (len > AES_BLOCK_SIZE) {
+AES_decrypt(inptr, outptr, key);
+inptr += AES_BLOCK_SIZE;
+outptr += AES_BLOCK_SIZE;
+len -= AES_BLOCK_SIZE;
+} else {
+uint8_t tmp1[AES_BLOCK_SIZE], tmp2[AES_BLOCK_SIZE];
+memcpy(tmp1, inptr, len);
+/* Fill with 0 to avoid valgrind uninitialized reads */
+memset(tmp1 + len, 0, sizeof(tmp1) - len);
+AES_decrypt(tmp1, tmp2, key);
+memcpy(outptr, tmp2, len);
+len = 0;
+}
+}
+}
+
+
 static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
   const void *in,
   void *out,
@@ -75,29 +131,18 @@ static int qcrypto_cipher_encrypt_aes(QCryptoCipher 
*cipher,
 {
 QCryptoCipherBuiltin *ctxt = cipher->opaque;
 
-if (cipher->mode == QCRYPTO_CIPHER_MODE_ECB) {
-const uint8_t *inptr = in;
-uint8_t *outptr = out;
-while (len) {
-if (len > AES_BLOCK_SIZE) {
-AES_encrypt(inptr, outptr, &ctxt->state.aes.encrypt_key);
-inptr += AES_BLOCK_SIZE;
-outptr += AES_BLOCK_SIZE;
-len -= AES_BLOCK_SIZE;
-} else {
-uint8_t tmp1[AES_BLOCK_SIZE], tmp2[AES_BLOCK_SIZE];
-memcpy(tmp1, inptr, len);
-/* Fill with 0 to avoid valgrind uninitialized reads */
-memset(tmp1 + len, 0, sizeof(tmp1) - len);
-AES_encrypt(tmp1, tmp2, &ctxt->state.aes.encrypt_key);
-memcpy(outptr, tmp2, len);
-len = 0;
-}
-}
-} else {
+switch (cipher->mode) {
+case QCRYPTO_CIPHER_MODE_ECB:
+qcrypto_cipher_aes_ecb_encrypt(&ctxt->state.aes.key.enc,
+   in, out, len);
+break;
+case QCRYPTO_CIPHER_MODE_CBC:
 AES_cbc_encrypt(in, out, len,
-&ctxt->state.aes.encrypt_key,
+&ctxt->state.aes.key.enc,
 ctxt->state.aes.iv, 1);
+break;
+default:
+g_assert_not_reached();
 }
 
 return 0;
@@ -112,29 +157,18 @@ static int qcrypto_cipher_decrypt_aes(QCryptoCipher 
*cipher,
 {
 QCryptoCipherBuiltin *ctxt = cipher->opaque;
 
-if (cipher->mode == QCRYPTO_CIPHER_MODE_ECB) {
-const uint8_t *inptr = in;
-uint8_t *outptr = out;
-   

[Qemu-block] [PATCH v4 12/26] crypto: add block encryption framework

[Daniel P. Berrange]

Add a generic framework for support different block encryption
formats. Upon instantiating a QCryptoBlock object, it will read
the encryption header and extract the encryption keys. It is
then possible to call methods to encrypt/decrypt data buffers.

There is also a mode whereby it will create/initialize a new
encryption header on a previously unformatted volume.

The initial framework comes with support for the legacy QCow
AES based encryption. This enables code in the QCow driver to
be consolidated later.

Signed-off-by: Daniel P. Berrange 
---
 crypto/Makefile.objs  |   2 +
 crypto/block-qcow.c   | 173 +++
 crypto/block-qcow.h   |  28 +
 crypto/block.c| 258 ++
 crypto/blockpriv.h|  92 +
 include/crypto/block.h| 232 +
 qapi/crypto.json  |  67 
 tests/.gitignore  |   1 +
 tests/Makefile|   2 +
 tests/test-crypto-block.c | 239 ++
 10 files changed, 1094 insertions(+)
 create mode 100644 crypto/block-qcow.c
 create mode 100644 crypto/block-qcow.h
 create mode 100644 crypto/block.c
 create mode 100644 crypto/blockpriv.h
 create mode 100644 include/crypto/block.h
 create mode 100644 tests/test-crypto-block.c

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 85a9edb..4f5510b 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -19,6 +19,8 @@ crypto-obj-y += ivgen-plain.o
 crypto-obj-y += ivgen-plain64.o
 crypto-obj-y += afsplit.o
 crypto-obj-y += xts.o
+crypto-obj-y += block.o
+crypto-obj-y += block-qcow.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/block-qcow.c b/crypto/block-qcow.c
new file mode 100644
index 000..9f378e8
--- /dev/null
+++ b/crypto/block-qcow.c
@@ -0,0 +1,173 @@
+/*
+ * QEMU Crypto block device encryption QCow/QCow2 AES-CBC format
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+/*
+ * Note that the block encryption implemented in this file is broken
+ * by design. This exists only to allow data to be liberated from
+ * existing qcow[2] images and should not be used in any new areas.
+ */
+
+#include "qemu/osdep.h"
+
+#include "crypto/block-qcow.h"
+#include "crypto/secret.h"
+
+#define QCRYPTO_BLOCK_QCOW_SECTOR_SIZE 512
+
+
+static bool
+qcrypto_block_qcow_has_format(const uint8_t *buf G_GNUC_UNUSED,
+  size_t buf_size G_GNUC_UNUSED)
+{
+return false;
+}
+
+
+static int
+qcrypto_block_qcow_init(QCryptoBlock *block,
+const char *keysecret,
+Error **errp)
+{
+char *password;
+int ret;
+uint8_t keybuf[16];
+int len;
+
+memset(keybuf, 0, 16);
+
+password = qcrypto_secret_lookup_as_utf8(keysecret, errp);
+if (!password) {
+return -1;
+}
+
+len = strlen(password);
+memcpy(keybuf, password, MIN(len, sizeof(keybuf)));
+g_free(password);
+
+block->niv = qcrypto_cipher_get_iv_len(QCRYPTO_CIPHER_ALG_AES_128,
+   QCRYPTO_CIPHER_MODE_CBC);
+block->ivgen = qcrypto_ivgen_new(QCRYPTO_IVGEN_ALG_PLAIN64,
+ 0, 0, NULL, 0, errp);
+if (!block->ivgen) {
+ret = -ENOTSUP;
+goto fail;
+}
+
+block->cipher = qcrypto_cipher_new(QCRYPTO_CIPHER_ALG_AES_128,
+   QCRYPTO_CIPHER_MODE_CBC,
+   keybuf, G_N_ELEMENTS(keybuf),
+   errp);
+if (!block->cipher) {
+ret = -ENOTSUP;
+goto fail;
+}
+
+block->payload_offset = 0;
+
+return 0;
+
+ fail:
+qcrypto_cipher_free(block->cipher);
+qcrypto_ivgen_free(block->ivgen);
+return ret;
+}
+
+
+static int
+qcrypto_block_qcow_open(QCryptoBlock *block,
+QCryptoBlockOpenOptions *options,
+QCryptoBlockReadFunc readfunc G_GNUC_UNUSED,
+void *opaque G_GNUC_UNUSED,
+unsigned int flags,
+Error **errp)
+{
+if (flags & QCRYPTO_BLOCK_OPEN_NO_IO) {
+return 0;
+} else {

[Qemu-block] [PATCH v4 01/26] crypto: add cryptographic random byte source

[Daniel P. Berrange]

There are three backend impls provided. The preferred
is gnutls, which is backed by nettle in modern distros.
The gcrypt impl is provided for cases where QEMU build
against gnutls is disabled, but crypto is still desired.
No nettle impl is provided, since it is non-trivial to
use the nettle APIs for random numbers. Users of nettle
should ensure gnutls is enabled for QEMU.

Reviewed-by: Fam Zheng 
Reviewed-by: Eric Blake 
Signed-off-by: Daniel P. Berrange 
---
 Makefile.objs   |  2 +-
 crypto/Makefile.objs|  4 
 crypto/random-gcrypt.c  | 33 +
 crypto/random-gnutls.c  | 43 +++
 crypto/random-stub.c| 31 +++
 include/crypto/random.h | 44 
 6 files changed, 156 insertions(+), 1 deletion(-)
 create mode 100644 crypto/random-gcrypt.c
 create mode 100644 crypto/random-gnutls.c
 create mode 100644 crypto/random-stub.c
 create mode 100644 include/crypto/random.h

diff --git a/Makefile.objs b/Makefile.objs
index fbcaa74..8f705f6 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,6 +1,6 @@
 ###
 # Common libraries for tools and emulators
-stub-obj-y = stubs/
+stub-obj-y = stubs/ crypto/
 util-obj-y = util/ qobject/ qapi/
 util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
 
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index a3135f1..0929c1c 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -8,6 +8,10 @@ crypto-obj-y += tlscredsanon.o
 crypto-obj-y += tlscredsx509.o
 crypto-obj-y += tlssession.o
 crypto-obj-y += secret.o
+crypto-obj-$(if $(CONFIG_GNUTLS),n,$(CONFIG_GCRYPT)) += random-gcrypt.o
+crypto-obj-$(CONFIG_GNUTLS) += random-gnutls.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
+
+stub-obj-y += random-stub.o
diff --git a/crypto/random-gcrypt.c b/crypto/random-gcrypt.c
new file mode 100644
index 000..0de9a09
--- /dev/null
+++ b/crypto/random-gcrypt.c
@@ -0,0 +1,33 @@
+/*
+ * QEMU Crypto random number provider
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "crypto/random.h"
+
+#include 
+
+int qcrypto_random_bytes(uint8_t *buf,
+ size_t buflen,
+ Error **errp G_GNUC_UNUSED)
+{
+gcry_randomize(buf, buflen, GCRY_STRONG_RANDOM);
+return 0;
+}
diff --git a/crypto/random-gnutls.c b/crypto/random-gnutls.c
new file mode 100644
index 000..04b45a8
--- /dev/null
+++ b/crypto/random-gnutls.c
@@ -0,0 +1,43 @@
+/*
+ * QEMU Crypto random number provider
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "crypto/random.h"
+
+#include 
+#include 
+
+int qcrypto_random_bytes(uint8_t *buf,
+ size_t buflen,
+ Error **errp)
+{
+int ret;
+
+ret = gnutls_rnd(GNUTLS_RND_RANDOM, buf, buflen);
+
+if (ret < 0) {
+error_setg(errp, "Cannot get random bytes: %s",
+   gnutls_strerror(ret));
+return -1;
+}
+
+return 0;
+}
diff --git a/crypto/random-stub.c b/crypto/random-stub.c
new file mode 100644
index 000..63bbf41
--- /dev/null
+++ b/crypto/random-stub.c
@@ -0,0 +1,31 @@
+/*
+ * QEMU Crypto random number provider
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Founda

[Qemu-block] [PATCH v4 06/26] crypto: add support for the cast5-128 cipher algorithm

[Daniel P. Berrange]

A new cipher algorithm 'cast-5-128' is defined for the
Cast-5 algorithm with 128 bit key size. Smaller key sizes
are supported by Cast-5, but nothing in QEMU should use
them, so only 128 bit keys are permitted.

The nettle and gcrypt cipher backends are updated to
support the new cipher and a test vector added to the
cipher test suite. The new algorithm is enabled in the
LUKS block encryption driver.

Signed-off-by: Daniel P. Berrange 
---
 crypto/cipher-gcrypt.c | 18 +-
 crypto/cipher-nettle.c | 26 ++
 crypto/cipher.c|  2 ++
 qapi/crypto.json   |  5 -
 tests/test-crypto-cipher.c |  9 +
 5 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index 56d4c9d..aa1d8c5 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -29,6 +29,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg)
 case QCRYPTO_CIPHER_ALG_AES_128:
 case QCRYPTO_CIPHER_ALG_AES_192:
 case QCRYPTO_CIPHER_ALG_AES_256:
+case QCRYPTO_CIPHER_ALG_CAST5_128:
 return true;
 default:
 return false;
@@ -84,6 +85,10 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
 gcryalg = GCRY_CIPHER_AES256;
 break;
 
+case QCRYPTO_CIPHER_ALG_CAST5_128:
+gcryalg = GCRY_CIPHER_CAST5;
+break;
+
 default:
 error_setg(errp, "Unsupported cipher algorithm %d", alg);
 return NULL;
@@ -113,7 +118,18 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
 ctx->blocksize = 8;
 } else {
 err = gcry_cipher_setkey(ctx->handle, key, nkey);
-ctx->blocksize = 16;
+switch (cipher->alg) {
+case QCRYPTO_CIPHER_ALG_AES_128:
+case QCRYPTO_CIPHER_ALG_AES_192:
+case QCRYPTO_CIPHER_ALG_AES_256:
+ctx->blocksize = 16;
+break;
+case QCRYPTO_CIPHER_ALG_CAST5_128:
+ctx->blocksize = 8;
+break;
+default:
+g_assert_not_reached();
+}
 }
 if (err != 0) {
 error_setg(errp, "Cannot set key: %s",
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index cd2675c..cfa69cc 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #if CONFIG_NETTLE_VERSION_MAJOR < 3
 typedef nettle_crypt_func nettle_cipher_func;
@@ -63,6 +64,18 @@ static void des_decrypt_wrapper(cipher_ctx_t ctx, 
cipher_length_t length,
 des_decrypt(ctx, length, dst, src);
 }
 
+static void cast128_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+uint8_t *dst, const uint8_t *src)
+{
+cast128_encrypt(ctx, length, dst, src);
+}
+
+static void cast128_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+uint8_t *dst, const uint8_t *src)
+{
+cast128_decrypt(ctx, length, dst, src);
+}
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
 void *ctx_encrypt;
@@ -80,6 +93,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg)
 case QCRYPTO_CIPHER_ALG_AES_128:
 case QCRYPTO_CIPHER_ALG_AES_192:
 case QCRYPTO_CIPHER_ALG_AES_256:
+case QCRYPTO_CIPHER_ALG_CAST5_128:
 return true;
 default:
 return false;
@@ -143,6 +157,18 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
 
 ctx->blocksize = AES_BLOCK_SIZE;
 break;
+
+case QCRYPTO_CIPHER_ALG_CAST5_128:
+ctx->ctx_encrypt = g_new0(struct cast128_ctx, 1);
+ctx->ctx_decrypt = NULL; /* 1 ctx can do both */
+
+cast5_set_key(ctx->ctx_encrypt, nkey, key);
+
+ctx->alg_encrypt = cast128_encrypt_wrapper;
+ctx->alg_decrypt = cast128_decrypt_wrapper;
+
+ctx->blocksize = CAST128_BLOCK_SIZE;
+break;
 default:
 error_setg(errp, "Unsupported cipher algorithm %d", alg);
 goto error;
diff --git a/crypto/cipher.c b/crypto/cipher.c
index 076dff0..9e0a226 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -27,6 +27,7 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
 [QCRYPTO_CIPHER_ALG_AES_192] = 24,
 [QCRYPTO_CIPHER_ALG_AES_256] = 32,
 [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
+[QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
 };
 
 static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
@@ -34,6 +35,7 @@ static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
 [QCRYPTO_CIPHER_ALG_AES_192] = 16,
 [QCRYPTO_CIPHER_ALG_AES_256] = 16,
 [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
+[QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
 };
 
 static bool mode_need_iv[QCRYPTO_CIPHER_MODE__MAX] = {
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 42b979a..0550ee7 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -59,11 +59,14 @@
 # @aes-192: AES with 192 bit / 24 byte keys
 # @aes-256: AES with 256 bit / 32 byte keys
 # @des

[Qemu-block] [PATCH v4 07/26] crypto: add support for the serpent cipher algorithm

[Daniel P. Berrange]

New cipher algorithms 'serpent-128', 'serpent-192' and
'serpent-256' are defined for the Serpent algorithm.

The nettle and gcrypt cipher backends are updated to
support the new cipher and a test vector added to the
cipher test suite. The new algorithm is enabled in the
LUKS block encryption driver.

Signed-off-by: Daniel P. Berrange 
---
 crypto/cipher-gcrypt.c | 18 ++
 crypto/cipher-nettle.c | 31 +++
 crypto/cipher.c|  6 ++
 qapi/crypto.json   |  6 +-
 tests/test-crypto-cipher.c | 39 +++
 5 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index aa1d8c5..a667d59 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -30,6 +30,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg)
 case QCRYPTO_CIPHER_ALG_AES_192:
 case QCRYPTO_CIPHER_ALG_AES_256:
 case QCRYPTO_CIPHER_ALG_CAST5_128:
+case QCRYPTO_CIPHER_ALG_SERPENT_128:
+case QCRYPTO_CIPHER_ALG_SERPENT_192:
+case QCRYPTO_CIPHER_ALG_SERPENT_256:
 return true;
 default:
 return false;
@@ -89,6 +92,18 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
 gcryalg = GCRY_CIPHER_CAST5;
 break;
 
+case QCRYPTO_CIPHER_ALG_SERPENT_128:
+gcryalg = GCRY_CIPHER_SERPENT128;
+break;
+
+case QCRYPTO_CIPHER_ALG_SERPENT_192:
+gcryalg = GCRY_CIPHER_SERPENT192;
+break;
+
+case QCRYPTO_CIPHER_ALG_SERPENT_256:
+gcryalg = GCRY_CIPHER_SERPENT256;
+break;
+
 default:
 error_setg(errp, "Unsupported cipher algorithm %d", alg);
 return NULL;
@@ -122,6 +137,9 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
 case QCRYPTO_CIPHER_ALG_AES_128:
 case QCRYPTO_CIPHER_ALG_AES_192:
 case QCRYPTO_CIPHER_ALG_AES_256:
+case QCRYPTO_CIPHER_ALG_SERPENT_128:
+case QCRYPTO_CIPHER_ALG_SERPENT_192:
+case QCRYPTO_CIPHER_ALG_SERPENT_256:
 ctx->blocksize = 16;
 break;
 case QCRYPTO_CIPHER_ALG_CAST5_128:
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index cfa69cc..74b55ab 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #if CONFIG_NETTLE_VERSION_MAJOR < 3
 typedef nettle_crypt_func nettle_cipher_func;
@@ -76,6 +77,18 @@ static void cast128_decrypt_wrapper(cipher_ctx_t ctx, 
cipher_length_t length,
 cast128_decrypt(ctx, length, dst, src);
 }
 
+static void serpent_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+uint8_t *dst, const uint8_t *src)
+{
+serpent_encrypt(ctx, length, dst, src);
+}
+
+static void serpent_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+uint8_t *dst, const uint8_t *src)
+{
+serpent_decrypt(ctx, length, dst, src);
+}
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
 void *ctx_encrypt;
@@ -94,6 +107,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg)
 case QCRYPTO_CIPHER_ALG_AES_192:
 case QCRYPTO_CIPHER_ALG_AES_256:
 case QCRYPTO_CIPHER_ALG_CAST5_128:
+case QCRYPTO_CIPHER_ALG_SERPENT_128:
+case QCRYPTO_CIPHER_ALG_SERPENT_192:
+case QCRYPTO_CIPHER_ALG_SERPENT_256:
 return true;
 default:
 return false;
@@ -169,6 +185,21 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
 
 ctx->blocksize = CAST128_BLOCK_SIZE;
 break;
+
+case QCRYPTO_CIPHER_ALG_SERPENT_128:
+case QCRYPTO_CIPHER_ALG_SERPENT_192:
+case QCRYPTO_CIPHER_ALG_SERPENT_256:
+ctx->ctx_encrypt = g_new0(struct serpent_ctx, 1);
+ctx->ctx_decrypt = NULL; /* 1 ctx can do both */
+
+serpent_set_key(ctx->ctx_encrypt, nkey, key);
+
+ctx->alg_encrypt = serpent_encrypt_wrapper;
+ctx->alg_decrypt = serpent_decrypt_wrapper;
+
+ctx->blocksize = SERPENT_BLOCK_SIZE;
+break;
+
 default:
 error_setg(errp, "Unsupported cipher algorithm %d", alg);
 goto error;
diff --git a/crypto/cipher.c b/crypto/cipher.c
index 9e0a226..0f6fe98 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -28,6 +28,9 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
 [QCRYPTO_CIPHER_ALG_AES_256] = 32,
 [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
 [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
+[QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
+[QCRYPTO_CIPHER_ALG_SERPENT_192] = 24,
+[QCRYPTO_CIPHER_ALG_SERPENT_256] = 32,
 };
 
 static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
@@ -36,6 +39,9 @@ static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
 [QCRYPTO_CIPHER_ALG_AES_256] = 16,
 [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
 [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
+[QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
+  

[Qemu-block] [PATCH v4 04/26] crypto: add support for anti-forensic split algorithm

[Daniel P. Berrange]

The LUKS format specifies an anti-forensic split algorithm which
is used to artificially expand the size of the key material on
disk. This is an implementation of that algorithm.

Signed-off-by: Daniel P. Berrange 
---
 crypto/Makefile.objs|   1 +
 crypto/afsplit.c| 158 
 include/crypto/afsplit.h| 135 +++
 tests/.gitignore|   1 +
 tests/Makefile  |   2 +
 tests/test-crypto-afsplit.c | 190 
 6 files changed, 487 insertions(+)
 create mode 100644 crypto/afsplit.c
 create mode 100644 include/crypto/afsplit.h
 create mode 100644 tests/test-crypto-afsplit.c

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 3040989..5136737 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -17,6 +17,7 @@ crypto-obj-y += ivgen.o
 crypto-obj-y += ivgen-essiv.o
 crypto-obj-y += ivgen-plain.o
 crypto-obj-y += ivgen-plain64.o
+crypto-obj-y += afsplit.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/afsplit.c b/crypto/afsplit.c
new file mode 100644
index 000..8074913
--- /dev/null
+++ b/crypto/afsplit.c
@@ -0,0 +1,158 @@
+/*
+ * QEMU Crypto anti forensic information splitter
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * Derived from cryptsetup package lib/luks1/af.c
+ *
+ * Copyright (C) 2004, Clemens Fruhwirth 
+ * Copyright (C) 2009-2012, Red Hat, Inc. All rights reserved.
+ *
+ * This library 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 library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/afsplit.h"
+#include "crypto/random.h"
+
+
+static void qcrypto_afsplit_xor(size_t blocklen,
+const uint8_t *in1,
+const uint8_t *in2,
+uint8_t *out)
+{
+size_t i;
+for (i = 0; i < blocklen; i++) {
+out[i] = in1[i] ^ in2[i];
+}
+}
+
+
+static int qcrypto_afsplit_hash(QCryptoHashAlgorithm hash,
+size_t blocklen,
+uint8_t *block,
+Error **errp)
+{
+size_t digestlen = qcrypto_hash_digest_len(hash);
+
+size_t hashcount = blocklen / digestlen;
+size_t finallen = blocklen % digestlen;
+uint32_t i;
+
+if (finallen) {
+hashcount++;
+} else {
+finallen = digestlen;
+}
+
+for (i = 0; i < hashcount; i++) {
+uint8_t *out = NULL;
+size_t outlen = 0;
+uint32_t iv = cpu_to_be32(i);
+struct iovec in[] = {
+{ .iov_base = &iv,
+  .iov_len = sizeof(iv) },
+{ .iov_base = block + (i * digestlen),
+  .iov_len = (i == (hashcount - 1)) ? finallen : digestlen },
+};
+
+if (qcrypto_hash_bytesv(hash,
+in,
+G_N_ELEMENTS(in),
+&out, &outlen,
+errp) < 0) {
+return -1;
+}
+
+assert(outlen == digestlen);
+memcpy(block + (i * digestlen), out,
+   (i == (hashcount - 1)) ? finallen : digestlen);
+g_free(out);
+}
+
+return 0;
+}
+
+
+int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
+   size_t blocklen,
+   uint32_t stripes,
+   const uint8_t *in,
+   uint8_t *out,
+   Error **errp)
+{
+uint8_t *block = g_new0(uint8_t, blocklen);
+size_t i;
+int ret = -1;
+
+for (i = 0; i < (stripes - 1); i++) {
+if (qcrypto_random_bytes(out + (i * blocklen), blocklen, errp) < 0) {
+goto cleanup;
+}
+
+qcrypto_afsplit_xor(blocklen,
+out + (i * blocklen),
+block,
+block);
+if (qcrypto_afsplit_hash(hash, blocklen, block,
+ errp) < 0) {
+goto cleanup;
+}
+}
+qcrypto_afsplit_xor(blocklen,
+in,
+block,
+out + (i * blocklen));
+ret = 0;
+
+ cleanup:
+g_free(block);
+return ret;
+}
+
+
+int qcrypto_afsplit_decode(QCryp

[Qemu-block] [PATCH v4 02/26] crypto: add support for PBKDF2 algorithm

[Daniel P. Berrange]

The LUKS data format includes use of PBKDF2 (Password-Based
Key Derivation Function). The Nettle library can provide
an implementation of this, but we don't want code directly
depending on a specific crypto library backend. Introduce
a new include/crypto/pbkdf.h header which defines a QEMU
API for invoking PBKDK2. The initial implementations are
backed by nettle & gcrypt, which are commonly available
with distros shipping GNUTLS.

The test suite data is taken from the cryptsetup codebase
under the LGPLv2.1+ license. This merely aims to verify
that whatever backend we provide for this function in QEMU
will comply with the spec.

Signed-off-by: Daniel P. Berrange 
---
 crypto/Makefile.objs  |   4 +
 crypto/pbkdf-gcrypt.c |  68 +
 crypto/pbkdf-nettle.c |  65 
 crypto/pbkdf-stub.c   |  42 ++
 crypto/pbkdf.c|  77 ++
 include/crypto/pbkdf.h| 152 +++
 tests/.gitignore  |   1 +
 tests/Makefile|   2 +
 tests/test-crypto-pbkdf.c | 377 ++
 9 files changed, 788 insertions(+)
 create mode 100644 crypto/pbkdf-gcrypt.c
 create mode 100644 crypto/pbkdf-nettle.c
 create mode 100644 crypto/pbkdf-stub.c
 create mode 100644 crypto/pbkdf.c
 create mode 100644 include/crypto/pbkdf.h
 create mode 100644 tests/test-crypto-pbkdf.c

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 0929c1c..4d2cd3e 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -10,8 +10,12 @@ crypto-obj-y += tlssession.o
 crypto-obj-y += secret.o
 crypto-obj-$(if $(CONFIG_GNUTLS),n,$(CONFIG_GCRYPT)) += random-gcrypt.o
 crypto-obj-$(CONFIG_GNUTLS) += random-gnutls.o
+crypto-obj-y += pbkdf.o
+crypto-obj-$(CONFIG_NETTLE) += pbkdf-nettle.o
+crypto-obj-$(if $(CONFIG_NETTLE),n,$(CONFIG_GCRYPT)) += pbkdf-gcrypt.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
 
 stub-obj-y += random-stub.o
+stub-obj-y += pbkdf-stub.o
diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c
new file mode 100644
index 000..885614d
--- /dev/null
+++ b/crypto/pbkdf-gcrypt.c
@@ -0,0 +1,68 @@
+/*
+ * QEMU Crypto PBKDF support (Password-Based Key Derivation Function)
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/pbkdf.h"
+#include "gcrypt.h"
+
+bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash)
+{
+switch (hash) {
+case QCRYPTO_HASH_ALG_MD5:
+case QCRYPTO_HASH_ALG_SHA1:
+case QCRYPTO_HASH_ALG_SHA256:
+return true;
+default:
+return false;
+}
+}
+
+int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
+   const uint8_t *key, size_t nkey,
+   const uint8_t *salt, size_t nsalt,
+   unsigned int iterations,
+   uint8_t *out, size_t nout,
+   Error **errp)
+{
+static const int hash_map[QCRYPTO_HASH_ALG__MAX] = {
+[QCRYPTO_HASH_ALG_MD5] = GCRY_MD_MD5,
+[QCRYPTO_HASH_ALG_SHA1] = GCRY_MD_SHA1,
+[QCRYPTO_HASH_ALG_SHA256] = GCRY_MD_SHA256,
+};
+int ret;
+
+if (hash >= G_N_ELEMENTS(hash_map) ||
+hash_map[hash] == GCRY_MD_NONE) {
+error_setg(errp, "Unexpected hash algorithm %d", hash);
+return -1;
+}
+
+ret = gcry_kdf_derive(key, nkey, GCRY_KDF_PBKDF2,
+  hash_map[hash],
+  salt, nsalt, iterations,
+  nout, out);
+if (ret != 0) {
+error_setg(errp, "Cannot derive password: %s",
+   gcry_strerror(ret));
+return -1;
+}
+
+return 0;
+}
diff --git a/crypto/pbkdf-nettle.c b/crypto/pbkdf-nettle.c
new file mode 100644
index 000..1aa7395
--- /dev/null
+++ b/crypto/pbkdf-nettle.c
@@ -0,0 +1,65 @@
+/*
+ * QEMU Crypto PBKDF support (Password-Based Key Derivation Function)
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; witho

[Qemu-block] [PATCH v4 05/26] crypto: skip testing of unsupported cipher algorithms

[Daniel P. Berrange]

We don't guarantee that all crypto backends will support
all cipher algorithms, so we should skip tests unless
the crypto backend indicates support.

Signed-off-by: Daniel P. Berrange 
---
 tests/test-crypto-cipher.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
index 9f912ec..7a073e9 100644
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -380,7 +380,9 @@ int main(int argc, char **argv)
 g_assert(qcrypto_init(NULL) == 0);
 
 for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
-g_test_add_data_func(test_data[i].path, &test_data[i], test_cipher);
+if (qcrypto_cipher_supports(test_data[i].alg)) {
+g_test_add_data_func(test_data[i].path, &test_data[i], 
test_cipher);
+}
 }
 
 g_test_add_func("/crypto/cipher/null-iv",
-- 
2.5.0

[Qemu-block] [PATCH v4 00/26] Support LUKS encryption in block devices

[Daniel P. Berrange]

This series was previously submitted here:

  v1: https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg04748.html
  v2: https://lists.gnu.org/archive/html/qemu-block/2016-01/msg00534.html
  v3: https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg03176.html

This patch series applies as is to master, since all the
dependent block tools code is now merged.

I'd like to try to get the crypto patches + the generic luks
block driver for QEMU 2.6, but leave the qcow2 enhancements
for 2.7

As can be guessed from the subject, the primary goal of this
patch series is to support LUKS encryption in the QEMU block
layer. QEMU has increasingly been adding native clients for
network block protocols (RBD, gluster, NFS, iSCSI, etc) and
apps like OpenStack are embracing them as it is much easier
to deal with this from a management POV than to deal with the
kernel block layer & userspace tools. Unfortunately when using
QEMU native clients, apps are locked out of using dm-crypt
and LUKS which is undesirable.

This series introduces two new features to the block layer.
First there is a general purpose 'luks' format driver which
can be layered over any other existing block driver. eg it
can be layed above RBD, iSCSI, etc. Second the qcow2 file
format is extended so that its embedded encryption can be
replaced with the LUKS data format. While you could just
layer the general purpose luks driver over qcow2, this is
slightly less desirable, as it removes the ability to
reliably auto-detect that LUKS is used by QEMU, as opposed
to used by the guest OS. Having use of LUKS encoded in the
qcow2 header addresses this.

The code is designed such that there is a strict separation
between the full disk encryption format and the block I/O
layer. Thus there is an generic API for dealing with full
disk encryption added to the crypto/ subsystem. The block
layer merely calls this FDE API when required, which serves
to minimize the code present in the already complex block
layer.

The big change in this posting is the addition of an integration
test in tests/qemu-iotests/145.  This uses 'sudo cryptsetup' to
create LUKS volumes on the host kernel and perform I/O to/from
them. This is validated against QEMU's implementation using
qemu-io. Conversely we also create LUKS volumes wth qemu-img
and then validate that the kernel can use them. This proves
good interoperability between the two implementations across
a wide range of crypto algorithms.

The big thing that I have *not* addressed is Fam's idea about
modifying the generic block layer so that it can automatically
instantitate a LUKS block driver above the qcow2 driver, if
qcow2 indicates that it needs encryption support. I'm still
not 100% sure this is the right way to go, but I've not had
time to investigate, since the focus since last posting has
been on dm-crypt interoperability for the generic 'luks'
block driver.

The first 11 patches add some supporting APIs and algorithms
to the crypto subsystem.

The 12-13 patches introduce the general full disk encryption
API to the crypto subsystem and LUKS implementation

Patches 14-18 do a little preparation to the block code for
future patches.

Patch 19 addes the new 'luks' block driver format along with
a test for interoperability with dm-crypt/cryptsetup

Patches 20-23 convert qcow & qcow2 to the new FDE APIs,
enabling LUKS in qcow2 (but not qcow) at the same time.

Patches 24-25 clean up the horrible password handling
cruft in the block layer and monitor.

Patch 26 blocks use of the legacy qcow[2] encryption
from the system emulators.

Chagnes since v3:

 - Rebased to resolve conflicts with recently merged code
 - Add qemu/osdep.h include to all new files
 - Fix unit test for new XTS default mode

Changes since v2:

 - Added support for XTS cipher usage mode
 - Added support for serpent, twofish & cast5 ciphers
 - Use qemu/osdep.h where applicable (Eric)
 - Use MAX macro where applicable (Eric)
 - Use bool instead of gboolean (Eric)
 - Use bytes instead of sectors in API to future proof
   for 4k sector sizes (Eric)
 - Add asserts for LUKS structs matching spec size (Eric)
 - Switch to XTS+plain64 instead of CBC+essiv for new
   LUKS volumes
 - Add qemu I/O test 145 for dm-crypt interoperability
 - Rearrange vol creation so that we treat size as the
   logical guest volume size, instead of physical
   container size (Fam).
 - Push malloc of bounce buffer out of loop in LUKS
   block driver (Eric)
 - Many misc typos/docs fixes (Fam, Eric)

Changes since v1:

 - Unit testing coverage of the full disk encryption
   APIs
 - Functional testing of LUKS driver via the
   qemu-iotests for the block layer
 - Use GNUTLS random API for the random byte source
 - Use Makefile.objs for conditional compilation of
   pbkdf files (Fam)
 - Use 'key-secret' instead of 'key-id' as property
   for decryption key (Paolo)
 - Rename 'fde' to 'crypto' in block code (Kevin)
 - Fix accounting of encryption header clusters when
   checking refcounts (Kevin)
 -