locking option doesn't work as expected

locking option doesn't work as expected

[Masayoshi Mizuma]

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.

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

Re: locking option doesn't work as expected

[Daniel P. Berrangé]

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.

> 
> 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 :|

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 :|
>