Commit Graph

54472 Commits

Author SHA1 Message Date
Daniel P. Berrange
d85f4222b4 qcow: convert QCow to use QCryptoBlock for encryption
This converts the qcow 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,file=/home/berrange/encrypted.pw \
    -drive file=/home/berrange/encrypted.qcow,encrypt.format=aes,\
           encrypt.key-secret=sec0

Though note that running QEMU system emulators with the AES
encryption is no longer supported, so while the above syntax
is valid, QEMU will refuse to actually run the VM in this
particular example.

Likewise when creating images with the legacy AES-CBC format

  qemu-img create -f qcow \
    --object secret,id=sec0,file=/home/berrange/encrypted.pw \
    -o encrypt.format=aes,encrypt.key-secret=sec0 \
    /home/berrange/encrypted.qcow 64M

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-10-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:56 +02:00
Daniel P. Berrange
1fad1f9400 qcow: make encrypt_sectors encrypt in place
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 uses the same buffer for input/output already
and the other two callers are easily converted to do so.

Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-9-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:56 +02:00
Daniel P. Berrange
0cb8d47ba9 block: deprecate "encryption=on" in favor of "encrypt.format=aes"
Historically the qcow & qcow2 image formats supported a property
"encryption=on" to enable their built-in AES encryption. We'll
soon be supporting LUKS for qcow2, so need a more general purpose
way to enable encryption, with a choice of formats.

This introduces an "encrypt.format" option, which will later be
joined by a number of other "encrypt.XXX" options. The use of
a "encrypt." prefix instead of "encrypt-" is done to facilitate
mapping to a nested QAPI schema at later date.

e.g. the preferred syntax is now

  qemu-img create -f qcow2 -o encrypt.format=aes demo.qcow2

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-8-berrange@redhat.com
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:55 +02:00
Daniel P. Berrange
06af39ecf9 iotests: skip 048 with qcow which doesn't support resize
Test 048 is designed to verify data preservation during an
image resize. The qcow (v1) format impl has never supported
resize so always fails.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-7-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:55 +02:00
Daniel P. Berrange
ebab5636f9 iotests: skip 042 with qcow which dosn't support zero sized images
Test 042 is designed to verify operation with zero sized images.
Such images are not supported with qcow (v1), so this test has
always failed.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-6-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:55 +02:00
Daniel P. Berrange
6aa837f7bd qcow: require image size to be > 1 for new images
The qcow driver refuses to open images which are less than
2 bytes in size, but will happily create such images. Add
a check in the create path to avoid this discrepancy.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-5-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:55 +02:00
Daniel P. Berrange
0b4ee9090e qcow: document another weakness of qcow AES encryption
Document that use of guest virtual sector numbers as the basis for
the initialization vectors is a potential weakness, when combined
with internal snapshots or multiple images using the same passphrase.
This fixes the formatting of the itemized list too.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-4-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:55 +02:00
Daniel P. Berrange
4a47f85431 block: add ability to set a prefix for opt names
When integrating the crypto support with qcow/qcow2, we don't
want to use the bare LUKS option names "hash-alg", "key-secret",
etc. We need to namespace them to match the nested QAPI schema.

e.g. "encrypt.hash-alg", "encrypt.key-secret"

so that they don't clash with any general qcow options at a later
date.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-3-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:55 +02:00
Daniel P. Berrange
306a06e5f7 block: expose crypto option names / defs to other drivers
The block/crypto.c defines a set of QemuOpts that provide
parameters for encryption. This will also be needed by
the qcow/qcow2 integration, so expose the relevant pieces
in a new block/crypto.h header. Some helper methods taking
QemuOpts are changed to take QDict to simplify usage in
other places.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-2-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:55 +02:00
Peter Maydell
29741be341 VFIO fixes 2017-07-10
- Don't iterate over non-realized devices (Alex Williamson)
  - Add PCIe capability version fixup (Alex Williamson)
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v2.0.14 (GNU/Linux)
 
 iQIbBAABAgAGBQJZY9AjAAoJECObm247sIsiQs4P9Ai2tGMihJDJUj71lb/5nQOw
 2yvzfJaSaB1136K9BXRiCEFj1x44e1EdweiPur3AqZ8mDVGY/WXWSqnVV/iCLTVR
 nM7GWtIXnA52LHjwFYomDD/9iz6gj6bPIPGPGAF7iv1A9lfDvgb1Yr9F5bHLE+DQ
 GBlfT5WM0SzfrN5bFI+yN82elDOTHlbRPtrM9P2GDvj9H9Zvd2RDUn5Uv9yGUbQF
 mBYMyBZoL3FL3ij09zFFSwuJGYTwxfWTPnc73BgRAyKMyuy2dnv19XwHB3DQSQ70
 EbWSzcTYjsuquVKni7hfCD+XrP3NMb2U/42hyOAhXVbKAQ5bVmKFPzwA3bHOWBpR
 yWhALZVtl1bTQG4J5nrN+VYQfEv3Yr0dyhVUKZXGvrmP8SoA5WWkdeSrDcEANtew
 VHT7eOsBsCYlqH/0kO772K2XNuvj2XgkDY9da4c5O88WHRk7fUYBZgkjaEQ1uNrq
 +DV4Eixc4UsQiQOTmfCXFWT6hC8vRaM5NA25EvsRXFn8DNEyD/mlRRkndD9Ujawc
 LW1enhleeMXOtR/b6M5TqfPmBMGzQm4ITvM9EumLX/1nF0wDG/Ia+9qNVwZmf2qK
 6/riDUIpbKMzsj9XerDayLp1vTWEuh8Y3ExoOjadKxNYPsN+xcguOknHEVxA0suh
 SR20xhlho/Lq+rzTvcY=
 =FrIF
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20170710.0' into staging

VFIO fixes 2017-07-10

 - Don't iterate over non-realized devices (Alex Williamson)
 - Add PCIe capability version fixup (Alex Williamson)

# gpg: Signature made Mon 10 Jul 2017 20:06:11 BST
# gpg:                using RSA key 0x239B9B6E3BB08B22
# gpg: Good signature from "Alex Williamson <alex.williamson@redhat.com>"
# gpg:                 aka "Alex Williamson <alex@shazbot.org>"
# gpg:                 aka "Alex Williamson <alwillia@redhat.com>"
# gpg:                 aka "Alex Williamson <alex.l.williamson@gmail.com>"
# Primary key fingerprint: 42F6 C04E 540B D1A9 9E7B  8A90 239B 9B6E 3BB0 8B22

* remotes/awilliam/tags/vfio-updates-20170710.0:
  vfio/pci: Fixup v0 PCIe capabilities
  vfio: Test realized when using VFIOGroup.device_list iterator

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-07-11 13:47:28 +01:00
Paolo Bonzini
b5ed2e11ef build: disable Xen on ARM
While ARM could present the xenpv machine, it does not and trying to enable
it breaks compilation.  Revert to the previous test which only looked at
$target_name, not $cpu.

Fixes: 3b6b75506d
Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170711100049.20513-1-pbonzini@redhat.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-07-11 11:23:47 +01:00
Peter Maydell
3d0bf8dfdf Migration pull 2017-07-10
-----BEGIN PGP SIGNATURE-----
 
 iQIcBAABAgAGBQJZY7O5AAoJEAUWMx68W/3nBvMP/Rhj/ldyo9xiKX47VtYElFwI
 RHnzGFYzGCeaEseowESlG0mLXoqVtz/g1k2DKvu6TNJLfZKvgUVHa4gG/+n95STL
 dyjQyRLI08FIezKU2iR95dCecbWJGytze/K53bdXLi82rKcIrLFhfgjhfOl9XSvm
 Law+6UjZB0mi4YBxNqJqAnyTCUSdKfHpqrdECmkkJb9iDGvgO+lZVap+kU04RcTH
 awu3dK68R+pcKXjoo3YvXzoReRJnaxWl/QkcpqSISuel4UT9vX3xfbZ8EnCmxY1t
 F5CiyjDl4yFzcuThzCaSr8xk/W22nCyUa4jojhxXV9UllXrCiqya18eW0p5BBaO7
 /ymuXlOEFOIkZBOmC/Iqh3gOc/wqGrpo6k0T+KJNFZeAPX500HghKgcpId697BoJ
 2AjNuY8t0ond3DLMOfYaIxCLt0q6deyFmPY+mqZzQoO4o5ZcOrebkdTE1feYj+Fg
 RZykuElVrcrW+e1sqnFZMrcTF7PJ14sIH1jq/Ln92q34sQEIm+JHb0rdgtBUTmPO
 Fi8umtX7Wim9hurcpRnGaWgm9kimVtWNUuJbgbP5gI7NHku9cOV4VujyqYpsx1rr
 d2Q8l1UvjUNGDSyG3K10KP2eNIc725dRI4GXhNw0X3gqQuJibGBjXP01VWyFkTvR
 Z8H2Dfb8/c7VFzoPjmm+
 =KZXr
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20170710a' into staging

