Re: [Qemu-devel] [PATCH] qemu-img info: show nocow info

[Chun Yan Liu]

>>> On 7/2/2014 at 09:03 PM, in message <address@hidden>, Eric Blake
<address@hidden> wrote: 
> On 07/02/2014 03:50 AM, Chunyan Liu wrote: 
> > Add nocow info in 'qemu-img info' output to show whether the file 
> > currently has NOCOW flag set or not. 
> >  
> > Signed-off-by: Chunyan Liu <address@hidden> 
> > --- 
> >  block/qapi.c         | 25 +++++++++++++++++++++++++ 
> >  qapi/block-core.json |  3 ++- 
> >  2 files changed, 27 insertions(+), 1 deletion(-) 
> >  
>  
> > + 
> > +    /* get NOCOW info */ 
> > +    fd = qemu_open(bs->filename, O_RDONLY | O_NONBLOCK); 
> > +    if (fd >= 0) { 
> > +        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0 && (attr & 
> > FS_NOCOW_FL)) { 
> > +            info->has_nocow = true; 
> > +            info->nocow = true; 
>  
> Better is: 
>  
> if (ioctl... == 0) { 
>     info->has_nocow = true; 
>     info->nocow = !!(attr & FS_NOCOW_FL) 
> } 
>  
It won't work. FS_IOC_GETFLAGS is a common ioctl to all file systems, it only
gets flags info that the file is currently set. Output 'attr' contains the 
flags info.
Even the filesystem doesn't support NOCOW, that ioctl could return 0.

> to explicitly document the cases where we know nocow works but is not 
> set (and omitting it in cases where nocow doesn't exist because of the fs) 
>  
>  
> > +++ b/qapi/block-core.json 
> > @@ -126,7 +126,8 @@ 
> >             '*backing-filename': 'str', '*full-backing-filename': 'str', 
> >             '*backing-filename-format': 'str', '*snapshots':  
> ['SnapshotInfo'], 
> >             '*backing-image': 'ImageInfo', 
> > -           '*format-specific': 'ImageInfoSpecific' } } 
> > +           '*format-specific': 'ImageInfoSpecific', 
> > +           '*nocow': 'bool' } } 
>  
> Missing documentation. When adding that, remember to add '(since 2.2)' 
> (or even since 2.1 if you think this is a bug fix that should go in 
> during hard freeze) 

OK. Thanks.

-Chunyan

>  
> --  
> Eric Blake   eblake redhat com    +1-919-301-3266 
> Libvirt virtualization library http://libvirt.org 
>  
>