diff mbox series

[v3,09/11] tests/9pfs: wipe local 9pfs test directory

Message ID 0c54a2ca2049e9824df9805324cac8dac029c113.1601655308.git.qemu_oss@crudebyte.com
State Superseded
Headers show
Series 9pfs: add tests using local fs driver | expand

Commit Message

Christian Schoenebeck Oct. 2, 2020, 4:15 p.m. UTC
Before running the first 9pfs test case, make sure the test directory
for running the 9pfs 'local' tests on is entirely empty. For that
reason simply delete the test directory (if any) before (re)creating
it on test suite startup.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Christian Schoenebeck Oct. 8, 2020, 12:26 p.m. UTC | #1
On Freitag, 2. Oktober 2020 18:15:05 CEST Christian Schoenebeck wrote:
> Before running the first 9pfs test case, make sure the test directory

> for running the 9pfs 'local' tests on is entirely empty. For that

> reason simply delete the test directory (if any) before (re)creating

> it on test suite startup.

> 

> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

> ---

>  tests/qtest/libqos/virtio-9p.c | 13 +++++++++++++

>  1 file changed, 13 insertions(+)

> 

> diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c

> index 1bada47af1..41a7c2baaa 100644

> --- a/tests/qtest/libqos/virtio-9p.c

> +++ b/tests/qtest/libqos/virtio-9p.c

> @@ -53,6 +53,18 @@ static void create_local_test_dir(void)

>      g_assert((st.st_mode & S_IFMT) == S_IFDIR);

>  }

> 

> +/* Deletes directory previously created by create_local_test_dir(). */

> +static void remove_local_test_dir(void)

> +{

> +    g_assert(local_test_path != NULL);

> +    char *cmd = g_strdup_printf("rm -r '%s'\n", local_test_path);

> +    int res = system(cmd);

> +    if (res < 0) {

> +        /* ignore error, dummy check to prevent compiler error */

> +    }

> +    g_free(cmd);

> +}

> +

>  static void virtio_9p_cleanup(QVirtio9P *interface)

>  {

>      qvirtqueue_cleanup(interface->vdev->bus, interface->vq, alloc);

> @@ -220,6 +232,7 @@ static void virtio_9p_register_nodes(void)

> 

>      /* make sure test dir for the 'local' tests exists and is clean */

>      init_local_test_path();

> +    remove_local_test_dir();

>      create_local_test_dir();

> 

>      QPCIAddress addr = {

>          .devfn = QPCI_DEVFN(4, 0),

>      };

>     

>      QOSGraphEdgeOptions opts = {

>          .before_cmd_line = "-fsdev synth,id=fsdev0",

>      };

>     

>      /* virtio-9p-device */

>      opts.extra_device_opts = str_simple,

>      qos_node_create_driver("virtio-9p-device", virtio_9p_device_create);

>      qos_node_consumes("virtio-9p-device", "virtio-bus", &opts);

>      qos_node_produces("virtio-9p-device", "virtio");

>      qos_node_produces("virtio-9p-device", "virtio-9p");

>     

>      /* virtio-9p-pci */

>      opts.extra_device_opts = str_addr;

>      add_qpci_address(&opts, &addr);

>      qos_node_create_driver("virtio-9p-pci", virtio_9p_pci_create);

>      qos_node_consumes("virtio-9p-pci", "pci-bus", &opts);

>      qos_node_produces("virtio-9p-pci", "pci-device");

>      qos_node_produces("virtio-9p-pci", "virtio");

>      qos_node_produces("virtio-9p-pci", "virtio-9p");

> }

> 

> libqos_init(virtio_9p_register_nodes);


I wonder why libqos is calling virtio_9p_register_nodes() again after all 
qtests ended.

That's somewhat suboptimal here, as it causes remove_local_test_dir() to be 
called again after all qtests completed. My intention was actually only to 
wipe the "qtest-9p-local" test directory at the *start* of the test suite run. 
Not at the end of the test suite. Because it would allow developers to look at 
the actual dirs/files created after the tests completed.

I could of course misuse and add a dedicated "wipedir" test as workaround, but 
that OTOH would break the option of running individual tests with the -p CL 
switch.

Best regards,
Christian Schoenebeck
Christian Schoenebeck Oct. 8, 2020, 3:34 p.m. UTC | #2
On Donnerstag, 8. Oktober 2020 14:26:28 CEST Christian Schoenebeck wrote:
> On Freitag, 2. Oktober 2020 18:15:05 CEST Christian Schoenebeck wrote:

> > Before running the first 9pfs test case, make sure the test directory

> > for running the 9pfs 'local' tests on is entirely empty. For that

> > reason simply delete the test directory (if any) before (re)creating

> > it on test suite startup.

> > 

> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

> > ---

> > 

> >  tests/qtest/libqos/virtio-9p.c | 13 +++++++++++++

> >  1 file changed, 13 insertions(+)

> > 

> > diff --git a/tests/qtest/libqos/virtio-9p.c

