[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v12 14/16] file-posix: Implement image locking
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v12 14/16] file-posix: Implement image locking |
Date: |
Wed, 8 Feb 2017 14:18:41 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 08.02.2017 07:00, Fam Zheng wrote:
> On Wed, 02/08 04:05, Max Reitz wrote:
>>> +static int raw_lt_write_to_write_share(RawLockTransOp op,
>>> + int old_lock_fd, int new_lock_fd,
>>> + BDRVRawLockMode old_lock,
>>> + BDRVRawLockMode new_lock,
>>> + Error **errp)
>>> +{
>>> + int ret = 0;
>>> +
>>> + assert(old_lock == RAW_L_WRITE);
>>> + assert(new_lock == RAW_L_WRITE_SHARE_RW);
>>> + /*
>>> + * lock byte "no other writer" lock byte "write"
>>> + * old X X
>>> + * new 0 S
>>> + *
>>> + * (0 = unlocked; S = shared; X = exclusive.)
>>> + */
>>> + switch (op) {
>>> + case RAW_LT_PREPARE:
>>> + break;
>>> + case RAW_LT_COMMIT:
>>> + ret = qemu_lock_fd(old_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
>>> + if (ret) {
>>> + error_report("Failed to downgrade old fd (share byte)");
>>> + break;
>>> + }
>>> + ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
>>> + if (ret) {
>>> + error_report("Failed to unlock new fd (share byte)");
>>> + break;
>>> + }
>>
>> The second one is not an "unlock", but a new shared lock.
>
> You are right.
>
>> Which brings
>> me to the point that both of these commands can fail and thus should be
>> in the prepare path.
>
> We cannot. If we lose the exclusive lock already in prepare, and some other
> things fail later in the transaction, abort() may not be able to restore that
> lock (another process took a shared lock in between).
>
> The reason for my code is, the lock semantics implies both of these commands
> can
> succeed, so it doesn't hurt if we ignore ret codes here. I'm just trying to
> catch the very unlikely abnormalities.
Indeed. Well, then raw_lt_write_to_read() should do the same, though.
Max
>> (This function should be a mirror of raw_lt_write_to_read, if I'm not
>> mistaken.)
>>
>>> + break;
>>> + case RAW_LT_ABORT:
>>> + break;
>>> + }
>>> + return ret;
>>> +}
>
> Fam
>
signature.asc
Description: OpenPGP digital signature