Re: [Qemu-block] [PATCH v2 0/3] Add new CD-ROM related qtests

[Hervé Poussineau]

Le 16/03/2018 à 06:39, Thomas Huth a écrit :

With one of my clean-up patches (see commit 1454509726719e0933c800), I
recently accidentially broke the "-cdrom" parameter (more precisely
"-drive if=scsi") on a couple of boards, since there was no error
detected during the "make check" regression testing. This is clearly an
indication that we are lacking tests in this area.
So this small patch series now introduces some tests for CD-ROM drives:
The first two patches introduce the possibility to check that booting
from CD-ROM drives still works fine for x86 and s390x, and the third
patch adds a test that certain machines can at least still be started
with the "-cdrom" parameter (i.e. that test would have catched the
mistake that I did with my SCSI cleanup patch).


Reviewed-by: Hervé Poussineau 



v2:
  - Use g_spawn_sync() instead of execlp() to run genisoimage
  - The "-cdrom" parameter test is now run on all architectures (with
machine "none" for the machines that are not explicitly checked)
  - Some rewordings and improved comments here and there

Thomas Huth (3):
   tests/boot-sector: Add magic bytes to s390x boot code header
   tests/cdrom-test: Test booting from CD-ROM ISO image file
   tests/cdrom-test: Test that -cdrom parameter is working

  tests/Makefile.include |   2 +
  tests/boot-sector.c|   9 +-
  tests/cdrom-test.c | 222 +
  3 files changed, 230 insertions(+), 3 deletions(-)
  create mode 100644 tests/cdrom-test.c

[Qemu-block] [PATCH v2 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Thomas Huth]

We're going to use the s390x boot code for testing CD-ROM booting.
But the ISO loader of the s390-ccw bios is a little bit more picky
than the network loader and expects some magic bytes in the header
of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
we've got to add them in our boot code here, too.

Reviewed-by: Christian Borntraeger 
Signed-off-by: Thomas Huth 
---
 tests/boot-sector.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index c373f0e..7824286 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
 };
 
 /* For s390x, use a mini "kernel" with the appropriate signature */
-static const uint8_t s390x_psw[] = {
-0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
+static const uint8_t s390x_psw_and_magic[] = {
+0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,  /* Program status word  */
+0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,  /* Magic:   */
+0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,  /* see linux_s390_magic */
+0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40   /* in the s390-ccw bios */
 };
 static const uint8_t s390x_code[] = {
 0xa7, 0xf4, 0x00, 0x0a,/* j 0x10010 */
@@ -110,7 +113,7 @@ int boot_sector_init(char *fname)
 } else if (g_str_equal(arch, "s390x")) {
 len = 0x1 + sizeof(s390x_code);
 boot_code = g_malloc0(len);
-memcpy(boot_code, s390x_psw, sizeof(s390x_psw));
+memcpy(boot_code, s390x_psw_and_magic, sizeof(s390x_psw_and_magic));
 memcpy(_code[0x1], s390x_code, sizeof(s390x_code));
 } else {
 g_assert_not_reached();
-- 
1.8.3.1

[Qemu-block] [PATCH v2 2/3] tests/cdrom-test: Test booting from CD-ROM ISO image file

[Thomas Huth]

We already have the code for a boot file in tests/boot-sector.c,
so if the genisoimage program is available, we can easily create
a bootable CD ISO image that we can use for testing whether our
CD-ROM emulation and the BIOS CD-ROM boot works correctly.

Signed-off-by: Thomas Huth 
---
 tests/Makefile.include |   2 +
 tests/cdrom-test.c | 164 +
 2 files changed, 166 insertions(+)
 create mode 100644 tests/cdrom-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index ef9b88c..37ca258 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -178,6 +178,7 @@ check-qtest-generic-y = tests/qmp-test$(EXESUF)
 gcov-files-generic-y = monitor.c qapi/qmp-dispatch.c
 check-qtest-generic-y += tests/device-introspect-test$(EXESUF)
 gcov-files-generic-y = qdev-monitor.c qmp.c
+check-qtest-generic-y += tests/cdrom-test$(EXESUF)
 
 gcov-files-ipack-y += hw/ipack/ipack.c
 check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
@@ -829,6 +830,7 @@ tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o 
$(test-util-obj-y)
 tests/numa-test$(EXESUF): tests/numa-test.o
 tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o 
tests/acpi-utils.o
 tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
+tests/cdrom-test$(EXESUF): tests/cdrom-test.o tests/boot-sector.o 
$(libqos-obj-y)
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< 
,"LINK","$(TARGET_DIR)$@")
diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
new file mode 100644
index 000..5bbf322
--- /dev/null
+++ b/tests/cdrom-test.c
@@ -0,0 +1,164 @@
+/*
+ * Various tests for emulated CD-ROM drives.
+ *
+ * Copyright (c) 2018 Red Hat Inc.
+ *
+ * Author:
+ *Thomas Huth 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2
+ * or later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "boot-sector.h"
+
+static char isoimage[] = "cdrom-boot-iso-XX";
+
+static int exec_genisoimg(const char **args)
+{
+gchar *out_err = NULL;
+gint exit_status = -1;
+bool success;
+
+success = g_spawn_sync(NULL, (gchar **)args, NULL,
+   G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL,
+   NULL, NULL, NULL, _err, _status, NULL);
+if (!success) {
+return -ENOENT;
+}
+if (out_err) {
+fputs(out_err, stderr);
+g_free(out_err);
+}
+
+return exit_status;
+}
+
+static int prepare_image(const char *arch, char *isoimage)
+{
+char srcdir[] = "cdrom-test-dir-XX";
+char *codefile = NULL;
+int ifh, ret = -1;
+const char *args[] = {
+"genisoimage", "-quiet", "-l", "-no-emul-boot",
+"-b", NULL, "-o", isoimage, srcdir, NULL
+};
+
+ifh = mkstemp(isoimage);
+if (ifh < 0) {
+perror("Error creating temporary iso image file");
+return -1;
+}
+if (!mkdtemp(srcdir)) {
+perror("Error creating temporary directory");
+goto cleanup;
+}
+
+if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64") ||
+g_str_equal(arch, "s390x")) {
+codefile = g_strdup_printf("%s/bootcode-XX", srcdir);
+ret = boot_sector_init(codefile);
+if (ret) {
+goto cleanup;
+}
+} else {
+/* Just create a dummy file */
+char txt[] = "empty disc";
+codefile = g_strdup_printf("%s/readme.txt", srcdir);
+if (!g_file_set_contents(codefile, txt, sizeof(txt) - 1, NULL)) {
+fprintf(stderr, "Failed to create '%s'\n", codefile);
+goto cleanup;
+}
+}
+
+args[5] = strchr(codefile, '/') + 1;
+ret = exec_genisoimg(args);
+if (ret) {
+fprintf(stderr, "genisoimage failed: %i\n", ret);
+}
+
+unlink(codefile);
+
+cleanup:
+g_free(codefile);
+rmdir(srcdir);
+close(ifh);
+
+return ret;
+}
+
+static void test_cdboot(gconstpointer data)
+{
+QTestState *qts;
+
+qts = qtest_startf("-accel kvm:tcg -no-shutdown %s%s", (const char *)data,
+   isoimage);
+boot_sector_test(qts);
+qtest_quit(qts);
+}
+
+static void add_x86_tests(void)
+{
+qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
+qtest_add_data_func("cdrom/boot/virtio-scsi",
+"-device virtio-scsi -device scsi-cd,drive=cdr "
+"-blockdev file,node-name=cdr,filename=", test_cdboot);
+qtest_add_data_func("cdrom/boot/isapc", "-M isapc "
+"-drive if=ide,media=cdrom,file=", test_cdboot);
+qtest_add_data_func("cdrom/boot/am53c974",
+"-device am53c974 -device scsi-cd,drive=cd1 "
+"-drive if=none,id=cd1,format=raw,file=", test_cdboot);
+

[Qemu-block] [PATCH v2 3/3] tests/cdrom-test: Test that -cdrom parameter is working

[Thomas Huth]

Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
on a couple of boards without us noticing it immediately. Thus let's
add a test which checks that "-cdrom" can at least be used to start
QEMU with certain machine types.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 tests/cdrom-test.c | 58 ++
 1 file changed, 58 insertions(+)

diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
index 5bbf322..7a1fce5 100644
--- a/tests/cdrom-test.c
+++ b/tests/cdrom-test.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "boot-sector.h"
+#include "qapi/qmp/qdict.h"
 
 static char isoimage[] = "cdrom-boot-iso-XX";
 
@@ -89,6 +90,32 @@ cleanup:
 return ret;
 }
 
+/**
+ * Check that at least the -cdrom parameter is basically working, i.e. we can
+ * see the filename of the ISO image in the output of "info block" afterwards
+ */
+static void test_cdrom_param(gconstpointer data)
+{
+QTestState *qts;
+char *resp;
+
+qts = qtest_startf("-M %s -cdrom %s", (const char *)data, isoimage);
+resp = qtest_hmp(qts, "info block");
+g_assert(strstr(resp, isoimage) != 0);
+g_free(resp);
+qtest_quit(qts);
+}
+
+static void add_cdrom_param_tests(const char **machines)
+{
+while (*machines) {
+char *testname = g_strdup_printf("cdrom/param/%s", *machines);
+qtest_add_data_func(testname, *machines, test_cdrom_param);
+g_free(testname);
+machines++;
+}
+}
+
 static void test_cdboot(gconstpointer data)
 {
 QTestState *qts;
@@ -154,6 +181,37 @@ int main(int argc, char **argv)
 add_x86_tests();
 } else if (g_str_equal(arch, "s390x")) {
 add_s390x_tests();
+} else if (g_str_equal(arch, "ppc64")) {
+const char *ppcmachines[] = {
+"pseries", "mac99", "g3beige", "40p", "prep", NULL
+};
+add_cdrom_param_tests(ppcmachines);
+} else if (g_str_equal(arch, "sparc")) {
+const char *sparcmachines[] = {
+"LX", "SPARCClassic", "SPARCbook", "SS-10", "SS-20", "SS-4",
+"SS-5", "SS-600MP", "Voyager", "leon3_generic", NULL
+};
+add_cdrom_param_tests(sparcmachines);
+} else if (g_str_equal(arch, "sparc64")) {
+const char *sparc64machines[] = {
+"niagara", "sun4u", "sun4v", NULL
+};
+add_cdrom_param_tests(sparc64machines);
+} else if (!strncmp(arch, "mips64", 6)) {
+const char *mips64machines[] = {
+"magnum", "malta", "mips", "pica61", NULL
+};
+add_cdrom_param_tests(mips64machines);
+} else if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
+const char *armmachines[] = {
+"realview-eb", "realview-eb-mpcore", "realview-pb-a8",
+"realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
+"vexpress-a9", "virt", NULL
+};
+add_cdrom_param_tests(armmachines);
+} else {
+const char *nonemachine[] = { "none", NULL };
+add_cdrom_param_tests(nonemachine);
 }
 
 ret = g_test_run();
