From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Fabian Ebner <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [PATCH v2 qemu-server 1/2] tests: mock storage locking for migration tests
Date: Tue, 12 Jan 2021 12:07:34 +0100 [thread overview]
Message-ID: <85d4649e-faaa-3cbb-3b72-f5cb3caa006b@proxmox.com> (raw)
In-Reply-To: <c573add7-5db1-d3f5-2deb-d2da3e99768d@proxmox.com>
first, I overlooked your v2 due to lack of reply to Stefans comment there I did
not actually thought there would come one, but my followup seems mostly in line
with your patch here, so no real harm done..
On 12.01.21 09:03, Fabian Ebner wrote:
> I didn't notice yesterday, but it's actually strange that volume_is_base_and_used uses a storage lock. Its callers that plan to change volumes on the storage based on the check need to hold the lock instead. Otherwise it can happen that:
> 1. volume_is_base_and_used is called and the result is used to decide how to branch
> 2. situation on the storage changes in the meantime
> 3. the branch we are taking might not be the one we should be taking anymore
>
> vdisk_free already uses its own lock around both the __no_lock-variant of the check and the modification on the storage it does, so it's fine.
>
> The only two callers for the normal variant are in qemu-server and they both serve as preliminary checks, while the real modification for both of them happens with vdisk_free. One of the callers makes the mocking below necessary, but it wouldn't if we were to remove the storage locking from volume_is_base_and_used.
Sounds sensible without looking into it in depth yet, can you come up with a patch
to do so and look at the specifics?
next prev parent reply other threads:[~2021-01-12 11:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-11 14:00 Fabian Ebner
2021-01-11 14:00 ` [pve-devel] [PATCH v2 qemu-server 2/2] tests: remove migration test run dir with make clean Fabian Ebner
2021-01-12 8:03 ` [pve-devel] [PATCH v2 qemu-server 1/2] tests: mock storage locking for migration tests Fabian Ebner
2021-01-12 11:07 ` Thomas Lamprecht [this message]
2021-01-15 7:47 ` Fabian Ebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=85d4649e-faaa-3cbb-3b72-f5cb3caa006b@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=f.ebner@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.