[libvirt PATCH v2 09/13] util: move virStorageFileProbe code into storage_file

Pavel Hrdina posted 13 patches 5 years ago
[libvirt PATCH v2 09/13] util: move virStorageFileProbe code into storage_file
Posted by Pavel Hrdina 5 years ago
Same as virStorageFileBackend, it doesn't belong into util directory.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 po/POTFILES.in                                         |  2 +-
 src/libvirt_private.syms                               | 10 +++++-----
 src/qemu/qemu_driver.c                                 |  2 +-
 src/storage/storage_backend_gluster.c                  |  2 +-
 src/storage/storage_util.c                             |  2 +-
 src/storage_file/meson.build                           |  1 +
 .../storage_file_probe.c}                              |  6 +++---
 .../storage_file_probe.h}                              |  2 +-
 src/storage_file/storage_source.c                      |  2 +-
 src/util/meson.build                                   |  1 -
 10 files changed, 15 insertions(+), 15 deletions(-)
 rename src/{util/virstoragefileprobe.c => storage_file/storage_file_probe.c} (99%)
 rename src/{util/virstoragefileprobe.h => storage_file/storage_file_probe.h} (95%)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 4da22ede8f..eeff3f9f2f 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -226,6 +226,7 @@
 @SRCDIR@src/storage_file/storage_file_backend.c
 @SRCDIR@src/storage_file/storage_file_fs.c
 @SRCDIR@src/storage_file/storage_file_gluster.c
+@SRCDIR@src/storage_file/storage_file_probe.c
 @SRCDIR@src/storage_file/storage_source.c
 @SRCDIR@src/test/test_driver.c
 @SRCDIR@src/util/iohelper.c
@@ -307,7 +308,6 @@
 @SRCDIR@src/util/virsocketaddr.c
 @SRCDIR@src/util/virstorageencryption.c
 @SRCDIR@src/util/virstoragefile.c
-@SRCDIR@src/util/virstoragefileprobe.c
 @SRCDIR@src/util/virstring.c
 @SRCDIR@src/util/virsysinfo.c
 @SRCDIR@src/util/virsystemd.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e7af8f0599..b49eee3750 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1622,6 +1622,11 @@ virSecurityXATTRNamespaceDefined;
 virStorageFileBackendRegister;
 
 
+# storage_file/storage_file_probe.h
+virStorageFileProbeFormat;
+virStorageFileProbeGetMetadata;
+
+
 # storage_file/storage_source.h
 virStorageFileAccess;
 virStorageFileChainLookup;
@@ -3214,11 +3219,6 @@ virStorageTypeFromString;
 virStorageTypeToString;
 
 
-# util/virstoragefileprobe.h
-virStorageFileProbeFormat;
-virStorageFileProbeGetMetadata;
-
-
 # util/virstring.h
 virSkipSpaces;
 virSkipSpacesAndBackslash;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a5c2488f2c..5363af3f56 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -83,7 +83,7 @@
 #include "domain_nwfilter.h"
 #include "virhook.h"
 #include "virstoragefile.h"
-#include "virstoragefileprobe.h"
+#include "storage_file_probe.h"
 #include "storage_source.h"
 #include "virfile.h"
 #include "virfdstream.h"
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index 782f8eb611..554c76ddb4 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -28,9 +28,9 @@
 #include "viralloc.h"
 #include "virerror.h"
 #include "virlog.h"
-#include "virstoragefileprobe.h"
 #include "virstring.h"
 #include "viruri.h"
+#include "storage_file_probe.h"
 #include "storage_util.h"
 #include "storage_source.h"
 
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index d51fa2b4c0..04f9a442f2 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -62,7 +62,7 @@
 #include "vircrypto.h"
 #include "viruuid.h"
 #include "virstoragefile.h"
-#include "virstoragefileprobe.h"
+#include "storage_file_probe.h"
 #include "storage_util.h"
 #include "storage_source.h"
 #include "virlog.h"