> > b/tests/qtest/libqos/virtio-9p.c index 1bada47af1..41a7c2baaa 100644

> > --- a/tests/qtest/libqos/virtio-9p.c

> > +++ b/tests/qtest/libqos/virtio-9p.c

> > @@ -53,6 +53,18 @@ static void create_local_test_dir(void)

> > 

> >      g_assert((st.st_mode & S_IFMT) == S_IFDIR);

> >  

> >  }

> > 

> > +/* Deletes directory previously created by create_local_test_dir(). */

> > +static void remove_local_test_dir(void)

> > +{

> > +    g_assert(local_test_path != NULL);

> > +    char *cmd = g_strdup_printf("rm -r '%s'\n", local_test_path);

> > +    int res = system(cmd);

> > +    if (res < 0) {

> > +        /* ignore error, dummy check to prevent compiler error */

> > +    }

> > +    g_free(cmd);

> > +}

> > +

> > 

> >  static void virtio_9p_cleanup(QVirtio9P *interface)

> >  {

> >  

> >      qvirtqueue_cleanup(interface->vdev->bus, interface->vq, alloc);

> > 

> > @@ -220,6 +232,7 @@ static void virtio_9p_register_nodes(void)

> > 

> >      /* make sure test dir for the 'local' tests exists and is clean */

> >      init_local_test_path();

> > 

> > +    remove_local_test_dir();

> > 

> >      create_local_test_dir();

> >      

> >      QPCIAddress addr = {

> >      

> >          .devfn = QPCI_DEVFN(4, 0),

> >      

> >      };

> >      

> >      QOSGraphEdgeOptions opts = {

> >      

> >          .before_cmd_line = "-fsdev synth,id=fsdev0",

> >      

> >      };

> >      

> >      /* virtio-9p-device */

> >      opts.extra_device_opts = str_simple,

> >      qos_node_create_driver("virtio-9p-device", virtio_9p_device_create);

> >      qos_node_consumes("virtio-9p-device", "virtio-bus", &opts);

> >      qos_node_produces("virtio-9p-device", "virtio");

> >      qos_node_produces("virtio-9p-device", "virtio-9p");

> >      

> >      /* virtio-9p-pci */

> >      opts.extra_device_opts = str_addr;

> >      add_qpci_address(&opts, &addr);

> >      qos_node_create_driver("virtio-9p-pci", virtio_9p_pci_create);

> >      qos_node_consumes("virtio-9p-pci", "pci-bus", &opts);

> >      qos_node_produces("virtio-9p-pci", "pci-device");

> >      qos_node_produces("virtio-9p-pci", "virtio");

> >      qos_node_produces("virtio-9p-pci", "virtio-9p");

> > 

> > }

> > 

> > libqos_init(virtio_9p_register_nodes);

> 

> I wonder why libqos is calling virtio_9p_register_nodes() again after all

> qtests ended.

> 

> That's somewhat suboptimal here, as it causes remove_local_test_dir() to be

> called again after all qtests completed. My intention was actually only to

> wipe the "qtest-9p-local" test directory at the *start* of the test suite

> run. Not at the end of the test suite. Because it would allow developers to

> look at the actual dirs/files created after the tests completed.

> 

> I could of course misuse and add a dedicated "wipedir" test as workaround,

> but that OTOH would break the option of running individual tests with the

> -p CL switch.


I probably leave that issue unfixed for now.

There seems to be no easy way to fix this, as libqos does not having something 
like "run this function once per test suite run", and due to the multi process 
approach a static variable hack would not be viable either.

Fortunately though, if a test case fails virtio_9p_register_nodes() would not 
be called again, so at least in case of failures the test dir is note wiped.

So I don't think it's worth to drill another hole into libqos just for this.

Best regards,
Christian Schoenebeck
diff mbox series

Patch

diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
index 1bada47af1..41a7c2baaa 100644
--- a/tests/qtest/libqos/virtio-9p.c
+++ b/tests/qtest/libqos/virtio-9p.c
@@ -53,6 +53,18 @@  static void create_local_test_dir(void)
     g_assert((st.st_mode & S_IFMT) == S_IFDIR);
 }
 
+/* Deletes directory previously created by create_local_test_dir(). */
+static void remove_local_test_dir(void)
+{
+    g_assert(local_test_path != NULL);
+    char *cmd = g_strdup_printf("rm -r '%s'\n", local_test_path);
+    int res = system(cmd);
+    if (res < 0) {
+        /* ignore error, dummy check to prevent compiler error */
+    }
+    g_free(cmd);
+}
+
 static void virtio_9p_cleanup(QVirtio9P *interface)
 {
     qvirtqueue_cleanup(interface->vdev->bus, interface->vq, alloc);
@@ -220,6 +232,7 @@  static void virtio_9p_register_nodes(void)
 
     /* make sure test dir for the 'local' tests exists and is clean */
     init_local_test_path();
+    remove_local_test_dir();
     create_local_test_dir();
 
     QPCIAddress addr = {