Migration pull 2017-07-10

# gpg: Signature made Mon 10 Jul 2017 18:04:57 BST
# gpg:                using RSA key 0x0516331EBC5BFDE7
# gpg: Good signature from "Dr. David Alan Gilbert (RH2) <dgilbert@redhat.com>"
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 45F5 C71B 4A0C B7FB 977A  9FA9 0516 331E BC5B FDE7

* remotes/dgilbert/tags/pull-migration-20170710a:
  migration: Make compression_threads use save/load_setup/cleanup()
  migration: Convert ram to use new load_setup()/load_cleanup()
  migration: Create load_setup()/cleanup() methods
  migration: Rename cleanup() to save_cleanup()
  migration: Rename save_live_setup() to save_setup()
  doc: update TYPE_MIGRATION documents
  doc: add item for "-M enforce-config-section"
  vl: move global property, migrate init earlier
  migration: fix handling for --only-migratable

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-07-10 18:13:03 +01:00
Juan Quintela
f0afa331ce migration: Make compression_threads use save/load_setup/cleanup()
Once there, be consistent and use
compress_thread_{save,load}_{setup,cleanup}.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20170628095228.4661-6-quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-07-10 17:52:21 +01:00
Juan Quintela
f265e0e437 migration: Convert ram to use new load_setup()/load_cleanup()
Once there, I rename ram_migration_cleanup() to ram_save_cleanup().
Notice that this is the first pass, and I only passed XBZRLE to the
new scheme.  Moved decoded_buf to inside XBZRLE struct.
As a bonus, I don't have to export xbzrle functions from ram.c.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

--

loaded_data pointer was needed because called can change it (dave)
spell loaded correctly in comment (dave)
Message-Id: <20170628095228.4661-5-quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-07-10 17:52:21 +01:00
Juan Quintela
acb5ea8697 migration: Create load_setup()/cleanup() methods
We need to do things at load time and at cleanup time.

Signed-off-by: Juan Quintela <quintela@redhat.com>

--

Move the printing of the error message so we can print the device
giving the error.
Add call to postcopy stuff
Message-Id: <20170628095228.4661-4-quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-07-10 17:52:21 +01:00
Juan Quintela
70f794fcfa migration: Rename cleanup() to save_cleanup()
We need a cleanup for loads, so we rename here to be consistent.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

--

Rename htab_cleanup to htap_save_cleanup as dave suggestion
Message-Id: <20170628095228.4661-3-quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-07-10 17:52:21 +01:00
Juan Quintela
9907e842d7 migration: Rename save_live_setup() to save_setup()
We are going to use it now for more than save live regions.
Once there rename qemu_savevm_state_begin() to qemu_savevm_state_setup().

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20170628095228.4661-2-quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-07-10 17:52:21 +01:00
Peter Xu
c8d3ff384f doc: update TYPE_MIGRATION documents
[Peter collected Eduardo's patch comment and formatted into patch]

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1499242883-2184-5-git-send-email-peterx@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-07-10 17:52:21 +01:00
Peter Xu
16f7244842 doc: add item for "-M enforce-config-section"
It's never documented, and now we have one more parameter for it (which
obsoletes this one). Document it properly.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1499396048-21657-1-git-send-email-peterx@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  Removed 'Although now' commit message as per Eduardo's review
2017-07-10 17:52:21 +01:00
Peter Xu
00b8ea4e34 vl: move global property, migrate init earlier
Currently drive_init_func() may call migrate_get_current() while the
migrate object is still not ready yet at that time. Move the migration
object init earlier, along with the global properties, right after
acceleration init.