diff --git a/src/storage_file/meson.build b/src/storage_file/meson.build
index bcd6f01906..eb9453e7b0 100644
--- a/src/storage_file/meson.build
+++ b/src/storage_file/meson.build
@@ -1,6 +1,7 @@
 storage_file_sources = [
   'storage_source.c',
   'storage_file_backend.c',
+  'storage_file_probe.c',
 ]
 
 stoarge_file_fs_sources = [
diff --git a/src/util/virstoragefileprobe.c b/src/storage_file/storage_file_probe.c
similarity index 99%
rename from src/util/virstoragefileprobe.c
rename to src/storage_file/storage_file_probe.c
index bca098cd35..e5e1b272ac 100644
--- a/src/util/virstoragefileprobe.c
+++ b/src/storage_file/storage_file_probe.c
@@ -1,5 +1,5 @@
 /*
- * virstoragefileprobe.c: file utility functions for FS storage backend
+ * storage_file_probe.c: file utility functions for FS storage backend
  *
  * Copyright (C) 2007-2017 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
@@ -26,17 +26,17 @@
 #include <fcntl.h>
 
 #include "internal.h"
+#include "storage_file_probe.h"
 #include "viralloc.h"
 #include "virbitmap.h"
 #include "virendian.h"
 #include "virfile.h"
 #include "virlog.h"
 #include "virstoragefile.h"
-#include "virstoragefileprobe.h"
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
-VIR_LOG_INIT("util.storagefileprobe");
+VIR_LOG_INIT("storage_file.storagefileprobe");
 
 enum lv_endian {
     LV_LITTLE_ENDIAN = 1, /* 1234 */
diff --git a/src/util/virstoragefileprobe.h b/src/storage_file/storage_file_probe.h
similarity index 95%
rename from src/util/virstoragefileprobe.h
rename to src/storage_file/storage_file_probe.h
index 2b94a4ae51..c443c11606 100644
--- a/src/util/virstoragefileprobe.h
+++ b/src/storage_file/storage_file_probe.h
@@ -1,5 +1,5 @@
 /*
- * virstoragefileprobe.h: file utility functions for FS storage backend
+ * storage_file_probe.h: file utility functions for FS storage backend
  *
  * Copyright (C) 2007-2009, 2012-2016 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c
index 744d3e94fd..d1e0aeb2c1 100644
--- a/src/storage_file/storage_source.c
+++ b/src/storage_file/storage_source.c
@@ -26,6 +26,7 @@
 
 #include "internal.h"
 #include "storage_file_backend.h"
+#include "storage_file_probe.h"
 #include "storage_source.h"
 #include "viralloc.h"
 #include "virerror.h"
@@ -35,7 +36,6 @@
 #include "virlog.h"
 #include "virobject.h"
 #include "virstoragefile.h"
-#include "virstoragefileprobe.h"
 #include "virstring.h"
 #include "viruri.h"
 #include "virutil.h"
diff --git a/src/util/meson.build b/src/util/meson.build
index 64c0f9df6d..b510f0ebe9 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -90,7 +90,6 @@ util_sources = [
   'virsocketaddr.c',
   'virstorageencryption.c',
   'virstoragefile.c',
-  'virstoragefileprobe.c',
   'virstring.c',
   'virsysinfo.c',
   'virsystemd.c',
-- 
2.29.2

Re: [libvirt PATCH v2 09/13] util: move virStorageFileProbe code into storage_file
Posted by Peter Krempa 5 years ago
On Thu, Jan 21, 2021 at 20:34:23 +0100, Pavel Hrdina wrote:
> Same as virStorageFileBackend, it doesn't belong into util directory.

Arguably, after you remove the file access, it kinda becomes just an
utility function.

On the other hand, it's use is strictly limited to the storage source
code, so it makes sense to stop linking it with the general util module.

> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  po/POTFILES.in                                         |  2 +-
>  src/libvirt_private.syms                               | 10 +++++-----
>  src/qemu/qemu_driver.c                                 |  2 +-
>  src/storage/storage_backend_gluster.c                  |  2 +-
>  src/storage/storage_util.c                             |  2 +-
>  src/storage_file/meson.build                           |  1 +
>  .../storage_file_probe.c}                              |  6 +++---
>  .../storage_file_probe.h}                              |  2 +-
>  src/storage_file/storage_source.c                      |  2 +-
>  src/util/meson.build                                   |  1 -
>  10 files changed, 15 insertions(+), 15 deletions(-)
>  rename src/{util/virstoragefileprobe.c => storage_file/storage_file_probe.c} (99%)
>  rename src/{util/virstoragefileprobe.h => storage_file/storage_file_probe.h} (95%)
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 4da22ede8f..eeff3f9f2f 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -226,6 +226,7 @@
>  @SRCDIR@src/storage_file/storage_file_backend.c
>  @SRCDIR@src/storage_file/storage_file_fs.c
>  @SRCDIR@src/storage_file/storage_file_gluster.c
> +@SRCDIR@src/storage_file/storage_file_probe.c

Hmm, I think we should add a prefix for the _fs and _gluster backend
drivers so that it doesn't get confused with the probing code.

Also same comment as previous patch regarding naming.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Re: [libvirt PATCH v2 09/13] util: move virStorageFileProbe code into storage_file
Posted by Pavel Hrdina 5 years ago
On Fri, Jan 22, 2021 at 09:57:03AM +0100, Peter Krempa wrote:
> On Thu, Jan 21, 2021 at 20:34:23 +0100, Pavel Hrdina wrote:
> > Same as virStorageFileBackend, it doesn't belong into util directory.
> 
> Arguably, after you remove the file access, it kinda becomes just an
> utility function.
> 
> On the other hand, it's use is strictly limited to the storage source
> code, so it makes sense to stop linking it with the general util module.
> 
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  po/POTFILES.in                                         |  2 +-
> >  src/libvirt_private.syms                               | 10 +++++-----
> >  src/qemu/qemu_driver.c                                 |  2 +-
> >  src/storage/storage_backend_gluster.c                  |  2 +-
> >  src/storage/storage_util.c                             |  2 +-
> >  src/storage_file/meson.build                           |  1 +
> >  .../storage_file_probe.c}                              |  6 +++---
> >  .../storage_file_probe.h}                              |  2 +-
> >  src/storage_file/storage_source.c                      |  2 +-
> >  src/util/meson.build                                   |  1 -
> >  10 files changed, 15 insertions(+), 15 deletions(-)
> >  rename src/{util/virstoragefileprobe.c => storage_file/storage_file_probe.c} (99%)
> >  rename src/{util/virstoragefileprobe.h => storage_file/storage_file_probe.h} (95%)
> > 
> > diff --git a/po/POTFILES.in b/po/POTFILES.in
> > index 4da22ede8f..eeff3f9f2f 100644
> > --- a/po/POTFILES.in
> > +++ b/po/POTFILES.in
> > @@ -226,6 +226,7 @@
> >  @SRCDIR@src/storage_file/storage_file_backend.c
> >  @SRCDIR@src/storage_file/storage_file_fs.c
> >  @SRCDIR@src/storage_file/storage_file_gluster.c
> > +@SRCDIR@src/storage_file/storage_file_probe.c
> 
> Hmm, I think we should add a prefix for the _fs and _gluster backend
> drivers so that it doesn't get confused with the probing code.

Good point. How about storage_file_backend_fs ?

> Also same comment as previous patch regarding naming.
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
> 
Re: [libvirt PATCH v2 09/13] util: move virStorageFileProbe code into storage_file
Posted by Peter Krempa 5 years ago
On Fri, Jan 22, 2021 at 10:15:52 +0100, Pavel Hrdina wrote:
> On Fri, Jan 22, 2021 at 09:57:03AM +0100, Peter Krempa wrote:
> > On Thu, Jan 21, 2021 at 20:34:23 +0100, Pavel Hrdina wrote:

[...]

> > >  @SRCDIR@src/storage_file/storage_file_backend.c
> > >  @SRCDIR@src/storage_file/storage_file_fs.c
> > >  @SRCDIR@src/storage_file/storage_file_gluster.c
> > > +@SRCDIR@src/storage_file/storage_file_probe.c
> > 
> > Hmm, I think we should add a prefix for the _fs and _gluster backend
> > drivers so that it doesn't get confused with the probing code.
> 
> Good point. How about storage_file_backend_fs ?

Sounds good to me.