[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v8 12/36] virtio-blk: Apply lock-mode when reali
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v8 12/36] virtio-blk: Apply lock-mode when realize |
Date: |
Sat, 22 Oct 2016 02:12:39 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 22.10.2016 02:08, Max Reitz wrote:
> On 30.09.2016 14:09, Fam Zheng wrote:
>> Signed-off-by: Fam Zheng <address@hidden>
>> ---
>> hw/block/virtio-blk.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
>> index 3a6112f..ce65615 100644
>> --- a/hw/block/virtio-blk.c
>> +++ b/hw/block/virtio-blk.c
>> @@ -896,6 +896,11 @@ static void virtio_blk_device_realize(DeviceState *dev,
>> Error **errp)
>> error_setg(errp, "num-queues property must be larger than 0");
>> return;
>> }
>> + blk_lock_image(conf->conf.blk, conf->conf.lock_mode, &err);
>> + if (err) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>>
>> blkconf_serial(&conf->conf, &conf->serial);
>> blkconf_apply_backend_options(&conf->conf);
>>
>
> Hmmm... Patch 3 introduced the conf.lock_mode field, but didn't do
> anything with it. That is a bit weird. Now you're applying it here but
> can't set it anywhere. That's a bit weird, too. Well, behavior won't
> change as this probably just means that now everything explicitly
> unlocked instead of implicitly "because we don't have locking yet".
>
> Maybe it would make sense to move the introduction of conf.lock_mode to
> an own patch and explain in the commit message that this field will be
> implicitly set to 0, so until the user is able to control the field,
> every BB (and thus BDS tree) will not be locked (thus not changing
> behavior).
Oops, just noticed that it won't be "unlocked" but "auto-locked" (all
the more reason to mention the implicit behavior somewhere). But maybe
my comment for patch 15 has obsoleted this comment altogether.
Max
signature.asc
Description: OpenPGP digital signature