storagepoolxml2argvtest: Populate test entries for macOS

[PATCH] storagepoolxml2argvtest: Populate test entries for macOS

Posted by Roman Bolshakov 4 years, 1 month ago

One of the cases fails on macOS:

  15) Storage Pool XML-2-argv pool-netfs-gluster
      ...
  In
  '/Users/roolebo/dev/libvirt/tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv':
  Offset 39
  Expect [-o direct-io-mode=1 /mnt/gluster]
  Actual [/mnt/gluster -o direct-io-mode=1]

glusterfs has not been tested on macOS but for now we can just make
tests happy by providing them with the data they expect. Likely,
there'll be updates to the argv files in the future.

storagepoolxml2argvtest passes after the change.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 tests/storagepoolxml2argvdata/pool-fs-darwin.argv               | 1 +
 tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv       | 1 +
 tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv       | 1 +
 tests/storagepoolxml2argvdata/pool-netfs-darwin.argv            | 1 +
 tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv    | 1 +
 .../storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv | 1 +
 tests/storagepoolxml2argvtest.c                                 | 2 ++
 7 files changed, 8 insertions(+)
 create mode 100644 tests/storagepoolxml2argvdata/pool-fs-darwin.argv
 create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv
 create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv
 create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-darwin.argv
 create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv
 create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv

diff --git a/tests/storagepoolxml2argvdata/pool-fs-darwin.argv b/tests/storagepoolxml2argvdata/pool-fs-darwin.argv
new file mode 100644
index 0000000000..537ce4cee5
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-fs-darwin.argv
@@ -0,0 +1 @@
+mount -t ext3 /dev/sda6 /mnt
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv b/tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv
new file mode 100644
index 0000000000..888a0161b8
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv
@@ -0,0 +1 @@
+mount localhost:/var/lib/libvirt/images /mnt
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv b/tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv
new file mode 100644
index 0000000000..2fef6f5782
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv
@@ -0,0 +1 @@
+mount -t cifs //example.com/samba_share /mnt/cifs -o guest
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-darwin.argv b/tests/storagepoolxml2argvdata/pool-netfs-darwin.argv
new file mode 100644
index 0000000000..04127c5087
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-darwin.argv
@@ -0,0 +1 @@
+mount -t nfs localhost:/var/lib/libvirt/images /mnt
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv b/tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv
new file mode 100644
index 0000000000..97be9cbeb3
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv
@@ -0,0 +1 @@
+mount -t glusterfs example.com:/volume /mnt/gluster -o direct-io-mode=1
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv
new file mode 100644
index 0000000000..f26656d5b8
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv
@@ -0,0 +1 @@
+mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nfsvers=3
diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c
index 967d1f21a8..ecce3ab7d0 100644
--- a/tests/storagepoolxml2argvtest.c
+++ b/tests/storagepoolxml2argvtest.c
@@ -126,6 +126,8 @@ mymain(void)
     const char *platform = "-linux";
 #elif defined(__FreeBSD__)
     const char *platform = "-freebsd";
+#elif defined(__APPLE__)
+    const char *platform = "-darwin";
 #else
     const char *platform = "";
 #endif
-- 
2.28.0

Re: [PATCH] storagepoolxml2argvtest: Populate test entries for macOS

Posted by Andrea Bolognani 4 years ago

On Mon, 2020-10-26 at 00:54 +0300, Roman Bolshakov wrote:
> One of the cases fails on macOS:
> 
>   15) Storage Pool XML-2-argv pool-netfs-gluster
>       ...
>   In
>   '/Users/roolebo/dev/libvirt/tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv':
>   Offset 39
>   Expect [-o direct-io-mode=1 /mnt/gluster]
>   Actual [/mnt/gluster -o direct-io-mode=1]
> 
> glusterfs has not been tested on macOS but for now we can just make
> tests happy by providing them with the data they expect. Likely,
> there'll be updates to the argv files in the future.
> 
> storagepoolxml2argvtest passes after the change.
> 
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>  tests/storagepoolxml2argvdata/pool-fs-darwin.argv               | 1 +
>  tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv       | 1 +
>  tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv       | 1 +
>  tests/storagepoolxml2argvdata/pool-netfs-darwin.argv            | 1 +
>  tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv    | 1 +
>  .../storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv | 1 +
>  tests/storagepoolxml2argvtest.c                                 | 2 ++
>  7 files changed, 8 insertions(+)
>  create mode 100644 tests/storagepoolxml2argvdata/pool-fs-darwin.argv
>  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv
>  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv
>  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-darwin.argv
>  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv
>  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv

This is unnecessarily complicated: all you need is

  diff --git a/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv b/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
  index 4303d514ef..97be9cbeb3 100644
  --- a/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
  +++ b/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
  @@ -1 +1 @@
  -mount -t glusterfs example.com:/volume -o direct-io-mode=1 /mnt/gluster
  +mount -t glusterfs example.com:/volume /mnt/gluster -o direct-io-mode=1

If you dig through the git history, you'll see that when
default_mount_opts was added to src/storage/storage_util.c, one of
the virCommandAddArgList() calls used to build the mount command line
has also seen its arguments reordered; however, since the per-OS
split of output files was performed as part of the same patch instead
of in a separate one, the existing output file was not updated
accordingly.

Since you're correcting a mistake introduced in an old commit, you
can include

  Fixes: f00cde7f1133fee96dc13a80d7f402c704346974

in your commit message.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] storagepoolxml2argvtest: Populate test entries for macOS

Posted by Roman Bolshakov 4 years ago

On Wed, Oct 28, 2020 at 09:52:47PM +0100, Andrea Bolognani wrote:
> On Mon, 2020-10-26 at 00:54 +0300, Roman Bolshakov wrote:
> >  tests/storagepoolxml2argvdata/pool-fs-darwin.argv               | 1 +
> >  tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv       | 1 +
> >  tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv       | 1 +
> >  tests/storagepoolxml2argvdata/pool-netfs-darwin.argv            | 1 +
> >  tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv    | 1 +
> >  .../storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv | 1 +
> >  tests/storagepoolxml2argvtest.c                                 | 2 ++
> >  7 files changed, 8 insertions(+)
> >  create mode 100644 tests/storagepoolxml2argvdata/pool-fs-darwin.argv
> >  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto-darwin.argv
> >  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs-darwin.argv
> >  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-darwin.argv
> >  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster-darwin.argv
> >  create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-darwin.argv
> 
> This is unnecessarily complicated: all you need is
> 
>   diff --git a/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv b/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
>   index 4303d514ef..97be9cbeb3 100644
>   --- a/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
>   +++ b/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
>   @@ -1 +1 @@
>   -mount -t glusterfs example.com:/volume -o direct-io-mode=1 /mnt/gluster
>   +mount -t glusterfs example.com:/volume /mnt/gluster -o direct-io-mode=1
> 
> If you dig through the git history, you'll see that when
> default_mount_opts was added to src/storage/storage_util.c, one of
> the virCommandAddArgList() calls used to build the mount command line
> has also seen its arguments reordered; however, since the per-OS
> split of output files was performed as part of the same patch instead
> of in a separate one, the existing output file was not updated
> accordingly.
> 
> Since you're correcting a mistake introduced in an old commit, you
> can include
> 
>   Fixes: f00cde7f1133fee96dc13a80d7f402c704346974
> 
> in your commit message.
> 

Hi Andrea,

Initially I updated only pool-netfs-gluster.argv but I wasn't sure if
I'm doing this right or wrong. Thanks for the commit reference, the hunk
proofs your point:

@@ -4278,8 +4305,8 @@ virStorageBackendFileSystemMountGlusterArgs(virCommandPtr cmd,
     const char *fmt;
 
     fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format);
-    virCommandAddArgList(cmd, "-t", fmt, src, "-o", "direct-io-mode=1",
-                         def->target.path, NULL);
+    virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL);
+    virStorageBackendFileSystemMountAddOptions(cmd, "direct-io-mode=1");
 }

Regards,
Roman