On 22.10.2016 02:08, Max Reitz wrote: > On 30.09.2016 14:09, Fam Zheng wrote: >> Signed-off-by: Fam Zheng <f...@redhat.com> >> --- >> 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