Series comparison

-[Qemu-devel] [PULL 0/5] Block patches
+[PULL 00/11] Block patches
-The following changes since commit a0def594286d9110a6035e02eef558cf3cf5d847:
+The following changes since commit 36609b4fa36f0ac934874371874416f7533a5408:
-  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-01-30 10:23:20 +0000)
+  Merge remote-tracking branch 'remotes/palmer/tags/palmer-for-master-4.2-sf1' into staging (2019-11-02 17:59:03 +0000)
-are available in the git repository at:
+are available in the Git repository at:
-  https://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
+  https://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to acf6e5f0962c4be670d4a93ede77423512521876:
+for you to fetch changes up to 9fdd7860adec188ed50d2530e9a819e8d953f9bb:
-  sheepdog: reorganize check for overlapping requests (2017-02-01 00:17:20 -0500)
+  image-fuzzer: Use OSerror.strerror instead of tuple subscript (2019-11-05 16:36:11 +0100)
 ----------------------------------------------------------------
-Block patches
+Pull request
 Let's get the image fuzzer Python 3 changes merged in QEMU 4.2.
 ----------------------------------------------------------------
-Paolo Bonzini (5):
+Eduardo Habkost (11):
-  sheepdog: remove unused cancellation support
+  image-fuzzer: Open image files in binary mode
-  sheepdog: reorganize coroutine flow
+  image-fuzzer: Write bytes instead of string to image file
-  sheepdog: do not use BlockAIOCB
+  image-fuzzer: Explicitly use integer division operator
-  sheepdog: simplify inflight_aio_head management
+  image-fuzzer: Use io.StringIO
-  sheepdog: reorganize check for overlapping requests
+  image-fuzzer: Use %r for all fiels at Field.__repr__()
   image-fuzzer: Return bytes objects on string fuzzing functions
   image-fuzzer: Use bytes constant for field values
   image-fuzzer: Encode file name and file format to bytes
   image-fuzzer: Run using python3
   image-fuzzer: Use errors parameter of subprocess.Popen()
   image-fuzzer: Use OSerror.strerror instead of tuple subscript
- block/sheepdog.c | 289 ++++++++++++++++---------------------------------------
+ tests/image-fuzzer/qcow2/__init__.py |  1 -
-file changed, 84 insertions(+), 205 deletions(-)
+ tests/image-fuzzer/qcow2/fuzz.py     | 54 +++++++++++++-------------
  tests/image-fuzzer/qcow2/layout.py   | 57 ++++++++++++++--------------
  tests/image-fuzzer/runner.py         | 16 ++++----
 files changed, 63 insertions(+), 65 deletions(-)
 --
-.9.3
+.23.0

-New patch
+[PULL 01/11] image-fuzzer: Open image files in binary mode
+From: Eduardo Habkost <ehabkost@redhat.com>
+This probably never caused problems because on Linux there's no
+actual newline conversion happening, but on Python 3 the
+binary/text distinction is stronger and we must explicitly open
+the image file in binary mode.
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20191016192430.25098-2-ehabkost@redhat.com
+Message-Id: <20191016192430.25098-2-ehabkost@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ tests/image-fuzzer/qcow2/layout.py | 2 +-
+file changed, 1 insertion(+), 1 deletion(-)
+diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/image-fuzzer/qcow2/layout.py
++++ b/tests/image-fuzzer/qcow2/layout.py
+@@ -XXX,XX +XXX,XX @@ class Image(object):
+     def write(self, filename):
+         """Write an entire image to the file."""
+-        image_file = open(filename, 'w')
++        image_file = open(filename, 'wb')
+         for field in self:
+             image_file.seek(field.offset)
+             image_file.write(struct.pack(field.fmt, field.value))
+--
+.23.0

-New patch
+[PULL 02/11] image-fuzzer: Write bytes instead of string to image file
+From: Eduardo Habkost <ehabkost@redhat.com>
+This is necessary for Python 3 compatibility.
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20191016192430.25098-3-ehabkost@redhat.com
+Message-Id: <20191016192430.25098-3-ehabkost@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ tests/image-fuzzer/qcow2/layout.py | 2 +-
+file changed, 1 insertion(+), 1 deletion(-)
+diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/image-fuzzer/qcow2/layout.py
++++ b/tests/image-fuzzer/qcow2/layout.py
+@@ -XXX,XX +XXX,XX @@ class Image(object):
+         rounded = (size + self.cluster_size - 1) & ~(self.cluster_size - 1)
+         if rounded > size:
+             image_file.seek(rounded - 1)
+-            image_file.write("\0")
++            image_file.write(b'\x00')
+         image_file.close()
+     @staticmethod
+--
+.23.0

-New patch
+[PULL 03/11] image-fuzzer: Explicitly use integer division operator
+From: Eduardo Habkost <ehabkost@redhat.com>
 Most of the division expressions in image-fuzzer assume integer
 division.  Use the // operator to keep the same behavior when we
 move to Python 3.
 Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
 Reviewed-by: John Snow <jsnow@redhat.com>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20191016192430.25098-4-ehabkost@redhat.com
 Message-Id: <20191016192430.25098-4-ehabkost@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  tests/image-fuzzer/qcow2/fuzz.py   | 12 ++++-----
  tests/image-fuzzer/qcow2/layout.py | 40 +++++++++++++++---------------
 files changed, 26 insertions(+), 26 deletions(-)
 diff --git a/tests/image-fuzzer/qcow2/fuzz.py b/tests/image-fuzzer/qcow2/fuzz.py
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/image-fuzzer/qcow2/fuzz.py
 +++ b/tests/image-fuzzer/qcow2/fuzz.py
@@ -XXX,XX +XXX,XX @@ UINT64 = 0xffffffffffffffff
  UINT32_M = 31
  UINT64_M = 63
  # Fuzz vectors
 -UINT8_V = [0, 0x10, UINT8/4, UINT8/2 - 1, UINT8/2, UINT8/2 + 1, UINT8 - 1,
 +UINT8_V = [0, 0x10, UINT8//4, UINT8//2 - 1, UINT8//2, UINT8//2 + 1, UINT8 - 1,
             UINT8]
 -UINT16_V = [0, 0x100, 0x1000, UINT16/4, UINT16/2 - 1, UINT16/2, UINT16/2 + 1,
 +UINT16_V = [0, 0x100, 0x1000, UINT16//4, UINT16//2 - 1, UINT16//2, UINT16//2 + 1,
              UINT16 - 1, UINT16]
 -UINT32_V = [0, 0x100, 0x1000, 0x10000, 0x100000, UINT32/4, UINT32/2 - 1,
 -            UINT32/2, UINT32/2 + 1, UINT32 - 1, UINT32]
 -UINT64_V = UINT32_V + [0x1000000, 0x10000000, 0x100000000, UINT64/4,
 -                       UINT64/2 - 1, UINT64/2, UINT64/2 + 1, UINT64 - 1,
 +UINT32_V = [0, 0x100, 0x1000, 0x10000, 0x100000, UINT32//4, UINT32//2 - 1,
 +            UINT32//2, UINT32//2 + 1, UINT32 - 1, UINT32]
 +UINT64_V = UINT32_V + [0x1000000, 0x10000000, 0x100000000, UINT64//4,
 +                       UINT64//2 - 1, UINT64//2, UINT64//2 + 1, UINT64 - 1,
                         UINT64]
  STRING_V = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
              '%d%d%d%d', '%s%s%s%s', '%99999999999s', '%08x', '%%20d', '%%20n',
 diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/image-fuzzer/qcow2/layout.py
 +++ b/tests/image-fuzzer/qcow2/layout.py
@@ -XXX,XX +XXX,XX @@ class Image(object):
                  ['>I', self.ext_offset, 0x6803f857, 'ext_magic'],
                  # One feature table contains 3 fields and takes 48 bytes
                  ['>I', self.ext_offset + UINT32_S,
 -                 len(feature_tables) / 3 * 48, 'ext_length']
 +                 len(feature_tables) // 3 * 48, 'ext_length']
              ] + feature_tables)
              self.ext_offset = inner_offset
@@ -XXX,XX +XXX,XX @@ class Image(object):
          def create_l2_entry(host, guest, l2_cluster):
              """Generate one L2 entry."""
              offset = l2_cluster * self.cluster_size
 -            l2_size = self.cluster_size / UINT64_S
 +            l2_size = self.cluster_size // UINT64_S
              entry_offset = offset + UINT64_S * (guest % l2_size)
              cluster_descriptor = host * self.cluster_size
              if not self.header['version'][0].value == 2:
@@ -XXX,XX +XXX,XX @@ class Image(object):
          def create_l1_entry(l2_cluster, l1_offset, guest):
              """Generate one L1 entry."""
 -            l2_size = self.cluster_size / UINT64_S
 -            entry_offset = l1_offset + UINT64_S * (guest / l2_size)
 +            l2_size = self.cluster_size // UINT64_S
 +            entry_offset = l1_offset + UINT64_S * (guest // l2_size)
              # While snapshots are not supported bit #63 = 1
              entry_val = (1 << 63) + l2_cluster * self.cluster_size
              return ['>Q', entry_offset, entry_val, 'l1_entry']
@@ -XXX,XX +XXX,XX @@ class Image(object):
              l2 = []
          else:
              meta_data = self._get_metadata()
 -            guest_clusters = random.sample(range(self.image_size /
 +            guest_clusters = random.sample(range(self.image_size //
                                                   self.cluster_size),
                                             len(self.data_clusters))
              # Number of entries in a L1/L2 table
 -            l_size = self.cluster_size / UINT64_S
 +            l_size = self.cluster_size // UINT64_S
              # Number of clusters necessary for L1 table
              l1_size = int(ceil((max(guest_clusters) + 1) / float(l_size**2)))
              l1_start = self._get_adjacent_clusters(self.data_clusters |
@@ -XXX,XX +XXX,XX @@ class Image(object):
              # L2 entries
              l2 = []
              for host, guest in zip(self.data_clusters, guest_clusters):
 -                l2_id = guest / l_size
 +                l2_id = guest // l_size
                  if l2_id not in l2_ids:
                      l2_ids.append(l2_id)
                      l2_clusters.append(self._get_adjacent_clusters(
@@ -XXX,XX +XXX,XX @@ class Image(object):
          def allocate_rfc_blocks(data, size):
              """Return indices of clusters allocated for refcount blocks."""
              cluster_ids = set()
 -            diff = block_ids = set([x / size for x in data])
 +            diff = block_ids = set([x // size for x in data])
              while len(diff) != 0:
                  # Allocate all yet not allocated clusters
                  new = self._get_available_clusters(data | cluster_ids,
                                                     len(diff))
                  # Indices of new refcount blocks necessary to cover clusters
                  # in 'new'
 -                diff = set([x / size for x in new]) - block_ids
 +                diff = set([x // size for x in new]) - block_ids
                  cluster_ids |= new
                  block_ids |= diff
              return cluster_ids, block_ids
@@ -XXX,XX +XXX,XX @@ class Image(object):
              blocks = set(init_blocks)
              clusters = set()
              # Number of entries in one cluster of the refcount table
 -            size = self.cluster_size / UINT64_S
 +            size = self.cluster_size // UINT64_S
              # Number of clusters necessary for the refcount table based on
              # the current number of refcount blocks
              table_size = int(ceil((max(blocks) + 1) / float(size)))
@@ -XXX,XX +XXX,XX @@ class Image(object):
                                                   table_size + 1))
              # New refcount blocks necessary for clusters occupied by the
              # refcount table
 -            diff = set([c / block_size for c in table_clusters]) - blocks
 +            diff = set([c // block_size for c in table_clusters]) - blocks
              blocks |= diff
              while len(diff) != 0:
                  # Allocate clusters for new refcount blocks
@@ -XXX,XX +XXX,XX @@ class Image(object):
                                                     len(diff))
                  # Indices of new refcount blocks necessary to cover
                  # clusters in 'new'
 -                diff = set([x / block_size for x in new]) - blocks
 +                diff = set([x // block_size for x in new]) - blocks
                  clusters |= new
                  blocks |= diff
                  # Check if the refcount table needs one more cluster
                  if int(ceil((max(blocks) + 1) / float(size))) > table_size:
 -                    new_block_id = (table_start + table_size) / block_size
 +                    new_block_id = (table_start + table_size) // block_size
                      # Check if the additional table cluster needs
                      # one more refcount block
                      if new_block_id not in blocks:
@@ -XXX,XX +XXX,XX @@ class Image(object):
          def create_table_entry(table_offset, block_cluster, block_size,
                                 cluster):
              """Generate a refcount table entry."""
 -            offset = table_offset + UINT64_S * (cluster / block_size)
 +            offset = table_offset + UINT64_S * (cluster // block_size)
              return ['>Q', offset, block_cluster * self.cluster_size,
                      'refcount_table_entry']
          def create_block_entry(block_cluster, block_size, cluster):
              """Generate a list of entries for the current block."""
 -            entry_size = self.cluster_size / block_size
 +            entry_size = self.cluster_size // block_size
              offset = block_cluster * self.cluster_size
              entry_offset = offset + entry_size * (cluster % block_size)
              # While snapshots are not supported all refcounts are set to 1
@@ -XXX,XX +XXX,XX @@ class Image(object):
          # Number of refcount entries per refcount block
          # Convert self.cluster_size from bytes to bits to have the same
          # base for the numerator and denominator
 -        block_size = self.cluster_size * 8 / refcount_bits
 +        block_size = self.cluster_size * 8 // refcount_bits
          meta_data = self._get_metadata()
          if len(self.data_clusters) == 0:
              # All metadata for an empty guest image needs 4 clusters:
@@ -XXX,XX +XXX,XX @@ class Image(object):
          rfc_blocks = []
          for cluster in sorted(self.data_clusters | meta_data):
 -            if cluster / block_size != block_id:
 -                block_id = cluster / block_size
 +            if cluster // block_size != block_id:
 +                block_id = cluster // block_size
                  block_cluster = block_clusters[block_ids.index(block_id)]
                  rfc_table.append(create_table_entry(table_offset,
                                                      block_cluster,
@@ -XXX,XX +XXX,XX @@ class Image(object):
      def _alloc_data(img_size, cluster_size):
          """Return a set of random indices of clusters allocated for guest data.
          """
 -        num_of_cls = img_size/cluster_size
 +        num_of_cls = img_size // cluster_size
          return set(random.sample(range(1, num_of_cls + 1),
                                   random.randint(0, num_of_cls)))
@@ -XXX,XX +XXX,XX @@ class Image(object):
          """Return indices of clusters allocated for image metadata."""
          ids = set()
          for x in self:
 -            ids.add(x.offset/self.cluster_size)
 +            ids.add(x.offset // self.cluster_size)
          return ids
 --
 .23.0

-New patch
+[PULL 04/11] image-fuzzer: Use io.StringIO
+From: Eduardo Habkost <ehabkost@redhat.com>
+StringIO.StringIO is not available on Python 3, but io.StringIO
+is available on both Python 2 and 3.  io.StringIO is slightly
+different from the Python 2 StringIO module, though, so we need
+bytes coming from subprocess.Popen() to be explicitly decoded.
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20191016192430.25098-5-ehabkost@redhat.com
+Message-Id: <20191016192430.25098-5-ehabkost@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ tests/image-fuzzer/runner.py | 14 +++++++++-----
+file changed, 9 insertions(+), 5 deletions(-)
+diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/image-fuzzer/runner.py
++++ b/tests/image-fuzzer/runner.py
+@@ -XXX,XX +XXX,XX @@ import shutil
+ from itertools import count
+ import time
+ import getopt
+-import StringIO
++import io
+ import resource
+ try:
+@@ -XXX,XX +XXX,XX @@ def run_app(fd, q_args):
+     try:
+         out, err = process.communicate()
+         signal.alarm(0)
+-        fd.write(out)
+-        fd.write(err)
++        # fd is a text file, so we need to decode the process output before
++        # writing to it.
++        # We could be simply using the `errors` parameter of subprocess.Popen(),
++        # but this will be possible only after migrating to Python 3
++        fd.write(out.decode(errors='replace'))
++        fd.write(err.decode(errors='replace'))
+         fd.flush()
+         return process.returncode
+@@ -XXX,XX +XXX,XX @@ class TestEnv(object):
+                                            MAX_BACKING_FILE_SIZE) * (1 << 20)
+         cmd = self.qemu_img + ['create', '-f', backing_file_fmt,
+                                backing_file_name, str(backing_file_size)]
+-        temp_log = StringIO.StringIO()
++        temp_log = io.StringIO()
+         retcode = run_app(temp_log, cmd)
+         if retcode == 0:
+             temp_log.close()
+@@ -XXX,XX +XXX,XX @@ class TestEnv(object):
+                            "Backing file: %s\n" \
+                            % (self.seed, " ".join(current_cmd),
+                               self.current_dir, backing_file_name)
+-            temp_log = StringIO.StringIO()
++            temp_log = io.StringIO()
+             try:
+                 retcode = run_app(temp_log, current_cmd)
+             except OSError as e:
+--
+.23.0

-New patch
+[PULL 05/11] image-fuzzer: Use %r for all fiels at Field.__repr__()
+From: Eduardo Habkost <ehabkost@redhat.com>
+This makes the formatting code simpler, and safer if we change
+the type of self.value from str to bytes.
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20191016192430.25098-6-ehabkost@redhat.com
+Message-Id: <20191016192430.25098-6-ehabkost@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ tests/image-fuzzer/qcow2/layout.py | 4 ++--
+file changed, 2 insertions(+), 2 deletions(-)
+diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/image-fuzzer/qcow2/layout.py
++++ b/tests/image-fuzzer/qcow2/layout.py
+@@ -XXX,XX +XXX,XX @@ class Field(object):
+         return iter([self.fmt, self.offset, self.value, self.name])
+     def __repr__(self):
+-        return "Field(fmt='%s', offset=%d, value=%s, name=%s)" % \
+-            (self.fmt, self.offset, str(self.value), self.name)
++        return "Field(fmt=%r, offset=%r, value=%r, name=%r)" % \
++            (self.fmt, self.offset, self.value, self.name)
+ class FieldsList(object):
+--
+.23.0

-New patch
+[PULL 06/11] image-fuzzer: Return bytes objects on string fuzzing functions
+From: Eduardo Habkost <ehabkost@redhat.com>
+No caller of fuzzer functions is interested in unicode string values,
+so replace them with bytes sequences.
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20191016192430.25098-7-ehabkost@redhat.com
+Message-Id: <20191016192430.25098-7-ehabkost@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ tests/image-fuzzer/qcow2/fuzz.py | 42 ++++++++++++++++----------------
+file changed, 21 insertions(+), 21 deletions(-)
+diff --git a/tests/image-fuzzer/qcow2/fuzz.py b/tests/image-fuzzer/qcow2/fuzz.py
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/image-fuzzer/qcow2/fuzz.py
++++ b/tests/image-fuzzer/qcow2/fuzz.py
+@@ -XXX,XX +XXX,XX @@ UINT32_V = [0, 0x100, 0x1000, 0x10000, 0x100000, UINT32//4, UINT32//2 - 1,
+ UINT64_V = UINT32_V + [0x1000000, 0x10000000, 0x100000000, UINT64//4,
+                        UINT64//2 - 1, UINT64//2, UINT64//2 + 1, UINT64 - 1,
+                        UINT64]
+-STRING_V = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
+-            '%d%d%d%d', '%s%s%s%s', '%99999999999s', '%08x', '%%20d', '%%20n',
+-            '%%20x', '%%20s', '%s%s%s%s%s%s%s%s%s%s', '%p%p%p%p%p%p%p%p%p%p',
+-            '%#0123456x%08x%x%s%p%d%n%o%u%c%h%l%q%j%z%Z%t%i%e%g%f%a%C%S%08x%%',
+-            '%s x 129', '%x x 257']
++BYTES_V = [b'%s%p%x%d', b'.1024d', b'%.2049d', b'%p%p%p%p', b'%x%x%x%x',
++           b'%d%d%d%d', b'%s%s%s%s', b'%99999999999s', b'%08x', b'%%20d', b'%%20n',
++           b'%%20x', b'%%20s', b'%s%s%s%s%s%s%s%s%s%s', b'%p%p%p%p%p%p%p%p%p%p',
++           b'%#0123456x%08x%x%s%p%d%n%o%u%c%h%l%q%j%z%Z%t%i%e%g%f%a%C%S%08x%%',
++           b'%s x 129', b'%x x 257']
+ def random_from_intervals(intervals):
+@@ -XXX,XX +XXX,XX @@ def random_bits(bit_ranges):
+     return val
+-def truncate_string(strings, length):
+-    """Return strings truncated to specified length."""
+-    if type(strings) == list:
+-        return [s[:length] for s in strings]
++def truncate_bytes(sequences, length):
++    """Return sequences truncated to specified length."""
++    if type(sequences) == list:
++        return [s[:length] for s in sequences]
+     else:
+-        return strings[:length]
++        return sequences[:length]
+ def validator(current, pick, choices):
+@@ -XXX,XX +XXX,XX @@ def bit_validator(current, bit_ranges):
+     return validator(current, random_bits, bit_ranges)
+-def string_validator(current, strings):
+-    """Return a random string value from the list not equal to the current.
++def bytes_validator(current, sequences):
++    """Return a random bytes value from the list not equal to the current.
+     This function is useful for selection from valid values except current one.
+     """
+-    return validator(current, random.choice, strings)
++    return validator(current, random.choice, sequences)
+ def selector(current, constraints, validate=int_validator):
+@@ -XXX,XX +XXX,XX @@ def header_length(current):
+ def bf_name(current):
+     """Fuzz the backing file name."""
+     constraints = [
+-        truncate_string(STRING_V, len(current))
++        truncate_bytes(BYTES_V, len(current))
+     ]
+-    return selector(current, constraints, string_validator)
++    return selector(current, constraints, bytes_validator)
+ def ext_magic(current):
+@@ -XXX,XX +XXX,XX @@ def ext_length(current):
+ def bf_format(current):
+     """Fuzz backing file format in the corresponding header extension."""
+     constraints = [
+-        truncate_string(STRING_V, len(current)),
+-        truncate_string(STRING_V, (len(current) + 7) & ~7)  # Fuzz padding
++        truncate_bytes(BYTES_V, len(current)),
++        truncate_bytes(BYTES_V, (len(current) + 7) & ~7)  # Fuzz padding
+     ]
+-    return selector(current, constraints, string_validator)
++    return selector(current, constraints, bytes_validator)
+ def feature_type(current):
+@@ -XXX,XX +XXX,XX @@ def feature_bit_number(current):
+ def feature_name(current):
+     """Fuzz feature name field of a feature name table header extension."""
+     constraints = [
+-        truncate_string(STRING_V, len(current)),
+-        truncate_string(STRING_V, 46)  # Fuzz padding (field length = 46)
++        truncate_bytes(BYTES_V, len(current)),
++        truncate_bytes(BYTES_V, 46)  # Fuzz padding (field length = 46)
+     ]
+-    return selector(current, constraints, string_validator)
++    return selector(current, constraints, bytes_validator)
+ def l1_entry(current):
+--
+.23.0

-[Qemu-devel] [PULL 4/5] sheepdog: simplify inflight_aio_head management
+[PULL 07/11] image-fuzzer: Use bytes constant for field values
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Eduardo Habkost <ehabkost@redhat.com>
-Add to the list in add_aio_request and, indirectly, resend_aioreq.  Inline
+Field values are supposed to be bytes objects, not unicode
-free_aio_req in the caller, it does not simply undo alloc_aio_req's job.
+strings.  Change two constants that were declared as strings.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
-Message-id: 20161129113245.32724-5-pbonzini@redhat.com
+Reviewed-by: John Snow <jsnow@redhat.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20191016192430.25098-8-ehabkost@redhat.com
 Message-Id: <20191016192430.25098-8-ehabkost@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 23 ++++++-----------------
+ tests/image-fuzzer/qcow2/layout.py | 4 ++--
-file changed, 6 insertions(+), 17 deletions(-)
+file changed, 2 insertions(+), 2 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/tests/image-fuzzer/qcow2/layout.py
-+++ b/block/sheepdog.c
++++ b/tests/image-fuzzer/qcow2/layout.py
-@@ -XXX,XX +XXX,XX @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
+@@ -XXX,XX +XXX,XX @@ class Image(object):
-     return aio_req;
+     def create_header(self, cluster_bits, backing_file_name=None):
- }
+         """Generate a random valid header."""
+         meta_header = [
--static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
+-            ['>4s', 0, "QFI\xfb", 'magic'],
--{
++            ['>4s', 0, b"QFI\xfb", 'magic'],
--    SheepdogAIOCB *acb = aio_req->aiocb;
+             ['>I', 4, random.randint(2, 3), 'version'],
--
+             ['>Q', 8, 0, 'backing_file_offset'],
--    QLIST_REMOVE(aio_req, aio_siblings);
+             ['>I', 16, 0, 'backing_file_size'],
--    g_free(aio_req);
+@@ -XXX,XX +XXX,XX @@ class Image(object):
--
+             feature_tables = []
--    acb->nr_pending--;
+             feature_ids = []
--}
+             inner_offset = self.ext_offset + ext_header_len
--
+-            feat_name = 'some cool feature'
- static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
++            feat_name = b'some cool feature'
-                          QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
+             while len(feature_tables) < num_fnt_entries * 3:
-                          int type)
+                 feat_type, feat_bit = gen_feat_ids()
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
+                 # Remove duplicates
      while (!QLIST_EMPTY(&s->failed_aio_head)) {
          aio_req = QLIST_FIRST(&s->failed_aio_head);
          QLIST_REMOVE(aio_req, aio_siblings);
 -        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
          resend_aioreq(s, aio_req);
      }
  }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
      */
      s->co_recv = NULL;
 +    QLIST_REMOVE(aio_req, aio_siblings);
      switch (rsp.result) {
      case SD_RES_SUCCESS:
          break;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
          break;
      }
 -    free_aio_req(s, aio_req);
 -    if (!acb->nr_pending) {
 +    g_free(aio_req);
 +
 +    if (!--acb->nr_pending) {
          /*
           * We've finished all requests which belong to the AIOCB, so
           * we can switch back to sd_co_readv/writev now.
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
      uint64_t old_oid = aio_req->base_oid;
      bool create = aio_req->create;
 +    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
 +
      if (!nr_copies) {
          error_report("bug");
      }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
          iov.iov_len = sizeof(s->inode);
          aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
                                  data_len, offset, 0, false, 0, offset);
 -        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
          add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
          if (--acb->nr_pending) {
              qemu_coroutine_yield();
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
                                  old_oid,
                                  acb->aiocb_type == AIOCB_DISCARD_OBJ ?
 : done);
 -        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
 -
          add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
                          acb->aiocb_type);
      done:
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
      acb.nr_pending++;
      aio_req = alloc_aio_req(s, &acb, vid_to_vdi_oid(s->inode.vdi_id),
 , 0, 0, false, 0, 0);
 -    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
      add_aio_request(s, aio_req, NULL, 0, acb.aiocb_type);
      if (--acb.nr_pending) {
 --
-.9.3
+.23.0

-[Qemu-devel] [PULL 3/5] sheepdog: do not use BlockAIOCB
+[PULL 08/11] image-fuzzer: Encode file name and file format to bytes
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Eduardo Habkost <ehabkost@redhat.com>
-Sheepdog's AIOCB are completely internal entities for a group of
+Callers of create_image() will pass strings as arguments, but the
-requests and do not need dynamic allocation.
+Image class will expect bytes objects to be provided.  Encode
 them inside create_image().
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
-Message-id: 20161129113245.32724-4-pbonzini@redhat.com
+Reviewed-by: John Snow <jsnow@redhat.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20191016192430.25098-9-ehabkost@redhat.com
 Message-Id: <20191016192430.25098-9-ehabkost@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 99 ++++++++++++++++++++++----------------------------------
+ tests/image-fuzzer/qcow2/layout.py | 4 ++--
-file changed, 39 insertions(+), 60 deletions(-)
+file changed, 2 insertions(+), 2 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/tests/image-fuzzer/qcow2/layout.py
-+++ b/block/sheepdog.c
++++ b/tests/image-fuzzer/qcow2/layout.py
-@@ -XXX,XX +XXX,XX @@ static inline size_t count_data_objs(const struct SheepdogInode *inode)
+@@ -XXX,XX +XXX,XX @@ class Image(object):
-     } while (0)
+ def create_image(test_img_path, backing_file_name=None, backing_file_fmt=None,
+                  fields_to_fuzz=None):
- typedef struct SheepdogAIOCB SheepdogAIOCB;
+     """Create a fuzzed image and write it to the specified file."""
-+typedef struct BDRVSheepdogState BDRVSheepdogState;
+-    image = Image(backing_file_name)
+-    image.set_backing_file_format(backing_file_fmt)
- typedef struct AIOReq {
++    image = Image(backing_file_name.encode())
-     SheepdogAIOCB *aiocb;
++    image.set_backing_file_format(backing_file_fmt.encode())
-@@ -XXX,XX +XXX,XX @@ enum AIOCBState {
+     image.create_feature_name_table()
-        || y->max_affect_data_idx < x->min_affect_data_idx))
+     image.set_end_of_extension_area()
+     image.create_l_structures()
  struct SheepdogAIOCB {
 -    BlockAIOCB common;
 +    BDRVSheepdogState *s;
      QEMUIOVector *qiov;
@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
      QLIST_ENTRY(SheepdogAIOCB) aiocb_siblings;
  };
 -typedef struct BDRVSheepdogState {
 +struct BDRVSheepdogState {
      BlockDriverState *bs;
      AioContext *aio_context;
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVSheepdogState {
      CoQueue overlapping_queue;
      QLIST_HEAD(inflight_aiocb_head, SheepdogAIOCB) inflight_aiocb_head;
 -} BDRVSheepdogState;
 +};
  typedef struct BDRVSheepdogReopenState {
      int fd;
@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
      acb->nr_pending--;
  }
 -static const AIOCBInfo sd_aiocb_info = {
 -    .aiocb_size     = sizeof(SheepdogAIOCB),
 -};
 -
 -static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
 -                                   int64_t sector_num, int nb_sectors)
 +static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
 +                         QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
 +                         int type)
  {
 -    SheepdogAIOCB *acb;
      uint32_t object_size;
 -    BDRVSheepdogState *s = bs->opaque;
      object_size = (UINT32_C(1) << s->inode.block_size_shift);
 -    acb = qemu_aio_get(&sd_aiocb_info, bs, NULL, NULL);
 +    acb->s = s;
      acb->qiov = qiov;
@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
      acb->min_dirty_data_idx = UINT32_MAX;
      acb->max_dirty_data_idx = 0;
 -
 -    return acb;
 +    acb->aiocb_type = type;
  }
  /* Return -EIO in case of error, file descriptor on success */
@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
   */
  static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
  {
 -    BDRVSheepdogState *s = acb->common.bs->opaque;
 +    BDRVSheepdogState *s = acb->s;
      struct iovec iov;
      AIOReq *aio_req;
      uint32_t offset, data_len, mn, mx;
@@ -XXX,XX +XXX,XX @@ out:
   * Returns 1 when we need to wait a response, 0 when there is no sent
   * request and -errno in error cases.
   */
 -static void coroutine_fn sd_co_rw_vector(void *p)
 +static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
  {
 -    SheepdogAIOCB *acb = p;
      int ret = 0;
      unsigned long len, done = 0, total = acb->nb_sectors * BDRV_SECTOR_SIZE;
      unsigned long idx;
      uint32_t object_size;
      uint64_t oid;
      uint64_t offset;
 -    BDRVSheepdogState *s = acb->common.bs->opaque;
 +    BDRVSheepdogState *s = acb->s;
      SheepdogInode *inode = &s->inode;
      AIOReq *aio_req;
@@ -XXX,XX +XXX,XX @@ static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
  static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
                          int nb_sectors, QEMUIOVector *qiov)
  {
 -    SheepdogAIOCB *acb;
 +    SheepdogAIOCB acb;
      int ret;
      int64_t offset = (sector_num + nb_sectors) * BDRV_SECTOR_SIZE;
      BDRVSheepdogState *s = bs->opaque;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
          }
      }
 -    acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
 -    acb->aiocb_type = AIOCB_WRITE_UDATA;
 +    sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_WRITE_UDATA);
  retry:
 -    if (check_overlapping_aiocb(s, acb)) {
 +    if (check_overlapping_aiocb(s, &acb)) {
          qemu_co_queue_wait(&s->overlapping_queue);
          goto retry;
      }
 -    sd_co_rw_vector(acb);
 -    sd_write_done(acb);
 +    sd_co_rw_vector(&acb);
 +    sd_write_done(&acb);
 -    QLIST_REMOVE(acb, aiocb_siblings);
 +    QLIST_REMOVE(&acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -    ret = acb->ret;
 -    qemu_aio_unref(acb);
 -    return ret;
 +    return acb.ret;
  }
  static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
                         int nb_sectors, QEMUIOVector *qiov)
  {
 -    SheepdogAIOCB *acb;
 -    int ret;
 +    SheepdogAIOCB acb;
      BDRVSheepdogState *s = bs->opaque;
 -    acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
 -    acb->aiocb_type = AIOCB_READ_UDATA;
 +    sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_READ_UDATA);
  retry:
 -    if (check_overlapping_aiocb(s, acb)) {
 +    if (check_overlapping_aiocb(s, &acb)) {
          qemu_co_queue_wait(&s->overlapping_queue);
          goto retry;
      }
 -    sd_co_rw_vector(acb);
 +    sd_co_rw_vector(&acb);
 -    QLIST_REMOVE(acb, aiocb_siblings);
 +    QLIST_REMOVE(&acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -    ret = acb->ret;
 -    qemu_aio_unref(acb);
 -    return ret;
 +    return acb.ret;
  }
  static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
  {
      BDRVSheepdogState *s = bs->opaque;
 -    SheepdogAIOCB *acb;
 -    int ret;
 +    SheepdogAIOCB acb;
      AIOReq *aio_req;
      if (s->cache_flags != SD_FLAG_CMD_CACHE) {
          return 0;
      }
 -    acb = sd_aio_setup(bs, NULL, 0, 0);
 -    acb->aiocb_type = AIOCB_FLUSH_CACHE;
 +    sd_aio_setup(&acb, s, NULL, 0, 0, AIOCB_FLUSH_CACHE);
 -    acb->nr_pending++;
 -    aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
 +    acb.nr_pending++;
 +    aio_req = alloc_aio_req(s, &acb, vid_to_vdi_oid(s->inode.vdi_id),
 , 0, 0, false, 0, 0);
      QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
 -    add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
 +    add_aio_request(s, aio_req, NULL, 0, acb.aiocb_type);
 -    if (--acb->nr_pending) {
 +    if (--acb.nr_pending) {
          qemu_coroutine_yield();
      }
 -    ret = acb->ret;
 -    qemu_aio_unref(acb);
 -    return ret;
 +    return acb.ret;
  }
  static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
@@ -XXX,XX +XXX,XX @@ static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
  static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
                                        int count)
  {
 -    SheepdogAIOCB *acb;
 +    SheepdogAIOCB acb;
      BDRVSheepdogState *s = bs->opaque;
 -    int ret;
      QEMUIOVector discard_iov;
      struct iovec iov;
      uint32_t zero = 0;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
      if (!QEMU_IS_ALIGNED(offset | count, BDRV_SECTOR_SIZE)) {
          return -ENOTSUP;
      }
 -    acb = sd_aio_setup(bs, &discard_iov, offset >> BDRV_SECTOR_BITS,
 -                       count >> BDRV_SECTOR_BITS);
 -    acb->aiocb_type = AIOCB_DISCARD_OBJ;
 +    sd_aio_setup(&acb, s, &discard_iov, offset >> BDRV_SECTOR_BITS,
 +                 count >> BDRV_SECTOR_BITS, AIOCB_DISCARD_OBJ);
  retry:
 -    if (check_overlapping_aiocb(s, acb)) {
 +    if (check_overlapping_aiocb(s, &acb)) {
          qemu_co_queue_wait(&s->overlapping_queue);
          goto retry;
      }
 -    sd_co_rw_vector(acb);
 +    sd_co_rw_vector(&acb);
 -    QLIST_REMOVE(acb, aiocb_siblings);
 +    QLIST_REMOVE(&acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -    ret = acb->ret;
 -    qemu_aio_unref(acb);
 -    return ret;
 +    return acb.ret;
  }
  static coroutine_fn int64_t
 --
-.9.3
+.23.0

-[Qemu-devel] [PULL 2/5] sheepdog: reorganize coroutine flow
+[PULL 09/11] image-fuzzer: Run using python3
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Eduardo Habkost <ehabkost@redhat.com>
-Delimit co_recv's lifetime clearly in aio_read_response.
+image-fuzzer is now supposed to be ready to run using Python 3.
 Remove the __future__ imports and change the interpreter line to
 "#!/usr/bin/env python3".
-Do a simple qemu_coroutine_enter in aio_read_response, letting
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
-sd_co_writev call sd_write_done.
+Reviewed-by: John Snow <jsnow@redhat.com>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20191016192430.25098-10-ehabkost@redhat.com
 Message-Id: <20191016192430.25098-10-ehabkost@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  tests/image-fuzzer/qcow2/__init__.py | 1 -
  tests/image-fuzzer/qcow2/layout.py   | 1 -
  tests/image-fuzzer/runner.py         | 3 +--
 files changed, 1 insertion(+), 4 deletions(-)
-Handle nr_pending in the same way in sd_co_rw_vector,
+diff --git a/tests/image-fuzzer/qcow2/__init__.py b/tests/image-fuzzer/qcow2/__init__.py
 sd_write_done and sd_co_flush_to_disk.
 Remove sd_co_rw_vector's return value; just leave with no
 pending requests.
 [Jeff: added missing 'return' back, spotted by Paolo after
        series was applied.]
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  block/sheepdog.c | 115 ++++++++++++++++++++-----------------------------------
 file changed, 42 insertions(+), 73 deletions(-)
 diff --git a/block/sheepdog.c b/block/sheepdog.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/tests/image-fuzzer/qcow2/__init__.py
-+++ b/block/sheepdog.c
++++ b/tests/image-fuzzer/qcow2/__init__.py
-@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
+@@ -1,2 +1 @@
-     enum AIOCBState aiocb_type;
+-from __future__ import absolute_import
+ from .layout import create_image
-     Coroutine *coroutine;
+diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
--    void (*aio_done_func)(SheepdogAIOCB *);
+index XXXXXXX..XXXXXXX 100644
--
+--- a/tests/image-fuzzer/qcow2/layout.py
-     int nr_pending;
++++ b/tests/image-fuzzer/qcow2/layout.py
+@@ -XXX,XX +XXX,XX @@
-     uint32_t min_affect_data_idx;
+ # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-@@ -XXX,XX +XXX,XX @@ static const char * sd_strerror(int err)
+ #
-  *
-  * 1. In sd_co_rw_vector, we send the I/O requests to the server and
+-from __future__ import absolute_import
-  *    link the requests to the inflight_list in the
+ import random
-- *    BDRVSheepdogState.  The function exits without waiting for
+ import struct
-+ *    BDRVSheepdogState.  The function yields while waiting for
+ from . import fuzz
-  *    receiving the response.
+diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
-  *
+index XXXXXXX..XXXXXXX 100755
-  * 2. We receive the response in aio_read_response, the fd handler to
+--- a/tests/image-fuzzer/runner.py
-- *    the sheepdog connection.  If metadata update is needed, we send
++++ b/tests/image-fuzzer/runner.py
-- *    the write request to the vdi object in sd_write_done, the write
+@@ -XXX,XX +XXX,XX @@
-- *    completion function.  We switch back to sd_co_readv/writev after
+-#!/usr/bin/env python
-- *    all the requests belonging to the AIOCB are finished.
++#!/usr/bin/env python3
-+ *    the sheepdog connection.  We switch back to sd_co_readv/sd_writev
-+ *    after all the requests belonging to the AIOCB are finished.  If
+ # Tool for running fuzz tests
-+ *    needed, sd_co_writev will send another requests for the vdi object.
+ #
-  */
+@@ -XXX,XX +XXX,XX @@
+ # along with this program.  If not, see <http://www.gnu.org/licenses/>.
- static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
+ #
-@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
-     acb->nr_pending--;
+-from __future__ import print_function
- }
+ import sys
+ import os
--static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
+ import signal
 -{
 -    qemu_coroutine_enter(acb->coroutine);
 -    qemu_aio_unref(acb);
 -}
 -
  static const AIOCBInfo sd_aiocb_info = {
      .aiocb_size     = sizeof(SheepdogAIOCB),
  };
@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
      acb->sector_num = sector_num;
      acb->nb_sectors = nb_sectors;
 -    acb->aio_done_func = NULL;
      acb->coroutine = qemu_coroutine_self();
      acb->ret = 0;
      acb->nr_pending = 0;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
      switch (acb->aiocb_type) {
      case AIOCB_WRITE_UDATA:
 -        /* this coroutine context is no longer suitable for co_recv
 -         * because we may send data to update vdi objects */
 -        s->co_recv = NULL;
          if (!is_data_obj(aio_req->oid)) {
              break;
          }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
          }
      }
 +    /* No more data for this aio_req (reload_inode below uses its own file
 +     * descriptor handler which doesn't use co_recv).
 +    */
 +    s->co_recv = NULL;
 +
      switch (rsp.result) {
      case SD_RES_SUCCESS:
          break;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
              aio_req->oid = vid_to_vdi_oid(s->inode.vdi_id);
          }
          resend_aioreq(s, aio_req);
 -        goto out;
 +        return;
      default:
          acb->ret = -EIO;
          error_report("%s", sd_strerror(rsp.result));
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
           * We've finished all requests which belong to the AIOCB, so
           * we can switch back to sd_co_readv/writev now.
           */
 -        acb->aio_done_func(acb);
 +        qemu_coroutine_enter(acb->coroutine);
      }
 -out:
 -    s->co_recv = NULL;
 +
      return;
 +
  err:
 -    s->co_recv = NULL;
      reconnect_to_sdog(opaque);
  }
@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
  /*
   * This function is called after writing data objects.  If we need to
   * update metadata, this sends a write request to the vdi object.
 - * Otherwise, this switches back to sd_co_readv/writev.
   */
  static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
  {
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
      mx = acb->max_dirty_data_idx;
      if (mn <= mx) {
          /* we need to update the vdi object. */
 +        ++acb->nr_pending;
          offset = sizeof(s->inode) - sizeof(s->inode.data_vdi_id) +
              mn * sizeof(s->inode.data_vdi_id[0]);
          data_len = (mx - mn + 1) * sizeof(s->inode.data_vdi_id[0]);
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
                                  data_len, offset, 0, false, 0, offset);
          QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
          add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
 -
 -        acb->aio_done_func = sd_finish_aiocb;
 -        acb->aiocb_type = AIOCB_WRITE_UDATA;
 -        return;
 +        if (--acb->nr_pending) {
 +            qemu_coroutine_yield();
 +        }
      }
 -
 -    sd_finish_aiocb(acb);
  }
  /* Delete current working VDI on the snapshot chain */
@@ -XXX,XX +XXX,XX @@ out:
   * Returns 1 when we need to wait a response, 0 when there is no sent
   * request and -errno in error cases.
   */
 -static int coroutine_fn sd_co_rw_vector(void *p)
 +static void coroutine_fn sd_co_rw_vector(void *p)
  {
      SheepdogAIOCB *acb = p;
      int ret = 0;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_rw_vector(void *p)
          ret = sd_create_branch(s);
          if (ret) {
              acb->ret = -EIO;
 -            goto out;
 +            return;
          }
      }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_rw_vector(void *p)
          idx++;
          done += len;
      }
 -out:
 -    if (!--acb->nr_pending) {
 -        return acb->ret;
 +    if (--acb->nr_pending) {
 +        qemu_coroutine_yield();
      }
 -    return 1;
  }
  static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
      }
      acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
 -    acb->aio_done_func = sd_write_done;
      acb->aiocb_type = AIOCB_WRITE_UDATA;
  retry:
@@ -XXX,XX +XXX,XX @@ retry:
          goto retry;
      }
 -    ret = sd_co_rw_vector(acb);
 -    if (ret <= 0) {
 -        QLIST_REMOVE(acb, aiocb_siblings);
 -        qemu_co_queue_restart_all(&s->overlapping_queue);
 -        qemu_aio_unref(acb);
 -        return ret;
 -    }
 -
 -    qemu_coroutine_yield();
 +    sd_co_rw_vector(acb);
 +    sd_write_done(acb);
      QLIST_REMOVE(acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -
 -    return acb->ret;
 +    ret = acb->ret;
 +    qemu_aio_unref(acb);
 +    return ret;
  }
  static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
      acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
      acb->aiocb_type = AIOCB_READ_UDATA;
 -    acb->aio_done_func = sd_finish_aiocb;
  retry:
      if (check_overlapping_aiocb(s, acb)) {
@@ -XXX,XX +XXX,XX @@ retry:
          goto retry;
      }
 -    ret = sd_co_rw_vector(acb);
 -    if (ret <= 0) {
 -        QLIST_REMOVE(acb, aiocb_siblings);
 -        qemu_co_queue_restart_all(&s->overlapping_queue);
 -        qemu_aio_unref(acb);
 -        return ret;
 -    }
 -
 -    qemu_coroutine_yield();
 +    sd_co_rw_vector(acb);
      QLIST_REMOVE(acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -    return acb->ret;
 +    ret = acb->ret;
 +    qemu_aio_unref(acb);
 +    return ret;
  }
  static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
  {
      BDRVSheepdogState *s = bs->opaque;
      SheepdogAIOCB *acb;
 +    int ret;
      AIOReq *aio_req;
      if (s->cache_flags != SD_FLAG_CMD_CACHE) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
      acb = sd_aio_setup(bs, NULL, 0, 0);
      acb->aiocb_type = AIOCB_FLUSH_CACHE;
 -    acb->aio_done_func = sd_finish_aiocb;
 +    acb->nr_pending++;
      aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
 , 0, 0, false, 0, 0);
      QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
      add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
 -    qemu_coroutine_yield();
 -    return acb->ret;
 +    if (--acb->nr_pending) {
 +        qemu_coroutine_yield();
 +    }
 +    ret = acb->ret;
 +    qemu_aio_unref(acb);
 +    return ret;
  }
  static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
      acb = sd_aio_setup(bs, &discard_iov, offset >> BDRV_SECTOR_BITS,
                         count >> BDRV_SECTOR_BITS);
      acb->aiocb_type = AIOCB_DISCARD_OBJ;
 -    acb->aio_done_func = sd_finish_aiocb;
  retry:
      if (check_overlapping_aiocb(s, acb)) {
@@ -XXX,XX +XXX,XX @@ retry:
          goto retry;
      }
 -    ret = sd_co_rw_vector(acb);
 -    if (ret <= 0) {
 -        QLIST_REMOVE(acb, aiocb_siblings);
 -        qemu_co_queue_restart_all(&s->overlapping_queue);
 -        qemu_aio_unref(acb);
 -        return ret;
 -    }
 -
 -    qemu_coroutine_yield();
 +    sd_co_rw_vector(acb);
      QLIST_REMOVE(acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -
 -    return acb->ret;
 +    ret = acb->ret;
 +    qemu_aio_unref(acb);
 +    return ret;
  }
  static coroutine_fn int64_t
 --
-.9.3
+.23.0

-[Qemu-devel] [PULL 1/5] sheepdog: remove unused cancellation support
+[PULL 10/11] image-fuzzer: Use errors parameter of subprocess.Popen()
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Eduardo Habkost <ehabkost@redhat.com>
-SheepdogAIOCB is internal to sheepdog.c, hence it is never canceled.
+Instead of manually encoding stderr and stdout output, use
 `errors` parameter of subprocess.Popen().  This will make
 process.communicate() return unicode strings instead of bytes
 objects.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
-Message-id: 20161129113245.32724-2-pbonzini@redhat.com
+Reviewed-by: John Snow <jsnow@redhat.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20191016192430.25098-11-ehabkost@redhat.com
 Message-Id: <20191016192430.25098-11-ehabkost@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 52 ----------------------------------------------------
+ tests/image-fuzzer/runner.py | 11 ++++-------
-file changed, 52 deletions(-)
+file changed, 4 insertions(+), 7 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
-index XXXXXXX..XXXXXXX 100644
+index XXXXXXX..XXXXXXX 100755
---- a/block/sheepdog.c
+--- a/tests/image-fuzzer/runner.py
-+++ b/block/sheepdog.c
++++ b/tests/image-fuzzer/runner.py
-@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
+@@ -XXX,XX +XXX,XX @@ def run_app(fd, q_args):
-     Coroutine *coroutine;
+     devnull = open('/dev/null', 'r+')
-     void (*aio_done_func)(SheepdogAIOCB *);
+     process = subprocess.Popen(q_args, stdin=devnull,
+                                stdout=subprocess.PIPE,
--    bool cancelable;
+-                               stderr=subprocess.PIPE)
-     int nr_pending;
++                               stderr=subprocess.PIPE,
++                               errors='replace')
-     uint32_t min_affect_data_idx;
+     try:
-@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
+         out, err = process.communicate()
- {
+         signal.alarm(0)
-     SheepdogAIOCB *acb = aio_req->aiocb;
+-        # fd is a text file, so we need to decode the process output before
+-        # writing to it.
--    acb->cancelable = false;
+-        # We could be simply using the `errors` parameter of subprocess.Popen(),
-     QLIST_REMOVE(aio_req, aio_siblings);
+-        # but this will be possible only after migrating to Python 3
-     g_free(aio_req);
+-        fd.write(out.decode(errors='replace'))
+-        fd.write(err.decode(errors='replace'))
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
++        fd.write(out)
-     qemu_aio_unref(acb);
++        fd.write(err)
- }
+         fd.flush()
+         return process.returncode
--/*
 - * Check whether the specified acb can be canceled
 - *
 - * We can cancel aio when any request belonging to the acb is:
 - *  - Not processed by the sheepdog server.
 - *  - Not linked to the inflight queue.
 - */
 -static bool sd_acb_cancelable(const SheepdogAIOCB *acb)
 -{
 -    BDRVSheepdogState *s = acb->common.bs->opaque;
 -    AIOReq *aioreq;
 -
 -    if (!acb->cancelable) {
 -        return false;
 -    }
 -
 -    QLIST_FOREACH(aioreq, &s->inflight_aio_head, aio_siblings) {
 -        if (aioreq->aiocb == acb) {
 -            return false;
 -        }
 -    }
 -
 -    return true;
 -}
 -
 -static void sd_aio_cancel(BlockAIOCB *blockacb)
 -{
 -    SheepdogAIOCB *acb = (SheepdogAIOCB *)blockacb;
 -    BDRVSheepdogState *s = acb->common.bs->opaque;
 -    AIOReq *aioreq, *next;
 -
 -    if (sd_acb_cancelable(acb)) {
 -        /* Remove outstanding requests from failed queue.  */
 -        QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings,
 -                           next) {
 -            if (aioreq->aiocb == acb) {
 -                free_aio_req(s, aioreq);
 -            }
 -        }
 -
 -        assert(acb->nr_pending == 0);
 -        if (acb->common.cb) {
 -            acb->common.cb(acb->common.opaque, -ECANCELED);
 -        }
 -        sd_finish_aiocb(acb);
 -    }
 -}
 -
  static const AIOCBInfo sd_aiocb_info = {
      .aiocb_size     = sizeof(SheepdogAIOCB),
 -    .cancel_async   = sd_aio_cancel,
  };
  static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
      acb->nb_sectors = nb_sectors;
      acb->aio_done_func = NULL;
 -    acb->cancelable = true;
      acb->coroutine = qemu_coroutine_self();
      acb->ret = 0;
      acb->nr_pending = 0;
 --
-.9.3
+.23.0

-[Qemu-devel] [PULL 5/5] sheepdog: reorganize check for overlapping requests
+[PULL 11/11] image-fuzzer: Use OSerror.strerror instead of tuple subscript
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Eduardo Habkost <ehabkost@redhat.com>
-Wrap the code that was copied repeatedly in the two functions,
+OSError can't be used like a tuple on Python 3, so change the
-sd_aio_setup and sd_aio_complete.
+code to use `e.sterror` instead of `e[1]`.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Reported-by: John Snow <jsnow@redhat.com>
-Message-id: 20161129113245.32724-6-pbonzini@redhat.com
+Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
 Message-id: 20191021214117.18091-1-ehabkost@redhat.com
 Message-Id: <20191021214117.18091-1-ehabkost@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 66 ++++++++++++++++++++++++++------------------------------
+ tests/image-fuzzer/runner.py | 4 ++--
-file changed, 30 insertions(+), 36 deletions(-)
+file changed, 2 insertions(+), 2 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
-index XXXXXXX..XXXXXXX 100644
+index XXXXXXX..XXXXXXX 100755
---- a/block/sheepdog.c
+--- a/tests/image-fuzzer/runner.py
-+++ b/block/sheepdog.c
++++ b/tests/image-fuzzer/runner.py
-@@ -XXX,XX +XXX,XX @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
+@@ -XXX,XX +XXX,XX @@ class TestEnv(object):
-     return aio_req;
+             os.makedirs(self.current_dir)
- }
+         except OSError as e:
+             print("Error: The working directory '%s' cannot be used. Reason: %s"\
-+static void wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *acb)
+-                % (self.work_dir, e[1]), file=sys.stderr)
-+{
++                % (self.work_dir, e.strerror), file=sys.stderr)
-+    SheepdogAIOCB *cb;
+             raise TestException
-+
+         self.log = open(os.path.join(self.current_dir, "test.log"), "w")
-+retry:
+         self.parent_log = open(run_log, "a")
-+    QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) {
+@@ -XXX,XX +XXX,XX @@ class TestEnv(object):
-+        if (AIOCBOverlapping(acb, cb)) {
+             except OSError as e:
-+            qemu_co_queue_wait(&s->overlapping_queue);
+                 multilog("%sError: Start of '%s' failed. Reason: %s\n\n"
-+            goto retry;
+                          % (test_summary, os.path.basename(current_cmd[0]),
-+        }
+-                            e[1]),
-+    }
++                            e.strerror),
-+}
+                          sys.stderr, self.log, self.parent_log)
-+
+                 raise TestException
  static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
                           QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
                           int type)
@@ -XXX,XX +XXX,XX @@ static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
      acb->min_dirty_data_idx = UINT32_MAX;
      acb->max_dirty_data_idx = 0;
      acb->aiocb_type = type;
 +
 +    if (type == AIOCB_FLUSH_CACHE) {
 +        return;
 +    }
 +
 +    wait_for_overlapping_aiocb(s, acb);
 +    QLIST_INSERT_HEAD(&s->inflight_aiocb_head, acb, aiocb_siblings);
  }
  /* Return -EIO in case of error, file descriptor on success */
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
      }
  }
 -static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
 +static void sd_aio_complete(SheepdogAIOCB *acb)
  {
 -    SheepdogAIOCB *cb;
 -
 -    QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) {
 -        if (AIOCBOverlapping(aiocb, cb)) {
 -            return true;
 -        }
 +    if (acb->aiocb_type == AIOCB_FLUSH_CACHE) {
 +        return;
      }
 -    QLIST_INSERT_HEAD(&s->inflight_aiocb_head, aiocb, aiocb_siblings);
 -    return false;
 +    QLIST_REMOVE(acb, aiocb_siblings);
 +    qemu_co_queue_restart_all(&acb->s->overlapping_queue);
  }
  static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
      }
      sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_WRITE_UDATA);
 -
 -retry:
 -    if (check_overlapping_aiocb(s, &acb)) {
 -        qemu_co_queue_wait(&s->overlapping_queue);
 -        goto retry;
 -    }
 -
      sd_co_rw_vector(&acb);
      sd_write_done(&acb);
 +    sd_aio_complete(&acb);
 -    QLIST_REMOVE(&acb, aiocb_siblings);
 -    qemu_co_queue_restart_all(&s->overlapping_queue);
      return acb.ret;
  }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
      BDRVSheepdogState *s = bs->opaque;
      sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_READ_UDATA);
 -
 -retry:
 -    if (check_overlapping_aiocb(s, &acb)) {
 -        qemu_co_queue_wait(&s->overlapping_queue);
 -        goto retry;
 -    }
 -
      sd_co_rw_vector(&acb);
 +    sd_aio_complete(&acb);
 -    QLIST_REMOVE(&acb, aiocb_siblings);
 -    qemu_co_queue_restart_all(&s->overlapping_queue);
      return acb.ret;
  }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
      if (--acb.nr_pending) {
          qemu_coroutine_yield();
      }
 +
 +    sd_aio_complete(&acb);
      return acb.ret;
  }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
      }
      sd_aio_setup(&acb, s, &discard_iov, offset >> BDRV_SECTOR_BITS,
                   count >> BDRV_SECTOR_BITS, AIOCB_DISCARD_OBJ);
 -
 -retry:
 -    if (check_overlapping_aiocb(s, &acb)) {
 -        qemu_co_queue_wait(&s->overlapping_queue);
 -        goto retry;
 -    }
 -
      sd_co_rw_vector(&acb);
 +    sd_aio_complete(&acb);
 -    QLIST_REMOVE(&acb, aiocb_siblings);
 -    qemu_co_queue_restart_all(&s->overlapping_queue);
      return acb.ret;
  }
 --
-.9.3
+.23.0

From: Paolo Bonzini <pbonzini@redhat.com>

SheepdogAIOCB is internal to sheepdog.c, hence it is never canceled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161129113245.32724-2-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 52 ----------------------------------------------------
 1 file changed, 52 deletions(-)

From: Paolo Bonzini <pbonzini@redhat.com>

Delimit co_recv's lifetime clearly in aio_read_response.

Do a simple qemu_coroutine_enter in aio_read_response, letting
sd_co_writev call sd_write_done.

Handle nr_pending in the same way in sd_co_rw_vector,
sd_write_done and sd_co_flush_to_disk.

Remove sd_co_rw_vector's return value; just leave with no
pending requests.

[Jeff: added missing 'return' back, spotted by Paolo after
       series was applied.]

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 115 ++++++++++++++++++++-----------------------------------
 1 file changed, 42 insertions(+), 73 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
     enum AIOCBState aiocb_type;
 
     Coroutine *coroutine;
-    void (*aio_done_func)(SheepdogAIOCB *);
-
     int nr_pending;
 
     uint32_t min_affect_data_idx;
@@ -XXX,XX +XXX,XX @@ static const char * sd_strerror(int err)
  *
  * 1. In sd_co_rw_vector, we send the I/O requests to the server and
  *    link the requests to the inflight_list in the
- *    BDRVSheepdogState.  The function exits without waiting for
+ *    BDRVSheepdogState.  The function yields while waiting for
  *    receiving the response.
  *
  * 2. We receive the response in aio_read_response, the fd handler to
- *    the sheepdog connection.  If metadata update is needed, we send
- *    the write request to the vdi object in sd_write_done, the write
- *    completion function.  We switch back to sd_co_readv/writev after
- *    all the requests belonging to the AIOCB are finished.
+ *    the sheepdog connection.  We switch back to sd_co_readv/sd_writev
+ *    after all the requests belonging to the AIOCB are finished.  If
+ *    needed, sd_co_writev will send another requests for the vdi object.
  */
 
 static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
     acb->nr_pending--;
 }
 
-static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
-{
-    qemu_coroutine_enter(acb->coroutine);
-    qemu_aio_unref(acb);
-}
-
 static const AIOCBInfo sd_aiocb_info = {
     .aiocb_size     = sizeof(SheepdogAIOCB),
 };
@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
     acb->sector_num = sector_num;
     acb->nb_sectors = nb_sectors;
 
-    acb->aio_done_func = NULL;
     acb->coroutine = qemu_coroutine_self();
     acb->ret = 0;
     acb->nr_pending = 0;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
 
     switch (acb->aiocb_type) {
     case AIOCB_WRITE_UDATA:
-        /* this coroutine context is no longer suitable for co_recv
-         * because we may send data to update vdi objects */
-        s->co_recv = NULL;
         if (!is_data_obj(aio_req->oid)) {
             break;
         }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
         }
     }
 
+    /* No more data for this aio_req (reload_inode below uses its own file
+     * descriptor handler which doesn't use co_recv).
+    */
+    s->co_recv = NULL;
+
     switch (rsp.result) {
     case SD_RES_SUCCESS:
         break;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
             aio_req->oid = vid_to_vdi_oid(s->inode.vdi_id);
         }
         resend_aioreq(s, aio_req);
-        goto out;
+        return;
     default:
         acb->ret = -EIO;
         error_report("%s", sd_strerror(rsp.result));
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
          * We've finished all requests which belong to the AIOCB, so
          * we can switch back to sd_co_readv/writev now.
          */
-        acb->aio_done_func(acb);
+        qemu_coroutine_enter(acb->coroutine);
     }
-out:
-    s->co_recv = NULL;
+
     return;
+
 err:
-    s->co_recv = NULL;
     reconnect_to_sdog(opaque);
 }
 
@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
 /*
  * This function is called after writing data objects.  If we need to
  * update metadata, this sends a write request to the vdi object.
- * Otherwise, this switches back to sd_co_readv/writev.
  */
 static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
 {
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
     mx = acb->max_dirty_data_idx;
     if (mn <= mx) {
         /* we need to update the vdi object. */
+        ++acb->nr_pending;
         offset = sizeof(s->inode) - sizeof(s->inode.data_vdi_id) +
             mn * sizeof(s->inode.data_vdi_id[0]);
         data_len = (mx - mn + 1) * sizeof(s->inode.data_vdi_id[0]);
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
                                 data_len, offset, 0, false, 0, offset);
         QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
         add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
-
-        acb->aio_done_func = sd_finish_aiocb;
-        acb->aiocb_type = AIOCB_WRITE_UDATA;
-        return;
+        if (--acb->nr_pending) {
+            qemu_coroutine_yield();
+        }
     }
-
-    sd_finish_aiocb(acb);
 }
 
 /* Delete current working VDI on the snapshot chain */
@@ -XXX,XX +XXX,XX @@ out:
  * Returns 1 when we need to wait a response, 0 when there is no sent
  * request and -errno in error cases.
  */
-static int coroutine_fn sd_co_rw_vector(void *p)
+static void coroutine_fn sd_co_rw_vector(void *p)
 {
     SheepdogAIOCB *acb = p;
     int ret = 0;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_rw_vector(void *p)
         ret = sd_create_branch(s);
         if (ret) {
             acb->ret = -EIO;
-            goto out;
+            return;
         }
     }
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_rw_vector(void *p)
         idx++;
         done += len;
     }
-out:
-    if (!--acb->nr_pending) {
-        return acb->ret;
+    if (--acb->nr_pending) {
+        qemu_coroutine_yield();
     }
-    return 1;
 }
 
 static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
     }
 
     acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
-    acb->aio_done_func = sd_write_done;
     acb->aiocb_type = AIOCB_WRITE_UDATA;
 
 retry:
@@ -XXX,XX +XXX,XX @@ retry:
         goto retry;
     }
 
-    ret = sd_co_rw_vector(acb);
-    if (ret <= 0) {
-        QLIST_REMOVE(acb, aiocb_siblings);
-        qemu_co_queue_restart_all(&s->overlapping_queue);
-        qemu_aio_unref(acb);
-        return ret;
-    }
-
-    qemu_coroutine_yield();
+    sd_co_rw_vector(acb);
+    sd_write_done(acb);
 
     QLIST_REMOVE(acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-
-    return acb->ret;
+    ret = acb->ret;
+    qemu_aio_unref(acb);
+    return ret;
 }
 
 static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
 
     acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
     acb->aiocb_type = AIOCB_READ_UDATA;
-    acb->aio_done_func = sd_finish_aiocb;
 
 retry:
     if (check_overlapping_aiocb(s, acb)) {
@@ -XXX,XX +XXX,XX @@ retry:
         goto retry;
     }
 
-    ret = sd_co_rw_vector(acb);
-    if (ret <= 0) {
-        QLIST_REMOVE(acb, aiocb_siblings);
-        qemu_co_queue_restart_all(&s->overlapping_queue);
-        qemu_aio_unref(acb);
-        return ret;
-    }
-
-    qemu_coroutine_yield();
+    sd_co_rw_vector(acb);
 
     QLIST_REMOVE(acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-    return acb->ret;
+    ret = acb->ret;
+    qemu_aio_unref(acb);
+    return ret;
 }
 
 static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
 {
     BDRVSheepdogState *s = bs->opaque;
     SheepdogAIOCB *acb;
+    int ret;
     AIOReq *aio_req;
 
     if (s->cache_flags != SD_FLAG_CMD_CACHE) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
 
     acb = sd_aio_setup(bs, NULL, 0, 0);
     acb->aiocb_type = AIOCB_FLUSH_CACHE;
-    acb->aio_done_func = sd_finish_aiocb;
 
+    acb->nr_pending++;
     aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
                             0, 0, 0, false, 0, 0);
     QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
     add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
 
-    qemu_coroutine_yield();
-    return acb->ret;
+    if (--acb->nr_pending) {
+        qemu_coroutine_yield();
+    }
+    ret = acb->ret;
+    qemu_aio_unref(acb);
+    return ret;
 }
 
 static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
     acb = sd_aio_setup(bs, &discard_iov, offset >> BDRV_SECTOR_BITS,
                        count >> BDRV_SECTOR_BITS);
     acb->aiocb_type = AIOCB_DISCARD_OBJ;
-    acb->aio_done_func = sd_finish_aiocb;
 
 retry:
     if (check_overlapping_aiocb(s, acb)) {
@@ -XXX,XX +XXX,XX @@ retry:
         goto retry;
     }
 
-    ret = sd_co_rw_vector(acb);
-    if (ret <= 0) {
-        QLIST_REMOVE(acb, aiocb_siblings);
-        qemu_co_queue_restart_all(&s->overlapping_queue);
-        qemu_aio_unref(acb);
-        return ret;
-    }
-
-    qemu_coroutine_yield();
+    sd_co_rw_vector(acb);
 
     QLIST_REMOVE(acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-
-    return acb->ret;
+    ret = acb->ret;
+    qemu_aio_unref(acb);
+    return ret;
 }
 
 static coroutine_fn int64_t
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Sheepdog's AIOCB are completely internal entities for a group of
requests and do not need dynamic allocation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161129113245.32724-4-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 99 ++++++++++++++++++++++----------------------------------
 1 file changed, 39 insertions(+), 60 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static inline size_t count_data_objs(const struct SheepdogInode *inode)
     } while (0)
 
 typedef struct SheepdogAIOCB SheepdogAIOCB;
+typedef struct BDRVSheepdogState BDRVSheepdogState;
 
 typedef struct AIOReq {
     SheepdogAIOCB *aiocb;
@@ -XXX,XX +XXX,XX @@ enum AIOCBState {
        || y->max_affect_data_idx < x->min_affect_data_idx))
 
 struct SheepdogAIOCB {
-    BlockAIOCB common;
+    BDRVSheepdogState *s;
 
     QEMUIOVector *qiov;
 
@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
     QLIST_ENTRY(SheepdogAIOCB) aiocb_siblings;
 };
 
-typedef struct BDRVSheepdogState {
+struct BDRVSheepdogState {
     BlockDriverState *bs;
     AioContext *aio_context;
 
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVSheepdogState {
 
     CoQueue overlapping_queue;
     QLIST_HEAD(inflight_aiocb_head, SheepdogAIOCB) inflight_aiocb_head;
-} BDRVSheepdogState;
+};
 
 typedef struct BDRVSheepdogReopenState {
     int fd;
@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
     acb->nr_pending--;
 }
 
-static const AIOCBInfo sd_aiocb_info = {
-    .aiocb_size     = sizeof(SheepdogAIOCB),
-};
-
-static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
-                                   int64_t sector_num, int nb_sectors)
+static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
+                         QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
+                         int type)
 {
-    SheepdogAIOCB *acb;
     uint32_t object_size;
-    BDRVSheepdogState *s = bs->opaque;
 
     object_size = (UINT32_C(1) << s->inode.block_size_shift);
 
-    acb = qemu_aio_get(&sd_aiocb_info, bs, NULL, NULL);
+    acb->s = s;
 
     acb->qiov = qiov;
 
@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
 
     acb->min_dirty_data_idx = UINT32_MAX;
     acb->max_dirty_data_idx = 0;
-
-    return acb;
+    acb->aiocb_type = type;
 }
 
 /* Return -EIO in case of error, file descriptor on success */
@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
  */
 static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
 {
-    BDRVSheepdogState *s = acb->common.bs->opaque;
+    BDRVSheepdogState *s = acb->s;
     struct iovec iov;
     AIOReq *aio_req;
     uint32_t offset, data_len, mn, mx;
@@ -XXX,XX +XXX,XX @@ out:
  * Returns 1 when we need to wait a response, 0 when there is no sent
  * request and -errno in error cases.
  */
-static void coroutine_fn sd_co_rw_vector(void *p)
+static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
 {
-    SheepdogAIOCB *acb = p;
     int ret = 0;
     unsigned long len, done = 0, total = acb->nb_sectors * BDRV_SECTOR_SIZE;
     unsigned long idx;
     uint32_t object_size;
     uint64_t oid;
     uint64_t offset;
-    BDRVSheepdogState *s = acb->common.bs->opaque;
+    BDRVSheepdogState *s = acb->s;
     SheepdogInode *inode = &s->inode;
     AIOReq *aio_req;
 
@@ -XXX,XX +XXX,XX @@ static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
 static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
                         int nb_sectors, QEMUIOVector *qiov)
 {
-    SheepdogAIOCB *acb;
+    SheepdogAIOCB acb;
     int ret;
     int64_t offset = (sector_num + nb_sectors) * BDRV_SECTOR_SIZE;
     BDRVSheepdogState *s = bs->opaque;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
         }
     }
 
-    acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
-    acb->aiocb_type = AIOCB_WRITE_UDATA;
+    sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_WRITE_UDATA);
 
 retry:
-    if (check_overlapping_aiocb(s, acb)) {
+    if (check_overlapping_aiocb(s, &acb)) {
         qemu_co_queue_wait(&s->overlapping_queue);
         goto retry;
     }
 
-    sd_co_rw_vector(acb);
-    sd_write_done(acb);
+    sd_co_rw_vector(&acb);
+    sd_write_done(&acb);
 
-    QLIST_REMOVE(acb, aiocb_siblings);
+    QLIST_REMOVE(&acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-    ret = acb->ret;
-    qemu_aio_unref(acb);
-    return ret;
+    return acb.ret;
 }
 
 static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
                        int nb_sectors, QEMUIOVector *qiov)
 {
-    SheepdogAIOCB *acb;
-    int ret;
+    SheepdogAIOCB acb;
     BDRVSheepdogState *s = bs->opaque;
 
-    acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
-    acb->aiocb_type = AIOCB_READ_UDATA;
+    sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_READ_UDATA);
 
 retry:
-    if (check_overlapping_aiocb(s, acb)) {
+    if (check_overlapping_aiocb(s, &acb)) {
         qemu_co_queue_wait(&s->overlapping_queue);
         goto retry;
     }
 
-    sd_co_rw_vector(acb);
+    sd_co_rw_vector(&acb);
 
-    QLIST_REMOVE(acb, aiocb_siblings);
+    QLIST_REMOVE(&acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-    ret = acb->ret;
-    qemu_aio_unref(acb);
-    return ret;
+    return acb.ret;
 }
 
 static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
 {
     BDRVSheepdogState *s = bs->opaque;
-    SheepdogAIOCB *acb;
-    int ret;
+    SheepdogAIOCB acb;
     AIOReq *aio_req;
 
     if (s->cache_flags != SD_FLAG_CMD_CACHE) {
         return 0;
     }
 
-    acb = sd_aio_setup(bs, NULL, 0, 0);
-    acb->aiocb_type = AIOCB_FLUSH_CACHE;
+    sd_aio_setup(&acb, s, NULL, 0, 0, AIOCB_FLUSH_CACHE);
 
-    acb->nr_pending++;
-    aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
+    acb.nr_pending++;
+    aio_req = alloc_aio_req(s, &acb, vid_to_vdi_oid(s->inode.vdi_id),
                             0, 0, 0, false, 0, 0);
     QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
-    add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
+    add_aio_request(s, aio_req, NULL, 0, acb.aiocb_type);
 
-    if (--acb->nr_pending) {
+    if (--acb.nr_pending) {
         qemu_coroutine_yield();
     }
-    ret = acb->ret;
-    qemu_aio_unref(acb);
-    return ret;
+    return acb.ret;
 }
 
 static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
@@ -XXX,XX +XXX,XX @@ static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
 static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
                                       int count)
 {
-    SheepdogAIOCB *acb;
+    SheepdogAIOCB acb;
     BDRVSheepdogState *s = bs->opaque;
-    int ret;
     QEMUIOVector discard_iov;
     struct iovec iov;
     uint32_t zero = 0;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
     if (!QEMU_IS_ALIGNED(offset | count, BDRV_SECTOR_SIZE)) {
         return -ENOTSUP;
     }
-    acb = sd_aio_setup(bs, &discard_iov, offset >> BDRV_SECTOR_BITS,
-                       count >> BDRV_SECTOR_BITS);
-    acb->aiocb_type = AIOCB_DISCARD_OBJ;
+    sd_aio_setup(&acb, s, &discard_iov, offset >> BDRV_SECTOR_BITS,
+                 count >> BDRV_SECTOR_BITS, AIOCB_DISCARD_OBJ);
 
 retry:
-    if (check_overlapping_aiocb(s, acb)) {
+    if (check_overlapping_aiocb(s, &acb)) {
         qemu_co_queue_wait(&s->overlapping_queue);
         goto retry;
     }
 
-    sd_co_rw_vector(acb);
+    sd_co_rw_vector(&acb);
 
-    QLIST_REMOVE(acb, aiocb_siblings);
+    QLIST_REMOVE(&acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-    ret = acb->ret;
-    qemu_aio_unref(acb);
-    return ret;
+    return acb.ret;
 }
 
 static coroutine_fn int64_t
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Add to the list in add_aio_request and, indirectly, resend_aioreq.  Inline
free_aio_req in the caller, it does not simply undo alloc_aio_req's job.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161129113245.32724-5-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
     return aio_req;
 }
 
-static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
-{
-    SheepdogAIOCB *acb = aio_req->aiocb;
-
-    QLIST_REMOVE(aio_req, aio_siblings);
-    g_free(aio_req);
-
-    acb->nr_pending--;
-}
-
 static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
                          QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
                          int type)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
     while (!QLIST_EMPTY(&s->failed_aio_head)) {
         aio_req = QLIST_FIRST(&s->failed_aio_head);
         QLIST_REMOVE(aio_req, aio_siblings);
-        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
         resend_aioreq(s, aio_req);
     }
 }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
     */
     s->co_recv = NULL;
 
+    QLIST_REMOVE(aio_req, aio_siblings);
     switch (rsp.result) {
     case SD_RES_SUCCESS:
         break;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
         break;
     }
 
-    free_aio_req(s, aio_req);
-    if (!acb->nr_pending) {
+    g_free(aio_req);
+
+    if (!--acb->nr_pending) {
         /*
          * We've finished all requests which belong to the AIOCB, so
          * we can switch back to sd_co_readv/writev now.
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
     uint64_t old_oid = aio_req->base_oid;
     bool create = aio_req->create;
 
+    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
+
     if (!nr_copies) {
         error_report("bug");
     }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
         iov.iov_len = sizeof(s->inode);
         aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
                                 data_len, offset, 0, false, 0, offset);
-        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
         add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
         if (--acb->nr_pending) {
             qemu_coroutine_yield();
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
                                 old_oid,
                                 acb->aiocb_type == AIOCB_DISCARD_OBJ ?
                                 0 : done);
-        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
-
         add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
                         acb->aiocb_type);
     done:
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
     acb.nr_pending++;
     aio_req = alloc_aio_req(s, &acb, vid_to_vdi_oid(s->inode.vdi_id),
                             0, 0, 0, false, 0, 0);
-    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
     add_aio_request(s, aio_req, NULL, 0, acb.aiocb_type);
 
     if (--acb.nr_pending) {
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Wrap the code that was copied repeatedly in the two functions,
sd_aio_setup and sd_aio_complete.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161129113245.32724-6-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 66 ++++++++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 36 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
     return aio_req;
 }
 
+static void wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *acb)
+{
+    SheepdogAIOCB *cb;
+
+retry:
+    QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) {
+        if (AIOCBOverlapping(acb, cb)) {
+            qemu_co_queue_wait(&s->overlapping_queue);
+            goto retry;
+        }
+    }
+}
+
 static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
                          QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
                          int type)
@@ -XXX,XX +XXX,XX @@ static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
     acb->min_dirty_data_idx = UINT32_MAX;
     acb->max_dirty_data_idx = 0;
     acb->aiocb_type = type;
+
+    if (type == AIOCB_FLUSH_CACHE) {
+        return;
+    }
+
+    wait_for_overlapping_aiocb(s, acb);
+    QLIST_INSERT_HEAD(&s->inflight_aiocb_head, acb, aiocb_siblings);
 }
 
 /* Return -EIO in case of error, file descriptor on success */
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
     }
 }
 
-static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
+static void sd_aio_complete(SheepdogAIOCB *acb)
 {
-    SheepdogAIOCB *cb;
-
-    QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) {
-        if (AIOCBOverlapping(aiocb, cb)) {
-            return true;
-        }
+    if (acb->aiocb_type == AIOCB_FLUSH_CACHE) {
+        return;
     }
 
-    QLIST_INSERT_HEAD(&s->inflight_aiocb_head, aiocb, aiocb_siblings);
-    return false;
+    QLIST_REMOVE(acb, aiocb_siblings);
+    qemu_co_queue_restart_all(&acb->s->overlapping_queue);
 }
 
 static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
     }
 
     sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_WRITE_UDATA);
-
-retry:
-    if (check_overlapping_aiocb(s, &acb)) {
-        qemu_co_queue_wait(&s->overlapping_queue);
-        goto retry;
-    }
-
     sd_co_rw_vector(&acb);
     sd_write_done(&acb);
+    sd_aio_complete(&acb);
 
-    QLIST_REMOVE(&acb, aiocb_siblings);
-    qemu_co_queue_restart_all(&s->overlapping_queue);
     return acb.ret;
 }
 
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
     BDRVSheepdogState *s = bs->opaque;
 
     sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_READ_UDATA);
-
-retry:
-    if (check_overlapping_aiocb(s, &acb)) {
-        qemu_co_queue_wait(&s->overlapping_queue);
-        goto retry;
-    }
-
     sd_co_rw_vector(&acb);
+    sd_aio_complete(&acb);
 
-    QLIST_REMOVE(&acb, aiocb_siblings);
-    qemu_co_queue_restart_all(&s->overlapping_queue);
     return acb.ret;
 }
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
     if (--acb.nr_pending) {
         qemu_coroutine_yield();
     }
+
+    sd_aio_complete(&acb);
     return acb.ret;
 }
 
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
     }
     sd_aio_setup(&acb, s, &discard_iov, offset >> BDRV_SECTOR_BITS,
                  count >> BDRV_SECTOR_BITS, AIOCB_DISCARD_OBJ);
-
-retry:
-    if (check_overlapping_aiocb(s, &acb)) {
-        qemu_co_queue_wait(&s->overlapping_queue);
-        goto retry;
-    }
-
     sd_co_rw_vector(&acb);
+    sd_aio_complete(&acb);
 
-    QLIST_REMOVE(&acb, aiocb_siblings);
-    qemu_co_queue_restart_all(&s->overlapping_queue);
     return acb.ret;
 }
 
-- 
2.9.3

The following changes since commit 36609b4fa36f0ac934874371874416f7533a5408:

Merge remote-tracking branch 'remotes/palmer/tags/palmer-for-master-4.2-sf1' into staging (2019-11-02 17:59:03 +0000)

are available in the Git repository at:

https://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 9fdd7860adec188ed50d2530e9a819e8d953f9bb:

image-fuzzer: Use OSerror.strerror instead of tuple subscript (2019-11-05 16:36:11 +0100)

----------------------------------------------------------------
Pull request

Let's get the image fuzzer Python 3 changes merged in QEMU 4.2.

----------------------------------------------------------------

Eduardo Habkost (11):
  image-fuzzer: Open image files in binary mode
  image-fuzzer: Write bytes instead of string to image file
  image-fuzzer: Explicitly use integer division operator
  image-fuzzer: Use io.StringIO
  image-fuzzer: Use %r for all fiels at Field.__repr__()
  image-fuzzer: Return bytes objects on string fuzzing functions
  image-fuzzer: Use bytes constant for field values
  image-fuzzer: Encode file name and file format to bytes
  image-fuzzer: Run using python3
  image-fuzzer: Use errors parameter of subprocess.Popen()
  image-fuzzer: Use OSerror.strerror instead of tuple subscript

tests/image-fuzzer/qcow2/__init__.py |  1 -
 tests/image-fuzzer/qcow2/fuzz.py     | 54 +++++++++++++-------------
 tests/image-fuzzer/qcow2/layout.py   | 57 ++++++++++++++--------------
 tests/image-fuzzer/runner.py         | 16 ++++----
 4 files changed, 63 insertions(+), 65 deletions(-)

-- 
2.23.0

From: Eduardo Habkost <ehabkost@redhat.com>

This probably never caused problems because on Linux there's no
actual newline conversion happening, but on Python 3 the
binary/text distinction is stronger and we must explicitly open
the image file in binary mode.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-2-ehabkost@redhat.com
Message-Id: <20191016192430.25098-2-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/layout.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

From: Eduardo Habkost <ehabkost@redhat.com>

This is necessary for Python 3 compatibility.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-3-ehabkost@redhat.com
Message-Id: <20191016192430.25098-3-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/layout.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

From: Eduardo Habkost <ehabkost@redhat.com>

Most of the division expressions in image-fuzzer assume integer
division.  Use the // operator to keep the same behavior when we
move to Python 3.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-4-ehabkost@redhat.com
Message-Id: <20191016192430.25098-4-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/fuzz.py   | 12 ++++-----
 tests/image-fuzzer/qcow2/layout.py | 40 +++++++++++++++---------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/fuzz.py b/tests/image-fuzzer/qcow2/fuzz.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/image-fuzzer/qcow2/fuzz.py
+++ b/tests/image-fuzzer/qcow2/fuzz.py
@@ -XXX,XX +XXX,XX @@ UINT64 = 0xffffffffffffffff
 UINT32_M = 31
 UINT64_M = 63
 # Fuzz vectors
-UINT8_V = [0, 0x10, UINT8/4, UINT8/2 - 1, UINT8/2, UINT8/2 + 1, UINT8 - 1,
+UINT8_V = [0, 0x10, UINT8//4, UINT8//2 - 1, UINT8//2, UINT8//2 + 1, UINT8 - 1,
            UINT8]
-UINT16_V = [0, 0x100, 0x1000, UINT16/4, UINT16/2 - 1, UINT16/2, UINT16/2 + 1,
+UINT16_V = [0, 0x100, 0x1000, UINT16//4, UINT16//2 - 1, UINT16//2, UINT16//2 + 1,
             UINT16 - 1, UINT16]
-UINT32_V = [0, 0x100, 0x1000, 0x10000, 0x100000, UINT32/4, UINT32/2 - 1,
-            UINT32/2, UINT32/2 + 1, UINT32 - 1, UINT32]
-UINT64_V = UINT32_V + [0x1000000, 0x10000000, 0x100000000, UINT64/4,
-                       UINT64/2 - 1, UINT64/2, UINT64/2 + 1, UINT64 - 1,
+UINT32_V = [0, 0x100, 0x1000, 0x10000, 0x100000, UINT32//4, UINT32//2 - 1,
+            UINT32//2, UINT32//2 + 1, UINT32 - 1, UINT32]
+UINT64_V = UINT32_V + [0x1000000, 0x10000000, 0x100000000, UINT64//4,
+                       UINT64//2 - 1, UINT64//2, UINT64//2 + 1, UINT64 - 1,
                        UINT64]
 STRING_V = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
             '%d%d%d%d', '%s%s%s%s', '%99999999999s', '%08x', '%%20d', '%%20n',
diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/image-fuzzer/qcow2/layout.py
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -XXX,XX +XXX,XX @@ class Image(object):
                 ['>I', self.ext_offset, 0x6803f857, 'ext_magic'],
                 # One feature table contains 3 fields and takes 48 bytes
                 ['>I', self.ext_offset + UINT32_S,
-                 len(feature_tables) / 3 * 48, 'ext_length']
+                 len(feature_tables) // 3 * 48, 'ext_length']
             ] + feature_tables)
             self.ext_offset = inner_offset
 
@@ -XXX,XX +XXX,XX @@ class Image(object):
         def create_l2_entry(host, guest, l2_cluster):
             """Generate one L2 entry."""
             offset = l2_cluster * self.cluster_size
-            l2_size = self.cluster_size / UINT64_S
+            l2_size = self.cluster_size // UINT64_S
             entry_offset = offset + UINT64_S * (guest % l2_size)
             cluster_descriptor = host * self.cluster_size
             if not self.header['version'][0].value == 2:
@@ -XXX,XX +XXX,XX @@ class Image(object):
 
         def create_l1_entry(l2_cluster, l1_offset, guest):
             """Generate one L1 entry."""
-            l2_size = self.cluster_size / UINT64_S
-            entry_offset = l1_offset + UINT64_S * (guest / l2_size)
+            l2_size = self.cluster_size // UINT64_S
+            entry_offset = l1_offset + UINT64_S * (guest // l2_size)
             # While snapshots are not supported bit #63 = 1
             entry_val = (1 << 63) + l2_cluster * self.cluster_size
             return ['>Q', entry_offset, entry_val, 'l1_entry']
@@ -XXX,XX +XXX,XX @@ class Image(object):
             l2 = []
         else:
             meta_data = self._get_metadata()
-            guest_clusters = random.sample(range(self.image_size /
+            guest_clusters = random.sample(range(self.image_size //
                                                  self.cluster_size),
                                            len(self.data_clusters))
             # Number of entries in a L1/L2 table
-            l_size = self.cluster_size / UINT64_S
+            l_size = self.cluster_size // UINT64_S
             # Number of clusters necessary for L1 table
             l1_size = int(ceil((max(guest_clusters) + 1) / float(l_size**2)))
             l1_start = self._get_adjacent_clusters(self.data_clusters |
@@ -XXX,XX +XXX,XX @@ class Image(object):
             # L2 entries
             l2 = []
             for host, guest in zip(self.data_clusters, guest_clusters):
-                l2_id = guest / l_size
+                l2_id = guest // l_size
                 if l2_id not in l2_ids:
                     l2_ids.append(l2_id)
                     l2_clusters.append(self._get_adjacent_clusters(
@@ -XXX,XX +XXX,XX @@ class Image(object):
         def allocate_rfc_blocks(data, size):
             """Return indices of clusters allocated for refcount blocks."""
             cluster_ids = set()
-            diff = block_ids = set([x / size for x in data])
+            diff = block_ids = set([x // size for x in data])
             while len(diff) != 0:
                 # Allocate all yet not allocated clusters
                 new = self._get_available_clusters(data | cluster_ids,
                                                    len(diff))
                 # Indices of new refcount blocks necessary to cover clusters
                 # in 'new'
-                diff = set([x / size for x in new]) - block_ids
+                diff = set([x // size for x in new]) - block_ids
                 cluster_ids |= new
                 block_ids |= diff
             return cluster_ids, block_ids
@@ -XXX,XX +XXX,XX @@ class Image(object):
             blocks = set(init_blocks)
             clusters = set()
             # Number of entries in one cluster of the refcount table
-            size = self.cluster_size / UINT64_S
+            size = self.cluster_size // UINT64_S
             # Number of clusters necessary for the refcount table based on
             # the current number of refcount blocks
             table_size = int(ceil((max(blocks) + 1) / float(size)))
@@ -XXX,XX +XXX,XX @@ class Image(object):
                                                  table_size + 1))
             # New refcount blocks necessary for clusters occupied by the
             # refcount table
-            diff = set([c / block_size for c in table_clusters]) - blocks
+            diff = set([c // block_size for c in table_clusters]) - blocks
             blocks |= diff
             while len(diff) != 0:
                 # Allocate clusters for new refcount blocks
@@ -XXX,XX +XXX,XX @@ class Image(object):
                                                    len(diff))
                 # Indices of new refcount blocks necessary to cover
                 # clusters in 'new'
-                diff = set([x / block_size for x in new]) - blocks
+                diff = set([x // block_size for x in new]) - blocks
                 clusters |= new
                 blocks |= diff
                 # Check if the refcount table needs one more cluster
                 if int(ceil((max(blocks) + 1) / float(size))) > table_size:
-                    new_block_id = (table_start + table_size) / block_size
+                    new_block_id = (table_start + table_size) // block_size
                     # Check if the additional table cluster needs
                     # one more refcount block
                     if new_block_id not in blocks:
@@ -XXX,XX +XXX,XX @@ class Image(object):
         def create_table_entry(table_offset, block_cluster, block_size,
                                cluster):
             """Generate a refcount table entry."""
-            offset = table_offset + UINT64_S * (cluster / block_size)
+            offset = table_offset + UINT64_S * (cluster // block_size)
             return ['>Q', offset, block_cluster * self.cluster_size,
                     'refcount_table_entry']
 
         def create_block_entry(block_cluster, block_size, cluster):
             """Generate a list of entries for the current block."""
-            entry_size = self.cluster_size / block_size
+            entry_size = self.cluster_size // block_size
             offset = block_cluster * self.cluster_size
             entry_offset = offset + entry_size * (cluster % block_size)
             # While snapshots are not supported all refcounts are set to 1
@@ -XXX,XX +XXX,XX @@ class Image(object):
         # Number of refcount entries per refcount block
         # Convert self.cluster_size from bytes to bits to have the same
         # base for the numerator and denominator
-        block_size = self.cluster_size * 8 / refcount_bits
+        block_size = self.cluster_size * 8 // refcount_bits
         meta_data = self._get_metadata()
         if len(self.data_clusters) == 0:
             # All metadata for an empty guest image needs 4 clusters:
@@ -XXX,XX +XXX,XX @@ class Image(object):
         rfc_blocks = []
 
         for cluster in sorted(self.data_clusters | meta_data):
-            if cluster / block_size != block_id:
-                block_id = cluster / block_size
+            if cluster // block_size != block_id:
+                block_id = cluster // block_size
                 block_cluster = block_clusters[block_ids.index(block_id)]
                 rfc_table.append(create_table_entry(table_offset,
                                                     block_cluster,
@@ -XXX,XX +XXX,XX @@ class Image(object):
     def _alloc_data(img_size, cluster_size):
         """Return a set of random indices of clusters allocated for guest data.
         """
-        num_of_cls = img_size/cluster_size
+        num_of_cls = img_size // cluster_size
         return set(random.sample(range(1, num_of_cls + 1),
                                  random.randint(0, num_of_cls)))
 
@@ -XXX,XX +XXX,XX @@ class Image(object):
         """Return indices of clusters allocated for image metadata."""
         ids = set()
         for x in self:
-            ids.add(x.offset/self.cluster_size)
+            ids.add(x.offset // self.cluster_size)
         return ids
 
 
-- 
2.23.0

From: Eduardo Habkost <ehabkost@redhat.com>

StringIO.StringIO is not available on Python 3, but io.StringIO
is available on both Python 2 and 3.  io.StringIO is slightly
different from the Python 2 StringIO module, though, so we need
bytes coming from subprocess.Popen() to be explicitly decoded.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-5-ehabkost@redhat.com
Message-Id: <20191016192430.25098-5-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/runner.py | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
index XXXXXXX..XXXXXXX 100755
--- a/tests/image-fuzzer/runner.py
+++ b/tests/image-fuzzer/runner.py
@@ -XXX,XX +XXX,XX @@ import shutil
 from itertools import count
 import time
 import getopt
-import StringIO
+import io
 import resource
 
 try:
@@ -XXX,XX +XXX,XX @@ def run_app(fd, q_args):
     try:
         out, err = process.communicate()
         signal.alarm(0)
-        fd.write(out)
-        fd.write(err)
+        # fd is a text file, so we need to decode the process output before
+        # writing to it.
+        # We could be simply using the `errors` parameter of subprocess.Popen(),
+        # but this will be possible only after migrating to Python 3
+        fd.write(out.decode(errors='replace'))
+        fd.write(err.decode(errors='replace'))
         fd.flush()
         return process.returncode
 
@@ -XXX,XX +XXX,XX @@ class TestEnv(object):
                                            MAX_BACKING_FILE_SIZE) * (1 << 20)
         cmd = self.qemu_img + ['create', '-f', backing_file_fmt,
                                backing_file_name, str(backing_file_size)]
-        temp_log = StringIO.StringIO()
+        temp_log = io.StringIO()
         retcode = run_app(temp_log, cmd)
         if retcode == 0:
             temp_log.close()
@@ -XXX,XX +XXX,XX @@ class TestEnv(object):
                            "Backing file: %s\n" \
                            % (self.seed, " ".join(current_cmd),
                               self.current_dir, backing_file_name)
-            temp_log = StringIO.StringIO()
+            temp_log = io.StringIO()
             try:
                 retcode = run_app(temp_log, current_cmd)
             except OSError as e:
-- 
2.23.0

From: Eduardo Habkost <ehabkost@redhat.com>

This makes the formatting code simpler, and safer if we change
the type of self.value from str to bytes.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-6-ehabkost@redhat.com
Message-Id: <20191016192430.25098-6-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/layout.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

From: Eduardo Habkost <ehabkost@redhat.com>

No caller of fuzzer functions is interested in unicode string values,
so replace them with bytes sequences.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-7-ehabkost@redhat.com
Message-Id: <20191016192430.25098-7-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/fuzz.py | 42 ++++++++++++++++----------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/fuzz.py b/tests/image-fuzzer/qcow2/fuzz.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/image-fuzzer/qcow2/fuzz.py
+++ b/tests/image-fuzzer/qcow2/fuzz.py
@@ -XXX,XX +XXX,XX @@ UINT32_V = [0, 0x100, 0x1000, 0x10000, 0x100000, UINT32//4, UINT32//2 - 1,
 UINT64_V = UINT32_V + [0x1000000, 0x10000000, 0x100000000, UINT64//4,
                        UINT64//2 - 1, UINT64//2, UINT64//2 + 1, UINT64 - 1,
                        UINT64]
-STRING_V = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
-            '%d%d%d%d', '%s%s%s%s', '%99999999999s', '%08x', '%%20d', '%%20n',
-            '%%20x', '%%20s', '%s%s%s%s%s%s%s%s%s%s', '%p%p%p%p%p%p%p%p%p%p',
-            '%#0123456x%08x%x%s%p%d%n%o%u%c%h%l%q%j%z%Z%t%i%e%g%f%a%C%S%08x%%',
-            '%s x 129', '%x x 257']
+BYTES_V = [b'%s%p%x%d', b'.1024d', b'%.2049d', b'%p%p%p%p', b'%x%x%x%x',
+           b'%d%d%d%d', b'%s%s%s%s', b'%99999999999s', b'%08x', b'%%20d', b'%%20n',
+           b'%%20x', b'%%20s', b'%s%s%s%s%s%s%s%s%s%s', b'%p%p%p%p%p%p%p%p%p%p',
+           b'%#0123456x%08x%x%s%p%d%n%o%u%c%h%l%q%j%z%Z%t%i%e%g%f%a%C%S%08x%%',
+           b'%s x 129', b'%x x 257']
 
 
 def random_from_intervals(intervals):
@@ -XXX,XX +XXX,XX @@ def random_bits(bit_ranges):
     return val
 
 
-def truncate_string(strings, length):
-    """Return strings truncated to specified length."""
-    if type(strings) == list:
-        return [s[:length] for s in strings]
+def truncate_bytes(sequences, length):
+    """Return sequences truncated to specified length."""
+    if type(sequences) == list:
+        return [s[:length] for s in sequences]
     else:
-        return strings[:length]
+        return sequences[:length]
 
 
 def validator(current, pick, choices):
@@ -XXX,XX +XXX,XX @@ def bit_validator(current, bit_ranges):
     return validator(current, random_bits, bit_ranges)
 
 
-def string_validator(current, strings):
-    """Return a random string value from the list not equal to the current.
+def bytes_validator(current, sequences):
+    """Return a random bytes value from the list not equal to the current.
 
     This function is useful for selection from valid values except current one.
     """
-    return validator(current, random.choice, strings)
+    return validator(current, random.choice, sequences)
 
 
 def selector(current, constraints, validate=int_validator):
@@ -XXX,XX +XXX,XX @@ def header_length(current):
 def bf_name(current):
     """Fuzz the backing file name."""
     constraints = [
-        truncate_string(STRING_V, len(current))
+        truncate_bytes(BYTES_V, len(current))
     ]
-    return selector(current, constraints, string_validator)
+    return selector(current, constraints, bytes_validator)
 
 
 def ext_magic(current):
@@ -XXX,XX +XXX,XX @@ def ext_length(current):
 def bf_format(current):
     """Fuzz backing file format in the corresponding header extension."""
     constraints = [
-        truncate_string(STRING_V, len(current)),
-        truncate_string(STRING_V, (len(current) + 7) & ~7)  # Fuzz padding
+        truncate_bytes(BYTES_V, len(current)),
+        truncate_bytes(BYTES_V, (len(current) + 7) & ~7)  # Fuzz padding
     ]
-    return selector(current, constraints, string_validator)
+    return selector(current, constraints, bytes_validator)
 
 
 def feature_type(current):
@@ -XXX,XX +XXX,XX @@ def feature_bit_number(current):
 def feature_name(current):
     """Fuzz feature name field of a feature name table header extension."""
     constraints = [
-        truncate_string(STRING_V, len(current)),
-        truncate_string(STRING_V, 46)  # Fuzz padding (field length = 46)
+        truncate_bytes(BYTES_V, len(current)),
+        truncate_bytes(BYTES_V, 46)  # Fuzz padding (field length = 46)
     ]
-    return selector(current, constraints, string_validator)
+    return selector(current, constraints, bytes_validator)
 
 
 def l1_entry(current):
-- 
2.23.0

From: Eduardo Habkost <ehabkost@redhat.com>

Field values are supposed to be bytes objects, not unicode
strings.  Change two constants that were declared as strings.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-8-ehabkost@redhat.com
Message-Id: <20191016192430.25098-8-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/layout.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/image-fuzzer/qcow2/layout.py
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -XXX,XX +XXX,XX @@ class Image(object):
     def create_header(self, cluster_bits, backing_file_name=None):
         """Generate a random valid header."""
         meta_header = [
-            ['>4s', 0, "QFI\xfb", 'magic'],
+            ['>4s', 0, b"QFI\xfb", 'magic'],
             ['>I', 4, random.randint(2, 3), 'version'],
             ['>Q', 8, 0, 'backing_file_offset'],
             ['>I', 16, 0, 'backing_file_size'],
@@ -XXX,XX +XXX,XX @@ class Image(object):
             feature_tables = []
             feature_ids = []
             inner_offset = self.ext_offset + ext_header_len
-            feat_name = 'some cool feature'
+            feat_name = b'some cool feature'
             while len(feature_tables) < num_fnt_entries * 3:
                 feat_type, feat_bit = gen_feat_ids()
                 # Remove duplicates
-- 
2.23.0

From: Eduardo Habkost <ehabkost@redhat.com>

Callers of create_image() will pass strings as arguments, but the
Image class will expect bytes objects to be provided.  Encode
them inside create_image().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-9-ehabkost@redhat.com
Message-Id: <20191016192430.25098-9-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/layout.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/image-fuzzer/qcow2/layout.py
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -XXX,XX +XXX,XX @@ class Image(object):
 def create_image(test_img_path, backing_file_name=None, backing_file_fmt=None,
                  fields_to_fuzz=None):
     """Create a fuzzed image and write it to the specified file."""
-    image = Image(backing_file_name)
-    image.set_backing_file_format(backing_file_fmt)
+    image = Image(backing_file_name.encode())
+    image.set_backing_file_format(backing_file_fmt.encode())
     image.create_feature_name_table()
     image.set_end_of_extension_area()
     image.create_l_structures()
-- 
2.23.0

From: Eduardo Habkost <ehabkost@redhat.com>

image-fuzzer is now supposed to be ready to run using Python 3.
Remove the __future__ imports and change the interpreter line to
"#!/usr/bin/env python3".

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-10-ehabkost@redhat.com
Message-Id: <20191016192430.25098-10-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/qcow2/__init__.py | 1 -
 tests/image-fuzzer/qcow2/layout.py   | 1 -
 tests/image-fuzzer/runner.py         | 3 +--
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/__init__.py b/tests/image-fuzzer/qcow2/__init__.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/image-fuzzer/qcow2/__init__.py
+++ b/tests/image-fuzzer/qcow2/__init__.py
@@ -1,2 +1 @@
-from __future__ import absolute_import
 from .layout import create_image
diff --git a/tests/image-fuzzer/qcow2/layout.py b/tests/image-fuzzer/qcow2/layout.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/image-fuzzer/qcow2/layout.py
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -XXX,XX +XXX,XX @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-from __future__ import absolute_import
 import random
 import struct
 from . import fuzz
diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
index XXXXXXX..XXXXXXX 100755
--- a/tests/image-fuzzer/runner.py
+++ b/tests/image-fuzzer/runner.py
@@ -XXX,XX +XXX,XX @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 # Tool for running fuzz tests
 #
@@ -XXX,XX +XXX,XX @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-from __future__ import print_function
 import sys
 import os
 import signal
-- 
2.23.0

From: Eduardo Habkost <ehabkost@redhat.com>

Instead of manually encoding stderr and stdout output, use
`errors` parameter of subprocess.Popen().  This will make
process.communicate() return unicode strings instead of bytes
objects.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191016192430.25098-11-ehabkost@redhat.com
Message-Id: <20191016192430.25098-11-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/runner.py | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
index XXXXXXX..XXXXXXX 100755
--- a/tests/image-fuzzer/runner.py
+++ b/tests/image-fuzzer/runner.py
@@ -XXX,XX +XXX,XX @@ def run_app(fd, q_args):
     devnull = open('/dev/null', 'r+')
     process = subprocess.Popen(q_args, stdin=devnull,
                                stdout=subprocess.PIPE,
-                               stderr=subprocess.PIPE)
+                               stderr=subprocess.PIPE,
+                               errors='replace')
     try:
         out, err = process.communicate()
         signal.alarm(0)
-        # fd is a text file, so we need to decode the process output before
-        # writing to it.
-        # We could be simply using the `errors` parameter of subprocess.Popen(),
-        # but this will be possible only after migrating to Python 3
-        fd.write(out.decode(errors='replace'))
-        fd.write(err.decode(errors='replace'))
+        fd.write(out)
+        fd.write(err)
         fd.flush()
         return process.returncode
 
-- 
2.23.0

From: Eduardo Habkost <ehabkost@redhat.com>

OSError can't be used like a tuple on Python 3, so change the
code to use `e.sterror` instead of `e[1]`.

Reported-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20191021214117.18091-1-ehabkost@redhat.com
Message-Id: <20191021214117.18091-1-ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/image-fuzzer/runner.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
index XXXXXXX..XXXXXXX 100755
--- a/tests/image-fuzzer/runner.py
+++ b/tests/image-fuzzer/runner.py
@@ -XXX,XX +XXX,XX @@ class TestEnv(object):
             os.makedirs(self.current_dir)
         except OSError as e:
             print("Error: The working directory '%s' cannot be used. Reason: %s"\
-                % (self.work_dir, e[1]), file=sys.stderr)
+                % (self.work_dir, e.strerror), file=sys.stderr)
             raise TestException
         self.log = open(os.path.join(self.current_dir, "test.log"), "w")
         self.parent_log = open(run_log, "a")
@@ -XXX,XX +XXX,XX @@ class TestEnv(object):
             except OSError as e:
                 multilog("%sError: Start of '%s' failed. Reason: %s\n\n"
                          % (test_summary, os.path.basename(current_cmd[0]),
-                            e[1]),
+                            e.strerror),
                          sys.stderr, self.log, self.parent_log)
                 raise TestException
 
-- 
2.23.0