Libguestfs - Mar 2018 - Re: [Qemu-devel] [Qemu-block] [PATCH] tests: regressions: make test-big-heap use a temporary empty file

Am 21.03.2018 um 14:48 hat Pino Toscano geschrieben:
> On Wednesday, 21 March 2018 14:45:38 CET Eric Blake wrote:
> > [adding qemu lists]
> > 
> > On 03/21/2018 07:51 AM, Richard W.M. Jones wrote:
> > > On Wed, Mar 21, 2018 at 01:44:17PM +0100, Pino Toscano wrote:
> > >> Newer versions of qemu use file locking for the images by
default, and
> > >> apparently that does not work with /dev/null.  Since this
test just
> > >> calls qemu-img to get the format of an empty image, create a
temporary
> > >> one instead.
> > > 
> > > ACK, but feels like this is a bug in qemu-img to me.
> > 
> > You're right that file locking on a character device like
/dev/null is
> > not going to work as expected, but is it a case where fcntl() actually
> > fails, or is it worse where the fcntl() claiming the locks
"succeeds"
> > but doesn't do what we want?  That is, what were the actual error 
> > messages you ran into?
> 
> $ qemu-img --version
> qemu-img version 2.10.1(qemu-2.10.1-2.fc27)
> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> $ qemu-img info /dev/null 
> qemu-img: Could not open '/dev/null': Failed to get
"consistent read" lock
> Is another process using the image?
Not sure where the difference is, but I can't reproduce this on
upstream, neither git master nor the v2.10.1 tag:

$ ./qemu-img --version
qemu-img version 2.10.1 (v2.10.1-dirty)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
$ ./qemu-img info /dev/null
image: /dev/null
file format: raw
virtual size: 0 (0 bytes)
disk size: 0

Also with strace:

open("/dev/null", O_RDONLY|O_CLOEXEC)   = 10
fcntl(10, F_OFD_SETLK, {l_type=F_RDLCK, l_whence=SEEK_SET, l_start=100,
l_len=1}) = 0
...

So my kernel (4.15.9-200.fc26.x86_64) seems to have no problems with
locking /dev/null.

Kevin

On 03/21/2018 03:44 PM, Kevin Wolf wrote:
>>>
>>> You're right that file locking on a character device like
/dev/null is
>>> not going to work as expected, but is it a case where fcntl()
actually
>>> fails, or is it worse where the fcntl() claiming the locks
"succeeds"
>>> but doesn't do what we want?  That is, what were the actual
error
>>> messages you ran into?
>>
>> $ qemu-img --version
>> qemu-img version 2.10.1(qemu-2.10.1-2.fc27)
>> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
>> $ qemu-img info /dev/null
>> qemu-img: Could not open '/dev/null': Failed to get
"consistent read" lock
>> Is another process using the image?
> 
> Not sure where the difference is, but I can't reproduce this on
> upstream, neither git master nor the v2.10.1 tag:
Is it a case where file locking actually works, and more than one 
process is trying to lock /dev/null at once?  (qemu-img info is 
short-lived, but could there be another longer-lived process also using 
/dev/null)?

Does using -r help (if the only reason you're telling qemu-img to 
operate on /dev/null is to probe qemu-img features, can you probe those 
same features without needing to write, which in turn requests less 
locking)?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Wed, Mar 21, 2018 at 03:58:05PM -0500, Eric Blake
wrote:
> On 03/21/2018 03:44 PM, Kevin Wolf wrote:
> >>>
> >>>You're right that file locking on a character device like
/dev/null is
> >>>not going to work as expected, but is it a case where fcntl()
actually
> >>>fails, or is it worse where the fcntl() claiming the locks
"succeeds"
> >>>but doesn't do what we want?  That is, what were the actual
error
> >>>messages you ran into?
> >>
> >>$ qemu-img --version
> >>qemu-img version 2.10.1(qemu-2.10.1-2.fc27)
> >>Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project
developers
> >>$ qemu-img info /dev/null
> >>qemu-img: Could not open '/dev/null': Failed to get
"consistent read" lock
> >>Is another process using the image?
> >
> >Not sure where the difference is, but I can't reproduce this on
> >upstream, neither git master nor the v2.10.1 tag:
> 
> Is it a case where file locking actually works, and more than one
> process is trying to lock /dev/null at once?  (qemu-img info is
> short-lived, but could there be another longer-lived process also
> using /dev/null)?
> 
> Does using -r help (if the only reason you're telling qemu-img to
> operate on /dev/null is to probe qemu-img features, can you probe
> those same features without needing to write, which in turn requests
> less locking)?
The original test (before Pino's patch) runs ‘qemu-img info /dev/null’
purely as a way to fork qemu-img.  It's a regression test for some
problem we had with the code that confines qemu-img using resource
limits.  So really nothing much to do with qemu-img at all.  The fix
is to create a temporary file and run qemu-img against that.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top