This fixes a breakage for iotest 055, which caused an assertion failure.

Reported-by: Max Reitz <mreitz@redhat.com>
Reported-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Tested-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
Fixes: 3df663 ("migration: move only_migratable to MigrationState")
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1499242883-2184-3-git-send-email-peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-07-10 17:52:21 +01:00
Peter Xu
b605c47b57 migration: fix handling for --only-migratable
MigrateState object is not ready at that time, so we'll get an
assertion. Use qemu_global_option() instead.

Reported-by: Eduardo Habkost <ehabkost@redhat.com>
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Fixes: 3df663e ("migration: move only_migratable to MigrationState")
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1499242883-2184-2-git-send-email-peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-07-10 17:52:21 +01:00
Alex Williamson
47985727e3 vfio/pci: Fixup v0 PCIe capabilities
Intel 82599 VFs report a PCIe capability version of 0, which is
invalid.  The earliest version of the PCIe spec used version 1.  This
causes Windows to fail startup on the device and it will be disabled
with error code 10.  Our choices are either to drop the PCIe cap on
such devices, which has the side effect of likely preventing the guest
from discovering any extended capabilities, or performing a fixup to
update the capability to the earliest valid version.  This implements
the latter.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
2017-07-10 10:39:43 -06:00
Alex Williamson
7da624e26a vfio: Test realized when using VFIOGroup.device_list iterator
VFIOGroup.device_list is effectively our reference tracking mechanism
such that we can teardown a group when all of the device references
are removed.  However, we also use this list from our machine reset
handler for processing resets that affect multiple devices.  Generally
device removals are fully processed (exitfn + finalize) when this
reset handler is invoked, however if the removal is triggered via
another reset handler (piix4_reset->acpi_pcihp_reset) then the device
exitfn may run, but not finalize.  In this case we hit asserts when
we start trying to access PCI helpers since much of the PCI state of
the device is released.  To resolve this, add a pointer to the Object
DeviceState in our common base-device and skip non-realized devices
as we iterate.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
2017-07-10 10:39:43 -06:00
Peter Maydell
6b06e3e49e nbd patches for 2017-07-10
- Eric Blake: MAINTAINERS: Promote NBD to supported, with new maintainer
 - Vladimir Sementsov-Ogievskiy: [00/10] nbd refactoring part 2
 -----BEGIN PGP SIGNATURE-----
 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg
 
 iQEcBAABCAAGBQJZY5ZGAAoJEKeha0olJ0Nq8w8H/23OZ/tr2Nb3qNu6mOMVSeDp
 gvBATbfatfaiBENDtaA76SAWbUlj262OiHGEfWt1VBnBFLIq+sTxGbUIRh34pqre
 AYkDyq+691YDuI54dl6m3KDLusTIZlPskEEsh/88+H1fuZP4lew4Fg5SBRQR00Uf
 N/s5NyS+FTy22GCS5nGWaDzcuKPb5QlVjB8D3vQ4ZWWUw1RMGHTiZ0VfnhBytg7e
 TFaq++Qn7uPVtodLeM5qNsc8XivzqlymBvM7Y9JC4oS0XgoYVFN1uQg47A3ZDoGz
 cTaTt9/RdkQnfPw9RLZBxKq/kEVF+sJIAtbGNc1oUagFOcWLgLtiWfjkA+1CT3k=
 =C+wT
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-07-10-v2' into staging

nbd patches for 2017-07-10

- Eric Blake: MAINTAINERS: Promote NBD to supported, with new maintainer
- Vladimir Sementsov-Ogievskiy: [00/10] nbd refactoring part 2

