On Sun, Jul 22, 2018 at 5:08 AM Max Reitz <mre...@redhat.com> wrote: > > On 2018-07-19 05:41, Fam Zheng wrote: > > On my Fedora 28, /dev/null is locked by some other process (couldn't > > inspect it due to the current lslocks limitation), so iotests 226 fails > > with some unexpected image locking errors because it uses qemu-io to > > open it. > > > > Actually it's safe to not use any lock on /dev/null or /dev/zero. > > > > Signed-off-by: Fam Zheng <f...@redhat.com> > > --- > > block/file-posix.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/block/file-posix.c b/block/file-posix.c > > index 60af4b3d51..8bf034108a 100644 > > --- a/block/file-posix.c > > +++ b/block/file-posix.c > > @@ -503,7 +503,12 @@ static int raw_open_common(BlockDriverState *bs, QDict > > *options, > > s->use_lock = false; > > break; > > case ON_OFF_AUTO_AUTO: > > - s->use_lock = qemu_has_ofd_lock(); > > + if (!strcmp(filename, "/dev/null") || > > + !strcmp(filename, "/dev/zero")) { > > I’m not sure I like a strcmp() based on filename (though it should work > for all of the cases where someone would want to specify either of those > for a qemu block device). Isn’t there some specific major/minor number > we can use to check for these two files? Or are those Linux-specific?
Yeah, I guess major/minor numbers are Linux-specific. > > I could also imagine just not locking any host_device, but since people > do use qcow2 immediately on block devices, maybe we do want to lock them. That's right. > > Finally, if really all you are trying to do is to make test code easier, > then I don’t know exactly why. Just disabling locking in 226 shouldn’t > be too hard. The tricky thing is in remembering to do that for future test cases. My machine seems to be somehow different than John's so that my 226 cannot lock /dev/null, but I'm not sure that is the case for other releases, distros or system instances. Fam > > Max > > > + s->use_lock = false; > > + } else { > > + s->use_lock = qemu_has_ofd_lock(); > > + } > > break; > > default: > > abort(); > > > >