Re: [PATCH 2/2] storage: Rename btrfsCloneFile to support other filesystems.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 06, 2018 at 16:57:17 +0200, Michal Privoznik wrote:
> On 07/06/2018 03:43 PM, Julio Faracco wrote:
> > This commit renames and adds other macros to support aother filesystems
> > when a reflink is performed. After that, XFS filesystems (and others)
> > with reflink support will be able to clone.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1565004
> > 
> > Signed-off-by: Julio Faracco <jcfaracco@xxxxxxxxx>
> > ---
> >  src/storage/storage_util.c | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> > index a701a75702..fd1239c6cb 100644
> > --- a/src/storage/storage_util.c
> > +++ b/src/storage/storage_util.c
> > @@ -36,6 +36,9 @@
> >  # ifndef FS_NOCOW_FL
> >  #  define FS_NOCOW_FL                     0x00800000 /* Do not cow file */
> >  # endif
> > +# ifdef FICLONE
> > +#  define REFLINK_IOC_CLONE FICLONE
> > +# endif
> >  #endif
> >  
> >  #if WITH_BLKID
> > @@ -48,6 +51,10 @@
> >  
> >  #if HAVE_LINUX_BTRFS_H
> >  # include <linux/btrfs.h>
> > +# define REFLINK_IOC_CLONE BTRFS_IOC_CLONE
> > +#elif HAVE_XFS_XFS_H
> > +# include <xfs/xfs.h>
> > +# define REFLINK_IOC_CLONE XFS_IOC_CLONE
> >  #endif
> >  
> 
> Problem is, REFLING_IOC_CLONE is defined already at this point (by hunk 
> above) so this redefines the macro.
> 
> Fixed by squashing this in:
> 
> diff --git i/src/storage/storage_util.c w/src/storage/storage_util.c
> index fd1239c6cb..da99043e0a 100644
> --- i/src/storage/storage_util.c
> +++ w/src/storage/storage_util.c
> @@ -36,9 +36,6 @@
>  # ifndef FS_NOCOW_FL
>  #  define FS_NOCOW_FL                     0x00800000 /* Do not cow file */
>  # endif
> -# ifdef FICLONE
> -#  define REFLINK_IOC_CLONE FICLONE
> -# endif
>  #endif
>  
>  #if WITH_BLKID
> @@ -55,6 +52,8 @@
>  #elif HAVE_XFS_XFS_H
>  # include <xfs/xfs.h>
>  # define REFLINK_IOC_CLONE XFS_IOC_CLONE
> +#elif defined(FICLONE)
> +# define REFLINK_IOC_CLONE FICLONE

While it fortunately does not matter functionally as:

/usr/include/linux/fs.h:#define FICLONE		_IOW(0x94, 9, int)

/usr/include/xfs/xfs_fs.h:#define XFS_IOC_CLONE		 _IOW (0x94, 9, int)

/usr/include/linux/btrfs.h:#define BTRFS_IOCTL_MAGIC 0x94
/usr/include/linux/btrfs.h:#define BTRFS_IOC_CLONE        _IOW(BTRFS_IOCTL_MAGIC, 9, int)

I think the logic should be the other way around and use the
most-generic definition first. This is just plain confusing for readers
e.g. if you have both headers for XFS and BTRFS. For linux users it does
not matter if any of the others are defined as the FICLONE should always
be defined.


>  #endif
>  
>  #include "datatypes.h"
> 
> Michal
> 
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux