* [Qemu-devel] Disable image locking for snapshot drive?
@ 2017-07-17 23:30 Andrew Baumann
2017-07-18 0:33 ` John Snow
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Baumann @ 2017-07-17 23:30 UTC (permalink / raw)
To: qemu-devel; +Cc: famz
Hi all,
I'm running a recent Linux build of qemu on Windows Subsystem for Linux (WSL) which doesn't appear to implement file locking:
$ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device virtio-blk-pci,drive=hd0
qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock byte 100
qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock byte 100
qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to lock byte 100
That's no big deal; I can switch it off:
$ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,file.locking=off,id=hd0 ...
(all good)
But how can I do the same for a snapshot drive?
$ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,file.locking=off,id=hd0 -snapshot ...
qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: Could not create temporary overlay '/var/tmp/vl.o83dxn': Failed to lock byte 100
(I also tried the snapshot=on drive option with similar results.)
Thanks,
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Disable image locking for snapshot drive?
2017-07-17 23:30 [Qemu-devel] Disable image locking for snapshot drive? Andrew Baumann
@ 2017-07-18 0:33 ` John Snow
2017-07-18 2:19 ` Andrew Baumann
2017-07-18 15:07 ` Eric Blake
0 siblings, 2 replies; 8+ messages in thread
From: John Snow @ 2017-07-18 0:33 UTC (permalink / raw)
To: Andrew Baumann, qemu-devel; +Cc: famz
On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> Hi all,
>
> I'm running a recent Linux build of qemu on Windows Subsystem for Linux (WSL) which doesn't appear to implement file locking:
>
> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device virtio-blk-pci,drive=hd0
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to lock byte 100
>
> That's no big deal; I can switch it off:
>
> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,file.locking=off,id=hd0 ...
> (all good)
>
> But how can I do the same for a snapshot drive?
>
> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,file.locking=off,id=hd0 -snapshot ...
> qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: Could not create temporary overlay '/var/tmp/vl.o83dxn': Failed to lock byte 100
>
> (I also tried the snapshot=on drive option with similar results.)
>
> Thanks,
> Andrew
>
Looks like the shorthand "-snapshot" doesn't let you specify any further
options, which is a bummer.
You may need to do something a little more manual, and create your own
temporary overlay, and launch QEMU pointing to that overlay instead.
That sounds like a bit of a hassle.
Can we compile locking support out of QEMU instead for this platform? Or
is there a runtime option for disabling it globally?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Disable image locking for snapshot drive?
2017-07-18 0:33 ` John Snow
@ 2017-07-18 2:19 ` Andrew Baumann
2017-07-18 15:07 ` Eric Blake
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Baumann @ 2017-07-18 2:19 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: famz
> From: John Snow [mailto:jsnow@redhat.com]
> Sent: Monday, 17 July 2017 17:34
> On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> > Hi all,
> >
> > I'm running a recent Linux build of qemu on Windows Subsystem for Linux
> (WSL) which doesn't appear to implement file locking:
> >
> > $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device
> virtio-blk-pci,drive=hd0
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to lock
> byte 100
> >
> > That's no big deal; I can switch it off:
> >
> > $ qemu-system-aarch64 ... -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0 ...
> > (all good)
> >
> > But how can I do the same for a snapshot drive?
> >
> > $ qemu-system-aarch64 ... -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0 -snapshot ...
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Could not create temporary
> overlay '/var/tmp/vl.o83dxn': Failed to lock byte 100
> >
> > (I also tried the snapshot=on drive option with similar results.)
> >
> > Thanks,
> > Andrew
> >
>
> Looks like the shorthand "-snapshot" doesn't let you specify any further
> options, which is a bummer.
>
> You may need to do something a little more manual, and create your own
> temporary overlay, and launch QEMU pointing to that overlay instead.
Can you give me some more clues what this might look like?
> That sounds like a bit of a hassle.
>
> Can we compile locking support out of QEMU instead for this platform? Or
> is there a runtime option for disabling it globally?
The compile target is just Linux, so at best it would need to be a runtime choice based on platform detection, and I doubt that is a good idea (WSL may well get around to implementing this syscall in the future). I agree a runtime command-line flag to disable it globally would be ideal, but from a quick look at the code it doesn't seem to exist at present.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Disable image locking for snapshot drive?
2017-07-18 0:33 ` John Snow
2017-07-18 2:19 ` Andrew Baumann
@ 2017-07-18 15:07 ` Eric Blake
2017-07-18 16:19 ` Andrew Baumann
1 sibling, 1 reply; 8+ messages in thread
From: Eric Blake @ 2017-07-18 15:07 UTC (permalink / raw)
To: John Snow, Andrew Baumann, qemu-devel; +Cc: famz
[-- Attachment #1: Type: text/plain, Size: 972 bytes --]
On 07/17/2017 07:33 PM, John Snow wrote:
>
>
> On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
>> Hi all,
>>
>> I'm running a recent Linux build of qemu on Windows Subsystem for Linux (WSL) which doesn't appear to implement file locking:
>>
>> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device virtio-blk-pci,drive=hd0
>> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock byte 100
Does WSL implement fcntl(F_SETLK) but not fcntl(F_OFD_SETLK)? We
already have code in place for compiling when F_OFD_SETLK is not
supported (which makes lock=auto do nothing, and issues a warning that
F_SETLK locks may be ineffective when locks are explicitly requested),
do we need to just expand that code into a runtime test of whether
F_OFD_SETLK appears to be unsupported?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Disable image locking for snapshot drive?
2017-07-18 15:07 ` Eric Blake
@ 2017-07-18 16:19 ` Andrew Baumann
2017-07-20 6:53 ` Fam Zheng
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Baumann @ 2017-07-18 16:19 UTC (permalink / raw)
To: Eric Blake, John Snow, qemu-devel; +Cc: famz
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Tuesday, 18 July 2017 8:07
> On 07/17/2017 07:33 PM, John Snow wrote:
> > On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> >> I'm running a recent Linux build of qemu on Windows Subsystem for Linux
> (WSL) which doesn't appear to implement file locking:
> >>
> >> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device
> virtio-blk-pci,drive=hd0
> >> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
>
> Does WSL implement fcntl(F_SETLK) but not fcntl(F_OFD_SETLK)?
Yes, this appears to be the case (there's also one report that it's broken):
https://github.com/Microsoft/BashOnWindows/issues/1927
> We
> already have code in place for compiling when F_OFD_SETLK is not
> supported (which makes lock=auto do nothing, and issues a warning that
> F_SETLK locks may be ineffective when locks are explicitly requested),
> do we need to just expand that code into a runtime test of whether
> F_OFD_SETLK appears to be unsupported?
That would be a nice fix, and it would avoid the need for yet another flag. On the other hand, WSL is aiming for ABI compatibility, so they should get around to implementing F_OFD_SETLK et al eventually.
Even if this were fixed in QEMU or implemented in WSL, shouldn't there to be a way to turn snapshot file locking off on a per-drive basis?
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Disable image locking for snapshot drive?
2017-07-18 16:19 ` Andrew Baumann
@ 2017-07-20 6:53 ` Fam Zheng
2017-07-20 21:49 ` Andrew Baumann
0 siblings, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2017-07-20 6:53 UTC (permalink / raw)
To: Andrew Baumann; +Cc: Eric Blake, John Snow, qemu-devel
On Tue, 07/18 16:19, Andrew Baumann wrote:
> > From: Eric Blake [mailto:eblake@redhat.com]
> > Sent: Tuesday, 18 July 2017 8:07
> > On 07/17/2017 07:33 PM, John Snow wrote:
> > > On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> > >> I'm running a recent Linux build of qemu on Windows Subsystem for Linux
> > (WSL) which doesn't appear to implement file locking:
> > >>
> > >> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device
> > virtio-blk-pci,drive=hd0
> > >> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> > unlock byte 100
> >
> > Does WSL implement fcntl(F_SETLK) but not fcntl(F_OFD_SETLK)?
>
> Yes, this appears to be the case (there's also one report that it's broken):
> https://github.com/Microsoft/BashOnWindows/issues/1927
What does fcntl(F_OFD_SETLK) return? If it is -ENOTSUP, we can probably detect
that and disable locking. Can you try the patch pasted in the end?
>
> > We
> > already have code in place for compiling when F_OFD_SETLK is not
> > supported (which makes lock=auto do nothing, and issues a warning that
> > F_SETLK locks may be ineffective when locks are explicitly requested),
> > do we need to just expand that code into a runtime test of whether
> > F_OFD_SETLK appears to be unsupported?
>
> That would be a nice fix, and it would avoid the need for yet another flag. On
> the other hand, WSL is aiming for ABI compatibility, so they should get around
> to implementing F_OFD_SETLK et al eventually.
>
> Even if this were fixed in QEMU or implemented in WSL, shouldn't there to be a
> way to turn snapshot file locking off on a per-drive basis?
A snapshot file is temporary and unlinked immediately, so applying a lock or not
doesn't matter that much to deserve an option for that.
---
diff --git a/block/file-posix.c b/block/file-posix.c
index cfbb236f6f..0be5bbbd53 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -493,6 +493,12 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
}
s->fd = fd;
+ if (s->use_lock) {
+ int ret0 = qemu_unlock_fd(fd, 0, 0);
+ if (ret0 == -ENOTSUP) {
+ s->use_lock = false;
+ }
+ }
s->lock_fd = -1;
if (s->use_lock) {
fd = qemu_open(filename, s->open_flags);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Disable image locking for snapshot drive?
2017-07-20 6:53 ` Fam Zheng
@ 2017-07-20 21:49 ` Andrew Baumann
2017-07-21 1:46 ` Fam Zheng
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Baumann @ 2017-07-20 21:49 UTC (permalink / raw)
To: Fam Zheng; +Cc: Eric Blake, John Snow, qemu-devel
> From: Fam Zheng [mailto:famz@redhat.com]
> Sent: Wednesday, 19 July 2017 23:53
>
> On Tue, 07/18 16:19, Andrew Baumann wrote:
> > > From: Eric Blake [mailto:eblake@redhat.com]
> > > Sent: Tuesday, 18 July 2017 8:07
> > > On 07/17/2017 07:33 PM, John Snow wrote:
> > > > On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> > > >> I'm running a recent Linux build of qemu on Windows Subsystem for
> Linux
> > > (WSL) which doesn't appear to implement file locking:
> > > >>
> > > >> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -
> device
> > > virtio-blk-pci,drive=hd0
> > > >> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> > > unlock byte 100
> > >
> > > Does WSL implement fcntl(F_SETLK) but not fcntl(F_OFD_SETLK)?
> >
> > Yes, this appears to be the case (there's also one report that it's broken):
> > https://github.com/Microsoft/BashOnWindows/issues/1927
>
> What does fcntl(F_OFD_SETLK) return? If it is -ENOTSUP, we can probably
> detect
> that and disable locking. Can you try the patch pasted in the end?
It returns -EINVAL. Your patch works, if I change the error code. Maybe testing for either ENOTSUP or EINVAL, and only allowing the fallback if the user didn't force locking on, would be a reasonable compromise?
Cheers,
Andrew
> ---
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index cfbb236f6f..0be5bbbd53 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -493,6 +493,12 @@ static int raw_open_common(BlockDriverState *bs,
> QDict *options,
> }
> s->fd = fd;
>
> + if (s->use_lock) {
> + int ret0 = qemu_unlock_fd(fd, 0, 0);
> + if (ret0 == -ENOTSUP) {
> + s->use_lock = false;
> + }
> + }
> s->lock_fd = -1;
> if (s->use_lock) {
> fd = qemu_open(filename, s->open_flags);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Disable image locking for snapshot drive?
2017-07-20 21:49 ` Andrew Baumann
@ 2017-07-21 1:46 ` Fam Zheng
0 siblings, 0 replies; 8+ messages in thread
From: Fam Zheng @ 2017-07-21 1:46 UTC (permalink / raw)
To: Andrew Baumann; +Cc: John Snow, qemu-devel
On Thu, 07/20 21:49, Andrew Baumann via Qemu-devel wrote:
> > From: Fam Zheng [mailto:famz@redhat.com]
> > Sent: Wednesday, 19 July 2017 23:53
> >
> > On Tue, 07/18 16:19, Andrew Baumann wrote:
> > > > From: Eric Blake [mailto:eblake@redhat.com]
> > > > Sent: Tuesday, 18 July 2017 8:07
> > > > On 07/17/2017 07:33 PM, John Snow wrote:
> > > > > On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> > > > >> I'm running a recent Linux build of qemu on Windows Subsystem for
> > Linux
> > > > (WSL) which doesn't appear to implement file locking:
> > > > >>
> > > > >> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -
> > device
> > > > virtio-blk-pci,drive=hd0
> > > > >> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> > > > unlock byte 100
> > > >
> > > > Does WSL implement fcntl(F_SETLK) but not fcntl(F_OFD_SETLK)?
> > >
> > > Yes, this appears to be the case (there's also one report that it's broken):
> > > https://github.com/Microsoft/BashOnWindows/issues/1927
> >
> > What does fcntl(F_OFD_SETLK) return? If it is -ENOTSUP, we can probably
> > detect
> > that and disable locking. Can you try the patch pasted in the end?
>
> It returns -EINVAL. Your patch works, if I change the error code. Maybe
> testing for either ENOTSUP or EINVAL, and only allowing the fallback if the
> user didn't force locking on, would be a reasonable compromise?
I'm less comfortable with treating -EINVAL as "not supported".
A better fallback simply disabling is probably F_SETLK. Anyway I think a message
to stderr should be printed like the "#ifndef F_OFD_SETLK" case.
I'll work on a formal patch which does that.
Thanks,
Fam
>
> Cheers,
> Andrew
>
> > ---
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index cfbb236f6f..0be5bbbd53 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -493,6 +493,12 @@ static int raw_open_common(BlockDriverState *bs,
> > QDict *options,
> > }
> > s->fd = fd;
> >
> > + if (s->use_lock) {
> > + int ret0 = qemu_unlock_fd(fd, 0, 0);
> > + if (ret0 == -ENOTSUP) {
> > + s->use_lock = false;
> > + }
> > + }
> > s->lock_fd = -1;
> > if (s->use_lock) {
> > fd = qemu_open(filename, s->open_flags);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-07-21 1:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-17 23:30 [Qemu-devel] Disable image locking for snapshot drive? Andrew Baumann
2017-07-18 0:33 ` John Snow
2017-07-18 2:19 ` Andrew Baumann
2017-07-18 15:07 ` Eric Blake
2017-07-18 16:19 ` Andrew Baumann
2017-07-20 6:53 ` Fam Zheng
2017-07-20 21:49 ` Andrew Baumann
2017-07-21 1:46 ` Fam Zheng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.