aboutsummaryrefslogtreecommitdiff
path: root/block.c
AgeCommit message (Collapse)Author
2021-06-02block: improve permission conflict error messageVladimir Sementsov-Ogievskiy
Now permissions are updated as follows: 1. do graph modifications ignoring permissions 2. do permission update (of course, we rollback [1] if [2] fails) So, on stage [2] we can't say which users are "old" and which are "new" and exist only since [1]. And current error message is a bit outdated. Let's improve it, to make everything clean. While being here, add also a comment and some good assertions. iotests 283, 307, qsd-jobs outputs are updated. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210601075218.79249-7-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-06-02block: simplify bdrv_child_user_desc()Vladimir Sementsov-Ogievskiy
All child classes have this callback. So, drop unreachable code. Still add an assertion to bdrv_attach_child_common(), to early detect bad classes. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210601075218.79249-6-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-06-02block: improve bdrv_child_get_parent_desc()Vladimir Sementsov-Ogievskiy
We have different types of parents: block nodes, block backends and jobs. So, it makes sense to specify type together with name. Next, this handler us used to compose an error message about permission conflict. And permission conflict occurs in a specific place of block graph. We shouldn't report name of parent device (as it refers another place in block graph), but exactly and only the name of the node. So, use bdrv_get_node_name() directly. iotest 283 output is updated. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20210601075218.79249-4-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-06-02block: document child argument of bdrv_attach_child_common()Vladimir Sementsov-Ogievskiy
The logic around **child is not obvious: this reference is used not only to return resulting child, but also to rollback NULL value on transaction abort. So, let's add documentation and some assertions. While being here, drop extra declaration of bdrv_attach_child_noperm(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210601075218.79249-2-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-06-02block: drop BlockDriverState::read_onlyVladimir Sementsov-Ogievskiy
This variable is just a cache for !(bs->open_flags & BDRV_O_RDWR), which we have to synchronize everywhere. Let's just drop it and consistently use bdrv_is_read_only(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210527154056.70294-3-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-06-02block: consistently use bdrv_is_read_only()Vladimir Sementsov-Ogievskiy
It's better to use accessor function instead of bs->read_only directly. In some places use bdrv_is_writable() instead of checking both BDRV_O_RDWR set and BDRV_O_INACTIVE not set. In bdrv_open_common() it's a bit strange to add one more variable, but we are going to drop bs->read_only in the next patch, so new ro local variable substitutes it here. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210527154056.70294-2-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-06-02block/vvfat: child_vvfat_qcow: add .get_parent_aio_context, fix crashVladimir Sementsov-Ogievskiy
Commit 3ca1f3225727419ba573673b744edac10904276f "block: BdrvChildClass: add .get_parent_aio_context handler" introduced new handler and commit 228ca37e12f97788e05bd0c92f89b3e5e4019607 "block: drop ctx argument from bdrv_root_attach_child" made a generic use of it. But 3ca1f3225727419ba573673b744edac10904276f didn't update child_vvfat_qcow. Fix that. Before that fix the command ./build/qemu-system-x86_64 -usb -device usb-storage,drive=fat16 \ -drive file=fat:rw:fat-type=16:"<path of a host folder>",id=fat16,format=raw,if=none crashes: 1 bdrv_child_get_parent_aio_context (c=0x559d62426d20) at ../block.c:1440 2 bdrv_attach_child_common (child_bs=0x559d62468190, child_name=0x559d606f9e3d "write-target", child_class=0x559d60c58d20 <child_vvfat_qcow>, child_role=3, perm=3, shared_perm=4, opaque=0x559d62445690, child=0x7ffc74c2acc8, tran=0x559d6246ddd0, errp=0x7ffc74c2ae60) at ../block.c:2795 3 bdrv_attach_child_noperm (parent_bs=0x559d62445690, child_bs=0x559d62468190, child_name=0x559d606f9e3d "write-target", child_class=0x559d60c58d20 <child_vvfat_qcow>, child_role=3, child=0x7ffc74c2acc8, tran=0x559d6246ddd0, errp=0x7ffc74c2ae60) at ../block.c:2855 4 bdrv_attach_child (parent_bs=0x559d62445690, child_bs=0x559d62468190, child_name=0x559d606f9e3d "write-target", child_class=0x559d60c58d20 <child_vvfat_qcow>, child_role=3, errp=0x7ffc74c2ae60) at ../block.c:2953 5 bdrv_open_child (filename=0x559d62464b80 "/var/tmp/vl.h3TIS4", options=0x559d6246ec20, bdref_key=0x559d606f9e3d "write-target", parent=0x559d62445690, child_class=0x559d60c58d20 <child_vvfat_qcow>, child_role=3, allow_none=false, errp=0x7ffc74c2ae60) at ../block.c:3351 6 enable_write_target (bs=0x559d62445690, errp=0x7ffc74c2ae60) at ../block/vvfat.c:3176 7 vvfat_open (bs=0x559d62445690, options=0x559d6244adb0, flags=155650, errp=0x7ffc74c2ae60) at ../block/vvfat.c:1236 8 bdrv_open_driver (bs=0x559d62445690, drv=0x559d60d4f7e0 <bdrv_vvfat>, node_name=0x0, options=0x559d6244adb0, open_flags=155650, errp=0x7ffc74c2af70) at ../block.c:1557 9 bdrv_open_common (bs=0x559d62445690, file=0x0, options=0x559d6244adb0, errp=0x7ffc74c2af70) at ... (gdb) fr 1 #1 0x0000559d603ea3bf in bdrv_child_get_parent_aio_context (c=0x559d62426d20) at ../block.c:1440 1440 return c->klass->get_parent_aio_context(c); (gdb) p c->klass $1 = (const BdrvChildClass *) 0x559d60c58d20 <child_vvfat_qcow> (gdb) p c->klass->get_parent_aio_context $2 = (AioContext *(*)(BdrvChild *)) 0x0 Fixes: 3ca1f3225727419ba573673b744edac10904276f Fixes: 228ca37e12f97788e05bd0c92f89b3e5e4019607 Reported-by: John Arbuckle <programmingkidx@gmail.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210524101257.119377-2-vsementsov@virtuozzo.com> Tested-by: John Arbuckle <programmingkidx@gmail.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-05-18block: Fix Transaction leak in bdrv_reopen_multiple()Kevin Wolf
Like other error paths, this one needs to call tran_finalize() and clean up the BlockReopenQueue, too. Fixes: CID 1452772 Fixes: 72373e40fbc7e4218061a8211384db362d3e7348 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210503110555.24001-3-kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-05-18block: Fix Transaction leak in bdrv_root_attach_child()Kevin Wolf
The error path needs to call tran_finalize(), too. Fixes: CID 1452773 Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210503110555.24001-2-kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-05-14block: drop write notifiersVladimir Sementsov-Ogievskiy
They are unused now. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20210506090621.11848-3-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2021-05-02Do not include sysemu/sysemu.h if it's not really necessaryThomas Huth
Stop including sysemu/sysemu.h in files that don't need it. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20210416171314.2074665-2-thuth@redhat.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
2021-04-30block: refactor bdrv_node_check_perm()Vladimir Sementsov-Ogievskiy
Now, bdrv_node_check_perm() is called only with fresh cumulative permissions, so its actually "refresh_perm". Move permission calculation to the function. Also, drop unreachable error message and rewrite the remaining one to be more generic (as now we don't know which node is added and which was already here). Add also Virtuozzo copyright, as big work is done at this point. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-37-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: rename bdrv_replace_child_safe() to bdrv_replace_child()Vladimir Sementsov-Ogievskiy
We don't have bdrv_replace_child(), so it's time for bdrv_replace_child_safe() to take its place. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-36-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: refactor bdrv_child_set_perm_safe() transaction actionVladimir Sementsov-Ogievskiy
Old interfaces dropped, nobody directly calls bdrv_child_set_perm_abort() and bdrv_child_set_perm_commit(), so we can use personal state structure for the action and stop exploiting BdrvChild structure. Also, drop "_safe" suffix which is redundant now. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-35-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: inline bdrv_replace_child()Vladimir Sementsov-Ogievskiy
bdrv_replace_child() has only one caller, the second argument is unused. Inline it now. This triggers deletion of some more unused interfaces. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-34-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: inline bdrv_check_perm_common()Vladimir Sementsov-Ogievskiy
bdrv_check_perm_common() has only one caller, so no more sense in "common". Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-33-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: drop unused permission update functionsVladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-32-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: bdrv_reopen_multiple: refresh permissions on updated graphVladimir Sementsov-Ogievskiy
Move bdrv_reopen_multiple to new paradigm of permission update: first update graph relations, then do refresh the permissions. We have to modify reopen process in file-posix driver: with new scheme we don't have prepared permissions in raw_reopen_prepare(), so we should reconfigure fd in raw_check_perm(). Still this seems more native and simple anyway. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-31-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepareVladimir Sementsov-Ogievskiy
During reopen we may add backing bs from other aio context, which may lead to changing original context of top bs. We are going to move graph modification to prepare stage. So, it will be possible that bdrv_flush() in bdrv_reopen_prepare called on bs in non-original aio context, which we didn't aquire which leads to crash. To avoid this problem move bdrv_flush() to be a separate reopen stage before bdrv_reopen_prepare(). This doesn't seem correct to acquire only one aio context and not all contexts participating in reopen. But it's not obvious how to do it correctly, keeping in mind: 1. rules of bdrv_set_aio_context_ignore() that requires new_context lock not being held 2. possible deadlocks because of holding all (or several?) AioContext locks Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-30-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: add bdrv_set_backing_noperm() transaction actionVladimir Sementsov-Ogievskiy
Split out no-perm part of bdrv_set_backing_hd() as a separate transaction action. Note the in case of existing BdrvChild we reuse it, not recreate, just to do less actions. We don't need to create extra reference to backing_hd as we don't lose it in bdrv_attach_child(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-29-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: make bdrv_refresh_limits() to be a transaction actionVladimir Sementsov-Ogievskiy
To be used in further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-28-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: make bdrv_unset_inherits_from to be a transaction actionVladimir Sementsov-Ogievskiy
To be used in the further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-27-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: drop ignore_children for permission update functionsVladimir Sementsov-Ogievskiy
This argument is always NULL. Drop it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-26-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: introduce bdrv_drop_filter()Vladimir Sementsov-Ogievskiy
Using bdrv_replace_node() for removing filter is not good enough: it keeps child reference of the filter, which may conflict with original top node during permission update. Instead let's create new interface, which will do all graph modifications first and then update permissions. Let's modify bdrv_replace_node_common(), allowing it additionally drop backing chain child link pointing to new node. This is quite appropriate for bdrv_drop_intermediate() and makes possible to add new bdrv_drop_filter() as a simple wrapper. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-24-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: add bdrv_remove_filter_or_cow transaction actionVladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-23-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: adapt bdrv_append() for inserting filtersVladimir Sementsov-Ogievskiy
bdrv_append is not very good for inserting filters: it does extra permission update as part of bdrv_set_backing_hd(). During this update filter may conflict with other parents of top_bs. Instead, let's first do all graph modifications and after it update permissions. append-greedy-filter test-case in test-bdrv-graph-mod is now works, so move it out of debug option. Note: bdrv_append() is still only works for backing-child based filters. It's something to improve later. Note2: we use the fact that bdrv_append() is used to append new nodes, without backing child, so we don't need frozen check and inherits_from logic from bdrv_set_backing_hd(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-22-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: split out bdrv_replace_node_noperm()Vladimir Sementsov-Ogievskiy
Split part of bdrv_replace_node_common() to be used separately. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-21-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: add bdrv_attach_child_noperm() transaction actionVladimir Sementsov-Ogievskiy
Split no-perm part of bdrv_attach_child as separate transaction action. It will be used in later commits. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-20-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: add bdrv_attach_child_common() transaction actionVladimir Sementsov-Ogievskiy
Split out no-perm part of bdrv_root_attach_child() into separate transaction action. bdrv_root_attach_child() now moves to new permission update paradigm: first update graph relations then update permissions. qsd-jobs test output updated. Seems now permission update goes in another order. Still, the test comment say that we only want to check that command doesn't crash, and it's still so. Error message is a bit misleading as it looks like job was added first. But actually in new paradigm of graph update we can't distinguish such things. We should update the error message, but let's not do it now. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210428151804.439460-19-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: fix bdrv_replace_node_commonVladimir Sementsov-Ogievskiy
inore_children thing doesn't help to track all propagated permissions of children we want to ignore. The simplest way to correctly update permissions is update graph first and then do permission update. In this case we just referesh permissions for the whole subgraph (in topological-sort defined order) and everything is correctly calculated automatically without any ignore_children. So, refactor bdrv_replace_node_common to first do graph update and then refresh the permissions. Test test_parallel_exclusive_write() now pass, so move it out of debugging "if". Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-18-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: add bdrv_replace_child_safe() transaction actionVladimir Sementsov-Ogievskiy
To be used in the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-17-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: add bdrv_list_* permission update functionsVladimir Sementsov-Ogievskiy
Add new interface, allowing use of existing node list. It will be used to fix bdrv_replace_node() in the further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-16-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: add bdrv_drv_set_perm transaction actionVladimir Sementsov-Ogievskiy
Refactor calling driver callbacks to a separate transaction action to be used later. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-15-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: use topological sort for permission updateVladimir Sementsov-Ogievskiy
Rewrite bdrv_check_perm(), bdrv_abort_perm_update() and bdrv_set_perm() to update nodes in topological sort order instead of simple DFS. With topologically sorted nodes, we update a node only when all its parents already updated. With DFS it's not so. Consider the following example: A -+ | | | v | B | | v | C<-+ A is parent for B and C, B is parent for C. Obviously, to update permissions, we should go in order A B C, so, when we update C, all parent permissions already updated. But with current approach (simple recursion) we can update in sequence A C B C (C is updated twice). On first update of C, we consider old B permissions, so doing wrong thing. If it succeed, all is OK, on second C update we will finish with correct graph. But if the wrong thing failed, we break the whole process for no reason (it's possible that updated B permission will be less strict, but we will never check it). Also new approach gives a way to simultaneously and correctly update several nodes, we just need to run bdrv_topological_dfs() several times to add all nodes and their subtrees into one topologically sorted list (next patch will update bdrv_replace_node() in this manner). Test test_parallel_perm_update() is now passing, so move it out of debugging "if". We also need to support ignore_children in bdrv_parent_perms_conflict() For test 283 order of conflicting parents check is changed. Note also that in bdrv_check_perm() we don't check for parents conflict at root bs, as we may be in the middle of permission update in bdrv_reopen_multiple(). bdrv_reopen_multiple() will be updated soon. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-14-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: inline bdrv_child_*() permission functions callsVladimir Sementsov-Ogievskiy
Each of them has only one caller. Open-coding simplifies further pemission-update system changes. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-13-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms()Vladimir Sementsov-Ogievskiy
We are going to drop recursive bdrv_child_* functions, so stop use them in bdrv_child_try_set_perm() as a first step. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-12-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: refactor bdrv_child* permission functionsVladimir Sementsov-Ogievskiy
Split out non-recursive parts, and refactor as block graph transaction action. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-11-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: bdrv_refresh_perms: check for parents permissions conflictVladimir Sementsov-Ogievskiy
Add additional check that node parents do not interfere with each other. This should not hurt existing callers and allows in further patch use bdrv_refresh_perms() to update a subtree of changed BdrvChild (check that change is correct). New check will substitute bdrv_check_update_perm() in following permissions refactoring, so keep error messages the same to avoid unit test result changes. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-10-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: make bdrv_reopen_{prepare,commit,abort} privateVladimir Sementsov-Ogievskiy
These functions are called only from bdrv_reopen_multiple() in block.c. No reason to publish them. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-8-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: drop ctx argument from bdrv_root_attach_childVladimir Sementsov-Ogievskiy
Passing parent aio context is redundant, as child_class and parent opaque pointer are enough to retrieve it. Drop the argument and use new bdrv_child_get_parent_aio_context() interface. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-7-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: BdrvChildClass: add .get_parent_aio_context handlerVladimir Sementsov-Ogievskiy
Add new handler to get aio context and implement it in all child classes. Add corresponding public interface to be used soon. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-6-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-04-30block: bdrv_append(): don't consume referenceVladimir Sementsov-Ogievskiy
We have too much comments for this feature. It seems better just don't do it. Most of real users (tests don't count) have to create additional reference. Drop also comment in external_snapshot_prepare: - bdrv_append doesn't "remove" old bs in common sense, it sounds strange - the fact that bdrv_append can fail is obvious from the context - the fact that we must rollback all changes in transaction abort is known (it's the direct role of abort) Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-5-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-03-19block: remove format defaults from QemuOpts in bdrv_create_file()Stefano Garzarella
QemuOpts is usually created merging the QemuOptsList of format and protocol. So, when the format calls bdr_create_file(), the 'opts' parameter contains a QemuOptsList with a combination of format and protocol default values. The format properly removes its options before calling bdr_create_file(), but the default values remain in 'opts->list'. So if the protocol has options with the same name (e.g. rbd has 'cluster_size' as qcow2), it will see the default values of the format, since for overlapping options, the format wins. To avoid this issue, lets convert QemuOpts to QDict, in this way we take only the set options, and then convert it back to QemuOpts, using the 'create_opts' of the protocol. So the new QemuOpts, will contain only the protocol defaults. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-Id: <20210308161232.248833-1-sgarzare@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-03-11Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2021-03-09' into ↵Peter Maydell
staging nbd patches for 2021-03-09 - Add Vladimir as NBD co-maintainer - Fix reporting of holes in NBD_CMD_BLOCK_STATUS - Improve command-line parsing accuracy of large numbers (anything going through qemu_strtosz), including the deprecation of hex+suffix - Improve some error reporting in the block layer # gpg: Signature made Tue 09 Mar 2021 15:38:10 GMT # gpg: using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A # gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full] # gpg: aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" [full] # gpg: aka "[jpeg image of size 6874]" [full] # Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2 F3AA A7A1 6B4A 2527 436A * remotes/ericb/tags/pull-nbd-2021-03-09: block/qcow2: refactor qcow2_update_options_prepare error paths block/qed: bdrv_qed_do_open: deal with errp block/qcow2: simplify qcow2_co_invalidate_cache() block/qcow2: read_cache_sizes: return status value block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface block/qcow2: qcow2_get_specific_info(): drop error propagation blockjob: return status from block_job_set_speed() block/mirror: drop extra error propagation in commit_active_start() block: drop extra error propagation for bdrv_set_backing_hd blockdev: fix drive_backup_prepare() missed error block: check return value of bdrv_open_child and drop error propagation utils: Deprecate hex-with-suffix sizes utils: Improve qemu_strtosz() to have 64 bits of precision utils: Enhance testsuite for do_strtosz() nbd: server: Report holes for raw images MAINTAINERS: add Vladimir as co-maintainer of NBD Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2021-03-08block: drop extra error propagation for bdrv_set_backing_hdVladimir Sementsov-Ogievskiy
bdrv_set_backing_hd now returns status, let's use it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20210202124956.63146-6-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2021-03-08block: Clarify error messages pertaining to 'node-name'Connor Kuehl
Some error messages contain ambiguous representations of the 'node-name' parameter. This can be particularly confusing when exchanging QMP messages (C = client, S = server): C: {"execute": "block_resize", "arguments": { "device": "my_file", "size": 26843545600 }} S: {"error": {"class": "GenericError", "desc": "Cannot find device=my_file nor node_name="}} ^^^^^^^^^ This error message suggests one could send a message with a key called 'node_name': C: {"execute": "block_resize", "arguments": { "node_name": "my_file", "size": 26843545600 }} ^^^^^^^^^ but using the underscore is actually incorrect, the parameter should be 'node-name': S: {"error": {"class": "GenericError", "desc": "Parameter 'node_name' is unexpected"}} This behavior was uncovered in bz1651437, but I ended up going down a rabbit hole looking for other areas where this miscommunication might occur and changing those accordingly as well. Fixes: https://bugzilla.redhat.com/1651437 Signed-off-by: Connor Kuehl <ckuehl@redhat.com> Message-Id: <20210305151929.1947331-2-ckuehl@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-02-15block: add bdrv_co_delete_file_noerrMaxim Levitsky
This function wraps bdrv_co_delete_file for the common case of removing a file, which was just created by format driver, on an error condition. It hides the -ENOTSUPP error, and reports all other errors otherwise. Use it in luks driver Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20201217170904.946013-3-mlevitsk@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-02-12block: use return status of bdrv_append()Vladimir Sementsov-Ogievskiy
Now bdrv_append returns status and we can drop all the local_err things around it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20210202124956.63146-3-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2021-02-12block: return status from bdrv_append and friendsVladimir Sementsov-Ogievskiy
The recommended use of qemu error api assumes returning status together with setting errp and avoid void functions with errp parameter. Let's improve bdrv_append and some friends to reduce error-propagation overhead in further patches. Choose int return status, because bdrv_replace_node_common() has call to bdrv_check_update_perm(), which reports int status, which seems correct to propagate. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210202124956.63146-2-vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2021-02-02block: move blk_exp_close_all() to qemu_cleanup()Sergio Lopez
Move blk_exp_close_all() from bdrv_close() to qemu_cleanup(), before bdrv_drain_all_begin(). Export drivers may have coroutines yielding at some point in the block layer, so we need to shut them down before draining the block layer, as otherwise they may get stuck blk_wait_while_drained(). RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505 Signed-off-by: Sergio Lopez <slp@redhat.com> Message-Id: <20210201125032.44713-3-slp@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-25 07:30:19 +0000