-- 
1.8.3.1

[Qemu-block] [PATCH v2 0/3] Add new CD-ROM related qtests

[Thomas Huth]

With one of my clean-up patches (see commit 1454509726719e0933c800), I
recently accidentially broke the "-cdrom" parameter (more precisely
"-drive if=scsi") on a couple of boards, since there was no error
detected during the "make check" regression testing. This is clearly an
indication that we are lacking tests in this area.
So this small patch series now introduces some tests for CD-ROM drives:
The first two patches introduce the possibility to check that booting
from CD-ROM drives still works fine for x86 and s390x, and the third
patch adds a test that certain machines can at least still be started
with the "-cdrom" parameter (i.e. that test would have catched the
mistake that I did with my SCSI cleanup patch).

v2:
 - Use g_spawn_sync() instead of execlp() to run genisoimage
 - The "-cdrom" parameter test is now run on all architectures (with
   machine "none" for the machines that are not explicitly checked)
 - Some rewordings and improved comments here and there

Thomas Huth (3):
  tests/boot-sector: Add magic bytes to s390x boot code header
  tests/cdrom-test: Test booting from CD-ROM ISO image file
  tests/cdrom-test: Test that -cdrom parameter is working

 tests/Makefile.include |   2 +
 tests/boot-sector.c|   9 +-
 tests/cdrom-test.c | 222 +
 3 files changed, 230 insertions(+), 3 deletions(-)
 create mode 100644 tests/cdrom-test.c

-- 
1.8.3.1

Re: [Qemu-block] [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Michael S. Tsirkin]

On Thu, Mar 15, 2018 at 12:47:08PM +0100, Philippe Mathieu-Daudé wrote:
> On 03/15/2018 08:49 AM, Thomas Huth wrote:
> > We're going to use the s390x boot code for testing CD-ROM booting.
> > But the ISO loader of the s390-ccw bios is a little bit more picky
> > than the network loader and expects some magic bytes in the header
> > of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
> > we've got to add them in our boot code here, too.
> > 
> > Signed-off-by: Thomas Huth 
> > ---
> >  tests/boot-sector.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> > index c373f0e..04721c9 100644
> > --- a/tests/boot-sector.c
> > +++ b/tests/boot-sector.c
> > @@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
> >  };
> >  
> >  /* For s390x, use a mini "kernel" with the appropriate signature */
> > -static const uint8_t s390x_psw[] = {
> > -0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
> > +static const uint8_t s390x_psw_and_magic[] = {
> 
> Can you add a comment such "see linux_s390_magic in
> pc-bios/s390-ccw/bootmap.c"?

Or better yet, copy the code comment from there.

> > +0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,
> > +0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
> > +0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
> > +0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
> >  };
> >  static const uint8_t s390x_code[] = {
> >  0xa7, 0xf4, 0x00, 0x0a,/* j 0x10010 */
> > @@ -110,7 +113,7 @@ int boot_sector_init(char *fname)
> >  } else if (g_str_equal(arch, "s390x")) {
> >  len = 0x1 + sizeof(s390x_code);
> >  boot_code = g_malloc0(len);
> > -memcpy(boot_code, s390x_psw, sizeof(s390x_psw));
> > +memcpy(boot_code, s390x_psw_and_magic, 
> > sizeof(s390x_psw_and_magic));
> >  memcpy(_code[0x1], s390x_code, sizeof(s390x_code));
> >  } else {
> >  g_assert_not_reached();
> > 

Re: [Qemu-block] [Qemu-devel] [PATCH v3 12/16] block/dirty-bitmap: Add bdrv_dirty_iter_next_area

[John Snow]

On 02/28/2018 01:05 PM, Max Reitz wrote:
> This new function allows to look for a consecutively dirty area in a
> dirty bitmap.
> 
> Signed-off-by: Max Reitz 
> ---
>  include/block/dirty-bitmap.h |  2 ++
>  block/dirty-bitmap.c | 55 
> 
>  2 files changed, 57 insertions(+)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index e3f4bbf51d..8c8f63e722 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -79,6 +79,8 @@ void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>  void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>  int64_t offset, int64_t bytes);
>  int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
> +bool bdrv_dirty_iter_next_area(BdrvDirtyBitmapIter *iter, uint64_t 
> max_offset,
> +   uint64_t *offset, int *bytes);
>  void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t offset);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>  int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index d8e999226e..5d6b8dba89 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -496,6 +496,61 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter)
>  return hbitmap_iter_next(>hbi, true);
>  }
>  
> +/**
> + * Return the next consecutively dirty area in the dirty bitmap
> + * belonging to the given iterator @iter.
> + *
> + * @max_offset: Maximum value that may be returned for
> + *  *offset + *bytes
> + * @offset: Will contain the start offset of the next dirty area
> + * @bytes:  Will contain the length of the next dirty area
> + *
> + * Returns: True if a dirty area could be found before max_offset
> + *  (which means that *offset and *bytes then contain valid
> + *  values), false otherwise.
> + *
> + * Note that @iter is never advanced if false is returned.  If an area
> + * is found (which means that true is returned), it will be advanced
> + * past that area.
> + */
> +bool bdrv_dirty_iter_next_area(BdrvDirtyBitmapIter *iter, uint64_t 
> max_offset,
> +   uint64_t *offset, int *bytes)
> +{
> +uint32_t granularity = bdrv_dirty_bitmap_granularity(iter->bitmap);
> +uint64_t gran_max_offset;
> +int64_t ret;
> +int size;
> +
> +if (max_offset == iter->bitmap->size) {
> +/* If max_offset points to the image end, round it up by the
> + * bitmap granularity */
> +gran_max_offset = ROUND_UP(max_offset, granularity);
> +} else {
> +gran_max_offset = max_offset;
> +}
> +
> +ret = hbitmap_iter_next(>hbi, false);
> +if (ret < 0 || ret + granularity > gran_max_offset) {
> +return false;
> +}
> +
> +*offset = ret;
> +size = 0;
> +
> +assert(granularity <= INT_MAX);
> +
> +do {
> +/* Advance iterator */
> +ret = hbitmap_iter_next(>hbi, true);
> +size += granularity;
> +} while (ret + granularity <= gran_max_offset &&
> + hbitmap_iter_next(>hbi, false) == ret + granularity &&
> + size <= INT_MAX - granularity);
> +
> +*bytes = MIN(size, max_offset - *offset);
> +return true;
> +}
> +
>  /* Called within bdrv_dirty_bitmap_lock..unlock */
>  void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>int64_t offset, int64_t bytes)
> 

10, 11, 12:

Reviewed-by: John Snow 

Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working

[Thomas Huth]

On 15.03.2018 12:42, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 03/15/2018 08:49 AM, Thomas Huth wrote:
>> Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
>> on a couple of boards without that we noticed it immediately. Thus
>> add a test which checks that "-cdrom" can at least be used to start
>> QEMU with certain machine types.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  tests/Makefile.include |  7 +-
>>  tests/cdrom-test.c | 63 
>> +-
>>  2 files changed, 68 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index a104222..b744fea 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -321,8 +321,9 @@ check-qtest-microblaze-y = 
>> tests/boot-serial-test$(EXESUF)
>>  check-qtest-mips-y = tests/endianness-test$(EXESUF)
>>  
>>  check-qtest-mips64-y = tests/endianness-test$(EXESUF)
>> +check-qtest-mips64-y += tests/cdrom-test$(EXESUF)
>>  
>> -check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
>> +check-qtest-mips64el-y = $(check-qtest-mips64-y)
>>  
>>  check-qtest-moxie-y = tests/boot-serial-test$(EXESUF)
>>  
>> @@ -356,6 +357,7 @@ check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
>>  check-qtest-ppc64-y += tests/numa-test$(EXESUF)
>>  check-qtest-ppc64-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
>>  check-qtest-ppc64-y += tests/cpu-plug-test$(EXESUF)
>> +check-qtest-ppc64-y += tests/cdrom-test$(EXESUF)
>>  
>>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>>  
>> @@ -365,10 +367,12 @@ check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
>>  check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
>>  gcov-files-sparc-y = hw/timer/m48t59.c
>>  check-qtest-sparc-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-sparc-y += tests/cdrom-test$(EXESUF)
>>  
>>  check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
>>  check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
>>  check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-sparc64-y += tests/cdrom-test$(EXESUF)
>>  
>>  check-qtest-arm-y = tests/tmp105-test$(EXESUF)
>>  check-qtest-arm-y += tests/ds1338-test$(EXESUF)
>> @@ -384,6 +388,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
>>  check-qtest-aarch64-y = tests/numa-test$(EXESUF)
>>  check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
>>  check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-aarch64-y += tests/cdrom-test$(EXESUF)
> 
> Since you use g_str_equal(arch,), maybe this test can be added
> regardless the arch in check-qtest-generic.

Yes, sounds like a good idea. I think the test can even be run on other
architectures by using the default machine or "-M none" ... I'll give it
a try ...

>>  ret = g_test_run();
>>
> 
> Reviewed-by: Philippe Mathieu-Daudé 

 Thanks!
  Thomas

Re: [Qemu-block] [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Thomas Huth]

