Re: locking option doesn't work as expected

[Masayoshi Mizuma]

On Wed, Nov 04, 2020 at 04:48:53PM +0000, Daniel P. Berrangé wrote:
> On Wed, Nov 04, 2020 at 11:35:56AM -0500, Masayoshi Mizuma wrote:
> > Hello,
> > 
> > It seems that locking option doesn't work as expected.
> > When I run qemu as following options, then I got an error and failed to
> > boot the guest:
> > 
> >   qemu-system-x86_64 \
> >     -machine pc \
> >     -enable-kvm \
> >     -cpu host \
> >     -smp 1 -m 4G \
> >     -nographic \
> >     -serial telnet::1235,server,nowait \
> >     -blockdev 
> > driver=qcow2,node-name=disk,file.driver=file,file.filename=/mnt/guest.qcow2,file.locking=auto
> >  \
> >     -device virtio-blk-pci,scsi=off,drive=disk,id=virtio-disk0,bootindex=1
> > 
> >   qemu-system-x86_64: -blockdev 
> > driver=qcow2,node-name=disk,file.driver=file,file.filename=/mnt/guest.qcow2,file.locking=auto:
> >  Failed to lock byte 100
> > 
> > The error happens when the filesystem doesn't support OFD lock.
> > qemu_probe_lock_ops() judges whether qemu can use OFD lock or not with doing
> > fcntl(F_OFD_GETLK) to /dev/null, so the error happens if /dev/null supports 
> > OFD lock,
> > but the filesystem doesn't support the lock.
> > 
> > I'm thinking how to fix the error. My idea is to add locking=posix option 
> > to use posix
> > lock to the file, but I'm not sure the idea is good way to fix the error...
> > 
> > I would appreciate it if you could give me some advises to fix the error.
> 
> Ideally we would not attempt to probe it on /dev/null at all. Instead just
> attempt to lock the actual file using F_OFD_SETLK and if that fails, then
> falback to F_SETLK.  We can do similar when checking whether the lock
> currently exists. AFAIK, the locking code isn't so performance critical
> that the extra syscall would hurt us in the case where OFD doesn't exist
> or isn't supported by the FS.

Thank you for your comments! I'll fix the error to lock the file using
F_OFD_SETLK and if that fails, then falback to F_SETLK.

Thanks!
Masa

> 
> > 
> > BTW, locking=off may be useful for the workaround so far, however, 
> > locking=off
> > doesn't work on the splitting blockdev configs as followings...
> > I split the blockdev option as libvirt doing.
> > 
> >    -blockdev 
> > driver=file,filename=/mnt/guest.qcow2,node-name=storage,auto-read-only=on,locking=off
> >  \
> >    -blockdev node-name=format,read-only=off,driver=qcow2,file=storage \
> > 
> > Thanks,
> > Masa
> > 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>