# gpg: Signature made Mon 10 Jul 2017 15:59:18 BST
# gpg:                using RSA key 0xA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>"
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>"
# gpg:                 aka "[jpeg image of size 6874]"
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2017-07-10-v2:
  nbd: use generic trace subsystem instead of TRACE macro
  nbd: refactor tracing
  nbd/server: rename clientflags var in nbd_negotiate_options
  nbd/server: fix TRACE in nbd_negotiate_send_rep_len
  nbd/client: refactor TRACE of NBD_MAGIC
  nbd/common: nbd_tls_handshake: remove extra TRACE
  nbd/server: add errp to nbd_send_reply()
  nbd/server: use errp instead of LOG
  nbd/server: refactor nbd_negotiate
  nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT
  MAINTAINERS: Promote NBD to supported, with new maintainer

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-07-10 16:12:47 +01:00
Vladimir Sementsov-Ogievskiy
9588463e74 nbd: use generic trace subsystem instead of TRACE macro
Let NBD use the trace mechanisms already present in qemu. Now you can
use the -trace optino of qemu, or the -T/--trace option of qemu-img,
qemu-io, and qemu-nbd, to select nbd traces. For qemu, the QMP commands
trace-event-{get,set}-state can also toggle tracing on the fly.

Example:
   qemu-nbd --trace 'nbd_*' <image file> # enables all nbd traces

Recompilation with CFLAGS=-DDEBUG_NBD is no more needed, furthermore,
DEBUG_NBD macro is removed from the code.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-11-vsementsov@virtuozzo.com>
[eblake: minor tweaks to a couple of traces]
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
6fb2b9726c nbd: refactor tracing
Reorganize traces: move, reword, add information, drop extra ones.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-10-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
7f9039cdaa nbd/server: rename clientflags var in nbd_negotiate_options
Rename 'clientflags' to just 'option'. This variable has nothing to do
with flags, but is a single integer representing the option requested
by the client.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-9-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
4875196163 nbd/server: fix TRACE in nbd_negotiate_send_rep_len
Fix wrong order of TRACE arguments.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-8-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
458d7a6939 nbd/client: refactor TRACE of NBD_MAGIC
We are going to switch from TRACE macro to trace points,
this TRACE complicates things, this patch simplifies it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-7-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
3e6bb543c2 nbd/common: nbd_tls_handshake: remove extra TRACE
Error is propagated to the caller, TRACE is not needed.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170707152918.23086-6-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
c7b9728250 nbd/server: add errp to nbd_send_reply()
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170707152918.23086-5-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
2fd2c8407e nbd/server: use errp instead of LOG
Move to modern errp scheme from just LOGging errors.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-4-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
76ff081d91 nbd/server: refactor nbd_negotiate
Combine two successive "if (oldStyle) {...} else {...}" into one.

Block "if (client->tlscreds)" under "if (oldStyle)" is unreachable,
as we have "oldStyle = client->exp != NULL && !client->tlscreds;".
So, delete this block.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-3-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Vladimir Sementsov-Ogievskiy
1e120ffead nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT
Separate the case when a client sends NBD_OPT_ABORT from all other
errors. It will be needed for the following patch, where errors will be
reported.
This particular case is not actually an error - it honestly follows the
NBD protocol. Therefore it should not be reported like an error.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170707152918.23086-2-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-07-10 09:57:24 -05:00
Eric Blake
99c62e70aa MAINTAINERS: Promote NBD to supported, with new maintainer
We are promising more than just odd fixes, and Paolo is hoping
to offload the pull requests to me.  Also, enough of NBD is related
to the block layer that it is worth including qemu-block on patches.