On 15.03.2018 12:47, Philippe Mathieu-Daudé wrote:
> On 03/15/2018 08:49 AM, Thomas Huth wrote:
>> We're going to use the s390x boot code for testing CD-ROM booting.
>> But the ISO loader of the s390-ccw bios is a little bit more picky
>> than the network loader and expects some magic bytes in the header
>> of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
>> we've got to add them in our boot code here, too.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  tests/boot-sector.c | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
>> index c373f0e..04721c9 100644
>> --- a/tests/boot-sector.c
>> +++ b/tests/boot-sector.c
>> @@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
>>  };
>>  
>>  /* For s390x, use a mini "kernel" with the appropriate signature */
>> -static const uint8_t s390x_psw[] = {
>> -0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
>> +static const uint8_t s390x_psw_and_magic[] = {
> 
> Can you add a comment such "see linux_s390_magic in
> pc-bios/s390-ccw/bootmap.c"?

Sure, I'll add a comment.

 Thomas

Re: [Qemu-block] [Qemu-devel] [PULL 00/41] Block layer patches

[John Snow]

On 03/15/2018 12:56 PM, Kevin Wolf wrote:
> Am 15.03.2018 um 17:42 hat Peter Maydell geschrieben:
>> On 13 March 2018 at 16:17, Kevin Wolf  wrote:
>>> The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:
>>>
>>>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' 
>>> into staging (2018-03-13 11:42:45 +)
>>>
>>> are available in the git repository at:
>>>
>>>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>>>
>>> for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:
>>>
>>>   block/mirror: change the semantic of 'force' of block-job-cancel 
>>> (2018-03-13 16:54:47 +0100)
>>>
>>> 
>>> Block layer patches
>>>
>>> 
>>
>> I get a compile failure here on some hosts:
>>
>> /home/pm215/qemu/blockjob.c: In function ‘block_job_txn_apply.isra.8’:
>> /home/pm215/qemu/blockjob.c:524:5: error: ‘rc’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>>  return rc;
>>  ^
>>
>> I guess the compiler can't always figure out whether there is
>> guaranteed to be at least one thing in the list ?
> 
> I think so.
> 
> John, I'll just modify your patch 'blockjobs: add prepare callback' to
> initialise rc = 0 in this function and send a v2 pull request.
> 
> Kevin
> 

Oh, interesting. I guess technically the list COULD be empty and that'd
be perfectly valid. I wonder why my compiler doesn't complain?

Anyway, initializing to zero seems like the correct behavior, thanks.

--js

[Qemu-block] [PULL v2 00/44] Block layer patches

[Kevin Wolf]

The following changes since commit 56e8698ffa8aba9f762f980bc21b5340b006f24b:

  Merge remote-tracking branch 
'remotes/stsquad/tags/pull-travis-speedup-130318-1' into staging (2018-03-15 
14:48:09 +)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to d3137181e5e1e5e6b651b58484fc8f705a48ee36:

  iscsi: fix iSER compilation (2018-03-15 17:58:26 +0100)


Block layer patches


Fam Zheng (4):
  block: Fix flags in reopen queue
  iotests: Add regression test for commit base locking
  vvfat: Fix inherit_options flags
  block: Fix leak of ignore_children in error path

John Snow (21):
  blockjobs: fix set-speed kick
  blockjobs: model single jobs as transactions
  Blockjobs: documentation touchup
  blockjobs: add status enum
  blockjobs: add state transition table
  iotests: add pause_wait
  blockjobs: add block_job_verb permission table
  blockjobs: add ABORTING state
  blockjobs: add CONCLUDED state
  blockjobs: add NULL state
  blockjobs: add block_job_dismiss
  blockjobs: ensure abort is called for cancelled jobs
  blockjobs: add commit, abort, clean helpers
  blockjobs: add block_job_txn_apply function
  blockjobs: add prepare callback
  blockjobs: add waiting status
  blockjobs: add PENDING status and event
  blockjobs: add block-job-finalize
  blockjobs: Expose manual property
  iotests: test manual job dismissal
  tests/test-blockjob: test cancellations

Kevin Wolf (14):
  luks: Separate image file creation from formatting
  luks: Create block_crypto_co_create_generic()
  luks: Support .bdrv_co_create
  luks: Turn invalid assertion into check
  luks: Catch integer overflow for huge sizes
  qemu-iotests: Test luks QMP image creation
  parallels: Support .bdrv_co_create
  qemu-iotests: Enable write tests for parallels
  qcow: Support .bdrv_co_create
  qed: Support .bdrv_co_create
  vdi: Make comments consistent with other drivers
  vhdx: Support .bdrv_co_create
  vpc: Support .bdrv_co_create
  vpc: Require aligned size in .bdrv_co_create

Liang Li (1):
  block/mirror: change the semantic of 'force' of block-job-cancel

Max Reitz (3):
  vdi: Pull option parsing from vdi_co_create
  vdi: Move file creation to vdi_co_create_opts
  vdi: Implement .bdrv_co_create

Paolo Bonzini (1):
  iscsi: fix iSER compilation

 qapi/block-core.json  | 363 --
 include/block/blockjob.h  |  71 -
 include/block/blockjob_int.h  |  17 +-
 block.c   |  10 +-
 block/backup.c|   5 +-
 block/commit.c|   2 +-
 block/crypto.c| 150 -
 block/iscsi.c |   2 +-
 block/mirror.c|  12 +-
 block/parallels.c | 199 +--
 block/qcow.c  | 196 +++
 block/qed.c   | 204 
 block/stream.c|   2 +-
 block/vdi.c   | 147 +
 block/vhdx.c  | 216 +++--
 block/vpc.c   | 241 +---
 block/vvfat.c |   2 +-
 blockdev.c|  71 +++--
 blockjob.c| 358 +++--
 tests/test-bdrv-drain.c   |   5 +-
 tests/test-blockjob-txn.c |  27 ++--
 tests/test-blockjob.c | 233 ++-
 block/trace-events|   7 +
 hmp-commands.hx   |   3 +-
 tests/qemu-iotests/030|   6 +-
 tests/qemu-iotests/055|  17 +-
 tests/qemu-iotests/056| 187 ++
 tests/qemu-iotests/056.out|   4 +-
 tests/qemu-iotests/109.out|  24 +--
 tests/qemu-iotests/153|  12 ++
 tests/qemu-iotests/153.out|   5 +
 tests/qemu-iotests/181|   2 +-
 tests/qemu-iotests/209| 210 
 tests/qemu-iotests/209.out| 136 
 tests/qemu-iotests/check  |   1 -
 tests/qemu-iotests/common.rc  |   2 +-
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/iotests.py |  12 +-
 38 files changed, 2645 insertions(+), 517 deletions(-)
 create mode 100755 tests/qemu-iotests/209
 create mode 100644 tests/qemu-iotests/209.out

Re: [Qemu-block] [PULL 00/41] Block layer patches

[Kevin Wolf]

Am 15.03.2018 um 17:42 hat Peter Maydell geschrieben:
> On 13 March 2018 at 16:17, Kevin Wolf  wrote:
> > The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:
> >
> >   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' 
> > into staging (2018-03-13 11:42:45 +)
> >
> > are available in the git repository at:
> >
> >   git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:
> >
> >   block/mirror: change the semantic of 'force' of block-job-cancel 
> > (2018-03-13 16:54:47 +0100)
> >
> > 
> > Block layer patches
> >
> > 
> 
> I get a compile failure here on some hosts:
> 
> /home/pm215/qemu/blockjob.c: In function ‘block_job_txn_apply.isra.8’:
> /home/pm215/qemu/blockjob.c:524:5: error: ‘rc’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>  return rc;
>  ^
> 
> I guess the compiler can't always figure out whether there is
> guaranteed to be at least one thing in the list ?

I think so.

John, I'll just modify your patch 'blockjobs: add prepare callback' to
initialise rc = 0 in this function and send a v2 pull request.

Kevin

Re: [Qemu-block] [PATCH] iscsi: fix iSER compilation

[Kevin Wolf]

Am 15.03.2018 um 15:30 hat Paolo Bonzini geschrieben:
> This fails in Fedora 28.
> 
> Reported-by: Andreas Schwab 
> Signed-off-by: Paolo Bonzini 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-block] [PULL 00/41] Block layer patches

[Peter Maydell]

On 13 March 2018 at 16:17, Kevin Wolf  wrote:
> The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:
>
>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into 
> staging (2018-03-13 11:42:45 +)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:
>
>   block/mirror: change the semantic of 'force' of block-job-cancel 
> (2018-03-13 16:54:47 +0100)
>
> 
> Block layer patches
>
> 

I get a compile failure here on some hosts:

/home/pm215/qemu/blockjob.c: In function ‘block_job_txn_apply.isra.8’:
/home/pm215/qemu/blockjob.c:524:5: error: ‘rc’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
 return rc;
 ^

I guess the compiler can't always figure out whether there is
guaranteed to be at least one thing in the list ?

thanks
-- PMM

Re: [Qemu-block] [PATCH] block: Fix leak of ignore_children in error path

[Kevin Wolf]

Am 15.03.2018 um 04:51 hat Fam Zheng geschrieben:
> Reported-by: Max Reitz 
> Signed-off-by: Fam Zheng 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-block] [PATCH] vvfat: Fix inherit_options flags

[Kevin Wolf]

Am 15.03.2018 um 04:45 hat Fam Zheng geschrieben:
> Overriding flags violates the precedence rules of
> bdrv_reopen_queue_child. Just like the read-only option, no-flush should
> be put into the options. The same is done in bdrv_temp_snapshot_options.
> 
> Reported-by: Stefan Hajnoczi  Signed-off-by: Fam Zheng 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH v2 for-2.12] iotests: Avoid realpath, for CentOS 6

[Philippe Mathieu-Daudé]

