summaryrefslogtreecommitdiff
path: root/block
AgeCommit message (Collapse)AuthorFilesLines
2017-04-04Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingPeter Maydell2-1/+1
* MemoryRegionCache revert * glib optimization workaround * fix "info lapic" segfault on isapc * fix QIOChannel memory leak # gpg: Signature made Mon 03 Apr 2017 18:17:00 BST # gpg: using RSA key 0xBFFBD25F78C7AE83 # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" # gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" # Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4 E2F7 7E15 100C CD36 69B1 # Subkey fingerprint: F133 3857 4B66 2389 866C 7682 BFFB D25F 78C7 AE83 * remotes/bonzini/tags/for-upstream: main-loop: Acquire main_context lock around os_host_main_loop_wait. exec: revert MemoryRegionCache nbd: fix memory leak on socket_connect failed ipmi: Fix macro issues target-i386: fix "info lapic" segfault on isapc iscsi: drop unused IscsiAIOCB.qiov field Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-04-03block/parallels: Avoid overflowsMax Reitz1-6/+13
Change the types of variables in allocate_clusters() to int64_t so we do not have to worry about potential overflows. Add an assertion that our accesses to s->bat[] do not result in a buffer overflow and that the implicit conversion performed when invoking bat_entry_off() does not result in an integer overflow. Coverity-id: 1307776 Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20170331170512.10381-1-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-03qcow2: Discard unaligned tail when wiping imageEric Blake1-6/+4
There is a subtle difference between the fast (qcow2v3 with no extra data) and slow path (qcow2v2 format [aka 0.10], or when a snapshot is present) of qcow2_make_empty(). The slow path fails to discard the final (partial) cluster of an unaligned image. The problem stems from the fact that qcow2_discard_clusters() was silently ignoring sub-cluster head and tail on unaligned requests. A quick audit of all callers shows that qcow2_snapshot_create() has always passed a cluster-aligned request since the call was added in commit 1ebf561; qcow2_co_pdiscard() has passed a cluster-aligned request since commit ecdbead taught the block layer about preferred discard alignment; and qcow2_make_empty() was fixed to pass an aligned start (but not necessarily end) in commit a3e1505. Asserting that the start is always aligned also points out that we now have a dead check: rounding the end offset down can never result in a value less than the aligned start offset (the check was rendered dead with commit ecdbead). Meanwhile, we do not want to round the end cluster down in the one case of the end offset matching the (unaligned) file size - that final partial cluster should still be discarded. With those fixes in place, the fast and slow paths are back in sync at discarding an entire image; the next patch will update qemu-iotests to ensure we don't regress. Note that bdrv_co_pdiscard ignores ALL partial cluster requests, including the partial cluster at the end of an image; it can be argued that the partial cluster at the end should be special-cased so that a guest issuing discard requests at proper alignments everywhere else can likewise empty the entire image. But that optimization is left for another day. Signed-off-by: Eric Blake <eblake@redhat.com> Message-id: 20170331185356.2479-3-eblake@redhat.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-03sheepdog: Fix blockdev-addMarkus Armbruster1-32/+58
Commit 831acdc "sheepdog: Implement bdrv_parse_filename()" and commit d282f34 "sheepdog: Support blockdev-add" have different ideas on how the QemuOpts parameters for the server address are named. Fix that. While there, rename BlockdevOptionsSheepdog member addr to server, for consistency with BlockdevOptionsSsh, BlockdevOptionsGluster, BlockdevOptionsNbd. Commit 831acdc's example becomes --drive driver=sheepdog,server.type=inet,server.host=fido,server.port=7000,vdi=dolly instead of --drive driver=sheepdog,host=fido,vdi=dolly Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Tested-by: Kashyap Chamarthy <kchamart@redhat.com> Message-id: 1490895797-29094-10-git-send-email-armbru@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-03nbd: Tidy up blockdev-add interfaceMarkus Armbruster1-25/+28
SocketAddress is a simple union, and simple unions are awkward: they have their variant members wrapped in a "data" object on the wire, and require additional indirections in C. I intend to limit its use to existing external interfaces, and convert all internal interfaces to SocketAddressFlat. BlockdevOptionsNbd is an external interface using SocketAddress. We already use SocketAddressFlat elsewhere in blockdev-add. Replace it by SocketAddressFlat while we can (it's new in 2.9) for simplicity and consistency. For example, { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": "nbd", "server": { "type": "inet", "data": { "host": "localhost", "port": "12345" } } } } becomes { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": "nbd", "server": { "type": "inet", "host": "localhost", "port": "12345" } } } Since the internal interfaces still take SocketAddress, this requires conversion function socket_address_crumple(). It'll go away when I update the interfaces. Unfortunately, SocketAddress is also visible in -drive since 2.8: -drive if=none,driver=nbd,server.type=inet,server.data.host=127.0.0.1,server.data.port=12345 Nobody should be using it, as it's fairly new and has never been documented, so adding still more compatibility gunk to keep it working isn't worth the trouble. You now have to use -drive if=none,driver=nbd,server.type=inet,server.host=127.0.0.1,server.port=12345 Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-id: 1490895797-29094-9-git-send-email-armbru@redhat.com [mreitz: Change iotest 147 accordingly] Because of this interface change, iotest 147 has to be adapted. Unfortunately, we cannot just flatten all of the addresses because nbd-server-start still takes a plain SocketAddress. Therefore, we need both and this is most easily achieved by writing the SocketAddress into the code and flattening it where necessary. Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20170330221243.17333-1-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-03qapi-schema: SocketAddressFlat variants 'vsock' and 'fd'Markus Armbruster1-0/+2
Note that the new variants are impossible in qemu_gluster_glfs_init(), because the gconf->server can only come from qemu_gluster_parse_uri() or qemu_gluster_parse_json(), and neither can create anything but 'inet' or 'unix'. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1490895797-29094-7-git-send-email-armbru@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-03gluster: Prepare for SocketAddressFlat extensionMarkus Armbruster1-10/+16
qemu_gluster_glfs_init() and qemu_gluster_parse_json() rely on the fact that SocketAddressFlatType has only two members SOCKET_ADDRESS_FLAT_TYPE_INET and SOCKET_ADDRESS_FLAT_TYPE_UNIX. Correct, but won't stay correct. Make them more robust. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490895797-29094-6-git-send-email-armbru@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-03block: Document -drive problematic code and bugsMarkus Armbruster5-0/+35
-blockdev and blockdev_add convert their arguments via QObject to BlockdevOptions for qmp_blockdev_add(), which converts them back to QObject, then to a flattened QDict. The QDict's members are typed according to the QAPI schema. -drive converts its argument via QemuOpts to a (flat) QDict. This QDict's members are all QString. Thus, the QType of a flat QDict member depends on whether it comes from -drive or -blockdev/blockdev_add, except when the QAPI type maps to QString, which is the case for 'str' and enumeration types. The block layer core extracts generic configuration from the flat QDict, and the block driver extracts driver-specific configuration. Both commonly do so by converting (parts of) the flat QDict to QemuOpts, which turns all values into strings. Not exactly elegant, but correct. However, A few places access the flat QDict directly: * Most of them access members that are always QString. Correct. * bdrv_open_inherit() accesses a boolean, carefully. Correct. * nfs_config() uses a QObject input visitor. Correct only because the visited type contains nothing but QStrings. * nbd_config() and ssh_config() use a QObject input visitor, and the visited types contain non-QStrings: InetSocketAddress members @numeric, @to, @ipv4, @ipv6. -drive works as long as you don't try to use them (they're all optional). @to is ignored anyway. Reproducer: -drive driver=ssh,server.host=h,server.port=22,server.ipv4,path=p -drive driver=nbd,server.type=inet,server.data.host=h,server.data.port=22,server.data.ipv4 both fail with "Invalid parameter type for 'data.ipv4', expected: boolean" Add suitable comments to all these places. Mark the buggy ones FIXME. "Fortunately", -drive's driver-specific options are entirely undocumented. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-id: 1490895797-29094-5-git-send-email-armbru@redhat.com [mreitz: Fixed two typos] Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-03nbd sockets vnc: Mark problematic address family tests TODOMarkus Armbruster1-0/+1
Certain features make sense only with certain address families. For instance, passing file descriptors requires AF_UNIX. Testing SocketAddress's saddr->type == SOCKET_ADDRESS_KIND_UNIX is obvious, but problematic: it can't recognize AF_UNIX when type == SOCKET_ADDRESS_KIND_FD. Mark such tests of saddr->type TODO. We may want to check the address family with getsockname() there. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1490895797-29094-2-git-send-email-armbru@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-02nbd: fix memory leak on socket_connect failedyaolujing1-0/+1
When TCP connection fails between nbd server and client, the local var, sioc, memory leak. This patch fixes the memory leak. Signed-off-by: yaolujing <yaolujing@huawei.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <1491005709-29989-1-git-send-email-yaolujing@huawei.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-04-02iscsi: drop unused IscsiAIOCB.qiov fieldStefan Hajnoczi1-1/+0
The IscsiAIOCB.qiov field has been unused since commit 063c3378a9e3c25cc0afac3c72e4823d0621e352 ("block/iscsi: introduce bdrv_co_{readv, writev, flush_to_disk}") back in 2013. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20170327165005.22038-1-stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-03-31block/curl: Check protocol prefixMax Reitz1-0/+10
If the user has explicitly specified a block driver and thus a protocol, we have to make sure the URL's protocol prefix matches. Otherwise the latter will silently override the former which might catch some users by surprise. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 20170331120431.1767-3-mreitz@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-31rbd: Fix regression in legacy key/values containing escaped :Eric Blake1-43/+44
Commit c7cacb3 accidentally broke legacy key-value parsing through pseudo-filename parsing of -drive file=rbd://..., for any key that contains an escaped ':'. Such a key is surprisingly common, thanks to mon_host specifying a 'host:port' string. The break happens because passing things from QDict through QemuOpts back to another QDict requires that we pack our parsed key/value pairs into a string, and then reparse that string, but the intermediate string that we created ("key1=value1:key2=value2") lost the \: escaping that was present in the original, so that we could no longer see which : were used as separators vs. those used as part of the original input. Fix it by collecting the key/value pairs through a QList, and sending that list on a round trip through a JSON QString (as in '["key1","value1","key2","value2"]') on its way through QemuOpts, rather than hand-rolling our own string. Since the string is only handled internally, this was faster than creating a full-blown struct of '[{"key1":"value1"},{"key2":"value2"}]', and safer at guaranteeing order compared to '{"key1":"value1","key2":"value2"}'. It would be nicer if we didn't have to round-trip through QemuOpts in the first place, but that's a much bigger task for later. Reproducer: ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ -drive 'file=rbd:volumes/volume-ea141b5c-cdb3-4765-910d-e7008b209a70'\ ':id=compute:key=AQAVkvxXAAAAABAA9ZxWFYdRmV+DSwKr7BKKXg=='\ ':auth_supported=cephx\;none:mon_host=192.168.1.2\:6789'\ ',format=raw,if=none,id=drive-virtio-disk0,'\ 'serial=ea141b5c-cdb3-4765-910d-e7008b209a70,cache=writeback' Even without an RBD setup, this serves a test of whether we get the incorrect parser error of: qemu-system-x86_64: -drive file=rbd:...cache=writeback: conf option 6789 has no value or the correct behavior of hanging while trying to connect to the requested mon_host of 192.168.1.2:6789. Reported-by: Alexandru Avadanii <Alexandru.Avadanii@enea.com> Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20170331152730.12514-1-eblake@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Fix bugs around -drive parameter "server"Markus Armbruster1-95/+32
qemu_rbd_open() takes option parameters as a flattened QDict, with keys of the form server.%d.host, server.%d.port, where %d counts up from zero. qemu_rbd_array_opts() extracts these values as follows. First, it calls qdict_array_entries() to find the list's length. For each list element, it formats the list's key prefix (e.g. "server.0."), then creates a new QDict holding the options with that key prefix, then converts that to a QemuOpts, so it can finally get the member values from there. If there's one surefire way to make code using QDict more awkward, it's creating more of them and mixing in QemuOpts for good measure. The extraction of keys starting with server.%d into another QDict makes us ignore parameters like server.0.neither-host-nor-port silently. The conversion to QemuOpts abuses runtime_opts, as described a few commits ago. Rewrite to simply get the values straight from the options QDict. Fixes -drive not to crash when server.*.* are present, but server.*.host is absent. Fixes -drive to reject invalid server.*.*. Permits cleaning up runtime_opts. Do that, and fix -drive to reject bogus parameters host and port instead of silently ignoring them. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-11-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Revert -blockdev and -drive parameter auth-supportedMarkus Armbruster1-28/+3
This reverts half of commit 0a55679. We're having second thoughts on the QAPI schema (and thus the external interface), and haven't reached consensus, yet. Issues include: * The implementation uses deprecated rados_conf_set() key "auth_supported". No biggie. * The implementation makes -drive silently ignore invalid parameters "auth" and "auth-supported.*.X" where X isn't "auth". Fixable (in fact I'm going to fix similar bugs around parameter server), so again no biggie. * BlockdevOptionsRbd member @password-secret applies only to authentication method cephx. Should it be a variant member of RbdAuthMethod? * BlockdevOptionsRbd member @user could apply to both methods cephx and none, but I'm not sure it's actually used with none. If it isn't, should it be a variant member of RbdAuthMethod? * The client offers a *set* of authentication methods, not a list. Should the methods be optional members of BlockdevOptionsRbd instead of members of list @auth-supported? The latter begs the question what multiple entries for the same method mean. Trivial question now that RbdAuthMethod contains nothing but @type, but less so when RbdAuthMethod acquires other members, such the ones discussed above. * How BlockdevOptionsRbd member @auth-supported interacts with settings from a configuration file specified with @conf is undocumented. I suspect it's untested, too. Let's avoid painting ourselves into a corner now, and revert the feature for 2.9. Note that users can still configure authentication methods with a configuration file. They probably do that anyway if they use Ceph outside QEMU as well. Further note that this doesn't affect use of key "auth-supported" in -drive file=rbd:...:key=value. qemu_rbd_array_opts()'s parameter @type now must be RBD_MON_HOST, which is silly. This will be cleaned up shortly. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-9-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Clean up qemu_rbd_create()'s detour through QemuOptsMarkus Armbruster1-15/+5
The conversion from QDict to QemuOpts is pointless. Simply get the stuff straight from the QDict. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-8-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Clean up runtime_opts, fix -drive to reject filenameMarkus Armbruster1-14/+24
runtime_opts is used for three different purposes: * qemu_rbd_open() uses it to accept options it recognizes, such as "pool" and "image". Other .bdrv_open() methods do it similarly. * qemu_rbd_open() accepts additional list-valued options auth-supported and server, with the help of qemu_rbd_array_opts(). The list elements are again dictionaries. qemu_rbd_array_opts() uses runtime_opts to accept their members. Thus, runtime_opts contains recognized sub-sub-options "auth", "host", "port" in addition to recognized options. No other block driver does that. * qemu_rbd_create() uses it to convert the QDict produced by qemu_rbd_parse_filename() to QemuOpts. No other block driver does that. The keys produced by qemu_rbd_parse_filename() are "pool", "image", "snapshot", "conf", "user" and "keyvalue-pairs". qemu_rbd_open() accepts these, so no additional ones here. This is a confusing mess. Dates back to commit 0f9d252. First step to clean it up is documenting runtime_opts.desc[]: * Reorder entries to match the QAPI schema, like we do in other block drivers. * Document why the schema's "server" and "auth-supported" aren't in .desc[]. * Document why "keyvalue-pairs", "host", "port" and "auth" are in .desc[], but not the schema. * Delete "filename", because none of the three users actually uses it. This fixes -drive to reject parameter filename instead of silently ignoring it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-7-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Don't accept -drive driver=rbd, keyvalue-pairs=...Markus Armbruster1-4/+8
The way we communicate extra key-value pairs from qemu_rbd_parse_filename() to qemu_rbd_open() exposes option parameter "keyvalue-pairs" on the command line. It's not wanted there. Hack: rename the parameter to "=keyvalue-pairs" to make it inaccessible. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-6-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Clean up after the previous commitMarkus Armbruster1-15/+9
This code in qemu_rbd_parse_filename() found_str = qemu_rbd_next_tok(p, '\0', &p); p = found_str; has no effect. Drop it, and simplify qemu_rbd_next_tok(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-5-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Don't limit length of parameter valuesMarkus Armbruster1-77/+14
We laboriously enforce that parameter values are between one and some arbitrary limit in length. Only RBD_MAX_IMAGE_NAME_SIZE comes from librbd.h, and I'm not sure it applies. Where the other limits come from is unclear. Drop the length checking. The limits librbd actually imposes must be checked by librbd anyway. There's one minor complication: BDRVRBDState member name is a fixed-size array. Depends on the length limit. Make it a pointer to a dynamically allocated string. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-4-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Fix to cleanly reject -drive without pool or imageMarkus Armbruster1-3/+7
qemu_rbd_open() neglects to check pool and image are present. Missing image is caught by rbd_open(), but missing pool crashes. Reproducer: $ qemu-system-x86_64 -nodefaults -drive driver=rbd,id=rbd,image=i,... terminate called after throwing an instance of 'std::logic_error' what(): basic_string::_M_construct null not valid Aborted (core dumped) where ... is a working server.0.{host,port} configuration. Doesn't affect -drive with file=..., because qemu_rbd_parse_filename() always sets both pool and image. Doesn't affect -blockdev, because pool and image are mandatory in the QAPI schema. Fix by adding the missing checks. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-3-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28parallels: wrong call to bdrv_truncateDenis V. Lunev1-1/+2
Parallels driver should not call bdrv_truncate if the image was opened in the read-only mode. Without the patch qemu-img check harddisk.hds asserts with bdrv_truncate: Assertion `child->perm & BLK_PERM_RESIZE' failed. Parameters used on the write path are not needed if the image is opened in the read-only mode. Signed-off-by: Denis V. Lunev <den@openvz.org> Reported-by: Edgar Kaziahmedov <edos@virtuozzo.mipt.ru> Message-id: 1490625488-7980-1-git-send-email-den@openvz.org CC: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2017-03-27Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingPeter Maydell2-4/+0
* MTTCG fix for win32 * virtio-scsi assertion failure * mem-prealloc coverity fix * x86 migration revert which requires more thought * x86 instruction limit (avoids >2 page translation blocks) * nbd dead code cleanup * small memory.c logic fix # gpg: Signature made Mon 27 Mar 2017 17:03:04 BST # gpg: using RSA key 0xBFFBD25F78C7AE83 # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" # gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" # Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4 E2F7 7E15 100C CD36 69B1 # Subkey fingerprint: F133 3857 4B66 2389 866C 7682 BFFB D25F 78C7 AE83 * remotes/bonzini/tags/for-upstream: scsi-generic: Fill in opt_xfer_len in INQUIRY reply if it is zero Revert "apic: save apic_delivered flag" nbd: drop unused NBDClientSession.is_unix field win32: replace custom mutex and condition variable with native primitives mem-prealloc: fix sysconf(_SC_NPROCESSORS_ONLN) failure case. tcg/i386: Check the size of instruction being translated virtio-scsi: Fix acquire/release in dataplane handlers virtio-scsi: Make virtio_scsi_acquire/release public clear pending status before calling memory commit Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-03-27block/file-posix.c: Fix unused variable warning on OpenBSDPeter Maydell1-14/+14
On OpenBSD none of the ioctls probe_logical_blocksize() tries exist, so the variable sector_size is unused. Refactor the code to avoid this (and reduce the duplicated code). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490279788-12995-1-git-send-email-peter.maydell@linaro.org Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-03-27file-posix: Make bdrv_flush() failure permanent without O_DIRECTKevin Wolf1-0/+22
Success for bdrv_flush() means that all previously written data is safe on disk. For fdatasync(), the best semantics we can hope for on Linux (without O_DIRECT) is that all data that was written since the last call was successfully written back. Therefore, and because we can't redo all writes after a flush failure, we have to give up after a single fdatasync() failure. After this failure, we would never be able to make the promise that a successful bdrv_flush() makes. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-id: 20170322210005.16533-1-kwolf@redhat.com Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-03-27nbd-client: fix handling of hungup connectionsPaolo Bonzini1-6/+6
After the switch to reading replies in a coroutine, nothing is reentering pending receive coroutines if the connection hangs. Move nbd_recv_coroutines_enter_all to the reply read coroutine, which is the place where hangups are detected. nbd_teardown_connection can simply wait for the reply read coroutine to detect the hangup and clean up after itself. This wouldn't be enough though because nbd_receive_reply returns 0 (rather than -EPIPE or similar) when reading from a hung connection. Fix the return value check in nbd_read_reply_entry. This fixes qemu-iotests 083. Reported-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20170314111157.14464-1-pbonzini@redhat.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-03-27nbd: drop unused NBDClientSession.is_unix fieldStefan Hajnoczi2-4/+0
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20170327123223.1199-1-stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-03-24trace: Fix backwards mirror_yield parametersEric Blake1-2/+3
block/trace-events lists the parameters for mirror_yield consistently with other mirror events (cnt just after s, like in mirror_before_sleep; in_flight last, like in mirror_yield_in_flight). But the callers were passing parameters in the wrong order, leading to poor trace messages, including type truncation when there are more than 4G dirty sectors involved. Broken since its introduction in commit bd48bde. While touching this, ensure that all callers use the same type (uint64_t) for cnt, as a later patch will enable the compiler to do stricter type-checking. Signed-off-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2017-03-23Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into stagingPeter Maydell1-2/+22
# gpg: Signature made Wed 22 Mar 2017 17:28:56 GMT # gpg: using RSA key 0xBDBE7B27C0DE3057 # gpg: Good signature from "Jeffrey Cody <jcody@redhat.com>" # gpg: aka "Jeffrey Cody <jeff@codyprime.org>" # gpg: aka "Jeffrey Cody <codyprime@gmail.com>" # Primary key fingerprint: 9957 4B4D 3474 90E7 9D98 D624 BDBE 7B27 C0DE 3057 * remotes/cody/tags/block-pull-request: blockjob: add devops to blockjob backends block-backend: add drained_begin / drained_end ops blockjob: add block_job_start_shim blockjob: avoid recursive AioContext locking Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-03-22block-backend: add drained_begin / drained_end opsJohn Snow1-2/+22
Allow block backends to forward drain requests to their devices/users. The initial intended purpose for this patch is to allow BBs to forward requests along to BlockJobs, which will want to pause if their associated BB has entered a drained region. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 20170316212351.13797-3-jsnow@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-21parallels: fix default options parsingEdgar Kaziahmedov1-1/+1
parallels block driver is completely broken since commit commit 75cdcd1553e74b5edc58aed23e3b2da8dabb1876 Author: Markus Armbruster <armbru@redhat.com> Date: Tue Feb 21 21:14:08 2017 +0100 option: Fix checking of sizes for overflow and trailing crap Right now even simple qemu-io -c "read 512 64k" 1.hds ends up with Unexpected error in parse_option_size() at util/qemu-option.c:188: Parameter 'prealloc-size' expects a non-negative number below 2^64 Aborted (core dumped) The cure is simple - we should use 'M' as a suffix in default option value instead of 'MiB'. Signed-off-by: Edgar Kaziahmedov <edos@virtuozzo.mipt.ru> Signed-off-by: Denis V. Lunev <den@openvz.org> Message-id: 1490002022-22653-1-git-send-email-den@openvz.org CC: Markus Armbruster <armbru@redhat.com> CC: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2017-03-17curl: fix compilation on OpenBSDPaolo Bonzini1-1/+1
EPROTO is not found in OpenBSD. We usually use EIO when no better errno is available, do that here too. Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20170317152412.8472-1-pbonzini@redhat.com Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-03-17block: Always call bdrv_child_check_perm firstFam Zheng1-2/+4
bdrv_child_set_perm alone is not very usable because the caller must call bdrv_child_check_perm first. This is already encapsulated conveniently in bdrv_child_try_set_perm, so remove the other prototypes from the header and fix the one wrong caller, block/mirror.c. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-17file-posix: Don't leak fd in hdev_get_max_segmentsFam Zheng1-0/+3
This fixes a leaked fd introduced in commit 9103f1ce. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-17replication: clarify permissionsChanglong Xie1-1/+13
Even if hidden_disk, secondary_disk are backing files, they all need write permissions in replication scenario. Otherwise we will encouter below exceptions on secondary side during adding nbd server: {'execute': 'nbd-server-add', 'arguments': {'device': 'colo-disk', 'writable': true } } {"error": {"class": "GenericError", "desc": "Conflicts with use by hidden-qcow2-driver as 'backing', which does not allow 'write' on sec-qcow2-driver-for-nbd"}} CC: Zhang Hailiang <zhang.zhanghailiang@huawei.com> CC: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> CC: Wen Congyang <wencongyang2@huawei.com> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-17file-posix: clean up max_segments buffer terminationStefan Hajnoczi1-1/+1
The following pattern is unsafe: char buf[32]; ret = read(fd, buf, sizeof(buf)); ... buf[ret] = 0; If read(2) returns 32 then a byte beyond the end of the buffer is zeroed. In practice this buffer overflow does not occur because the sysfs max_segments file only contains an unsigned short + '\n'. The string is always shorter than 32 bytes. Regardless, avoid this pattern because static analysis tools might complain and it could lead to real buffer overflows if copy-pasted elsewhere in the codebase. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-13commit: Implement .bdrv_refresh_filenameKevin Wolf1-0/+8
We want query-block to return the right filename, even if a commit job put a bdrv_commit_top on top of the actual image format driver. Let bdrv_commit_top.bdrv_refresh_filename get the filename from its backing file. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2017-03-13mirror: Implement .bdrv_refresh_filenameKevin Wolf1-0/+9
We want query-block to return the right filename, even if a mirror job put a bdrv_mirror_top on top of the actual image format driver. Let bdrv_mirror_top.bdrv_refresh_filename get the filename from its backing file. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2017-03-13commit: Implement bdrv_commit_top.bdrv_co_get_block_statusKevin Wolf1-4/+16
In some cases, bdrv_co_get_block_status() is called recursively for the whole backing chain. The automatically inserted bdrv_commit_top filter driver must not stop the recursion, so implement a callback that simply forwards the request to bs->backing. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2017-03-13block: Request block status from *file for BDRV_BLOCK_RAWKevin Wolf1-1/+1
This fixes bdrv_co_get_block_status() for the bdrv_mirror_top block driver, which must fall through to bs->backing instead of bs->file. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2017-03-13vvfat: React to bdrv_is_allocated() errorsEric Blake1-3/+19
If bdrv_is_allocated() fails, we should react to that failure. For 2 of the 3 callers, reporting the error was easy. But in cluster_was_modified() and its lone caller get_cluster_count_for_direntry(), it's rather invasive to update the logic to pass the error back; so there, I went with merely documenting the issue by changing the return type to bool (in all likelihood, treating the cluster as modified will then trigger a read which will also fail, and eventually get to an error - but given the appalling number of abort() calls in this code, I'm not making it any worse). Signed-off-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-13backup: React to bdrv_is_allocated() errorsEric Blake1-4/+10
If bdrv_is_allocated() fails, we should immediately do the backup error action, rather than attempting backup_do_cow() (although that will likely fail too). Signed-off-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-13block: Drop unmaintained 'archipelago' driverEric Blake2-1081/+0
The driver has failed to build since commit da34e65, in qemu 2.6, due to a missing include of qapi/error.h for error_setg(). Since no one has complained in three releases, it is easier to remove the dead code than to keep it around, especially since it is not being built by default and therefore prone to bitrot. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-13file-posix: Consider max_segments for BlockLimits.max_transferFam Zheng1-0/+47
BlockLimits.max_transfer can be too high without this fix, guest will encounter I/O error or even get paused with werror=stop or rerror=stop. The cause is explained below. Linux has a separate limit, /sys/block/.../queue/max_segments, which in the worst case can be more restrictive than the BLKSECTGET which we already consider (note that they are two different things). So, the failure scenario before this patch is: 1) host device has max_sectors_kb = 4096 and max_segments = 64; 2) guest learns max_sectors_kb limit from QEMU, but doesn't know max_segments; 3) guest issues e.g. a 512KB request thinking it's okay, but actually it's not, because it will be passed through to host device as an SG_IO req that has niov > 64; 4) host kernel doesn't like the segmenting of the request, and returns -EINVAL; This patch checks the max_segments sysfs entry for the host device and calculates a "conservative" bytes limit using the page size, which is then merged into the existing max_transfer limit. Guest will discover this from the usual virtual block device interfaces. (In the case of scsi-generic, it will be done in the INQUIRY reply interception in device model.) The other possibility is to actually propagate it as a separate limit, but it's not better. On the one hand, there is a big complication: the limit is per-LUN in QEMU PoV (because we can attach LUNs from different host HBAs to the same virtio-scsi bus), but the channel to communicate it in a per-LUN manner is missing down the stack; on the other hand, two limits versus one doesn't change much about the valid size of I/O (because guest has no control over host segmenting). Also, the idea to fall back to bounce buffering in QEMU, upon -EINVAL, was explored. Unfortunately there is no neat way to ensure the bounce buffer is less segmented (in terms of DMA addr) than the guest buffer. Practically, this bug is not very common. It is only reported on a Emulex (lpfc), so it's okay to get it fixed in the easier way. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-13backup: allow target without .bdrv_get_infoVladimir Sementsov-Ogievskiy1-1/+11
Currently backup to nbd target is broken, as nbd doesn't have .bdrv_get_info realization. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-07commit: Don't use error_abort in commit_startFam Zheng1-2/+14
bdrv_set_backing_hd failure needn't be abort. Since we already have error parameter, use it. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-07block: Don't use error_abort in blk_new_openFam Zheng1-1/+6
We have an errp and bdrv_root_attach_child can fail permission check, error_abort is not the best choice here. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-07qapi-schema: Rename SocketAddressFlat's variant tcp to inetMarkus Armbruster1-28/+31
QAPI type SocketAddressFlat differs from SocketAddress pointlessly: the discriminator value for variant InetSocketAddress is 'tcp' instead of 'inet'. Rename. The type is so far only used by the Gluster block drivers. Take care to keep 'tcp' working in things like -drive's file.server.0.type=tcp. The "gluster+tcp" URI scheme in pseudo-filenames stays the same. blockdev-add changes, but it has changed incompatibly since 2.8 already. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-07qapi-schema: Rename GlusterServer to SocketAddressFlatMarkus Armbruster1-19/+19
As its documentation says, it's not specific to Gluster. Rename it, as I'm going to use it for something else. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-03-07gluster: Plug memory leaks in qemu_gluster_parse_json()Markus Armbruster1-11/+11
To reproduce, run $ valgrind qemu-system-x86_64 --nodefaults -S --drive driver=gluster,volume=testvol,path=/a/b/c,server.0.type=xxx Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Niels de Vos <ndevos@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>