While at it, include blockdev-nbd.c and qemu-nbd.texi in the set
of maintained files.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170707182151.29872-1-eblake@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
2017-07-10 09:56:01 -05:00
Peter Maydell
94c56652b9 Block layer patches
-----BEGIN PGP SIGNATURE-----
 Version: GnuPG v2.0.22 (GNU/Linux)
 
 iQIcBAABAgAGBQJZY2R0AAoJEH8JsnLIjy/WFNcQALUMh6O/eaXzHrAER3xAA3oF
 WghkekAbVa6TiGoH90UEOrdBS64I7HuWYtwVQQ/9FVV/3Hc1Z15VnBUMrsvcwhvw
 xwIfMilj/av6bk/DoG1S5wb8T+6yUPBpuBzQhsiGnrnI0o9wMzWpSZn41alxCqpe
 jVMpOvDbLIDB9foQhyNAFUGBjhvkBIAe8O8yIkninZB07r87gUN78388NTybLLNh
 I15R9kWKJlqQxWrX4OT7ShcPnuTJClQyjfu1P4BAHzXpv8ZicBYZ/ZR6Q2verZpu
 kBwXlBxnWKJDOPyd1sKBNe3Mo3KqKXfSL0NOg00PHPRTcgh+e+QYZ4ZxG0Dv6Cyl
 rb6Cy0FBuM6JtjhuCLt9yZ1eErCtpHJq901T7DMb4LQ3IuFrDJToojdPTaOx4st4
 lh7tiraQtp3OKzh/H7smZ5WT7V8Sg2l3mPAs3r7iPihzceS9yPUbka3yB2xbwnpn
 e7H8IzwHFgqRprR+Ii8Z+0eWHApOJDSK2mCcR1OGJEpxXz2gYn6+WrvKBKxDScln
 deIo7Kz6/5OehRKjPGWJmYsgpQBzlLoPUXk1K75OMOu2y4sLGaFr/ZqZgLjOxunZ
 jJ2h5TGgzkRZOsIKQAhrBkBZ09MOI7PMyxkbGu4o+qbovG3InE9kHzH2HBsIIDKS
 zhG+zHFGha8YsyKS6a6Q
 =+05n
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging

Block layer patches

# gpg: Signature made Mon 10 Jul 2017 12:26:44 BST
# gpg:                using RSA key 0x7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream: (40 commits)
  block: Make bdrv_is_allocated_above() byte-based
  block: Minimize raw use of bds->total_sectors
  block: Make bdrv_is_allocated() byte-based
  backup: Switch backup_run() to byte-based
  backup: Switch backup_do_cow() to byte-based
  backup: Switch block_backup.h to byte-based
  backup: Switch BackupBlockJob to byte-based
  block: Drop unused bdrv_round_sectors_to_clusters()
  mirror: Switch mirror_iteration() to byte-based
  mirror: Switch mirror_do_read() to byte-based
  mirror: Switch mirror_cow_align() to byte-based
  mirror: Update signature of mirror_clip_sectors()
  mirror: Switch mirror_do_zero_or_discard() to byte-based
  mirror: Switch MirrorBlockJob to byte-based
  commit: Switch commit_run() to byte-based
  commit: Switch commit_populate() to byte-based
  stream: Switch stream_run() to byte-based
  stream: Drop reached_end for stream_complete()
  stream: Switch stream_populate() to byte-based
  trace: Show blockjob actions via bytes, not sectors
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-07-10 14:06:49 +01:00
Eric Blake
51b0a48888 block: Make bdrv_is_allocated_above() byte-based
We are gradually moving away from sector-based interfaces, towards
byte-based.  In the common case, allocation is unlikely to ever use
values that are not naturally sector-aligned, but it is possible
that byte-based values will let us be more precise about allocation
at the end of an unaligned file that can do byte-based access.

Changing the signature of the function to use int64_t *pnum ensures
that the compiler enforces that all callers are updated.  For now,
the io.c layer still assert()s that all callers are sector-aligned,
but that can be relaxed when a later patch implements byte-based
block status.  Therefore, for the most part this patch is just the
addition of scaling at the callers followed by inverse scaling at
bdrv_is_allocated().  But some code, particularly stream_run(),
gets a lot simpler because it no longer has to mess with sectors.
Leave comments where we can further simplify by switching to
byte-based iterations, once later patches eliminate the need for
sector-aligned operations.

For ease of review, bdrv_is_allocated() was tackled separately.

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:07 +02:00
Eric Blake
c00716beb3 block: Minimize raw use of bds->total_sectors
bdrv_is_allocated_above() was relying on intermediate->total_sectors,
which is a field that can have stale contents depending on the value
of intermediate->has_variable_length.  An audit shows that we are safe
(we were first calling through bdrv_co_get_block_status() which in
turn calls bdrv_nb_sectors() and therefore just refreshed the current
length), but it's nicer to favor our accessor functions to avoid having
to repeat such an audit, even if it means refresh_total_sectors() is
called more frequently.