On 03/15/2018 12:51 PM, Eric Blake wrote:
> CentOS 6 lacks a realpath binary on the base install, which makes
> all iotests runs fail since the 2.11 release:
> 
> 001 - output mismatch (see 001.out.bad)
> ./check: line 815: realpath: command not found
> diff: missing operand after `/home/dummy/qemu/tests/qemu-iotests/001.out'
> diff: Try `diff --help' for more information.
> 
> Many of the uses of 'realpath' in the check script were being
> used on the output of 'type -p' - but that is already an
> absolute file name.  While a canonical name can often be
> shorter (realpath gets rid of /../), it can also be longer (due
> to symlink expansion); and we really don't care if the name is
> canonical, merely that it was an executable file with an
> absolute path.  These were broken in commit cceaf1db.
> 
> The remaining use of realpath was to convert a possibly relative
> filename into an absolute one before calling diff to make it
> easier to copy-and-paste the filename for moving the .bad file
> into place as the new reference file even when running iotests
> out-of-tree (see commit 93e53fb6), but $PWD can achieve the same
> purpose.
> 
> Signed-off-by: Eric Blake 

TIL type -p :)

Reviewed-by: Philippe Mathieu-Daudé 

> 
> ---
> v2: Don't revert 93e53fb6, add commit id mentions in commit
> message, retitle
> ---
>  tests/qemu-iotests/check | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 469142cd586..ec8033350d1 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -92,7 +92,7 @@ set_prog_path()
>  {
>  p=`command -v $1 2> /dev/null`
>  if [ -n "$p" -a -x "$p" ]; then
> -realpath -- "$(type -p "$p")"
> +type -p "$p"
>  else
>  return 1
>  fi
> @@ -554,7 +554,7 @@ then
>  [ "$QEMU_PROG" = "" ] && _init_error "qemu not found"
>  fi
>  fi
> -export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
> +export QEMU_PROG="$(type -p "$QEMU_PROG")"
> 
>  if [ -z "$QEMU_IMG_PROG" ]; then
>  if [ -x "$build_iotests/qemu-img" ]; then
> @@ -565,7 +565,7 @@ if [ -z "$QEMU_IMG_PROG" ]; then
>  _init_error "qemu-img not found"
>  fi
>  fi
> -export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
> +export QEMU_IMG_PROG="$(type -p "$QEMU_IMG_PROG")"
> 
>  if [ -z "$QEMU_IO_PROG" ]; then
>  if [ -x "$build_iotests/qemu-io" ]; then
> @@ -576,7 +576,7 @@ if [ -z "$QEMU_IO_PROG" ]; then
>  _init_error "qemu-io not found"
>  fi
>  fi
> -export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
> +export QEMU_IO_PROG="$(type -p "$QEMU_IO_PROG")"
> 
>  if [ -z $QEMU_NBD_PROG ]; then
>  if [ -x "$build_iotests/qemu-nbd" ]; then
> @@ -587,7 +587,7 @@ if [ -z $QEMU_NBD_PROG ]; then
>  _init_error "qemu-nbd not found"
>  fi
>  fi
> -export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
> +export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"
> 
>  if [ -z "$QEMU_VXHS_PROG" ]; then
>export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
> @@ -811,7 +811,7 @@ do
>  else
>  echo " - output mismatch (see $seq.out.bad)"
>  mv $tmp.out $seq.out.bad
> -$diff -w "$reference" $(realpath $seq.out.bad)
> +$diff -w "$reference" "$PWD"/$seq.out.bad
>  err=true
>  fi
>  fi
> 

Re: [Qemu-block] [PATCH v2 for-2.12] iotests: Avoid realpath, for CentOS 6

[Jeff Cody]

On Thu, Mar 15, 2018 at 06:51:44AM -0500, Eric Blake wrote:
> CentOS 6 lacks a realpath binary on the base install, which makes
> all iotests runs fail since the 2.11 release:
> 
> 001 - output mismatch (see 001.out.bad)
> ./check: line 815: realpath: command not found
> diff: missing operand after `/home/dummy/qemu/tests/qemu-iotests/001.out'
> diff: Try `diff --help' for more information.
> 
> Many of the uses of 'realpath' in the check script were being
> used on the output of 'type -p' - but that is already an
> absolute file name.  While a canonical name can often be
> shorter (realpath gets rid of /../), it can also be longer (due
> to symlink expansion); and we really don't care if the name is
> canonical, merely that it was an executable file with an
> absolute path.  These were broken in commit cceaf1db.
> 
> The remaining use of realpath was to convert a possibly relative
> filename into an absolute one before calling diff to make it
> easier to copy-and-paste the filename for moving the .bad file
> into place as the new reference file even when running iotests
> out-of-tree (see commit 93e53fb6), but $PWD can achieve the same
> purpose.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v2: Don't revert 93e53fb6, add commit id mentions in commit
> message, retitle
> ---
>  tests/qemu-iotests/check | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 469142cd586..ec8033350d1 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -92,7 +92,7 @@ set_prog_path()
>  {
>  p=`command -v $1 2> /dev/null`
>  if [ -n "$p" -a -x "$p" ]; then
> -realpath -- "$(type -p "$p")"
> +type -p "$p"
>  else
>  return 1
>  fi
> @@ -554,7 +554,7 @@ then
>  [ "$QEMU_PROG" = "" ] && _init_error "qemu not found"
>  fi
>  fi
> -export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
> +export QEMU_PROG="$(type -p "$QEMU_PROG")"
> 
>  if [ -z "$QEMU_IMG_PROG" ]; then
>  if [ -x "$build_iotests/qemu-img" ]; then
> @@ -565,7 +565,7 @@ if [ -z "$QEMU_IMG_PROG" ]; then
>  _init_error "qemu-img not found"
>  fi
>  fi
> -export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
> +export QEMU_IMG_PROG="$(type -p "$QEMU_IMG_PROG")"
> 
>  if [ -z "$QEMU_IO_PROG" ]; then
>  if [ -x "$build_iotests/qemu-io" ]; then
> @@ -576,7 +576,7 @@ if [ -z "$QEMU_IO_PROG" ]; then
>  _init_error "qemu-io not found"
>  fi
>  fi
> -export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
> +export QEMU_IO_PROG="$(type -p "$QEMU_IO_PROG")"
> 
>  if [ -z $QEMU_NBD_PROG ]; then
>  if [ -x "$build_iotests/qemu-nbd" ]; then
> @@ -587,7 +587,7 @@ if [ -z $QEMU_NBD_PROG ]; then
>  _init_error "qemu-nbd not found"
>  fi
>  fi
> -export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
> +export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"
> 
>  if [ -z "$QEMU_VXHS_PROG" ]; then
>export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
> @@ -811,7 +811,7 @@ do
>  else
>  echo " - output mismatch (see $seq.out.bad)"
>  mv $tmp.out $seq.out.bad
> -$diff -w "$reference" $(realpath $seq.out.bad)
> +$diff -w "$reference" "$PWD"/$seq.out.bad
>  err=true
>  fi
>  fi
> -- 
> 2.14.3
> 
> 

Reviewed-by: Jeff Cody 

[Qemu-block] [PATCH] iscsi: fix iSER compilation

[Paolo Bonzini]

This fails in Fedora 28.

Reported-by: Andreas Schwab 
Signed-off-by: Paolo Bonzini 
---
 block/iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index a82170f16e..f5aecfc883 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2244,7 +2244,7 @@ static BlockDriver bdrv_iser = {
 .create_opts= _create_opts,
 .bdrv_reopen_prepare= iscsi_reopen_prepare,
 .bdrv_reopen_commit = iscsi_reopen_commit,
-.bdrv_invalidate_cache  = iscsi_invalidate_cache,
+.bdrv_co_invalidate_cache  = iscsi_co_invalidate_cache,
 
 .bdrv_getlength  = iscsi_getlength,
 .bdrv_get_info   = iscsi_get_info,
-- 
2.14.3

Re: [Qemu-block] [PATCH v2 for-2.12] iotests: Avoid realpath, for CentOS 6

[Paolo Bonzini]

On 15/03/2018 12:51, Eric Blake wrote:
> CentOS 6 lacks a realpath binary on the base install, which makes
> all iotests runs fail since the 2.11 release:
> 
> 001 - output mismatch (see 001.out.bad)
> ./check: line 815: realpath: command not found
> diff: missing operand after `/home/dummy/qemu/tests/qemu-iotests/001.out'
> diff: Try `diff --help' for more information.
> 
> Many of the uses of 'realpath' in the check script were being
> used on the output of 'type -p' - but that is already an
> absolute file name.  While a canonical name can often be
> shorter (realpath gets rid of /../), it can also be longer (due
> to symlink expansion); and we really don't care if the name is
> canonical, merely that it was an executable file with an
> absolute path.  These were broken in commit cceaf1db.
> 
> The remaining use of realpath was to convert a possibly relative
> filename into an absolute one before calling diff to make it
> easier to copy-and-paste the filename for moving the .bad file
> into place as the new reference file even when running iotests
> out-of-tree (see commit 93e53fb6), but $PWD can achieve the same
> purpose.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v2: Don't revert 93e53fb6, add commit id mentions in commit
> message, retitle
> ---
>  tests/qemu-iotests/check | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 469142cd586..ec8033350d1 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -92,7 +92,7 @@ set_prog_path()
>  {
>  p=`command -v $1 2> /dev/null`
>  if [ -n "$p" -a -x "$p" ]; then
> -realpath -- "$(type -p "$p")"
> +type -p "$p"
>  else
>  return 1
>  fi
> @@ -554,7 +554,7 @@ then
>  [ "$QEMU_PROG" = "" ] && _init_error "qemu not found"
>  fi
>  fi
> -export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
> +export QEMU_PROG="$(type -p "$QEMU_PROG")"
> 
>  if [ -z "$QEMU_IMG_PROG" ]; then
>  if [ -x "$build_iotests/qemu-img" ]; then
> @@ -565,7 +565,7 @@ if [ -z "$QEMU_IMG_PROG" ]; then
>  _init_error "qemu-img not found"
>  fi
>  fi
> -export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
> +export QEMU_IMG_PROG="$(type -p "$QEMU_IMG_PROG")"
> 
>  if [ -z "$QEMU_IO_PROG" ]; then
>  if [ -x "$build_iotests/qemu-io" ]; then
> @@ -576,7 +576,7 @@ if [ -z "$QEMU_IO_PROG" ]; then
>  _init_error "qemu-io not found"
>  fi
>  fi
> -export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
> +export QEMU_IO_PROG="$(type -p "$QEMU_IO_PROG")"
> 
>  if [ -z $QEMU_NBD_PROG ]; then
>  if [ -x "$build_iotests/qemu-nbd" ]; then
> @@ -587,7 +587,7 @@ if [ -z $QEMU_NBD_PROG ]; then
>  _init_error "qemu-nbd not found"
>  fi
>  fi
> -export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
> +export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"
> 
>  if [ -z "$QEMU_VXHS_PROG" ]; then
>export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
> @@ -811,7 +811,7 @@ do
>  else
>  echo " - output mismatch (see $seq.out.bad)"
>  mv $tmp.out $seq.out.bad
> -$diff -w "$reference" $(realpath $seq.out.bad)
> +$diff -w "$reference" "$PWD"/$seq.out.bad
>  err=true
>  fi
>  fi
> 

Reviewed-by: Paolo Bonzini 

Re: [Qemu-block] [PATCH 5/7] block: convert bdrv_invalidate_cache callback to coroutine_fn

[Andreas Schwab]

block/iscsi.c:2247:6: error: 'BlockDriver {aka struct BlockDriver}' has no 
member named 'bdrv_invalidate_cache'; did you mean 'bdrv_co_invalidate_cache'?
 .bdrv_invalidate_cache  = iscsi_invalidate_cache,
  ^
  bdrv_co_invalidate_cache
block/iscsi.c:2247:31: error: 'iscsi_invalidate_cache' undeclared here (not in 
a function); did you mean 'iscsi_co_invalidate_cache'?
 .bdrv_invalidate_cache  = iscsi_invalidate_cache,
   ^~
   iscsi_co_invalidate_cache

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [Qemu-block] [PATCH] block: Fix leak of ignore_children in error path

[Alberto Garcia]

On Thu 15 Mar 2018 04:51:57 AM CET, Fam Zheng wrote:
> Reported-by: Max Reitz 
> Signed-off-by: Fam Zheng 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix leak of ignore_children in error path

[Eric Blake]

On 03/14/2018 10:51 PM, Fam Zheng wrote:

Reported-by: Max Reitz 
Signed-off-by: Fam Zheng 
---
  block.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Eric Blake 



diff --git a/block.c b/block.c
index 75a9fd49de..c1fda9fd57 100644
--- a/block.c
+++ b/block.c
@@ -3671,12 +3671,12 @@ int bdrv_drop_intermediate(BlockDriverState *top, 
BlockDriverState *base,
  GSList *ignore_children = g_slist_prepend(NULL, c);
  bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm,
 ignore_children, _err);
+g_slist_free(ignore_children);
  if (local_err) {
  ret = -EPERM;
  error_report_err(local_err);
  goto exit;
  }
-g_slist_free(ignore_children);
  
  /* If so, update the backing file path in the image file */

  if (c->role->update_filename) {



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [Qemu-devel] [PATCH] vvfat: Fix inherit_options flags

[Eric Blake]

On 03/14/2018 10:45 PM, Fam Zheng wrote:

Overriding flags violates the precedence rules of
bdrv_reopen_queue_child. Just like the read-only option, no-flush should
be put into the options. The same is done in bdrv_temp_snapshot_options.

Reported-by: Stefan Hajnoczi 
---
  block/vvfat.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Eric Blake 


diff --git a/block/vvfat.c b/block/vvfat.c
index 4a17a49e12..1569783b0f 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3129,7 +3129,7 @@ static void vvfat_qcow_options(int *child_flags, QDict 
*child_options,
 int parent_flags, QDict *parent_options)
  {
  qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "off");
-*child_flags = BDRV_O_NO_FLUSH;
+qdict_set_default_str(child_options, BDRV_OPT_CACHE_NO_FLUSH, "on");
  }
  
  static const BdrvChildRole child_vvfat_qcow = {




--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working

[Eric Blake]

On 03/15/2018 02:49 AM, Thomas Huth wrote:

Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
on a couple of boards without that we noticed it immediately. Thus


s/without that we noticed/without us noticing/


add a test which checks that "-cdrom" can at least be used to start
QEMU with certain machine types.

Signed-off-by: Thomas Huth 
---
  tests/Makefile.include |  7 +-
  tests/cdrom-test.c | 63 +-
  2 files changed, 68 insertions(+), 2 deletions(-)


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file

[Eric Blake]

On 03/15/2018 05:48 AM, Thomas Huth wrote:


+pid = fork();
+if (pid == 0) {
+va_start(args, fmt);
+params = g_strdup_vprintf(fmt, args);
+va_end(args);
+command = g_strdup_printf("exec genisoimage %s", params);
+g_free(params);
+execlp("/bin/sh", "sh", "-c", command, NULL);
+exit(1);
+}
+wait();


IMHO this should just use g_spawn_sync(), also the use of
shell seems rather unneccessary


and potentially dangerous - if we aren't absolutely positive that we 
aren't going to improperly expand shell metacharacters embedded in params.



- why not just run genisoimage
directly ?


That code was simply "inspired" from the execlp() code in
qtest_init_without_qmp_handshake()


Sounds like a good idea for a future cleanup patch ;)

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-block] [PATCH v2 for-2.12] iotests: Avoid realpath, for CentOS 6

[Eric Blake]

CentOS 6 lacks a realpath binary on the base install, which makes
all iotests runs fail since the 2.11 release:

001 - output mismatch (see 001.out.bad)
./check: line 815: realpath: command not found
diff: missing operand after `/home/dummy/qemu/tests/qemu-iotests/001.out'
diff: Try `diff --help' for more information.

Many of the uses of 'realpath' in the check script were being
used on the output of 'type -p' - but that is already an
absolute file name.  While a canonical name can often be
shorter (realpath gets rid of /../), it can also be longer (due
to symlink expansion); and we really don't care if the name is
canonical, merely that it was an executable file with an
absolute path.  These were broken in commit cceaf1db.

The remaining use of realpath was to convert a possibly relative
filename into an absolute one before calling diff to make it
easier to copy-and-paste the filename for moving the .bad file
into place as the new reference file even when running iotests
out-of-tree (see commit 93e53fb6), but $PWD can achieve the same
purpose.

Signed-off-by: Eric Blake 

---
v2: Don't revert 93e53fb6, add commit id mentions in commit
message, retitle
---
 tests/qemu-iotests/check | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 469142cd586..ec8033350d1 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -92,7 +92,7 @@ set_prog_path()
 {
 p=`command -v $1 2> /dev/null`
 if [ -n "$p" -a -x "$p" ]; then
-realpath -- "$(type -p "$p")"
+type -p "$p"
 else
 return 1
 fi
@@ -554,7 +554,7 @@ then
 [ "$QEMU_PROG" = "" ] && _init_error "qemu not found"
 fi
 fi
-export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
+export QEMU_PROG="$(type -p "$QEMU_PROG")"

 if [ -z "$QEMU_IMG_PROG" ]; then
 if [ -x "$build_iotests/qemu-img" ]; then
@@ -565,7 +565,7 @@ if [ -z "$QEMU_IMG_PROG" ]; then
 _init_error "qemu-img not found"
 fi
 fi
-export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
+export QEMU_IMG_PROG="$(type -p "$QEMU_IMG_PROG")"

 if [ -z "$QEMU_IO_PROG" ]; then
 if [ -x "$build_iotests/qemu-io" ]; then
@@ -576,7 +576,7 @@ if [ -z "$QEMU_IO_PROG" ]; then
 _init_error "qemu-io not found"
 fi
 fi
-export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
+export QEMU_IO_PROG="$(type -p "$QEMU_IO_PROG")"

 if [ -z $QEMU_NBD_PROG ]; then
 if [ -x "$build_iotests/qemu-nbd" ]; then
@@ -587,7 +587,7 @@ if [ -z $QEMU_NBD_PROG ]; then
 _init_error "qemu-nbd not found"
 fi
 fi
-export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
+export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"

 if [ -z "$QEMU_VXHS_PROG" ]; then
   export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
@@ -811,7 +811,7 @@ do
 else
 echo " - output mismatch (see $seq.out.bad)"
 mv $tmp.out $seq.out.bad
-$diff -w "$reference" $(realpath $seq.out.bad)
+$diff -w "$reference" "$PWD"/$seq.out.bad
 err=true
 fi
 fi
-- 
2.14.3

Re: [Qemu-block] [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Philippe Mathieu-Daudé]

On 03/15/2018 08:49 AM, Thomas Huth wrote:
> We're going to use the s390x boot code for testing CD-ROM booting.
> But the ISO loader of the s390-ccw bios is a little bit more picky
> than the network loader and expects some magic bytes in the header
> of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
> we've got to add them in our boot code here, too.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/boot-sector.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index c373f0e..04721c9 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
>  };
>  
>  /* For s390x, use a mini "kernel" with the appropriate signature */
> -static const uint8_t s390x_psw[] = {
> -0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
> +static const uint8_t s390x_psw_and_magic[] = {

Can you add a comment such "see linux_s390_magic in
pc-bios/s390-ccw/bootmap.c"?

> +0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,
> +0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
> +0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
> +0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
>  };
>  static const uint8_t s390x_code[] = {
>  0xa7, 0xf4, 0x00, 0x0a,/* j 0x10010 */
> @@ -110,7 +113,7 @@ int boot_sector_init(char *fname)
>  } else if (g_str_equal(arch, "s390x")) {
>  len = 0x1 + sizeof(s390x_code);
>  boot_code = g_malloc0(len);
> -memcpy(boot_code, s390x_psw, sizeof(s390x_psw));
> +memcpy(boot_code, s390x_psw_and_magic, sizeof(s390x_psw_and_magic));
>  memcpy(_code[0x1], s390x_code, sizeof(s390x_code));
>  } else {
>  g_assert_not_reached();
> 

Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working

[Philippe Mathieu-Daudé]

Hi Thomas,

On 03/15/2018 08:49 AM, Thomas Huth wrote:
> Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
> on a couple of boards without that we noticed it immediately. Thus
> add a test which checks that "-cdrom" can at least be used to start
> QEMU with certain machine types.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include |  7 +-
>  tests/cdrom-test.c | 63 
> +-
>  2 files changed, 68 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a104222..b744fea 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -321,8 +321,9 @@ check-qtest-microblaze-y = tests/boot-serial-test$(EXESUF)
>  check-qtest-mips-y = tests/endianness-test$(EXESUF)
>  
>  check-qtest-mips64-y = tests/endianness-test$(EXESUF)
> +check-qtest-mips64-y += tests/cdrom-test$(EXESUF)
>  
> -check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
> +check-qtest-mips64el-y = $(check-qtest-mips64-y)
>  
>  check-qtest-moxie-y = tests/boot-serial-test$(EXESUF)
>  
> @@ -356,6 +357,7 @@ check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
>  check-qtest-ppc64-y += tests/numa-test$(EXESUF)
>  check-qtest-ppc64-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
>  check-qtest-ppc64-y += tests/cpu-plug-test$(EXESUF)
> +check-qtest-ppc64-y += tests/cdrom-test$(EXESUF)
>  
>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>  
> @@ -365,10 +367,12 @@ check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
>  check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
>  gcov-files-sparc-y = hw/timer/m48t59.c
>  check-qtest-sparc-y += tests/boot-serial-test$(EXESUF)
> +check-qtest-sparc-y += tests/cdrom-test$(EXESUF)
>  
>  check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
>  check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
>  check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
> +check-qtest-sparc64-y += tests/cdrom-test$(EXESUF)
>  
>  check-qtest-arm-y = tests/tmp105-test$(EXESUF)
>  check-qtest-arm-y += tests/ds1338-test$(EXESUF)
> @@ -384,6 +388,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
>  check-qtest-aarch64-y = tests/numa-test$(EXESUF)
>  check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
>  check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
> +check-qtest-aarch64-y += tests/cdrom-test$(EXESUF)

Since you use g_str_equal(arch,), maybe this test can be added
regardless the arch in check-qtest-generic.

>  
>  check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
>  
> diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
> index 1fc5230..5e516dd 100644
> --- a/tests/cdrom-test.c
> +++ b/tests/cdrom-test.c
> @@ -13,6 +13,7 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "boot-sector.h"
> +#include "qapi/qmp/qdict.h"
>  
>  static char isoimage[] = "cdrom-boot-iso-XX";
>  
> @@ -62,7 +63,13 @@ static int prepare_image(const char *arch, char *isoimage)
>  goto cleanup;
>  }
>  } else {
> -g_assert_not_reached();
> +/* Just create a dummy file */
> +char txt[] = "empty disc";
> +codefile = g_strdup_printf("%s/readme.txt", srcdir);
> +if (!g_file_set_contents(codefile, txt, sizeof(txt) - 1, NULL)) {
> +fprintf(stderr, "Failed to create '%s'\n", codefile);
> +goto cleanup;
> +}
>  }
>  
>  ret = gen_iso("-quiet -l -no-emul-boot -b %s -o %s %s",
> @@ -81,6 +88,32 @@ cleanup:
>  return ret;
>  }
>  
> +/**
> + * Check that at least the -cdrom parameter is basically working, i.e. we can
> + * see the filename of the ISO image in the output of "info block" afterwards
> + */
> +static void test_cdrom_param(gconstpointer data)
> +{
> +QTestState *qts;
> +char *resp;
> +
> +qts = qtest_startf("-M %s -cdrom %s", (const char *)data, isoimage);
> +resp = qtest_hmp(qts, "info block");
> +g_assert(strstr(resp, isoimage) != 0);
> +g_free(resp);
> +qtest_quit(qts);
> +}
> +
> +static void add_cdrom_param_tests(const char **machines)
> +{
> +while (*machines) {
> +char *testname = g_strdup_printf("cdrom/param/%s", *machines);
> +qtest_add_data_func(testname, *machines, test_cdrom_param);
> +g_free(testname);
> +machines++;
> +}
> +}
> +
>  static void test_cdboot(gconstpointer data)
>  {
>  QTestState *qts;
> @@ -145,6 +178,34 @@ int main(int argc, char **argv)
>  add_x86_tests();
>  } else if (g_str_equal(arch, "s390x")) {
>  add_s390x_tests();
> +} else if (g_str_equal(arch, "ppc64")) {
> +const char *ppcmachines[] = {
> +"pseries", "mac99", "g3beige", "40p", "prep", NULL
> +};
> +add_cdrom_param_tests(ppcmachines);
> +} else if (g_str_equal(arch, "sparc")) {
> +const char *sparcmachines[] = {
> +"LX", "SPARCClassic", "SPARCbook", "SS-10", "SS-20", "SS-4",
> +"SS-5", 

Re: [Qemu-block] [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file

[Thomas Huth]

On 15.03.2018 10:21, Daniel P. Berrangé wrote:
> On Thu, Mar 15, 2018 at 08:49:04AM +0100, Thomas Huth wrote:
>> We already have the code for a boot file in tests/boot-sector.c,
>> so if the genisoimage program is available, we can easily create
>> a bootable CD ISO image that we can use for testing whether our
>> CD-ROM emulation and the BIOS CD-ROM boot works correctly.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  tests/Makefile.include |   3 +
>>  tests/cdrom-test.c | 155 
>> +
>>  2 files changed, 158 insertions(+)
>>  create mode 100644 tests/cdrom-test.c
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index ef9b88c..a104222 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -304,6 +304,7 @@ check-qtest-i386-$(CONFIG_POSIX) += 
>> tests/test-filter-redirector$(EXESUF)
>>  check-qtest-i386-y += tests/migration-test$(EXESUF)
>>  check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
>>  check-qtest-i386-y += tests/numa-test$(EXESUF)
>> +check-qtest-i386-y += tests/cdrom-test$(EXESUF)
>>  check-qtest-x86_64-y += $(check-qtest-i386-y)
>>  check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
>>  gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
>> @@ -398,6 +399,7 @@ check-qtest-s390x-y += tests/virtio-balloon-test$(EXESUF)
>>  check-qtest-s390x-y += tests/virtio-console-test$(EXESUF)
>>  check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF)
>>  check-qtest-s390x-y += tests/cpu-plug-test$(EXESUF)
>> +check-qtest-s390x-y += tests/cdrom-test$(EXESUF)
>>  
>>  check-qtest-generic-y += tests/qom-test$(EXESUF)
>>  check-qtest-generic-y += tests/test-hmp$(EXESUF)
>> @@ -829,6 +831,7 @@ tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o 
>> $(test-util-obj-y)
>>  tests/numa-test$(EXESUF): tests/numa-test.o
>>  tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o 
>> tests/acpi-utils.o
>>  tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
>> +tests/cdrom-test$(EXESUF): tests/cdrom-test.o tests/boot-sector.o 
>> $(libqos-obj-y)
>>  
>>  tests/migration/stress$(EXESUF): tests/migration/stress.o
>>  $(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< 
>> ,"LINK","$(TARGET_DIR)$@")
>> diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
>> new file mode 100644
>> index 000..1fc5230
>> --- /dev/null
>> +++ b/tests/cdrom-test.c
>> @@ -0,0 +1,155 @@
>> +/*
>> + * Various tests for emulated CD-ROM drives.
>> + *
>> + * Copyright (c) 2018 Red Hat Inc.
>> + *
>> + * Author:
>> + *Thomas Huth 
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2
>> + * or later. See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "libqtest.h"
>> +#include "boot-sector.h"
>> +
>> +static char isoimage[] = "cdrom-boot-iso-XX";
>> +
>> +static int gen_iso(const char *fmt, ...)
>> +{
>> +char *params, *command;
>> +va_list args;
>> +pid_t pid;
>> +int status;
>> +
>> +pid = fork();
>> +if (pid == 0) {
>> +va_start(args, fmt);
>> +params = g_strdup_vprintf(fmt, args);
>> +va_end(args);
>> +command = g_strdup_printf("exec genisoimage %s", params);
>> +g_free(params);
>> +execlp("/bin/sh", "sh", "-c", command, NULL);
>> +exit(1);
>> +}
>> +wait();
> 
> IMHO this should just use g_spawn_sync(), also the use of
> shell seems rather unneccessary - why not just run genisoimage
> directly ?

That code was simply "inspired" from the execlp() code in
qtest_init_without_qmp_handshake() ... but g_spawn_sync() sounds like a
good alternative here, so I'll try to switch to that one instead.

 Thanks,
  Thomas

Re: [Qemu-block] [PULL 0/1] Block patches

[Peter Maydell]

On 13 March 2018 at 12:29, Jeff Cody  wrote:
> The following changes since commit 834eddf22ec762839b724538c7be1d1d3b2d9d3b:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' 
> into staging (2018-03-13 10:49:02 +)
>
> are available in the git repository at:
>
>   git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
>
> for you to fetch changes up to 44acd46f60ce6f16d369cd443e77949deca56a2c:
>
>   block: include original filename when reporting invalid URIs (2018-03-13 
> 08:06:55 -0400)
>
> 
> Block patch
> 
>
> Daniel P. Berrangé (1):
>   block: include original filename when reporting invalid URIs
>
>  block/gluster.c  | 2 +-
>  block/sheepdog.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

Applied, thanks.

-- PMM

Re: [Qemu-block] [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file

[Daniel P . Berrangé]

On Thu, Mar 15, 2018 at 08:49:04AM +0100, Thomas Huth wrote:
> We already have the code for a boot file in tests/boot-sector.c,
> so if the genisoimage program is available, we can easily create
> a bootable CD ISO image that we can use for testing whether our
> CD-ROM emulation and the BIOS CD-ROM boot works correctly.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include |   3 +
>  tests/cdrom-test.c | 155 
> +
>  2 files changed, 158 insertions(+)
>  create mode 100644 tests/cdrom-test.c
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index ef9b88c..a104222 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -304,6 +304,7 @@ check-qtest-i386-$(CONFIG_POSIX) += 
> tests/test-filter-redirector$(EXESUF)
>  check-qtest-i386-y += tests/migration-test$(EXESUF)
>  check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
>  check-qtest-i386-y += tests/numa-test$(EXESUF)
> +check-qtest-i386-y += tests/cdrom-test$(EXESUF)
>  check-qtest-x86_64-y += $(check-qtest-i386-y)
>  check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
>  gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
> @@ -398,6 +399,7 @@ check-qtest-s390x-y += tests/virtio-balloon-test$(EXESUF)
>  check-qtest-s390x-y += tests/virtio-console-test$(EXESUF)
>  check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF)
>  check-qtest-s390x-y += tests/cpu-plug-test$(EXESUF)
> +check-qtest-s390x-y += tests/cdrom-test$(EXESUF)
>  
>  check-qtest-generic-y += tests/qom-test$(EXESUF)
>  check-qtest-generic-y += tests/test-hmp$(EXESUF)
> @@ -829,6 +831,7 @@ tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o 
> $(test-util-obj-y)
>  tests/numa-test$(EXESUF): tests/numa-test.o
>  tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o 
> tests/acpi-utils.o
>  tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
> +tests/cdrom-test$(EXESUF): tests/cdrom-test.o tests/boot-sector.o 
> $(libqos-obj-y)
>  
>  tests/migration/stress$(EXESUF): tests/migration/stress.o
>   $(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< 
> ,"LINK","$(TARGET_DIR)$@")
> diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
> new file mode 100644
> index 000..1fc5230
> --- /dev/null
> +++ b/tests/cdrom-test.c
> @@ -0,0 +1,155 @@
> +/*
> + * Various tests for emulated CD-ROM drives.
> + *
> + * Copyright (c) 2018 Red Hat Inc.
> + *
> + * Author:
> + *Thomas Huth 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2
> + * or later. See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "boot-sector.h"
> +
> +static char isoimage[] = "cdrom-boot-iso-XX";
> +
> +static int gen_iso(const char *fmt, ...)
> +{
> +char *params, *command;
> +va_list args;
> +pid_t pid;
> +int status;
> +
> +pid = fork();
> +if (pid == 0) {
> +va_start(args, fmt);
> +params = g_strdup_vprintf(fmt, args);
> +va_end(args);
> +command = g_strdup_printf("exec genisoimage %s", params);
> +g_free(params);
> +execlp("/bin/sh", "sh", "-c", command, NULL);
> +exit(1);
> +}
> +wait();

IMHO this should just use g_spawn_sync(), also the use of
shell seems rather unneccessary - why not just run genisoimage
directly ?

  
https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#g-spawn-sync

> +
> +return WEXITSTATUS(status);
> +}
> +
> +static int prepare_image(const char *arch, char *isoimage)
> +{
> +char srcdir[] = "cdrom-test-dir-XX";
> +char *codefile = NULL;
> +int ifh, ret = -1;
> +
> +ifh = mkstemp(isoimage);
> +if (ifh < 0) {
> +perror("Error creating temporary iso image file");
> +return -1;
> +}
> +if (!mkdtemp(srcdir)) {
> +perror("Error creating temporary directory");
> +goto cleanup;
> +}
> +
> +if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64") ||
> +g_str_equal(arch, "s390x")) {
> +codefile = g_strdup_printf("%s/bootcode-XX", srcdir);
> +ret = boot_sector_init(codefile);
> +if (ret) {
> +goto cleanup;
> +}
> +} else {
> +g_assert_not_reached();
> +}
> +
> +ret = gen_iso("-quiet -l -no-emul-boot -b %s -o %s %s",
> +  strrchr(codefile, '/') + 1, isoimage, srcdir);

It would be easy to just declare the args as an array here

  char *genisoargv[] = {
"genisoimage", "-quiet", "-l", "-no-emul-boot", "-b",
strrchr(codefile, "/") + 1, "-o", isoimage, srcdir,
NULL,
  };

to then pass to g_spawn_sync

> +if (ret) {
> +fprintf(stderr, "genisoimage failed: %i\n", ret);
> +}
> +
> +unlink(codefile);
> +
> +cleanup:
> +g_free(codefile);
> +rmdir(srcdir);
> +close(ifh);
> +
> +return ret;
> +}

Regards,
Daniel

Re: [Qemu-block] [qemu-s390x] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Christian Borntraeger]

On 03/15/2018 08:49 AM, Thomas Huth wrote:
> We're going to use the s390x boot code for testing CD-ROM booting.
> But the ISO loader of the s390-ccw bios is a little bit more picky
> than the network loader and expects some magic bytes in the header
> of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
> we've got to add them in our boot code here, too.
> 
> Signed-off-by: Thomas Huth 

Reviewed-by: Christian Borntraeger 
> ---
>  tests/boot-sector.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index c373f0e..04721c9 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
>  };
> 
>  /* For s390x, use a mini "kernel" with the appropriate signature */
> -static const uint8_t s390x_psw[] = {
> -0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
> +static const uint8_t s390x_psw_and_magic[] = {
> +0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,
> +0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
> +0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
> +0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
>  };
>  static const uint8_t s390x_code[] = {
>  0xa7, 0xf4, 0x00, 0x0a,/* j 0x10010 */
> @@ -110,7 +113,7 @@ int boot_sector_init(char *fname)
>  } else if (g_str_equal(arch, "s390x")) {
>  len = 0x1 + sizeof(s390x_code);
>  boot_code = g_malloc0(len);
> -memcpy(boot_code, s390x_psw, sizeof(s390x_psw));
> +memcpy(boot_code, s390x_psw_and_magic, sizeof(s390x_psw_and_magic));
>  memcpy(_code[0x1], s390x_code, sizeof(s390x_code));
>  } else {
>  g_assert_not_reached();
> 

[Qemu-block] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working

[Thomas Huth]

Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
on a couple of boards without that we noticed it immediately. Thus
add a test which checks that "-cdrom" can at least be used to start
QEMU with certain machine types.

Signed-off-by: Thomas Huth 
---
 tests/Makefile.include |  7 +-
 tests/cdrom-test.c | 63 +-
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a104222..b744fea 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -321,8 +321,9 @@ check-qtest-microblaze-y = tests/boot-serial-test$(EXESUF)
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
 
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
+check-qtest-mips64-y += tests/cdrom-test$(EXESUF)
 
-check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
+check-qtest-mips64el-y = $(check-qtest-mips64-y)
 
 check-qtest-moxie-y = tests/boot-serial-test$(EXESUF)
 
@@ -356,6 +357,7 @@ check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
 check-qtest-ppc64-y += tests/numa-test$(EXESUF)
 check-qtest-ppc64-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
 check-qtest-ppc64-y += tests/cpu-plug-test$(EXESUF)
+check-qtest-ppc64-y += tests/cdrom-test$(EXESUF)
 
 check-qtest-sh4-y = tests/endianness-test$(EXESUF)
 
@@ -365,10 +367,12 @@ check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
 check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
 gcov-files-sparc-y = hw/timer/m48t59.c
 check-qtest-sparc-y += tests/boot-serial-test$(EXESUF)
+check-qtest-sparc-y += tests/cdrom-test$(EXESUF)
 
 check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
 check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
 check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
+check-qtest-sparc64-y += tests/cdrom-test$(EXESUF)
 
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 check-qtest-arm-y += tests/ds1338-test$(EXESUF)
@@ -384,6 +388,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
+check-qtest-aarch64-y += tests/cdrom-test$(EXESUF)
 
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 
diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
index 1fc5230..5e516dd 100644
--- a/tests/cdrom-test.c
+++ b/tests/cdrom-test.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "boot-sector.h"
+#include "qapi/qmp/qdict.h"
 
 static char isoimage[] = "cdrom-boot-iso-XX";
 
@@ -62,7 +63,13 @@ static int prepare_image(const char *arch, char *isoimage)
 goto cleanup;
 }
 } else {
-g_assert_not_reached();
+/* Just create a dummy file */
+char txt[] = "empty disc";
+codefile = g_strdup_printf("%s/readme.txt", srcdir);
+if (!g_file_set_contents(codefile, txt, sizeof(txt) - 1, NULL)) {
+fprintf(stderr, "Failed to create '%s'\n", codefile);
+goto cleanup;
+}
 }
 
 ret = gen_iso("-quiet -l -no-emul-boot -b %s -o %s %s",
@@ -81,6 +88,32 @@ cleanup:
 return ret;
 }
 
+/**
+ * Check that at least the -cdrom parameter is basically working, i.e. we can
+ * see the filename of the ISO image in the output of "info block" afterwards
+ */
+static void test_cdrom_param(gconstpointer data)
+{
+QTestState *qts;
+char *resp;
+
+qts = qtest_startf("-M %s -cdrom %s", (const char *)data, isoimage);
+resp = qtest_hmp(qts, "info block");
+g_assert(strstr(resp, isoimage) != 0);
+g_free(resp);
+qtest_quit(qts);
+}
+
+static void add_cdrom_param_tests(const char **machines)
+{
+while (*machines) {
+char *testname = g_strdup_printf("cdrom/param/%s", *machines);
+qtest_add_data_func(testname, *machines, test_cdrom_param);
+g_free(testname);
+machines++;
+}
+}
+
 static void test_cdboot(gconstpointer data)
 {
 QTestState *qts;
@@ -145,6 +178,34 @@ int main(int argc, char **argv)
 add_x86_tests();
 } else if (g_str_equal(arch, "s390x")) {
 add_s390x_tests();
+} else if (g_str_equal(arch, "ppc64")) {
+const char *ppcmachines[] = {
+"pseries", "mac99", "g3beige", "40p", "prep", NULL
+};
+add_cdrom_param_tests(ppcmachines);
+} else if (g_str_equal(arch, "sparc")) {
+const char *sparcmachines[] = {
+"LX", "SPARCClassic", "SPARCbook", "SS-10", "SS-20", "SS-4",
+"SS-5", "SS-600MP", "Voyager", "leon3_generic", NULL
+};
+add_cdrom_param_tests(sparcmachines);
+} else if (g_str_equal(arch, "sparc64")) {
+const char *sparc64machines[] = {
+"niagara", "sun4u", "sun4v", NULL
+};
+add_cdrom_param_tests(sparc64machines);
+} else if (!strncmp(arch, "mips64", 6)) {
+const char *mips64machines[] = {
+"magnum", "malta", 

[Qemu-block] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Thomas Huth]

We're going to use the s390x boot code for testing CD-ROM booting.
But the ISO loader of the s390-ccw bios is a little bit more picky
than the network loader and expects some magic bytes in the header
of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so
we've got to add them in our boot code here, too.

Signed-off-by: Thomas Huth 
---
 tests/boot-sector.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index c373f0e..04721c9 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
 };
 
 /* For s390x, use a mini "kernel" with the appropriate signature */
-static const uint8_t s390x_psw[] = {
-0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
+static const uint8_t s390x_psw_and_magic[] = {
+0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,
+0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
+0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
+0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
 };
 static const uint8_t s390x_code[] = {
 0xa7, 0xf4, 0x00, 0x0a,/* j 0x10010 */
@@ -110,7 +113,7 @@ int boot_sector_init(char *fname)
 } else if (g_str_equal(arch, "s390x")) {
 len = 0x1 + sizeof(s390x_code);
 boot_code = g_malloc0(len);
-memcpy(boot_code, s390x_psw, sizeof(s390x_psw));
+memcpy(boot_code, s390x_psw_and_magic, sizeof(s390x_psw_and_magic));
 memcpy(_code[0x1], s390x_code, sizeof(s390x_code));
 } else {
 g_assert_not_reached();
-- 
1.8.3.1

[Qemu-block] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file

[Thomas Huth]

We already have the code for a boot file in tests/boot-sector.c,
so if the genisoimage program is available, we can easily create
a bootable CD ISO image that we can use for testing whether our
CD-ROM emulation and the BIOS CD-ROM boot works correctly.

Signed-off-by: Thomas Huth 
---
 tests/Makefile.include |   3 +
 tests/cdrom-test.c | 155 +
 2 files changed, 158 insertions(+)
 create mode 100644 tests/cdrom-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index ef9b88c..a104222 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -304,6 +304,7 @@ check-qtest-i386-$(CONFIG_POSIX) += 
tests/test-filter-redirector$(EXESUF)
 check-qtest-i386-y += tests/migration-test$(EXESUF)
 check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
 check-qtest-i386-y += tests/numa-test$(EXESUF)
+check-qtest-i386-y += tests/cdrom-test$(EXESUF)
 check-qtest-x86_64-y += $(check-qtest-i386-y)
 check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
@@ -398,6 +399,7 @@ check-qtest-s390x-y += tests/virtio-balloon-test$(EXESUF)
 check-qtest-s390x-y += tests/virtio-console-test$(EXESUF)
 check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF)
 check-qtest-s390x-y += tests/cpu-plug-test$(EXESUF)
+check-qtest-s390x-y += tests/cdrom-test$(EXESUF)
 
 check-qtest-generic-y += tests/qom-test$(EXESUF)
 check-qtest-generic-y += tests/test-hmp$(EXESUF)
@@ -829,6 +831,7 @@ tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o 
$(test-util-obj-y)
 tests/numa-test$(EXESUF): tests/numa-test.o
 tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o 
tests/acpi-utils.o
 tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
+tests/cdrom-test$(EXESUF): tests/cdrom-test.o tests/boot-sector.o 
$(libqos-obj-y)
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< 
,"LINK","$(TARGET_DIR)$@")
diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
new file mode 100644
index 000..1fc5230
--- /dev/null
+++ b/tests/cdrom-test.c
@@ -0,0 +1,155 @@
+/*
+ * Various tests for emulated CD-ROM drives.
+ *
+ * Copyright (c) 2018 Red Hat Inc.
+ *
+ * Author:
+ *Thomas Huth 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2
+ * or later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "boot-sector.h"
+
+static char isoimage[] = "cdrom-boot-iso-XX";
+
+static int gen_iso(const char *fmt, ...)
+{
+char *params, *command;
+va_list args;
+pid_t pid;
+int status;
+
+pid = fork();
+if (pid == 0) {
+va_start(args, fmt);
+params = g_strdup_vprintf(fmt, args);
+va_end(args);
+command = g_strdup_printf("exec genisoimage %s", params);
+g_free(params);
+execlp("/bin/sh", "sh", "-c", command, NULL);
+exit(1);
+}
+wait();
+
+return WEXITSTATUS(status);
+}
+
+static int prepare_image(const char *arch, char *isoimage)
+{
+char srcdir[] = "cdrom-test-dir-XX";
+char *codefile = NULL;
+int ifh, ret = -1;
+
+ifh = mkstemp(isoimage);
+if (ifh < 0) {
+perror("Error creating temporary iso image file");
+return -1;
+}
+if (!mkdtemp(srcdir)) {
+perror("Error creating temporary directory");
+goto cleanup;
+}
+
+if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64") ||
+g_str_equal(arch, "s390x")) {
+codefile = g_strdup_printf("%s/bootcode-XX", srcdir);
+ret = boot_sector_init(codefile);
+if (ret) {
+goto cleanup;
+}
+} else {
+g_assert_not_reached();
+}
+
+ret = gen_iso("-quiet -l -no-emul-boot -b %s -o %s %s",
+  strrchr(codefile, '/') + 1, isoimage, srcdir);
+if (ret) {
+fprintf(stderr, "genisoimage failed: %i\n", ret);
+}
+
+unlink(codefile);
+
+cleanup:
+g_free(codefile);
+rmdir(srcdir);
+close(ifh);
+
+return ret;
+}
+
+static void test_cdboot(gconstpointer data)
+{
+QTestState *qts;
+
+qts = qtest_startf("-accel kvm:tcg -no-shutdown %s%s", (const char *)data,
+   isoimage);
+boot_sector_test(qts);
+qtest_quit(qts);
+}
+
+static void add_x86_tests(void)
+{
+qtest_add_data_func("cdboot/default", "-cdrom ", test_cdboot);
+qtest_add_data_func("cdboot/virtio-scsi",
+"-device virtio-scsi -device scsi-cd,drive=cdr "
+"-blockdev file,node-name=cdr,filename=", test_cdboot);
+qtest_add_data_func("cdboot/isapc", "-M isapc "
+"-drive if=ide,media=cdrom,file=", test_cdboot);
+qtest_add_data_func("cdboot/am53c974",
+"-device am53c974 -device scsi-cd,drive=cd1 "
+   

[Qemu-block] [PATCH 0/3] Add new CD-ROM related qtests

[Thomas Huth]

With one of my clean-up patches (see commit 1454509726719e0933c800), I
recently accidentially broke the "-cdrom" parameter (more precisely
"-drive if=scsi") on a couple of boards, since there was no error
detected during the "make check" regression testing. This is clearly an
indication that we are lacking tests in this area.
So this small patch series now introduces some tests for CD-ROM drives:
The first two patches introduce the possibility to check that booting
from CD-ROM drives still works fine for x86 and s390x, and the third
patch adds a test that certain machines can at least still be started
with the "-cdrom" parameter (i.e. that test would have catched the
mistake that I did with my SCSI cleanup patch).

Thomas Huth (3):
  tests/boot-sector: Add magic bytes to s390x boot code header
  tests/cdboot: Test booting from CD-ROM ISO image file
  tests/cdrom-test: Test that -cdrom parameter is working

 tests/Makefile.include |  10 ++-
 tests/boot-sector.c|   9 ++-
 tests/cdrom-test.c | 216 +
 3 files changed, 231 insertions(+), 4 deletions(-)
 create mode 100644 tests/cdrom-test.c

-- 
1.8.3.1

Re: [Qemu-block] [Qemu-devel] [PATCH for 2.12] iotests: Avoid realpath

[Fam Zheng]

On Wed, 03/14 09:47, Eric Blake wrote:
> > The remaining use was using realpath to convert a possibly
> > relative filename into an absolute one before calling diff,
> > but diff works just fine on the relative name.
> 
> Hmm, this last change reverts commit 93e53fb6 that added realpath on purpose
> for ease of diagnosing failed tests.  Maybe it's worth a v2 that tests
> whether realpath exists, and if so uses it, but does a safe fallback to just
> using the filename as-is.

Yes, thanks for spotting that. I am used to do out-of-tree builds/tests and
modifying XXX.out files by copying from .out.bad. Having realpath saves some
typing so..

Your fallback idea sounds good to me.

Fam

Re: [Qemu-block] [Qemu-devel] Question: an IO hang problem

[Fam Zheng]

On Tue, 03/13 17:38, sochin.jiang wrote:
> 
>  Hi, guys,
> 
>  Recently, I encountered an IO hang problem in occasion which I cannot 
> reproduce it now.
> 
>  I analyzed this problem carefully, the critical stack is as following:
> 
> 
> After reading the codes in linux-aio.c(see ioq_submit() function), I found 
> two situations could lead us here.
> 
> 1) no AIOs are in flight(s->ioq.in_flight is 0) and another call to io_submit 
> returns -EAGAIN

So if there is no inflight I/O, why it would return -EAGAIN? The tricky thing
here is that since we're not expecting a completion, when should we retry?

> 
> 2) no AIOs are in flight(s->ioq.in_flight is 0) and s->io_q.pending IOs reach 
> to MAX_EVENTS at once

I don't understand this case. We have,

len = 0;
QSIMPLEQ_FOREACH(aiocb, >io_q.pending, next) {
iocbs[len++] = >iocb;
if (s->io_q.in_flight + len >= MAX_EVENTS) {
break;
}
}

ret = io_submit(s->ctx, len, iocbs);

If in_flight is 0, only (MAX_EVENTS - 1) requests can be added to iocbs, so
io_submit shouldn't return -EAGAIN.

> 
> In both the two situations above, the do{...}while loop breaks out and set 
> s->io_q.blocked true.
> 
> After that, AIO completion callback will never be called,  ioq_submit() 
> either, all pended requests will hang.
> 
> 
> Is there a proper way we can fix this while do not affect(stuck) the guest ?

Fam