Re: [PATCH v3 37/54] tests/qtest: {ahci, ide}-test: Use relative path for temporary files for win32

2022-09-26 Thread Bin Meng
Hi Thomas,

On Tue, Sep 27, 2022 at 12:20 AM Thomas Huth  wrote:
>
> On 25/09/2022 13.30, Bin Meng wrote:
> > From: Bin Meng 
> >
> > These test cases uses "blkdebug:path/to/config:path/to/image" for
> > testing. On Windows, absolute file paths contain the delimiter ':'
> > which causes the blkdebug filename parser fail to parse filenames.
> >
> > Signed-off-by: Bin Meng 
> > Reviewed-by: Marc-André Lureau 
> > ---
> >
> > (no changes since v1)
> >
> >   tests/qtest/ahci-test.c | 21 ++---
> >   tests/qtest/ide-test.c  | 20 ++--
> >   2 files changed, 36 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> > index 1d5929d8c3..66652fed04 100644
> > --- a/tests/qtest/ahci-test.c
> > +++ b/tests/qtest/ahci-test.c
> > @@ -1833,7 +1833,7 @@ static void create_ahci_io_test(enum IOMode type, 
> > enum AddrMode addr,
> >
> >   int main(int argc, char **argv)
> >   {
> > -const char *arch;
> > +const char *arch, *base;
> >   int ret;
> >   int fd;
> >   int c;
> > @@ -1871,8 +1871,22 @@ int main(int argc, char **argv)
> >   return 0;
> >   }
> >
> > +/*
> > + * "base" stores the starting point where we create temporary files.
> > + *
> > + * On Windows, this is set to the relative path of current working
> > + * directory, because the absolute path causes the blkdebug filename
> > + * parser fail to parse "blkdebug:path/to/config:path/to/image".
> > + */
> > +#ifndef _WIN32
> > +base = g_get_tmp_dir();
> > +#else
> > +base = ".";
> > +#endif
> > +
> >   /* Create a temporary image */
> > -fd = g_file_open_tmp("qtest.XX", _path, NULL);
> > +tmp_path = g_strdup_printf("%s/qtest.XX", base);
> > +fd = g_mkstemp(tmp_path);
> >   g_assert(fd >= 0);
> >   if (have_qemu_img()) {
> >   imgfmt = "qcow2";
> > @@ -1889,7 +1903,8 @@ int main(int argc, char **argv)
> >   close(fd);
> >
> >   /* Create temporary blkdebug instructions */
> > -fd = g_file_open_tmp("qtest-blkdebug.XX", _path, NULL);
> > +debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", base);
> > +fd = g_mkstemp(debug_path);
> >   g_assert(fd >= 0);
> >   close(fd);
>
> It would maybe make sense to merge this with patch 05 ("tests/qtest:
> ahci-test: Avoid using hardcoded /tmp") ? ... but if you want to keep it
> separate, that's fine for me, too.

I'd prefer to keep these two patches separate as they are resolving
different issues.

> Reviewed-by: Thomas Huth 
>

Thanks for the review!

Regards,
Bin

Regards,
Bin



Re: [PATCH v3 37/54] tests/qtest: {ahci, ide}-test: Use relative path for temporary files for win32

2022-09-26 Thread Thomas Huth

On 25/09/2022 13.30, Bin Meng wrote:

From: Bin Meng 

These test cases uses "blkdebug:path/to/config:path/to/image" for
testing. On Windows, absolute file paths contain the delimiter ':'
which causes the blkdebug filename parser fail to parse filenames.

Signed-off-by: Bin Meng 
Reviewed-by: Marc-André Lureau 
---

(no changes since v1)

  tests/qtest/ahci-test.c | 21 ++---
  tests/qtest/ide-test.c  | 20 ++--
  2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index 1d5929d8c3..66652fed04 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1833,7 +1833,7 @@ static void create_ahci_io_test(enum IOMode type, enum 
AddrMode addr,
  
  int main(int argc, char **argv)

  {
-const char *arch;
+const char *arch, *base;
  int ret;
  int fd;
  int c;
@@ -1871,8 +1871,22 @@ int main(int argc, char **argv)
  return 0;
  }
  
+/*

+ * "base" stores the starting point where we create temporary files.
+ *
+ * On Windows, this is set to the relative path of current working
+ * directory, because the absolute path causes the blkdebug filename
+ * parser fail to parse "blkdebug:path/to/config:path/to/image".
+ */
+#ifndef _WIN32
+base = g_get_tmp_dir();
+#else
+base = ".";
+#endif
+
  /* Create a temporary image */
-fd = g_file_open_tmp("qtest.XX", _path, NULL);
+tmp_path = g_strdup_printf("%s/qtest.XX", base);
+fd = g_mkstemp(tmp_path);
  g_assert(fd >= 0);
  if (have_qemu_img()) {
  imgfmt = "qcow2";
@@ -1889,7 +1903,8 @@ int main(int argc, char **argv)
  close(fd);
  
  /* Create temporary blkdebug instructions */

-fd = g_file_open_tmp("qtest-blkdebug.XX", _path, NULL);
+debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", base);
+fd = g_mkstemp(debug_path);
  g_assert(fd >= 0);
  close(fd);


It would maybe make sense to merge this with patch 05 ("tests/qtest: 
ahci-test: Avoid using hardcoded /tmp") ? ... but if you want to keep it 
separate, that's fine for me, too.


Reviewed-by: Thomas Huth 




[PATCH v3 37/54] tests/qtest: {ahci, ide}-test: Use relative path for temporary files for win32

2022-09-25 Thread Bin Meng
From: Bin Meng 

These test cases uses "blkdebug:path/to/config:path/to/image" for
testing. On Windows, absolute file paths contain the delimiter ':'
which causes the blkdebug filename parser fail to parse filenames.

Signed-off-by: Bin Meng 
Reviewed-by: Marc-André Lureau 
---

(no changes since v1)

 tests/qtest/ahci-test.c | 21 ++---
 tests/qtest/ide-test.c  | 20 ++--
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index 1d5929d8c3..66652fed04 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1833,7 +1833,7 @@ static void create_ahci_io_test(enum IOMode type, enum 
AddrMode addr,
 
 int main(int argc, char **argv)
 {
-const char *arch;
+const char *arch, *base;
 int ret;
 int fd;
 int c;
@@ -1871,8 +1871,22 @@ int main(int argc, char **argv)
 return 0;
 }
 
+/*
+ * "base" stores the starting point where we create temporary files.
+ *
+ * On Windows, this is set to the relative path of current working
+ * directory, because the absolute path causes the blkdebug filename
+ * parser fail to parse "blkdebug:path/to/config:path/to/image".
+ */
+#ifndef _WIN32
+base = g_get_tmp_dir();
+#else
+base = ".";
+#endif
+
 /* Create a temporary image */
-fd = g_file_open_tmp("qtest.XX", _path, NULL);
+tmp_path = g_strdup_printf("%s/qtest.XX", base);
+fd = g_mkstemp(tmp_path);
 g_assert(fd >= 0);
 if (have_qemu_img()) {
 imgfmt = "qcow2";
@@ -1889,7 +1903,8 @@ int main(int argc, char **argv)
 close(fd);
 
 /* Create temporary blkdebug instructions */
-fd = g_file_open_tmp("qtest-blkdebug.XX", _path, NULL);
+debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", base);
+fd = g_mkstemp(debug_path);
 g_assert(fd >= 0);
 close(fd);
 
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 25302be6dc..5e3e28aea2 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -1011,16 +1011,32 @@ static void test_cdrom_dma(void)
 
 int main(int argc, char **argv)
 {
+const char *base;
 int fd;
 int ret;
 
+/*
+ * "base" stores the starting point where we create temporary files.
+ *
+ * On Windows, this is set to the relative path of current working
+ * directory, because the absolute path causes the blkdebug filename
+ * parser fail to parse "blkdebug:path/to/config:path/to/image".
+ */
+#ifndef _WIN32
+base = g_get_tmp_dir();
+#else
+base = ".";
+#endif
+
 /* Create temporary blkdebug instructions */
-fd = g_file_open_tmp("qtest-blkdebug.XX", _path, NULL);
+debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", base);
+fd = g_mkstemp(debug_path);
 g_assert(fd >= 0);
 close(fd);
 
 /* Create a temporary raw image */
-fd = g_file_open_tmp("qtest.XX", _path, NULL);
+tmp_path = g_strdup_printf("%s/qtest.XX", base);
+fd = g_mkstemp(tmp_path);
 g_assert(fd >= 0);
 ret = ftruncate(fd, TEST_IMAGE_SIZE);
 g_assert(ret == 0);
-- 
2.34.1