Suggested-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:07 +02:00
Eric Blake
d6a644bbfe block: Make bdrv_is_allocated() byte-based
We are gradually moving away from sector-based interfaces, towards
byte-based.  In the common case, allocation is unlikely to ever use
values that are not naturally sector-aligned, but it is possible
that byte-based values will let us be more precise about allocation
at the end of an unaligned file that can do byte-based access.

Changing the signature of the function to use int64_t *pnum ensures
that the compiler enforces that all callers are updated.  For now,
the io.c layer still assert()s that all callers are sector-aligned
on input and that *pnum is sector-aligned on return to the caller,
but that can be relaxed when a later patch implements byte-based
block status.  Therefore, this code adds usages like
DIV_ROUND_UP(,BDRV_SECTOR_SIZE) to callers that still want aligned
values, where the call might reasonbly give non-aligned results
in the future; on the other hand, no rounding is needed for callers
that should just continue to work with byte alignment.

For the most part this patch is just the addition of scaling at the
callers followed by inverse scaling at bdrv_is_allocated().  But
some code, particularly bdrv_commit(), gets a lot simpler because it
no longer has to mess with sectors; also, it is now possible to pass
NULL if the caller does not care how much of the image is allocated
beyond the initial offset.  Leave comments where we can further
simplify once a later patch eliminates the need for sector-aligned
requests through bdrv_is_allocated().

For ease of review, bdrv_is_allocated_above() will be tackled
separately.

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:07 +02:00
Eric Blake
6f8e35e241 backup: Switch backup_run() to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Change the internal
loop iteration of backups to track by bytes instead of sectors
(although we are still guaranteed that we iterate by steps that
are cluster-aligned).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
03f5d60bbf backup: Switch backup_do_cow() to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Convert another internal
function (no semantic change).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
f6ac207893 backup: Switch block_backup.h to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Continue by converting
the public interface to backup jobs (no semantic change), including
a change to CowRequest to track by bytes instead of cluster indices.

Note that this does not change the difference between the public
interface (starting point, and size of the subsequent range) and
the internal interface (starting and end points).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Xie Changlong <xiechanglong@cmss.chinamobile.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
cf79cdf662 backup: Switch BackupBlockJob to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Continue by converting an
internal structure (no semantic change), and all references to
tracking progress.  Drop a redundant local variable bytes_per_cluster.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
e8a81e9cad block: Drop unused bdrv_round_sectors_to_clusters()
Now that the last user [mirror_iteration()] has converted to using
bytes, we no longer need a function to round sectors to clusters.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
fb2ef7919b mirror: Switch mirror_iteration() to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Change the internal
loop iteration of mirroring to track by bytes instead of sectors
(although we are still guaranteed that we iterate by steps that
are both sector-aligned and multiples of the granularity).  Drop
the now-unused mirror_clip_sectors().

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
ae4cc8777b mirror: Switch mirror_do_read() to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Convert another internal
function, preserving all existing semantics, and adding one more
assertion that things are still sector-aligned (so that conversions
to sectors in mirror_read_complete don't need to round).

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
782d97efec mirror: Switch mirror_cow_align() to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Convert another internal
function (no semantic change), and add mirror_clip_bytes() as a
counterpart to mirror_clip_sectors().  Some of the conversion is
a bit tricky, requiring temporaries to convert between units; it
will be cleared up in a following patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
931e52607f mirror: Update signature of mirror_clip_sectors()
Rather than having a void function that modifies its input
in-place as the output, change the signature to reduce a layer
of indirection and return the result.

Suggested-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
e6f2419389 mirror: Switch mirror_do_zero_or_discard() to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Convert another internal
function (no semantic change).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00
Eric Blake
b436982f04 mirror: Switch MirrorBlockJob to byte-based
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Continue by converting an
internal structure (no semantic change), and all references to the
buffer size.

Add an assertion that our use of s->granularity >> BDRV_SECTOR_BITS
(necessary for interaction with sector-based dirty bitmaps, until
a later patch converts those to be byte-based) does not suffer from
truncation problems.

[checkpatch has a false positive on use of MIN() in this patch]

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-10 13:18:06 +02:00